From 6f78b9e824c053d062188578635c575017b587c5 Mon Sep 17 00:00:00 2001 From: Kurt Roeckx Date: Fri, 4 Dec 2015 22:22:31 +0100 Subject: [PATCH] Remove support for SSL_{CTX_}set_tmp_ecdh_callback(). This only gets used to set a specific curve without actually checking that the peer supports it or not and can therefor result in handshake failures that can be avoided by selecting a different cipher. Reviewed-by: Dr. Stephen Henson --- CHANGES | 4 ++++ include/openssl/ssl.h | 9 --------- ssl/s3_lib.c | 24 ------------------------ ssl/ssl_cert.c | 1 - ssl/ssl_lib.c | 19 +------------------ ssl/ssl_locl.h | 2 -- ssl/statem/statem_srvr.c | 6 ------ ssl/t1_lib.c | 9 +++------ util/ssleay.num | 4 ++-- 9 files changed, 10 insertions(+), 68 deletions(-) diff --git a/CHANGES b/CHANGES index 55362fe00f..b365cb0ad1 100644 --- a/CHANGES +++ b/CHANGES @@ -13,6 +13,10 @@ pages. This work was developed in partnership with Intel Corp. [Matt Caswell] + *) Remove support for SSL_{CTX_}set_tmp_ecdh_callback(). You should set the + curve you want to support using SSL_{CTX_}set1_curves(). + [Kurt Roeckx] + *) State machine rewrite. The state machine code has been significantly refactored in order to remove much duplication of code and solve issues with the old code (see ssl/statem/README for further details). This change diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 2f3f514690..759f746b21 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1119,7 +1119,6 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) # define SSL_CTRL_SET_TMP_ECDH 4 # define SSL_CTRL_SET_TMP_RSA_CB 5 # define SSL_CTRL_SET_TMP_DH_CB 6 -# define SSL_CTRL_SET_TMP_ECDH_CB 7 # define SSL_CTRL_GET_SESSION_REUSED 8 # define SSL_CTRL_GET_CLIENT_CERT_REQUEST 9 # define SSL_CTRL_GET_NUM_RENEGOTIATIONS 10 @@ -1772,14 +1771,6 @@ void SSL_set_tmp_dh_callback(SSL *ssl, DH *(*dh) (SSL *ssl, int is_export, int keylength)); # endif -# ifndef OPENSSL_NO_EC -void SSL_CTX_set_tmp_ecdh_callback(SSL_CTX *ctx, - EC_KEY *(*ecdh) (SSL *ssl, int is_export, - int keylength)); -void SSL_set_tmp_ecdh_callback(SSL *ssl, - EC_KEY *(*ecdh) (SSL *ssl, int is_export, - int keylength)); -# endif __owur const COMP_METHOD *SSL_get_current_compression(SSL *s); __owur const COMP_METHOD *SSL_get_current_expansion(SSL *s); diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index bf7336cbfc..0df228e50a 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -4095,11 +4095,6 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) ret = 1; } break; - case SSL_CTRL_SET_TMP_ECDH_CB: - { - SSLerr(SSL_F_SSL3_CTRL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); - return (ret); - } #endif /* !OPENSSL_NO_EC */ case SSL_CTRL_SET_TLSEXT_HOSTNAME: if (larg == TLSEXT_NAMETYPE_host_name) { @@ -4422,13 +4417,6 @@ long ssl3_callback_ctrl(SSL *s, int cmd, void (*fp) (void)) s->cert->dh_tmp_cb = (DH *(*)(SSL *, int, int))fp; } break; -#endif -#ifndef OPENSSL_NO_EC - case SSL_CTRL_SET_TMP_ECDH_CB: - { - s->cert->ecdh_tmp_cb = (EC_KEY *(*)(SSL *, int, int))fp; - } - break; #endif case SSL_CTRL_SET_TLSEXT_DEBUG_CB: s->tlsext_debug_cb = (void (*)(SSL *, int, int, @@ -4558,11 +4546,6 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) return 1; } /* break; */ - case SSL_CTRL_SET_TMP_ECDH_CB: - { - SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); - return (0); - } #endif /* !OPENSSL_NO_EC */ case SSL_CTRL_SET_TLSEXT_SERVERNAME_ARG: ctx->tlsext_servername_arg = parg; @@ -4732,13 +4715,6 @@ long ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp) (void)) cert->dh_tmp_cb = (DH *(*)(SSL *, int, int))fp; } break; -#endif -#ifndef OPENSSL_NO_EC - case SSL_CTRL_SET_TMP_ECDH_CB: - { - cert->ecdh_tmp_cb = (EC_KEY *(*)(SSL *, int, int))fp; - } - break; #endif case SSL_CTRL_SET_TLSEXT_SERVERNAME_CB: ctx->tlsext_servername_callback = (int (*)(SSL *, int *, void *))fp; diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 6f9fcdb350..45b1d164ba 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -239,7 +239,6 @@ CERT *ssl_cert_dup(CERT *cert) goto err; } } - ret->ecdh_tmp_cb = cert->ecdh_tmp_cb; ret->ecdh_tmp_auto = cert->ecdh_tmp_auto; #endif diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index cac692dd38..9343e7dff2 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2037,7 +2037,7 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) #endif #ifndef OPENSSL_NO_EC - have_ecdh_tmp = (c->ecdh_tmp || c->ecdh_tmp_cb || c->ecdh_tmp_auto); + have_ecdh_tmp = (c->ecdh_tmp || c->ecdh_tmp_auto); #endif cpk = &(c->pkeys[SSL_PKEY_RSA_ENC]); rsa_enc = pvalid[SSL_PKEY_RSA_ENC] & CERT_PKEY_VALID; @@ -3142,23 +3142,6 @@ void SSL_set_tmp_dh_callback(SSL *ssl, DH *(*dh) (SSL *ssl, int is_export, } #endif -#ifndef OPENSSL_NO_EC -void SSL_CTX_set_tmp_ecdh_callback(SSL_CTX *ctx, - EC_KEY *(*ecdh) (SSL *ssl, int is_export, - int keylength)) -{ - SSL_CTX_callback_ctrl(ctx, SSL_CTRL_SET_TMP_ECDH_CB, - (void (*)(void))ecdh); -} - -void SSL_set_tmp_ecdh_callback(SSL *ssl, - EC_KEY *(*ecdh) (SSL *ssl, int is_export, - int keylength)) -{ - SSL_callback_ctrl(ssl, SSL_CTRL_SET_TMP_ECDH_CB, (void (*)(void))ecdh); -} -#endif - #ifndef OPENSSL_NO_PSK int SSL_CTX_use_psk_identity_hint(SSL_CTX *ctx, const char *identity_hint) { diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index b4c6244896..2da47f1d35 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1570,8 +1570,6 @@ typedef struct cert_st { # endif # ifndef OPENSSL_NO_EC EC_KEY *ecdh_tmp; - /* Callback for generating ephemeral ECDH keys */ - EC_KEY *(*ecdh_tmp_cb) (SSL *ssl, int is_export, int keysize); /* Select ECDH parameters automatically */ int ecdh_tmp_auto; # endif diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index bdeaf7e0e0..fb64106350 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1874,12 +1874,6 @@ int tls_construct_server_key_exchange(SSL *s) int nid = tls1_shared_curve(s, -2); if (nid != NID_undef) ecdhp = EC_KEY_new_by_curve_name(nid); - } else if ((ecdhp == NULL) && s->cert->ecdh_tmp_cb) { - ecdhp = s->cert->ecdh_tmp_cb(s, - SSL_C_IS_EXPORT(s->s3-> - tmp.new_cipher), - SSL_C_EXPORT_PKEYLENGTH(s-> - s3->tmp.new_cipher)); } if (ecdhp == NULL) { al = SSL_AD_HANDSHAKE_FAILURE; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 02ad438a6e..951be10d2d 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -868,8 +868,8 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid) /* Check this curve is acceptable */ if (!tls1_check_ec_key(s, curve_id, NULL)) return 0; - /* If auto or setting curve from callback assume OK */ - if (s->cert->ecdh_tmp_auto || s->cert->ecdh_tmp_cb) + /* If auto assume OK */ + if (s->cert->ecdh_tmp_auto) return 1; /* Otherwise check curve is acceptable */ else { @@ -892,10 +892,7 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid) return 0; } if (!ec) { - if (s->cert->ecdh_tmp_cb) - return 1; - else - return 0; + return 0; } if (!tls1_set_ec_id(curve_id, NULL, ec)) return 0; diff --git a/util/ssleay.num b/util/ssleay.num index f737aac440..1d23afb1c3 100755 --- a/util/ssleay.num +++ b/util/ssleay.num @@ -217,8 +217,8 @@ SSL_renegotiate_pending 265 EXIST::FUNCTION: SSL_CTX_set_msg_callback 266 EXIST::FUNCTION: SSL_set_msg_callback 267 EXIST::FUNCTION: DTLSv1_client_method 268 EXIST::FUNCTION: -SSL_CTX_set_tmp_ecdh_callback 269 EXIST::FUNCTION:EC -SSL_set_tmp_ecdh_callback 270 EXIST::FUNCTION:EC +SSL_CTX_set_tmp_ecdh_callback 269 NOEXIST::FUNCTION: +SSL_set_tmp_ecdh_callback 270 NOEXIST::FUNCTION: SSL_COMP_get_name 271 EXIST::FUNCTION: SSL_get_current_compression 272 EXIST::FUNCTION: DTLSv1_method 273 EXIST::FUNCTION: -- GitLab