Skip to content

Commit 732b147

Browse files
committed
Address review: dedup control materialization, gate to AF_UNIX, fail closed on EIO
Extracted the duplicated control-buffer handling of send_msghdr_on_behalf and send_named_unix_msghdr into materialize_control(), which: - fails closed with EIO when the control buffer can't be read from the child (previously it silently sent the message without its cmsgs, dropping the caller's SCM_RIGHTS fds), and - translates SCM_RIGHTS / rejects SCM_CREDENTIALS only for AF_UNIX sockets (probed via SO_DOMAIN, restored as socket_is_unix). SCM_RIGHTS and SCM_CREDENTIALS are unix-only, so an IP socket's control (e.g. IP_PKTINFO) now passes through untouched instead of being walked as if it could carry fds.
1 parent a51a58c commit 732b147

1 file changed

Lines changed: 65 additions & 36 deletions

File tree

crates/sandlock-core/src/network.rs

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,59 @@ fn translate_scm_rights(child_pid: u32, control: &[u8]) -> Result<(Vec<u8>, Vec<
531531
Ok((out, held))
532532
}
533533

534+
/// True iff `fd` is an `AF_UNIX` socket, probed via `SO_DOMAIN`. `SCM_RIGHTS`
535+
/// and `SCM_CREDENTIALS` are unix-only, so control rewriting/gating is applied
536+
/// only to unix sockets — an IP socket's control (e.g. `IP_PKTINFO`) carries no
537+
/// fds or credentials and passes through untouched.
538+
fn socket_is_unix(fd: RawFd) -> bool {
539+
let mut domain: libc::c_int = 0;
540+
let mut len = std::mem::size_of::<libc::c_int>() as libc::socklen_t;
541+
let rc = unsafe {
542+
libc::getsockopt(
543+
fd,
544+
libc::SOL_SOCKET,
545+
libc::SO_DOMAIN,
546+
&mut domain as *mut _ as *mut libc::c_void,
547+
&mut len,
548+
)
549+
};
550+
rc == 0 && domain == libc::AF_UNIX
551+
}
552+
553+
/// Copy (and, for a unix socket, translate) the control buffer of an on-behalf
554+
/// send. Shared by `send_msghdr_on_behalf` and `send_named_unix_msghdr`.
555+
///
556+
/// Returns `(control_bytes, held_fds)` — the fds keep the translated
557+
/// `SCM_RIGHTS` files open across the send. Fails closed: oversized control →
558+
/// `EMSGSIZE`; unreadable control → `EIO` (never a silent send that drops the
559+
/// caller's cmsgs). For a unix socket, `SCM_RIGHTS` fds are translated and
560+
/// `SCM_CREDENTIALS` is rejected; a non-unix socket's control passes through
561+
/// verbatim.
562+
fn materialize_control(
563+
notif: &SeccompNotif,
564+
notif_fd: RawFd,
565+
msg_control_ptr: u64,
566+
msg_controllen: u64,
567+
is_unix: bool,
568+
) -> Result<(Option<Vec<u8>>, Vec<OwnedFd>), i32> {
569+
if msg_control_ptr == 0 || msg_controllen == 0 {
570+
return Ok((None, Vec::new()));
571+
}
572+
// Fail closed on an oversized control buffer instead of silently truncating
573+
// (which could drop SCM_RIGHTS fds or send a malformed tail).
574+
if msg_controllen as usize > MAX_CONTROL_BUF {
575+
return Err(libc::EMSGSIZE);
576+
}
577+
let raw = read_child_mem(notif_fd, notif.id, notif.pid, msg_control_ptr, msg_controllen as usize)
578+
.map_err(|_| libc::EIO)?;
579+
if is_unix {
580+
let (buf, fds) = translate_scm_rights(notif.pid, &raw)?;
581+
Ok((Some(buf), fds))
582+
} else {
583+
Ok((Some(raw), Vec::new()))
584+
}
585+
}
586+
534587
// ============================================================
535588
// connect_on_behalf — perform connect() on behalf of the child (TOCTOU-safe)
536589
// ============================================================
@@ -1092,24 +1145,9 @@ fn send_named_unix_msghdr(
10921145
})
10931146
.collect();
10941147

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-
}
1110-
} else {
1111-
(None, Vec::new())
1112-
};
1148+
// Named target is always AF_UNIX, so translate SCM_RIGHTS / reject creds.
1149+
let (control_buf, _scm_fds) =
1150+
materialize_control(notif, notif_fd, msg_control_ptr, msg_controllen, true)?;
11131151

11141152
let dup_fd = crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd)
11151153
.map_err(|e| e.raw_os_error().unwrap_or(libc::EBADF))?;
@@ -1600,24 +1638,15 @@ async fn send_msghdr_on_behalf(
16001638
});
16011639
}
16021640

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-
}
1618-
} else {
1619-
(None, Vec::new())
1620-
};
1641+
// Translate SCM_RIGHTS / reject creds only for a unix socket; an IP socket's
1642+
// control carries no fds or credentials and passes through untouched.
1643+
let (control_buf, _scm_fds) = materialize_control(
1644+
notif,
1645+
notif_fd,
1646+
msg_control_ptr,
1647+
msg_controllen,
1648+
socket_is_unix(dup_fd.as_raw_fd()),
1649+
)?;
16211650

16221651
let mut msg: libc::msghdr = unsafe { std::mem::zeroed() };
16231652
if !connected {

0 commit comments

Comments
 (0)