Skip to content

refactor: import Autocrypt-Gossip keys without checking the addresses - #8638

Open
link2xt wants to merge 1 commit into
mainfrom
link2xt/autocrypt-gossip-import-no-checks
Open

refactor: import Autocrypt-Gossip keys without checking the addresses#8638
link2xt wants to merge 1 commit into
mainfrom
link2xt/autocrypt-gossip-import-no-checks

Conversation

@link2xt

@link2xt link2xt commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

It is safe to import any keys into the keychain.
Keys can anyway be imported from vCards
and Autocrypt headers without any checks.

These checks are from the time before we had key-contacts and maintained Autocrypt peerstates table.

It is safe to import any keys into the keychain.
Keys can anyway be imported from vCards
and Autocrypt headers without any checks.

These checks are from the time before we had key-contacts
and maintained Autocrypt `peerstates` table.
@link2xt
link2xt force-pushed the link2xt/autocrypt-gossip-import-no-checks branch from b75ee18 to 0092e25 Compare August 28, 2026 21:08
@link2xt

link2xt commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

This is from @Hocuri comment at #8620 (comment)

Comment thread src/mimeparser.rs
@@ -555,8 +555,7 @@ impl MimeMessage {
// but only if the mail was correctly signed. Probably it's ok to not require
// encryption here, but let's follow the standard.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably also drop this check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autocrypt/gossip and securejoin will need a concerted revisit at some point, but i'd ask for caution now going for "refactors" when they drop checks in this area. Tests still passing after removing checks does not mean much, if the checks were never covered by tests.

@link2xt
link2xt marked this pull request as ready for review August 28, 2026 21:14

@hpk42 hpk42 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is just a "refactor", when it's dropping checks in the autocrypt/securejoin handling which is known for intricacies. Existing tests keep passing, but the removed code was not actually tested so that doesn't say much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants