Closed Bug 1721668 Opened 2 years ago Closed 6 months ago

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

Categories

(MailNews Core :: Security: OpenPGP, defect)

defect

Tracking

(thunderbird_esr102 fixed)

RESOLVED FIXED
109 Branch
Tracking Status
thunderbird_esr102 --- fixed

People

(Reporter: andrew, Assigned: KaiE, NeedInfo)

Details

Attachments

(7 files, 1 obsolete file)

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 edward-en@fsf.org <mailto:edward-en@fsf.org>. Make sure there's no public key in your database.
  3. Acknowledge the missing key message and press OK.
  4. Select the edward-en@fsf.org <mailto:edward-en@fsf.org> entry in the OpenPGP Message Security window and press "Manage keys for selected recipient..."
  5. Press "Discover new or updated key".

Actual results:

  1. Get the error "Can't read public key file.".
  2. 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.

Component: Untriaged → Security: OpenPGP
Product: Thunderbird → MailNews Core

I set up WKD for the edward-en@fsf.org GPG Key, including its other identities, so testing that specific key will not reproduce this reported issue for now.

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.

Flags: needinfo?(andrew)

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

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → kaie
Status: NEW → ASSIGNED
Attached patch 1721668-backport-esr102.patch (obsolete) — Splinter Review

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

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.

Attachment #9305836 - Attachment description: WIP: Bug 1721668 - Add a null check. rs=me → Bug 1721668 - Add a null check. r=mkmelin

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.

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
Attachment #9305713 - Attachment is obsolete: true

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

Attachment #9306597 - Flags: approval-comm-esr102?

Please see the attached backport patches.

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

[Triage Comment]
Approved for esr102

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