Closed Bug 1643292 Opened 4 months ago Closed 4 months ago

Add a size limitation for OpenPGP key import

Categories

(MailNews Core :: Security: OpenPGP, enhancement)

enhancement

Tracking

(thunderbird78 fixed)

RESOLVED FIXED
Thunderbird 79.0
Tracking Status
thunderbird78 --- fixed

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Some testers of the OpenPGP code were trying to import their complete GnuPG keyring, but reported problems.

Because some keys cannot be imported by RNP, it's likely that an attempt to import many keys once will fail. If just one key isn't accepted by RNP, then the whole import will fail.

While the RNP developers are trying to help us, by offering a flag for a more relaxed import, with the ability to ignore problematic keys, I'm still worried about very big keyrings.

My own public GnuPG keyring is 56 MB, when exported with armor it's 75 MB. When trying to import that key, it takes a long time. Also I've watched memory consumption of the Thunderbird process, which went up to a peak of 7.6 Gigabyte of resident RAM. This might cause less powerful machines of users to fail, or get stuck with swapping to disk etc.

Therefore I suggest that we define a limit for the largest key file that we're willing to import.

The intention is to allow the import of regular keys, but prevent large "import all" attempts. For the migration from Enigmail, we should use a loop that imports keys one by one, so we won't hit any limits, and one bad key cannot affect the import of other good keys.

I suggest an initial file size limit of 5 MB. We can adopt if necessary.

Summary: Size limitation for OpenPGP key import → Add a size limitation for OpenPGP key import

Will need merging with bug 1638822

See Also: → 1638822

For outreach to community testers, can I clarify both the current change here and the long-term goal?

Current: RNP, the upstream library processing PGP keys, is working on enabling key errors to not block import of good keys. Currently, however, it's not working and this is an attempt to improve the experience temporarily to unblock testing other elements.

The release intention is that the actual process will step through a keyring of any size, skip (and log/flag?) failed keys while continuing through and importing the rest, and by doing this in a one-at-a-time manner, also limit memory use?

(I'll note that this loop import process should probably by available for importing any keyring, and not only available from the enigmail migration process, though that's definitely the most important place for the release/transiton)

(In reply to Jon from comment #3)

Current: RNP, the upstream library processing PGP keys, is working on enabling key errors to not block import of good keys.

Correct.

Currently, however, it's not working and this is an attempt to improve the experience temporarily to unblock testing other elements.

Actually, there will be different fixes to make this work. We'll have a separate fixes that will make that work shortly. We'll use a lenient approach on importing (ignore bad keys, remove bad sigs).

The intention of this fix here is: Don't allow people to kill their system by exhausting resources.

The release intention is that the actual process will step through a keyring of any size, skip (and log/flag?) failed keys while continuing through and importing the rest, and by doing this in a one-at-a-time manner, also limit memory use?

Please distinguish between the import mechanism that we offer in product, and the migration tool.

We intend to have a migration tool (managed by Patrick B) that can directly to GnuPG. If you talk directly to GnuPG, then you can control how many keys you export and import at once. I believe Patrick intends to do it one by one. If yes, then memory resources won't be an issue.

The code here isn't about the migration tool. Rather, it's about the scenario that some blob of data has been prepared by someone manually, and tries to import it.

Right now, due to developer constraints I want to have it like this:

  • only offer large import as part of the migration tool (which will use the old way of talking to gnupg - using the executable)
  • restrict import of data blobs to a size that won't cause us to run into resource exhaustion (with the current implementation to process everything at once)
  • at a later time, we can try implement a better mechanism for importing large blobs of data; that will require the ability to split a big key block into the individual pieces, potentially by using future RNP APIs

(I'll note that this loop import process should probably by available for importing any keyring, and not only available from the enigmail migration process, though that's definitely the most important place for the release/transiton)

I agree that ideally we should have this ability eventually, but that's one of many TODOs, and as you say, we might postpone this a bit.

Blocks: 1643330

Filed bug 1643330 to track the follow up work.

Another argument is: Once we have done the initial step of the import (get a listing of the keys found in the file), we'll present a confirmation prompt to the user. That prompt contains a list of the keys that were found, and asks the user to confirm that importing those keys is really wanted (also for giving the user feedback about what is happening).

This dialog is limited in size, and currently doesn't have a scrolling list. So clearly, the initial mechanism aren't yet prepared for dealing with a large set of files. So an initial limitation for the current mechanism seems reasonable, and a smarter mechanism, with smarter user feedback, should be implemented as part of bug 1643330

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/41b6b30638f2
Add a size limitation for OpenPGP key import. r=PatrickBrunschwig

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED

Comment on attachment 9154163 [details]
Bug 1643292 - Add a size limitation for OpenPGP key import. r=PatrickBrunschwig

avoid resource exhaustion on openpgp key import

Attachment #9154163 - Flags: approval-comm-beta?

Comment on attachment 9154163 [details]
Bug 1643292 - Add a size limitation for OpenPGP key import. r=PatrickBrunschwig

Approved for beta

Attachment #9154163 - Flags: approval-comm-beta? → approval-comm-beta+
Target Milestone: --- → Thunderbird 79.0
You need to log in before you can comment on or make changes to this bug.