1686984 - OpenPGP: Unable to decrypt messages where ciphertext is less than a tenth of the cleartext, error: rnp_op_verify_execute returned unexpected: 285212674

Status: RESOLVED FIXED

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

Description

[worryelectric]

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

Steps to reproduce:

PGP encrypted email encrypted for my key is in my inbox, and will not display on new versions of Thunderbird. Previous versions with Enigmail are unaffected. I've tested this on Thunderbird 78 on Windows, Thunderbird 78 on Linux, and a debug build on Linux from trunk.

Actual results:

The message fails to display. OpenPGP icon shows "Message cannot be decrypted: There are unknown problems with this decrypted message." The debug console shows "rnp_op_verify_execute returned unexpected: 285212674".

The debug output is as follows:
[mem_dst_write() /home/electricworry/projects/thunderbird/source/comm/third_party/rnp/src/librepgp/stream-common.cpp:991] attempt to alloc more then allowed
[process_pgp_source() /home/electricworry/projects/thunderbird/source/comm/third_party/rnp/src/librepgp/stream-parse.cpp:2483] failed to output data
console.debug: "rnp_op_verify_execute returned unexpected: 285212674"

Expected results:

The message should successfully decrypt and its content should be displayed.

Comment 1

[worryelectric]

I've not been able to identify the root cause so far as I can't see the whole call stack due to JIT compiled code. Will update if I find out more. Happy to answer any questions to help resolve.

Comment 2

[worryelectric]

In rnp_output_to_memory(), if I artificially inflate the value of max_alloc by around 1MB then the issue disappears. So, if someone can help me find from where this function is called, I'll possibly be able to identify if max_alloc is being incorrectly set.
However, it appear that value of max_alloc (which is 0x19797c) matches the length of the encrypted message (everything between and including the BEGIN/END PGP MESSAGE marks) plus one or two.

Comment 3

[worryelectric]

Scratch my last comment. max_alloc is 10 times the size of the encrypted message.

I've found the root cause. In comm/mail/extensions/openpgp/content/modules/RNP.jsm, in decrypt(), we have

    let max_out = encrypted.length * 10;

which sets the decrypt buffer to 10 times the encrypted message length. However, in my case the decrypted message is more than 10 times larger...

$ gpg --decrypt enc.txt > plain.txt 2> /dev/null
$ ls -l
total 2724
-rw-rw-r-- 1 ele ele  166949 Jan 15 16:56 enc.txt
-rw-rw-r-- 1 ele ele 2619379 Jan 15 19:12 plain.txt

The message simply had attachments that carried a lot of repeated information hence the excellent encryption ratio.

I propose that max_out is either set to zero to remove the limitation, or a higher arbitrary multiplier is used (though presumably for any value there will be someone that one day exceeds the buffer).

Comment 4

[Magnus Melin [:mkmelin]]

Probably 0 would be good, if it does what you say. Can you submit a patch? (see https://developer.thunderbird.net/)

Comment 5

[worryelectric]

Yes, I am happy to. I'll get one submitted asap.

Comment 6

[worryelectric]

Attachment: decrypt-compression-issue

I've created this patch based on the guidance in the developer docs for mq. My name isn't included; not sure why as I followed the Mercurial configuration to the letter. Is this ok?

Comment 7

[Magnus Melin [:mkmelin]]

Thanks, looks ok!
You can set your username by adding something like this to your ~/.hgrc:

[ui]
username = First Last <[e-mail]>

Comment 8

[worryelectric]

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

Thanks, looks ok!
You can set your username by adding something like this to your ~/.hgrc:

[ui]
username = First Last <[e-mail]>

I did do this :) but it does not appear in my patch as created by mq. Anyway, I'll tackle this problem if I find myself contributing to Thunderbird more. Thanks!

Comment 9

[Nickolay Olshevsky]

I would suggest to not use 0 value, as this could lead to the 'zip bombing', when decompressed comparably small message takes huge amount of memory.
From the following: https://superuser.com/questions/139253/what-is-the-maximum-compression-ratio-of-gzip maximum normal compression ratio would be around ~1:1000. Probably it should be also limited by some upper bound, say 100Mb or so. Does Thunderbird have some limitation on maximum message size? This may be used here as well.

Comment 10

[Magnus Melin [:mkmelin]]

As far as I know there is no direct maximum message size. Of course messages over 10MB, or 25/50 MB at the most are very unlikely to get delivered to you due to server side limitations. Probably 100MB could be used to be super safe.

Comment 11

[Kai Engert [:KaiE:]]

The current value 10 was chosen based on the denial-of-service risk using the attack that Nickolay mentioned.
It was an arbitrary value chosen during development, and was waiting to be improved to something smarter.

I'm still not sure what a good upper bound should be.

It seems to me that mail server limits are irrelevant here. If the sender attaches a file that is 1000 MB big, but which can be compressed to 20 MB as part of the encryption, then the message will be sent and delivered.

We simply have to decide what we want to accept as the maximum message size that we are willing to process.

The issue here is that we don't stream this data. We will allocate buffers of the size of the decrypted and decompressed message, maybe we'll have 2 or 3 buffers of this size in parallel.

We can try 100 MB.

Comment 12

[Kai Engert [:KaiE:]]

Attachment: Bug 1686984 - Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a1b3d8f3762f
Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

Comment 14

[Kai Engert [:KaiE:]]

Comment on attachment 9202946 [details]
Bug 1686984 - Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: some messages cannot be decrypted
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): low, but user workstations are allowed to allocate much more memory for incoming messages

Comment 15

[Magnus Melin [:mkmelin]]

There's an error in the landed code, the ES is not a linting error.

Comment 16

[Magnus Melin [:mkmelin]]

I'll land a fix.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6f167dc97eb1
use right variable for max_decrypted_message_size. rs=me

Comment 18

[Kai Engert [:KaiE:]]

Thanks Magnus. The fix was in my local tree already, but I missed to run "hg amend"...

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9202946 [details]
Bug 1686984 - Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

[Triage Comment]
no uplift needed - already in "current beta"

Comment 20

[Kai Engert [:KaiE:]]

I failed to request uplift to esr78. I think we should uplift.

Comment 21

[Kai Engert [:KaiE:]]

Comment on attachment 9202946 [details]
Bug 1686984 - Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: some messages cannot be decrypted
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): low, but user workstations are allowed to allocate much more memory for incoming messages

Comment 22

[neal]

RNP has a streaming API. Using that is a much better fix.

Independently of that, I agree that an upper bound is needed, but x * encrypted_size is wrong. Also, you should use two upper bounds: a buffer threshold (100 MB?) and a decryption threshold (1 GB?). After the buffer threshold has been reached, Thunderbird should continue to decrypt and discard the decrypted data. Then, there is a chance that any signature could still verify correctly.

Despite this issues, the patch should go in (it's better than the status quo), but IMHO this issue should stay open until a better fix is applied.

Comment 23

[Jonas Woerlein]

This bug is currently blocking me from reading a daily notification email in Thunderbird 78.10.0. It would be really great if this bugfix could be delivered with the 78 release as well.

Comment 24

[Kai Engert [:KaiE:]]

Sorry for not having uplifted this fix to the stable 78.x branch yet.
We will consider to uplift it for the next 78.11 update.

In the meantime, you may help to test an experimental build, based on 78.10, which includes this fix.
You may download it from here:

linux 32bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Ys9h5MLbRfeRTms5OAmDqA/runs/0/artifacts/public/build/target.tar.bz2
linux 64bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/WdsT49XGQf6tOsYjlf6b0Q/runs/0/artifacts/public/build/target.tar.bz2
win 32bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/fYdctww6QqmPWfG51tbEPA/runs/0/artifacts/public/build/target.zip
win 64bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/OIJ81Vm2STiMAjoctGAiJQ/runs/0/artifacts/public/build/target.zip
macos: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/MhC5VaccQrqVI4ToLMMcYg/runs/0/artifacts/public/build/target.dmg

Note that if you chose to run this experimental build, you will not get automatic updates.
After testing, you should switch back to a regular Thunderbird build to continue to get updates.

Comment 25

[Kai Engert [:KaiE:]]

For reference, the above build can be seen here, including its patches:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=1a010761513bf231681810f50304362810a2cddc

Comment 26

[Wayne Mery (:wsmwk)]

Comment on attachment 9202946 [details]
Bug 1686984 - Allow larger compressed encrypted messages, max factor 1200, up to 100 MB. r=mkmelin

[Triage Comment]
Approved for esr78

Kai, up to you when you uplift to 78, sooner, or wait to 78.11

Comment 27

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/releases/comm-esr78/rev/48d9e8fcc601b2746e31fb6120ffc858363817fe
78.10.2

Comment 28

[Jonas Woerlein]

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

In the meantime, you may help to test an experimental build, based on 78.10, which includes this fix.

I'm sorry for my late reply to your message.

I have just tested opening the affected messages using the experimental build you referenced. The issue is resolved with this build.

Thanks for uplifting the fix to the 78 branch!