diff --git a/crates/kumo-dmarc/src/lib.rs b/crates/kumo-dmarc/src/lib.rs index b87f9cf7..9d12eb5c 100644 --- a/crates/kumo-dmarc/src/lib.rs +++ b/crates/kumo-dmarc/src/lib.rs @@ -1,7 +1,7 @@ #![allow(dead_code)] use crate::types::record::Record; -use crate::types::results::DmarcResultWithContext; +pub use crate::types::results::{Disposition, DispositionWithContext}; use dns_resolver::Resolver; use std::collections::BTreeMap; use std::net::IpAddr; @@ -32,7 +32,7 @@ pub struct CheckHostParams { } impl CheckHostParams { - pub async fn check(self, resolver: &dyn Resolver) -> DmarcResultWithContext { + pub async fn check(self, resolver: &dyn Resolver) -> DispositionWithContext { let Self { from_domain, mail_from_domain, @@ -52,6 +52,15 @@ impl CheckHostParams { } } +pub(crate) enum SenderDomainAlignment { + /// Sender domain is an exact match to the dmarc record + Exact, + + /// Sender domain has no exact matching dmarc record + /// but its organizational domain does + OrganizationalDomain, +} + struct DmarcContext<'a> { pub(crate) from_domain: &'a str, pub(crate) mail_from_domain: Option<&'a str>, @@ -71,7 +80,7 @@ impl<'a> DmarcContext<'a> { mail_from_domain: Option<&'a str>, client_ip: IpAddr, dkim: &'a [BTreeMap], - ) -> Result { + ) -> Result { Ok(Self { from_domain, mail_from_domain, @@ -81,46 +90,80 @@ impl<'a> DmarcContext<'a> { }) } - pub async fn check(&self, resolver: &dyn Resolver) -> DmarcResultWithContext { - let initial_txt = match resolver.resolve_txt(self.from_domain).await { - Ok(answer) => { - if answer.records.is_empty() || answer.nxdomain { - return DmarcResultWithContext { - result: DmarcResult::Fail, + pub async fn check(&self, resolver: &dyn Resolver) -> DispositionWithContext { + if let Some(records) = + fetch_dmarc_records(&format!("_dmarc.{}", self.from_domain), resolver).await + { + for record in records { + return record.evaluate(self, SenderDomainAlignment::Exact).await; + } + } else if let Some(organizational_domain) = psl::domain_str(self.from_domain) { + if organizational_domain != self.from_domain { + if let Some(records) = + fetch_dmarc_records(&format!("_dmarc.{}", organizational_domain), resolver) + .await + { + for record in records { + return record + .evaluate(self, SenderDomainAlignment::OrganizationalDomain) + .await; + } + } else { + return DispositionWithContext { + result: Disposition::None, context: format!("no DMARC records found for {}", &self.from_domain), }; - } else { - answer.as_txt() } - } - Err(err) => { - return DmarcResultWithContext { - result: DmarcResult::Fail, - context: format!("{err}"), + } else { + return DispositionWithContext { + result: Disposition::None, + context: format!("no DMARC records found for {}", &self.from_domain), }; } + } else { + return DispositionWithContext { + result: Disposition::None, + context: format!("no DMARC records found for {}", &self.from_domain), + }; }; - // TXT records can contain all sorts of stuff, let's walk through - // the set that we retrieved and take the first one that parses - for txt in initial_txt { - if txt.starts_with("v=DMARC1") { - match Record::from_str(&txt) { - Ok(record) => { - return record.evaluate(self, resolver).await; - } - Err(err) => { - return DmarcResultWithContext { - result: DmarcResult::Fail, - context: format!("failed to parse DMARC record: {err}"), - }; - } - } - } - } - DmarcResultWithContext { - result: DmarcResult::Fail, + DispositionWithContext { + result: Disposition::None, context: format!("no DMARC records found for {}", &self.from_domain), } } } + +pub async fn fetch_dmarc_records(address: &str, resolver: &dyn Resolver) -> Option> { + let initial_txt = match resolver.resolve_txt(address).await { + Ok(answer) => { + if answer.records.is_empty() || answer.nxdomain { + return None; + } else { + eprintln!("answer: {:?}", answer); + answer.as_txt() + } + } + Err(_) => { + return None; + } + }; + + let mut records = vec![]; + + // TXT records can contain all sorts of stuff, let's walk through + // the set that we retrieved and take the first one that parses + for txt in initial_txt { + if txt.starts_with("v=DMARC1;") { + if let Ok(record) = Record::from_str(&txt) { + records.push(record); + } + } + } + + if records.is_empty() { + return None; + } + + Some(records) +} diff --git a/crates/kumo-dmarc/src/tests.rs b/crates/kumo-dmarc/src/tests.rs index 31013099..83361cda 100644 --- a/crates/kumo-dmarc/src/tests.rs +++ b/crates/kumo-dmarc/src/tests.rs @@ -1,5 +1,5 @@ -use crate::types::results::DmarcResultWithContext; -use crate::{DmarcContext, DmarcResult}; +use crate::types::results::DispositionWithContext; +use crate::{Disposition, DmarcContext}; use dns_resolver::{Resolver, TestResolver}; use std::collections::BTreeMap; use std::net::Ipv4Addr; @@ -18,7 +18,7 @@ async fn dmarc_dkim_relaxed_subdomain() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "sample.example.com", + "_dmarc.sample.example.com", "v=DMARC1; p=reject; adkim=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -33,7 +33,7 @@ async fn dmarc_dkim_relaxed_subdomain() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Pass); + k9::assert_equal!(result.result, Disposition::Pass); } #[tokio::test] @@ -42,7 +42,7 @@ async fn dmarc_dkim_relaxed_subdomain_deep() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "a.b.c.sample.example.com", + "_dmarc.a.b.c.sample.example.com", "v=DMARC1; p=reject; adkim=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -57,7 +57,7 @@ async fn dmarc_dkim_relaxed_subdomain_deep() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Pass); + k9::assert_equal!(result.result, Disposition::Pass); } #[tokio::test] @@ -66,7 +66,7 @@ async fn dmarc_dkim_relaxed_subdomain_fail() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "sample.example.com", + "_dmarc.sample.example.com", "v=DMARC1; p=reject; adkim=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -81,7 +81,31 @@ async fn dmarc_dkim_relaxed_subdomain_fail() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); +} + +#[tokio::test] +async fn dmarc_dkim_relaxed_subdomain_sp_quarantine_fail() { + let resolver = TestResolver::default() + .with_zone(EXAMPLE_COM) + .unwrap() + .with_txt( + "_dmarc.example.com", + "v=DMARC1; p=reject; sp=quarantine; adkim=r; \ + rua=mailto:dmarc-feedback@example.com" + .to_string(), + ); + + let result = evaluate_ip(TestData { + client_ip: Ipv4Addr::LOCALHOST, + from_domain: "sample.example.com", + mail_from_domain: "sample.example.com", + dkim_domains: &[Some("example.org")], + resolver: &resolver, + }) + .await; + + k9::assert_equal!(result.result, Disposition::Quarantine); } #[tokio::test] @@ -90,7 +114,31 @@ async fn dmarc_dkim_strict_subdomain() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", + "v=DMARC1; p=reject; adkim=s; \ + rua=mailto:dmarc-feedback@example.com" + .to_string(), + ); + + let result = evaluate_ip(TestData { + client_ip: Ipv4Addr::LOCALHOST, + from_domain: "example.com", + mail_from_domain: "example.com", + dkim_domains: &[Some("example.com")], + resolver: &resolver, + }) + .await; + + k9::assert_equal!(result.result, Disposition::Pass); +} + +#[tokio::test] +async fn dmarc_dkim_strict_subdomain_fail() { + let resolver = TestResolver::default() + .with_zone(EXAMPLE_COM) + .unwrap() + .with_txt( + "_dmarc.example.com", "v=DMARC1; p=reject; adkim=s; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -105,7 +153,7 @@ async fn dmarc_dkim_strict_subdomain() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); } #[tokio::test] @@ -114,7 +162,7 @@ async fn dmarc_dkim_relaxed_illformed() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; adkim=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -129,7 +177,7 @@ async fn dmarc_dkim_relaxed_illformed() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); k9::assert_equal!(result.context.contains("d="), true); } @@ -139,7 +187,7 @@ async fn dmarc_dkim_strict_illformed() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; adkim=s; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -154,7 +202,7 @@ async fn dmarc_dkim_strict_illformed() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); k9::assert_equal!(result.context.contains("d="), true); } @@ -164,7 +212,7 @@ async fn dmarc_spf_relaxed_subdomain() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; aspf=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -179,7 +227,7 @@ async fn dmarc_spf_relaxed_subdomain() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Pass); + k9::assert_equal!(result.result, Disposition::Pass); } #[tokio::test] @@ -188,7 +236,7 @@ async fn dmarc_spf_relaxed_subdomain_deep() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; aspf=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -203,7 +251,7 @@ async fn dmarc_spf_relaxed_subdomain_deep() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Pass); + k9::assert_equal!(result.result, Disposition::Pass); } #[tokio::test] @@ -212,7 +260,7 @@ async fn dmarc_spf_relaxed_subdomain_fail() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; aspf=r; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -227,7 +275,7 @@ async fn dmarc_spf_relaxed_subdomain_fail() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); } #[tokio::test] @@ -236,7 +284,7 @@ async fn dmarc_spf_strict_subdomain() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", "v=DMARC1; p=reject; aspf=s; \ rua=mailto:dmarc-feedback@example.com" .to_string(), @@ -251,7 +299,7 @@ async fn dmarc_spf_strict_subdomain() { }) .await; - k9::assert_equal!(result.result, DmarcResult::Fail); + k9::assert_equal!(result.result, Disposition::Reject); } #[tokio::test] @@ -264,7 +312,7 @@ async fn dmarc_pct_rate() { .with_zone(EXAMPLE_COM) .unwrap() .with_txt( - "example.com", + "_dmarc.example.com", format!( "v=DMARC1; p=reject; aspf=s; pct={pct}; \ rua=mailto:dmarc-feedback@example.com" @@ -281,7 +329,7 @@ async fn dmarc_pct_rate() { }) .await; - if matches!(result.result, DmarcResult::Fail) { + if matches!(result.result, Disposition::Reject) { total_failures += 1; } } @@ -302,7 +350,7 @@ async fn evaluate_ip<'a>( dkim_domains, resolver, }: TestData<'a>, -) -> DmarcResultWithContext { +) -> DispositionWithContext { let mut dkim_vec = vec![]; for dkim_domain in dkim_domains { @@ -336,7 +384,8 @@ $ORIGIN example.com. A 192.0.2.11 amy A 192.0.2.65 bob A 192.0.2.66 -sample A 192.0.2.67 +_dmarc A 192.0.2.67 +sample A 192.0.2.68 mail-a A 192.0.2.129 mail-b A 192.0.2.130 www CNAME example.com."#; diff --git a/crates/kumo-dmarc/src/types/record.rs b/crates/kumo-dmarc/src/types/record.rs index 67311f2f..a91192ef 100644 --- a/crates/kumo-dmarc/src/types/record.rs +++ b/crates/kumo-dmarc/src/types/record.rs @@ -3,9 +3,8 @@ use crate::types::format::Format; use crate::types::mode::Mode; 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 crate::types::results::{Disposition, DispositionWithContext}; +use crate::{DmarcContext, SenderDomainAlignment}; use std::str::FromStr; #[derive(Debug)] @@ -19,18 +18,18 @@ pub struct Record { interval: u32, aggregate_feedback: Vec, message_failure: Vec, - subdomain_policy: Policy, + subdomain_policy: Option, } impl Record { pub(crate) async fn evaluate( &self, cx: &DmarcContext<'_>, - _resolver: &dyn Resolver, - ) -> DmarcResultWithContext { + sender_location: SenderDomainAlignment, + ) -> DispositionWithContext { if rand::random::() % 100 >= self.rate { - return DmarcResultWithContext { - result: DmarcResult::Pass, + return DispositionWithContext { + result: Disposition::Pass, context: format!("sampled_out due to pct={}", self.rate), }; } @@ -41,14 +40,14 @@ impl Record { let organizational_domain = psl::domain_str(cx.from_domain); if cx.from_domain != result && organizational_domain != Some(result) { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: DKIM relaxed check failed".into(), }; } } else { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: DKIM signature missing 'd=' tag".into(), }; } @@ -58,14 +57,14 @@ impl Record { for dkim in cx.dkim { if let Some(result) = dkim.get("header.d") { if cx.from_domain != result { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: DKIM strict check failed".into(), }; } } else { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: DKIM signature missing 'd=' tag".into(), }; } @@ -81,8 +80,8 @@ impl Record { if mail_from_domain != cx.from_domain && organizational_domain != Some(cx.from_domain) { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: SPF relaxed check failed".into(), }; } @@ -91,8 +90,8 @@ impl Record { Mode::Strict => { if let Some(mail_from_domain) = cx.mail_from_domain { if mail_from_domain != cx.from_domain { - return DmarcResultWithContext { - result: DmarcResult::Fail, + return DispositionWithContext { + result: self.select_failure_mode(sender_location), context: "DMARC: SPF strict check failed".into(), }; } @@ -100,11 +99,24 @@ impl Record { } } - DmarcResultWithContext { - result: DmarcResult::Pass, + DispositionWithContext { + result: Disposition::Pass, context: "Success".into(), } } + + fn select_failure_mode(&self, sender_location: SenderDomainAlignment) -> Disposition { + match sender_location { + SenderDomainAlignment::OrganizationalDomain => { + if let Some(policy) = self.subdomain_policy { + policy.into() + } else { + self.policy.into() + } + } + SenderDomainAlignment::Exact => self.policy.into(), + } + } } impl FromStr for Record { @@ -121,7 +133,7 @@ impl FromStr for Record { interval: 86400, aggregate_feedback: Vec::new(), message_failure: Vec::new(), - subdomain_policy: Policy::None, + subdomain_policy: None, }; let (mut version, mut policy) = (false, false); @@ -144,7 +156,6 @@ impl FromStr for Record { match key { "p" => { new.policy = Policy::from_str(value)?; - new.subdomain_policy = new.policy; policy = true; } "adkim" => new.align_dkim = Mode::from_str(value)?, @@ -170,7 +181,7 @@ impl FromStr for Record { new.message_failure.push(FeedbackAddress::from_str(addr)?); } } - "sp" => new.subdomain_policy = Policy::from_str(value)?, + "sp" => new.subdomain_policy = Some(Policy::from_str(value)?), _ => return Err(format!("invalid key {key:?}")), } } diff --git a/crates/kumo-dmarc/src/types/results.rs b/crates/kumo-dmarc/src/types/results.rs index 0f826635..f2e3f26c 100644 --- a/crates/kumo-dmarc/src/types/results.rs +++ b/crates/kumo-dmarc/src/types/results.rs @@ -79,11 +79,42 @@ impl fmt::Display for DmarcResult { } } +// A coverage of both success and various failure modes +#[derive(Debug, Eq, PartialEq, ToXml, Serialize, Clone, Copy)] +#[xml(scalar)] +pub enum Disposition { + Pass, + None, + Quarantine, + Reject, +} + +impl ToString for Disposition { + fn to_string(&self) -> String { + match self { + Disposition::None => "None".to_string(), + Disposition::Pass => "Pass".to_string(), + Disposition::Quarantine => "Quarantine".to_string(), + Disposition::Reject => "Reject".to_string(), + } + } +} + +impl Into for Policy { + fn into(self) -> Disposition { + match self { + Policy::None => Disposition::None, + Policy::Quarantine => Disposition::Quarantine, + Policy::Reject => Disposition::Reject, + } + } +} + // A synthetic type to bundle the result with a reason #[derive(Debug, Eq, PartialEq, ToXml, Serialize)] #[xml(rename_all = "lowercase")] -pub struct DmarcResultWithContext { - pub result: DmarcResult, +pub struct DispositionWithContext { + pub result: Disposition, pub context: String, } diff --git a/crates/kumod/src/dmarc.rs b/crates/kumod/src/dmarc.rs index 5eb61d0f..b1590c91 100644 --- a/crates/kumod/src/dmarc.rs +++ b/crates/kumod/src/dmarc.rs @@ -1,6 +1,6 @@ use crate::smtp_server::ConnectionMetaData; use config::{get_or_create_sub_module, serialize_options}; -use kumo_dmarc::{CheckHostParams, DmarcResult}; +use kumo_dmarc::{CheckHostParams, Disposition}; use mailparsing::AuthenticationResult; use message::Message; use mlua::{Lua, LuaSerdeExt, UserDataRef}; @@ -11,7 +11,7 @@ use std::str::FromStr; #[derive(Debug, Serialize)] struct CheckHostOutput { - disposition: DmarcResult, + disposition: Disposition, result: AuthenticationResult, } @@ -44,9 +44,10 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> { if let Ok(from_domain) = from.domain() { from_domain.to_string() } else { + // Handling a missing RFC5322.From domain is outside of dmarc return Ok(lua.to_value_with( &CheckHostOutput { - disposition: DmarcResult::Fail, + disposition: Disposition::None, result: AuthenticationResult { method: "dmarc".to_string(), method_version: None, @@ -59,9 +60,10 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> { )) } } else { + // The current implementation expects only a single RFC5322.From domain return Ok(lua.to_value_with( &CheckHostOutput { - disposition: DmarcResult::Fail, + disposition: Disposition::None, result: AuthenticationResult { method: "dmarc".to_string(), method_version: None,