Commit 26505153 authored by Richard Levitte's avatar Richard Levitte Committed by Matt Caswell
Browse files

Better check of DH parameters in TLS data 



When the client reads DH parameters from the TLS stream, we only
checked that they all are non-zero.  This change updates the check to
use DH_check_params()

DH_check_params() is a new function for light weight checking of the p
and g parameters:

    check that p is odd
    check that 1 < g < p - 1

Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
parent 2198b3a5

crypto/dh/dh_check.c

+40 −0
Original line number Diff line number Diff line
@@ -12,6 +12,46 @@ 
#include <openssl/bn.h>

#include "dh_locl.h"



/*-

 * Check that p and g are suitable enough

 *

 * p is odd

 * 1 < g < p - 1

 */



int DH_check_params(const DH *dh, int *ret)

{

    int ok = 0;

    BIGNUM *tmp = NULL;

    BN_CTX *ctx = NULL;



    *ret = 0;

    ctx = BN_CTX_new();

    if (ctx == NULL)

        goto err;

    BN_CTX_start(ctx);

    tmp = BN_CTX_get(ctx);

    if (tmp == NULL)

        goto err;



    if (!BN_is_odd(dh->p))

        *ret |= DH_CHECK_P_NOT_PRIME;

    if (BN_is_negative(dh->g) || BN_is_zero(dh->g) || BN_is_one(dh->g))

        *ret |= DH_NOT_SUITABLE_GENERATOR;

    if (BN_copy(tmp, dh->p) == NULL || !BN_sub_word(tmp, 1))

        goto err;

    if (BN_cmp(dh->g, tmp) >= 0)

        *ret |= DH_NOT_SUITABLE_GENERATOR;



    ok = 1;

 err:

    if (ctx != NULL) {

        BN_CTX_end(ctx);

        BN_CTX_free(ctx);

    }

    return (ok);

}



/*-

 * Check that p is a safe prime and

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

include/openssl/dh.h

+1 −0
Original line number Diff line number Diff line
@@ -124,6 +124,7 @@ DEPRECATEDIN_0_9_8(DH *DH_generate_parameters(int prime_len, int generator, 
int DH_generate_parameters_ex(DH *dh, int prime_len, int generator,

                              BN_GENCB *cb);



int DH_check_params(const DH *dh, int *ret);

int DH_check(const DH *dh, int *codes);

int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *codes);

int DH_generate_key(DH *dh);

ssl/statem/statem_clnt.c

+10 −1
Original line number Diff line number Diff line
@@ -1640,6 +1640,8 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) 
    DH *dh = NULL;

    BIGNUM *p = NULL, *g = NULL, *bnpub_key = NULL;



    int check_bits = 0;



    if (!PACKET_get_length_prefixed_2(pkt, &prime)

        || !PACKET_get_length_prefixed_2(pkt, &generator)

        || !PACKET_get_length_prefixed_2(pkt, &pub_key)) {
@@ -1669,7 +1671,8 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) 
        goto err;

    }



    if (BN_is_zero(p) || BN_is_zero(g) || BN_is_zero(bnpub_key)) {

    /* test non-zero pupkey */

    if (BN_is_zero(bnpub_key)) {

        *al = SSL_AD_DECODE_ERROR;

        SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE);

        goto err;
@@ -1682,6 +1685,12 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) 
    }

    p = g = NULL;



    if (DH_check_params(dh, &check_bits) == 0 || check_bits != 0) {

        *al = SSL_AD_DECODE_ERROR;

        SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE);

        goto err;

    }



    if (!DH_set0_key(dh, bnpub_key, NULL)) {

        *al = SSL_AD_INTERNAL_ERROR;

        SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, ERR_R_BN_LIB);

util/libcrypto.num

+1 −0
Original line number Diff line number Diff line
@@ -4229,3 +4229,4 @@ UI_method_get_ex_data 4179 1_1_1 EXIST::FUNCTION:UI 
UI_UTIL_wrap_read_pem_callback          4180	1_1_1	EXIST::FUNCTION:UI

X509_VERIFY_PARAM_get_time              4181	1_1_0d	EXIST::FUNCTION:

EVP_PKEY_get0_poly1305                  4182	1_1_1	EXIST::FUNCTION:POLY1305

DH_check_params                         4183	1_1_0d	EXIST::FUNCTION:DH