TB 78.2.0-pre build1, Linux

A plain text, inline PGP encrypted list message from one particular sender fails to decrypt. I.e. after pressing the 'Decrypt' button, the cipher text is still there.

The Error Console has this:
rnp_op_verify_execute returned unexpected: 268435457 3 RNP.jsm:861:17

The RNP log is attached.
The messages decrypts just fine with TB68/Enigmail, and it has a good signature.

(Not a regression unless it worked in Thunderbird core without Enigmail at some point)

Could you please save the raw message to a separate file (between and including the BEGIN PGP and END PGP lines), and then run the following command on the file:

gpg --list-packets filename >dump.txt  2>&1

and then provide the dump.txt file?

If you're worried about privacy you could send me the message by email, but the log you have attached already revealed some information about the message (recipient key IDs). If you don't like that, let me know and I can hide the attachment.

Yes, please hide the attachment. I wasn't aware of the key IDs.
I'll send the dump information via email.
Thank you.

(In reply to Christian Riechers from comment #3)

Yes, please hide the attachment.

done

FYI, I haven't received email yet.

You should have now.

I've obscured all key IDs and dates in the file that was provided.

Nickolay, does anything in this packet dump look unusual, and could trigger the BAD_FORMAT error in RNP?

Kai, packet dump looks fine to me. The only guess is that some of the pubenc entries is malformed (or contains unknown curve/algorithm/whatsoever).
Is it possible to get access to the RNP error log? Or is it empty?

Another thing is that signature is made in text mode, and we had certain problems with this (have a PR which waits to be merged). Anyway decryption must work, just invalid signature could be reported.

Nickolay, the error log says:
src/librepgp/stream-armor.cpp:647] unknown header ' '

And then followed by many additional lines with "unknown header", and almost every full line from the key data is logged.

Most likely this is caused by absence of single empty line (or too much empty lines) between -----BEGIN PGP PUBLIC KEY BLOCK----- and base64-encoded stuff, which may happen after copy-pasting or because of erroneous implementation.

Christian, does comment 11 apply to your message?

Not exactly. There is a single empty line, but then there's also an extra line 'Charset: UTF-8'. I wouldn't rule out some sort of erroneous implementation either. Example below:

-----BEGIN PGP MESSAGE-----
Charset: UTF-8

hQIMA+6GUX4qXDZdAQ//RehDKp4iPSyoucJmUEaT8bizU8IQFWqVeV7BNQxyil83
KZ8+0He6kl4/m6XV4K/Eun59pPeGaOimlueTSz6aNbXSPxYDzWGAMnSvz1vqGxWM
...

Here's another example which fails to decrypt:

-----BEGIN PGP MESSAGE-----
Charset: UTF-8
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

hF4DJZOClgRIYTcSAQdA2SzlCcsk7LW9m5yc8YMXXM62smTHSk2qMZoUfNF+fFcw
dy50OuepkBUss1kJYoqvfZjtTWpE93jqWz1jJXGq24LvSyz4A0+TDfo+IpshUpjx

Thanks for updating. Quick-checking doesn't expose the aforementioned behavior. Could you please check which line ending is used in those message, by some sort of hex-editor? Is it ordinary LF/CRLF, or something special?

The separator line between header and contains isn't empty. It contains the bytes C2 A0.
This article explains that it is the sequence for a non-breaking space:
https://stackoverflow.com/questions/2774471/what-is-c2-a0-in-mime-encoded-quoted-printable-text

I assume RNP is strict and only allows fully empty separator lines.
I cannot tell if this special whitespace is allowed in this place, but it probably isn't.

Is it correct to reject it, or should we try to support it?

Yeah, RNP interprets armored message as ASCII-only, according to the RFC 4880-bis. And these bytes are not something expected.
Since some implementation (or process atop of implementation) generates such messages, TB should support them. The questions is - on which layer we should interpret that C2-A0. At first glance it should be removed (or transformed into ordinary space) before being fed to RNP.

Yes agreed, Thunderbird should try to sanitize.

Since my bug ticket (#1670202) was closed, I am putting the beginning of my message here
(extracted and base64 decoded PGPexch.html.gpg as shown by hexdump):

00000000  2d 2d 2d 2d 2d 42 45 47  49 4e 20 50 47 50 20 4d  |-----BEGIN PGP M|
00000010  45 53 53 41 47 45 2d 2d  2d 2d 2d 0d 0a 56 65 72  |ESSAGE-----..Ver|
00000020  73 69 6f 6e 3a 20 31 30  2e 33 2e 32 20 28 42 75  |sion: 10.3.2 (Bu|
00000030  69 6c 64 20 32 31 34 39  35 29 0d 0a 0d 0a 71 41  |ild 21495)....qA|

So my issue is definitely related but does not have the C2-A0 issue.

This is now in limbo state since 2 month. Any chance for progress here?

Bug 1675939 is a separate issue, but with a similar symptom. Bug 1675939 had an initial attempt to fix it, which did not help for bug 1675939.

However, Christian mentioned the initial attempt to fix bug 1675939 had a side effect - if fixed this bug.

Christian, let's talk about your setup. Do you have allow_external_gnupg enabled? Do you have your private key in both Thunderbird and GnuPG?
Here's a theory what might have happened:

• for the message you have received, RNP tried to decrypt, but failed
• the original (and current) code treats it as a fatal error, and stops processing.
• with the initial attempt to fix bug 1675939
  ( https://hg.mozilla.org/comm-central/rev/bfa8183cf7b8 )
  TB attempted to decrypt using GnuPG, which worked because it has the private key

Does this explain it?

(In reply to Nickolay Olshevsky from comment #17)

Yeah, RNP interprets armored message as ASCII-only, according to the RFC 4880-bis.

If the RFC says it must be ASCII, then why does the sender's software of the problematic message add a Charset: header inside the PGP message section? Is there any standard that allows it? Do we know which software creates those messages?

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

(In reply to Nickolay Olshevsky from comment #17)

Yeah, RNP interprets armored message as ASCII-only, according to the RFC 4880-bis.

If the RFC says it must be ASCII, then why does the sender's software of the problematic message add a Charset: header inside the PGP message section? Is there any standard that allows it? Do we know which software creates those messages?

Never mind, it is about the charset of the inner plaintext.

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

However, Christian mentioned the initial attempt to fix bug 1675939 had a side effect - if fixed this bug.

Christian, let's talk about your setup. Do you have allow_external_gnupg enabled?

yes

Do you have your private key in both Thunderbird and GnuPG?

The private key is in Gnupg only. That was the whole point, because it failed to import into Thunderbird OpenPGP due to an offline primary key. The public key is in Thunderbird.

Here's a theory what might have happened:

• for the message you have received, RNP tried to decrypt, but failed
• the original (and current) code treats it as a fatal error, and stops processing.
• with the initial attempt to fix bug 1675939
  ( https://hg.mozilla.org/comm-central/rev/bfa8183cf7b8 )
  TB attempted to decrypt using GnuPG, which worked because it has the private key

Does this explain it?

Could be, but I'm not sure.

Christian, to confirm we fix it correctly, we need to see more of an example message.

You could remove all the data inside the PGP block, and you could remove most of the headers of the message. But I'd like to see the structure and the content type and encoding values.

Ideally, please save an example message to a file, use a good editor to edit the file, manually remove most headers, manually remove the inner part of the data block, but keep everything else unmodified. Then attach the file.

I found the place in the code (inherited from Enigmail) that does whitespace sanitizing. I have a patch that works for a test message that I created based on comment 15.

The current code uses the following strategy (inherited from Enigmail):

First, the message text is obtained, by reading the loaded text from the loaded document, base64 decoding and applying charset information, etc. Then the beginning of all lines is trimmed.

The trimming causes bug 1679769. But to fix this bug, we'd have to extend trimming.

If the message decryption or verification fails, then the code will "retry" with different parameters.

If the original message isn't UTF-8, then the (optional) first retry assumes the message text is unicode, converts it to UTF-8, and attempts to process the message again. (This retry is skipped, if the charset is already UTF-8.)

If this still doesn't work, then a second retry is used. For this second retry, no decoding or charset conversions are used at all, it attempts to process the raw message (and without trimming).

And if this wasn't successful, a third retry is used. which converts from UTF-8 to Unicode.

I don't understand why Enigmail attempts to trim lines of signed messages. That seems wrong, because whitespace is part of the signature data calculation.

For messages that contain significant whitespace, it seems it could only potentially work in retry 2 - but will fail if the raw message cannot be used to successfully verify, and rather the decoding/charset conversion is necessary.

I wish we had a good set of test messages and automated tests for all the various kinds of messages. WIthout having set, I have to guess what is best.

My guess is, we shouldn't trim signed messages at all.

I'd limit trimming to encrypted messages. We should trim at least the separator line (complaint from this bug containing non-breaking space), but it also be fine to trim the whole message, including header lines and the data block.

Comment 32 was wrong. I'm hiding it as obsolete.

The reproducer message has a multipart/alternative structure, with a plain text part, and a text/html part.

The message isn't PGP/MIME, it's PGP/INLINE.

When processing, the display preference may select the html part, which indeed contains a non-breaking-space in my example message.

Christian, can you open a message that doesn't work for you. Then use the menu command "view/message body as/plain text".

After this action, is the message automatically decrypted?

The relevant portion in the HTML message part is (redacted):

<div>-----BEGIN PGP MESSAGE-----</div>
<div>Version: .... (Build ....)</div>
<div>Charset: us-ascii</div>
<div>&nbsp;</div>
<div>ABCABCABCABC...</=
div>

I think the use of   by the sending software is a bug. It should use <br> to enforce a blank line.

Christian, you no longer need to do what I asked for in comment 26 and 27.

We have a tricky decision to make.

For this bug, we need to trim "more".
For bug 1679769, we need to trim "less".

The current attachment here can potentially fix both problems, but I'm not yet certain if the fix has side effects.

The problem here seems more urgent to fix than bug 1679769.

So I'd like to use the following approach:

• move the current patch over to bug 1679769 and give it more testing and review
• use a simpler patch here, which does the additional trimming, and should be safe,
  and which could be used on the stable branch immediately little risk

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

Christian, can you open a message that doesn't work for you. Then use the menu command "view/message body as/plain text".

After this action, is the message automatically decrypted?

No, it isn't. I always use 'plain text', and those messages won't decrypt.

Ok, in that case, please do what I asked in comment 26 / 27, it will allow me to analyze if additional changes are required for your scenario.

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

Ok, in that case, please do what I asked in comment 26 / 27, it will allow me to analyze if additional changes are required for your scenario.

You have mail.

(In reply to Christian Riechers from comment #40)

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

Ok, in that case, please do what I asked in comment 26 / 27, it will allow me to analyze if additional changes are required for your scenario.

You have mail.

Thanks a lot. I could confirm that the attached patch also fixes the structure found in your emails.

Your scenario is different, because it already contains the non-breaking-space character encoded using quoted printable. However, after decoding that, it's the same issue.

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/ea005658227e
Also trim non-breaking-space in OpenPGP message input. r=mkmelin

Comment on attachment 9191704 [details]
Bug 1660041 - Also trim non-breaking-space in OpenPGP message input. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: certain messages cannot be decrypted
Testing completed (on c-c, etc.): yes, test added
Risk to taking this patch (and alternatives if risky): low, unconditional trim of space is extended to include non-breaking-space

Comment on attachment 9191704 [details]
Bug 1660041 - Also trim non-breaking-space in OpenPGP message input. r=mkmelin

[Triage Comment]
Approved for eser78 (having tests helps - thanks)

78.6.0
https://hg.mozilla.org/releases/comm-esr78/rev/2066d31e5b07ad4c3b543abc9bc0990e8ca719d9