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

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

This was found by executing a Firefox PSM test, 
  (security/manager/ssl/tests/unit/test_pkcs11_token.js)
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

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Looks like sdb_Reset (eventually called from PK11_ResetToken) drops the key/certificate tables from the DBs: https://dxr.mozilla.org/nss/rev/f212be04f3d0265340bf5ae20ffbbccdda68b0aa/nss/lib/softoken/sdb.c#1603
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)).

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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.

Comment 3

[Franziskus Kiefer [:franziskus]]

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?

https://hg.mozilla.org/projects/nss/rev/102381007af090fba357b7b777a5a8dcb67a600f

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I think so. Kai can confirm when he gets back from vacation.

Comment 5

[Kai Engert (:KaiE:)]

With the commit from comment 3 the original test now works correctly.

Great work, David, thank you!