diff --git a/crates/openshell-sandbox/src/network_broker.rs b/crates/openshell-sandbox/src/network_broker.rs index 4d84c8734c..d4a4bd7816 100644 --- a/crates/openshell-sandbox/src/network_broker.rs +++ b/crates/openshell-sandbox/src/network_broker.rs @@ -1226,6 +1226,9 @@ fn connect_local_tcp( } return match started { Ok(()) => listener.respond_value(notification.id, 0), + Err(error) if error.raw_os_error() == Some(libc::EINPROGRESS) => { + listener.respond_errno(notification.id, libc::EINPROGRESS) + } Err(error) => Err(error), }; } @@ -3036,6 +3039,132 @@ mod tests { assert_eq!(nonblocking_result, Some(libc::EINPROGRESS)); } + const DENIAL_WARNING: &str = "sandbox network notification denied"; + + /// Process-wide capture of the broker dispatcher's denial warnings. + /// + /// The dispatcher logs on the broker thread, which a thread-local + /// subscriber cannot observe, so the capture is installed globally once and + /// each test scopes its assertions to its own workload thread ID. + #[derive(Clone, Default)] + struct DenialCapture(Arc>>); + + #[derive(Default)] + struct DenialVisitor { + tid: Option, + message: String, + } + + impl tracing::field::Visit for DenialVisitor { + fn record_u64(&mut self, field: &tracing::field::Field, value: u64) { + if field.name() == "tid" { + self.tid = Some(value); + } + } + + fn record_i64(&mut self, field: &tracing::field::Field, value: i64) { + if field.name() == "tid" { + self.tid = u64::try_from(value).ok(); + } + } + + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + if field.name() == "message" { + self.message = format!("{value:?}"); + } + } + } + + impl tracing_subscriber::Layer for DenialCapture { + fn on_event( + &self, + event: &tracing::Event<'_>, + _ctx: tracing_subscriber::layer::Context<'_, S>, + ) { + if *event.metadata().level() != tracing::Level::WARN { + return; + } + let mut visitor = DenialVisitor::default(); + event.record(&mut visitor); + if let Some(tid) = visitor.tid + && visitor.message.starts_with(DENIAL_WARNING) + { + lock(&self.0).push((tid, visitor.message)); + } + } + } + + fn denial_capture() -> &'static DenialCapture { + static CAPTURE: std::sync::OnceLock = std::sync::OnceLock::new(); + CAPTURE.get_or_init(|| { + use tracing_subscriber::layer::SubscriberExt as _; + let capture = DenialCapture::default(); + tracing::subscriber::set_global_default( + tracing_subscriber::registry().with(capture.clone()), + ) + .expect("install global denial capture"); + capture + }) + } + + fn denial_warnings_for(tid: u32) -> Vec { + lock(&denial_capture().0) + .iter() + .filter(|(captured, _)| *captured == u64::from(tid)) + .map(|(_, message)| message.clone()) + .collect() + } + + /// Issue one nonblocking connect from a fresh workload thread and return + /// that thread's ID with the errno the workload observed. + fn nonblocking_workload_connect(address: SocketAddr) -> (u32, Option) { + denial_capture(); + let (launcher, listener) = + crate::linux::workload_launcher::start().expect("start workload launcher"); + let _broker = NetworkBroker::start_for_test(listener).expect("start network broker"); + launcher + .execute(move || { + // SAFETY: gettid has no arguments or side effects. + let tid = u32::try_from(unsafe { libc::syscall(libc::SYS_gettid) }) + .expect("positive tid"); + let socket = socket2::Socket::new( + socket2::Domain::IPV4, + socket2::Type::STREAM.nonblocking(), + None, + ) + .unwrap(); + let errno = socket + .connect(&address.into()) + .err() + .and_then(|error| error.raw_os_error()); + (tid, errno) + }) + .expect("launcher result") + } + + #[test] + fn nonblocking_loopback_in_progress_connect_is_not_logged_as_denied() { + // A nonblocking loopback connect that is still in progress is the + // normal outcome, not a denial: the workload gets EINPROGRESS and the + // dispatcher must not emit its denial warning. + let server = TcpListener::bind("127.0.0.1:0").unwrap(); + let (tid, errno) = nonblocking_workload_connect(server.local_addr().unwrap()); + assert_eq!(errno, Some(libc::EINPROGRESS)); + assert_eq!(denial_warnings_for(tid), Vec::::new()); + } + + #[test] + fn nonblocking_loopback_connect_failure_is_logged_as_denied() { + // A real loopback connect failure still takes the dispatcher's + // warning and errno-response path. + let (tid, errno) = + nonblocking_workload_connect(SocketAddr::from(([127, 255, 255, 255], 80))); + assert_eq!(errno, Some(libc::ENETUNREACH)); + let warnings = denial_warnings_for(tid); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert!(warnings[0].contains(&format!("tid={tid}")), "{warnings:?}"); + } + #[test] fn workload_cannot_bind_a_non_loopback_source_address() { // Workload sockets can only present loopback source addresses. A