Closed Bug 1658730 Opened 2 years ago Closed 2 years ago

Cannot import keys containing UTF-8-invalid strings


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

Thunderbird 81


(thunderbird_esr78 fixed, thunderbird80 fixed)

81 Branch
Tracking Status
thunderbird_esr78 --- fixed
thunderbird80 --- fixed


(Reporter: flo.sammueller, Assigned: flo.sammueller)



(3 files, 1 obsolete file)

Attached file test.asc

The import of a public key fails if a User-ID packet of the key contains characters that are invalid in UTF-8 encoding. I will attach a test key that is issued for "Test Müller" in ISO-8859-15 encoding that reveals this behaviour. "ü" in ISO-8859-15 has a hex value of 0xfc which is not a valid UTF-8 hexcode.
At import, the console shows this stacktrace:

TypeError: malformed UTF-8 character sequence at offset 6
    getKeyInfoFromHandle chrome://openpgp/content/modules/RNP.jsm:432
    getKeysFromFFI chrome://openpgp/content/modules/RNP.jsm:234
    importKeyBlockImpl chrome://openpgp/content/modules/RNP.jsm:1519
    importKeyBlockAPI chrome://openpgp/content/modules/cryptoAPI/RNPCryptoAPI.jsm:102
    importKeyAsync chrome://openpgp/content/modules/keyRing.jsm:801
    importKey chrome://openpgp/content/modules/keyRing.jsm:683
    EnigmailCommon_importObjectFromFile chrome://openpgp/content/ui/commonWorkflows.js:189
    oncommand chrome://openpgp/content/ui/enigmailKeyManager.xhtml:1

Note that the key can be handled by RNP on the command line, so it is not a problem of the library.
Especially users with quite old keys in their keyring (pre UTF-8-only times) could be affected by this bug.

Attached patch 1658730.patchSplinter Review

The thrown TypeError is actually the defined behaviour of readString() [1] which is used for collecting information about the key that is imported. Maybe this should be replaced by readStringReplaceMalformed() in all places where UTF-8-invalid characters can occur.


Florian, thanks a lot, I had seen this exception before, but it was still on my TODO list. Your analysis and example is very helpful. I'll look into this soon.

Priority: -- → P1
Assignee: nobody → kaie
Ever confirmed: true

Your suggestion to use readStringReplaceMalformed for user IDs seems good. You missed one place, prim_uid_str.readString().

However, you also changed readString to readStringReplaceMalformed where we shouldn't do it. If our code produces an ASCII ARMOR buffer using RNP, then the result should be valid UTF-8. It would be good to keep that check, as a failure would indicate that unexpected data is given to us.

You're right, the returned char arrays after encryption/signing must be ASCII.


Patrick didn't have time yet to review this, so let's do something else.

Because Florian has agreed to my suggested modifications, we can take this patch with Florian as the author, and myself as the reviewer.

Assignee: kaie → flo.sammueller
Attachment #9169713 - Attachment is obsolete: true
Attached patch 1658730-v2.patchSplinter Review

Patch by Florian, removed the places that are unnecessary, added one forgotton place.


Attachment #9170196 - Flags: review+

Pushed by
Allow importing of OpenPGP keys with user IDs that aren't UTF-8 encoded. r=kaie

Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch

Comment on attachment 9170196 [details] [diff] [review]

OpenPGP correctness fix. For backwards compatibility, allow import/use of older keys that use a non UTF-8 encoding in the user-id meta information.

Attachment #9170196 - Flags: approval-comm-esr78?
Attachment #9170196 - Flags: approval-comm-beta?

Comment on attachment 9170196 [details] [diff] [review]

[Triage Comment]
Approved for beta
Approved for esr78

Attachment #9170196 - Flags: approval-comm-esr78?
Attachment #9170196 - Flags: approval-comm-esr78+
Attachment #9170196 - Flags: approval-comm-beta?
Attachment #9170196 - Flags: approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.