From 54cbcb2f8267396448b9c5cc2deeb1ea5fdc0c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20K=C4=99ska?= <372403+keskad@users.noreply.github.com> Date: Wed, 5 Aug 2026 21:42:55 +0200 Subject: [PATCH 1/3] feat: expose daemon mode, IPC allowlist, and initgroups for runAsUser Report init vs supervise in DaemonInfo, let configured users reach the control socket, and preserve supplementary groups when dropping privileges. Co-authored-by: Cursor --- docs/api.md | 1 + docs/architecture.md | 3 +- docs/configuration.md | 4 +++ docs/operator.md | 7 +++- docs/sdk/golang.md | 3 +- go/client/client.go | 45 +++++++++++++++++--------- src/cli.rs | 1 + src/config.rs | 48 +++++++++++++++++++++++++++ src/init.rs | 8 +++++ src/ipc.rs | 70 +++++++++++++++++++++++++++++++++------- src/protocol.rs | 30 +++++++++++++++++ src/security.rs | 38 +++++++++++++++++----- src/supervisor.rs | 5 +++ tests/ipc_test.rs | 1 + tests/protocol_test.rs | 1 + tests/supervisor_test.rs | 3 ++ 16 files changed, 230 insertions(+), 38 deletions(-) diff --git a/docs/api.md b/docs/api.md index a6c800a..7fd6019 100644 --- a/docs/api.md +++ b/docs/api.md @@ -59,6 +59,7 @@ Daemon build/runtime snapshot: version (ELF release section or `dev`), build com | `hostname` | Host name | | `uptime_secs` | Seconds since supervisor start | | `socket` | IPC socket path | +| `mode` | `init` (machine reboot/poweroff) or `supervise` (process exit only) | | `services_total` | Registered services | | `services_running` | Services in `running` state | | `otel_enabled` | Effective telemetry on/off | diff --git a/docs/architecture.md b/docs/architecture.md index 1e8c9eb..73e0c07 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -141,7 +141,8 @@ Constraints for a normal Google Play app sandbox: notification) — a hidden background daemon violates Play policy. - Bind the control socket under **app-private storage** (e.g. `getFilesDir()`), never `/run/…`. Pass `--socket` / JSON `socket` accordingly. Same-UID clients only - (`SO_PEERCRED` + `0600`). + (`SO_PEERCRED`; socket `0600` by default, or `0660` + allowlisted uids when + `socketAllowUsers` is set). - Service shell is `/system/bin/sh` with an Android `PATH`. - Ordered shutdown **stops services, syncs, and exits** — no `reboot(2)` and no BusyBox `/sbin/*` fallback. diff --git a/docs/configuration.md b/docs/configuration.md index 5645c04..e67b5cd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -199,6 +199,10 @@ microinit list Requires **microinit restart** (on PID 1 hosts: reboot): - `socket` path +- `socketAllowUsers` — optional list of login names allowed to connect to the + control socket in addition to the daemon uid (resolved via passwd at load; + unknown names abort config). When non-empty, the socket is `0660` owned by + `root:` (typically `bigfred` on the hub). - `logs.*` (TTYs, `logToFiles`, buffer size) - `console` diff --git a/docs/operator.md b/docs/operator.md index 051977a..5108347 100644 --- a/docs/operator.md +++ b/docs/operator.md @@ -172,7 +172,12 @@ HTTP / TCP examples: | `runAsGroup` | Group name **or** numeric gid; optional when the user has a passwd entry (defaults to primary gid). **Required** for numeric uids with no passwd entry | | `capabilities` | Linux capability names (`CAP_` prefix optional). The list is **exclusive** (replaces the parent's capability set; it is not additive) | -Supplementary groups are cleared with `setgroups([])` (fail-closed). Environments that deny `setgroups` (e.g. user namespaces with `/proc/self/setgroups=deny`) cannot use `runAsUser`/`runAsGroup`. +Supplementary groups come from **`initgroups(3)`** using the passwd username +when `runAsUser` resolves to a named account (so memberships in `/etc/group`, +e.g. `dialout`, apply). Numeric uids without a passwd entry still use +`setgroups([])` (fail-closed — no inherited root groups). Environments that +deny `setgroups` (e.g. user namespaces with `/proc/self/setgroups=deny`) cannot +use `runAsUser`/`runAsGroup`. After the drop, microinit also: - shrinks the capability **bounding set** to the requested caps (or empty), diff --git a/docs/sdk/golang.md b/docs/sdk/golang.md index cba291d..7fe3dc0 100644 --- a/docs/sdk/golang.md +++ b/docs/sdk/golang.md @@ -180,7 +180,8 @@ err = config.WriteDropin(dropinDir, "infra", "redis", svc) | `List()` | All services | | `Status(name)` | One service | | `Control(name, start\|stop\|restart)` | Lifecycle | -| `Shutdown()` | Halt-mode shutdown (IPC) | +| `Shutdown()` | Halt-mode shutdown (IPC); alias of `ShutdownMode("halt")` | +| `ShutdownMode(mode)` | Shutdown with `reboot` \| `poweroff` \| `halt` | | `FollowLogs` / `ReadResponse` | Log stream | | `ValidateName` / `FormatLogLine` | Helpers | diff --git a/go/client/client.go b/go/client/client.go index 748da81..65eda37 100644 --- a/go/client/client.go +++ b/go/client/client.go @@ -37,21 +37,22 @@ type ServiceStatus struct { // DaemonInfo mirrors microinit IPC `info` response. type DaemonInfo struct { - Version string `json:"version"` - TagCommit string `json:"tag_commit"` - BuildCommit string `json:"build_commit"` - BuildTime string `json:"build_time"` - PID uint32 `json:"pid"` - Hostname string `json:"hostname"` - UptimeSecs uint64 `json:"uptime_secs"` - Socket string `json:"socket"` - ServicesTotal int `json:"services_total"` - ServicesRunning int `json:"services_running"` - OtelEnabled bool `json:"otel_enabled"` - OtelEndpoint string `json:"otel_endpoint"` - OtelProtocol string `json:"otel_protocol"` - OtelServiceName string `json:"otel_service_name"` - OtelExportIntervalSecs uint64 `json:"otel_export_interval_secs"` + Version string `json:"version"` + TagCommit string `json:"tag_commit"` + BuildCommit string `json:"build_commit"` + BuildTime string `json:"build_time"` + PID uint32 `json:"pid"` + Hostname string `json:"hostname"` + UptimeSecs uint64 `json:"uptime_secs"` + Socket string `json:"socket"` + Mode string `json:"mode"` // "init" | "supervise" + ServicesTotal int `json:"services_total"` + ServicesRunning int `json:"services_running"` + OtelEnabled bool `json:"otel_enabled"` + OtelEndpoint string `json:"otel_endpoint"` + OtelProtocol string `json:"otel_protocol"` + OtelServiceName string `json:"otel_service_name"` + OtelExportIntervalSecs uint64 `json:"otel_export_interval_secs"` } // LogLine is one captured log line from microinit. @@ -213,8 +214,20 @@ func (c *Client) Control(name, action string) error { // Shutdown requests a halt-mode shutdown (used when stopping a supervise // instance started by the caller). func (c *Client) Shutdown() error { + return c.ShutdownMode("halt") +} + +// ShutdownMode requests a machine/process shutdown with mode reboot|poweroff|halt. +// In supervise mode microinit ignores the machine power aspect and exits after +// stopping services; in init mode it finalizes via reboot(2). +func (c *Client) ShutdownMode(mode string) error { + switch mode { + case "reboot", "poweroff", "halt": + default: + return fmt.Errorf("invalid shutdown mode %q (want reboot|poweroff|halt)", mode) + } var resp Response - if err := c.roundTrip(request{Type: "shutdown", Mode: "halt"}, &resp); err != nil { + if err := c.roundTrip(request{Type: "shutdown", Mode: mode}, &resp); err != nil { return err } switch resp.Type { diff --git a/src/cli.rs b/src/cli.rs index c28142b..1650ae8 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -101,6 +101,7 @@ fn print_daemon_info(info: &DaemonInfo) { print!(" {}", info.build_time); } println!(); + println!("Mode: {}", info.mode); println!("PID: {}", info.pid); println!("Hostname: {}", info.hostname); println!("Uptime: {}", format_uptime(Some(info.uptime_secs))); diff --git a/src/config.rs b/src/config.rs index 6edcda1..2b4ebe2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -461,6 +461,10 @@ pub struct Config { pub socket: String, #[serde(default = "default_console")] pub console: String, + /// Extra Unix-socket peer uids allowed besides the daemon's own uid. + /// Login names resolved against `/etc/passwd` at load time (fail-closed). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub socket_allow_users: Vec, #[serde(default)] pub open_telemetry: OpenTelemetryConfig, #[serde(default)] @@ -486,6 +490,7 @@ impl Default for Config { logs: LogsConfig::default(), socket: default_socket(), console: default_console(), + socket_allow_users: Vec::new(), open_telemetry: OpenTelemetryConfig::default(), services: Vec::new(), } @@ -603,9 +608,51 @@ impl Config { } } } + // Fail-closed: unknown names in socketAllowUsers abort config load. + let _ = self.resolved_ipc_allow()?; Ok(()) } + /// Resolve [`Self::socket_allow_users`] into IPC peer allowlist + socket group. + /// + /// Empty allowlist → only the daemon uid may connect; socket stays `0600`. + /// Non-empty → those uids (plus daemon uid) may connect; socket is `0660` + /// owned by `root:`. + pub fn resolved_ipc_allow(&self) -> Result { + use nix::unistd::{Gid, Group, User}; + let mut allow_uids = Vec::new(); + let mut socket_gid: Option = None; + for raw in &self.socket_allow_users { + let name = raw.trim(); + if name.is_empty() { + return Err(Error::Config( + "socketAllowUsers entry must not be empty".into(), + )); + } + let u = User::from_name(name) + .map_err(|e| Error::Config(format!("socketAllowUsers lookup '{name}': {e}")))? + .ok_or_else(|| { + Error::Config(format!("socketAllowUsers: unknown user '{name}'")) + })?; + allow_uids.push(u.uid.as_raw()); + if socket_gid.is_none() { + // Prefer the user's primary group name matching the login when + // present (e.g. bigfred:bigfred); otherwise use passwd gid. + let gid = Group::from_name(name) + .ok() + .flatten() + .map(|g| g.gid.as_raw()) + .unwrap_or_else(|| u.gid.as_raw()); + let _ = Gid::from_raw(gid); + socket_gid = Some(gid); + } + } + Ok(crate::ipc::IpcAllow { + allow_uids, + socket_gid, + }) + } + pub fn get_mut(&mut self, name: &str) -> Option<&mut ServiceConfig> { self.services.iter_mut().find(|s| s.name == name) } @@ -791,6 +838,7 @@ pub fn example_config() -> Config { }, socket: DEFAULT_SOCKET.to_string(), console: DEFAULT_CONSOLE.to_string(), + socket_allow_users: Vec::new(), open_telemetry: OpenTelemetryConfig::default(), services: vec![ ServiceConfig { diff --git a/src/init.rs b/src/init.rs index 9170c15..4200f36 100644 --- a/src/init.rs +++ b/src/init.rs @@ -204,6 +204,12 @@ pub fn run(opts: InitOpts) -> Result<()> { let config_path = opts.paths.config.clone(); let dropins_dir = opts.paths.dropins_dir.clone(); + let mode = if opts.machine_shutdown { + crate::protocol::DaemonMode::Init + } else { + crate::protocol::DaemonMode::Supervise + }; + let ipc_allow = cfg.resolved_ipc_allow()?; let supervisor = Supervisor::new( cfg, hub.clone(), @@ -211,6 +217,7 @@ pub fn run(opts: InitOpts) -> Result<()> { override_path, config_path, dropins_dir, + mode, ); let sup = Arc::clone(&supervisor); @@ -218,6 +225,7 @@ pub fn run(opts: InitOpts) -> Result<()> { ipc::serve( Path::new(&socket_path), Arc::new(move |req, stream| handle_ipc(req, stream, &sup, &hub_ipc, lines_default)), + ipc_allow, )?; hub.emit_init(LogLevel::Info, format!("IPC listening on {socket_path}")); diff --git a/src/ipc.rs b/src/ipc.rs index dd434e7..221fb5b 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -91,12 +91,24 @@ pub fn request(socket_path: &Path, req: &Request) -> Result { read_frame(&mut stream) } -/// Peer credential check: peer uid must match the daemon's uid (root when PID 1). -fn peer_allowed(stream: &UnixStream) -> bool { +/// Peer allowlist for the control socket (from `socketAllowUsers`). +#[derive(Debug, Clone, Default)] +pub struct IpcAllow { + /// Extra uids allowed besides the daemon's own uid. + pub allow_uids: Vec, + /// When set with a non-empty allowlist: socket mode `0660`, owner `root:gid`. + pub socket_gid: Option, +} + +/// Peer credential check: daemon uid, or an entry in `allow_uids`. +fn peer_allowed(stream: &UnixStream, allow_uids: &[u32]) -> bool { use nix::sys::socket::{getsockopt, sockopt::PeerCredentials}; use nix::unistd::Uid; match getsockopt(stream, PeerCredentials) { - Ok(cred) => cred.uid() == Uid::current().as_raw(), + Ok(cred) => { + let uid = cred.uid(); + uid == Uid::current().as_raw() || allow_uids.contains(&uid) + } Err(_) => false, } } @@ -107,7 +119,11 @@ pub type Handler = Arc Result<()> + Send + S /// /// Concurrent handlers are capped at [`MAX_IPC_CLIENTS`]; excess clients receive /// an immediate error response. -pub fn serve(socket_path: &Path, handler: Handler) -> Result<()> { +/// +/// When `allow.allow_uids` is non-empty, the socket is immediately set to +/// `0660` and `chown`ed to `root:` (fail-closed if gid missing). +/// Otherwise the socket stays `0600` (daemon-uid-only). +pub fn serve(socket_path: &Path, handler: Handler, allow: IpcAllow) -> Result<()> { if let Some(parent) = socket_path.parent() { if !parent.as_os_str().is_empty() { std::fs::create_dir_all(parent).map_err(|e| Error::io_at(parent, e))?; @@ -119,19 +135,15 @@ pub fn serve(socket_path: &Path, handler: Handler) -> Result<()> { Err(e) => return Err(Error::io_at(socket_path, e)), } let listener = UnixListener::bind(socket_path).map_err(|e| Error::io_at(socket_path, e))?; - use std::os::unix::fs::PermissionsExt; - let mut perms = std::fs::metadata(socket_path) - .map_err(|e| Error::io_at(socket_path, e))? - .permissions(); - perms.set_mode(0o600); - std::fs::set_permissions(socket_path, perms).map_err(|e| Error::io_at(socket_path, e))?; + apply_socket_perms(socket_path, &allow)?; let path = socket_path.to_path_buf(); + let allow_uids = allow.allow_uids; thread::spawn(move || { for conn in listener.incoming() { match conn { Ok(mut stream) => { - if !peer_allowed(&stream) { + if !peer_allowed(&stream, &allow_uids) { let _ = write_frame( &mut stream, &Response::Error { @@ -181,3 +193,39 @@ pub fn serve(socket_path: &Path, handler: Handler) -> Result<()> { }); Ok(()) } + +fn apply_socket_perms(socket_path: &Path, allow: &IpcAllow) -> Result<()> { + use std::os::unix::fs::PermissionsExt; + if allow.allow_uids.is_empty() { + let mut perms = std::fs::metadata(socket_path) + .map_err(|e| Error::io_at(socket_path, e))? + .permissions(); + perms.set_mode(0o600); + std::fs::set_permissions(socket_path, perms).map_err(|e| Error::io_at(socket_path, e))?; + return Ok(()); + } + let gid = allow.socket_gid.ok_or_else(|| { + Error::Config( + "socketAllowUsers set but no socket group could be resolved".into(), + ) + })?; + // chmod + chown immediately after bind — no window with 0600 for allowlisted peers. + let mut perms = std::fs::metadata(socket_path) + .map_err(|e| Error::io_at(socket_path, e))? + .permissions(); + perms.set_mode(0o660); + std::fs::set_permissions(socket_path, perms).map_err(|e| Error::io_at(socket_path, e))?; + use nix::unistd::{chown, Gid, Uid}; + chown( + socket_path, + Some(Uid::from_raw(0)), + Some(Gid::from_raw(gid)), + ) + .map_err(|e| { + Error::io_at( + socket_path, + std::io::Error::new(std::io::ErrorKind::PermissionDenied, e), + ) + })?; + Ok(()) +} diff --git a/src/protocol.rs b/src/protocol.rs index bf3e2ee..97e3e47 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -160,6 +160,34 @@ pub struct ServiceDescribe { pub source: Option, } +/// Daemon run mode reported by `microinit info` / `Request::Info`. +/// +/// - `init` — full PID-1 path (`machine_shutdown`); reboot/poweroff/halt finalize +/// - `supervise` — stop services and exit; machine power modes are ignored +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +#[non_exhaustive] +pub enum DaemonMode { + Init, + Supervise, +} + +impl DaemonMode { + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Init => "init", + Self::Supervise => "supervise", + } + } +} + +impl std::fmt::Display for DaemonMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + /// Daemon snapshot for `microinit info` / `Request::Info`. #[non_exhaustive] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -172,6 +200,8 @@ pub struct DaemonInfo { pub hostname: String, pub uptime_secs: u64, pub socket: String, + /// `init` or `supervise` (from boot opts, not JSON config). + pub mode: DaemonMode, pub services_total: usize, pub services_running: usize, pub otel_enabled: bool, diff --git a/src/security.rs b/src/security.rs index 45b2514..a33ed39 100644 --- a/src/security.rs +++ b/src/security.rs @@ -257,8 +257,12 @@ pub fn resolve(ctx: &SecurityContext) -> Result> { /// Apply identity in the child after fork, before exec. /// -/// Order: keepcaps → bounding-set drop → setgroups([]) → setgid → setuid → -/// capset + ambient → `PR_SET_NO_NEW_PRIVS`. +/// Order: keepcaps → bounding-set drop → initgroups (or setgroups([])) → +/// setgid → setuid → capset + ambient → `PR_SET_NO_NEW_PRIVS`. +/// +/// When a passwd username is known, [`unistd::initgroups`] installs that +/// user's supplementary groups from `/etc/group` (e.g. `bigfred` ∈ `dialout`). +/// Numeric uids without a passwd entry keep the fail-closed `setgroups([])`. /// /// # Safety /// @@ -287,15 +291,12 @@ pub fn apply_pre_exec(ident: &ResolvedIdentity) -> Result<()> { // Shrink the capability bounding set while still privileged. drop_bounding_set(&ident.caps)?; - // Fail-closed: when dropping uid/gid we must clear supplementary groups. + // Fail-closed vs inheriting the parent's (often root) supplementary groups. + // Prefer initgroups(username) so /etc/group memberships (e.g. dialout) apply. // User namespaces with `/proc/self/setgroups=deny` are unsupported for // securityContext identity drops. if drop_id { - unistd::setgroups(&[]).map_err(|e| { - Error::Security(format!( - "setgroups: {e} (required when runAsUser/runAsGroup is set)" - )) - })?; + apply_groups(ident)?; } if let Some(gid) = ident.gid { @@ -327,6 +328,27 @@ pub fn apply_pre_exec(ident: &ResolvedIdentity) -> Result<()> { Ok(()) } +fn apply_groups(ident: &ResolvedIdentity) -> Result<()> { + use std::ffi::CString; + if let (Some(ref name), Some(gid)) = (&ident.username, ident.gid) { + let cname = CString::new(name.as_str()).map_err(|_| { + Error::Security(format!("username '{name}' contains NUL")) + })?; + unistd::initgroups(&cname, Gid::from_raw(gid)).map_err(|e| { + Error::Security(format!( + "initgroups({name}, {gid}): {e} (required when runAsUser/runAsGroup is set)" + )) + })?; + return Ok(()); + } + unistd::setgroups(&[]).map_err(|e| { + Error::Security(format!( + "setgroups: {e} (required when runAsUser/runAsGroup is set)" + )) + })?; + Ok(()) +} + /// Install [`apply_pre_exec`] on `cmd` via `CommandExt::pre_exec`. pub fn attach_pre_exec(cmd: &mut std::process::Command, ident: &ResolvedIdentity) { use std::os::unix::process::CommandExt; diff --git a/src/supervisor.rs b/src/supervisor.rs index 9a41737..3ff8f81 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -286,6 +286,8 @@ pub struct Supervisor { exits: Arc, ctl: Mutex>>, started_at: Instant, + /// Boot mode (`init` vs `supervise`); fixed for process lifetime. + mode: crate::protocol::DaemonMode, } enum CtlMsg { @@ -303,6 +305,7 @@ impl Supervisor { override_path: PathBuf, config_path: PathBuf, dropins_dir: PathBuf, + mode: crate::protocol::DaemonMode, ) -> Arc { let mut runtimes = HashMap::new(); for svc in &config.services { @@ -323,6 +326,7 @@ impl Supervisor { exits: global_exits(), ctl: Mutex::new(HashMap::new()), started_at: Instant::now(), + mode, }) } @@ -735,6 +739,7 @@ impl Supervisor { hostname: crate::version::hostname(), uptime_secs: self.started_at.elapsed().as_secs(), socket, + mode: self.mode, services_total, services_running, otel_enabled: otel.enable, diff --git a/tests/ipc_test.rs b/tests/ipc_test.rs index a1c82e8..46af25a 100644 --- a/tests/ipc_test.rs +++ b/tests/ipc_test.rs @@ -96,6 +96,7 @@ fn serve_list_roundtrip() { } Ok(()) }), + IpcAllow::default(), ) .unwrap(); diff --git a/tests/protocol_test.rs b/tests/protocol_test.rs index defddd6..12e379b 100644 --- a/tests/protocol_test.rs +++ b/tests/protocol_test.rs @@ -56,6 +56,7 @@ fn request_response_serde_roundtrip() { "hostname": "hub", "uptime_secs": 60, "socket": "/data/run/microinit.sock", + "mode": "init", "services_total": 2, "services_running": 1, "otel_enabled": true, diff --git a/tests/supervisor_test.rs b/tests/supervisor_test.rs index 01d3eca..24b5a27 100644 --- a/tests/supervisor_test.rs +++ b/tests/supervisor_test.rs @@ -100,6 +100,7 @@ fn make_sup(services: Vec) -> (Arc, std::path::PathBu }, socket: dir.join("sock").to_string_lossy().into(), console: "/dev/null".into(), + socket_allow_users: Vec::new(), open_telemetry: Default::default(), services, }; @@ -115,6 +116,7 @@ fn make_sup(services: Vec) -> (Arc, std::path::PathBu override_path.clone(), config_path, dropins, + microinit::protocol::DaemonMode::Supervise, ); (sup, dir) } @@ -138,6 +140,7 @@ fn info_reports_services_and_otel() { assert_eq!(info.version, "dev"); assert!(!info.build_commit.is_empty()); assert!(info.socket.contains("sock")); + assert_eq!(info.mode, microinit::protocol::DaemonMode::Supervise); std::env::set_var("ENABLE_TELEMETRY", "true"); std::env::set_var("OTEL_EXPORTER_OTLP_ENDPOINT", "http://127.0.0.1:4318"); From fc2b4ab607ba273e6e37ec3366cf90380047f732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20K=C4=99ska?= <372403+keskad@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:09:33 +0200 Subject: [PATCH 2/3] fix: initgroups only for named runAsUser, not numeric uid strings Numeric uid specs keep the fail-closed setgroups([]) path so existing identity-drop tests and configs behave as before. Co-authored-by: Cursor --- src/security.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/security.rs b/src/security.rs index a33ed39..fddd9da 100644 --- a/src/security.rs +++ b/src/security.rs @@ -153,6 +153,8 @@ pub struct ResolvedIdentity { pub home: Option, /// Suggested `USER` / `LOGNAME` from passwd (best-effort). pub username: Option, + /// `true` when `runAsUser` was a login name (not a numeric uid string). + pub named_user: bool, } impl ResolvedIdentity { @@ -188,6 +190,7 @@ pub fn resolve(ctx: &SecurityContext) -> Result> { let mut primary_gid: Option = None; let mut home: Option = None; let mut username: Option = None; + let mut named_user = false; if let Some(ref user_spec) = ctx.run_as_user { let spec = user_spec.trim(); @@ -206,6 +209,7 @@ pub fn resolve(ctx: &SecurityContext) -> Result> { ))); } } else { + named_user = true; let u = User::from_name(spec) .map_err(|e| Error::Security(format!("lookup user '{spec}': {e}")))? .ok_or_else(|| Error::Security(format!("unknown user '{spec}'")))?; @@ -247,6 +251,7 @@ pub fn resolve(ctx: &SecurityContext) -> Result> { caps, home, username, + named_user, }; if ident.is_noop() { Ok(None) @@ -330,7 +335,8 @@ pub fn apply_pre_exec(ident: &ResolvedIdentity) -> Result<()> { fn apply_groups(ident: &ResolvedIdentity) -> Result<()> { use std::ffi::CString; - if let (Some(ref name), Some(gid)) = (&ident.username, ident.gid) { + if ident.named_user { + if let (Some(ref name), Some(gid)) = (&ident.username, ident.gid) { let cname = CString::new(name.as_str()).map_err(|_| { Error::Security(format!("username '{name}' contains NUL")) })?; @@ -339,7 +345,8 @@ fn apply_groups(ident: &ResolvedIdentity) -> Result<()> { "initgroups({name}, {gid}): {e} (required when runAsUser/runAsGroup is set)" )) })?; - return Ok(()); + return Ok(()); + } } unistd::setgroups(&[]).map_err(|e| { Error::Security(format!( From 022153bd93910ed8a0b1c912b4da3c67cd197e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20K=C4=99ska?= <372403+keskad@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:28:50 +0200 Subject: [PATCH 3/3] fix: tighten IPC allowlist, initgroups invariants, and DaemonMode tests Fail closed when named_user lacks username/gid; capture daemon_uid for peer checks and socket chown; document socketAllowUsers group ordering; document halt vs reboot/poweroff in the Go client. Co-authored-by: Cursor --- docs/configuration.md | 6 +++++- go/client/client.go | 10 ++++++++-- src/config.rs | 16 ++++++++++------ src/ipc.rs | 41 +++++++++++++++++++++++++++-------------- src/security.rs | 22 ++++++++++++++-------- tests/protocol_test.rs | 16 ++++++++++++++++ 6 files changed, 80 insertions(+), 31 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index e67b5cd..b05c9e1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -202,7 +202,11 @@ Requires **microinit restart** (on PID 1 hosts: reboot): - `socketAllowUsers` — optional list of login names allowed to connect to the control socket in addition to the daemon uid (resolved via passwd at load; unknown names abort config). When non-empty, the socket is `0660` owned by - `root:` (typically `bigfred` on the hub). + `daemon_uid:` (prefer a group matching the login, + else that user's primary gid — typically `bigfred:bigfred` on the hub). + **Order matters:** later names are allowed by uid peer-check only; they must + still be able to open a `0660` socket for that group (put the intended + socket-group owner first). - `logs.*` (TTYs, `logToFiles`, buffer size) - `console` diff --git a/go/client/client.go b/go/client/client.go index 65eda37..c751141 100644 --- a/go/client/client.go +++ b/go/client/client.go @@ -211,8 +211,10 @@ func (c *Client) Control(name, action string) error { } } -// Shutdown requests a halt-mode shutdown (used when stopping a supervise -// instance started by the caller). +// Shutdown requests a halt-mode shutdown. Prefer [Client.ShutdownMode] when +// you need reboot/poweroff (e.g. host power control). Halt is for stopping a +// supervise instance started by the caller (CLI / embedder); BigFred's admin +// HTTP API does not expose halt to the UI. func (c *Client) Shutdown() error { return c.ShutdownMode("halt") } @@ -220,6 +222,10 @@ func (c *Client) Shutdown() error { // ShutdownMode requests a machine/process shutdown with mode reboot|poweroff|halt. // In supervise mode microinit ignores the machine power aspect and exits after // stopping services; in init mode it finalizes via reboot(2). +// +// Modes: +// - reboot / poweroff — host power (init mode); used by BigFred admin UI +// - halt — process exit after stopping services; for direct SDK/CLI use only func (c *Client) ShutdownMode(mode string) error { switch mode { case "reboot", "poweroff", "halt": diff --git a/src/config.rs b/src/config.rs index 2b4ebe2..214a1b6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -617,9 +617,15 @@ impl Config { /// /// Empty allowlist → only the daemon uid may connect; socket stays `0600`. /// Non-empty → those uids (plus daemon uid) may connect; socket is `0660` - /// owned by `root:`. + /// owned by `daemon_uid:`. + /// + /// **`socket_gid` is taken from the first entry** in `socketAllowUsers`: + /// prefer a group whose name matches the login (e.g. `bigfred:bigfred`), + /// else the user's primary gid from passwd. Later entries are allowed by + /// uid check only — they must share that group (or be root) to open a + /// `0660` socket. Put the intended socket group owner first. pub fn resolved_ipc_allow(&self) -> Result { - use nix::unistd::{Gid, Group, User}; + use nix::unistd::{Group, Uid, User}; let mut allow_uids = Vec::new(); let mut socket_gid: Option = None; for raw in &self.socket_allow_users { @@ -631,9 +637,7 @@ impl Config { } let u = User::from_name(name) .map_err(|e| Error::Config(format!("socketAllowUsers lookup '{name}': {e}")))? - .ok_or_else(|| { - Error::Config(format!("socketAllowUsers: unknown user '{name}'")) - })?; + .ok_or_else(|| Error::Config(format!("socketAllowUsers: unknown user '{name}'")))?; allow_uids.push(u.uid.as_raw()); if socket_gid.is_none() { // Prefer the user's primary group name matching the login when @@ -643,11 +647,11 @@ impl Config { .flatten() .map(|g| g.gid.as_raw()) .unwrap_or_else(|| u.gid.as_raw()); - let _ = Gid::from_raw(gid); socket_gid = Some(gid); } } Ok(crate::ipc::IpcAllow { + daemon_uid: Uid::current().as_raw(), allow_uids, socket_gid, }) diff --git a/src/ipc.rs b/src/ipc.rs index 221fb5b..7838415 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -92,22 +92,34 @@ pub fn request(socket_path: &Path, req: &Request) -> Result { } /// Peer allowlist for the control socket (from `socketAllowUsers`). -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct IpcAllow { - /// Extra uids allowed besides the daemon's own uid. + /// Daemon uid (always allowed). Captured at config resolve time. + pub daemon_uid: u32, + /// Extra uids allowed besides [`Self::daemon_uid`]. pub allow_uids: Vec, - /// When set with a non-empty allowlist: socket mode `0660`, owner `root:gid`. + /// When set with a non-empty allowlist: socket mode `0660`, owner + /// `daemon_uid:socket_gid`. pub socket_gid: Option, } +impl Default for IpcAllow { + fn default() -> Self { + Self { + daemon_uid: nix::unistd::Uid::current().as_raw(), + allow_uids: Vec::new(), + socket_gid: None, + } + } +} + /// Peer credential check: daemon uid, or an entry in `allow_uids`. -fn peer_allowed(stream: &UnixStream, allow_uids: &[u32]) -> bool { +fn peer_allowed(stream: &UnixStream, daemon_uid: u32, allow_uids: &[u32]) -> bool { use nix::sys::socket::{getsockopt, sockopt::PeerCredentials}; - use nix::unistd::Uid; match getsockopt(stream, PeerCredentials) { Ok(cred) => { let uid = cred.uid(); - uid == Uid::current().as_raw() || allow_uids.contains(&uid) + uid == daemon_uid || allow_uids.contains(&uid) } Err(_) => false, } @@ -121,8 +133,8 @@ pub type Handler = Arc Result<()> + Send + S /// an immediate error response. /// /// When `allow.allow_uids` is non-empty, the socket is immediately set to -/// `0660` and `chown`ed to `root:` (fail-closed if gid missing). -/// Otherwise the socket stays `0600` (daemon-uid-only). +/// `0660` and `chown`ed to `daemon_uid:` (fail-closed if +/// gid missing). Otherwise the socket stays `0600` (daemon-uid-only). pub fn serve(socket_path: &Path, handler: Handler, allow: IpcAllow) -> Result<()> { if let Some(parent) = socket_path.parent() { if !parent.as_os_str().is_empty() { @@ -138,12 +150,13 @@ pub fn serve(socket_path: &Path, handler: Handler, allow: IpcAllow) -> Result<() apply_socket_perms(socket_path, &allow)?; let path = socket_path.to_path_buf(); + let daemon_uid = allow.daemon_uid; let allow_uids = allow.allow_uids; thread::spawn(move || { for conn in listener.incoming() { match conn { Ok(mut stream) => { - if !peer_allowed(&stream, &allow_uids) { + if !peer_allowed(&stream, daemon_uid, &allow_uids) { let _ = write_frame( &mut stream, &Response::Error { @@ -205,9 +218,7 @@ fn apply_socket_perms(socket_path: &Path, allow: &IpcAllow) -> Result<()> { return Ok(()); } let gid = allow.socket_gid.ok_or_else(|| { - Error::Config( - "socketAllowUsers set but no socket group could be resolved".into(), - ) + Error::Config("socketAllowUsers set but no socket group could be resolved".into()) })?; // chmod + chown immediately after bind — no window with 0600 for allowlisted peers. let mut perms = std::fs::metadata(socket_path) @@ -216,15 +227,17 @@ fn apply_socket_perms(socket_path: &Path, allow: &IpcAllow) -> Result<()> { perms.set_mode(0o660); std::fs::set_permissions(socket_path, perms).map_err(|e| Error::io_at(socket_path, e))?; use nix::unistd::{chown, Gid, Uid}; + // Owner is the daemon uid (usually 0 on hub), not hardcoded root — so + // non-root supervise / tests can still chown successfully. chown( socket_path, - Some(Uid::from_raw(0)), + Some(Uid::from_raw(allow.daemon_uid)), Some(Gid::from_raw(gid)), ) .map_err(|e| { Error::io_at( socket_path, - std::io::Error::new(std::io::ErrorKind::PermissionDenied, e), + std::io::Error::other(format!("chown socket: {e}")), ) })?; Ok(()) diff --git a/src/security.rs b/src/security.rs index fddd9da..2748b72 100644 --- a/src/security.rs +++ b/src/security.rs @@ -265,9 +265,10 @@ pub fn resolve(ctx: &SecurityContext) -> Result> { /// Order: keepcaps → bounding-set drop → initgroups (or setgroups([])) → /// setgid → setuid → capset + ambient → `PR_SET_NO_NEW_PRIVS`. /// -/// When a passwd username is known, [`unistd::initgroups`] installs that +/// When `runAsUser` was a **login name**, [`unistd::initgroups`] installs that /// user's supplementary groups from `/etc/group` (e.g. `bigfred` ∈ `dialout`). -/// Numeric uids without a passwd entry keep the fail-closed `setgroups([])`. +/// Numeric uid strings keep the fail-closed `setgroups([])` even if passwd +/// has a matching entry. /// /// # Safety /// @@ -336,17 +337,22 @@ pub fn apply_pre_exec(ident: &ResolvedIdentity) -> Result<()> { fn apply_groups(ident: &ResolvedIdentity) -> Result<()> { use std::ffi::CString; if ident.named_user { - if let (Some(ref name), Some(gid)) = (&ident.username, ident.gid) { - let cname = CString::new(name.as_str()).map_err(|_| { - Error::Security(format!("username '{name}' contains NUL")) - })?; + let (name, gid) = match (&ident.username, ident.gid) { + (Some(n), Some(g)) => (n, g), + _ => { + return Err(Error::Security( + "named_user set but username/gid missing (invariant violated)".into(), + )); + } + }; + let cname = CString::new(name.as_str()) + .map_err(|_| Error::Security(format!("username '{name}' contains NUL")))?; unistd::initgroups(&cname, Gid::from_raw(gid)).map_err(|e| { Error::Security(format!( "initgroups({name}, {gid}): {e} (required when runAsUser/runAsGroup is set)" )) })?; - return Ok(()); - } + return Ok(()); } unistd::setgroups(&[]).map_err(|e| { Error::Security(format!( diff --git a/tests/protocol_test.rs b/tests/protocol_test.rs index 12e379b..1a79d49 100644 --- a/tests/protocol_test.rs +++ b/tests/protocol_test.rs @@ -14,6 +14,22 @@ fn service_state_display() { ); } +#[test] +fn daemon_mode_wire_roundtrip() { + for (mode, wire) in [ + (DaemonMode::Init, "init"), + (DaemonMode::Supervise, "supervise"), + ] { + assert_eq!(mode.as_str(), wire); + assert_eq!(mode.to_string(), wire); + let json = serde_json::to_string(&mode).unwrap(); + assert_eq!(json, format!("\"{wire}\"")); + let back: DaemonMode = serde_json::from_str(&json).unwrap(); + assert_eq!(back, mode); + } + assert!(serde_json::from_str::("\"unknown\"").is_err()); +} + #[test] fn request_response_serde_roundtrip() { let cases = vec![