fix(net): close a sendto non-IP destination-policy TOCTOU (sendto half of #130)#154
fix(net): close a sendto non-IP destination-policy TOCTOU (sendto half of #130)#154dzerik wants to merge 4 commits into
Conversation
congwang-mk
left a comment
There was a problem hiding this comment.
The TOCTOU diagnosis is correct, and the fail-closed non-unix arm plus the pure classify_send_path helper are the right shape. Two problems in the UnixOnBehalf arm block this as written:
1. On-behalf abstract sends bypass Landlock abstract-socket scoping. The _ arm receives abstract addresses (named_unix_socket_path returns None for them; its comment even says "Landlock scope handles it"). LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET is enforced against the credentials of the process doing the sendto. Pre-PR, Continue ran the child's own syscall under the child's Landlock domain. Post-PR the unconfined supervisor performs the send, so enabling an IP destination policy deterministically disables abstract-socket scoping for unconnected sendto: the child can message any abstract socket on the host. That trades a racy IP-policy bypass for a deterministic Landlock-boundary bypass (same shape as #27).
Fix: fail closed for abstract destinations (EAFNOSUPPORT), matching what send_msghdr_on_behalf already does and what #130 aligns with anyway.
2. Named paths (fs gate off) resolve in the supervisor's context. The arm passes the raw child addr_bytes to resolve_send, so sun_path resolves against the supervisor's cwd/root instead of the child's. A relative sun_path silently targets the wrong path, and a chroot child's absolute path would resolve host-side. Every other named-unix on-behalf path resolves via /proc/<pid>/root (resolve_named_unix_target) for exactly this reason.
Fix: pin the inode via a /proc/<pid>/root resolution variant (grant check skipped) and send to /proc/self/fd/<pin>; rejecting relative sun_path with an errno is fine, resolving it against the wrong cwd is not.
Both fixes are user-visible behavior (abstract datagrams under a destination policy go from racy-working to EAFNOSUPPORT), so please note them in the behavior-scope section. No family carve-out needed for AF_VSOCK/AF_PACKET; fail-closed there is fine as flagged.
Also worth adding: a happy-path integration test that a named unix datagram still sends under a net_allow policy; that is the path both issues live on, and it would have caught the relative-path regression.
…olicy TOCTOU Splits the high-priority sendto half out of multikernel#130 (per the maintainer's request to land it as a fix that can go in quickly). A non-IP sendto with no fs-path gate previously fell through to `_ => Continue`, letting the kernel re-read `sockfd` and the destination after the supervisor's decision. Under an active destination policy a child could race a `dup2(inet_sock, sockfd)` + address swap into that window and ride the Continue out to a denied IP. Under `has_net_destination_policy` the arm no longer Continues: it pins the fd via `dup_fd_from_pid`, classifies on the STABLE socket domain via the new pure `classify_send_path` helper, and either sends the unix datagram on-behalf on the pinned fd (UnixOnBehalf) or fails closed with EAFNOSUPPORT. With no destination policy there is nothing to bypass, so Continue is preserved. The non-UnixOnBehalf outcomes collapse to a single fail-closed errno arm rather than unreachable!(), so any future drift in the classifier fails closed instead of panicking (a panic on this seccomp-notif path would unwind the supervisor and DoS the sandbox). The pure `classify_send_path` helper and its 4 unit tests land here (sendto is its first consumer); `send_path_non_ip_on_non_unix_socket_is_rejected` is the deterministic fail-without-fix witness (pre-fix logic yielded Continue). The sendmsg/sendmmsg symmetric rework consumes the same helper and stays in draft multikernel#130 — so until multikernel#130 lands, sendto and sendmsg differ on a non-IP unix datagram (sendto sends on-behalf, sendmsg still returns EAFNOSUPPORT); this preserves the pre-existing sendto behavior (it already sent via Continue) while closing its TOCTOU, and multikernel#130 aligns sendmsg. Behavior scope (fail-closed hardening, only under a destination policy): a non-IP send on a non-unix/non-IP datagram family (AF_PACKET/AF_VSOCK/...) now returns EAFNOSUPPORT instead of Continue; abstract/named-fs-gate-off unix datagrams now send on-behalf, so the receiver observes the supervisor's SO_PEERCRED rather than the child's. Same Continue-shape as the chroot AF_UNIX gate report (multikernel#143).
…in the child's context Review follow-up on the sendto non-IP arm. Moving the send from the child to the supervisor changes two things the first version did not account for: whose Landlock domain applies, and whose cwd/root resolves `sun_path`. 1. Abstract unix destinations now fail closed with EAFNOSUPPORT. LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET is enforced against the credentials of the process performing the send. Before this handler the child executed its own syscall inside its own scoped domain (Continue), so an abstract send was confined; performing it on-behalf from the unconfined supervisor would mean that enabling an IP destination policy deterministically switches abstract scoping off for unconnected sendto, letting the child reach any abstract socket on the host. `named_unix_socket_path` returns None for an abstract address (also for an empty one and for a non-UTF-8 `sun_path`), so the arm now keys on it and refuses every unix destination it cannot pin, matching `send_msghdr_on_behalf`'s `None => EAFNOSUPPORT`. That also closes the non-UTF-8 `sun_path` case, which reaches this arm even with the fs gate ON and would otherwise have skipped the fs-write grant check. 2. A named `sun_path` is resolved in the CHILD's context. The arm used to pass the child's raw sockaddr through to `resolve_send`, so the supervisor's sendmsg resolved `sun_path` against the supervisor's cwd/root: a relative path silently addressed a different socket, and an absolute path was resolved host-side. Every other named-unix on-behalf path resolves through `/proc/<pid>/root` for exactly this reason. `resolve_named_unix_target` is split into a pin-only core (`pin_child_unix_target`) and the existing grant-checking wrapper; the new `sendto_pinned_unix_on_behalf` pins the target inode through `/proc/<pid>/root` and sends to `/proc/self/fd/<pin>`, keeping the pin alive in the message. No grant check there: the arm is only reachable with `has_unix_fs_gate == false`, i.e. the sandbox declares no fs grants to check against. A relative `sun_path` is now rejected explicitly (ECONNREFUSED, the errno the malformed `/proc/<pid>/root` concatenation already produced) instead of being resolved against the supervisor's cwd. `classify_send_path` takes the destination shape as a fourth input (`named_unix_dest`) and `UnixOnBehalf` becomes `NamedUnixOnBehalf`; the abstract case is a Reject with a unit test that is its fail-without-fix witness (the same input classified as UnixOnBehalf before). `pin_child_unix_target` gets a unit test for the relative rejection plus a positive control so the rejection is not vacuous. The module-level Continue-safety note is updated: non-IP families Continue only without a destination policy. The named-unix datagram delivery test grows a destination-policy variant: under a non-empty `net_allow` the datagram must still arrive at the receiver byte-for-byte. That is the path both fixes sit on, and it is the regression guard for a handler that stops delivering (wrong sockaddr, dropped inode pin, or a grant check applied where there are no grants).
… family
The previous revision keyed the non-IP sendto arm on "did a pathname come out
of the sockaddr", which collapsed two unrelated shapes into one Reject:
a) an AF_UNIX destination with no pathname (abstract / unnamed / non-UTF-8) —
which must fail closed, and is the point of that change;
b) a NON-AF_UNIX destination on a unix socket — which is what sandlock's own
NETLINK_ROUTE virtualization produces. `handle_socket` hands the child one
end of a socketpair(AF_UNIX, SOCK_SEQPACKET) and glibc addresses it with a
sockaddr_nl, so every netlink query took EAFNOSUPPORT as soon as a
destination policy was active. Measured: `socket.if_nameindex()` returned 1
interface before, OSError(97) after. `sandlock learn` sets a destination
policy (policy_fn, plus its own net_allow rules), so the observational mode
lost netlink too.
`classify_dest_shape` now derives a `DestShape` from the destination's ADDRESS
FAMILY first: UnixNamed(path) / UnixNoPath / NotUnix. `classify_send_path` takes
it in place of the `named_unix_dest` bool and gains `RawDestOnBehalf` for (b):
send on-behalf on the already-pinned fd with the child's address bytes verbatim.
That is TOCTOU-safe for the same reason the other arms are — the fd is pinned
before its stable SO_DOMAIN is read, so no dup2 can redirect the send — and the
virtualized socket is connected, so the kernel ignores msg_name outright. For
any other non-unix family on a unix socket the kernel rejects the mismatch
itself, so nothing is widened. (a) keeps failing closed with EAFNOSUPPORT.
Tests. The delivery test added with the previous revision does not witness
either hardening change: the sandbox it builds declares fs grants, so
has_unix_fs_gate is true and the send takes the pre-existing named-unix gate arm
instead of the fall-through. Verified by mutation — with the fall-through's
named arm turned into a Reject it still passes. Its comment claimed the
opposite; it now states the narrow property it does guard, and the tests that
cover the rest are named there:
- test_abstract_unix_dgram_sendto_refused_under_destination_policy: a host
listener binds an abstract name outside the sandbox; the child's sendto must
return EAFNOSUPPORT and the listener must receive nothing. Restoring the
previous on-behalf send delivers "abstract-escape" to the host socket.
- if_nameindex_works_under_destination_policy: the netlink regression above.
- classify_dest_shape / classify_send_path unit tests for the two pathless
shapes.
- child_root_path unit tests. The relative-sun_path guard had no real test:
pin_child_unix_target returns ECONNREFUSED with or without it, because the
concatenation ("/proc/<pid>/rootsvc.dgram") fails to open anyway. The path
builder is now a pure function and both the guard and the /proc/<pid>/root
resolution context are asserted on its output, where deleting either is
visible. Without a mount namespace neither is observable from a live
sandbox, which is why no integration test can carry them.
Two claims committed with the previous revision were wrong and are corrected
rather than kept: the delivery test's "routes every sendto through the non-IP
arm" / "regression witness for the resolution context" (above), and the
module-level note in network/mod.rs saying non-IP families do not Continue under
a destination policy. Only sendto's non-chroot fall-through stops Continuing;
connect's abstract/unnamed arm and both chroot named-unix arms still Continue,
which is the residue tracked by multikernel#27 / multikernel#143.
sendto_pinned_unix_on_behalf is documented as unreachable today:
has_unix_fs_gate == false means the sandbox declares no fs grant at all and
Landlock then denies execve (verified), while Sandbox::fork installs a deny-only
filter with no notification supervisor, so no send handler runs there. It is
kept as defence in depth, with the reachability argument written down.
All three are comment-only; no behaviour changes. 1. test_named_unix_dgram_sendto_delivers_under_destination_policy claimed it guards "a destination policy does not divert a named unix datagram away from that gate". A mutation that performs exactly that diversion (skip the gate arm under a destination policy, let the send fall through to sendto_pinned_unix_on_behalf) leaves the test, all seven named_unix_dgram tests and the full suite green. The property the test really carries is narrower: the datagram is still DELIVERED; which arm delivers it is not pinned down. Reworded to that, with the surviving mutation named inline. 2. The same doc comment mapped "the fall-through's fd pinning" to if_nameindex_works_under_destination_policy, which only exercises the RawDestOnBehalf arm. The named-unix fall-through (sendto_pinned_unix_on_behalf) has no test at all — gutting it leaves the suite green. Stated as untested by design, pointing at the REACHABILITY note that explains why it is unreachable wherever code can execute. 3. network/mod.rs called the sendto non-IP fall-through "(non-chroot)". That arm has no chroot check: under --chroot a pathless AF_UNIX destination also misses the named-path gate and lands there, so it fails closed too. The qualifier under-claimed rather than over-claimed, but it sits in the exact paragraph that delimits the multikernel#27/multikernel#143 residue, which is what the maintainer reads it for. Replaced with the arm's real precondition.
d3075dc to
4292141
Compare
|
Both fixes are in, plus a third one that the first attempt at (1) turned out to need. (1) Abstract sends fail closed. (2) Named paths resolve in the child's root view. (3) Classification had to move to the destination's address family. Worth flagging, because the obvious form of fix (1) is wrong. "AF_UNIX with no pathname" and "a non-AF_UNIX sockaddr on a unix socket" are different shapes, and collapsing both into the fail-closed arm takes out sandlock's own NETLINK_ROUTE virtualization: its child-side fd is an So Behavior scope is updated for both user-visible changes. One addition you may not expect: On the happy-path test — added as What the test does carry is that a named unix datagram is still delivered while a destination policy is active; it does not pin down which arm delivers it (mutating the gate arm so the send falls through to One coverage gap, disclosed rather than papered over. Suites on the tip: 297 integration / 529 lib, no failures (main: 294 / 517). |
Splits the high-priority sendto half out of #130, as you suggested on that PR ("The sendto tightening is a high-priority fix... Maybe you want to separate it out as a fix which could land quickly"). The sendmsg/sendmmsg symmetry stays in draft #130; I'll rebase it onto this once this lands.
The bug
A non-IP
sendtowith no fs-path gate fell through to_ => NotifAction::Continue, so the kernel re-readsockfdand the destination address after the supervisor's decision. Under an active destination policy a child could race adup2(inet_sock, sockfd)+ address swap into that window and ride theContinueout to a denied IP. (SameContinue-shape as the chroot AF_UNIX gate report, #143.)The fix
Under
has_net_destination_policythe arm no longerContinues: it pins the fd viadup_fd_from_pid, classifies on the stable socket domain of the pinned fd (via the new pureclassify_send_path), and either sends the unix datagram on-behalf on that pinned fd or fails closed withEAFNOSUPPORT. With no destination policy there is nothing to bypass, soContinueis preserved unchanged.The non-
UnixOnBehalfoutcomes collapse to a single fail-closed errno arm rather thanunreachable!(), so any future drift in the classifier fails closed instead of panicking — a panic on this seccomp-notif path would unwind the supervisor task and DoS the sandbox.Testing
The pure
classify_send_pathhelper + its 4 unit tests land here (sendto is its first consumer).send_path_non_ip_on_non_unix_socket_is_rejectedis the deterministic fail-without-fix witness: pre-fix logic yieldedContinuefor that input, the helper now yieldsReject/EAFNOSUPPORT.I deliberately did not add an integration test for the fail-closed errno:
sendto(AF_INET dgram fd, AF_UNIX sockaddr)returnsEAFNOSUPPORTfrom the kernel too, so such a test is green pre- and post-fix and would be decorative. The bypass this closes is a race that isn't deterministically reproducible from a guest, so the pure-helper unit test is the honest witness.classify_send_pathispub(crate)(no SDK/FFI surface). It is the shared classifier the sendmsg/sendmmsg rework in #130 consumes fully; landing it here with sendto as first consumer lets #130 become a pure consumer on rebase.Behavior scope (please review)
This is fail-closed hardening that only triggers under a destination policy, but it does change some behavior — flagging explicitly:
sendtoon a non-unix, non-IP datagram family (AF_PACKET/AF_VSOCK/…) now returnsEAFNOSUPPORTinstead ofContinue. (Narrow:AF_NETLINKis virtualized to a unix socket and stays on the on-behalf path;AF_PACKETneedsCAP_NET_RAW.)connect_on_behalfstillContinues these families, so there is a connect-vs-sendto asymmetry.SO_PEERCRED/SCM_CREDENTIALSrather than the child's. Enabling an IP allowlist thus changes unix-datagram credential semantics (D-Bus/journald/etc.). This is intrinsic to closing the TOCTOU (Continue is unsafe once the fd can be swapped).sendtoandsendmsgdiffer on a non-IP unix datagram (sendto sends on-behalf; sendmsg still returnsEAFNOSUPPORT). This preserves sendto's pre-existing behavior (it already sent, viaContinue) while closing its TOCTOU; [draft] fix(net): symmetric named-AF_UNIX datagram gating (reference implementation) #130 aligns sendmsg.Happy to add a family carve-out (fail closed only for
AF_INET/AF_INET6sockets) if you'd prefer to keep exotic non-unix datagram families working.