PGP contacts #6796

link2xt · 2025-04-12T03:19:34Z

JSON-RPC: reset_contact_encryption API is removed.
In deltachat-rpc-client API Contact.reset_encryption is removed.
"Group tracking plugin" in legacy Python API was removed because it relied on parsing email addresses from system messages with regexps.

Known multi-device (non-)issue: pinning a 1:1 chat from legacy client pins email chat via sync message.

Still missing, not all of this should be fixed before merging but at least CI should pass:

Hocuri

Just 3 small things I was wondering while skimming over

deltachat-repl/src/cmdline.rs

src/securejoin/securejoin_tests.rs

Hocuri · 2025-04-15T19:16:52Z

src/test_utils.rs

+    /// This first creates a contact, but does not import the key,
+    /// so may create a PGP-contact with a fingerprint
+    /// but without the key.
+    pub async fn get_pgp_chat(&self, other: &TestContext) -> Chat {


Since PGP chats are the more common type of chat now, I think it would make sense to call this function get_chat() and the other one get_email_chat() or get_unencrypted_chat() or similar. But it's not that important, e.g. if this would mean that a lot of test have to be changed.

adbenitez · 2025-04-16T16:20:12Z

a way to know if a chat is an encrypted chat is missing, currently there is only "isPgpContact" that can be used to mark contacts/1:1 chat but for unencrypted groups/threads of classic email non-pgp contacts there is no API to recognize such chats and also put the "classic email" marker as for classic email contacts

Hocuri · 2025-04-18T20:42:37Z

deltachat-jsonrpc/src/api/types/chat.rs

@@ -187,6 +191,7 @@ impl BasicChat {
            id: chat_id,
            name: chat.name.clone(),
            is_protected: chat.is_protected(),
+            is_encrypted: chat.is_encrypted(context).await?,


This adds up to 2 additional database calls to loading BasicChat, though I can't think of a good way to avoid it, so maybe it's fine. Or maybe it should only be added to FullChat, idk.

Hocuri · 2025-04-20T19:42:57Z

src/contact.rs

+    ) -> Result<(ContactId, Modifier)> {
+        Self::add_or_lookup_ex(context, name, addr, "", origin).await
+    }
+
    /// Lookup a contact and create it if it does not exist yet.
    /// The contact is identified by the email-address, a name and an "origin" can be given.


The comment here should explain how fingerprint & addr parameters relate; the way I understand the code:

/// If `fingerprint` is passed, a PGP-contact with this fingerprint added / looked up. /// Otherwise, an email-contact with `addr` is added / looked up. /// A name and an "origin" can be given.

Hocuri · 2025-04-20T20:17:19Z

src/contact.rs

+        if !fingerprint.is_empty() {
+            let fingerprint_self = load_self_public_key(context).await?.dc_fingerprint().hex();
+            if fingerprint == fingerprint_self {
+                return Ok((ContactId::SELF, sth_modified));
+            }
+        }


Loading the self fingerprint everytime a contact is looked up, just to check whether it's ContactId::SELF, seems unnecessarily slow to me.

I wonder if we can simplify & speed up things by not using the kepairs table anymore, and instead storing the private and public key in configs. This would mean that it's not possible to have multiple self keys anymore, but I'm not sure there is anyone who is actually making use of the fact that DC theoretically supports using multiple self keys. It would also mean that we have to store the key as base64, not sure about the consequences there. An alternative would be to store the self fingerprint in a OnceCell, since we don't allow changing the primary key anyway.

But we can open an issue for this, and do it in a follow-up PR.

src/contact.rs

src/context.rs

src/mimefactory.rs

Hocuri · 2025-04-22T16:40:50Z

src/receive_imf.rs

-    to_ids: &[ContactId],
-    past_ids: &[ContactId],
+    to_ids: &[Option<ContactId>],
+    past_ids: &[Option<ContactId>],


Would be good to put a comment what a None element means.

src/receive_imf.rs

deltachat-jsonrpc/src/api/types/chat.rs

…ent addr If a contact did AEAP in the past, then it can happen that there are multiple contacts with the same email address but different keys. In this case, we should make sure that we don't send messages to a different email address all of a sudden, and that there aren't two 1:1 chats with the same contact. Therefore, if the PGP contact has a different email address than the email contact, we keep the email contact in 1:1 chats.

Hocuri

Another day, another set of review comments

deltachat-rpc-client/tests/test_securejoin.py

python/tests/test_0_complex_or_slow.py

Hocuri · 2025-05-06T20:04:31Z

python/tests/test_0_complex_or_slow.py

@@ -474,7 +397,7 @@ def test_verified_group_vs_delete_server_after(acfactory, tmp_path, lp):
    ac2_offl.start_io()
    msg_in = ac2_offl._evtracker.wait_next_incoming_message()
    assert not msg_in.is_system_message()
-    assert msg_in.text.startswith("[The message was sent with non-verified encryption")
+    assert msg_in.text.startswith("[The message was sent by non-verified contact")


Just a question: IIRC plan is to show the verified icon iff all group members are verified? In this case, we won't need this message anymore. But this can also come in a subsequent PR.

Yes, I would like to get rid of this message, but this is out of scope for this PR which is already large.

Hocuri · 2025-05-06T21:37:26Z

src/receive_imf.rs

+    let to_ids: Vec<Option<ContactId>>;
+    let past_ids: Vec<Option<ContactId>>;


That's a lot of code to calculate the to and past ids, would be nice to extract a function here:

async fn get_to_and_past_contacts( context: &Context, mime_parser: &MimeMessage, incoming_origin: Origin, ) -> Result<(Vec<Option<ContactId>>, Vec<Option<ContactId>>)> { let chat_id = if let Some(grpid) = mime_parser.get_chat_group_id() { ... past_ids = add_or_lookup_contacts_by_address_list( context, &mime_parser.past_members, Origin::Hidden, ) .await?; }; Ok((to_ids, past_ids)) }

let (to_ids, past_ids) = get_to_and_past_contacts(context, &mime_parser, incoming_origin).await?;

Hocuri · 2025-05-07T14:01:10Z

src/receive_imf.rs

+/// * `is_partial_download`: the message is partially downloaded.
+///   We only know the email address and not the contact fingerprint,
+///   but try to assign the message to some PGP-contact.
+///   If we get it wrong, the message will be placed into the correct
+///   chat after downloading.


What about unencrypted emails that are partially downloaded? They should be assigned to the email contact, not to some PGP contact with this email address. Maybe there is some code for this already and I just haven't seen it, otherwise we need a check for whether Content-Type is multipart/encrypted.

We don't know if partially downloaded email is encrypted or not, we don't even prefetch content-type. Messages are assigned to some PGP-contact. After downloading, they will go to email-contact.

But this is not about prefetch but about partial downloads? And for partial downloads, we fetch the full headers, which includes Content-Type:

const BODY_PARTIAL: &str = "(FLAGS RFC822.SIZE BODY.PEEK[HEADER])";

Co-authored-by: Hocuri <hocuri@gmx.de>

deltachat-jsonrpc/src/api.rs

link2xt · 2025-05-07T19:42:52Z

There is a problem with 1:1 chat assignment. If there is no Autocrypt-Gossip on outgoing message, then to_id gets converted to email contact. If there is also a Chat-Verified header, receive_imf fails with an error like DeltaChat: [accId=1] src/imap.rs:1456: receive_imf error: Non-PGP contact Contact#10 cannot be verified.. and the message does not show up at all. So we should assign by References or In-Reply-To and 1:1 chat before converting to_ids.

Hocuri · 2025-05-08T13:03:26Z

src/receive_imf.rs

@@ -2397,27 +2486,32 @@ async fn apply_group_changes(
    }

    if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
-        removed_id = Contact::lookup_id_by_addr(context, removed_addr, Origin::Unknown).await?;
+        removed_id = lookup_pgp_contact_by_address(context, removed_addr, chat_id).await?;


Consider the following case:

Alice's current key is member of a group. Her past key (on the same email address) is a past member of the group, since she reinstalled DC once without restoring a backup.

Bob removes Alice from the group.

This code looks like that might not work reliably? Because lookup_pgp_contact_by_address() also looks up past contacts, so that if Bob is unlucky, then this will try to remove Alice's old key again, which won't work of course, because Alice's old key already is a past member.

Or will old-Alice-key be removed past-members as soon as new-Alice-key is added, and everything is fine?

src/receive_imf.rs

src/chat.rs

Hocuri · 2025-05-08T21:36:44Z

src/chat.rs

+                    SyncAction::Block => {
+                        return contact::set_blocked(self, Nosync, contact_id, true).await
+                    }


Just a non-actionable remark: It's a bit weird that this calls contact::set_blocked(.., true), and then a few lines below, chat_id.block_ex() is called, which again calls contact::set_blocked(.., true). But how (un)blocking a chat and (un)blocking a contact interacts is just super weird anyways. Not sure if anyone really uses the "block" feature, a spammer can just create a new account anyways, and if you want a chat to be out of your view you can archive&mute it. So, maybe it's fine that this feature is not too "polished".

Doing nothing here, and relying on chat_id.block.ex() to also block the contact, would be even weirder, so, I'm not proposing to change anything here.

link2xt mentioned this pull request Apr 12, 2025

WIP: PGP-contacts #6683

Closed

link2xt changed the title ~~Link2xt/pgp contacts~~ WIP: PGP contacts Apr 12, 2025

link2xt force-pushed the link2xt/pgp-contacts branch 2 times, most recently from 7acd920 to 280e5dd Compare April 13, 2025 05:49

adbenitez mentioned this pull request Apr 14, 2025

mark classic/unencrypted email contacts and threads/groups as such deltachat/deltachat-android#3734

Open

9 tasks

link2xt force-pushed the link2xt/pgp-contacts branch from 7e5dab4 to 479879a Compare April 14, 2025 19:13

Hocuri reviewed Apr 15, 2025

View reviewed changes

link2xt force-pushed the link2xt/pgp-contacts branch 2 times, most recently from ebe2a09 to cac8d20 Compare April 16, 2025 16:13

link2xt force-pushed the link2xt/pgp-contacts branch 2 times, most recently from 625f8f4 to c87c241 Compare April 21, 2025 21:24

Hocuri reviewed Apr 22, 2025

View reviewed changes

This was referenced Apr 23, 2025

fix: Allow to send to chats after failed securejoin again #6817

Merged

secure-join timeout is not useful for chatmail #6706

Open

link2xt force-pushed the link2xt/pgp-contacts branch from fb034c4 to ea48d78 Compare April 23, 2025 22:59

Hocuri mentioned this pull request Apr 24, 2025

[WIP] feat: Migration for PGP-contacts #6818

Merged

5 tasks

link2xt force-pushed the link2xt/pgp-contacts branch 2 times, most recently from 918277f to 9cf065a Compare April 30, 2025 06:32

adbenitez reviewed Apr 30, 2025

View reviewed changes

deltachat-jsonrpc/src/api/types/chat.rs Show resolved Hide resolved

adbenitez mentioned this pull request Apr 30, 2025

Migrate to new PGP contacts API deltachat/deltachat-desktop#5059

Open

12 tasks

link2xt force-pushed the link2xt/pgp-contacts branch 2 times, most recently from 1cff5d4 to 907628e Compare May 2, 2025 17:13

link2xt marked this pull request as ready for review May 2, 2025 17:53

link2xt added 6 commits May 6, 2025 14:56

api!: remove reset_encryption() APIs

344495d

PGP-contacts

77ffd3d

remove allow_aeap

6b964ce

TODO: mark as verified

d51b1f3

Encrypt all messages to chats with a group ID

d0b37fa

test fixup

156b636

Hocuri and others added 6 commits May 6, 2025 19:22

remove TODO

fc92ee0

unprotect 1:1 chats with email contacts

3c22c70

deal with NULL verifier in acpeerstates

4ee7e8a

mark device chat as encrypted

5e16777

hide non-pgp contacts from contact list

b7284c0

link2xt force-pushed the link2xt/pgp-contacts branch from dbade62 to b7284c0 Compare May 6, 2025 22:43

link2xt added 2 commits May 6, 2025 23:28

rustfmt

8748b79

fix python tests

171034e

link2xt changed the title ~~WIP: PGP contacts~~ PGP contacts May 7, 2025

Hocuri reviewed May 7, 2025

View reviewed changes

test fixup

34eeb5a

Co-authored-by: Hocuri <hocuri@gmx.de>

link2xt commented May 7, 2025

View reviewed changes

deltachat-jsonrpc/src/api.rs Show resolved Hide resolved

nicodh mentioned this pull request May 7, 2025

Pgp contacts deltachat/deltachat-desktop#5090

Draft

Hocuri and others added 6 commits May 8, 2025 14:49

Add doc comment for get_migration_error()

9bcb231

typo fix

b29f88c

typo fix

83a1a91

remove dots from errors

5dfe539

failing test for chat assignment

eee176a

assign outgoing chat messages by references

bc43763

Hocuri reviewed May 8, 2025

View reviewed changes

link2xt added 6 commits May 9, 2025 20:32

comment why we dont assign the message to chat before recipient lookup

0ec760a

remove TODO

1a2dfc2

remove FIXME

c99ed72

missingkey

ae77e3a

remove securejoin timeout

d3ea81d

clippy

1684809

link2xt force-pushed the link2xt/pgp-contacts branch from 9bd2506 to 1684809 Compare May 10, 2025 21:36

link2xt mentioned this pull request May 10, 2025

canSend is true before encryption suceeded #6847

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PGP contacts #6796

PGP contacts #6796

link2xt commented Apr 12, 2025 •

edited

Loading

Hocuri left a comment

Hocuri Apr 15, 2025

adbenitez commented Apr 16, 2025

Hocuri Apr 18, 2025

Hocuri Apr 20, 2025

Hocuri Apr 20, 2025

Hocuri Apr 22, 2025

Hocuri left a comment

Hocuri May 6, 2025

link2xt May 7, 2025

Hocuri May 6, 2025

Hocuri May 7, 2025

link2xt May 7, 2025

Hocuri May 7, 2025

link2xt commented May 7, 2025

Hocuri May 8, 2025

Hocuri May 8, 2025 •

edited

Loading

		let to_ids: Vec<Option<ContactId>>;
		let past_ids: Vec<Option<ContactId>>;

PGP contacts #6796

Are you sure you want to change the base?

PGP contacts #6796

Conversation

link2xt commented Apr 12, 2025 • edited Loading

Hocuri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adbenitez commented Apr 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hocuri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

link2xt commented May 7, 2025

Choose a reason for hiding this comment

Hocuri May 8, 2025 • edited Loading

Choose a reason for hiding this comment

link2xt commented Apr 12, 2025 •

edited

Loading

Hocuri May 8, 2025 •

edited

Loading