From 9eefa6e2a7fbeadc9a0290b65bd885e61abda5b0 Mon Sep 17 00:00:00 2001 From: vanruch Date: Fri, 13 Sep 2024 16:58:26 +0200 Subject: [PATCH 1/2] Extract Resolve trait, call async function in sync context --- Cargo.toml | 4 +-- src/arc/seal.rs | 2 +- src/arc/verify.rs | 2 +- src/common/mod.rs | 1 + src/common/resolve.rs | 65 +++++++++++++++++++++++++++++++++++++++ src/common/resolver.rs | 48 ----------------------------- src/dkim/sign.rs | 2 +- src/dkim/verify.rs | 67 +++++++++++++++++++++++++++++++--------- src/dmarc/verify.rs | 70 +++++++++++++++++++++--------------------- src/spf/verify.rs | 13 ++++---- 10 files changed, 165 insertions(+), 109 deletions(-) create mode 100644 src/common/resolve.rs diff --git a/Cargo.toml b/Cargo.toml index b21deeb..41819b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ name = "mail-auth" description = "DKIM, ARC, SPF and DMARC library for Rust" version = "0.5.0" edition = "2021" -authors = [ "Stalwart Labs "] +authors = ["Stalwart Labs "] license = "Apache-2.0 OR MIT" repository = "https://github.com/stalwartlabs/mail-auth" homepage = "https://github.com/stalwartlabs/mail-auth" @@ -40,7 +40,7 @@ sha2 = { version = "0.10.6", features = ["oid"], optional = true } hickory-resolver = { version = "0.24", features = ["dns-over-rustls", "dnssec-ring"] } zip = "2.1.1" rand = { version = "0.8.5", optional = true } - +futures = { version = "0.3.30", features = ["executor"] } [dev-dependencies] tokio = { version = "1.16", features = ["net", "io-util", "time", "rt-multi-thread", "macros"] } rustls-pemfile = "2" diff --git a/src/arc/seal.rs b/src/arc/seal.rs index e28415b..5539f5c 100644 --- a/src/arc/seal.rs +++ b/src/arc/seal.rs @@ -314,7 +314,7 @@ mod test { pk: impl SigningKey, ) -> String { let message = AuthenticatedMessage::parse(raw_message.as_bytes()).unwrap(); - let dkim_result = DkimVerifier::verify_dkim(&resolver, &message).await; + let dkim_result = DkimVerifier::verify_dkim(resolver, &message).await; let arc_result = resolver.verify_arc(&message).await; assert!( matches!(arc_result.result(), DkimResult::Pass | DkimResult::None), diff --git a/src/arc/verify.rs b/src/arc/verify.rs index 770a1f1..2dd1dd4 100644 --- a/src/arc/verify.rs +++ b/src/arc/verify.rs @@ -19,7 +19,7 @@ use crate::{ dkim::{verify::Verifier, Canonicalization}, ArcOutput, AuthenticatedMessage, DkimResult, Error, Resolver, }; - +use crate::common::resolve::Resolve; use super::{ChainValidation, Set}; impl Resolver { diff --git a/src/common/mod.rs b/src/common/mod.rs index 8d55fbb..dffda96 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -19,6 +19,7 @@ pub mod message; pub mod parse; pub mod resolver; pub mod verify; +pub(crate) mod resolve; impl From for IprevResult { fn from(err: Error) -> Self { diff --git a/src/common/resolve.rs b/src/common/resolve.rs new file mode 100644 index 0000000..0453372 --- /dev/null +++ b/src/common/resolve.rs @@ -0,0 +1,65 @@ +use std::sync::Arc; +use hickory_resolver::Name; +use std::borrow::Cow; +use crate::common::parse::TxtRecordParser; +use crate::{Error, Resolver, Txt}; +use crate::common::lru::DnsCache; +#[cfg(any(test, feature = "test"))] +use crate::common::resolver; +use crate::common::resolver::{IntoFqdn, UnwrapTxtRecord}; + +impl Resolve for Resolver { + async fn txt_lookup<'x, T: TxtRecordParser + Into + UnwrapTxtRecord>( + &self, + key: impl IntoFqdn<'x>, + ) -> crate::Result> { + let key = key.into_fqdn(); + if let Some(value) = self.cache_txt.get(key.as_ref()) { + return T::unwrap_txt(value); + } + + #[cfg(any(test, feature = "test"))] + if true { + return resolver::mock_resolve(key.as_ref()); + } + + let txt_lookup = self + .resolver + .txt_lookup(Name::from_str_relaxed(key.as_ref())?) + .await?; + let mut result = Err(Error::InvalidRecordType); + let records = txt_lookup.as_lookup().record_iter().filter_map(|r| { + let txt_data = r.data()?.as_txt()?.txt_data(); + match txt_data.len() { + 1 => Cow::from(txt_data[0].as_ref()).into(), + 0 => None, + _ => { + let mut entry = Vec::with_capacity(255 * txt_data.len()); + for data in txt_data { + entry.extend_from_slice(data); + } + Cow::from(entry).into() + } + } + }); + + for record in records { + result = T::parse(record.as_ref()); + if result.is_ok() { + break; + } + } + T::unwrap_txt(self.cache_txt.insert( + key.into_owned(), + result.into(), + txt_lookup.valid_until(), + )) + } +} + +pub trait Resolve { + async fn txt_lookup<'x, T: TxtRecordParser + Into + UnwrapTxtRecord>( + &self, + key: impl IntoFqdn<'x>, + ) -> crate::Result>; +} \ No newline at end of file diff --git a/src/common/resolver.rs b/src/common/resolver.rs index bbb6783..97de565 100644 --- a/src/common/resolver.rs +++ b/src/common/resolver.rs @@ -32,7 +32,6 @@ use crate::{ use super::{ lru::{DnsCache, LruCache}, - parse::TxtRecordParser, verify::DomainKey, }; @@ -119,53 +118,6 @@ impl Resolver { Ok(result) } - pub async fn txt_lookup<'x, T: TxtRecordParser + Into + UnwrapTxtRecord>( - &self, - key: impl IntoFqdn<'x>, - ) -> crate::Result> { - let key = key.into_fqdn(); - if let Some(value) = self.cache_txt.get(key.as_ref()) { - return T::unwrap_txt(value); - } - - #[cfg(any(test, feature = "test"))] - if true { - return mock_resolve(key.as_ref()); - } - - let txt_lookup = self - .resolver - .txt_lookup(Name::from_str_relaxed(key.as_ref())?) - .await?; - let mut result = Err(Error::InvalidRecordType); - let records = txt_lookup.as_lookup().record_iter().filter_map(|r| { - let txt_data = r.data()?.as_txt()?.txt_data(); - match txt_data.len() { - 1 => Cow::from(txt_data[0].as_ref()).into(), - 0 => None, - _ => { - let mut entry = Vec::with_capacity(255 * txt_data.len()); - for data in txt_data { - entry.extend_from_slice(data); - } - Cow::from(entry).into() - } - } - }); - - for record in records { - result = T::parse(record.as_ref()); - if result.is_ok() { - break; - } - } - T::unwrap_txt(self.cache_txt.insert( - key.into_owned(), - result.into(), - txt_lookup.valid_until(), - )) - } - pub async fn mx_lookup<'x>(&self, key: impl IntoFqdn<'x>) -> crate::Result>> { let key = key.into_fqdn(); if let Some(value) = self.cache_mx.get(key.as_ref()) { diff --git a/src/dkim/sign.rs b/src/dkim/sign.rs index f1a9ebb..4f234d4 100644 --- a/src/dkim/sign.rs +++ b/src/dkim/sign.rs @@ -522,7 +522,7 @@ pub mod test { message.extend_from_slice(message_.as_bytes()); let message = AuthenticatedMessage::parse_with_opts(&message, strict).unwrap(); - let dkim = DkimVerifier::verify_dkim(&resolver, &message).await; + let dkim = DkimVerifier::verify_dkim(resolver, &message).await; match (dkim.last().unwrap().result(), &expect) { (DkimResult::Pass, Ok(_)) => (), diff --git a/src/dkim/verify.rs b/src/dkim/verify.rs index 2d13dd5..e417497 100644 --- a/src/dkim/verify.rs +++ b/src/dkim/verify.rs @@ -8,17 +8,15 @@ * except according to those terms. */ -use std::time::SystemTime; - use crate::{ common::{ base32::Base32Writer, headers::Writer, verify::{DomainKey, VerifySignature}, }, - is_within_pct, AuthenticatedMessage, DkimOutput, DkimResult, Error, Resolver, + is_within_pct, AuthenticatedMessage, DkimOutput, DkimResult, Error, }; - +use crate::common::resolve::Resolve; use super::{ Atps, DomainKeyReport, Flag, HashAlgorithm, Signature, RR_DNS, RR_EXPIRATION, RR_OTHER, RR_SIGNATURE, RR_VERIFICATION, @@ -27,14 +25,12 @@ use super::{ pub struct DkimVerifier {} impl DkimVerifier { + // TODO replace with argument fn current_timestamp() -> u64 { - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs() + 1667843664 } - pub async fn verify_dkim<'x>(resolver: &Resolver, message: &'x AuthenticatedMessage<'x>) -> Vec> { + pub async fn verify_dkim<'x, R: Resolve>(resolver: &R, message: &'x AuthenticatedMessage<'x>) -> Vec> { let now = Self::current_timestamp(); let mut output = Vec::with_capacity(message.dkim_headers.len()); @@ -383,11 +379,7 @@ mod test { time::{Duration, Instant}, }; - use crate::{ - common::{parse::TxtRecordParser, verify::DomainKey}, - dkim::verify::Verifier, - AuthenticatedMessage, DkimResult, Resolver, - }; + use crate::{common::{parse::TxtRecordParser, verify::DomainKey}, dkim::verify::Verifier, AuthenticatedMessage, DkimResult, Resolver, Txt}; use crate::dkim::verify::DkimVerifier; #[ignore] @@ -416,6 +408,53 @@ mod test { } } + use std::future::{ready, Ready}; + use std::sync::Arc; + use crate::common::resolve::Resolve; + use crate::common::resolver::{IntoFqdn, UnwrapTxtRecord}; + + struct MockResolver { + pub dns_records: String, + } + + impl Resolve for MockResolver { + async fn txt_lookup<'x, T: TxtRecordParser + Into + UnwrapTxtRecord>( + &self, + _key: impl IntoFqdn<'x>, + ) -> Result, super::Error> { + Ok(Arc::new(T::parse(self.dns_records.as_bytes())?)) + } + } + + #[test] + fn dkim_verify_sync() { + let mut test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + test_dir.push("resources"); + test_dir.push("dkim"); + + for file_name in fs::read_dir(&test_dir).unwrap() { + let file_name = file_name.unwrap().path(); + /*if !file_name.to_str().unwrap().contains("002") { + continue; + }*/ + println!("DKIM verifying {}", file_name.to_str().unwrap()); + + let test = String::from_utf8(fs::read(&file_name).unwrap()).unwrap(); + let (dns_records, raw_message) = test.split_once("\n\n").unwrap(); + let resolver = MockResolver { + dns_records: dns_records.to_string(), + }; + let raw_message = raw_message.replace('\n', "\r\n"); + let message = AuthenticatedMessage::parse(raw_message.as_bytes()).unwrap(); + + let future = DkimVerifier::verify_dkim(&resolver, &message); + let dkim = futures::executor::block_on(future); + + + assert_eq!(dkim.last().unwrap().result(), &DkimResult::Pass); + } + } + #[test] fn dkim_strip_signature() { for (value, stripped_value) in [ diff --git a/src/dmarc/verify.rs b/src/dmarc/verify.rs index 5f84969..ad3e27b 100644 --- a/src/dmarc/verify.rs +++ b/src/dmarc/verify.rs @@ -14,7 +14,7 @@ use crate::{ AuthenticatedMessage, DkimOutput, DkimResult, DmarcOutput, DmarcResult, Error, Resolver, SpfOutput, SpfResult, }; - +use crate::common::resolve::Resolve; use super::{Alignment, Dmarc, URI}; impl Resolver { @@ -91,10 +91,10 @@ impl Resolver { DmarcResult::Pass } else if dmarc.adkim == Alignment::Relaxed && dkim_output.iter().any(|o| { - o.result == DkimResult::Pass - && domain_suffix_fn(&o.signature.as_ref().unwrap().d) - == rfc5322_from_subdomain - }) + o.result == DkimResult::Pass + && domain_suffix_fn(&o.signature.as_ref().unwrap().d) + == rfc5322_from_subdomain + }) { output.policy = dmarc.sp; DmarcResult::Pass @@ -102,7 +102,7 @@ impl Resolver { if dkim_output.iter().any(|o| { o.result == DkimResult::Pass && domain_suffix_fn(&o.signature.as_ref().unwrap().d) - == rfc5322_from_subdomain + == rfc5322_from_subdomain }) { output.policy = dmarc.sp; } @@ -124,21 +124,21 @@ impl Resolver { for address in addresses { if address.uri.ends_with(domain) || match self - .txt_lookup::(format!( - "{}._report._dmarc.{}.", - domain, - address - .uri - .rsplit_once('@') - .map(|(_, d)| d) - .unwrap_or_default() - )) - .await - { - Ok(_) => true, - Err(Error::DnsError(_)) => return None, - _ => false, - } + .txt_lookup::(format!( + "{}._report._dmarc.{}.", + domain, + address + .uri + .rsplit_once('@') + .map(|(_, d)| d) + .unwrap_or_default() + )) + .await + { + Ok(_) => true, + Err(Error::DnsError(_)) => return None, + _ => false, + } { result.push(address); } @@ -219,8 +219,8 @@ mod test { ( "_dmarc.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@example.org\r\n\r\n", "example.org", @@ -235,8 +235,8 @@ mod test { ( "_dmarc.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@example.org\r\n\r\n", "subdomain.example.org", @@ -251,8 +251,8 @@ mod test { ( "_dmarc.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@example.org\r\n\r\n", "subdomain.example.org", @@ -267,8 +267,8 @@ mod test { ( "_dmarc.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@a.b.c.example.org\r\n\r\n", "a.b.c.example.org", @@ -283,8 +283,8 @@ mod test { ( "_dmarc.c.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@a.b.c.example.org\r\n\r\n", "example.org", @@ -299,8 +299,8 @@ mod test { ( "_dmarc.c.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=r; adkim=r; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@a.b.c.example.org\r\n\r\n", "z.example.org", @@ -315,8 +315,8 @@ mod test { ( "_dmarc.example.org.", concat!( - "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", - "rua=mailto:dmarc-feedback@example.org" + "v=DMARC1; p=reject; sp=quarantine; np=None; aspf=s; adkim=s; fo=1;", + "rua=mailto:dmarc-feedback@example.org" ), "From: hello@example.org\r\n\r\n", "example.org", diff --git a/src/spf/verify.rs b/src/spf/verify.rs index 9bc781b..9e53cb9 100644 --- a/src/spf/verify.rs +++ b/src/spf/verify.rs @@ -14,7 +14,7 @@ use std::{ }; use crate::{Error, Resolver, SpfOutput, SpfResult}; - +use crate::common::resolve::Resolve; use super::{Macro, Mechanism, Qualifier, Spf, Variables}; #[allow(clippy::iter_skip_zero)] @@ -34,7 +34,7 @@ impl Resolver { host_domain, &format!("postmaster@{helo_domain}"), ) - .await + .await } else { SpfOutput::new(helo_domain.to_string()).with_result(SpfResult::None) } @@ -55,7 +55,7 @@ impl Resolver { host_domain, sender, ) - .await + .await } /// Verifies both the SPF EHLO and MAIL FROM identities @@ -274,9 +274,9 @@ impl Resolver { { matches = record == &target_addr || record - .strip_suffix('.') - .unwrap_or(record.as_str()) - .ends_with(&target_sub_addr); + .strip_suffix('.') + .unwrap_or(record.as_str()) + .ends_with(&target_sub_addr); if matches { break; } @@ -532,7 +532,6 @@ impl HasValidLabels for &str { #[cfg(test)] #[allow(unused)] mod test { - use std::{ fs, net::{IpAddr, Ipv4Addr, Ipv6Addr}, From 008c5c5726af710f61e7529b5742abf3afaaeadb Mon Sep 17 00:00:00 2001 From: vanruch Date: Tue, 17 Sep 2024 12:28:03 +0200 Subject: [PATCH 2/2] make resolve pub --- src/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/mod.rs b/src/common/mod.rs index dffda96..d19590b 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -19,7 +19,7 @@ pub mod message; pub mod parse; pub mod resolver; pub mod verify; -pub(crate) mod resolve; +pub mod resolve; impl From for IprevResult { fn from(err: Error) -> Self {