linux: pass an over-long bwrap profile through --args - #504
linux: pass an over-long bwrap profile through --args#504ronleizrowice-ant wants to merge 10 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
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
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.
2 participants
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 a file and bwrap reads them through--argsat the same position. The string becomes/bin/sh -c 'exec 9<"$1" && /usr/bin/rm -f -- "$1" && shift && exec "$@"' srt-args <file> 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./bin/shopens the file on fd 9 (dash takes single-digit redirections only), unlinks it withrmfrom a fixed system path, and bwrap closes the fd before the command starts. The environment and the command stay on the command line; only mount paths go to the file.The file lives in a per-process directory,
<os.tmpdir()>/.srt-bwrap-args-*, that every profile the process generates binds read-only over itself, so a sandbox the process launched cannot rewrite a profile bwrap has yet to read. The directory is created once, by resolved path, and checked by device and inode on every wrap. If it cannot be created, or is later removed or replaced, profiles that fit are unaffected and an over-long one is refused with an error until the process restarts: a sandbox started earlier would lack a new directory's bind, and the library cannot tell whether it is still running.Limits, stated in the README. A pending file is readable (not writable) in the process's other sandboxes. A sandbox started by another process of the same user with tmpdir writable, or any sandbox when
os.tmpdir()lies below a writable directory, can swap a pending file; keep tmpdir out ofallowWritewhere profiles can be over-long. 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 now fails at wrap time with an error instead of at spawn or inside bwrap.Blast radius. Profiles that fit render as before plus one trailing
--ro-bindof the new directory (skipped when tmpdir is unusable); a sandbox with tmpdir writable sees that directory as an entry it cannot delete. 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 directory; a memfd in place of the file is the smaller step.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 file's contents, cleanup of a file never spawned, the NUL, 9000-argument and over-long-command refusals, and in fresh processes a replaced directory, one that vanishes mid-wrap and an unusable tmpdir. End to end: a profile past the cap applies its masks withtimeout 60 <string> && echo AFTERaround it, fd 9 is closed inside and the directory is read-only there with tmpdir writable; a tmpdir behind an absolute symlink starts under 0.11.2. The rest of the suite is unchanged frommain.