Skip to content

Commit a51a58c

Browse files
committed
fix(net): send connected AF_UNIX sendmsg on-behalf under a destination policy
When a `net_allow`/`net_deny` destination policy is active, `sendmsg_on_behalf` (and `sendmmsg_on_behalf`) skipped the connected / non-IP-family Continue fast path and routed every send through the IP on-behalf path. For a connected `AF_UNIX` socket that path calls `query_socket_protocol`, which returns `None` for a unix socket (no IP protocol), and the send was refused with `ECONNREFUSED`. This broke every connected-socket `sendmsg` user — Wayland clients, D-Bus, anything that uses `sendmsg` for its fd-passing capability — whenever any `--net-allow` rule was set: `connect()` and `recvmsg()` succeeded but the client's outbound `sendmsg()` was rejected, so the peer never received the request. `send()`/`sendto` were unaffected (they already Continue connected sockets), so only `sendmsg` regressed. Continue can't fix this under a destination policy: on `RESP_CONTINUE` the kernel re-resolves the fd and `msg_name` against the live child, so a racing `dup2(inet_sock, sockfd)` could redirect the send onto an IP socket to a denied destination. Stay on the TOCTOU-safe on-behalf path: - `send_msghdr_on_behalf` takes `protocol: Option<Protocol>` and consumes it only in the non-connected IP branch. A connected send (every AF_UNIX send that reaches here — its connection was gated at connect time) needs no protocol and goes out on the immune `dup_fd`; a non-connected send with no resolvable protocol still fails closed. - On-behalf sends copy the control buffer, so `SCM_RIGHTS` fds were passed with the child's fd numbers — silently corrupting fd-passing. `translate_scm_rights` now `pidfd_getfd`s each fd into the supervisor and rewrites its number before the send, for both the IP and named-unix on-behalf paths. The control-buffer parser is hardened against a hostile child: - `cmsg_len` is bounds-checked against the *remaining* buffer with no `off + cmsg_len` overflow; a malformed header fails closed (EINVAL) instead of panicking or scanning out of bounds. - `SCM_CREDENTIALS` is rejected (EPERM): the on-behalf sender is the supervisor, so forwarding the child's crafted pid/uid/gid would let a privileged supervisor forge credentials the child can't assert. - An oversized control buffer (> 16 KiB) fails closed (EMSGSIZE) rather than being silently truncated into a partial cmsg chain. IP enforcement is untouched: non-unix sockets still resolve their protocol and validate every destination. Tests: connected `AF_UNIX` `sendmsg` under `net_allow` passes through; a `SCM_RIGHTS` send delivers a working fd (receiver reads the passed file's bytes); and unit tests cover the cmsg parser's overflow/short-header/ credential/pass-through cases.
1 parent 766af10 commit a51a58c

2 files changed

Lines changed: 346 additions & 21 deletions

File tree

crates/sandlock-core/src/network.rs

Lines changed: 192 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ use crate::sys::structs::{SeccompNotif, AF_INET, AF_INET6, ECONNREFUSED};
2020
/// Prevents a sandboxed process from triggering OOM in the supervisor.
2121
const MAX_SEND_BUF: usize = 64 << 20;
2222

23+
/// Maximum ancillary (control) buffer we copy for an on-behalf `sendmsg`.
24+
/// A control buffer larger than this fails closed with `EMSGSIZE` rather than
25+
/// being silently truncated into a partial cmsg chain (`SCM_MAX_FD` is 253 fds
26+
/// ≈ 1 KiB, so 16 KiB is far above any legitimate use while bounding supervisor
27+
/// memory per trapped send).
28+
const MAX_CONTROL_BUF: usize = 16 << 10;
29+
2330
/// An IPv4 or IPv6 address with a prefix length, used by `--net-deny`
2431
/// to match destination IPs by exact address (`/32`, `/128`) or by range.
2532
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
@@ -459,6 +466,71 @@ pub(crate) fn query_socket_protocol(fd: RawFd) -> Option<Protocol> {
459466
}
460467
}
461468

469+
/// Rewrite the `SCM_RIGHTS` file descriptors in a copied control buffer from
470+
/// the child's fd numbers to supervisor fd numbers, rejecting identity cmsgs.
471+
///
472+
/// On-behalf sends run in the supervisor, so a control buffer copied verbatim
473+
/// from the child carries fd numbers that are meaningless (or, worse, alias
474+
/// unrelated files) in the supervisor. For every `SOL_SOCKET`/`SCM_RIGHTS`
475+
/// message we `pidfd_getfd` each child fd into the supervisor and patch its
476+
/// number in place. The returned `OwnedFd`s must stay alive until after
477+
/// `sendmsg` (the kernel installs its own copies into the socket buffer during
478+
/// the send), then drop to close the supervisor's copies.
479+
///
480+
/// `SCM_CREDENTIALS` is rejected with `EPERM`: on the on-behalf path the
481+
/// *supervisor* is the syscall's sender, so forwarding the child's crafted
482+
/// `pid/uid/gid` would either fail `EPERM` anyway (an unprivileged supervisor
483+
/// can't assert them) or, for a privileged supervisor, let the child forge
484+
/// credentials it could never send itself. Failing closed is the safe choice.
485+
///
486+
/// Errors: `EBADF` if a child fd can't be fetched (matching the kernel's own
487+
/// error for a bad fd), `EPERM` for a credential cmsg, `EINVAL` for a malformed
488+
/// cmsg header (too short or extending past the buffer) — all fail closed, none
489+
/// sends a partial or forged control chain.
490+
fn translate_scm_rights(child_pid: u32, control: &[u8]) -> Result<(Vec<u8>, Vec<OwnedFd>), i32> {
491+
// sizeof(struct cmsghdr) on LP64: cmsg_len(8) + cmsg_level(4) + cmsg_type(4).
492+
// CMSG_ALIGN(16) == 16, so cmsg data begins right after the header.
493+
const CMSG_HDR: usize = 16;
494+
const FD: usize = std::mem::size_of::<i32>();
495+
let mut out = control.to_vec();
496+
let mut held: Vec<OwnedFd> = Vec::new();
497+
let mut off = 0usize;
498+
while off + CMSG_HDR <= out.len() {
499+
let cmsg_len = usize::from_ne_bytes(out[off..off + 8].try_into().unwrap());
500+
let level = i32::from_ne_bytes(out[off + 8..off + 12].try_into().unwrap());
501+
let ctype = i32::from_ne_bytes(out[off + 12..off + 16].try_into().unwrap());
502+
// `cmsg_len` is child-controlled. Compare against the *remaining* space
503+
// (never `off + cmsg_len`, which could overflow `usize`). A header that
504+
// is too short or claims to run past the buffer is malformed — fail
505+
// closed, as the kernel would `EINVAL` it. `off <= out.len() - CMSG_HDR`
506+
// holds from the loop guard, so `out.len() - off` cannot underflow.
507+
if cmsg_len < CMSG_HDR || cmsg_len > out.len() - off {
508+
return Err(libc::EINVAL);
509+
}
510+
if level == libc::SOL_SOCKET {
511+
if ctype == libc::SCM_RIGHTS {
512+
let data_off = off + CMSG_HDR;
513+
let nfds = (cmsg_len - CMSG_HDR) / FD;
514+
for i in 0..nfds {
515+
let p = data_off + i * FD;
516+
let child_fd = i32::from_ne_bytes(out[p..p + FD].try_into().unwrap());
517+
let sup_fd = crate::seccomp::notif::dup_fd_from_pid(child_pid, child_fd)
518+
.map_err(|e| e.raw_os_error().unwrap_or(libc::EBADF))?;
519+
out[p..p + FD].copy_from_slice(&sup_fd.as_raw_fd().to_ne_bytes());
520+
held.push(sup_fd);
521+
}
522+
} else if ctype == libc::SCM_CREDENTIALS {
523+
return Err(libc::EPERM);
524+
}
525+
}
526+
// Advance to CMSG_ALIGN(cmsg_len). `cmsg_len <= out.len() - off` bounds
527+
// the aligned add well under `usize::MAX`; each step is >= CMSG_HDR so
528+
// the loop always makes progress.
529+
off += (cmsg_len + 7) & !7;
530+
}
531+
Ok((out, held))
532+
}
533+
462534
// ============================================================
463535
// connect_on_behalf — perform connect() on behalf of the child (TOCTOU-safe)
464536
// ============================================================
@@ -1020,11 +1092,23 @@ fn send_named_unix_msghdr(
10201092
})
10211093
.collect();
10221094

1023-
let control_buf = if msg_control_ptr != 0 && msg_controllen > 0 {
1024-
let len = (msg_controllen as usize).min(4096);
1025-
read_child_mem(notif_fd, notif.id, notif.pid, msg_control_ptr, len).ok()
1095+
// Rewrite SCM_RIGHTS fds from child to supervisor numbers; `_scm_fds` keeps
1096+
// them open across the send (same reasoning as `send_msghdr_on_behalf`).
1097+
let (control_buf, _scm_fds) = if msg_control_ptr != 0 && msg_controllen > 0 {
1098+
// Fail closed on an oversized control buffer instead of silently
1099+
// truncating (which could drop SCM_RIGHTS fds or send a malformed tail).
1100+
if msg_controllen as usize > MAX_CONTROL_BUF {
1101+
return Err(libc::EMSGSIZE);
1102+
}
1103+
match read_child_mem(notif_fd, notif.id, notif.pid, msg_control_ptr, msg_controllen as usize) {
1104+
Ok(raw) => {
1105+
let (buf, fds) = translate_scm_rights(notif.pid, &raw)?;
1106+
(Some(buf), fds)
1107+
}
1108+
Err(_) => (None, Vec::new()),
1109+
}
10261110
} else {
1027-
None
1111+
(None, Vec::new())
10281112
};
10291113

10301114
let dup_fd = crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd)
@@ -1332,10 +1416,19 @@ async fn sendmsg_on_behalf(
13321416
Ok(fd) => fd,
13331417
Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)),
13341418
};
1335-
let protocol = match query_socket_protocol(dup_fd.as_raw_fd()) {
1336-
Some(p) => p,
1337-
None => return NotifAction::Errno(ECONNREFUSED),
1338-
};
1419+
// Resolve the protocol as `Option`: it is only consumed to validate a
1420+
// non-connected IP destination. `query_socket_protocol` returns `None` for
1421+
// an AF_UNIX socket (no IP protocol), and a connected send (every AF_UNIX
1422+
// send that reaches here — its connection was gated at connect time) never
1423+
// consumes it, so the send goes through the TOCTOU-safe on-behalf path on
1424+
// our immune `dup_fd` rather than being refused. A non-connected send with
1425+
// no resolvable protocol fails closed inside `send_msghdr_on_behalf`.
1426+
//
1427+
// On-behalf (not Continue) is load-bearing under a destination policy: a
1428+
// Continue would let the kernel re-resolve `sockfd`/`msg_name` against the
1429+
// live child, so a racing `dup2(inet_sock, sockfd)` after a domain check
1430+
// could redirect the send onto an IP socket to a denied destination.
1431+
let protocol = query_socket_protocol(dup_fd.as_raw_fd());
13391432

13401433
match send_msghdr_on_behalf(notif, ctx, notif_fd, &dup_fd, protocol, msghdr_ptr, flags).await {
13411434
Ok(n) => NotifAction::ReturnValue(n as i64),
@@ -1395,9 +1488,16 @@ fn prescan_msghdr(
13951488
/// the child. Caller is responsible for:
13961489
/// - dup'ing the child fd (`dup_fd`),
13971490
/// - resolving the socket protocol (`protocol`) via
1398-
/// `query_socket_protocol` on that dup,
1399-
/// - having confirmed via `prescan_msghdr` that `msghdr_ptr` points
1400-
/// at an IP-family destination (non-NULL `msg_name`).
1491+
/// `query_socket_protocol` on that dup.
1492+
///
1493+
/// `protocol` is `Option` because it is only consumed to validate a
1494+
/// *non-connected* IP destination against the allowlist. A connected send
1495+
/// (`msg_name == NULL`) — which is every send that reaches here on an AF_UNIX
1496+
/// socket, since its connection was already gated at connect time — carries no
1497+
/// destination and needs no protocol, so `None` is passed through unused. When
1498+
/// the message *is* non-connected, a missing protocol fails closed
1499+
/// (`ECONNREFUSED`), so an IP send whose protocol can't be resolved is refused
1500+
/// rather than escaping the allowlist.
14011501
///
14021502
/// Returns the byte count returned by `sendmsg`, or an errno suitable
14031503
/// for `NotifAction::Errno`. ECONNREFUSED is used both for "destination
@@ -1408,7 +1508,7 @@ async fn send_msghdr_on_behalf(
14081508
ctx: &Arc<SupervisorCtx>,
14091509
notif_fd: RawFd,
14101510
dup_fd: &std::os::unix::io::OwnedFd,
1411-
protocol: Protocol,
1511+
protocol: Option<Protocol>,
14121512
msghdr_ptr: u64,
14131513
flags: i32,
14141514
) -> Result<isize, i32> {
@@ -1447,6 +1547,9 @@ async fn send_msghdr_on_behalf(
14471547
None => return Err(libc::EAFNOSUPPORT),
14481548
};
14491549
let dest_port = parse_port_from_sockaddr(&addr_bytes);
1550+
// A non-connected IP send must have a resolved protocol to key the
1551+
// per-protocol allowlist. If it couldn't be resolved, fail closed.
1552+
let protocol = protocol.ok_or(ECONNREFUSED)?;
14501553

14511554
let ns = ctx.network.lock().await;
14521555
let live_policy = {
@@ -1497,11 +1600,23 @@ async fn send_msghdr_on_behalf(
14971600
});
14981601
}
14991602

1500-
let control_buf = if msg_control_ptr != 0 && msg_controllen > 0 {
1501-
let len = (msg_controllen as usize).min(4096);
1502-
read_child_mem(notif_fd, notif.id, notif.pid, msg_control_ptr, len).ok()
1603+
// Copy the control buffer and rewrite any SCM_RIGHTS fds from child to
1604+
// supervisor numbers; `_scm_fds` keeps the fetched fds open across the send.
1605+
let (control_buf, _scm_fds) = if msg_control_ptr != 0 && msg_controllen > 0 {
1606+
// Fail closed on an oversized control buffer instead of silently
1607+
// truncating (which could drop SCM_RIGHTS fds or send a malformed tail).
1608+
if msg_controllen as usize > MAX_CONTROL_BUF {
1609+
return Err(libc::EMSGSIZE);
1610+
}
1611+
match read_child_mem(notif_fd, notif.id, notif.pid, msg_control_ptr, msg_controllen as usize) {
1612+
Ok(raw) => {
1613+
let (buf, fds) = translate_scm_rights(notif.pid, &raw)?;
1614+
(Some(buf), fds)
1615+
}
1616+
Err(_) => (None, Vec::new()),
1617+
}
15031618
} else {
1504-
None
1619+
(None, Vec::new())
15051620
};
15061621

15071622
let mut msg: libc::msghdr = unsafe { std::mem::zeroed() };
@@ -1617,10 +1732,13 @@ async fn sendmmsg_on_behalf(
16171732
Ok(fd) => fd,
16181733
Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)),
16191734
};
1620-
let protocol = match query_socket_protocol(dup_fd.as_raw_fd()) {
1621-
Some(p) => p,
1622-
None => return NotifAction::Errno(ECONNREFUSED),
1623-
};
1735+
// Protocol is resolved as `Option` and consumed only by a non-connected
1736+
// IP entry (see `send_msghdr_on_behalf`). It is `None` for an AF_UNIX
1737+
// socket — whose connected entries send through the immune `dup_fd`
1738+
// without a destination check — so the batch is handled on-behalf here
1739+
// rather than refused with ECONNREFUSED. On-behalf (not Continue) keeps
1740+
// it TOCTOU-safe against a racing fd swap.
1741+
let protocol = query_socket_protocol(dup_fd.as_raw_fd());
16241742
let mut sent: usize = 0;
16251743
let mut first_errno: Option<i32> = None;
16261744
for i in 0..vlen {
@@ -1680,7 +1798,9 @@ async fn sendmmsg_on_behalf(
16801798

16811799
for i in 0..vlen {
16821800
let entry_ptr = msgvec_ptr + (i * MMSGHDR_SIZE) as u64;
1683-
match send_msghdr_on_behalf(notif, ctx, notif_fd, &dup_fd, protocol, entry_ptr, flags).await {
1801+
// Every entry is OnBehalf (IP, non-connected) per the prescan above, so
1802+
// the resolved protocol is always required and present here.
1803+
match send_msghdr_on_behalf(notif, ctx, notif_fd, &dup_fd, Some(protocol), entry_ptr, flags).await {
16841804
Ok(n) => {
16851805
let bytes = (n as u32).to_ne_bytes();
16861806
let _ = write_child_mem(
@@ -2024,6 +2144,57 @@ pub fn compose_virtual_etc_hosts(
20242144
mod tests {
20252145
use super::*;
20262146

2147+
// --- translate_scm_rights tests (control-buffer parsing, child-controlled) ---
2148+
2149+
fn cmsg_hdr(cmsg_len: usize, level: i32, ctype: i32) -> Vec<u8> {
2150+
let mut b = vec![0u8; 16];
2151+
b[0..8].copy_from_slice(&cmsg_len.to_ne_bytes());
2152+
b[8..12].copy_from_slice(&level.to_ne_bytes());
2153+
b[12..16].copy_from_slice(&ctype.to_ne_bytes());
2154+
b
2155+
}
2156+
2157+
#[test]
2158+
fn scm_rights_rejects_overflowing_cmsg_len() {
2159+
// A child-crafted cmsg_len near usize::MAX must fail closed (EINVAL),
2160+
// never overflow-panic (debug) or wrap past the bounds check (release).
2161+
let buf = cmsg_hdr(usize::MAX - 7, libc::SOL_SOCKET, libc::SCM_RIGHTS);
2162+
assert_eq!(translate_scm_rights(0, &buf).map(drop), Err(libc::EINVAL));
2163+
}
2164+
2165+
#[test]
2166+
fn scm_rights_rejects_short_header() {
2167+
let buf = cmsg_hdr(8, libc::SOL_SOCKET, libc::SCM_RIGHTS); // < CMSG_HDR (16)
2168+
assert_eq!(translate_scm_rights(0, &buf).map(drop), Err(libc::EINVAL));
2169+
}
2170+
2171+
#[test]
2172+
fn scm_rights_rejects_cmsg_running_past_buffer() {
2173+
let buf = cmsg_hdr(17, libc::SOL_SOCKET, libc::SCM_RIGHTS); // claims 17, has 16
2174+
assert_eq!(translate_scm_rights(0, &buf).map(drop), Err(libc::EINVAL));
2175+
}
2176+
2177+
#[test]
2178+
fn scm_rights_rejects_credentials() {
2179+
// Identity ancillary data on the on-behalf path must fail closed, not be
2180+
// forwarded with the child's crafted pid/uid/gid.
2181+
let buf = cmsg_hdr(16, libc::SOL_SOCKET, libc::SCM_CREDENTIALS);
2182+
assert_eq!(translate_scm_rights(0, &buf).map(drop), Err(libc::EPERM));
2183+
}
2184+
2185+
#[test]
2186+
fn scm_rights_passes_through_empty_and_non_socket_cmsg() {
2187+
// Empty control → no-op. A non-SOL_SOCKET cmsg (e.g. an IP-level control)
2188+
// passes through byte-for-byte with no fetched fds.
2189+
let (out, fds) = translate_scm_rights(0, &[]).unwrap();
2190+
assert!(out.is_empty() && fds.is_empty());
2191+
2192+
let buf = cmsg_hdr(16, libc::IPPROTO_IP, 2 /* IP_TTL-ish */);
2193+
let (out, fds) = translate_scm_rights(0, &buf).unwrap();
2194+
assert_eq!(out, buf);
2195+
assert!(fds.is_empty());
2196+
}
2197+
20272198
// --- NetAllow::parse tests ---
20282199

20292200
#[test]

0 commit comments

Comments
 (0)