Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce uid and gid to the CellService start method #532

Merged
merged 3 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions api/v0/cells/cells.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
28 changes: 22 additions & 6 deletions auraed/src/cells/cell_service/cell_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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))]
Expand Down Expand Up @@ -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,
Expand All @@ -660,4 +676,4 @@ mod tests {
// Return the validated allocate request
ValidatedCellServiceAllocateRequest { cell }
}
}
}
24 changes: 17 additions & 7 deletions auraed/src/cells/cell_service/executables/executable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
dmah42 marked this conversation as resolved.
Show resolved Hide resolved
.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");
Expand Down Expand Up @@ -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)))
}
}
}
4 changes: 3 additions & 1 deletion auraed/src/cells/cell_service/executables/executables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion auraed/src/cells/cell_service/executables/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ pub struct ExecutableSpec {
pub name: ExecutableName,
pub description: String,
pub command: Command,
}
}
6 changes: 5 additions & 1 deletion auraed/src/cells/cell_service/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -540,4 +544,4 @@ mod tests {
assert!(validated.is_ok());
assert_eq!(validated.unwrap(), OsString::from("command"));
}
}
}
23 changes: 21 additions & 2 deletions auraed/tests/common/cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
}
Loading