perf: Only load shared secrets if the message actually is symm-encrypted

This improves performance, though not a lot - 8% performance improvement
if there is a huge number of shared secrets.

NUM_BROADCAST_SECRETS=500
NUM_AUTH_TOKENS=5000
Decrypt/Decrypt and parse a symmetrically encrypted message
                        time:   [12.354 ms 12.364 ms 12.376 ms]
                        change: [−0.5375% −0.3815% −0.2345%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe
Decrypt/Decrypt and parse a public-key encrypted message
                        time:   [12.269 ms 12.273 ms 12.278 ms]
                        change: [−7.9163% −7.8439% −7.7724%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  8 (8.00%) high mild
  5 (5.00%) high severe

Author: Hocuri

src/decrypt.rs

@@ -1,34 +1,28 @@
-//! End-to-end decryption support.
+//! Helper functions for decryption.
+//! The actual decryption is done in the [`crate::pgp`] module.
 
 use std::collections::HashSet;
+use std::io::Cursor;
 
+use ::pgp::composed::Message;
 use anyhow::Result;
 use mailparse::ParsedMail;
 
-use crate::key::{Fingerprint, SignedPublicKey, SignedSecretKey};
+use crate::key::{Fingerprint, SignedPublicKey};
 use crate::pgp;
 
-/// Tries to decrypt a message, but only if it is structured as an Autocrypt message.
-///
-/// If successful and the message was encrypted,
-/// returns the decrypted and decompressed message.
-pub fn try_decrypt<'a>(
-    mail: &'a ParsedMail<'a>,
-    private_keyring: &'a [SignedSecretKey],
-    shared_secrets: &[String],
-) -> Result<Option<::pgp::composed::Message<'static>>> {
+pub fn get_encrypted_pgp_message<'a>(mail: &'a ParsedMail<'a>) -> Result<Option<Message<'static>>> {
     let Some(encrypted_data_part) = get_encrypted_mime(mail) else {
         return Ok(None);
     };
-
     let data = encrypted_data_part.get_body_raw()?;
-    let msg = pgp::decrypt(data, private_keyring, shared_secrets)?;
-
+    let cursor = Cursor::new(data);
+    let (msg, _headers) = Message::from_armor(cursor)?;
     Ok(Some(msg))
 }
 
 /// Returns a reference to the encrypted payload of a message.
-pub(crate) fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> {
+pub fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> {
     get_autocrypt_mime(mail)
         .or_else(|| get_mixed_up_mime(mail))
         .or_else(|| get_attachment_mime(mail))

src/internals_for_benches.rs

@@ -35,4 +35,4 @@ pub fn create_dummy_keypair(addr: &str) -> Result<KeyPair> {
 
 pub fn create_broadcast_secret() -> String {
     crate::tools::create_broadcast_secret()
-}
+}

src/mimeparser.rs

@@ -20,7 +20,7 @@ use crate::chat::ChatId;
 use crate::config::Config;
 use crate::contact::ContactId;
 use crate::context::Context;
-use crate::decrypt::{try_decrypt, validate_detached_signature};
+use crate::decrypt::{get_encrypted_pgp_message, validate_detached_signature};
 use crate::dehtml::dehtml;
 use crate::download::PostMsgMetadata;
 use crate::events::EventType;
@@ -380,82 +380,93 @@ impl MimeMessage {
             PreMessageMode::None
         };
 
-        let mail_raw; // Memory location for a possible decrypted message.
-        let decrypted_msg; // Decrypted signed OpenPGP message.
-
         // TODO performance:
         // - maybe we should start sorting by timestamp
-        // - we shouldn't do 3 SQL requests even if the message isn't symm-encrypted
         // - we're loading the whole bobstate, just to get the auth token
-        let mut secrets: Vec<String> = context
-            .sql
-            .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| {
-                let secret: String = row.get(0)?;
-                Ok(secret)
-            })
-            .await?;
-        secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?);
-        context
-            .sql
-            .query_map(
-                "SELECT id, invite FROM bobstate",
-                (),
-                |row| {
-                    let invite: crate::securejoin::QrInvite = row.get(1)?;
-                    Ok(invite)
-                },
-                |rows| {
-                    for row in rows {
-                        secrets.push(row?.authcode().to_string());
-                    }
-                    Ok(())
-                },
-            )
-            .await?;
 
-        let (mail, is_encrypted) =
-            match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) {
-                Ok(Some(mut msg)) => {
-                    mail_raw = msg.as_data_vec().unwrap_or_default();
+        let encrypted_pgp_message = get_encrypted_pgp_message(&mail)?;
 
-                    let decrypted_mail = mailparse::parse_mail(&mail_raw)?;
-                    if std::env::var(crate::DCC_MIME_DEBUG).is_ok() {
-                        info!(
-                            context,
-                            "decrypted message mime-body:\n{}",
-                            String::from_utf8_lossy(&mail_raw),
-                        );
-                    }
+        let mut secrets: Vec<String>;
+        if let Some(e) = &encrypted_pgp_message
+            && crate::pgp::check_symmetric_encryption(e).is_ok()
+        {
+            secrets = context
+                .sql
+                .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| {
+                    let secret: String = row.get(0)?;
+                    Ok(secret)
+                })
+                .await?;
+            secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?);
+            context
+                .sql
+                .query_map(
+                    "SELECT id, invite FROM bobstate",
+                    (),
+                    |row| {
+                        let invite: crate::securejoin::QrInvite = row.get(1)?;
+                        Ok(invite)
+                    },
+                    |rows| {
+                        for row in rows {
+                            secrets.push(row?.authcode().to_string());
+                        }
+                        Ok(())
+                    },
+                )
+                .await?;
+        } else {
+            // No need to load all the secrets if the message isn't symmetrically encrypted
+            secrets = vec![];
+        }
 
-                    decrypted_msg = Some(msg);
+        let mail_raw; // Memory location for a possible decrypted message.
+        let decrypted_msg; // Decrypted signed OpenPGP message.
 
-                    timestamp_sent = Self::get_timestamp_sent(
-                        &decrypted_mail.headers,
-                        timestamp_sent,
-                        timestamp_rcvd,
+        let (mail, is_encrypted) = match tokio::task::block_in_place(|| {
+            encrypted_pgp_message.map(|e| crate::pgp::decrypt(e, &private_keyring, &secrets))
+        }) {
+            Some(Ok(mut msg)) => {
+                mail_raw = msg.as_data_vec().unwrap_or_default();
+
+                let decrypted_mail = mailparse::parse_mail(&mail_raw)?;
+                if std::env::var(crate::DCC_MIME_DEBUG).is_ok() {
+                    info!(
+                        context,
+                        "decrypted message mime-body:\n{}",
+                        String::from_utf8_lossy(&mail_raw),
                     );
+                }
 
-                    let protected_aheader_values = decrypted_mail
-                        .headers
-                        .get_all_values(HeaderDef::Autocrypt.into());
-                    if !protected_aheader_values.is_empty() {
-                        aheader_values = protected_aheader_values;
-                    }
+                decrypted_msg = Some(msg);
 
-                    (Ok(decrypted_mail), true)
-                }
-                Ok(None) => {
-                    mail_raw = Vec::new();
-                    decrypted_msg = None;
-                    (Ok(mail), false)
-                }
-                Err(err) => {
-                    mail_raw = Vec::new();
-                    decrypted_msg = None;
-                    warn!(context, "decryption failed: {:#}", err);
-                    (Err(err), false)
+                timestamp_sent = Self::get_timestamp_sent(
+                    &decrypted_mail.headers,
+                    timestamp_sent,
+                    timestamp_rcvd,
+                );
+
+                let protected_aheader_values = decrypted_mail
+                    .headers
+                    .get_all_values(HeaderDef::Autocrypt.into());
+                if !protected_aheader_values.is_empty() {
+                    aheader_values = protected_aheader_values;
                 }
-            };
+
+                (Ok(decrypted_mail), true)
+            }
+            None => {
+                mail_raw = Vec::new();
+                decrypted_msg = None;
+                (Ok(mail), false)
+            }
+            Some(Err(err)) => {
+                mail_raw = Vec::new();
+                decrypted_msg = None;
+                warn!(context, "decryption failed: {:#}", err);
+                (Err(err), false)
+            }
+        };
 
         let mut autocrypt_header = None;
         if incoming {

src/pgp.rs

@@ -327,13 +327,10 @@ pub fn pk_calc_signature(
 ///
 /// Returns the decrypted and decompressed message.
 pub fn decrypt(
-    ctext: Vec<u8>,
+    msg: Message<'static>,
     private_keys_for_decryption: &[SignedSecretKey],
     mut shared_secrets: &[String],
 ) -> Result<pgp::composed::Message<'static>> {
-    let cursor = Cursor::new(ctext);
-    let (msg, _headers) = Message::from_armor(cursor)?;
-
     let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.iter().collect();
     let empty_pw = Password::empty();
 
@@ -389,7 +386,9 @@ pub fn decrypt(
 /// with all of the known shared secrets.
 /// In order to prevent this, we do not try to symmetrically decrypt messages
 /// that use a string2key algorithm other than 'Salted'.
-fn check_symmetric_encryption(msg: &Message<'_>) -> std::result::Result<(), &'static str> {
+pub(crate) fn check_symmetric_encryption(
+    msg: &Message<'_>,
+) -> std::result::Result<(), &'static str> {
     let Message::Encrypted { esk, .. } = msg else {
         return Err("not encrypted");
     };
@@ -542,6 +541,7 @@ mod tests {
 
     use super::*;
     use crate::{
+        decrypt::get_encrypted_pgp_message,
         key::{load_self_public_key, load_self_secret_key},
         test_utils::{TestContextManager, alice_keypair, bob_keypair},
     };
@@ -558,7 +558,11 @@ mod tests {
         HashMap<Fingerprint, Vec<Fingerprint>>,
         Vec<u8>,
     )> {
-        let mut msg = decrypt(ctext.to_vec(), private_keys_for_decryption, &[])?;
+        let mut msg = decrypt(
+            get_encrypted_pgp_message(ctext.to_vec()),
+            private_keys_for_decryption,
+            &[],
+        )?;
         let content = msg.as_data_vec()?;
         let ret_signature_fingerprints =
             valid_signature_fingerprints(&msg, public_keys_for_validation);
@@ -747,7 +751,7 @@ mod tests {
 
         let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?;
         let mut decrypted = decrypt(
-            ctext.into(),
+            get_encrypted_pgp_message(ctext.into()),
             &bob_private_keyring,
             &[shared_secret.to_string()],
         )?;
@@ -790,7 +794,7 @@ mod tests {
 
         let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?;
         let error = decrypt(
-            ctext.into(),
+            get_encrypted_pgp_message(ctext.into()),
             &bob_private_keyring,
             &[shared_secret.to_string()],
         )
@@ -826,7 +830,12 @@ mod tests {
 
         // Trying to decrypt it should fail with an OK error message:
         let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?;
-        let error = decrypt(ctext.into(), &bob_private_keyring, &[]).unwrap_err();
+        let error = decrypt(
+            get_encrypted_pgp_message(ctext.into()),
+            &bob_private_keyring,
+            &[],
+        )
+        .unwrap_err();
 
         assert_eq!(
             error.to_string(),