Bump the oci-spec-rs to 0.6.1 to resolve seccomp rule issue

crates/libcgroups/Cargo.toml

@@ -22,7 +22,7 @@ cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc"]
 [dependencies]
 nix = "0.26.2"
 procfs = "0.15.1"
-oci-spec = { version = "^0.6.0", features = ["runtime"] }
+oci-spec = { version = "^0.6.1", features = ["runtime"] }
 dbus = { version = "0.9.7", optional = true }
 fixedbitset = "0.4.2"
 serde = { version = "1.0", features = ["derive"] }

crates/libcontainer/Cargo.toml

@@ -28,7 +28,7 @@ fastrand = "^1.7.0"
 futures = { version = "0.3", features = ["thread-pool"] }
 libc = "0.2.146"
 nix = "0.26.2"
-oci-spec = { version = "^0.6.0", features = ["runtime"] }
+oci-spec = { version = "^0.6.1", features = ["runtime"] }
 once_cell = "1.18.0"
 procfs = "0.15.1"
 prctl = "1.0.0"

crates/libcontainer/src/namespaces.rs

@@ -22,6 +22,8 @@ pub enum NamespaceError {
     IO(#[from] std::io::Error),
     #[error(transparent)]
     Syscall(#[from] crate::syscall::SyscallError),
+    #[error("Namespace type not supported: {0}")]
+    NotSupported(String),
 }
 
 static ORDERED_NAMESPACES: &[CloneFlags] = &[
@@ -40,31 +42,41 @@ pub struct Namespaces {
     namespace_map: collections::HashMap<CloneFlags, LinuxNamespace>,
 }
 
-fn get_clone_flag(namespace_type: LinuxNamespaceType) -> CloneFlags {
-    match namespace_type {
+fn get_clone_flag(namespace_type: LinuxNamespaceType) -> Result<CloneFlags> {
+    let flag = match namespace_type {
         LinuxNamespaceType::User => CloneFlags::CLONE_NEWUSER,
         LinuxNamespaceType::Pid => CloneFlags::CLONE_NEWPID,
         LinuxNamespaceType::Uts => CloneFlags::CLONE_NEWUTS,
         LinuxNamespaceType::Ipc => CloneFlags::CLONE_NEWIPC,
         LinuxNamespaceType::Network => CloneFlags::CLONE_NEWNET,
         LinuxNamespaceType::Cgroup => CloneFlags::CLONE_NEWCGROUP,
         LinuxNamespaceType::Mount => CloneFlags::CLONE_NEWNS,
-    }
+        LinuxNamespaceType::Time => return Err(NamespaceError::NotSupported("time".to_string())),
+    };
+
+    Ok(flag)
 }
 
-impl From<Option<&Vec<LinuxNamespace>>> for Namespaces {
-    fn from(namespaces: Option<&Vec<LinuxNamespace>>) -> Self {
+impl TryFrom<Option<&Vec<LinuxNamespace>>> for Namespaces {
+    type Error = NamespaceError;
+
+    fn try_from(namespaces: Option<&Vec<LinuxNamespace>>) -> Result<Self> {
         let command: Box<dyn Syscall> = create_syscall();
         let namespace_map: collections::HashMap<CloneFlags, LinuxNamespace> = namespaces
             .unwrap_or(&vec![])
             .iter()
-            .map(|ns| (get_clone_flag(ns.typ()), ns.clone()))
+            .map(|ns| match get_clone_flag(ns.typ()) {
+                Ok(flag) => Ok((flag, ns.clone())),
+                Err(err) => Err(err),
+            })
+            .collect::<Result<Vec<(CloneFlags, LinuxNamespace)>>>()?
+            .into_iter()
             .collect();
 
-        Namespaces {
+        Ok(Namespaces {
             command,
             namespace_map,
-        }
+        })
     }
 }
 
@@ -93,7 +105,7 @@ impl Namespaces {
                     },
                 )?;
                 self.command
-                    .set_ns(fd, get_clone_flag(namespace.typ()))
+                    .set_ns(fd, get_clone_flag(namespace.typ())?)
                     .map_err(|err| {
                         tracing::error!(?err, ?namespace, "failed to set namespace");
                         err
@@ -105,7 +117,7 @@ impl Namespaces {
             }
             None => {
                 self.command
-                    .unshare(get_clone_flag(namespace.typ()))
+                    .unshare(get_clone_flag(namespace.typ())?)
                     .map_err(|err| {
                         tracing::error!(?err, ?namespace, "failed to unshare namespace");
                         err
@@ -116,8 +128,8 @@ impl Namespaces {
         Ok(())
     }
 
-    pub fn get(&self, k: LinuxNamespaceType) -> Option<&LinuxNamespace> {
-        self.namespace_map.get(&get_clone_flag(k))
+    pub fn get(&self, k: LinuxNamespaceType) -> Result<Option<&LinuxNamespace>> {
+        Ok(self.namespace_map.get(&get_clone_flag(k)?))
     }
 }
 
@@ -159,7 +171,8 @@ mod tests {
     #[serial]
     fn test_apply_namespaces() {
         let sample_linux_namespaces = gen_sample_linux_namespaces();
-        let namespaces = Namespaces::from(Some(&sample_linux_namespaces));
+        let namespaces = Namespaces::try_from(Some(&sample_linux_namespaces))
+            .expect("create namespace struct should be good");
         let test_command: &TestHelperSyscall = namespaces.command.as_any().downcast_ref().unwrap();
         assert!(namespaces
             .apply_namespaces(|ns_type| { ns_type != CloneFlags::CLONE_NEWIPC })

crates/libcontainer/src/process/container_init_process.rs

@@ -271,7 +271,7 @@ fn apply_rest_namespaces(
         })?;
 
     // Only set the host name if entering into a new uts namespace
-    if let Some(uts_namespace) = namespaces.get(LinuxNamespaceType::Uts) {
+    if let Some(uts_namespace) = namespaces.get(LinuxNamespaceType::Uts)? {
         if uts_namespace.path().is_none() {
             if let Some(hostname) = spec.hostname() {
                 syscall.set_hostname(hostname).map_err(|err| {
@@ -340,7 +340,7 @@ pub fn container_init_process(
     let rootfs_path = args.rootfs;
     let hooks = spec.hooks().as_ref();
     let container = args.container.as_ref();
-    let namespaces = Namespaces::from(linux.namespaces().as_ref());
+    let namespaces = Namespaces::try_from(linux.namespaces().as_ref())?;
 
     setsid().map_err(|err| {
         tracing::error!(?err, "failed to setsid to create a session");
@@ -370,14 +370,14 @@ pub fn container_init_process(
             })?;
         }
 
-        let bind_service = namespaces.get(LinuxNamespaceType::User).is_some();
+        let bind_service = namespaces.get(LinuxNamespaceType::User)?.is_some();
         let rootfs = RootFS::new();
         rootfs
             .prepare_rootfs(
                 spec,
                 rootfs_path,
                 bind_service,
-                namespaces.get(LinuxNamespaceType::Cgroup).is_some(),
+                namespaces.get(LinuxNamespaceType::Cgroup)?.is_some(),
             )
             .map_err(|err| {
                 tracing::error!(?err, "failed to prepare rootfs");
@@ -388,7 +388,7 @@ pub fn container_init_process(
         // we use pivot_root, but if we are on the host mount namespace, we will
         // use simple chroot. Scary things will happen if you try to pivot_root
         // in the host mount namespace...
-        if namespaces.get(LinuxNamespaceType::Mount).is_some() {
+        if namespaces.get(LinuxNamespaceType::Mount)?.is_some() {
             // change the root of filesystem of the process to the rootfs
             syscall.pivot_rootfs(rootfs_path).map_err(|err| {
                 tracing::error!(?err, ?rootfs_path, "failed to pivot root");
@@ -848,7 +848,7 @@ mod tests {
                 .typ(LinuxNamespaceType::Pid)
                 .build()?,
         ];
-        let namespaces = Namespaces::from(Some(&linux_spaces));
+        let namespaces = Namespaces::try_from(Some(&linux_spaces))?;
 
         apply_rest_namespaces(&namespaces, &spec, syscall.as_ref())?;
 

crates/libcontainer/src/process/container_intermediate_process.rs

@@ -5,7 +5,6 @@ use nix::unistd::{close, write};
 use nix::unistd::{Gid, Pid, Uid};
 use oci_spec::runtime::{LinuxNamespaceType, LinuxResources};
 use procfs::process::Process;
-use std::convert::From;
 
 use super::args::{ContainerArgs, ContainerType};
 use super::container_init_process::container_init_process;
@@ -43,7 +42,7 @@ pub fn container_intermediate_process(
     let command = &args.syscall;
     let spec = &args.spec;
     let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-    let namespaces = Namespaces::from(linux.namespaces().as_ref());
+    let namespaces = Namespaces::try_from(linux.namespaces().as_ref())?;
 
     // this needs to be done before we create the init process, so that the init
     // process will already be captured by the cgroup. It also needs to be done
@@ -65,7 +64,7 @@ pub fn container_intermediate_process(
     // namespace will be created, check
     // https://man7.org/linux/man-pages/man7/user_namespaces.7.html for more
     // information
-    if let Some(user_namespace) = namespaces.get(LinuxNamespaceType::User) {
+    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");
@@ -104,7 +103,7 @@ pub fn container_intermediate_process(
     }
 
     // Pid namespace requires an extra fork to enter, so we enter pid namespace now.
-    if let Some(pid_namespace) = namespaces.get(LinuxNamespaceType::Pid) {
+    if let Some(pid_namespace) = namespaces.get(LinuxNamespaceType::Pid)? {
         namespaces.unshare_or_setns(pid_namespace)?;
     }
 

crates/libcontainer/src/rootless.rs

@@ -1,5 +1,5 @@
 use crate::error::MissingSpecError;
-use crate::namespaces::Namespaces;
+use crate::namespaces::{NamespaceError, Namespaces};
 use nix::unistd::Pid;
 use oci_spec::runtime::{Linux, LinuxIdMapping, LinuxNamespace, LinuxNamespaceType, Mount, Spec};
 use std::fs;
@@ -103,6 +103,8 @@ pub enum ValidateSpecError {
     MountGidMapping(u32),
     #[error("mount options require mapping gid inside the rootless container")]
     MountUidMapping(u32),
+    #[error(transparent)]
+    Namespaces(#[from] NamespaceError),
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -140,8 +142,11 @@ pub struct Rootless<'a> {
 impl<'a> Rootless<'a> {
     pub fn new(spec: &'a Spec) -> Result<Option<Rootless<'a>>> {
         let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-        let namespaces = Namespaces::from(linux.namespaces().as_ref());
-        let user_namespace = namespaces.get(LinuxNamespaceType::User);
+        let namespaces = Namespaces::try_from(linux.namespaces().as_ref())
+            .map_err(ValidateSpecError::Namespaces)?;
+        let user_namespace = namespaces
+            .get(LinuxNamespaceType::User)
+            .map_err(ValidateSpecError::Namespaces)?;
 
         // If conditions requires us to use rootless, we must either create a new
         // user namespace or enter an existing.
@@ -156,7 +161,7 @@ impl<'a> Rootless<'a> {
                 tracing::error!("failed to validate spec for rootless container: {}", err);
                 err
             })?;
-            let mut rootless = Rootless::from(linux);
+            let mut rootless = Rootless::try_from(linux)?;
             if let Some((uid_binary, gid_binary)) = lookup_map_binaries(linux)? {
                 rootless.newuidmap = Some(uid_binary);
                 rootless.newgidmap = Some(gid_binary);
@@ -200,19 +205,24 @@ impl<'a> Rootless<'a> {
     }
 }
 
-impl<'a> From<&'a Linux> for Rootless<'a> {
-    fn from(linux: &'a Linux) -> Self {
-        let namespaces = Namespaces::from(linux.namespaces().as_ref());
-        let user_namespace = namespaces.get(LinuxNamespaceType::User);
-        Self {
+impl<'a> TryFrom<&'a Linux> for Rootless<'a> {
+    type Error = RootlessError;
+
+    fn try_from(linux: &'a Linux) -> Result<Self> {
+        let namespaces = Namespaces::try_from(linux.namespaces().as_ref())
+            .map_err(ValidateSpecError::Namespaces)?;
+        let user_namespace = namespaces
+            .get(LinuxNamespaceType::User)
+            .map_err(ValidateSpecError::Namespaces)?;
+        Ok(Self {
             newuidmap: None,
             newgidmap: None,
             uid_mappings: linux.uid_mappings().as_ref(),
             gid_mappings: linux.gid_mappings().as_ref(),
             user_namespace: user_namespace.cloned(),
             privileged: nix::unistd::geteuid().is_root(),
             rootless_id_mapper: RootlessIDMapper::new(),
-        }
+        })
     }
 }
 
@@ -250,8 +260,8 @@ pub fn unprivileged_user_ns_enabled() -> Result<bool> {
 fn validate_spec_for_rootless(spec: &Spec) -> std::result::Result<(), ValidateSpecError> {
     tracing::debug!(?spec, "validating spec for rootless container");
     let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-    let namespaces = Namespaces::from(linux.namespaces().as_ref());
-    if namespaces.get(LinuxNamespaceType::User).is_none() {
+    let namespaces = Namespaces::try_from(linux.namespaces().as_ref())?;
+    if namespaces.get(LinuxNamespaceType::User)?.is_none() {
         return Err(ValidateSpecError::NoUserNamespace);
     }
 

crates/libcontainer/src/seccomp/mod.rs

@@ -78,6 +78,7 @@ fn translate_arch(arch: Arch) -> ScmpArch {
 }
 
 fn translate_action(action: LinuxSeccompAction, errno: Option<u32>) -> Result<ScmpAction> {
+    tracing::trace!(?action, ?errno, "translating action");
     let errno = errno.map(|e| e as i32).unwrap_or(libc::EPERM);
     let action = match action {
         LinuxSeccompAction::ScmpActKill => ScmpAction::KillThread,
@@ -94,6 +95,7 @@ fn translate_action(action: LinuxSeccompAction, errno: Option<u32>) -> Result<Sc
         LinuxSeccompAction::ScmpActLog => ScmpAction::Log,
     };
 
+    tracing::trace!(?action, "translated action");
     Ok(action)
 }
 
@@ -139,9 +141,11 @@ fn check_seccomp(seccomp: &LinuxSeccomp) -> Result<()> {
     Ok(())
 }
 
+#[tracing::instrument(level = "trace", skip(seccomp))]
 pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
     check_seccomp(seccomp)?;
 
+    tracing::trace!(default_action = ?seccomp.default_action(), errno = ?seccomp.default_errno_ret(), "initializing seccomp");
     let default_action = translate_action(seccomp.default_action(), seccomp.default_errno_ret())?;
     let mut ctx =
         ScmpFilterContext::new_filter(default_action).map_err(|err| SeccompError::NewFilter {
@@ -165,6 +169,7 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
 
     if let Some(architectures) = seccomp.architectures() {
         for &arch in architectures {
+            tracing::trace!(?arch, "adding architecture");
             ctx.add_arch(translate_arch(arch))
                 .map_err(|err| SeccompError::AddArch { source: err, arch })?;
         }
@@ -226,6 +231,7 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
                                 translate_op(arg.op(), arg.value_two()),
                                 arg.value(),
                             );
+                            tracing::trace!(?name, ?action, ?arg, "add seccomp conditional rule");
                             ctx.add_rule_conditional(action, sc, &[cmp])
                                 .map_err(|err| {
                                     tracing::error!(
@@ -238,6 +244,7 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
                         }
                     }
                     None => {
+                        tracing::trace!(?name, ?action, "add seccomp rule");
                         ctx.add_rule(action, sc).map_err(|err| {
                             tracing::error!(
                                 "failed to add seccomp rule: {:?}. Syscall: {name}",

crates/liboci-cli/src/lib.rs

@@ -70,9 +70,10 @@ pub struct GlobalOpts {
     // Example in future : '--debug     change log level to debug. (default: "warn")'
     #[clap(long)]
     pub debug: bool,
-    // Set a consistent behavior like in runc and crun: set log to the last given value
+    /// set the log file to write youki logs to (default is '/dev/stderr')
     #[clap(short, long, overrides_with("log"))]
     pub log: Option<PathBuf>,
+    /// set the log format ('text' (default), or 'json') (default: "text")
     #[clap(long)]
     pub log_format: Option<String>,
     /// root directory to store container state

crates/youki/Cargo.toml

@@ -32,7 +32,7 @@ libcgroups = { version = "0.0.5", path = "../libcgroups", default-features = fal
 libcontainer = { version = "0.0.5", path = "../libcontainer", default-features = false }
 liboci-cli = { version = "0.0.5", path = "../liboci-cli" }
 nix = "0.26.2"
-oci-spec = { version = "^0.6.0", features = ["runtime"] }
+oci-spec = { version = "^0.6.1", features = ["runtime"] }
 once_cell = "1.18.0"
 pentacle = "1.0.0"
 procfs = "0.15.1"