Closed Bug 469221 Opened 16 years ago Closed 14 years ago

[1.8 branch only] Creating a Master Password should also set wallet.crypto pref to true

Categories

(Thunderbird :: Security, defect)

Product:
Thunderbird
Component:
Security
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: jbecerra, Assigned: dmosedale)

References

Details

(Whiteboard: [not a trunk issue])

While testing Thunderbird 2.0.0.19, if you rename the key3.db file found in the user's Thunderbird profile, you can by-pass the master password and be able to retrieve stored passwords. Steps: 1. Run Thunderbird on a new profile and set a master password 2. Configure Thunderbird to check email from, say, gmail 3. When you check email for the first time, specify to remember the password using the password manager. 4. Go to the preferences and check that your account/password are listed. 5. Close Thunderbird 6. Go to the user's Thuderbird profile and rename the key3.db file, for example "meh.db" 7. Restart the browser and check the list of stored passwords Expected: accounts/passwords should not be listed Actual: The passwords are there. The problem doen't happen in Firefox 2.0.0.19. It seems to happen on earlier versions of Thunderbird, like Tbird 2.0.0.4 on linux, for instance.
Flags: blocking1.8.1.19?
I verified that this occurs on TB 2.0.0.19 on Linux as well.
Who on the mail team knows this code? We need it looked at as soon as possible.
Mark & David, do either of you know that code? I suspect the people with the most knowledge about it are likely whoever knew the corresponding FF code. Would Kai be useful?
As some of you will recall, long ago, TB offered two choices of how to save passwords: - encrypted and bas64 encoded, or - merely "obfuscated" (unencrypted but base64 encoded). Sounds to me like the version with the problem is setting up to do encryption (establishing a master password) but then is merely obfuscating the passwords. That would be bad. It would probably not be an error in NSS.
The puzzling part is that the password manager component is what takes care of password encyption, and that part is shared between Thunderbird and Firefox. I don't understand why they would differ in this regard.
I don't know the code, other than the code we use to get and set passwords with the password manager when doing mail protocol. I'll look to see if we're somehow setting an option to merely obfuscate. But I share Gavin's confusion. I don't know if Kai would be useful or not...
Following the same steps in both Firefox and Thunderbird (with dummy passwords), and posting the resulting signons.txt files from each to see how they differ would probably be useful.
we're using wallet to store our passwords, as we always have: - http://mxr.mozilla.org/comm-central/source/mozilla/extensions/wallet/src/nsWalletService.cpp#208
(In reply to comment #4) > As some of you will recall, long ago, TB offered two choices of how to > save passwords: > - encrypted and bas64 encoded, or > - merely "obfuscated" (unencrypted but base64 encoded). > > Sounds to me like the version with the problem is setting up to do > encryption (establishing a master password) but then is merely obfuscating > the passwords. That would be bad. It would probably not be an error in NSS. How is this controlled?
Ok, so I was wrong about Firefox 2 and Thunderbird 2 sharing a password manager implementation then, since Firefox dropped wallet before 2.0. Sounds like this might be a problem only with wallet code.
It's not, but it should be set to true when you check the "Use a master password" checkbox. Mine's set to true and I can't reproduce the problem.
Dan, what's wallet.crypto in your config?
I've verified that turning on the master password in TB (trunk) sets wallet.crypto to true. I have not tried 2.0.0.19, but I doubt this code has changed on the trunk. I'll try a 2.0.0.x build next.
same thing with 2.0.0.x - turning on the master password sets wallet.crypto to true. Is it possible that wallet is caching the value of this pref, so that passwords you enter before a restart are not obfuscated?
On a related note: bug 316084 deals with the issue of *existing* base64-obscured passwords not being reencrypted (using the NSS SecretDecoderRing stuff) when enabling a master password.
So according to bug 316084 comment 9 we encrypt obscured passwords opportunistically, which means that simply flipping on the master password wouldn't necessarily have touched any of the passwords. But in the STR the password was saved AFTER the master password was set, so it really ought to have been encrypted.
(In reply to comment #17) > But in the STR the > password was saved AFTER the master password was set, so it really ought to > have been encrypted. Yes, hence the "on a related note". This bug clearly a different problem. [Also note that it's the new password manager that does the opportunistic reencryption, not wallet.]
I can't repro this on Thunderbird 2.0.0.18 or .19 following approximately the STR in comment 0. What I did: 1. Create new profile 2. Follow wizard to set up mail account 3. Read mail (asks for and saves password) 4. Set master password 5. Check that my password is listed in the password manager 6. Quit. move key3.db out of the way 7. Restart and try to view saved passwords or get mail. At this point I cannot open the password manager dialog -- it flashes briefly but disappears (no error on console). When I try to get mail it asks for the server password, it doesn't ask for the master password. The difference appears to be that Juan set the master password before saving a password, but I can't see how that would be different. I also was checking a regular IMAP account while I believe Juan said he was using a gmail account -- would that be handled differently?
I went ahead and tried going through the gmail account wizard -- same thing, passwords are encrypted and can't be retrieved when key3.db is missing.
I reproduced it at the same time as Juan. 1. Set up several accounts (imap, pop, gmail) 2. Retrieve mail, storing passwords for each permanently. 3. Turn on Master Password 4. Exit TB and rename key3.db to foo.db in profile. 5. Start TB 6. Go to preferences, check passwords. Master password is now not set but passwords are there in the clear for accounts. This was with TB 2.0.0.19 on Linux.
Al, that's the wallet/TB equivalent of bug 316084... TB 3 is going to use the password manager, not wallet - does that mean we'll get the fix for bug 316084 for free?
Well, it turns out I wasn't checking the "use a master password to encrypt your passwords" checkbox. But since I thought setting a master password would take care of that, and Tbird always prompted me to provide a master password thereafter to see my stored passwords, I expected my passwords not to be there when I renamed/removed the key3.db file. There might be a UI bug in here, but now this doesn't seem as big a problem.
I think most users assume that "setting a master password" does indeed enable encryption of passwords, and further, that all passwords are encrypted immediately when they do that. If the software does not work that way, then IMO that is a serious security problem.
(In reply to comment #22) > Al, that's the wallet/TB equivalent of bug 316084... Yep. Enabling the MP *after* the login was added is 316084. This bug is if enabling the MP *before* adding a login results in a new base-64 obscured entry. It's sounding like the original STR are not accurate. > TB 3 is going to use the password manager, not wallet - does that mean we'll > get the fix for bug 316084 for free? Yes. But... It was fixed as a side effect of fixing bug 288040, but that caused bug 451267, so we might end up unfixing 316084 and correcting it a different way.
I had no problem saving the password first and then checking the box to use the master password -- it was encrypted right away it appears. I don't think wallet/signons suffered from bug 316084. > I think most users assume that "setting a master password" does indeed > enable encryption of passwords, and further, that all passwords are > encrypted immediately when they do that. I agree. At the low level there's creating a master password through PSM/NSS, and separately telling Firefox to use PSM to encrypt things. We are unfortunately exposing that technical detail to users. On the other hand, if you really want your S/MIME certs to be secure and didn't care if your gmail/IMAP password was then we support that :-) I'm going to unhide this bug, there's no security problem and users will be potentially safer if it's public that you have to check the "Use a master password" checkbox. We should morph this bug: 1) when you create a Master Password in Thunderbird, the "use a master password" checkbox should be enabled and the wallet.crypto pref set to true. 2) if you remove a Master Password, the wallet.crypt pref should be set to false, passwords unencrypted first if possible (the user might have forgotten the master password and want to reset things), and then the master password removed. Or alternatively, remove the checkbox and behind the scenes set wallet.crypto according to whether there is or isn't a master password.
Group: core-security
Summary: renaming key3.db allows you to see stored passwords → Creating a Master Password should also set wallet.crypt pref to true
Since it's not broken or a regression we're not going to block a 2.0.0.x release, but I strongly urge the Thunderbird 3 team to come up with a less confusing UI.
Flags: blocking1.8.1.19?
Flags: blocking1.8.1.19-
Flags: blocking-thunderbird3?
(In reply to comment #27) > Since it's not broken or a regression we're not going to block a 2.0.0.x > release, but I strongly urge the Thunderbird 3 team to come up with a less > confusing UI. Given we're more than likely going to be moving onto toolkit's password manager, the preferences UI will change to be the same as Firefox's. So setting this dependent on the relevant bug. We won't need to do anything here if we fix that bug, but if we don't, then we will need to do something, hence granting blocking status.
Depends on: 239131
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Whiteboard: [Fixing bug 239131 means we don't have to fix this]
Target Milestone: --- → Thunderbird 3.0rc1
Given that (barring unforeseen lossage), this should be completely trivial to fix in JS by either auto-checking that checkbox, or simply removing it entirely and always encrypting when a password is set, I'd like to request that this does block 2.0.0.next. Assigning to clarkbw for his input on what UI he would prefer...
Assignee: nobody → clarkbw
Flags: blocking1.8.1.next+
Flags: blocking1.8.1.next+ → blocking1.8.1.next?
Dan, I see you accidentally said you wanted to "request" this block and then marked it as blocking+ only to move it back to blocking?. This must have been a unintended mistake since anything you'd like to block in the Thunderbird (or Mailnews) product is ultimately your decision. :) Let's get a 1.8 branch patch ready to go as soon as possible so it can get some testing in the nightlies.
Flags: blocking1.8.1.next? → blocking1.8.1.next+
OK, fair enough. I did that because I wasn't sure how much ownership you and Dan felt over the 2.0.0.x process.
Flags: wanted1.8.1.x+
If I'm understanding correctly that we can just fix this to auto-check the checkbox such that it chooses the "right thing" I'm leaning toward just removing the checkbox all together. Would we get our user base into trouble if the option was removed completely? Assuming as always that if it's required for debug / troubleshooting that it'd still be available via about:config
Bug 239131 is now fixed, so this is no longer an issue on trunk.
Whiteboard: [Fixing bug 239131 means we don't have to fix this] → [not a trunk issue]
Since this is no longer an issue on the trunk, setting blocking-thunderbird3-.
Flags: blocking-thunderbird3+ → blocking-thunderbird3-
Assignee: clarkbw → nobody
Summary: Creating a Master Password should also set wallet.crypt pref to true → [1.8 branch only] Creating a Master Password should also set wallet.crypt pref to true
Target Milestone: Thunderbird 3.0rc1 → ---
dmose: Who can own this bug?
Summary: [1.8 branch only] Creating a Master Password should also set wallet.crypt pref to true → [1.8 branch only] Creating a Master Password should also set wallet.crypto pref to true
Assignee: nobody → dmose
clarkbw: I'm not aware of any such problems, but I don't have a tremendous amount of context with the history of this specific bit of UI. Perhaps philor has some thoughts?
The only issue I know about is the comment 26 one: it's currently possible to have a master password which was forced upon you while importing an S/MIME cert, which you did not use to encrypt your server passwords, and which you forgot in 2003. Of course, we're going to hose those people when they update to 3.0, but that doesn't make it appropriate to hose them in a .21. Offhand, I don't see a problem with autochecking the checkbox when someone is actively setting a master password through the prefs UI, but if you remove the checkbox then you make it impossible for someone to encrypt their passwords if their steps are 1. Create account, don't sign in 2. Import S/MIME cert, create master password as part of import 3. Go to prefs to try to have the master password used for saved server passwords unless you make just opening the prefpane set wallet.crypto, in which case you are silently hosing the people with a master password they've forgotten. (But, um, you really sure you want to do this in a .21? This is exactly how things have been since 1.0, no? And I don't mean Thunderbird 1.0, I mean Mozilla 1.0.)
Phil's comment makes it clear that there's a bunch of hidden complexity here which makes it scarier for a stability branch that I had realized. I agree with his suggestion: since we've lived with this for so long, we should let this sleeping dog lie in the 2.0.0.x time frame. Adjusting the flags appropriately.
Flags: wanted1.8.1.x-
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.next-
Flags: blocking1.8.1.next+
my reading of the comments is this amounts to WONTFIX (and 2.0 is gone). please adjust if this is not correct.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.