Attachment 945050 Details for Bug 1150645 – Change RSA_PrivateKeyCheck to not require p > q

[patch] Change RSA_PrivateKeyCheck to not require p > q

RSA_PrivateKeyCheckFix.patch (text/plain), 5.08 KB, created by Elio Maldonado Batiz on 2014-10-08 15:05:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Elio Maldonado Batiz

Created: 2014-10-08 15:05:33 UTC

Size: 5.08 KB

patch

obsolete

>diff --git a/lib/freebl/rsa.c b/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;            \
>     }

Flags:

rrelyea: review+

Actions: View | Diff

Attachments on bug 1150645: 945050