299995 - Unable to decrypt hardware token pins stored in the svrcore pin cache

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect, P3)

Description

[Rob Crittenden]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040805 Netscape/7.2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040805 Netscape/7.2

The svrcore library has the ability to securely store pin passwords. This works
for the internal database but not on a hardware token device. The pin is stored
successfully but when I try to retrieve it the decryption fails with
SEC_ERROR_INVALID_KEY (-8152).

The fix is to use PK11_TokenKeyGenWithFlags() instead of PK11_KeyGen() so one
can pass CKF_ENCRYPT|CKF_DECRYPT as the keygen flags.

Reproducible: Always

Comment 1

[Wan-Teh Chang]

PK11_TokenKeyGenWithFlags is a new function that isn't in any
NSS release yet (see bug 298957).

Perhaps we can fix this bug by having the existing PK11_TokenKeyGen
function set both the CKA_ENCRYPT and CKA_DECRYPT attributes?

Bob, why does PK11_TokenKeyGen only set the CKA_ENCRYPT attribute?

Comment 2

[Robert Relyea]

Because that is the typical useage....


GenerateKey()
Encrypt()
WrapKey()


UnwrapKey()
Decrypt()

The problem could also be fixed by wrapping the key out and back in.

bob

Comment 3

[Rich Megginson]

Rob, is this a bug in the svrcore lib that the ldap c sdk and the ldap server use?  If so, I'm going to move this bug from NSS to LDAP C SDK.

Comment 4

[Rob Crittenden]

Yes. The bug should be moved.

Comment 5

[Rich Megginson]

Bob R., can you move this bug to Directory -> LDAP C SDK and assign it to me?  Thanks.

Comment 6

[Robert Relyea]

done!

Comment 7

[Rich Megginson]

Here is the old code:
    store->key = PK11_KeyGen(store->slot, store->mech->type,
                   0, 0, 0);

Here is the new code:
    store->key = PK11_TokenKeyGenWithFlags(store->slot, store->mech->type,
                 0, 0, 0, CKF_ENCRYPT|CKF_DECRYPT,
                 0, 0);

Basically, every parameter is NULL except for the keygen flags.  Does this look ok?

Comment 8

[Rob Crittenden]

Not to be pedantic but I'd use:

    store->key = PK11_TokenKeyGenWithFlags(store->slot, store->mech->type,
                 NULL, 0, NULL, CKF_ENCRYPT|CKF_DECRYPT, PR_FALSE, NULL);

Comment 9

[Rich Megginson]

(In reply to comment #8)
> Not to be pedantic but I'd use:
> 
>     store->key = PK11_TokenKeyGenWithFlags(store->slot, store->mech->type,
>                  NULL, 0, NULL, CKF_ENCRYPT|CKF_DECRYPT, PR_FALSE, NULL);
> 

Thanks, but that function signature is 
PK11SymKey *
PK11_TokenKeyGenWithFlags(PK11SlotInfo *slot, CK_MECHANISM_TYPE type,
    SECItem *param, int keySize, SECItem *keyid, CK_FLAGS opFlags,
    PK11AttrFlags attrFlags, void *wincx)

I think just a plain 0 is a more appropriate argument for attrFlags instead of PR_FALSE.

Comment 10

[Rob Crittenden]

Nothing worse than being anal and wrong. 0 is more appropriate.

Comment 11

[Rich Megginson]

Committed.  This will be included in SVRCORE_4_0_3_RTM.