From 107dd9188108e8b32d76c03b0af1c7e1395cf180 Mon Sep 17 00:00:00 2001 From: utam0k Date: Mon, 20 Sep 2021 16:33:20 +0900 Subject: [PATCH 1/2] refactor around uid and gid mapping. --- src/container/builder_impl.rs | 27 ++++++++------ src/rootless.rs | 67 ++++++++++++++++------------------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs index a1ea85553..eb226ac09 100644 --- a/src/container/builder_impl.rs +++ b/src/container/builder_impl.rs @@ -2,12 +2,13 @@ use crate::{ hooks, notify_socket::NotifyListener, process::{args::ContainerArgs, channel, fork, intermediate}, - rootless::{self, Rootless}, + rootless::Rootless, syscall::Syscall, utils, }; use anyhow::{Context, Result}; use cgroups; +use nix::unistd::Pid; use oci_spec::runtime::Spec; use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf}; @@ -139,17 +140,9 @@ impl<'a> ContainerBuilderImpl<'a> { // If creating a rootless container, the intermediate process will ask // the main process to set up uid and gid mapping, once the intermediate // process enters into a new user namespace. - if self.rootless.is_some() { + if let Some(rootless) = self.rootless.as_ref() { receiver_from_intermediate.wait_for_mapping_request()?; - log::debug!("write mapping for pid {:?}", intermediate_pid); - let rootless = self.rootless.as_ref().unwrap(); - if !rootless.privileged { - // The main process is running as an unprivileged user and cannot write the mapping - // until "deny" has been written to setgroups. See CVE-2014-8989. - utils::write_file(format!("/proc/{}/setgroups", intermediate_pid), "deny")?; - } - rootless::write_uid_mapping(intermediate_pid, self.rootless.as_ref())?; - rootless::write_gid_mapping(intermediate_pid, self.rootless.as_ref())?; + setup_mapping(rootless, intermediate_pid)?; sender_to_intermediate.mapping_written()?; } @@ -188,3 +181,15 @@ impl<'a> ContainerBuilderImpl<'a> { Ok(()) } } + +fn setup_mapping(rootless: &Rootless, pid: Pid) -> Result<()> { + log::debug!("write mapping for pid {:?}", pid); + if !rootless.privileged { + // The main process is running as an unprivileged user and cannot write the mapping + // until "deny" has been written to setgroups. See CVE-2014-8989. + utils::write_file(format!("/proc/{}/setgroups", pid), "deny")?; + } + rootless.write_uid_mapping(pid)?; + rootless.write_gid_mapping(pid)?; + Ok(()) +} diff --git a/src/rootless.rs b/src/rootless.rs index a7db9b0ba..52b449b4a 100644 --- a/src/rootless.rs +++ b/src/rootless.rs @@ -6,20 +6,19 @@ use std::path::Path; use std::process::Command; use std::{env, path::PathBuf}; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct Rootless<'a> { /// Location of the newuidmap binary pub newuidmap: Option, /// Location of the newgidmap binary pub newgidmap: Option, /// Mappings for user ids - pub uid_mappings: Option<&'a Vec>, + pub(crate) uid_mappings: Option<&'a Vec>, /// Mappings for group ids - pub gid_mappings: Option<&'a Vec>, + pub(crate) gid_mappings: Option<&'a Vec>, /// Info on the user namespaces - user_namespace: Option, - /// Is rootless container requested by a privileged - /// user + pub(crate) user_namespace: Option, + /// Is rootless container requested by a privileged user pub privileged: bool, } @@ -54,6 +53,32 @@ impl<'a> Rootless<'a> { Ok(None) } } + + pub fn write_uid_mapping(&self, target_pid: Pid) -> Result<()> { + log::debug!("Write UID mapping for {:?}", target_pid); + if let Some(uid_mappings) = self.uid_mappings { + write_id_mapping( + &format!("/proc/{}/uid_map", target_pid), + uid_mappings, + self.newuidmap.as_deref(), + ) + } else { + Ok(()) + } + } + + pub fn write_gid_mapping(&self, target_pid: Pid) -> Result<()> { + log::debug!("Write GID mapping for {:?}", target_pid); + if let Some(gid_mappings) = self.gid_mappings { + return write_id_mapping( + &format!("/proc/{}/gid_map", target_pid), + gid_mappings, + self.newgidmap.as_deref(), + ); + } else { + Ok(()) + } + } } impl<'a> From<&'a Linux> for Rootless<'a> { @@ -199,36 +224,6 @@ fn lookup_map_binary(binary: &str) -> Result> { .map(PathBuf::from)) } -pub fn write_uid_mapping(target_pid: Pid, rootless: Option<&Rootless>) -> Result<()> { - log::debug!("Write UID mapping for {:?}", target_pid); - if let Some(rootless) = rootless { - if let Some(uid_mappings) = rootless.uid_mappings { - return write_id_mapping( - &format!("/proc/{}/uid_map", target_pid), - uid_mappings, - rootless.newuidmap.as_deref(), - ); - } - } - - Ok(()) -} - -pub fn write_gid_mapping(target_pid: Pid, rootless: Option<&Rootless>) -> Result<()> { - log::debug!("Write GID mapping for {:?}", target_pid); - if let Some(rootless) = rootless { - if let Some(gid_mappings) = rootless.gid_mappings { - return write_id_mapping( - &format!("/proc/{}/gid_map", target_pid), - gid_mappings, - rootless.newgidmap.as_deref(), - ); - } - } - - Ok(()) -} - fn write_id_mapping( map_file: &str, mappings: &[LinuxIdMapping], From b3d0aaf3b665821277cf577e91a31a5e67bc5cf7 Mon Sep 17 00:00:00 2001 From: utam0k Date: Mon, 20 Sep 2021 16:33:49 +0900 Subject: [PATCH 2/2] add unit tests for gid and uid mapping in builder_impl(). --- src/container/builder_impl.rs | 95 +++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs index eb226ac09..eaa270192 100644 --- a/src/container/builder_impl.rs +++ b/src/container/builder_impl.rs @@ -193,3 +193,98 @@ fn setup_mapping(rootless: &Rootless, pid: Pid) -> Result<()> { rootless.write_gid_mapping(pid)?; Ok(()) } + +#[cfg(test)] +mod tests { + use nix::{ + sched::{unshare, CloneFlags}, + unistd::{self, getgid, getuid}, + }; + use oci_spec::runtime::LinuxIdMapping; + use serial_test::serial; + + use crate::process::channel::intermediate_to_main; + + use super::*; + + #[test] + #[serial] + fn setup_uid_mapping_should_succeed() -> Result<()> { + let uid_mapping = LinuxIdMapping { + host_id: u32::from(getuid()), + container_id: 0, + size: 1, + }; + let uid_mappings = vec![uid_mapping]; + let rootless = Rootless { + uid_mappings: Some(&uid_mappings), + privileged: true, + ..Default::default() + }; + let (mut sender, mut receiver) = intermediate_to_main()?; + match unsafe { unistd::fork()? } { + unistd::ForkResult::Parent { child } => { + receiver.wait_for_mapping_request()?; + setup_mapping(&rootless, child)?; + let line = fs::read_to_string(format!("/proc/{}/uid_map", child.as_raw()))?; + let line_splited = line.split_whitespace(); + for (act, expect) in line_splited.zip([ + uid_mapping.container_id.to_string().as_str(), + uid_mapping.host_id.to_string().as_str(), + uid_mapping.size.to_string().as_str(), + ]) { + assert_eq!(act, expect); + } + Ok(()) + } + unistd::ForkResult::Child => { + unshare(CloneFlags::CLONE_NEWUSER)?; + prctl::set_dumpable(true).unwrap(); + sender.identifier_mapping_request()?; + Ok(()) + } + } + } + + #[test] + #[serial] + fn setup_gid_mapping_should_successed() -> Result<()> { + let gid_mapping = LinuxIdMapping { + host_id: u32::from(getgid()), + container_id: 0, + size: 1, + }; + let gid_mappings = vec![gid_mapping]; + let rootless = Rootless { + gid_mappings: Some(&gid_mappings), + ..Default::default() + }; + let (mut sender, mut receiver) = intermediate_to_main()?; + match unsafe { unistd::fork()? } { + unistd::ForkResult::Parent { child } => { + receiver.wait_for_mapping_request()?; + setup_mapping(&rootless, child)?; + let line = fs::read_to_string(format!("/proc/{}/gid_map", child.as_raw()))?; + let line_splited = line.split_whitespace(); + for (act, expect) in line_splited.zip([ + gid_mapping.container_id.to_string().as_str(), + gid_mapping.host_id.to_string().as_str(), + gid_mapping.size.to_string().as_str(), + ]) { + assert_eq!(act, expect); + } + assert_eq!( + fs::read_to_string(format!("/proc/{}/setgroups", child.as_raw()))?, + "deny\n", + ); + Ok(()) + } + unistd::ForkResult::Child => { + unshare(CloneFlags::CLONE_NEWUSER)?; + prctl::set_dumpable(true).unwrap(); + sender.identifier_mapping_request()?; + Ok(()) + } + } + } +}