diff --git a/src/tools/genpolicy/genpolicy-settings.json b/src/tools/genpolicy/genpolicy-settings.json index 56f8154fc4..8411f3ee39 100644 --- a/src/tools/genpolicy/genpolicy-settings.json +++ b/src/tools/genpolicy/genpolicy-settings.json @@ -34,6 +34,13 @@ "io.katacontainers.pkg.oci.bundle_path": "/run/containerd/io.containerd.runtime.v2.task/k8s.io/$(bundle-id)" }, "Process": { + "NoNewPrivileges": true, + "User": { + "UID": 65535, + "GID": 65535, + "AdditionalGids": [], + "Username": "" + }, "Args": [ "/pause" ] diff --git a/src/tools/genpolicy/src/cronjob.rs b/src/tools/genpolicy/src/cronjob.rs index 3719a89c2f..2f6d4e2ffa 100644 --- a/src/tools/genpolicy/src/cronjob.rs +++ b/src/tools/genpolicy/src/cronjob.rs @@ -148,10 +148,11 @@ impl yaml::K8sResource for CronJob { false } - fn get_process_fields(&self, process: &mut policy::KataProcess) { + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { yaml::get_process_fields( process, &self.spec.jobTemplate.spec.template.spec.securityContext, + must_check_passwd, ); } diff --git a/src/tools/genpolicy/src/daemon_set.rs b/src/tools/genpolicy/src/daemon_set.rs index 012e25adbd..159fd67c9c 100644 --- a/src/tools/genpolicy/src/daemon_set.rs +++ b/src/tools/genpolicy/src/daemon_set.rs @@ -148,8 +148,12 @@ impl yaml::K8sResource for DaemonSet { .or_else(|| Some(String::new())) } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/deployment.rs b/src/tools/genpolicy/src/deployment.rs index 3a59bccb76..5d0fdf22e0 100644 --- a/src/tools/genpolicy/src/deployment.rs +++ b/src/tools/genpolicy/src/deployment.rs @@ -146,8 +146,12 @@ impl yaml::K8sResource for Deployment { .or_else(|| Some(String::new())) } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/job.rs b/src/tools/genpolicy/src/job.rs index f5a723a1ac..0e4cbf82c0 100644 --- a/src/tools/genpolicy/src/job.rs +++ b/src/tools/genpolicy/src/job.rs @@ -111,8 +111,12 @@ impl yaml::K8sResource for Job { false } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/pod.rs b/src/tools/genpolicy/src/pod.rs index 4866a97d30..4604f49484 100644 --- a/src/tools/genpolicy/src/pod.rs +++ b/src/tools/genpolicy/src/pod.rs @@ -911,8 +911,8 @@ impl yaml::K8sResource for Pod { .or_else(|| Some(String::new())) } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields(process, &self.spec.securityContext, must_check_passwd); } fn get_sysctls(&self) -> Vec { @@ -970,6 +970,19 @@ impl Container { if let Some(context) = &self.securityContext { if let Some(uid) = context.runAsUser { process.User.UID = uid.try_into().unwrap(); + // Changing the UID can break the GID mapping + // if a /etc/passwd file is present. + // The proper GID is determined, in order of preference: + // 1. the securityContext runAsGroup field (applied last in code) + // 2. lacking an explicit runAsGroup, /etc/passwd (get_gid_from_passwd_uid) + // 3. fall back to pod-level GID if there is one (unwrap_or) + // + // This behavior comes from the containerd runtime implementation: + // WithUser https://github.com/containerd/containerd/blob/main/pkg/oci/spec_opts.go#L592 + process.User.GID = self + .registry + .get_gid_from_passwd_uid(process.User.UID) + .unwrap_or(process.User.GID); } if let Some(gid) = context.runAsGroup { diff --git a/src/tools/genpolicy/src/policy.rs b/src/tools/genpolicy/src/policy.rs index b9e7e187c9..aeb3ed44b9 100644 --- a/src/tools/genpolicy/src/policy.rs +++ b/src/tools/genpolicy/src/policy.rs @@ -713,7 +713,17 @@ impl AgentPolicy { substitute_args_env_variables(&mut process.Args, &process.Env); c_settings.get_process_fields(&mut process); - resource.get_process_fields(&mut process); + let mut must_check_passwd = false; + resource.get_process_fields(&mut process, &mut must_check_passwd); + + // The actual GID of the process run by the CRI + // Depends on the contents of /etc/passwd in the container + if must_check_passwd { + process.User.GID = yaml_container + .registry + .get_gid_from_passwd_uid(process.User.UID) + .unwrap_or(0); + } yaml_container.get_process_fields(&mut process); process diff --git a/src/tools/genpolicy/src/registry.rs b/src/tools/genpolicy/src/registry.rs index 7b0e3c7c2c..99e9931460 100644 --- a/src/tools/genpolicy/src/registry.rs +++ b/src/tools/genpolicy/src/registry.rs @@ -356,6 +356,12 @@ impl Container { debug!("Parsing gid from user[1] = {:?}", user[1]); process.User.GID = self.parse_group_string(user[1]); + + debug!( + "Overriding OCI container GID with UID:GID mapping from /etc/passwd" + ); + process.User.GID = + self.get_gid_from_passwd_uid(process.User.UID).unwrap_or(0); } } else { debug!("Parsing uid from image_user = {}", image_user); diff --git a/src/tools/genpolicy/src/replica_set.rs b/src/tools/genpolicy/src/replica_set.rs index 97d189f469..7d7c3781fc 100644 --- a/src/tools/genpolicy/src/replica_set.rs +++ b/src/tools/genpolicy/src/replica_set.rs @@ -109,8 +109,12 @@ impl yaml::K8sResource for ReplicaSet { false } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/replication_controller.rs b/src/tools/genpolicy/src/replication_controller.rs index f5920b8639..711971d7cb 100644 --- a/src/tools/genpolicy/src/replication_controller.rs +++ b/src/tools/genpolicy/src/replication_controller.rs @@ -111,8 +111,12 @@ impl yaml::K8sResource for ReplicationController { false } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/stateful_set.rs b/src/tools/genpolicy/src/stateful_set.rs index afa859ef04..a1613998ef 100644 --- a/src/tools/genpolicy/src/stateful_set.rs +++ b/src/tools/genpolicy/src/stateful_set.rs @@ -193,8 +193,12 @@ impl yaml::K8sResource for StatefulSet { .or_else(|| Some(String::new())) } - fn get_process_fields(&self, process: &mut policy::KataProcess) { - yaml::get_process_fields(process, &self.spec.template.spec.securityContext); + fn get_process_fields(&self, process: &mut policy::KataProcess, must_check_passwd: &mut bool) { + yaml::get_process_fields( + process, + &self.spec.template.spec.securityContext, + must_check_passwd, + ); } fn get_sysctls(&self) -> Vec { diff --git a/src/tools/genpolicy/src/yaml.rs b/src/tools/genpolicy/src/yaml.rs index 10506d9755..bd2ab91be2 100644 --- a/src/tools/genpolicy/src/yaml.rs +++ b/src/tools/genpolicy/src/yaml.rs @@ -96,7 +96,11 @@ pub trait K8sResource { None } - fn get_process_fields(&self, _process: &mut policy::KataProcess) { + fn get_process_fields( + &self, + _process: &mut policy::KataProcess, + _must_check_passwd: &mut bool, + ) { // No need to implement support for securityContext or similar fields // for some of the K8s resource types. } @@ -386,14 +390,32 @@ fn handle_unused_field(path: &str, silent_unsupported_fields: bool) { pub fn get_process_fields( process: &mut policy::KataProcess, security_context: &Option, + must_check_passwd: &mut bool, ) { if let Some(context) = security_context { if let Some(uid) = context.runAsUser { process.User.UID = uid.try_into().unwrap(); + // Changing the UID can break the GID mapping + // if a /etc/passwd file is present. + // The proper GID is determined, in order of preference: + // 1. the securityContext runAsGroup field (applied last in code) + // 2. lacking an explicit runAsGroup, /etc/passwd + // (parsed in policy::get_container_process()) + // 3. lacking an /etc/passwd, 0 (unwrap_or) + // + // This behavior comes from the containerd runtime implementation: + // WithUser https://github.com/containerd/containerd/blob/main/pkg/oci/spec_opts.go#L592 + // + // We can't parse the /etc/passwd file here because + // we are in the resource context. Defer execution to outside + // the resource context, in policy::get_container_process() + // IFF the UID is changed by the resource securityContext but not the GID. + *must_check_passwd = true; } if let Some(gid) = context.runAsGroup { process.User.GID = gid.try_into().unwrap(); + *must_check_passwd = false; } if let Some(allow) = context.allowPrivilegeEscalation {