Address feedback

This commit is contained in:
Sophia Turner
2025-09-27 15:38:42 +10:00
committed by Wez Furlong
parent 3dedcd3226
commit 19e80001cd
6 changed files with 99 additions and 51 deletions
Generated
-1
View File
@@ -3264,7 +3264,6 @@ version = "0.1.0"
dependencies = [
"chrono",
"dns-resolver",
"hickory-resolver",
"instant-xml",
"k9",
"kumo-spf",
-1
View File
@@ -6,7 +6,6 @@ edition = "2021"
[dependencies]
chrono = {workspace=true}
dns-resolver = {path="../dns-resolver"}
hickory-resolver = {workspace=true}
instant-xml = {workspace=true}
kumo-spf = { path = "../kumo-spf" }
serde = {workspace=true}
+6 -18
View File
@@ -1,15 +1,8 @@
#![allow(dead_code)]
mod types;
#[cfg(test)]
mod tests;
use crate::types::record::Record;
use crate::types::results::DmarcResultWithContext;
use dns_resolver::Resolver;
use hickory_resolver::proto::rr::RecordType;
use hickory_resolver::Name;
use std::collections::BTreeMap;
use std::net::IpAddr;
use std::str::FromStr;
@@ -17,6 +10,11 @@ use std::time::SystemTime;
pub use types::results::DmarcResult;
mod types;
#[cfg(test)]
mod tests;
pub struct CheckHostParams {
/// Domain of the sender in the "From:"
pub from_domain: String,
@@ -84,17 +82,7 @@ impl<'a> DmarcContext<'a> {
}
pub async fn check(&self, resolver: &dyn Resolver) -> DmarcResultWithContext {
let name = match Name::from_utf8(self.from_domain) {
Ok(name) => name,
Err(_) => {
return DmarcResultWithContext {
result: DmarcResult::Fail,
context: format!("invalid domain name: {}", self.from_domain),
}
}
};
let initial_txt = match resolver.resolve(name, RecordType::TXT).await {
let initial_txt = match resolver.resolve_txt(self.from_domain).await {
Ok(answer) => {
if answer.records.is_empty() || answer.nxdomain {
return DmarcResultWithContext {
+66 -13
View File
@@ -17,7 +17,7 @@ async fn dmarc_dkim_relaxed_subdomain() {
Ipv4Addr::LOCALHOST,
"sample.example.com",
"sample.example.com",
Some("example.com"),
&[Some("example.com")],
&resolver,
)
.await;
@@ -38,7 +38,7 @@ async fn dmarc_dkim_strict_subdomain() {
Ipv4Addr::LOCALHOST,
"sample.example.com",
"example.com",
Some("example.com"),
&[Some("example.com")],
&resolver,
)
.await;
@@ -46,6 +46,50 @@ async fn dmarc_dkim_strict_subdomain() {
k9::assert_equal!(result.result, DmarcResult::Fail);
}
#[tokio::test]
async fn dmarc_dkim_relaxed_illformed() {
let resolver = TestResolver::default().with_zone(EXAMPLE_COM).with_txt(
"example.com",
"v=DMARC1; p=reject; adkim=r; \
rua=mailto:dmarc-feedback@example.com"
.to_string(),
);
let result = evaluate_ip(
Ipv4Addr::LOCALHOST,
"example.com",
"example.com",
&[None],
&resolver,
)
.await;
k9::assert_equal!(result.result, DmarcResult::Fail);
k9::assert_equal!(result.context.contains("d="), true);
}
#[tokio::test]
async fn dmarc_dkim_strict_illformed() {
let resolver = TestResolver::default().with_zone(EXAMPLE_COM).with_txt(
"example.com",
"v=DMARC1; p=reject; adkim=s; \
rua=mailto:dmarc-feedback@example.com"
.to_string(),
);
let result = evaluate_ip(
Ipv4Addr::LOCALHOST,
"example.com",
"example.com",
&[None],
&resolver,
)
.await;
k9::assert_equal!(result.result, DmarcResult::Fail);
k9::assert_equal!(result.context.contains("d="), true);
}
#[tokio::test]
async fn dmarc_spf_relaxed_subdomain() {
let resolver = TestResolver::default().with_zone(EXAMPLE_COM).with_txt(
@@ -59,7 +103,7 @@ async fn dmarc_spf_relaxed_subdomain() {
Ipv4Addr::LOCALHOST,
"example.com",
"helper.example.com",
None,
&[],
&resolver,
)
.await;
@@ -80,7 +124,7 @@ async fn dmarc_spf_strict_subdomain() {
Ipv4Addr::LOCALHOST,
"example.com",
"helper.example.com",
None,
&[],
&resolver,
)
.await;
@@ -92,19 +136,28 @@ async fn evaluate_ip(
client_ip: impl Into<IpAddr>,
from_domain: &str,
mail_from_domain: &str,
dkim_domain: Option<&str>,
dkim_domains: &[Option<&str>],
resolver: &dyn Resolver,
) -> DmarcResultWithContext {
let dkim = if let Some(dkim_domain) = dkim_domain {
let mut map = BTreeMap::new();
map.insert("header.d".to_string(), dkim_domain.to_string());
let mut dkim_vec = vec![];
vec![map]
} else {
vec![]
};
for dkim_domain in dkim_domains {
if let Some(dkim_domain) = dkim_domain {
let mut map = BTreeMap::new();
map.insert("header.d".to_string(), dkim_domain.to_string());
match DmarcContext::new(from_domain, Some(mail_from_domain), client_ip.into(), &dkim) {
dkim_vec.push(map);
} else {
dkim_vec.push(BTreeMap::new());
}
}
match DmarcContext::new(
from_domain,
Some(mail_from_domain),
client_ip.into(),
&dkim_vec,
) {
Ok(cx) => cx.check(resolver).await,
Err(result) => result,
}
+15 -6
View File
@@ -1,5 +1,3 @@
use dns_resolver::Resolver;
use crate::types::feedback_address::FeedbackAddress;
use crate::types::format::Format;
use crate::types::mode::Mode;
@@ -7,6 +5,7 @@ use crate::types::policy::Policy;
use crate::types::report_failure::ReportFailure;
use crate::types::results::DmarcResultWithContext;
use crate::{DmarcContext, DmarcResult};
use dns_resolver::Resolver;
use std::str::FromStr;
#[derive(Debug)]
@@ -33,14 +32,19 @@ impl Record {
Mode::Relaxed => {
for dkim in cx.dkim {
if let Some(result) = dkim.get("header.d") {
if !cx.from_domain.ends_with(&format!(".{}", result))
&& cx.from_domain != result
if cx.from_domain != result
&& !cx.from_domain.ends_with(&format!(".{}", result))
{
return DmarcResultWithContext {
result: DmarcResult::Fail,
context: "DMARC: DKIM relaxed check failed".into(),
};
}
} else {
return DmarcResultWithContext {
result: DmarcResult::Fail,
context: "DMARC: DKIM signature missing 'd=' tag".into(),
};
}
}
}
@@ -53,6 +57,11 @@ impl Record {
context: "DMARC: DKIM strict check failed".into(),
};
}
} else {
return DmarcResultWithContext {
result: DmarcResult::Fail,
context: "DMARC: DKIM signature missing 'd=' tag".into(),
};
}
}
}
@@ -61,8 +70,8 @@ impl Record {
match self.align_spf {
Mode::Relaxed => {
if let Some(mail_from_domain) = cx.mail_from_domain {
if !mail_from_domain.ends_with(&format!(".{}", cx.from_domain))
&& mail_from_domain != cx.from_domain
if mail_from_domain != cx.from_domain
&& !mail_from_domain.ends_with(&format!(".{}", cx.from_domain))
{
return DmarcResultWithContext {
result: DmarcResult::Fail,
+12 -12
View File
@@ -1,15 +1,13 @@
use std::collections::BTreeMap;
use std::net::SocketAddr;
use std::str::FromStr;
use crate::smtp_server::ConnectionMetaData;
use config::{get_or_create_sub_module, serialize_options};
use kumo_dmarc::{CheckHostParams, DmarcResult};
use mailparsing::AuthenticationResult;
use message::Message;
use mlua::{Lua, LuaSerdeExt, UserDataRef};
use serde::Serialize;
use crate::smtp_server::ConnectionMetaData;
use std::collections::BTreeMap;
use std::net::SocketAddr;
use std::str::FromStr;
#[derive(Debug, Serialize)]
struct CheckHostOutput {
@@ -24,9 +22,9 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> {
"verify",
lua.create_async_function(
|lua,
(msg, dkim_result, meta): (
(msg, dkim_results, meta): (
UserDataRef<Message>,
UserDataRef<Vec<AuthenticationResult>>,
mlua::Value,
UserDataRef<ConnectionMetaData>,
)| async move {
let addr = meta
@@ -39,7 +37,7 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> {
// MAIL FROM
let msg_sender = msg.sender();
let mail_from_domain = msg_sender.ok().map(|x| x.to_string());
let mail_from_domain = msg_sender.ok().map(|x| x.domain().to_string());
// From:
let from_domain = if let Ok(Some(from)) = msg.get_address_header("From") {
@@ -52,8 +50,8 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> {
result: AuthenticationResult {
method: "dmarc".to_string(),
method_version: None,
result: "Only single 'From:' header supported".to_string(),
reason: Some("Only single 'From:' header supported".to_string()),
result: "'From:' header missing domain".to_string(),
reason: Some("'From:' header missing domain".to_string()),
props: BTreeMap::default(),
},
},
@@ -76,11 +74,13 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> {
))
};
let dkim_results: Vec<AuthenticationResult> = config::from_lua_value(&lua, dkim_results)?;
let result = CheckHostParams {
from_domain,
mail_from_domain,
client_ip: addr.ip(),
dkim: dkim_result.clone().into_iter().map(|x| x.props).collect(),
dkim: dkim_results.clone().into_iter().map(|x| x.props).collect(),
}
.check(&**resolver)
.await;