From 7d4fac698f485a59bfbf01784bf52496587114fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jagoda=20Estera=20=C5=9Al=C4=85zak?= <128227338+j-g00da@users.noreply.github.com> Date: Thu, 19 Feb 2026 14:33:40 +0100 Subject: [PATCH] fix(dkim): Make simple header canonicalization work properly (#53) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously header parsing caused leading whitespaces to be trimmed in HeaderBody, which resulted in incorrect simple header canonicalization. Signed-off-by: Jagoda Ślązak --- filtermail/src/dkim_verifier.rs | 120 +++++++++++++++++------- filtermail/test_data/asm.eml | 112 +++++++++++----------- filtermail/test_data/dkim-abjadiyah.eml | 47 ++++++++++ 3 files changed, 191 insertions(+), 88 deletions(-) create mode 100644 filtermail/test_data/dkim-abjadiyah.eml diff --git a/filtermail/src/dkim_verifier.rs b/filtermail/src/dkim_verifier.rs index 5ec7e18d..f61b9746 100644 --- a/filtermail/src/dkim_verifier.rs +++ b/filtermail/src/dkim_verifier.rs @@ -116,17 +116,67 @@ impl LookupTxt for CachedResolver { } } +/// Dummy resolver that always returns the same TXT record, for testing purposes. +#[derive(Clone)] +struct MockResolver(String); + +impl LookupTxt for MockResolver { + type Answer = Box>>>; + type Query<'a> = Pin> + Send + 'a>>; + + fn lookup_txt(&self, _domain: &str) -> Self::Query<'_> { + Box::pin(async move { + let txts: Self::Answer = Box::new(std::iter::once(Ok(self.0.clone().into_bytes()))); + Ok(txts) + }) + } +} + +/// Either a real resolver or a mock. +#[allow(clippy::large_enum_variant)] +#[derive(Clone)] +enum Resolver { + /// A [`CachedResolver`] + Real(CachedResolver), + /// A [`MockResolver`] + Mock(MockResolver), +} + +impl LookupTxt for Resolver { + type Answer = Box>>>; + type Query<'a> = Pin> + Send + 'a>>; + + fn lookup_txt(&self, domain: &str) -> Self::Query<'_> { + match self { + Resolver::Real(resolver) => resolver.lookup_txt(domain), + Resolver::Mock(resolver) => resolver.lookup_txt(domain), + } + } +} + +impl From for Resolver { + fn from(value: CachedResolver) -> Self { + Resolver::Real(value) + } +} + +impl From for Resolver { + fn from(value: MockResolver) -> Self { + Resolver::Mock(value) + } +} + /// DKIM verifier using a pre-configured [`viadkim`] verifier, a [`CachedResolver`] for DNS lookups, /// and strict domain name alignment check. pub struct DkimVerifier { - resolver: CachedResolver, + resolver: Resolver, config: viadkim::Config, } impl DkimVerifier { /// Creates a new [`DkimVerifier`] with the provided resolver. pub fn new() -> Result { - let resolver = CachedResolver::new()?; + let resolver = CachedResolver::new()?.into(); let config = viadkim::Config { lookup_timeout: Duration::from_secs(60), ..Default::default() @@ -134,44 +184,34 @@ impl DkimVerifier { Ok(Self { resolver, config }) } + /// Creates a new [`DkimVerifier`] with a mock resolver that always returns the provided TXT record. + #[cfg(test)] + fn mock(txt: String) -> Self { + let resolver = MockResolver(txt).into(); + let config = viadkim::Config { + lookup_timeout: Duration::from_secs(60), + ..Default::default() + }; + Self { resolver, config } + } + /// Verifies the DKIM signature of a raw email message and its alignment with the provided /// domain. pub async fn verify(&self, raw_mail: &[u8], from_domain: &str) -> Result<(), String> { - let (headers, body_start) = { - use viadkim::{FieldBody, FieldName, HeaderField}; + let mail_data = String::from_utf8_lossy(raw_mail); + let (header, body) = mail_data + .split_once("\r\n\r\n") + .ok_or("554 Malformed data")?; - let Ok((headers, body_start)) = mailparse::parse_headers(raw_mail) else { - return Err("500 Failed to parse message headers".to_string()); - }; - - let mut viadkim_headers: Vec = Vec::new(); - for header in headers { - match ( - FieldName::new(header.get_key()), - FieldBody::new(header.get_value_raw()), - ) { - (Ok(name), Ok(body)) => viadkim_headers.push((name, body)), - (Err(e), _) | (_, Err(e)) => { - log::debug!("Failed to parse header {header:?}, skipping: {e}"); - } - } - } - - let viadkim_headers = viadkim::HeaderFields::new(viadkim_headers).map_err(|e| { - log::error!("Failed to parse headers for DKIM verification: {e}"); - "500 Failed to parse message headers".to_string() - })?; - - (viadkim_headers, body_start) - }; + let header = header.parse().map_err(|_| "554 Malformed header")?; let Some(mut verifier) = - viadkim::Verifier::verify_header(&self.resolver, &headers, &self.config).await + viadkim::Verifier::verify_header(&self.resolver, &header, &self.config).await else { return Err("554 5.7.1 No DKIM signature found".to_string()); }; - 'hasher: for chunk in raw_mail.get(body_start..).unwrap_or_default().chunks(8192) { + 'hasher: for chunk in body.as_bytes().chunks(8192) { if verifier.process_body_chunk(chunk) == BodyHasherStance::Done { break 'hasher; } @@ -189,8 +229,10 @@ impl DkimVerifier { log::debug!("Signature {}: Verification failed, skipping", res.index); // We only invalidate cache on actual validation error, and not alignment error. // TODO: ideally we should retry without cache and swap cached value only on success. - self.resolver - .invalidate_cache(signature.selector.as_ref(), signature.domain.as_ref()); + if let Resolver::Real(resolver) = &self.resolver { + resolver + .invalidate_cache(signature.selector.as_ref(), signature.domain.as_ref()); + } continue; } @@ -212,3 +254,17 @@ impl DkimVerifier { Err("554 5.7.1 No valid DKIM signature found".to_string()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn test_dkim_verifier() { + let verifier = DkimVerifier::mock( + r#"v=DKIM1;k=rsa;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5krC4Xi5Wkr6eMlla38LCFmV645E3FLAgsRl2YJ0SrZ4N2Vw1/yH0mefvtk7HYE7ytV7RQl/er2CkSsaHLJSYLmPCBw5CO6PSsBSXuh6DBqdylh/1t9vVQ9p38fTwn9gU1QvplcpRQL9eepRra1k24VMIaVy2ZZcu3LI9zkPsR7o7TyNaeMhsL8ouWInWc1NSid+p0SgliQuwHIejZhlTPE60JLbJE0OR9I4wmq3377H6z/QrO8XeabCgtmTuzE/hTRyIyNS40jql/99pjlhIcjM2U+P2B0FjwYt7BwLHsgANr74ctlnKY+SdH25rNwVpPmkotaULG5SJCByKBkfCwIDAQAB;s=email;t=s"#.to_string() + ); + let raw_mail = include_bytes!("../test_data/dkim-abjadiyah.eml"); + verifier.verify(raw_mail, "abjadiyah.xyz").await.unwrap(); + } +} diff --git a/filtermail/test_data/asm.eml b/filtermail/test_data/asm.eml index d95bb683..372d0041 100644 --- a/filtermail/test_data/asm.eml +++ b/filtermail/test_data/asm.eml @@ -1,56 +1,56 @@ -From: one@example.org -To: two@example.org -Autocrypt-Setup-Message: v1 -Subject: Autocrypt Setup Message -Date: Tue, 22 Jan 2019 12:56:29 +0100 -Content-type: multipart/mixed; boundary="Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ" - ---Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ -Content-Type: text/plain - -This message contains all information to transfer your Autocrypt -settings along with your secret key securely from your original -device. - -To set up your new device for Autocrypt, please follow the -instuctions that should be presented by your new device. - -You can keep this message and use it as a backup for your secret -key. If you want to do this, you should write down the Setup Code -and store it securely. ---Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ -Content-Type: application/autocrypt-setup -Content-Disposition: attachment; filename="autocrypt-setup-message.html" - - -

-This is the Autocrypt setup file used to transfer settings and -keys between clients. You can decrypt it using the Setup Code -presented on your old device, and then import the contained key -into your keyring. -

- -
------BEGIN PGP MESSAGE-----
-Passphrase-Format: numeric9x4
-Passphrase-Begin: 17
-
-jA0EBwMCFAxADoCdzeX/0ukBlqI5+pfpKb751qd/7nLNbkpy3gVcaf1QwRPZYt40
-Ynp08UqRQ2g48ZlnzHLSwlTGOPTuv2Jt8ka+pgZ45xzvJSG2gau03xP4VsC271kR
-VmCjdb0Y6Rk96mAwfGzrkbaRQ9Z7fIoL866GOv6h9neiVIkp+JYlTV6ISD0ZQJ4Q
-I6dOQkB/TWZyVjtiJDOQHdfNWliA6NtqaLq19wlu9L5xXjuNpY95KwR8EJXWe0+o
-Y3d2U/KxOAkXKghP2Qg1GtlPVeGC5T4p03TGI6pzKT+kHX6Rrm9wK6sM9aTquMmF
-Vok84Jg1DFnwivWC2RILR81rXi7k/+Y6MUbveFgJ9cQduqpxnmD7TjOblYu7M6zp
-YGAUxh8DRKlIMn2QsA++DBYQ6ACZvwuY8qTDLkqPDo4WqM313dsMJbyGjDdVE7EM
-PESS+RlABETpZXz8g/ycr6DIUNdlbPcmYlsBfHWDOuR2GFFTwmlv5slWS39dJv38
-E0eIe1CwdxI801Se7t7dUUS/ZF8wb6GlmxOcqGbF8eko1Z0S64IAm7/h13MRQCxI
-geQnHfGYVJ2FOimoCMEKwfa9x++RFTDW0u7spDC2uWvK/1viV8OfRppFhLr/kmKb
-18lWXuAz80DAjUDUsVqEq2MvJBJGoCJUEyjuRsLkHYRM5jYk4v50LyyR0Om73nWF
-nZBqmqNzdr7Xb9PHHdFhnEc0VvoYbrcM0RVYcEMW3YbmejM891j1d6Iv+/n/qND/
-NdebGrfWJMmFLf/iEkzTZ3/v5inW9LpWoRc94ioCjJTaEo8Rib6ARRFaJVIsmNXi
-YicFGO98D+zX+a2t9Yz6IpPajVslnOp6ScpmXgts/2XWD7oE+JgxSAqo/dLVsHgP
-Ufo=
-=pulM
------END PGP MESSAGE-----
-
---Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ-- +From: one@example.org +To: two@example.org +Autocrypt-Setup-Message: v1 +Subject: Autocrypt Setup Message +Date: Tue, 22 Jan 2019 12:56:29 +0100 +Content-type: multipart/mixed; boundary="Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ" + +--Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ +Content-Type: text/plain + +This message contains all information to transfer your Autocrypt +settings along with your secret key securely from your original +device. + +To set up your new device for Autocrypt, please follow the +instuctions that should be presented by your new device. + +You can keep this message and use it as a backup for your secret +key. If you want to do this, you should write down the Setup Code +and store it securely. +--Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ +Content-Type: application/autocrypt-setup +Content-Disposition: attachment; filename="autocrypt-setup-message.html" + + +

+This is the Autocrypt setup file used to transfer settings and +keys between clients. You can decrypt it using the Setup Code +presented on your old device, and then import the contained key +into your keyring. +

+ +
+-----BEGIN PGP MESSAGE-----
+Passphrase-Format: numeric9x4
+Passphrase-Begin: 17
+
+jA0EBwMCFAxADoCdzeX/0ukBlqI5+pfpKb751qd/7nLNbkpy3gVcaf1QwRPZYt40
+Ynp08UqRQ2g48ZlnzHLSwlTGOPTuv2Jt8ka+pgZ45xzvJSG2gau03xP4VsC271kR
+VmCjdb0Y6Rk96mAwfGzrkbaRQ9Z7fIoL866GOv6h9neiVIkp+JYlTV6ISD0ZQJ4Q
+I6dOQkB/TWZyVjtiJDOQHdfNWliA6NtqaLq19wlu9L5xXjuNpY95KwR8EJXWe0+o
+Y3d2U/KxOAkXKghP2Qg1GtlPVeGC5T4p03TGI6pzKT+kHX6Rrm9wK6sM9aTquMmF
+Vok84Jg1DFnwivWC2RILR81rXi7k/+Y6MUbveFgJ9cQduqpxnmD7TjOblYu7M6zp
+YGAUxh8DRKlIMn2QsA++DBYQ6ACZvwuY8qTDLkqPDo4WqM313dsMJbyGjDdVE7EM
+PESS+RlABETpZXz8g/ycr6DIUNdlbPcmYlsBfHWDOuR2GFFTwmlv5slWS39dJv38
+E0eIe1CwdxI801Se7t7dUUS/ZF8wb6GlmxOcqGbF8eko1Z0S64IAm7/h13MRQCxI
+geQnHfGYVJ2FOimoCMEKwfa9x++RFTDW0u7spDC2uWvK/1viV8OfRppFhLr/kmKb
+18lWXuAz80DAjUDUsVqEq2MvJBJGoCJUEyjuRsLkHYRM5jYk4v50LyyR0Om73nWF
+nZBqmqNzdr7Xb9PHHdFhnEc0VvoYbrcM0RVYcEMW3YbmejM891j1d6Iv+/n/qND/
+NdebGrfWJMmFLf/iEkzTZ3/v5inW9LpWoRc94ioCjJTaEo8Rib6ARRFaJVIsmNXi
+YicFGO98D+zX+a2t9Yz6IpPajVslnOp6ScpmXgts/2XWD7oE+JgxSAqo/dLVsHgP
+Ufo=
+=pulM
+-----END PGP MESSAGE-----
+
+--Y6fyGi9SoGeH8WwRaEdC6bbBcYOedDzrQ-- diff --git a/filtermail/test_data/dkim-abjadiyah.eml b/filtermail/test_data/dkim-abjadiyah.eml new file mode 100644 index 00000000..17c5e03e --- /dev/null +++ b/filtermail/test_data/dkim-abjadiyah.eml @@ -0,0 +1,47 @@ +DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=abjadiyah.xyz; + s=default; t=1771356412; + bh=EVfAHeUMDygbJe0SkMWJHjgXGjtiTLZnMQbyWqzsrCY=; + h=From:To:Subject:Date:From; + b=3GHd4aCp7sjNR7RfHzLUUQ0PLk4FM8WLlOO5d8BXvpa6oCjERP0vsP545EDo4lBFC + xg9dlSrhIwEWsrJDXSFStA7VU1WmWBVTzKoN8bZk5w/2HJf7BcB1BG2SQ0tyu2iEtl + sv21HCXLw1IAMd2IiZNPQRlk0PgrKDqFmgULbktIWkc917GNAWhJaVCQhk+YF6cfql + AKP3i8NPK53JY6vsMBO3YuYb/DcRGyvhUUJGofgG2+NwESA20ayI85EPiUzbzGpx0z + r5IsLwiMApO5W0svfzgU52VhtkAqVyi38ZvjYsCJlKXB885SRsHsojCCSIznvFSmNw + pWF4qKBvNrQpQ== +From: +To: "hidden-recipients": ; +Subject: [...] +Date: Sat, 14 Feb 2026 21:13:38 +0000 +X-Last-TLS-Session-Version: TLSv1.3 +X-Spamd-Result: default: False [5.18 / 15.00]; + MISSING_MID(2.50)[]; + FORGED_RECIPIENTS(2.00)[m:,s:link2xt@testrun.org]; + DATE_IN_PAST(1.00)[70]; + DMARC_POLICY_ALLOW(-0.50)[abjadiyah.xyz,reject]; + R_MISSING_CHARSET(0.50)[]; + R_DKIM_ALLOW(-0.20)[abjadiyah.xyz:s=default]; + R_SPF_ALLOW(-0.20)[+mx]; + MIME_GOOD(-0.10)[text/plain]; + FISHY_TLD(0.10)[abjadiyah.xyz]; + ONCE_RECEIVED(0.10)[]; + IP_REPUTATION_HAM(-0.01)[asn: 29670(0.00), country: DE(-0.01), ip: 2001:67c:1400:21d0::(0.00)]; + MX_GOOD(-0.01)[]; + MISSING_XM_UA(0.00)[]; + RCPT_COUNT_ONE(0.00)[1]; + BCC(0.00)[]; + ARC_NA(0.00)[]; + MIME_TRACE(0.00)[0:+]; + DWL_DNSWL_BLOCKED(0.00)[abjadiyah.xyz:dkim]; + DKIM_TRACE(0.00)[abjadiyah.xyz:+]; + RCPT_MAILCOW_DOMAIN(0.00)[testrun.org]; + SINGLE_SHORT_PART(0.00)[]; + ARC_SIGNED(0.00)[testrun.org:s=dkim:i=1]; + RCVD_COUNT_ZERO(0.00)[0]; + TO_DN_ALL(0.00)[]; + ASN(0.00)[asn:29670, ipnet:2001:67c:1400::/45, country:DE]; + FROM_NO_DN(0.00)[]; + FROM_EQ_ENVFROM(0.00)[] +X-Rspamd-Queue-Id: E471A4105C + +Hello! +