From d7e026f25104c275eef55593cf73882bf7d80b8d Mon Sep 17 00:00:00 2001 From: Dominic Hamon Date: Fri, 16 Aug 2024 14:59:54 +0100 Subject: [PATCH] Introduce uid and gid to the CellService start method 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. --- Makefile | 26 ++++++++--------- api/v0/cells/cells.proto | 8 ++++-- auraed/src/cells/cell_service/cell_service.rs | 28 +++++++++++++++---- .../cell_service/executables/executable.rs | 24 +++++++++++----- .../cell_service/executables/executables.rs | 4 ++- .../src/cells/cell_service/executables/mod.rs | 2 +- auraed/src/cells/cell_service/validation.rs | 6 +++- auraed/tests/common/cells.rs | 23 +++++++++++++-- 8 files changed, 88 insertions(+), 33 deletions(-) diff --git a/Makefile b/Makefile index c9b767a2f..e16ed2fe5 100644 --- a/Makefile +++ b/Makefile @@ -204,35 +204,35 @@ $(1)-lint: $(GEN_RS) $(GEN_TS) .PHONY: $(1)-test $(1)-test: $(GEN_RS) $(GEN_TS) auraed - $(cargo) test -p $(1) + $(cargo) test --locked -p $(1) .PHONY: $(1)-test-all $(1)-test-all: $(GEN_RS) $(GEN_TS) auraed - $(root_cargo) test -p $(1) -- --include-ignored + $(root_cargo) test --locked -p $(1) -- --include-ignored .PHONY: $(1)-test-integration $(1)-test-integration: $(GEN_RS) $(GEN_TS) auraed - $(root_cargo) test -p $(1) --test '*' -- --include-ignored + $(root_cargo) test --locked -p $(1) --test '*' -- --include-ignored .PHONY: $(1)-test-watch $(1)-test-watch: $(GEN_RS) $(GEN_TS) auraed # Use cargo-watch to continuously run a test (e.g. make $(1)-test-watch name=path::to::test) - $(root_cargo) watch -- $(cargo) test -p $(1) $(name) -- --include-ignored --nocapture + $(root_cargo) watch -- $(cargo) test --locked -p $(1) $(name) -- --include-ignored --nocapture .PHONY: $(1)-build $(1)-build: $(GEN_RS) $(GEN_TS) - $(cargo) build -p $(1) + $(cargo) build --locked -p $(1) .PHONY: $(1)-build-release $(1)-build-release: $(GEN_RS) $(GEN_TS) - $(cargo) build -p $(1) --release + $(cargo) build --locked -p $(1) --release .PHONY: $(1)-debug $(1)-debug: $(GEN_RS) $(GEN_TS) $(1)-lint - $(cargo) install --path ./$(1) --debug --force + $(cargo) install --locked --path ./$(1) --debug --force .PHONY: $(1)-release $(1)-release: $(GEN_RS) $(GEN_TS) $(1)-lint $(1)-test ## Lint, test, and install $(1) - $(cargo) install --path ./$(1) --force + $(cargo) install --locked --path ./$(1) --force endef $(foreach p,$(PROGS),$(eval $(call AURAE_template,$(p),$(if $(findstring auraed,$(p)),)))) @@ -257,7 +257,7 @@ endif .PHONY: not-auraed-build not-auraed-build: $(GEN_RS) $(GEN_TS) - $(cargo) build --workspace --exclude auraed + $(cargo) build --locked --workspace --exclude auraed .PHONY: not-auraed-lint not-auraed-lint: $(GEN_RS) $(GEN_TS) @@ -265,11 +265,11 @@ not-auraed-lint: $(GEN_RS) $(GEN_TS) .PHONY: not-auraed-test not-auraed-test: $(GEN_RS) $(GEN_TS) - $(cargo) test --workspace --exclude auraed + $(cargo) test --locked --workspace --exclude auraed .PHONY: not-auraed-test-all not-auraed-test-all: $(GEN_RS) $(GEN_TS) - $(cargo) test --workspace --exclude auraed -- --include-ignored + $(cargo) test --locked --workspace --exclude auraed -- --include-ignored #------------------------------------------------------------------------------# @@ -281,11 +281,11 @@ libs-lint: $(GEN_RS) $(GEN_TS) .PHONY: libs-test libs-test: $(GEN_RS) $(GEN_TS) - $(cargo) test --workspace --exclude auraed --exclude auraescript --exclude aer + $(cargo) test --locked --workspace --exclude auraed --exclude auraescript --exclude aer .PHONY: libs-test-all libs-test-all: $(GEN_RS) $(GEN_TS) - $(cargo) test --workspace --exclude auraed --exclude auraescript --exclude aer -- --include-ignored + $(cargo) test --locked --workspace --exclude auraed --exclude auraescript --exclude aer -- --include-ignored .PHONY: ebpf ebpf: diff --git a/api/v0/cells/cells.proto b/api/v0/cells/cells.proto index 619369acc..ab99159f4 100644 --- a/api/v0/cells/cells.proto +++ b/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 } diff --git a/auraed/src/cells/cell_service/cell_service.rs b/auraed/src/cells/cell_service/cell_service.rs index bf2b6fbfa..93bbd0dce 100644 --- a/auraed/src/cells/cell_service/cell_service.rs +++ b/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, 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 } } -} \ No newline at end of file +} diff --git a/auraed/src/cells/cell_service/executables/executable.rs b/auraed/src/cells/cell_service/executables/executable.rs index a844fd195..74d6fee1d 100644 --- a/auraed/src/cells/cell_service/executables/executable.rs +++ b/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, + gid: Option, + ) -> 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> { - 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))) } -} \ No newline at end of file +} diff --git a/auraed/src/cells/cell_service/executables/executables.rs b/auraed/src/cells/cell_service/executables/executables.rs index 2df13e1f8..62adb8a84 100644 --- a/auraed/src/cells/cell_service/executables/executables.rs +++ b/auraed/src/cells/cell_service/executables/executables.rs @@ -30,6 +30,8 @@ impl Executables { pub fn start>( &mut self, executable_spec: T, + uid: Option, + gid: Option, ) -> 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, diff --git a/auraed/src/cells/cell_service/executables/mod.rs b/auraed/src/cells/cell_service/executables/mod.rs index f8978eb84..ee8e90b18 100644 --- a/auraed/src/cells/cell_service/executables/mod.rs +++ b/auraed/src/cells/cell_service/executables/mod.rs @@ -29,4 +29,4 @@ pub struct ExecutableSpec { pub name: ExecutableName, pub description: String, pub command: Command, -} \ No newline at end of file +} diff --git a/auraed/src/cells/cell_service/validation.rs b/auraed/src/cells/cell_service/validation.rs index f95d3a1ec..57f5d6a6c 100644 --- a/auraed/src/cells/cell_service/validation.rs +++ b/auraed/src/cells/cell_service/validation.rs @@ -238,6 +238,10 @@ pub struct ValidatedCellServiceStartRequest { pub cell_name: Option, #[field_type(Option)] pub executable: ValidatedExecutable, + #[validate(none)] + pub uid: Option, + #[validate(none)] + pub gid: Option, } impl CellServiceStartRequestTypeValidator for CellServiceStartRequestValidator { @@ -540,4 +544,4 @@ mod tests { assert!(validated.is_ok()); assert_eq!(validated.unwrap(), OsString::from("command")); } -} \ No newline at end of file +} diff --git a/auraed/tests/common/cells.rs b/auraed/tests/common/cells.rs index 076cb22d7..2c14a82e7 100644 --- a/auraed/tests/common/cells.rs +++ b/auraed/tests/common/cells.rs @@ -115,11 +115,18 @@ impl ExecutableBuilder { pub(crate) struct CellServiceStartRequestBuilder { cell_name: Option, executable_builder: ExecutableBuilder, + uid: Option, + gid: Option, } 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, } } -} \ No newline at end of file +}