Cannot import keys containing UTF-8-invalid strings
Categories
(MailNews Core :: Security: OpenPGP, defect, P1)
Tracking
(thunderbird_esr78 fixed, thunderbird80 fixed)
People
(Reporter: flo.sammueller, Assigned: flo.sammueller)
Details
Attachments
(3 files, 1 obsolete file)
1.71 KB,
text/plain
|
Details | |
5.95 KB,
patch
|
Details | Diff | Splinter Review | |
2.45 KB,
patch
|
KaiE
:
review+
wsmwk
:
approval-comm-beta+
wsmwk
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•4 years ago
|
||
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.
[1] https://developer.mozilla.org/en-US/docs/Mozilla/js-ctypes/js-ctypes_reference/CData#readString
Comment 2•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 3•4 years ago
|
||
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.
Comment 4•4 years ago
|
||
Assignee | ||
Comment 5•4 years ago
|
||
You're right, the returned char arrays after encryption/signing must be ASCII.
Updated•4 years ago
|
Comment 6•4 years ago
|
||
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.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 7•4 years ago
|
||
Patch by Florian, removed the places that are unnecessary, added one forgotton place.
r=kaie
Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/6f4962ef80ed
Allow importing of OpenPGP keys with user IDs that aren't UTF-8 encoded. r=kaie
Updated•4 years ago
|
Comment 9•4 years ago
|
||
Comment on attachment 9170196 [details] [diff] [review]
1658730-v2.patch
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.
Comment 10•4 years ago
|
||
Comment on attachment 9170196 [details] [diff] [review]
1658730-v2.patch
[Triage Comment]
Approved for beta
Approved for esr78
Comment 11•4 years ago
|
||
Description
•