From 13310587ed259cd6fe76d062289eaa955c56a64d Mon Sep 17 00:00:00 2001 From: Markus Rudy Date: Fri, 24 May 2024 12:29:27 +0200 Subject: [PATCH] genpolicy: check requested devices CreateContainerRequest objects can specify devices to be created inside the guest VM. This change ensures that requested devices have a corresponding entry in the PodSpec. Devices that are added to the pod dynamically, for example via the Device Plugin architecture, can be allowlisted globally by adding their definition to the settings file. Fixes: #9651 Signed-off-by: Markus Rudy --- src/tools/genpolicy/rules.rego | 25 +++++++ src/tools/genpolicy/src/containerd.rs | 2 + src/tools/genpolicy/src/policy.rs | 37 +++++++++++ .../kubernetes/k8s-policy-pvc.bats | 66 +++++++++++++++++++ .../kubernetes/run_kubernetes_tests.sh | 1 + .../k8s-policy-pod-pvc.yaml | 22 +++++++ .../k8s-policy-pvc.yaml | 16 +++++ 7 files changed, 169 insertions(+) create mode 100644 tests/integration/kubernetes/k8s-policy-pvc.bats create mode 100644 tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod-pvc.yaml create mode 100644 tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pvc.yaml diff --git a/src/tools/genpolicy/rules.rego b/src/tools/genpolicy/rules.rego index f123d661d8..1cddda0ea3 100644 --- a/src/tools/genpolicy/rules.rego +++ b/src/tools/genpolicy/rules.rego @@ -54,6 +54,7 @@ default AllowRequestsFailingPolicy := false CreateContainerRequest { i_oci := input.OCI i_storages := input.storages + i_devices := input.devices some p_container in policy_data.containers print("======== CreateContainerRequest: trying next policy container") @@ -77,6 +78,9 @@ CreateContainerRequest { p_storages := p_container.storages allow_by_anno(p_oci, i_oci, p_storages, i_storages) + p_devices := p_container.devices + allow_devices(p_devices, i_devices) + allow_linux(p_oci, i_oci) print("CreateContainerRequest: true") @@ -328,6 +332,16 @@ allow_log_directory(p_oci, i_oci) { print("allow_log_directory: true") } +allow_devices(p_devices, i_devices) { + print("allow_devices: start") + every i_device in i_devices { + print("allow_devices: i_device =", i_device) + some p_device in p_devices + p_device.container_path == i_device.container_path + } + print("allow_devices: true") +} + allow_linux(p_oci, i_oci) { p_namespaces := p_oci.Linux.Namespaces print("allow_linux: p namespaces =", p_namespaces) @@ -339,6 +353,7 @@ allow_linux(p_oci, i_oci) { allow_masked_paths(p_oci, i_oci) allow_readonly_paths(p_oci, i_oci) + allow_linux_devices(p_oci.Linux.Devices, i_oci.Linux.Devices) print("allow_linux: true") } @@ -427,6 +442,16 @@ allow_readonly_path(p_elem, i_array, masked_paths) { print("allow_readonly_path 2: true") } +allow_linux_devices(p_devices, i_devices) { + print("allow_linux_devices: start") + every i_device in i_devices { + print("allow_linux_devices: i_device =", i_device) + some p_device in p_devices + i_device.Path == p_device.Path + } + print("allow_linux_devices: true") +} + # Check the consistency of the input "io.katacontainers.pkg.oci.bundle_path" # and io.kubernetes.cri.sandbox-id" values with other fields. allow_by_bundle_or_sandbox_id(p_oci, i_oci, p_storages, i_storages) { diff --git a/src/tools/genpolicy/src/containerd.rs b/src/tools/genpolicy/src/containerd.rs index 19751bfa7d..22edb32f3d 100644 --- a/src/tools/genpolicy/src/containerd.rs +++ b/src/tools/genpolicy/src/containerd.rs @@ -152,12 +152,14 @@ pub fn get_linux(privileged_container: bool) -> policy::KataLinux { "/proc/sys".to_string(), "/proc/sysrq-trigger".to_string(), ], + Devices: vec![], } } else { policy::KataLinux { Namespaces: vec![], MaskedPaths: vec![], ReadonlyPaths: vec![], + Devices: vec![], } } } diff --git a/src/tools/genpolicy/src/policy.rs b/src/tools/genpolicy/src/policy.rs index e0bde0efb9..2d1eda2ee7 100644 --- a/src/tools/genpolicy/src/policy.rs +++ b/src/tools/genpolicy/src/policy.rs @@ -186,6 +186,10 @@ pub struct KataLinux { /// ReadonlyPaths sets the provided paths as RO inside the container. pub ReadonlyPaths: Vec, + + /// Devices contains devices to be created inside the container. + #[serde(default)] + pub Devices: Vec, } /// OCI container LinuxNamespace struct. This struct is similar to the LinuxNamespace @@ -201,6 +205,18 @@ pub struct KataLinuxNamespace { pub Path: String, } +/// OCI container LinuxDevice struct. This struct is similar to the LinuxDevice +/// struct generated from oci.proto, but includes just the fields that are currently +/// relevant for automatic generation of policy. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +pub struct KataLinuxDevice { + /// Type is the type of device. + pub Type: String, + + /// Path is the path where the device should be created. + pub Path: String, +} + /// OCI container LinuxCapabilities struct. This struct is very similar to the /// LinuxCapabilities struct generated from oci.proto. The main difference is /// that it preserves the upper case field names from oci.proto, for consistency @@ -252,6 +268,9 @@ pub struct ContainerPolicy { /// Data compared with req.storages for CreateContainerRequest calls. storages: Vec, + /// Data compared with req.devices for CreateContainerRequest calls. + devices: Vec, + /// Data compared with req.sandbox_pidns for CreateContainerRequest calls. sandbox_pidns: bool, @@ -546,6 +565,23 @@ impl AgentPolicy { }; let exec_commands = yaml_container.get_exec_commands(); + let mut devices: Vec = vec![]; + if let Some(volumeDevices) = &yaml_container.volumeDevices { + for volumeDevice in volumeDevices { + let mut device = agent::Device::new(); + device.set_container_path(volumeDevice.devicePath.clone()); + devices.push(device); + + linux.Devices.push(KataLinuxDevice { + Type: "".to_string(), + Path: volumeDevice.devicePath.clone(), + }) + } + } + for default_device in &c_settings.Linux.Devices { + linux.Devices.push(default_device.clone()) + } + ContainerPolicy { OCI: KataSpec { Version: version_default(), @@ -557,6 +593,7 @@ impl AgentPolicy { Linux: linux, }, storages, + devices, sandbox_pidns, exec_commands, } diff --git a/tests/integration/kubernetes/k8s-policy-pvc.bats b/tests/integration/kubernetes/k8s-policy-pvc.bats new file mode 100644 index 0000000000..7495e6efbf --- /dev/null +++ b/tests/integration/kubernetes/k8s-policy-pvc.bats @@ -0,0 +1,66 @@ +#!/usr/bin/env bats +# +# Copyright (c) 2024 Edgeless Systems GmbH +# +# SPDX-License-Identifier: Apache-2.0 +# + +load "${BATS_TEST_DIRNAME}/../../common.bash" +load "${BATS_TEST_DIRNAME}/tests_common.sh" + +setup() { + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." + + pod_name="policy-pod-pvc" + pvc_name="policy-dev" + + get_pod_config_dir + + correct_pod_yaml="${pod_config_dir}/k8s-policy-pod-pvc.yaml" + incorrect_pod_yaml="${pod_config_dir}/k8s-policy-pod-pvc-incorrect.yaml" + pvc_yaml="${pod_config_dir}/k8s-policy-pvc.yaml" + + # Save some time by executing genpolicy a single time. + if [ "${BATS_TEST_NUMBER}" == "1" ]; then + # Add policy to the correct pod yaml file + auto_generate_policy "${pod_config_dir}" "${correct_pod_yaml}" + fi + + # Start each test case with a copy of the correct yaml files. + cp "${correct_pod_yaml}" "${incorrect_pod_yaml}" +} + +@test "Successful pod with auto-generated policy" { + kubectl create -f "${correct_pod_yaml}" + kubectl create -f "${pvc_yaml}" + kubectl wait --for=condition=Ready "--timeout=${timeout}" pod "${pod_name}" +} + +# Common function for several test cases from this bats script. +test_pod_policy_error() { + kubectl create -f "${incorrect_pod_yaml}" + kubectl create -f "${pvc_yaml}" + wait_for_blocked_request "CreateContainerRequest" "${pod_name}" +} + +@test "Policy failure: unexpected device mount" { + # Changing the location of a mounted device after policy generation should fail the policy check. + yq write -i \ + "${incorrect_pod_yaml}" \ + "spec.containers[0].volumeDevices.[0].devicePath" \ + "/dev/unexpected" + + test_pod_policy_error +} + +teardown() { + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." + + # Debugging information. Don't print the "Message:" line because it contains a truncated policy log. + kubectl describe pod "${pod_name}" | grep -v "Message:" + + # Clean-up + kubectl delete -f "${correct_pod_yaml}" + kubectl delete -f "${pvc_yaml}" + rm -f "${incorrect_pod_yaml}" +} diff --git a/tests/integration/kubernetes/run_kubernetes_tests.sh b/tests/integration/kubernetes/run_kubernetes_tests.sh index 81c572773f..9596098b41 100755 --- a/tests/integration/kubernetes/run_kubernetes_tests.sh +++ b/tests/integration/kubernetes/run_kubernetes_tests.sh @@ -55,6 +55,7 @@ else "k8s-pod-quota.bats" \ "k8s-policy-job.bats" \ "k8s-policy-pod.bats" \ + "k8s-policy-pvc.bats" \ "k8s-policy-rc.bats" \ "k8s-port-forward.bats" \ "k8s-projected-volume.bats" \ diff --git a/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod-pvc.yaml b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod-pvc.yaml new file mode 100644 index 0000000000..2f4e847a93 --- /dev/null +++ b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pod-pvc.yaml @@ -0,0 +1,22 @@ +# +# Copyright (c) 2024 Edgeless Systems GmbH +# +# SPDX-License-Identifier: Apache-2.0 +# +apiVersion: v1 +kind: Pod +metadata: + name: policy-pod-pvc +spec: + terminationGracePeriodSeconds: 0 + runtimeClassName: kata + containers: + - name: busybox + image: "quay.io/prometheus/busybox:latest" + volumeDevices: + - name: dev + devicePath: /dev/csi0 + volumes: + - name: dev + persistentVolumeClaim: + claimName: policy-dev diff --git a/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pvc.yaml b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pvc.yaml new file mode 100644 index 0000000000..5284667998 --- /dev/null +++ b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-pvc.yaml @@ -0,0 +1,16 @@ +# +# Copyright (c) 2024 Edgeless Systems GmbH +# +# SPDX-License-Identifier: Apache-2.0 +# +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: policy-dev +spec: + accessModes: + - ReadWriteOnce + volumeMode: Block + resources: + requests: + storage: 1Mi