Bug 1049435: Change RSA_PrivateKeyCheck to not require p > q. NSS_3_17_2_BETA2
authorWan-Teh Chang <wtc@google.com>
Tue, 07 Oct 2014 11:24:50 -0700
changeset 11276 358730f7261da6aa7b1ee19aafb22c0fc40506f8
parent 11275 d1d1fbbc1fa14c6c92968db3b1feb7da3eb6e958
child 11277 5ac45edb51594f5a7b63e9ada5423a335d1f80fc
push id493
push userwtc@google.com
push dateTue, 07 Oct 2014 18:25:00 +0000
bugs1049435
Bug 1049435: Change RSA_PrivateKeyCheck to not require p > q. Change RSA_PrivateKeyCheck and rsa_build_from_primes (called by RSA_NewKey and RSA_PopulatePrivateKey) to require p != q. Continue to allow RSA_NewKey and RSA_PopulatePrivateKey to force p > q, but add a comment to note that it is not necessary. Remove unused variable prevbp in get_blinding_params. r=rlb,rrelyea.
lib/freebl/rsa.c
--- a/lib/freebl/rsa.c
+++ b/lib/freebl/rsa.c
@@ -92,18 +92,18 @@ static struct RSABlindingParamsListStr b
 /* Number of times to reuse (f, g).  Suggested by Paul Kocher */
 #define RSA_BLINDING_PARAMS_MAX_REUSE 50
 
 /* Global, allows optional use of blinding.  On by default. */
 /* Cannot be changed at the moment, due to thread-safety issues. */
 static PRBool nssRSAUseBlinding = PR_TRUE;
 
 static SECStatus
-rsa_build_from_primes(mp_int *p, mp_int *q, 
-		mp_int *e, PRBool needPublicExponent, 
+rsa_build_from_primes(const mp_int *p, const mp_int *q,
+		mp_int *e, PRBool needPublicExponent,
 		mp_int *d, PRBool needPrivateExponent,
 		RSAPrivateKey *key, unsigned int keySizeInBits)
 {
     mp_int n, phi;
     mp_int psub1, qsub1, tmp;
     mp_err   err = MP_OKAY;
     SECStatus rv = SECSuccess;
     MP_DIGITS(&n)     = 0;
@@ -111,16 +111,22 @@ rsa_build_from_primes(mp_int *p, mp_int 
     MP_DIGITS(&psub1) = 0;
     MP_DIGITS(&qsub1) = 0;
     MP_DIGITS(&tmp)   = 0;
     CHECK_MPI_OK( mp_init(&n)     );
     CHECK_MPI_OK( mp_init(&phi)   );
     CHECK_MPI_OK( mp_init(&psub1) );
     CHECK_MPI_OK( mp_init(&qsub1) );
     CHECK_MPI_OK( mp_init(&tmp)   );
+    /* p and q must be distinct. */
+    if (mp_cmp(p, q) == 0) {
+	PORT_SetError(SEC_ERROR_NEED_RANDOM);
+	rv = SECFailure;
+	goto cleanup;
+    }
     /* 1.  Compute n = p*q */
     CHECK_MPI_OK( mp_mul(p, q, &n) );
     /*     verify that the modulus has the desired number of bits */
     if ((unsigned)mpl_significant_bits(&n) != keySizeInBits) {
 	PORT_SetError(SEC_ERROR_NEED_RANDOM);
 	rv = SECFailure;
 	goto cleanup;
     }
@@ -275,17 +281,21 @@ RSA_NewKey(int keySizeInBits, SECItem *p
     /* 3.  Set the public exponent */
     SECITEM_TO_MPINT(*publicExponent, &e);
     kiter = 0;
     do {
 	prerr = 0;
 	PORT_SetError(0);
 	CHECK_SEC_OK( generate_prime(&p, primeLen) );
 	CHECK_SEC_OK( generate_prime(&q, primeLen) );
-	/* Assure q < p */
+	/* Assure p > q */
+	/* NOTE: PKCS #1 does not require p > q, and NSS doesn't use any
+	 * implementation optimization that requires p > q. We can remove
+	 * this code in the future.
+	 */
 	if (mp_cmp(&p, &q) < 0)
 	    mp_exch(&p, &q);
 	/* Attempt to use these primes to generate a key */
 	rv = rsa_build_from_primes(&p, &q, 
 			&e, PR_FALSE,  /* needPublicExponent=false */
 			&d, PR_TRUE,   /* needPrivateExponent=true */
 			key, keySizeInBits);
 	if (rv == SECSuccess)
@@ -757,17 +767,21 @@ RSA_PopulatePrivateKey(RSAPrivateKey *ke
 			&n,hasModulus,keySizeInBits));
 	} else {
 	    /* not enough given parameters to get both primes */
 	    err = MP_BADARG;
 	    goto cleanup;
 	}
      }
 
-     /* force p to the the larger prime */
+     /* Assure p > q */
+     /* NOTE: PKCS #1 does not require p > q, and NSS doesn't use any
+      * implementation optimization that requires p > q. We can remove
+      * this code in the future.
+      */
      if (mp_cmp(&p, &q) < 0)
 	mp_exch(&p, &q);
 
      /* we now have our 2 primes and at least one exponent, we can fill
       * in the key */
      rv = rsa_build_from_primes(&p, &q, 
 			&e, needPublicExponent,
 			&d, needPrivateExponent,
@@ -1088,17 +1102,17 @@ init_blinding_params(RSABlindingParams *
 }
 
 static SECStatus
 get_blinding_params(RSAPrivateKey *key, mp_int *n, unsigned int modLen,
                     mp_int *f, mp_int *g)
 {
     RSABlindingParams *rsabp           = NULL;
     blindingParams    *bpUnlinked      = NULL;
-    blindingParams    *bp, *prevbp     = NULL;
+    blindingParams    *bp;
     PRCList           *el;
     SECStatus          rv              = SECSuccess;
     mp_err             err             = MP_OKAY;
     int                cmp             = -1;
     PRBool             holdingLock     = PR_FALSE;
 
     do {
 	if (blindingParamsList.lock == NULL) {
@@ -1178,17 +1192,16 @@ get_blinding_params(RSAPrivateKey *key, 
 		PR_NotifyCondVar( blindingParamsList.cVar );
 		blindingParamsList.waitCount--;
 	    }
 	    PZ_Unlock(blindingParamsList.lock); 
 	    return SECSuccess;
 	}
 	/* We did not find a usable set of blinding params.  Can we make one? */
 	/* Find a free bp struct. */
-	prevbp = NULL;
 	if ((bp = rsabp->free) != NULL) {
 	    /* unlink this bp */
 	    rsabp->free  = bp->next;
 	    bp->next     = NULL;
 	    bpUnlinked   = bp;  /* In case we fail */
 
 	    PZ_Unlock(blindingParamsList.lock); 
 	    holdingLock = PR_FALSE;
@@ -1395,18 +1408,18 @@ RSA_PrivateKeyCheck(const RSAPrivateKey 
     SECITEM_TO_MPINT(key->modulus,         &n);
     SECITEM_TO_MPINT(key->prime1,          &p);
     SECITEM_TO_MPINT(key->prime2,          &q);
     SECITEM_TO_MPINT(key->publicExponent,  &e);
     SECITEM_TO_MPINT(key->privateExponent, &d);
     SECITEM_TO_MPINT(key->exponent1,       &d_p);
     SECITEM_TO_MPINT(key->exponent2,       &d_q);
     SECITEM_TO_MPINT(key->coefficient,     &qInv);
-    /* p > q */
-    if (mp_cmp(&p, &q) <= 0) {
+    /* p and q must be distinct. */
+    if (mp_cmp(&p, &q) == 0) {
 	rv = SECFailure;
 	goto cleanup;
     }
 #define VERIFY_MPI_EQUAL(m1, m2) \
     if (mp_cmp(m1, m2) != 0) {   \
 	rv = SECFailure;         \
 	goto cleanup;            \
     }