Commit a39aa186 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 00d96547

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
@@ -1414,6 +1414,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)) {
@@ -1441,7 +1443,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;
@@ -1454,6 +1457,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
@@ -4213,3 +4213,4 @@ CT_POLICY_EVAL_CTX_set_time 4173 1_1_0d EXIST::FUNCTION:CT 
X509_VERIFY_PARAM_set_inh_flags         4174	1_1_0d	EXIST::FUNCTION:

X509_VERIFY_PARAM_get_inh_flags         4175	1_1_0d	EXIST::FUNCTION:

X509_VERIFY_PARAM_get_time              4181	1_1_0d	EXIST::FUNCTION:

DH_check_params                         4183	1_1_0d	EXIST::FUNCTION:DH