1865620 - PGP messages cannot be decrypted - 'There are unknown problems with this encrypted message'

Status: RESOLVED FIXED

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

Description

[NoSpamProxy]

Attachment: Error of 'export RNP_LOG_CONSOLE=1'

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36 OPR/104.0.0.0

Steps to reproduce:

• create a PGP key in thunderbird < 115 (exact version: 115.4.3)
• used this key with another product to send an encrypted message to my mailbox
• opened the message in thunderbird

Actual results:

The message cannot be opened because 'There are unkonwn problems with this encrypted message'.

Expected results:

The message should have been decrypted and displayed like it was done before the upgrade to 115.

Comment 1

[NoSpamProxy]

Attachment: decryption issue information

Comment 2

[NoSpamProxy]

I tested the behavior against an older version: 102.15.1
Here everything was working fine.

The issue exists under Windows and Linux.

Comment 3

[NoSpamProxy]

Attachment: suspicious comment about an RNP change.png

Accessed the code through the developer tools.
Clicking on the message opens up this area.

Comment 4

[NoSpamProxy]

Attachment: Example email which is not decryptable in Thundebrid 115

This message does not contain any sensitive data and can be used in public.
It was send in an internal test environment.

Comment 5

[NoSpamProxy]

Attachment: Secret key to decrypt example message

This key is not sensitive can be used in public.

This key is used in my local test environment only.
The passphrase is: bugzilla1865620

Comment 6

[input]

Same Problem here, no interest to fix this issue?

Comment 7

[Kai Engert [:KaiE:]]

(In reply to input from comment #6)

Same Problem here, no interest to fix this issue?

Too few engineers, too little time, to many bugs.

Comment 8

[Kai Engert [:KaiE:]]

I have looked at the issue, and I'm guessing is the following:

• our code keeps all keys locked most of the time
• we only unlock keys as necessary

The current strategy for unlocking secret keys that are required for decryption is:

• look at the message
• find out what key is necessary
• unlock that key
• ask the library to decrypt

Your example message has something that I haven't seen before.
It says, the message can be decrypted with the following secret key IDs:

• 0x3866a9bc84adf235
• 0xe9cc3c1b03aa0148
• 0x1ea7bf21a0c91a55

The secret key you have attached contains the following key IDs:

• primary key (signing/certification):
  0x3866a9bc84adf235
• subkey (encryption)
  e9cc3c1b03aa0148

It seems very unusual that your message says the recipients signing/certification key can be used for decryption.

But probably Thunderbird saw the first key ID, and decided that it will be sufficient to unlock that one.

Then, RNP tried to decrypt, and decided that it wants to use the encryption subkey for decryption, and it asks for that one to be unlocked.
It apparently doesn't try the other key.

Because the desired subkey isn't unlocked, decryption fails.

So, in order to fix that, Thunderbird either needs to:

• be certain which key is should unlock, maybe in this scenario, it should ignore key IDs of secret keys that aren't marked for purpose encrption?
• or, Thunderbird chould unlock all the secret keys that can potentially be used to unlock
• or, Thunderbird needs to revisit the strategy to only unlock some of the subkeys, and always unlock all of them

Comment 9

[Kai Engert [:KaiE:]]

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

• be certain which key is should unlock, maybe in this scenario, it should ignore key IDs of secret keys that aren't marked for purpose encrption?

This strategy would fail, if the sign/cert key is the only key listed as a potential decryption key. I don't know if RNP would be willing to use it for decryption.

Comment 10

[Kai Engert [:KaiE:]]

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

• or, Thunderbird chould unlock all the secret keys that can potentially be used to unlock

I think that may be best, even if could potentially mean, if different passwords are set of separate subkeys, that the user would have to get prompted twice for unlocking keys (if they do have separate passphrases).

Comment 11

[Kai Engert [:KaiE:]]

Hmm. That strategy also isn't perfect. The user might have two (or three?) completely separate secret keys (not primary/subkey pairs) which can be used to unlock the message.

So Thunderbird would have no idea which one of those RNP would try to use to unlock, and unlocking all of them seems over the top...

I probably must look at on-demand unlocking again, and satisfy what RNP wants to use. I need to refresh my memory why I had moved away from on-demand unlocking.

Comment 12

[Kai Engert [:KaiE:]]

I couldn't yet find where we discussed the need for the current unlocking strategy.

I have some memories that it was related to a limitation in RNP, that RNP wouldn't retry the callback, when the first returned password from the callback wouldn't work. I might have tried to work around that limitation.

Maybe the following new idea could work:

The callback function requests that I return the password. However, I don't know the right password, until I have attempted to unlock the key myself.

Previously, I was hoping that RNP would allow me to say: "here, take this password, try it, and if it doesn't work, call me again". But that wasn't possible, because of the no-retry limitation.

So instead, from within the callback, I could try to unlock the key. At first, I will have to try if it can be unlocked with the "automatic passphrase" (that is tied to the optional primary password). If that doesn't work, I conclude that an individual passphrase was set for that key. I could handle prompting from within the callback, and if the initially entered password doesn't work, I'd repeat and retry until a correct passphrase was entered by the user - of until the user cancels the prompt.

This would require that I unlock the key, and on success, lock the key again, and then give the known good password back to RNP, which would then unlock the key again...

Comment 13

[Kai Engert [:KaiE:]]

After more experiments, I am reminded of additional problems with the approach "prompt during the callback from RNP that asks for the right password".

We're calling from JavaScript into C code for decrypting, and that callback calls back from C into JavaScript for the password callback. And inside that JavaScript callback I'd need to execute some async code, which results in the need to spin a nested event loop, and when I try to do so, I run into a crash that I don't understand, and which I might not be able to understand without a lot of much deeper analysis, and I have no idea if it would be avoidable.

But during this journey, I had another idea, which is simpler, and should be doable:

I could execute the decryption attempt twice. During the first time, I will use an implementation for the password callback that simply records the information which key needs to be unlocked. (This code would replace my current code to guess, which isn't reliable producing the same decision.)

When returning from RNP to the JS code, I can then do the unlocking test from my primary JavaScript execution context - avoiding the problems with a nested event loop and the JS <-> C <-> JS transition.

Then, once I have learned both the required key and the password that works to unlock it, I'll ask RNP again to decrypt, and this time I'll use a password callback that has been prepared to provide the correct callback.

I hope that any decryption operation will always require at most one key to be unlocked.

Comment 14

[Kai Engert [:KaiE:]]

Attachment: Bug 1865620 - Ensure the correct key is unlocked when decrypting an OpenPGP message. r=mkmelin

Comment 15

[Nickolay Olshevsky]

Hi Kai,
This seems to be the case where standard meets the real world, where things do not go by standard :)
Since signing key should not be used for encryption, RNP skips keys which doesn't have encryption usage explicitly, in :

            if (!seckey->has_secret() || !seckey->can_encrypt()) {
                continue;
            }

However, having these examples, we should loosen this check, probably just issuing a worning.
P.S. Is it known which implementation generates such packet and how widely it is deployed?

Comment 16

[Kai Engert [:KaiE:]]

Hello reporters, can you please tell us which software was used to create your encrypted messages?

Comment 17

[Kai Engert [:KaiE:]]

Thanks Nickolay for researching.

Comment 18

[Jacob Taylor]

I have just today experienced this decryption failure where the sender used PostBoxApp/7.0.60 per email headers.

Comment 20

[[:AK]]

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

Hello reporters, can you please tell us which software was used to create your encrypted messages?

Hi Kai,
from our side affected messages were sent and encrypted using NoSpamProxy gateway.

greetings
Andreas

Comment 21

[floerke]

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

Hello reporters, can you please tell us which software was used to create your encrypted messages?

Hi Kai,
we noticed the problem when receiving emails from one of our partners. I don't know their setup, but there are a lot of "outlook.com" and "microsoft.com" received-headers and also some "boundary=NoSpamProxy..."-headers.

Holger

Comment 22

[NoSpamProxy]

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

Hello reporters, can you please tell us which software was used to create your encrypted messages?

Hello Kai,

excuse my late response.
We are using NoSpamProxy too and we are the vendor of NoSpamProxy.
Regarding the analysis we will re-check if / why a key for sign only is used for encryption. We don't like the "real world" behavior and prefer to follow rules and RFCs.

Jan

Comment 23

[Kai Engert [:KaiE:]]

(In reply to NoSpamProxy from comment #22)

Regarding the analysis we will re-check if / why a key for sign only is used for encryption. We don't like the "real world" behavior and prefer to follow rules and RFCs.

Hello Jan, thank you in advance for investigating.

Comment 24

[NoSpamProxy]

Hello Kai,

some good news:
We can confirm that the sign only key is used on our end for encryption. This was a bug in the library.
We are working on an update and fix right now.

Regarding Thunderbird I still think it should be an improved handling in such a case because like Nickolay said the real world looks different :(

Comment 25

[Kai Engert [:KaiE:]]

Thank you.

I think we should do the fix on the TB side anyway, because it will also fix other scenarios, like the user having two separate private keys (maybe pseudonyms), and might be a recipient twice of the same message.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/23ca5f76f633
Ensure the correct key is unlocked when decrypting an OpenPGP message. r=mkmelin

Comment 27

[NoSpamProxy]

With the nightly build from today (thunderbird-124.0a1) we are able to confirm that this issue is fixed.

Comment 28

[Kai Engert [:KaiE:]]

Thanks for the feedback.

I notice this is already contained in the 123 branch.

Comment 29

[NoSpamProxy]

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

Thanks for the feedback.

I notice this is already contained in the 123 branch.

Thank you for the information.
We directly tested the 123 beta build and its working too. :)

Comment 30

[Kai Engert [:KaiE:]]

Attachment: Backport to esr-115 branch

We can let the patch bake for a bit in beta, prior to uplifting to esr115.

Comment 31

[Kai Engert [:KaiE:]]

Attachment: Backport patch that ignores whitespace changes

Just for the record, when considering backport approval, that the patch is much smaller than it appears. This patch shows the changes without whitespace changes.

Comment 32

[Kai Engert [:KaiE:]]

I'm marking the previous two backport patches as obsolete, because they were earlier versions that I had backported earlier.
Need to create updated backport patches.

Comment 33

[Kai Engert [:KaiE:]]

Attachment: Backport to esr-115 branch v2

Comment 34

[Kai Engert [:KaiE:]]

Attachment: Backport patch that ignores whitespace changes v2

Comment 35

[NoSpamProxy]

Great news - thank you very much,

Comment 36

[Kai Engert [:KaiE:]]

Comment on attachment 9375965 [details] [diff] [review]
Backport to esr-115 branch v2

[Approval Request Comment]
Regression caused by (bug #): 1679278
User impact if declined: cannot decrypt some messages
Testing completed (on c-c, etc.): yes
Risk to taking this patch (and alternatives if risky): medium, but risk is limited to scenarios to actions the require unlocking the secret key to decrypt a message or process a key

Comment 37

[Wayne Mery (:wsmwk)]

Comment on attachment 9375965 [details] [diff] [review]
Backport to esr-115 branch v2

[Triage Comment]
Approved for esr115

Comment 38

[Daniel Darnell [:dandarnell]]

Thunderbird 115.8.0:
https://hg.mozilla.org/releases/comm-esr115/rev/acefd05c4b38