1778509 - Cannot import OpenPGP ASCII secret key with non-ASCII comment. Cannot import binary OpenPGP secret keys.

Status: RESOLVED FIXED

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

Description

[Mantas Mikulėnas (grawity)]

Attachment: test key generated via sq

Steps to reproduce:

I exported my private key from gpg and tried to import it via "Account Settings".

The key's userID has an 'ė' (U+0117) as part of my last name. I also generated a new "test" key using sequoia (sq key generate --userid "Test ė <test@test>" --export test2.pgp) to make sure it's not something about how the key was exported.

I'm using 102.0.0 from the official Windows installer (I wanted to see the new shiny folder icons).

Actual results:

For the first 20 attempts or so, the message "Error! Failed to import file. TypeError: can't convert the number 279 to element 112 of the type ctypes.uint8_t.array(1953)" showed up. (chr(279) is 'ė' so I assume it's trying to treat the Unicode userid as raw bytes...)

Then, mysteriously, the error message went away and the key was imported – though the "test" key I generated with sequoia still triggers the same error.

Expected results:

The key should have been imported on the first try.

(This used to work in the past, as on another machine I already have my personal key imported without problems.)

Comment 1

[Thomas D. (:thomas8)]

Kai, can you assess the impact?
Feel free to change priority/severity.

Comment 2

[Magnus Melin [:mkmelin]]

That key imports fine for me.

Comment 3

[Kai Engert (:KaiE:)]

The trigger here is the comment in the key block, which contains the non-ASCII char.

But that real reason of this bug (regression) is that the code to prepare the input was changed in bug 1677088.

The function here expects the raw the bytes of the file.
Because the input was created using ioutils.readUTF8, it contains multi-byte characters, that the conversion here cannot handle.

Comment 4

[Kai Engert (:KaiE:)]

This code path is also broken when attempting to import a binary key file, which worked in the past.

Comment 5

[Kai Engert (:KaiE:)]

Attachment: Bug 1778509 - Restore binary reading of input key file. r=mkmelin

Comment 6

[Kai Engert (:KaiE:)]

Being able to properly import secret key files from a backup is a critical basic functionality.

I remember having seen another bug report, which had complained that we could no longer import binary secret keys (while ascii secret keys still worked), but at that time I didn't have time to follow up, and right now I cannot find it.

So this is a regression that probably many people have ran into.

Comment 7

[Magnus Melin [:mkmelin]]

Hmm, sure binary imports are broken? I'm pretty sure I tested that back when I did the conversion. Or maybe it depends on the file and I was just lucky...

Comment 8

[Kai Engert (:KaiE:)]

(In reply to Magnus Melin [:mkmelin] from comment #7)

Hmm, sure binary imports are broken?

yes they are broken for me

Comment 9

[Kai Engert (:KaiE:)]

Attachment: 1778509-esr102.patch (backport)

Comment 10

[Mantas Mikulėnas (grawity)]

(In reply to Kai Engert (:KaiE:) from comment #3)

The trigger here is the comment in the key block, which contains the non-ASCII char.

Ah, that explains why importing the real key suddenly started working for me (at first I was trying an older backup which did have UTF8 text alongside the key, but at some point switched to trying a freshly exported one).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6861a09feb3a
Restore binary reading of input key file. r=mkmelin

Comment 12

[Kai Engert (:KaiE:)]

Comment on attachment 9285309 [details]
Bug 1778509 - Restore binary reading of input key file. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): 1677088
User impact if declined: user cannot import valid file formats
Testing completed (on c-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low, the new code is more correct

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9285309 [details]
Bug 1778509 - Restore binary reading of input key file. r=mkmelin

[Triage Comment]
Approved for beta

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 103.0b6:
https://hg.mozilla.org/releases/comm-beta/rev/3acc3a3a1e5d

Comment 15

[Magnus Melin [:mkmelin]]

Time to uplift to 102?

Comment 16

[Kai Engert (:KaiE:)]

Comment on attachment 9285490 [details]
1778509-esr102.patch (backport)

[Approval Request Comment]
see comment 12

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9285490 [details]
1778509-esr102.patch (backport)

[Triage Comment]
approved for esr102

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 102.1.1:
https://hg.mozilla.org/releases/comm-esr102/rev/8a2dcb91a716