dns: resolve ambiguity of null mx and failed host lookups

Tom "Edge Case" Mairs found an issue where what I think happened
was that the DNS records changed since we first established our
own internal data structures.

When the new set of records doesn't resolve to any A or MX entries
we'd return the same empty list of addresses that we used to signal
a null mx and incorrectly fail the remainder of that queue.

This commit introduces a separate state for the null mx state so that
we can instead delay the queue.
This commit is contained in:
Wez Furlong
2023-06-19 15:02:23 -07:00
parent 3a74b56a52
commit ab80d4ca5f
2 changed files with 59 additions and 27 deletions
+33 -21
View File
@@ -140,13 +140,13 @@ impl MailExchanger {
Ok(mx)
}
pub async fn resolve_addresses(&self) -> Vec<ResolvedAddress> {
pub async fn resolve_addresses(&self) -> ResolvedMxAddresses {
let mut result = vec![];
for mx_host in &self.hosts {
// '.' is a null mx; skip trying to resolve it
if mx_host == "." {
continue;
return ResolvedMxAddresses::NullMx;
}
// Handle the literal address case
@@ -174,10 +174,16 @@ impl MailExchanger {
}
}
result.reverse();
result
ResolvedMxAddresses::Addresses(result)
}
}
#[derive(Debug, Clone)]
pub enum ResolvedMxAddresses {
NullMx,
Addresses(Vec<ResolvedAddress>),
}
struct ByPreference {
hosts: Vec<String>,
pref: u16,
@@ -416,12 +422,14 @@ MailExchanger {
k9::snapshot!(
v4_loopback.resolve_addresses().await,
r#"
[
ResolvedAddress {
name: "127.0.0.1",
addr: 127.0.0.1,
},
]
Addresses(
[
ResolvedAddress {
name: "127.0.0.1",
addr: 127.0.0.1,
},
],
)
"#
);
@@ -447,12 +455,14 @@ MailExchanger {
k9::snapshot!(
v6_loopback_non_conforming.resolve_addresses().await,
r#"
[
ResolvedAddress {
name: "::1",
addr: ::1,
},
]
Addresses(
[
ResolvedAddress {
name: "::1",
addr: ::1,
},
],
)
"#
);
@@ -478,12 +488,14 @@ MailExchanger {
k9::snapshot!(
v6_loopback.resolve_addresses().await,
r#"
[
ResolvedAddress {
name: "::1",
addr: ::1,
},
]
Addresses(
[
ResolvedAddress {
name: "::1",
addr: ::1,
},
],
)
"#
);
}
+26 -6
View File
@@ -7,6 +7,7 @@ use crate::runtime::{rt_spawn, spawn};
use crate::spool::SpoolManager;
use anyhow::Context;
use async_trait::async_trait;
use dns_resolver::ResolvedMxAddresses;
use kumo_log_types::ResolvedAddress;
use message::Message;
use rfc5321::{ClientError, EnhancedStatusCode, ForwardPath, Response, ReversePath, SmtpClient};
@@ -34,7 +35,7 @@ impl SmtpDispatcher {
},
};
let mut addresses = dispatcher
let addresses = dispatcher
.mx
.as_ref()
.expect("to have mx when doing smtp")
@@ -42,16 +43,35 @@ impl SmtpDispatcher {
.await;
tracing::trace!("mx resolved to {addresses:?}");
let mut addresses = match addresses {
ResolvedMxAddresses::NullMx => {
dispatcher
.bulk_ready_queue_operation(Response {
code: 556,
enhanced_code: Some(EnhancedStatusCode {
class: 5,
subject: 1,
detail: 10,
}),
content: "Recipient address has a null MX".to_string(),
command: None,
})
.await;
return Ok(None);
}
ResolvedMxAddresses::Addresses(a) => a,
};
if addresses.is_empty() {
dispatcher
.bulk_ready_queue_operation(Response {
code: 556,
code: 451,
enhanced_code: Some(EnhancedStatusCode {
class: 5,
subject: 1,
detail: 10,
class: 4,
subject: 4,
detail: 4,
}),
content: "Recipient address has a null MX".to_string(),
content: "MX didn't resolve to any hosts".to_string(),
command: None,
})
.await;