diff --git a/docs/ech.md b/docs/ech.md index e1949e43..6a0ff679 100644 --- a/docs/ech.md +++ b/docs/ech.md @@ -55,10 +55,13 @@ automatically and requires no extra flags. server that holds the corresponding private key decrypts the inner ClientHello. -3. **DNS privacy**: ECH is most effective when paired with encrypted DNS - (`--dns-server` with DoH, DoT, or DoQ). Without encrypted DNS, the - SVCB query for the ECH config leaks the hostname. fetch emits a warning - in verbose mode when ECH is used with plaintext DNS. +3. **DNS privacy**: ECH is most effective when paired with verified encrypted + DNS (`--dns-server` with HTTPS DoH, DoT, or DoQ). In `-vvv` mode, fetch + warns once when ECH discovery uses system DNS, UDP, TCP, an HTTPS DoH + endpoint with certificate verification disabled, or a plaintext HTTP + endpoint in a build that permits one. System DNS is included because fetch + cannot verify its + transport protection. `--silent` suppresses this warning. ## Configuration diff --git a/src/dns/custom.rs b/src/dns/custom.rs index a1bce885..e97c6c84 100644 --- a/src/dns/custom.rs +++ b/src/dns/custom.rs @@ -11,6 +11,13 @@ const DEFAULT_DNS_PORT: u16 = 53; const DEFAULT_DNS_OVER_TLS_PORT: u16 = 853; const DEFAULT_DNS_OVER_QUIC_PORT: u16 = 853; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum DnsTransportSecurity { + Verified, + Plaintext, + Unknown, +} + #[derive(Debug, Clone)] pub(crate) enum ParsedDnsServer { Udp(SocketAddr), @@ -29,18 +36,33 @@ pub(crate) enum ParsedDnsServer { } impl ParsedDnsServer { - /// Returns whether DNS responses have transport authentication. + /// Classifies the transport protection for DNS queries. /// /// DoT and DoQ always verify the configured resolver identity. HTTPS DoH - /// is authenticated unless certificate verification was disabled by the - /// caller. Plain HTTP, UDP, and TCP do not authenticate responses. - pub(crate) fn is_authenticated(&self, verify_doh_certificate: bool) -> bool { + /// is verified unless certificate verification was disabled by the caller. + /// Plain HTTP, UDP, and TCP do not protect the DNS query transport. + pub(crate) fn transport_security(&self, verify_doh_certificate: bool) -> DnsTransportSecurity { match self { - Self::Tls { .. } | Self::Quic { .. } => true, - Self::Doh(url) => url.scheme() == "https" && verify_doh_certificate, - Self::Udp(_) | Self::Tcp(_) => false, + Self::Tls { .. } | Self::Quic { .. } => DnsTransportSecurity::Verified, + Self::Doh(url) if url.scheme() == "https" && verify_doh_certificate => { + DnsTransportSecurity::Verified + } + Self::Doh(_) | Self::Udp(_) | Self::Tcp(_) => DnsTransportSecurity::Plaintext, } } + + pub(crate) fn is_authenticated(&self, verify_doh_certificate: bool) -> bool { + self.transport_security(verify_doh_certificate) == DnsTransportSecurity::Verified + } +} + +pub(crate) fn dns_transport_security( + value: Option<&str>, + verify_doh_certificate: bool, +) -> Result { + value.map_or(Ok(DnsTransportSecurity::Unknown), |value| { + Ok(parse_dns_server(value)?.transport_security(verify_doh_certificate)) + }) } pub(crate) fn dns_server_is_authenticated( @@ -609,6 +631,38 @@ mod tests { assert!(matches!(parsed, ParsedDnsServer::Doh(_))); } + #[test] + fn dns_transport_security_classifies_system_and_custom_transports() { + assert_eq!( + dns_transport_security(None, true).unwrap(), + DnsTransportSecurity::Unknown + ); + for value in ["1.1.1.1", "tcp://1.1.1.1"] { + assert_eq!( + dns_transport_security(Some(value), true).unwrap(), + DnsTransportSecurity::Plaintext + ); + } + for value in [ + "tls://dns.example", + "doq://dns.example", + "https://dns.example/dns-query", + ] { + assert_eq!( + dns_transport_security(Some(value), true).unwrap(), + DnsTransportSecurity::Verified + ); + } + assert_eq!( + dns_transport_security(Some("https://dns.example/dns-query"), false).unwrap(), + DnsTransportSecurity::Plaintext + ); + assert_eq!( + dns_transport_security(Some("http://127.0.0.1:8080/dns-query"), true).unwrap(), + DnsTransportSecurity::Plaintext + ); + } + #[test] fn parse_dns_server_rejects_doh_url_without_host() { assert!(parse_dns_server("https://").is_err()); diff --git a/src/grpc/reflection.rs b/src/grpc/reflection.rs index c7a26d42..942cd136 100644 --- a/src/grpc/reflection.rs +++ b/src/grpc/reflection.rs @@ -54,6 +54,7 @@ pub async fn execute_discovery(cli: &Cli) -> Result { .flatten(); crate::tls::install_default_crypto_provider(); let connect_timing = crate::http::client::ConnectionTiming::default(); + let ech_dns_warning_emitted = std::sync::atomic::AtomicBool::new(false); let client_build = crate::http::client::ClientBuildContext { mode: crate::http::client::ClientMode::GrpcReflection, request_timeout, @@ -62,6 +63,7 @@ pub async fn execute_discovery(cli: &Cli) -> Result { session: session.as_ref(), connect_timing: Some(&connect_timing), har: None, + ech_dns_warning_emitted: &ech_dns_warning_emitted, }; let client = crate::http::client::build_client_for_url(cli, &url, &client_build) .await? diff --git a/src/http/client.rs b/src/http/client.rs index 691ac44f..3db05dae 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -79,6 +79,7 @@ pub(crate) struct ClientBuildContext<'a> { pub(crate) session: Option<&'a crate::session::Session>, pub(crate) connect_timing: Option<&'a ConnectionTiming>, pub(crate) har: Option<&'a crate::har::Recorder>, + pub(crate) ech_dns_warning_emitted: &'a std::sync::atomic::AtomicBool, } #[derive(Clone, Debug)] @@ -115,6 +116,18 @@ pub(crate) async fn build_client_for_url( }); let dns_timeout = connect_budget.remaining()?; let effective_proxy = effective_proxy_for_url(cli.proxy.as_deref(), http_version, url)?; + let ech_discovery = should_configure_tls(cli, url) + && is_ech_active(cli) + && cli.unix.is_none() + && url + .host_str() + .is_some_and(|host| host.parse::().is_err()); + if ech_discovery { + crate::tls::ech::warn_for_unverified_dns_transport( + cli, + Some(context.ech_dns_warning_emitted), + )?; + } let auto_http3 = auto_http3_allowed(context.mode, url, cli.unix.as_deref(), effective_proxy); let discovery = if dynamic_dns_for_client(cli, url, effective_proxy, auto_http3)? { let debug_dns = cli.timing || cli.har.is_some() || (cli.verbose >= 3 && !cli.silent); @@ -789,6 +802,7 @@ pub(crate) async fn resolve_websocket_ech_mode( if host.parse::().is_ok() { return Ok(None); } + crate::tls::ech::warn_for_unverified_dns_transport(cli, None)?; let (records, _) = lookup_ech_https_records(cli, cli.dns_server.as_deref(), host, timeout).await?; let candidates = ech_candidates_from_records(&records.records); diff --git a/src/http/mod.rs b/src/http/mod.rs index 1276f65a..b00dd4c5 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -152,6 +152,7 @@ async fn execute_request( crate::tls::install_default_crypto_provider(); let connect_timing = client::ConnectionTiming::default(); + let ech_dns_warning_emitted = std::sync::atomic::AtomicBool::new(false); let client_build = client::ClientBuildContext { mode: client::ClientMode::Request(http_version), request_timeout, @@ -160,6 +161,7 @@ async fn execute_request( session, connect_timing: Some(&connect_timing), har: har_recorder.as_ref(), + ech_dns_warning_emitted: &ech_dns_warning_emitted, }; let mut initial_client = None; if cli.grpc && grpc_method.is_none() { diff --git a/src/tls/ech.rs b/src/tls/ech.rs index 6b18b5e3..d38befed 100644 --- a/src/tls/ech.rs +++ b/src/tls/ech.rs @@ -83,6 +83,32 @@ pub(crate) fn generate_ech_grease_config() -> EchGreaseConfig { EchGreaseConfig::new(suite, public_key) } +/// Warns when ECH discovery can reveal the target name on the DNS transport. +pub(crate) fn warn_for_unverified_dns_transport( + cli: &Cli, + emitted: Option<&std::sync::atomic::AtomicBool>, +) -> Result<(), FetchError> { + if cli.verbose < 3 || cli.silent { + return Ok(()); + } + let security = + crate::dns::custom::dns_transport_security(cli.dns_server.as_deref(), !cli.insecure)?; + if security == crate::dns::custom::DnsTransportSecurity::Verified { + return Ok(()); + } + if emitted.is_some_and(|emitted| emitted.swap(true, std::sync::atomic::Ordering::Relaxed)) { + return Ok(()); + } + + let mut printer = core::stdio().stderr_printer(cli.color.as_deref()); + core::write_warning_msg_no_flush( + &mut printer, + "ECH discovery is using DNS without verified transport security; the DNS query can reveal the hostname", + ); + core::flush_stderr(printer); + Ok(()) +} + /// Handle a failure to discover ECH configuration in DNS. /// /// Required ECH reports the original discovery error. Automatic ECH can use diff --git a/src/tls/inspect.rs b/src/tls/inspect.rs index 9482f979..2cc89d00 100644 --- a/src/tls/inspect.rs +++ b/src/tls/inspect.rs @@ -214,6 +214,7 @@ async fn lookup_inspect_ech_candidates( host: &str, timeout: TimeoutBudget, ) -> Result>, FetchError> { + super::ech::warn_for_unverified_dns_transport(cli, None)?; let resolver = cli .dns_server .as_deref() diff --git a/src/update/client.rs b/src/update/client.rs index 7b582ade..0399631f 100644 --- a/src/update/client.rs +++ b/src/update/client.rs @@ -226,6 +226,7 @@ impl UpdateClient { }); }; + let ech_dns_warning_emitted = std::sync::atomic::AtomicBool::new(false); let context = client::ClientBuildContext { mode: client::ClientMode::Request(None), request_timeout: None, @@ -234,6 +235,7 @@ impl UpdateClient { session: None, connect_timing: None, har: None, + ech_dns_warning_emitted: &ech_dns_warning_emitted, }; client::build_client_for_url(cli, url, &context).await } diff --git a/tests/network.rs b/tests/network.rs index acaefeca..994e69de 100644 --- a/tests/network.rs +++ b/tests/network.rs @@ -199,6 +199,53 @@ fn ech_dns_discovery_failure_is_reported_and_auto_falls_back() { assert!(requested.stderr.contains("ECH discovery failed")); } +#[test] +fn ech_discovery_warns_once_for_unverified_dns_and_silent_suppresses_it() { + const WARNING: &str = "ECH discovery is using DNS without verified transport security"; + let target = start_tls_server(|_| TestResponse::ok("ECH DNS warning")); + let target_port = Url::parse(&target.url).unwrap().port().unwrap(); + let target_url = format!("https://fetch-ech-dns-warning-b.test:{target_port}/warning"); + let redirect = start_tls_server(move |_| { + TestResponse::status(302, "Found", "").header("Location", &target_url) + }); + let redirect_port = Url::parse(&redirect.url).unwrap().port().unwrap(); + let dns_addr = start_udp_dns_server_with_hosts(vec![ + ("fetch-ech-dns-warning-a.test.", Ipv4Addr::new(127, 0, 0, 1)), + ("fetch-ech-dns-warning-b.test.", Ipv4Addr::new(127, 0, 0, 1)), + ]); + let url = format!("https://fetch-ech-dns-warning-a.test:{redirect_port}/redirect"); + + let verbose = run_fetch(&[ + "-vvv", + "--insecure", + "--dns-server", + &dns_addr, + "--ech", + "auto", + &url, + ]); + assert_exit(&verbose, 0); + assert_eq!( + verbose.stderr.matches(WARNING).count(), + 1, + "{}", + verbose.stderr + ); + + let silent = run_fetch(&[ + "-vvv", + "--silent", + "--insecure", + "--dns-server", + &dns_addr, + "--ech", + "auto", + &url, + ]); + assert_exit(&silent, 0); + assert!(!silent.stderr.contains(WARNING), "{}", silent.stderr); +} + #[test] fn ech_dns_timeout_is_reported_instead_of_no_configuration() { let tls = start_tls_server(|_| TestResponse::ok("ECH timeout")); @@ -248,6 +295,7 @@ fn inspect_ech_discovery_uses_custom_doh_tls_config() { let result = run_fetch(&[ "--inspect-tls", + "-vvv", "--ech", "auto", "--ca-cert", @@ -259,6 +307,13 @@ fn inspect_ech_discovery_uses_custom_doh_tls_config() { assert_exit(&result, 0); assert_eq!(https_queries.load(Ordering::SeqCst), 1, "{}", result.stderr); + assert!( + !result + .stderr + .contains("ECH discovery is using DNS without verified transport security"), + "{}", + result.stderr + ); } #[test]