Bidirectional verification

deltachat-rpc-client/tests/test_securejoin.py

@@ -4,7 +4,7 @@
 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 => {

src/param.rs

@@ -84,7 +84,7 @@ pub enum Param {
     /// For Messages
     Arg2 = b'F',
 
-    /// For Messages
+    /// `Secure-Join-Fingerprint` header for `{vc,vg}-request-with-auth` messages.
     Arg3 = b'G',
 
     /// For Messages

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,24 @@ 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 {
+            // NOTE: it might be better to remember ID of the key
+            // that we used to decrypt the message, but
+            // it is unlikely that default key ever changes
+            // as it only happens when user imports a new default key.
+            //
+            // Backward verification is not security-critical,
+            // it is only needed to avoid adding user who does not
+            // have our key as verified to protected chats.
+            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 +2545,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(