From 3a529dd8879d018214396f631ac34e42cab41c80 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Wed, 29 Jul 2026 08:35:20 -0500 Subject: [PATCH 1/3] dns: add the resolver-transport axis (DoT/TCP/UDP/system), orthogonal to wire shaping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Realizes the "diversity across ... AND transports" spread that design §6 already called for but only implemented for DoH. `pool::Kind` now names the DNS axis of a proxyless strategy: Doh, Dot, Tcp, Udp, System. Keeps that axis independent of the wire-shaping axis (§5) instead of folding shaping into the resolver. `Resolver::strategy_with(wire)` and `resolve_one_shaped` compose the two, so a DoH lookup can itself be carried over a fragmented, jittered ClientHello — the same shaping vocabulary used for a destination dial, aimed at the DNS dial. That makes the strategy space a product (`resolver × wire`) a search can enumerate, rather than a fixed list of hand-written combinations. flint-dial re-exports WirePlan/RecordFragment/ SegmentSplit/DelaySpec so a consumer can express that without taking a direct flint-shaping dependency. Trust across the axis is not uniform, and the code is explicit about it: - DoH/DoT are encrypted; only they are shapeable (no ClientHello otherwise). - Plaintext TCP/UDP and the system resolver are poisonable. `validate` rejects bogons but cannot prove an answer is *correct*, so these kinds are excluded from `default_pool()` and are sound only inside a search that verifies the answer end-to-end via a valid-certificate TLS handshake to the resolved address. - Where plaintext is used, the query carries a CSPRNG transaction ID that is verified on return (`build_query_with_id` / `parse_response_with_id`), and UDP `connect`s its socket on a random ephemeral port so the kernel drops datagrams from any other source. That is the standard bar against off-path injection, which censors including the GFW perform by blasting forged answers without ever seeing the query. DoH keeps ID 0 per RFC 8484 §4.1, since its own framing binds the response. Stream framing (2-byte length prefix, RFC 1035 §4.2.2) is shared by DoT and plaintext TCP, which differ only in how the stream was obtained; both cap the response so a hostile length prefix cannot drive a large allocation. POOL_MAGIC FRP1 → FRP2: signed pools are postcard, which is not self-describing, so `Resolver`'s new field shifts every following byte. An FRP1 client handed an FRP2 pool would mis-parse the whole list rather than merely miss `kind` — hence a magic bump, which is exactly what the constant documents itself as being for. Also fixes a pre-existing clippy failure (useless_borrows_in_formatting under clippy 1.97) in flint-fronted's meek_live test, which was blocking the workspace `-D warnings` gate. Gate: cargo fmt --all --check, cargo clippy --workspace --all-targets -D warnings, and cargo test --workspace all clean; flint-dns 25 tests (7 new, covering stream framing round-trip, zero/oversized length prefixes, UDP round trip, off-path-source rejection, and transaction-ID set/verify). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BufGsK81otiqihvrZdfoJX --- crates/flint-dial/src/lib.rs | 4 + crates/flint-dns/Cargo.toml | 11 +- crates/flint-dns/src/codec.rs | 91 +++++++++- crates/flint-dns/src/lib.rs | 122 ++++++++++++-- crates/flint-dns/src/plain.rs | 213 ++++++++++++++++++++++++ crates/flint-dns/src/pool.rs | 156 +++++++++++++++-- crates/flint-dns/src/signed.rs | 40 +++-- crates/flint-fronted/tests/meek_live.rs | 2 +- docs/design.md | 25 +++ 9 files changed, 609 insertions(+), 55 deletions(-) create mode 100644 crates/flint-dns/src/plain.rs diff --git a/crates/flint-dial/src/lib.rs b/crates/flint-dial/src/lib.rs index fad6722..242ae98 100644 --- a/crates/flint-dial/src/lib.rs +++ b/crates/flint-dial/src/lib.rs @@ -28,6 +28,10 @@ mod race; mod strategy; pub use engine::{dial, dial_alpn, dial_over, dial_over_alpn}; +// Re-exported because `BootstrapStrategy::wire` is part of this crate's public API: a consumer cannot +// compose opening-handshake shaping (record fragmentation, segment splitting, inter-segment jitter) +// without these types, and should not have to take a direct `flint-shaping` dependency to do it. +pub use flint_shaping::{DelaySpec, RecordFragment, SegmentSplit, WirePlan}; pub use flint_tls::CertVerification; pub use race::{probe_windowed, race, race_windowed, race_with}; pub use strategy::{BootstrapStrategy, TlsEngine}; diff --git a/crates/flint-dns/Cargo.toml b/crates/flint-dns/Cargo.toml index 7c5fc89..2bac6e2 100644 --- a/crates/flint-dns/Cargo.toml +++ b/crates/flint-dns/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flint-dns" -description = "Resilient DoH resolver (race, validate, cache) + a minimal A/AAAA DNS codec." +description = "Resilient DNS resolver (DoH/DoT/TCP/UDP/system; race, validate, cache) + a minimal A/AAAA DNS codec." version.workspace = true edition.workspace = true rust-version.workspace = true @@ -22,14 +22,17 @@ futures = { workspace = true } h2 = { workspace = true } http = { workspace = true } postcard = { workspace = true } +# CSPRNG for plaintext DNS transaction IDs (the off-path-injection defense). The same `ring` the rest +# of flint already uses, so this adds no new crypto dependency. +ring = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } -tokio = { version = "1", default-features = false, features = ["io-util", "rt", "time"] } +# `net` is required by the non-DoH transports: a UDP socket, a plain TCP connect, and the system +# resolver's `lookup_host`. +tokio = { version = "1", default-features = false, features = ["io-util", "net", "rt", "time"] } [dev-dependencies] # A local in-memory h2 server for the DoH round-trip test (no network). tokio = { version = "1", default-features = false, features = ["macros", "rt", "rt-multi-thread", "net", "io-util"] } h2 = { workspace = true } http = { workspace = true } -# Ed25519 *signing* to forge a signed pool in the signed-update tests (clients only ever verify). -ring = { workspace = true } diff --git a/crates/flint-dns/src/codec.rs b/crates/flint-dns/src/codec.rs index 210853a..e8125a6 100644 --- a/crates/flint-dns/src/codec.rs +++ b/crates/flint-dns/src/codec.rs @@ -27,13 +27,36 @@ pub enum DnsError { /// The server returned a non-zero RCODE (e.g. 3 = NXDOMAIN, 2 = SERVFAIL). #[error("DNS server returned RCODE {0}")] Rcode(u8), + /// The response's transaction ID did not match the query's. Only checked on plaintext transports + /// (see [`parse_response_with_id`]), where a mismatch means an injected/stale answer. + #[error("DNS response ID {got:#06x} does not match query ID {want:#06x}")] + IdMismatch { + /// The ID the response carried. + got: u16, + /// The ID our query used. + want: u16, + }, } /// Build a standard recursive query for `name`/`qtype` (class IN). The transaction ID is `0`, as /// recommended for DoH (RFC 8484 §4.1 — improves cache friendliness since DoH has its own framing). +/// +/// **Encrypted transports only.** On a plaintext transport an ID of `0` is guessable, so an off-path +/// injector can forge an answer; use [`build_query_with_id`] with a random ID there, and verify it with +/// [`parse_response_with_id`]. pub fn build_query(name: &str, qtype: u16) -> Result, DnsError> { + build_query_with_id(name, qtype, 0) +} + +/// Like [`build_query`], but with an explicit transaction `id`. +/// +/// Plaintext DNS (UDP/TCP) has no channel binding, so the 16-bit ID plus a random source port is all +/// that stops an **off-path** attacker from injecting a forged answer — a technique censors use in +/// practice (the GFW blasts forged responses without seeing the query). Callers on those transports +/// must pass a CSPRNG-drawn `id` and check it on the way back. +pub fn build_query_with_id(name: &str, qtype: u16, id: u16) -> Result, DnsError> { let mut q = Vec::with_capacity(name.len() + 18); - q.extend_from_slice(&[0x00, 0x00]); // ID = 0 + q.extend_from_slice(&id.to_be_bytes()); q.extend_from_slice(&[0x01, 0x00]); // flags: RD (recursion desired) q.extend_from_slice(&[0x00, 0x01]); // QDCOUNT = 1 q.extend_from_slice(&[0x00, 0x00, 0x00, 0x00, 0x00, 0x00]); // AN/NS/AR counts = 0 @@ -59,9 +82,31 @@ fn encode_name(name: &str, out: &mut Vec) -> Result<(), DnsError> { Ok(()) } +/// Like [`parse_response`], but first requires the response's transaction ID to equal `expected_id`. +/// +/// **Use this on every plaintext transport.** Inside DoH/DoT the encrypted stream already binds a +/// response to our query, but on UDP/TCP the ID is the binding, so an unchecked ID means accepting +/// whatever arrives first — including an off-path forgery. Pair with [`build_query_with_id`]. +pub fn parse_response_with_id(buf: &[u8], expected_id: u16) -> Result, DnsError> { + if buf.len() < 12 { + return Err(DnsError::Truncated); + } + let got = u16::from_be_bytes([buf[0], buf[1]]); + if got != expected_id { + return Err(DnsError::IdMismatch { + got, + want: expected_id, + }); + } + parse_response(buf) +} + /// Parse a DNS response, returning the A/AAAA addresses in its answer section. Errors on a truncated /// message, a query (not a response), or a non-zero RCODE. An empty answer list is *not* an error here /// (the validation layer decides what an empty/poisoned answer means). +/// +/// Does **not** check the transaction ID — safe for DoH/DoT (the channel binds the response) but not +/// for plaintext; see [`parse_response_with_id`]. pub fn parse_response(buf: &[u8]) -> Result, DnsError> { if buf.len() < 12 { return Err(DnsError::Truncated); @@ -223,4 +268,48 @@ mod tests { let long = "a".repeat(64); assert_eq!(build_query(&long, TYPE_A), Err(DnsError::BadName)); } + + #[test] + fn build_query_with_id_sets_the_id_and_plain_build_query_stays_zero() { + let q = build_query_with_id("example.com", TYPE_A, 0xbeef).unwrap(); + assert_eq!(&q[0..2], &[0xbe, 0xef]); + // DoH deliberately keeps ID 0 (RFC 8484 §4.1); only the plaintext paths randomize. + assert_eq!(&build_query("example.com", TYPE_A).unwrap()[0..2], &[0, 0]); + // The ID must be the only difference from the ID-0 query. + let mut zeroed = q.clone(); + zeroed[0] = 0; + zeroed[1] = 0; + assert_eq!(zeroed, build_query("example.com", TYPE_A).unwrap()); + } + + #[test] + fn parse_response_with_id_rejects_a_mismatched_id() { + let mut resp = build_response( + "example.com", + TYPE_A, + 0, + &[(TYPE_A, vec![93, 184, 216, 34])], + ); + resp[0] = 0x12; + resp[1] = 0x34; + + // The matching ID is accepted. + assert_eq!( + parse_response_with_id(&resp, 0x1234).unwrap(), + vec!["93.184.216.34".parse::().unwrap()] + ); + // An answer carrying anyone else's ID is an injection or a stale reply — reject it. + assert_eq!( + parse_response_with_id(&resp, 0x9999), + Err(DnsError::IdMismatch { + got: 0x1234, + want: 0x9999 + }) + ); + // A runt message is rejected before the ID is even read. + assert_eq!( + parse_response_with_id(&[0u8; 4], 0x1234), + Err(DnsError::Truncated) + ); + } } diff --git a/crates/flint-dns/src/lib.rs b/crates/flint-dns/src/lib.rs index ac74470..56e63c3 100644 --- a/crates/flint-dns/src/lib.rs +++ b/crates/flint-dns/src/lib.rs @@ -1,31 +1,43 @@ -//! Resilient DNS-over-HTTPS: un-poisoned answers in censored regions (design §6). +//! Resilient DNS: un-poisoned answers in censored regions (design §6). //! -//! The first [`flint_dial`] consumer. [`resolve`] races a diverse [`pool`] of DoH resolvers, each -//! reached by a composable bootstrap dial (boring Chrome-mimicry TLS), runs a [`codec`]-built A/AAAA -//! query over [`doh`] (HTTP/2), [`validate`]s the answer (drops poison/bogons), and returns the first -//! resolver that yields a real answer. Because DoH is encrypted transport, a censor can't poison an -//! answer — only block a connection — so "uncensored DNS" reduces to "reach *one* resolver", which is -//! exactly what the raced bootstrap dials are for. +//! The first [`flint_dial`] consumer. [`resolve`] races a diverse [`pool`] of resolvers, each reached +//! by a composable bootstrap dial (boring Chrome-mimicry TLS), runs a [`codec`]-built A/AAAA query, +//! [`validate`]s the answer (drops poison/bogons), and returns the first resolver that yields a real +//! answer. Because an encrypted transport keeps a censor from poisoning an answer — only from blocking +//! a connection — "uncensored DNS" reduces to "reach *one* resolver", which is exactly what the raced +//! bootstrap dials are for. +//! +//! **Two independent axes.** A resolver's [`Kind`] picks the DNS protocol and endpoint (DoH, DoT, +//! plaintext TCP/UDP, or the system resolver); a [`WirePlan`] picks how the opening handshake looks on +//! the wire (record fragmentation, segment splitting, inter-segment jitter). [`resolve_one_shaped`] +//! composes them, so a DoH lookup can itself be carried over a fragmented, jittered ClientHello — the +//! same shaping vocabulary used for a destination dial, aimed at the DNS dial. Encrypted kinds are the +//! trustworthy ones; the plaintext kinds are poisonable and stay out of [`default_pool`] (see there). //! //! Build pieces: [`codec`] (minimal A/AAAA wire codec), [`validate`] (poison rejection), [`pool`] -//! (the diverse resolver set), [`doh`] (DoH-over-h2), and [`resolve`] (the smart-dialer). Per-network -//! caching of the winning composition and Ed25519-signed pool updates are follow-ups (design §6). +//! (the diverse resolver set + the [`Kind`] axis), [`doh`] (DoH-over-h2), [`plain`] (DoT/TCP framing + +//! UDP), and [`resolve`] (the smart-dialer). Per-network caching of the winning composition and +//! Ed25519-signed pool updates are follow-ups (design §6). #![forbid(unsafe_code)] use std::io; use std::net::IpAddr; use std::time::Duration; +use ring::rand::{SecureRandom, SystemRandom}; + pub mod cache; pub mod codec; pub mod doh; +pub mod plain; pub mod pool; pub mod signed; pub mod validate; pub use cache::ResolverCache; pub use codec::{TYPE_A, TYPE_AAAA}; -pub use pool::{default_pool, Resolver}; +pub use flint_dial::WirePlan; +pub use pool::{default_pool, Kind, Resolver}; pub use signed::{load_signed_pool, PoolUpdate}; /// Why a resolution failed. @@ -50,17 +62,93 @@ const DEFAULT_WINDOW: usize = 16; /// instead of hanging on the slowest resolver. const ATTEMPT_TIMEOUT: Duration = Duration::from_secs(5); -/// Resolve `name`/`qtype` through a single `resolver`: dial it (composable bootstrap dial), run the -/// DoH query, parse, and validate. Returns the validated public addresses, or an `io::Error` (which -/// the smart-dialer funnels into the race's per-resolver failures). +/// Resolve `name`/`qtype` through a single `resolver`: reach it over whatever transport its +/// [`Kind`] names, run the query, parse, and validate. Returns the validated public addresses, or an +/// `io::Error` (which the smart-dialer funnels into the race's per-resolver failures). +/// +/// Applies no opening-handshake shaping; see [`resolve_one_shaped`]. pub async fn resolve_one(resolver: &Resolver, name: &str, qtype: u16) -> io::Result> { - let query = codec::build_query(name, qtype).map_err(io::Error::other)?; - let stream = flint_dial::dial(&resolver.strategy()).await?; - let response = doh::query(stream, &resolver.host, &resolver.path, &query).await?; - let answers = codec::parse_response(&response).map_err(io::Error::other)?; + resolve_one_shaped(resolver, name, qtype, &WirePlan::default()).await +} + +/// Like [`resolve_one`], but composes opening-handshake shaping `wire` onto the dial that reaches the +/// resolver. +/// +/// This is the seam that makes the two axes independent: `resolver` picks *which DNS protocol and +/// endpoint*, `wire` picks *how the opening handshake looks on the wire* (record fragmentation, segment +/// splitting, inter-segment jitter). So a DoH lookup can itself be carried over a fragmented, jittered +/// ClientHello — the same shaping vocabulary applied to a destination dial, pointed at the DNS dial. +/// +/// `wire` is ignored for kinds that expose no ClientHello to shape ([`Kind::is_shapeable`]). +/// +/// Transaction IDs: DoH uses ID 0 per RFC 8484 §4.1, since its own framing binds the response. Every +/// other transport draws a random ID and verifies it on return — mandatory for the plaintext kinds, +/// harmless for DoT. +pub async fn resolve_one_shaped( + resolver: &Resolver, + name: &str, + qtype: u16, + wire: &WirePlan, +) -> io::Result> { + let answers = match resolver.kind { + Kind::Doh => { + let query = codec::build_query(name, qtype).map_err(io::Error::other)?; + let stream = flint_dial::dial(&resolver.strategy_with(wire.clone())).await?; + let response = doh::query(stream, &resolver.host, &resolver.path, &query).await?; + codec::parse_response(&response).map_err(io::Error::other)? + } + Kind::Dot => { + let id = random_id()?; + let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?; + let stream = flint_dial::dial(&resolver.strategy_with(wire.clone())).await?; + let response = plain::query_stream(stream, &query).await?; + codec::parse_response_with_id(&response, id).map_err(io::Error::other)? + } + Kind::Tcp => { + let id = random_id()?; + let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?; + let stream = tokio::net::TcpStream::connect(resolver.target).await?; + let response = plain::query_stream(stream, &query).await?; + codec::parse_response_with_id(&response, id).map_err(io::Error::other)? + } + Kind::Udp => { + let id = random_id()?; + let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?; + let response = plain::query_udp(resolver.target, &query).await?; + codec::parse_response_with_id(&response, id).map_err(io::Error::other)? + } + Kind::System => system_lookup(name, qtype).await?, + }; validate::validate_answers(answers).map_err(io::Error::other) } +/// A CSPRNG-drawn DNS transaction ID. Uses `ring` like the rest of flint rather than adding an RNG. +fn random_id() -> io::Result { + let mut bytes = [0u8; 2]; + SystemRandom::new() + .fill(&mut bytes) + .map_err(|_| io::Error::other("CSPRNG failure drawing a DNS transaction ID"))?; + Ok(u16::from_be_bytes(bytes)) +} + +/// Resolve through the OS resolver, keeping only the family `qtype` asked for. +/// +/// Worth trying because plenty of networks do not interfere with DNS at all, and it costs no +/// connection of our own. Trust it exactly as much as any plaintext answer: the OS resolver usually +/// speaks unencrypted DNS to a network-provided server, so the result is poisonable. +async fn system_lookup(name: &str, qtype: u16) -> io::Result> { + let addrs = tokio::net::lookup_host((name, 0u16)).await?; + Ok(addrs + .map(|addr| addr.ip()) + .filter(|ip| match qtype { + TYPE_A => ip.is_ipv4(), + TYPE_AAAA => ip.is_ipv6(), + // Not a family query — the codec only builds A/AAAA, so this is unreachable in practice. + _ => true, + }) + .collect()) +} + /// Resolve `name`/`qtype` resiliently: race every resolver in `pool` and return the first that yields /// a **validated** answer. Slower resolvers are cancelled once one succeeds. Errors only if all fail. pub async fn resolve( diff --git a/crates/flint-dns/src/plain.rs b/crates/flint-dns/src/plain.rs new file mode 100644 index 0000000..4bae8b8 --- /dev/null +++ b/crates/flint-dns/src/plain.rs @@ -0,0 +1,213 @@ +//! The non-DoH query transports: length-prefixed DNS over a byte stream, and plaintext DNS over UDP. +//! +//! Three of the four non-DoH [`Kind`](crate::pool::Kind)s land here: +//! +//! - [`Kind::Dot`](crate::pool::Kind::Dot) — [`query_stream`] over a TLS stream from +//! [`flint_dial::dial`], so it composes with opening-handshake [`WirePlan`](flint_dial::WirePlan) +//! shaping exactly like DoH does. +//! - [`Kind::Tcp`](crate::pool::Kind::Tcp) — [`query_stream`] over a bare TCP socket. Same framing as +//! DoT (RFC 1035 §4.2.2); the only difference is the absence of TLS. +//! - [`Kind::Udp`](crate::pool::Kind::Udp) — [`query_udp`], a single datagram exchange. +//! +//! **Plaintext transports are attacker-writable.** A censor can inject a forged answer without ever +//! seeing the query (the GFW does exactly this), so callers on these paths must use a random +//! transaction ID and verify it on the way back +//! ([`codec::parse_response_with_id`](crate::codec::parse_response_with_id)). UDP additionally +//! `connect`s its socket so the kernel drops datagrams from any source other than the resolver, and +//! binds an ephemeral port for source-port entropy. That is the standard off-path-injection bar; it is +//! *not* protection against an on-path censor, which is why these kinds stay out of +//! [`default_pool`](crate::pool::default_pool). + +use std::io; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; + +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::net::UdpSocket; + +/// The largest DNS response we will read. Bootstrap only asks for A/AAAA records, whose answers are +/// far smaller; this caps what a hostile resolver can make us allocate off a 2-byte length field. +const MAX_RESPONSE: usize = 8 * 1024; + +/// Send `query` and read one response over a byte stream, using DNS's stream framing: a 2-byte +/// big-endian length prefix before each message (RFC 1035 §4.2.2). +/// +/// Shared by DoT (stream = TLS) and plaintext TCP (stream = raw socket) — the framing is identical, so +/// those transports differ only in how the stream was obtained. +pub async fn query_stream(mut stream: S, query: &[u8]) -> io::Result> +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let len = u16::try_from(query.len()).map_err(|_| { + io::Error::new(io::ErrorKind::InvalidInput, "DNS query exceeds 65535 bytes") + })?; + + // One write for prefix+message: servers accept a split, but a single segment avoids handing a DPI + // box a gratuitously distinctive two-packet pattern for a ~30-byte query. + let mut framed = Vec::with_capacity(2 + query.len()); + framed.extend_from_slice(&len.to_be_bytes()); + framed.extend_from_slice(query); + stream.write_all(&framed).await?; + stream.flush().await?; + + let mut prefix = [0u8; 2]; + stream.read_exact(&mut prefix).await?; + let want = usize::from(u16::from_be_bytes(prefix)); + if want == 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "DNS response length prefix was zero", + )); + } + if want > MAX_RESPONSE { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "DNS response length prefix exceeds the response cap", + )); + } + let mut response = vec![0u8; want]; + stream.read_exact(&mut response).await?; + Ok(response) +} + +/// Send `query` to `target` over UDP and read one response. +/// +/// Binds an ephemeral local port (source-port entropy) and `connect`s to `target`, so the kernel +/// discards datagrams from any other source — an off-path injector must then also guess the port. The +/// caller still must verify the transaction ID; see the module docs. +/// +/// No retry and no timeout: a single attempt keeps this cancel-safe and leaves both policies to the +/// caller, which already bounds each attempt and races resolvers. +pub async fn query_udp(target: SocketAddr, query: &[u8]) -> io::Result> { + // Bind in the target's address family — a v4-bound socket cannot reach a v6 resolver. + let bind = if target.is_ipv4() { + SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), 0) + } else { + SocketAddr::new(IpAddr::V6(Ipv6Addr::UNSPECIFIED), 0) + }; + let socket = UdpSocket::bind(bind).await?; + socket.connect(target).await?; + socket.send(query).await?; + + let mut buf = vec![0u8; MAX_RESPONSE]; + // `recv` on a connected socket only yields datagrams from `target`; the kernel drops anything else + // before it reaches us. + let n = socket.recv(&mut buf).await?; + buf.truncate(n); + Ok(buf) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Frame `msg` the way a DNS stream server would reply. + fn framed(msg: &[u8]) -> Vec { + let mut out = (msg.len() as u16).to_be_bytes().to_vec(); + out.extend_from_slice(msg); + out + } + + #[tokio::test] + async fn stream_round_trip_uses_length_prefix_framing() { + let (client, mut server) = tokio::io::duplex(1024); + let expected = b"\xab\xcd response bytes".to_vec(); + let reply = expected.clone(); + let server_task = tokio::spawn(async move { + // Read the client's length-prefixed query back out, then answer. + let mut prefix = [0u8; 2]; + server.read_exact(&mut prefix).await.unwrap(); + let n = usize::from(u16::from_be_bytes(prefix)); + let mut q = vec![0u8; n]; + server.read_exact(&mut q).await.unwrap(); + server.write_all(&framed(&reply)).await.unwrap(); + server.flush().await.unwrap(); + q + }); + + let got = query_stream(client, b"query").await.unwrap(); + assert_eq!(got, expected); + assert_eq!(server_task.await.unwrap(), b"query".to_vec()); + } + + #[tokio::test] + async fn stream_rejects_an_oversized_length_prefix() { + let (client, mut server) = tokio::io::duplex(1024); + tokio::spawn(async move { + let mut prefix = [0u8; 2]; + let _ = server.read_exact(&mut prefix).await; + let mut q = vec![0u8; usize::from(u16::from_be_bytes(prefix))]; + let _ = server.read_exact(&mut q).await; + // Claim a response far larger than the cap; we must refuse rather than allocate it. + let _ = server.write_all(&u16::MAX.to_be_bytes()).await; + let _ = server.flush().await; + }); + + let err = query_stream(client, b"query").await.unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::InvalidData); + } + + #[tokio::test] + async fn stream_rejects_a_zero_length_prefix() { + let (client, mut server) = tokio::io::duplex(1024); + tokio::spawn(async move { + let mut prefix = [0u8; 2]; + let _ = server.read_exact(&mut prefix).await; + let mut q = vec![0u8; usize::from(u16::from_be_bytes(prefix))]; + let _ = server.read_exact(&mut q).await; + let _ = server.write_all(&0u16.to_be_bytes()).await; + let _ = server.flush().await; + }); + + let err = query_stream(client, b"query").await.unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::InvalidData); + } + + #[tokio::test] + async fn udp_round_trip_against_a_local_server() { + let server = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + let addr = server.local_addr().unwrap(); + tokio::spawn(async move { + let mut buf = [0u8; 512]; + let (n, from) = server.recv_from(&mut buf).await.unwrap(); + // Echo the query back with the QR bit set, so it looks like a response. + let mut reply = buf[..n].to_vec(); + reply[2] |= 0x80; + server.send_to(&reply, from).await.unwrap(); + }); + + let query = + crate::codec::build_query_with_id("example.com", crate::TYPE_A, 0x1234).unwrap(); + let response = query_udp(addr, &query).await.unwrap(); + assert_eq!(&response[..2], &0x1234u16.to_be_bytes()); + } + + #[tokio::test] + async fn udp_ignores_datagrams_from_another_source() { + // A connected UDP socket must drop an off-path injection from a different address, so the real + // resolver's later answer is the one we read. This is the off-path-forgery defense. + let resolver = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + let resolver_addr = resolver.local_addr().unwrap(); + let injector = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + + tokio::spawn(async move { + let mut buf = [0u8; 512]; + let (n, from) = resolver.recv_from(&mut buf).await.unwrap(); + // The injector fires first, from the wrong source address. + let mut forged = buf[..n].to_vec(); + forged[2] |= 0x80; + forged[0] = 0xff; + forged[1] = 0xff; + injector.send_to(&forged, from).await.unwrap(); + // Then the genuine answer arrives from the connected peer. + let mut real = buf[..n].to_vec(); + real[2] |= 0x80; + resolver.send_to(&real, from).await.unwrap(); + }); + + let query = + crate::codec::build_query_with_id("example.com", crate::TYPE_A, 0x4321).unwrap(); + let response = query_udp(resolver_addr, &query).await.unwrap(); + // We got the resolver's ID, not the injector's 0xffff. + assert_eq!(&response[..2], &0x4321u16.to_be_bytes()); + } +} diff --git a/crates/flint-dns/src/pool.rs b/crates/flint-dns/src/pool.rs index e822cae..1c32fa0 100644 --- a/crates/flint-dns/src/pool.rs +++ b/crates/flint-dns/src/pool.rs @@ -19,30 +19,155 @@ use std::net::SocketAddr; -use flint_dial::BootstrapStrategy; +use flint_dial::{BootstrapStrategy, WirePlan}; -/// One DoH resolver, addressed for a fixed-IP dial. Fields are **owned** (not `&'static str`) so a +/// Which DNS protocol a resolver speaks — the **DNS axis** of a proxyless strategy. +/// +/// [`Doh`](Kind::Doh) and [`Dot`](Kind::Dot) are encrypted, so a censor can only *block* the +/// connection, never poison the answer. [`Tcp`](Kind::Tcp) and [`Udp`](Kind::Udp) are **plaintext and +/// therefore poisonable**; they earn a place in the strategy space only because some networks filter +/// encrypted DNS while leaving plaintext queries to an unfiltered resolver alone. They are deliberately +/// absent from [`default_pool`] — see that function for why. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub enum Kind { + /// DNS-over-HTTPS (RFC 8484) over HTTP/2, port 443. Uses `sni`, `host`, and `path`. + #[default] + Doh, + /// DNS-over-TLS (RFC 7858), port 853: length-prefixed DNS inside TLS. Uses `sni`; ignores + /// `host`/`path`. + Dot, + /// Plaintext DNS over TCP (RFC 1035 §4.2.2), port 53: length-prefixed. Ignores `sni`/`host`/`path`. + Tcp, + /// Plaintext DNS over UDP (RFC 1035), port 53. Ignores `sni`/`host`/`path`. + Udp, + /// The operating system's own resolver. Ignores every addressing field — useful because on many + /// networks the system resolver is simply not interfered with, and it costs nothing to try. + System, +} + +impl Kind { + /// True if this transport encrypts the query, so the channel itself binds the response to it (and + /// a censor cannot forge an answer, only block). Plaintext kinds must instead rely on a random + /// transaction ID — see [`crate::codec::build_query_with_id`]. + pub fn is_encrypted(self) -> bool { + matches!(self, Kind::Doh | Kind::Dot) + } + + /// True if this kind dials a TLS stream, and therefore composes with opening-handshake + /// [`WirePlan`] shaping. Plaintext DNS has no ClientHello to fragment, and [`Kind::System`] + /// exposes no socket at all. + pub fn is_shapeable(self) -> bool { + self.is_encrypted() + } +} + +/// One resolver, addressed for a fixed-IP dial. Fields are **owned** (not `&'static str`) so a /// pool can be decoded from an Ed25519-signed update at runtime (see [`crate::signed`]), not only /// baked in. Serializable for that signed-blob payload. +/// +/// Which addressing fields apply depends on [`kind`](Self::kind) — the per-variant docs on [`Kind`] +/// say which. Prefer the typed constructors ([`Resolver::doh`], [`dot`](Resolver::dot), +/// [`udp`](Resolver::udp), [`tcp`](Resolver::tcp), [`system`](Resolver::system)) over a struct literal +/// so unused fields are never filled in with something misleading. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct Resolver { /// Short operator label (logs / metrics; never a secret). pub name: String, - /// The TCP endpoint to dial (the resolver IP, port 443). + /// Which DNS protocol this resolver speaks. + pub kind: Kind, + /// The endpoint to dial. Unused for [`Kind::System`]. pub target: SocketAddr, - /// The SNI to present in the ClientHello (the resolver hostname, covered by its cert). + /// The SNI to present in the ClientHello (the resolver hostname, covered by its cert). Used by the + /// TLS-based kinds only. pub sni: String, - /// The DoH `:authority` (HTTP host) — the resolver hostname. + /// The DoH `:authority` (HTTP host) — the resolver hostname. [`Kind::Doh`] only. pub host: String, - /// The DoH path (RFC 8484), almost always `/dns-query`. + /// The DoH path (RFC 8484), almost always `/dns-query`. [`Kind::Doh`] only. pub path: String, } impl Resolver { /// The bootstrap-dial strategy for this resolver: boring Chrome-mimicry to its IP, presenting its - /// hostname as SNI, with no wire shaping (the dialer layers shaping on per network). + /// hostname as SNI, with **no** wire shaping. Shorthand for [`strategy_with`](Self::strategy_with) + /// and a default [`WirePlan`]. pub fn strategy(&self) -> BootstrapStrategy { - BootstrapStrategy::boring_chrome(self.target, self.sni.clone()) + self.strategy_with(WirePlan::default()) + } + + /// The bootstrap-dial strategy with opening-handshake shaping `wire` composed onto it. + /// + /// This is the composition seam between the two axes: the resolver says *where and how* to reach + /// DNS, `wire` says *how to shape the opening handshake* getting there. That is what makes + /// "DoH lookups carried over a fragmented, jittered ClientHello" expressible — the same shaping + /// vocabulary used for a destination dial, applied to the DNS dial itself. + /// + /// Shaping is only meaningful for the TLS-based kinds ([`Kind::is_shapeable`]); for plaintext or + /// system resolvers there is no ClientHello and `wire` is ignored by the query path. + pub fn strategy_with(&self, wire: WirePlan) -> BootstrapStrategy { + BootstrapStrategy::boring_chrome(self.target, self.sni.clone()).with_wire(wire) + } + + /// A DNS-over-HTTPS resolver at `target`, presenting `sni`, querying `host``path`. + pub fn doh( + name: impl Into, + target: SocketAddr, + sni: impl Into, + host: impl Into, + path: impl Into, + ) -> Self { + Self { + name: name.into(), + kind: Kind::Doh, + target, + sni: sni.into(), + host: host.into(), + path: path.into(), + } + } + + /// A DNS-over-TLS resolver at `target` (conventionally port 853), presenting `sni`. + pub fn dot(name: impl Into, target: SocketAddr, sni: impl Into) -> Self { + Self { + name: name.into(), + kind: Kind::Dot, + target, + sni: sni.into(), + host: String::new(), + path: String::new(), + } + } + + /// A plaintext DNS-over-TCP resolver at `target` (conventionally port 53). + pub fn tcp(name: impl Into, target: SocketAddr) -> Self { + Self::plain(name, Kind::Tcp, target) + } + + /// A plaintext DNS-over-UDP resolver at `target` (conventionally port 53). + pub fn udp(name: impl Into, target: SocketAddr) -> Self { + Self::plain(name, Kind::Udp, target) + } + + fn plain(name: impl Into, kind: Kind, target: SocketAddr) -> Self { + Self { + name: name.into(), + kind, + target, + sni: String::new(), + host: String::new(), + path: String::new(), + } + } + + /// The OS resolver. Carries no addressing at all; `target` is an explicit unused placeholder. + pub fn system() -> Self { + Self { + name: "system".to_owned(), + kind: Kind::System, + target: SocketAddr::from(([0, 0, 0, 0], 0)), + sni: String::new(), + host: String::new(), + path: String::new(), + } } } @@ -52,13 +177,7 @@ impl Resolver { /// fronting is blocked by some CDNs (Cloudflare/Google), so the default pool prefers `sni == host` /// CDN-edge entries. fn entry(name: &str, ip: [u8; 4], sni: &str, host: &str) -> Resolver { - Resolver { - name: name.to_owned(), - target: SocketAddr::from((ip, 443)), - sni: sni.to_owned(), - host: host.to_owned(), - path: "/dns-query".to_owned(), - } + Resolver::doh(name, SocketAddr::from((ip, 443)), sni, host, "/dns-query") } /// A plain raw-IP / CDN-edge entry (SNI == DoH host). @@ -70,6 +189,13 @@ fn v4(name: &str, ip: [u8; 4], host: &str) -> Resolver { /// jurisdictions (US clouds, Swiss Quad9, Swedish Mullvad) — see the design's provider survey. The /// CDN-edge Cloudflare entries lead (the high-collateral spearhead). Quad9 uses the /// **no-threat-blocking** `9.9.9.10` so a flagged config host is never `NXDOMAIN`'d out from under us. +/// +/// **Encrypted kinds only, on purpose.** [`Kind::Udp`]/[`Kind::Tcp`]/[`Kind::System`] answers are +/// poisonable, and nothing in [`crate::resolve`] proves an answer is *correct* — [`crate::validate`] +/// only rejects bogons, so a censor returning a plausible wrong IP would pass. Plaintext resolvers are +/// therefore safe to try only where the answer gets verified end-to-end by actually completing a TLS +/// handshake with a valid certificate against the resolved address (the proxyless strategy search). +/// Callers who want them must add them explicitly rather than getting them by default here. pub fn default_pool() -> Vec { vec![ // CDN-edge spearhead: Cloudflare runs its DoH resolver on the *same* global anycast edge that diff --git a/crates/flint-dns/src/signed.rs b/crates/flint-dns/src/signed.rs index 2dbddd6..70ad003 100644 --- a/crates/flint-dns/src/signed.rs +++ b/crates/flint-dns/src/signed.rs @@ -15,9 +15,15 @@ use flint_verify::{SignedBlobVerifier, VerifyError, SIG_LEN}; use crate::pool::Resolver; -/// Artifact magic for a flint resolver-pool update, v1. Bump (e.g. `FRP2`) on a schema change so an -/// old client rejects a new-format pool rather than mis-decoding it. -pub const POOL_MAGIC: [u8; 4] = *b"FRP1"; +/// Artifact magic for a flint resolver-pool update, v2. Bump on a schema change so an old client +/// rejects a new-format pool rather than mis-decoding it. +/// +/// Bumped `FRP1` → `FRP2` when [`Resolver`] gained its `kind` field (the DoH/DoT/TCP/UDP/system axis). +/// The payload is **postcard**, which is not self-describing: fields are positional with no names or +/// defaults, so an added field shifts every byte after it. An `FRP1` client handed an `FRP2` pool would +/// not merely miss `kind`, it would mis-parse the whole list — hence a magic bump rather than a +/// tolerated schema drift. +pub const POOL_MAGIC: [u8; 4] = *b"FRP2"; /// Errors loading a signed pool update. #[derive(Debug, thiserror::Error)] @@ -114,20 +120,20 @@ mod tests { fn sample_pool() -> Vec { vec![ - Resolver { - name: "edge".into(), - target: "104.16.249.249:443".parse().unwrap(), - sni: "cloudflare-dns.com".into(), - host: "cloudflare-dns.com".into(), - path: "/dns-query".into(), - }, - Resolver { - name: "quad9".into(), - target: "9.9.9.10:443".parse().unwrap(), - sni: "dns.quad9.net".into(), - host: "dns.quad9.net".into(), - path: "/dns-query".into(), - }, + Resolver::doh( + "edge", + "104.16.249.249:443".parse().unwrap(), + "cloudflare-dns.com", + "cloudflare-dns.com", + "/dns-query", + ), + Resolver::doh( + "quad9", + "9.9.9.10:443".parse().unwrap(), + "dns.quad9.net", + "dns.quad9.net", + "/dns-query", + ), ] } diff --git a/crates/flint-fronted/tests/meek_live.rs b/crates/flint-fronted/tests/meek_live.rs index d5d3645..09dc883 100644 --- a/crates/flint-fronted/tests/meek_live.rs +++ b/crates/flint-fronted/tests/meek_live.rs @@ -94,6 +94,6 @@ async fn meek_live_auto_alpn_through_akamai_to_example_com() { assert!( text.starts_with("HTTP/1.1 200") && text.contains("Example Domain"), "unexpected response (first 200B): {:?}", - &text.chars().take(200).collect::() + text.chars().take(200).collect::() ); } diff --git a/docs/design.md b/docs/design.md index b04482b..4c56d37 100644 --- a/docs/design.md +++ b/docs/design.md @@ -232,6 +232,31 @@ the next composition. bytes; bootstrap only needs A/AAAA of a few hostnames) to protect the binary budget, vs pulling `hickory-proto` (pure-Rust, handles DoH/DoT/DoQ + full wire format, but sizable). Open question §11. +### 6.1 The resolver-transport axis (built) — and why it is orthogonal to shaping + +The "AND transports" spread above is realized by `pool::Kind`: **DoH** (RFC 8484 over h2), **DoT** +(RFC 7858), plaintext **TCP** and **UDP** (RFC 1035), and the **system** resolver. This is the *DNS +axis* of a proxyless strategy: where and by what protocol an answer is obtained. + +It is deliberately **independent of the wire-shaping axis** (§5). A resolver contributes addressing; +a `WirePlan` contributes opening-handshake shape (record fragmentation, segment splitting, +inter-segment jitter). `Resolver::strategy_with(wire)` / `resolve_one_shaped` compose the two, so a +DoH lookup can itself be carried over a fragmented, jittered ClientHello — the same shaping +vocabulary used for a destination dial, aimed at the DNS dial. Keeping them orthogonal is what makes +the strategy space a product (`resolver × wire`) that a search can enumerate, rather than a fixed +list of hand-written combinations. Shaping applies to the TLS-based kinds only (`Kind::is_shapeable`); +plaintext DNS has no ClientHello and the system resolver exposes no socket. + +**Trust is not uniform across the axis, and the code says so.** DoH/DoT are encrypted, so a censor +can only block them; plaintext TCP/UDP and the system resolver are *poisonable*. Answer validation +above rejects **bogons**, but nothing here proves an answer is *correct* — a censor returning a +plausible wrong address passes validation. So the plaintext kinds are excluded from `default_pool()` +and are sound only inside a search that verifies the answer end-to-end by completing a TLS handshake +with a valid certificate against the resolved address. Where they are used, the query carries a +CSPRNG transaction ID that is checked on return, and UDP `connect`s its socket (kernel-level source +filtering) on a random ephemeral port — the standard bar against **off-path** injection, which +censors including the GFW perform by blasting forged answers without seeing the query. + ## 7. Why boring Chrome-CH is the default for DNS-over-TLS Browsers do DoH with *their own* ClientHello, so a generic (rustls) TLS stack doing DoH is **itself a From 35f31a5a5a9cac0a5ccc47bf03b0a288f1f25e68 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Wed, 29 Jul 2026 08:54:37 -0500 Subject: [PATCH 2/3] =?UTF-8?q?dns:=20stop=20claiming=20encrypted=20resolv?= =?UTF-8?q?ers=20cannot=20be=20poisoned=20=E2=80=94=20the=20dial=20is=20un?= =?UTF-8?q?authenticated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch (CodeRabbit, PR #13), verified and correct. The docs added in this PR asserted that DoH/DoT answers "can only be blocked, never poisoned". That is false as implemented: `Resolver::strategy` builds on `BootstrapStrategy::boring_chrome`, which sets `CertVerification::None`, and flint-tls's connector enables `SslVerifyMode::PEER` + `set_verify_hostname` only in the `Roots` branch. So neither the chain nor the hostname is checked, and an on-path attacker can complete the handshake with any certificate and hand back forged answers over a properly encrypted channel. `validate` would catch a clumsy sentinel like 10.10.34.x but not a plausible attacker-chosen address. Corrects the claim everywhere it appeared (`Kind`, `Kind::is_encrypted`, `default_pool`, the crate docs, design §6.1): encrypted kinds resist passive reading and *off-path* forgery, not an active on-path MITM, until a caller supplies `CertVerification::Roots` via `with_verification`. No behavior change here. The underlying gap is pre-existing — it predates this PR and affects the existing DoH path — and closing it alters a live bootstrap path, so it is recorded as an explicit §11 open item rather than folded into a PR about adding the resolver axis. That item also captures the part that needs a decision: verifying the real host is straightforward for `sni == host` plain/CDN-edge entries, but a fronted entry presents a cert matching the camouflage SNI rather than the DoH `:authority`, so those need a distinct policy or exclusion. flint-fronted already dials with `Roots`, so the pattern exists. Also notes in §6.1 that this undercuts the "raw-IP works because the cert carries IP SANs" rationale, which only means anything when the cert is verified. Gate: cargo fmt --all --check, cargo clippy --workspace --all-targets -D warnings, cargo test --workspace (25 flint-dns tests), and cargo doc -p flint-dns all clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BufGsK81otiqihvrZdfoJX --- crates/flint-dns/src/lib.rs | 13 ++++++++--- crates/flint-dns/src/pool.rs | 43 +++++++++++++++++++++++++++--------- docs/design.md | 26 ++++++++++++++++++++-- 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/crates/flint-dns/src/lib.rs b/crates/flint-dns/src/lib.rs index 56e63c3..ca23509 100644 --- a/crates/flint-dns/src/lib.rs +++ b/crates/flint-dns/src/lib.rs @@ -3,9 +3,16 @@ //! The first [`flint_dial`] consumer. [`resolve`] races a diverse [`pool`] of resolvers, each reached //! by a composable bootstrap dial (boring Chrome-mimicry TLS), runs a [`codec`]-built A/AAAA query, //! [`validate`]s the answer (drops poison/bogons), and returns the first resolver that yields a real -//! answer. Because an encrypted transport keeps a censor from poisoning an answer — only from blocking -//! a connection — "uncensored DNS" reduces to "reach *one* resolver", which is exactly what the raced -//! bootstrap dials are for. +//! answer. Because an encrypted transport keeps a censor from *rewriting* an answer in flight — mostly +//! leaving it the blunter option of blocking the connection — "uncensored DNS" largely reduces to +//! "reach *one* resolver", which is exactly what the raced bootstrap dials are for. +//! +//! **Caveat, and it is a real one:** the default dial does not authenticate the resolver +//! ([`CertVerification::None`](flint_dial::CertVerification::None) via +//! [`BootstrapStrategy::boring_chrome`](flint_dial::BootstrapStrategy::boring_chrome)), so an +//! **on-path** attacker can terminate TLS with any certificate and inject answers. [`validate`] would +//! catch a clumsy sentinel but not a plausible attacker-chosen address. Callers wanting that closed +//! must pass [`CertVerification::Roots`](flint_dial::CertVerification::Roots); see [`Kind`]. //! //! **Two independent axes.** A resolver's [`Kind`] picks the DNS protocol and endpoint (DoH, DoT, //! plaintext TCP/UDP, or the system resolver); a [`WirePlan`] picks how the opening handshake looks on diff --git a/crates/flint-dns/src/pool.rs b/crates/flint-dns/src/pool.rs index 1c32fa0..3206fdf 100644 --- a/crates/flint-dns/src/pool.rs +++ b/crates/flint-dns/src/pool.rs @@ -23,11 +23,25 @@ use flint_dial::{BootstrapStrategy, WirePlan}; /// Which DNS protocol a resolver speaks — the **DNS axis** of a proxyless strategy. /// -/// [`Doh`](Kind::Doh) and [`Dot`](Kind::Dot) are encrypted, so a censor can only *block* the -/// connection, never poison the answer. [`Tcp`](Kind::Tcp) and [`Udp`](Kind::Udp) are **plaintext and -/// therefore poisonable**; they earn a place in the strategy space only because some networks filter +/// [`Doh`](Kind::Doh) and [`Dot`](Kind::Dot) encrypt the query, so an observer cannot read or rewrite +/// it *in flight*. [`Tcp`](Kind::Tcp) and [`Udp`](Kind::Udp) are **plaintext and therefore poisonable +/// by anyone on the path**; they earn a place in the strategy space only because some networks filter /// encrypted DNS while leaving plaintext queries to an unfiltered resolver alone. They are deliberately /// absent from [`default_pool`] — see that function for why. +/// +///
+/// +/// **Encryption is not authentication, and the default strategy does not authenticate.** +/// [`Resolver::strategy`] builds on [`BootstrapStrategy::boring_chrome`], whose +/// `verification` is [`CertVerification::None`](flint_dial::CertVerification::None) — the peer +/// certificate and hostname are *not* checked. So an **on-path** attacker can complete the handshake +/// with any certificate it likes and hand back forged answers over a perfectly encrypted channel. The +/// encrypted kinds therefore resist *off-path* forgery and passive reading, not an active on-path +/// MITM, until a caller supplies [`CertVerification::Roots`](flint_dial::CertVerification::Roots) via +/// [`BootstrapStrategy::with_verification`]. Tracked as a follow-up; `flint-fronted` already does this +/// for its dials. +/// +///
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub enum Kind { /// DNS-over-HTTPS (RFC 8484) over HTTP/2, port 443. Uses `sni`, `host`, and `path`. @@ -46,9 +60,13 @@ pub enum Kind { } impl Kind { - /// True if this transport encrypts the query, so the channel itself binds the response to it (and - /// a censor cannot forge an answer, only block). Plaintext kinds must instead rely on a random + /// True if this transport encrypts the query, so the channel binds the response to it and an + /// **off-path** attacker cannot forge an answer. Plaintext kinds must instead rely on a random /// transaction ID — see [`crate::codec::build_query_with_id`]. + /// + /// This says nothing about an **on-path** attacker: with the default + /// [`CertVerification::None`](flint_dial::CertVerification::None) the peer is unauthenticated, so + /// encryption alone does not make the answer trustworthy. See the [`Kind`] docs. pub fn is_encrypted(self) -> bool { matches!(self, Kind::Doh | Kind::Dot) } @@ -191,11 +209,16 @@ fn v4(name: &str, ip: [u8; 4], host: &str) -> Resolver { /// **no-threat-blocking** `9.9.9.10` so a flagged config host is never `NXDOMAIN`'d out from under us. /// /// **Encrypted kinds only, on purpose.** [`Kind::Udp`]/[`Kind::Tcp`]/[`Kind::System`] answers are -/// poisonable, and nothing in [`crate::resolve`] proves an answer is *correct* — [`crate::validate`] -/// only rejects bogons, so a censor returning a plausible wrong IP would pass. Plaintext resolvers are -/// therefore safe to try only where the answer gets verified end-to-end by actually completing a TLS -/// handshake with a valid certificate against the resolved address (the proxyless strategy search). -/// Callers who want them must add them explicitly rather than getting them by default here. +/// poisonable by anyone on the path, and nothing in [`crate::resolve`] proves an answer is *correct* — +/// [`crate::validate`] only rejects bogons, so a censor returning a plausible wrong IP would pass. +/// Plaintext resolvers are therefore safe to try only where the answer gets verified end-to-end by +/// actually completing a TLS handshake with a valid certificate against the resolved address (the +/// proxyless strategy search). Callers who want them must add them explicitly rather than getting them +/// by default here. +/// +/// Note this is a *relative* preference, not a clean bill of health: while the dial stays on +/// [`CertVerification::None`](flint_dial::CertVerification::None) the entries below are unauthenticated +/// too, so they resist off-path forgery rather than an on-path MITM. See [`Kind`]. pub fn default_pool() -> Vec { vec![ // CDN-edge spearhead: Cloudflare runs its DoH resolver on the *same* global anycast edge that diff --git a/docs/design.md b/docs/design.md index 4c56d37..41f2f5e 100644 --- a/docs/design.md +++ b/docs/design.md @@ -247,8 +247,9 @@ the strategy space a product (`resolver × wire`) that a search can enumerate, r list of hand-written combinations. Shaping applies to the TLS-based kinds only (`Kind::is_shapeable`); plaintext DNS has no ClientHello and the system resolver exposes no socket. -**Trust is not uniform across the axis, and the code says so.** DoH/DoT are encrypted, so a censor -can only block them; plaintext TCP/UDP and the system resolver are *poisonable*. Answer validation +**Trust is not uniform across the axis, and the code says so.** DoH/DoT encrypt the query, so it +cannot be read or rewritten in flight; plaintext TCP/UDP and the system resolver are *poisonable by +anyone on the path*. Answer validation above rejects **bogons**, but nothing here proves an answer is *correct* — a censor returning a plausible wrong address passes validation. So the plaintext kinds are excluded from `default_pool()` and are sound only inside a search that verifies the answer end-to-end by completing a TLS handshake @@ -257,6 +258,19 @@ CSPRNG transaction ID that is checked on return, and UDP `connect`s its socket ( filtering) on a random ephemeral port — the standard bar against **off-path** injection, which censors including the GFW perform by blasting forged answers without seeing the query. +**Known gap: the resolver dial is not authenticated.** `BootstrapStrategy::boring_chrome` — which +`Resolver::strategy` builds on — sets `CertVerification::None`, so neither the certificate chain nor +the hostname is checked (`flint-tls`'s connector only enables `SslVerifyMode::PEER` and +`set_verify_hostname` in the `Roots` branch). Encryption without authentication stops a passive +reader and an off-path forger, but **not an active on-path MITM**, which can terminate the handshake +with any certificate and return whatever answers it likes; bogon validation catches a clumsy sentinel, +not a plausible attacker-chosen address. This undercuts the "raw-IP works because the cert carries IP +SANs" rationale above, which is only meaningful when the cert is actually verified. `flint-fronted` +already dials with `CertVerification::Roots`, so the fix is to do the same here — noting the identity +to verify is the **real host** for plain/CDN-edge entries (where `sni == host`) and would need care for +any fronted entry, whose cert matches the camouflage SNI rather than the DoH `:authority`. Tracked in +§11. + ## 7. Why boring Chrome-CH is the default for DNS-over-TLS Browsers do DoH with *their own* ClientHello, so a generic (rustls) TLS stack doing DoH is **itself a @@ -328,6 +342,14 @@ keeps every channel simple. strategy-update list relates to the broader bootstrap config and the server-side P5 loop. - **`record_fragment` interop** — confirm no widely-deployed resolver/middlebox rejects a record-fragmented CH (probe; the success-gated dialer tolerates it either way). +- **Authenticate the resolver dial (§6.1 known gap).** `Resolver::strategy` inherits + `CertVerification::None` from `boring_chrome`, so DoH/DoT connections are encrypted but + **unauthenticated** — an on-path MITM can inject answers. Switch to `CertVerification::Roots` + (empty `roots_pem` = system roots, as `flint-fronted` already does). Open: which identity to verify + per addressing form — the real host for `sni == host` plain/CDN-edge entries is straightforward, but + a *fronted* entry presents a cert for the camouflage SNI, not the DoH `:authority`, so those need + either a distinct policy or exclusion. Also decide whether verification failure should down-rank a + resolver or hard-fail the attempt. ## 12. References From 3b8a6ed2d1f3a837de7311cd803fc0e3cb820e18 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Wed, 29 Jul 2026 08:57:49 -0500 Subject: [PATCH 3/3] dns: make a TLS dial strategy unrepresentable for non-TLS resolver kinds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch (Copilot, PR #13). `Resolver::strategy()` returned a TLS `BootstrapStrategy` for every kind, including the ones that have no TLS dial at all: plaintext TCP/UDP, and `System`, whose `target` is an unused placeholder — so it cheerfully described a TLS dial to 0.0.0.0:0. Internally the `resolve_one_shaped` match never took that path, but both methods are `pub`, so nothing stopped a consumer from dialing TLS at a UDP resolver on port 53. Renamed to `tls_strategy` / `tls_strategy_with` and returns `Option`, `None` for any kind where `!kind.is_shapeable()`. The name now says what it builds, and the `Option` keeps the invalid combination out of the type at the call site instead of trusting each caller to check `kind` first — the same reasoning behind the typed constructors added earlier in this PR. The two internal call sites are the `Doh`/`Dot` match arms, which have already established the kind is TLS-based, so `None` there would mean `is_shapeable` and the dispatch disagree — a bug rather than a runtime condition. A small helper converts it to an `io::Error` naming the resolver and kind, so that impossible case cannot become a panic (no `unwrap` on a production path). Adds `only_tls_kinds_have_a_dial_strategy`: DoH/DoT yield a strategy carrying the right SNI, shaping composes through `tls_strategy_with`, and Tcp/Udp/System all yield `None`. Also fixes the `querying host path` doc typo Copilot flagged, which rendered as one identifier. Gate: fmt, clippy --workspace --all-targets -D warnings, and cargo test --workspace all clean; flint-dns now 26 tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BufGsK81otiqihvrZdfoJX --- crates/flint-dns/src/lib.rs | 18 ++++++++- crates/flint-dns/src/pool.rs | 74 ++++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/crates/flint-dns/src/lib.rs b/crates/flint-dns/src/lib.rs index ca23509..e28b3a1 100644 --- a/crates/flint-dns/src/lib.rs +++ b/crates/flint-dns/src/lib.rs @@ -100,14 +100,14 @@ pub async fn resolve_one_shaped( let answers = match resolver.kind { Kind::Doh => { let query = codec::build_query(name, qtype).map_err(io::Error::other)?; - let stream = flint_dial::dial(&resolver.strategy_with(wire.clone())).await?; + let stream = flint_dial::dial(&tls_strategy(resolver, wire)?).await?; let response = doh::query(stream, &resolver.host, &resolver.path, &query).await?; codec::parse_response(&response).map_err(io::Error::other)? } Kind::Dot => { let id = random_id()?; let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?; - let stream = flint_dial::dial(&resolver.strategy_with(wire.clone())).await?; + let stream = flint_dial::dial(&tls_strategy(resolver, wire)?).await?; let response = plain::query_stream(stream, &query).await?; codec::parse_response_with_id(&response, id).map_err(io::Error::other)? } @@ -129,6 +129,20 @@ pub async fn resolve_one_shaped( validate::validate_answers(answers).map_err(io::Error::other) } +/// The TLS strategy for a resolver whose kind is known to be TLS-based. +/// +/// The `Doh`/`Dot` match arms have already established that, so `None` here would mean +/// [`Kind::is_shapeable`] and this dispatch disagree — a bug, not a runtime condition. Surfacing it as +/// an error rather than unwrapping keeps that impossible case from becoming a panic. +fn tls_strategy(resolver: &Resolver, wire: &WirePlan) -> io::Result { + resolver.tls_strategy_with(wire.clone()).ok_or_else(|| { + io::Error::other(format!( + "resolver {} has kind {:?}, which has no TLS dial strategy", + resolver.name, resolver.kind + )) + }) +} + /// A CSPRNG-drawn DNS transaction ID. Uses `ring` like the rest of flint rather than adding an RNG. fn random_id() -> io::Result { let mut bytes = [0u8; 2]; diff --git a/crates/flint-dns/src/pool.rs b/crates/flint-dns/src/pool.rs index 3206fdf..15abc2f 100644 --- a/crates/flint-dns/src/pool.rs +++ b/crates/flint-dns/src/pool.rs @@ -105,27 +105,34 @@ pub struct Resolver { } impl Resolver { - /// The bootstrap-dial strategy for this resolver: boring Chrome-mimicry to its IP, presenting its - /// hostname as SNI, with **no** wire shaping. Shorthand for [`strategy_with`](Self::strategy_with) - /// and a default [`WirePlan`]. - pub fn strategy(&self) -> BootstrapStrategy { - self.strategy_with(WirePlan::default()) + /// The TLS dial strategy for this resolver — boring Chrome-mimicry to its IP presenting its + /// hostname as SNI — with **no** wire shaping, or `None` if this [`Kind`] has no TLS dial. + /// Shorthand for [`tls_strategy_with`](Self::tls_strategy_with) and a default [`WirePlan`]. + pub fn tls_strategy(&self) -> Option { + self.tls_strategy_with(WirePlan::default()) } - /// The bootstrap-dial strategy with opening-handshake shaping `wire` composed onto it. + /// The TLS dial strategy with opening-handshake shaping `wire` composed onto it, or `None` if this + /// [`Kind`] has no TLS dial to describe. /// /// This is the composition seam between the two axes: the resolver says *where and how* to reach /// DNS, `wire` says *how to shape the opening handshake* getting there. That is what makes /// "DoH lookups carried over a fragmented, jittered ClientHello" expressible — the same shaping /// vocabulary used for a destination dial, applied to the DNS dial itself. /// - /// Shaping is only meaningful for the TLS-based kinds ([`Kind::is_shapeable`]); for plaintext or - /// system resolvers there is no ClientHello and `wire` is ignored by the query path. - pub fn strategy_with(&self, wire: WirePlan) -> BootstrapStrategy { - BootstrapStrategy::boring_chrome(self.target, self.sni.clone()).with_wire(wire) + /// Returns `None` for every non-TLS kind ([`Kind::is_shapeable`]): plaintext TCP/UDP dial no TLS at + /// all, and [`Kind::System`] carries no endpoint (its `target` is an unused placeholder, so a + /// strategy would describe a dial to `0.0.0.0:0`). Handing back `Option` keeps that invalid + /// combination unrepresentable at the call site instead of trusting each caller to check `kind` + /// first — the same reason the constructors above exist. + pub fn tls_strategy_with(&self, wire: WirePlan) -> Option { + if !self.kind.is_shapeable() { + return None; + } + Some(BootstrapStrategy::boring_chrome(self.target, self.sni.clone()).with_wire(wire)) } - /// A DNS-over-HTTPS resolver at `target`, presenting `sni`, querying `host``path`. + /// A DNS-over-HTTPS resolver at `target`, presenting `sni`, querying `path` on `host`. pub fn doh( name: impl Into, target: SocketAddr, @@ -300,6 +307,8 @@ pub fn default_pool() -> Vec { #[cfg(test)] mod tests { + use flint_dial::RecordFragment; + use super::*; #[test] @@ -311,13 +320,54 @@ mod tests { assert!(r.target.ip().is_ipv4()); assert!(!r.sni.is_empty() && r.host == r.sni); assert_eq!(r.path, "/dns-query"); - assert_eq!(r.strategy().engine.kind(), "boring-chrome"); + // Every default entry is an encrypted kind, so each has a TLS strategy. + assert_eq!(r.kind, Kind::Doh); + let strategy = r.tls_strategy().expect("a DoH resolver has a TLS strategy"); + assert_eq!(strategy.engine.kind(), "boring-chrome"); } // Operator diversity (not all one provider). let hosts: std::collections::HashSet<_> = pool.iter().map(|r| r.host.as_str()).collect(); assert!(hosts.len() >= 4, "pool should span several operators"); } + #[test] + fn only_tls_kinds_have_a_dial_strategy() { + let addr = "9.9.9.10:443".parse().unwrap(); + + // TLS kinds: a strategy, carrying the SNI and any shaping asked for. + let doh = Resolver::doh("q", addr, "dns.quad9.net", "dns.quad9.net", "/dns-query"); + let dot = Resolver::dot("q-dot", "9.9.9.10:853".parse().unwrap(), "dns.quad9.net"); + for r in [&doh, &dot] { + let s = r + .tls_strategy() + .expect("a TLS kind must have a dial strategy"); + assert_eq!(s.sni, "dns.quad9.net"); + assert!(s.wire.is_noop(), "default strategy applies no shaping"); + } + let shaped = doh + .tls_strategy_with(WirePlan { + record_fragment: RecordFragment::SniStraddle, + ..Default::default() + }) + .expect("shaping composes onto a TLS kind"); + assert!(!shaped.wire.is_noop(), "the wire plan must be carried"); + + // Non-TLS kinds have none — the invalid combination is unrepresentable, so nothing can + // accidentally dial TLS to a plaintext resolver or to System's placeholder 0.0.0.0:0. + let plaintext = "9.9.9.10:53".parse().unwrap(); + for r in [ + Resolver::tcp("q-tcp", plaintext), + Resolver::udp("q-udp", plaintext), + Resolver::system(), + ] { + assert!( + r.tls_strategy().is_none(), + "{:?} must not produce a TLS strategy", + r.kind + ); + } + } + #[test] fn pool_includes_a_cdn_edge_entry() { // At least one entry dials Cloudflare's high-collateral CDN range (104.16/12) rather than a