diff --git a/.config/mise/tasks/ci.toml b/.config/mise/tasks/ci.toml index a29693fa..d1cca0f0 100644 --- a/.config/mise/tasks/ci.toml +++ b/.config/mise/tasks/ci.toml @@ -161,6 +161,10 @@ description = "CI's `mutants` job's command: the wrapper over cargo-mutants. The # `--in-place` mutates this checkout, which the runner throws away, so rust-cache can restore # dependencies into its target. Run from a worktree you keep, pass it without `--in-place`, or # run `mise run mutants`. +# The CLI's parity test spawns microvms-core's `parity-tunnel-peer` example from the test +# binary's target directory, and a mutant's `cargo test -p microvms-cli` never builds another +# package's example, so the fixture is built first into the target `--in-place` tests use. +depends = ["parity:fixtures"] tools = { "cargo:cargo-mutants" = "{{vars.cargo_mutants}}" } run = "./tools/check-mutants.py" diff --git a/.config/trivyignore.yaml b/.config/trivyignore.yaml index aac65de2..60db3be1 100644 --- a/.config/trivyignore.yaml +++ b/.config/trivyignore.yaml @@ -43,7 +43,8 @@ misconfigurations: (docs/TRUST.md), and the MicroVM, one tenant per Firecracker VM with its own kernel, is the isolation boundary for the host. DS-0002's goal, an unprivileged workload process, applies to agentd's execs: an exec runs - as the user its caller names, and as root when it names none. Revisit + as the user its caller names, as the image's AGENTD_DEFAULT_USER when + it names none and the image sets one, and as root otherwise. Revisit this exception if agentd stops needing to demote execs. The strands-box-codex-sdk example also needs root at start to give its sandbox a private mount namespace with a fresh /proc before diff --git a/changelog.d/595.added.md b/changelog.d/595.added.md new file mode 100644 index 00000000..2fd506a0 --- /dev/null +++ b/changelog.d/595.added.md @@ -0,0 +1,16 @@ +- **An image can set the user an exec without `user` runs as (#595).** The daemon reads + `AGENTD_DEFAULT_USER` from the image's environment at startup: a user name, or a uid as + digits. A start request that omits `user`, from `exec` or `run --exec` alike, resolves it at + each start to a row of the guest's `/etc/passwd`, by name or by uid, so a user an earlier exec + created resolves on the next. The child gets that row's uid, primary gid, `HOME`, `USER` and + `LOGNAME`, a uid included. A request's own `user` still runs as given, `root` and `0` + included. If no row matches, digits included, each start without a `user` answers + `400 unknown_user`, and the detail mentions the variable. Uploads don't follow the variable: + what `PUT /v1/fs/tar` and `PUT /v1/fs/file` create stays root's, so the default user can read + the tree `run . --exec` and `cp` upload but can't write into it. An exec that must write there + runs with `user: 0`, or the image or an exec with `user: 0` hands the tree to the default user + with `chown` first. With the variable unset, nothing changes. Workload hook handlers still run + as root. The agent recipe's `chown` of `/workspace` and a sync's in-guest deletions now ask + for uid 0 by number, so an image's default user can't make them fail. An image that sets the + variable needs a client at this release or later: an older client sends that `chown` and those + deletions with no `user`, so they run as the default user and fail. diff --git a/crates/agentd/src/config.rs b/crates/agentd/src/config.rs index da3513a8..24deb935 100644 --- a/crates/agentd/src/config.rs +++ b/crates/agentd/src/config.rs @@ -93,8 +93,25 @@ pub struct Config { /// The group database a start request's group name is resolved against. `/etc/group` /// outside tests, for `passwd_path`'s reason. pub group_path: std::path::PathBuf, + /// The user a start request runs as when it names no `user`, from + /// [`DEFAULT_USER_VAR`] (AGENTD-22). `None` keeps the daemon's own user, which is root. + /// + /// Kept as the image wrote it and resolved per request exactly as a request's `user` + /// string is: a passwd name, or digits that name no row as a uid. Per request because a + /// user an earlier exec created resolves on the next. A request's own `user` always wins, + /// `root` and `0` included (AGENTD-23), so an authorized caller can still ask for root. + /// + /// Read from the image's environment like every other `AGENTD_*` setting, and never from + /// a request or the launch environment: who a workload runs as when its caller doesn't + /// say is the image author's decision. Workload hook handlers don't take it; they are + /// image-owned code and run as the daemon's user. + pub default_user: Option, } +/// The variable naming [`Config::default_user`]. A refusal of the default names it, so an +/// image author who misspelled the user can find where it was set. +pub const DEFAULT_USER_VAR: &str = "AGENTD_DEFAULT_USER"; + /// The handler budget's ceiling: five seconds under the platform's 60 s hook limit. pub const MAX_HOOK_HANDLER_SECS: u64 = 55; @@ -128,6 +145,7 @@ impl Default for Config { hook_handler_timeout: Duration::from_secs(20), passwd_path: std::path::PathBuf::from("/etc/passwd"), group_path: std::path::PathBuf::from("/etc/group"), + default_user: None, } } } @@ -175,10 +193,32 @@ impl Config { if let Some(secs) = env_parse::("AGENTD_HOOK_HANDLER_TIMEOUT_SECS") { cfg.hook_handler_timeout = Duration::from_secs(secs.clamp(1, MAX_HOOK_HANDLER_SECS)); } + cfg.default_user = default_user(std::env::var_os(DEFAULT_USER_VAR)); + // Logged to the image's log group because `/v1/health` can't carry it yet: `Health` is + // constructible field by field, so a new field is a breaking change to the protocol + // crate. + if let Some(user) = &cfg.default_user { + tracing::info!( + default_user = %user, + "a start request that omits user runs as {DEFAULT_USER_VAR}" + ); + } cfg } } +/// [`DEFAULT_USER_VAR`] as the daemon keeps it: set means set, an empty value included. +/// +/// Unlike the numeric knobs, a value that names nobody is not ignored. It resolves the way a +/// request's `user` string does, so an empty or misspelled one refuses every start that +/// names no user, naming the variable, instead of quietly running the workload as root. That +/// costs no reachability: the daemon still boots, and a request that names its user still +/// runs. A value that isn't UTF-8 is kept lossily, which no passwd name matches, so it is +/// refused the same way rather than dropped. +fn default_user(raw: Option) -> Option { + raw.map(|raw| raw.to_string_lossy().into_owned()) +} + /// Parses a boolean knob, accepting the spellings a Dockerfile or a Terraform /// template actually produces. `bool::from_str` takes only `true`/`false`, so /// `AGENTD_REPAIR_IDENTITY=0` would otherwise be ignored with a warning — and a @@ -219,5 +259,44 @@ mod tests { // single noisy command can kill a daemon nothing can restart. assert!(cfg.max_output_bytes < 64 * 1024 * 1024); assert!(cfg.max_drain_bytes < cfg.max_body_bytes); + assert_eq!(cfg.default_user, None, "unset keeps the daemon's own user"); + } + + /// **AGENTD-22.** The default user is kept as the image set it: unset is none, and any + /// set value, empty or not UTF-8 included, is kept to be resolved, so a value that names + /// nobody is refused at the first start instead of falling back to root. + #[test] + fn the_default_user_is_kept_as_set() { + use std::os::unix::ffi::OsStringExt; + assert_eq!(default_user(None), None); + assert_eq!(default_user(Some("agent".into())), Some("agent".into())); + assert_eq!(default_user(Some("1000".into())), Some("1000".into())); + assert_eq!(default_user(Some("".into())), Some(String::new())); + let lossy = default_user(Some(std::ffi::OsString::from_vec(vec![b'a', 0xff]))) + .expect("a non-UTF-8 value is kept"); + assert_eq!(lossy, "a\u{fffd}"); + } + + /// **AGENTD-22.** `from_env` reads the default user from the daemon's environment. The + /// read happens in a copy of this test binary started with the variable set, because this + /// crate can't set its own environment: `set_var` is `unsafe`, which the crate forbids. + #[test] + fn from_env_reads_the_default_user_from_the_environment() { + const CHILD: &str = "AGENTD_TEST_FROM_ENV_CHILD"; + const NAME: &str = "config::tests::from_env_reads_the_default_user_from_the_environment"; + if std::env::var_os(CHILD).is_some() { + assert_eq!(Config::from_env().default_user.as_deref(), Some("probe")); + return; + } + let output = std::process::Command::new(std::env::current_exe().expect("test binary")) + .args(["--exact", NAME]) + .env(CHILD, "1") + .env(DEFAULT_USER_VAR, "probe") + .output() + .expect("child test"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(output.status.success(), "{stdout}"); + // A filter that matched nothing would pass too, having checked nothing. + assert!(stdout.contains("1 passed"), "{stdout}"); } } diff --git a/crates/agentd/src/exec.rs b/crates/agentd/src/exec.rs index e4007b7e..c6bd7f9f 100644 --- a/crates/agentd/src/exec.rs +++ b/crates/agentd/src/exec.rs @@ -1202,10 +1202,13 @@ fn build_command( let launch_env = state.launch_env(); let config = state.config(); // Read per request rather than cached: a workload that runs `useradd` before its next - // exec is asking for exactly that user, and the files are a few kilobytes. - let passwd = match req.user { - Some(_) => read_database(&config.passwd_path), - None => String::new(), + // exec is asking for exactly that user, and the files are a few kilobytes. The image's + // default user is read the same way, for the same reason (AGENTD-22). + let default_user = config.default_user.as_deref(); + let passwd = if req.user.is_some() || default_user.is_some() { + read_database(&config.passwd_path) + } else { + String::new() }; let group = match req.group { Some(protocol::exec::NameOrId::Name(_)) => read_database(&config.group_path), @@ -1218,6 +1221,7 @@ fn build_command( image_env: state.image_env(), passwd: &passwd, group: &group, + default_user, is_executable: &is_executable_file, }, )?; @@ -2946,6 +2950,15 @@ mod tests { /// `/home/tester`) and whose group file lists `crew` (this process's gid), holding /// `inherited` as the environment it inherited at startup, bootstrapped with `launch`. fn guest(inherited: &[(&str, &str)], launch: &[(&str, &str)]) -> (AppState, tempfile::TempDir) { + guest_with_default(inherited, launch, None) + } + + /// [`guest`] whose image sets `AGENTD_DEFAULT_USER` to `default_user`. + fn guest_with_default( + inherited: &[(&str, &str)], + launch: &[(&str, &str)], + default_user: Option<&str>, + ) -> (AppState, tempfile::TempDir) { let dir = tempfile::tempdir().expect("a guest directory"); let (uid, gid) = (own_id("-u"), own_id("-g")); std::fs::write( @@ -2966,6 +2979,7 @@ mod tests { let config = Config { passwd_path: dir.path().join("passwd"), group_path: dir.path().join("group"), + default_user: default_user.map(str::to_string), ..Config::default() }; let state = AppState::with_image_env(config, launch_env(inherited)); @@ -3075,6 +3089,102 @@ mod tests { assert_eq!(outcome.stdout.trim(), "/from-launch tester"); } + /// **AGENTD-22, AGENTD-24.** Through the real handler, a start that names no user runs as + /// the image's default user, with that row's identity. The default is resolved at each + /// request: while the guest has no such user the start is 400 `unknown_user` naming the + /// variable and nothing runs, and once a row for it appears the next start runs as it. + #[tokio::test] + async fn a_start_that_names_no_user_runs_as_the_default_resolved_per_request() { + let probe = "echo \"$(id -u) $HOME $USER\""; + let (state, _dir) = guest_with_default(&[], &[], Some("tester")); + let mut request = req("default-tester", &[probe]); + request.shell = true.into(); + let outcome = started(&state, request).await; + assert_eq!( + outcome.stdout.trim(), + format!("{} /home/tester tester", own_id("-u")) + ); + + let (state, dir) = guest_with_default(&[], &[], Some("latecomer")); + let marker = dir.path().join("ran"); + let mut request = req("default-missing", &[&format!("touch {}", marker.display())]); + request.shell = true.into(); + let detail = refused(&state, request, "unknown_user", &marker).await; + assert!( + detail.contains("\"latecomer\"") && detail.contains("AGENTD_DEFAULT_USER"), + "{detail}" + ); + + // What a `useradd` in an earlier exec leaves behind. + let mut passwd = std::fs::read_to_string(dir.path().join("passwd")).expect("passwd"); + passwd.push_str(&format!( + "latecomer:x:{}:{}::/home/latecomer:/bin/sh\n", + own_id("-u"), + own_id("-g") + )); + std::fs::write(dir.path().join("passwd"), passwd).expect("passwd"); + let mut request = req("default-late", &[probe]); + request.shell = true.into(); + let outcome = started(&state, request).await; + assert_eq!( + outcome.stdout.trim(), + format!("{} /home/latecomer latecomer", own_id("-u")) + ); + } + + /// **AGENTD-22, AGENTD-24.** Through the real handler, a default spelled as digits runs as + /// the row of that uid, with its identity, and digits no row carries are refused naming + /// the variable and the missing row, where a request's own digits would run. + #[tokio::test] + async fn a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused() { + let probe = "echo \"$(id -u) $(id -g) $HOME $USER\""; + let uid = own_id("-u").to_string(); + let (state, _dir) = guest_with_default(&[], &[], Some(&uid)); + let mut request = req("default-digits", &[probe]); + request.shell = true.into(); + let outcome = started(&state, request).await; + assert_eq!( + outcome.stdout.trim(), + format!("{} {} /home/tester tester", own_id("-u"), own_id("-g")) + ); + + let (state, dir) = guest_with_default(&[], &[], Some("4242424")); + let marker = dir.path().join("ran"); + let mut request = req( + "default-digits-no-row", + &[&format!("touch {}", marker.display())], + ); + request.shell = true.into(); + let detail = refused(&state, request, "unknown_user", &marker).await; + assert!( + detail.contains("\"4242424\"") + && detail.contains("AGENTD_DEFAULT_USER") + && detail.contains("needs a passwd row"), + "{detail}" + ); + } + + /// **AGENTD-23.** A user the start names wins over the default. The default here is an + /// account this non-root test cannot become (uid 65534), so a daemon that applied it + /// would fail the spawn; naming `tester` runs as `tester`, by name and by uid. + #[tokio::test] + async fn a_user_the_start_names_wins_over_the_default() { + let (state, dir) = guest_with_default(&[], &[], Some("elsewhere")); + let mut passwd = std::fs::read_to_string(dir.path().join("passwd")).expect("passwd"); + passwd.push_str("elsewhere:x:65534:65534::/nonexistent:/bin/sh\n"); + std::fs::write(dir.path().join("passwd"), passwd).expect("passwd"); + + let mut request = req("named-wins", &["/bin/sh", "-c", "echo $(id -u) $USER"]); + request.user = Some("tester".into()); + let outcome = started(&state, request).await; + assert_eq!(outcome.stdout.trim(), format!("{} tester", own_id("-u"))); + + let mut request = req("uid-wins", &["/bin/sh", "-c", "echo $(id -u) $USER"]); + request.user = Some(own_id("-u").into()); + let outcome = started(&state, request).await; + assert_eq!(outcome.stdout.trim(), format!("{} tester", own_id("-u"))); + } + /// **AGENTD-14.** `shell: "bash"` runs the script under bash: a pipefail pipeline fails /// where a POSIX `sh` that is dash would refuse the option outright, and `$0` names bash. #[tokio::test] diff --git a/crates/agentd/src/exec_start.rs b/crates/agentd/src/exec_start.rs index 4694e558..3ddcca02 100644 --- a/crates/agentd/src/exec_start.rs +++ b/crates/agentd/src/exec_start.rs @@ -19,6 +19,17 @@ //! fuzz harness can drive. Supplementary groups are not set: `Command::groups` is still //! unstable, and std clears them anyway when a root daemon demotes (`setgroups(0)`). //! +//! # A request that names no user +//! +//! It runs as the image's default user when the image sets `AGENTD_DEFAULT_USER`, and as the +//! daemon's own user (root) when it doesn't (AGENTD-22). The default is resolved here, per +//! request, to a passwd row: the row its name matches, or the row of the uid its digits +//! spell. Either way the child takes the row's uid, primary gid and identity, so a numeric +//! default doesn't keep the daemon's gid 0. A default with no row, digits included, is +//! refused as `unknown_user` with a detail that names the variable (AGENTD-24). A request's +//! own `user` always wins over the default, `root` and `0` included (AGENTD-23): the default +//! decides what a caller gets by not saying, not what a caller may ask for. +//! //! # The environment is five layers, lowest first //! //! 1. The image environment, only when the request sets `inherit_image_env`: what the @@ -52,6 +63,8 @@ use protocol::exec::{ NameOrId, Shell, StartRequest, }; +use crate::config::DEFAULT_USER_VAR; + /// A child's environment, or one layer of it. pub type Env = HashMap; @@ -159,6 +172,8 @@ pub struct Guest<'a> { pub passwd: &'a str, /// The group file's text. Empty when the file is absent. pub group: &'a str, + /// The image's default user (`Config::default_user`), for a request that names none. + pub default_user: Option<&'a str>, /// Whether a path is an executable regular file. The real check is /// [`is_executable_file`]; tests and the fuzz harness hand in a fake filesystem. pub is_executable: &'a dyn Fn(&Path) -> bool, @@ -201,8 +216,10 @@ impl Plan { struct User<'a> { uid: u32, row: Option<&'a PasswdRow>, - /// Given as a name that matched a row, as opposed to a number. - named: bool, + /// Takes the row's primary gid when the request names no group: a name the request + /// gave that matched a row, or the image's default user. A uid the request gave, as an + /// integer or as digits, keeps the daemon's group (AGENTD-16). + primary_gid: bool, } /// All ASCII digits that fit a `u32`: a string read as an id when it names no row. @@ -221,21 +238,21 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, NameOrId::Id(uid) => Ok(User { uid: *uid, row: by_uid(*uid), - named: false, + primary_gid: false, }), NameOrId::Name(name) => { if let Some(row) = rows.iter().find(|row| row.name == *name) { return Ok(User { uid: row.uid, row: Some(row), - named: true, + primary_gid: true, }); } match digits(name) { Some(uid) => Ok(User { uid, row: by_uid(uid), - named: false, + primary_gid: false, }), None => Err(Rejection::new( ERROR_UNKNOWN_USER, @@ -246,6 +263,56 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, } } +/// The image's default user as its passwd row (AGENTD-22): the row its name matches, or the +/// row of the uid its digits spell. +/// +/// Stricter than a request's `user`, where digits with no row run as a bare uid. The default +/// runs every start that names no user, and a uid with no row has no primary gid to give it, +/// so the child would keep the daemon's gid 0. A default with no row is refused instead +/// (AGENTD-24). The error is the clause a refusal's detail ends with, naming the value and +/// the variable that set it, because the caller sent no name to look for and the fix is in +/// the image. +fn default_user_row<'a>(name: &str, rows: &'a [PasswdRow]) -> Result<&'a PasswdRow, String> { + match resolve_user(&NameOrId::Name(name.to_string()), rows) { + Ok(User { row: Some(row), .. }) => Ok(row), + Ok(User { row: None, .. }) => Err(format!( + "the default user {name:?} that {DEFAULT_USER_VAR} sets is a uid with no row in \ + the guest's /etc/passwd (the image needs a passwd row for it)" + )), + Err(_) => Err(format!( + "the default user {name:?} that {DEFAULT_USER_VAR} sets is not in the guest's \ + /etc/passwd" + )), + } +} + +/// The user a request resolves to: its own `user` when it names one (AGENTD-23), else the +/// image's default user's row (AGENTD-22), else none, which is the daemon's own user. +/// +/// A default with no row is `unknown_user` like a request's unknown name (AGENTD-24), with +/// a detail that says the request named no user and which variable set the one that failed. +fn resolve_request_user<'a>( + request: &StartRequest, + default_user: Option<&str>, + rows: &'a [PasswdRow], +) -> Result>, Rejection> { + match (&request.user, default_user) { + (Some(spec), _) => resolve_user(spec, rows).map(Some), + (None, Some(name)) => match default_user_row(name, rows) { + Ok(row) => Ok(Some(User { + uid: row.uid, + row: Some(row), + primary_gid: true, + })), + Err(why) => Err(Rejection::new( + ERROR_UNKNOWN_USER, + format!("the request names no user, and {why}"), + )), + }, + (None, None) => Ok(None), + } +} + /// A group by gid or name (AGENTD-7); an unmatched name is `unknown_group` (AGENTD-8). fn resolve_group(spec: &NameOrId, rows: &[GroupRow]) -> Result { match spec { @@ -321,16 +388,12 @@ pub fn resolve_shell( /// environment's `PATH`), so the first field that cannot be resolved names the error. The /// empty-argv check comes last, as it did in `build_command` before this module existed. pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result { - let passwd = if request.user.is_some() { + let passwd = if request.user.is_some() || guest.default_user.is_some() { parse_passwd(guest.passwd) } else { Vec::new() }; - let user = request - .user - .as_ref() - .map(|spec| resolve_user(spec, &passwd)) - .transpose()?; + let user = resolve_request_user(request, guest.default_user, &passwd)?; let groups = match &request.group { Some(NameOrId::Name(_)) => parse_group(guest.group), @@ -338,11 +401,12 @@ pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result Some(resolve_group(spec, &groups)?), - // A named user takes its row's primary group, the way `su` and `docker -u name` do. - // A numeric uid keeps the daemon's group, which is what protocol 1 did (AGENTD-16). + // A named user takes its row's primary group, the way `su` and `docker -u name` do, + // and so does the default user, however the image spelled it (AGENTD-22). A uid the + // request gives keeps the daemon's group, which is what protocol 1 did (AGENTD-16). None => user .as_ref() - .filter(|user| user.named) + .filter(|user| user.primary_gid) .and_then(|user| user.row) .map(|row| row.gid), }; @@ -472,6 +536,24 @@ mod tests { request: &StartRequest, launch: &Env, image: Option<&Env>, + ) -> Result { + resolve_in(request, launch, image, None) + } + + /// [`resolve`] against a guest whose image sets `default_user`, with no launch or image + /// environment. + fn resolve_with_default( + request: &StartRequest, + default_user: Option<&str>, + ) -> Result { + resolve_in(request, &Env::new(), None, default_user) + } + + fn resolve_in( + request: &StartRequest, + launch: &Env, + image: Option<&Env>, + default_user: Option<&str>, ) -> Result { let exec = only(&["/usr/bin/bash", "/bin/sh", "/image/bin/zsh"]); plan( @@ -481,6 +563,7 @@ mod tests { image_env: image, passwd: PASSWD, group: GROUP, + default_user, is_executable: &exec, }, ) @@ -708,6 +791,159 @@ mod tests { ); } + /// **AGENTD-22.** A request that names no user resolves the image's default to a passwd + /// row: a name to its row, digits to the row of that uid, each with the row's uid, + /// primary gid and identity, and a named group still overrides the primary. With no + /// default it runs as the daemon, as it always did. + #[test] + fn a_request_that_names_no_user_resolves_the_default_as_its_user() { + let nameless = request(None, None, false.into()); + + let plan = resolve_with_default(&nameless, None).expect("resolves"); + assert_eq!( + (plan.uid, plan.gid), + (None, None), + "no default, the daemon's" + ); + assert!(plan.environment().is_empty(), "no default, no identity"); + + let plan = resolve_with_default(&nameless, Some("agent")).expect("resolves"); + assert_eq!((plan.uid, plan.gid), (Some(1000), Some(1001))); + assert_eq!( + plan.environment(), + env(&[ + ("HOME", "/home/agent"), + ("USER", "agent"), + ("LOGNAME", "agent") + ]) + ); + assert_eq!( + plan, + resolve_with_default(&request(Some("agent".into()), None, false.into()), None) + .expect("resolves"), + "the default resolves exactly as the same name in the request" + ); + + // Digits are the row of that uid, with its primary gid: a request's "1000" keeps the + // daemon's group, and a default's doesn't, or the child would keep gid 0. + let plan = resolve_with_default(&nameless, Some("1000")).expect("resolves"); + assert_eq!( + plan, + resolve_with_default(&nameless, Some("agent")).expect("resolves"), + "a uid the default spells resolves as its row's name" + ); + let plan = resolve_with_default(&nameless, Some("0")).expect("resolves"); + assert_eq!((plan.uid, plan.gid), (Some(0), Some(0))); + assert_eq!(plan.environment()["HOME"], "/root"); + + let grouped = request(None, Some("staff".into()), false.into()); + let plan = resolve_with_default(&grouped, Some("agent")).expect("resolves"); + assert_eq!((plan.uid, plan.gid), (Some(1000), Some(50))); + let plan = resolve_with_default(&grouped, Some("1002")).expect("resolves"); + assert_eq!((plan.uid, plan.gid), (Some(1002), Some(50))); + } + + /// **AGENTD-23.** A user the request names wins over the default, `root` and `0` + /// included, and an unknown one is refused for its own name rather than replaced by the + /// default; a default that names nobody doesn't stop a request that names someone. + #[test] + fn a_user_the_request_names_wins_over_the_default() { + let root = resolve_with_default( + &request(Some("root".into()), None, false.into()), + Some("agent"), + ) + .expect("resolves"); + assert_eq!((root.uid, root.gid), (Some(0), Some(0))); + assert_eq!(root.environment()["HOME"], "/root"); + + let zero = + resolve_with_default(&request(Some(0.into()), None, false.into()), Some("agent")) + .expect("resolves"); + assert_eq!( + (zero.uid, zero.gid), + (Some(0), None), + "an integer keeps the group" + ); + + let builder = resolve_with_default( + &request(Some("builder".into()), None, false.into()), + Some("ghost"), + ) + .expect("an unknown default is never resolved for a request that names a user"); + assert_eq!((builder.uid, builder.gid), (Some(1002), Some(1002))); + + let refused = resolve_with_default( + &request(Some("ghost".into()), None, false.into()), + Some("agent"), + ) + .expect_err("an unknown user the request names"); + assert_eq!(refused.error, ERROR_UNKNOWN_USER); + assert!(refused.detail.contains("\"ghost\""), "{}", refused.detail); + assert!( + !refused.detail.contains(DEFAULT_USER_VAR), + "the caller named this user, so the detail is about it: {}", + refused.detail + ); + } + + /// **AGENTD-24.** A default with no passwd row, empty and all-digit included, refuses a + /// request that names no user as `unknown_user`, with a detail naming the value and the + /// variable, and it is resolved first, ahead of the group and the shell. Digits with no + /// row say the image needs one, since a request's same digits would run. + #[test] + fn a_default_that_names_nobody_is_refused_naming_the_variable() { + for default in ["ghost", "", "12a", "4242"] { + let refused = resolve_with_default(&request(None, None, false.into()), Some(default)) + .expect_err(default); + assert_eq!(refused.error, ERROR_UNKNOWN_USER, "{default:?}"); + assert!( + refused.detail.contains(&format!("{default:?}")) + && refused.detail.contains(DEFAULT_USER_VAR), + "{}", + refused.detail + ); + assert_eq!( + refused.detail.contains("needs a passwd row"), + default == "4242", + "{}", + refused.detail + ); + } + assert_eq!( + resolve_with_default( + &request(Some("4242".into()), None, false.into()), + Some("4242") + ) + .expect("a request's own digits run with no row") + .uid, + Some(4242) + ); + let all_bad = request(None, Some("ghosts".into()), "fish".into()); + assert_eq!( + resolve_with_default(&all_bad, Some("ghost")) + .unwrap_err() + .error, + ERROR_UNKNOWN_USER + ); + } + + /// **AGENTD-22, AGENTD-24.** The default's row is the one a name matches before the one a + /// uid matches, so a user named with digits is that user, and a miss is the clause a + /// refusal ends with. + #[test] + fn the_default_user_row_prefers_a_name_to_a_uid() { + let rows = + parse_passwd("1000:x:1500:1500::/home/digits:/bin/sh\nagent:x:1000:1001::/a:/bin/sh\n"); + assert_eq!(default_user_row("1000", &rows).map(|row| row.uid), Ok(1500)); + assert_eq!(default_user_row("1500", &rows).map(|row| row.gid), Ok(1500)); + assert_eq!( + default_user_row("agent", &rows).map(|row| row.gid), + Ok(1001) + ); + let missing = default_user_row("77", &rows).expect_err("no row"); + assert!(missing.starts_with("the default user \"77\""), "{missing}"); + } + /// **AGENTD-12.** The snapshot drops every `AGENTD_` variable and anything not UTF-8. #[test] fn the_snapshot_drops_agentd_configuration_and_non_utf8() { diff --git a/crates/agentd/src/exec_start_fuzz.rs b/crates/agentd/src/exec_start_fuzz.rs index dc81e096..eb3c1e89 100644 --- a/crates/agentd/src/exec_start_fuzz.rs +++ b/crates/agentd/src/exec_start_fuzz.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 -//! The fuzz harness for AGENTD-7 through AGENTD-16: start requests whose `user`, `group` and -//! `shell` are every JSON spelling the union types admit, deserialized and resolved against -//! a fuzzed guest. +//! The fuzz harness for AGENTD-7 through AGENTD-16 and AGENTD-22 through AGENTD-24: start +//! requests whose `user`, `group` and `shell` are every JSON spelling the union types admit, +//! deserialized and resolved against a fuzzed guest whose image may set a default user. //! //! `bolero::check!` runs this as an ordinary `#[test]` under stable `cargo test`, and as a //! coverage-guided target under @@ -12,8 +12,8 @@ //! //! A start body built as JSON (so the union deserialization is exercised, not bypassed), //! a passwd file of fuzzed rows, a launch environment, the environment the daemon -//! "inherited" (which may carry `AGENTD_*` keys and a token-shaped value), and a fake -//! filesystem saying which shells exist. Names, keys and values come from small pools so +//! "inherited" (which may carry `AGENTD_*` keys and a token-shaped value), the image's +//! default user if it sets one, and a fake filesystem saying which shells exist. Names, keys and values come from small pools so //! the interesting collisions — a name that matches a row, digits, a key set in two //! layers — happen on almost every case rather than almost never. //! @@ -25,7 +25,11 @@ //! layers (AGENTD-9); the image layer is absent without the flag (AGENTD-10) and lowest with it //! (AGENTD-11); no `AGENTD_*` image key and no token reaches the plan (AGENTD-12); the snapshot's //! count is its non-`AGENTD_` keys (AGENTD-13); a named shell runs as the file it resolved to -//! (AGENTD-14); integers and booleans mean what they did (AGENTD-16). A second check feeds raw +//! (AGENTD-14); integers and booleans mean what they did (AGENTD-16); a request that names no +//! user runs as the default's passwd row, found by name or by the uid its digits spell, with +//! that row's primary gid (AGENTD-22), a request's own user is never replaced by it +//! (AGENTD-23), and a default with no row is refused naming the variable (AGENTD-24). A +//! second check feeds raw //! bytes to the `StartRequest` deserializer, which must refuse or accept but never panic. use std::path::Path; @@ -37,6 +41,7 @@ use protocol::exec::{ }; use serde_json::{Value, json}; +use crate::config::DEFAULT_USER_VAR; use crate::exec_start::{Env, Guest, Program, parse_passwd, plan, snapshot_image_env}; const NAMES: [&str; 8] = [ @@ -109,6 +114,8 @@ struct Case { inherited: Vec<(u8, u8)>, export_token: bool, rows: Vec, + /// The image's `AGENTD_DEFAULT_USER`, a [`NAMES`] index, when it sets one. + default_user: Option, } fn env_of(pairs: &[(u8, u8)]) -> Env { @@ -176,8 +183,9 @@ fn digits(raw: &str) -> Option { } /// **AGENTD-7, AGENTD-8, AGENTD-9, AGENTD-10, AGENTD-11, AGENTD-12, AGENTD-13, AGENTD-14, -/// AGENTD-15 and AGENTD-16.** A fuzzed start request resolved against a fuzzed guest keeps -/// each of the model's properties; the module docs say which assertion is which. +/// AGENTD-15, AGENTD-16, AGENTD-22, AGENTD-23 and AGENTD-24.** A fuzzed start request resolved +/// against a fuzzed guest keeps each of the model's properties; the module docs say which +/// assertion is which. #[test] fn start_resolution() { bolero::check!().with_type::().for_each(|case| { @@ -190,7 +198,10 @@ fn start_resolution() { let written = serde_json::to_value(&request).expect("serializes"); for field in ["user", "group", "shell"] { if let Some(sent) = body.get(field) { - assert_eq!(&written[field], sent, "{field} changed shape on the way back"); + assert_eq!( + &written[field], sent, + "{field} changed shape on the way back" + ); } } @@ -208,7 +219,10 @@ fn start_resolution() { assert!(snapshot.keys().all(|key| !key.starts_with("AGENTD_"))); assert_eq!( snapshot.len(), - inherited.keys().filter(|key| !key.starts_with("AGENTD_")).count() + inherited + .keys() + .filter(|key| !key.starts_with("AGENTD_")) + .count() ); let passwd = passwd_text(&case.rows); @@ -216,6 +230,7 @@ fn start_resolution() { let group = "staff:x:50:\nagent:x:1001:\n"; let launch = env_of(&case.launch_env); let exists = |path: &Path| PRESENT.iter().any(|present| Path::new(present) == path); + let default_user = case.default_user.map(|index| pick(&NAMES, index)); let result = plan( &request, &Guest { @@ -223,15 +238,23 @@ fn start_resolution() { image_env: Some(&snapshot), passwd: &passwd, group, + default_user, is_executable: &exists, }, ); - // What the resolution must decide, derived independently of it. + // What the resolution must decide, derived independently of it. The user is the + // request's own when it names one, whatever the default (AGENTD-23), and otherwise the + // default's row: the one its name matches, else the one with the uid its digits spell + // (AGENTD-22). A default with neither is refused (AGENTD-24). let user_row = |name: &str| rows.iter().find(|row| row.name == name); - let user_ok = match &request.user { - Some(NameOrId::Name(name)) => user_row(name).is_some() || digits(name).is_some(), - _ => true, + let uid_row = |uid: u32| rows.iter().find(|row| row.uid == uid); + let default_row = + default_user.map(|name| user_row(name).or_else(|| digits(name).and_then(uid_row))); + let user_ok = match (&request.user, default_row) { + (Some(NameOrId::Name(name)), _) => user_row(name).is_some() || digits(name).is_some(), + (Some(NameOrId::Id(_)), _) | (None, None) => true, + (None, Some(row)) => row.is_some(), }; let group_ok = match &request.group { Some(NameOrId::Name(name)) => { @@ -253,6 +276,24 @@ fn start_resolution() { ERROR_UNKNOWN_SHELL }; assert_eq!(rejection.error, expected, "{rejection:?} for {body}"); + // AGENTD-24: a refused default names itself and the variable, and says the + // image needs a row when its digits are a uid; a refused name the request + // gave is about that name alone. + if expected == ERROR_UNKNOWN_USER { + match (&request.user, default_user) { + (None, Some(name)) => assert!( + rejection.detail.contains(&format!("{name:?}")) + && rejection.detail.contains(DEFAULT_USER_VAR) + && (digits(name).is_none() + || rejection.detail.contains("needs a passwd row")), + "{rejection:?}" + ), + _ => assert!( + !rejection.detail.contains(DEFAULT_USER_VAR), + "{rejection:?}" + ), + } + } if let Shell::Named(name) = &request.shell && expected == ERROR_UNKNOWN_SHELL { @@ -260,17 +301,21 @@ fn start_resolution() { } } Ok(plan) => { - assert!(user_ok && group_ok, "an unresolvable name was planned: {body}"); + assert!( + user_ok && group_ok, + "an unresolvable name was planned: {body}" + ); let env = plan.environment(); - // AGENTD-7 and AGENTD-16: the uid is the row's for a matching name, the number - // otherwise; the gid is the group's, a named user's primary, or unchanged. + // AGENTD-7, AGENTD-16 and AGENTD-22: the uid is the row's for a matching name + // or a default, the number otherwise; the gid is the group's, a named user's or + // the default's primary, or unchanged. let row = match &request.user { - Some(NameOrId::Name(name)) => user_row(name).or_else(|| { - digits(name).and_then(|uid| rows.iter().find(|row| row.uid == uid)) - }), - Some(NameOrId::Id(uid)) => rows.iter().find(|row| row.uid == *uid), - None => None, + Some(NameOrId::Name(name)) => { + user_row(name).or_else(|| digits(name).and_then(uid_row)) + } + Some(NameOrId::Id(uid)) => uid_row(*uid), + None => default_row.flatten(), }; match &request.user { Some(NameOrId::Name(name)) => match user_row(name) { @@ -278,11 +323,19 @@ fn start_resolution() { None => assert_eq!(plan.uid, digits(name)), }, Some(NameOrId::Id(uid)) => assert_eq!(plan.uid, Some(*uid)), - None => assert_eq!(plan.uid, None), + None => assert_eq!(plan.uid, row.map(|row| row.uid), "{body}"), } if request.group.is_none() { - let named = matches!(&request.user, Some(NameOrId::Name(name)) if user_row(name).is_some()); - let expected = if named { row.map(|row| row.gid) } else { None }; + let primary = match &request.user { + Some(NameOrId::Name(name)) => user_row(name).is_some(), + Some(NameOrId::Id(_)) => false, + None => default_user.is_some(), + }; + let expected = if primary { + row.map(|row| row.gid) + } else { + None + }; assert_eq!(plan.gid, expected, "{body}"); } diff --git a/crates/agentd/src/hook_handlers.rs b/crates/agentd/src/hook_handlers.rs index 3a6fbbdd..1549ac9f 100644 --- a/crates/agentd/src/hook_handlers.rs +++ b/crates/agentd/src/hook_handlers.rs @@ -24,7 +24,9 @@ //! //! A handler is image-owned code and runs as the daemon's user (root), with the //! daemon's environment plus the launch environment and `AGENTD_HOOK=`. The -//! agent token is never in either. The hook routes are unauthenticated and reachable +//! agent token is never in either. `AGENTD_DEFAULT_USER` doesn't apply: it names who a +//! start request runs as when its caller names nobody, and a handler has no caller, only +//! the image that put it there. The hook routes are unauthenticated and reachable //! over loopback from inside the guest, so any process in the VM can make a handler //! run: handlers must be safe to run spuriously. The hook log's cap bounds how many //! times that can happen. @@ -339,6 +341,39 @@ mod tests { ); } + /// A handler runs as the daemon's own user even when the image sets a default user for + /// start requests. The default here is an account this test can't become (uid 65534), + /// so a handler demoted to it would fail to spawn rather than print this process's uid. + #[tokio::test] + async fn a_handler_runs_as_the_daemon_whatever_default_user_the_image_sets() { + let dir = tempfile::tempdir().expect("tempdir"); + let passwd = dir.path().join("passwd"); + std::fs::write(&passwd, "elsewhere:x:65534:65534::/nonexistent:/bin/sh\n").expect("passwd"); + let marker = dir.path().join("uid"); + script(dir.path(), "run", &format!("id -u > {}", marker.display())); + let state = AppState::new(Config { + hooks_dir: dir.path().to_path_buf(), + hook_handler_timeout: Duration::from_secs(5), + passwd_path: passwd, + default_user: Some("elsewhere".into()), + ..Config::default() + }); + let slot = state.record_hook("run"); + run(&state, "run", slot).await; + + let (hooks, _) = state.hook_report(); + let outcome = hooks[0].handler.as_ref().expect("an outcome"); + assert!(outcome.succeeded(), "{outcome:?}"); + let own = std::process::Command::new("/usr/bin/id") + .arg("-u") + .output() + .expect("id -u"); + assert_eq!( + std::fs::read_to_string(&marker).expect("ran"), + String::from_utf8_lossy(&own.stdout) + ); + } + /// Past the hook log's cap nothing is recorded, so nothing runs: the cap is also /// the bound on how often an in-guest caller can trigger a handler. #[tokio::test] diff --git a/crates/agentd/tests/bdd_exec_start.rs b/crates/agentd/tests/bdd_exec_start.rs index caec529a..be06096b 100644 --- a/crates/agentd/tests/bdd_exec_start.rs +++ b/crates/agentd/tests/bdd_exec_start.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 -//! The Gherkin behavior spec for AGENTD-7 through AGENTD-16, run against the daemon's real -//! router in-process. +//! The Gherkin behavior spec for AGENTD-7 through AGENTD-16 and AGENTD-22 through AGENTD-24, +//! run against the daemon's real router in-process. //! //! The scenarios live in `tests/features/exec_start.feature`, tagged with the requirement each //! one verifies; this file is their step definitions and runner. It is a `harness = false` @@ -46,6 +46,8 @@ struct Daemon { dir: tempfile::TempDir, /// What the daemon inherited, before it filtered anything. inherited: HashMap, + /// The launch environment the run hook delivers. + launch: HashMap, state: Option, token: String, /// The start body being composed. @@ -79,6 +81,7 @@ impl Daemon { Self { dir: tempfile::tempdir().expect("a scenario directory"), inherited: HashMap::new(), + launch: HashMap::new(), state: None, token: String::new(), request: json!({"exec_id": EXEC_ID}), @@ -121,6 +124,32 @@ impl Daemon { fn authorized(&self, builder: axum::http::request::Builder) -> axum::http::request::Builder { builder.header("authorization", format!("Bearer {}", self.token)) } + + /// Starts a daemon over the scenario's databases, inherited environment and + /// `default_user`, and bootstraps it through the run-hook route with the scenario's token + /// and launch environment. + async fn boot(&mut self, default_user: Option) { + let config = Config { + passwd_path: self.passwd(), + group_path: self.group(), + default_user, + ..Config::default() + }; + self.state = Some(AppState::with_image_env(config, self.inherited.clone())); + let body = json!({ + "runHookPayload": serde_json::to_string(&json!({ + "agent_token": self.token, + "env": self.launch, + })) + .expect("serializes"), + }); + let request = Request::post(format!("{}/run", routes::HOOK_PREFIX)) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .expect("a request"); + let (status, _) = self.call(request).await; + assert_eq!(status, StatusCode::OK, "the run hook bootstraps the daemon"); + } } /// `id -u` or `id -g` of this test process, as the child would print it. @@ -173,26 +202,46 @@ async fn inherited(world: &mut Daemon, step: &Step) { #[given(expr = "the run hook installed the token {string} with the launch environment:")] async fn run_hook(world: &mut Daemon, token: String, step: &Step) { - let config = Config { - passwd_path: world.passwd(), - group_path: world.group(), - ..Config::default() - }; - world.state = Some(AppState::with_image_env(config, world.inherited.clone())); - world.token = token.clone(); - let body = json!({ - "runHookPayload": serde_json::to_string(&json!({ - "agent_token": token, - "env": table(step), - })) - .expect("serializes"), - }); - let request = Request::post(format!("{}/run", routes::HOOK_PREFIX)) - .header("content-type", "application/json") - .body(Body::from(body.to_string())) - .expect("a request"); - let (status, _) = world.call(request).await; - assert_eq!(status, StatusCode::OK, "the run hook bootstraps the daemon"); + world.token = token; + world.launch = table(step); + world.boot(None).await; +} + +/// The daemon reads its configuration once, at startup, so this starts the scenario's daemon +/// again with the default set, the way an image's `ENV` line would have started it. +#[given(expr = "the image sets AGENTD_DEFAULT_USER to {string}")] +async fn default_user(world: &mut Daemon, name: String) { + world.boot(Some(name)).await; +} + +/// [`default_user`] with the default spelled as this process's uid, which the background's +/// "tester" row carries. +#[given("the image sets AGENTD_DEFAULT_USER to this process's uid")] +async fn default_user_uid(world: &mut Daemon) { + world.boot(Some(own("-u"))).await; +} + +#[given( + expr = "the guest's passwd also lists {string} with this process's uid and gid and home {string}" +)] +async fn passwd_append_self(world: &mut Daemon, name: String, home: String) { + let line = format!("{name}:x:{}:{}::{home}:/bin/sh\n", own("-u"), own("-g")); + append_passwd(world, &line); +} + +#[given(expr = "the guest's passwd also lists {string} with uid and gid {int}")] +async fn passwd_append_id(world: &mut Daemon, name: String, id: u32) { + append_passwd( + world, + &format!("{name}:x:{id}:{id}::/nonexistent:/bin/sh\n"), + ); +} + +/// What a `useradd` in an earlier exec leaves behind. +fn append_passwd(world: &Daemon, line: &str) { + let mut text = std::fs::read_to_string(world.passwd()).expect("reads passwd"); + text.push_str(line); + std::fs::write(world.passwd(), text).expect("writes passwd"); } #[given(expr = "a start of {string} under shell {string}")] diff --git a/crates/agentd/tests/features/exec_start.feature b/crates/agentd/tests/features/exec_start.feature index 2a0f2772..c7127529 100644 --- a/crates/agentd/tests/features/exec_start.feature +++ b/crates/agentd/tests/features/exec_start.feature @@ -1,7 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 # -# The behavior spec for AGENTD-7 through AGENTD-16 in verify/spec/agentd.symspec.json (issues #224, -# #225, #226). `crates/model/src/exec_start.rs` checks the same requirements as a Stateright model, +# The behavior spec for AGENTD-7 through AGENTD-16 and AGENTD-22 through AGENTD-24 in +# verify/spec/agentd.symspec.json (issues #224, #225, #226, #595). `crates/model/src/exec_start.rs` checks the same requirements as a Stateright model, # and the bolero harness in `crates/agentd/src/exec_start_fuzz.rs` fuzzes the resolution against # them. Each scenario's tags name the requirements it verifies. # @@ -197,3 +197,67 @@ Feature: A start request names its user, group and shell, and may inherit the im And the child's environment is exactly: | FROM_LAUNCH | launch | | LOGNAME | launch-logname | + + Rule: AGENTD-22 — a start that names no user runs as the image's default user + + @AGENTD-22 @AGENTD-9 + Scenario: a start that names no user runs as the default user's passwd row + Given the image sets AGENTD_DEFAULT_USER to "tester" + And a start of "id -u; id -g; echo $HOME $USER" under shell "true" + When the daemon answers the start + Then the start was accepted and the child exited 0 + And output line 1 is this process's uid + And output line 2 is this process's gid + And output line 3 is "/home/tester tester" + + @AGENTD-22 @AGENTD-9 + Scenario: an all-digit default runs as the passwd row of that uid + Given the image sets AGENTD_DEFAULT_USER to this process's uid + And a start of "id -u; id -g; echo $HOME $USER" under shell "true" + When the daemon answers the start + Then the start was accepted and the child exited 0 + And output line 1 is this process's uid + And output line 2 is this process's gid + And output line 3 is "/home/tester tester" + + @AGENTD-22 + Scenario: the default is resolved at each start, so a user added after startup resolves + Given the image sets AGENTD_DEFAULT_USER to "latecomer" + And the guest's passwd also lists "latecomer" with this process's uid and gid and home "/home/latecomer" + And a start of "echo $HOME $USER" under shell "true" + When the daemon answers the start + Then the start was accepted and the child exited 0 + And output line 1 is "/home/latecomer latecomer" + + Rule: AGENTD-23 — a user the start names wins over the default + + @AGENTD-23 + Scenario: a start that names its user runs as that user, not the default + Given the guest's passwd also lists "elsewhere" with uid and gid 65534 + And the image sets AGENTD_DEFAULT_USER to "elsewhere" + And a start of "id -u; echo $USER" under shell "true" + And the user named "tester" + When the daemon answers the start + Then the start was accepted and the child exited 0 + And output line 1 is this process's uid + And output line 2 is "tester" + + Rule: AGENTD-24 — a default the guest does not have is a 400 naming the setting, and nothing is spawned + + @AGENTD-24 + Scenario: a default user the guest does not have refuses a start that names no user + Given the image sets AGENTD_DEFAULT_USER to "nobody-here" + And a start that would leave a marker file, under shell "true" + When the daemon answers the start + Then the answer is 400 with error "unknown_user" naming "AGENTD_DEFAULT_USER" + And the answer is 400 with error "unknown_user" naming "nobody-here" + And no child was spawned + + @AGENTD-24 + Scenario: an all-digit default with no passwd row refuses a start that names no user + Given the image sets AGENTD_DEFAULT_USER to "4242424" + And a start that would leave a marker file, under shell "true" + When the daemon answers the start + Then the answer is 400 with error "unknown_user" naming "AGENTD_DEFAULT_USER" + And the answer is 400 with error "unknown_user" naming "needs a passwd row" + And no child was spawned diff --git a/crates/microvms-app/src/agents/mod.rs b/crates/microvms-app/src/agents/mod.rs index ab0bd713..333dfae5 100644 --- a/crates/microvms-app/src/agents/mod.rs +++ b/crates/microvms-app/src/agents/mod.rs @@ -492,7 +492,9 @@ pub fn provisioning_files( /// Places the provisioning files and hands `/workspace` to the agent uid (AGENT-5). /// /// The daemon writes every upload as root, so the `chown` is not optional and it runs -/// as the daemon's own user, after the uploads, in one exec. Re-runnable: a second call +/// as root, after the uploads, in one exec. Root is asked for as uid 0 rather than by naming +/// no user, because an image that sets `AGENTD_DEFAULT_USER` runs a start that names no user +/// as that user, and the `chown` would fail as anyone else. Re-runnable: a second call /// with a fresh token overwrites the same files, which is how a 12-hour token is /// refreshed on a long-lived VM (AGENT-8). pub async fn install_access( @@ -510,6 +512,7 @@ pub async fn install_access( vec![format!("chown -R {AGENT_UID}:{AGENT_GID} {WORKDIR}")], ) .with_shell(true) + .with_user(Some(crate::session::ROOT_UID.into())) .with_timeout_sec(Some(CHOWN_TIMEOUT.as_secs_f64())); let result = session.run_sync(request, CHOWN_TIMEOUT).await?; if !result.succeeded() { @@ -1142,8 +1145,8 @@ mod tests { // ── install and read-back through the recorder ─────────────────────────── /// **The uploads go out in file order, then exactly one root chown.** The exec that - /// hands the workspace over runs as the daemon's own user (no `user` on the wire), - /// after every PUT. + /// hands the workspace over asks for uid 0 on the wire, after every PUT: naming no user + /// would run it as the image's default user when the image sets one. #[tokio::test(start_paused = true)] async fn install_access_uploads_then_chowns_as_root() { let recorder = Recorder::with([ @@ -1181,10 +1184,7 @@ mod tests { let start: serde_json::Value = serde_json::from_slice(&seen[3].body).expect("json"); assert_eq!(start["command"][0], "chown -R 1000:1000 /workspace"); assert_eq!(start["shell"], true); - assert!( - start["user"].is_null(), - "the chown runs as the daemon's own user" - ); + assert_eq!(start["user"], 0, "the chown asks for root by uid"); assert_eq!(seen[5].path, "/v1/exec/c1/ack"); } diff --git a/crates/microvms-app/src/session/mod.rs b/crates/microvms-app/src/session/mod.rs index 16a4ab80..e83657ca 100644 --- a/crates/microvms-app/src/session/mod.rs +++ b/crates/microvms-app/src/session/mod.rs @@ -79,6 +79,13 @@ pub const DEFAULT_READY_TIMEOUT: Duration = DEFAULT_BOOTSTRAP_TIMEOUT; /// How often to re-poll health while waiting for bootstrap. const READY_POLL_INTERVAL: Duration = Duration::from_secs(2); +/// The uid this crate's own execs that need root ask for by number. +/// +/// Naming no user isn't asking for root: a daemon whose image sets `AGENTD_DEFAULT_USER` +/// runs such a start as that user. A user the request names always wins, and an integer is +/// what every daemon release accepts. +pub(crate) const ROOT_UID: u32 = 0; + /// The transport: a backend, the agent token, and the proxy auth every request needs. /// /// Separate from [`Session`] because [`ExecHandle`] needs it and holding a whole session diff --git a/crates/microvms-app/src/workspace.rs b/crates/microvms-app/src/workspace.rs index 2dbf3d00..11db62c7 100644 --- a/crates/microvms-app/src/workspace.rs +++ b/crates/microvms-app/src/workspace.rs @@ -367,8 +367,11 @@ impl Session { .map(String::from) .chain(doomed) .collect(); + // As root, by uid: the daemon wrote the tree as root, and an image's default user + // would be refused deleting it. let request = protocol::exec::StartRequest::new(self.mint_exec_id(), command) .with_cwd(Some(REMOTE_WORKDIR.into())) + .with_user(Some(crate::session::ROOT_UID.into())) .with_timeout_sec(Some(delete_timeout.as_secs_f64())); let result = self.run_sync(request, delete_wait(delete_timeout)).await?; match result.outcome { @@ -611,6 +614,10 @@ mod tests { ); assert_eq!(body["cwd"], "/workspace"); assert_eq!(body["timeout_sec"], 60.0); + assert_eq!( + body["user"], 0, + "the rm asks for root by uid, which an image's default user doesn't replace" + ); assert_eq!((pass.deleted, pass.refused_deletions), (1, 1)); } diff --git a/crates/microvms-cli/src/cli.rs b/crates/microvms-cli/src/cli.rs index f3a820e7..7a4dc66e 100644 --- a/crates/microvms-cli/src/cli.rs +++ b/crates/microvms-cli/src/cli.rs @@ -978,7 +978,7 @@ pub struct RunArgs { pub launch_env: Vec<(String, String)>, /// User to run --exec's command as: a name or a numeric uid. Omitted runs as the - /// daemon's own user. + /// image's AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. /// /// Resolved by the daemon against the guest's `/etc/passwd`, the only place the answer /// exists; see `exec --user`. @@ -1353,8 +1353,8 @@ pub struct ExecArgs { #[arg(long, value_name = "KEY=VALUE", value_parser = parse_env_pair)] pub env: Vec<(String, String)>, - /// User to run the command as: a name or a numeric uid. Omitted runs as the daemon's own - /// user. + /// User to run the command as: a name or a numeric uid. Omitted runs as the image's + /// AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. /// /// All digits is sent as an integer uid, anything else as a name (AGENTD-7, AGENTD-16). /// The daemon resolves a name against the guest's `/etc/passwd` before it spawns anything and answers diff --git a/crates/microvms-cli/src/commands/lifecycle.rs b/crates/microvms-cli/src/commands/lifecycle.rs index ef6a544b..bf71c6ac 100644 --- a/crates/microvms-cli/src/commands/lifecycle.rs +++ b/crates/microvms-cli/src/commands/lifecycle.rs @@ -1898,7 +1898,8 @@ pub struct StartSpec<'a> { /// agent token must never leak into a child — so this map is not merged into anything: /// what is here is everything the child sees. pub env: std::collections::HashMap, - /// The user to demote to, by name or uid. `None` runs as the daemon's own user. + /// The user to demote to, by name or uid. `None` runs as the image's default user, or as + /// the daemon's own user when the image sets none. pub user: Option, /// The group to demote to, by name or gid. `None` keeps the daemon's own group, or a /// named user's primary group. @@ -1974,7 +1975,7 @@ pub fn start_request(spec: StartSpec<'_>) -> microvms_core::protocol::exec::Star // accounts are unknowable from *any* client, and the daemon resolves a name against // the guest's `/etc/passwd` and `/etc/group` before it spawns, answering // `unknown_user`/`unknown_group` for one it does not have. Omission stays the default, - // which is "run as the daemon's own user". + // which is "run as the image's `AGENTD_DEFAULT_USER`, else as the daemon's own user". .with_user(spec.user) .with_group(spec.group) // No `with_timeout_sec`: the client-side deadline is the caller's `--timeout`, applied diff --git a/crates/microvms-core/README.md b/crates/microvms-core/README.md index 8c361e69..129c3a25 100644 --- a/crates/microvms-core/README.md +++ b/crates/microvms-core/README.md @@ -111,8 +111,9 @@ transfers; `Session::run` returns an `ExecHandle` for streaming and cancellation For built-in coding agents, start with [running Claude Code or Codex](https://laithalsaadoon.github.io/microvms-agentd/learn/operations/run-coding-agents-on-bedrock/). Rust's `agents::AgentVm` composes image preparation, launch, Bedrock credential -installation, and prompts. Agent prompts run as uid/gid 1000; the generic command -example above runs as the daemon's user, normally root. +installation, and prompts. Agent prompts run as uid/gid 1000. The generic command +example above runs as the image's `AGENTD_DEFAULT_USER` when the image sets one, +and as the daemon's user, normally root, when it doesn't. The VM separates workload execution from your host. Processes within one guest share that VM; uid demotion is not a separate security boundary. Guest workloads diff --git a/crates/model/src/exec_start.rs b/crates/model/src/exec_start.rs index 788f26a5..da392509 100644 --- a/crates/model/src/exec_start.rs +++ b/crates/model/src/exec_start.rs @@ -5,7 +5,8 @@ //! [`crate::client`] and the output model in [`crate::output`]. It specifies AGENTD-7 through //! AGENTD-16 in `verify/spec/agentd.symspec.json`: the three protocol additions of issues #224, #225 //! and #226, which let a start request name its user and group, inherit the image's `ENV`, -//! and name its shell. +//! and name its shell. It also specifies AGENTD-22 through AGENTD-24, issue #595's default +//! user: the user an image's `AGENTD_DEFAULT_USER` names for a request that names none. //! //! # What the model settles //! @@ -34,6 +35,9 @@ //! The guest's databases are reduced to the cases that behave differently: a user given as //! an integer with and without a passwd row, a name that resolves, a name that does not, and //! an all-digit string with no row (read as a uid, the way `docker exec -u 1000` reads it). +//! The image's default user is always a string, and resolves only to a passwd row: a name +//! with or without one, or digits with or without a row of that uid, or unset. The daemon +//! reads it once at startup, so the model fixes it at [`Action::Boot`]. //! //! # Every always-property has a sometimes-property beside it //! @@ -151,7 +155,8 @@ impl Env { /// The request's `user`, reduced to the cases that behave differently. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum UserReq { - /// Omitted: the child runs as the daemon's own user. + /// Omitted: the child runs as the image's default user, or as the daemon's own user when + /// the image sets none. Absent, /// An integer uid with a passwd row. IdWithRow, @@ -165,6 +170,62 @@ pub enum UserReq { DigitsNoRow, } +/// The image's `AGENTD_DEFAULT_USER`: unset, or a string, reduced to the cases that behave +/// differently. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum DefaultUser { + /// Not set: a request that names no user runs as the daemon's own user. + Unset, + /// A name with a passwd row. + NameKnown, + /// A name with no row, not all digits. + NameUnknown, + /// All digits, the uid of a passwd row. + DigitsWithRow, + /// All digits, the uid of no row. + DigitsNoRow, +} + +impl DefaultUser { + /// Every default the model boots with. + pub const ALL: [DefaultUser; 5] = [ + DefaultUser::Unset, + DefaultUser::NameKnown, + DefaultUser::NameUnknown, + DefaultUser::DigitsWithRow, + DefaultUser::DigitsNoRow, + ]; + + /// Whether the specification resolves this default to a passwd row, by its name or by + /// the uid its digits spell. A set default without one is refused (AGENTD-24). + pub fn has_row(self) -> bool { + matches!(self, DefaultUser::NameKnown | DefaultUser::DigitsWithRow) + } + + /// The `user` a request would have to send to be resolved the same way under the + /// specification, or `None` when the default is unset. A default with a row is that + /// row's name, digits included, so it takes the row's primary gid (AGENTD-22); one with + /// none is a name nobody has (AGENTD-24). + pub fn as_request(self) -> Option { + match self { + DefaultUser::Unset => None, + DefaultUser::NameKnown | DefaultUser::DigitsWithRow => Some(UserReq::NameKnown), + DefaultUser::NameUnknown | DefaultUser::DigitsNoRow => Some(UserReq::NameUnknown), + } + } + + /// The `user` the same string would be in a request: digits are a uid, with a row or + /// without, which keeps the daemon's group. [`Behavior::DefaultReadAsRequestUser`] + /// resolves the default this way. + fn as_request_string(self) -> Option { + match self { + DefaultUser::DigitsWithRow => Some(UserReq::IdWithRow), + DefaultUser::DigitsNoRow => Some(UserReq::DigitsNoRow), + default => default.as_request(), + } + } +} + /// The request's `group`. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum GroupReq { @@ -268,12 +329,35 @@ impl Request { && matches!(self.shell, ShellReq::Off | ShellReq::On) } - /// Whether the user resolves to a passwd row. - pub fn has_row(&self) -> bool { - matches!(self.user, UserReq::IdWithRow | UserReq::NameKnown) + /// The user the specification resolves for this request under `default`: the request's + /// own when it names one (AGENTD-23), else the default's (AGENTD-22). + pub fn user_under(&self, default: DefaultUser) -> UserReq { + match self.user { + UserReq::Absent => default.as_request().unwrap_or(UserReq::Absent), + user => user, + } + } + + /// Whether the user the specification resolves under `default` has a passwd row. + pub fn has_row(&self, default: DefaultUser) -> bool { + matches!( + self.user_under(default), + UserReq::IdWithRow | UserReq::NameKnown + ) } } +/// Whose account a child runs as. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum Account { + /// The daemon's own: the request named no user and the image sets no default. + Daemon, + /// The user the request named. + Requested, + /// The image's default user. + Default, +} + /// The uid a child runs as. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum Uid { @@ -321,6 +405,8 @@ pub enum Code { /// A child as spawned. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub struct Child { + /// Whose account `uid` and `gid` were resolved from. + pub account: Account, pub uid: Uid, pub gid: Gid, pub program: Program, @@ -342,6 +428,8 @@ pub struct World { /// The image-env layer, as this daemon reads it. pub image: Env, pub launch: Env, + /// The image's default user, read at startup. + pub default_user: DefaultUser, } /// **The specification.** What the daemon spawns for a request, or which 400 it answers. @@ -356,7 +444,29 @@ pub struct World { /// user with a passwd row was overwritten by the row. Restored after; BFS then explores /// 21,323 unique states with every property holding. pub fn specified(request: &Request, world: &World) -> Result { - plan(request, world, Layering::Specified, false, false) + plan( + request, + world, + Layering::Specified, + false, + false, + Defaulting::Specified, + ) +} + +/// How a daemon applies the image's default user. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +enum Defaulting { + /// To a request that names no user, resolved as that string would be. + Specified, + /// Never: the setting is read and dropped. + Ignored, + /// Ahead of the request's own user: `default.or(user)` where `user.or(default)` belongs. + OverridesRequest, + /// As specified, except a default that names nobody falls back to the daemon's user. + UnknownRunsAsDaemon, + /// As a request's `user` string: digits are a bare uid that keeps the daemon's group. + AsRequestString, } /// Which layer order a daemon uses. @@ -376,8 +486,25 @@ fn plan( layering: Layering, numeric_primary_group: bool, shell_fallback: bool, + defaulting: Defaulting, ) -> Result { - let uid = match request.user { + // The user to resolve, and whose it is. The specification is the request's own, then the + // default's, then the daemon's. + let default = match defaulting { + Defaulting::AsRequestString => world.default_user.as_request_string(), + _ => world.default_user.as_request(), + }; + let (account, user) = match (request.user, default, defaulting) { + (_, Some(default), Defaulting::OverridesRequest) => (Account::Default, default), + (UserReq::Absent, Some(_), Defaulting::Ignored) => (Account::Daemon, UserReq::Absent), + (UserReq::Absent, Some(UserReq::NameUnknown), Defaulting::UnknownRunsAsDaemon) => { + (Account::Daemon, UserReq::Absent) + } + (UserReq::Absent, Some(default), _) => (Account::Default, default), + (UserReq::Absent, None, _) => (Account::Daemon, UserReq::Absent), + (user, _, _) => (Account::Requested, user), + }; + let uid = match user { UserReq::Absent => Uid::Daemon, UserReq::IdWithRow | UserReq::IdNoRow | UserReq::DigitsNoRow => Uid::Requested, UserReq::NameKnown => Uid::Row, @@ -387,7 +514,7 @@ fn plan( GroupReq::Id => Gid::Requested, GroupReq::NameKnown => Gid::GroupRow, GroupReq::NameUnknown => return Err(Code::UnknownGroup), - GroupReq::Absent => match request.user { + GroupReq::Absent => match user { UserReq::NameKnown => Gid::RowPrimary, UserReq::IdWithRow if numeric_primary_group => Gid::RowPrimary, _ => Gid::Daemon, @@ -407,7 +534,7 @@ fn plan( Env::default() }; let mut identity = Env::default(); - if request.has_row() { + if matches!(user, UserReq::IdWithRow | UserReq::NameKnown) { identity.set(Key::Home, Source::Passwd); identity.set(Key::User, Source::Passwd); } @@ -421,6 +548,7 @@ fn plan( env.overlay(layer); } Ok(Child { + account, uid, gid, program, @@ -455,6 +583,16 @@ pub enum Behavior { ShellFallback, /// Reports the live environment's key count on health, `AGENTD_*` included. HealthCountsLiveEnv, + /// Reads the default user and never applies it. + DefaultIgnored, + /// Applies the default user ahead of the one the request names. + DefaultOverridesRequest, + /// Runs a request that names no user as the daemon when the default names nobody: the + /// silent fallback to root an image author would not see. + UnknownDefaultRunsAsDaemon, + /// Resolves the default as the same string in a request's `user`: an all-digit default + /// runs as a bare uid in the daemon's group 0, with a row or without one. + DefaultReadAsRequestUser, } impl Behavior { @@ -498,12 +636,20 @@ impl Behavior { Behavior::ImageOnTop => Layering::ImageOnTop, _ => Layering::Specified, }; + let defaulting = match self { + Behavior::DefaultIgnored => Defaulting::Ignored, + Behavior::DefaultOverridesRequest => Defaulting::OverridesRequest, + Behavior::UnknownDefaultRunsAsDaemon => Defaulting::UnknownRunsAsDaemon, + Behavior::DefaultReadAsRequestUser => Defaulting::AsRequestString, + _ => Defaulting::Specified, + }; plan( &request, world, layering, self == Behavior::NumericPrimaryGroup, self == Behavior::ShellFallback, + defaulting, ) } @@ -534,6 +680,8 @@ pub enum Step { pub struct State { /// The process has started and taken its snapshot. pub booted: bool, + /// The image's default user, read at startup. + pub default_user: DefaultUser, /// The daemon's own process environment right now. pub daemon_env: Env, /// The startup snapshot, when this daemon takes one. @@ -556,8 +704,8 @@ pub struct State { #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum Action { /// The daemon starts as the container `CMD` with the image environment and its own - /// `AGENTD_*` configuration. - Boot, + /// `AGENTD_*` configuration, which sets this default user. + Boot(DefaultUser), /// The platform's run hook, carrying the token and a launch environment. RunHook { home: bool, path: bool }, /// `POST /v1/exec/start`. @@ -598,22 +746,29 @@ impl ExecStart { World { image: self.behavior.image_layer(state), launch: state.launch, + default_user: state.default_user, } } /// Spawns whatever a spawn-first daemon can run before it has resolved anything: the - /// command under `/bin/sh` as the daemon's own user. + /// command under `/bin/sh` as the daemon's own user, which is also what it runs before + /// it has resolved the default. fn blind_child(&self, state: &State, request: &Request) -> Child { let mut unresolved = *request; unresolved.user = UserReq::Absent; unresolved.group = GroupReq::Absent; unresolved.shell = ShellReq::On; + let world = World { + default_user: DefaultUser::Unset, + ..self.world(state) + }; plan( &unresolved, - &self.world(state), + &world, Layering::Specified, false, false, + Defaulting::Specified, ) .expect("an unresolved request always plans") } @@ -626,6 +781,7 @@ impl Model for ExecStart { fn init_states(&self) -> Vec { vec![State { booted: false, + default_user: DefaultUser::Unset, daemon_env: Env::default(), snapshot: None, token_installed: false, @@ -641,7 +797,9 @@ impl Model for ExecStart { fn actions(&self, state: &State, actions: &mut Vec) { if !state.booted { - actions.push(Action::Boot); + for default_user in DefaultUser::ALL { + actions.push(Action::Boot(default_user)); + } return; } if state.health.is_none() { @@ -675,8 +833,9 @@ impl Model for ExecStart { fn next_state(&self, last: &State, action: Action) -> Option { let mut next = *last; match action { - Action::Boot => { + Action::Boot(default_user) => { next.booted = true; + next.default_user = default_user; next.daemon_env = Self::boot_env(); next.snapshot = self.behavior.snapshot(&next.daemon_env); } @@ -793,7 +952,7 @@ impl Model for ExecStart { Property::::always( "AGENTD-9 HOME and USER follow request over launch over passwd", |_, s| match started(s) { - Some((r, c)) if r.has_row() => { + Some((r, c)) if r.has_row(s.default_user) => { let home = if r.sets_home { Source::Request } else if s.launch.get(Key::Home).is_some() { @@ -818,7 +977,7 @@ impl Model for ExecStart { "AGENTD-9 witness: a request HOME overrides the passwd HOME", |_, s| { started(s).is_some_and(|(r, c)| { - r.has_row() && c.env.get(Key::Home) == Some(Source::Request) + r.has_row(s.default_user) && c.env.get(Key::Home) == Some(Source::Request) }) }, ), @@ -858,7 +1017,7 @@ impl Model for ExecStart { Source::Request } else if launch(Key::Home) { Source::Launch - } else if r.has_row() { + } else if r.has_row(s.default_user) { Source::Passwd } else { Source::Image @@ -956,10 +1115,16 @@ impl Model for ExecStart { |_, s| refused(s).is_some_and(|(_, code)| code == Code::UnknownShell), ), // ── AGENTD-16 ───────────────────────────────────────────────────── + // Over requests that name their user, or under an image that sets no default: a + // request that names nobody under a default is AGENTD-22's. Property::::always( "AGENTD-16 integer ids and a boolean shell keep their protocol-1 meaning", |_, s| match started(s) { - Some((r, c)) if r.is_protocol_one() => { + Some((r, c)) + if r.is_protocol_one() + && (r.user != UserReq::Absent + || s.default_user == DefaultUser::Unset) => + { let uid = match r.user { UserReq::Absent => Uid::Daemon, _ => Uid::Requested, @@ -981,6 +1146,96 @@ impl Model for ExecStart { "AGENTD-16 witness: an integer user with a passwd row runs", |_, s| started(s).is_some_and(|(r, _)| r.user == UserReq::IdWithRow), ), + // ── AGENTD-22 ───────────────────────────────────────────────────── + // + // A default with no row never starts, which is AGENTD-24's to say. One with a row + // runs as that row however the image spelled it: its uid, its primary gid unless + // the request names a group, and its identity. + Property::::always( + "AGENTD-22 a request that names no user runs as the default user", + |_, s| match started(s) { + Some((r, c)) if r.user == UserReq::Absent => match s.default_user { + DefaultUser::Unset => c.account == Account::Daemon && c.uid == Uid::Daemon, + default if default.has_row() => { + c.account == Account::Default + && c.uid == Uid::Row + && (r.group != GroupReq::Absent || c.gid == Gid::RowPrimary) + && c.env.get(Key::User) == Some(Source::Passwd) + } + _ => true, + }, + _ => true, + }, + ), + Property::::sometimes( + "AGENTD-22 witness: a request that names no user runs as a named default", + |_, s| { + started(s).is_some_and(|(r, _)| { + r.user == UserReq::Absent && s.default_user == DefaultUser::NameKnown + }) + }, + ), + Property::::sometimes( + "AGENTD-22 witness: a request that names no user runs as an all-digit default", + |_, s| { + started(s).is_some_and(|(r, _)| { + r.user == UserReq::Absent && s.default_user == DefaultUser::DigitsWithRow + }) + }, + ), + // ── AGENTD-23 ───────────────────────────────────────────────────── + Property::::always( + "AGENTD-23 a user the request names wins over the default", + |_, s| { + started(s).is_none_or(|(r, c)| { + r.user == UserReq::Absent || c.account == Account::Requested + }) + }, + ), + Property::::sometimes( + "AGENTD-23 witness: a request names a user under a default and runs as it", + |_, s| { + started(s).is_some_and(|(r, _)| { + r.user != UserReq::Absent && s.default_user != DefaultUser::Unset + }) + }, + ), + // ── AGENTD-24 ───────────────────────────────────────────────────── + // A default with no row, digits included: a request's same digits would run as a + // bare uid, and the default refuses instead. + Property::::always( + "AGENTD-24 a default that names nobody is refused unknown_user and spawns no child", + |_, s| match (&s.request, s.answer) { + (Some(r), Some(answer)) + if r.user == UserReq::Absent + && s.default_user != DefaultUser::Unset + && !s.default_user.has_row() => + { + answer == Answer::Refused(Code::UnknownUser) && s.child.is_none() + } + _ => true, + }, + ), + Property::::sometimes( + "AGENTD-24 witness: a default that names nobody is refused", + |_, s| { + refused(s).is_some_and(|(r, code)| { + r.user == UserReq::Absent + && s.default_user == DefaultUser::NameUnknown + && code == Code::UnknownUser + }) + }, + ), + Property::::sometimes( + "AGENTD-24 witness: an all-digit default with no row is refused", + |_, s| { + refused(s).is_some_and(|(r, code)| { + r.user == UserReq::Absent + && s.default_user == DefaultUser::DigitsNoRow + && code == Code::UnknownUser + }) + }, + ), // ── liveness ────────────────────────────────────────────────────── // // Sound because the model is acyclic: every action moves a state that no action @@ -1001,8 +1256,9 @@ mod tests { ExecStart::new(behavior).checker().spawn_bfs().join() } - /// The headline: the specified daemon satisfies AGENTD-7 through AGENTD-16 for every - /// request against every launch environment, and witnesses each case. + /// The headline: the specified daemon satisfies AGENTD-7 through AGENTD-16 and AGENTD-22 + /// through AGENTD-24 for every request against every launch environment and every + /// default user, and witnesses each case. #[test] fn the_specified_daemon_satisfies_every_property() { let checker = checked(Behavior::Specified); @@ -1120,6 +1376,67 @@ mod tests { .assert_any_discovery("AGENTD-13 health reports the snapshot's key count"); } + /// **AGENTD-22.** A default that is read and never applied leaves a request that names + /// nobody running as the daemon, which is root. + #[test] + fn an_ignored_default_runs_a_nameless_request_as_the_daemon() { + checked(Behavior::DefaultIgnored).assert_any_discovery( + "AGENTD-22 a request that names no user runs as the default user", + ); + } + + /// **AGENTD-23.** A default applied ahead of the request's own user takes away a + /// caller's way to ask for root, or for anyone else. + #[test] + fn a_default_ahead_of_the_requests_user_overrides_the_caller() { + let checker = checked(Behavior::DefaultOverridesRequest); + checker.assert_any_discovery("AGENTD-23 a user the request names wins over the default"); + checker + .assert_no_discovery("AGENTD-22 a request that names no user runs as the default user"); + } + + /// **AGENTD-24.** Falling back to the daemon when the default names nobody runs the + /// workload as root with no signal, which is the misconfiguration the setting exists to + /// prevent. + #[test] + fn an_unknown_default_falling_back_to_the_daemon_starts_as_root() { + let checker = checked(Behavior::UnknownDefaultRunsAsDaemon); + let path = checker + .assert_any_discovery( + "AGENTD-24 a default that names nobody is refused unknown_user and spawns no child", + ) + .into_actions(); + assert!( + path.contains(&Action::Boot(DefaultUser::NameUnknown)) + || path.contains(&Action::Boot(DefaultUser::DigitsNoRow)), + "the fallback needs a default that names nobody, got {path:?}" + ); + } + + /// **AGENTD-22, AGENTD-24.** A default read as a request's `user` string runs an + /// all-digit default as a bare uid: in the daemon's group 0 when the uid has a row, and + /// at all when it has none. + #[test] + fn a_default_read_as_a_request_string_keeps_the_daemons_group() { + let checker = checked(Behavior::DefaultReadAsRequestUser); + let path = checker + .assert_any_discovery("AGENTD-22 a request that names no user runs as the default user") + .into_actions(); + assert!( + path.contains(&Action::Boot(DefaultUser::DigitsWithRow)), + "the daemon's group needs an all-digit default with a row, got {path:?}" + ); + let path = checker + .assert_any_discovery( + "AGENTD-24 a default that names nobody is refused unknown_user and spawns no child", + ) + .into_actions(); + assert!( + path.contains(&Action::Boot(DefaultUser::DigitsNoRow)), + "the start needs an all-digit default with no row, got {path:?}" + ); + } + /// The table the daemon's tests mirror: the specification's answer for the cases the /// issues name. #[test] @@ -1129,6 +1446,7 @@ mod tests { let world = World { image: ExecStart::boot_env().without_agentd(), launch, + default_user: DefaultUser::Unset, }; let base = Request { user: UserReq::Absent, @@ -1139,9 +1457,10 @@ mod tests { sets_path: false, }; - // Flag unset, no user: exactly the launch map. + // Flag unset, no user: exactly the launch map, as the daemon. let child = specified(&base, &world).expect("starts"); assert_eq!(child.env, launch); + assert_eq!((child.account, child.uid), (Account::Daemon, Uid::Daemon)); // A named user: its row's uid and primary gid, HOME and USER from the row. let named = Request { @@ -1189,6 +1508,74 @@ mod tests { assert_eq!(specified(&request, &world), Err(code), "{request:?}"); } + // A default user: a request that names nobody resolves it as the same name would + // resolve, one that names someone keeps its own, and one that names nobody under a + // default that names nobody is refused. + let defaulted = World { + default_user: DefaultUser::NameKnown, + ..world + }; + let child = specified(&base, &defaulted).expect("starts"); + assert_eq!(child.account, Account::Default); + assert_eq!((child.uid, child.gid, child.env), { + let named = specified(&named, &world).expect("starts"); + (named.uid, named.gid, named.env) + }); + let own = Request { + user: UserReq::IdNoRow, + ..base + }; + let child = specified(&own, &defaulted).expect("starts"); + assert_eq!( + (child.account, child.uid, child.gid), + (Account::Requested, Uid::Requested, Gid::Daemon) + ); + let nobody = World { + default_user: DefaultUser::NameUnknown, + ..world + }; + assert_eq!(specified(&base, &nobody), Err(Code::UnknownUser)); + assert_eq!( + specified(&own, &nobody).map(|child| child.account), + Ok(Account::Requested) + ); + + // Digits the default spells are the row of that uid, primary gid and identity + // included, where a request's digits keep the daemon's group; with no row of that uid + // the default is refused, where a request's digits run as a bare uid. + let numbered = World { + default_user: DefaultUser::DigitsWithRow, + ..world + }; + let child = specified(&base, &numbered).expect("starts"); + assert_eq!( + (child.account, child.uid, child.gid, child.env), + (Account::Default, Uid::Row, Gid::RowPrimary, { + specified(&named, &world).expect("starts").env + }) + ); + let numbered_request = Request { + user: UserReq::IdWithRow, + ..base + }; + assert_eq!( + specified(&numbered_request, &world).map(|child| child.gid), + Ok(Gid::Daemon) + ); + let unnumbered = World { + default_user: DefaultUser::DigitsNoRow, + ..world + }; + assert_eq!(specified(&base, &unnumbered), Err(Code::UnknownUser)); + let digits_request = Request { + user: UserReq::DigitsNoRow, + ..base + }; + assert_eq!( + specified(&digits_request, &unnumbered).map(|child| child.uid), + Ok(Uid::Requested) + ); + // Flag set: image < passwd < launch < request. let inheriting = Request { inherit_image_env: true, diff --git a/crates/protocol/src/exec.rs b/crates/protocol/src/exec.rs index bba07545..748189c3 100644 --- a/crates/protocol/src/exec.rs +++ b/crates/protocol/src/exec.rs @@ -232,8 +232,10 @@ pub struct StartRequest { #[serde(default)] pub env: HashMap, /// The user to demote to: a numeric uid, or a name resolved against the guest's - /// `/etc/passwd`. Optional; omitted means run as the daemon's own user. A name the - /// guest does not have answers 400 `unknown_user` before anything is spawned. + /// `/etc/passwd`. Optional; omitted means the image's `AGENTD_DEFAULT_USER`, resolved to + /// the passwd row its name or uid matches, with that row's primary gid, or the daemon's + /// own user when the image sets none. A name the guest does not have, or a default with + /// no row, answers 400 `unknown_user` before anything is spawned. /// /// A user with a passwd row, however it was given, gets `HOME`, `USER` and `LOGNAME` /// from the row, beneath the launch environment and `env`. A user given by name also @@ -550,7 +552,8 @@ impl StartRequest { self } - /// The user to demote to; `None` runs as the daemon's own user. + /// The user to demote to; `None` runs as the image's `AGENTD_DEFAULT_USER`, or as the + /// daemon's own user when the image sets none. pub fn with_user(mut self, user: Option) -> Self { self.user = user; self diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 7474e8fb..f6c9d08b 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -128,8 +128,9 @@ log group), never returned on `/v1/health`, which needs no agent token. A service that needs to keep running after its handler exits should redirect its output streams so the daemon can finish reading them. -A handler runs as the daemon's user (root) with the daemon's environment, the -launch environment, and `AGENTD_HOOK=`; never the agent token. Any process +A handler runs as the daemon's user (root), whatever user `AGENTD_DEFAULT_USER` +holds, with the daemon's environment, the launch environment, and +`AGENTD_HOOK=`; never the agent token. Any process in the guest can post a hook path, so a handler must be safe to run spuriously. See [Trust](TRUST.md). @@ -211,7 +212,8 @@ semantics rather than `realpath`. Symlinks resolve relative to their own directory, while hard links resolve against the archive root. Member count and total size are capped. Modes are applied after content lands. A regular-file member replaces the old entry before creation, so other hard links keep their -bytes and modes. +bytes and modes. Extraction drops the archive's owners, so everything it +creates is root's, as [Upload ownership](#upload-ownership) describes. **A symlink an archive wrote cannot redirect a later member, and the kernel is what enforces that.** The `normpath` rule above judges a member at the depth its @@ -339,6 +341,27 @@ as before. A user given by *name* also gets the row's primary gid when `group` i omitted, the way `su` does; a user given as an integer keeps the daemon's group, which is what protocol 1 always did. +**An image can set the user a start without `user` runs as.** The image's +`AGENTD_DEFAULT_USER` holds a user name or a string of digits. A start request +that omits `user` resolves that string to a row of the guest's `/etc/passwd`: +the row a name matches, or the row whose uid the digits spell. Either way the +child gets the row's uid, primary gid, `HOME`, `USER` and `LOGNAME`. That's +stricter than `user`, where digits keep the daemon's gid and don't need a row. The +daemon resolves it at each request, so a user an earlier exec created resolves +on the next. A request's own `user` always wins, `root` and `0` included, and +`group` still overrides the primary gid. If no row matches, the daemon answers +each start that omits `user` with `400 unknown_user`, and the `detail` mentions +`AGENTD_DEFAULT_USER`. For digits, the `detail` also says the image needs a +passwd row for that uid. The daemon refuses an empty value the same way. It still +boots, and a request that sets `user` still runs. With the variable unset, a +start that omits `user` runs as the daemon's own user, root, as it always has. +Uploads don't follow the variable: what they create stays root's, as +[Upload ownership](#upload-ownership) describes. +Workload hook handlers ignore the variable, and the image snapshot drops it with +every other `AGENTD_*` variable. `/v1/health` doesn't report it, because a new +`Health` field would break the protocol crate's API. The daemon logs the value +at startup instead. + **A named shell is resolved on the child's `PATH`, the image's `PATH`, `/bin`, then `/usr/bin`.** The first executable regular file wins and runs as ` -c `. An absolute path is checked as given. A shell the guest @@ -387,6 +410,23 @@ shape (AGENTD-7 through AGENTD-16 in `verify/spec/agentd.symspec.json`), includi no refused request ever has a child and that neither the token nor `AGENTD_*` configuration reaches one. +## Upload ownership + +**Uploads are root's, whatever `AGENTD_DEFAULT_USER` holds.** `PUT /v1/fs/tar` +and `PUT /v1/fs/file` write as the daemon's user, root, so each entry they +create belongs to root: files, directories, symlinks, and the parent +directories made for them. Tar extraction masks each member's mode to `0755` at +most, so it doesn't set a group or other write bit on a file or directory. +`PUT /v1/fs/file` gives the file the `mode` the request sets, or `0644` without +one, and the parent directories it creates are `0755`. The image's default user +can read an uploaded tree, then, but can't write into the directories an upload +created, or change a file unless its requested `mode` lets other users write. +An exec that must write there runs with +`user: 0`, or the image or an exec started with `user: 0` hands the tree to the +default user first with `chown`. The agent recipe's `chown` of `/workspace` +does that. A default with no passwd row doesn't refuse an upload, because an +upload doesn't read the variable. + ## Line-ranged text reads `GET /v1/fs/file` takes optional `start_line` and `end_line`. The semantics are diff --git a/docs/TRACEABILITY.md b/docs/TRACEABILITY.md index 99878d75..5c80d3bd 100644 --- a/docs/TRACEABILITY.md +++ b/docs/TRACEABILITY.md @@ -28,6 +28,9 @@ traced requirement is missing a layer. Requirements are defined in | AGENTD-19 | 1 | waived | waived | 1 | 1 | waived | | AGENTD-20 | 1 | waived | 1 | 3 | 1 | waived | | AGENTD-21 | 1 | waived | 1 | 1 | 1 | waived | +| AGENTD-22 | 1 | 1 | 1 | 3 | 3 | waived | +| AGENTD-23 | 1 | 1 | 1 | 2 | 2 | waived | +| AGENTD-24 | 1 | 1 | 1 | 2 | 1 | waived | | ARCH-1 | waived | waived | waived | 1 | 2 | waived | | ARCH-2 | waived | waived | waived | 1 | 4 | waived | | ARCH-3 | waived | waived | waived | 1 | 1 | waived | @@ -351,6 +354,39 @@ If a frame of an identity-verified tunnel does not authenticate at the frame's p - **impl:** `crates/agentd/src/tunnel.rs` - **live:** waived: a live daemon and the endpoint proxy neither replay nor forge a frame, so a live run can't present one; the relay tests send both to the daemon's real route +## AGENTD-22 + +Where the image environment sets the default user, the agentd shall resolve the default user against the guest's /etc/passwd as the user of each start request that names no user. + +- **model:** `crates/model/src/exec_start.rs` +- **gherkin:** `crates/agentd/tests/features/exec_start.feature` +- **fuzz:** `crates/agentd/src/exec_start_fuzz.rs` +- **test:** `crates/agentd/src/config.rs`, `crates/agentd/src/exec.rs`, `crates/agentd/src/exec_start.rs` +- **impl:** `crates/agentd/src/config.rs`, `crates/agentd/src/exec.rs`, `crates/agentd/src/exec_start.rs` +- **live:** waived: the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier spawn real children under a configured default + +## AGENTD-23 + +Where the image environment sets the default user, the agentd shall demote the child of a start request that names its user to the named user rather than the default user. + +- **model:** `crates/model/src/exec_start.rs` +- **gherkin:** `crates/agentd/tests/features/exec_start.feature` +- **fuzz:** `crates/agentd/src/exec_start_fuzz.rs` +- **test:** `crates/agentd/src/exec.rs`, `crates/agentd/src/exec_start.rs` +- **impl:** `crates/agentd/src/config.rs`, `crates/agentd/src/exec_start.rs` +- **live:** waived: the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier spawn real children under a configured default + +## AGENTD-24 + +If a start request names no user and the guest's /etc/passwd does not contain the default user, then the agentd shall answer Bad Request with the unknown_user error naming the default user setting and spawn no child. + +- **model:** `crates/model/src/exec_start.rs` +- **gherkin:** `crates/agentd/tests/features/exec_start.feature` +- **fuzz:** `crates/agentd/src/exec_start_fuzz.rs` +- **test:** `crates/agentd/src/exec.rs`, `crates/agentd/src/exec_start.rs` +- **impl:** `crates/agentd/src/exec_start.rs` +- **live:** waived: the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier refuse a start under a default the guest does not have + ## ARCH-1 The microvms-core shall be the single library crate the CLI crate and the language bindings depend on, composing microvms-app with microvms-edges and re-exporting microvms-domain. @@ -1365,6 +1401,7 @@ trace it. | A guest answers a verified tunnel's handshake with a key other than the pinned VM key | BIND-21 | `crates/microvms-core/tests/tunnel_end_to_end.rs::a_wrong_pin_fails_closed_with_a_diagnosis`, `crates/microvms-core/tests/tunnel_end_to_end.rs::a_reply_that_does_not_verify_against_the_pin_fails_the_tunnel` | guarded | | A guest or the path replays, reorders or forges tunnel frames after the handshake | AGENTD-21, BIND-24 | `crates/agentd/tests/tunnel_relay.rs::agentd_21_a_replayed_frame_ends_the_tunnel_unrelayed`, `crates/agentd/tests/tunnel_relay.rs::agentd_21_a_forged_frame_ends_the_tunnel_unrelayed`, `crates/microvms-edges/tests/tunnel_end_of_stream.rs::bind_24_a_replayed_frame_fails_the_tunnel_unwritten`, `crates/microvms-edges/tests/tunnel_end_of_stream.rs::bind_24_a_forged_frame_fails_the_tunnel_unwritten`, `crates/model/src/tunnel.rs::bind_24_the_model_finds_a_replay_when_frames_carry_no_position`, `crates/agentd/src/tunnel_fuzz.rs::frames_open_in_order_and_the_callers_end_only_after_every_chunk`, `crates/microvms-edges/src/session/tunnel_fuzz.rs::frames_open_in_order_and_the_daemons_end_only_after_every_chunk` | guarded | | A guest streams hostile server-sent events to the client | BIND-22 | `crates/microvms-app/src/session/sse.rs::an_unterminated_stream_is_refused_at_the_pending_ceiling`, `crates/microvms-app/src/session/sse.rs::an_unrecognized_or_unparseable_frame_is_dropped_rather_than_raised`, `crates/microvms-app/src/session/sse_fuzz.rs::hostile_stream_bytes_stay_bounded_and_every_event_round_trips` | guarded | +| A workload whose caller omits `user` runs as guest root | AGENTD-22, AGENTD-24 | `crates/agentd/src/exec.rs::a_start_that_names_no_user_runs_as_the_default_resolved_per_request`, `crates/agentd/src/exec.rs::a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused`, `crates/agentd/src/exec_start.rs::a_default_that_names_nobody_is_refused_naming_the_variable`, `crates/model/src/exec_start.rs::an_unknown_default_falling_back_to_the_daemon_starts_as_root`, `crates/model/src/exec_start.rs::a_default_read_as_a_request_string_keeps_the_daemons_group` | guarded when the image sets `AGENTD_DEFAULT_USER`. Without it such a workload runs as root, and with it an authorized caller can still ask for root | | A replaced or tampered daemon release asset | BIND-18 | `crates/microvms-edges/src/provision/release.rs::another_signer_identity_is_refused`, `crates/microvms-edges/src/provision/release.rs::one_flipped_byte_in_the_asset_is_refused` | guarded | | A release directory (`$MICROVM_RELEASE_DIR`) holding an asset and a matching `SHA256SUMS` but no bundle, or another release's files under the requested tag | BIND-18 | `crates/microvms-edges/src/provision/release.rs::a_release_directory_without_its_bundle_is_refused_despite_a_matching_checksum`, `crates/microvms-edges/src/provision/release.rs::another_releases_files_under_the_requested_tag_are_refused` | guarded | | An on-path party ends a verified tunnel early, with a plaintext close frame or by dropping the connection | BIND-23, AGENTD-19, AGENTD-20 | `crates/microvms-edges/tests/tunnel_end_of_stream.rs::a_verified_tcp_consumer_observes_the_stream_verdict`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_a_close_frame_forged_mid_stream_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_a_connection_dropped_mid_stream_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_the_daemons_end_of_stream_withheld_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::agentd_20_the_clients_end_of_stream_withheld_resets_the_guest`, `crates/agentd/tests/tunnel_relay.rs::agentd_19_a_guest_eof_reaches_the_caller_as_the_end_of_stream` | guarded. A protocol 2 peer that omits the end-of-stream offer ends `ClosedUnproven`. Each side refuses a protocol 1 handshake | diff --git a/docs/TRUST.md b/docs/TRUST.md index 9d3e2ff7..9a365f1e 100644 --- a/docs/TRUST.md +++ b/docs/TRUST.md @@ -19,6 +19,15 @@ daemon memory with `ptrace` or `/proc//mem`, change files, or exhaust guest resources. User demotion changes the workload UID within the same sandbox. The model and authentication tests do not prove isolation from guest root. +A start request that omits `user` runs as root unless the image sets +`AGENTD_DEFAULT_USER`. With it set, that start runs as the image's user, with +its passwd row's uid and primary gid, whether the image gives the user's name +or its uid. A value with no passwd row, digits included, refuses the start +instead of running it as root or in root's group. Uploads are still written as +root, so that user can read an uploaded tree but can't write into it. An authorized +caller can still execute as root by asking for `root` or `0`, and workload hook +handlers still run as root. + ## Platform isolation guarantees AWS documents two relevant properties: @@ -118,6 +127,7 @@ and it renders the table in [Traceability](TRACEABILITY.md). A guard is written | A guest answers a verified tunnel's handshake with a key other than the pinned VM key | `BIND-21` | `crates/microvms-core/tests/tunnel_end_to_end.rs::a_wrong_pin_fails_closed_with_a_diagnosis`, `crates/microvms-core/tests/tunnel_end_to_end.rs::a_reply_that_does_not_verify_against_the_pin_fails_the_tunnel` | guarded | | A guest or the path replays, reorders or forges tunnel frames after the handshake | `AGENTD-21`, `BIND-24` | `crates/agentd/tests/tunnel_relay.rs::agentd_21_a_replayed_frame_ends_the_tunnel_unrelayed`, `crates/agentd/tests/tunnel_relay.rs::agentd_21_a_forged_frame_ends_the_tunnel_unrelayed`, `crates/microvms-edges/tests/tunnel_end_of_stream.rs::bind_24_a_replayed_frame_fails_the_tunnel_unwritten`, `crates/microvms-edges/tests/tunnel_end_of_stream.rs::bind_24_a_forged_frame_fails_the_tunnel_unwritten`, `crates/model/src/tunnel.rs::bind_24_the_model_finds_a_replay_when_frames_carry_no_position`, `crates/agentd/src/tunnel_fuzz.rs::frames_open_in_order_and_the_callers_end_only_after_every_chunk`, `crates/microvms-edges/src/session/tunnel_fuzz.rs::frames_open_in_order_and_the_daemons_end_only_after_every_chunk` | guarded | | A guest streams hostile server-sent events to the client | `BIND-22` | `crates/microvms-app/src/session/sse.rs::an_unterminated_stream_is_refused_at_the_pending_ceiling`, `crates/microvms-app/src/session/sse.rs::an_unrecognized_or_unparseable_frame_is_dropped_rather_than_raised`, `crates/microvms-app/src/session/sse_fuzz.rs::hostile_stream_bytes_stay_bounded_and_every_event_round_trips` | guarded | +| A workload whose caller omits `user` runs as guest root | `AGENTD-22`, `AGENTD-24` | `crates/agentd/src/exec.rs::a_start_that_names_no_user_runs_as_the_default_resolved_per_request`, `crates/agentd/src/exec.rs::a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused`, `crates/agentd/src/exec_start.rs::a_default_that_names_nobody_is_refused_naming_the_variable`, `crates/model/src/exec_start.rs::an_unknown_default_falling_back_to_the_daemon_starts_as_root`, `crates/model/src/exec_start.rs::a_default_read_as_a_request_string_keeps_the_daemons_group` | guarded when the image sets `AGENTD_DEFAULT_USER`. Without it such a workload runs as root, and with it an authorized caller can still ask for root | | A replaced or tampered daemon release asset | `BIND-18` | `crates/microvms-edges/src/provision/release.rs::another_signer_identity_is_refused`, `crates/microvms-edges/src/provision/release.rs::one_flipped_byte_in_the_asset_is_refused` | guarded | | A release directory (`$MICROVM_RELEASE_DIR`) holding an asset and a matching `SHA256SUMS` but no bundle, or another release's files under the requested tag | `BIND-18` | `crates/microvms-edges/src/provision/release.rs::a_release_directory_without_its_bundle_is_refused_despite_a_matching_checksum`, `crates/microvms-edges/src/provision/release.rs::another_releases_files_under_the_requested_tag_are_refused` | guarded | | An on-path party ends a verified tunnel early, with a plaintext close frame or by dropping the connection | `BIND-23`, `AGENTD-19`, `AGENTD-20` | `crates/microvms-edges/tests/tunnel_end_of_stream.rs::a_verified_tcp_consumer_observes_the_stream_verdict`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_a_close_frame_forged_mid_stream_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_a_connection_dropped_mid_stream_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::bind_23_the_daemons_end_of_stream_withheld_ends_truncated`, `crates/model-conformance/tests/tunnel_end_of_stream.rs::agentd_20_the_clients_end_of_stream_withheld_resets_the_guest`, `crates/agentd/tests/tunnel_relay.rs::agentd_19_a_guest_eof_reaches_the_caller_as_the_end_of_stream` | guarded. A protocol 2 peer that omits the end-of-stream offer ends `ClosedUnproven`. Each side refuses a protocol 1 handshake | @@ -195,8 +205,8 @@ An executable at `/` runs when the platform posts that lifecycle hook ([Protocol](PROTOCOL.md), "Workload hook handlers"). It is image-owned code with the daemon's privileges: -- It runs as root, like the daemon, with the daemon's environment, the launch - environment, and `AGENTD_HOOK`. The agent token is never in either +- It runs as root, like the daemon, whatever user `AGENTD_DEFAULT_USER` holds, + with the daemon's environment, the launch environment, and `AGENTD_HOOK`. The agent token is never in either environment. Keep secrets out of shared images; a handler is part of the image. - The hook routes are unauthenticated and reachable over loopback from inside the guest, so any guest process can make a handler run. A handler must be @@ -263,12 +273,14 @@ instead of broadening the shared VM role. Blocking MMDS inside the guest takes `CAP_NET_ADMIN`. A guest from an image built without `--repair-identity` lacks it, and route, rule, and link changes -return `EPERM`. On a guest from an image built with it, root can replace the guest's host route for `169.254.169.254` -with a blackhole. Afterward, a process that isn't root can neither reach MMDS -nor remove the route ([Platform](PLATFORM.md), measured 2026-10-12). Any root -process can remove it, including an exec that doesn't ask for another user. The -block protects only workloads that run as another user. VPC internet isolation -does not remove metadata access or make an overprivileged role safe. +return `EPERM`. On a guest from an image built with it, root can replace the +guest's host route for `169.254.169.254` with a blackhole. Afterward, a process +that isn't root can neither reach MMDS nor remove the route +([Platform](PLATFORM.md), measured 2026-10-12). Any root process can remove it, +including an exec started with `user: 0`, and one started without `user` on an +image without `AGENTD_DEFAULT_USER`. The block protects only workloads that run as +another user. VPC internet isolation does not remove metadata access or make an +overprivileged role safe. ## Outside this contract diff --git a/docs/manifest.json b/docs/manifest.json index 4e8778ef..cc281ac1 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -231,7 +231,7 @@ { "choices": null, "default": null, - "help": "User to run --exec's command as: a name or a numeric uid. Omitted runs as the daemon's own user", + "help": "User to run --exec's command as: a name or a numeric uid. Omitted runs as the image's AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none", "name": "user", "positional": false, "required": false, @@ -1408,7 +1408,7 @@ { "choices": null, "default": null, - "help": "User to run the command as: a name or a numeric uid. Omitted runs as the daemon's own user", + "help": "User to run the command as: a name or a numeric uid. Omitted runs as the image's AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none", "name": "user", "positional": false, "required": false, diff --git a/docs/schema.json b/docs/schema.json index 45f5126d..67796dbc 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -631,7 +631,7 @@ } ], "default": null, - "description": "The user to demote to: a numeric uid, or a name resolved against the guest's\n`/etc/passwd`. Optional; omitted means run as the daemon's own user. A name the\nguest does not have answers 400 `unknown_user` before anything is spawned.\n\nA user with a passwd row, however it was given, gets `HOME`, `USER` and `LOGNAME`\nfrom the row, beneath the launch environment and `env`. A user given by name also\ngets the row's primary gid when `group` is omitted; a numeric uid keeps the\ndaemon's group, as it always did." + "description": "The user to demote to: a numeric uid, or a name resolved against the guest's\n`/etc/passwd`. Optional; omitted means the image's `AGENTD_DEFAULT_USER`, resolved to\nthe passwd row its name or uid matches, with that row's primary gid, or the daemon's\nown user when the image sets none. A name the guest does not have, or a default with\nno row, answers 400 `unknown_user` before anything is spawned.\n\nA user with a passwd row, however it was given, gets `HOME`, `USER` and `LOGNAME`\nfrom the row, beneath the launch environment and `env`. A user given by name also\ngets the row's primary gid when `group` is omitted; a numeric uid keeps the\ndaemon's group, as it always did." } }, "required": [ diff --git a/site/authored/learn/operations/write-a-guest-dockerfile.md b/site/authored/learn/operations/write-a-guest-dockerfile.md index b96baa78..9ba7ad63 100644 --- a/site/authored/learn/operations/write-a-guest-dockerfile.md +++ b/site/authored/learn/operations/write-a-guest-dockerfile.md @@ -71,6 +71,16 @@ WORKDIR /workspace The daemon itself stays root; demotion is per command, with `exec --user 1000 --group 1000`. [Run coding agents on Bedrock](/learn/operations/run-coding-agents-on-bedrock/) shows why that uid is not decoration. +An exec without `--user` runs as root unless the image sets a default user: + +```dockerfile +ENV AGENTD_DEFAULT_USER=agent +``` + +The daemon resolves the value at each exec to its `/etc/passwd` row, so the row can come from an earlier exec. A name and its uid are the same: `AGENTD_DEFAULT_USER=1000` runs as `agent` with group 1000 and `HOME=/workspace`, and a uid with no row fails the exec rather than run it in root's group. An exec with `--user` runs as that user instead, `--user root` included, and hook handlers still run as root. + +Uploads don't follow the variable. `microvm cp` and `run . --exec` write through the daemon as root, so what they create is root's, with no group or other write bit on a synced tree. The default user can read the synced files but can't change them or write into the directories the sync made, even under a `/workspace` the image handed to it. An exec that must write there runs with `--user 0`. Otherwise, a `chown` run with `--user 0` gives the tree to the default user first, as this section's Dockerfile does for `/workspace`. + ## 5. No secret goes in the image The image becomes a shared snapshot, so every VM launched from it sees the same bytes. Per-VM credentials are delivered through the platform's one-shot `runHookPayload` at launch: the agent token always, and anything you add with `run --launch-env KEY=VALUE`, which shares the payload's 4096-byte ceiling. Larger material goes in after bootstrap with `microvm cp --mode 0600` over the authenticated channel. @@ -81,7 +91,7 @@ The artifact `microvm build` uploads carries exactly the Dockerfile and the `age ## 7. The daemon's knobs -Every `AGENTD_*` variable is read at startup, and an unset or unparseable value keeps the default rather than refusing to boot, because a daemon that will not start strands the VM with no way in. Set them as `ENV` lines. The stanza already sets `AGENTD_PORT` and `AGENTD_LOG`; the others bound request bodies, captured output, stream replay, stdin writes, the disk reserve, and the identity repair that gives each VM restored from one snapshot its own machine-id, hostname, and boot_id. The daemon configuration is defined in `crates/agentd/src/config.rs`; +Every `AGENTD_*` variable is read at startup, and an unset or unparseable value keeps the default rather than refusing to boot, because a daemon that will not start strands the VM with no way in. `AGENTD_DEFAULT_USER` doesn't fall back: a user the guest lacks fails each exec without `--user` with `unknown_user`, and the error mentions the variable. Set them as `ENV` lines. The stanza already sets `AGENTD_PORT` and `AGENTD_LOG`; the others bound request bodies, captured output, stream replay, stdin writes, the disk reserve, and the identity repair that gives each VM restored from one snapshot its own machine-id, hostname, and boot_id. The daemon configuration is defined in `crates/agentd/src/config.rs`; [Protocol](/internals/protocol/) documents the request limits and hook behavior. ## 8. Run code on suspend and resume diff --git a/verify/guards/faults/default-user.toml b/verify/guards/faults/default-user.toml new file mode 100644 index 00000000..f9ad7a5a --- /dev/null +++ b/verify/guards/faults/default-user.toml @@ -0,0 +1,167 @@ +# ── default-user: AGENTD_DEFAULT_USER, the image's default user for a start that omits `user` (#595) ── + +# The daemon's tests call what the change adds (`Guest::default_user`, `Config::default_user`), +# so the merge base doesn't build them and FAIL_TO_PASS can't prove them. Each fault here takes +# one part of the change out where the tests still compile. + +# AGENTD-22: the default read and never applied. +[[fault]] +id = "default-user-ignored" +guard = "exec_start::tests::a_request_that_names_no_user_resolves_the_default_as_its_user" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start::tests::a_request_that_names_no_user_resolves_the_default_as_its_user"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " match (&request.user, default_user) {\n", with = " let default_user: Option<&str> = default_user.and(None);\n match (&request.user, default_user) {\n" } + +[[fault]] +id = "default-user-ignored-fuzz" +guard = "exec_start_fuzz::start_resolution" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start_fuzz::start_resolution"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " match (&request.user, default_user) {\n", with = " let default_user: Option<&str> = default_user.and(None);\n match (&request.user, default_user) {\n" } + +# AGENTD-22 through the handler: the default dropped where the start builds its `Guest`, and +# the passwd file left unread for a start that omits `user`. +[[family]] +guard = "exec::tests::a_start_that_names_no_user_runs_as_the_default_resolved_per_request" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec::tests::a_start_that_names_no_user_runs_as_the_default_resolved_per_request"] +expect = "test-failed" +suite = "rust" + +[[family.fault]] +id = "default-user-dropped-at-build" +transform = { file = "crates/agentd/src/exec.rs", replace = " default_user,\n is_executable: &is_executable_file,\n", with = " default_user: None,\n is_executable: &is_executable_file,\n" } + +[[family.fault]] +id = "default-user-passwd-unread" +transform = { file = "crates/agentd/src/exec.rs", replace = " let passwd = if req.user.is_some() || default_user.is_some() {\n", with = " let passwd = if req.user.is_some() {\n" } + +# AGENTD-23: the default applied ahead of a user the request names. +[[fault]] +id = "default-user-overrides-request" +guard = "exec_start::tests::a_user_the_request_names_wins_over_the_default" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start::tests::a_user_the_request_names_wins_over_the_default"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " (Some(spec), _) => resolve_user(spec, rows).map(Some),\n", with = " (Some(_), Some(name)) => resolve_user(&NameOrId::Name(name.to_string()), rows).map(Some),\n (Some(spec), _) => resolve_user(spec, rows).map(Some),\n" } + +[[fault]] +id = "default-user-overrides-request-handler" +guard = "exec::tests::a_user_the_start_names_wins_over_the_default" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec::tests::a_user_the_start_names_wins_over_the_default"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " (Some(spec), _) => resolve_user(spec, rows).map(Some),\n", with = " (Some(_), Some(name)) => resolve_user(&NameOrId::Name(name.to_string()), rows).map(Some),\n (Some(spec), _) => resolve_user(spec, rows).map(Some),\n" } + +# AGENTD-24: a default the guest doesn't have falls back to the daemon's user, which is root. +[[fault]] +id = "default-user-unknown-runs-as-daemon" +guard = "exec_start::tests::a_default_that_names_nobody_is_refused_naming_the_variable" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start::tests::a_default_that_names_nobody_is_refused_naming_the_variable"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " Err(why) => Err(Rejection::new(\n ERROR_UNKNOWN_USER,\n format!(\"the request names no user, and {why}\"),\n )),\n", with = " Err(_) => Ok(None),\n" } + +# AGENTD-22: an all-digit default resolved as a request's digits are, which keeps the daemon's +# gid 0 for a uid with a row (the review finding on #600: `uid=1000(agent) gid=0(root)`). +[[fault]] +id = "default-user-numeric-keeps-daemon-gid" +guard = "exec_start::tests::a_request_that_names_no_user_resolves_the_default_as_its_user" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start::tests::a_request_that_names_no_user_resolves_the_default_as_its_user"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " row: Some(row),\n primary_gid: true,\n })),\n", with = " row: Some(row),\n primary_gid: row.name == name,\n })),\n" } + +[[fault]] +id = "default-user-numeric-keeps-daemon-gid-fuzz" +guard = "exec_start_fuzz::start_resolution" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start_fuzz::start_resolution"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " row: Some(row),\n primary_gid: true,\n })),\n", with = " row: Some(row),\n primary_gid: row.name == name,\n })),\n" } + +# AGENTD-24: an all-digit default with no row run as a bare uid, as a request's digits are. +[[family]] +guard = "exec_start::tests::a_default_that_names_nobody_is_refused_naming_the_variable" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec_start::tests::a_default_that_names_nobody_is_refused_naming_the_variable"] +expect = "test-failed" +suite = "rust" + +[[family.fault]] +id = "default-user-digits-without-row-run" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " (None, Some(name)) => match default_user_row(name, rows) {\n", with = " (None, Some(name)) if digits(name).is_some() => {\n resolve_user(&NameOrId::Name(name.to_string()), rows).map(Some)\n }\n (None, Some(name)) => match default_user_row(name, rows) {\n" } + +[[fault]] +id = "default-user-digits-without-row-run-handler" +guard = "exec::tests::a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "exec::tests::a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/exec_start.rs", replace = " (None, Some(name)) => match default_user_row(name, rows) {\n", with = " (None, Some(name)) if digits(name).is_some() => {\n resolve_user(&NameOrId::Name(name.to_string()), rows).map(Some)\n }\n (None, Some(name)) => match default_user_row(name, rows) {\n" } + +# The model's AGENTD-22 property: an all-digit default with a row resolved as a request's digits +# are, a bare uid in the daemon's group. +[[fault]] +id = "model-exec-start-numeric-default-as-request" +guard = "exec_start::tests::the_specified_daemon_satisfies_every_property" +run = ["cargo", "test", "-p", "agentd-model", "--lib", "--", "--exact", "exec_start::tests::the_specified_daemon_satisfies_every_property"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/model/src/exec_start.rs", replace = " DefaultUser::NameKnown | DefaultUser::DigitsWithRow => Some(UserReq::NameKnown),\n", with = " DefaultUser::NameKnown => Some(UserReq::NameKnown),\n DefaultUser::DigitsWithRow => Some(UserReq::IdWithRow),\n" } + +# An empty or non-UTF-8 value read as unset, which would run every workload as root. +[[fault]] +id = "default-user-empty-read-as-unset" +guard = "config::tests::the_default_user_is_kept_as_set" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "config::tests::the_default_user_is_kept_as_set"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/config.rs", replace = " raw.map(|raw| raw.to_string_lossy().into_owned())\n", with = " raw.and_then(|raw| raw.into_string().ok())\n .filter(|raw| !raw.is_empty())\n" } + +# `from_env` that never reads the variable, the mutant `replace Config::from_env -> Self with +# Default::default()` stands for: only the child copy of the test binary sees the variable set. +[[fault]] +id = "default-user-env-unread" +guard = "config::tests::from_env_reads_the_default_user_from_the_environment" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "config::tests::from_env_reads_the_default_user_from_the_environment"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/config.rs", replace = " cfg.default_user = default_user(std::env::var_os(DEFAULT_USER_VAR));\n", with = "" } + +# A workload hook handler demoted to the image's default user (uid 65534 in the test). +[[fault]] +id = "default-user-hook-handler-demoted" +guard = "hook_handlers::tests::a_handler_runs_as_the_daemon_whatever_default_user_the_image_sets" +run = ["cargo", "test", "-p", "agentd", "--lib", "--", "--exact", "hook_handlers::tests::a_handler_runs_as_the_daemon_whatever_default_user_the_image_sets"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/agentd/src/hook_handlers.rs", replace = " .process_group(0)\n .kill_on_drop(true);\n", with = " .process_group(0)\n .uid(65534)\n .kill_on_drop(true);\n" } + +# The model's AGENTD-22 property: a specification that runs a nameless request as the daemon +# under any default. +[[fault]] +id = "model-exec-start-default-ignored" +guard = "exec_start::tests::the_specified_daemon_satisfies_every_property" +run = ["cargo", "test", "-p", "agentd-model", "--lib", "--", "--exact", "exec_start::tests::the_specified_daemon_satisfies_every_property"] +expect = "test-failed" +suite = "rust" +transform = { file = "crates/model/src/exec_start.rs", replace = " (UserReq::Absent, Some(default), _) => (Account::Default, default),\n", with = " (UserReq::Absent, Some(_), _) => (Account::Daemon, UserReq::Absent),\n" } + +# FAIL_TO_PASS: fails on f410ec2f4c80 with the fix taken out (tools/fail-to-pass.py). +[[fault]] +id = "f2p-install-access-uploads-then-chowns-as-root" +guard = "agents::tests::install_access_uploads_then_chowns_as_root" +run = ["cargo", "test", "-p", "microvms-app", "--lib", "--", "--exact", "agents::tests::install_access_uploads_then_chowns_as_root"] +expect = "test-failed" +suite = "rust" +patch = "verify/guards/faults/f2p-install-access-uploads-then-chowns-as-root.patch" + +# FAIL_TO_PASS: fails on f410ec2f4c80 with the fix taken out (tools/fail-to-pass.py). +[[fault]] +id = "f2p-a-hostile-deletion-path-is-refused-not-executed" +guard = "workspace::tests::a_hostile_deletion_path_is_refused_not_executed" +run = ["cargo", "test", "-p", "microvms-app", "--lib", "--", "--exact", "workspace::tests::a_hostile_deletion_path_is_refused_not_executed"] +expect = "test-failed" +suite = "rust" +patch = "verify/guards/faults/f2p-a-hostile-deletion-path-is-refused-not-executed.patch" diff --git a/verify/guards/faults/f2p-a-hostile-deletion-path-is-refused-not-executed.patch b/verify/guards/faults/f2p-a-hostile-deletion-path-is-refused-not-executed.patch new file mode 100644 index 00000000..8ad8bb19 --- /dev/null +++ b/verify/guards/faults/f2p-a-hostile-deletion-path-is-refused-not-executed.patch @@ -0,0 +1,417 @@ +diff --git a/crates/agentd/src/config.rs b/crates/agentd/src/config.rs +index 24deb93..9ea2777 100644 +--- a/crates/agentd/src/config.rs ++++ b/crates/agentd/src/config.rs +@@ -93,25 +93,8 @@ pub struct Config { + /// The group database a start request's group name is resolved against. `/etc/group` + /// outside tests, for `passwd_path`'s reason. + pub group_path: std::path::PathBuf, +- /// The user a start request runs as when it names no `user`, from +- /// [`DEFAULT_USER_VAR`] (AGENTD-22). `None` keeps the daemon's own user, which is root. +- /// +- /// Kept as the image wrote it and resolved per request exactly as a request's `user` +- /// string is: a passwd name, or digits that name no row as a uid. Per request because a +- /// user an earlier exec created resolves on the next. A request's own `user` always wins, +- /// `root` and `0` included (AGENTD-23), so an authorized caller can still ask for root. +- /// +- /// Read from the image's environment like every other `AGENTD_*` setting, and never from +- /// a request or the launch environment: who a workload runs as when its caller doesn't +- /// say is the image author's decision. Workload hook handlers don't take it; they are +- /// image-owned code and run as the daemon's user. +- pub default_user: Option, + } + +-/// The variable naming [`Config::default_user`]. A refusal of the default names it, so an +-/// image author who misspelled the user can find where it was set. +-pub const DEFAULT_USER_VAR: &str = "AGENTD_DEFAULT_USER"; +- + /// The handler budget's ceiling: five seconds under the platform's 60 s hook limit. + pub const MAX_HOOK_HANDLER_SECS: u64 = 55; + +@@ -145,7 +128,6 @@ impl Default for Config { + hook_handler_timeout: Duration::from_secs(20), + passwd_path: std::path::PathBuf::from("/etc/passwd"), + group_path: std::path::PathBuf::from("/etc/group"), +- default_user: None, + } + } + } +@@ -193,32 +175,10 @@ impl Config { + if let Some(secs) = env_parse::("AGENTD_HOOK_HANDLER_TIMEOUT_SECS") { + cfg.hook_handler_timeout = Duration::from_secs(secs.clamp(1, MAX_HOOK_HANDLER_SECS)); + } +- cfg.default_user = default_user(std::env::var_os(DEFAULT_USER_VAR)); +- // Logged to the image's log group because `/v1/health` can't carry it yet: `Health` is +- // constructible field by field, so a new field is a breaking change to the protocol +- // crate. +- if let Some(user) = &cfg.default_user { +- tracing::info!( +- default_user = %user, +- "a start request that omits user runs as {DEFAULT_USER_VAR}" +- ); +- } + cfg + } + } + +-/// [`DEFAULT_USER_VAR`] as the daemon keeps it: set means set, an empty value included. +-/// +-/// Unlike the numeric knobs, a value that names nobody is not ignored. It resolves the way a +-/// request's `user` string does, so an empty or misspelled one refuses every start that +-/// names no user, naming the variable, instead of quietly running the workload as root. That +-/// costs no reachability: the daemon still boots, and a request that names its user still +-/// runs. A value that isn't UTF-8 is kept lossily, which no passwd name matches, so it is +-/// refused the same way rather than dropped. +-fn default_user(raw: Option) -> Option { +- raw.map(|raw| raw.to_string_lossy().into_owned()) +-} +- + /// Parses a boolean knob, accepting the spellings a Dockerfile or a Terraform + /// template actually produces. `bool::from_str` takes only `true`/`false`, so + /// `AGENTD_REPAIR_IDENTITY=0` would otherwise be ignored with a warning — and a +diff --git a/crates/agentd/src/exec.rs b/crates/agentd/src/exec.rs +index c6bd7f9..e9684f0 100644 +--- a/crates/agentd/src/exec.rs ++++ b/crates/agentd/src/exec.rs +@@ -1202,13 +1202,10 @@ fn build_command( + let launch_env = state.launch_env(); + let config = state.config(); + // Read per request rather than cached: a workload that runs `useradd` before its next +- // exec is asking for exactly that user, and the files are a few kilobytes. The image's +- // default user is read the same way, for the same reason (AGENTD-22). +- let default_user = config.default_user.as_deref(); +- let passwd = if req.user.is_some() || default_user.is_some() { +- read_database(&config.passwd_path) +- } else { +- String::new() ++ // exec is asking for exactly that user, and the files are a few kilobytes. ++ let passwd = match req.user { ++ Some(_) => read_database(&config.passwd_path), ++ None => String::new(), + }; + let group = match req.group { + Some(protocol::exec::NameOrId::Name(_)) => read_database(&config.group_path), +@@ -1221,7 +1218,6 @@ fn build_command( + image_env: state.image_env(), + passwd: &passwd, + group: &group, +- default_user, + is_executable: &is_executable_file, + }, + )?; +diff --git a/crates/agentd/src/exec_start.rs b/crates/agentd/src/exec_start.rs +index 3ddcca0..4e0d7b3 100644 +--- a/crates/agentd/src/exec_start.rs ++++ b/crates/agentd/src/exec_start.rs +@@ -19,17 +19,6 @@ + //! fuzz harness can drive. Supplementary groups are not set: `Command::groups` is still + //! unstable, and std clears them anyway when a root daemon demotes (`setgroups(0)`). + //! +-//! # A request that names no user +-//! +-//! It runs as the image's default user when the image sets `AGENTD_DEFAULT_USER`, and as the +-//! daemon's own user (root) when it doesn't (AGENTD-22). The default is resolved here, per +-//! request, to a passwd row: the row its name matches, or the row of the uid its digits +-//! spell. Either way the child takes the row's uid, primary gid and identity, so a numeric +-//! default doesn't keep the daemon's gid 0. A default with no row, digits included, is +-//! refused as `unknown_user` with a detail that names the variable (AGENTD-24). A request's +-//! own `user` always wins over the default, `root` and `0` included (AGENTD-23): the default +-//! decides what a caller gets by not saying, not what a caller may ask for. +-//! + //! # The environment is five layers, lowest first + //! + //! 1. The image environment, only when the request sets `inherit_image_env`: what the +@@ -63,8 +52,6 @@ use protocol::exec::{ + NameOrId, Shell, StartRequest, + }; + +-use crate::config::DEFAULT_USER_VAR; +- + /// A child's environment, or one layer of it. + pub type Env = HashMap; + +@@ -172,8 +159,6 @@ pub struct Guest<'a> { + pub passwd: &'a str, + /// The group file's text. Empty when the file is absent. + pub group: &'a str, +- /// The image's default user (`Config::default_user`), for a request that names none. +- pub default_user: Option<&'a str>, + /// Whether a path is an executable regular file. The real check is + /// [`is_executable_file`]; tests and the fuzz harness hand in a fake filesystem. + pub is_executable: &'a dyn Fn(&Path) -> bool, +@@ -216,10 +201,8 @@ impl Plan { + struct User<'a> { + uid: u32, + row: Option<&'a PasswdRow>, +- /// Takes the row's primary gid when the request names no group: a name the request +- /// gave that matched a row, or the image's default user. A uid the request gave, as an +- /// integer or as digits, keeps the daemon's group (AGENTD-16). +- primary_gid: bool, ++ /// Given as a name that matched a row, as opposed to a number. ++ named: bool, + } + + /// All ASCII digits that fit a `u32`: a string read as an id when it names no row. +@@ -238,21 +221,21 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, + NameOrId::Id(uid) => Ok(User { + uid: *uid, + row: by_uid(*uid), +- primary_gid: false, ++ named: false, + }), + NameOrId::Name(name) => { + if let Some(row) = rows.iter().find(|row| row.name == *name) { + return Ok(User { + uid: row.uid, + row: Some(row), +- primary_gid: true, ++ named: true, + }); + } + match digits(name) { + Some(uid) => Ok(User { + uid, + row: by_uid(uid), +- primary_gid: false, ++ named: false, + }), + None => Err(Rejection::new( + ERROR_UNKNOWN_USER, +@@ -263,56 +246,6 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, + } + } + +-/// The image's default user as its passwd row (AGENTD-22): the row its name matches, or the +-/// row of the uid its digits spell. +-/// +-/// Stricter than a request's `user`, where digits with no row run as a bare uid. The default +-/// runs every start that names no user, and a uid with no row has no primary gid to give it, +-/// so the child would keep the daemon's gid 0. A default with no row is refused instead +-/// (AGENTD-24). The error is the clause a refusal's detail ends with, naming the value and +-/// the variable that set it, because the caller sent no name to look for and the fix is in +-/// the image. +-fn default_user_row<'a>(name: &str, rows: &'a [PasswdRow]) -> Result<&'a PasswdRow, String> { +- match resolve_user(&NameOrId::Name(name.to_string()), rows) { +- Ok(User { row: Some(row), .. }) => Ok(row), +- Ok(User { row: None, .. }) => Err(format!( +- "the default user {name:?} that {DEFAULT_USER_VAR} sets is a uid with no row in \ +- the guest's /etc/passwd (the image needs a passwd row for it)" +- )), +- Err(_) => Err(format!( +- "the default user {name:?} that {DEFAULT_USER_VAR} sets is not in the guest's \ +- /etc/passwd" +- )), +- } +-} +- +-/// The user a request resolves to: its own `user` when it names one (AGENTD-23), else the +-/// image's default user's row (AGENTD-22), else none, which is the daemon's own user. +-/// +-/// A default with no row is `unknown_user` like a request's unknown name (AGENTD-24), with +-/// a detail that says the request named no user and which variable set the one that failed. +-fn resolve_request_user<'a>( +- request: &StartRequest, +- default_user: Option<&str>, +- rows: &'a [PasswdRow], +-) -> Result>, Rejection> { +- match (&request.user, default_user) { +- (Some(spec), _) => resolve_user(spec, rows).map(Some), +- (None, Some(name)) => match default_user_row(name, rows) { +- Ok(row) => Ok(Some(User { +- uid: row.uid, +- row: Some(row), +- primary_gid: true, +- })), +- Err(why) => Err(Rejection::new( +- ERROR_UNKNOWN_USER, +- format!("the request names no user, and {why}"), +- )), +- }, +- (None, None) => Ok(None), +- } +-} +- + /// A group by gid or name (AGENTD-7); an unmatched name is `unknown_group` (AGENTD-8). + fn resolve_group(spec: &NameOrId, rows: &[GroupRow]) -> Result { + match spec { +@@ -388,12 +321,16 @@ pub fn resolve_shell( + /// environment's `PATH`), so the first field that cannot be resolved names the error. The + /// empty-argv check comes last, as it did in `build_command` before this module existed. + pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result { +- let passwd = if request.user.is_some() || guest.default_user.is_some() { ++ let passwd = if request.user.is_some() { + parse_passwd(guest.passwd) + } else { + Vec::new() + }; +- let user = resolve_request_user(request, guest.default_user, &passwd)?; ++ let user = request ++ .user ++ .as_ref() ++ .map(|spec| resolve_user(spec, &passwd)) ++ .transpose()?; + + let groups = match &request.group { + Some(NameOrId::Name(_)) => parse_group(guest.group), +@@ -401,12 +338,11 @@ pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result Some(resolve_group(spec, &groups)?), +- // A named user takes its row's primary group, the way `su` and `docker -u name` do, +- // and so does the default user, however the image spelled it (AGENTD-22). A uid the +- // request gives keeps the daemon's group, which is what protocol 1 did (AGENTD-16). ++ // A named user takes its row's primary group, the way `su` and `docker -u name` do. ++ // A numeric uid keeps the daemon's group, which is what protocol 1 did (AGENTD-16). + None => user + .as_ref() +- .filter(|user| user.primary_gid) ++ .filter(|user| user.named) + .and_then(|user| user.row) + .map(|row| row.gid), + }; +diff --git a/crates/agentd/src/hook_handlers.rs b/crates/agentd/src/hook_handlers.rs +index 1549ac9..b30f398 100644 +--- a/crates/agentd/src/hook_handlers.rs ++++ b/crates/agentd/src/hook_handlers.rs +@@ -24,9 +24,7 @@ + //! + //! A handler is image-owned code and runs as the daemon's user (root), with the + //! daemon's environment plus the launch environment and `AGENTD_HOOK=`. The +-//! agent token is never in either. `AGENTD_DEFAULT_USER` doesn't apply: it names who a +-//! start request runs as when its caller names nobody, and a handler has no caller, only +-//! the image that put it there. The hook routes are unauthenticated and reachable ++//! agent token is never in either. The hook routes are unauthenticated and reachable + //! over loopback from inside the guest, so any process in the VM can make a handler + //! run: handlers must be safe to run spuriously. The hook log's cap bounds how many + //! times that can happen. +diff --git a/crates/microvms-app/src/agents/mod.rs b/crates/microvms-app/src/agents/mod.rs +index 333dfae..6ca3bcc 100644 +--- a/crates/microvms-app/src/agents/mod.rs ++++ b/crates/microvms-app/src/agents/mod.rs +@@ -492,9 +492,7 @@ pub fn provisioning_files( + /// Places the provisioning files and hands `/workspace` to the agent uid (AGENT-5). + /// + /// The daemon writes every upload as root, so the `chown` is not optional and it runs +-/// as root, after the uploads, in one exec. Root is asked for as uid 0 rather than by naming +-/// no user, because an image that sets `AGENTD_DEFAULT_USER` runs a start that names no user +-/// as that user, and the `chown` would fail as anyone else. Re-runnable: a second call ++/// as the daemon's own user, after the uploads, in one exec. Re-runnable: a second call + /// with a fresh token overwrites the same files, which is how a 12-hour token is + /// refreshed on a long-lived VM (AGENT-8). + pub async fn install_access( +@@ -512,7 +510,6 @@ pub async fn install_access( + vec![format!("chown -R {AGENT_UID}:{AGENT_GID} {WORKDIR}")], + ) + .with_shell(true) +- .with_user(Some(crate::session::ROOT_UID.into())) + .with_timeout_sec(Some(CHOWN_TIMEOUT.as_secs_f64())); + let result = session.run_sync(request, CHOWN_TIMEOUT).await?; + if !result.succeeded() { +diff --git a/crates/microvms-app/src/session/mod.rs b/crates/microvms-app/src/session/mod.rs +index e83657c..16a4ab8 100644 +--- a/crates/microvms-app/src/session/mod.rs ++++ b/crates/microvms-app/src/session/mod.rs +@@ -79,13 +79,6 @@ pub const DEFAULT_READY_TIMEOUT: Duration = DEFAULT_BOOTSTRAP_TIMEOUT; + /// How often to re-poll health while waiting for bootstrap. + const READY_POLL_INTERVAL: Duration = Duration::from_secs(2); + +-/// The uid this crate's own execs that need root ask for by number. +-/// +-/// Naming no user isn't asking for root: a daemon whose image sets `AGENTD_DEFAULT_USER` +-/// runs such a start as that user. A user the request names always wins, and an integer is +-/// what every daemon release accepts. +-pub(crate) const ROOT_UID: u32 = 0; +- + /// The transport: a backend, the agent token, and the proxy auth every request needs. + /// + /// Separate from [`Session`] because [`ExecHandle`] needs it and holding a whole session +diff --git a/crates/microvms-app/src/workspace.rs b/crates/microvms-app/src/workspace.rs +index 11db62c..f3d4bd5 100644 +--- a/crates/microvms-app/src/workspace.rs ++++ b/crates/microvms-app/src/workspace.rs +@@ -367,11 +367,8 @@ impl Session { + .map(String::from) + .chain(doomed) + .collect(); +- // As root, by uid: the daemon wrote the tree as root, and an image's default user +- // would be refused deleting it. + let request = protocol::exec::StartRequest::new(self.mint_exec_id(), command) + .with_cwd(Some(REMOTE_WORKDIR.into())) +- .with_user(Some(crate::session::ROOT_UID.into())) + .with_timeout_sec(Some(delete_timeout.as_secs_f64())); + let result = self.run_sync(request, delete_wait(delete_timeout)).await?; + match result.outcome { +diff --git a/crates/microvms-cli/src/cli.rs b/crates/microvms-cli/src/cli.rs +index 7a4dc66..f3a820e 100644 +--- a/crates/microvms-cli/src/cli.rs ++++ b/crates/microvms-cli/src/cli.rs +@@ -978,7 +978,7 @@ pub struct RunArgs { + pub launch_env: Vec<(String, String)>, + + /// User to run --exec's command as: a name or a numeric uid. Omitted runs as the +- /// image's AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. ++ /// daemon's own user. + /// + /// Resolved by the daemon against the guest's `/etc/passwd`, the only place the answer + /// exists; see `exec --user`. +@@ -1353,8 +1353,8 @@ pub struct ExecArgs { + #[arg(long, value_name = "KEY=VALUE", value_parser = parse_env_pair)] + pub env: Vec<(String, String)>, + +- /// User to run the command as: a name or a numeric uid. Omitted runs as the image's +- /// AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. ++ /// User to run the command as: a name or a numeric uid. Omitted runs as the daemon's own ++ /// user. + /// + /// All digits is sent as an integer uid, anything else as a name (AGENTD-7, AGENTD-16). + /// The daemon resolves a name against the guest's `/etc/passwd` before it spawns anything and answers +diff --git a/crates/microvms-cli/src/commands/lifecycle.rs b/crates/microvms-cli/src/commands/lifecycle.rs +index bf71c6a..ef6a544 100644 +--- a/crates/microvms-cli/src/commands/lifecycle.rs ++++ b/crates/microvms-cli/src/commands/lifecycle.rs +@@ -1898,8 +1898,7 @@ pub struct StartSpec<'a> { + /// agent token must never leak into a child — so this map is not merged into anything: + /// what is here is everything the child sees. + pub env: std::collections::HashMap, +- /// The user to demote to, by name or uid. `None` runs as the image's default user, or as +- /// the daemon's own user when the image sets none. ++ /// The user to demote to, by name or uid. `None` runs as the daemon's own user. + pub user: Option, + /// The group to demote to, by name or gid. `None` keeps the daemon's own group, or a + /// named user's primary group. +@@ -1975,7 +1974,7 @@ pub fn start_request(spec: StartSpec<'_>) -> microvms_core::protocol::exec::Star + // accounts are unknowable from *any* client, and the daemon resolves a name against + // the guest's `/etc/passwd` and `/etc/group` before it spawns, answering + // `unknown_user`/`unknown_group` for one it does not have. Omission stays the default, +- // which is "run as the image's `AGENTD_DEFAULT_USER`, else as the daemon's own user". ++ // which is "run as the daemon's own user". + .with_user(spec.user) + .with_group(spec.group) + // No `with_timeout_sec`: the client-side deadline is the caller's `--timeout`, applied +diff --git a/crates/protocol/src/exec.rs b/crates/protocol/src/exec.rs +index 748189c..bba0754 100644 +--- a/crates/protocol/src/exec.rs ++++ b/crates/protocol/src/exec.rs +@@ -232,10 +232,8 @@ pub struct StartRequest { + #[serde(default)] + pub env: HashMap, + /// The user to demote to: a numeric uid, or a name resolved against the guest's +- /// `/etc/passwd`. Optional; omitted means the image's `AGENTD_DEFAULT_USER`, resolved to +- /// the passwd row its name or uid matches, with that row's primary gid, or the daemon's +- /// own user when the image sets none. A name the guest does not have, or a default with +- /// no row, answers 400 `unknown_user` before anything is spawned. ++ /// `/etc/passwd`. Optional; omitted means run as the daemon's own user. A name the ++ /// guest does not have answers 400 `unknown_user` before anything is spawned. + /// + /// A user with a passwd row, however it was given, gets `HOME`, `USER` and `LOGNAME` + /// from the row, beneath the launch environment and `env`. A user given by name also +@@ -552,8 +550,7 @@ impl StartRequest { + self + } + +- /// The user to demote to; `None` runs as the image's `AGENTD_DEFAULT_USER`, or as the +- /// daemon's own user when the image sets none. ++ /// The user to demote to; `None` runs as the daemon's own user. + pub fn with_user(mut self, user: Option) -> Self { + self.user = user; + self diff --git a/verify/guards/faults/f2p-install-access-uploads-then-chowns-as-root.patch b/verify/guards/faults/f2p-install-access-uploads-then-chowns-as-root.patch new file mode 100644 index 00000000..8ad8bb19 --- /dev/null +++ b/verify/guards/faults/f2p-install-access-uploads-then-chowns-as-root.patch @@ -0,0 +1,417 @@ +diff --git a/crates/agentd/src/config.rs b/crates/agentd/src/config.rs +index 24deb93..9ea2777 100644 +--- a/crates/agentd/src/config.rs ++++ b/crates/agentd/src/config.rs +@@ -93,25 +93,8 @@ pub struct Config { + /// The group database a start request's group name is resolved against. `/etc/group` + /// outside tests, for `passwd_path`'s reason. + pub group_path: std::path::PathBuf, +- /// The user a start request runs as when it names no `user`, from +- /// [`DEFAULT_USER_VAR`] (AGENTD-22). `None` keeps the daemon's own user, which is root. +- /// +- /// Kept as the image wrote it and resolved per request exactly as a request's `user` +- /// string is: a passwd name, or digits that name no row as a uid. Per request because a +- /// user an earlier exec created resolves on the next. A request's own `user` always wins, +- /// `root` and `0` included (AGENTD-23), so an authorized caller can still ask for root. +- /// +- /// Read from the image's environment like every other `AGENTD_*` setting, and never from +- /// a request or the launch environment: who a workload runs as when its caller doesn't +- /// say is the image author's decision. Workload hook handlers don't take it; they are +- /// image-owned code and run as the daemon's user. +- pub default_user: Option, + } + +-/// The variable naming [`Config::default_user`]. A refusal of the default names it, so an +-/// image author who misspelled the user can find where it was set. +-pub const DEFAULT_USER_VAR: &str = "AGENTD_DEFAULT_USER"; +- + /// The handler budget's ceiling: five seconds under the platform's 60 s hook limit. + pub const MAX_HOOK_HANDLER_SECS: u64 = 55; + +@@ -145,7 +128,6 @@ impl Default for Config { + hook_handler_timeout: Duration::from_secs(20), + passwd_path: std::path::PathBuf::from("/etc/passwd"), + group_path: std::path::PathBuf::from("/etc/group"), +- default_user: None, + } + } + } +@@ -193,32 +175,10 @@ impl Config { + if let Some(secs) = env_parse::("AGENTD_HOOK_HANDLER_TIMEOUT_SECS") { + cfg.hook_handler_timeout = Duration::from_secs(secs.clamp(1, MAX_HOOK_HANDLER_SECS)); + } +- cfg.default_user = default_user(std::env::var_os(DEFAULT_USER_VAR)); +- // Logged to the image's log group because `/v1/health` can't carry it yet: `Health` is +- // constructible field by field, so a new field is a breaking change to the protocol +- // crate. +- if let Some(user) = &cfg.default_user { +- tracing::info!( +- default_user = %user, +- "a start request that omits user runs as {DEFAULT_USER_VAR}" +- ); +- } + cfg + } + } + +-/// [`DEFAULT_USER_VAR`] as the daemon keeps it: set means set, an empty value included. +-/// +-/// Unlike the numeric knobs, a value that names nobody is not ignored. It resolves the way a +-/// request's `user` string does, so an empty or misspelled one refuses every start that +-/// names no user, naming the variable, instead of quietly running the workload as root. That +-/// costs no reachability: the daemon still boots, and a request that names its user still +-/// runs. A value that isn't UTF-8 is kept lossily, which no passwd name matches, so it is +-/// refused the same way rather than dropped. +-fn default_user(raw: Option) -> Option { +- raw.map(|raw| raw.to_string_lossy().into_owned()) +-} +- + /// Parses a boolean knob, accepting the spellings a Dockerfile or a Terraform + /// template actually produces. `bool::from_str` takes only `true`/`false`, so + /// `AGENTD_REPAIR_IDENTITY=0` would otherwise be ignored with a warning — and a +diff --git a/crates/agentd/src/exec.rs b/crates/agentd/src/exec.rs +index c6bd7f9..e9684f0 100644 +--- a/crates/agentd/src/exec.rs ++++ b/crates/agentd/src/exec.rs +@@ -1202,13 +1202,10 @@ fn build_command( + let launch_env = state.launch_env(); + let config = state.config(); + // Read per request rather than cached: a workload that runs `useradd` before its next +- // exec is asking for exactly that user, and the files are a few kilobytes. The image's +- // default user is read the same way, for the same reason (AGENTD-22). +- let default_user = config.default_user.as_deref(); +- let passwd = if req.user.is_some() || default_user.is_some() { +- read_database(&config.passwd_path) +- } else { +- String::new() ++ // exec is asking for exactly that user, and the files are a few kilobytes. ++ let passwd = match req.user { ++ Some(_) => read_database(&config.passwd_path), ++ None => String::new(), + }; + let group = match req.group { + Some(protocol::exec::NameOrId::Name(_)) => read_database(&config.group_path), +@@ -1221,7 +1218,6 @@ fn build_command( + image_env: state.image_env(), + passwd: &passwd, + group: &group, +- default_user, + is_executable: &is_executable_file, + }, + )?; +diff --git a/crates/agentd/src/exec_start.rs b/crates/agentd/src/exec_start.rs +index 3ddcca0..4e0d7b3 100644 +--- a/crates/agentd/src/exec_start.rs ++++ b/crates/agentd/src/exec_start.rs +@@ -19,17 +19,6 @@ + //! fuzz harness can drive. Supplementary groups are not set: `Command::groups` is still + //! unstable, and std clears them anyway when a root daemon demotes (`setgroups(0)`). + //! +-//! # A request that names no user +-//! +-//! It runs as the image's default user when the image sets `AGENTD_DEFAULT_USER`, and as the +-//! daemon's own user (root) when it doesn't (AGENTD-22). The default is resolved here, per +-//! request, to a passwd row: the row its name matches, or the row of the uid its digits +-//! spell. Either way the child takes the row's uid, primary gid and identity, so a numeric +-//! default doesn't keep the daemon's gid 0. A default with no row, digits included, is +-//! refused as `unknown_user` with a detail that names the variable (AGENTD-24). A request's +-//! own `user` always wins over the default, `root` and `0` included (AGENTD-23): the default +-//! decides what a caller gets by not saying, not what a caller may ask for. +-//! + //! # The environment is five layers, lowest first + //! + //! 1. The image environment, only when the request sets `inherit_image_env`: what the +@@ -63,8 +52,6 @@ use protocol::exec::{ + NameOrId, Shell, StartRequest, + }; + +-use crate::config::DEFAULT_USER_VAR; +- + /// A child's environment, or one layer of it. + pub type Env = HashMap; + +@@ -172,8 +159,6 @@ pub struct Guest<'a> { + pub passwd: &'a str, + /// The group file's text. Empty when the file is absent. + pub group: &'a str, +- /// The image's default user (`Config::default_user`), for a request that names none. +- pub default_user: Option<&'a str>, + /// Whether a path is an executable regular file. The real check is + /// [`is_executable_file`]; tests and the fuzz harness hand in a fake filesystem. + pub is_executable: &'a dyn Fn(&Path) -> bool, +@@ -216,10 +201,8 @@ impl Plan { + struct User<'a> { + uid: u32, + row: Option<&'a PasswdRow>, +- /// Takes the row's primary gid when the request names no group: a name the request +- /// gave that matched a row, or the image's default user. A uid the request gave, as an +- /// integer or as digits, keeps the daemon's group (AGENTD-16). +- primary_gid: bool, ++ /// Given as a name that matched a row, as opposed to a number. ++ named: bool, + } + + /// All ASCII digits that fit a `u32`: a string read as an id when it names no row. +@@ -238,21 +221,21 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, + NameOrId::Id(uid) => Ok(User { + uid: *uid, + row: by_uid(*uid), +- primary_gid: false, ++ named: false, + }), + NameOrId::Name(name) => { + if let Some(row) = rows.iter().find(|row| row.name == *name) { + return Ok(User { + uid: row.uid, + row: Some(row), +- primary_gid: true, ++ named: true, + }); + } + match digits(name) { + Some(uid) => Ok(User { + uid, + row: by_uid(uid), +- primary_gid: false, ++ named: false, + }), + None => Err(Rejection::new( + ERROR_UNKNOWN_USER, +@@ -263,56 +246,6 @@ fn resolve_user<'a>(spec: &NameOrId, rows: &'a [PasswdRow]) -> Result, + } + } + +-/// The image's default user as its passwd row (AGENTD-22): the row its name matches, or the +-/// row of the uid its digits spell. +-/// +-/// Stricter than a request's `user`, where digits with no row run as a bare uid. The default +-/// runs every start that names no user, and a uid with no row has no primary gid to give it, +-/// so the child would keep the daemon's gid 0. A default with no row is refused instead +-/// (AGENTD-24). The error is the clause a refusal's detail ends with, naming the value and +-/// the variable that set it, because the caller sent no name to look for and the fix is in +-/// the image. +-fn default_user_row<'a>(name: &str, rows: &'a [PasswdRow]) -> Result<&'a PasswdRow, String> { +- match resolve_user(&NameOrId::Name(name.to_string()), rows) { +- Ok(User { row: Some(row), .. }) => Ok(row), +- Ok(User { row: None, .. }) => Err(format!( +- "the default user {name:?} that {DEFAULT_USER_VAR} sets is a uid with no row in \ +- the guest's /etc/passwd (the image needs a passwd row for it)" +- )), +- Err(_) => Err(format!( +- "the default user {name:?} that {DEFAULT_USER_VAR} sets is not in the guest's \ +- /etc/passwd" +- )), +- } +-} +- +-/// The user a request resolves to: its own `user` when it names one (AGENTD-23), else the +-/// image's default user's row (AGENTD-22), else none, which is the daemon's own user. +-/// +-/// A default with no row is `unknown_user` like a request's unknown name (AGENTD-24), with +-/// a detail that says the request named no user and which variable set the one that failed. +-fn resolve_request_user<'a>( +- request: &StartRequest, +- default_user: Option<&str>, +- rows: &'a [PasswdRow], +-) -> Result>, Rejection> { +- match (&request.user, default_user) { +- (Some(spec), _) => resolve_user(spec, rows).map(Some), +- (None, Some(name)) => match default_user_row(name, rows) { +- Ok(row) => Ok(Some(User { +- uid: row.uid, +- row: Some(row), +- primary_gid: true, +- })), +- Err(why) => Err(Rejection::new( +- ERROR_UNKNOWN_USER, +- format!("the request names no user, and {why}"), +- )), +- }, +- (None, None) => Ok(None), +- } +-} +- + /// A group by gid or name (AGENTD-7); an unmatched name is `unknown_group` (AGENTD-8). + fn resolve_group(spec: &NameOrId, rows: &[GroupRow]) -> Result { + match spec { +@@ -388,12 +321,16 @@ pub fn resolve_shell( + /// environment's `PATH`), so the first field that cannot be resolved names the error. The + /// empty-argv check comes last, as it did in `build_command` before this module existed. + pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result { +- let passwd = if request.user.is_some() || guest.default_user.is_some() { ++ let passwd = if request.user.is_some() { + parse_passwd(guest.passwd) + } else { + Vec::new() + }; +- let user = resolve_request_user(request, guest.default_user, &passwd)?; ++ let user = request ++ .user ++ .as_ref() ++ .map(|spec| resolve_user(spec, &passwd)) ++ .transpose()?; + + let groups = match &request.group { + Some(NameOrId::Name(_)) => parse_group(guest.group), +@@ -401,12 +338,11 @@ pub fn plan(request: &StartRequest, guest: &Guest<'_>) -> Result Some(resolve_group(spec, &groups)?), +- // A named user takes its row's primary group, the way `su` and `docker -u name` do, +- // and so does the default user, however the image spelled it (AGENTD-22). A uid the +- // request gives keeps the daemon's group, which is what protocol 1 did (AGENTD-16). ++ // A named user takes its row's primary group, the way `su` and `docker -u name` do. ++ // A numeric uid keeps the daemon's group, which is what protocol 1 did (AGENTD-16). + None => user + .as_ref() +- .filter(|user| user.primary_gid) ++ .filter(|user| user.named) + .and_then(|user| user.row) + .map(|row| row.gid), + }; +diff --git a/crates/agentd/src/hook_handlers.rs b/crates/agentd/src/hook_handlers.rs +index 1549ac9..b30f398 100644 +--- a/crates/agentd/src/hook_handlers.rs ++++ b/crates/agentd/src/hook_handlers.rs +@@ -24,9 +24,7 @@ + //! + //! A handler is image-owned code and runs as the daemon's user (root), with the + //! daemon's environment plus the launch environment and `AGENTD_HOOK=`. The +-//! agent token is never in either. `AGENTD_DEFAULT_USER` doesn't apply: it names who a +-//! start request runs as when its caller names nobody, and a handler has no caller, only +-//! the image that put it there. The hook routes are unauthenticated and reachable ++//! agent token is never in either. The hook routes are unauthenticated and reachable + //! over loopback from inside the guest, so any process in the VM can make a handler + //! run: handlers must be safe to run spuriously. The hook log's cap bounds how many + //! times that can happen. +diff --git a/crates/microvms-app/src/agents/mod.rs b/crates/microvms-app/src/agents/mod.rs +index 333dfae..6ca3bcc 100644 +--- a/crates/microvms-app/src/agents/mod.rs ++++ b/crates/microvms-app/src/agents/mod.rs +@@ -492,9 +492,7 @@ pub fn provisioning_files( + /// Places the provisioning files and hands `/workspace` to the agent uid (AGENT-5). + /// + /// The daemon writes every upload as root, so the `chown` is not optional and it runs +-/// as root, after the uploads, in one exec. Root is asked for as uid 0 rather than by naming +-/// no user, because an image that sets `AGENTD_DEFAULT_USER` runs a start that names no user +-/// as that user, and the `chown` would fail as anyone else. Re-runnable: a second call ++/// as the daemon's own user, after the uploads, in one exec. Re-runnable: a second call + /// with a fresh token overwrites the same files, which is how a 12-hour token is + /// refreshed on a long-lived VM (AGENT-8). + pub async fn install_access( +@@ -512,7 +510,6 @@ pub async fn install_access( + vec![format!("chown -R {AGENT_UID}:{AGENT_GID} {WORKDIR}")], + ) + .with_shell(true) +- .with_user(Some(crate::session::ROOT_UID.into())) + .with_timeout_sec(Some(CHOWN_TIMEOUT.as_secs_f64())); + let result = session.run_sync(request, CHOWN_TIMEOUT).await?; + if !result.succeeded() { +diff --git a/crates/microvms-app/src/session/mod.rs b/crates/microvms-app/src/session/mod.rs +index e83657c..16a4ab8 100644 +--- a/crates/microvms-app/src/session/mod.rs ++++ b/crates/microvms-app/src/session/mod.rs +@@ -79,13 +79,6 @@ pub const DEFAULT_READY_TIMEOUT: Duration = DEFAULT_BOOTSTRAP_TIMEOUT; + /// How often to re-poll health while waiting for bootstrap. + const READY_POLL_INTERVAL: Duration = Duration::from_secs(2); + +-/// The uid this crate's own execs that need root ask for by number. +-/// +-/// Naming no user isn't asking for root: a daemon whose image sets `AGENTD_DEFAULT_USER` +-/// runs such a start as that user. A user the request names always wins, and an integer is +-/// what every daemon release accepts. +-pub(crate) const ROOT_UID: u32 = 0; +- + /// The transport: a backend, the agent token, and the proxy auth every request needs. + /// + /// Separate from [`Session`] because [`ExecHandle`] needs it and holding a whole session +diff --git a/crates/microvms-app/src/workspace.rs b/crates/microvms-app/src/workspace.rs +index 11db62c..f3d4bd5 100644 +--- a/crates/microvms-app/src/workspace.rs ++++ b/crates/microvms-app/src/workspace.rs +@@ -367,11 +367,8 @@ impl Session { + .map(String::from) + .chain(doomed) + .collect(); +- // As root, by uid: the daemon wrote the tree as root, and an image's default user +- // would be refused deleting it. + let request = protocol::exec::StartRequest::new(self.mint_exec_id(), command) + .with_cwd(Some(REMOTE_WORKDIR.into())) +- .with_user(Some(crate::session::ROOT_UID.into())) + .with_timeout_sec(Some(delete_timeout.as_secs_f64())); + let result = self.run_sync(request, delete_wait(delete_timeout)).await?; + match result.outcome { +diff --git a/crates/microvms-cli/src/cli.rs b/crates/microvms-cli/src/cli.rs +index 7a4dc66..f3a820e 100644 +--- a/crates/microvms-cli/src/cli.rs ++++ b/crates/microvms-cli/src/cli.rs +@@ -978,7 +978,7 @@ pub struct RunArgs { + pub launch_env: Vec<(String, String)>, + + /// User to run --exec's command as: a name or a numeric uid. Omitted runs as the +- /// image's AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. ++ /// daemon's own user. + /// + /// Resolved by the daemon against the guest's `/etc/passwd`, the only place the answer + /// exists; see `exec --user`. +@@ -1353,8 +1353,8 @@ pub struct ExecArgs { + #[arg(long, value_name = "KEY=VALUE", value_parser = parse_env_pair)] + pub env: Vec<(String, String)>, + +- /// User to run the command as: a name or a numeric uid. Omitted runs as the image's +- /// AGENTD_DEFAULT_USER, or as the daemon's own user when the image sets none. ++ /// User to run the command as: a name or a numeric uid. Omitted runs as the daemon's own ++ /// user. + /// + /// All digits is sent as an integer uid, anything else as a name (AGENTD-7, AGENTD-16). + /// The daemon resolves a name against the guest's `/etc/passwd` before it spawns anything and answers +diff --git a/crates/microvms-cli/src/commands/lifecycle.rs b/crates/microvms-cli/src/commands/lifecycle.rs +index bf71c6a..ef6a544 100644 +--- a/crates/microvms-cli/src/commands/lifecycle.rs ++++ b/crates/microvms-cli/src/commands/lifecycle.rs +@@ -1898,8 +1898,7 @@ pub struct StartSpec<'a> { + /// agent token must never leak into a child — so this map is not merged into anything: + /// what is here is everything the child sees. + pub env: std::collections::HashMap, +- /// The user to demote to, by name or uid. `None` runs as the image's default user, or as +- /// the daemon's own user when the image sets none. ++ /// The user to demote to, by name or uid. `None` runs as the daemon's own user. + pub user: Option, + /// The group to demote to, by name or gid. `None` keeps the daemon's own group, or a + /// named user's primary group. +@@ -1975,7 +1974,7 @@ pub fn start_request(spec: StartSpec<'_>) -> microvms_core::protocol::exec::Star + // accounts are unknowable from *any* client, and the daemon resolves a name against + // the guest's `/etc/passwd` and `/etc/group` before it spawns, answering + // `unknown_user`/`unknown_group` for one it does not have. Omission stays the default, +- // which is "run as the image's `AGENTD_DEFAULT_USER`, else as the daemon's own user". ++ // which is "run as the daemon's own user". + .with_user(spec.user) + .with_group(spec.group) + // No `with_timeout_sec`: the client-side deadline is the caller's `--timeout`, applied +diff --git a/crates/protocol/src/exec.rs b/crates/protocol/src/exec.rs +index 748189c..bba0754 100644 +--- a/crates/protocol/src/exec.rs ++++ b/crates/protocol/src/exec.rs +@@ -232,10 +232,8 @@ pub struct StartRequest { + #[serde(default)] + pub env: HashMap, + /// The user to demote to: a numeric uid, or a name resolved against the guest's +- /// `/etc/passwd`. Optional; omitted means the image's `AGENTD_DEFAULT_USER`, resolved to +- /// the passwd row its name or uid matches, with that row's primary gid, or the daemon's +- /// own user when the image sets none. A name the guest does not have, or a default with +- /// no row, answers 400 `unknown_user` before anything is spawned. ++ /// `/etc/passwd`. Optional; omitted means run as the daemon's own user. A name the ++ /// guest does not have answers 400 `unknown_user` before anything is spawned. + /// + /// A user with a passwd row, however it was given, gets `HOME`, `USER` and `LOGNAME` + /// from the row, beneath the launch environment and `env`. A user given by name also +@@ -552,8 +550,7 @@ impl StartRequest { + self + } + +- /// The user to demote to; `None` runs as the image's `AGENTD_DEFAULT_USER`, or as the +- /// daemon's own user when the image sets none. ++ /// The user to demote to; `None` runs as the daemon's own user. + pub fn with_user(mut self, user: Option) -> Self { + self.user = user; + self diff --git a/verify/spec/agentd.symspec.json b/verify/spec/agentd.symspec.json index 003d858e..b2ef40e4 100644 --- a/verify/spec/agentd.symspec.json +++ b/verify/spec/agentd.symspec.json @@ -143,6 +143,25 @@ "verificationMethod": "test", "verifies": [] }, + "4e5a190c-e8f7-42c8-a839-556390df52ef": { + "createdAt": "2026-10-12T16:35:15.580Z", + "derives": [], + "id": "4e5a190c-e8f7-42c8-a839-556390df52ef", + "key": "AGENTD-23", + "negated": false, + "patternType": "optional-feature", + "preCondition": "the image environment sets the default user", + "priority": "critical", + "refines": [], + "satisfies": [], + "sentence": "Where the image environment sets the default user, the agentd shall demote the child of a start request that names its user to the named user rather than the default user.", + "status": "approved", + "systemName": "agentd", + "systemResponse": "demote the child of a start request that names its user to the named user rather than the default user", + "updatedAt": "2026-10-12T16:35:15.580Z", + "verificationMethod": "test", + "verifies": [] + }, "52611cba-ebc5-4e49-836c-b50fcf4227cf": { "createdAt": "2026-09-24T21:09:26.751Z", "derives": [], @@ -218,6 +237,25 @@ "verificationMethod": "test", "verifies": [] }, + "63daa046-6a0e-4c15-9f1a-2f830345d8e3": { + "createdAt": "2026-10-12T16:35:16.173Z", + "derives": [], + "id": "63daa046-6a0e-4c15-9f1a-2f830345d8e3", + "key": "AGENTD-24", + "negated": false, + "patternType": "unwanted-behavior", + "priority": "critical", + "refines": [], + "satisfies": [], + "sentence": "If a start request names no user and the guest's /etc/passwd does not contain the default user, then the agentd shall answer Bad Request with the unknown_user error naming the default user setting and spawn no child.", + "status": "approved", + "systemName": "agentd", + "systemResponse": "answer Bad Request with the unknown_user error naming the default user setting and spawn no child", + "trigger": "a start request names no user and the guest's /etc/passwd does not contain the default user", + "updatedAt": "2026-10-12T16:35:16.173Z", + "verificationMethod": "test", + "verifies": [] + }, "64c3f6b9-b3e1-459f-95db-969638de56e6": { "createdAt": "2026-08-05T17:40:49.854Z", "derives": [], @@ -350,6 +388,25 @@ "verificationMethod": "test", "verifies": [] }, + "d9e700f3-1ff4-4f97-975a-4c62102edd67": { + "createdAt": "2026-10-12T16:35:15.039Z", + "derives": [], + "id": "d9e700f3-1ff4-4f97-975a-4c62102edd67", + "key": "AGENTD-22", + "negated": false, + "patternType": "optional-feature", + "preCondition": "the image environment sets the default user", + "priority": "high", + "refines": [], + "satisfies": [], + "sentence": "Where the image environment sets the default user, the agentd shall resolve the default user against the guest's /etc/passwd as the user of each start request that names no user.", + "status": "approved", + "systemName": "agentd", + "systemResponse": "resolve the default user against the guest's /etc/passwd as the user of each start request that names no user", + "updatedAt": "2026-10-12T16:35:15.039Z", + "verificationMethod": "test", + "verifies": [] + }, "db931bc9-7cf8-473d-8db1-59409b935ce2": { "createdAt": "2026-09-30T12:00:00.000Z", "derives": [], diff --git a/verify/spec/traced/AGENTD.toml b/verify/spec/traced/AGENTD.toml index 0be41729..7c9bd9b7 100644 --- a/verify/spec/traced/AGENTD.toml +++ b/verify/spec/traced/AGENTD.toml @@ -61,6 +61,21 @@ issue = "#226" [AGENTD-16] issue = "#224" +# The default user, AGENTD-22 to AGENTD-24, is read from the image's environment at daemon +# start, so a live check needs an image of its own that sets it: the live suite's images set +# none, and setting one there would change who every other live exec runs as. +[AGENTD-22] +issue = "#595" +waive.live = "the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier spawn real children under a configured default" + +[AGENTD-23] +issue = "#595" +waive.live = "the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier spawn real children under a configured default" + +[AGENTD-24] +issue = "#595" +waive.live = "the live suite's images set no AGENTD_DEFAULT_USER, and one that did would change who every other live exec runs as; the handler tests and the Gherkin tier refuse a start under a default the guest does not have" + # The tunnel's keys, AGENTD-17 to AGENTD-21 here and BIND-21, BIND-23 and BIND-24 in BIND.toml, # share their gherkin waiver, so a change to one is a change to all of them. crates/model/src/ # tunnel.rs carries their model layer, and the tunnel_fuzz harnesses in agentd and the edges