1657111 - openpgp account settings should hide external gnupg keys if pref is false

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Kai Engert [:KaiE:]]

If a user switches mail.openpgp.allow_external_gnupg back to false, the account settings should no longer show/offer an external key ID.

If the account's setting is_gnupg_key_id is true, then the value of openpgp_key_id should be ignored. The key configuration should be treated as None.

Alessandro, does this make sense?

Comment 1

[Kai Engert [:KaiE:]]

Besides the UI change I mentioned above, I think I should also work on an additional backend code change.

The code that uses the openpgp_key_id should ignore it, if allow_external_gnupg is false, and is_gnupg_key_id is true. (Currently it would attempt to look for openpgp_key_id as a RNP key.)

Comment 2

[Alessandro Castellani [:aleca]]

If a user switches mail.openpgp.allow_external_gnupg back to false, the account settings should no longer show/offer an external key ID.

Are you sure about this?
Shouldn't that option only affect the ability to add an external key, but if the user already has one we should allow it to be deleted, or at least let the user be aware that an external key is still registered with his identity?

The code that uses the openpgp_key_id should ignore it, if allow_external_gnupg is false, and is_gnupg_key_id is true. (Currently it would attempt to look for openpgp_key_id as a RNP key.)

Sorry, I'm a bit confused by this.
Wouldn't be easier to keep the allow_external_gnupg pref only for the Key Wizard, and let the user manage all the keys he has created/saved independently?

I'm saying this because, I think, with the external tag it's pretty obvious which key is which and we won't mislead the user in using an external key when they don't want it.

Comment 3

[Alessandro Castellani [:aleca]]

I think I found another small UI issue:

• Have 2 identities, one with a regularly created OpenPGP Key, the other with no keys.
• Configure an external key for each identity.
• The second identity, the one with no keys and only the external key, should be set to None.
• The first identity, the one with the external key and another normal key, should have the normal key selected.

In the Account Settings, switching between the two e2e pages of the two identities causes the None radio button of the second identity to be unselected, even if the configuration remains correct and the UI of the None selection gets properly highlighted.

Comment 4

[Alessandro Castellani [:aleca]]

The issues listed in comment 3 were fixed in bug 1663422.

Kai, would you have time to answer my questions in comment 2?

Comment 5

[Kai Engert [:KaiE:]]

(In reply to Alessandro Castellani (:aleca) from comment #2)

If a user switches mail.openpgp.allow_external_gnupg back to false, the account settings should no longer show/offer an external key ID.

Are you sure about this?
Shouldn't that option only affect the ability to add an external key, but if the user already has one we should allow it to be deleted, or at least let the user be aware that an external key is still registered with his identity?

If that pref is false, the backend code will never use an external ID.
Showing the external key ID will mislead the user into thinking we might use it.

The code that uses the openpgp_key_id should ignore it, if allow_external_gnupg is false, and is_gnupg_key_id is true. (Currently it would attempt to look for openpgp_key_id as a RNP key.)

Sorry, I'm a bit confused by this.
Wouldn't be easier to keep the allow_external_gnupg pref only for the Key Wizard, and let the user manage all the keys he has created/saved independently?

The pref controls what happens in the backend in multiple places. It's not limited to using a configured key id. It also controls if we attempt to load an external third party library. It controls if we attempt decryption attempts to the external third party code (which is independent of configured key IDs).

That pref is designed to disable all use of the external gnupg crypto library interaction.

Comment 6

[Alessandro Castellani [:aleca]]

Sounds good, thanks for the detailed explanation.
Patch incoming.

Comment 7

[Alessandro Castellani [:aleca]]

Attachment: 1657111-external-gnupg.diff

This should do it.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/79e7f41d84a5
Hide external GnuPG key if allow_external_gnupg pref is false. r=KaiE

Comment 9

[Kai Engert [:KaiE:]]

Comment on attachment 9174811 [details] [diff] [review]
1657111-external-gnupg.diff

OpenPGP edge case correctness fix, let's bake in Beta.

Comment 10

[Wayne Mery (:wsmwk)]

Comment on attachment 9174811 [details] [diff] [review]
1657111-external-gnupg.diff

[Triage Comment]
Approved for beta

Comment 11

[Rob Lemley [:rjl]]

Thunderbird 81.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/e15650b0c25f

Comment 12

[Kai Engert [:KaiE:]]

Comment on attachment 9174811 [details] [diff] [review]
1657111-external-gnupg.diff

Should be low risk to take this OpenPGP correctness fix on esr78

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9174811 [details] [diff] [review]
1657111-external-gnupg.diff

[Triage Comment]
Approved for esr78

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 78.3.0:
https://hg.mozilla.org/releases/comm-esr78/rev/69b0e246b3f0