302219 - Enable NSS to use tokens that support X9.31 RSA key pair generation.

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Wan-Teh Chang]

This enhancement request is related to but different
from bug 181570.

This enhancement request merely asks that NSS be able
to invoke the CKM_RSA_X9_31_KEY_PAIR_GEN mechanism on
tokens that support it.  The RSA public/private key
objects generated by this mechanism has the same key
type (CKK_RSA) as the RSA public/private key objects
generated by the CKM_RSA_PKCS_KEY_PAIR_GEN, and therefore
can used with PKCS #1 RSA mechanisms.

(Note: in contrast, the Diffie Hellman public/private
key objects in PKCS #11 have different key types for
PKCS #3 and ANSI X9.41.)

I compared Sec. 12.1.4 "PKCS #1 RSA key pair generation"
and Sec. 12.1.5 "X9.31 RSA key pair generation" in PKCS #11
v2.20 carefully.  The only two differences are:

1. CKM_RSA_PKCS_KEY_PAIR_GEN allows the CKA_PUBLIC_EXPONENT
attribute to be omitted from the template for the public key.

Since NSS's PK11_GenerateKeyPair function requires that the
"pe" member of the PK11RSAGenParams structure be nonzero and
always specifies the CKA_PUBLIC_EXPONENT attribute in
"rsaPubTemplate", this difference is irrelevant to NSS.

2. CKM_RSA_X9_31_KEY_PAIR_GEN is guaranteed to generate p and
q values that meet the strong primes requirement of X9.31.

This is an action performed inside the token and so is
transparent to NSS.

So I believe it is very simple to implement this enhancement.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch for PK11_GenerateKeyPair (checked in)

I believe we just need to add CKM_RSA_X9_31_KEY_PAIR_GEN
whenever we see CKM_RSA_PKCS_KEY_PAIR_GEN in PK11_GenerateKeyPair.

I also added code to test for a zero public exponent input
argument (rsaParams->pe == 0).	This code backs up the assertion
PORT_Assert(peCount != 0) a few lines below.

I don't know I need to add CKM_RSA_X9_31_KEY_PAIR_GEN to other
functions in NSS, in particular PK11_GetKeyType and
PK11_GetKeyGenWithSize in lib/pk11wrap/pk11mech.c.  Bob, could
you advise?

Comment 2

[Wan-Teh Chang]

Attachment: Proposed patch for pk11mech.c (checked in)

I believe this is how the functions in lib/pk11wrap/pk11mech.c
should be updated to support CKM_RSA_X9_31_KEY_PAIR_GEN.

I still don't know what to do with the PK11_GetSlotList function
in lib/pk11wrap/pk11slot.c.

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch for pk11slot.c

Comment 4

[Robert Relyea]

Comment on attachment 190583 [details] [diff] [review]
Proposed patch for PK11_GenerateKeyPair (checked in)

This one looks good. r=relyea

The second switch statement  kinda calls for merging with
PK11_GetPubKeyIndexKeyID().
I'm not sure how many versions of this are scattered throughtout the  wrapper
code... that that's a preexisting issue.

Comment 5

[Robert Relyea]

Comment on attachment 190599 [details] [diff] [review]
Proposed patch for pk11mech.c (checked in)

r+

Comment 6

[Robert Relyea]

Comment on attachment 190601 [details] [diff] [review]
Proposed patch for pk11slot.c

r-,

let's not link this with the default RSA lists. If we do, then if you issue a
GetBestSlot with CKM_RSA_X9_31_KEY_PAIR_GEN could potentially return slots that
can't do CKM_RSA_X9_31_KEY_PAIR_GEN. (slots need to support all the 'prereq'
mechanisms to be on these lists). PK11_GetBestSlot and PK11_GetBestSlotMultiple
can easily find slots that can do functions that are not specified in
PK11_GetSlotList().

bob

Comment 7

[Robert Relyea]

On reviewing the patches, and knowing where the request comes from, it seems to
me it might be desirable to define a 'psuedo'-Mechanism: CKM_RSA_KEY_PAIR_GEN
which tells PK11_GenerateKeyPair to use either CKM_RSA_PKCS_KEY_PAIR_GEN or
CKM_RSA_X9_31_KEY_PAIR_GEN since the resultant keys are identical. Applications
would provide CKM_RSA_PKCS_XXX or CKM_RSA_X9_31_XXX if they need the specific
semantics of these key generation algorithms, or CKM_RSA_XXX if the application
just needs an RSA Key.

bob

Comment 8

[Wan-Teh Chang]

Comment on attachment 190601 [details] [diff] [review]
Proposed patch for pk11slot.c

Bob: doesn't PK11_GetBestSlotMultiple verify that the slot on
the list can actually do the mechanisms?  Here is the code snippet
that I think does this check:  

lib/pk11wrap/pk11slot.c
2054		 for (i=0; i < mech_count; i++) {
2055		     if (!PK11_DoesMechanism(le->slot,type[i])) {
2056			 doExit = PR_TRUE;
2057			 break;
2058		     }
2059		 }

Comment 9

[Robert Relyea]

Hmm that's correct, I initially though it only verified subsequent, mechanisms.
I still think it's better to leave it off the list, though. The list is your
'default' or preferred devices. It's not a good idea to have a default or
preferred device which doesn't implement the specified mechanism. Note that if
you use the slot list only devices registered on the slot list will be searched
(we don't search the general list of slots unless we don't get a slotlist).

It's really mute for this algorithm, though. Usually you aren't looking for the
'best slot to generate RSA keys', you are generating the keys in the pre-chosen
target slot.

bob

Comment 10

[Wan-Teh Chang]

According to the document "Differences between ANSI X9.31 and RSA PKCS #1"
(http://www.corsec.com/copy/pdf/X931_PKCS1.pdf),

    All key components generated with compliance to ANSI X9.31
    with an odd e are also RSA PKCS #1 compliant.

So if our customer is to use CKM_RSA_X9_31_KEY_PAIR_GEN to generate an
RSA key pair for use with PKCS #1, he must specify an odd public exponent e.