From 08b2374fcfd6b847f7a5a9a22c654ba66a286125 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Fri, 21 Aug 2026 06:49:03 +0200 Subject: [PATCH] feat: allow to not sign asymmetrically encrypted multi-recipient messages An unsigned message carries no intended recipient fingerprints, so recipients of an encrypted unsigned message learn nothing about other recipients from the PGP packets. --- src/mimefactory.rs | 10 +- .../shared_secret_decryption_tests.rs | 2 +- src/pgp.rs | 107 +++++++++++------- src/pgp/pgp_tests.rs | 30 ++++- src/reaction.rs | 2 +- src/test_utils.rs | 4 +- 6 files changed, 98 insertions(+), 57 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0880af4f6..0c5807fc3 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -407,6 +407,8 @@ pub(crate) fn render_queued_mail( } } + let sign_key = if should_sign { Some(secret_key) } else { None }; + let message = match encryption { Encryption::No => raw_message, Encryption::Asymmetric { encryption_pubkeys } => { @@ -434,7 +436,7 @@ pub(crate) fn render_queued_mail( let encrypted = crate::pgp::pk_encrypt( full_raw_message, encryption_keyring, - secret_key.clone(), + sign_key, should_compress, seipd_version, )?; @@ -446,12 +448,6 @@ pub(crate) fn render_queued_mail( let mut full_raw_message = inner_headers.clone(); full_raw_message.extend(raw_message); - let sign_key = if should_sign { - Some(secret_key.clone()) - } else { - None - }; - let encrypted = crate::pgp::symm_encrypt_message( full_raw_message, sign_key, diff --git a/src/mimeparser/shared_secret_decryption_tests.rs b/src/mimeparser/shared_secret_decryption_tests.rs index beec12a1d..d0cfd8ff3 100644 --- a/src/mimeparser/shared_secret_decryption_tests.rs +++ b/src/mimeparser/shared_secret_decryption_tests.rs @@ -46,7 +46,7 @@ async fn test_shared_secret_decryption_ext( let encrypted_msg = pgp::symm_encrypt_message( plain_text.as_bytes().to_vec(), - signer_key, + signer_key.as_ref(), secret_for_encryption.to_string(), true, )?; diff --git a/src/pgp.rs b/src/pgp.rs index 466d81a6d..0e3f68c38 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -106,13 +106,46 @@ pub enum SeipdVersion { V2, } -/// Encrypts `plain` text using `public_keys_for_encryption` -/// and signs it using `private_key_for_signing`. +/// Returns the subpackets for a signature over a message +/// encrypted to `public_keys_for_encryption`. #[expect(clippy::arithmetic_side_effects)] +fn signature_subpackets( + private_key_for_signing: &SignedSecretKey, + public_keys_for_encryption: &[SignedPublicKey], +) -> Result { + let mut hashed = Vec::with_capacity(1 + public_keys_for_encryption.len() + 1); + hashed.push(Subpacket::critical(SubpacketData::SignatureCreationTime( + pgp::types::Timestamp::now(), + ))?); + for key in public_keys_for_encryption { + let data = SubpacketData::IntendedRecipientFingerprint(key.fingerprint()); + let subpkt = match private_key_for_signing.version() < KeyVersion::V6 { + true => Subpacket::regular(data)?, + false => Subpacket::critical(data)?, + }; + hashed.push(subpkt); + } + hashed.push(Subpacket::regular(SubpacketData::IssuerFingerprint( + private_key_for_signing.fingerprint(), + ))?); + let mut unhashed = vec![]; + if private_key_for_signing.version() <= KeyVersion::V4 { + unhashed.push(Subpacket::regular(SubpacketData::IssuerKeyId( + private_key_for_signing.legacy_key_id(), + ))?); + } + Ok(SubpacketConfig::UserDefined { hashed, unhashed }) +} + +/// Encrypts `plain` text using `public_keys_for_encryption`, +/// signing it with `private_key_for_signing` if there is one. +/// +/// An unsigned message carries no intended recipient fingerprints, +/// so its recipients do not learn who else received it. pub fn pk_encrypt( plain: Vec, public_keys_for_encryption: Vec, - private_key_for_signing: SignedSecretKey, + private_key_for_signing: Option<&SignedSecretKey>, compress: bool, seipd_version: SeipdVersion, ) -> Result { @@ -122,30 +155,6 @@ pub fn pk_encrypt( let pkeys = public_keys_for_encryption .iter() .filter_map(select_pk_for_encryption); - let subpkts = { - let mut hashed = Vec::with_capacity(1 + public_keys_for_encryption.len() + 1); - hashed.push(Subpacket::critical(SubpacketData::SignatureCreationTime( - pgp::types::Timestamp::now(), - ))?); - for key in &public_keys_for_encryption { - let data = SubpacketData::IntendedRecipientFingerprint(key.fingerprint()); - let subpkt = match private_key_for_signing.version() < KeyVersion::V6 { - true => Subpacket::regular(data)?, - false => Subpacket::critical(data)?, - }; - hashed.push(subpkt); - } - hashed.push(Subpacket::regular(SubpacketData::IssuerFingerprint( - private_key_for_signing.fingerprint(), - ))?); - let mut unhashed = vec![]; - if private_key_for_signing.version() <= KeyVersion::V4 { - unhashed.push(Subpacket::regular(SubpacketData::IssuerKeyId( - private_key_for_signing.legacy_key_id(), - ))?); - } - SubpacketConfig::UserDefined { hashed, unhashed } - }; let msg = MessageBuilder::from_bytes("", plain); let encoded_msg = match seipd_version { @@ -156,13 +165,16 @@ pub fn pk_encrypt( msg.encrypt_to_key_anonymous(&mut rng, &pkey)?; } - let hash_algorithm = private_key_for_signing.hash_alg(); - msg.sign_with_subpackets( - &*private_key_for_signing, - Password::empty(), - hash_algorithm, - subpkts, - ); + if let Some(secret_key) = private_key_for_signing { + let subpkts = signature_subpackets(secret_key, &public_keys_for_encryption)?; + let hash_algorithm = secret_key.hash_alg(); + msg.sign_with_subpackets( + &**secret_key, + Password::empty(), + hash_algorithm, + subpkts, + ); + } if compress { msg.compression(CompressionAlgorithm::ZLIB); } @@ -181,13 +193,16 @@ pub fn pk_encrypt( msg.encrypt_to_key_anonymous(&mut rng, &pkey)?; } - let hash_algorithm = private_key_for_signing.hash_alg(); - msg.sign_with_subpackets( - &*private_key_for_signing, - Password::empty(), - hash_algorithm, - subpkts, - ); + if let Some(secret_key) = private_key_for_signing { + let subpkts = signature_subpackets(secret_key, &public_keys_for_encryption)?; + let hash_algorithm = secret_key.hash_alg(); + msg.sign_with_subpackets( + &**secret_key, + Password::empty(), + hash_algorithm, + subpkts, + ); + } if compress { msg.compression(CompressionAlgorithm::ZLIB); } @@ -254,7 +269,7 @@ pub fn pk_validate( /// `shared secret` is the secret that will be used for symmetric encryption. pub fn symm_encrypt_message( plain: Vec, - private_key_for_signing: Option, + private_key_for_signing: Option<&SignedSecretKey>, shared_secret: String, compress: bool, ) -> Result { @@ -277,9 +292,13 @@ pub fn symm_encrypt_message( ); msg.encrypt_with_password(&mut rng, s2k, &shared_secret)?; - if let Some(private_key_for_signing) = private_key_for_signing.as_deref() { + if let Some(private_key_for_signing) = private_key_for_signing { let hash_algorithm = private_key_for_signing.hash_alg(); - msg.sign(private_key_for_signing, Password::empty(), hash_algorithm); + msg.sign( + &**private_key_for_signing, + Password::empty(), + hash_algorithm, + ); } if compress { msg.compression(CompressionAlgorithm::ZLIB); diff --git a/src/pgp/pgp_tests.rs b/src/pgp/pgp_tests.rs index 383e700a8..bd2b61767 100644 --- a/src/pgp/pgp_tests.rs +++ b/src/pgp/pgp_tests.rs @@ -101,7 +101,7 @@ async fn ctext_signed() -> &'static String { pk_encrypt( CLEARTEXT.to_vec(), keyring, - KEYS.alice_secret.clone(), + Some(&KEYS.alice_secret), compress, SeipdVersion::V2, ) @@ -120,6 +120,32 @@ async fn test_encrypt_signed() { ); } +/// Tests that a message encrypted without a signing key has no signature, +/// and therefore no intended recipient fingerprints naming the other recipients. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_encrypt_unsigned() { + let keyring = vec![KEYS.alice_public.clone(), KEYS.bob_public.clone()]; + let compress = true; + let ctext = pk_encrypt( + CLEARTEXT.to_vec(), + keyring, + None, + compress, + SeipdVersion::V2, + ) + .unwrap(); + + let decrypt_keyring = vec![KEYS.bob_secret.clone()]; + let sig_check_keyring = vec![KEYS.alice_public.clone()]; + let (msg, valid_signatures, content) = + pk_decrypt_and_validate(ctext.as_bytes(), &decrypt_keyring, &sig_check_keyring) + .await + .unwrap(); + assert_eq!(content, CLEARTEXT); + assert!(!msg.is_signed()); + assert_eq!(valid_signatures.len(), 0); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_decrypt_signed() { // Check decrypting as Alice @@ -291,7 +317,7 @@ async fn test_decryption_error_msg() -> Result<()> { let ctext = pk_encrypt( plain, vec![pk_for_encryption], - KEYS.alice_secret.clone(), + Some(&KEYS.alice_secret), compress, SeipdVersion::V2, )?; diff --git a/src/reaction.rs b/src/reaction.rs index c90c4a6c8..770d6136d 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -1241,7 +1241,7 @@ Content-Transfer-Encoding: base64\r let encrypted_payload = pk_encrypt( plain_text.as_bytes().to_vec(), public_keys_for_encryption, - alice_secret_key, + Some(&alice_secret_key), compress, SeipdVersion::V2, )?; diff --git a/src/test_utils.rs b/src/test_utils.rs index 810217fcc..a20f01934 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -1224,11 +1224,11 @@ pub async fn encrypt_raw_message( let mut cleartext = format!("Autocrypt: {aheader}").into_bytes(); cleartext.extend_from_slice(b"\r\n"); cleartext.extend_from_slice(payload); - let sign_key = key::load_self_secret_key(context).await?; + let sign_key = Some(key::load_self_secret_key(context).await?); let encrypted_payload = crate::pgp::pk_encrypt( cleartext, encryption_keyring, - sign_key, + sign_key.as_ref(), compress, SeipdVersion::V2, )?;