From c7e25b798dd2e088618c56081626907508b54cfc Mon Sep 17 00:00:00 2001 From: utam0k Date: Wed, 29 Nov 2023 21:33:53 +0900 Subject: [PATCH 1/5] Set up userns in a straightforward way Signed-off-by: utam0k --- .../process/container_intermediate_process.rs | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index 1d02637a5..d2403ab6b 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -3,10 +3,11 @@ use crate::{namespaces::Namespaces, process::channel, process::fork}; use libcgroups::common::CgroupManager; use nix::unistd::{close, write}; use nix::unistd::{Gid, Pid, Uid}; -use oci_spec::runtime::{LinuxNamespaceType, LinuxResources}; +use oci_spec::runtime::{LinuxNamespace, LinuxNamespaceType, LinuxResources}; use procfs::process::Process; use super::args::{ContainerArgs, ContainerType}; +use super::channel::{IntermediateReceiver, MainSender}; use super::container_init_process::container_init_process; use super::fork::CloneCb; @@ -68,22 +69,7 @@ pub fn container_intermediate_process( // https://man7.org/linux/man-pages/man7/user_namespaces.7.html for more // information if let Some(user_namespace) = namespaces.get(LinuxNamespaceType::User)? { - namespaces.unshare_or_setns(user_namespace)?; - if user_namespace.path().is_none() { - tracing::debug!("creating new user namespace"); - // child needs to be dumpable, otherwise the non root parent is not - // allowed to write the uid/gid maps - prctl::set_dumpable(true).unwrap(); - main_sender.identifier_mapping_request().map_err(|err| { - tracing::error!("failed to send id mapping request: {}", err); - err - })?; - inter_receiver.wait_for_mapping_ack().map_err(|err| { - tracing::error!("failed to receive id mapping ack: {}", err); - err - })?; - prctl::set_dumpable(false).unwrap(); - } + setup_userns(&namespaces, user_namespace, main_sender, inter_receiver)?; // After UID and GID mapping is configured correctly in the Youki main // process, We want to make sure continue as the root user inside the @@ -201,6 +187,33 @@ pub fn container_intermediate_process( Ok(()) } +fn setup_userns( + namespaces: &Namespaces, + user_namespace: &LinuxNamespace, + sender: &mut MainSender, + receiver: &mut IntermediateReceiver, +) -> Result<()> { + namespaces.unshare_or_setns(user_namespace)?; + if user_namespace.path().is_some() { + return Ok(()); + } + + tracing::debug!("creating new user namespace"); + // child needs to be dumpable, otherwise the non root parent is not + // allowed to write the uid/gid maps + prctl::set_dumpable(true).unwrap(); + sender.identifier_mapping_request().map_err(|err| { + tracing::error!("failed to send id mapping request: {}", err); + err + })?; + receiver.wait_for_mapping_ack().map_err(|err| { + tracing::error!("failed to receive id mapping ack: {}", err); + err + })?; + prctl::set_dumpable(false).unwrap(); + return Ok(()); +} + fn apply_cgroups< C: CgroupManager + ?Sized, E: std::error::Error + Send + Sync + 'static, From d137a429c547909f687605b7b75a77b29dbd8bd9 Mon Sep 17 00:00:00 2001 From: utam0k Date: Thu, 30 Nov 2023 20:19:18 +0900 Subject: [PATCH 2/5] Fix lint Signed-off-by: utam0k --- .../process/container_intermediate_process.rs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index d2403ab6b..ad1401b2e 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -211,7 +211,7 @@ fn setup_userns( err })?; prctl::set_dumpable(false).unwrap(); - return Ok(()); + Ok(()) } fn apply_cgroups< @@ -249,11 +249,13 @@ fn apply_cgroups< #[cfg(test)] mod tests { - use super::apply_cgroups; + use super::*; + use crate::process::channel; use anyhow::Result; use libcgroups::test_manager::TestManager; use nix::unistd::Pid; use oci_spec::runtime::LinuxResources; + use oci_spec::runtime::{LinuxNamespaceBuilder, LinuxNamespaceType}; use procfs::process::Process; #[test] @@ -308,4 +310,20 @@ mod tests { assert!(!cmanager.apply_called()); Ok(()) } + + #[test] + fn test_setup_userns() -> Result<()> { + let (mut sender, _) = channel::main_channel()?; + let (_, mut receiver) = channel::intermediate_channel()?; + let user_namespace = LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::User) + .build() + .unwrap(); + let namespaces = + crate::namespaces::Namespaces::try_from(Some(&vec![user_namespace.clone()]))?; + + setup_userns(&namespaces, &user_namespace, &mut sender, &mut receiver)?; + + Ok(()) + } } From c41e4b6d36e612c940cdc7d3adcceaad6ddb2a6a Mon Sep 17 00:00:00 2001 From: utam0k Date: Thu, 30 Nov 2023 20:41:24 +0900 Subject: [PATCH 3/5] Extend timeouts in github actions Signed-off-by: utam0k --- .github/workflows/e2e.yaml | 4 ++-- .github/workflows/main.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 687155738..8a3e1385e 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -116,7 +116,7 @@ jobs: oci-validation-rust: runs-on: ubuntu-22.04 needs: [youki-build] - timeout-minutes: 15 + timeout-minutes: 20 steps: - uses: actions/checkout@v3 with: @@ -141,7 +141,7 @@ jobs: rootless-podman-test: runs-on: ubuntu-22.04 needs: [youki-build] - timeout-minutes: 15 + timeout-minutes: 20 steps: - uses: actions/checkout@v3 with: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 54733051a..9c786784e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -57,7 +57,7 @@ jobs: needs: [changes] if: needs.changes.outputs.any_modified == 'true' runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 20 steps: - uses: actions/checkout@v3 - name: Setup Rust toolchain and cache @@ -102,7 +102,7 @@ jobs: needs: [changes] if: needs.changes.outputs.any_modified == 'true' runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 20 name: Run test coverage steps: - uses: actions/checkout@v3 From 465d0bb9e05f3060749b1ab8266ec2ea1f0f80da Mon Sep 17 00:00:00 2001 From: utam0k Date: Sun, 3 Dec 2023 21:22:39 +0900 Subject: [PATCH 4/5] Incorrect conditional branch Signed-off-by: utam0k --- .../libcontainer/src/process/container_intermediate_process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index ad1401b2e..a155491d9 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -194,7 +194,7 @@ fn setup_userns( receiver: &mut IntermediateReceiver, ) -> Result<()> { namespaces.unshare_or_setns(user_namespace)?; - if user_namespace.path().is_some() { + if !user_namespace.path().is_some() { return Ok(()); } From 67685bbd1aff18764be67f047b1827fcbd4c1b81 Mon Sep 17 00:00:00 2001 From: utam0k Date: Mon, 4 Dec 2023 20:40:29 +0900 Subject: [PATCH 5/5] Fix CI Signed-off-by: utam0k --- .../process/container_intermediate_process.rs | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index a155491d9..8feb71ee7 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -194,7 +194,7 @@ fn setup_userns( receiver: &mut IntermediateReceiver, ) -> Result<()> { namespaces.unshare_or_setns(user_namespace)?; - if !user_namespace.path().is_some() { + if user_namespace.path().is_some() { return Ok(()); } @@ -250,12 +250,10 @@ fn apply_cgroups< #[cfg(test)] mod tests { use super::*; - use crate::process::channel; use anyhow::Result; use libcgroups::test_manager::TestManager; use nix::unistd::Pid; use oci_spec::runtime::LinuxResources; - use oci_spec::runtime::{LinuxNamespaceBuilder, LinuxNamespaceType}; use procfs::process::Process; #[test] @@ -310,20 +308,4 @@ mod tests { assert!(!cmanager.apply_called()); Ok(()) } - - #[test] - fn test_setup_userns() -> Result<()> { - let (mut sender, _) = channel::main_channel()?; - let (_, mut receiver) = channel::intermediate_channel()?; - let user_namespace = LinuxNamespaceBuilder::default() - .typ(LinuxNamespaceType::User) - .build() - .unwrap(); - let namespaces = - crate::namespaces::Namespaces::try_from(Some(&vec![user_namespace.clone()]))?; - - setup_userns(&namespaces, &user_namespace, &mut sender, &mut receiver)?; - - Ok(()) - } }