From b1bc5ed6e051bcabfb867aa6874949cf0c71ab81 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 27 Sep 2026 21:45:13 -0700 Subject: [PATCH] Recheck DNSSEC lookups that hickory wrongly calls bogus hickory 0.26.3 rejects two kinds of valid answers, and outbound delivery then retries those hosts until the message expires: - A zone delegated beneath an unsigned zone (l.google.com under google.com). Proving the delegation insecure needs an SOA record in the DS reply, and public resolvers often leave it out. Every Google MX host behind a signed MX record was unreachable. - A signed CNAME to a signed name that lacks the queried type. The NSEC denial is checked against the original name, not the target's. On a bogus verdict, follow a signed CNAME and repeat the lookup at its target; otherwise look up the name's zone and its parents, nearest first. A zone that validates as unsigned means nothing below it can be signed, so the plain resolver answers and the result is insecure. A zone that validates as signed first leaves the verdict standing. --- crates/smtp/src/outbound/dane/dnssec.rs | 307 +++++++++++++++++++++--- 1 file changed, 276 insertions(+), 31 deletions(-) diff --git a/crates/smtp/src/outbound/dane/dnssec.rs b/crates/smtp/src/outbound/dane/dnssec.rs index d2010d8..4c29da1 100644 --- a/crates/smtp/src/outbound/dane/dnssec.rs +++ b/crates/smtp/src/outbound/dane/dnssec.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use common::{ @@ -13,6 +15,8 @@ use mail_auth::{ MX, RecordSet, common::resolver::ToFqdn, hickory_resolver::{ + TokioResolver, + lookup::Lookup, net::{DnsError, NetError}, proto::{ dnssec::Proof, @@ -83,16 +87,15 @@ impl TlsaLookup for Server { return mail_auth::common::resolver::mock_resolve(key.as_ref()); } - let mx_lookup = match self - .core - .smtp - .resolvers - .dnssec - .resolver - .mx_lookup(Name::from_str_relaxed::<&str>(key.as_ref())?) - .await + let (mx_lookup, forced_insecure) = match validated_lookup( + &self.core.smtp.resolvers.dnssec.resolver, + self.core.smtp.resolvers.dns.resolver(), + Name::from_str_relaxed::<&str>(key.as_ref())?, + RecordType::MX, + ) + .await { - Ok(mx_lookup) => mx_lookup, + Ok(validated) => (validated.lookup, validated.insecure), Err(err) => { if let Some(denial) = NegativeAnswer::from_error(&err) && denial.response_code == ResponseCode::NoError @@ -144,7 +147,11 @@ impl TlsaLookup for Server { .collect::>(); let records = RecordSet { rrset, - dnssec_status: dnssec_status.unwrap_or(DnssecStatus::Indeterminate), + dnssec_status: if forced_insecure { + DnssecStatus::Insecure + } else { + dnssec_status.unwrap_or(DnssecStatus::Indeterminate) + }, }; self.inner @@ -285,16 +292,15 @@ impl TlsaLookup for Server { } let name = Name::from_str_relaxed::<&str>(key.as_ref())?; - let lookup = match self - .core - .smtp - .resolvers - .dnssec - .resolver - .ipv4_lookup(name.clone()) - .await + let (lookup, forced_insecure) = match validated_lookup( + &self.core.smtp.resolvers.dnssec.resolver, + self.core.smtp.resolvers.dns.resolver(), + name.clone(), + RecordType::A, + ) + .await { - Ok(lookup) => lookup, + Ok(validated) => (validated.lookup, validated.insecure), Err(err) => { if let Some(denial) = NegativeAnswer::from_error(&err) && denial.response_code == ResponseCode::NoError @@ -325,7 +331,11 @@ impl TlsaLookup for Server { _ => None, }) .collect::>(), - dnssec_status: tlsa_base_status(&name, answers, RecordType::A), + dnssec_status: if forced_insecure { + DnssecStatus::Insecure + } else { + tlsa_base_status(&name, answers, RecordType::A) + }, }; self.inner @@ -363,16 +373,15 @@ impl TlsaLookup for Server { } let name = Name::from_str_relaxed::<&str>(key.as_ref())?; - let lookup = match self - .core - .smtp - .resolvers - .dnssec - .resolver - .ipv6_lookup(name.clone()) - .await + let (lookup, forced_insecure) = match validated_lookup( + &self.core.smtp.resolvers.dnssec.resolver, + self.core.smtp.resolvers.dns.resolver(), + name.clone(), + RecordType::AAAA, + ) + .await { - Ok(lookup) => lookup, + Ok(validated) => (validated.lookup, validated.insecure), Err(err) => { if let Some(denial) = NegativeAnswer::from_error(&err) && denial.response_code == ResponseCode::NoError @@ -403,7 +412,11 @@ impl TlsaLookup for Server { _ => None, }) .collect::>(), - dnssec_status: tlsa_base_status(&name, answers, RecordType::AAAA), + dnssec_status: if forced_insecure { + DnssecStatus::Insecure + } else { + tlsa_base_status(&name, answers, RecordType::AAAA) + }, }; self.inner @@ -415,6 +428,115 @@ impl TlsaLookup for Server { } } +// inbuxa: hickory 0.26.3 calls some valid answers bogus, and the queue then +// retries those hosts until the message expires. Two cases seen in production: +// +// - A zone delegated beneath an unsigned zone, such as `l.google.com` under +// `google.com`. To prove the delegation insecure, hickory wants an SOA +// record in the DS reply, and public resolvers often send none. +// - A signed CNAME to a signed name without the record type queried. Hickory +// checks the denial of existence against the name first asked for, not the +// target's, and rejects it. +// +// When hickory says bogus, check the answer again with lookups it gets right. +// A signed CNAME is followed and the lookup repeated at its target. Otherwise +// the name's zone and its parents are looked up, nearest first. If one +// validates as unsigned, nothing below it can be signed, so the plain resolver +// answers and the result is insecure. If one validates as signed first, the +// verdict stands. + +const MAX_BOGUS_ALIASES: usize = 8; + +struct ValidatedLookup { + lookup: Lookup, + insecure: bool, +} + +enum BogusRecheck { + Alias(Name), + Insecure, + Bogus, +} + +async fn validated_lookup( + dnssec: &TokioResolver, + plain: &TokioResolver, + name: Name, + record_type: RecordType, +) -> Result { + let mut query = name; + let mut aliases = 0; + + loop { + let err = match dnssec.lookup(query.clone(), record_type).await { + Ok(lookup) => { + return Ok(ValidatedLookup { + lookup, + insecure: false, + }); + } + Err(err @ NetError::Dns(DnsError::DnssecBogus)) => err, + Err(err) => return Err(err), + }; + + match recheck_bogus(dnssec, &query).await { + BogusRecheck::Alias(target) if aliases < MAX_BOGUS_ALIASES => { + aliases += 1; + query = target; + } + BogusRecheck::Insecure => { + return plain + .lookup(query, record_type) + .await + .map(|lookup| ValidatedLookup { + lookup, + insecure: true, + }); + } + BogusRecheck::Alias(_) | BogusRecheck::Bogus => return Err(err), + } + } +} + +async fn recheck_bogus(dnssec: &TokioResolver, name: &Name) -> BogusRecheck { + if let Ok(lookup) = dnssec.lookup(name.clone(), RecordType::CNAME).await + && let Some(target) = secure_alias(name, lookup.answers()) + { + return BogusRecheck::Alias(target); + } + + let mut zone = name.clone(); + while !zone.is_root() { + if let Ok(lookup) = dnssec.lookup(zone.clone(), RecordType::SOA).await { + match apex_status(&zone, lookup.answers()) { + Some(DnssecStatus::Insecure) => return BogusRecheck::Insecure, + Some(DnssecStatus::Secure) => return BogusRecheck::Bogus, + _ => {} + } + } + zone = zone.base_name(); + } + + BogusRecheck::Bogus +} + +fn secure_alias(query: &Name, answers: &[Record]) -> Option { + answers.iter().find_map(|record| match &record.data { + RData::CNAME(target) if &record.name == query && record.proof.is_secure() => { + Some(target.0.clone()) + } + _ => None, + }) +} + +fn apex_status(zone: &Name, answers: &[Record]) -> Option { + answers + .iter() + .filter(|record| record.record_type() == RecordType::SOA && &record.name == zone) + .map(|record| proof_to_dnssec_status(record.proof)) + .reduce(least_secure) +} + struct NegativeAnswer { response_code: ResponseCode, dnssec_status: DnssecStatus, @@ -511,7 +633,7 @@ pub(crate) fn least_secure(a: DnssecStatus, b: DnssecStatus) -> DnssecStatus { #[cfg(test)] mod tests { use super::*; - use mail_auth::hickory_resolver::proto::rr::rdata::{A, CNAME}; + use mail_auth::hickory_resolver::proto::rr::rdata::{A, CNAME, SOA}; use std::net::Ipv4Addr; fn name(value: &str) -> Name { @@ -624,4 +746,127 @@ mod tests { DnssecStatus::Insecure ); } + + fn soa(owner: &str, proof: Proof) -> Record { + let mut record = Record::from_rdata( + name(owner), + 3600, + RData::SOA(SOA::new( + name("ns1.example.org."), + name("hostmaster.example.org."), + 1, + 900, + 900, + 1800, + 60, + )), + ); + record.proof = proof; + record + } + + #[test] + fn secure_alias_follows_signed_cname() { + assert_eq!( + secure_alias( + &name("mail.example.org."), + &[alias("mail.example.org.", "mx.example.net.", Proof::Secure)] + ), + Some(name("mx.example.net.")) + ); + } + + #[test] + fn secure_alias_ignores_unsigned_or_other_cname() { + let query = name("mail.example.org."); + + assert_eq!( + secure_alias( + &query, + &[alias( + "mail.example.org.", + "mx.example.net.", + Proof::Insecure + )] + ), + None + ); + assert_eq!( + secure_alias( + &query, + &[alias( + "other.example.org.", + "mx.example.net.", + Proof::Secure + )] + ), + None + ); + } + + #[test] + fn apex_status_reads_the_zone_soa() { + let zone = name("example.com."); + + for (proof, expected) in [ + (Proof::Secure, Some(DnssecStatus::Secure)), + (Proof::Insecure, Some(DnssecStatus::Insecure)), + (Proof::Bogus, Some(DnssecStatus::Bogus)), + ] { + assert_eq!( + apex_status(&zone, &[soa("example.com.", proof)]), + expected, + "proof {proof}" + ); + } + } + + #[test] + fn apex_status_ignores_other_records() { + assert_eq!( + apex_status( + &name("example.com."), + &[ + soa("sub.example.com.", Proof::Insecure), + address("example.com.", Proof::Insecure), + ] + ), + None + ); + } + + // Needs the network: a signed MX pointing into a zone delegated beneath an + // unsigned one. Run with `--ignored` to check a hickory upgrade. + #[tokio::test] + #[ignore] + async fn validated_lookup_proves_delegation_below_unsigned_zone() { + use mail_auth::hickory_resolver::{ + config::{CLOUDFLARE, ResolverConfig, ResolverOpts}, + net::runtime::TokioRuntimeProvider, + }; + + let build = |validate: bool| { + // Same options as the server's DNSSEC resolver; hickory fails + // validation with concurrent requests. + let mut opts = ResolverOpts::default(); + opts.validate = validate; + opts.num_concurrent_reqs = 1; + opts.cache_size = 0; + TokioResolver::builder_with_config( + ResolverConfig::udp_and_tcp(&CLOUDFLARE), + TokioRuntimeProvider::default(), + ) + .with_options(opts) + .build() + .unwrap() + }; + let (dnssec, plain) = (build(true), build(false)); + + let validated = + validated_lookup(&dnssec, &plain, name("aspmx.l.google.com."), RecordType::A) + .await + .unwrap(); + assert!(validated.insecure); + assert!(!validated.lookup.answers().is_empty()); + } } -- 2.54.0