From 90d4856a1c112b1e9f066ce5aa9e787fead7d21e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Fri, 8 Aug 2025 16:27:33 +0200 Subject: [PATCH] comments/naming: Make sure that I consistently use shared_secret --- benches/benchmark_decrypting.rs | 2 +- src/chat.rs | 2 +- src/decrypt.rs | 7 ++----- src/mimefactory.rs | 11 +++++------ src/pgp.rs | 24 ++++++++++++++---------- src/securejoin/bob.rs | 1 + 6 files changed, 24 insertions(+), 23 deletions(-) diff --git a/benches/benchmark_decrypting.rs b/benches/benchmark_decrypting.rs index d36f3b2c4..8e5f5092f 100644 --- a/benches/benchmark_decrypting.rs +++ b/benches/benchmark_decrypting.rs @@ -115,7 +115,7 @@ fn criterion_benchmark(c: &mut Criterion) { .map(|_| create_broadcast_shared_secret_pub()) .collect(); - // "secret" is the symmetric secret that was used to encrypt text_symmetrically_encrypted.eml: + // "secret" is the shared secret that was used to encrypt text_symmetrically_encrypted.eml: secrets[NUM_SECRETS / 2] = "secret".to_string(); let context = rt.block_on(async { diff --git a/src/chat.rs b/src/chat.rs index d47f8e32c..395dccccd 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2947,7 +2947,7 @@ async fn prepare_send_msg( msg.param .get_bool(Param::ForcePlaintext) .unwrap_or_default() - // V2 securejoin messages are symmetrically encrypted, no need for the public key: + // V2 securejoin messages are symmetrically encrypted, no need for the public key: || msg.securejoin_step() == Some("vb-request-v2") } _ => false, diff --git a/src/decrypt.rs b/src/decrypt.rs index bf49fc6ab..060e87344 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -10,11 +10,8 @@ use crate::pgp; /// Tries to decrypt a message, but only if it is structured as an Autocrypt message. /// -/// If successful and the message is encrypted, returns a tuple of: -/// -/// - The decrypted and decompressed message -/// - If the message was symmetrically encrypted: -/// The index in `shared_secrets` of the secret used to decrypt the message. +/// If successful and the message is encrypted, +/// returns the decrypted and decompressed message. pub fn try_decrypt<'a>( mail: &'a ParsedMail<'a>, private_keyring: &'a [SignedSecretKey], diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0f75b3bd5..4ec123cb4 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1180,7 +1180,7 @@ impl MimeFactory { Loaded::Mdn { .. } => true, }; - let symmetric_key: Option = match &self.loaded { + let shared_secret: Option = match &self.loaded { Loaded::Message { msg, .. } if should_encrypt_with_auth_token(msg) => { // TODO rather than setting Arg2, bob.rs could set a param `Param::SharedSecretForEncryption` or similar msg.param.get(Param::Arg2).map(|s| s.to_string()) @@ -1188,7 +1188,7 @@ impl MimeFactory { Loaded::Message { chat, msg } if should_encrypt_with_broadcast_secret(msg, chat) => { - // If there is no symmetric key yet + // If there is no shared secret yet // (because this is an old broadcast channel, // created before we had symmetric encryption), // we just encrypt asymmetrically. @@ -1200,11 +1200,10 @@ impl MimeFactory { _ => None, }; - let encrypted = if let Some(symmetric_key) = symmetric_key { - info!(context, "Symmetrically encrypting for broadcast channel."); - info!(context, "secret: {symmetric_key}"); // TODO + let encrypted = if let Some(shared_secret) = shared_secret { + info!(context, "Encrypting symmetrically."); encrypt_helper - .encrypt_for_broadcast(context, &symmetric_key, message, compress) + .encrypt_for_broadcast(context, &shared_secret, message, compress) .await? } else { // Asymmetric encryption diff --git a/src/pgp.rs b/src/pgp.rs index 1b3031a54..f1be2117d 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -238,10 +238,7 @@ pub fn pk_calc_signature( /// shared secrets used for symmetric encryption /// are passed in `shared_secrets`. /// -/// Returns a tuple of: -/// - The decrypted and decompressed message -/// - If the message was symmetrically encrypted: -/// The index in `shared_secrets` of the secret used to decrypt the message. +/// Returns the decrypted and decompressed message. pub fn decrypt( ctext: Vec, private_keys_for_decryption: &[SignedSecretKey], @@ -253,7 +250,13 @@ pub fn decrypt( let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.iter().collect(); let empty_pw = Password::empty(); - // TODO it may degrade performance that we always try out all passwords here + // We always try out all passwords here, which is not great for performance. + // But benchmarking (see `benchmark_decrypting.rs`) + // showed that the performance penalty is acceptable. + // We could include a short (~2 character) identifier of the secret + // in + // (or just include the first 2 characters of the secret in clear-text) + // in order to let message_password: Vec = shared_secrets .iter() .map(|p| Password::from(p.as_str())) @@ -322,7 +325,7 @@ pub async fn symm_encrypt(passphrase: &str, plain: Vec) -> Result { tokio::task::spawn_blocking(move || { let mut rng = thread_rng(); let s2k = StringToKey::new_default(&mut rng); - let builder: MessageBuilder<'_> = MessageBuilder::from_bytes("", plain); + let builder = MessageBuilder::from_bytes("", plain); let mut builder = builder.seipd_v1(&mut rng, SYMMETRIC_KEY_ALGORITHM); builder.encrypt_with_password(s2k, &passphrase)?; @@ -333,14 +336,15 @@ pub async fn symm_encrypt(passphrase: &str, plain: Vec) -> Result { .await? } -/// Symmetric encryption. +/// Symmetrically encrypt the message to be sent into a broadcast channel. +/// `shared secret` is the secret that will be used for symmetric encryption. pub async fn encrypt_for_broadcast( plain: Vec, - passphrase: &str, + shared_secret: &str, private_key_for_signing: SignedSecretKey, compress: bool, ) -> Result { - let passphrase = Password::from(passphrase.to_string()); + let shared_secret = Password::from(shared_secret.to_string()); tokio::task::spawn_blocking(move || { let msg = MessageBuilder::from_bytes("", plain); @@ -357,7 +361,7 @@ pub async fn encrypt_for_broadcast( AeadAlgorithm::Ocb, ChunkSize::C8KiB, ); - msg.encrypt_with_password(&mut rng, s2k, &passphrase)?; + msg.encrypt_with_password(&mut rng, s2k, &shared_secret)?; msg.sign(&*private_key_for_signing, Password::empty(), HASH_ALGORITHM); if compress { diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 0c11e54a1..069d8da94 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -71,6 +71,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul let mut msg = Message { viewtype: Viewtype::Text, + // TODO I may want to make this generic also for group/contacts text: "Secure-Join: vb-request-v2".to_string(), hidden: true, ..Default::default()