Repository navigation
Conversation
A nested grant's symlink check ran before bwrap re-resolved the grant's path, so a program with write access to the parent could swap in a link between them. Where bwrap lists --bind-fd and the kernel has openat2, each grant is now opened once as an O_PATH descriptor, a nested one beneath its parent with RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH, and bound with --bind-fd. Descriptors are mapped only in the child after fork (command-fds), never inheritable in the host. Elsewhere grants stay bound by path, as before, and BubblewrapBackend::binds_by_descriptor reports which is in effect.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): Bind bwrap grants from descriptors, closing the check-then-mount gap. The change closes item 1 of #104: `reject_symlinked_grants` validated a grant nested in a writable directory, but `bwrap --bind PATH` re-resolved that path, leaving a window in which a program with write access to the parent could swap a directory for a symlink between the check and the mount. Where `bwrap` lists `--bind-fd` and the kernel provides `openat2` (Linux 5.6+), each grant is opened once as an `O_PATH` descriptor and bound with `--bind-fd`/`--ro-bind-fd`, so what was checked is what is mounted; otherwise grants fall back to path binds and the documented gap remains. The latest-push-delta section is unavailable: the review state has the prior reviewed head and rendered evidence but no trusted base-to-head diff, so no push delta is reported here. Unreviewed paths
Review snapshot
Completeness: Complete What changedGrant binding moves from path re-resolution to descriptors opened at spawn time in `crates/tinybox-jail/src/bwrap/args.rs`: each jail grant is opened as an `O_PATH | O_CLOEXEC` descriptor, nested grants relative to their parent's descriptor with `RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH`, top-level grants by their declared path, and a missing read-only grant is skipped as `--ro-bind-try` would skip it. Emitted argv uses `--bind-fd`/`--ro-bind-fd` with the child fd number for grants while baseline system paths stay path-bound. Descriptors are mapped onto 10, 11, … via `command_fds::CommandFdExt::fd_mappings` only in the child after fork; the shell launcher keeps fds 10 through 10+k open and still closes every other inherited descriptor. `BubblewrapBackend::binds_by_descriptor()` and a debug log report which mode is active, gated on `bwrap --help` listing `--bind-fd` plus a working `openat2`, and the module's known-gaps documentation is updated to scope the remaining check-then-use window to builds and kernels without that support. `crates/tinybox-jail/Cargo.toml` gains `command-fds` (pinned =0.3.3) and `nix` 0.31 with the `fs` feature. Features
Tests
FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
# Conflicts: # crates/tinybox-jail/src/bwrap/launcher_tests.rs
Summary
Closes item 1 of #104: the check-then-mount gap on nested grants.
reject_symlinked_grantschecks that a grant nested in a writable directory doesn't reach its target through a symlink. Thenbwrap --bind PATH PATHresolved the path again. A program with write access to the parent, already running in another sandbox, could swap the directory for a symlink between the two steps.Where
bwrapsupports it, each grant is now opened once as anO_PATHdescriptor and bound with--bind-fd/--ro-bind-fd, so what was checked is what is mounted.RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH: no symlink on that stretch is followed.--ro-bind-tryskips it.The descriptors are mapped onto 10, 11, … only in the child after fork, using the
command-fdscrate, so they are never inheritable in the host process, where another thread could spawn at the same time. The launcher keeps them open and still closes every other inherited descriptor.Fd binds need
bwraplisting--bind-fd(bubblewrap 0.9.0 does; the code comment claiming 0.10+ was wrong) andopenat2(Linux 5.6+). Without them grants are bound by path as before.BubblewrapBackend::binds_by_descriptor()and a debug log report which mode is in effect, and the module docs describe the remaining gap for that case.Related issue
Part of #104 (item 1).
API or behavior changes
BubblewrapBackend::binds_by_descriptor().tinybox-jail:command-fds=0.3.3 (Apache-2.0, by Google), which does thedup2in apre_exechook without allocating, so the workspace keeps forbiddingunsafe;nix0.31 (fs), already in the lockfile, for a safeopenat2.ELOOP, where before it was mounted. Before, the earlier check refused it only if the link was already there.Validation
On our Linux CI runner (bubblewrap 0.9.0), at 30c2f2ddf5. The head differs only by moving the path checks at the top of
jail_args_with_fds, unchanged, intocheck_paths, to keep it under the line limit. On the head,cargo clippy --all-targets --all-features -- -D warningsreports 0 errors, the same as main.cargo build --locked --all-targets --all-features: ok. The new lockfile entry is exactly what cargo resolves.cargo deny --locked check: the new dependencies add only a duplicate-version warning fornix(0.27 is already in the tree). No licence or ban findings.cargo fmt --all -- --checkandRUSTDOCFLAGS="-D warnings" cargo doc: ok.TINYBOX_REQUIRE_BWRAP=1 cargo test -p tinybox-jail bwrap: 51 passed. The live run reportsbinds_by_descriptor=true.cargo test --workspace --no-fail-fast: 1582 passed, 0 failed, with no failures unique to this branch.Tests
--bind-fd 10 <root>and--ro-bind-fd 11 <nested>(nested after its parent), never also by path; baseline paths are still bound by path; without support everything is bound by path.a_symlink_swapped_in_after_the_check_is_not_followed: a nested grant replaced by a link out after the root was opened is refused.a_symlink_above_a_grant_is_left_to_the_host: a top-level grant spelled through a host symlink still opens.a_missing_read_only_grant_is_skipped_as_ro_bind_try_skips_it.bwrap, and an inheritable stray descriptor is still closed.bwrap_isolation):binds_by_descriptor()matches whetherbwrap --helplists--bind-fd. With 0.9.0 every live nested-grant test runs through descriptors.Documentation
The known-gaps section in
tinybox-jail/src/bwrap/mod.rs, and rustdoc on the new accessor and the builder.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description