diff --git a/Cargo.lock b/Cargo.lock index af16eda6..c09ecadb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3264,7 +3264,6 @@ version = "0.1.0" dependencies = [ "chrono", "dns-resolver", - "hickory-resolver", "instant-xml", "k9", "kumo-spf", diff --git a/crates/kumo-dmarc/Cargo.toml b/crates/kumo-dmarc/Cargo.toml index 34918259..d5e063ac 100644 --- a/crates/kumo-dmarc/Cargo.toml +++ b/crates/kumo-dmarc/Cargo.toml @@ -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} diff --git a/crates/kumo-dmarc/src/lib.rs b/crates/kumo-dmarc/src/lib.rs index 932717de..b87f9cf7 100644 --- a/crates/kumo-dmarc/src/lib.rs +++ b/crates/kumo-dmarc/src/lib.rs @@ -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 { diff --git a/crates/kumo-dmarc/src/tests.rs b/crates/kumo-dmarc/src/tests.rs index ff82cac2..77707d63 100644 --- a/crates/kumo-dmarc/src/tests.rs +++ b/crates/kumo-dmarc/src/tests.rs @@ -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, 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, } diff --git a/crates/kumo-dmarc/src/types/record.rs b/crates/kumo-dmarc/src/types/record.rs index 09dbabb5..548a3ebf 100644 --- a/crates/kumo-dmarc/src/types/record.rs +++ b/crates/kumo-dmarc/src/types/record.rs @@ -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, diff --git a/crates/kumod/src/dmarc.rs b/crates/kumod/src/dmarc.rs index 695a2075..5eb61d0f 100644 --- a/crates/kumod/src/dmarc.rs +++ b/crates/kumod/src/dmarc.rs @@ -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, - UserDataRef>, + mlua::Value, UserDataRef, )| 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 = 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;