Commit ddd16c2f authored by Bernd Edlinger's avatar Bernd Edlinger
Browse files

Change DH parameters to generate the order q subgroup instead of 2q 



This avoids leaking bit 0 of the private key.

Backport-of: #9363

Reviewed-by: default avatarKurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/9435)
parent 8e747338

CHANGES

+6 −0
Original line number Diff line number Diff line
@@ -9,6 +9,12 @@ 














 Changes between 1.1.1c and 1.1.1d [xx XXX xxxx]



























  *) Changed DH parameters to generate the order q subgroup instead of 2q.













     Previously generated DH parameters are still accepted by DH_check













     but DH_generate_key works around that by clearing bit 0 of the













     private key for those. This avoids leaking bit 0 of the private key.













     [Bernd Edlinger]



























  *) Revert the DEVRANDOM_WAIT feature for Linux systems





























     The DEVRANDOM_WAIT feature added a select() call to wait for the

































crypto/dh/dh_check.c



+9
−25






























Original line number
Diff line number
Diff line








@@ -24,7 +24,8 @@ int DH_check_params_ex(const DH *dh)













{















    int errflags = 0;





























    (void)DH_check_params(dh, &errflags);













    if (!DH_check_params(dh, &errflags))













        return 0;































    if ((errflags & DH_CHECK_P_NOT_PRIME) != 0)















        DHerr(DH_F_DH_CHECK_PARAMS_EX, DH_R_CHECK_P_NOT_PRIME);










@@ -67,18 +68,14 @@ int DH_check_params(const DH *dh, int *ret)





























/*-















 * Check that p is a safe prime and













 * if g is 2, 3 or 5, check that it is a suitable generator













 * where













 * for 2, p mod 24 == 11













 * for 3, p mod 12 == 5













 * for 5, p mod 10 == 3 or 7













 * should hold.













 * g is a suitable generator.















 */















int DH_check_ex(const DH *dh)















{















    int errflags = 0;





























    (void)DH_check(dh, &errflags);













    if (!DH_check(dh, &errflags))













        return 0;































    if ((errflags & DH_NOT_SUITABLE_GENERATOR) != 0)















        DHerr(DH_F_DH_CHECK_EX, DH_R_NOT_SUITABLE_GENERATOR);









@@ -102,10 +99,11 @@ int DH_check(const DH *dh, int *ret)













{















    int ok = 0, r;















    BN_CTX *ctx = NULL;













    BN_ULONG l;















    BIGNUM *t1 = NULL, *t2 = NULL;





























    *ret = 0;













    if (!DH_check_params(dh, ret))













        return 0;





























    ctx = BN_CTX_new();















    if (ctx == NULL)















        goto err;










@@ -139,21 +137,7 @@ int DH_check(const DH *dh, int *ret)













            *ret |= DH_CHECK_INVALID_Q_VALUE;















        if (dh->j && BN_cmp(dh->j, t1))















            *ret |= DH_CHECK_INVALID_J_VALUE;



























    } else if (BN_is_word(dh->g, DH_GENERATOR_2)) {













        l = BN_mod_word(dh->p, 24);













        if (l == (BN_ULONG)-1)













            goto err;













        if (l != 11)













            *ret |= DH_NOT_SUITABLE_GENERATOR;













    } else if (BN_is_word(dh->g, DH_GENERATOR_5)) {













        l = BN_mod_word(dh->p, 10);













        if (l == (BN_ULONG)-1)













            goto err;













        if ((l != 3) && (l != 7))













            *ret |= DH_NOT_SUITABLE_GENERATOR;













    } else













        *ret |= DH_UNABLE_TO_CHECK_GENERATOR;













    }































    r = BN_is_prime_ex(dh->p, DH_NUMBER_ITERATIONS_FOR_PRIME, ctx, NULL);















    if (r < 0)

































crypto/dh/dh_gen.c



+24
−28






























Original line number
Diff line number
Diff line








@@ -30,30 +30,29 @@ int DH_generate_parameters_ex(DH *ret, int prime_len, int generator,





























/*-















 * We generate DH parameters as follows













 * find a prime q which is prime_len/2 bits long.













 * p=(2*q)+1 or (p-1)/2 = q













 * For this case, g is a generator if













 * g^((p-1)/q) mod p != 1 for values of q which are the factors of p-1.













 * Since the factors of p-1 are q and 2, we just need to check













 * g^2 mod p != 1 and g^q mod p != 1.













 * find a prime p which is prime_len bits long,













 * where q=(p-1)/2 is also prime.













 * In the following we assume that g is not 0, 1 or p-1, since it













 * would generate only trivial subgroups.













 * For this case, g is a generator of the order-q subgroup if













 * g^q mod p == 1.













 * Or in terms of the Legendre symbol: (g/p) == 1.















 *















 * Having said all that,















 * there is another special case method for the generators 2, 3 and 5.













 * for 2, p mod 24 == 11













 * for 3, p mod 12 == 5  <<<<< does not work for safe primes.













 * for 5, p mod 10 == 3 or 7













 * Using the quadratic reciprocity law it is possible to solve













 * (g/p) == 1 for the special values 2, 3, 5:













 * (2/p) == 1 if p mod 8 == 1 or 7.













 * (3/p) == 1 if p mod 12 == 1 or 11.













 * (5/p) == 1 if p mod 5 == 1 or 4.













 * See for instance: https://en.wikipedia.org/wiki/Legendre_symbol















 *













 * Thanks to Phil Karn for the pointers about the













 * special generators and for answering some of my questions.













 *













 * I've implemented the second simple method :-).













 * Since DH should be using a safe prime (both p and q are prime),













 * this generator function can take a very very long time to run.













 */













/*













 * Actually there is no reason to insist that 'generator' be a generator.













 * It's just as OK (and in some sense better) to use a generator of the













 * order-q subgroup.













 * Since all safe primes > 7 must satisfy p mod 12 == 11













 * and all safe primes > 11 must satisfy p mod 5 != 1













 * we can further improve the condition for g = 2, 3 and 5:













 * for 2, p mod 24 == 23













 * for 3, p mod 12 == 11













 * for 5, p mod 60 == 59















 */















static int dh_builtin_genparams(DH *ret, int prime_len, int generator,















                                BN_GENCB *cb)










@@ -84,17 +83,14 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator,













    if (generator == DH_GENERATOR_2) {















        if (!BN_set_word(t1, 24))















            goto err;













        if (!BN_set_word(t2, 11))













        if (!BN_set_word(t2, 23))















            goto err;















        g = 2;















    } else if (generator == DH_GENERATOR_5) {













        if (!BN_set_word(t1, 10))













        if (!BN_set_word(t1, 60))















            goto err;













        if (!BN_set_word(t2, 3))













        if (!BN_set_word(t2, 59))















            goto err;













        /*













         * BN_set_word(t3,7); just have to miss out on these ones :-(













         */















        g = 5;















    } else {















        /*









@@ -102,9 +98,9 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator,













         * not: since we are using safe primes, it will generate either an















         * order-q or an order-2q group, which both is OK















         */













        if (!BN_set_word(t1, 2))













        if (!BN_set_word(t1, 12))















            goto err;













        if (!BN_set_word(t2, 1))













        if (!BN_set_word(t2, 11))















            goto err;















        g = generator;















    }

































crypto/dh/dh_key.c



+11
−2






























Original line number
Diff line number
Diff line








@@ -125,6 +125,15 @@ static int generate_key(DH *dh)













            l = dh->length ? dh->length : BN_num_bits(dh->p) - 1;















            if (!BN_priv_rand(priv_key, l, BN_RAND_TOP_ONE, BN_RAND_BOTTOM_ANY))















                goto err;













            /*













             * We handle just one known case where g is a quadratic non-residue:













             * for g = 2: p % 8 == 3













             */













            if (BN_is_word(dh->g, DH_GENERATOR_2) && !BN_is_bit_set(dh->p, 2)) {













                /* clear bit 0, since it won't be a secret anyway */













                if (!BN_clear_bit(priv_key, 0))













                    goto err;













            }















        }















    }

























@@ -136,11 +145,11 @@ static int generate_key(DH *dh)













        BN_with_flags(prk, priv_key, BN_FLG_CONSTTIME);































        if (!dh->meth->bn_mod_exp(dh, pub_key, dh->g, prk, dh->p, ctx, mont)) {













            BN_free(prk);













            BN_clear_free(prk);















            goto err;















        }















        /* We MUST free prk before any further use of priv_key */













        BN_free(prk);













        BN_clear_free(prk);















    }































    dh->pub_key = pub_key;

































test/dhtest.c



+72
−1






























Original line number
Diff line number
Diff line








@@ -17,6 +17,7 @@













#include <openssl/bn.h>















#include <openssl/rand.h>















#include <openssl/err.h>













#include <openssl/obj_mac.h>















#include "testutil.h"































#ifndef OPENSSL_NO_DH










@@ -62,6 +63,17 @@ static int dh_test(void)













        || !TEST_true(DH_set0_pqg(dh, p, q, g)))















        goto err1;





























    if (!DH_check(dh, &i))













        goto err2;













    if (!TEST_false(i & DH_CHECK_P_NOT_PRIME)













            || !TEST_false(i & DH_CHECK_P_NOT_SAFE_PRIME)













            || !TEST_false(i & DH_CHECK_INVALID_Q_VALUE)













            || !TEST_false(i & DH_CHECK_Q_NOT_PRIME)













            || !TEST_false(i & DH_UNABLE_TO_CHECK_GENERATOR)













            || !TEST_false(i & DH_NOT_SUITABLE_GENERATOR)













            || !TEST_false(i))













        goto err2;





























    /* test the combined getter for p, q, and g */















    DH_get0_pqg(dh, &p2, &q2, &g2);















    if (!TEST_ptr_eq(p2, p)










@@ -130,7 +142,8 @@ static int dh_test(void)













    if (!TEST_false(i & DH_CHECK_P_NOT_PRIME)















            || !TEST_false(i & DH_CHECK_P_NOT_SAFE_PRIME)















            || !TEST_false(i & DH_UNABLE_TO_CHECK_GENERATOR)













            || !TEST_false(i & DH_NOT_SUITABLE_GENERATOR))













            || !TEST_false(i & DH_NOT_SUITABLE_GENERATOR)













            || !TEST_false(i))















        goto err3;































    DH_get0_pqg(a, &ap, NULL, &ag);










@@ -609,6 +622,63 @@ static int rfc5114_test(void)













    TEST_error("Test failed RFC5114 set %d\n", i + 1);















    return 0;















}



























static int rfc7919_test(void)













{













    DH *a = NULL, *b = NULL;













    const BIGNUM *apub_key = NULL, *bpub_key = NULL;













    unsigned char *abuf = NULL;













    unsigned char *bbuf = NULL;













    int i, alen, blen, aout, bout;













    int ret = 0;



























    if (!TEST_ptr(a = DH_new_by_nid(NID_ffdhe2048)))













         goto err;



























    if (!DH_check(a, &i))













        goto err;













    if (!TEST_false(i & DH_CHECK_P_NOT_PRIME)













            || !TEST_false(i & DH_CHECK_P_NOT_SAFE_PRIME)













            || !TEST_false(i & DH_UNABLE_TO_CHECK_GENERATOR)













            || !TEST_false(i & DH_NOT_SUITABLE_GENERATOR)













            || !TEST_false(i))













        goto err;



























    if (!DH_generate_key(a))













        goto err;













    DH_get0_key(a, &apub_key, NULL);



























    /* now create another copy of the DH group for the peer */













    if (!TEST_ptr(b = DH_new_by_nid(NID_ffdhe2048)))













        goto err;



























    if (!DH_generate_key(b))













        goto err;













    DH_get0_key(b, &bpub_key, NULL);



























    alen = DH_size(a);













    if (!TEST_ptr(abuf = OPENSSL_malloc(alen))













            || !TEST_true((aout = DH_compute_key(abuf, bpub_key, a)) != -1))













        goto err;



























    blen = DH_size(b);













    if (!TEST_ptr(bbuf = OPENSSL_malloc(blen))













            || !TEST_true((bout = DH_compute_key(bbuf, apub_key, b)) != -1))













        goto err;



























    if (!TEST_true(aout >= 20)













            || !TEST_mem_eq(abuf, aout, bbuf, bout))













        goto err;



























    ret = 1;



























 err:













    OPENSSL_free(abuf);













    OPENSSL_free(bbuf);













    DH_free(a);













    DH_free(b);













    return ret;













}















#endif









































@@ -619,6 +689,7 @@ int setup_tests(void)













#else















    ADD_TEST(dh_test);















    ADD_TEST(rfc5114_test);













    ADD_TEST(rfc7919_test);















#endif















    return 1;















}