Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 60 additions & 9 deletions crates/openshell-sandbox/src/linux/proc_fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,36 @@ pub fn installed_socket_inodes_excluding(
/// notification, task generation, and any retained socket cookie before a
/// state-changing operation.
pub fn socket_inode(tid: u32, fd: RawFd) -> io::Result<u64> {
socket_inode_if_socket(tid, fd)?.ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
"procfs descriptor is not a socket",
)
})
}

/// Return `None` only after procfs confirms that the descriptor is not a socket.
///
/// An unreadable or missing descriptor remains an error so callers cannot
/// mistake failed inspection for a safe native socket operation.
pub fn socket_inode_if_socket(tid: u32, fd: RawFd) -> io::Result<Option<u64>> {
if tid == 0 || fd < 0 {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"TID must be nonzero and FD must be nonnegative",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"It would be much more helpful for debugging if the error message included the actual values that triggered it. Could we update the string to print the invalid TID and FD values?"

));
}
let target = fs::read_link(format!("/proc/{tid}/fd/{fd}"))?;
let target = match fs::read_link(format!("/proc/{tid}/fd/{fd}")) {
Ok(target) => target,
Err(error) if error.raw_os_error() == Some(libc::ENOENT) => {
// A missing descriptor should keep the syscall's EBADF result.
// Check the parent so a vanished task or inaccessible procfs does
// not get mistaken for a closed descriptor.
fs::metadata(format!("/proc/{tid}/fd"))?;
return Err(io::Error::from_raw_os_error(libc::EBADF));
}
Err(error) => return Err(error),
};
let target = target.to_str().ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidData,
Expand All @@ -74,20 +97,24 @@ pub fn socket_inode(tid: u32, fd: RawFd) -> io::Result<u64> {
})?;
let digits = target
.strip_prefix("socket:[")
.and_then(|value| value.strip_suffix(']'))
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
"procfs descriptor is not a socket",
)
})?;
.and_then(|value| value.strip_suffix(']'));
let Some(digits) = digits else {
return if target.starts_with("socket:") {
Err(io::Error::new(
io::ErrorKind::InvalidData,
"procfs socket inode has an invalid representation",
))
} else {
Ok(None)
};
};
if digits.is_empty() || !digits.bytes().all(|byte| byte.is_ascii_digit()) {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"procfs socket inode has an invalid representation",
));
}
digits.parse::<u64>().map_err(|error| {
digits.parse::<u64>().map(Some).map_err(|error| {
io::Error::new(
io::ErrorKind::InvalidData,
format!("procfs socket inode does not fit u64: {error}"),
Expand All @@ -107,8 +134,16 @@ mod tests {
fn identifies_socket_and_rejects_regular_file() {
let (left, _right) = UnixStream::pair().expect("socketpair");
assert!(socket_inode(std::process::id(), left.as_raw_fd()).unwrap() > 0);
assert_eq!(
socket_inode_if_socket(std::process::id(), left.as_raw_fd()).unwrap(),
Some(socket_inode(std::process::id(), left.as_raw_fd()).unwrap())
);

let file = File::open("/dev/null").expect("open regular descriptor");
assert_eq!(
socket_inode_if_socket(std::process::id(), file.as_raw_fd()).unwrap(),
None
);
assert_eq!(
socket_inode(std::process::id(), file.as_raw_fd())
.expect_err("regular descriptor")
Expand All @@ -117,6 +152,22 @@ mod tests {
);
}

#[test]
fn missing_descriptor_is_not_classified_as_non_socket() {
assert_eq!(
socket_inode_if_socket(std::process::id(), i32::MAX)
.expect_err("closed descriptor cannot be inspected")
.raw_os_error(),
Some(libc::EBADF)
);
assert_eq!(
socket_inode_if_socket(u32::MAX, 3)
.expect_err("missing task must remain an inspection error")
.raw_os_error(),
Some(libc::ENOENT)
);
}

#[test]
fn installed_socket_snapshot_can_exclude_the_broker() {
let (left, _right) = UnixStream::pair().expect("socketpair");
Expand Down
143 changes: 143 additions & 0 deletions crates/openshell-sandbox/src/linux/socket_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,25 @@ impl SocketRegistry {
Ok(entry)
}

/// Return `None` only when procfs positively identifies an unregistered
/// descriptor. Inspection and retained-identity failures stay errors.
pub fn resolve_for_native_fallback(
&self,
tid: u32,
fd: RawFd,
) -> io::Result<Option<&SocketEntry>> {
let Some(inode) = proc_fd::socket_inode_if_socket(tid, fd)? else {
return Ok(None);
};
let Some(entry) = self.entries.get(&inode) else {
return Ok(None);
};
if entry.retained_preconnect.is_some() {
entry.validate_retained_identity()?;
}
Ok(Some(entry))
}

/// Mutable form of [`Self::resolve`].
pub fn resolve_mut(&mut self, tid: u32, fd: RawFd) -> io::Result<&mut SocketEntry> {
let inode = proc_fd::socket_inode(tid, fd)?;
Expand All @@ -316,6 +335,24 @@ impl SocketRegistry {
Ok(entry)
}

/// Mutable form of [`Self::resolve_for_native_fallback`].
pub fn resolve_mut_for_native_fallback(
&mut self,
tid: u32,
fd: RawFd,
) -> io::Result<Option<&mut SocketEntry>> {
let Some(inode) = proc_fd::socket_inode_if_socket(tid, fd)? else {
return Ok(None);
};
let Some(entry) = self.entries.get_mut(&inode) else {
return Ok(None);
};
if entry.retained_preconnect.is_some() {
entry.validate_retained_identity()?;
}
Ok(Some(entry))
}

/// Remove metadata after descendant-FD collection proves no installed
/// alias remains.
pub fn remove_inode(&mut self, inode: u64) -> bool {
Expand Down Expand Up @@ -347,6 +384,9 @@ fn socket_identity(fd: BorrowedFd<'_>, listener_generation: u64) -> io::Result<S

#[cfg(test)]
mod tests {
use std::io::{BufRead as _, Read as _, Write as _};
use std::process::{Command, Stdio};

use rustix::net::{AddressFamily, SocketFlags, SocketType, ipproto};

use super::*;
Expand Down Expand Up @@ -420,6 +460,109 @@ mod tests {
);
}

#[test]
fn native_fallback_requires_a_verified_descriptor() {
let mut registry = SocketRegistry::new(7, 1).unwrap();
let source = tcp_socket();
let installed = rustix::io::fcntl_dupfd_cloexec(&source, 3).unwrap();
let tentative = registry.stage(source, metadata()).unwrap();
registry.commit(tentative).unwrap();
assert!(
registry
.resolve_for_native_fallback(std::process::id(), installed.as_raw_fd())
.unwrap()
.is_some()
);
assert!(
registry
.resolve_mut_for_native_fallback(std::process::id(), installed.as_raw_fd())
.unwrap()
.is_some()
);

let unregistered = tcp_socket();
assert!(
registry
.resolve_for_native_fallback(std::process::id(), unregistered.as_raw_fd())
.unwrap()
.is_none()
);
let regular = std::fs::File::open("/dev/null").unwrap();
assert!(
registry
.resolve_for_native_fallback(std::process::id(), regular.as_raw_fd())
.unwrap()
.is_none()
);
assert_eq!(
registry
.resolve_for_native_fallback(std::process::id(), i32::MAX)
.expect_err("vanished descriptor must fail closed")
.raw_os_error(),
Some(libc::EBADF)
);
assert_eq!(
registry
.resolve_mut_for_native_fallback(std::process::id(), i32::MAX)
.expect_err("vanished descriptor must fail closed")
.raw_os_error(),
Some(libc::EBADF)
);
}

#[test]
#[allow(unsafe_code)]
fn native_fallback_rejects_unreadable_task_descriptors() {
const CHILD: &str = "OPENSHELL_UNREADABLE_DESCRIPTOR_TEST_CHILD";
if std::env::var_os(CHILD).is_some() {
let socket = tcp_socket();
// SAFETY: the disposable child changes only its own dumpability.
assert_eq!(unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) }, 0);
println!("{}", socket.as_raw_fd());
io::stdout().flush().unwrap();
let mut release = [0_u8];
io::stdin().read_exact(&mut release).unwrap();
return;
}
if rustix::process::geteuid().is_root() {
return;
}

let executable = std::env::current_exe().unwrap();
let mut child = Command::new(executable)
.arg("--exact")
.arg("linux::socket_registry::tests::native_fallback_rejects_unreadable_task_descriptors")
.arg("--nocapture")
.env(CHILD, "1")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
.unwrap();
let mut output = io::BufReader::new(child.stdout.take().unwrap());
let mut line = String::new();
loop {
line.clear();
assert_ne!(
output.read_line(&mut line).unwrap(),
0,
"child exited early"
);
if let Ok(fd) = line.trim().parse::<RawFd>() {
let registry = SocketRegistry::new(7, 1).unwrap();
assert_eq!(
registry
.resolve_for_native_fallback(child.id(), fd)
.expect_err("unreadable descriptor must fail closed")
.kind(),
io::ErrorKind::PermissionDenied
);
child.stdin.take().unwrap().write_all(&[1]).unwrap();
break;
}
}
assert!(child.wait().unwrap().success());
}

#[test]
fn dup_alias_resolves_to_same_open_file_description() {
let mut registry = SocketRegistry::new(9, 4).unwrap();
Expand Down
Loading
Loading