Introduce uid and gid to the CellService start method (#532)

This allows the caller to override the uid and gid of the spawned
executable within the Cell.  The response now includes the uid and gid,
even if they've been inherited from the parent auraed.

api/v0/cells/cells.proto

@@ -122,6 +122,8 @@ message CellServiceFreeResponse {}
 message CellServiceStartRequest {
   optional string cell_name = 1;
   Executable executable = 2;
+  optional uint32 uid = 3;
+  optional uint32 gid = 4;
 }
 
 // The response after starting an executable within a Cell.
@@ -130,8 +132,10 @@ message CellServiceStartResponse {
   // in various libc libraries.
   int32 pid = 1;
 
-  // int32 gid = 2;     // TODO
-  // int32 uid = 3;     // TODO
+  // Return uid and gid of the spawned child which should either match the
+  // `CellServiceStartRequest` or be inherited from the auraed process.
+  uint32 uid = 2;
+  uint32 gid = 3;
   // string user = 4;   // TODO
   // string group = 5;  // TODO
 }

auraed/src/cells/cell_service/cell_service.rs

@@ -39,6 +39,7 @@ use proto::{
     },
     observe::LogChannelType,
 };
+use std::os::unix::fs::MetadataExt;
 use std::time::Duration;
 use std::{process::ExitStatus, sync::Arc};
 use tokio::sync::Mutex;
@@ -201,8 +202,12 @@ impl CellService {
         &self,
         request: ValidatedCellServiceStartRequest,
     ) -> std::result::Result<Response<CellServiceStartResponse>, Status> {
-        let ValidatedCellServiceStartRequest { cell_name, executable } =
-            request;
+        let ValidatedCellServiceStartRequest {
+            cell_name,
+            executable,
+            uid,
+            gid,
+        } = request;
 
         assert!(cell_name.is_none());
         info!("CellService: start() executable={:?}", executable);
@@ -211,7 +216,7 @@ impl CellService {
 
         // Start the executable and handle any errors
         let executable = executables
-            .start(executable)
+            .start(executable, uid, gid)
             .map_err(CellsServiceError::ExecutablesError)?;
 
         // Retrieve the process ID (PID) of the started executable
@@ -247,7 +252,14 @@ impl CellService {
             warn!("failed to register stderr channel for pid {pid}: {e}");
         }
 
-        Ok(Response::new(CellServiceStartResponse { pid }))
+        let (self_uid, self_gid) =
+            std::fs::metadata("/proc/self").map(|m| (m.uid(), m.gid()))?;
+
+        Ok(Response::new(CellServiceStartResponse {
+            pid,
+            uid: uid.unwrap_or(self_uid),
+            gid: gid.unwrap_or(self_gid),
+        }))
     }
 
     #[tracing::instrument(skip(self))]
@@ -646,7 +658,11 @@ mod tests {
         // Create a validated cell for the allocate request
         let cell = ValidatedCell {
             name: CellName::from(cell_name),
-            cpu: Some(ValidatedCpuController { weight: None, max: None, period: None }),
+            cpu: Some(ValidatedCpuController {
+                weight: None,
+                max: None,
+                period: None,
+            }),
             cpuset: Some(ValidatedCpusetController { cpus: None, mems: None }),
             memory: Some(ValidatedMemoryController {
                 min: None,
@@ -660,4 +676,4 @@ mod tests {
         // Return the validated allocate request
         ValidatedCellServiceAllocateRequest { cell }
     }
-}
+}

auraed/src/cells/cell_service/executables/executable.rs

@@ -27,7 +27,6 @@ use tracing::info_span;
 
 // TODO: decide if we're going to use the description or not.  Remove if not.
 #[allow(dead_code)]
-
 #[derive(Debug)]
 pub struct Executable {
     pub name: ExecutableName,
@@ -65,17 +64,27 @@ impl Executable {
 
     /// Starts the underlying process.
     /// Does nothing if [Executable] has previously been started.
-    pub fn start(&mut self) -> io::Result<()> {
+    pub fn start(
+        &mut self,
+        uid: Option<u32>,
+        gid: Option<u32>,
+    ) -> io::Result<()> {
         let ExecutableState::Init { command } = &mut self.state else {
             return Ok(());
         };
 
-        let mut child = command
+        let mut command = command
             .kill_on_drop(true)
             .current_dir("/")
             .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()?;
+            .stderr(Stdio::piped());
+        if uid.is_some() {
+            command = command.uid(uid.expect("uid"));
+        }
+        if gid.is_some() {
+            command = command.gid(gid.expect("gid"));
+        }
+        let mut child = command.spawn()?;
 
         let log_channel = self.stdout.clone();
         let stdout = child.stdout.take().expect("stdout");
@@ -146,10 +155,11 @@ impl Executable {
 
     /// Returns the [Pid] while [Executable] is running, otherwise returns [None].
     pub fn pid(&self) -> io::Result<Option<Pid>> {
-        let ExecutableState::Started { child: process, .. } = &self.state else {
+        let ExecutableState::Started { child: process, .. } = &self.state
+        else {
             return Ok(None);
         };
 
         Ok(process.id().map(|id| Pid::from_raw(id as i32)))
     }
-}
+}

auraed/src/cells/cell_service/executables/executables.rs

@@ -30,6 +30,8 @@ impl Executables {
     pub fn start<T: Into<ExecutableSpec>>(
         &mut self,
         executable_spec: T,
+        uid: Option<u32>,
+        gid: Option<u32>,
     ) -> Result<&Executable> {
         let executable_spec = executable_spec.into();
 
@@ -46,7 +48,7 @@ impl Executables {
 
         // start the exe before we add it to the cache, as otherwise a failure leads to the
         // executable remaining in the cache and start cannot be called again.
-        executable.start().map_err(|e| {
+        executable.start(uid, gid).map_err(|e| {
             ExecutablesError::FailedToStartExecutable {
                 executable_name: executable_name.clone(),
                 source: e,

auraed/src/cells/cell_service/executables/mod.rs

@@ -29,4 +29,4 @@ pub struct ExecutableSpec {
     pub name: ExecutableName,
     pub description: String,
     pub command: Command,
-}
+}

auraed/src/cells/cell_service/validation.rs

@@ -238,6 +238,10 @@ pub struct ValidatedCellServiceStartRequest {
     pub cell_name: Option<CellName>,
     #[field_type(Option<Executable>)]
     pub executable: ValidatedExecutable,
+    #[validate(none)]
+    pub uid: Option<u32>,
+    #[validate(none)]
+    pub gid: Option<u32>,
 }
 
 impl CellServiceStartRequestTypeValidator for CellServiceStartRequestValidator {
@@ -540,4 +544,4 @@ mod tests {
         assert!(validated.is_ok());
         assert_eq!(validated.unwrap(), OsString::from("command"));
     }
-}
+}

auraed/tests/common/cells.rs

@@ -115,11 +115,18 @@ impl ExecutableBuilder {
 pub(crate) struct CellServiceStartRequestBuilder {
     cell_name: Option<String>,
     executable_builder: ExecutableBuilder,
+    uid: Option<u32>,
+    gid: Option<u32>,
 }
 
 impl CellServiceStartRequestBuilder {
     pub fn new() -> Self {
-        Self { cell_name: None, executable_builder: ExecutableBuilder::new() }
+        Self {
+            cell_name: None,
+            executable_builder: ExecutableBuilder::new(),
+            uid: None,
+            gid: None,
+        }
     }
 
     pub fn cell_name(&mut self, cell_name: String) -> &mut Self {
@@ -132,11 +139,23 @@ impl CellServiceStartRequestBuilder {
         self
     }
 
+    pub fn uid(&mut self, uid: u32) -> &mut Self {
+        self.uid = Some(uid);
+        self
+    }
+
+    pub fn gid(&mut self, gid: u32) -> &mut Self {
+        self.gid = Some(gid);
+        self
+    }
+
     pub fn build(&self) -> CellServiceStartRequest {
         assert!(self.cell_name.is_some(), "cell_name needs to be set");
         CellServiceStartRequest {
             cell_name: self.cell_name.clone(),
             executable: Some(self.executable_builder.build()),
+            uid: self.uid,
+            gid: self.gid,
         }
     }
-}
+}