1673239 - (CVE-2021-29950) RNP-01-012 WP1 Thunderbird: Logic issue potentially leaves key material unlocked (Medium)

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Wayne Mery (:wsmwk)]

During the audit of the RNP source code, it was discovered that there is a potential logic flaw related to locking/unlocking and protecting/unprotecting keys. The function - named rnp_key_unlock() - unlocks a key and has secret key material for use without password protection. This puts all values into memory so the key becomes usable for corresponding operations (e.g. changing attributes or exporting keys).

As an example, the flow of operations is as follows:

1. Unlock the key by invoking rnp_key_unlock()
2. Perform some operations on the key, e.g. export it by using rnp_key_export()
3. Lock the key again by invoking rnp_key_lock()

It was identified that the rnp_key_lock() might not be invoked in case the desired operation, in the example above rnp_key_export(), is running into an error. This situation potentially allows an attacker to perform operations on already unlocked keys, without needing to unlock them beforehand.

A similar pattern has been observed for rnp_key_protect/rnp_key_unprotect(), which gets invoked when keys are imported through importKeyBlockImpl(). A protected key is encrypted and can be safely held in memory. Invoking rnp_key_unprotect() on a given key removes the encryption from the key.

Affected File:
comm/mail/extensions/openpgp/content/modules/RNP.jsm

Affected Code:
refer to Attachment 9182160 [details]

Locking/protecting the keys in case an operation performed on an unlocked/unprotected key fails is important because it reduces the potential risk of having unlocked/unprotected keys in memory. Therefore, it is recommended to properly catch any occurring exceptions and re-lock/protect keys again.

Comment 1

[Kai Engert [:KaiE:]]

I will attach a first revision of a fix, which is currently untested. I'm attaching it anyway, phabricator might help me self-review more easily.

The fix will affect the following actions. We need to test these actions still work correctly with the new patch.

• key generation
• import secret key
• backup secret key
• change expiration date

Comment 2

[Kai Engert [:KaiE:]]

Attachment: Bug 1673239 - OpenPGP key material should remain locked in memory in failure scenarios. r=mkmelin

Comment 3

[Kai Engert [:KaiE:]]

I've implemented automated tests for secret keys in bug 1693554.
Tests pass without and with the fixes from this bug.

Comment 4

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/b14afc010507d8d5857bc739dc75ed022e30792e

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9203762 [details]
Bug 1673239 - OpenPGP key material should remain locked in memory in failure scenarios. r=mkmelin

[Approval Request Comment]
User impact if declined: sec issue
Testing completed (on c-c, etc.): on beta
Risk to taking this patch (and alternatives if risky): doesn't seem very high

Comment 6

[Wayne Mery (:wsmwk)]

Comment on attachment 9203762 [details]
Bug 1673239 - OpenPGP key material should remain locked in memory in failure scenarios. r=mkmelin

[Triage Comment]
Approved for esr78

Comment 7

[Rob Lemley [:rjl]]

Thunderbird 78.8.1:
https://hg.mozilla.org/releases/comm-esr78/rev/0c8606e7f45d

Comment 8

[Wayne Mery (:wsmwk)]

Kaie, will you be arranging a security advisory for this?

Comment 9

[Kai Engert [:KaiE:]]

Sorry for having missed this at the time of release. I will work with Tom to check if we can release a CVE late.