From d97b403bd88050c2b2987622c93d0c6b2391e980 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Thu, 8 Jun 2023 16:48:54 -0700 Subject: [PATCH] remove lifetime from the Signer struct It is rather difficult to reuse the Signer in a dynamic application with the lifetime present in the struct. This commit removes the lifetime to make it easier to cache and reuse. The logger has also been removed from the API for the signer. It was used only in one place to log a single debug line, and it is a bit of an imposition on the embedding application to require the use of the `slog` crate just for that, and its removal helps with the goal of removing lifetimes from the struct as well. Some builder methods have been made a little more ergonomic by accepting values that are convertible to String rather than requiring str references. Since this commit changes the API signature, it also bumps the version from 0.2 to 0.3. --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 3 +- src/hash.rs | 14 ++++---- src/header.rs | 9 +++-- src/lib.rs | 2 +- src/roundtrip_test.rs | 4 +-- src/sign.rs | 79 +++++++++++++++++++------------------------ 8 files changed, 53 insertions(+), 62 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc133ed..f7af0d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -105,7 +105,7 @@ checksum = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f" [[package]] name = "cfdkim" -version = "0.2.6" +version = "0.3.0" dependencies = [ "base64 0.21.0", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 48e119e..146f512 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cfdkim" -version = "0.2.6" +version = "0.3.0" authors = ["Sven Sauleau "] edition = "2021" description = "DKIM (RFC6376) implementation" diff --git a/README.md b/README.md index 8ddd3f2..04add35 100644 --- a/README.md +++ b/README.md @@ -30,10 +30,9 @@ let private_key = rsa::RsaPrivateKey::read_pkcs1_pem_file(Path::new("./test/keys/2022.private"))?; let signer = SignerBuilder::new() - .with_signed_headers(&["From", "Subject"])? + .with_signed_headers(["From", "Subject"])? .with_private_key(private_key) .with_selector("2020") - .with_logger(&logger) .with_signing_domain("example.com") .build()?; let signature = signer.sign(&email)?; diff --git a/src/hash.rs b/src/hash.rs index 032a9b5..205548a 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -104,7 +104,7 @@ fn select_headers<'a>( } pub(crate) fn compute_headers_hash<'a, 'b>( - logger: &slog::Logger, + logger: Option<&slog::Logger>, canonicalization_type: canonicalization::Type, headers: &'b str, hash_algo: HashAlgo, @@ -139,7 +139,9 @@ pub(crate) fn compute_headers_hash<'a, 'b>( input.extend_from_slice(&canonicalized_value); } - debug!(logger, "headers to hash: {:?}", input); + if let Some(logger) = logger { + debug!(logger, "headers to hash: {:?}", input); + } let hash = match hash_algo { HashAlgo::RsaSha1 => hash_sha1(&input), @@ -323,7 +325,7 @@ Hello Alice let logger = slog::Logger::root(slog::Discard, slog::o!()); assert_eq!( compute_headers_hash( - &logger, + Some(&logger), canonicalization_type.clone(), &headers, hash_algo, @@ -339,7 +341,7 @@ Hello Alice let hash_algo = HashAlgo::RsaSha256; assert_eq!( compute_headers_hash( - &logger, + Some(&logger), canonicalization_type, &headers, hash_algo, @@ -373,7 +375,7 @@ Hello Alice let logger = slog::Logger::root(slog::Discard, slog::o!()); assert_eq!( compute_headers_hash( - &logger, + Some(&logger), canonicalization_type.clone(), &headers, hash_algo, @@ -389,7 +391,7 @@ Hello Alice let hash_algo = HashAlgo::RsaSha256; assert_eq!( compute_headers_hash( - &logger, + Some(&logger), canonicalization_type, &headers, hash_algo, diff --git a/src/header.rs b/src/header.rs index 6ea64de..597fd93 100644 --- a/src/header.rs +++ b/src/header.rs @@ -67,8 +67,7 @@ impl DKIMHeaderBuilder { self } - pub(crate) fn set_signed_headers(self, headers: &[&str]) -> Self { - let headers: Vec = headers.iter().map(|h| h.to_lowercase()).collect(); + pub(crate) fn set_signed_headers(self, headers: &Vec) -> Self { let value = headers.join(":"); self.add_tag("h", &value) } @@ -106,11 +105,15 @@ mod tests { assert_eq!(header.raw_bytes, "v=1; a=something;".to_owned()); } + fn signed_header_list(headers: &[&str]) -> Vec { + headers.into_iter().map(|h| h.to_lowercase()).collect() + } + #[test] fn test_dkim_header_builder_signed_headers() { let header = DKIMHeaderBuilder::new() .add_tag("v", "2") - .set_signed_headers(&["header1", "header2", "header3"]) + .set_signed_headers(&signed_header_list(&["header1", "header2", "header3"])) .build() .unwrap(); assert_eq!( diff --git a/src/lib.rs b/src/lib.rs index 4f23be3..3a09c85 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -186,7 +186,7 @@ async fn verify_email_header<'a>( email, )?; let computed_headers_hash = hash::compute_headers_hash( - logger, + Some(logger), header_canonicalization_type.clone(), &dkim_header.get_required_tag("h"), hash_algo.clone(), diff --git a/src/roundtrip_test.rs b/src/roundtrip_test.rs index 4cb11d6..f0e2fe1 100644 --- a/src/roundtrip_test.rs +++ b/src/roundtrip_test.rs @@ -31,15 +31,13 @@ mod tests { let private_key = rsa::RsaPrivateKey::read_pkcs1_pem_file(Path::new("./test/keys/2022.private")).unwrap(); - let logger = test_logger(); let time = chrono::Utc.with_ymd_and_hms(2021, 1, 1, 0, 0, 1).unwrap(); let signer = SignerBuilder::new() - .with_signed_headers(&["From", "Subject"]) + .with_signed_headers(["From", "Subject"]) .unwrap() .with_private_key(DkimPrivateKey::Rsa(private_key)) .with_selector("2022") - .with_logger(&logger) .with_signing_domain(domain) .with_time(time) .build() diff --git a/src/sign.rs b/src/sign.rs index 58111f8..db12685 100644 --- a/src/sign.rs +++ b/src/sign.rs @@ -9,26 +9,24 @@ use crate::header::DKIMHeaderBuilder; use crate::{canonicalization, hash, DKIMError, DkimPrivateKey, HEADER}; /// Builder for the Signer -pub struct SignerBuilder<'a> { - signed_headers: Option<&'a [&'a str]>, +pub struct SignerBuilder { + signed_headers: Option>, private_key: Option, - selector: Option<&'a str>, - signing_domain: Option<&'a str>, + selector: Option, + signing_domain: Option, time: Option>, header_canonicalization: canonicalization::Type, body_canonicalization: canonicalization::Type, - logger: Option<&'a slog::Logger>, expiry: Option, } -impl<'a> SignerBuilder<'a> { +impl SignerBuilder { /// New builder pub fn new() -> Self { Self { signed_headers: None, private_key: None, selector: None, - logger: None, signing_domain: None, expiry: None, time: None, @@ -40,9 +38,16 @@ impl<'a> SignerBuilder<'a> { /// Specify headers to be used in the DKIM signature /// The From: header is required. - pub fn with_signed_headers(mut self, headers: &'a [&'a str]) -> Result { - let from = headers.iter().find(|h| h.to_lowercase() == "from"); - if from.is_none() { + pub fn with_signed_headers( + mut self, + headers: impl IntoIterator>, + ) -> Result { + let headers: Vec = headers + .into_iter() + .map(|h| h.into().to_lowercase()) + .collect(); + + if !headers.iter().any(|h| h.eq_ignore_ascii_case("from")) { return Err(DKIMError::BuilderError("missing From in signed headers")); } @@ -57,14 +62,14 @@ impl<'a> SignerBuilder<'a> { } /// Specify the private key used to sign the email - pub fn with_selector(mut self, value: &'a str) -> Self { - self.selector = Some(value); + pub fn with_selector(mut self, value: impl Into) -> Self { + self.selector = Some(value.into()); self } /// Specify for which domain the email should be signed for - pub fn with_signing_domain(mut self, value: &'a str) -> Self { - self.signing_domain = Some(value); + pub fn with_signing_domain(mut self, value: impl Into) -> Self { + self.signing_domain = Some(value.into()); self } @@ -80,12 +85,6 @@ impl<'a> SignerBuilder<'a> { self } - /// Specify a logger - pub fn with_logger(mut self, logger: &'a slog::Logger) -> Self { - self.logger = Some(logger); - self - } - /// Specify current time. Mostly used for testing pub fn with_time(mut self, value: chrono::DateTime) -> Self { self.time = Some(value); @@ -99,9 +98,9 @@ impl<'a> SignerBuilder<'a> { } /// Build an instance of the Signer - /// Must be provided: signed_headers, private_key, selector, logger and + /// Must be provided: signed_headers, private_key, selector and /// signing_domain. - pub fn build(self) -> Result, DKIMError> { + pub fn build(self) -> Result { use DKIMError::BuilderError; let private_key = self @@ -120,10 +119,9 @@ impl<'a> SignerBuilder<'a> { selector: self .selector .ok_or(BuilderError("missing required selector"))?, - logger: self.logger.ok_or(BuilderError("missing required logger"))?, signing_domain: self .signing_domain - .ok_or(BuilderError("missing required logger"))?, + .ok_or(BuilderError("missing required signing domain"))?, header_canonicalization: self.header_canonicalization, body_canonicalization: self.body_canonicalization, expiry: self.expiry, @@ -133,27 +131,26 @@ impl<'a> SignerBuilder<'a> { } } -impl<'a> Default for SignerBuilder<'a> { +impl Default for SignerBuilder { fn default() -> Self { Self::new() } } -pub struct Signer<'a> { - signed_headers: &'a [&'a str], +pub struct Signer { + signed_headers: Vec, private_key: DkimPrivateKey, - selector: &'a str, - signing_domain: &'a str, + selector: String, + signing_domain: String, header_canonicalization: canonicalization::Type, body_canonicalization: canonicalization::Type, - logger: &'a slog::Logger, expiry: Option, hash_algo: hash::HashAlgo, time: Option>, } /// DKIM signer. Use the [SignerBuilder] to build an instance. -impl<'a> Signer<'a> { +impl Signer { /// Sign a message /// As specified in pub fn sign<'b>(&self, email: &'b mailparse::ParsedMail<'b>) -> Result { @@ -203,8 +200,8 @@ impl<'a> Signer<'a> { let mut builder = DKIMHeaderBuilder::new() .add_tag("v", "1") .add_tag("a", hash_algo) - .add_tag("d", self.signing_domain) - .add_tag("s", self.selector) + .add_tag("d", &self.signing_domain) + .add_tag("s", &self.selector) .add_tag( "c", &format!( @@ -214,7 +211,7 @@ impl<'a> Signer<'a> { ), ) .add_tag("bh", body_hash) - .set_signed_headers(self.signed_headers); + .set_signed_headers(&self.signed_headers); if let Some(expiry) = self.expiry { builder = builder.set_expiry(expiry)?; } @@ -248,7 +245,7 @@ impl<'a> Signer<'a> { let signed_headers = dkim_header.get_required_tag("h"); hash::compute_headers_hash( - self.logger, + None, canonicalization, &signed_headers, self.hash_algo.clone(), @@ -265,10 +262,6 @@ mod tests { use rsa::pkcs1::DecodeRsaPrivateKey; use std::{fs, path::Path}; - fn test_logger() -> slog::Logger { - slog::Logger::root(slog::Discard, slog::o!()) - } - #[test] fn test_sign_rsa() { let email = mailparse::parse_mail( @@ -283,15 +276,13 @@ Hello Alice let private_key = rsa::RsaPrivateKey::read_pkcs1_pem_file(Path::new("./test/keys/2022.private")).unwrap(); - let logger = test_logger(); let time = chrono::Utc.with_ymd_and_hms(2021, 1, 1, 0, 0, 1).unwrap(); let signer = SignerBuilder::new() - .with_signed_headers(&["From", "Subject"]) + .with_signed_headers(["From", "Subject"]) .unwrap() .with_private_key(DkimPrivateKey::Rsa(private_key)) .with_selector("s20") - .with_logger(&logger) .with_signing_domain("example.com") .with_time(time) .build() @@ -330,13 +321,12 @@ Joe."# secret: secret_key, }; - let logger = test_logger(); let time = chrono::Utc .with_ymd_and_hms(2018, 6, 10, 13, 38, 29) .unwrap(); let signer = SignerBuilder::new() - .with_signed_headers(&[ + .with_signed_headers([ "From", "To", "Subject", @@ -351,7 +341,6 @@ Joe."# .with_body_canonicalization(canonicalization::Type::Relaxed) .with_header_canonicalization(canonicalization::Type::Relaxed) .with_selector("brisbane") - .with_logger(&logger) .with_signing_domain("football.example.com") .with_time(time) .build()