From f2332701dcd74a9997aedfb0da2b82cb17b253ee Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 16 Jul 2026 11:55:26 -0700 Subject: [PATCH] refactor!: make the CONNECT request optional for raw QUIC sessions Raw QUIC carries no in-band request URL, but `Session::raw()` required a `ConnectRequest`. Callers had to synthesize a URL from the TLS SNI purely to satisfy the signature, and that URL was then discarded. SNI is optional (a client dialing a bare IP sends none, per RFC 6066), so the synthesis also forced a needless "SNI is required" constraint on the accept path. Drop the request from `Session::raw()` and make it optional instead: - `Session::raw(conn, response)` in the quinn, noq, and quiche backends - `Session::request() -> Option<&ConnectRequest>` - `web_transport::Session::url() -> Option<&Url>` The response stays on `raw()` because it carries the negotiated ALPN, which `protocol()` reads. The wasm router returns `Some` unconditionally so the signature is identical across targets and cross-platform callers still compile. No `path()` or `host()` accessor is added. Decomposing the URL was only ever needed to paper over the synthesized authority, and combining an unauthenticated `:authority` header with a cert-bound SNI behind one accessor would hide which of the two a caller was trusting. `ConnectRequest.url` is unchanged: it is still needed to encode the `:scheme`, `:authority`, and `:path` pseudo-headers. The FFI, Node, and browser bindings have no raw QUIC support, so their `url()` accessors are untouched. Adds tests/raw.rs, which covers raw sessions for the first time. BREAKING CHANGE: `Session::raw()` no longer takes a `ConnectRequest`, and both `Session::request()` and `web_transport::Session::url()` now return an `Option`. Co-Authored-By: Claude Opus 4.8 --- rs/web-transport-noq/src/session.rs | 29 ++-- rs/web-transport-quiche/src/connection.rs | 32 ++--- rs/web-transport-quinn/Cargo.toml | 1 + rs/web-transport-quinn/src/session.rs | 29 ++-- rs/web-transport-quinn/tests/raw.rs | 155 ++++++++++++++++++++++ rs/web-transport/src/quinn.rs | 7 +- rs/web-transport/src/wasm.rs | 7 +- 7 files changed, 212 insertions(+), 48 deletions(-) create mode 100644 rs/web-transport-quinn/tests/raw.rs diff --git a/rs/web-transport-noq/src/session.rs b/rs/web-transport-noq/src/session.rs index d812d8c9..c904b6f0 100644 --- a/rs/web-transport-noq/src/session.rs +++ b/rs/web-transport-noq/src/session.rs @@ -54,8 +54,8 @@ pub struct Session { // Uses OnceLock for set-once, first-writer-wins semantics with lock-free reads. error: Arc>, - // The request sent by the client. - request: ConnectRequest, + // The request sent by the client, or None for a raw QUIC session. + request: Option, // The response sent by the server. response: ConnectResponse, @@ -93,7 +93,7 @@ impl Session { settings: Some(Arc::new(settings)), connect_send: Arc::new(Mutex::new(Some(connect.send))), error: error.clone(), - request: connect.request.clone(), + request: Some(connect.request.clone()), response: connect.response.clone(), }; @@ -479,15 +479,14 @@ impl Session { } } - /// Create a new session from a raw QUIC connection and a URL. + /// Create a new session from a raw QUIC connection. /// - /// This is used to pretend like a QUIC connection is a WebTransport session. - /// It's a hack, but it makes it much easier to support WebTransport and raw QUIC simultaneously. - pub fn raw( - conn: noq::Connection, - request: impl Into, - response: impl Into, - ) -> Self { + /// This is used to pretend like a QUIC connection is a WebTransport session, + /// making it easier to support WebTransport and raw QUIC simultaneously. + /// + /// There is no CONNECT request, so [`Self::request`] returns `None`. The response + /// is supplied by the caller to carry the negotiated ALPN via [`Self::protocol`]. + pub fn raw(conn: noq::Connection, response: impl Into) -> Self { Self { conn, session_id: None, @@ -498,13 +497,15 @@ impl Session { settings: None, connect_send: Arc::new(Mutex::new(None)), error: Arc::new(OnceLock::new()), - request: request.into(), + request: None, response: response.into(), } } - pub fn request(&self) -> &ConnectRequest { - &self.request + /// Returns the [`ConnectRequest`] if this session was established over HTTP/3, + /// or `None` for a raw QUIC session. + pub fn request(&self) -> Option<&ConnectRequest> { + self.request.as_ref() } pub fn response(&self) -> &ConnectResponse { diff --git a/rs/web-transport-quiche/src/connection.rs b/rs/web-transport-quiche/src/connection.rs index cb12a3bb..0d6f76c0 100644 --- a/rs/web-transport-quiche/src/connection.rs +++ b/rs/web-transport-quiche/src/connection.rs @@ -61,7 +61,8 @@ pub struct Connection { settings: Option>, // The request and response that were sent and received. - request: ConnectRequest, + // The request is None for a raw QUIC session. + request: Option, response: ConnectResponse, } @@ -99,16 +100,16 @@ impl Connection { header_uni, header_bi, header_datagram, - request: connect.request.clone(), + request: Some(connect.request.clone()), response: connect.response.clone(), settings: Some(Arc::new(settings)), }; + tracing::debug!(url = %connect.request.url, "WebTransport connection established"); + // Run a background task to check if the connect stream is closed. tokio::spawn(this.clone().run_closed(connect)); - tracing::debug!(url = %this.request().url, "WebTransport connection established"); - this } @@ -304,15 +305,14 @@ impl Connection { self.conn.closed().await.into() } - /// Create a new session from a raw QUIC connection and a URL. + /// Create a new session from a raw QUIC connection. /// - /// This is used to pretend like a QUIC connection is a WebTransport session. - /// It's a hack, but it makes it much easier to support WebTransport and raw QUIC simultaneously. - pub fn raw( - conn: ez::Connection, - request: impl Into, - response: impl Into, - ) -> Self { + /// This is used to pretend like a QUIC connection is a WebTransport session, + /// making it easier to support WebTransport and raw QUIC simultaneously. + /// + /// There is no CONNECT request, so [`Self::request`] returns `None`. The response + /// is supplied by the caller to carry the negotiated ALPN via [`Self::protocol`]. + pub fn raw(conn: ez::Connection, response: impl Into) -> Self { let drop = Arc::new(ConnectionDrop { conn: conn.clone() }); Self { conn, @@ -323,13 +323,15 @@ impl Connection { header_datagram: Default::default(), accept: None, settings: None, - request: request.into(), + request: None, response: response.into(), } } - pub fn request(&self) -> &ConnectRequest { - &self.request + /// Returns the [`ConnectRequest`] if this session was established over HTTP/3, + /// or `None` for a raw QUIC session. + pub fn request(&self) -> Option<&ConnectRequest> { + self.request.as_ref() } pub fn response(&self) -> &ConnectResponse { diff --git a/rs/web-transport-quinn/Cargo.toml b/rs/web-transport-quinn/Cargo.toml index 2b67fa08..ed219715 100644 --- a/rs/web-transport-quinn/Cargo.toml +++ b/rs/web-transport-quinn/Cargo.toml @@ -50,6 +50,7 @@ web-transport-trait = { workspace = true } [dev-dependencies] anyhow = "1" clap = { version = "4", features = ["derive"] } +rcgen = "0.14" rustls-pemfile = "2" tokio = { version = "1", features = ["full"] } tracing-subscriber = { version = "0.3", features = ["env-filter"] } diff --git a/rs/web-transport-quinn/src/session.rs b/rs/web-transport-quinn/src/session.rs index 5959fc5c..e43e1f95 100644 --- a/rs/web-transport-quinn/src/session.rs +++ b/rs/web-transport-quinn/src/session.rs @@ -54,8 +54,8 @@ pub struct Session { // Uses OnceLock for set-once, first-writer-wins semantics with lock-free reads. error: Arc>, - // The request sent by the client. - request: ConnectRequest, + // The request sent by the client, or None for a raw QUIC session. + request: Option, // The response sent by the server. response: ConnectResponse, @@ -93,7 +93,7 @@ impl Session { settings: Some(Arc::new(settings)), connect_send: Arc::new(Mutex::new(Some(connect.send))), error: error.clone(), - request: connect.request.clone(), + request: Some(connect.request.clone()), response: connect.response.clone(), }; @@ -488,15 +488,14 @@ impl Session { } } - /// Create a new session from a raw QUIC connection and a URL. + /// Create a new session from a raw QUIC connection. /// - /// This is used to pretend like a QUIC connection is a WebTransport session. - /// It's a hack, but it makes it much easier to support WebTransport and raw QUIC simultaneously. - pub fn raw( - conn: quinn::Connection, - request: impl Into, - response: impl Into, - ) -> Self { + /// This is used to pretend like a QUIC connection is a WebTransport session, + /// making it easier to support WebTransport and raw QUIC simultaneously. + /// + /// There is no CONNECT request, so [`Self::request`] returns `None`. The response + /// is supplied by the caller to carry the negotiated ALPN via [`Self::protocol`]. + pub fn raw(conn: quinn::Connection, response: impl Into) -> Self { Self { conn, session_id: None, @@ -507,13 +506,15 @@ impl Session { settings: None, connect_send: Arc::new(Mutex::new(None)), error: Arc::new(OnceLock::new()), - request: request.into(), + request: None, response: response.into(), } } - pub fn request(&self) -> &ConnectRequest { - &self.request + /// Returns the [`ConnectRequest`] if this session was established over HTTP/3, + /// or `None` for a raw QUIC session. + pub fn request(&self) -> Option<&ConnectRequest> { + self.request.as_ref() } pub fn response(&self) -> &ConnectResponse { diff --git a/rs/web-transport-quinn/tests/raw.rs b/rs/web-transport-quinn/tests/raw.rs new file mode 100644 index 00000000..12c1a4a9 --- /dev/null +++ b/rs/web-transport-quinn/tests/raw.rs @@ -0,0 +1,155 @@ +//! Raw QUIC sessions: a QUIC connection using a non-HTTP/3 ALPN, wrapped in a +//! [`Session`] so callers can treat WebTransport and raw QUIC uniformly. +//! +//! There is no CONNECT request on this path, so there is no request URL. The +//! negotiated ALPN is carried by the response instead. + +// A crypto provider is needed to establish the connection, matching the gate on +// the crate's own builders. +#![cfg(any(feature = "aws-lc-rs", feature = "ring"))] + +use std::{ + net::{Ipv4Addr, SocketAddr}, + sync::Arc, +}; + +use anyhow::{Context, Result}; +use rcgen::{CertifiedKey, KeyPair}; +use rustls::pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer}; +// The trait provides `protocol()`; imported anonymously to avoid clashing with +// the concrete `Session`. +use web_transport_quinn::generic::Session as _; +use web_transport_quinn::{proto::ConnectResponse, Session}; + +/// A raw QUIC ALPN, i.e. anything other than the `h3` used by WebTransport. +const RAW_ALPN: &str = "moq-00"; + +fn self_signed() -> Result<(CertificateDer<'static>, PrivateKeyDer<'static>)> { + let CertifiedKey { cert, signing_key } = + rcgen::generate_simple_self_signed(vec!["localhost".into()]).context("rcgen")?; + + let cert_der = CertificateDer::from(cert.der().to_vec()); + let key_der = PrivateKeyDer::Pkcs8(PrivatePkcs8KeyDer::from(KeyPair::serialize_der( + &signing_key, + ))); + + Ok((cert_der, key_der)) +} + +/// The provider is passed explicitly rather than relying on the process-level +/// default, which rustls cannot determine when both backend features are enabled +/// (as `--all-features` does). +fn crypto_provider() -> Arc { + #[cfg(feature = "aws-lc-rs")] + { + Arc::new(rustls::crypto::aws_lc_rs::default_provider()) + } + #[cfg(all(feature = "ring", not(feature = "aws-lc-rs")))] + { + Arc::new(rustls::crypto::ring::default_provider()) + } +} + +/// Bind a QUIC server and connect a client to it, both offering only `RAW_ALPN`. +async fn connect_raw() -> Result<(quinn::Connection, quinn::Connection)> { + let (cert, key) = self_signed()?; + let provider = crypto_provider(); + + let mut server_crypto = rustls::ServerConfig::builder_with_provider(provider.clone()) + .with_protocol_versions(&[&rustls::version::TLS13])? + .with_no_client_auth() + .with_single_cert(vec![cert.clone()], key)?; + server_crypto.alpn_protocols = vec![RAW_ALPN.as_bytes().to_vec()]; + + let server_config = quinn::ServerConfig::with_crypto(Arc::new( + quinn::crypto::rustls::QuicServerConfig::try_from(server_crypto)?, + )); + + let bind: SocketAddr = (Ipv4Addr::LOCALHOST, 0).into(); + let server = quinn::Endpoint::server(server_config, bind)?; + let server_addr = server.local_addr()?; + + let mut roots = rustls::RootCertStore::empty(); + roots.add(cert)?; + + let mut client_crypto = rustls::ClientConfig::builder_with_provider(provider) + .with_protocol_versions(&[&rustls::version::TLS13])? + .with_root_certificates(roots) + .with_no_client_auth(); + client_crypto.alpn_protocols = vec![RAW_ALPN.as_bytes().to_vec()]; + + let client_config = quinn::ClientConfig::new(Arc::new( + quinn::crypto::rustls::QuicClientConfig::try_from(client_crypto)?, + )); + + let mut client = quinn::Endpoint::client((Ipv4Addr::LOCALHOST, 0).into())?; + client.set_default_client_config(client_config); + + let accept = tokio::spawn(async move { + let conn = server.accept().await.context("no connection")?.await?; + anyhow::Ok(conn) + }); + + let client_conn = client.connect(server_addr, "localhost")?.await?; + let server_conn = accept.await??; + + Ok((client_conn, server_conn)) +} + +/// The ALPN the peers negotiated, as an accept path would read it. +fn negotiated_alpn(conn: &quinn::Connection) -> Result { + let data = conn + .handshake_data() + .context("no handshake data")? + .downcast::() + .ok() + .context("unexpected handshake data")?; + + Ok(String::from_utf8(data.protocol.context("no ALPN")?)?) +} + +/// A raw session has no CONNECT request, so it has no URL. Before this was an +/// `Option`, callers had to synthesize a fake request to construct one. +#[tokio::test] +async fn raw_session_has_no_request() -> Result<()> { + let (client_conn, _server_conn) = connect_raw().await?; + + let session = Session::raw(client_conn, ConnectResponse::OK); + assert!(session.request().is_none()); + + Ok(()) +} + +/// The response is what carries the negotiated ALPN on the raw path. +#[tokio::test] +async fn raw_session_reports_alpn_as_protocol() -> Result<()> { + let (client_conn, _server_conn) = connect_raw().await?; + + let alpn = negotiated_alpn(&client_conn)?; + assert_eq!(alpn, RAW_ALPN); + + let session = Session::raw(client_conn, ConnectResponse::OK.with_protocol(&alpn)); + assert_eq!(session.protocol(), Some(RAW_ALPN)); + + Ok(()) +} + +/// A raw session must not write the WebTransport stream header, since the peer +/// is reading plain QUIC. +#[tokio::test] +async fn raw_session_streams_omit_webtransport_header() -> Result<()> { + let (client_conn, server_conn) = connect_raw().await?; + + let client = Session::raw(client_conn, ConnectResponse::OK); + let server = Session::raw(server_conn, ConnectResponse::OK); + + let mut send = client.open_uni().await?; + send.write_all(b"hello").await?; + send.finish()?; + + let mut recv = server.accept_uni().await?; + let data = recv.read_to_end(1024).await?; + assert_eq!(&data, b"hello"); + + Ok(()) +} diff --git a/rs/web-transport/src/quinn.rs b/rs/web-transport/src/quinn.rs index 133cb1b2..00890d81 100644 --- a/rs/web-transport/src/quinn.rs +++ b/rs/web-transport/src/quinn.rs @@ -173,9 +173,10 @@ impl Session { self.inner.closed().await.into() } - /// Return the URL used to create the session. - pub fn url(&self) -> &Url { - &self.inner.request().url + /// Return the URL used to create the session, or `None` for a raw QUIC + /// session established without an HTTP/3 CONNECT request. + pub fn url(&self) -> Option<&Url> { + self.inner.request().map(|request| &request.url) } /// Return the application protocol used to create the session. diff --git a/rs/web-transport/src/wasm.rs b/rs/web-transport/src/wasm.rs index 76732061..00925351 100644 --- a/rs/web-transport/src/wasm.rs +++ b/rs/web-transport/src/wasm.rs @@ -108,8 +108,11 @@ impl Session { } /// Return the URL used to create the session. - pub fn url(&self) -> &Url { - self.0.url() + /// + /// Always `Some` on this platform; the browser API has no raw QUIC session. + /// Matches the native signature so callers compile on both targets. + pub fn url(&self) -> Option<&Url> { + Some(self.0.url()) } /// Return the application protocol used to create the session.