Last Comment Bug 397122 - NSS 3.12 alpha treats a key3.db with no global salt as having no password
: NSS 3.12 alpha treats a key3.db with no global salt as having no password
Status: RESOLVED FIXED
: regression
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: x86 Windows XP
: P1 critical (vote)
: 3.12
Assigned To: Nelson Bolyard (seldom reads bugmail)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-21 16:29 PDT by Justin Dolske [:Dolske]
Modified: 2007-10-25 18:19 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Testcase: key3.db (12.00 KB, application/octet-stream)
2007-09-21 16:29 PDT, Justin Dolske [:Dolske]
no flags Details
Testcase: signons2.txt (230 bytes, text/plain)
2007-09-21 16:30 PDT, Justin Dolske [:Dolske]
no flags Details
ignore missing salt when checking for password (983 bytes, patch)
2007-09-22 00:56 PDT, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2007-09-21 16:29:51 PDT
Created attachment 281895 [details]
Testcase: key3.db 

[Spinoff from bug 393835, which was getting long and messy.]

Reporter filed a bug regarding a loss of stored passwords after switching from Firefox 2 to a Filefix 3 alpha (trunk). We were able to recreate the problem using the reporter's key3.db (on XP and OS X).

If I enable password manager debugging, I see this logged to the console when it's invoked:

PwMgr Storage: Initializing key3.db with default blank password.

This is generated in http://mxr.mozilla.org/seamonkey/source/toolkit/components/passwordmgr/src/storage-Legacy.js:

 132         // Check to see if the internal PKCS#11 token has been initialized.
 133         // If not, set a blank password.
 134         var tokenDB = Cc["@mozilla.org/security/pk11tokendb;1"]
 135                             .getService(Ci.nsIPK11TokenDB);
 136 
 137         var token = tokenDB.getInternalKeyToken();
 138         if (token.needsUserInit) {
 139             this.log("Initializing key3.db with default blank password.");
 140             token.initPassword("");
 141         }

This indicates that |token.needsUserInit| is returning true for some reason. The branch version of this code is identical (although in C++).

[Another issue here is that the .initPassword() call seems to succeed, but it shouldn't because there's already a master password set. I'm hoping that's just a side-effect of whatever is broken.]

To reproduce:
1) Create a new profile with Firefox 2, and quit browser.
2) Copy the attacked key3.db / signons2.txt into the new profile.
3) Start Firefox 2, view all passwords. Enter "TestPwd01" when prompted for the master password. Note that there is 1 login stored (google.com, user = "username", pass = "password").
4) Quit Firefox 2, start Firefox 3.
5) Go to view all passwords... Note that the "Use a Master Password" checkbox is no longer checked, and that no passwords appear to be stored.
6) Enabling the MP as "TestPwd01" doesn't help.

I also tried to recreate to problem *without* using the reporter's key3.db, by starting with a new FF2 profile, doing a variety of things, and then updating to Firefox 3... That always worked. I'm not sure what's special about this supplied key3.db, but if it works fine in FF2 it ought to work fine in FF3.
Comment 1 Justin Dolske [:Dolske] 2007-09-21 16:30:20 PDT
Created attachment 281896 [details]
Testcase: signons2.txt
Comment 2 Nelson Bolyard (seldom reads bugmail) 2007-09-21 23:47:11 PDT
This key3.db file is very strange. 
It is only 12KB in length.  
All Key3.db files created since 3.9 (at least) at minimum 16KB in size.  
It IS a version 3 key DB But it has no Key DB Global Salt.  

Because it is so strange, I suspect it was created with a VERY old 
version of NSS.  I wonder if the system on which it was created has some
old OLD NSS shared libraries in the PATH somewhere (LD_LIBRARY_PATH on 
Linux/Unix).

When I put it into a directory with a vanilla empty cert8.db and minimal
secmod.db (no root module), and test it with the command
   certutil -K -d dbdir 
I get different results depending on the version of NSS that I am using.

NSS 3.9.x and 3.11.x prompt for the password and then list the DB contents 
as follows:

Enter Password or Pin for "NSS Certificate DB":
<0> Imported Certificate
<1> Imported Certificate #3
<2> Imported Certificate #2
<3> Root CA ID von CAcert WoT User

NSS 3.12 alpha (trunk) does not prompt for a password, and reports:

certutil: no keys found

I am still debugging this, but this seems very likely related to the 
big DB changes in 3.12, so I am assigning this to Bob.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2007-09-22 00:16:38 PDT
I stepped through the certutil -K command on both 3.11 and the trunk.
Both processes got through NSS initialization without any problem.
Both observed that the version was 3 and there was no global key DB salt.

The first difference that I observed was when certutil's function listKeys
called PK11_Authenticate, which called PK11_DoPassword, which called 
PK11_NeedUserInit.  In 3.12, PK11_NeedUserInit returned PR_TRUE but in 
3.11.x it returned PR_FALSE.

Further investigation shows that when NSC_GetTokenInfo calls 
sftkdb_HasPasswordSet(handle), that function returns SECFailure.

sftkdb_HasPasswordSet(handle) calls lg_GetMetaData (a new function in 3.12)
which calls nsslowkey_GetPWCheckEntry, which returns SECFailure because
there is no key DB global salt.  GetKeyDBGlobalSalt returns NULL.

So, in NSS 3.12 alpha, the absence of of a key DB global salt causes 
NSC_GetTokenInfo to conclude that there is no password set.  It sets
CKF_LOGIN_REQUIRED but not CKF_USER_PIN_INITIALIZED, and that makes 
PK11_NeedUserInit return true.  It's all downhill from there.

Next I will go back and look at the behavior of NSC_GetTokenInfo in 3.11.x
Comment 4 Nelson Bolyard (seldom reads bugmail) 2007-09-22 00:44:05 PDT
As an experiment, I changed nsslowkey_GetPWCheckEntry to simple set the 
entry.salt.len to zero if there was no salt, and to continue, rather than 
failing.  This makes certutil -K run to completion as before.

I will attach a patch with the change that I made to nsslowkey_GetPWCheckEntry
However, I am not at all certain that this is the right change to make.

The questions I have are: 
- What is the significance of the key DB not having a global salt?  and,
- What should we do about it?  Should we create one?  and,
- How does a key3.db get into that state of having no global salt?
Comment 5 Nelson Bolyard (seldom reads bugmail) 2007-09-22 00:56:22 PDT
Created attachment 281924 [details] [diff] [review]
ignore missing salt when checking for password

Bob, please review, and please read the preceding comments in this bug 
before giving your review comments.  Again, while this solves the 
immediate symptom, I am not sure it is right.  The cure may be worse
than the disease, if it leaves the key DB in some insecure state.

If you know a better solution, please take this bug and implement that 
better solution.
Comment 6 Robert Relyea 2007-09-24 10:40:05 PDT
Comment on attachment 281924 [details] [diff] [review]
ignore missing salt when checking for password

r+ rrelyea
Comment 7 Robert Relyea 2007-09-24 10:53:52 PDT
More comments about lack of a global salt:

The global salt is used to 'salt' the password entry directly. When NSS uses the PBE for it's key database, it passes in a 'Password' to the PBE which is the hash of the users real password with the global salt. If the gobal salt does not exist, then a given password will always hash to the same key.

The loss of the global salt in a single, or small number of databases isn't a big problem, but a widespread outage would be.

Changing the password on the database should correct the problem.
Comment 8 Robert Relyea 2007-09-24 11:00:19 PDT
BTW each entry also has a local salt value, to things like the password check entry will have the different values even for the same key without any global salt.

bob
Comment 9 Nelson Bolyard (seldom reads bugmail) 2007-09-24 14:49:03 PDT
I think we should not silently ignore the absence of a global salt.
If we have the ability to create one, we should.  If we need to decrypt
and re-encrypt some entries to use the new salt (do we?), we should.  

I agree that we should find out how widespread this problem is.  If some
code is now routinely generating key DBs without global salts, we should 
fix that ASAP.

The first place I'd suggest we look is in the reset master password (key db)
code.
Comment 10 Nelson Bolyard (seldom reads bugmail) 2007-09-24 15:11:52 PDT
In reply to comment 8, then what is the global salt used for?
Comment 11 Robert Relyea 2007-09-24 17:10:40 PDT
It salts the password key (the magic key NSS keeps internally whenever you are logged in).

master reset removes all the entries in the key database and returns the keydb to NEED_INITIALIZE state (all the keys and the password entry are gone).

If it had just removed the salt, there would be no way to decrypt any of the entries in the database (even if it had 'no password'). It's clear the password entry was generated with no global salt.

Anyway the attack vector for having no global salt is an on machine-invade-the-process attack (not really very interesting).

bob
Comment 12 Nelson Bolyard (seldom reads bugmail) 2007-09-24 18:07:01 PDT
Bob, can't a dictionary attack be mounted against a password encrypted with
no salt?  I wouldn't call that an "invade-the-process" attack.  The attacker
needs only to get a copy of the key3.db file.
Comment 13 Nelson Bolyard (seldom reads bugmail) 2007-09-24 18:15:19 PDT
Checking in keydb.c;  new revision: 1.5; previous revision: 1.4

The immediatel symptom is fixed by this commit, but I'm not sure I'd 
call the problem fixed.
Comment 14 Nelson Bolyard (seldom reads bugmail) 2007-09-24 20:06:26 PDT
I will file a separate bug about the existence of key DBs without global salts.
Comment 15 Robert Relyea 2007-09-25 08:56:58 PDT
re comment 12: No. each entry also has it's own salt (including the password check entry), so even without the global salt, each entry is diversified. The global salt only affects the intermediate derived key, so the only practical dictionary attack is one which can access the intermediate derived key (which requires access to the process).

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