Closed Bug 1510212 Opened 6 years ago Closed 5 years ago

Upgrading to Thunderbird 60.x from a berkeley DB based Profile to a sqlite one causes key data loss

Categories

(Thunderbird :: Security, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: b.neuburger, Unassigned)

References

Details

(Keywords: regression)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 Steps to reproduce: 1. Have a legacy (BDB based) Thunderbird Profile 2. Have a master password set, but no mail server passwords stored that get queried on startup 3. Upgrade from 60.2.1 or 52.9.1 to 60.3 4. Restart thunderbird 5. Close thunderbird 6. Start thunderbird again Actual results: 1. Stored passwords (e.g. for outgoing mail server) are gone 2. Stored certificates (my own and those of other people) and private keys are gone So it seems that in some cases the migration from a BDB based profile to a sqlite one fails for secret information since no master password is queried Expected results: Upon thunderbird deleting my BDB stored secrets it should have promped for my master password in order to succesfully migrate them to sqlite. For more details and please c.f. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913645
Component: Untriaged → Security
Summary: Upgrading from a berkeley DB based Profile to a sqlite one causes data loss → Upgrading from a berkeley DB based Profile to a sqlite one causes password data loss
Summary: Upgrading from a berkeley DB based Profile to a sqlite one causes password data loss → Upgrading from a berkeley DB based Profile to a sqlite one causes key data loss
Also see bug 1505038. All coming from bug 783994. This has already been discussed on the tb-drivers list, CC'ing Carsten Schoenert. Kai, any clues here?
Flags: needinfo?(kaie)
See Also: → 1505038
Sorry for the delay, I was away for a few days. I wonder why the bug is triggered with 60.3, but not with 60.2.1. Does 60.2.1 not yet use sql/key4 ? The migration to sql was already part of mozilla 58. Could the explanation be bug 1475775 ? Firefox decided to delete key3.db, and that was backported to ESR 60.2.1. It later was found to cause dataloss on RHEL/Fedora/Centos systems that remain at the BDB key3 storage. The upstream rememdy seems to target ESR 60.4 (bug 1496736). It seems I'll have to debug what exactly happens with ESR 60.3.
To clear something up: I only tested the migration with upstream version 60.3, the 60.2.1 my colleagues were coming from was the Debian packaged version. I am not sure if they delayed the migration in their packages or made it optional until 60.3. I was asked in the original Debian bug report linked above to try deleting pkcs11.txt as described in bug 1505038, I will report back as soon as I have done that, but this might take some more time.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(kaie)
I did experiments with TB 52.9, 60.2.1, 60.3.2, and my local build of the tip of the 60.x branch. All of those 60.x version contain the bug. The bug happens, if the old (pre-60) NSS DB (key3): - contains keys - has a master password set - is not unlocked during the first program session of a 60.x version What happens is this: - on startup, we init the gecko PSM module and the NSS library, which performs an automatic conversion to SQL. However, because the master password isn't available at the time of initial conversion, the contents of key3.db cannot yet be migrated to key4.db You can confirm that the database information is available in memory, during the first 60.x session, by opening the preferences, security, passwords tab. You can see that "use a master password" is CHECKED during this initial 60.x session. - Because of the code for bug 1475775, which unconditionally erases the key3.db, the old data is gone from disk. If you exit this session, without entering the (correct) master password, for example, because you don't use the saved password feature during this session, then the migrated data is not saved into key4.db and completely lost. - Bug 1496736 attempted a remedy for another issue related to the key3.db deletion, and the local ESS 60.x branch build I'm testing already contains that commit from bug 1496736 comment 29. It doesn't help. I think it wasn't ideal that the automatic deletion of key3.db was added into the Mozilla application code. It should have been integated into the NSS internal conversion code instead. Only NSS knows if the contents of key3.db have already been migrated to key4.db. The existence of key4.db is insufficient to draw an "already migrated" conclusion. If you want to produce a hotfix release to avoid the dataloss, I see the following options: (a) revert the changes from BOTH bug 1475775 and bug 1496736, and accept that we're temporarily exposed to CVE-2018-12383. That's easy. (b) revert the changes from BOTH bug 1475775 and bug 1496736, and implement a better fix to delete key3.db after migration inside of NSS. This will be require careful engineering to get it right.
Summary: Upgrading from a berkeley DB based Profile to a sqlite one causes key data loss → Upgrading to Thunderbird 60.x from a berkeley DB based Profile to a sqlite one causes key data loss
There's a potential third option, but which isn't guaranteed to work: (c) If during Thunderbird startup, you discover that the NSS database has a master password set, try to unlock it. This will be painful for users who don't like being prompted in sessions that don't require the private data in the NSS database. If the user cancels the prompt, and exits, this doesn't help at all.
Here's a workaround, for everyone who knows what they are doing. The workaround must be used exactly the first time you're starting up a thunderbird 60.x version, after having always used older versions before. After you have started thunderbird, open preferences, security, password. Click the "saved passwords" button. This will prompt you to enter your master password, and if successful, show you the list of saved accounts. Then quit Thunderbird. Reopen Thunderbird, open preferences again. If you still see the "use a master password" checkbox checked, the migration should have worked. Click the "saved passwords" button again, and they should still be there.
Depends on: CVE-2018-12383
Keywords: regression
Thank you for the analysis. There seem to be two aspects of this: It started with bug 1505038 where people lost password information; BTW, how is pkcs11.txt related? That's annoying but not fatal, users just have to enter their passwords again. The issue here is that certificates (see comment #0) get lost. They are harder to restore than typing a password in again. Do those get migrated with the same process as the stored passwords and get lost when the master password is not available? As for the solutions: I think it's a little late at 60.4 which is due out next week since TB 52.x users are being upgraded to TB 60.3.2 as we speak. That said, I like solution (c): A big promt saying: Security database upgrade, enter you master password or forever lose stored passwords and certificates.
(In reply to Jorg K (GMT+1) from comment #7) > Thank you for the analysis. There seem to be two aspects of this: It started > with bug 1505038 where people lost password information; BTW, how is > pkcs11.txt related? That's annoying but not fatal, users just have to enter > their passwords again. I don't understand yet what went wrong in the scenarios described in bug 1505038. pkcs11.txt replaces the old file secmod.db It doesn't contain sensitive information, it's simply a place for some configuration, such as smartcard drivers etc. Deleting pkcs11.txt deletes the manual configuration that some users might have applied. I read a comment (cannot find it anymore) that DOS line endings in file pkcs11.txt caused trouble? Maybe that caused NSS to ignore the NSS database (declared failure to open because of pkcs11.txt), and cause the inability to read the stored password? Files key3.db and key4.db contain a "symmetric private key", unrelated to certificates. If key3.db contains a master password, it cannot be migrated to key4.db until the master password is provided. Firefox/Thunderbird always store their saved passwords in encrypted form, using the symmetric private key. If you lose it, you can no longer access the saved logins. When not using a master password, the symmetric private key is stored without protection on disk. This approach (always encrypt saved logins) simplifies the switch between "have master password" and "no master password", because you don't need to re-encrypt the saved logins on switching. You only protect or unprotect that symmetric key. > The issue here is that certificates (see comment #0) get lost. They are > harder to restore than typing a password in again. Yes, unless the user has a backup, they cannot be recovered. The user has to obtain new certificates. > Do those get migrated > with the same process as the stored passwords and get lost when the master > password is not available? Yes. key3.db/key4.db contain both the symmetric private key used for saved passwords and the assymetric private keys that are related to certificates. > As for the solutions: I think it's a little late at 60.4 which is due out > next week since TB 52.x users are being upgraded to TB 60.3.2 as we speak. That's bad. I really would prefer to use the easy approach (a) in an immediate hotfix, hoping to reduce the amount of people that will experience dataloss. I think the CVE isn't really that dramatic. Because even if you automatically delete the key3.db file, the contents will most likely survive in the unused areas of filesystems for quite a while, and a user with physical access to a computer can do a forensic analysis of the harddisk to find it. > That said, I like solution (c): A big promt saying: Security database > upgrade, enter you master password or forever lose stored passwords and > certificates. You're suggesting that we automatically detect the single time this prompt is necessary, but that's difficult to do. The conversion happens very deep down in NSS, completely decoupled from GUI. And if we're already too late, because the wide upgrades are already being deployed, we cannot get this implemented quickly.
(In reply to Kai Engert (:kaie:) from comment #8) > That's bad. I really would prefer to use the easy approach (a) in an > immediate hotfix, hoping to reduce the amount of people that will experience > dataloss. OK, I can back those two bugs out on our release branch on mozilla-esr60 (as I have already backed out various other things), but we will only ship this fix in TB 60.4 in mid-December unless Wayne wants to spin a 60.3.3 right now.
Flags: needinfo?(vseerror)
This only affects users with Master Password, and then only those that go from 52.x to 60.x, correct? I hate this issue, so I like that we dug into this and that we have options. I also think CVE-2018-12383 isn't that bad. OTOH, I estimate only 25% of users are still on 52.9.1 and at current update rates will likely be at 10-15% by end of week (with some percentage 52.x users staying or reverting to 52.x because of lack of add-ons). Does this change your perspective? So I'm slightly on the fence about doing "hotfix" a). But I'm inclined to do it if we can ship a patched version within a day or two, because users who revert to 52.x (for whatever reason) are also affected. If it is longer then we might just wait for 60.4.0. The caveat is, does backing this out complicate future potential permanent fixes? Magnus, do you have any concerns?
Flags: needinfo?(vseerror) → needinfo?(mkmelin+mozilla)
For the record: Backing out those two bugs means backing out these two changesets: https://hg.mozilla.org/releases/mozilla-esr60/rev/300efdbc9fe1 https://hg.mozilla.org/releases/mozilla-esr60/rev/3bed863ee656 but hold on, the second one was uplifted to 60.4 and where' not there your. So it's the backout of one changeset for now. That can be done in five minutes and you can build 60.3.3. in ten minutes.
Wow, so much bad English: ... the second one was uplifted to 60.4 and we're not there yet. So it's the backout of one changeset for now.
The configuration to trigger this would appear to be pretty rare, but sure, not nice for the ones that are affected. A hotfix or waiting for 60.4.0 both work for me. This bug is further evidence the master password is a bad idea.
Flags: needinfo?(mkmelin+mozilla)
(In reply to Magnus Melin [:mkmelin] from comment #13) > This bug is further evidence the master password is a bad idea. This old argument again. We should keep it out of here. The master password is not a bad idea, it's only badly implemented, see bug 524403 and bug 973759.
https://hg.mozilla.org/releases/mozilla-esr60/rev/f0bcd8dbddfb8aa6c8992cf505501834a128e50a Backed out changeset 300efdbc9fe1 (bug 1475775 for causing bug 1510212) to build Thunderbird 60.3.3. a=jorgk
OK, so what does the backout buy us? The old versions are no longer deleted. So will TB attempt a new conversion next time it's started? I'm writing the release notes, current draft: - note: 'Thunderbird will upgrade security databases (key3.db, cert8.db to key4.db, cert9.db). The old version of those databases were deleted even if the databases were protected by a master password and the master password was not available during the upgrade. This is now no longer the case.'
Flags: needinfo?(kaie)
I made it this: Thunderbird will upgrade security databases (key3.db, cert8.db to key4.db, cert9.db). The old databases were deleted even if the databases were protected by a master password and the master password was not available during the upgrade. This is now no longer the case. So what can we add to this? Thunderbird will re-attempt the upgrade until successful??? The way I see it is that the old databases are now not deleted, but users have to remove the new ones to force another attempt? Or what's the story?
OK, I've got enough information via PM to improve the text. Thanks.
Flags: needinfo?(kaie)
After merging mozilla-esr60 60.4.0, re-landed first bug and then backed out both bugs: https://hg.mozilla.org/releases/mozilla-esr60/rev/28aff45820d64b2fe9ccfd2708da5a08ea608fd0 Revert backout of bug 1475775 (changeset f0bcd8dbddfb) to allow merge with 60.4. a=backout https://hg.mozilla.org/releases/mozilla-esr60/rev/0ced5dcd70178aa6b380084dd07bf5e45102779c Backed out changeset 3bed863ee656 (bug 1496736 for causing bug 1510212) to build Thunderbird 60.4.0. a=jorgk https://hg.mozilla.org/releases/mozilla-esr60/rev/c23331af8ac635f86d748c0eb1e5bbb8fcfd86d9 Backed out changeset 28aff45820d6 (bug 1475775 for causing bug 1510212) to build Thunderbird 60.4.0. a=jorgk DONTBUILD
Note that the backout in comment #15 for TB 60.3.3 apparently also fixed a crash. See bug 1509519 comment #5.

This seems to have stalled. How do we move forward?

Flags: needinfo?(kaie)

(In reply to Wayne Mery (:wsmwk) from comment #10)

The caveat is, does backing this out complicate future potential permanent
fixes?

No

(In reply to Jorg K (GMT+2) from comment #14)

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

This bug is further evidence the master password is a bad idea.
This old argument again. We should keep it out of here. The master password
is not a bad idea, it's only badly implemented, see bug 524403 and bug

Given that the master password protection is considered easy to break, it's another argument why CVE-2018-12383 isn't that dramatic for us.

On one hand we should push to get some improvements in bug 524403, because we might want to protect additional secrets with the master password in the future.

On the other hand, the master password only protects against casual attackers, who can easily copy files from the victim's disk - or against bugs, which allow a remote attacker to exploit a limited bug that can be used to steal files from disk, but not install backdoor software.

It cannot help against an advanced attacker, who uses the opportunity to access the victim's computer to install a backdoor and keystroke recorders, which can then be used to defeat any level of encryption we could use with the master password.

In other words, the master password feature is nice to have for people casually accessing your computer, but cannot be a real protection, even if we improve it. To protect against that, you need to use full disk encryption, always lock your display when you're away etc.

We didn't record the answer to comment 16 and 17, but comment 18 shows that I had communicated with Jörg outside bugzilla to provide the required information. Yes, if the old key3.db isn't deleted, then we will continue to attempt the migration at the next time the information is required.

(In reply to Wayne Mery (:wsmwk) from comment #21)

This seems to have stalled. How do we move forward?

We have fixed the issue for 60.x, accepting that the CVE remains unfixed.
I suggest to close this bug.

If we assume that some users are still on TB 52.x and might migrate at a later time (realistic, given TB 52.x supported add-ons), we need to continue protect users against this problem.

I suggest we again back this out on the TB 68.x branch. I'll file a new bug to track that.

We haven't yet worked on a safer solution for NSS and Firefox yet. Firefox still has the code to delete key3.db.

Flags: needinfo?(kaie)
See Also: → 1561366

On the other hand, the master password only protects against casual attackers, who can easily copy files from the victim's disk - or against bugs, which allow a remote attacker to exploit a limited bug that can be used to steal files from disk, but not install backdoor software.

There's another scenario that I didn't think of earlier: If the user stores backups to shared computers, not encrypting the backup files, it's much easier for an attacker to get access. With many users using cloud services, maybe that's a frequent scenario. Having a master password set could provide some protection against brute-forcing the backup files, if the master password protection is strong.

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

We have fixed the issue for 60.x, accepting that the CVE remains unfixed.
I suggest to close this bug.

closing

I suggest we again back this out on the TB 68.x branch. I'll file a new bug to track that.

bug 1561366

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
See Also: → CVE-2020-6794

As already explained, this issue was initially triggered by the fix for bug 1475775 (CVE-2018-12383), which we had backed out for Thunderbird.

We now have a better fix for Thunderbird, that we intend to ship in the next Thunderbird 68.x update, in a way that should avoid this dataloss. That new fix is tracked in bug 1606619.

You need to log in before you can comment on or make changes to this bug.