v0.4.0

CHANGELOG.md

@@ -1,3 +1,8 @@
+mail-auth 0.4.0
+================================
+- DKIM verification defaults to `strict` mode and ignores signatures with a `l=` tag to avoid exploits (see https://stalw.art/blog/dkim-exploit). Use `AuthenticatedMessage::parse_with_opts(&message, false)` to enable `relaxed` mode.
+- Parsed fields are now public.
+
 mail-auth 0.3.11
 ================================
 - Added: DKIM keypair generation for both RSA and Ed25519.

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "mail-auth"
 description = "DKIM, ARC, SPF and DMARC library for Rust"
-version = "0.3.11"
+version = "0.4.0"
 edition = "2021"
 authors = [ "Stalwart Labs <[email protected]>"]
 license = "Apache-2.0 OR MIT"
@@ -38,7 +38,7 @@ serde_json = "1.0"
 sha1 = { version = "0.10", features = ["oid"], optional = true }
 sha2 = { version = "0.10.6", features = ["oid"], optional = true }
 hickory-resolver = { version = "0.24", features = ["dns-over-rustls", "dnssec-ring"] }
-zip = "0.6.3"
+zip = "1.3.0"
 rand = { version = "0.8.5", optional = true }
 
 [dev-dependencies]

src/common/auth_results.rs

@@ -344,6 +344,7 @@ impl AsAuthResult for Error {
             Error::ArcBrokenChain => "broken ARC chain",
             Error::NotAligned => "policy not aligned",
             Error::InvalidRecordType => "invalid dns record type",
+            Error::SignatureLength => "signature length ignored due to security risk",
         });
         header.push(')');
     }

src/common/message.rs

@@ -16,6 +16,10 @@ use super::headers::{AuthenticatedHeader, Header, HeaderParser};
 
 impl<'x> AuthenticatedMessage<'x> {
     pub fn parse(raw_message: &'x [u8]) -> Option<Self> {
+        Self::parse_with_opts(raw_message, true)
+    }
+
+    pub fn parse_with_opts(raw_message: &'x [u8], strict: bool) -> Option<Self> {
         let mut message = AuthenticatedMessage {
             headers: Vec::new(),
             from: Vec::new(),
@@ -35,90 +39,103 @@ impl<'x> AuthenticatedMessage<'x> {
         let mut has_arc_errors = false;
 
         for (header, value) in &mut headers {
-            let name = match header {
-                AuthenticatedHeader::Ds(name) => {
-                    let signature = dkim::Signature::parse(value);
-                    if let Ok(signature) = &signature {
-                        let ha = HashAlgorithm::from(signature.a);
-                        if !message
-                            .body_hashes
-                            .iter()
-                            .any(|(c, h, l, _)| c == &signature.cb && h == &ha && l == &signature.l)
-                        {
-                            message
-                                .body_hashes
-                                .push((signature.cb, ha, signature.l, Vec::new()));
-                        }
-                    }
-                    message
-                        .dkim_headers
-                        .push(Header::new(name, value, signature));
-                    name
-                }
-                AuthenticatedHeader::Aar(name) => {
-                    let results = arc::Results::parse(value);
-                    if !has_arc_errors {
-                        has_arc_errors = results.is_err();
+            let name =
+                match header {
+                    AuthenticatedHeader::Ds(name) => {
+                        let signature = match dkim::Signature::parse(value) {
+                            Ok(signature) if signature.l == 0 || !strict => {
+                                let ha = HashAlgorithm::from(signature.a);
+                                if !message.body_hashes.iter().any(|(c, h, l, _)| {
+                                    c == &signature.cb && h == &ha && l == &signature.l
+                                }) {
+                                    message.body_hashes.push((
+                                        signature.cb,
+                                        ha,
+                                        signature.l,
+                                        Vec::new(),
+                                    ));
+                                }
+                                Ok(signature)
+                            }
+                            Ok(_) => Err(crate::Error::SignatureLength),
+                            Err(err) => Err(err),
+                        };
+
+                        message
+                            .dkim_headers
+                            .push(Header::new(name, value, signature));
+                        name
                     }
-                    message.aar_headers.push(Header::new(name, value, results));
-                    name
-                }
-                AuthenticatedHeader::Ams(name) => {
-                    let signature = arc::Signature::parse(value);
-
-                    if let Ok(signature) = &signature {
-                        let ha = HashAlgorithm::from(signature.a);
-                        if !message
-                            .body_hashes
-                            .iter()
-                            .any(|(c, h, l, _)| c == &signature.cb && h == &ha && l == &signature.l)
-                        {
-                            message
-                                .body_hashes
-                                .push((signature.cb, ha, signature.l, Vec::new()));
+                    AuthenticatedHeader::Aar(name) => {
+                        let results = arc::Results::parse(value);
+                        if !has_arc_errors {
+                            has_arc_errors = results.is_err();
                         }
-                    } else {
-                        has_arc_errors = true;
+                        message.aar_headers.push(Header::new(name, value, results));
+                        name
                     }
-
-                    message
-                        .ams_headers
-                        .push(Header::new(name, value, signature));
-                    name
-                }
-                AuthenticatedHeader::As(name) => {
-                    let seal = arc::Seal::parse(value);
-                    if !has_arc_errors {
-                        has_arc_errors = seal.is_err();
+                    AuthenticatedHeader::Ams(name) => {
+                        let signature = match arc::Signature::parse(value) {
+                            Ok(signature) if signature.l == 0 || !strict => {
+                                let ha = HashAlgorithm::from(signature.a);
+                                if !message.body_hashes.iter().any(|(c, h, l, _)| {
+                                    c == &signature.cb && h == &ha && l == &signature.l
+                                }) {
+                                    message.body_hashes.push((
+                                        signature.cb,
+                                        ha,
+                                        signature.l,
+                                        Vec::new(),
+                                    ));
+                                }
+                                Ok(signature)
+                            }
+                            Ok(_) => {
+                                has_arc_errors = true;
+                                Err(crate::Error::SignatureLength)
+                            }
+                            Err(err) => {
+                                has_arc_errors = true;
+                                Err(err)
+                            }
+                        };
+
+                        message
+                            .ams_headers
+                            .push(Header::new(name, value, signature));
+                        name
                     }
-                    message.as_headers.push(Header::new(name, value, seal));
-                    name
-                }
-                AuthenticatedHeader::From(name) => {
-                    match MessageStream::new(value).parse_address() {
-                        HeaderValue::Address(Address::List(list)) => {
-                            message.from.extend(
-                                list.into_iter()
-                                    .filter_map(|a| a.address.map(|a| a.to_lowercase())),
-                            );
+                    AuthenticatedHeader::As(name) => {
+                        let seal = arc::Seal::parse(value);
+                        if !has_arc_errors {
+                            has_arc_errors = seal.is_err();
                         }
-                        HeaderValue::Address(Address::Group(group_list)) => {
-                            message
+                        message.as_headers.push(Header::new(name, value, seal));
+                        name
+                    }
+                    AuthenticatedHeader::From(name) => {
+                        match MessageStream::new(value).parse_address() {
+                            HeaderValue::Address(Address::List(list)) => {
+                                message.from.extend(
+                                    list.into_iter()
+                                        .filter_map(|a| a.address.map(|a| a.to_lowercase())),
+                                );
+                            }
+                            HeaderValue::Address(Address::Group(group_list)) => message
                                 .from
                                 .extend(group_list.into_iter().flat_map(|group| {
                                     group
                                         .addresses
                                         .into_iter()
                                         .filter_map(|a| a.address.map(|a| a.to_lowercase()))
-                                }))
+                                })),
+                            _ => (),
                         }
-                        _ => (),
-                    }
 
-                    name
-                }
-                AuthenticatedHeader::Other(name) => name,
-            };
+                        name
+                    }
+                    AuthenticatedHeader::Other(name) => name,
+                };
 
             message.headers.push((name, value));
         }

src/dkim/sign.rs

@@ -106,6 +106,7 @@ impl<'a> Writable for SignableMessage<'a> {
 #[cfg(test)]
 #[allow(unused)]
 pub mod test {
+    use core::str;
     use std::time::{Duration, Instant};
 
     use hickory_resolver::proto::op::ResponseCode;
@@ -351,12 +352,12 @@ pub mod test {
         )
         .await;
 
-        dbg!("Test RSA-SHA256 simple/relaxed with fixed body length");
+        dbg!("Test RSA-SHA256 simple/relaxed with fixed body length (relaxed)");
         #[cfg(feature = "rust-crypto")]
         let pk_rsa = RsaKey::<Sha256>::from_pkcs1_pem(RSA_PRIVATE_KEY).unwrap();
         #[cfg(all(feature = "ring", not(feature = "rust-crypto")))]
         let pk_rsa = RsaKey::<Sha256>::from_rsa_pem(RSA_PRIVATE_KEY).unwrap();
-        verify(
+        verify_with_opts(
             &resolver,
             DkimSigner::from_key(pk_rsa)
                 .domain("example.com")
@@ -368,6 +369,28 @@ pub mod test {
                 .unwrap(),
             &(message.to_string() + "\r\n----- Mailing list"),
             Ok(()),
+            false,
+        )
+        .await;
+
+        dbg!("Test RSA-SHA256 simple/relaxed with fixed body length (strict)");
+        #[cfg(feature = "rust-crypto")]
+        let pk_rsa = RsaKey::<Sha256>::from_pkcs1_pem(RSA_PRIVATE_KEY).unwrap();
+        #[cfg(all(feature = "ring", not(feature = "rust-crypto")))]
+        let pk_rsa = RsaKey::<Sha256>::from_rsa_pem(RSA_PRIVATE_KEY).unwrap();
+        verify_with_opts(
+            &resolver,
+            DkimSigner::from_key(pk_rsa)
+                .domain("example.com")
+                .selector("default")
+                .headers(["From", "To", "Subject"])
+                .header_canonicalization(Canonicalization::Simple)
+                .body_length(true)
+                .sign(message.as_bytes())
+                .unwrap(),
+            &(message.to_string() + "\r\n----- Mailing list"),
+            Err(super::Error::SignatureLength),
+            true,
         )
         .await;
 
@@ -486,17 +509,18 @@ pub mod test {
         .await;
     }
 
-    pub async fn verify<'x>(
+    pub async fn verify_with_opts<'x>(
         resolver: &Resolver,
         signature: Signature,
         message_: &'x str,
         expect: Result<(), super::Error>,
+        strict: bool,
     ) -> Vec<DkimOutput<'x>> {
         let mut message = Vec::with_capacity(message_.len() + 100);
         signature.write(&mut message, true);
         message.extend_from_slice(message_.as_bytes());
 
-        let message = AuthenticatedMessage::parse(&message).unwrap();
+        let message = AuthenticatedMessage::parse_with_opts(&message, strict).unwrap();
         let dkim = resolver.verify_dkim(&message).await;
 
         match (dkim.last().unwrap().result(), &expect) {
@@ -517,4 +541,13 @@ pub mod test {
             })
             .collect()
     }
+
+    pub async fn verify<'x>(
+        resolver: &Resolver,
+        signature: Signature,
+        message_: &'x str,
+        expect: Result<(), super::Error>,
+    ) -> Vec<DkimOutput<'x>> {
+        verify_with_opts(resolver, signature, message_, expect, true).await
+    }
 }

src/dkim/verify.rs

@@ -221,6 +221,7 @@ impl Resolver {
                             | Error::ArcInvalidCV
                             | Error::ArcHasHeaderTag
                             | Error::ArcBrokenChain
+                            | Error::SignatureLength
                             | Error::NotAligned => (record.rr & RR_OTHER) != 0,
                         };
 

src/lib.rs

@@ -461,6 +461,7 @@ pub enum Error {
     RevokedPublicKey,
     IncompatibleAlgorithms,
     SignatureExpired,
+    SignatureLength,
     DnsError(String),
     DnsRecordNotFound(ResponseCode),
     ArcChainTooLong,
@@ -501,6 +502,7 @@ impl Display for Error {
             ),
             Error::FailedVerification => write!(f, "Signature verification failed"),
             Error::SignatureExpired => write!(f, "Signature expired"),
+            Error::SignatureLength => write!(f, "Insecure 'l=' tag found in Signature"),
             Error::FailedAuidMatch => write!(f, "AUID does not match domain name"),
             Error::ArcInvalidInstance(i) => {
                 write!(f, "Invalid 'i={i}' value found in ARC header")