301528 - RSA certificate request succeeds even when underlying pkcs11 module returns error

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Martin Leung]

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

When making RSA certificate request using hardware pkcs11 token, the request
successes even though the token reported failure.


Reproducible: Always

Actual Results:  
Certificate request successful

Expected Results:  
Firefox reports keygen failure


  in seamonkey/ security/ nss/ cmd/ certcgi/ certcgi.c 
   privkey should be checked for null after keygen

510             slot = PK11_GetBestSlot(CKM_DSA_KEY_PAIR_GEN, NULL);
511             privkey = PK11_GenerateKeyPair(slot, CKM_DSA_KEY_PAIR_GEN, 
512                                            pqgParams,&pubkey, PR_FALSE, 
513                                            PR_TRUE, NULL);
514         } else {
515             privkey = SECKEY_CreateRSAPrivateKey(keySizeInBits, &pubkey, NULL);
516         }
517         privkeys[which_priv_key] = privkey;
518         spki = SECKEY_CreateSubjectPublicKeyInfo(pubkey);
519     } else {
520         spki = SECKEY_ConvertAndDecodePublicKeyAndChallenge(key, challenge, 
521                                                             NULL);
522         if (spki == NULL) {
523             error_out("ERROR: Unable to decode Public Key and Challenge
String");
524         }
525     }


  in seamonkey/ security/ nss/ lib/ cryptohi/ seckey.c 

   param pubk should be checked for null before use

1590 SECKEY_CreateSubjectPublicKeyInfo(SECKEYPublicKey *pubk)
1591 {
1592     CERTSubjectPublicKeyInfo *spki;
1593     PRArenaPool *arena;
1594     SECItem params = { siBuffer, NULL, 0 };
...
1608         switch(pubk->keyType) {

Comment 1

[Julien Pierre]

The bug in certcgi is real, but certcgi is not being actively maintained.
The bug is seckey is worth fixing, though it is worth noting that most NSS functions don't check for argument validity.

Comment 2

[Julien Pierre]

Attachment: Check input argument

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 266871 [details] [diff] [review]
Check input argument

r=nelson

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Martin, when you experienced this bug, was your PKCS#11 token being used
by the browser?  Or was it being used by a CGI program that was run by a 
web server?  

Based on the information in comment 0, I would say that one of two things
happened.  They are:

1) you tried to generate some certs with a web server that was running the 
certcgi CGI program.  There was a PKCS#11 module in use by the CGI program.
When the RSA Key generation (done by the CGI program) failed, the CGI 
program crashed (which would not appear to be a success).  

2. You tried to generate a certificate request with FireFox, with a PKCS#11
token being used by FireFox.  The KeyGen failed in the token being used by
FireFox, but the transaction appeared to complete successfully.  You tried
to find the responsible code, and found code in certcgi that you thought was
part of FireFox, but isn't.

I suspect the latter, because if the certcgi program crashed, it would not
appear to be a success.  but in comment 0, you say it appeared to succeed.

So, I think we need to find out where (in which program) the failure really
occurred (browser, or certcgi), and then plan a resolution based on that
information.

Comment 5

[Robert Relyea]

Comment on attachment 266871 [details] [diff] [review]
Check input argument

r+= rrelyea

Comment 6

[Julien Pierre]

Nelson, Bob, thanks for the reviews.

I checked this in to NSS_3_11_BRANCH :
Checking in seckey.c;
/cvsroot/mozilla/security/nss/lib/cryptohi/seckey.c,v  <--  seckey.c
new revision: 1.36.2.8; previous revision: 1.36.2.7
done

And to the trunk :
Checking in seckey.c;
/cvsroot/mozilla/security/nss/lib/cryptohi/seckey.c,v  <--  seckey.c
new revision: 1.44; previous revision: 1.43
done