diff --git a/doc/bin/cli.md b/doc/bin/cli.md index edd7ae8477..46705ef820 100644 --- a/doc/bin/cli.md +++ b/doc/bin/cli.md @@ -44,7 +44,8 @@ moq fetch [options] The **MoQ side** goes first and attaches the process to the network: `--connect ` dials a relay (the path is the auth path, `?jwt=` carries a token), and `--broadcast ` names the broadcast. A process can -instead host sessions with `--listen`, or both at once. A listener admits +instead host sessions with `--listen`, or both at once; without a listener, +the `--listen-*` and `--auth-*` flags are refused. A listener admits clients by `--auth-url` or `--auth-public`, as the relay does (see [Authentication](/bin/relay/auth)); public rules ignore certificates, so `--auth-public` refuses to start with `--listen-tls-root`, and only `--listen` diff --git a/doc/bin/relay/config.md b/doc/bin/relay/config.md index d7fc698709..a2dda75a77 100644 --- a/doc/bin/relay/config.md +++ b/doc/bin/relay/config.md @@ -44,6 +44,12 @@ with the MoQ timeout code once its transport is up. The `[web]` listeners apply headers and, for the WebSocket fallback, to the SETUP after the upgrade. The `io_uring` workers do not apply it yet. +A setting no configured listener reads stops startup rather than being ignored. +A stream-only relay refuses `preferred_v4`, `preferred_v6`, and `lb_id` (or `load_balancer`), which +only QUIC reads, and a `[listen.tls]` `cert`, `key`, or `generate` unless +`tcp.tls` serves it. +`unix.allow` needs `unix.bind`. + ## \[quic] Transport tuning, applied to accepted and dialed connections alike. @@ -125,7 +131,7 @@ listen = "[::]:4443" # HTTP: fingerprint, announced, fetch, heal [web.https] listen = "[::]:443" # HTTPS plus the WebSocket fallback. -cert = "cert.pem" +cert = "cert.pem" # cert, key, and root need listen. key = "key.pem" [internal] diff --git a/doc/setup/upgrade.md b/doc/setup/upgrade.md index de3dc8c821..bd2ef26499 100644 --- a/doc/setup/upgrade.md +++ b/doc/setup/upgrade.md @@ -70,6 +70,14 @@ These land with the next breaking release, not the 2026-09-23 train. `linger`, like any unknown setting. `MOQ_CLUSTER_MESH` and `MOQ_CLUSTER_LINGER` are no longer read, so drop them from the environment. In Rust, `cluster::Config` has no `mesh` or `linger` field. +- **Settings no listener reads stop startup.** A stream-only relay or `moq` + listener (TCP or Unix, no `--listen`) refuses the QUIC-only + `--listen-preferred-v4`/`-v6`, `--listen-quic-lb-id`, and pinned `tls.peers`, + and a `--listen-tls-cert`, `-key`, or `-generate` unless `--listen-tcp-tls` + serves it. `--listen-unix-allow-*` needs `--listen-unix-bind`, and + `web.https.cert`, `key`, and `root` need `web.https.listen`. `moq` without a + listener refuses `--listen-*` and `--auth-*` flags. Each used to be ignored; + drop it, or add the listener it configures. - **moq-mux data producers take a broadcast-clock `Timestamp`.** `json` and `binary` `Snapshot::update` and `Stream::append` take `Timed<_, Timestamp>` instead of `Timed<_, Instant>`, and publish it as given. Convert a capture diff --git a/quest/m1/README.md b/quest/m1/README.md index 45b2059842..b5cc6008f8 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -68,7 +68,6 @@ blocks. The quests that gated m0 lines moved under them. - [CI runner stalls](/quest/m1/ci-runner-stalls.md) - the 0.4 to 0.8 s freezes of both interop tracks on CI are attributed from a week of nightlies and fixed or told apart from playback bugs - [Wire compatibility](/quest/m1/wire-compat.md) - a nightly run tests this checkout against the last published release for tokens, session wire, and catalog/container - [Same-epoch importers](/quest/m1/hop-aligned-import.md) - importers sharing one `--epoch` and fed one stream publish identical groups and timestamps, so failover between a redundant pair survives -- [Refuse ignored TLS config](/quest/m1/refuse-ignored-tls.md) - TLS options no listener would use fail at load instead of being ignored - [cpal loads libasound at runtime](/quest/m1/cpal-alsa-runtime.md) - condition: a cpal release whose Linux build carries no load-time libasound requirement - [Audio capture without ALSA link](/quest/m1/capture-alsa-link.md) - moq-audio capture and playback build on Linux without linking libasound - [Capture by default](/quest/m1/capture-default.md) - moq-video and moq-audio build `capture` by default, so pre-merge checks test it and the capture gate goes away diff --git a/quest/m1/refuse-ignored-tls.md b/quest/m1/refuse-ignored-tls.md deleted file mode 100644 index addd24303d..0000000000 --- a/quest/m1/refuse-ignored-tls.md +++ /dev/null @@ -1,19 +0,0 @@ -# [S] TLS settings that would be ignored are refused at load - -## Goal - -A relay or `moq` CLI config that sets a TLS option the configured listeners -would never use fails at load with an error naming the option, instead of -starting and silently ignoring it. Known cases: `tls.peers` on a server with -no QUIC listener, and `web.https.root` without `web.https.listen`. - -## Plan - -Same class of bug as the listener client CA that #4912 proposes refusing in -`moq_tokio::Server::build`. Audit the other TLS and listener options for -settings that only take effect on a listener that may not exist, and refuse -each where the config is assembled. TOML cannot express clap's `requires`, so -the check belongs in validation, not in the flag definitions. - -Public API: none. Behavior: configs that relied on an ignored option now fail -to start, so note it in `doc/setup/upgrade.md`. diff --git a/quest/m2/tls-listener-mtls.md b/quest/m2/tls-listener-mtls.md index ee62aad762..b743e3641d 100644 --- a/quest/m2/tls-listener-mtls.md +++ b/quest/m2/tls-listener-mtls.md @@ -23,10 +23,10 @@ leaving it open. Keep the token path working on the same listener, and keep pinned peers and CA roots meaning the same as on QUIC. -[Relay client-CA validation](https://github.com/moq-dev/moq/pull/4912) refuses a -listener TLS client CA on a stream-only relay (the `MtlsUnsupported` case in -`rs/moq-tokio/src/server.rs`), since nothing checks it there. Once a `tls://` -listener verifies the certificate, this quest lifts that refusal for a relay -with a `tls://` listener and updates the refusal's test. +A stream-only server refuses a listener client CA or pinned `tls.peers` (the +`MtlsUnsupported` case in `rs/moq-tokio/src/server.rs`), since nothing checks +them there. Once a `tls://` listener verifies the certificate, this quest lifts +that refusal for a server with a `tls://` listener and updates the refusal's +test. Public API: relay config may gain a listener option. Wire: none. diff --git a/rs/moq-cli/src/args.rs b/rs/moq-cli/src/args.rs index f010234f5a..d215674dc7 100644 --- a/rs/moq-cli/src/args.rs +++ b/rs/moq-cli/src/args.rs @@ -154,6 +154,11 @@ impl std::fmt::Display for ParseError { impl std::error::Error for ParseError {} +/// Whether `flag` is one of the flags `T` declares. +fn owns(flag: &usage::Flag<'_>) -> bool { + T::COMMAND.flags.iter().any(|own| own.key == flag.key) +} + impl Invocation { /// Parse the process arguments, exiting with Usage's rendered message on error. /// @@ -194,11 +199,6 @@ impl Invocation { /// `--iroh-*` settings it dials with. Answered from the command line, like /// [`Self::reject`]. pub fn dial_only(&self, command: &str, allow: &[&str]) -> anyhow::Result<()> { - use usage::spec::CommandArgs; - - fn owns(flag: &usage::Flag<'_>) -> bool { - T::COMMAND.flags.iter().any(|own| own.key == flag.key) - } let dials = |flag: &&&usage::Flag<'_>| { #[cfg(feature = "iroh")] if owns::(flag) { @@ -217,6 +217,23 @@ impl Invocation { Ok(()) } + /// Refuse the listener flags (`--listen-*`, `--auth-*`) when nothing listens, + /// rather than silently ignoring them. Answered from the command line, like + /// [`Self::reject`]. + pub fn unserved(&self) -> anyhow::Result<()> { + if self.moq.serves() { + return Ok(()); + } + let listens = + |flag: &&&usage::Flag<'_>| owns::(flag) || owns::(flag); + if let Some(flags) = Self::names(self.given.iter().filter(listens)) { + anyhow::bail!( + "{flags} configure a listener, but none is bound; add --listen, --listen-tcp-bind, --listen-unix-bind, or --cluster-lan" + ); + } + Ok(()) + } + /// `--a, --b` for the flags given, or `None` when there are none. fn names<'a>(flags: impl Iterator>) -> Option { let names: Vec = flags.map(|flag| format!("--{}", flag.longs[0])).collect(); @@ -1895,6 +1912,34 @@ mod tests { } } + /// A listener or auth flag with nothing listening is refused, naming it, and + /// accepted once a listener is bound. + #[test] + fn listener_flags_need_a_listener() { + let parse = |flags: &[&str]| { + let argv = ["moq", "--connect", "https://relay.example"] + .iter() + .chain(flags) + .chain(&["import", "ts"]) + .copied(); + Invocation::try_parse_from(argv).unwrap_or_else(|err| panic!("{flags:?}: {err}")) + }; + for flag in [ + &["--listen-tls-root", "ca.pem"][..], + &["--listen-tls-cert", "cert.pem"], + &["--listen-tcp-tls"], + &["--listen-version", "moq-lite-02"], + &["--auth-public", "**"], + ] { + let err = parse(flag).unserved().unwrap_err().to_string(); + assert!(err.contains(flag[0]), "{flag:?}: {err}"); + } + parse(&["--listen", "[::]:0", "--listen-tls-root", "ca.pem"]) + .unserved() + .expect("a listener reads it"); + parse(&[]).unserved().expect("nothing to refuse"); + } + /// `--cluster-connect` / `--cluster-connect-api` attach the process as a /// cluster peer, so they are a MoQ side on their own. `--cluster-node` is /// identity, not an attachment. diff --git a/rs/moq-cli/src/main.rs b/rs/moq-cli/src/main.rs index 4d4fddbeb9..4686c73ad5 100644 --- a/rs/moq-cli/src/main.rs +++ b/rs/moq-cli/src/main.rs @@ -327,6 +327,7 @@ async fn main() -> anyhow::Result<()> { } else if let [Command::Announced(_)] = stages.as_slice() { cli.dial_only("announced", &[])?; } else { + cli.unserved()?; cli.moq.validate()?; } diff --git a/rs/moq-relay/src/web.rs b/rs/moq-relay/src/web.rs index a443e0cd7b..2163c23ee6 100644 --- a/rs/moq-relay/src/web.rs +++ b/rs/moq-relay/src/web.rs @@ -233,6 +233,16 @@ impl Web { /// Bind the configured listeners now, so [`addrs`](Self::addrs) reports ephemeral ports before serving. pub fn bind(mut self) -> anyhow::Result { + let https = &self.config.https; + if https.listen.is_none() { + for (flag, set) in [ + ("--web-https-cert", !https.cert.is_empty()), + ("--web-https-key", !https.key.is_empty()), + ("--web-https-root", !https.root.is_empty()), + ] { + anyhow::ensure!(!set, "{flag} requires --web-https-listen"); + } + } if self.https_tls.is_none() && self.config.https.listen.is_some() { let tls = build_https_config(&self.config.https.cert, &self.config.https.key, &self.config.https.root)?; self.https_tls = Some(RustlsConfig::from_config(tls)); @@ -1420,6 +1430,32 @@ mod tests { serving.abort(); } + /// HTTPS settings without the HTTPS listener that would read them are refused, + /// not ignored. + #[tokio::test] + async fn https_settings_require_the_https_listener() { + type Set = fn(&mut Https); + let cases: [(&str, Set); 3] = [ + ("--web-https-cert", |https| https.cert = vec!["cert.pem".into()]), + ("--web-https-key", |https| https.key = vec!["key.pem".into()]), + ("--web-https-root", |https| https.root = vec!["ca.pem".into()]), + ]; + let dir = TempDir::new().unwrap(); + let (_, cert, _) = make_certs(&dir); + let certificates = moq_tokio::tls::Certificates::from_pem(&std::fs::read(&cert).unwrap()).unwrap(); + for (flag, set) in cases { + let mut config = Config::default(); + set(&mut config.https); + let cluster = cluster::Cluster::new(crate::cluster::Options::default()).unwrap(); + let web = Web::new(crate::auth::Auth::refuse("test"), cluster, certificates.clone(), config); + let err = match web.bind() { + Ok(_) => panic!("{flag} was ignored"), + Err(err) => err.to_string(), + }; + assert!(err.contains(flag) && err.contains("--web-https-listen"), "{err}"); + } + } + /// A timeout past the clock's range waits forever, rather than reaching hyper, /// which would panic adding it to the clock on every connection. #[tokio::test] diff --git a/rs/moq-tokio/src/error.rs b/rs/moq-tokio/src/error.rs index baa74cdc85..6be8b8f04a 100644 --- a/rs/moq-tokio/src/error.rs +++ b/rs/moq-tokio/src/error.rs @@ -83,7 +83,7 @@ pub enum Error { #[error("failed to initialize Android logcat layer")] Logcat(#[source] Arc), - /// No backend feature is compiled in that can serve this URL. The string names the features to enable. + /// No compiled-in backend or configured listener can serve this. The string names what is missing. #[error("{0}")] NoBackend(&'static str), @@ -138,9 +138,9 @@ pub enum Error { #[error("Iroh support is not enabled")] IrohDisabled, - /// A client CA was configured on a stream-only server, which has no QUIC listener to verify it. + /// A client CA or pinned peers were configured on a stream-only server, which has no QUIC listener to verify them. #[error( - "--listen-tls-root (mTLS) needs a QUIC listener (--listen); the TCP and Unix listeners never ask for a client certificate" + "mTLS (--listen-tls-root or tls.peers) needs a QUIC listener (--listen); the TCP and Unix listeners never ask for a client certificate" )] MtlsUnsupported, diff --git a/rs/moq-tokio/src/listen.rs b/rs/moq-tokio/src/listen.rs index 03610d519b..ddea557f9e 100644 --- a/rs/moq-tokio/src/listen.rs +++ b/rs/moq-tokio/src/listen.rs @@ -427,9 +427,63 @@ impl Config { if self.tcp.tls == Some(true) && self.tcp.bind.is_none() { return Err(crate::Error::NoBackend("--listen-tcp-tls requires --listen-tcp-bind")); } + #[cfg(all(feature = "uds", unix))] + if !self.unix.allow.is_empty() && self.unix.bind.is_none() { + return Err(crate::Error::NoBackend( + "--listen-unix-allow-* requires --listen-unix-bind", + )); + } Ok(()) } + /// Refuse what only the QUIC listener reads, for a server that has none. + #[cfg(feature = "_transport")] + pub(crate) fn validate_stream_only(&self) -> crate::Result<()> { + #[cfg(feature = "tcp")] + let tcp_tls = self.tcp.tls == Some(true); + #[cfg(not(feature = "tcp"))] + let tcp_tls = false; + #[cfg(any(feature = "aws-lc-rs", feature = "ring"))] + let identity = self.tls.identity.is_some(); + #[cfg(not(any(feature = "aws-lc-rs", feature = "ring")))] + let identity = false; + + let refused = [ + ( + !tcp_tls && !self.tls.cert.is_empty(), + "--listen-tls-cert needs a TLS listener (--listen or --listen-tcp-tls)", + ), + ( + !tcp_tls && !self.tls.key.is_empty(), + "--listen-tls-key needs a TLS listener (--listen or --listen-tcp-tls)", + ), + ( + !tcp_tls && !self.tls.generate.is_empty(), + "--listen-tls-generate needs a TLS listener (--listen or --listen-tcp-tls)", + ), + ( + !tcp_tls && identity, + "tls.identity needs a TLS listener (--listen or --listen-tcp-tls)", + ), + ( + self.preferred_v4.is_some(), + "--listen-preferred-v4 needs a QUIC listener (--listen)", + ), + ( + self.preferred_v6.is_some(), + "--listen-preferred-v6 needs a QUIC listener (--listen)", + ), + ( + self.lb_id.is_some() || self.load_balancer.is_some(), + "--listen-quic-lb-id (load_balancer) needs a QUIC listener (--listen)", + ), + ]; + match refused.into_iter().find(|(set, _)| *set) { + Some((_, reason)) => Err(crate::Error::NoBackend(reason)), + None => Ok(()), + } + } + /// How long an accepted connection has to finish its handshake, or `None` to wait /// forever: for zero, and for a timeout too long for the clock to reach. pub fn resolved_timeout(&self) -> Option { @@ -610,6 +664,68 @@ load_balancer = { id = "ab", nonce = 8 } assert!(matches!(server.local_addr(), Err(crate::Error::NoBackend(_)))); } + /// A stream-only server refuses each setting only QUIC reads rather than + /// ignoring it, and accepts them once QUIC lives elsewhere. + #[cfg(feature = "tcp")] + #[tokio::test] + async fn stream_only_refuses_quic_settings() { + let stream_only = || Config { + tcp: crate::tcp::Config { + bind: Some("127.0.0.1:0".parse().unwrap()), + ..Default::default() + }, + ..Default::default() + }; + type Set = fn(&mut Config); + let cases: [(&str, Set); 7] = [ + ("--listen-tls-cert", |c| c.tls.cert = vec!["cert.pem".into()]), + ("--listen-tls-key", |c| c.tls.key = vec!["key.pem".into()]), + ("--listen-tls-generate", |c| c.tls.generate = vec!["localhost".into()]), + ("--listen-preferred-v4", |c| { + c.preferred_v4 = Some("192.0.2.1:443".parse().unwrap()) + }), + ("--listen-preferred-v6", |c| { + c.preferred_v6 = Some("[2001:db8::1]:443".parse().unwrap()) + }), + ("--listen-quic-lb-id", |c| c.lb_id = Some("ab".parse().unwrap())), + ("--listen-quic-lb-id (load_balancer)", |c| { + c.load_balancer = Some(crate::quic::LoadBalancer { + id: "ab".parse().unwrap(), + nonce: 8, + }) + }), + ]; + for (name, set) in cases { + let mut config = stream_only(); + set(&mut config); + match config.clone().init(Default::default()) { + Err(crate::Error::NoBackend(reason)) => assert!(reason.starts_with(name), "{name}: {reason}"), + Err(err) => panic!("{name}: {err}"), + Ok(_) => panic!("{name} was ignored"), + } + config.init_streams().expect("streams beside worker-owned QUIC"); + } + + // `tls://` serves the certificate. + let mut config = stream_only(); + config.tcp.tls = Some(true); + config.tls.generate = vec!["localhost".into()]; + config.init(Default::default()).expect("a TLS stream listener"); + } + + /// A Unix allowlist without the Unix listener it gates is refused. + #[cfg(all(feature = "uds", unix))] + #[test] + fn unix_allow_requires_unix_bind() { + let mut config = Config::default(); + config.unix.allow.uid = vec![0]; + match config.init(Default::default()) { + Err(crate::Error::NoBackend(reason)) => assert!(reason.contains("--listen-unix-bind"), "{reason}"), + Err(err) => panic!("{err}"), + Ok(_) => panic!("the allowlist was ignored"), + } + } + /// The default constructor still binds QUIC when nothing else is configured, /// which is the behavior `init_streams` had to be a separate call to avoid /// changing. diff --git a/rs/moq-tokio/src/server.rs b/rs/moq-tokio/src/server.rs index 4eb61eda06..b76b409b7a 100644 --- a/rs/moq-tokio/src/server.rs +++ b/rs/moq-tokio/src/server.rs @@ -290,6 +290,11 @@ impl Server { quic.validate()?; let build_quic = parts.quic() && (config.bind.is_some() || !config.has_stream_listener()); + // A stream-only server would silently ignore what only QUIC reads. A caller + // opening only the streams owns QUIC elsewhere, which reads them. + if parts.quic() && !build_quic { + config.validate_stream_only()?; + } // Read before the member is taken out below, which consumes `parts`. #[cfg(any(feature = "tcp", all(feature = "uds", unix)))] let build_streams = parts.streams(); @@ -303,11 +308,12 @@ impl Server { // Only the QUIC backend verifies client certificates; the qmux listeners // (tcp/unix/websocket) never ask for one, even over `tls://`, so a - // stream-only server would ignore the CA. A caller opening only the streams owns QUIC elsewhere. + // stream-only server would ignore the CA or pinned peers. A caller opening + // only the streams owns QUIC elsewhere. + if parts.quic() && !build_quic && (!config.tls.root.is_empty() || config.tls.peers.is_some()) { + return Err(Error::MtlsUnsupported); + } if parts.quic() && !config.tls.root.is_empty() { - if !build_quic { - return Err(Error::MtlsUnsupported); - } #[cfg(not(feature = "noq"))] return Err(Error::NoBackend("--listen-tls-root requires the noq feature")); } @@ -2336,21 +2342,28 @@ mod tests { drop((session, connection)); } - /// A client CA on a stream-only server is refused: no QUIC listener would - /// verify it, and the stream listeners never ask for a client certificate. + /// A client CA or pinned peers on a stream-only server are refused: no QUIC + /// listener would verify them, and the stream listeners never ask for a client certificate. #[cfg(feature = "tcp")] #[tokio::test] - async fn client_ca_without_a_quic_listener_is_rejected() { - let mut config = crate::listen::Config::default(); - config.tcp.bind = Some("127.0.0.1:0".parse().unwrap()); - config.tls.root = vec!["ca.pem".into()]; - - assert!(matches!( - config.clone().init(Default::default()), - Err(Error::MtlsUnsupported) - )); - // A worker group owning QUIC verifies it, so the streams alone accept it. - config.init_streams().expect("streams beside worker-owned QUIC"); + async fn client_auth_without_a_quic_listener_is_rejected() { + type Set = fn(&mut crate::listen::Config); + let cases: [Set; 2] = [ + |c| c.tls.root = vec!["ca.pem".into()], + |c| c.tls.peers = Some(crate::tls::Peers::new()), + ]; + for set in cases { + let mut config = crate::listen::Config::default(); + config.tcp.bind = Some("127.0.0.1:0".parse().unwrap()); + set(&mut config); + + assert!(matches!( + config.clone().init(Default::default()), + Err(Error::MtlsUnsupported) + )); + // A worker group owning QUIC verifies it, so the streams alone accept it. + config.init_streams().expect("streams beside worker-owned QUIC"); + } } /// An explicit QUIC bind cannot be honored without a QUIC backend. diff --git a/rs/moq-tokio/src/tcp.rs b/rs/moq-tokio/src/tcp.rs index b5b1b89dd6..45b3a07b0a 100644 --- a/rs/moq-tokio/src/tcp.rs +++ b/rs/moq-tokio/src/tcp.rs @@ -370,19 +370,19 @@ mod tests { listen.tcp.bind = Some("127.0.0.1:0".parse().unwrap()); listen.tcp.tls = Some(true); listen.tls.generate = vec!["localhost".into()]; + // Client-certificate policy is refused without a QUIC listener to read it. + if !cfg!(feature = "noq") { + continue; + } + listen.bind = Some("127.0.0.1:0".parse().unwrap()); match pinned { true => listen.tls.peers = Some(crate::tls::Peers::new()), - // A client CA is refused without a QUIC listener to verify it. false => { - if !cfg!(feature = "noq") { - continue; - } let ca = dir.path().join("client-ca.pem"); let cert = rcgen::generate_simple_self_signed(["client-ca".to_string()]) .unwrap() .cert; std::fs::write(&ca, cert.pem()).unwrap(); - listen.bind = Some("127.0.0.1:0".parse().unwrap()); listen.tls.root = vec![ca]; } }