309701 - Softtoken C_CreateObject() should not require CKA_NETSCAPE_DB attribute to be present

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Andreas Sterbenz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

When a DSA and DH private key is created via C_CreateObject(), the NSS softtoken
requires that the CKA_NETSCAPE_DB attribute is specified (it should be set to
the public value of the key).

If CKA_NETSCAPE_DB is not specified, CKR_TEMPLATE_INCOMPLETE is returned. This
is not in accordance with the PKCS#11 spec.

NSS should work if the attribute is not specified, possibly by calculating the
value itself. For DSA and DH, this can be done from the private value and the
group parameters (y = g ^ x mod p).

This seems to be caused by a check in sftk_handlePrivateKeyObject
(http://lxr.mozilla.org/security/source/security/nss/lib/softoken/pkcs11.c#1254)
and apparently the same issue also exists for EC keys.


Reproducible: Always

Actual Results:  
CKR_TEMPLATE_INCOMPLETE.

Expected Results:  
CKR_OK and the key is created.

Comment 1

[Andreas Sterbenz]

The same issue also exists for CKK_EC private keys. This is a major problem for Java and we would like to have this resolved at the same time that the ECC updates are integrated.

If a complete fix is not practical in that timeframe, a fix for session keys (CKA_TOKEN == false) would be sufficient for now.

As far as I understand, the CKA_NETSCAPE_DB attribute is only needed for token objects anyway. For session objects, the sftk_handlePrivateKeyObject() checks for the presence of the attribute, but it seems that it is never actually used later on. If that is the indeed case, the fix could simply be to make the check for CKA_NETSCAPE_DB conditional on the object being a token object.

Comment 2

[Wan-Teh Chang]

Indicated Andreas' requested fix timeframe in the target milestone.

Comment 3

[Julien Pierre]

Andreas,

Have you verified that this is broken for both session keys and token keys ? Bob thinks this is only broken for token keys right now. If so, we can later the full fix for 3.12 

Comment 4

[Andreas Sterbenz]

(In reply to comment #3)
> 
> Have you verified that this is broken for both session keys and token keys ?
> Bob thinks this is only broken for token keys right now. If so, we can later
> the full fix for 3.12 

NSS requires this attribute to be present for all private keys (session and token). The check that enforces that is in sftk_handlePrivateKeyObject (http://lxr.mozilla.org/security/source/security/nss/lib/softoken/pkcs11.c#1307). In sftk_mkPrivKey (http://lxr.mozilla.org/security/source/security/nss/lib/softoken/pkcs11.c#2088), the value of CKA_NETSCAPE_DB is then copied into &privKey->u.ec.publicValue. Similarly for DSA and DH.

I have not had a chance to investigate all the places where ec.publicValue of an ECPrivateKey is referenced and whether those code paths use session objects.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Given comment 4, I think this bug is P1, at least until it is no longer an
issue for session keys.

Comment 6

[Robert Relyea]

Attachment: Allow ephemeral keys to be created without the NETSCAPE_DB attribute.

I've determined that the only potential issue is wrapping of non-token private keys.
This patch created no regressions in all.sh
This patch does not address unwrapping of private keys, which will still fail without the NETSCAPE_DB attribute.

Comment 7

[Robert Relyea]

Comment on attachment 215790 [details] [diff] [review]
Allow ephemeral keys to be created without the NETSCAPE_DB attribute.

I did it again... new -u patch coming shortly..

Comment 8

[Robert Relyea]

Attachment: Allow ephemeral keys to be created without the NETSCAPE_DB attribute.

OK, this one should be reviewable.

bob

Comment 9

[Robert Relyea]

There are 2 other changes I made in this patch, prompted by compilier warnings. One is a bug (class not initialized). The other was a cast mismatch. please review these changes as part of your review as well (they will get checked in with this patch).

bob

Comment 10

[Julien Pierre]

Comment on attachment 215791 [details] [diff] [review]
Allow ephemeral keys to be created without the NETSCAPE_DB attribute.

Bob,

I have questions about this patch. It seems we simply don't try to get the public value if CKA_NETSCAPE_DB is missing. Is softoken going to operate properly without it ?

Andreas said we could calculate it for some cases. I guess we are putting that work off until later ?

Also, Andreas, did you test this patch ?

Comment 11

[Alexei Volkov]

Comment on attachment 215791 [details] [diff] [review]
Allow ephemeral keys to be created without the NETSCAPE_DB attribute.

Looks ok.

Comment 12

[Robert Relyea]

Julien,

Yes, I've basically verified that we only use it for token keys (there are not publicKeyFromPrivateKey() calls within softoken). It's the database key we use to store the keys under (as well as part of the data we store in the key database for the private keys).

The hope is when I finally get back to the multiaccess database we can elliminate that database key value (and key off the CKA_ID), thus removing the need for the parameter.

bob

Comment 13

[Andreas Sterbenz]

(In reply to comment #10)
> 
> Also, Andreas, did you test this patch ?

Works fine for me. Tested on a build of NSS_3_11_BRANCH plus this patch.

Comment 14

[Robert Relyea]

checked in on tip (pkcs11.c rev 1.20) and
branch:
jordan.sfbay.redhat.com(85) cvs commit pkcs11.c
Checking in pkcs11.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.112.2.5; previous revision: 1.112.2.4
done