From d0deb08d2c4a2f6858aaf2e22547e7a9a660d880 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Tue, 7 Apr 2026 22:51:52 +0300 Subject: [PATCH] feat: DoT write timeout and ALPN "dot" advertisement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two DoS/interop hardening items: 1. Bound write_framed by WRITE_TIMEOUT (10s) so a slow-reader attacker can't indefinitely hold a worker task and its connection permit. Symmetric to the existing handshake timeout. 2. Advertise ALPN "dot" per RFC 7858 §3.2. Required by some strict DoT clients (newer Apple stacks, some Android versions). rustls ServerConfig exposes alpn_protocols as a pub field so we set it after with_single_cert: - load_tls_config (user-provided cert/key): set directly - self_signed_tls (new, replaces fallback_tls): builds a fresh DoT-specific TLS config via build_tls_config with the ALPN list build_tls_config now takes an `alpn: Vec>` parameter so DoT and the proxy can pass different ALPN lists while sharing the same CA. Proxy callers pass Vec::new() (unchanged behavior). Dropped the ctx.tls_config reuse branch: we can't mutate a shared Arc to add DoT-specific ALPN, and reusing the proxy config was already quietly broken re: SAN (proxy cert covers *.{tld}, not the DoT server's bind hostname/IP). Added dot_negotiates_alpn test that asserts conn.alpn_protocol() returns Some(b"dot") after handshake. 126/126 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/dot.rs | 48 ++++++++++++++++++++++++++++++++++++------------ src/main.rs | 2 +- src/tls.rs | 13 ++++++++++--- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/dot.rs b/src/dot.rs index 360bf4a..898c2a1 100644 --- a/src/dot.rs +++ b/src/dot.rs @@ -19,10 +19,16 @@ use crate::packet::DnsPacket; const MAX_CONNECTIONS: usize = 512; const IDLE_TIMEOUT: Duration = Duration::from_secs(30); const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10); +const WRITE_TIMEOUT: Duration = Duration::from_secs(10); // Matches BytePacketBuffer::BUF_SIZE — RFC 7858 allows up to 65535 but our // buffer would silently truncate anything larger. const MAX_MSG_LEN: usize = 4096; +/// ALPN protocol identifier for DNS-over-TLS (RFC 7858 §3.2). +fn dot_alpn() -> Vec> { + vec![b"dot".to_vec()] +} + /// Build a TLS ServerConfig for DoT from user-provided cert/key PEM files. fn load_tls_config(cert_path: &Path, key_path: &Path) -> crate::Result> { let cert_pem = std::fs::read(cert_path)?; @@ -32,18 +38,18 @@ fn load_tls_config(cert_path: &Path, key_path: &Path) -> crate::Result Option> { - if let Some(arc_swap) = ctx.tls_config.as_ref() { - return Some(Arc::clone(&*arc_swap.load())); - } - match crate::tls::build_tls_config(&ctx.proxy_tld, &[]) { +/// Build a self-signed DoT TLS config. Can't reuse `ctx.tls_config` (the +/// proxy's shared config) because DoT needs its own ALPN advertisement. +fn self_signed_tls(ctx: &ServerCtx) -> Option> { + match crate::tls::build_tls_config(&ctx.proxy_tld, &[], dot_alpn()) { Ok(cfg) => Some(cfg), Err(e) => { warn!( @@ -65,7 +71,7 @@ pub async fn start_dot(ctx: Arc, config: &DotConfig) { return; } }, - _ => match fallback_tls(&ctx) { + _ => match self_signed_tls(&ctx) { Some(cfg) => cfg, None => return, }, @@ -228,6 +234,7 @@ where } /// Write a DNS message with its 2-byte length prefix, coalesced into one syscall. +/// Bounded by WRITE_TIMEOUT so a stalled reader can't indefinitely hold a worker. async fn write_framed(stream: &mut S, msg: &[u8]) -> std::io::Result<()> where S: AsyncWriteExt + Unpin, @@ -235,9 +242,15 @@ where let mut out = Vec::with_capacity(2 + msg.len()); out.extend_from_slice(&(msg.len() as u16).to_be_bytes()); out.extend_from_slice(msg); - stream.write_all(&out).await?; - stream.flush().await?; - Ok(()) + match tokio::time::timeout(WRITE_TIMEOUT, async { + stream.write_all(&out).await?; + stream.flush().await + }) + .await + { + Ok(result) => result, + Err(_) => Err(std::io::Error::other("write timeout")), + } } #[cfg(test)] @@ -271,16 +284,18 @@ mod tests { let cert_der = CertificateDer::from(cert.der().to_vec()); let key_der = PrivateKeyDer::Pkcs8(PrivatePkcs8KeyDer::from(key_pair.serialize_der())); - let server_config = ServerConfig::builder() + let mut server_config = ServerConfig::builder() .with_no_client_auth() .with_single_cert(vec![cert_der.clone()], key_der) .unwrap(); + server_config.alpn_protocols = dot_alpn(); let mut root_store = rustls::RootCertStore::empty(); root_store.add(cert_der).unwrap(); - let client_config = rustls::ClientConfig::builder() + let mut client_config = rustls::ClientConfig::builder() .with_root_certificates(root_store) .with_no_client_auth(); + client_config.alpn_protocols = dot_alpn(); (Arc::new(server_config), Arc::new(client_config)) } @@ -443,6 +458,15 @@ mod tests { assert_eq!(resp.questions[0].name, "nonexistent.test"); } + #[tokio::test] + async fn dot_negotiates_alpn() { + let (addr, client_config) = spawn_dot_server().await; + let stream = dot_connect(addr, &client_config).await; + // After handshake, the negotiated ALPN protocol should be "dot" (RFC 7858 §3.2). + let (_io, conn) = stream.get_ref(); + assert_eq!(conn.alpn_protocol(), Some(&b"dot"[..])); + } + #[tokio::test] async fn dot_concurrent_connections() { let (addr, client_config) = spawn_dot_server().await; diff --git a/src/main.rs b/src/main.rs index b9316b8..adf266e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -207,7 +207,7 @@ async fn main() -> numa::Result<()> { // Build initial TLS config before ServerCtx (so ArcSwap is ready at construction) let initial_tls = if config.proxy.enabled && config.proxy.tls_port > 0 { let service_names = service_store.names(); - match numa::tls::build_tls_config(&config.proxy.tld, &service_names) { + match numa::tls::build_tls_config(&config.proxy.tld, &service_names, Vec::new()) { Ok(tls_config) => Some(ArcSwap::from(tls_config)), Err(e) => { log::warn!("TLS setup failed, HTTPS proxy disabled: {}", e); diff --git a/src/tls.rs b/src/tls.rs index a4d91bf..5746f3b 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -24,7 +24,7 @@ pub fn regenerate_tls(ctx: &ServerCtx) { names.extend(ctx.lan_peers.lock().unwrap().names()); let names: Vec = names.into_iter().collect(); - match build_tls_config(&ctx.proxy_tld, &names) { + match build_tls_config(&ctx.proxy_tld, &names, Vec::new()) { Ok(new_config) => { tls.store(new_config); info!("TLS cert regenerated for {} services", names.len()); @@ -36,7 +36,13 @@ pub fn regenerate_tls(ctx: &ServerCtx) { /// Build a TLS config with a cert covering all provided service names. /// Wildcards under single-label TLDs (*.numa) are rejected by browsers, /// so we list each service explicitly as a SAN. -pub fn build_tls_config(tld: &str, service_names: &[String]) -> crate::Result> { +/// `alpn` is advertised in the TLS ServerHello — pass empty for the proxy +/// (which accepts any ALPN), or `[b"dot"]` for DoT (RFC 7858 §3.2). +pub fn build_tls_config( + tld: &str, + service_names: &[String], + alpn: Vec>, +) -> crate::Result> { let dir = crate::data_dir(); let (ca_cert, ca_key) = ensure_ca(&dir)?; let (cert_chain, key) = generate_service_cert(&ca_cert, &ca_key, tld, service_names)?; @@ -44,9 +50,10 @@ pub fn build_tls_config(tld: &str, service_names: &[String]) -> crate::Result