diff --git a/src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl b/src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl index d950dae8c..aa1f36c7d 100644 --- a/src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl +++ b/src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl @@ -5,15 +5,28 @@ ; Static write paths for temporary files. ; These are needed by many tools and libraries for temporary storage. ; -; Box shared directory and user volumes are added dynamically by seatbelt.rs. +; Box shared directory, box-scoped tmp, and user volumes are added dynamically +; by seatbelt.rs. ; ; Why these paths are needed: -; - /private/tmp: Standard temporary directory (macOS canonical path) -; - /private/var/tmp: Alternative temporary directory +; - /private/tmp: Standard temporary directory (macOS canonical path). +; Required for the short binding-symlink parent `/tmp/bl-{uid}/{box_id}` +; (see src/boxlite/src/net/socket_path.rs — moved here in #742) and any +; tools using TempDir::new_in("/tmp"). +; - /private/var/tmp: Alternative temporary directory used by some libs. ; -; NOT allowed: +; NOT allowed (intentionally narrower than older revisions): +; - /private/var/folders ($TMPDIR / DARWIN_USER_*_DIR): the shim's TMPDIR is +; redirected to the box-scoped tmp dir by configure_env() in +; src/boxlite/src/vmm/controller/spawn.rs whenever the built-in seatbelt +; profile is in effect, so the whole per-user temp tree never needs to be +; writable. Granting it would cover the entire per-user temp/cache tree +; (browser caches, app sqlite, token caches, …) for a dependency the shim +; no longer has. If you re-add a write grant here, scope it to a single +; subpath (e.g. via `sandbox-exec -D` like DARWIN_USER_CACHE_DIR), not the +; full tree. ; - ~/.boxlite (could affect other boxes, image cache) -; - {box_dir} entirely (only shared/ subdirectory, added dynamically) +; - {box_dir} entirely (only shared/ and tmp/ subdirectories, added dynamically) ; - System paths ; ; ============================================================================ @@ -24,9 +37,6 @@ ; Using canonical paths (/private/tmp instead of /tmp symlink) (subpath "/private/tmp") (subpath "/private/var/tmp") - ; /private/var/folders: Per-user temp directory ($TMPDIR on macOS) - ; gvproxy creates Unix sockets here for VM networking - (subpath "/private/var/folders") ) ; ============================================================================ @@ -34,5 +44,7 @@ ; ============================================================================ ; Dynamic write paths added by seatbelt.rs: ; - {box_dir}/shared/ (guest-visible directory) +; - {box_dir}/tmp/ (box-scoped TMPDIR for shim + libkrun transient files) +; - {box_dir}/sockets/ + /tmp/bl-{uid}/{box_id}/ (gvproxy + control sockets) ; - User volumes with read_only=false ; ============================================================================ diff --git a/src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl b/src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl index 6cd6a544d..5d9ec0de7 100644 --- a/src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl +++ b/src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl @@ -23,6 +23,21 @@ ; NETWORK OPERATIONS ; ============================================================================ ; Allow all network connections (needed for gvproxy) +; +; TODO(security): `(allow network-inbound)` is broader than required. gvproxy +; only needs inbound on host ports listed in the box's EXPOSE/port-mappings; +; everything else could be denied. Implementation is non-trivial: +; +; - port list is per-box runtime data → cannot use `sandbox-exec -D` (string +; substitution doesn't fit inside `(local tcp "*:N")`); must be built into +; the policy string by seatbelt.rs from NetworkBackendConfig.final_mappings +; before the shim is spawned. +; - SBPL `network-bind` vs `network-inbound` semantics must be pinned down: +; bind(2) is `network-bind`, accept(2)/UDP receive is `network-inbound`. +; Gvproxy needs both for forwarded ports and may also bind ephemeral ports +; for the NAT return path — validate before tightening or EXPOSE silently +; breaks. +; - Must run real macOS e2e with EXPOSE before merging. (allow network-outbound) (allow network-inbound) (allow system-socket) diff --git a/src/boxlite/src/jailer/sandbox/seatbelt.rs b/src/boxlite/src/jailer/sandbox/seatbelt.rs index b2efc69d6..d1f060286 100644 --- a/src/boxlite/src/jailer/sandbox/seatbelt.rs +++ b/src/boxlite/src/jailer/sandbox/seatbelt.rs @@ -522,6 +522,30 @@ mod tests { assert!(SEATBELT_FILE_WRITE_POLICY.contains("(subpath \"/private/var/tmp\")")); } + /// The static write policy must NOT grant the whole per-user temp/cache + /// tree (`/private/var/folders`, a.k.a. `$TMPDIR` / `DARWIN_USER_*_DIR`). + /// The shim's TMPDIR is redirected to a box-scoped path by + /// `configure_env()` in `vmm/controller/spawn.rs`, so no blanket grant is + /// needed under the built-in profile. If you re-add a grant here, scope it + /// to a single subpath (`-D` parameter), not the whole tree. + #[test] + fn test_file_write_policy_excludes_per_user_temp_tree() { + // Match the grant form, not bare mentions in doc comments. SBPL + // comments start with `;` so we look for `(subpath "..."` which is + // unambiguously a live allow expression. + for grant in [ + "(subpath \"/private/var/folders\")", + "(subpath \"/private/var/folders/", + ] { + assert!( + !SEATBELT_FILE_WRITE_POLICY.contains(grant), + "static file-write policy must not grant `{grant}`; \ + shim TMPDIR is redirected to box-scoped tmp by \ + vmm/controller/spawn.rs::configure_env" + ); + } + } + #[test] fn test_dynamic_read_paths_empty() { let binary_path = PathBuf::from("/usr/local/bin/boxlite-shim");