Closed Bug 1673239 (CVE-2021-29950) Opened 1 year ago Closed 10 months ago

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

Categories

(MailNews Core :: Security: OpenPGP, defect)

defect

Tracking

(thunderbird_esr78+ fixed)

RESOLVED FIXED
87 Branch
Tracking Status
thunderbird_esr78 + fixed

People

(Reporter: wsmwk, Assigned: KaiE)

References

Details

(Keywords: sec-moderate)

Attachments

(1 file)

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.

Assignee: o.nickolay → nobody
Summary: RNP-01-012 WP1 RNP: encrypt_secret_key() does not wipe keybuf from memory (Low) → RNP-01-012 WP1 Thunderbird: Logic issue potentially leaves key material unlocked (Medium)
Whiteboard: [RNP]
Assignee: nobody → mkmelin+mozilla

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

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

Assignee: mkmelin+mozilla → kaie
Status: NEW → ASSIGNED
Target Milestone: --- → 87 Branch
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED

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

Attachment #9203762 - Flags: approval-comm-esr78?

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

Attachment #9203762 - Flags: approval-comm-esr78? → approval-comm-esr78+

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

Flags: needinfo?(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.

Flags: needinfo?(kaie)
Regressions: CVE-2021-29956
Group: mail-core-security
You need to log in before you can comment on or make changes to this bug.