381164 - when Master Password is used, it is asked at every startup

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070518 Minefield/3.0a5pre ID:2007051801 [cairo]

repro:
start FF
set Master Password

close FF

open FF

result:
A dialog pops up that want's you to type the Master Password.
It can't be cancelled (X and [Cancel] don't work)

expected:
Not
This is very confusing

Comment 1

[u88484]

I know this wasn't an expected feature but I think it is great to have it this way.  I love being able to actually lock the browser and hopefully I can now do the same with thunderbird (haven't tried it yet).  There should have always been a way to lock a profile per se.  Why have profiles if you can't lock them.

Comment 2

[Peter van der Woude [:Peter6]]

(In reply to comment #1)
> I know this wasn't an expected feature but I think it is great to have it this
> way.  I love being able to actually lock the browser and hopefully I can now do
> the same with thunderbird (haven't tried it yet).  There should have always
> been a way to lock a profile per se.  Why have profiles if you can't lock them.
> 
I agree 100%, but the dialog does need some lines of text to explain why the Master Password is needed.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #1)
> I know this wasn't an expected feature but I think it is great to have it this
> way.

The master password doesn't actually "lock the profile", so showing it at startup is not a benefit.

I guess this is a side effect of the different way we read the passwords - I guess we decrypt them as they're loaded, or something?

Comment 4

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070518 Minefield/3.0a5pre
I just set a master password, but I see no dialog on startup. Maybe I could reproduce it when I'm using session restore while I'm logged in somewhere.

Comment 5

[Ria Klaassen (not reading all bugmail)]

But I do see the unresponsive close buttons so that you are forced to use your master password because you can't cancel out.

Comment 6

[Justin Dolske [:Dolske]]

(In reply to comment #0)

> A dialog pops up that want's you to type the Master Password.

Are you using any extensions that log into a site automatically? That's probably causing the storage module to fire up, which then reads and decrypts passwords. We've talked about changing this to better match the old behavior, which is probably the best thing to do.

(In reply to comment #5)
> But I do see the unresponsive close buttons so that you are forced to use your
> master password because you can't cancel out.

Please file a separate bug for that, as these two things will probably require separate fixes.

Comment 7

[u60234]

Mozilla/5.0 (Windows; U; Windows NT 5.1; sv-SE; rv:1.9a5pre) Gecko/20070518 Minefield/3.0a5pre

Steps to reproduce:
1. Start with a new profile
2. Go to a web site, let the Password Manager remember one web site login, and protect it with a master password.
3. Close and restart.

Actual result:
The master password prompt comes up on your start page.

If you don't want to type it in, you have to cancel out of two master password prompts for each password protected by the master password. If you have 10 saved password, you have to click "Cancel" on 20 master password prompts.

Comment 8

[Spencer Selander [greenknight]]

It doesn't actually happen at every startup, if you open to a blank page it doesn't happen. It seems to happen on any Web page, though, even ones that don't have a login.

Comment 9

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #6)
> 
> (In reply to comment #5)
> > But I do see the unresponsive close buttons so that you are forced to use your
> > master password because you can't cancel out.
> 
> Please file a separate bug for that, as these two things will probably require
> separate fixes.
>

Created Bug 381244 for this issue.

Comment 10

[Ria Klaassen (not reading all bugmail)]

I retested; the bug only happens when the start page has *some* input field.

No alert on:
http://histoforum.digischool.nl/ontstaan/kaarten.htm
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-win32-tbox-trunk/
http://newsrss.bbc.co.uk/rss/newsonline_world_edition/health/rss.xml

Alert on:
http://www.nu.nl/
http://www.google.com/
http://www.gezondheid.be/

Comment 11

[Bill Gianopoulos [:WG9s]]

OK so the real description of the issue here should be something like:

when Master Password is used, it is asked for on the first page with form elements instead of just on login pages.

It appears that the Master Password is being asked for before trying to auto-fill any form field, not just username and password.  It also happens if there is no saved data for the form, which is also not the previous behavior.

Previously it only asked for the master password if the page was asking for a login and there was saved information to complete the login sequence.

Prompting for a Master Password on a page where there is no saved login information will also be confusing to long time users.

Comment 12

[Michel VERET]

If this can help ...
I've 21 user name/password saved with a master password set.

I opened in order :
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-win32-tbox-trunk/
OK - no master password needed (Ria Klaassen site info).
http://histoforum.digischool.nl/ontstaan/kaarten.htm
OK - no master password needed (Ria Klaassen site info).
http://e107.org/news.php
Master Password request - I've no user and pass in that site but the fields exists in the main page
I had to clic 44 times on cancel before accessing the site page
http://www.google.fr/
Idem - not password item needed - same working (44 times cancel)

Came to bugzilla.mozilla.org :
I had to type my account and the password (they are saved)
I came to http://forum.hardware.fr/login.php?config=hardwarefr.inc
I got the same result.
I suppose that after "killing twice" (44 *2) the master password system seems "freezed"
The user name and the password fields are empty in the "password safe"
Only the user name and password saved during the session are shown.
I closed minefield and start it again (-P option)

I typed the master password (minefield startup page)
The user names and passwords are shown in the "password safe" (tools - option - security - show passwords)
Accessing to the "password save site" works well

Comment 13

[Justin Dolske [:Dolske]]

Attachment: Draft patch

This is mostly complete, just needs some more error checking and testcases. The basic idea is to defer decryption until something asks for the logins (which is similar to what the old pwmgr did).

This should also help with (if not outright resolve) a couple other bugs; dealing with invalid signons2.txt entries and having to click Cancel on the master password prompt a zillion times.

Comment 14

[Bill Gianopoulos [:WG9s]]

(In reply to comment #13)
> Created an attachment (id=266597) [details]
> Draft patch
> 

I tried building under windows with this patch and got idl errors about _encryptedUsername and _encryptedPassword:

c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:48: `_encryptedUsername' is not a valid identifier in IDL
c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:48: (Identifiers cannot start with an underscore)
c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:49: `_encryptedPassword' is not a valid identifier in IDL
c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:49: (Identifiers cannot start with an underscore)

Comment 15

[Bill Gianopoulos [:WG9s]]

(In reply to comment #14)
> (In reply to comment #13)
> > Created an attachment (id=266597) [details] [details]
> > Draft patch
> > 
> 
> I tried building under windows with this patch and got idl errors about
> _encryptedUsername and _encryptedPassword:
> 
> c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:48:
> `_encryptedUsername' is not a valid identifier in IDL
> c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:48:
> (Identifiers cannot start with an underscore)
> c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:49:
> `_encryptedPassword' is not a valid identifier in IDL
> c:/mozilla/trunk/toolkit/components/passwordmgr/public/nsILegacyLoginInfo.idl:49:
> (Identifiers cannot start with an underscore)
> 

I caned the names of those identifiers to not begin with '_', but the results were worse than without the patch.  It now never fills in login information nor does it ever prompt for the master password for a login attempt.

Additionally, the list of saved passwords is now blank.  If you are viewing Tools -> options -> Security -> Show Passwords and click on the show passwords button it then prompts for the Master password, but still only shows an empty window.  That is the only case I can find where it actually prompts for the Master Password.

Comment 16

[Justin Dolske [:Dolske]]

Hmm, I'm told |_foo| identifiers ought to work (they do on an OS X build), but I'll go ahead and change them. Thanks for catching some build-bustage early! :)

I haven't fully tested that patch, but I'll look at the issues to noted as I finalize things today.

Comment 17

[Justin Dolske [:Dolske]]

Attachment: Patch v1.

Patch for review.

This should bring behavior largely back to the way it was in FF2. I think it's close enough, considering that the way the master password prompting in FF2 worked was nowhere near 100% ideal. We can clean up those issues at part of the FF3 UI rework, though.

Comment 18

[Justin Dolske [:Dolske]]

Comment on attachment 266707 [details] [diff] [review]
Patch v1.

Ehh, this is slightly wrong. I'm not implementing and using nsILoginInfoInternal right, it ends up just being a JS object. Surprisingly, this works as long as you avoid calling .equals() [because everything else is just an attribute].

Comment 19

[Justin Dolske [:Dolske]]

Deferring to Alpha 6. (Should land in the next day or two though).

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #17)
> This should bring behavior largely back to the way it was in FF2. I think it's
> close enough, considering that the way the master password prompting in FF2
> worked was nowhere near 100% ideal. We can clean up those issues at part of the
> FF3 UI rework, though.

Justin, as you can read in bug 381164 (which was duped to that one) it appears also for Fiefox 2 and not only trunk. If you have an extension which automatically login on start of Firefox and you have a auth protected site open from your last session you will get this dialog multiple times on Windows or none dialog on Mac. So this is also broken on 1.8 branch and should ideally be fixed there. It's really annoying for everyone who wants to use that auto-login feature.

Comment 29

[Michael Olofsson]

(In reply to comment #12)

Confirmed in Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a6pre) Gecko/20070606 Minefield/3.0a6pre

If setting a non-login site as homepage there is no master password prompt on startup but setting for instance bugzilla.mozilla.org as homepage will invoke the prompt.

Comment 30

[John Collins]

Just an update, this was also present in 1.0, as seen here:
https://bugzilla.mozilla.org/show_bug.cgi?id=269138

Comment 32

[Tony-E]

The behaviour has changed believed due to the checkin of bug 380222

Rather than asking for the password at startup, it now asks for the master password the first time something is entered into a form field. The behaviour changed between build ids 2007062514 and 2007062515

Comment 33

[u88484]

^ Confirmed but I don't get the prompt when entering info into a form field, only when I load a page that I have my username/password saved (like it should be).  But I still can not cancel the dialog or close it without hitting canceling 2x amount of saved login info.  So this is only partially fixed now.

Comment 34

[Magnus]

I would like to add that the most annoying aspect of this behavior is not the first time it asks, but all the others. I have a master password protecting my login information, and guests using my computer are fine with pressing cancel once, but they get annoyed when they have to press cancel on every single login for forms that I have saved (gmail, university accounts, etc.). 

That is: The issue is not so much when it pops up for the first time, it is that it keeps popping up again and again.

(I am reporting based on the behavior for FF2, but just wanted to clarify this a bit).

Comment 35

[Justin Dolske [:Dolske]]

Yeah, 380222 probably helped things out some because it made fillDoc() smarter about when it requests passwords to (potentially) fill in the form... It ends up delaying the initialization of the storage module, but the module still decrypts on init -- and that's the root issue here.

I have a patch I'm doing some final testing on, and will attach later today.

(In reply to comment #34)
> I would like to add that the most annoying aspect of this behavior is not the
> first time it asks, but all the others. I have a master password protecting my
> login information, and guests using my computer are fine with pressing cancel
> once, but they get annoyed when they have to press cancel

Magnus, can you file a new bug on that issue? It's a different issue than this bug. Specifically, we don't currently have any way for a user to temporarily disable master password prompting. I think this would be a good feature, and help make using a master password less annoying in some cases.

Comment 36

[Magnus]

Thanks for your comments Justin, I did describe this behavior as case B in Bug 358617 (first comment in that bug), which is now resolved as a duplicate of this bug. I'm not using the nightly builds, so I am not sure how that behavior is affected by the check-in for this bug. Nevertheless, I did report this as an enhancement request in bug 385951. Thanks for your hard work!

Comment 37

[Justin Dolske [:Dolske]]

Attachment: Patch for review, v.2

The basic approach here is that the nsLoginInfo objects managed by the storage module will be kept in the encrypted state (by stashing .encryptedUsername and .encryptedPassword properties on the underlying JS object). Decryption will generally only be done when returning nsLoginInfo objects to a caller.

There is also some related code added to better handle canceling a master password prompt without causing sticky failures, and not keep prompting a zillion times when processing a page.

Comment 38

[Thorsten]

Just realised some strange behaviour of the pwmanager, when administrating my board. Login name and password were inserted in a completely wrong place and situation (i.e. creating a new category)

The image on http://img244.imageshack.us/my.php?image=pwmanagerme5.png shows the login name (changed it) written into the field of the forum's icon path, whereas the password was inserted into the first of two password fields used to define a password for a category.
Quite annoying, if you have to delete it every time creating a category.

I'm not sure what caused this behaviour and if it is related to your fix of bug 380222; hopefully, you have an idea.

[using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070626 Minefield/3.0a6pre ID:2007062604]

Comment 39

[Mike Connor [:mconnor]]

moving out, needs to make b1

Comment 40

[Sebastian Tschan]

(In reply to comment #37)
> The basic approach here is that the nsLoginInfo objects managed by the storage
> module will be kept in the encrypted state (by stashing .encryptedUsername and
> .encryptedPassword properties on the underlying JS object). Decryption will
> generally only be done when returning nsLoginInfo objects to a caller.

Would it be possible to keep user+pass in encrypted state even if a nsLoginInfo object is returned and only decrypt if user or pass is accessed?

This would make it possible to display the number of login possibilities without requiring to enter the Master Password - like with Firefox 2 Password Manager.

Comment 41

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 269944 [details] [diff] [review]
Patch for review, v.2

>Index: toolkit/components/passwordmgr/src/storage-Legacy.js

>     addLogin : function (login) {
>+        // We rely on using login.wrappedJSObject. addLogin is the
>+        // only entry point where we might get a nsLoginInfo object
>+        // that wasn't created by us (and so might not be a JS
>+        // implementation being wrapped)
>+        if (!login.wrapedJSObject) {

wrappedJSObject

>+            if (e.name == "NS_ERROR_NOT_AVAILABLE")
>+                userCanceled = true;

>+            if (e.name == "NS_ERROR_NOT_AVAILABLE")
>+                userCanceled = true;

Use e.result and compare against Components.results.NS_ERROR_NOT_AVAILABLE.

I'm a bit concerned about not having the "cancel" actually cancel the adding of a login should you later make a change that requires the master password, as mentioned on IRC. We can deal with that in followup bugs, though.

Comment 42

[:Gavin Sharp [email: [e-mail]]]

Oh, one other thing: you're removing the "remove bogus entry" code from _readFile, and don't seem to add it back anywhere. Is that going to cause problems?

Comment 43

[Justin Dolske [:Dolske]]

I filed bug 387617 about clicking "cancel" when adding a login.

I don't think the "bogus entry" removal is useful now. It was added for bug 231042, which (coincidentally) dealt with the user clicking cancel when adding a login. Apparently at the old pwmgr once write out an empty entry. That doesn't happen now. Seems unlikely that such entries would exist for anyone now.

Comment 44

[Justin Dolske [:Dolske]]

Attachment: Patch, checked in

Checking in toolkit/components/passwordmgr/src/nsLoginInfo.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginInfo.js,v  <--  nsLoginInfo.js
new revision: 1.5; previous revision: 1.4
done
Checking in toolkit/components/passwordmgr/src/storage-Legacy.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/storage-Legacy.js,v  <--  storage-Legacy.js
new revision: 1.10; previous revision: 1.9
done

Comment 45

[Carsten Book [:Tomcat]]

verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a8pre) Gecko/2007090304 Minefield/3.0a8pre ID:2007090304

Comment 46

[Jeff Walden [:Waldo]]

This is probably already covered by existing tests, but someone should double-check that.

Comment 47

[Frédéric Buclin]

(In reply to comment #45)
> verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US;
> rv:1.9a8pre) Gecko/2007090304 Minefield/3.0a8pre ID:2007090304

This bug seems to be back with Fx3b1. Does someone else see this problem again?

Comment 48

[Justin Dolske [:Dolske]]

No. Most likely you're loading a page/extension that has a password, and thus triggers the MP prompt.

Comment 49

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sorry Justin but than you really have duped bugs against this one which are totally different. I'll go through them and trying to fix it.

Comment 50

[Marcia Knous [:marcia]]

Litmus triage team: tomcat will handle Litmus testcase.

Comment 51

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=7727 added to Litmus.