Recheck DNSSEC lookups that hickory wrongly calls bogus #72

Merged
jcoffey-dev merged 1 commits from fix/dnssec-insecure-fallback into main 2026-09-28 05:33:21 +00:00
Showing only changes of commit b1bc5ed6e0 - Show all commits
+273 -28
View File
@@ -2,6 +2,8 @@
* SPDX-FileCopyrightText: 2020 Stalwart Labs LLC <[email protected]> * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC <[email protected]>
* *
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL
*
* Modified by Coffey Labs in 2026 for INBUXA.
*/ */
use common::{ use common::{
@@ -13,6 +15,8 @@ use mail_auth::{
MX, RecordSet, MX, RecordSet,
common::resolver::ToFqdn, common::resolver::ToFqdn,
hickory_resolver::{ hickory_resolver::{
TokioResolver,
lookup::Lookup,
net::{DnsError, NetError}, net::{DnsError, NetError},
proto::{ proto::{
dnssec::Proof, dnssec::Proof,
@@ -83,16 +87,15 @@ impl TlsaLookup for Server {
return mail_auth::common::resolver::mock_resolve(key.as_ref()); return mail_auth::common::resolver::mock_resolve(key.as_ref());
} }
let mx_lookup = match self let (mx_lookup, forced_insecure) = match validated_lookup(
.core &self.core.smtp.resolvers.dnssec.resolver,
.smtp self.core.smtp.resolvers.dns.resolver(),
.resolvers Name::from_str_relaxed::<&str>(key.as_ref())?,
.dnssec RecordType::MX,
.resolver )
.mx_lookup(Name::from_str_relaxed::<&str>(key.as_ref())?)
.await .await
{ {
Ok(mx_lookup) => mx_lookup, Ok(validated) => (validated.lookup, validated.insecure),
Err(err) => { Err(err) => {
if let Some(denial) = NegativeAnswer::from_error(&err) if let Some(denial) = NegativeAnswer::from_error(&err)
&& denial.response_code == ResponseCode::NoError && denial.response_code == ResponseCode::NoError
@@ -144,7 +147,11 @@ impl TlsaLookup for Server {
.collect::<Arc<[MX]>>(); .collect::<Arc<[MX]>>();
let records = RecordSet { let records = RecordSet {
rrset, 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 self.inner
@@ -285,16 +292,15 @@ impl TlsaLookup for Server {
} }
let name = Name::from_str_relaxed::<&str>(key.as_ref())?; let name = Name::from_str_relaxed::<&str>(key.as_ref())?;
let lookup = match self let (lookup, forced_insecure) = match validated_lookup(
.core &self.core.smtp.resolvers.dnssec.resolver,
.smtp self.core.smtp.resolvers.dns.resolver(),
.resolvers name.clone(),
.dnssec RecordType::A,
.resolver )
.ipv4_lookup(name.clone())
.await .await
{ {
Ok(lookup) => lookup, Ok(validated) => (validated.lookup, validated.insecure),
Err(err) => { Err(err) => {
if let Some(denial) = NegativeAnswer::from_error(&err) if let Some(denial) = NegativeAnswer::from_error(&err)
&& denial.response_code == ResponseCode::NoError && denial.response_code == ResponseCode::NoError
@@ -325,7 +331,11 @@ impl TlsaLookup for Server {
_ => None, _ => None,
}) })
.collect::<Arc<[Ipv4Addr]>>(), .collect::<Arc<[Ipv4Addr]>>(),
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 self.inner
@@ -363,16 +373,15 @@ impl TlsaLookup for Server {
} }
let name = Name::from_str_relaxed::<&str>(key.as_ref())?; let name = Name::from_str_relaxed::<&str>(key.as_ref())?;
let lookup = match self let (lookup, forced_insecure) = match validated_lookup(
.core &self.core.smtp.resolvers.dnssec.resolver,
.smtp self.core.smtp.resolvers.dns.resolver(),
.resolvers name.clone(),
.dnssec RecordType::AAAA,
.resolver )
.ipv6_lookup(name.clone())
.await .await
{ {
Ok(lookup) => lookup, Ok(validated) => (validated.lookup, validated.insecure),
Err(err) => { Err(err) => {
if let Some(denial) = NegativeAnswer::from_error(&err) if let Some(denial) = NegativeAnswer::from_error(&err)
&& denial.response_code == ResponseCode::NoError && denial.response_code == ResponseCode::NoError
@@ -403,7 +412,11 @@ impl TlsaLookup for Server {
_ => None, _ => None,
}) })
.collect::<Arc<[Ipv6Addr]>>(), .collect::<Arc<[Ipv6Addr]>>(),
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 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<ValidatedLookup, NetError> {
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<Name> {
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<DnssecStatus> {
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 { struct NegativeAnswer {
response_code: ResponseCode, response_code: ResponseCode,
dnssec_status: DnssecStatus, dnssec_status: DnssecStatus,
@@ -511,7 +633,7 @@ pub(crate) fn least_secure(a: DnssecStatus, b: DnssecStatus) -> DnssecStatus {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; 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; use std::net::Ipv4Addr;
fn name(value: &str) -> Name { fn name(value: &str) -> Name {
@@ -624,4 +746,127 @@ mod tests {
DnssecStatus::Insecure 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());
}
} }