1921728 - Binary OpenPGP keys ending with whitespace cannot be imported

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Aras Ergus]

Attachment: 61FB8C42F9F5892D-pub-ends_with_whitespace.pgp

Steps to reproduce:

In the OpenPGP key manager, attempted to import an OpenPGP public key file in binary format (i.e. not ASCII-armored) that ends with a whitespace (more precisely: any character that is removed by the "trim" method of the JavaScript string type, see attached file for an example).

Actual results:

An alert popup with "Importing the keys failed" was shown and a warning "rnp_import_keys FAILED; rv=285212673" was logged.

Expected results:

The key should have been imported. (The example file can be imported with other OpenPGP tools such as rnpkeys and GPG.)

---

This happens because in "RNP.sys.mjs", the trim function is called on the input string before passing it to RNP independent of whether it is ASCII-armored or not -- see the statement

const trimmed = keyBlockStr.replace(/^Comment:.*(\r?\n|\r)/gm, "").trim();

in mail/extensions/openpgp/content/modules/RNP.sys.mjs in the comm-central repository 1.

The issue seems to be introduced in the changeset b615fc82fe5c19ba59b07e23f35aecf686827d03 2 when fixing the bug 1883855 3. The RNP issue 4 opened in the context of the aforementioned Bugzilla ticket appears to have been fixed in RNP 0.17.1, so the changes made in "RNP.sys.mjs" could probably be reverted to fix this problem.

Comment 1

[Magnus Melin [:mkmelin]]

Aras, thanks for the test case.
What commands did you use to create the test key? Can you attach a test case that hasn't expired?

61FB8C42F9F5892D-pub-ends_with_whitespace.pgp is expired, so I can't use it in my test. Or well, the primary key is expired, but the encryption isn't. RNP.findKeyByEmail can't find such keys. I'm not sure that's correct, but...

Comment 2

[Aras Ergus]

Thanks for the prompt reply.

I wrote a small Go program to "brute force" test cases, which I will attach in a moment. Originally it created keys that expired 1 second after creation to exclude the possibility of the test keys being unintentionally used for actual communication.

I have created a test key that doesn't expire, which I will also upload.

Comment 3

[Aras Ergus]

Attachment: Go program that generates an OpenPGP key ending with a whitespace byte

Comment 4

[Aras Ergus]

Attachment: DC1A523730F62AE8-pub-does_not_expire-ends_with_whitespace.pgp

Comment 5

[Nickolay Olshevsky]

JFYI: This key artifact includes subpacket 39 (v6 preferred AEAD ciphersuites), which support is defined only in RFC 9580.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: Bug 1921728 - Handle binary OpenPGP keys ending with whitespace. r=kaie

Comment 7

[Kai Engert (:KaiE:)]

(In reply to Nickolay Olshevsky from comment #5)

JFYI: This key artifact includes subpacket 39 (v6 preferred AEAD ciphersuites), which support is defined only in RFC 9580.

Nickolay, thanks for highlighting that.

As I understand it, the subpacket is in the unhashed area, and is NOT marked critical.
According to RFC 4880, section 5.2.3.1:
"An implementation SHOULD ignore any subpacket of a type that it does not recognize."

I understand that RNP v 0.17.1 currently ignores this subpacket, which matches the recommendation of the RFC.

Can we expect that RNP will continue to ignore such subpackets?

If yes, it might be acceptable to use this example key for this text.

Comment 8

[Nickolay Olshevsky]

Hi Kai,
This subpacket is actually in hashed area, but, correct, it's not critical so RNP just ignores it. And, yeah, we do not plan to change this behaviour.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ffe45a6b0a9
Handle binary OpenPGP keys ending with whitespace. r=kaie

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9428224 [details]
Bug 1921728 - Handle binary OpenPGP keys ending with whitespace. r=kaie

[Approval Request Comment]
Regression caused by (bug #): bug 1883855
User impact if declined: can't import keys that happen to binary-end with whitespace
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): safe

Comment 11

[Corey Bryant]

Comment on attachment 9428224 [details]
Bug 1921728 - Handle binary OpenPGP keys ending with whitespace. r=kaie

[Triage Comment]
Approved for beta

Comment 12

[Corey Bryant]

Thunderbird 132.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/89eb23984b51