From e01a610db8dc7b86422fd08447de25756ddc21e9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 8 Jul 2016 15:08:50 +0100 Subject: [PATCH] Split out DHE from tls_process_key_exchange() Continuing from the previous commit. Refactor tls_process_key_exchange() to split out into a separate function the DHE aspects. Reviewed-by: Richard Levitte --- ssl/statem/statem_clnt.c | 185 +++++++++++++++++++++------------------ 1 file changed, 99 insertions(+), 86 deletions(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 48503d3a88..6d937a4c6b 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1393,6 +1393,102 @@ static int tls_process_ske_srp(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) #endif } +static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) +{ +#ifndef OPENSSL_NO_DH + PACKET prime, generator, pub_key; + EVP_PKEY *peer_tmp = NULL; + + DH *dh = NULL; + BIGNUM *p = NULL, *g = NULL, *bnpub_key = NULL; + + if (!PACKET_get_length_prefixed_2(pkt, &prime) + || !PACKET_get_length_prefixed_2(pkt, &generator) + || !PACKET_get_length_prefixed_2(pkt, &pub_key)) { + *al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_LENGTH_MISMATCH); + return 0; + } + + peer_tmp = EVP_PKEY_new(); + dh = DH_new(); + + if (peer_tmp == NULL || dh == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + goto err; + } + + p = BN_bin2bn(PACKET_data(&prime), PACKET_remaining(&prime), NULL); + g = BN_bin2bn(PACKET_data(&generator), PACKET_remaining(&generator), + NULL); + bnpub_key = BN_bin2bn(PACKET_data(&pub_key), PACKET_remaining(&pub_key), + NULL); + if (p == NULL || g == NULL || bnpub_key == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); + goto err; + } + + if (BN_is_zero(p) || BN_is_zero(g) || BN_is_zero(bnpub_key)) { + *al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_BAD_DH_VALUE); + goto err; + } + + if (!DH_set0_pqg(dh, p, NULL, g)) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); + goto err; + } + p = g = NULL; + + if (!DH_set0_key(dh, bnpub_key, NULL)) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); + goto err; + } + bnpub_key = NULL; + + if (!ssl_security(s, SSL_SECOP_TMP_DH, DH_security_bits(dh), 0, dh)) { + *al = SSL_AD_HANDSHAKE_FAILURE; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_DH_KEY_TOO_SMALL); + goto err; + } + + if (EVP_PKEY_assign_DH(peer_tmp, dh) == 0) { + *al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); + goto err; + } + + s->s3->peer_tmp = peer_tmp; + + /* + * FIXME: This makes assumptions about which ciphersuites come with + * public keys. We should have a less ad-hoc way of doing this + */ + if (s->s3->tmp.new_cipher->algorithm_auth & (SSL_aRSA|SSL_aDSS)) + *pkey = X509_get0_pubkey(s->session->peer); + /* else anonymous DH, so no certificate or pkey. */ + + return 1; + + err: + BN_free(p); + BN_free(g); + BN_free(bnpub_key); + DH_free(dh); + EVP_PKEY_free(peer_tmp); + + return 0; +#else + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + *al = SSL_AD_INTERNAL_ERROR; + return 0; +#endif +} + MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) { EVP_MD_CTX *md_ctx; @@ -1429,93 +1525,10 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) } else if (alg_k & SSL_kSRP) { if (!tls_process_ske_srp(s, pkt, &pkey, &al)) goto err; + } else if (alg_k & (SSL_kDHE | SSL_kDHEPSK)) { + if (!tls_process_ske_dhe(s, pkt, &pkey, &al)) + goto err; } -#ifndef OPENSSL_NO_DH - else if (alg_k & (SSL_kDHE | SSL_kDHEPSK)) { - PACKET prime, generator, pub_key; - EVP_PKEY *peer_tmp = NULL; - - DH *dh = NULL; - BIGNUM *p = NULL, *g = NULL, *bnpub_key = NULL; - - if (!PACKET_get_length_prefixed_2(pkt, &prime) - || !PACKET_get_length_prefixed_2(pkt, &generator) - || !PACKET_get_length_prefixed_2(pkt, &pub_key)) { - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_LENGTH_MISMATCH); - goto f_err; - } - - peer_tmp = EVP_PKEY_new(); - dh = DH_new(); - - if (peer_tmp == NULL || dh == NULL) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); - goto dherr; - } - - p = BN_bin2bn(PACKET_data(&prime), PACKET_remaining(&prime), NULL); - g = BN_bin2bn(PACKET_data(&generator), PACKET_remaining(&generator), - NULL); - bnpub_key = BN_bin2bn(PACKET_data(&pub_key), PACKET_remaining(&pub_key), - NULL); - if (p == NULL || g == NULL || bnpub_key == NULL) { - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); - goto dherr; - } - - if (BN_is_zero(p) || BN_is_zero(g) || BN_is_zero(bnpub_key)) { - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_BAD_DH_VALUE); - goto dherr; - } - - if (!DH_set0_pqg(dh, p, NULL, g)) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); - goto dherr; - } - p = g = NULL; - - if (!DH_set0_key(dh, bnpub_key, NULL)) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_BN_LIB); - goto dherr; - } - bnpub_key = NULL; - - if (!ssl_security(s, SSL_SECOP_TMP_DH, DH_security_bits(dh), 0, dh)) { - al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_DH_KEY_TOO_SMALL); - goto dherr; - } - - if (EVP_PKEY_assign_DH(peer_tmp, dh) == 0) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); - goto dherr; - } - - s->s3->peer_tmp = peer_tmp; - - goto dhend; - dherr: - BN_free(p); - BN_free(g); - BN_free(bnpub_key); - DH_free(dh); - EVP_PKEY_free(peer_tmp); - goto f_err; - dhend: - /* - * FIXME: This makes assumptions about which ciphersuites come with - * public keys. We should have a less ad-hoc way of doing this - */ - if (alg_a & (SSL_aRSA|SSL_aDSS)) - pkey = X509_get0_pubkey(s->session->peer); - /* else anonymous DH, so no certificate or pkey. */ - } -#endif /* !OPENSSL_NO_DH */ - #ifndef OPENSSL_NO_EC else if (alg_k & (SSL_kECDHE | SSL_kECDHEPSK)) { PACKET encoded_pt; -- 2.25.1