fix(tls): warn when ECH uses unverified DNS

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
 

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<DnsTransportSecurity, FetchError> {
+    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());

src/grpc/reflection.rs

@@ -54,6 +54,7 @@ pub async fn execute_discovery(cli: &Cli) -> Result<i32, FetchError> {
         .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<i32, FetchError> {
         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?

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::<IpAddr>().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::<IpAddr>().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);

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() {

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

src/tls/inspect.rs

@@ -214,6 +214,7 @@ async fn lookup_inspect_ech_candidates(
     host: &str,
     timeout: TimeoutBudget,
 ) -> Result<Vec<Vec<u8>>, FetchError> {
+    super::ech::warn_for_unverified_dns_transport(cli, None)?;
     let resolver = cli
         .dns_server
         .as_deref()

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
     }

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]