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()); + } }