Closed Bug 1379273 Opened 5 years ago Closed 5 years ago

With NSS sql: file format, after PK11_ResetToken(), call to PK11_InitPin() fails, produces broken database


(NSS :: Libraries, defect)

Not set


(Not tracked)



(Reporter: KaiE, Assigned: keeler)



This was found by executing a Firefox PSM test, 
when using NSS that defaults to the SQL file format.

To reproduce this more easily, I've patched certutil.

Existing code in certutil.c for command -T :
    if (certutil.commands[cmd_TokenReset].activated) {
        char *sso_pass = "";

        if (certutil.options[opt_SSOPass].activated) {
            sso_pass = certutil.options[opt_SSOPass].arg;
        rv = PK11_ResetToken(slot, sso_pass);

Immediately after that line, I added:
        rv = PK11_InitPin(slot, NULL, sso_pass);

Now run:

mkdir /tmp/sql
certutil -d sql:/tmp/sql -N
(enter any password twice, just pressing enter is ok, too)

certutil -d sql:/tmp/sql -T
(which calls both PK11_ResetToken and PK11_InitPin.

When executing this in a debugger, PK11_InitPin returns failure, and error code is SEC_ERROR_BAD_PASSWORD.

Now list the database:

certutil -d sql:/tmp/sql -L
certutil: function failed: SEC_ERROR_BAD_DATABASE: security library: bad database.

The same series of actions works when using the DBM file format, e.g.
when using -d dbm:/tmp/dbm
Blocks: 1377940
Looks like sdb_Reset (eventually called from PK11_ResetToken) drops the key/certificate tables from the DBs:
It doesn't seem like there's a code path from PK11_InitPin where they would be re-created again. A simple fix seems to be changing the "DROP TABLE IF EXISTS" to a "DELETE FROM" (although since the original code seems to imply that the tables might not exist, we should probably also add an equivalent check ("IF EXISTS" doesn't seem to be compatible with the DELETE command)).
Another issue is the SFTKSlot for that token needs its value of needLogin updated sometime in PK11_ResetToken. From what I can tell, this value is true if there is a non-empty password set or if there is no password set. Since PK11_ResetToken resets the key DB, there is no password set, so needLogin needs to be true.
This solves the issue that 
1) The backing DB tables would be dropped and never recreated, preventing future operations from working.
2) The needLogin property of the SFTKSlot would not be updated properly, preventing PK11_InitPin (and thus other operations) from succeeding.

Does this solve all issues in this bug?
Flags: needinfo?(kaie)
Flags: needinfo?(dkeeler)
I think so. Kai can confirm when he gets back from vacation.
Flags: needinfo?(dkeeler)
With the commit from comment 3 the original test now works correctly.

Great work, David, thank you!
Assignee: rrelyea → dkeeler
Closed: 5 years ago
Flags: needinfo?(kaie)
Resolution: --- → FIXED
Target Milestone: --- → 3.33
You need to log in before you can comment on or make changes to this bug.