Apply the fix for NSS bug 1021102 from the NSS upstream.

nss/lib/freebl/rsa.c

 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * RSA key generation, public key op, private key op.
  */
 #ifdef FREEBL_NO_DEPEND
 #include "stubs.h"
 #endif
@@ skipping 909 matching lines @@
     CHECK_MPI_OK( mp_init(&d_q)  );
     CHECK_MPI_OK( mp_init(&qInv) );
     CHECK_MPI_OK( mp_init(&m1)   );
     CHECK_MPI_OK( mp_init(&m2)   );
     CHECK_MPI_OK( mp_init(&h)    );
     CHECK_MPI_OK( mp_init(&ctmp) );
     /* copy private key parameters into mp integers */
     SECITEM_TO_MPINT(key->prime1,      &p);    /* p */
     SECITEM_TO_MPINT(key->prime2,      &q);    /* q */
     SECITEM_TO_MPINT(key->exponent1,   &d_p);  /* d_p  = d mod (p-1) */
     SECITEM_TO_MPINT(key->exponent2,   &d_q);  /* d_q  = d mod (q-1) */

[wtc, 2014/06/11 19:35:24]

Another option is to do the p > q check here and swap p with q and d_p with d_q
if necessary.

     SECITEM_TO_MPINT(key->coefficient, &qInv); /* qInv = q**-1 mod p */
     /* 1. m1 = c**d_p mod p */
     CHECK_MPI_OK( mp_mod(c, &p, &ctmp) );
     CHECK_MPI_OK( mp_exptmod(&ctmp, &d_p, &p, &m1) );
     /* 2. m2 = c**d_q mod q */
     CHECK_MPI_OK( mp_mod(c, &q, &ctmp) );
     CHECK_MPI_OK( mp_exptmod(&ctmp, &d_q, &q, &m2) );
     /* 3.  h = (m1 - m2) * qInv mod p */
     CHECK_MPI_OK( mp_submod(&m1, &m2, &p, &h) );
     CHECK_MPI_OK( mp_mulmod(&h, &qInv, &p, &h)  );
@@ skipping 405 matching lines @@
 }
 
 SECStatus 
 RSA_PrivateKeyOpDoubleChecked(RSAPrivateKey *key, 
                               unsigned char *output, 
                               const unsigned char *input)
 {
     return rsa_PrivateKeyOp(key, output, input, PR_TRUE);
 }
 
-static SECStatus
-swap_in_key_value(PLArenaPool *arena, mp_int *mpval, SECItem *buffer)
-{
-    int len;
-    mp_err err = MP_OKAY;
-    memset(buffer->data, 0, buffer->len);
-    len = mp_unsigned_octet_size(mpval);
-    if (len <= 0) return SECFailure;
-    if ((unsigned int)len <= buffer->len) {
-        /* The new value is no longer than the old buffer, so use it */
-        err = mp_to_unsigned_octets(mpval, buffer->data, len);
-        if (err >= 0) err = MP_OKAY;
-        buffer->len = len;
-    } else if (arena) {
-        /* The new value is longer, but working within an arena */
-        (void)SECITEM_AllocItem(arena, buffer, len);
-        err = mp_to_unsigned_octets(mpval, buffer->data, len);
-        if (err >= 0) err = MP_OKAY;
-    } else {
-        /* The new value is longer, no arena, can't handle this key */
-        return SECFailure;
-    }
-    return (err == MP_OKAY) ? SECSuccess : SECFailure;
-}
-
 SECStatus
-RSA_PrivateKeyCheck(RSAPrivateKey *key)
+RSA_PrivateKeyCheck(const RSAPrivateKey *key)
 {
     mp_int p, q, n, psub1, qsub1, e, d, d_p, d_q, qInv, res;
     mp_err   err = MP_OKAY;
     SECStatus rv = SECSuccess;
     MP_DIGITS(&p)    = 0;
     MP_DIGITS(&q)    = 0;
     MP_DIGITS(&n)    = 0;
     MP_DIGITS(&psub1)= 0;
     MP_DIGITS(&qsub1)= 0;
     MP_DIGITS(&e)    = 0;
@@ skipping 25 matching lines @@
     }
 
     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  */
+    /* The qInv check depends on p > q. */
     if (mp_cmp(&p, &q) <= 0) {
         /* mind the p's and q's (and d_p's and d_q's) */
-        SECItem tmp;
         mp_exch(&p, &q);
         mp_exch(&d_p,&d_q);

[Ryan Sleevi, 2014/06/11 19:11:16]

This now means we're allowing invalid qInvs. I suspect this should really be
rv = SECFailure;
goto cleanup;

Otherwise, we'll allow qInv = p^-1 mod q

[wtc, 2014/06/11 19:35:24]

I see. We can also just delete this p > q check (lines 1403 - 1408) and rely on
the qInv check on line 1449-1451.

[Ryan Sleevi, 2014/06/11 19:41:13]

We could, but it seems like that'd be more computationally expensive for an
error condition we can short-circuit on here.

-        tmp = key->prime1;
-        key->prime1 = key->prime2;
-        key->prime2 = tmp;
-        tmp = key->exponent1;
-        key->exponent1 = key->exponent2;
-        key->exponent2 = tmp;
     }
 #define VERIFY_MPI_EQUAL(m1, m2) \
     if (mp_cmp(m1, m2) != 0) {   \
         rv = SECFailure;         \
         goto cleanup;            \
     }
 #define VERIFY_MPI_EQUAL_1(m)    \
     if (mp_cmp_d(m, 1) != 0) {   \
         rv = SECFailure;         \
         goto cleanup;            \
@@ skipping 122 matching lines @@
 PRBool bl_parentForkedAfterC_Initialize;
 
 /*
  * Set fork flag so it can be tested in SKIP_AFTER_FORK on relevant platforms.
  */
 void BL_SetForkState(PRBool forked)
 {
     bl_parentForkedAfterC_Initialize = forked;
 }