linux: pass an over-long bwrap profile through --args - #504
linux: pass an over-long bwrap profile through --args#504ronleizrowice-ant wants to merge 13 commits into
Conversation
The wrapped command is one `sh -c` argument, and Linux caps a single argv element at 128 KiB (MAX_ARG_STRLEN), so a profile with enough mounts failed every spawn with an opaque E2BIG before the shell started. When the rendered line would exceed the cap, the string form hands the options to bwrap through `--args 3 -- <shell> -c <script> 3<file`, NUL-separated in a temporary file that is removed with the other per-command artifacts; bwrap reads and closes the fd while parsing, before the command starts. Below the cap the command line is unchanged, so callers need no change either way.
wrapCommandWithSandboxLinux takes concrete paths; glob expansion happens in SandboxManager, so the pattern the test passed matched nothing and no mask was emitted on Linux. Pass the created files themselves.
…file ro-binds A plain file under tmpdir could be rewritten by a running sandbox before bwrap read it. The directory is created on the process's first wrap (inside the try, re-created if tmpdir is cleaned, kept across reset), is the last ro-bind of every Linux profile, and the rendered string unlinks the file once the shell has opened it. The suite pins the file's contents, the exact 128 KiB boundary in bytes, cleanup, and a run under bwrap.
The 2000-mount sandbox took longer than bun's 5 s default on the x86-64 runner, so the synchronous spawn was cut off with a null status. Fewer, longer-named masks still overflow one argument, the runner's tmpdir is scanned one level deep, and the case gets a 60 s budget.
…annot fit one argument dash takes single-digit redirection fds only, and low fds belong to the embedder (an extra stdio pipe, or a helper binary handed over as /proc/self/fd/N), so an fd-3 redirect could take away a descriptor the command needs. fd 9 is used (8 when seccompConfig.applyPath names 9). A command still past 128 KiB with the options moved out now fails at wrap time with the sizes named instead of E2BIG at spawn; the args directory is re-created with a warning if a tmpdir cleaner removed it; the unlink uses `command rm -f --`.
34aeb6e to
5bbd064
Compare
…er fd check The overflow branch names the directory returned by the ensureBwrapArgsDir() call made before the filesystem arguments were generated — the one this profile ro-binds — instead of calling it a second time, and the fd collision test reads the optional match in one expression.
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
The per-process bwrapArgsDir is mounted read-only into every sandbox, but read-only still exposes its contents. A long-running earlier sandbox can watch/list that directory and read a later invocation's pending profile before bwrap opens/unlinks it; those files include that later invocation's --setenv values and policy. The comment compares this to a sandbox seeing its own env/cmdline, but this is cross-invocation disclosure. Could the profile be handed through an already-open fd/memfd or otherwise not be shared readably between sandboxes?
One conflict in src/sandbox/linux-sandbox-utils.ts: main (anthropics#505) and this branch each added top-level declarations at the same spot (capabilityArgs and the --args helpers); both are kept.
…mand-size cap to the kernel The --args directory reaches generateFilesystemArgs as a required parameter and is ro-bound unconditionally, instead of a skip-if-unset read of module state. Rendering the invocation (inline, or through the file) moves into renderBwrapInvocation, out of the wrap function. When the directory has been removed under the process (an age-based tmpdir clean), a fresh one is still made so profiles that fit the command line keep starting, but an over-long profile is refused while a sandbox that predates the new directory, and so never bound it, may still be running: such a sandbox could rewrite the next command's profile between the write and the spawn. The refusal lifts once no sandbox is active. The second size check is gone. MAX_ARG_STRLEN is 32 pages, so the "limit is 131071" error was false on 16 KiB and 64 KiB-page kernels and refused commands that spawned before; switching to --args at 128 KiB is harmless everywhere, and a command that alone exceeds the kernel's cap fails at spawn as it always did. The fd 9 to 8 step-aside had no caller: nothing in the tree hands the seccomp helper over on fd 9, and the README already documents fd 9 as taken. An args file is tracked before it is written, so a failed write is cleaned up. Docstrings are cut to the invariants; the README note moves to Filesystem Isolation, where it applies. Tests: one behaviour per case; the cleanup lifecycle, the over-long command and the replaced directory get their own.
|
Merged
Local run (Linux x86-64, bubblewrap 0.11): eslint and |
…mple command, and refuse what bwrap cannot run The directory is created before the first sandbox is wrapped and never a second time. A sandbox wrapped earlier would lack a new directory's bind, and the library cannot tell whether it is still running: reset() does not end sandboxes and the active count is only as exact as its caller, so the latch that waited for the count to drain could be cleared by reset(), skipped when reset() came first, or left set for good by the wrap's own catch. Now the directory is identified by device, inode, owner and mode (a same-path replacement was accepted by existsSync), and once it cannot be created or is found removed or replaced, profiles stop binding it and an over-long one is refused, with the reason, until the process restarts. A profile that fits no longer fails because tmpdir is unusable. The directory is bound by its resolved path, since bwrap before 0.12 cannot follow an absolute symlink in a bind destination, and its name starts with a dot, so `rm -rf "$TMPDIR"/*` inside a sandbox does not trip on the mount point. The bind is pushed at the call site; generateFilesystemArgs is as it was. Past the cap the string was a brace group that exec'd the caller's shell: a prefix was a syntax error, a suffix was skipped, csh could not run it, and `command rm` was looked up in PATH by the unsandboxed shell. It is now `/bin/sh -c '<open, unlink, exec>' srt-args <file> bwrap ...`, a simple command like the inline form, with rm from /usr/bin or /bin. Only the mounts go to the file, which is readable in the process's other sandboxes; the environment and the command stay on the line, with `--args` at the mounts' position. The cap is 32 pages, read from AT_PAGESZ, so a 64 KiB-page kernel keeps its 2 MiB, and the switch leaves 4 KiB for a prefix of the caller's own. A command too long for one argument by itself, a profile past bwrap's 9000 arguments, and a mount path containing NUL (which bwrap would split into several options) fail at wrap time with an error, before a file is written. Tests: every case fails on the previous commit. Process-wide state (a replaced directory, one that vanishes mid-wrap, an unusable or symlinked tmpdir) runs in a fresh process; the end-to-end case runs the string between `timeout 60` and `&& echo AFTER`. The bwrap capability probe moves to test/helpers.
…d what does not protect it The note now states that a pending file is readable in the process's other sandboxes, that the directory shows up as an undeletable entry under a writable tmpdir, that it is created once and an over-long profile is refused until restart when it is gone, and the two cases the read-only bind does not cover (another process's sandbox, and a tmpdir below a writable directory) with what closes them.
|
Second round on top of The replaced-directory logic is gone, not patched. The earlier version re-created the Callers that never overflow are no longer affected by the directory. An unwritable or missing The over-long rendering stays a simple command: Fail-closed parity with the inline path: a mount path containing NUL is refused (it would split into several options in the file); bubblewrap's 9000-word cap is checked at wrap time on both paths instead of surfacing as the sandboxed command's own failure; a command too long by itself throws before any file is written. Only the mounts go to the file. The environment and the command stay on the command line with Twelve new cases fail on |
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Thanks for keeping the environment and command out of the shared profile file; that addresses the secret-value exposure I raised. I still see a policy-integrity blocker on fe87913.
The returned wrapper opens the args file by pathname at execution time, while directoryIdentity() checks it earlier, during wrapping. As the new README notes, a sandbox that can replace a writable ancestor of TMPDIR, or another runtime process's writable tmpdir, can substitute the pending profile before that open. A read-only bind in this process's sandbox does not protect those cases.
Please enforce a safe-storage precondition and refuse the file fallback when it cannot be met, or pass the profile through an immutable/open descriptor. A wrap-then-replace-then-execute regression should verify that replacement is rejected rather than consumed. This is a source review; I have not run a Linux reproduction.
…file The file the wrapper opened at execution time was reachable by name. A sandbox that can rename a writable directory above TMPDIR, or one launched by another process of the same user with that tmpdir writable, could put its own profile at that name between the wrap and the open: the read-only bind this process placed on the directory covered neither case, and the identity check ran at wrap time, before the window it had to cover. The mounts now go to an unnamed file: O_TMPFILE under os.tmpdir(), or /dev/shm when that takes none. The process keeps the descriptor open and the string reopens it through /proc/<pid>/fd/<n>, a fresh read-only description of the same inode at offset 0. Nothing is named at any point, so there is nothing to substitute; reaching the descriptor needs the wrapping process's own /proc entry, and every sandbox this library starts has --unshare-pid and a fresh --proc /proc, so none of them sees it. That ends the exposure the directory came with as well — a pending profile was readable in the process's other sandboxes — along with the bind every profile carried for it, the identity check, the latch that refused over-long profiles until restart, and the rm in the wrapper. When no directory takes an unnamed file, or the profile cannot be opened again through /proc, an over-long profile is refused at wrap time with the reason. There is no fallback to a named file: such a profile failed with E2BIG before any of this existed, so refusing takes nothing away. Descriptors are released where the files were removed before — when the last sandbox of a batch is cleaned up, and at exit. A string run after that fails in the redirection and never reaches the command.
The case the transport exists for: a wrap in a fresh process whose TMPDIR sits under a scratch directory, then that directory renamed aside and a profile binding / read-write planted at every path the string could still name, then the string run. The sandbox stays under the profile it was wrapped with — its write is still denied — and the string names only /proc/<pid>/fd/<n>, with nothing of ours left under TMPDIR. Also: the wrap-time refusal when neither candidate directory takes an O_TMPFILE file; one descriptor per pending profile, all of them back at cleanup and none held by a rendering that was refused; a string run after its cleanup running no command; and, end to end, a sandboxed command that reaches neither fd 9 nor the wrapping process's /proc. Dropped with the directory they pinned: the replaced-directory and vanishing-directory cases, the file left by a wrap that was never spawned, the trailing read-only bind, and tmpdir behind an absolute symlink — no path is bound now, so there is nothing for bubblewrap to resolve.
What carries the mounts now and why nothing can be put in its place; the one thing that is not covered, another process of the same user outside a sandbox reading a pending profile through /proc, which it could already get by reading this process's memory; that the string must be run while the wrapping process is alive and before the runtime cleans up after that command; and what an over-long profile needs in order to be accepted at all.
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Rechecked current 016a6687. The pathname-substitution blocker is resolved: the over-long mount profile now lives in an unnamed O_TMPFILE, stays held by the wrapping process, and is reopened through /proc/<pid>/fd/<n> rather than by a replaceable pathname. The new tests cover the unnamed profile path. The lone macOS-arm64 CI failure is in an unrelated proxy-attribution test; Linux x86-64/arm64 and Docker jobs are green. No remaining blocker from my review.
Problem.
wrapCommandWithSandboxLinuxreturns one string that the embedder runs as a single argument ofsh -c, and Linux caps one argument at 32 pages (128 KiB with 4 KiB pages). A profile with enough mounts (a checkout with committed read-deny globs, a tree with hundreds of nested repositories) goes past that, and then every sandboxed command fails at spawn withE2BIG, whatever the command is.Change. When the rendered string would come within 4 KiB of the cap (read from
AT_PAGESZ, so 64 KiB-page kernels keep their 2 MiB), the mounts are written NUL-separated to an unnamed file and bwrap reads them through--argsat the same position. The string becomes/bin/sh -c 'exec 9<"$1" && shift && exec "$@"' srt-args /proc/<wrapping pid>/fd/<n> bwrap … --args 9 … -- <shell> -c <cmd>: still a simple command, so a prefix (exec,timeout 30) or a suffix (&& next) composes as it does for the inline form. The environment and the command stay on the command line; only mount paths go to the file.The profile has no name. It is created with
O_TMPFILEunderos.tmpdir()(else/dev/shm), written, set to 0400 and kept open by the wrapping process./bin/shopens it again through/proc/<pid>/fd/<n>— a fresh read-only description of the same inode at offset 0 — and bwrap reads it to EOF and closes fd 9 before the command starts. Because the file is never linked into a directory, nothing can be put in its place between the wrap and the open: not a command this process sandboxed, not a sandbox another process of the same user started with tmpdir writable, not a rename of a directory aboveTMPDIR. Reaching it needs/proc/<wrapping pid>, and every sandbox this library starts runs with--unshare-pidand a fresh--proc /proc, so no sandbox sees that directory at all. Nothing is bound into profiles for this any more, and nothing is left on disk to clean up.Refusals. If neither
os.tmpdir()nor/dev/shmtakes anO_TMPFILEfile, or the profile cannot be opened again through/proc, an over-long profile is refused at wrap time with the reason; there is no fallback to a named file, and such a profile failed withE2BIGbefore this change. bwrap parses at most 9000 arguments: a profile past that, a mount path containing NUL, or a command too long for one argument by itself also fails at wrap time with an error instead of at spawn or inside bwrap.Limits, stated in the README. Another process of the same user, running outside a sandbox, can read a pending profile through
/proc; it can already read this process's memory, so that is not a new capability. The string must be run while the wrapping process is alive and before the runtime cleans up after that command, which is when the descriptor is released.Blast radius. Profiles that fit the command line render exactly as before — no file, no descriptor, no extra mount. Larger profiles failed at spawn before. Not behind a flag.
Follow-up. The size limit exists because the argv is flattened into one string. Returning
[bwrap, ...args]fromwrapWithSandboxArgvon Linux would leave only the 2 MiB total and bwrap's 9000 words, with no file, fd or shell wrapper at all.Tested.
bun test test/sandbox/linux-bwrap-args-file.test.tson Linux x86-64 with bubblewrap 0.11.2 and unprivileged user namespaces: the exact switch point, the profile's contents and that the string names only/proc/<pid>/fd/<n>(and that the file is unlinked), one descriptor per pending profile with all of them released at cleanup — a refused rendering included — a string run after its cleanup running no command, the NUL, 9000-argument and over-long-command refusals, and, in a fresh process with both candidate directories read-only, the wrap-time refusal with its reason. End to end: a profile past the cap applies its masks withtimeout 60 <string> && echo AFTERaround it and the sandboxed command reaches neither fd 9 nor the wrapping process's/proc; and a wrap whoseTMPDIRparent is renamed aside, with a profile binding/read-write planted at every path the string could still name, still runs under the profile it was wrapped with. The rest of the suite is unchanged frommain.