From 3ead72580d5197d6f92ccaf338303fc52608ec18 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 20:16:18 -0700 Subject: [PATCH 1/3] chore(quest): claim quest/m1/cut-audit/uds-allowlist-first.md Quest-Claim: 41a5e764944505d9 From c4b563439823e6bd8ba13e45a8164d512586fc0b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 20:20:52 -0700 Subject: [PATCH 2/3] fix(tokio): refuse Unix peers before qmux negotiation Co-Authored-By: GPT-6 --- quest/m1/cut-audit/uds-allowlist-first.md | 19 ++++++ rs/moq-tokio/src/server.rs | 70 +++++++++++++++++++---- rs/moq-tokio/src/unix.rs | 16 +++++- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/quest/m1/cut-audit/uds-allowlist-first.md b/quest/m1/cut-audit/uds-allowlist-first.md index 076d1d1f455..ba2c905308b 100644 --- a/quest/m1/cut-audit/uds-allowlist-first.md +++ b/quest/m1/cut-audit/uds-allowlist-first.md @@ -18,3 +18,22 @@ Read `peer_cred` and check the allowlist first, then negotiate. A test connects as a refused credential and asserts no negotiation runs. Public API: none. Wire: none. + +### Checkpoint (2026-10-26) + +The source fix checks the existing credential allowlist inside the private Unix +pending handshake, before `qmux::uds::accept` can read or write. Public API, +wire format, allowed negotiation, and allowlist matching semantics are unchanged. + +A real local socket test reproduced the original boundary: a refused UID received +QMux's first preface byte (`64`). The fixed test closes that silent peer without +any negotiation bytes and also completes ALPN negotiation for an allowed UID. +The four Unix server tests pass, including stalled-peer concurrency, shutdown, +and the configured publisher. No host user, timer, or credential was changed. + +Validation remains unfinished. `nix develop --command just fix` fails in the +unchanged `rs/moq-mux/benches/catalog.rs:226`: `hang::catalog::MAX_RENDITIONS` +does not exist. The two changed source files are formatted separately. Finish +the existing benchmark repair, then run the full required `just fix` and clean +committed-head `just ci` before ready/review. Do not bypass a host limitation +reported by that gate. There is no API, wire, or allowlist-policy decision. diff --git a/rs/moq-tokio/src/server.rs b/rs/moq-tokio/src/server.rs index 51fde53c749..5a5b8d8dd15 100644 --- a/rs/moq-tokio/src/server.rs +++ b/rs/moq-tokio/src/server.rs @@ -1502,7 +1502,7 @@ fn spawn_unix_loop( let mut handshakes = tokio::task::JoinSet::new(); loop { let pending = tokio::select! { - pending = listener.accept_pending() => pending, + pending = listener.accept_pending(allow.as_ref()) => pending, Some(_) = handshakes.join_next() => continue, }; // Closed for drain: dropping the pending handshake closes its socket. @@ -1512,17 +1512,9 @@ fn spawn_unix_loop( let deadline = setup.deadline(); let setup = setup.clone(); let tx = tx.clone(); - let allow = allow.clone(); handshakes.spawn(crate::panic::isolate(async move { match within(deadline, pending).await { - Ok((session, cred)) => { - // Enforce the allowlist (if any) before reading SETUP bytes from the peer. - if let Some(allow) = &allow - && !allow.permits(&cred) - { - tracing::warn!(uid = cred.uid, gid = cred.gid, pid = ?cred.pid, "unix connection rejected by allow list"); - return; - } + Ok((session, _cred)) => { stream_request( session, &setup, @@ -2020,6 +2012,64 @@ mod tests { let _ = task.await; } + /// A refused peer must be closed without exchanging even the qmux preface. + /// Use our real kernel credentials and change only the configured allowlist. + #[cfg(all(feature = "uds", unix))] + #[tokio::test] + async fn unix_allowlist_precedes_negotiation() { + use tokio::io::AsyncReadExt; + let (peer, _other) = tokio::net::UnixStream::pair().unwrap(); + let cred = peer.peer_cred().unwrap(); + for allowed in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("listener.sock"); + let listener = crate::unix::Listener::bind(&path) + .await + .unwrap() + .with_protocols(["moq-lite-06"]); + let (tx, _rx) = tokio::sync::mpsc::channel(1); + let task = spawn_unix_loop( + listener, + Setup { + activity: Default::default(), + server: moq_net::Server::new(), + timeout: None, + }, + Some(crate::unix::Allow { + uid: vec![if allowed { + cred.uid() + } else { + cred.uid().wrapping_add(1) + }], + ..Default::default() + }), + tx, + ); + if allowed { + let mut config = qmux::Config::new(qmux::Version::QMux01); + config.protocol = qmux::Protocol::Negotiate(vec!["moq-lite-06".into()]); + let session = qmux::uds::connect(&path, config) + .await + .expect("allowed peer negotiates"); + assert_eq!(session.protocol(), Some("moq-lite-06")); + } else { + let mut peer = tokio::net::UnixStream::connect(&path).await.unwrap(); + let mut byte = [0]; + let read = peer.read(&mut byte).await; + // Some kernels report reset instead of EOF when the socket closes. + assert!( + matches!(read, Ok(0)) + || read + .as_ref() + .is_err_and(|err| err.kind() == std::io::ErrorKind::ConnectionReset), + "refused peer reached qmux negotiation: {read:?}, byte={byte:?}" + ); + } + task.abort(); + let _ = task.await; + } + } + #[cfg(all(feature = "uds", unix))] #[tokio::test] async fn unix_stalled_handshake_does_not_block_accept() { diff --git a/rs/moq-tokio/src/unix.rs b/rs/moq-tokio/src/unix.rs index 5a39f7599dc..cd2ec99532e 100644 --- a/rs/moq-tokio/src/unix.rs +++ b/rs/moq-tokio/src/unix.rs @@ -349,12 +349,16 @@ impl Listener { /// /// As in [`crate::tcp`], the `Option` has no `None` case left to report. pub async fn accept(&self) -> Option> { - Some(self.accept_pending().await.await) + Some(self.accept_pending(None).await.await) } /// Accept a socket and return its independently driven qmux handshake. - pub(crate) async fn accept_pending(&self) -> impl Future> + use<> { + pub(crate) async fn accept_pending( + &self, + allow: Option<&Allow>, + ) -> impl Future> + use<> { let stream = self.accept_socket().await; + let allow = allow.cloned(); let mut config = crate::qmux_config(); config.version = WIRE_VERSION; config.protocol = qmux::Protocol::Negotiate(self.protocols.clone()); @@ -365,6 +369,14 @@ impl Listener { gid: cred.gid(), pid: cred.pid(), }; + // Qmux negotiates in-band, so enforce credentials before it can read or + // write any bytes, rather than after it returns a session. + if let Some(allow) = &allow + && !allow.permits(&cred) + { + tracing::warn!(uid = cred.uid, gid = cred.gid, pid = ?cred.pid, "unix connection rejected by allow list"); + return Err(io::Error::new(io::ErrorKind::PermissionDenied, "unix peer outside allow list").into()); + } let session = qmux::uds::accept(stream, config) .await .map_err(|err| Error::Accept(crate::error::message(err)))?; From e21ded1cc0c5a85d28f362e5e3c821df996ec0cb Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 20:38:25 -0700 Subject: [PATCH 3/3] chore(quest): complete early Unix credential refusal Co-Authored-By: GPT-6 --- quest/m1/cut-audit/README.md | 1 - quest/m1/cut-audit/uds-allowlist-first.md | 39 ----------------------- 2 files changed, 40 deletions(-) delete mode 100644 quest/m1/cut-audit/uds-allowlist-first.md diff --git a/quest/m1/cut-audit/README.md b/quest/m1/cut-audit/README.md index a84b26c68c0..ef5d922a26e 100644 --- a/quest/m1/cut-audit/README.md +++ b/quest/m1/cut-audit/README.md @@ -77,7 +77,6 @@ leaving JS-only redesigns of `js/net` to [rs2ts](/quest/m2/rs2ts/README.md). - [Rust and JS accept the same catalogs](/quest/m1/cut-audit/catalog-parity.md) - one strictness, per rendition, with a shared fixture corpus - [Lite datagrams obey one size limit](/quest/m1/cut-audit/datagram-size.md) - no datagram one hop accepts is dropped by the next for size - [Every relay runtime honors or refuses each listen setting](/quest/m1/cut-audit/uring-listen-config.md) - io_uring stops ignoring preferred addresses -- [The Unix-socket allowlist is checked first](/quest/m1/cut-audit/uds-allowlist-first.md) - refused local users never reach the qmux parser - [A route's terminal state is one phase](/quest/m1/cut-audit/route-phase.md) - three resume flags collapse into one check - [Lite's version gates, refusals, and draft text agree](/quest/m1/cut-audit/lite-wire-nits.md) - small wire and draft mismatches fixed - [Small @moq/net divergences from Rust are fixed](/quest/m1/cut-audit/js-net-cleanups.md) - lite-04 probes, group consumers, accept deadlines, empty JSON diff --git a/quest/m1/cut-audit/uds-allowlist-first.md b/quest/m1/cut-audit/uds-allowlist-first.md deleted file mode 100644 index ba2c905308b..00000000000 --- a/quest/m1/cut-audit/uds-allowlist-first.md +++ /dev/null @@ -1,39 +0,0 @@ -# [XS] The Unix-socket allowlist is checked before any peer bytes are parsed - -## Goal - -A local user outside a Unix-socket listener's allowlist is refused on its -peer credentials before the server parses a single byte from it. - -## Plan - -Found by the 2026-10-24 pre-cut audit at 151a529670. Source-inferred, -inherited; local only. - -`rs/moq-tokio/src/server.rs:1355` makes the socket `0o666`, and the -allowlist check (`allow.permits`, about line 1516) runs only after -`within(deadline, pending)` has run `qmux::uds::accept` -(`unix.rs:356-372`), so any local user reaches the qmux negotiation parser. -Read `peer_cred` and check the allowlist first, then negotiate. A test -connects as a refused credential and asserts no negotiation runs. - -Public API: none. Wire: none. - -### Checkpoint (2026-10-26) - -The source fix checks the existing credential allowlist inside the private Unix -pending handshake, before `qmux::uds::accept` can read or write. Public API, -wire format, allowed negotiation, and allowlist matching semantics are unchanged. - -A real local socket test reproduced the original boundary: a refused UID received -QMux's first preface byte (`64`). The fixed test closes that silent peer without -any negotiation bytes and also completes ALPN negotiation for an allowed UID. -The four Unix server tests pass, including stalled-peer concurrency, shutdown, -and the configured publisher. No host user, timer, or credential was changed. - -Validation remains unfinished. `nix develop --command just fix` fails in the -unchanged `rs/moq-mux/benches/catalog.rs:226`: `hang::catalog::MAX_RENDITIONS` -does not exist. The two changed source files are formatted separately. Finish -the existing benchmark repair, then run the full required `just fix` and clean -committed-head `just ci` before ready/review. Do not bypass a host limitation -reported by that gate. There is no API, wire, or allowlist-policy decision.