From 10a70da729948bb573d27cef4459077c49f3eb46 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Tue, 19 May 2015 11:53:31 +0200 Subject: [PATCH] client: reject handshakes with DH parameters < 768 bits. Since the client has no way of communicating her supported parameter range to the server, connections to servers that choose weak DH will simply fail. Reviewed-by: Kurt Roeckx --- CHANGES | 3 ++- ssl/s3_clnt.c | 73 +++++++++++++++++++++++++++++++++++---------------- ssl/ssl.h | 2 ++ ssl/ssl_err.c | 4 ++- 4 files changed, 57 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index 47237036e7..6dabf879fe 100644 --- a/CHANGES +++ b/CHANGES @@ -4,7 +4,8 @@ Changes between 1.0.2a and 1.0.2b [xx XXX xxxx] - *) + *) Reject DH handshakes with parameters shorter than 768 bits. + [Kurt Roeckx and Emilia Kasper] Changes between 1.0.2 and 1.0.2a [19 Mar 2015] diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index eebd4231d2..c25e077288 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -2361,6 +2361,25 @@ int ssl3_get_server_done(SSL *s) return (ret); } +#ifndef OPENSSL_NO_DH +static DH *get_server_static_dh_key(SESS_CERT *scert) +{ + DH *dh_srvr = NULL; + EVP_PKEY *spkey = NULL; + int idx = scert->peer_cert_type; + + if (idx >= 0) + spkey = X509_get_pubkey(scert->peer_pkeys[idx].x509); + if (spkey) { + dh_srvr = EVP_PKEY_get1_DH(spkey); + EVP_PKEY_free(spkey); + } + if (dh_srvr == NULL) + SSLerr(SSL_F_GET_SERVER_STATIC_DH_KEY, ERR_R_INTERNAL_ERROR); + return dh_srvr; +} +#endif + int ssl3_send_client_key_exchange(SSL *s) { unsigned char *p; @@ -2603,25 +2622,14 @@ int ssl3_send_client_key_exchange(SSL *s) goto err; } - if (scert->peer_dh_tmp != NULL) + if (scert->peer_dh_tmp != NULL) { dh_srvr = scert->peer_dh_tmp; - else { - /* we get them from the cert */ - int idx = scert->peer_cert_type; - EVP_PKEY *spkey = NULL; - dh_srvr = NULL; - if (idx >= 0) - spkey = X509_get_pubkey(scert->peer_pkeys[idx].x509); - if (spkey) { - dh_srvr = EVP_PKEY_get1_DH(spkey); - EVP_PKEY_free(spkey); - } - if (dh_srvr == NULL) { - SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); + } else { + dh_srvr = get_server_static_dh_key(scert); + if (dh_srvr == NULL) goto err; - } } + if (s->s3->flags & TLS1_FLAGS_SKIP_CERT_VERIFY) { /* Use client certificate key */ EVP_PKEY *clkey = s->cert->key->privatekey; @@ -3464,25 +3472,44 @@ int ssl3_check_cert_and_algorithm(SSL *s) } #endif #ifndef OPENSSL_NO_DH - if ((alg_k & SSL_kEDH) && - !(has_bits(i, EVP_PK_DH | EVP_PKT_EXCH) || (dh != NULL))) { - SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_MISSING_DH_KEY); + if ((alg_k & SSL_kEDH) && dh == NULL) { + SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, ERR_R_INTERNAL_ERROR); goto f_err; - } else if ((alg_k & SSL_kDHr) && !SSL_USE_SIGALGS(s) && + } + if ((alg_k & SSL_kDHr) && !SSL_USE_SIGALGS(s) && !has_bits(i, EVP_PK_DH | EVP_PKS_RSA)) { SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_MISSING_DH_RSA_CERT); goto f_err; } # ifndef OPENSSL_NO_DSA - else if ((alg_k & SSL_kDHd) && !SSL_USE_SIGALGS(s) && - !has_bits(i, EVP_PK_DH | EVP_PKS_DSA)) { + if ((alg_k & SSL_kDHd) && !SSL_USE_SIGALGS(s) && + !has_bits(i, EVP_PK_DH | EVP_PKS_DSA)) { SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_MISSING_DH_DSA_CERT); goto f_err; } # endif -#endif + + if (alg_k & (SSL_kDHE | SSL_kDHr | SSL_kDHd)) { + int dh_size; + if (alg_k & SSL_kDHE) { + dh_size = BN_num_bits(dh->p); + } else { + DH *dh_srvr = get_server_static_dh_key(sc); + if (dh_srvr == NULL) + goto f_err; + dh_size = BN_num_bits(dh_srvr->p); + DH_free(dh_srvr); + } + + if ((!SSL_C_IS_EXPORT(s->s3->tmp.new_cipher) && dh_size < 768) + || (SSL_C_IS_EXPORT(s->s3->tmp.new_cipher) && dh_size < 512)) { + SSLerr(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM, SSL_R_DH_KEY_TOO_SMALL); + goto f_err; + } + } +#endif /* !OPENSSL_NO_DH */ if (SSL_C_IS_EXPORT(s->s3->tmp.new_cipher) && !has_bits(i, EVP_PKT_EXP)) { #ifndef OPENSSL_NO_RSA diff --git a/ssl/ssl.h b/ssl/ssl.h index 70fa00bb3b..8eb852a0b6 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -2641,6 +2641,7 @@ void ERR_load_SSL_strings(void); # define SSL_F_GET_CLIENT_MASTER_KEY 107 # define SSL_F_GET_SERVER_FINISHED 108 # define SSL_F_GET_SERVER_HELLO 109 +# define SSL_F_GET_SERVER_STATIC_DH_KEY 340 # define SSL_F_GET_SERVER_VERIFY 110 # define SSL_F_I2D_SSL_SESSION 111 # define SSL_F_READ_N 112 @@ -2906,6 +2907,7 @@ void ERR_load_SSL_strings(void); # define SSL_R_DATA_LENGTH_TOO_LONG 146 # define SSL_R_DECRYPTION_FAILED 147 # define SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC 281 +# define SSL_R_DH_KEY_TOO_SMALL 372 # define SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG 148 # define SSL_R_DIGEST_CHECK_FAILED 149 # define SSL_R_DTLS_MESSAGE_TOO_BIG 334 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 76c92ae77c..fc0fb8f4e0 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -1,6 +1,6 @@ /* ssl/ssl_err.c */ /* ==================================================================== - * Copyright (c) 1999-2014 The OpenSSL Project. All rights reserved. + * Copyright (c) 1999-2015 The OpenSSL Project. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -119,6 +119,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_GET_CLIENT_MASTER_KEY), "GET_CLIENT_MASTER_KEY"}, {ERR_FUNC(SSL_F_GET_SERVER_FINISHED), "GET_SERVER_FINISHED"}, {ERR_FUNC(SSL_F_GET_SERVER_HELLO), "GET_SERVER_HELLO"}, + {ERR_FUNC(SSL_F_GET_SERVER_STATIC_DH_KEY), "GET_SERVER_STATIC_DH_KEY"}, {ERR_FUNC(SSL_F_GET_SERVER_VERIFY), "GET_SERVER_VERIFY"}, {ERR_FUNC(SSL_F_I2D_SSL_SESSION), "i2d_SSL_SESSION"}, {ERR_FUNC(SSL_F_READ_N), "READ_N"}, @@ -459,6 +460,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = { {ERR_REASON(SSL_R_DECRYPTION_FAILED), "decryption failed"}, {ERR_REASON(SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC), "decryption failed or bad record mac"}, + {ERR_REASON(SSL_R_DH_KEY_TOO_SMALL), "dh key too small"}, {ERR_REASON(SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG), "dh public value length is wrong"}, {ERR_REASON(SSL_R_DIGEST_CHECK_FAILED), "digest check failed"}, -- 2.25.1