refactor: remove Secure-Join-Fingerprint header

It was named notify_peer_verified()
because it added info message,
but this is no longer true since
https://github.com/deltachat/deltachat-core-rust/pull/4998
(commit c6ea4e389a)
is merged.

deltachat-rpc-client/tests/test_securejoin.py

@@ -4,7 +4,7 @@ import pytest
 from deltachat_rpc_client import Chat, SpecialContactId
 
 
-def test_qr_setup_contact(acfactory) -> None:
+def test_qr_setup_contact(acfactory, tmp_path) -> None:
     alice, bob = acfactory.get_online_accounts(2)
 
     qr_code, _svg = alice.get_qr_code()
@@ -24,6 +24,18 @@ def test_qr_setup_contact(acfactory) -> None:
     bob_contact_alice_snapshot = bob_contact_alice.get_snapshot()
     assert bob_contact_alice_snapshot.is_verified
 
+    # Test that if Bob changes the key, backwards verification is lost.
+    logging.info("Bob 2 is created")
+    bob2 = acfactory.new_configured_account()
+    bob2.export_self_keys(tmp_path)
+
+    logging.info("Bob imports a key")
+    bob.import_self_keys(tmp_path / "private-key-default.asc")
+
+    assert bob.get_config("key_id") == "2"
+    bob_contact_alice_snapshot = bob_contact_alice.get_snapshot()
+    assert not bob_contact_alice_snapshot.is_verified
+
 
 @pytest.mark.parametrize("protect", [True, False])
 def test_qr_securejoin(acfactory, protect):

src/contact.rs

@@ -1268,13 +1268,30 @@ impl Contact {
             return Ok(true);
         }
 
-        if let Some(peerstate) = Peerstate::from_addr(context, &self.addr).await? {
-            if peerstate.is_using_verified_key() {
-                return Ok(true);
-            }
+        let Some(peerstate) = Peerstate::from_addr(context, &self.addr).await? else {
+            return Ok(false);
+        };
+
+        let forward_verified = peerstate.is_using_verified_key();
+        let backward_verified = peerstate.is_backward_verified(context).await?;
+        Ok(forward_verified && backward_verified)
+    }
+
+    /// Returns true if we have a verified key for the contact
+    /// and it is the same as Autocrypt key.
+    /// This is enough to send messages to the contact in verified chat
+    /// and verify received messages, but not enough to display green checkmark
+    /// or add the contact to verified groups.
+    pub async fn is_forward_verified(&self, context: &Context) -> Result<bool> {
+        if self.id == ContactId::SELF {
+            return Ok(true);
         }
 
-        Ok(false)
+        let Some(peerstate) = Peerstate::from_addr(context, &self.addr).await? else {
+            return Ok(false);
+        };
+
+        Ok(peerstate.is_using_verified_key())
     }
 
     /// Returns the `ContactId` that verified the contact.

src/e2ee.rs

@@ -312,6 +312,7 @@ Sent with my Delta Chat Messenger: https://delta.chat";
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
         vec![(Some(peerstate), addr)]

src/mimefactory.rs

@@ -1003,17 +1003,6 @@ impl<'a> MimeFactory<'a> {
                             "Secure-Join".to_string(),
                             "vg-member-added".to_string(),
                         ));
-                        // FIXME: Old clients require Secure-Join-Fingerprint header. Remove this
-                        // eventually.
-                        let fingerprint = Peerstate::from_addr(context, email_to_add)
-                            .await?
-                            .context("No peerstate found in db")?
-                            .public_key_fingerprint
-                            .context("No public key fingerprint in db for the member to add")?;
-                        headers.protected.push(Header::new(
-                            "Secure-Join-Fingerprint".into(),
-                            fingerprint.hex(),
-                        ));
                     }
                 }
                 SystemMessage::GroupNameChanged => {
@@ -1096,13 +1085,6 @@ impl<'a> MimeFactory<'a> {
                         ));
                     }
 
-                    let fingerprint = msg.param.get(Param::Arg3).unwrap_or_default();
-                    if !fingerprint.is_empty() {
-                        headers.protected.push(Header::new(
-                            "Secure-Join-Fingerprint".into(),
-                            fingerprint.into(),
-                        ));
-                    }
                     if let Some(id) = msg.param.get(Param::Arg4) {
                         headers
                             .protected

src/param.rs

@@ -84,9 +84,6 @@ pub enum Param {
     /// For Messages
     Arg2 = b'F',
 
-    /// For Messages
-    Arg3 = b'G',
-
     /// For Messages
     Arg4 = b'H',
 

src/peerstate.rs

@@ -6,6 +6,7 @@ use num_traits::FromPrimitive;
 use crate::aheader::{Aheader, EncryptPreference};
 use crate::chat::{self, Chat};
 use crate::chatlist::Chatlist;
+use crate::config::Config;
 use crate::constants::Chattype;
 use crate::contact::{addr_cmp, Contact, ContactAddress, Origin};
 use crate::context::Context;
@@ -83,6 +84,10 @@ pub struct Peerstate {
     /// The address that introduced secondary verified key.
     pub secondary_verifier: Option<String>,
 
+    /// Row ID of the key in the `keypairs` table
+    /// that we think the peer knows as verified.
+    pub backward_verified_key_id: Option<i64>,
+
     /// True if it was detected
     /// that the fingerprint of the key used in chats with
     /// opportunistic encryption was changed after Peerstate creation.
@@ -108,6 +113,7 @@ impl Peerstate {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         }
     }
@@ -137,6 +143,7 @@ impl Peerstate {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         }
     }
@@ -148,7 +155,8 @@ impl Peerstate {
                      verified_key, verified_key_fingerprint, \
                      verifier, \
                      secondary_verified_key, secondary_verified_key_fingerprint, \
-                     secondary_verifier \
+                     secondary_verifier, \
+                     backward_verified_key_id \
                      FROM acpeerstates \
                      WHERE addr=? COLLATE NOCASE LIMIT 1;";
         Self::from_stmt(context, query, (addr,)).await
@@ -164,7 +172,8 @@ impl Peerstate {
                      verified_key, verified_key_fingerprint, \
                      verifier, \
                      secondary_verified_key, secondary_verified_key_fingerprint, \
-                     secondary_verifier \
+                     secondary_verifier, \
+                     backward_verified_key_id \
                      FROM acpeerstates  \
                      WHERE public_key_fingerprint=? \
                      OR gossip_key_fingerprint=? \
@@ -187,7 +196,8 @@ impl Peerstate {
                      verified_key, verified_key_fingerprint, \
                      verifier, \
                      secondary_verified_key, secondary_verified_key_fingerprint, \
-                     secondary_verifier \
+                     secondary_verifier, \
+                     backward_verified_key_id \
                      FROM acpeerstates  \
                      WHERE verified_key_fingerprint=? \
                      OR addr=? COLLATE NOCASE \
@@ -255,6 +265,7 @@ impl Peerstate {
                         let secondary_verifier: Option<String> = row.get("secondary_verifier")?;
                         secondary_verifier.filter(|s| !s.is_empty())
                     },
+                    backward_verified_key_id: row.get("backward_verified_key_id")?,
                     fingerprint_changed: false,
                 };
 
@@ -435,6 +446,17 @@ impl Peerstate {
         verified.is_some() && verified == self.peek_key_fingerprint(false)
     }
 
+    pub(crate) async fn is_backward_verified(&self, context: &Context) -> Result<bool> {
+        let Some(backward_verified_key_id) = self.backward_verified_key_id else {
+            return Ok(false);
+        };
+
+        let self_key_id = context.get_config_i64(Config::KeyId).await?;
+
+        let backward_verified = backward_verified_key_id == self_key_id;
+        Ok(backward_verified)
+    }
+
     /// Set this peerstate to verified
     /// Make sure to call `self.save_to_db` to save these changes
     /// Params:
@@ -510,8 +532,9 @@ impl Peerstate {
                 secondary_verified_key,
                 secondary_verified_key_fingerprint,
                 secondary_verifier,
+                backward_verified_key_id,
                 addr)
-                VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
+                VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
                 ON CONFLICT (addr)
                 DO UPDATE SET
                   last_seen = excluded.last_seen,
@@ -527,7 +550,8 @@ impl Peerstate {
                   verifier = excluded.verifier,
                   secondary_verified_key = excluded.secondary_verified_key,
                   secondary_verified_key_fingerprint = excluded.secondary_verified_key_fingerprint,
-                  secondary_verifier = excluded.secondary_verifier",
+                  secondary_verifier = excluded.secondary_verifier,
+                  backward_verified_key_id = excluded.backward_verified_key_id",
             (
                 self.last_seen,
                 self.last_seen_autocrypt,
@@ -545,6 +569,7 @@ impl Peerstate {
                     .as_ref()
                     .map(|fp| fp.hex()),
                 self.secondary_verifier.as_deref().unwrap_or(""),
+                self.backward_verified_key_id,
                 &self.addr,
             ),
         )
@@ -806,6 +831,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
 
@@ -849,6 +875,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
 
@@ -885,6 +912,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
 
@@ -951,6 +979,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
 

src/qr.rs

@@ -1057,6 +1057,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
         assert!(

src/receive_imf.rs

@@ -346,6 +346,16 @@ pub(crate) async fn receive_imf_inner(
     let verified_encryption =
         has_verified_encryption(context, &mime_parser, from_id, &to_ids).await?;
 
+    if verified_encryption == VerifiedEncryption::Verified
+        && mime_parser.get_header(HeaderDef::ChatVerified).is_some()
+    {
+        if let Some(peerstate) = &mut mime_parser.decryption_info.peerstate {
+            peerstate.backward_verified_key_id =
+                Some(context.get_config_i64(Config::KeyId).await?).filter(|&id| id > 0);
+            peerstate.save_to_db(&context.sql).await?;
+        }
+    }
+
     let received_msg = if let Some(received_msg) = received_msg {
         received_msg
     } else {
@@ -2527,6 +2537,8 @@ async fn mark_recipients_as_verified(
                     info!(context, "{verifier_addr} has verified {to_addr}.");
                     if let Some(fp) = peerstate.gossip_key_fingerprint.clone() {
                         peerstate.set_verified(PeerstateKeyType::GossipKey, fp, verifier_addr)?;
+                        peerstate.backward_verified_key_id =
+                            Some(context.get_config_i64(Config::KeyId).await?).filter(|&id| id > 0);
                         peerstate.save_to_db(&context.sql).await?;
 
                         let (to_contact_id, _) = Contact::add_or_lookup(

src/securejoin.rs

@@ -20,6 +20,7 @@ use crate::mimeparser::{MimeMessage, SystemMessage};
 use crate::param::Param;
 use crate::peerstate::{Peerstate, PeerstateKeyType};
 use crate::qr::check_qr;
+use crate::securejoin::bob::JoinerProgress;
 use crate::stock_str;
 use crate::sync::Sync::*;
 use crate::token;
@@ -173,7 +174,6 @@ async fn send_alice_handshake_msg(
     context: &Context,
     contact_id: ContactId,
     step: &str,
-    fingerprint: Option<Fingerprint>,
 ) -> Result<()> {
     let mut msg = Message {
         viewtype: Viewtype::Text,
@@ -183,9 +183,6 @@ async fn send_alice_handshake_msg(
     };
     msg.param.set_cmd(SystemMessage::SecurejoinMessage);
     msg.param.set(Param::Arg, step);
-    if let Some(fp) = fingerprint {
-        msg.param.set(Param::Arg3, fp.hex());
-    }
     msg.param.set_int(Param::GuaranteeE2ee, 1);
     chat::send_msg(
         context,
@@ -204,7 +201,9 @@ async fn info_chat_id(context: &Context, contact_id: ContactId) -> Result<ChatId
     Ok(chat_id_blocked.id)
 }
 
-async fn fingerprint_equals_sender(
+/// Checks fingerprint and marks the contact as forward verified
+/// if fingerprint matches.
+async fn verify_sender_by_fingerprint(
     context: &Context,
     fingerprint: &Fingerprint,
     contact_id: ContactId,
@@ -223,13 +222,17 @@ async fn fingerprint_equals_sender(
         }
     };
 
-    if let Some(peerstate) = peerstate {
+    if let Some(mut peerstate) = peerstate {
         if peerstate
             .public_key_fingerprint
             .as_ref()
             .filter(|&fp| fp == fingerprint)
             .is_some()
         {
+            let verifier = contact.get_addr().to_owned();
+            peerstate.set_verified(PeerstateKeyType::PublicKey, fingerprint.clone(), verifier)?;
+            peerstate.prefer_encrypt = EncryptPreference::Mutual;
+            peerstate.save_to_db(&context.sql).await?;
             return Ok(true);
         }
     }
@@ -327,7 +330,6 @@ pub(crate) async fn handle_securejoin_handshake(
                 context,
                 contact_id,
                 &format!("{}-auth-required", &step[..2]),
-                None,
             )
             .await
             .context("failed sending auth-required handshake message")?;
@@ -372,7 +374,7 @@ pub(crate) async fn handle_securejoin_handshake(
                 .await?;
                 return Ok(HandshakeMessage::Ignore);
             }
-            if !fingerprint_equals_sender(context, &fingerprint, contact_id).await? {
+            if !verify_sender_by_fingerprint(context, &fingerprint, contact_id).await? {
                 could_not_establish_secure_connection(
                     context,
                     contact_id,
@@ -408,8 +410,14 @@ pub(crate) async fn handle_securejoin_handshake(
                 .await?
                 .get_addr()
                 .to_owned();
-            let fingerprint_found =
-                mark_peer_as_verified(context, fingerprint.clone(), contact_addr).await?;
+            let backward_verified = true;
+            let fingerprint_found = mark_peer_as_verified(
+                context,
+                fingerprint.clone(),
+                contact_addr,
+                backward_verified,
+            )
+            .await?;
             if !fingerprint_found {
                 could_not_establish_secure_connection(
                     context,
@@ -467,14 +475,9 @@ pub(crate) async fn handle_securejoin_handshake(
                     mime_message.timestamp_sent,
                 )
                 .await?;
-                send_alice_handshake_msg(
-                    context,
-                    contact_id,
-                    "vc-contact-confirm",
-                    Some(fingerprint),
-                )
-                .await
-                .context("failed sending vc-contact-confirm message")?;
+                send_alice_handshake_msg(context, contact_id, "vc-contact-confirm")
+                    .await
+                    .context("failed sending vc-contact-confirm message")?;
 
                 inviter_progress(context, contact_id, 1000);
             }
@@ -484,11 +487,18 @@ pub(crate) async fn handle_securejoin_handshake(
         ====             Bob - the joiner's side             ====
         ====   Step 7 in "Setup verified contact" protocol   ====
         =======================================================*/
-        "vc-contact-confirm" => match BobState::from_db(&context.sql).await? {
-            Some(bobstate) => bob::handle_contact_confirm(context, bobstate, mime_message).await,
-            None => Ok(HandshakeMessage::Ignore),
-        },
+        "vc-contact-confirm" => {
+            if let Some(mut bobstate) = BobState::from_db(&context.sql).await? {
+                if !bobstate.is_msg_expected(context, step.as_str()) {
+                    warn!(context, "Unexpected vc-contact-confirm.");
+                    return Ok(HandshakeMessage::Ignore);
+                }
 
+                bobstate.step_contact_confirm(context).await?;
+                bobstate.emit_progress(context, JoinerProgress::Succeeded);
+            }
+            Ok(HandshakeMessage::Ignore)
+        }
         "vg-member-added" => {
             let Some(member_added) = mime_message
                 .get_header(HeaderDef::ChatGroupMemberAdded)
@@ -496,23 +506,27 @@ pub(crate) async fn handle_securejoin_handshake(
             else {
                 warn!(
                     context,
-                    "vg-member-added without Chat-Group-Member-Added header"
+                    "vg-member-added without Chat-Group-Member-Added header."
                 );
                 return Ok(HandshakeMessage::Propagate);
             };
             if !context.is_self_addr(member_added).await? {
                 info!(
                     context,
-                    "Member {member_added} added by unrelated SecureJoin process"
+                    "Member {member_added} added by unrelated SecureJoin process."
                 );
                 return Ok(HandshakeMessage::Propagate);
             }
-            match BobState::from_db(&context.sql).await? {
-                Some(bobstate) => {
-                    bob::handle_contact_confirm(context, bobstate, mime_message).await
+            if let Some(mut bobstate) = BobState::from_db(&context.sql).await? {
+                if !bobstate.is_msg_expected(context, step.as_str()) {
+                    warn!(context, "Unexpected vg-member-added.");
+                    return Ok(HandshakeMessage::Propagate);
                 }
-                None => Ok(HandshakeMessage::Propagate),
+
+                bobstate.step_contact_confirm(context).await?;
+                bobstate.emit_progress(context, JoinerProgress::Succeeded);
             }
+            Ok(HandshakeMessage::Propagate)
         }
 
         "vg-member-added-received" | "vc-contact-confirm-received" => {
@@ -526,23 +540,25 @@ pub(crate) async fn handle_securejoin_handshake(
     }
 }
 
-/// observe_securejoin_on_other_device() must be called when a self-sent securejoin message is seen.
+/// Observe self-sent Securejoin message.
 ///
-/// in a multi-device-setup, there may be other devices that "see" the handshake messages.
-/// if the seen messages seen are self-sent messages encrypted+signed correctly with our key,
-/// we can make some conclusions of it:
+/// In a multi-device-setup, there may be other devices that "see" the handshake messages.
+/// If we see self-sent messages encrypted+signed correctly with our key,
+/// we can make some conclusions of it.
 ///
-/// - if we see the self-sent-message vg-member-added/vc-contact-confirm,
-///   we know that we're an inviter-observer.
-///   The inviting device has marked a peer as verified on vg-request-with-auth/vc-request-with-auth
-///   before sending vg-member-added/vc-contact-confirm - so, if we observe vg-member-added/vc-contact-confirm,
-///   we can mark the peer as verified as well.
+/// If we see self-sent {vc,vg}-request-with-auth,
+/// we know that we are Bob (joiner-observer)
+/// that just marked peer (Alice) as forward-verified
+/// either after receiving {vc,vg}-auth-required
+/// or immediately after scanning the QR-code
+/// if the key was already known.
 ///
-/// - if we see the self-sent-message vg-request-with-auth/vc-request-with-auth
-///   we know that we're an joiner-observer.
-///   the joining device has marked the peer as verified
-///   before sending vg-request-with-auth/vc-request-with-auth - so, if we observe vg-member-added-received,
-///   we can mark the peer as verified as well.
+/// If we see self-sent vc-contact-confirm or vg-member-added message,
+/// we know that we are Alice (inviter-observer)
+/// that just marked peer (Bob) as forward (and backward)-verified
+/// in response to correct vc-request-with-auth message.
+///
+/// In both cases we can mark the peer as forward-verified.
 pub(crate) async fn observe_securejoin_on_other_device(
     context: &Context,
     mime_message: &MimeMessage,
@@ -556,127 +572,96 @@ pub(crate) async fn observe_securejoin_on_other_device(
         .context("Not a Secure-Join message")?;
     info!(context, "Observing secure-join message {step:?}.");
 
-    match step.as_str() {
-        "vg-request-with-auth"
-        | "vc-request-with-auth"
-        | "vg-member-added"
-        | "vc-contact-confirm" => {
-            if !encrypted_and_signed(
-                context,
-                mime_message,
-                get_self_fingerprint(context).await.as_ref(),
-            ) {
-                could_not_establish_secure_connection(
-                    context,
-                    contact_id,
-                    info_chat_id(context, contact_id).await?,
-                    "Message not encrypted correctly.",
-                )
-                .await?;
-                return Ok(HandshakeMessage::Ignore);
-            }
-            let addr = Contact::get_by_id(context, contact_id)
-                .await?
-                .get_addr()
-                .to_lowercase();
-            if mime_message.gossiped_addr.contains(&addr) {
-                let mut peerstate = match Peerstate::from_addr(context, &addr).await? {
-                    Some(p) => p,
-                    None => {
-                        could_not_establish_secure_connection(
-                            context,
-                            contact_id,
-                            info_chat_id(context, contact_id).await?,
-                            &format!("No peerstate in db for '{}' at step {}", &addr, step),
-                        )
-                        .await?;
-                        return Ok(HandshakeMessage::Ignore);
-                    }
-                };
-                let fingerprint = match peerstate.gossip_key_fingerprint.clone() {
-                    Some(fp) => fp,
-                    None => {
-                        could_not_establish_secure_connection(
-                            context,
-                            contact_id,
-                            info_chat_id(context, contact_id).await?,
-                            &format!(
-                                "No gossip key fingerprint in db for '{}' at step {}",
-                                &addr, step,
-                            ),
-                        )
-                        .await?;
-                        return Ok(HandshakeMessage::Ignore);
-                    }
-                };
-                peerstate.set_verified(PeerstateKeyType::GossipKey, fingerprint, addr)?;
-                peerstate.prefer_encrypt = EncryptPreference::Mutual;
-                peerstate.save_to_db(&context.sql).await?;
+    if !matches!(
+        step.as_str(),
+        "vg-request-with-auth" | "vc-request-with-auth" | "vg-member-added" | "vc-contact-confirm"
+    ) {
+        return Ok(HandshakeMessage::Ignore);
+    };
 
-                ChatId::set_protection_for_contact(
-                    context,
-                    contact_id,
-                    mime_message.timestamp_sent,
-                )
-                .await?;
-            } else if let Some(fingerprint) =
-                mime_message.get_header(HeaderDef::SecureJoinFingerprint)
-            {
-                // FIXME: Old versions of DC send this header instead of gossips. Remove this
-                // eventually.
-                let fingerprint = fingerprint.parse()?;
-                let fingerprint_found = mark_peer_as_verified(
-                    context,
-                    fingerprint,
-                    Contact::get_by_id(context, contact_id)
-                        .await?
-                        .get_addr()
-                        .to_owned(),
-                )
-                .await?;
-                if !fingerprint_found {
-                    could_not_establish_secure_connection(
-                        context,
-                        contact_id,
-                        info_chat_id(context, contact_id).await?,
-                        format!("Fingerprint mismatch on observing {step}.").as_ref(),
-                    )
-                    .await?;
-                    return Ok(HandshakeMessage::Ignore);
-                }
-            } else {
-                could_not_establish_secure_connection(
-                    context,
-                    contact_id,
-                    info_chat_id(context, contact_id).await?,
-                    &format!(
-                        "No gossip header for '{}' at step {}, please update Delta Chat on all \
+    if !encrypted_and_signed(
+        context,
+        mime_message,
+        get_self_fingerprint(context).await.as_ref(),
+    ) {
+        could_not_establish_secure_connection(
+            context,
+            contact_id,
+            info_chat_id(context, contact_id).await?,
+            "Message not encrypted correctly.",
+        )
+        .await?;
+        return Ok(HandshakeMessage::Ignore);
+    }
+
+    let addr = Contact::get_by_id(context, contact_id)
+        .await?
+        .get_addr()
+        .to_lowercase();
+
+    if !mime_message.gossiped_addr.contains(&addr) {
+        could_not_establish_secure_connection(
+            context,
+            contact_id,
+            info_chat_id(context, contact_id).await?,
+            &format!(
+                "No gossip header for '{}' at step {}, please update Delta Chat on all \
                         your devices.",
-                        &addr, step,
-                    ),
-                )
-                .await?;
-                return Ok(HandshakeMessage::Ignore);
-            }
-            if step.as_str() == "vg-member-added" {
-                inviter_progress(context, contact_id, 800);
-            }
-            if step.as_str() == "vg-member-added" || step.as_str() == "vc-contact-confirm" {
-                inviter_progress(context, contact_id, 1000);
-            }
-            if step.as_str() == "vg-request-with-auth" || step.as_str() == "vc-request-with-auth" {
-                // This actually reflects what happens on the first device (which does the secure
-                // join) and causes a subsequent "vg-member-added" message to create an unblocked
-                // verified group.
-                ChatId::create_for_contact_with_blocked(context, contact_id, Blocked::Not).await?;
-            }
-            Ok(if step.as_str() == "vg-member-added" {
-                HandshakeMessage::Propagate
-            } else {
-                HandshakeMessage::Ignore
-            })
-        }
-        _ => Ok(HandshakeMessage::Ignore),
+                &addr, step,
+            ),
+        )
+        .await?;
+        return Ok(HandshakeMessage::Ignore);
+    }
+
+    let Some(mut peerstate) = Peerstate::from_addr(context, &addr).await? else {
+        could_not_establish_secure_connection(
+            context,
+            contact_id,
+            info_chat_id(context, contact_id).await?,
+            &format!("No peerstate in db for '{}' at step {}", &addr, step),
+        )
+        .await?;
+        return Ok(HandshakeMessage::Ignore);
+    };
+
+    let Some(fingerprint) = peerstate.gossip_key_fingerprint.clone() else {
+        could_not_establish_secure_connection(
+            context,
+            contact_id,
+            info_chat_id(context, contact_id).await?,
+            &format!(
+                "No gossip key fingerprint in db for '{}' at step {}",
+                &addr, step,
+            ),
+        )
+        .await?;
+        return Ok(HandshakeMessage::Ignore);
+    };
+    peerstate.set_verified(PeerstateKeyType::GossipKey, fingerprint, addr)?;
+    peerstate.prefer_encrypt = EncryptPreference::Mutual;
+    peerstate.save_to_db(&context.sql).await?;
+
+    ChatId::set_protection_for_contact(context, contact_id, mime_message.timestamp_sent).await?;
+
+    if step.as_str() == "vg-member-added" {
+        inviter_progress(context, contact_id, 800);
+    }
+    if step.as_str() == "vg-member-added" || step.as_str() == "vc-contact-confirm" {
+        inviter_progress(context, contact_id, 1000);
+    }
+
+    if step.as_str() == "vg-request-with-auth" || step.as_str() == "vc-request-with-auth" {
+        // This actually reflects what happens on the first device (which does the secure
+        // join) and causes a subsequent "vg-member-added" message to create an unblocked
+        // verified group.
+        ChatId::create_for_contact_with_blocked(context, contact_id, Blocked::Not).await?;
+    }
+
+    if step.as_str() == "vg-member-added" {
+        Ok(HandshakeMessage::Propagate)
+    } else {
+        Ok(HandshakeMessage::Ignore)
     }
 }
 
@@ -724,12 +709,17 @@ async fn mark_peer_as_verified(
     context: &Context,
     fingerprint: Fingerprint,
     verifier: String,
+    backward_verified: bool,
 ) -> Result<bool> {
     let Some(ref mut peerstate) = Peerstate::from_fingerprint(context, &fingerprint).await? else {
         return Ok(false);
     };
     peerstate.set_verified(PeerstateKeyType::PublicKey, fingerprint, verifier)?;
     peerstate.prefer_encrypt = EncryptPreference::Mutual;
+    if backward_verified {
+        peerstate.backward_verified_key_id =
+            Some(context.get_config_i64(Config::KeyId).await?).filter(|&id| id > 0);
+    }
     peerstate.save_to_db(&context.sql).await?;
     Ok(true)
 }
@@ -980,6 +970,7 @@ mod tests {
             secondary_verified_key: None,
             secondary_verified_key_fingerprint: None,
             secondary_verifier: None,
+            backward_verified_key_id: None,
             fingerprint_changed: false,
         };
         peerstate.save_to_db(&bob.ctx.sql).await?;
@@ -1313,4 +1304,70 @@ First thread."#;
 
         Ok(())
     }
+
+    /// Tests that Bob gets Alice as verified
+    /// if `vc-contact-confirm` is lost but Alice then sends
+    /// a message to Bob in a verified 1:1 chat with a `Chat-Verified` header.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_lost_contact_confirm() {
+        let mut tcm = TestContextManager::new();
+        let alice = tcm.alice().await;
+        let bob = tcm.bob().await;
+        alice
+            .set_config(Config::VerifiedOneOnOneChats, Some("1"))
+            .await
+            .unwrap();
+        bob.set_config(Config::VerifiedOneOnOneChats, Some("1"))
+            .await
+            .unwrap();
+
+        let qr = get_securejoin_qr(&alice.ctx, None).await.unwrap();
+        join_securejoin(&bob.ctx, &qr).await.unwrap();
+
+        // vc-request
+        let sent = bob.pop_sent_msg().await;
+        alice.recv_msg(&sent).await;
+
+        // vc-auth-required
+        let sent = alice.pop_sent_msg().await;
+        bob.recv_msg(&sent).await;
+
+        // vc-request-with-auth
+        let sent = bob.pop_sent_msg().await;
+        alice.recv_msg(&sent).await;
+
+        // Alice has Bob verified now.
+        let contact_bob_id =
+            Contact::lookup_id_by_addr(&alice.ctx, "bob@example.net", Origin::Unknown)
+                .await
+                .expect("Error looking up contact")
+                .expect("Contact not found");
+        let contact_bob = Contact::get_by_id(&alice.ctx, contact_bob_id)
+            .await
+            .unwrap();
+        assert_eq!(contact_bob.is_verified(&alice.ctx).await.unwrap(), true);
+
+        // Alice sends vc-contact-confirm, but it gets lost.
+        let _sent_vc_contact_confirm = alice.pop_sent_msg().await;
+
+        // Bob should not yet have Alice verified
+        let contact_alice_id =
+            Contact::lookup_id_by_addr(&bob, "alice@example.org", Origin::Unknown)
+                .await
+                .expect("Error looking up contact")
+                .expect("Contact not found");
+        let contact_alice = Contact::get_by_id(&bob, contact_alice_id).await.unwrap();
+        assert_eq!(contact_alice.is_verified(&bob).await.unwrap(), false);
+
+        // Alice sends a text message to Bob.
+        let received_hello = tcm.send_recv(&alice, &bob, "Hello!").await;
+        let chat_id = received_hello.chat_id;
+        let chat = Chat::load_from_db(&bob, chat_id).await.unwrap();
+        assert_eq!(chat.is_protected(), true);
+
+        // Received text message in a verified 1:1 chat results in backward verification
+        // and Bob now marks alice as verified.
+        let contact_alice = Contact::get_by_id(&bob, contact_alice_id).await.unwrap();
+        assert_eq!(contact_alice.is_verified(&bob).await.unwrap(), true);
+    }
 }

src/securejoin/bob.rs

@@ -86,7 +86,7 @@ pub(super) async fn handle_auth_required(
         return Ok(HandshakeMessage::Ignore);
     };
 
-    match bobstate.handle_message(context, message).await? {
+    match bobstate.handle_auth_required(context, message).await? {
         Some(BobHandshakeStage::Terminated(why)) => {
             bobstate.notify_aborted(context, why).await?;
             Ok(HandshakeMessage::Done)
@@ -100,6 +100,9 @@ pub(super) async fn handle_auth_required(
                 let chat_id = bobstate.joining_chat_id(context).await?;
                 chat::add_info_msg(context, chat_id, &msg, time()).await?;
             }
+            bobstate
+                .set_peer_verified(context, message.timestamp_sent)
+                .await?;
             bobstate.emit_progress(context, JoinerProgress::RequestWithAuthSent);
             Ok(HandshakeMessage::Done)
         }
@@ -107,46 +110,6 @@ pub(super) async fn handle_auth_required(
     }
 }
 
-/// Handles `vc-contact-confirm` and `vg-member-added` handshake messages.
-///
-/// # Bob - the joiner's side
-/// ## Step 7 in the "Setup Contact protocol"
-pub(super) async fn handle_contact_confirm(
-    context: &Context,
-    mut bobstate: BobState,
-    message: &MimeMessage,
-) -> Result<HandshakeMessage> {
-    let retval = if bobstate.is_join_group() {
-        HandshakeMessage::Propagate
-    } else {
-        HandshakeMessage::Ignore
-    };
-    match bobstate.handle_message(context, message).await? {
-        Some(BobHandshakeStage::Terminated(why)) => {
-            bobstate.notify_aborted(context, why).await?;
-            Ok(HandshakeMessage::Done)
-        }
-        Some(BobHandshakeStage::Completed) => {
-            // Note this goes to the 1:1 chat, as when joining a group we implicitly also
-            // verify both contacts (this could be a bug/security issue, see
-            // e.g. https://github.com/deltachat/deltachat-core-rust/issues/1177).
-            bobstate
-                .notify_peer_verified(context, message.timestamp_sent)
-                .await?;
-            bobstate.emit_progress(context, JoinerProgress::Succeeded);
-            Ok(retval)
-        }
-        Some(_) => {
-            warn!(
-                context,
-                "Impossible state returned from handling handshake message"
-            );
-            Ok(retval)
-        }
-        None => Ok(retval),
-    }
-}
-
 /// Private implementations for user interactions about this [`BobState`].
 impl BobState {
     fn is_join_group(&self) -> bool {
@@ -156,7 +119,7 @@ impl BobState {
         }
     }
 
-    fn emit_progress(&self, context: &Context, progress: JoinerProgress) {
+    pub(crate) fn emit_progress(&self, context: &Context, progress: JoinerProgress) {
         let contact_id = self.invite().contact_id();
         context.emit_event(EventType::SecurejoinJoinerProgress {
             contact_id,
@@ -219,12 +182,9 @@ impl BobState {
         Ok(())
     }
 
-    /// Notifies the user that the SecureJoin peer is verified.
-    ///
-    /// This creates an info message in the chat being joined.
-    async fn notify_peer_verified(&self, context: &Context, timestamp: i64) -> Result<()> {
+    /// Turns 1:1 chat with SecureJoin peer into protected chat.
+    pub(crate) async fn set_peer_verified(&self, context: &Context, timestamp: i64) -> Result<()> {
         let contact = Contact::get_by_id(context, self.invite().contact_id()).await?;
-        let chat_id = self.joining_chat_id(context).await?;
         self.alice_chat()
             .set_protection(
                 context,
@@ -233,7 +193,6 @@ impl BobState {
                 Some(contact.id),
             )
             .await?;
-        context.emit_event(EventType::ChatModified(chat_id));
         Ok(())
     }
 }
@@ -242,7 +201,7 @@ impl BobState {
 ///
 /// This has an `From<JoinerProgress> for usize` impl yielding numbers between 0 and a 1000
 /// which can be shown as a progress bar.
-enum JoinerProgress {
+pub(crate) enum JoinerProgress {
     /// An error occurred.
     Error,
     /// vg-vc-request-with-auth sent.

src/securejoin/bobstate.rs

@@ -11,17 +11,19 @@ use anyhow::Result;
 use rusqlite::Connection;
 
 use super::qrinvite::QrInvite;
-use super::{encrypted_and_signed, fingerprint_equals_sender, mark_peer_as_verified};
+use super::{encrypted_and_signed, verify_sender_by_fingerprint};
 use crate::chat::{self, ChatId};
+use crate::config::Config;
 use crate::contact::{Contact, Origin};
 use crate::context::Context;
 use crate::events::EventType;
 use crate::headerdef::HeaderDef;
-use crate::key::{load_self_public_key, DcKey};
 use crate::message::{Message, Viewtype};
 use crate::mimeparser::{MimeMessage, SystemMessage};
 use crate::param::Param;
+use crate::securejoin::Peerstate;
 use crate::sql::Sql;
+use crate::tools::time;
 
 /// The stage of the [`BobState`] securejoin handshake protocol state machine.
 ///
@@ -30,14 +32,9 @@ use crate::sql::Sql;
 #[derive(Clone, Copy, Debug, Display)]
 pub enum BobHandshakeStage {
     /// Step 2 completed: (vc|vg)-request message sent.
-    ///
-    /// Note that this is only ever returned by [`BobState::start_protocol`] and never by
-    /// [`BobState::handle_message`].
     RequestSent,
     /// Step 4 completed: (vc|vg)-request-with-auth message sent.
     RequestWithAuthSent,
-    /// The protocol completed successfully.
-    Completed,
     /// The protocol prematurely terminated with given reason.
     Terminated(&'static str),
 }
@@ -92,21 +89,26 @@ impl BobState {
         invite: QrInvite,
         chat_id: ChatId,
     ) -> Result<(Self, BobHandshakeStage, Vec<Self>)> {
-        let (stage, next) =
-            if fingerprint_equals_sender(context, invite.fingerprint(), invite.contact_id()).await?
-            {
-                // The scanned fingerprint matches Alice's key, we can proceed to step 4b.
-                info!(context, "Taking securejoin protocol shortcut");
-                send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::RequestWithAuth)
-                    .await?;
-                (
-                    BobHandshakeStage::RequestWithAuthSent,
-                    SecureJoinStep::ContactConfirm,
-                )
-            } else {
-                send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::Request).await?;
-                (BobHandshakeStage::RequestSent, SecureJoinStep::AuthRequired)
-            };
+        let peer_verified =
+            verify_sender_by_fingerprint(context, invite.fingerprint(), invite.contact_id())
+                .await?;
+
+        let (stage, next);
+        if peer_verified {
+            // The scanned fingerprint matches Alice's key, we can proceed to step 4b.
+            info!(context, "Taking securejoin protocol shortcut");
+            send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::RequestWithAuth)
+                .await?;
+
+            stage = BobHandshakeStage::RequestWithAuthSent;
+            next = SecureJoinStep::ContactConfirm;
+        } else {
+            send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::Request).await?;
+
+            stage = BobHandshakeStage::RequestSent;
+            next = SecureJoinStep::AuthRequired;
+        };
+
         let (id, aborted_states) =
             Self::insert_new_db_entry(context, next, invite.clone(), chat_id).await?;
         let state = Self {
@@ -115,6 +117,12 @@ impl BobState {
             next,
             chat_id,
         };
+
+        if peer_verified {
+            // Mark 1:1 chat as verified already.
+            state.set_peer_verified(context, time()).await?;
+        }
+
         Ok((state, stage, aborted_states))
     }
 
@@ -230,13 +238,13 @@ impl BobState {
         Ok(())
     }
 
-    /// Handles the given message for the securejoin handshake for Bob.
+    /// Handles {vc,vg}-auth-required message of the securejoin handshake for Bob.
     ///
     /// If the message was not used for this handshake `None` is returned, otherwise the new
-    /// stage is returned.  Once [`BobHandshakeStage::Completed`] or
-    /// [`BobHandshakeStage::Terminated`] are reached this [`BobState`] should be destroyed,
+    /// stage is returned.  Once [`BobHandshakeStage::Terminated`] is reached this
+    /// [`BobState`] should be destroyed,
     /// further calling it will just result in the messages being unused by this handshake.
-    pub(crate) async fn handle_message(
+    pub(crate) async fn handle_auth_required(
         &mut self,
         context: &Context,
         mime_message: &MimeMessage,
@@ -256,39 +264,7 @@ impl BobState {
             info!(context, "{} message out of sync for BobState", step);
             return Ok(None);
         }
-        match step.as_str() {
-            "vg-auth-required" | "vc-auth-required" => {
-                self.step_auth_required(context, mime_message).await
-            }
-            "vg-member-added" | "vc-contact-confirm" => {
-                self.step_contact_confirm(context, mime_message).await
-            }
-            _ => {
-                warn!(context, "Invalid step for BobState: {}", step);
-                Ok(None)
-            }
-        }
-    }
 
-    /// Returns `true` if the message is expected according to the protocol.
-    fn is_msg_expected(&self, context: &Context, step: &str) -> bool {
-        let variant_matches = match self.invite {
-            QrInvite::Contact { .. } => step.starts_with("vc-"),
-            QrInvite::Group { .. } => step.starts_with("vg-"),
-        };
-        let step_matches = self.next.matches(context, step);
-        variant_matches && step_matches
-    }
-
-    /// Handles a *vc-auth-required* or *vg-auth-required* message.
-    ///
-    /// # Bob - the joiner's side
-    /// ## Step 4 in the "Setup Contact protocol", section 2.1 of countermitm 0.10.0
-    async fn step_auth_required(
-        &mut self,
-        context: &Context,
-        mime_message: &MimeMessage,
-    ) -> Result<Option<BobHandshakeStage>> {
         info!(
             context,
             "Bob Step 4 - handling {{vc,vg}}-auth-required message."
@@ -303,14 +279,19 @@ impl BobState {
                 .await?;
             return Ok(Some(BobHandshakeStage::Terminated(reason)));
         }
-        if !fingerprint_equals_sender(context, self.invite.fingerprint(), self.invite.contact_id())
-            .await?
+        if !verify_sender_by_fingerprint(
+            context,
+            self.invite.fingerprint(),
+            self.invite.contact_id(),
+        )
+        .await?
         {
             self.update_next(&context.sql, SecureJoinStep::Terminated)
                 .await?;
             return Ok(Some(BobHandshakeStage::Terminated("Fingerprint mismatch")));
         }
         info!(context, "Fingerprint verified.",);
+
         self.update_next(&context.sql, SecureJoinStep::ContactConfirm)
             .await?;
         self.send_handshake_message(context, BobHandshakeMsg::RequestWithAuth)
@@ -318,36 +299,39 @@ impl BobState {
         Ok(Some(BobHandshakeStage::RequestWithAuthSent))
     }
 
+    /// Returns `true` if the message is expected according to the protocol.
+    pub(crate) fn is_msg_expected(&self, context: &Context, step: &str) -> bool {
+        let variant_matches = match self.invite {
+            QrInvite::Contact { .. } => step.starts_with("vc-"),
+            QrInvite::Group { .. } => step.starts_with("vg-"),
+        };
+        let step_matches = self.next.matches(context, step);
+        variant_matches && step_matches
+    }
+
     /// Handles a *vc-contact-confirm* or *vg-member-added* message.
     ///
     /// # Bob - the joiner's side
     /// ## Step 7 in the "Setup Contact protocol", section 2.1 of countermitm 0.10.0
-    ///
-    /// This deviates from the protocol by also sending a confirmation message in response
-    /// to the *vc-contact-confirm* message.  This has no specific value to the protocol and
-    /// is only done out of symmetry with *vg-member-added* handling.
-    async fn step_contact_confirm(
-        &mut self,
-        context: &Context,
-        mime_message: &MimeMessage,
-    ) -> Result<Option<BobHandshakeStage>> {
-        info!(
-            context,
-            "Bob Step 7 - handling vc-contact-confirm/vg-member-added message."
-        );
-        mark_peer_as_verified(
-            context,
-            self.invite.fingerprint().clone(),
-            mime_message.from.addr.to_string(),
-        )
-        .await?;
+    pub(crate) async fn step_contact_confirm(&mut self, context: &Context) -> Result<()> {
+        let fingerprint = self.invite.fingerprint();
+        let Some(ref mut peerstate) = Peerstate::from_fingerprint(context, fingerprint).await?
+        else {
+            return Ok(());
+        };
+
+        // Mark peer as backward verified.
+        peerstate.backward_verified_key_id =
+            Some(context.get_config_i64(Config::KeyId).await?).filter(|&id| id > 0);
+        peerstate.save_to_db(&context.sql).await?;
+
         Contact::scaleup_origin_by_id(context, self.invite.contact_id(), Origin::SecurejoinJoined)
             .await?;
         context.emit_event(EventType::ContactsChanged(None));
 
         self.update_next(&context.sql, SecureJoinStep::Completed)
             .await?;
-        Ok(Some(BobHandshakeStage::Completed))
+        Ok(())
     }
 
     /// Sends the requested handshake message to Alice.
@@ -392,10 +376,6 @@ async fn send_handshake_message(
         }
     };
 
-    // Sends our own fingerprint in the Secure-Join-Fingerprint header.
-    let bob_fp = load_self_public_key(context).await?.fingerprint();
-    msg.param.set(Param::Arg3, bob_fp.hex());
-
     // Sends the grpid in the Secure-Join-Group header.
     if let QrInvite::Group { ref grpid, .. } = invite {
         msg.param.set(Param::Arg4, grpid);

src/sql/migrations.rs

@@ -886,6 +886,20 @@ CREATE INDEX msgs_status_updates_index2 ON msgs_status_updates (uid);
         sql.set_db_version_in_cache(version).await?;
     }
 
+    if dbversion < 109 {
+        sql.execute_migration(
+            r#"ALTER TABLE acpeerstates
+               ADD COLUMN backward_verified_key_id -- What we think the contact has as our verified key
+               INTEGER;
+               UPDATE acpeerstates
+               SET backward_verified_key_id=(SELECT value FROM config WHERE keyname='key_id')
+               WHERE verified_key IS NOT NULL
+               "#,
+            109,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?

src/test_utils.rs

@@ -1047,7 +1047,8 @@ fn print_logevent(logevent: &LogEvent) {
     }
 }
 
-/// Saves the other account's public key as verified.
+/// Saves the other account's public key as verified
+/// and peerstate as backwards verified.
 pub(crate) async fn mark_as_verified(this: &TestContext, other: &TestContext) {
     let mut peerstate = Peerstate::from_header(
         &EncryptHelper::new(other).await.unwrap().get_aheader(),
@@ -1063,6 +1064,7 @@ pub(crate) async fn mark_as_verified(this: &TestContext, other: &TestContext) {
 
     peerstate.verified_key = peerstate.public_key.clone();
     peerstate.verified_key_fingerprint = peerstate.public_key_fingerprint.clone();
+    peerstate.backward_verified_key_id = Some(this.get_config_i64(Config::KeyId).await.unwrap());
 
     peerstate.save_to_db(&this.sql).await.unwrap();
 }