use additional gids,user,group in exec, inject path iif not given

Signed-off-by: Yashodhan Joshi <[email protected]>

crates/libcontainer/src/container/tenant_builder.rs

@@ -14,7 +14,7 @@ use nix::unistd::{pipe2, read, Pid};
 use oci_spec::runtime::{
     Capabilities as SpecCapabilities, Capability as SpecCapability, LinuxBuilder,
     LinuxCapabilities, LinuxCapabilitiesBuilder, LinuxNamespace, LinuxNamespaceBuilder,
-    LinuxNamespaceType, LinuxSchedulerPolicy, Process, ProcessBuilder, Spec,
+    LinuxNamespaceType, LinuxSchedulerPolicy, Process, ProcessBuilder, Spec, UserBuilder,
 };
 use procfs::process::Namespace;
 
@@ -32,6 +32,22 @@ const NAMESPACE_TYPES: &[&str] = &["ipc", "uts", "net", "pid", "mnt", "cgroup"];
 const TENANT_NOTIFY: &str = "tenant-notify-";
 const TENANT_TTY: &str = "tenant-tty-";
 
+fn get_path_from_spec(spec: &Spec) -> Option<String> {
+    let process = match spec.process() {
+        Some(p) => p,
+        None => return None,
+    };
+    let env = match process.env() {
+        Some(e) => e,
+        None => return None,
+    };
+    env.iter()
+        .find(|e| e.starts_with("PATH"))
+        .iter()
+        .next()
+        .map(|s| s.to_string())
+}
+
 /// Builder that can be used to configure the properties of a process
 /// that will join an existing container sandbox
 pub struct TenantContainerBuilder {
@@ -44,6 +60,9 @@ pub struct TenantContainerBuilder {
     process: Option<PathBuf>,
     detached: bool,
     as_sibling: bool,
+    additional_gids: Vec<u32>,
+    user: Option<u32>,
+    group: Option<u32>,
 }
 
 impl TenantContainerBuilder {
@@ -61,6 +80,9 @@ impl TenantContainerBuilder {
             process: None,
             detached: false,
             as_sibling: false,
+            additional_gids: vec![],
+            user: None,
+            group: None,
         }
     }
 
@@ -109,6 +131,21 @@ impl TenantContainerBuilder {
         self
     }
 
+    pub fn with_additional_gids(mut self, gids: Vec<u32>) -> Self {
+        self.additional_gids = gids;
+        self
+    }
+
+    pub fn with_user(mut self, user: Option<u32>) -> Self {
+        self.user = user;
+        self
+    }
+
+    pub fn with_group(mut self, group: Option<u32>) -> Self {
+        self.group = group;
+        self
+    }
+
     /// Joins an existing container
     pub fn build(self) -> Result<Pid, LibcontainerError> {
         let container_dir = self.lookup_container_dir()?;
@@ -323,9 +360,10 @@ impl TenantContainerBuilder {
         let process = if let Some(process) = &self.process {
             self.get_process(process)?
         } else {
+            let original_path_env = get_path_from_spec(spec);
             let mut process_builder = ProcessBuilder::default()
                 .args(self.get_args()?)
-                .env(self.get_environment());
+                .env(self.get_environment(original_path_env));
             if let Some(cwd) = self.get_working_dir()? {
                 process_builder = process_builder.cwd(cwd);
             }
@@ -338,6 +376,22 @@ impl TenantContainerBuilder {
                 process_builder = process_builder.capabilities(caps);
             }
 
+            let mut user_builder = UserBuilder::default();
+
+            if !self.additional_gids.is_empty() {
+                user_builder = user_builder.additional_gids(self.additional_gids.clone());
+            }
+
+            if let Some(uid) = self.user {
+                user_builder = user_builder.uid(uid);
+            }
+
+            if let Some(gid) = self.group {
+                user_builder = user_builder.gid(gid);
+            }
+
+            process_builder = process_builder.user(user_builder.build()?);
+
             process_builder.build()?
         };
 
@@ -397,8 +451,28 @@ impl TenantContainerBuilder {
         Ok(self.args.clone())
     }
 
-    fn get_environment(&self) -> Vec<String> {
-        self.env.iter().map(|(k, v)| format!("{k}={v}")).collect()
+    fn get_environment(&self, path: Option<String>) -> Vec<String> {
+        let mut env_exists = false;
+        let mut env: Vec<String> = self
+            .env
+            .iter()
+            .map(|(k, v)| {
+                if k == "PATH" {
+                    env_exists = true;
+                }
+                format!("{k}={v}")
+            })
+            .collect();
+        // It is not possible in normal flow that path is None. The original container
+        // creation would have failed if path was absent. However we use Option
+        // just as a caution, and if neither exec cmd not original spec has PATH,
+        // the container creation will fail later which is ok
+        if let Some(p) = path {
+            if !env_exists {
+                env.push(p);
+            }
+        }
+        env
     }
 
     fn get_no_new_privileges(&self) -> Option<bool> {

crates/youki/src/commands/exec.rs

@@ -9,6 +9,11 @@ use nix::sys::wait::{waitpid, WaitStatus};
 use crate::workload::executor::default_executor;
 
 pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
+    // TODO: not all values from exec are used here. We need to support
+    // the remaining ones.
+    let user = args.user.map(|(u, _)| u);
+    let group = args.user.and_then(|(_, g)| g);
+
     let pid = ContainerBuilder::new(args.container_id.clone(), SyscallType::default())
         .with_executor(default_executor())
         .with_root_path(root_path)?
@@ -22,6 +27,9 @@ pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
         .with_process(args.process.as_ref())
         .with_no_new_privs(args.no_new_privs)
         .with_container_args(args.command.clone())
+        .with_additional_gids(args.additional_gids)
+        .with_user(user)
+        .with_group(group)
         .build()?;
 
     // See https://github.com/containers/youki/pull/1252 for a detailed explanation