Closed Bug 1874715 Opened 8 months ago Closed 4 months ago

Warn user about imported OpenPGP secret keys that advertise unsupported features

Categories

(MailNews Core :: Security: OpenPGP, defect, P1)

Product:
MailNews Core
Component:
Security: OpenPGP
Type:
defect

Tracking

(thunderbird_esr115 wontfix)

Status:
RESOLVED FIXED
Milestone:
128 Branch
Tracking Flags:
Tracking Status
thunderbird_esr115 --- wontfix

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 4 obsolete files)

Bug 1874715 - Warn about imported or existing OpenPGP secret keys that advertise unsupported features. r=mkmelin
48 bytes, text/x-phabricator-request
Details | Review
1581796-info.png
40.81 KB, image/png
Details
noname-feat-sec.asc (password: x)
5.07 KB, text/plain
Details

When importing an OpenPGP secret key into Thunderbird, that key might be either a backup, or a key that was created with other software.

The other software might have set OpenPGP feature flags, which Thunderbird doesn't support.

If Thunderbird publishes such keys, for example by including them as attachments in email to correspondents, the software used by the correspondent will draw incorrect conclusions about our technical abilities.

A recent example is GnuPG 2.4, which by default enables the --rfc4880bis mode.
This mode is related to the draft-koch-librepgp specification, which Thunderbird doesn't support (at this time).

Keys created with that mode will advertise support for two new features:

  • v5 keys
  • AEAD encryption

At this time, Thunderbird does not support the use of LibrePGP v5 keys.
It's uncertain whether Thunderbird should support AEAD encryption, see also bug 1830858 and bug 1872833.

Both RFC 4880 and draft-koch-librepgp include the following recommendation in section 5.2.3.3:
"It is good practice to verify that a self-signature imported into an implementation doesn't advertise features that the implementation doesn't support, rewriting the signature as appropriate."

I suggest that Thunderbird follows that advice, and removes the feature flags on import.

Because production versions of Thunderbird have already allowed importing keys without such cleanup, we should consider a way to fix keys the user already has.

To fix this bug, we require new APIs in the RNP library.

I have written a patch for RNP and submitted a pull request here:
https://github.com/rnpgp/rnp/pull/2179

The patch works for me locally.

To this bug, I will attach changes for Thunderbird that allow it to rewrite the signature when importing a secret key, if unsupported feature flags are found.

The code doesn't yet remove the feature flags in other scenarios, that's a TODO.

status-thunderbird_esr115: --- → affected

Hi Kai,
I don't think Thunderbird should overwrite already existing signature(s), generated by other implementation, as user may have a need for that functionality once he/she generated key outside of the Thunderbird. For instance, imagine the case when user uploaded his original public key to some keyserver, and then Thunderbird do the same. Then on keyserver we'll have two signatures differing only with key flags, which would be confusing to users and implementations.

Instead, Thunderbird may add it's own additional signature with features subpacket (and maybe some notation data with explanation). This would perfectly align with RFC 4880:

   An implementation that encounters multiple self-signatures on the
   same object may resolve the ambiguity in any way it sees fit, but it
   is RECOMMENDED that priority be given to the most recent self-
   signature.

P.S. I'm working on FFI interface which would allow to add custom signatures to the key/userid, including cloning signatures for later re-signing, which would help to handle key expiration change more gracefully then it is done now.

P.P.S. In practice, most implementations would just ignore features subpacket, like it is done with prefered encryption/hash algorithm or even key flags, like we see in one of the recent bugs.

(In reply to Nickolay Olshevsky from comment #2)

I don't think Thunderbird should overwrite already existing signature(s), generated by other implementation, as user may have a need for that functionality once he/she generated key outside of the Thunderbird.

That's one possibility.

Another possibility is that a user has followed advice found on the Internet, for example FSF's guide to email self-defense.
(https://emailselfdefense.fsf.org/en/ )

That guide suggests to generate a key pair using GnuPG, and then import the key into Thunderbird.

(In reply to Nickolay Olshevsky from comment #2)

For instance, imagine the case when user uploaded his original public key to some keyserver, and then Thunderbird do the same. Then on keyserver we'll have two signatures differing only with key flags, which would be confusing to users and implementations.

I don't completely understand your concern, because when changing the key's expiration, RNP overwrites the the already existing signature.

If two keys with a different expiration are uploaded, the same problem happens: Two signatures with different information.

I'm guessing this isn't a problem in practice, because of the signature creation time. I assume that already the later one will win.

Wouldn't this be the same for the features subpacket? When overwriting, the newest signature would have a new creation date. So keyservers and implementation already can see which signature was produced more recently and should be preferred.

(In reply to Nickolay Olshevsky from comment #2)

P.P.S. In practice, most implementations would just ignore features subpacket, like it is done with prefered encryption/hash algorithm or even key flags, like we see in one of the recent bugs.

This might have been true in the past, when there was universal agreement how OpenPGP works.

Now that we have two competing specifications, implementations might be required to follow the feature/preferences attributes of keys more closely.

Nickolay, the part of the RFC I have quoted explicitly recommends that the fix is "rewriting the signature as appropriate".

It says rewriting, not adding another signature.

Thanks to Vincent and Heiko who gave me some explanations.

As I understand it, adding another signature seems fine. When there are multiple signatures, the latest should take precedence.

The approach I implemented was easiest for me to do, because of my limited time and my non-existing knowledge of the RNP code - I only had to modify other code that does a similar modification.

I consider this bug as urgent.

I think Thunderbird should stop advertising these features as quickly as possible, including in an update on the stable 115 branch.

Severity: -- → S2
Priority: -- → P1

(In reply to Nickolay Olshevsky from comment #2)

user may have a need for that functionality once he/she generated key outside of the Thunderbird

If the user has a strong need to use a functionality that an application doesn't officially support, then the user probably shouldn't use that key with the application that doesn't support it (yet).

(In reply to Kai Engert (:KaiE:) from comment #4)

I don't completely understand your concern, because when changing the key's expiration, RNP overwrites the the already existing signature.

Yeah, however later I find out that just adding new signature with new expiration time way more robust/clearer in implementation/has less edge cases.

If two keys with a different expiration are uploaded, the same problem happens: Two signatures with different information.

I'm guessing this isn't a problem in practice, because of the signature creation time. I assume that already the later one will win.

Wouldn't this be the same for the features subpacket? When overwriting, the newest signature would have a new creation date. So keyservers and implementation already can see which signature was produced more recently and should be preferred.

This all would depend on implementation, I doubt that all of them would correctly select where to pick features from and whether to take it in account at all. All the formalities of RFC are not met in the real world.

When wondering about real world support for specific functionality in PGP implementations, https://tests.sequoia-pgp.org/ is a very useful resource. If there are questions about interoperability that this test suite doesn't cover, it might be good to add more tests to it.

I would frankly be very surprised if there were any serious problems around interoperability when updating and/or replacing binding signatures. This mechanism has been around at least RFC 2440, with the clear purpose of updating metadata on a key, including feature signaling. It has literally seen a quarter of a century of use. It's not like Kai is doing something new and unheard of, here.

Sequoia tests are comprehensive and advertised in every OpenPGP-related mailing lists and posts, but for instance they do not tell what to do if some implementation sends email, encrypted with signing primary key instead of subkey.

It's not about OpenPGP specification or practice but about the user experience. I would not be happy if some application take my personal data
(and secret key is definitely personal data) and then without my consent modify it in one or another way. So at least some warning like 'Your key includes support for v5 AEAD encrypted data, however Thunderbird doesn't support this feature yet. Would you like to modify it...' is expected.

Limiting users on what they want to do could lead to problems, like it happened with SHA1 algorithm some years ago. This case should not be that major one, but still could lead to consequences which could not be predicted right now, bringing blame on the Thunderbird.

Again, aside from political reasons, from the practical reason this is pointless - if some other implementation would like to send v5 AEAD-encrypted data with 99% probability it would do that without looking at flags, like this happens with encryption algorithms now.

(In reply to Nickolay Olshevsky from comment #12)

Sequoia tests are comprehensive and advertised in every OpenPGP-related mailing lists and posts, but for instance they do not tell what to do if some implementation sends email, encrypted with signing primary key instead of subkey.

Just for reference, Nickolay is probably refering to bug 1865620, where we recently saw this behavior.

(In reply to Nickolay Olshevsky from comment #12)

It's not about OpenPGP specification or practice but about the user experience. I would not be happy if some application take my personal data
(and secret key is definitely personal data) and then without my consent modify it in one or another way. So at least some warning like 'Your key includes support for v5 AEAD encrypted data, however Thunderbird doesn't support this feature yet. Would you like to modify it...' is expected.

Independently of the decision to involve the user or not, we'll require the suggested functionality from RNP.

Your suggestion to get confirmation from the user might be acceptable, although it makes it slightly more complicated to get it done in the stable 115 release of Thunderbird, because anything that involves new UI needs new strings that need to be localized.

(In reply to Kai Engert (:KaiE:) from comment #14)

Independently of the decision to involve the user or not, we'll require the suggested functionality from RNP.

Sure, generation of custom key signatures is a long awaited functionality, and implementation started via this PR: https://github.com/rnpgp/rnp/pull/2160/files

Basically, it is planned to look like the following for the FFI caller:

rnp_key_signature_clone(); // or rnp_key_direct_signature_create()/rnp_key_certification_signature_create().
rnp_key_signature_set_features();
...rnp_key_signature_set_something()...;
rnp_key_signature_sign();

"This key (you are trying to import / that you have already imported) was created with other OpenPGP software and requests the use of encryption technology that Thunderbird doesn't support. To ensure your correspondents send messages to you that Thunderbird can decrypt, the incompatible preferences should be removed. [Cancel] [Make key compatible]"

...also it could be worth adding some link to the detailed explanation in some article on TB site/blog.

(In reply to Kai Engert (:KaiE:) from comment #13)

(In reply to Nickolay Olshevsky from comment #12)

Sequoia tests are comprehensive and advertised in every OpenPGP-related mailing lists and posts, but for instance they do not tell what to do if some implementation sends email, encrypted with signing primary key instead of subkey.

Just for reference, Nickolay is probably refering to bug 1865620, where we recently saw this behavior.

Thanks for that pointer, Kai.

@Nickolay: I agree that on the application level, additional complications can come up. Libraries can't always fully abstract away all stumbling blocks, for the application. In the case you hint at, as I understand it, a sender sent messages that contained an extra, erroneous, PKESK packet.
However, as I understand it, the packet can safely be ignored on the Thunderbird/RNP side. The sending software is clearly broken, in this case - but recipients have an easy enough way to deal with their messages, within the boundaries of what the specification describes.

Coming back to the topic of this issue: Kai plans to produce updated binding signatures, to adjust the supported features that Thunderbird advertises on behalf of the user, via their public key.
This procedure follows the guidance of RFC 4880, and it seems close to impossible to me that this could cause any problems.

Again, aside from political reasons, from the practical reason this is pointless - if some other implementation would like to send v5 AEAD-encrypted data with 99% probability it would do that without looking at flags, like this happens with encryption algorithms now.

If a sender sends formats that the recipient hasn't signaled that they can read, and this goes wrong, then the sending software is at fault. This is fine, when senders know that the feature is indeed supported by virtually all recipients. In the case of the "OCB packet" from draft-koch, that is clearly not the case. (For example, there has recently been a concrete issue of an OpenKeychain user who had email decryption issues because their key's features signaled support).

I hope that most software won't send draft-koch encrypted messages when the recipient doesn't signal support. GnuPG doesn't, as far as I am aware. Does RNP?

(In reply to Nickolay Olshevsky from comment #12)

It's not about OpenPGP specification or practice but about the user experience. I would not be happy if some application take my personal data
(and secret key is definitely personal data) and then without my consent modify it in one or another way. So at least some warning like 'Your key includes support for v5 AEAD encrypted data, however Thunderbird doesn't support this feature yet. Would you like to modify it...' is expected.

Part of the current situation is that GnuPG is doing exactly this, in the other direction, however: GnuPG silently enables the two new feature flags to signal support for new formats from draft-koch. This also happens on pre-existing keys (e.g. when updating the expiration setting of a key), and on keys for which the user may have previously explicitly disabled the features.

For feature 0x04 [see 1], I couldn't find any UX in gpg that allows a user to inspect or alter the setting of the feature flag. GnuPG 2.4 enables the flag, whenever a new binding signature is written, but doesn't offer any way even for advanced users to check that the "features" setting has been changed.

Most users are definitely unaware of these feature flags, and I assume they prefer not to think about them. I have no strong feelings if Thunderbird should notify users about changes it will make. But I'll point out that such a notification would be in some contrast to established practice, for example in GnuPG.

Thanks Heiko for these explanations and for reminding me about GnuPG's behavior, which I also had experienced.

I think Heiko's argument is good.

If GnuPG doesn't consider it necessary to inform users, prior to adding feature flags, it should be acceptable that Thunderbird silently repairs/removes the incompatible flags.

(In reply to Heiko Schaefer from comment #18)

@Nickolay: I agree that on the application level, additional complications can come up. Libraries can't always fully abstract away all stumbling blocks, for the application. In the case you hint at, as I understand it, a sender sent messages that contained an extra, erroneous, PKESK packet.
However, as I understand it, the packet can safely be ignored on the Thunderbird/RNP side. The sending software is clearly broken, in this case - but recipients have an easy enough way to deal with their messages, within the boundaries of what the specification describes.

Actually situation is worse - sender actually encrypts message using the sign-only primary key (most likely RSA), without using encryption subkeys.
So on RNP/Thunderbird side we'll need to handle this by allowing to decrypt with sign-only key unless software is fixed. Users don't like to dig into details, they just want to get their data decrypted.

Coming back to the topic of this issue: Kai plans to produce updated binding signatures, to adjust the supported features that Thunderbird advertises on behalf of the user, via their public key.
This procedure follows the guidance of RFC 4880, and it seems close to impossible to me that this could cause any problems.

Okay.

If a sender sends formats that the recipient hasn't signaled that they can read, and this goes wrong, then the sending software is at fault. This is fine, when senders know that the feature is indeed supported by virtually all recipients. In the case of the "OCB packet" from draft-koch, that is clearly not the case. (For example, there has recently been a concrete issue of an OpenKeychain user who had email decryption issues because their key's features signaled support).

Logically it is correct, however in practice users would like to see their message decrypted/verified in any case, despite the faulty software and detauls of the RFC. Years ago I met some commercial implementation which used CRCRLF line ending or something similar for cleartext signature generation. And users requested to support this case as well as they didn't have access to the sending party.

I hope that most software won't send draft-koch encrypted messages when the recipient doesn't signal support. GnuPG doesn't, as far as I am aware. Does RNP?

GnuPG allows to encrypt with AEAD-OCB if --force-ocb option is given (previously --force-aead). RNP doesn't use AEAD by default but would do if user specify it explicitly via CLI option or FFI call.

(In reply to Heiko Schaefer from comment #19)

Part of the current situation is that GnuPG is doing exactly this, in the other direction, however: GnuPG silently enables the two new feature flags to signal support for new formats from draft-koch. This also happens on pre-existing keys (e.g. when updating the expiration setting of a key), and on keys for which the user may have previously explicitly disabled the features.

Sure, GnuPG does what it finds appropriate - GnuPG supports draft-koch and it signals it in features flag.

Most users are definitely unaware of these feature flags, and I assume they prefer not to think about them. I have no strong feelings if Thunderbird should notify users about changes it will make. But I'll point out that such a notification would be in some contrast to established practice, for example in GnuPG.

Thunderbird doesn't want to support this flag so it is free to generate keys without it. However, there is another case when importing data from other implementations, what if they would like to add some more flags, or use some private flags? For instance, GnuPG doesn't add AEAD flag when importing keys without it as well as it doesn't re-sign key.

I'd stick to 'Read as flexibly as possible, write as strict as appropriate' strategy.

For completeness, the original text from RFC4880 quoted in comment #0 saying that

It is good practice to verify that a self-signature imported into an implementation doesn't advertise features that the implementation doesn't support, rewriting the signature as appropriate.

was replaced in the crypto refresh with text saying that

When an implementation imports a secret key, it SHOULD verify that the key's internal self-signatures do not advertise features or algorithms that the implementation doesn't support.
If an implementation observes such a mismatch, it SHOULD warn the user and offer to create new self-signatures that advertise the actual set of features and algorithms supported by the implementation.

Separate from the spec, my personal opinion: while ideally we shouldn't bother users about details like this, in this particular case (where Thunderbird imports a private key advertising a feature that it doesn't support), I do think it's reasonable to warn the user, because they might've uploaded the key to a keyserver themselves, and (if Thunderbird updates the key silently) wouldn't know that they need to update it there as well. That being said..:

(In reply to Nickolay Olshevsky from comment #22)

I'd stick to 'Read as flexibly as possible, write as strict as appropriate' strategy.

Since Thunderbird also writes/sends the public key in some cases, the second part of the mantra should also apply here. I.e., Thunderbird shouldn't advertise features it doesn't support, in order to avoid breakage.

So, IMHO, fixing that part first (i.e. modifying the sent key silently) could still be good first step, and is still more in line with both specs than not doing anything. And then warning the user could be a second step, if that's more work / takes more time, perhaps?

(In reply to Nickolay Olshevsky from comment #22)

Thunderbird doesn't want to support this flag so it is free to generate keys without it.

That's a theoretical angle, but impractical.

If someone has generated their key in the past with GnuPG, they might want to continue to use it, even if they switch to a non-GnuPG email application completely.

Or, a user might prefer to have an offline-primary key setup, which currently cannot be implemented with Thunderbird itself.

In my scenario, I had generated my key in the past with GnuPG, and I had revoked user IDs, and Thunderbird didn't allow me yet to edit the expiration date for the key (a current limitation). So I had edited the expiration with GnuPG, and then exported from GnuPG, and imported into Thunderbird.

So despite me using Thunderbird exclusively for OpenPGP-email, I still got the problematic flags.

However, there is another case when importing data from other implementations, what if they would like to add some more flags, or use some private flags? For instance, GnuPG doesn't add AEAD flag when importing keys without it as well as it doesn't re-sign key.

I assume my key to be person-bound, not application bound.

(In reply to Nickolay Olshevsky from comment #12)

Sequoia tests are comprehensive and advertised in every OpenPGP-related mailing lists and posts, but for instance they do not tell what to do if some implementation sends email, encrypted with signing primary key instead of subkey.

Thanks, Nickolay, that's a good catch. I've noted this as a proposed new test for the interop test suite: https://gitlab.com/sequoia-pgp/openpgp-interoperability-test-suite/-/issues/126 .

If you have other ideas for what would improve the interop test suite, please don't hesitate to suggest them!

Thanks everyone for this discussion.

Kai wrote:

I assume my key to be person-bound, not application bound.

This is a really insightful summary of the situation, and perhaps of a concern about the protocol itself.

If the OpenPGP certificate is person-bound, but it is advertising preferences that indicate support for protocol features in a specific implementation, then we have an impedance mismatch. I don't think that anyone particularly wants their certificate to indicate which specific client they're using.

It sounds like we need a mechanism whereby two applications that share use of some secret key can agree on what featureset should be advertised. We don't have any infrastructure like that that i'm aware of.

Failing that, Kai's earlier instinct about removing the features without notifying the user sounds right to me. Here's a framework that (i think) has some baseline properties we want (caveats will follow):

  • when generating a new OpenPGP secret key and matching certificate, an implementation should only indicate features that it supports.
  • when importing an OpenPGP secret key along with its matching certificate, an implementation should strip feature flags that it knows it does not support (maybe all feature flags that it does not know anything about?), and try to re-publish the updated certificate via whatever channels it is aware of.

The result of these two policies is that a certificate whose secret key is used by multiple active agents will advertise the features of the intersection (not the union) of all the agents who have touched the certificate.

On balance, this is a recipe for an ossified protocol: a certificate will never be able to gain feature advertisements without manual intervention (or full key+cert replacement). This is unfortunate, but probably the right way to avoid unreadable messages with the protocol as it currently stands.

We could add a third rule to mitigate this somewhat, which says something like:

  • if an implementation knows for certain that it is the only user of this secret key, it MAY add advertisements for features that it supports, and try to re-publish the updated certificate via whatever channels it is aware of.

("knows for certain" might mean "this application generated this key internally and never exported it", or it might mean explicitly asking the user "is any other application using this key?" -- that's at least an answerable question for most users, compared to the proposed prompt in comment #16, which i find hard to understand even having some amount of expertise)

I'd be happy to explore strategies that would permit certificate evolution for secret keys that are shared across multiple implementations in a separate conversation.

Note that one consequence of the above proposed strategy is that someone keeping track of a certificate will be able to see which feature flags have been removed. This means, for instance, that an implementation that refreshes its own key from standard publication locations can detect when some other application strips features from (or adds features to) its certificate.

If it notices that another implementation has stripped a feature flag that it does support, it can consider prompting the user about that change ("are you using this key with another implementation? that implementation appears to lack important feature X (URL for more info). Please consider asking that implementer to support it.")

If it notices that another implementation has added a feature flag that it does support (perhaps in contravention to the above policy), then it can just graciously (and silently) accept the update.

If it notices that another implementation has added a feature flag that it does not support (again, in contravention to the above policy), then it can re-strip the feature, and (maybe) warn the user ("some other implementation using this key changed it in ways that might make you receive unreadable messages here. We've fixed the problem.") and consider re-publishing.

After stepping away for dinner: i am coming around to Kai's prompt in comment #16, though i might try to make it a bit less wordy and more action-oriented.

In particular, i'd revise the second bullet in my sketch of a policy above to give the user exactly the choice Kai proposes upon importing a secret key + certificate that includes an advertisement for a feature that the implementation doesn't support:

  • offer the user a chance to either (a) cancel the import or (b) strip the cert of these unknown advertisements, and re-publish.

my attempt at making it less wordy and more action-oriented isn't really much better than comment #16, but i'll leave it here anyway.

This OpenPGP key was made by another tool which offers features Thunderbird doesn't support.
If you use it here without cleaning it up, you might receive unreadable messages.

[Do not import] [Import the key and clean it up for Thunderbird]

I should note that flags in the Features packet are not the only flags that are relevant here. You probably want this check for:

  • any unknown or unsupported flag in the Features subpacket
  • any unknown or unsupported algorithm in the Symmetric Cipher Preferences subpacket
  • (for crypto-refresh implementations) any unknown or unsupported pair of <symmetric cipher,AEAD mode> in AEAD Ciphersuite Preferences subpacket
  • any unknown or unsupported algorithm in the Compression Preferences subpacket

You might not need to worry about:

  • any unknown or unsupported algorithm in the Hash Preferences subpacket (this could result in you not being able to validate the signature inside an encrypted message, but there are many reasons why you might not be able to validate a signature other than this (e.g., you don't know the signing key)), and the message will still be reasonable, if unsigned.

(In reply to Nickolay Olshevsky from comment #15)

Sure, generation of custom key signatures is a long awaited functionality, and implementation started via this PR: https://github.com/rnpgp/rnp/pull/2160/files

Those patches aren't small. They probably cannot be easily backported to the v0.17.0 release that Thunderbird stable 115 is currently using.

I'm looking for a minimal short term solution, that could safely be applied to the TB 115.x branch.

I was hoping that my suggested patch could be a solution for that.

Do you think the patch at https://github.com/rnpgp/rnp/pull/2179
is correct and safe?

I'd like to add a counterpoint to the warning wording discussion above: Any dialog you'll come up with will likely boil down to "[tech mumbo jumbo], click this button to continue". Approximately zero users are equipped to meaningfully acknowledge the technicality, let alone make meaningful decisions based on it.

If you are updating the key in a way that is not expected to produce compatibility problems with other software, and improves compatibility with the software that the user is indicating they want to use their key with, I believe such a dialog to be counterproductive for improving the user's understanding and confidence in using TB.

Thanks for the sanity check, Vincent! Your usability instincts are generally good, and i agree that it might make sense to just avoid the prompting entirely. The user did ask to import the key+cert, so do what they asked (and do it safely/cleanly), rather than asking "blah blah blah".

at all costs, avoid giving the users the option to import and use a certificate which will result in people sending them mail that they cannot read, though.

I also agree that bothering the user with technobabble isn't useful, but the one thing that might be worth telling the user is that they might need to distribute the updated key on keyservers and such (if they previously distributed a version advertising a feature that Thunderbird doesn't support) - otherwise they might still receive emails they can't decrypt.

Hello Nickolay, I'm relying on your expertise to give me feedback about the short term approach I've suggested above.

I'm reluctant to take a patch that modifies keys without your approval.

Nickolay, if you aren't comfortable with that patch, Thunderbird could alternatively do the following:

From the github patch, only take the subset that implements rnp_signature_get_features - which is a ready only function, and thus doesn't have any risks of introducing data corruption etc.

Then if Thunderbird attempts to import a key that was generated with GnuPG 2.4, and which contains the new feature flag, then Thunderbird could simply refuse to import the key with an error message "unsupported key".

Hi Kai, ooops, somehow missed this thread, sorry. Don't you mind to continue technical discussion on the Github's PR thread?

Attached patch fix-instead-of-reject.patch (obsolete) — — Splinter Review

In this patch I'm recording the changes on top of the "rejection" patch, that would change it into the "fixing" behavior.

Assignee: nobody → kaie
Attachment #9378531 - Attachment is obsolete: true
Attachment #9378532 - Attachment is obsolete: true

The API to query the features of a key has been approved upstream, thanks Nickolay!

I'd like to make use of the API in the stable 115.x release. This requires either a new upstream release with this API, or we'd have to carry a patch locally in Thunderbird.

I've asked upstream whether a RNP v0.17.1 release with this feature addition is possible, see https://github.com/rnpgp/rnp/commit/74b70af135d0a1b05fe8ad63aac3f20319ebc4e8
I think this approach would be best, as it can allow Linux distributions to cleanly pick up the new Thunderbird dependency.

If we carried this patch locally in Thunderbird, then those Linux distributions that package RNP separately would have to apply our patch themselves on top of the latest RNP release.

See Also: → 1872833
Depends on: 1885353

RNP version 0.17.1 was released, thanks a lot to the RNP team.

That means, we can update Thunderbird with that release.
It allows us to detect whether a key advertises features that are unsupported by Thunderbird.

However, we don't have the ability yet to fix those keys and strip the feature advertisements.
This wasn't done, because we don't have an API to change those feature flags yet.
Nickolay mentioned there is a general API planned for that, which isn't yet ready/released, and it wouldn't make sense for RNP to ship the minimal API that I had suggested/implemented.

It took 3 months to get the "query" ability added and released.
I don't want to wait further for the "fixing" API, but I think I need to take action immediately.

That means we are now facing a tough decision.

Our options are:
(a) reject importing personal keys with the unsupported feature flag,
and potentially warn the user if they have already imported such keys.

(b) allow importing such keys, and do a silent warning on the error console,
so it's at least discoverable.

(c) introduce my API for fixing the keys in Thunderbird builds (patch RNP),
and make it discoverable, whether the available RNP library contains it.
If the "fixing" API is available, then automatically fix existing personal keys,
and also automatically fix them at import time.
If the "fixing" API is unavailable in the RNP library that's available at runtime,
then do either (a) or (b)

Wait. The release document says
"Added functions rnp_key_set_features() and rnp_signature_get_features()."

Was the "set" function really added?

(In reply to Kai Engert (:KaiE:) from comment #44)

Was the "set" function really added?

No it wasn't.
https://github.com/rnpgp/rnp/commit/d58bb09181df5bd80fdbff40c2cf1341fdc96892

Which means comment 43 is accurate.

(In reply to Kai Engert (:KaiE:) from comment #44)

"Added functions rnp_key_set_features() and rnp_signature_get_features()."

Was the "set" function really added?

Nope, sorry, this is just mistyping in the changelog.

(In reply to Kai Engert (:KaiE:) from comment #43)

(c) introduce my API for fixing the keys in Thunderbird builds (patch RNP),
and make it discoverable, whether the available RNP library contains it.
If the "fixing" API is available, then automatically fix existing personal keys,
and also automatically fix them at import time.
If the "fixing" API is unavailable in the RNP library that's available at runtime,
then do either (a) or (b)

This certainly would be a dirty approach, and introduce unexpected difference in behavior across platforms.
Also, I'm not sure it would be even doable with our library linking approach, because we have a file of expected symbols in the library.
If we consider this approach, I'll have to chat with Rob whether it's doable.

I consider another option (d).

Looking at the past, Thunderbird had repeatedly applied patches on top of the RNP library.

Looking at major Linux distributions, I see that Fedora provides an RNP package that is built using the sources from the Thunderbird sources, and Debian bundles the RNP library within the Thunderbird package.

My impression is that distributions are already prepared to ship changes that Thunderbird applies on top of RNP.

I think the current situation is again an exceptional situation, in which we temporarily require a change to RNP.

Because adding API rnp_key_set_features didn't make sense to the RNP project overall,
but because Thunderbird can fix an issue in a simple way with that function being available,
I think it's best if Thunderbird adds this API, temporarily, to the version of RNP it distributes.

The new option would be:

(d) patch RNP to add API thunderbird_rnp_key_set_features
for fixing the feature flags of keys in Thunderbird builds.
Require that all Thunderbird builds use a version of RNP with that API available.
We use the API to automatically fix personal keys on import, and also fix
personal keys that have already been imported.

At a future time, when a newer RNP version offers an API to fix the feature flags,
Thunderbird can stop using that function, instead use an official replacement RNP API,
and no longer patch RNP to have that API.

Rob, would have any concerns regarding option (d) ?

Flags: needinfo?(rob)

(a) reject importing personal keys with the unsupported feature flag,
and potentially warn the user if they have already imported such keys.

(b) allow importing such keys, and do a silent warning on the error console,
so it's at least discoverable.

There is a middle way between these, and that is to allow import, but also warn the user that some breakage may be expected. It can be technobabble-free, e.g.:

"This key advertises advanced features that Thunderbird does not yet support. You may therefore receive encrypted messages that Thunderbird can not process. Please see <DOCUMENT> for more information."

The linked document could include instructions for how to manually fix up the key, or generate a new key without the usage flags. Once TB gains the ability to fix up keys itself, this warning could be removed.

Maybe Andrew's suggestion is best.

In the meantime I have learned there are platforms that distribute a global RNP, and don't use a privacy copy of the RNP library for Thunderbird. I wouldn't want a OS-global library to contain APIs that aren't officially supported by RNP.

Flags: needinfo?(rob)

(In reply to Kai Engert (:KaiE:) from comment #49)

Rob, would have any concerns regarding option (d) ?

I've got no problem with that. I just need the patch against the RNP sources. Can you add a link or attach it to bug 1885353?

Thanks Rob. Still undecided. If necessary, will do it after the rebase.

Just my few cents: I'd stick to Andrew's suggestion as well, first informing user, and then investigating in practice which amount of problems usage of such keys could emit. My guess is that would be around a zero as I doubt that any widely used pgp email implementations already force or even try AEAD encryption.

We (Proton Mail) did have issues because of imported keys that advertise support for the AEAD Encrypted Data packet. GnuPG generates messages using it when it sees the feature flag.

Daniel, thanks for the input. Are you aware which implementations generated them and how widely those implementations are used?

GnuPG 2.3+ generates AEAD Encrypted Data packets, I believe. I don't know how widely those versions are used, no. We can try to get some statistics from the messages in our database, to find out.

Attachment #9378551 - Attachment is obsolete: true
Attachment #9378552 - Attachment description: WIP: Bug 1874715 - Experimental rejection - Skip import of secret keys with unsupported feature flags → Bug 1874715 - Warn about imported or existing OpenPGP secret keys that advertise unsupported features. r=mkmelin

I'm changing the subject of this bug to Andrew's suggestion.
I've already implemented the change, see the attached phabricator revision.

I'll file a separate follow-up bug to automatically fix the flags in the future, when we're ready to do so.

Summary: Don't advertise unsupported OpenPGP features after importing keys that were created with other software → Warn user about imported OpenPGP secret keys that advertise unsupported features
Blocks: 1896885
Attachment #9378617 - Attachment is obsolete: true
Attached image 1581796-info.png —

You may use this secret key file for testing.
Password: x

Pushed by daniel@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/83d4700f3ccb
Warn about imported or existing OpenPGP secret keys that advertise unsupported features. r=mkmelin

Status: NEW → RESOLVED
Closed: 4 months ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
Target Milestone: --- → 128 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: