Skip to content

Commit d3075dc

Browse files
committed
fix(net): tighten sendto non-IP fall-through to close a destination-policy TOCTOU
Splits the high-priority sendto half out of #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 #130 — so until #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 #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 (#143).
1 parent 8b5eccf commit d3075dc

2 files changed

Lines changed: 129 additions & 2 deletions

File tree

crates/sandlock-core/src/network/send.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use super::unix::{
2121
mmsg_entry_named_unix_path, sendmmsg_named_unix_on_behalf, sendto_named_unix_on_behalf,
2222
unix_sendmsg_gate,
2323
};
24-
use super::verdict::{check_ip_destination, path_under_any};
24+
use super::verdict::{check_ip_destination, classify_send_path, path_under_any, SendPath};
2525
use super::{query_socket_protocol, socket_is_unix, Protocol};
2626

2727
// ============================================================
@@ -127,7 +127,54 @@ pub(super) async fn sendto_on_behalf(
127127
)
128128
}
129129
}
130-
_ => NotifAction::Continue,
130+
_ => {
131+
// Non-IP destination with no fs-path gate: an abstract unix
132+
// address, a named unix address with the fs-gate off, or a non-IP
133+
// address on a non-unix socket. With NO destination policy there
134+
// is nothing to bypass, so Continue (matching the connected fast
135+
// path). Under a destination policy do NOT Continue: the kernel
136+
// would re-read `sockfd` and the address after our decision, and a
137+
// racing `dup2(inet_sock, sockfd)` + address swap could ride the
138+
// Continue out to a denied IP. Pin the fd and gate on its STABLE
139+
// socket domain instead. NOTE: this fails closed for non-IP sends
140+
// on non-unix/non-IP datagram families (AF_PACKET/AF_VSOCK/...)
141+
// under a destination policy — see the PR description.
142+
if !ctx.policy.has_net_destination_policy {
143+
return NotifAction::Continue;
144+
}
145+
let dup_fd = match crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd) {
146+
Ok(fd) => fd,
147+
Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)),
148+
};
149+
// The address is non-IP here (parse_ip_from_sockaddr returned
150+
// None), so classify on the pinned socket's stable domain.
151+
match classify_send_path(false, None, socket_is_unix(dup_fd.as_raw_fd())) {
152+
SendPath::UnixOnBehalf => {
153+
let data =
154+
match read_child_mem(notif_fd, notif.id, notif.pid, buf_ptr, buf_len) {
155+
Ok(b) => b,
156+
Err(_) => return NotifAction::Errno(libc::EIO),
157+
};
158+
let m = MaterializedMsg {
159+
data,
160+
control: None,
161+
addr: addr_bytes,
162+
_scm_fds: Vec::new(),
163+
_pinned: None,
164+
};
165+
let blocking = wants_blocking(dup_fd.as_raw_fd(), flags);
166+
resolve_send(dup_fd, m, flags, blocking)
167+
}
168+
// Reject (non-IP address on a non-unix socket — the
169+
// address-family-swap shape) and, defensively, the
170+
// connected/IP variants that classify_send_path(false, None, _)
171+
// cannot return: fail closed with an errno (parity with the
172+
// sendmsg Reject arm) rather than panic in the seccomp-notif
173+
// supervisor — a panic on this untrusted path would unwind the
174+
// supervisor task and DoS the whole sandbox.
175+
_ => NotifAction::Errno(libc::EAFNOSUPPORT),
176+
}
177+
}
131178
}
132179
}
133180
}

crates/sandlock-core/src/network/verdict.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,47 @@ pub(crate) fn path_under_any(path: &std::path::Path, prefixes: &[std::path::Path
7676
prefixes.iter().any(|p| norm.starts_with(p))
7777
}
7878

79+
/// The send path selected for one message, from the already-parsed destination
80+
/// shape and the *stable* socket domain. Pure: the caller supplies whether the
81+
/// message is connected, the parsed IP (if any), and whether the pinned socket
82+
/// is `AF_UNIX`, so the decision is unit-testable and cannot re-read child state.
83+
///
84+
/// The unix-datagram arm sends on-behalf on the pinned fd rather than returning
85+
/// `Continue`: gating on the transient address family and Continuing would let a
86+
/// racing `dup2(inet_sock, sockfd)` + `msg_name` swap ride out on the kernel's
87+
/// re-read and reach a denied IP. Sending on the pinned fd we already checked
88+
/// closes that window.
89+
#[derive(Debug, PartialEq, Eq)]
90+
pub(crate) enum SendPath {
91+
/// Connected socket: no per-message destination; send on-behalf, no check.
92+
ConnectedOnBehalf,
93+
/// IP destination: validate `ip` against the allowlist, then send on-behalf.
94+
IpChecked(IpAddr),
95+
/// Non-IP address on a unix-domain socket: a unix datagram; send on-behalf on
96+
/// the pinned fd with no IP check (the kernel constrains a unix socket to
97+
/// unix peers, and we never Continue).
98+
UnixOnBehalf,
99+
/// Non-IP address on a non-unix socket: the address-family-swap shape; fail
100+
/// closed with `EAFNOSUPPORT`.
101+
Reject,
102+
}
103+
104+
/// Classify a send destination into the [`SendPath`] the handler should take.
105+
pub(crate) fn classify_send_path(
106+
connected: bool,
107+
ip: Option<IpAddr>,
108+
is_unix_socket: bool,
109+
) -> SendPath {
110+
if connected {
111+
return SendPath::ConnectedOnBehalf;
112+
}
113+
match ip {
114+
Some(ip) => SendPath::IpChecked(ip),
115+
None if is_unix_socket => SendPath::UnixOnBehalf,
116+
None => SendPath::Reject,
117+
}
118+
}
119+
79120
#[cfg(test)]
80121
mod tests {
81122
use super::*;
@@ -120,4 +161,43 @@ mod tests {
120161
let allowed: IpAddr = "10.0.0.1".parse().unwrap();
121162
assert_eq!(destination_verdict(&p, allowed, None), Err(ECONNREFUSED));
122163
}
164+
165+
#[test]
166+
fn send_path_connected_never_checks_address() {
167+
// A connected socket carries no per-message destination; the parsed
168+
// address and socket domain are irrelevant.
169+
let ip: IpAddr = "10.0.0.1".parse().unwrap();
170+
assert_eq!(
171+
classify_send_path(true, Some(ip), false),
172+
SendPath::ConnectedOnBehalf
173+
);
174+
assert_eq!(
175+
classify_send_path(true, None, true),
176+
SendPath::ConnectedOnBehalf
177+
);
178+
}
179+
180+
#[test]
181+
fn send_path_ip_destination_is_checked() {
182+
let ip: IpAddr = "10.0.0.1".parse().unwrap();
183+
assert_eq!(classify_send_path(false, Some(ip), false), SendPath::IpChecked(ip));
184+
// Socket domain does not override a parsed IP destination.
185+
assert_eq!(classify_send_path(false, Some(ip), true), SendPath::IpChecked(ip));
186+
}
187+
188+
#[test]
189+
fn send_path_non_ip_on_unix_socket_goes_on_behalf() {
190+
// A non-IP address on a unix-domain socket is a unix datagram: send it
191+
// on-behalf on the pinned fd (never Continue), so a raced fd/addr swap
192+
// cannot redirect it to a denied IP.
193+
assert_eq!(classify_send_path(false, None, true), SendPath::UnixOnBehalf);
194+
}
195+
196+
#[test]
197+
fn send_path_non_ip_on_non_unix_socket_is_rejected() {
198+
// A non-IP address on a non-unix (IP) socket is the address-family-swap
199+
// shape; fail closed. Pre-fix logic returned Continue for this input, so
200+
// this assertion is the deterministic fail-without-fix witness.
201+
assert_eq!(classify_send_path(false, None, false), SendPath::Reject);
202+
}
123203
}

0 commit comments

Comments
 (0)