Closed Bug 398886 Opened 17 years ago Closed 17 years ago

resetting a master password does not clear existing password manager logins

Categories

(Toolkit :: Password Manager, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: tracythefreak, Assigned: Dolske)

References

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7

I reset my master password, which cleared all my save passwords as well.
After doing so, the password manager stopped prompting me to save passwords for any websites that I had on the list before the reset.

Reproducible: Always

Steps to Reproduce:
1.set a master password
2.reset it
3.enter a password for a previous website.
Actual Results:  
the password manager did not open a prompt asking if I would like my password saved for the website.
This fixes the obvious problem on trunk, in that when resetting the master password the stored logins are not cleared.

The only side effect of not clearing the stored logins *should* be that some undecryptable entries will lurk around, so I'm not completely what's happening with the reported problem.

Unfortunately resetting the master password is crashing on trunk right now, so I can't test this further just yet.
Assignee: nobody → dolske
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #283880 - Flags: review?(kaie)
Assignee: dolske → nobody
Status: ASSIGNED → NEW
Component: Password Manager → Libraries
Depends on: 397178
Product: Firefox → NSS
QA Contact: password.manager → libraries
Version: unspecified → trunk
Moving to NSS.

Also, note that this UI isn't exposed in FF so to test it you need to enter  chrome://pippki/content/resetpassword.xul in the URL bar.
Assignee: nobody → dolske
Severity: major → normal
I see no description of any NSS problem here.  Nothing in this bug 
describes NSS doing something it is not intended to do.

A user is not prompted for a master password when his key3.db has no 
master password.  

Resetting the master password is NOT the same thing as simply decrypting
the signons.txt file.  

Resetting the master password means erasing the key3.db and putting it 
in a state where it has no master password.   All encryption keys 
formerly in the key3.db are lost.  All passwords in signons.txt that were 
encrypted with the keys in the old key3db are effectively lost forever.  
Resetting the master password is to be done only when the master password
has been lost.  

It is possible to decrypt the entire signons.txt file, and preserve the (formerly encrypted) website passwords, provided that the master password 
is known (not lost).  But AFAIK, FireFox does not provide any UI by which 
the user may do so.  SeaMonkey does provide such UI, but FireFox does not.

If FireFox wishes to enforce a policy in which, after the master password
is reset, the user is then forced to set a new master password before the
password manager will remember any new site passwords, it is up to FireFox
to implement that, not NSS.
Assignee: dolske → nobody
Component: Libraries → Password Manager
Product: NSS → Firefox
QA Contact: libraries → password.manager
Version: trunk → unspecified
Giving this back to Justin.
Assignee: nobody → dolske
Attachment #283880 - Flags: review?(kaie) → review?(kengert)
In general, when you reset your master password (and therefore throw away the secret key that was used to encrypt your stored passwords), then I'm ok to erase all such stored passwords.

In comment 0 you said, you reset the master password. How did you do that? I can't find how one does that in Firefox prefs. (I only know how to do it in SeaMonkey.)
Kai: see comment #2: The "Reset MP" page is not exposed in the FF UI (we probably should...), but doing it this way is listed at both kb.mozillazine.org and support.mozilla.com.
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

(In reply to comment #5)
> In comment 0 you said, you reset the master password. How did you do that? I
> can't find how one does that in Firefox prefs. (I only know how to do it in
> SeaMonkey.)

Oh, you answered that question in comment 2 already. 

So right now, this is actually core or SeaMonkey bug.

Because SeaMonkey gives a clear warning that all stored passwords will be lost, I'm ok with adding such a call to PSM (although I can't review whether the call itself is correct).

r=kengert


Someone who knows about login-manager should probably sr this patch.
Attachment #283880 - Flags: review?(kengert) → review+
(In reply to comment #1)
> Unfortunately resetting the master password is crashing on trunk right now, so
> I can't test this further just yet.

Where does it crash? Do you have a stack?
(In reply to comment #8)
> (In reply to comment #1)
> > Unfortunately resetting the master password is crashing on trunk right now, so
> > I can't test this further just yet.
> 
> Where does it crash? Do you have a stack?

See bug 397178 (and bug 397296).
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

As this is about password manager, I'd like to ask Mike if these calls look right.

(Note that "reset password" is not currently exposed in Firefox UI.)
Attachment #283880 - Flags: superreview?(mconnor)
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

so, this is ok for now.

The cleaner/extensible solution would be to somehow fire a notification for the various consumers to observe on reset, since anything like an extension using this won't be able to hack themselves into PSM (or might be noscript C++ interfaces for some unholy reason)

future fodder though.  Ideally we'd let users reset their MP in the UI (I thought we had this when I added MP support to the UI...)
Attachment #283880 - Flags: superreview?(mconnor) → superreview+
Attachment #283880 - Flags: approvalM9?
Attachment #283880 - Flags: approval1.9?
This is extremely low-risk, so if we can get it into M9 that would be handy.
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

a=endgame drivers for M9
Attachment #283880 - Flags: approvalM9?
Attachment #283880 - Flags: approvalM9+
Attachment #283880 - Flags: approval1.9?
Attachment #283880 - Flags: approval1.9+
Summary: password manager is not prompting me with the question of saving my password for websites → resetting a master password does not clear existing password manager logins
Checking in security/manager/pki/resources/content/resetpassword.js;
/cvsroot/mozilla/security/manager/pki/resources/content/resetpassword.js,v  <--  resetpassword.js
new revision: 1.10; previous revision: 1.9
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M9
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

>+    loginManger.removeAllLogins();

s/loginManger/loginManager/
Oops, typo, fixed.

/cvsroot/mozilla/security/manager/pki/resources/content/resetpassword.js,v  <--  resetpassword.js
new revision: 1.11; previous revision: 1.10
Blocks: 137197
Product: Firefox → Toolkit
Comment on attachment 283880 [details] [diff] [review]
Patch for review, v.1

>+  try {
>+    var loginManager = Components.classes["@mozilla.org/login-manager;1"].
>+                       getService(Components.interfaces.nsILoginManager);
>+    loginManger.removeAllLogins();
>+  } catch (e) {
>+  }
>+
>   var pref = Components.classes['@mozilla.org/preferences-service;1'].getService(Components.interfaces.nsIPrefService);
>   if (pref) {
>     pref = pref.getBranch(null);
>     try {
>       if (pref.getBoolPref("wallet.crypto")) {
>         // data in wallet is encrypted, clear it
>         var wallet = Components.classes['@mozilla.org/wallet/wallet-service;1'];
>         if (wallet) {
[This code that did the equivalent for wallet can go, as wallet is dead.]
Perhaps you should file a new bug instead of just dropping a comment into a bug that was fixed 2 1/2 years ago.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: