From 05e52d4ce6ff4b7c991f978268aad5a340f05f8e Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Tue, 22 Aug 2023 17:22:41 -0700 Subject: [PATCH] message: switch ed25519 dkim signing to cfdkim With the upgraded ed25519-dalek crate, it's now possible to pass in either DER or PEM encoded PKCS8 signing keys, which makes it feasible to remove the mail-auth dep from this crate. That in turns reduces the amount of code in here, which is nice. --- Cargo.lock | 1 - crates/dkim/Cargo.toml | 2 +- crates/dkim/src/lib.rs | 24 +++++- crates/message/Cargo.toml | 1 - crates/message/src/dkim.rs | 93 +++------------------- docs/changelog/main.md | 2 + docs/reference/kumo.dkim/ed25519_signer.md | 4 + 7 files changed, 39 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 13573d30e..df75db321 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2962,7 +2962,6 @@ dependencies = [ "kumo-log-types", "lazy_static", "lruttl", - "mail-auth", "mail-parser", "mailparse", "mlua", diff --git a/crates/dkim/Cargo.toml b/crates/dkim/Cargo.toml index 4d80c38ba..a1667cf10 100644 --- a/crates/dkim/Cargo.toml +++ b/crates/dkim/Cargo.toml @@ -18,7 +18,7 @@ default = ["openssl"] [dependencies] base64 = "0.21.0" chrono = { version = "0.4.26", default-features = false, features = ["clock", "std"] } -ed25519-dalek = {workspace=true} +ed25519-dalek = {workspace=true, features=["pkcs8"]} futures = "0.3.28" indexmap = "1.9.3" mailparse = "0.14" diff --git a/crates/dkim/src/lib.rs b/crates/dkim/src/lib.rs index 7055dc681..46e44a9f8 100644 --- a/crates/dkim/src/lib.rs +++ b/crates/dkim/src/lib.rs @@ -3,6 +3,7 @@ use crate::hash::HeaderList; use base64::engine::general_purpose; use base64::Engine; +use ed25519_dalek::SigningKey; use rsa::pkcs1::DecodeRsaPrivateKey; use rsa::pkcs8::DecodePrivateKey; use rsa::{Pkcs1v15Sign, RsaPrivateKey, RsaPublicKey}; @@ -47,7 +48,7 @@ pub(crate) enum DkimPublicKey { #[derive(Debug)] pub enum DkimPrivateKey { Rsa(RsaPrivateKey), - Ed25519(ed25519_dalek::SigningKey), + Ed25519(SigningKey), #[cfg(feature = "openssl")] OpenSSLRsa(openssl::rsa::Rsa), } @@ -106,6 +107,27 @@ impl DkimPrivateKey { })?; Self::rsa_key(&data) } + + /// Parse PKCS8 encoded ed25519 key data into a DkimPrivateKey. + /// Both DER and PEM are supported + pub fn ed25519_key(data: &[u8]) -> Result { + let mut errors = vec![]; + + match SigningKey::from_pkcs8_der(data) { + Ok(key) => return Ok(Self::Ed25519(key)), + Err(err) => errors.push(format!("Ed25519 SigningKey::from_pkcs8_der: {err:#}")), + } + + match std::str::from_utf8(data) { + Ok(s) => match SigningKey::from_pkcs8_pem(s) { + Ok(key) => return Ok(Self::Ed25519(key)), + Err(err) => errors.push(format!("Ed25519 SigningKey::from_pkcs8_pem: {err:#}")), + }, + Err(err) => errors.push(format!("ed25519_key: data is not UTF-8: {err:#}")), + } + + Err(DKIMError::PrivateKeyLoadError(errors.join(". "))) + } } // https://datatracker.ietf.org/doc/html/rfc6376#section-6.1.3 Step 4 diff --git a/crates/message/Cargo.toml b/crates/message/Cargo.toml index c7e9cbe80..f74460a7c 100644 --- a/crates/message/Cargo.toml +++ b/crates/message/Cargo.toml @@ -18,7 +18,6 @@ futures = "0.3" kumo-log-types = {path="../kumo-log-types"} lazy_static = "1.4" lruttl = {path="../lruttl"} -mail-auth = "0.3" mailparse = "0.14" mail-parser = "0.8" mlua = {workspace=true, features=["vendored", "lua54", "async", "send", "serialize"]} diff --git a/crates/message/src/dkim.rs b/crates/message/src/dkim.rs index 1efc4ccef..29ecfbd8f 100644 --- a/crates/message/src/dkim.rs +++ b/crates/message/src/dkim.rs @@ -3,9 +3,6 @@ use cfdkim::DkimPrivateKey; use config::{from_lua_value, get_or_create_sub_module}; use data_loader::KeySource; use lruttl::LruCacheWithTtl; -use mail_auth::common::crypto::{Ed25519Key, HashAlgorithm, RsaKey, Sha256, SigningKey}; -use mail_auth::common::headers::HeaderWriter; -use mail_auth::dkim::{Canonicalization, DkimSigner, Done, NeedDomain}; use mlua::prelude::LuaUserData; use mlua::{Lua, Value}; use serde::Deserialize; @@ -13,7 +10,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; lazy_static::lazy_static! { - static ref SIGNER_CACHE: LruCacheWithTtl> = LruCacheWithTtl::new(1024); + static ref SIGNER_CACHE: LruCacheWithTtl> = LruCacheWithTtl::new(1024); } #[derive(Deserialize, Hash, Eq, PartialEq, Copy, Clone)] @@ -28,30 +25,12 @@ impl Default for Canon { } } -impl Into for Canon { - fn into(self) -> Canonicalization { - match self { - Self::Relaxed => Canonicalization::Relaxed, - Self::Simple => Canonicalization::Simple, - } - } -} - #[derive(Deserialize, Hash, Eq, PartialEq, Copy, Clone)] pub enum HashAlgo { Sha1, Sha256, } -impl Into for HashAlgo { - fn into(self) -> HashAlgorithm { - match self { - Self::Sha1 => HashAlgorithm::Sha1, - Self::Sha256 => HashAlgorithm::Sha256, - } - } -} - #[derive(Deserialize, Hash, PartialEq, Eq, Clone)] pub struct SignerConfig { domain: String, @@ -85,34 +64,6 @@ impl SignerConfig { 300 } - fn configure_signer( - &self, - signer: DkimSigner, - ) -> DkimSigner { - let mut signer = signer - .domain(self.domain.clone()) - .selector(self.selector.clone()) - .headers(self.headers.clone()); - if let Some(atps) = &self.atps { - signer = signer.atps(atps.clone()); - } - if let Some(atpsh) = self.atpsh { - signer = signer.atpsh(atpsh.into()); - } - if let Some(agent_user_identifier) = &self.agent_user_identifier { - signer = signer.agent_user_identifier(agent_user_identifier); - } - if let Some(expiration) = self.expiration { - signer = signer.expiration(expiration); - } - signer = signer.body_length(self.body_length); - signer = signer.reporting(self.reporting); - signer = signer.header_canonicalization(self.header_canonicalization.into()); - signer = signer.body_canonicalization(self.body_canonicalization.into()); - - signer - } - fn configure_cfdkim(&self, key: DkimPrivateKey) -> anyhow::Result { if self.atps.is_some() { anyhow::bail!("atps is not currently supported for RSA keys"); @@ -152,14 +103,8 @@ impl SignerConfig { } } -pub enum SignerInner { - RsaSha256(DkimSigner, Done>), - Ed25519(DkimSigner), - CFDKIM(CFSigner), -} - #[derive(Clone)] -pub struct Signer(Arc); +pub struct Signer(Arc); impl Signer { pub fn sign(&self, message: &[u8]) -> anyhow::Result { @@ -167,20 +112,6 @@ impl Signer { } } -impl SignerInner { - fn sign(&self, message: &[u8]) -> anyhow::Result { - let sig = match self { - Self::RsaSha256(signer) => signer.sign(message), - Self::Ed25519(signer) => signer.sign(message), - Self::CFDKIM(signer) => return signer.sign(message), - } - .map_err(|err| anyhow::anyhow!("{err:#}"))?; - - let header = sig.to_header(); - Ok(header) - } -} - impl LuaUserData for Signer {} pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> { @@ -207,7 +138,7 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> { .configure_cfdkim(key) .map_err(|err| mlua::Error::external(format!("{err:#}")))?; - let inner = Arc::new(SignerInner::CFDKIM(CFSigner { signer })); + let inner = Arc::new(CFSigner { signer }); let expiration = Instant::now() + Duration::from_secs(params.ttl); SIGNER_CACHE.insert(params, Arc::clone(&inner), expiration); @@ -231,21 +162,15 @@ pub fn register<'lua>(lua: &'lua Lua) -> anyhow::Result<()> { .await .map_err(|err| mlua::Error::external(format!("{:?}: {err:#}", params.key)))?; - let mut errors = vec![]; + let key = DkimPrivateKey::ed25519_key(&data) + .map_err(|err| mlua::Error::external(format!("{:?}: {err}", params.key)))?; - let key = Ed25519Key::from_pkcs8_der(&data) - .or_else(|err| { - errors.push(format!("from_pkcs8_der: {err:#}")); - Ed25519Key::from_pkcs8_maybe_unchecked_der(&data) - }) - .map_err(|err| { - errors.push(format!("from_pkcs8_maybe_unchecked_der: {err:#}")); - mlua::Error::external(format!("{:?}: {}", params.key, errors.join(", "))) - })?; + let signer = params + .configure_cfdkim(key) + .map_err(|err| mlua::Error::external(format!("{err:#}")))?; - let signer = params.configure_signer(DkimSigner::from_key(key)); + let inner = Arc::new(CFSigner { signer }); - let inner = Arc::new(SignerInner::Ed25519(signer)); let expiration = Instant::now() + Duration::from_secs(params.ttl); SIGNER_CACHE.insert(params, Arc::clone(&inner), expiration); diff --git a/docs/changelog/main.md b/docs/changelog/main.md index b94a31851..2c9d59ff8 100644 --- a/docs/changelog/main.md +++ b/docs/changelog/main.md @@ -39,6 +39,8 @@ base32, base64 and hex strings. * [kumo.dns.configure_resolver](../reference/kumo.dns/configure_resolver.md) for adjusting DNS resolver configuration. +* [kumo.dkim.ed25519_signer](../reference/kumo.dkim/ed25519_signer.md) now also + supports loading signing keys that are in PEM encoded PKCS8 format. ## Fixes * Loading secrets from HashiCorp Vault failed to parse underlying json data into diff --git a/docs/reference/kumo.dkim/ed25519_signer.md b/docs/reference/kumo.dkim/ed25519_signer.md index 981d02e00..8435a1dce 100644 --- a/docs/reference/kumo.dkim/ed25519_signer.md +++ b/docs/reference/kumo.dkim/ed25519_signer.md @@ -10,6 +10,10 @@ which must contain the matching public and private key pair. If the data cannot be loaded as V2, then it will fall back to try to load V1 data, which contains just the private key. +{{since('dev', indent=True)}} + We now support loading either DER or PEM encoded PKCS8 + private keys. + ```lua -- Called once the body has been received. -- For multi-recipient mail, this is called for each recipient.