1748918 - OpenPGP decryption broken on comm-central caused by whitespace passed to Base64Decode

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Kai Engert (:KaiE:)]

Open an encrypted message with current comm-central, it fails to decrypt, and the error console shows:

Exception { name: "NS_ERROR_ILLEGAL_VALUE", message: "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsISecretDecoderRing.decryptString]", result: 2147942487, filename: "chrome://openpgp/content/modules/masterpass.jsm", lineNumber: 191, columnNumber: 0, data: null, stack: "_readPasswordFromFile@chrome://openpgp/content/modules/masterpass.jsm:191:16\nsync@chrome://openpgp/content/modules/cryptoAPI/interface.js:56:15\npassword_cb@chrome://openpgp/content/modules/RNPLib.jsm:576:23\ndecrypt@chrome://openpgp/content/modules/RNP.jsm:953:30\ndecrypt@chrome://openpgp/content/modules/cryptoAPI/RNPCryptoAPI.jsm:245:16\ndecryptMime@chrome://openpgp/content/modules/cryptoAPI/RNPCryptoAPI.jsm:272:17\nonStopRequest@chrome://openpgp/content/modules/mimeDecrypt.jsm:544:42\n", location: XPCWrappedNative_NoHelper }
​

Comment 1

[Kai Engert (:KaiE:)]

The failure is in SecretDecoderRing::DecryptString, when trying to access the passphrase that we store in encrypted-openpgp-passphrase.txt

The string we pass includes \n and causes Base64Decode to fail.

I don't know if recent changes cause us to include the newline, or if recent changes cause the Base64Decode to fail.

Regardless of the cause, it's fine to add a call to trim() to fix the issue.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Bug 1748918 - Trim whitespace from base64 string read from encrypted-openpgp-passphrase.txt. r=mkmelin

Comment 3

[Magnus Melin [:mkmelin]]

I don't see a problem - encrypted messages decrypt just fine, and automated tests would also have picked this up, no? I wonder if you accidentally had modified that file yourself manually in the profile?

Comment 4

[Kai Engert (:KaiE:)]

The file in this test profile was last changed almost 2 years ago. It contains a newline. I've been using this profile regularly.

In my primary profile, there's no newline in the file.

You could try to edit that file yourself and add a newline, and see if it causes you to crash with a daily build.

I don't think the bug is in a change to the file, it's how our code handles these files. Apparently we were less strict in the past (whether "we" is gecko or comm I do not know.)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/21dcdc4da813
Trim whitespace from base64 string read from encrypted-openpgp-passphrase.txt. r=mkmelin

Comment 6

[Kai Engert (:KaiE:)]

This bug was introduced with this changeset
https://hg.mozilla.org/comm-central/diff/d7dc20e53bb81c31891c2a1a74744a450e335312/mail/extensions/openpgp/content/modules/masterpass.jsm
from bug 1677088.

The old code in function had used "EnigmailFiles.readFile(path).trim()",
but the new code didn't have trim.