By Neal | May 22, 2021
In a blog post that I published two weeks ago, I wrote that RNP is dangerous, because it is too easy to misuse. I also explained how we avoid those problems in Sequoia. In this blog post, I will lay out how not one, but two CVEs have since come up in Thunderbird that seem to underline that danger.
One of the changes between Enigmail and Thunderbird 78’s OpenPGP
support is how secret key material is protected both on disk and in
memory. In Enigmail, OpenPGP keys were managed by gpg agent, which
is a separate process. GnuPG lets the user set a different password
for each key, and only caches that password for a configurable amount
of time. After that time, the password is forgotten, and the next
time the user wants to sign or decrypt a message, they need to reenter
it.
In Thunderbird’s reworked OpenPGP integration, the developers decided to use a different strategy. First, to simplify the integration, Thunderbird does not use any process separation. Instead, the secret keys are kept in the same address space as Thunderbird itself. This architecture makes Thunderbird potentially vulnerable to Heartbleed-style bugs. Second, when secret key material is imported, Thunderbird prompts the user for the key’s password, decrypts the key, and then reencrypts it using Thunderbird’s master password (if one is set).
A few days ago, Thunderbird issued CVE-2021-29956. The basic issue is that starting in Thunderbird 78.8.1 (released in March), a change accidentally removed the step where secret key material is reencrypted with the master password. Thus, secret keys imported since then were no longer protected by a password.
It’s useful to understand how those mistakes happen so that we can avoid them again in the future. In the issue, the Thunderbird developer explains what happened:
To fix the primary bug in Thunderbird, the following change needs to be reverted …
That will ensure that importing keys in the future will again result in protected keys.
It was my mistake to remove those lines. I had assumed that the new calls to
rnp_key_protectwould be sufficient to protect the keys. I should have verified my assumption. (The protection got lost when exporting the keys from the temporary area, although it shouldn’t have gotten lost.)
In short, the developer explains that he misunderstood the API.
It’s instructive to look at the context. The fix that the developer mentioned is a fix for another vulnerability, CVE-2021-29950. In that case, the developer forgot to reprotect the secret key material after use thereby leaving the secret key material in memory without any protection.
Because protection is a non-functional property, this bug was only found when someone was actively testing the protection, and asked if Thunderbird is working as intended. This nicely demonstrates the difficulty of testing non-functional properties: exactly because they are non-functional, everything appears to be working as intended. It is only when we actively think about the non-functional properties that we are able to test them. And an API that requires the user to thinking about non-functional properties places a great burden on the user of that API.
These mistakes were not made by an amateur; these issues weren’t introduced by a drive-by patch. The Thunderbird developer is the primary OpenPGP developer for Thunderbird and an experienced RNP user. That developer misunderstood RNP’s API. Twice. In the more recent vulnerability, RNP’s documentation for the relevant function is sparse and doesn’t contain an example. In the earlier vulnerability, the developer forgot to reprotect the secret key material after using it. This requirement is part of RNP’s API contract. But, it would be better if that constraint were enforced automatically. For instance, when the unprotected key goes out of scope, it would automatically be reprotected. (This is how we do it in Sequoia.)
I don’t claim that it is possible to design a useful API, which would always prevent these mistakes. Bugs happen. But, it is often possible to design an API so that its user doesn’t have to worry so much about the non-functional properties, and can instead concentrate on implementing the features that they want. This was exactly the focus of my post from two weeks: RNP’s API is dangerous. Unfortunately, these two CVEs appear to further make my point.
