Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1339464 - Fix DH_GenParam, r=franziskus
Summary: Fix DH_GenParam by repeating mpp_make_prime calls on failure

Differential Revision: https://nss-review.dev.mozaws.net/D308

--HG--
extra : amend_source : 16c562e5b148bbe3290fbf1b2137ead2b7054176
  • Loading branch information
mozmark committed May 10, 2017
1 parent 2c7d12e commit ffa9d06
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 22 deletions.
3 changes: 1 addition & 2 deletions fuzz/mpi_invmod_target.cc
Expand Up @@ -32,8 +32,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
bp[primeLen - 1] |= 0x01; /* set low-order bit */
++count;
assert(mp_read_unsigned_octets(&b, bp, primeLen) == MP_OKAY);
} while ((res = mpp_make_prime(&b, primeLen * 8, PR_FALSE, nullptr)) !=
MP_YES &&
} while ((res = mpp_make_prime(&b, primeLen * 8, PR_FALSE)) != MP_YES &&
count < 10);
if (res != MP_YES) {
return 0;
Expand Down
26 changes: 26 additions & 0 deletions gtests/freebl_gtest/dh_unittest.cc
@@ -0,0 +1,26 @@
// 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/.

#include "blapi.h"
#include "gtest/gtest.h"

namespace nss_test {

class DHTest : public ::testing::Test {
protected:
void TestGenParamSuccess(int size) {
DHParams *params;
for (int i = 0; i < 10; i++) {
EXPECT_EQ(SECSuccess, DH_GenParam(size, &params));
PORT_FreeArena(params->arena, PR_TRUE);
}
}
};

// Test parameter generation for minimum and some common key sizes
TEST_F(DHTest, DhGenParamSuccessTest16) { TestGenParamSuccess(16); }
TEST_F(DHTest, DhGenParamSuccessTest224) { TestGenParamSuccess(224); }
TEST_F(DHTest, DhGenParamSuccessTest256) { TestGenParamSuccess(256); }

} // nss_test
1 change: 1 addition & 0 deletions gtests/freebl_gtest/freebl_gtest.gyp
Expand Up @@ -12,6 +12,7 @@
'type': 'executable',
'sources': [
'mpi_unittest.cc',
'dh_unittest.cc',
'<(DEPTH)/gtests/common/gtests.cc'
],
'dependencies': [
Expand Down
2 changes: 2 additions & 0 deletions lib/freebl/blapii.h
Expand Up @@ -9,6 +9,7 @@
#define _BLAPII_H_

#include "blapit.h"
#include "mpi.h"

/* max block size of supported block ciphers */
#define MAX_BLOCK_SIZE 16
Expand Down Expand Up @@ -59,6 +60,7 @@ SEC_END_PROTOS
#undef HAVE_NO_SANITIZE_ATTR

SECStatus RSA_Init();
SECStatus generate_prime(mp_int *prime, int primeLen);

/* Freebl state. */
PRBool aesni_support();
Expand Down
13 changes: 2 additions & 11 deletions lib/freebl/dh.c
Expand Up @@ -14,9 +14,9 @@
#include "secerr.h"

#include "blapi.h"
#include "blapii.h"
#include "secitem.h"
#include "mpi.h"
#include "mpprime.h"
#include "secmpi.h"

#define KEA_DERIVED_SECRET_LEN 128
Expand Down Expand Up @@ -46,9 +46,7 @@ DH_GenParam(int primeLen, DHParams **params)
{
PLArenaPool *arena;
DHParams *dhparams;
unsigned char *pb = NULL;
unsigned char *ab = NULL;
unsigned long counter = 0;
mp_int p, q, a, h, psub1, test;
mp_err err = MP_OKAY;
SECStatus rv = SECSuccess;
Expand Down Expand Up @@ -81,12 +79,7 @@ DH_GenParam(int primeLen, DHParams **params)
CHECK_MPI_OK(mp_init(&psub1));
CHECK_MPI_OK(mp_init(&test));
/* generate prime with MPI, uses Miller-Rabin to generate strong prime. */
pb = PORT_Alloc(primeLen);
CHECK_SEC_OK(RNG_GenerateGlobalRandomBytes(pb, primeLen));
pb[0] |= 0x80; /* set high-order bit */
pb[primeLen - 1] |= 0x01; /* set low-order bit */
CHECK_MPI_OK(mp_read_unsigned_octets(&p, pb, primeLen));
CHECK_MPI_OK(mpp_make_prime(&p, primeLen * 8, PR_TRUE, &counter));
CHECK_SEC_OK(generate_prime(&p, primeLen));
/* construct Sophie-Germain prime q = (p-1)/2. */
CHECK_MPI_OK(mp_sub_d(&p, 1, &psub1));
CHECK_MPI_OK(mp_div_2(&psub1, &q));
Expand Down Expand Up @@ -121,8 +114,6 @@ DH_GenParam(int primeLen, DHParams **params)
mp_clear(&h);
mp_clear(&psub1);
mp_clear(&test);
if (pb)
PORT_ZFree(pb, primeLen);
if (ab)
PORT_ZFree(ab, primeLen);
if (err) {
Expand Down
5 changes: 1 addition & 4 deletions lib/freebl/mpi/mpprime.c
Expand Up @@ -402,8 +402,7 @@ mpp_sieve(mp_int *trial, const mp_digit *primes, mp_size nPrimes,
#define SIEVE_SIZE 32 * 1024

mp_err
mpp_make_prime(mp_int *start, mp_size nBits, mp_size strong,
unsigned long *nTries)
mpp_make_prime(mp_int *start, mp_size nBits, mp_size strong)
{
mp_digit np;
mp_err res;
Expand Down Expand Up @@ -548,8 +547,6 @@ mpp_make_prime(mp_int *start, mp_size nBits, mp_size strong,
CLEANUP:
mp_clear(&trial);
mp_clear(&q);
if (nTries)
*nTries += i;
if (sieve != NULL) {
memset(sieve, 0, SIEVE_SIZE);
free(sieve);
Expand Down
3 changes: 1 addition & 2 deletions lib/freebl/mpi/mpprime.h
Expand Up @@ -34,8 +34,7 @@ mp_err mpp_fermat_list(mp_int *a, const mp_digit *primes, mp_size nPrimes);
mp_err mpp_pprime(mp_int *a, int nt);
mp_err mpp_sieve(mp_int *trial, const mp_digit *primes, mp_size nPrimes,
unsigned char *sieve, mp_size nSieve);
mp_err mpp_make_prime(mp_int *start, mp_size nBits, mp_size strong,
unsigned long *nTries);
mp_err mpp_make_prime(mp_int *start, mp_size nBits, mp_size strong);

SEC_END_PROTOS

Expand Down
6 changes: 3 additions & 3 deletions lib/freebl/rsa.c
Expand Up @@ -190,12 +190,12 @@ rsa_build_from_primes(const mp_int *p, const mp_int *q,
}
return rv;
}
static SECStatus

SECStatus
generate_prime(mp_int *prime, int primeLen)
{
mp_err err = MP_OKAY;
SECStatus rv = SECSuccess;
unsigned long counter = 0;
int piter;
unsigned char *pb = NULL;
pb = PORT_Alloc(primeLen);
Expand All @@ -208,7 +208,7 @@ generate_prime(mp_int *prime, int primeLen)
pb[0] |= 0xC0; /* set two high-order bits */
pb[primeLen - 1] |= 0x01; /* set low-order bit */
CHECK_MPI_OK(mp_read_unsigned_octets(prime, pb, primeLen));
err = mpp_make_prime(prime, primeLen * 8, PR_FALSE, &counter);
err = mpp_make_prime(prime, primeLen * 8, PR_FALSE);
if (err != MP_NO)
goto cleanup;
/* keep going while err == MP_NO */
Expand Down

0 comments on commit ffa9d06

Please sign in to comment.