From b23ea508d5f6592d412f074564f52b17e09ef99c Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Tue, 16 Jul 2024 23:03:18 +0000 Subject: [PATCH 1/2] tests: k8s: container.exec_commands policy tests Add tests for genpolicy's handling of container.exec_commands. These are commands allowed by the policy and originating from these input K8s YAML fields: - livenessProbe - readinessProbe - startupProbe - lifecycle.postStart - lifecycle.preStop Signed-off-by: Dan Mihai --- .../kubernetes/k8s-policy-pod.bats | 27 +++++++++++++- .../k8s-policy-pod.yaml | 21 +++++++++++ tests/integration/kubernetes/tests_common.sh | 35 +++++++++++++++++-- 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/tests/integration/kubernetes/k8s-policy-pod.bats b/tests/integration/kubernetes/k8s-policy-pod.bats index 920b951d9f..a76c3cf622 100644 --- a/tests/integration/kubernetes/k8s-policy-pod.bats +++ b/tests/integration/kubernetes/k8s-policy-pod.bats @@ -46,12 +46,17 @@ setup() { cp "${pre_generate_pod_yaml}" "${testcase_pre_generate_pod_yaml}" } -@test "Successful pod with auto-generated policy" { +# Common function for several test cases from this bats script. +wait_for_pod_ready() { kubectl create -f "${correct_configmap_yaml}" kubectl create -f "${correct_pod_yaml}" kubectl wait --for=condition=Ready "--timeout=${timeout}" pod "${pod_name}" } +@test "Successful pod with auto-generated policy" { + wait_for_pod_ready +} + @test "Successful pod with auto-generated policy and runtimeClassName filter" { runtime_class_name=$(yq ".spec.runtimeClassName" < "${testcase_pre_generate_pod_yaml}") @@ -192,6 +197,26 @@ test_pod_policy_error() { kubectl wait --for=condition=Ready "--timeout=${timeout}" pod "${pod_name}" } +@test "ExecProcessRequest tests" { + wait_for_pod_ready + + # Execute commands allowed by the policy. + pod_exec_allowed_command "${pod_name}" "echo" "livenessProbe" "test" + pod_exec_allowed_command "${pod_name}" "sh" "-c" "ls -l /" + pod_exec_allowed_command "${pod_name}" "echo" "startupProbe" "test" + + # This test should fail but it passes because genpolicy joins the exec args from its + # input K8s YAML file and from the command being executed, and compares the joined + # command lines instead of comparing each argument. + pod_exec_allowed_command "${pod_name}" "echo" "livenessProbe test" + + # Try to execute commands disallowed by the policy. + pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test2" + pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test" "yes" + pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test foo" + pod_exec_blocked_command "${pod_name}" "echo" "hello" +} + teardown() { auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." diff --git a/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod.yaml b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod.yaml index 0bf903691c..c220731b82 100644 --- a/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod.yaml +++ b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod.yaml @@ -28,6 +28,27 @@ spec: runAsUser: 1000 seccompProfile: type: RuntimeDefault + livenessProbe: + exec: + command: + - echo + - livenessProbe + - test + failureThreshold: 1 + periodSeconds: 5 + timeoutSeconds: 10 + readinessProbe: + exec: + command: + - "sh" + - "-c" + - "ls -l /" + startupProbe: + exec: + command: + - echo + - startupProbe + - test topologySpreadConstraints: - maxSkew: 2 topologyKey: kubernetes.io/hostname diff --git a/tests/integration/kubernetes/tests_common.sh b/tests/integration/kubernetes/tests_common.sh index 7a46bedab0..afdc05ee59 100644 --- a/tests/integration/kubernetes/tests_common.sh +++ b/tests/integration/kubernetes/tests_common.sh @@ -321,10 +321,39 @@ add_allow_all_policy_to_yaml() { # Execute "kubectl describe ${pod}" in a loop, until its output contains "${endpoint} is blocked by policy" wait_for_blocked_request() { - endpoint="$1" - pod="$2" + local -r endpoint="$1" + local -r pod="$2" - command="kubectl describe pod ${pod} | grep \"${endpoint} is blocked by policy\"" + local -r command="kubectl describe pod ${pod} | grep \"${endpoint} is blocked by policy\"" info "Waiting ${wait_time} seconds for: ${command}" waitForProcess "${wait_time}" "$sleep_time" "${command}" >/dev/null 2>/dev/null } + +# Execute in a pod a command that is allowed by policy. +pod_exec_allowed_command() { + local -r pod_name="$1" + shift + + local -r exec_output=$(kubectl exec "${pod_name}" -- "${@}" 2>&1) + + local -r exec_args=$(printf '"%s",' "${@}") + info "Pod ${pod_name}: <${exec_args::-1}>:" + info "${exec_output}" + + (echo "${exec_output}" | grep "policy") && die "exec was blocked by policy!" + return 0 +} + +# Execute in a pod a command that is blocked by policy. +pod_exec_blocked_command() { + local -r pod_name="$1" + shift + + local -r exec_output=$(kubectl exec "${pod_name}" -- "${@}" 2>&1) + + local -r exec_args=$(printf '"%s",' "${@}") + info "Pod ${pod_name}: <${exec_args::-1}>:" + info "${exec_output}" + + (echo "${exec_output}" | grep "ExecProcessRequest is blocked by policy" > /dev/null) || die "exec was not blocked by policy!" +} From 9f4d1ffd43792c36d810165973a1397fdc640096 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Mon, 15 Jul 2024 22:59:38 +0000 Subject: [PATCH 2/2] genpolicy: container.exec_commands args validation Keep track of individual exec args instead of joining them in the policy text. Verifying each arg results in a more precise policy, because some of the args might include space characters. This improved validation applies to commands specified in K8s YAML files using: - livenessProbe - readinessProbe - startupProbe - lifecycle.postStart - lifecycle.preStop Signed-off-by: Dan Mihai --- src/tools/genpolicy/rules.rego | 5 +---- src/tools/genpolicy/src/pod.rs | 12 ++++++------ src/tools/genpolicy/src/policy.rs | 2 +- tests/integration/kubernetes/k8s-policy-pod.bats | 6 +----- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/tools/genpolicy/rules.rego b/src/tools/genpolicy/rules.rego index 4bf7be6ad6..ec2e8ac6d0 100644 --- a/src/tools/genpolicy/rules.rego +++ b/src/tools/genpolicy/rules.rego @@ -1124,15 +1124,12 @@ ExecProcessRequest { print("ExecProcessRequest 2: input =", input) # TODO: match input container ID with its corresponding container.exec_commands. - i_command = concat(" ", input.process.Args) - print("ExecProcessRequest 3: i_command =", i_command) - some container in policy_data.containers some p_command in container.exec_commands print("ExecProcessRequest 2: p_command =", p_command) # TODO: should other input data fields be validated as well? - p_command == i_command + p_command == input.process.Args print("ExecProcessRequest 2: true") } diff --git a/src/tools/genpolicy/src/pod.rs b/src/tools/genpolicy/src/pod.rs index e18c30ad4f..16cd9c517a 100644 --- a/src/tools/genpolicy/src/pod.rs +++ b/src/tools/genpolicy/src/pod.rs @@ -614,36 +614,36 @@ impl Container { (yaml_has_command, yaml_has_args) } - pub fn get_exec_commands(&self) -> Vec { + pub fn get_exec_commands(&self) -> Vec> { let mut commands = Vec::new(); if let Some(probe) = &self.livenessProbe { if let Some(exec) = &probe.exec { - commands.push(exec.command.join(" ")); + commands.push(exec.command.clone()); } } if let Some(probe) = &self.readinessProbe { if let Some(exec) = &probe.exec { - commands.push(exec.command.join(" ")); + commands.push(exec.command.clone()); } } if let Some(probe) = &self.startupProbe { if let Some(exec) = &probe.exec { - commands.push(exec.command.join(" ")); + commands.push(exec.command.clone()); } } if let Some(lifecycle) = &self.lifecycle { if let Some(postStart) = &lifecycle.postStart { if let Some(exec) = &postStart.exec { - commands.push(exec.command.join(" ")); + commands.push(exec.command.clone()); } } if let Some(preStop) = &lifecycle.preStop { if let Some(exec) = &preStop.exec { - commands.push(exec.command.join(" ")); + commands.push(exec.command.clone()); } } } diff --git a/src/tools/genpolicy/src/policy.rs b/src/tools/genpolicy/src/policy.rs index 50f80310ed..d0ce1b2d10 100644 --- a/src/tools/genpolicy/src/policy.rs +++ b/src/tools/genpolicy/src/policy.rs @@ -271,7 +271,7 @@ pub struct ContainerPolicy { /// Allow list of ommand lines that are allowed to be executed using /// ExecProcessRequest. By default, all ExecProcessRequest calls are blocked /// by the policy. - exec_commands: Vec, + exec_commands: Vec>, } /// See Reference / Kubernetes API / Config and Storage Resources / Volume. diff --git a/tests/integration/kubernetes/k8s-policy-pod.bats b/tests/integration/kubernetes/k8s-policy-pod.bats index a76c3cf622..0e78a23d22 100644 --- a/tests/integration/kubernetes/k8s-policy-pod.bats +++ b/tests/integration/kubernetes/k8s-policy-pod.bats @@ -205,12 +205,8 @@ test_pod_policy_error() { pod_exec_allowed_command "${pod_name}" "sh" "-c" "ls -l /" pod_exec_allowed_command "${pod_name}" "echo" "startupProbe" "test" - # This test should fail but it passes because genpolicy joins the exec args from its - # input K8s YAML file and from the command being executed, and compares the joined - # command lines instead of comparing each argument. - pod_exec_allowed_command "${pod_name}" "echo" "livenessProbe test" - # Try to execute commands disallowed by the policy. + pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe test" pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test2" pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test" "yes" pod_exec_blocked_command "${pod_name}" "echo" "livenessProbe" "test foo"