test_select_only_expired_subkey

Test key is generated with:
```
#!/bin/sh
set -e
export GNUPGHOME="$PWD/gnupghome"
rm -fr  "$GNUPGHOME"
mkdir -p "$GNUPGHOME"
chmod 700 "$GNUPGHOME"

GPG="gpg --batch --quiet --faked-system-time 20070924T154812"
USERID='Alice <alice@example.org>'
$GPG --passphrase '' --quick-generate-key "$USERID" ed25519 default never
FINGERPRINT="$($GPG --list-secret-keys --with-colons | awk -F: '$1 == "fpr" { print $10 }')"
$GPG --passphrase '' --quick-add-key "$FINGERPRINT" cv25519 encr 1y
$GPG --armor --export "$FINGERPRINT" > alice.tpk.asc
```

deltachat-repl/src/cmdline.rs

@@ -27,55 +27,6 @@ use deltachat::sql;
 use deltachat::tools::*;
 use tokio::fs;
 
-/// Reset database tables.
-/// Argument is a bitmask, executing single or multiple actions in one call.
-/// e.g. bitmask 7 triggers actions defined with bits 1, 2 and 4.
-async fn reset_tables(context: &Context, bits: i32) {
-    println!("Resetting tables ({bits})...");
-    if 0 != bits & 4 {
-        context
-            .sql()
-            .execute("DELETE FROM keypairs;", ())
-            .await
-            .unwrap();
-        println!("(4) Private keypairs reset.");
-    }
-    if 0 != bits & 8 {
-        context
-            .sql()
-            .execute("DELETE FROM contacts WHERE id>9;", ())
-            .await
-            .unwrap();
-        context
-            .sql()
-            .execute("DELETE FROM chats WHERE id>9;", ())
-            .await
-            .unwrap();
-        context
-            .sql()
-            .execute("DELETE FROM chats_contacts;", ())
-            .await
-            .unwrap();
-        context
-            .sql()
-            .execute("DELETE FROM msgs WHERE id>9;", ())
-            .await
-            .unwrap();
-        context
-            .sql()
-            .execute(
-                "DELETE FROM config WHERE keyname LIKE 'imap.%' OR keyname LIKE 'configured%';",
-                (),
-            )
-            .await
-            .unwrap();
-        context.sql().config_cache().write().await.clear();
-        println!("(8) Rest but server config reset.");
-    }
-
-    context.emit_msgs_changed_without_ids();
-}
-
 async fn poke_eml_file(context: &Context, filename: &Path) -> Result<()> {
     let data = read_file(context, filename).await?;
 
@@ -304,7 +255,6 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu
                  export-keys\n\
                  import-keys <key-file>\n\
                  poke [<eml-file>|<folder>|<addr> <key-file>]\n\
-                 reset <flags>\n\
                  stop\n\
                  ============================================="
             ),
@@ -444,15 +394,6 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu
         "poke" => {
             ensure!(poke_spec(&context, Some(arg1)).await, "Poke failed");
         }
-        "reset" => {
-            ensure!(
-                !arg1.is_empty(),
-                "Argument <bits> missing: 4=private keys, 8=rest but server config"
-            );
-            let bits: i32 = arg1.parse()?;
-            ensure!(bits < 16, "<bits> must be lower than 16.");
-            reset_tables(&context, bits).await;
-        }
         "stop" => {
             context.stop_ongoing().await;
         }

deltachat-repl/src/main.rs

@@ -147,7 +147,7 @@ impl Completer for DcHelper {
     }
 }
 
-const IMEX_COMMANDS: [&str; 10] = [
+const IMEX_COMMANDS: [&str; 9] = [
     "has-backup",
     "export-backup",
     "import-backup",
@@ -156,7 +156,6 @@ const IMEX_COMMANDS: [&str; 10] = [
     "export-keys",
     "import-keys",
     "poke",
-    "reset",
     "stop",
 ];
 

src/autorelay.rs

@@ -163,7 +163,7 @@ async fn maybe_add_additional_relays_inner(context: &Context, skip_network: bool
     for _ in 0..NUM_TRANSPORTS_TARGET {
         if context.count_transports().await? >= NUM_TRANSPORTS_TARGET {
             context
-                .set_config_internal(Config::AutorelayFinished, config::from_bool(true))
+                .set_config_internal(Config::AutorelayFinished, Some(config::from_bool(true)))
                 .await?;
 
             return Ok(relay_added);

src/chat.rs

@@ -3639,7 +3639,7 @@ pub(crate) async fn create_out_broadcast_ext(
         )?;
         ensure!(cnt == 0, "{cnt} chats exist with grpid {grpid}");
         let mut params: Params = Params::new();
-        params.update_timestamp(Param::GroupNameTimestamp, time())?;
+        params.update_timestamp(Param::GroupNameTimestamp, time());
 
         t.execute(
             "INSERT INTO chats

src/config.rs

@@ -809,7 +809,7 @@ impl Context {
 
     /// Set the given config to a boolean value.
     pub async fn set_config_bool(&self, key: Config, value: bool) -> Result<()> {
-        self.set_config(key, from_bool(value)).await?;
+        self.set_config(key, Some(from_bool(value))).await?;
         Ok(())
     }
 
@@ -830,8 +830,8 @@ impl Context {
 }
 
 /// Returns a value for use in `Context::set_config_*()` for the given `bool`.
-pub(crate) fn from_bool(val: bool) -> Option<&'static str> {
-    Some(if val { "1" } else { "0" })
+pub(crate) fn from_bool(val: bool) -> &'static str {
+    if val { "1" } else { "0" }
 }
 
 pub(crate) fn bool_from_config(config: Option<&str>) -> bool {

src/configure/server_params.rs

@@ -29,7 +29,7 @@ impl ServerParams {
         if self.username.is_empty() {
             vec![Self {
                 username: addr.to_string(),
-                ..self.clone()
+                ..self
             }]
         } else {
             vec![self]

src/imap.rs

@@ -1454,7 +1454,7 @@ impl Session {
     /// or flags have been changed.
     /// In this case we may want to skip next IDLE and do a round
     /// of fetching new messages and synchronizing seen flags.
-    fn drain_unsolicited_responses(&self, context: &Context) -> Result<bool> {
+    fn drain_unsolicited_responses(&self, context: &Context) -> bool {
         use UnsolicitedResponse::*;
         use async_imap::imap_proto::Response;
         use async_imap::imap_proto::ResponseCode;
@@ -1499,7 +1499,7 @@ impl Session {
                 }
             }
         }
-        Ok(should_refetch)
+        should_refetch
     }
 }
 

src/imap/idle.rs

@@ -31,7 +31,7 @@ impl Session {
 
         self.select_with_uidvalidity(context, folder).await?;
 
-        if self.drain_unsolicited_responses(context)? {
+        if self.drain_unsolicited_responses(context) {
             self.new_mail = true;
         }
 

src/key.rs

@@ -124,14 +124,11 @@ pub trait DcKey: Serialize + Deserializable + Clone {
 
 /// Converts secret key to public key.
 pub(crate) fn secret_key_to_public_key(
-    context: &Context,
     mut signed_secret_key: SignedSecretKey,
     timestamp: u32,
     addr: &str,
     relay_addrs: &str,
 ) -> Result<SignedPublicKey> {
-    info!(context, "Converting secret key to public key.");
-
     // Make sure timestamp of created signatures
     // is not in the past compared to the primary key timestamp.
     let timestamp = std::cmp::max(
@@ -304,7 +301,7 @@ pub(crate) async fn load_self_public_key_opt(context: &Context) -> Result<Option
     let addr = context.get_primary_self_addr().await?;
     let all_addrs = context.get_self_addrs().await?.join(",");
     let signed_public_key =
-        secret_key_to_public_key(context, signed_secret_key, timestamp, &addr, &all_addrs)?;
+        secret_key_to_public_key(signed_secret_key, timestamp, &addr, &all_addrs)?;
     *lock = Some(signed_public_key.clone());
 
     Ok(Some(signed_public_key))

src/lib.rs

@@ -15,7 +15,8 @@
     clippy::explicit_iter_loop,
     clippy::explicit_into_iter_loop,
     clippy::cloned_instead_of_copied,
-    clippy::manual_is_variant_and
+    clippy::manual_is_variant_and,
+    clippy::unnecessary_wraps
 )]
 #![cfg_attr(not(test), warn(clippy::arithmetic_side_effects))]
 #![cfg_attr(not(test), forbid(clippy::indexing_slicing))]

src/message.rs

@@ -580,7 +580,7 @@ impl Message {
 
         if let Some(msg) = &mut msg {
             msg.additional_text =
-                Self::get_additional_text(context, msg.download_state, &msg.param)?;
+                Self::get_additional_text(context, msg.download_state, &msg.param);
         }
 
         Ok(msg)
@@ -618,7 +618,7 @@ impl Message {
         context: &Context,
         download_state: DownloadState,
         param: &Params,
-    ) -> Result<String> {
+    ) -> String {
         if download_state != DownloadState::Done {
             let file_size = param
                 .get(Param::PostMessageFileBytes)
@@ -635,14 +635,14 @@ impl Message {
                 .unwrap_or("?".to_owned());
 
             return match viewtype {
-                Viewtype::File => Ok(format!(" [{file_name} – {file_size}]")),
+                Viewtype::File => format!(" [{file_name} – {file_size}]"),
                 _ => {
                     let translated_viewtype = viewtype.to_locale_string(context);
-                    Ok(format!(" [{translated_viewtype} – {file_size}]"))
+                    format!(" [{translated_viewtype} – {file_size}]")
                 }
             };
         }
-        Ok(String::new())
+        String::new()
     }
 
     /// Returns the MIME type of an attached file if it exists.

src/message/message_tests.rs

@@ -784,7 +784,7 @@ async fn test_get_existing_msg_ids() -> Result<()> {
 }
 
 #[test]
-fn test_can_fail() -> Result<()> {
+fn test_can_fail() {
     use MessageState::*;
 
     // states that are not allowed to transition to OutFailed
@@ -799,6 +799,4 @@ fn test_can_fail() -> Result<()> {
     assert!(OutPending.can_fail());
     assert!(OutDelivered.can_fail());
     assert!(OutFailed.can_fail());
-
-    Ok(())
 }

src/mimefactory/mimefactory_tests.rs

@@ -341,7 +341,6 @@ async fn test_mdn_sent_to_all_relays() -> Result<()> {
     // Bob's key gets a second relay address and Alice merges the newer key.
     let bob_secret_key = load_self_secret_key(bob).await?;
     let bob_public_key = secret_key_to_public_key(
-        bob,
         bob_secret_key,
         u32::try_from(time())? + 100,
         "bob@example.net",
@@ -795,7 +794,7 @@ async fn test_protected_headers_directive() -> Result<()> {
     // Long messages are truncated and MimeMessage::decoded_data is set for them. We need
     // decoded_data to check presence of the necessary headers.
     msg.set_text("a".repeat(constants::DC_DESIRED_TEXT_LEN + 1));
-    msg.set_file_from_bytes(&bob, "foo.bar", "content".as_bytes(), None)?;
+    msg.set_file_from_bytes(&bob, "foo.bar", b"content", None)?;
     let sent = bob.send_msg(chat, &mut msg).await;
     assert!(msg.get_showpadlock());
     assert!(sent.payload.contains("\r\nSubject: [...]\r\n"));
@@ -1127,7 +1126,7 @@ async fn test_render_unencrypted_msg_with_attachment() -> Result<()> {
         .await;
     let mut msg = Message::new(Viewtype::File);
     msg.set_text("Hello!".to_string());
-    msg.set_file_from_bytes(alice, "foo.bar", "content".as_bytes(), None)?;
+    msg.set_file_from_bytes(alice, "foo.bar", b"content", None)?;
     let sent = alice.send_msg(chat.id, &mut msg).await;
     let unencrypted = normalized_payload(sent).await;
 

src/mimeparser.rs

@@ -294,11 +294,7 @@ impl MimeMessage {
             &mut wants_mdn,
             &mail,
         );
-        headers_removed.extend(
-            headers
-                .extract_if(|k, _v| is_hidden(k))
-                .map(|(k, _v)| k.to_string()),
-        );
+        headers_removed.extend(headers.extract_if(|k, _v| is_hidden(k)).map(|(k, _v)| k));
 
         // Parse hidden headers.
         let mimetype = mail.ctype.mimetype.parse::<Mime>()?;
@@ -1751,7 +1747,7 @@ impl MimeMessage {
         headers_removed.extend(
             headers
                 .extract_if(|k, _v| has_header_protection || is_protected(k))
-                .map(|(k, _v)| k.to_string()),
+                .map(|(k, _v)| k),
         );
 
         if has_header_protection {

src/peer_channels.rs

@@ -46,7 +46,7 @@ use crate::mimeparser::SystemMessage;
 
 /// The length of an ed25519 `PublicKey`, in bytes.
 const PUBLIC_KEY_LENGTH: usize = 32;
-const PUBLIC_KEY_STUB: &[u8] = "static_string".as_bytes();
+const PUBLIC_KEY_STUB: &[u8] = b"static_string";
 
 /// Store Iroh peer channels for the context.
 #[derive(Debug)]

src/peer_channels/peer_channels_tests.rs

@@ -86,14 +86,14 @@ async fn test_can_communicate() {
         .get_or_try_init_peer_channel()
         .await
         .unwrap()
-        .send_webxdc_realtime_data(alice, alice_webxdc.id, "alice -> bob".as_bytes().to_vec())
+        .send_webxdc_realtime_data(alice, alice_webxdc.id, b"alice -> bob".to_vec())
         .await
         .unwrap();
 
     loop {
         let event = bob.evtracker.recv().await.unwrap();
         if let EventType::WebxdcRealtimeData { data, .. } = event.typ {
-            if data == "alice -> bob".as_bytes() {
+            if data == b"alice -> bob" {
                 break;
             } else {
                 panic!(
@@ -107,14 +107,14 @@ async fn test_can_communicate() {
     bob.get_or_try_init_peer_channel()
         .await
         .unwrap()
-        .send_webxdc_realtime_data(bob, bob_webxdc.id, "bob -> alice".as_bytes().to_vec())
+        .send_webxdc_realtime_data(bob, bob_webxdc.id, b"bob -> alice".to_vec())
         .await
         .unwrap();
 
     loop {
         let event = alice.evtracker.recv().await.unwrap();
         if let EventType::WebxdcRealtimeData { data, .. } = event.typ {
-            if data == "bob -> alice".as_bytes() {
+            if data == b"bob -> alice" {
                 break;
             } else {
                 panic!(
@@ -149,14 +149,14 @@ async fn test_can_communicate() {
     bob.get_or_try_init_peer_channel()
         .await
         .unwrap()
-        .send_webxdc_realtime_data(bob, bob_webxdc.id, "bob -> alice 2".as_bytes().to_vec())
+        .send_webxdc_realtime_data(bob, bob_webxdc.id, b"bob -> alice 2".to_vec())
         .await
         .unwrap();
 
     loop {
         let event = alice.evtracker.recv().await.unwrap();
         if let EventType::WebxdcRealtimeData { data, .. } = event.typ {
-            if data == "bob -> alice 2".as_bytes() {
+            if data == b"bob -> alice 2" {
                 break;
             } else {
                 panic!(
@@ -314,14 +314,14 @@ async fn test_can_reconnect() {
         .get_or_try_init_peer_channel()
         .await
         .unwrap()
-        .send_webxdc_realtime_data(alice, alice_webxdc.id, "alice -> bob".as_bytes().to_vec())
+        .send_webxdc_realtime_data(alice, alice_webxdc.id, b"alice -> bob".to_vec())
         .await
         .unwrap();
 
     loop {
         let event = bob.evtracker.recv().await.unwrap();
         if let EventType::WebxdcRealtimeData { data, .. } = event.typ {
-            if data == "alice -> bob".as_bytes() {
+            if data == b"alice -> bob" {
                 break;
             } else {
                 panic!(
@@ -373,14 +373,14 @@ async fn test_can_reconnect() {
     bob.get_or_try_init_peer_channel()
         .await
         .unwrap()
-        .send_webxdc_realtime_data(bob, bob_webxdc.id, "bob -> alice".as_bytes().to_vec())
+        .send_webxdc_realtime_data(bob, bob_webxdc.id, b"bob -> alice".to_vec())
         .await
         .unwrap();
 
     loop {
         let event = alice.evtracker.recv().await.unwrap();
         if let EventType::WebxdcRealtimeData { data, .. } = event.typ {
-            if data == "bob -> alice".as_bytes() {
+            if data == b"bob -> alice" {
                 break;
             } else {
                 panic!(

src/pgp.rs

@@ -1,5 +1,6 @@
 //! OpenPGP helper module using [rPGP facilities](https://github.com/rpgp/rpgp).
 
+use std::cmp::Ordering;
 use std::collections::{HashMap, HashSet};
 use std::io::Cursor;
 
@@ -83,13 +84,62 @@ pub(crate) fn create_keypair(addr: EmailAddress) -> Result<SignedSecretKey> {
 
 /// Selects a subkey of the public key to use for encryption.
 ///
-/// Returns `None` if the public key cannot be used for encryption.
+/// The key is selected according to
+/// <https://www.ietf.org/archive/id/draft-autocrypt-openpgp-v2-cert-03.html#section-4.3-4>.
+/// If multiple keys are available, the one that will expire sooner is selected.
 ///
-/// TODO: take key flags and expiration dates into account
-fn select_pk_for_encryption(key: &SignedPublicKey) -> Option<&SignedPublicSubKey> {
+/// Returns `None` if the public key cannot be used for encryption.
+fn select_pk_for_encryption(now: u32, key: &SignedPublicKey) -> Option<&SignedPublicSubKey> {
     key.public_subkeys
         .iter()
-        .find(|subkey| subkey.algorithm().can_encrypt())
+        .filter(|subkey| subkey.algorithm().can_encrypt())
+        .filter_map(|subkey| {
+            let signature = subkey.signatures.first()?;
+
+            let key_flags = signature.key_flags();
+            if !key_flags.encrypt_comms() {
+                return None;
+            }
+
+            if let Some(expiration_duration) = signature
+                .key_expiration_time()
+                .filter(|duration| duration.as_secs() != 0)
+                && now
+                    > subkey
+                        .created_at()
+                        .as_secs()
+                        .saturating_add(expiration_duration.as_secs())
+            {
+                // Key is expired.
+                return None;
+            }
+            Some((subkey, signature))
+        })
+        .min_by(|(subkey1, signature1), (subkey2, signature2)| {
+            match (
+                signature1
+                    .key_expiration_time()
+                    .filter(|duration| duration.as_secs() != 0),
+                signature2
+                    .key_expiration_time()
+                    .filter(|duration| duration.as_secs() != 0),
+            ) {
+                (None, None) => Ordering::Equal,
+                (None, Some(_)) => Ordering::Greater,
+                (Some(_), None) => Ordering::Less,
+                (Some(expiration1), Some(expiration2)) => (subkey1
+                    .created_at()
+                    .as_secs()
+                    .saturating_add(expiration1.as_secs()))
+                .cmp(
+                    &(subkey2
+                        .created_at()
+                        .as_secs()
+                        .saturating_add(expiration2.as_secs())),
+                ),
+            }
+        })
+        .map(|(subkey, _signature)| subkey)
 }
 
 /// Version of SEIPD packet to use.
@@ -151,10 +201,11 @@ pub fn pk_encrypt(
 ) -> Result<String> {
     tokio::task::block_in_place(|| {
         let mut rng = thread_rng();
+        let now = pgp::types::Timestamp::now();
 
         let pkeys = public_keys_for_encryption
             .iter()
-            .filter_map(select_pk_for_encryption);
+            .filter_map(|key| select_pk_for_encryption(now.as_secs(), key));
 
         let msg = MessageBuilder::from_bytes("", plain);
         let encoded_msg = match seipd_version {
@@ -401,7 +452,7 @@ pub fn merge_openpgp_certificates(
     // such as Alice's key in `test-data/key/alice-secret.asc`.
     let best_user: Option<SignedUser> = old_users
         .into_iter()
-        .chain(new_users.clone())
+        .chain(new_users)
         .filter_map(|SignedUser { id, signatures }| {
             // Select the best signature for each User ID.
             // If User ID has no valid signatures, it is filtered out.
@@ -485,7 +536,8 @@ pub(crate) fn relay_addrs(public_key: &SignedPublicKey, addr: &str) -> Vec<Strin
 
 /// Returns true if the key can be encrypted to, i.e. has an encryption subkey.
 pub(crate) fn pubkey_can_encrypt(public_key: &SignedPublicKey) -> bool {
-    select_pk_for_encryption(public_key).is_some()
+    let now = pgp::types::Timestamp::now();
+    select_pk_for_encryption(now.as_secs(), public_key).is_some()
 }
 
 /// Returns true if public key advertises SEIPDv2 feature.

src/pgp/pgp_tests.rs

@@ -384,7 +384,7 @@ fn test_merge_openpgp_certificates() {
 
     // Cannot merge certificates with different primary key.
     assert!(merge_openpgp_certificates(alice.clone(), bob.clone()).is_err());
-    assert!(merge_openpgp_certificates(bob.clone(), alice.clone()).is_err());
+    assert!(merge_openpgp_certificates(bob, alice).is_err());
 }
 
 /// Test PQC support.
@@ -423,3 +423,110 @@ async fn test_securejoin_pqc_joiner() {
 
     tcm.execute_securejoin(bob, pqc).await;
 }
+
+/// Tests that public subkey selection for encryption follows Autocrypt 2 rules.
+///
+/// If there is an expiring subkey, it is preferred, otherwise non-expiring subkey is selected.
+/// Non-encryption subkeys such as RSA subkey for authentication are ignored.
+#[test]
+fn test_select_pk_for_encryption() {
+    // Public key generated with GnuPG 2.4.9 with the following subkeys:
+    // 1. Auth-only RSA subkey (92E762B9084CA740).
+    // 2. Expired Curve25519 encryption subkey with 1-day expiration (C8F382BD0F35C49E)
+    // 3. Ed25519 signing subkey (F177AC3118F923CC).
+    // 4. Curve25519 encryption subkey with fingerprint (36188C6FFC8E267B)
+    // 5. Curve25519 encryption subkey with 1 year expiration, valid in the beginning of 2008, with key ID 9223FCEE7546CDE7
+    // 6. Curve25519 encryption subkey with no expiration (FD2C0567967223D8).
+    // Primary key is an Ed25519 not expiring key.
+    // Key 4 is the one that should be selected.
+
+    // Subkey 4 fingerprint.
+    let expected_fallback_fingerprint = "cdeb3ba3999bf7880f0ee1f536188c6ffc8e267b";
+
+    // Subkey 5 fingerprint, should be preferred to fallback when not expired.
+    let expected_expiring_fingerprint = "5133fab157c4a46ca41f6dc39223fcee7546cde7";
+
+    let alice_tpk_asc = "
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMERvfcPBYJKwYBBAHaRw8BAQdAimvPsr7NdJ4dBoFPySwhpTQqoYOoHL3AzfE7
+mGWQOOC0GUFsaWNlIDxhbGljZUBleGFtcGxlLm9yZz6IkAQTFgoAOBYhBCi19Yqv
+ugVVkhyHgicqAms0FFoiBQJG99w8AhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheA
+AAoJECcqAms0FFoiUGEA/3VZMBCoRq0ZpHarzmvzgdZCoL3r3m9en/eZScFzxITx
+AP42Mn27r0SOKwIln0VcPTdAQCk49mBW/EX3CMOlLPU3DLkBjQRG99w8AQwArsYe
+Jkdl6sSM/hfoEw0vgx/RdUBQ6QRYi1uc0UUNlIGy8mlczLFdkD3JF/hGocjPvt45
+XAQoK110zAkZlfpFRqNT1M/IC68Er8rLkYPC4OeFh6W4Iyn17fcUanP0lf8em/jh
+Vffvgy8sFOMdO235lvFA3txNA98s4fHdmU3PScyd1hc3C4M0yP83LnYyWt4X59Xc
+E/Om5Dm458eKCSeYkLI6752W0mXsBxSi3/dLn0XeuNRpgmKxSkm562FHOFaLbKtR
+Y5hobAI9PkNcVgRxZZvWQls5PHTZWjqngF21lKlaLfdqZ/Uae1i2hzZOihgitL43
+Le00qwNBi5hKYMeuDnHaQrLmb+A+0/IAEE4Ub+TkZhzI+2ZP2k1cAT0qZmZi0w+q
+xqP9INk0hY/oZFCnV2wkHN7zvQmVlUIcQ2rmfbafK1yiEL1qeGT96zyjbdXeGPqJ
+3O9h+YIG38JMRJBijsFujUN34Z546zS/kzOPXsz/WlGUMjwu8n5s5uf2TFyFABEB
+AAGIeAQYFgoAIBYhBCi19YqvugVVkhyHgicqAms0FFoiBQJG99w8AhsgAAoJECcq
+Ams0FFoiCOUBAPafRLDpWN9iT4hcCXjESf1Hw5KNVkJpwfzPfu2H9BkMAQCaHhKg
+pq9ywH4pyOHZCPV8P2ywkyn+EsjBC3fG+GBBBbg4BEb33DwSCisGAQQBl1UBBQEB
+B0DfI8AJFT3nWa6ZXLkHSf7W8W7S6AWIO7LAcjoyHwb8CwMBCAeIfgQYFgoAJhYh
+BCi19YqvugVVkhyHgicqAms0FFoiBQJG99w8AhsMBQkAAVGAAAoJECcqAms0FFoi
+U5EA/3G74HRwIMJlNOEW5gkYYV5KJW2qgtMfxHCUjoHvNWU1AQCHt/bLU2aviAiS
+of1R43qojxKUqzzoi8lYRQ+1sYhvB7gzBEb33DwWCSsGAQQB2kcPAQEHQKZXUJ7s
+xqH3kVcMnhasw6DrFMwCxHDdj+qvkg8r/DvtiO8EGBYKACAWIQQotfWKr7oFVZIc
+h4InKgJrNBRaIgUCRvfcPAIbAgCBCRAnKgJrNBRaInYgBBkWCgAdFiEEI8hstnVQ
+9sgylIYg8XesMRj5I8wFAkb33DwACgkQ8XesMRj5I8xo6QEAu4o/TyEZwFcyqZpw
+LEo9vTLCsc7fo0nx0ssiP6FyV5cBAOWal1DznDhsXWCNt+U8UaafXsU2DTV51KaD
+VBOVFo4CyeQBAMirIjXV5PbUV674TNLhYl2s0jTtNz+GKtOjSdZuRm1mAPwPG6ya
+K1b7iMRdBT92gNZMw30LbtcXmttCxpZAwr9lBLg4BEb33DwSCisGAQQBl1UBBQEB
+B0C5fFb4WTHoIoI6ou/31+1N1wn8ghsSkUVzpbtv/aTkegMBCAeIeAQYFgoAIBYh
+BCi19YqvugVVkhyHgicqAms0FFoiBQJG99w8AhsMAAoJECcqAms0FFoi8z8BALPL
+7V0ICLEY5YSUa4lQ2rjiXOcVTlWkG3h4TATPrr08AP9tIAQIE0o50IGdQAcKJoTn
+Lyxnf2wfjZ16vL3JLLjSBrg4BEb33DwSCisGAQQBl1UBBQEBB0DXDrcGrnuLjAUO
+eo/t8MQNOe+ZKYSDPGTkO7iM5IloSAMBCAeIfgQYFgoAJhYhBCi19YqvugVVkhyH
+gicqAms0FFoiBQJG99w8AhsMBQkB4TOAAAoJECcqAms0FFoivQIA/2PcZ1vcImAa
+7ldPY00JkcW6WlSSd6yOIZsVa4TdA1FiAP42gOji+4RrLps2+NX6L1znSc8EJBXo
+RMbND/CZQWXfA7g4BEb33DwSCisGAQQBl1UBBQEBB0BHxCvo5zuygw2XiluYNobx
+7iFJqlmCkjekKyoVFquKHQMBCAeIeAQYFgoAIBYhBCi19YqvugVVkhyHgicqAms0
+FFoiBQJG99w8AhsMAAoJECcqAms0FFoirSMA/0gVP98sPFga+UhQ3uJxJw5bO2Rs
+7hxVk6aPREWgBYg1AQCT7AE8m7j17SP/1fl8OjpxsQQmCJyv2wNcP48OfKGOCA==
+=AXAi
+-----END PGP PUBLIC KEY BLOCK-----
+    ";
+
+    let alice_tpk = SignedPublicKey::from_asc(alice_tpk_asc).unwrap();
+    let now = pgp::types::Timestamp::now();
+    let encryption_subkey = select_pk_for_encryption(now.as_secs(), &alice_tpk).unwrap();
+    assert_eq!(
+        encryption_subkey.fingerprint().to_string().as_str(),
+        expected_fallback_fingerprint
+    );
+
+    // In the beginning of 2008 expiring subkey is not expired yet and should be used.
+    let encryption_subkey = select_pk_for_encryption(1199149200, &alice_tpk).unwrap();
+    assert_eq!(
+        encryption_subkey.fingerprint().to_string().as_str(),
+        expected_expiring_fingerprint
+    );
+}
+
+/// Tests that key selection fails if there is only an expired subkey.
+#[test]
+fn test_select_only_expired_subkey() {
+    let alice_tpk_asc = "
+    -----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMERvfcPBYJKwYBBAHaRw8BAQdAUz6AZXIRE8T04Vh8RReFiP3tEV8UfSs2EiYs
+8b8i4ce0GUFsaWNlIDxhbGljZUBleGFtcGxlLm9yZz6IkAQTFgoAOBYhBNRG3w/A
+qWyPitBaqkGLyqDx3CaUBQJG99w8AhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheA
+AAoJEEGLyqDx3CaUMjIA/Rq9/iORLP360s6EsIDe9qSmlmSCggtivafH+uVBWa0X
+AP0Wb+kailDwISq2O9Ef/jceZw7ozyzDLeDskKpCYiL3A7g4BEb33DwSCisGAQQB
+l1UBBQEBB0BXhbrks9iskW1GfT3B022W3KhJCgz8gw81z9lAWv7OZgMBCAeIfgQY
+FgoAJhYhBNRG3w/AqWyPitBaqkGLyqDx3CaUBQJG99w8AhsMBQkB4TOAAAoJEEGL
+yqDx3CaU6/YA/jaYJsZMXvu5grrMq1wq3Z/yzGXd15zeWp+alY/6UYxoAQCmSrBC
+SOtE3ODLZN9tC3F7k1N9clme1cHXyUiH3EliBQ==
+=zWPq
+-----END PGP PUBLIC KEY BLOCK-----
+";
+
+    let alice_tpk = SignedPublicKey::from_asc(alice_tpk_asc).unwrap();
+    let now = pgp::types::Timestamp::now();
+
+    assert_eq!(select_pk_for_encryption(now.as_secs(), &alice_tpk), None);
+}

src/qr.rs

@@ -764,7 +764,7 @@ fn decode_tg_socks_proxy(_context: &Context, qr: &str) -> Result<Qr> {
 fn decode_shadowsocks_proxy(qr: &str) -> Result<Qr> {
     let server_config = shadowsocks::config::ServerConfig::from_url(qr)?;
     let addr = server_config.addr();
-    let host = addr.host().to_string();
+    let host = addr.host();
     let port = addr.port();
     Ok(Qr::Proxy {
         url: qr.to_string(),
@@ -1124,7 +1124,7 @@ fn normalize_address(addr: &str) -> Result<String> {
 
     ensure!(may_be_valid_addr(&new_addr), "Bad e-mail address");
 
-    Ok(new_addr.to_string())
+    Ok(new_addr)
 }
 
 #[cfg(test)]

src/qr/dclogin_scheme.rs

@@ -120,7 +120,7 @@ pub(super) fn decode_login(qr: &str) -> Result<Qr> {
         };
 
         Ok(Qr::Login {
-            address: addr.to_owned(),
+            address: addr,
             options,
         })
     } else {

src/reaction.rs

@@ -165,7 +165,7 @@ async fn set_msg_id_reaction(
             .await?;
         if chat
             .param
-            .update_timestamp(Param::LastReactionTimestamp, timestamp)?
+            .update_timestamp(Param::LastReactionTimestamp, timestamp)
         {
             chat.param
                 .set_i64(Param::LastReactionMsgId, i64::from(msg_id.to_u32()));

src/receive_imf.rs

@@ -2311,7 +2311,7 @@ INSERT INTO msgs
         // This way, `LastSubject` actually refers to the most recent message _shown_ in the chat.
         if chat
             .param
-            .update_timestamp(Param::SubjectTimestamp, sort_timestamp)?
+            .update_timestamp(Param::SubjectTimestamp, sort_timestamp)
         {
             // write the last subject even if empty -
             // otherwise a reply may get an outdated subject.
@@ -3414,7 +3414,7 @@ async fn apply_chat_name_avatar_and_description_changes(
         && is_from_in_chat
         && chat
             .param
-            .update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
+            .update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)
     {
         info!(context, "Group-avatar change for {}.", chat.id);
         match avatar_action {

src/securejoin.rs

@@ -33,14 +33,14 @@ pub(crate) use qrinvite::QrInvite;
 
 use crate::token::Namespace;
 
-const DISALLOWED_CHARACTERS: &AsciiSet = &NON_ALPHANUMERIC_WITHOUT_DOT.remove(b'_');
+const DISALLOWED_CHARACTERS: &AsciiSet = &NON_ALPHANUMERIC_WITHOUT_DOT.remove(b'_').remove(b'@');
 
 fn inviter_progress(
     context: &Context,
     contact_id: ContactId,
     chat_id: ChatId,
     chat_type: Chattype,
-) -> Result<()> {
+) {
     // No other values are used.
     let progress = 1000;
     context.emit_event(EventType::SecurejoinInviterProgress {
@@ -49,8 +49,6 @@ fn inviter_progress(
         chat_type,
         progress,
     });
-
-    Ok(())
 }
 
 /// Shorten name to max. `length` characters.
@@ -124,18 +122,19 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
     let self_addr = context.get_primary_self_addr().await?;
     let self_addr_urlencoded = utf8_percent_encode(&self_addr, DISALLOWED_CHARACTERS).to_string();
 
-    let r_param = context
+    let encoded_extra_relays: Vec<String> = context
         .get_self_addrs()
         .await?
         .into_iter()
         .filter(|addr| *addr != self_addr)
-        .reduce(|acc, addr| {
-            format!(
-                "{acc},{}",
-                utf8_percent_encode(&addr, DISALLOWED_CHARACTERS)
-            )
-        })
-        .map_or(String::default(), |addrs| format!("&r={addrs}"));
+        .map(|addr| utf8_percent_encode(&addr, DISALLOWED_CHARACTERS).to_string())
+        .collect();
+
+    let r_param = if encoded_extra_relays.is_empty() {
+        "".to_string()
+    } else {
+        format!("&r={}", encoded_extra_relays.join(","))
+    };
 
     let self_name = context
         .get_config(Config::Displayname)
@@ -659,7 +658,7 @@ pub(crate) async fn handle_securejoin_handshake(
                     context.emit_event(EventType::ContactsChanged(Some(contact_id)));
                 }
 
-                inviter_progress(context, contact_id, joining_chat_id, chat.typ)?;
+                inviter_progress(context, contact_id, joining_chat_id, chat.typ);
                 // IMAP-delete the message to avoid handling it by another device and adding the
                 // member twice. Another device will know the member's key from Autocrypt-Gossip.
                 Ok(HandshakeMessage::Done)
@@ -670,7 +669,7 @@ pub(crate) async fn handle_securejoin_handshake(
                     .await
                     .context("failed sending vc-contact-confirm message")?;
 
-                inviter_progress(context, contact_id, chat_id, Chattype::Single)?;
+                inviter_progress(context, contact_id, chat_id, Chattype::Single);
                 Ok(HandshakeMessage::Ignore) // "Done" would delete the message and break multi-device (the key from Autocrypt-header is needed)
             }
         }
@@ -817,7 +816,7 @@ pub(crate) async fn observe_securejoin_on_other_device(
         // and tests which don't care about the chat ID,
         // so we pass invalid chat ID here.
         let chat_id = ChatId::new(0);
-        inviter_progress(context, contact_id, chat_id, chat_type)?;
+        inviter_progress(context, contact_id, chat_id, chat_type);
     }
 
     if matches!(step, SecureJoinStep::MemberAdded) {

src/securejoin/securejoin_tests.rs

@@ -13,6 +13,7 @@ use crate::test_utils::{
     AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager,
     TimeShiftFalsePositiveNote, get_chat_msg, sync,
 };
+use crate::transport::add_pseudo_transport;
 
 #[derive(PartialEq)]
 enum SetupContactCase {
@@ -1142,6 +1143,43 @@ async fn test_get_securejoin_qr_name_is_last() -> Result<()> {
     Ok(())
 }
 
+/// Test that addresses in QR codes are percent-encoded.
+/// `@` should not be encoded unnecessarily,
+/// since this would just make the QR code longer.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_get_securejoin_qr_encoding() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    // `@` in email addresses must not be percent-encoded:
+    add_pseudo_transport(alice, "asdf@example.org").await?;
+    // But `%` does need percent-encoding:
+    add_pseudo_transport(alice, "jk%l@example.net").await?;
+
+    let qr = get_securejoin_qr(alice, None).await?;
+    assert!(
+        qr.contains("a=alice@example.org"),
+        "{qr} doesn't contain 'a=alice@example.org'"
+    );
+    assert!(
+        qr.contains("r=jk%25l@example.net,asdf@example.org"),
+        "{qr} doesn't contain 'r=jk%25l@example.net,asdf@example.org'"
+    );
+
+    let qr = check_qr(bob, &qr).await?;
+    let Qr::AskVerifyContact { mut addrs, .. } = qr else {
+        unreachable!()
+    };
+    addrs.sort();
+    assert_eq!(
+        addrs,
+        vec!["alice@example.org", "asdf@example.org", "jk%l@example.net",]
+    );
+
+    Ok(())
+}
+
 /// QR codes should not get arbitrary big because of long names.
 /// The truncation, however, should not let the url end with a `.`, which is a call for trouble in linkfiers.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

src/sql/migrations.rs

@@ -32,8 +32,6 @@ fn migrate_key_contacts(
     context: &Context,
     transaction: &mut rusqlite::Transaction<'_>,
 ) -> std::result::Result<(), anyhow::Error> {
-    info!(context, "Starting key-contact transition.");
-
     // =============================== Step 1: ===============================
     //                              Alter tables
     transaction.execute_batch(
@@ -79,13 +77,12 @@ fn migrate_key_contacts(
         .optional()
         .context("Step 0")?
     else {
-        info!(
-            context,
-            "Not yet configured, no need to migrate key-contacts"
-        );
+        // Not yet configured, no need to migrate key-contacts.
         return Ok(());
     };
 
+    info!(context, "Starting key-contact transition.");
+
     // =============================== Step 2: ===============================
     // Create up to 3 new contacts for every contact that has a peerstate:
     // one from the Autocrypt key fingerprint, one from the verified key fingerprint,
@@ -1936,14 +1933,9 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint);
 
     inc_and_check(&mut migration_version, 132)?;
     if dbversion < migration_version {
-        let start = Time::now();
         sql.execute_migration_transaction(|t| migrate_key_contacts(context, t), migration_version)
             .await?;
-        info!(
-            context,
-            "key-contacts migration took {:?} in total.",
-            time_elapsed(&start),
-        );
+
         // Schedule `msgs_to_key_contacts()`.
         context
             .set_config_internal(Config::LastHousekeeping, None)

src/update_helper.rs

@@ -27,7 +27,7 @@ impl Context {
                         Ok(param.parse().unwrap_or_default())
                     },
                 )?;
-                let update = param.update_timestamp(scope, new_timestamp)?;
+                let update = param.update_timestamp(scope, new_timestamp);
                 if update {
                     transaction.execute(
                         "UPDATE contacts SET param=? WHERE id=?",
@@ -57,7 +57,7 @@ impl ChatId {
                         let param: String = row.get(0)?;
                         Ok(param.parse().unwrap_or_default())
                     })?;
-                let update = param.update_timestamp(scope, new_timestamp)?;
+                let update = param.update_timestamp(scope, new_timestamp);
                 if update {
                     transaction.execute(
                         "UPDATE chats SET param=? WHERE id=?",
@@ -73,13 +73,13 @@ impl ChatId {
 impl Params {
     /// Updates a param's timestamp in memory, if reasonable.
     /// Returns true if the caller shall update the settings belonging to the scope.
-    pub(crate) fn update_timestamp(&mut self, scope: Param, new_timestamp: i64) -> Result<bool> {
+    pub(crate) fn update_timestamp(&mut self, scope: Param, new_timestamp: i64) -> bool {
         let old_timestamp = self.get_i64(scope).unwrap_or_default();
         if new_timestamp >= old_timestamp {
             self.set_i64(scope, new_timestamp);
-            return Ok(true);
+            return true;
         }
-        Ok(false)
+        false
     }
 }
 
@@ -96,18 +96,18 @@ mod tests {
         let mut params = Params::new();
         let ts = time();
 
-        assert!(params.update_timestamp(Param::LastSubject, ts)?);
-        assert!(params.update_timestamp(Param::LastSubject, ts)?); // same timestamp -> update
-        assert!(params.update_timestamp(Param::LastSubject, ts + 10)?);
-        assert!(!params.update_timestamp(Param::LastSubject, ts)?); // `ts` is now too old
-        assert!(!params.update_timestamp(Param::LastSubject, 0)?);
+        assert!(params.update_timestamp(Param::LastSubject, ts));
+        assert!(params.update_timestamp(Param::LastSubject, ts)); // same timestamp -> update
+        assert!(params.update_timestamp(Param::LastSubject, ts + 10));
+        assert!(!params.update_timestamp(Param::LastSubject, ts)); // `ts` is now too old
+        assert!(!params.update_timestamp(Param::LastSubject, 0));
         assert_eq!(params.get_i64(Param::LastSubject).unwrap(), ts + 10);
 
-        assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)?); // stay unset -> update ...
-        assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)?); // ... also on multiple calls
+        assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)); // stay unset -> update ...
+        assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)); // ... also on multiple calls
         assert_eq!(params.get_i64(Param::GroupNameTimestamp).unwrap(), 0);
 
-        assert!(!params.update_timestamp(Param::AvatarTimestamp, -1)?);
+        assert!(!params.update_timestamp(Param::AvatarTimestamp, -1));
         assert_eq!(params.get_i64(Param::AvatarTimestamp), None);
 
         Ok(())

src/webxdc.rs

@@ -350,7 +350,7 @@ impl Context {
         if let Some(ref document) = status_update_item.document
             && instance
                 .param
-                .update_timestamp(Param::WebxdcDocumentTimestamp, timestamp)?
+                .update_timestamp(Param::WebxdcDocumentTimestamp, timestamp)
         {
             instance.param.set(Param::WebxdcDocument, document);
             param_changed = true;
@@ -359,10 +359,10 @@ impl Context {
         if let Some(ref summary) = status_update_item.summary
             && instance
                 .param
-                .update_timestamp(Param::WebxdcSummaryTimestamp, timestamp)?
+                .update_timestamp(Param::WebxdcSummaryTimestamp, timestamp)
         {
             let summary = sanitize_bidi_characters(summary);
-            instance.param.set(Param::WebxdcSummary, summary.clone());
+            instance.param.set(Param::WebxdcSummary, summary);
             param_changed = true;
         }
 

src/webxdc/webxdc_tests.rs

@@ -1201,30 +1201,28 @@ async fn test_get_webxdc_blob_with_subdirs() -> Result<()> {
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_parse_webxdc_manifest() -> Result<()> {
-    let result = parse_webxdc_manifest(r#"key = syntax error"#.as_bytes());
+    let result = parse_webxdc_manifest(br#"key = syntax error"#);
     assert!(result.is_err());
 
-    let manifest = parse_webxdc_manifest(r#"no_name = "no name, no icon""#.as_bytes())?;
+    let manifest = parse_webxdc_manifest(br#"no_name = "no name, no icon""#)?;
     assert_eq!(manifest.name, None);
 
-    let manifest = parse_webxdc_manifest(r#"name = "name, no icon""#.as_bytes())?;
+    let manifest = parse_webxdc_manifest(br#"name = "name, no icon""#)?;
     assert_eq!(manifest.name, Some("name, no icon".to_string()));
 
     let manifest = parse_webxdc_manifest(
-        r#"name = "foo"
-icon = "bar""#
-            .as_bytes(),
+        br#"name = "foo"
+icon = "bar""#,
     )?;
     assert_eq!(manifest.name, Some("foo".to_string()));
 
     let manifest = parse_webxdc_manifest(
-        r#"name = "foz"
+        br#"name = "foz"
 icon = "baz"
 add_item = "that should be just ignored"
 
 [section]
-sth_for_the = "future""#
-            .as_bytes(),
+sth_for_the = "future""#,
     )?;
     assert_eq!(manifest.name, Some("foz".to_string()));
     Ok(())
@@ -1232,13 +1230,13 @@ sth_for_the = "future""#
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_parse_webxdc_manifest_min_api() -> Result<()> {
-    let manifest = parse_webxdc_manifest(r#"min_api = 3"#.as_bytes())?;
+    let manifest = parse_webxdc_manifest(br#"min_api = 3"#)?;
     assert_eq!(manifest.min_api, Some(3));
 
-    let result = parse_webxdc_manifest(r#"min_api = "1""#.as_bytes());
+    let result = parse_webxdc_manifest(br#"min_api = "1""#);
     assert!(result.is_err());
 
-    let result = parse_webxdc_manifest(r#"min_api = 1.2"#.as_bytes());
+    let result = parse_webxdc_manifest(br#"min_api = 1.2"#);
     assert!(result.is_err());
 
     Ok(())
@@ -1246,10 +1244,10 @@ async fn test_parse_webxdc_manifest_min_api() -> Result<()> {
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_parse_webxdc_manifest_source_code_url() -> Result<()> {
-    let result = parse_webxdc_manifest(r#"source_code_url = 3"#.as_bytes());
+    let result = parse_webxdc_manifest(br#"source_code_url = 3"#);
     assert!(result.is_err());
 
-    let manifest = parse_webxdc_manifest(r#"source_code_url = "https://foo.bar""#.as_bytes())?;
+    let manifest = parse_webxdc_manifest(br#"source_code_url = "https://foo.bar""#)?;
     assert_eq!(
         manifest.source_code_url,
         Some("https://foo.bar".to_string())