diff --git a/rs/moq-native/src/client.rs b/rs/moq-native/src/client.rs index 05fda2457..281d56041 100644 --- a/rs/moq-native/src/client.rs +++ b/rs/moq-native/src/client.rs @@ -97,7 +97,17 @@ pub struct Client { backoff: Backoff, #[cfg(feature = "websocket")] websocket: crate::websocket::Client, - tls: rustls::ClientConfig, + /// Prebuilt rustls config for the resolved verification policy, or `None` when + /// no CA roots resolved. Left unresolved so a per-connection `http://` + /// fingerprint bootstrap can still connect without roots; any other connection + /// then surfaces [`crate::tls::Error::NoRoots`] at connect time. + tls: Option, + /// The TLS config itself, kept to build a freshly pinned rustls config per + /// connection when bootstrapping a self-signed relay over `http://`. Only the + /// rustls-based quinn/noq backends rebuild from it; quiche carries its own + /// resolved policy. + #[cfg(any(feature = "quinn", feature = "noq"))] + tls_config: crate::tls::Client, #[cfg(feature = "noq")] noq: Option, #[cfg(feature = "quinn")] @@ -153,7 +163,14 @@ impl Client { panic!("no QUIC backend compiled; enable noq, quinn, or quiche feature"); }); - let tls = config.tls.build()?; + // Resolve the verification policy once. Building the rustls config is + // deferred to `None` when no roots resolved, so construction doesn't fail + // before a per-connection `http://` bootstrap gets a chance to pin a cert. + let tls = config + .tls + .verification()? + .map(|v| config.tls.build_with(v)) + .transpose()?; #[cfg(feature = "noq")] #[allow(unreachable_patterns)] @@ -183,6 +200,8 @@ impl Client { #[cfg(feature = "websocket")] websocket: config.websocket, tls, + #[cfg(any(feature = "quinn", feature = "noq"))] + tls_config: config.tls, #[cfg(feature = "noq")] noq, #[cfg(feature = "quinn")] @@ -302,9 +321,12 @@ impl Client { #[cfg(feature = "noq")] if let Some(noq) = self.noq.as_ref() { - let tls = self.tls.clone(); let quic_url = url.clone(); - let quic_handle = async { noq.connect(&tls, quic_url).await.map_err(Error::from) }; + let quic_handle = async { + noq.connect(self.tls.as_ref(), &self.tls_config, quic_url) + .await + .map_err(Error::from) + }; #[cfg(feature = "websocket")] { @@ -320,9 +342,13 @@ impl Client { #[cfg(feature = "quinn")] if let Some(quinn) = self.quinn.as_ref() { - let tls = self.tls.clone(); let quic_url = url.clone(); - let quic_handle = async { quinn.connect(&tls, quic_url).await.map_err(Error::from) }; + let quic_handle = async { + quinn + .connect(self.tls.as_ref(), &self.tls_config, quic_url) + .await + .map_err(Error::from) + }; #[cfg(feature = "websocket")] { @@ -356,7 +382,7 @@ impl Client { #[cfg(feature = "websocket")] { let alpns = self.versions.alpns(); - let session = crate::websocket::connect(&self.websocket, &self.tls, url, &alpns).await?; + let session = crate::websocket::connect(&self.websocket, self.tls.as_ref(), url, &alpns).await?; return Ok(self.moq.connect(session).await?); } @@ -374,7 +400,7 @@ impl Client { let ws_config = self.websocket.clone(); let ws_tls = self.tls.clone(); let websocket = async move { - crate::websocket::race_handle(&ws_config, &ws_tls, url, &alpns) + crate::websocket::race_handle(&ws_config, ws_tls.as_ref(), url, &alpns) .await .map(|res| res.map_err(Error::from)) }; @@ -525,6 +551,22 @@ mod tests { assert_eq!(config.tls.disable_verify, None); } + #[cfg(feature = "quinn")] + #[tokio::test] + async fn init_defers_no_roots() { + // Regression: a roots-less config (system roots off, no custom root, no + // fingerprint) used to fail construction with NoRoots before any URL scheme + // was known. It now builds, so a per-connection http:// fingerprint + // bootstrap can still connect; other schemes surface NoRoots at connect time. + let mut tls = crate::tls::Client::default(); + tls.system_roots = Some(false); + let config = ClientConfig { + tls, + ..Default::default() + }; + assert!(config.init().is_ok()); + } + #[test] fn test_toml_fingerprint_survives_update_from() { let toml = r#" diff --git a/rs/moq-native/src/lib.rs b/rs/moq-native/src/lib.rs index 96717cfcb..5dac06346 100644 --- a/rs/moq-native/src/lib.rs +++ b/rs/moq-native/src/lib.rs @@ -13,6 +13,11 @@ /// Default maximum number of concurrent QUIC streams (bidi and uni) per connection. pub(crate) const DEFAULT_MAX_STREAMS: u64 = 1024; +/// Overall timeout for the insecure `http://` certificate-fingerprint fetch, so a +/// stalled `/certificate.sha256` response can't block connect indefinitely. +#[cfg(any(feature = "quinn", feature = "noq", feature = "quiche"))] +pub(crate) const FINGERPRINT_FETCH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + pub mod bind; mod client; mod connect; diff --git a/rs/moq-native/src/noq.rs b/rs/moq-native/src/noq.rs index a43320324..5d8e41a14 100644 --- a/rs/moq-native/src/noq.rs +++ b/rs/moq-native/src/noq.rs @@ -1,6 +1,6 @@ use crate::client::ClientConfig; use crate::server::{ServerConfig, ServerId}; -use crate::tls::{FingerprintVerifier, ServeCerts}; +use crate::tls::ServeCerts; use std::sync::{Arc, RwLock}; use std::time::Duration; use std::{net, time}; @@ -50,6 +50,10 @@ pub enum Error { #[error("invalid fingerprint")] InvalidFingerprint(#[from] hex::FromHexError), + /// The fetched `http://` certificate fingerprint didn't decode to a 32-byte SHA-256 digest. + #[error("certificate fingerprint must be 32 bytes (SHA-256), got {0}")] + FingerprintLength(usize), + #[error("url scheme must be 'https', 'moqt', or 'moql'")] InvalidScheme, @@ -156,9 +160,20 @@ impl NoqClient { }) } - pub async fn connect(&self, tls: &rustls::ClientConfig, url: Url) -> Result { + /// Connect to `url`, resolving TLS now that the scheme is known. + /// + /// `base` is the prebuilt config for the resolved verification policy, or + /// `None` when no roots resolved. An eligible `http://` URL fetches + /// `/certificate.sha256`, pins it (rebuilding from `tls_config`, no roots + /// needed), and upgrades to `https://`; any other scheme reuses `base` and + /// surfaces [`crate::tls::Error::NoRoots`] when it is `None`. + pub async fn connect( + &self, + base: Option<&rustls::ClientConfig>, + tls_config: &crate::tls::Client, + url: Url, + ) -> Result { let mut url = url; - let mut config = tls.clone(); let host = url.host().ok_or(Error::InvalidDnsName)?.to_string(); let port = url.port().unwrap_or(443); @@ -174,36 +189,28 @@ impl NoqClient { .map_err(Error::DnsLookup)?; let ip = crate::util::pick_addr(addrs, local).ok_or(Error::NoDnsEntries)?; - if url.scheme() == "http" { - // Insecure per-connection bootstrap: only honored when no stronger - // verification is configured, so an attacker controlling the plaintext - // fetch can't weaken an explicit pin or re-enable disabled verification. - if self.http_bootstrap { - // Perform a HTTP request to fetch the certificate fingerprint. - let mut fingerprint = url.clone(); - fingerprint.set_path("/certificate.sha256"); - fingerprint.set_query(None); - fingerprint.set_fragment(None); - - tracing::warn!(url = %fingerprint, "performing insecure HTTP request for certificate"); - - let resp = reqwest::get(fingerprint.as_str()) - .await - .map_err(Error::FetchFingerprint)? - .error_for_status() - .map_err(Error::FingerprintStatus)?; - - let fingerprint = resp.text().await.map_err(Error::ReadFingerprint)?; - let fingerprint = hex::decode(fingerprint.trim())?; - - let verifier = FingerprintVerifier::new(config.crypto_provider().clone(), vec![fingerprint]); - config.dangerous().set_certificate_verifier(Arc::new(verifier)); - } else { + // Resolve the per-connection rustls config now that the scheme is known. + // An `http://` bootstrap pins a freshly fetched fingerprint (no roots + // needed); anything else reuses the resolved policy, surfacing the + // deferred `NoRoots` when none was configured. + let mut config = if url.scheme() == "http" && self.http_bootstrap { + // Insecure per-connection bootstrap: an `http://` URL is a deliberate + // opt-in to replace CA validation with a freshly fetched pin for this + // connection. It's refused for an explicit `--client-tls-fingerprint` + // or disabled verification, so an attacker controlling the plaintext + // fetch can't weaken a stronger choice the user already made. + let pin = fetch_fingerprint(&url).await?; + tls_config.build_with(crate::tls::Verification::Fingerprints(vec![pin]))? + } else { + if url.scheme() == "http" { tracing::warn!( "ignoring insecure http:// fingerprint bootstrap; using the configured TLS verification" ); } + base.cloned().ok_or(crate::tls::Error::NoRoots)? + }; + if url.scheme() == "http" { url.set_scheme("https").expect("failed to set scheme"); } @@ -259,6 +266,33 @@ impl NoqClient { } } +/// Fetch a relay's certificate SHA-256 over an insecure `http://` request and +/// return it as a pin. Mirrors the browser's self-signed bootstrap: GET +/// `/certificate.sha256` and pin the returned hash. +async fn fetch_fingerprint(url: &Url) -> Result<[u8; 32]> { + let mut fp = url.clone(); + fp.set_path("/certificate.sha256"); + fp.set_query(None); + fp.set_fragment(None); + + tracing::warn!(url = %fp, "performing insecure HTTP request for certificate"); + + // Bound the fetch so a stalled response can't block connect indefinitely. + let resp = reqwest::Client::builder() + .timeout(crate::FINGERPRINT_FETCH_TIMEOUT) + .build() + .map_err(Error::FetchFingerprint)? + .get(fp.as_str()) + .send() + .await + .map_err(Error::FetchFingerprint)? + .error_for_status() + .map_err(Error::FingerprintStatus)?; + let text = resp.text().await.map_err(Error::ReadFingerprint)?; + let bytes = hex::decode(text.trim())?; + bytes.try_into().map_err(|v: Vec| Error::FingerprintLength(v.len())) +} + impl Error { pub(crate) fn connect_error(&self) -> Option { match self { diff --git a/rs/moq-native/src/quiche.rs b/rs/moq-native/src/quiche.rs index d51791189..12ee406ea 100644 --- a/rs/moq-native/src/quiche.rs +++ b/rs/moq-native/src/quiche.rs @@ -105,7 +105,9 @@ type Result = std::result::Result; pub(crate) struct QuicheClient { pub bind: net::SocketAddr, /// Resolved server-verification policy, shared with the other backends. - pub verification: crate::tls::Verification, + /// `None` when no CA roots resolved: deferred so an `http://` bootstrap can + /// still pin a fetched fingerprint, while other schemes surface `NoRoots`. + pub verification: Option, /// Whether an `http://` URL may bootstrap a pin (see [crate::tls::Client::allows_http_bootstrap]). pub http_bootstrap: bool, pub max_streams: u64, @@ -145,10 +147,10 @@ impl QuicheClient { tracing::warn!( "ignoring insecure http:// fingerprint bootstrap; using the configured TLS verification" ); - (https, self.verification.clone()) + (https, self.verification.clone().ok_or(crate::tls::Error::NoRoots)?) } } else { - (url, self.verification.clone()) + (url, self.verification.clone().ok_or(crate::tls::Error::NoRoots)?) }; let alpns: Vec> = match url.scheme() { @@ -238,7 +240,13 @@ async fn fetch_fingerprint(url: &Url) -> Result<[u8; 32]> { tracing::warn!(url = %fp, "performing insecure HTTP request for certificate fingerprint"); - let resp = reqwest::get(fp.as_str()) + // Bound the fetch so a stalled response can't block connect indefinitely. + let resp = reqwest::Client::builder() + .timeout(crate::FINGERPRINT_FETCH_TIMEOUT) + .build() + .map_err(Error::FetchFingerprint)? + .get(fp.as_str()) + .send() .await .map_err(Error::FetchFingerprint)? .error_for_status() diff --git a/rs/moq-native/src/quinn.rs b/rs/moq-native/src/quinn.rs index ac747634f..0333ce3cc 100644 --- a/rs/moq-native/src/quinn.rs +++ b/rs/moq-native/src/quinn.rs @@ -1,6 +1,6 @@ use crate::client::ClientConfig; use crate::server::{ServerConfig, ServerId}; -use crate::tls::{FingerprintVerifier, ServeCerts}; +use crate::tls::ServeCerts; use std::sync::{Arc, RwLock}; use std::time::Duration; use std::{net, time}; @@ -48,6 +48,10 @@ pub enum Error { #[error("invalid fingerprint")] InvalidFingerprint(#[from] hex::FromHexError), + /// The fetched `http://` certificate fingerprint didn't decode to a 32-byte SHA-256 digest. + #[error("certificate fingerprint must be 32 bytes (SHA-256), got {0}")] + FingerprintLength(usize), + #[error("url scheme must be 'https', 'moqt', or 'moql'")] InvalidScheme, @@ -154,9 +158,20 @@ impl QuinnClient { }) } - pub async fn connect(&self, tls: &rustls::ClientConfig, url: Url) -> Result { + /// Connect to `url`, resolving TLS now that the scheme is known. + /// + /// `base` is the prebuilt config for the resolved verification policy, or + /// `None` when no roots resolved. An eligible `http://` URL fetches + /// `/certificate.sha256`, pins it (rebuilding from `tls_config`, no roots + /// needed), and upgrades to `https://`; any other scheme reuses `base` and + /// surfaces [`crate::tls::Error::NoRoots`] when it is `None`. + pub async fn connect( + &self, + base: Option<&rustls::ClientConfig>, + tls_config: &crate::tls::Client, + url: Url, + ) -> Result { let mut url = url; - let mut config = tls.clone(); let host = url.host().ok_or(Error::InvalidDnsName)?.to_string(); let port = url.port().unwrap_or(443); @@ -172,36 +187,28 @@ impl QuinnClient { .map_err(Error::DnsLookup)?; let ip = crate::util::pick_addr(addrs, local).ok_or(Error::NoDnsEntries)?; - if url.scheme() == "http" { - // Insecure per-connection bootstrap: only honored when no stronger - // verification is configured, so an attacker controlling the plaintext - // fetch can't weaken an explicit pin or re-enable disabled verification. - if self.http_bootstrap { - // Perform a HTTP request to fetch the certificate fingerprint. - let mut fingerprint = url.clone(); - fingerprint.set_path("/certificate.sha256"); - fingerprint.set_query(None); - fingerprint.set_fragment(None); - - tracing::warn!(url = %fingerprint, "performing insecure HTTP request for certificate"); - - let resp = reqwest::get(fingerprint.as_str()) - .await - .map_err(Error::FetchFingerprint)? - .error_for_status() - .map_err(Error::FingerprintStatus)?; - - let fingerprint = resp.text().await.map_err(Error::ReadFingerprint)?; - let fingerprint = hex::decode(fingerprint.trim())?; - - let verifier = FingerprintVerifier::new(config.crypto_provider().clone(), vec![fingerprint]); - config.dangerous().set_certificate_verifier(Arc::new(verifier)); - } else { + // Resolve the per-connection rustls config now that the scheme is known. + // An `http://` bootstrap pins a freshly fetched fingerprint (no roots + // needed); anything else reuses the resolved policy, surfacing the + // deferred `NoRoots` when none was configured. + let mut config = if url.scheme() == "http" && self.http_bootstrap { + // Insecure per-connection bootstrap: an `http://` URL is a deliberate + // opt-in to replace CA validation with a freshly fetched pin for this + // connection. It's refused for an explicit `--client-tls-fingerprint` + // or disabled verification, so an attacker controlling the plaintext + // fetch can't weaken a stronger choice the user already made. + let pin = fetch_fingerprint(&url).await?; + tls_config.build_with(crate::tls::Verification::Fingerprints(vec![pin]))? + } else { + if url.scheme() == "http" { tracing::warn!( "ignoring insecure http:// fingerprint bootstrap; using the configured TLS verification" ); } + base.cloned().ok_or(crate::tls::Error::NoRoots)? + }; + if url.scheme() == "http" { url.set_scheme("https").expect("failed to set scheme"); } @@ -257,6 +264,33 @@ impl QuinnClient { } } +/// Fetch a relay's certificate SHA-256 over an insecure `http://` request and +/// return it as a pin. Mirrors the browser's self-signed bootstrap: GET +/// `/certificate.sha256` and pin the returned hash. +async fn fetch_fingerprint(url: &Url) -> Result<[u8; 32]> { + let mut fp = url.clone(); + fp.set_path("/certificate.sha256"); + fp.set_query(None); + fp.set_fragment(None); + + tracing::warn!(url = %fp, "performing insecure HTTP request for certificate"); + + // Bound the fetch so a stalled response can't block connect indefinitely. + let resp = reqwest::Client::builder() + .timeout(crate::FINGERPRINT_FETCH_TIMEOUT) + .build() + .map_err(Error::FetchFingerprint)? + .get(fp.as_str()) + .send() + .await + .map_err(Error::FetchFingerprint)? + .error_for_status() + .map_err(Error::FingerprintStatus)?; + let text = resp.text().await.map_err(Error::ReadFingerprint)?; + let bytes = hex::decode(text.trim())?; + bytes.try_into().map_err(|v: Vec| Error::FingerprintLength(v.len())) +} + impl Error { pub(crate) fn connect_error(&self) -> Option { match self { diff --git a/rs/moq-native/src/tls.rs b/rs/moq-native/src/tls.rs index 2ae4ee3d3..8bc95b50c 100644 --- a/rs/moq-native/src/tls.rs +++ b/rs/moq-native/src/tls.rs @@ -238,10 +238,11 @@ struct Deprecated { /// The resolved server-certificate verification policy. /// -/// Computed once by [Client::verification] and shared by every backend (the -/// rustls-based quinn/noq via [Client::build], and quiche directly) so they +/// Resolved once by [Client::verification] and reused by every backend (the +/// rustls-based quinn/noq via [Client::build_with], and quiche directly) so they /// agree on precedence, the system-roots default, and which flag combinations -/// are valid. +/// are valid. A per-connection `http://` bootstrap substitutes its own +/// [`Verification::Fingerprints`] without touching the resolved policy. #[derive(Clone)] pub(crate) enum Verification { /// No verification at all. Insecure; only via `--client-tls-disable-verify`. @@ -308,11 +309,16 @@ impl Client { /// - Otherwise, verify against the system roots (default) plus any custom /// roots. The system roots are dropped once a custom root is given unless /// `--client-tls-system-roots` re-enables them. - pub(crate) fn verification(&self) -> Result { + /// + /// Returns `Ok(None)` when standard root verification is selected but no roots + /// resolved: WebPKI needs at least one trusted root, but the caller may still + /// connect if a per-connection `http://` fingerprint bootstrap replaces + /// verification. Callers that can't bootstrap map `None` to [`Error::NoRoots`]. + pub(crate) fn verification(&self) -> Result> { self.warn_deprecated(); if self.effective_disable_verify().unwrap_or_default() { - return Ok(Verification::Disabled); + return Ok(Some(Verification::Disabled)); } let fingerprints = self.fingerprints()?; @@ -320,7 +326,7 @@ impl Client { if !self.effective_root().is_empty() || self.effective_system_roots() == Some(true) { return Err(Error::FingerprintWithRoots); } - return Ok(Verification::Fingerprints(fingerprints)); + return Ok(Some(Verification::Fingerprints(fingerprints))); } let root = self.effective_root(); @@ -344,13 +350,13 @@ impl Client { roots.extend(certs); } - // WebPKI needs at least one trusted root to ever succeed, so fail fast - // instead of producing confusing handshake errors later. + // No roots resolved: defer to the caller, which may still bootstrap a pin + // over `http://` for a single connection. if roots.is_empty() { - return Err(Error::NoRoots); + return Ok(None); } - Ok(Verification::Roots(roots)) + Ok(Some(Verification::Roots(roots))) } /// Whether an insecure `http://` certificate-fingerprint bootstrap may be @@ -379,10 +385,24 @@ impl Client { /// Build a [`rustls::ClientConfig`] from this configuration. /// /// Resolves the verification policy, optionally attaches a client identity - /// for mTLS, and installs the matching verifier. + /// for mTLS, and installs the matching verifier. Errors with [`Error::NoRoots`] + /// when standard verification is selected but no roots resolved (the native + /// client defers that decision per connection so an `http://` bootstrap can + /// still pin a cert). pub fn build(&self) -> Result { + let verification = self.verification()?.ok_or(Error::NoRoots)?; + self.build_with(verification) + } + + /// Build a [`rustls::ClientConfig`] for an already-resolved verification policy. + /// + /// Lets the native client resolve [`Self::verification`] once, then build a + /// config per connection: an `http://` URL can swap in a freshly fetched pin + /// without any configured roots, while other connections reuse the resolved + /// policy. Optionally attaches a client identity for mTLS and installs the + /// matching verifier. + pub(crate) fn build_with(&self, verification: Verification) -> Result { let provider = crypto::provider(); - let verification = self.verification()?; let mut roots = rustls::RootCertStore::empty(); if let Verification::Roots(certs) = &verification { @@ -799,6 +819,18 @@ mod tests { assert!(matches!(config.build(), Err(Error::NoRoots))); } + #[test] + fn verification_defers_no_roots() { + // Same roots-less config: resolution defers (Ok(None)) instead of erroring + // so a per-connection http:// bootstrap can still pin a fetched cert. The + // eager build() above is what maps that None to NoRoots. + let config = Client { + system_roots: Some(false), + ..Default::default() + }; + assert!(matches!(config.verification(), Ok(None))); + } + #[test] fn build_allows_no_roots_when_verification_overridden() { // disable_verify swaps in its own verifier, so an empty store is fine. diff --git a/rs/moq-native/src/websocket.rs b/rs/moq-native/src/websocket.rs index 43b5efb58..490588a77 100644 --- a/rs/moq-native/src/websocket.rs +++ b/rs/moq-native/src/websocket.rs @@ -38,6 +38,10 @@ pub enum Error { #[error("WebSocket accept failed")] Accept(#[source] qmux::Error), + + /// TLS resolution failed for a secure (`wss://`) connection, e.g. no roots were configured. + #[error(transparent)] + Tls(#[from] crate::tls::Error), } type Result = std::result::Result; @@ -85,7 +89,7 @@ impl Default for Client { pub(crate) async fn race_handle( config: &Client, - tls: &rustls::ClientConfig, + tls: Option<&rustls::ClientConfig>, url: Url, alpns: &[&str], ) -> Option> { @@ -109,7 +113,7 @@ pub(crate) async fn race_handle( pub(crate) async fn connect( config: &Client, - tls: &rustls::ClientConfig, + tls: Option<&rustls::ClientConfig>, mut url: Url, alpns: &[&str], ) -> Result { @@ -154,8 +158,11 @@ pub(crate) async fn connect( tracing::debug!(%url, "connecting via WebSocket"); - // Use the existing TLS config (which respects tls-disable-verify) for secure connections. + // Use the existing TLS config (which respects tls-disable-verify) for secure + // connections. Plaintext `ws://` needs no roots, so only a secure scheme + // surfaces the deferred [`crate::tls::Error::NoRoots`] when none resolved. let connector = if needs_tls { + let tls = tls.ok_or(crate::tls::Error::NoRoots)?; tokio_tungstenite::Connector::Rustls(Arc::new(tls.clone())) } else { tokio_tungstenite::Connector::Plain