Move userns spec validation into utils, add tests for it

crates/libcontainer/src/container/init_builder.rs

@@ -88,10 +88,6 @@ impl InitContainerBuilder {
 
         let user_ns_config = UserNamespaceConfig::new(&spec)?;
 
-        if utils::rootless_required() && !utils::is_in_new_userns() && user_ns_config.is_none() {
-            return Err(LibcontainerError::NoUserNamespace);
-        }
-
         let config = YoukiConfig::from_spec(&spec, container.id(), user_ns_config.is_some())?;
         config.save(&container_dir).map_err(|err| {
             tracing::error!(?container_dir, "failed to save config: {}", err);
@@ -197,6 +193,8 @@ impl InitContainerBuilder {
             }
         }
 
+        utils::validate_spec_for_new_user_ns(spec)?;
+
         Ok(())
     }
 

crates/libcontainer/src/container/tenant_builder.rs

@@ -121,10 +121,6 @@ impl TenantContainerBuilder {
         let use_systemd = self.should_use_systemd(&container);
         let user_ns_config = UserNamespaceConfig::new(&spec)?;
 
-        if utils::rootless_required() && !utils::is_in_new_userns() && user_ns_config.is_none() {
-            return Err(LibcontainerError::NoUserNamespace);
-        }
-
         let (read_end, write_end) =
             pipe2(OFlag::O_CLOEXEC).map_err(LibcontainerError::OtherSyscall)?;
 
@@ -228,6 +224,8 @@ impl TenantContainerBuilder {
             }
         }
 
+        utils::validate_spec_for_new_user_ns(spec)?;
+
         Ok(())
     }
 

crates/libcontainer/src/utils.rs

@@ -10,6 +10,10 @@ use std::path::{Component, Path, PathBuf};
 use nix::sys::stat::Mode;
 use nix::sys::statfs;
 use nix::unistd::{Uid, User};
+use oci_spec::runtime::Spec;
+
+use crate::error::LibcontainerError;
+use crate::user_ns::UserNamespaceConfig;
 
 #[derive(Debug, thiserror::Error)]
 pub enum PathBufExtError {
@@ -273,10 +277,28 @@ pub fn rootless_required() -> bool {
     is_in_new_userns()
 }
 
+/// checks if given spec is valid for current user namespace setup
+pub fn validate_spec_for_new_user_ns(spec: &Spec) -> Result<(), LibcontainerError> {
+    let config = UserNamespaceConfig::new(spec)?;
+
+    // In case of rootless, there are 2 possible cases :
+    // we have a new user ns specified in the spec
+    // or the youki is launched in a new user ns (this is how podman does it)
+    // So here, we check if rootless is required,
+    // but we are neither in a new user ns nor a new user ns is specified in spec
+    // then it is an error
+    if rootless_required() && !is_in_new_userns() && config.is_none() {
+        return Err(LibcontainerError::NoUserNamespace);
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test_utils;
     use anyhow::{bail, Result};
+    use serial_test::serial;
 
     #[test]
     pub fn test_get_unix_user() {
@@ -379,4 +401,36 @@ mod tests {
 
         Ok(())
     }
+
+    // the following test is marked as serial because
+    // we are doing unshare of user ns and fork, so better to run in serial,
+    #[test]
+    #[serial]
+    fn test_userns_spec_validation() -> Result<(), test_utils::TestError> {
+        use nix::sched::{unshare, CloneFlags};
+        // default rootful spec
+        let rootful_spec = Spec::default();
+        // as we are not in a user ns, and spec does not have user ns
+        // we should get error here
+        assert!(validate_spec_for_new_user_ns(&rootful_spec).is_err());
+
+        let rootless_spec = Spec::rootless(1000, 1000);
+        // because the spec contains user ns info, we should not get error
+        assert!(validate_spec_for_new_user_ns(&rootless_spec).is_ok());
+
+        test_utils::test_in_child_process(|| {
+            unshare(CloneFlags::CLONE_NEWUSER).unwrap();
+            // here we are in a new user namespace
+            let rootful_spec = Spec::default();
+            // because we are already in a new user ns, it is fine if spec
+            // does not have user ns, and because the test is running as
+            // non root
+            assert!(validate_spec_for_new_user_ns(&rootful_spec).is_ok());
+
+            let rootless_spec = Spec::rootless(1000, 1000);
+            // following should succeed irrespective if we're in user ns or not
+            assert!(validate_spec_for_new_user_ns(&rootless_spec).is_ok());
+            Ok(())
+        })
+    }
 }