1721668 - TypeError when trying to import PGP key with "Discover new or updated key"

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Andrew E]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

1. Use Thunderbird 78.x and setup a mail account with OpenPGP encryption.
2. Try to send an OpenPGP encrypted mail to [e-mail] <mailto:[e-mail]>. Make sure there's no public key in your database.
3. Acknowledge the missing key message and press OK.
4. Select the [e-mail] <mailto:[e-mail]> entry in the OpenPGP Message Security window and press "Manage keys for selected recipient..."
5. Press "Discover new or updated key".

Actual results:

6. Get the error "Can't read public key file.".
7. Message from the error console:

CryptoAPI.sync() failed result: TypeError: can't convert the number 351 to element 165 of the type ctypes.uint8_t.array(9442)
importToFFI chrome://openpgp/content/modules/RNP.jsm:1600
getKeyListFromKeyBlockImpl chrome://openpgp/content/modules/RNP.jsm:1670
getKeyListFromKeyBlockAPI chrome://openpgp/content/modules/cryptoAPI/RNPCryptoAPI.jsm:302
getKeyListFromKeyBlock chrome://openpgp/content/modules/key.jsm:155
lookupAndImportOnKeyserver chrome://openpgp/content/modules/keyLookupHelper.jsm:42
viewSelectedEmail chrome://openpgp/content/ui/composeKeyStatus.js:203
oncommand chrome://openpgp/content/ui/composeKeyStatus.xhtml:1
showMessageComposeSecurityStatus chrome://messenger/content/messengercompose/MsgComposeCommands.js:1857
prepareSendMsg chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js:2112
sync chrome://openpgp/content/modules/cryptoAPI/interface.js:56
sendMessageListener chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js:2628
CompleteGenericSendMessage chrome://messenger/content/messengercompose/MsgComposeCommands.js:4758
GenericSendMessage chrome://messenger/content/messengercompose/MsgComposeCommands.js:4695
SendMessage chrome://messenger/content/messengercompose/MsgComposeCommands.js:5000
doCommand chrome://messenger/content/messengercompose/MsgComposeCommands.js:996
doCommand chrome://messenger/content/messengercompose/MsgComposeCommands.js:1192
goDoCommand chrome://global/content/globalOverlay.js:101
oncommand chrome://messenger/content/messengercompose/messengercompose.xhtml:1

Expected results:

The key should have been imported automatically.

Going to keys.openpgp.org in a Web browser, downloading the same key, then importing that file manually through the key manager does work.

https://keys.openpgp.org/search?q=edward-en%40fsf.org

The comment at the top of the key contains some non-English characters.

Comment 1

[Andrew E]

I set up WKD for the [e-mail] GPG Key, including its other identities, so testing that specific key will not reproduce this reported issue for now.

Comment 2

[cryptgoat]

This is still happening in Thunderbird 102.x. It's even worse now: Importing the key manually no longer works, throwing this error: CryptoAPI.sync() failed result: TypeError: can't convert the number 351 to element 165 of the type ctypes.uint8_t.array(9442).

I'd say this is a pretty major bug since it renders the absolutely wonderful email self sefense guide by the FSF useless: https://emailselfdefense.fsf.org/

Importing the key via WKD does work but it's far off from being an adequate solution. After trigger the online search for public keys in Thunderbird it just searches forever instead of throwing an error or showing the key found via WKD leaving the user clueless. It's already very difficult to promote the usage of PGP encrypted mails, issues like these are a cause for a lot of frustration and turn people away.

Comment 3

[Kai Engert (:KaiE:)]

Thanks for the ping. I never noticed this bug previously, sorry. Looking now.

Comment 4

[Kai Engert (:KaiE:)]

Today, the key data found on WKD is binary, and we import it correctly.

However, I still see the bug with the keydata you have uploaded on keys.openpgp.org, that triggers the error you have reported.

The cause is an ascii armored key block, which has comments with non-ascii characters.
As stated in RFC 4880 section 6.2:
"A comment may be any UTF-8 string. However, the whole point of armoring is to provide seven-bit-clean data. Consequently, if a comment has characters that are outside the US-ASCII range of UTF, they may very well not survive transport."

In our scenario, when we convert input key data for handing it to the RNP library, we have to convert the received data from JavaScript string types to an 8-bit type.

Our current code doesn't care whether the input is binary or plain ASCII, it works fine for both.
However, the current code doesn't expect that a key block might contain unicode characters, as in your key block comments.

We could possibly use the following logic whenever we process PGP input:
Check if the input contains any characters with a char code over 255.
If it does, conclude the input isn't a byte stream, but is a unicode string.
Try to remove all "Comment" lines prior to processing it.
If the input still contains any characters with a char code over 255, then reject it as an invalid input.

Comment 5

[Kai Engert (:KaiE:)]

Attachment: Bug 1721668 - Fix importing key blocks with non-ASCII comment lines. r=mkmelin

Comment 6

[Kai Engert (:KaiE:)]

Attachment: 1721668-backport-esr102.patch

Comment 7

[Kai Engert (:KaiE:)]

The patch fixes an additional error, keyList may be null in the error scenario.

Comment 8

[Kai Engert (:KaiE:)]

I have a code merging issue.

I want to split the null check into a separate patch, and handle it separately, to allow me to land other bugs earlier.

Comment 9

[Kai Engert (:KaiE:)]

Attachment: Bug 1721668 - Add a null check. r=mkmelin

Comment 10

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/comm-central/rev/9428f31d3f0fc024cc9ca2c4d00264c4e01f5190

Comment 11

[Kai Engert (:KaiE:)]

Attachment: Bug 1721668 - Test for importing an OpenPGP ASCII key block with Unicode comments. r=mkmelin

Comment 12

[Kai Engert (:KaiE:)]

Andrew and cryptogoat: If you don't want to wait until this fix becomes available in a stable Thunderbird release, as an immediate workaround, you could upload a version of your public key that is a plain text ascii armored key block, without comments having international characters.

Comment 13

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/comm-central/rev/596e66f7f1d76e3e19a74ae1ec93271aeca95b98
https://hg.mozilla.org/comm-central/rev/a6f9291a60d261f89534d7d180fe68a29d6ba90b

Comment 14

[Kai Engert (:KaiE:)]

Attachment: WIP: Bug 1721668 - Follow-up, fix lint. rs=lint

Comment 15

[Kai Engert (:KaiE:)]

Attachment: 1721668-backport-102-nullcheck.patch

Comment 16

[Kai Engert (:KaiE:)]

Attachment: 1721668-backport-102-test.patch

Comment 17

[Kai Engert (:KaiE:)]

Attachment: 1721668-backport-102-fix.patch

Comment 18

[Rob Lemley [:rjl]]

https://hg.mozilla.org/comm-central/rev/70599852784a88ebb51dfce6e083d1495fa79af3

Comment 19

[Kai Engert (:KaiE:)]

Comment on attachment 9306597 [details] [diff] [review]
1721668-backport-102-fix.patch

Request to uplift all patches from this bug

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: failure to import certain key files
Testing completed (on c-c, etc.): yes, full beta 109 cycle
Risk to taking this patch (and alternatives if risky): low

Comment 20

[Kai Engert (:KaiE:)]

Please see the attached backport patches.

Comment 21

[Wayne Mery (:wsmwk)]

Comment on attachment 9306597 [details] [diff] [review]
1721668-backport-102-fix.patch

[Triage Comment]
Approved for esr102

Comment 22

[Rob Lemley [:rjl]]

Thunderbird 102.7.0:
https://hg.mozilla.org/releases/comm-esr102/rev/b8b229dcac6c
https://hg.mozilla.org/releases/comm-esr102/rev/db7acb28e521
https://hg.mozilla.org/releases/comm-esr102/rev/ccf827db8e2b