From fe6ef2472db933f01b59cad82aa925736935984b Mon Sep 17 00:00:00 2001 From: Kurt Roeckx Date: Fri, 4 Dec 2015 22:30:36 +0100 Subject: [PATCH] Remove SSL_{CTX_}set_ecdh_auto() and always enable ECDH Reviewed-by: Dr. Stephen Henson --- CHANGES | 5 +++ doc/ssl/SSL_CTX_set1_curves.pod | 28 ++-------------- include/openssl/ssl.h | 5 --- ssl/s3_lib.c | 8 ----- ssl/ssl_cert.c | 4 --- ssl/ssl_ciph.c | 2 -- ssl/ssl_conf.c | 57 +++++++++------------------------ ssl/ssl_lib.c | 15 +++------ ssl/ssl_locl.h | 4 --- ssl/statem/statem_srvr.c | 27 ++++------------ ssl/t1_lib.c | 29 +++++------------ test/ssltest.c | 1 - 12 files changed, 43 insertions(+), 142 deletions(-) diff --git a/CHANGES b/CHANGES index 21d95f18c8..c8da88ccd0 100644 --- a/CHANGES +++ b/CHANGES @@ -13,6 +13,11 @@ pages. This work was developed in partnership with Intel Corp. [Matt Caswell] + *) SSL_{CTX_}set_ecdh_auto() has been removed and ECDH is support is + always enabled now. If you want to disable the support you should + exclude it using the list of supported ciphers. + [Kurt Roeckx] + *) SSL_{CTX}_set_tmp_ecdh() which can set 1 EC curve now internally calls SSL_{CTX_}set1_curves() which can set a list. [Kurt Roeckx] diff --git a/doc/ssl/SSL_CTX_set1_curves.pod b/doc/ssl/SSL_CTX_set1_curves.pod index e2d4803e00..4b6d1af963 100644 --- a/doc/ssl/SSL_CTX_set1_curves.pod +++ b/doc/ssl/SSL_CTX_set1_curves.pod @@ -3,8 +3,7 @@ =head1 NAME SSL_CTX_set1_curves, SSL_CTX_set1_curves_list, SSL_set1_curves, -SSL_set1_curves_list, SSL_get1_curves, SSL_get_shared_curve, -SSL_CTX_set_ecdh_auto, SSL_set_ecdh_auto - EC supported curve functions +SSL_set1_curves_list, SSL_get1_curves, SSL_get_shared_curve - EC supported curve functions =head1 SYNOPSIS @@ -19,9 +18,6 @@ SSL_CTX_set_ecdh_auto, SSL_set_ecdh_auto - EC supported curve functions int SSL_get1_curves(SSL *ssl, int *curves); int SSL_get_shared_curve(SSL *s, int n); - int SSL_CTX_set_ecdh_auto(SSL_CTX *ctx, int onoff); - int SSL_set_ecdh_auto(SSL *s, int onoff); - =head1 DESCRIPTION SSL_CTX_set1_curves() sets the supported curves for B to B @@ -52,11 +48,6 @@ most applications will only be interested in the first shared curve so B is normally set to zero. If the value B is out of range, NID_undef is returned. -SSL_CTX_set_ecdh_auto() and SSL_set_ecdh_auto() set automatic curve -selection for server B or B to B. If B is 1 then -the highest preference curve is automatically used for ECDH temporary -keys used during key exchange. - All these functions are implemented as macros. =head1 NOTES @@ -65,23 +56,10 @@ If an application wishes to make use of several of these functions for configuration purposes either on a command line or in a file it should consider using the SSL_CONF interface instead of manually parsing options. -The functions SSL_CTX_set_ecdh_auto() and SSL_set_ecdh_auto() can be used to -make a server always choose the most appropriate curve for a client. If set -it will override any temporary ECDH parameters set by a server. Previous -versions of OpenSSL could effectively only use a single ECDH curve set -using a function such as SSL_CTX_set_ecdh_tmp(). Newer applications should -just call: - - SSL_CTX_set_ecdh_auto(ctx, 1); - -and they will automatically support ECDH using the most appropriate shared -curve. - =head1 RETURN VALUES -SSL_CTX_set1_curves(), SSL_CTX_set1_curves_list(), SSL_set1_curves(), -SSL_set1_curves_list(), SSL_CTX_set_ecdh_auto() and SSL_set_ecdh_auto() -return 1 for success and 0 for failure. +SSL_CTX_set1_curves(), SSL_CTX_set1_curves_list(), SSL_set1_curves() and +SSL_set1_curves_list(), return 1 for success and 0 for failure. SSL_get1_curves() returns the number of curves, which may be zero. diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 759f746b21..e4a22dcfbd 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1202,7 +1202,6 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) # define SSL_CTRL_SET_CURVES 91 # define SSL_CTRL_SET_CURVES_LIST 92 # define SSL_CTRL_GET_SHARED_CURVE 93 -# define SSL_CTRL_SET_ECDH_AUTO 94 # define SSL_CTRL_SET_SIGALGS 97 # define SSL_CTRL_SET_SIGALGS_LIST 98 # define SSL_CTRL_CERT_FLAGS 99 @@ -1335,10 +1334,6 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) SSL_ctrl(ctx,SSL_CTRL_SET_CURVES_LIST,0,(char *)s) # define SSL_get_shared_curve(s, n) \ SSL_ctrl(s,SSL_CTRL_GET_SHARED_CURVE,n,NULL) -# define SSL_CTX_set_ecdh_auto(ctx, onoff) \ - SSL_CTX_ctrl(ctx,SSL_CTRL_SET_ECDH_AUTO,onoff,NULL) -# define SSL_set_ecdh_auto(s, onoff) \ - SSL_ctrl(s,SSL_CTRL_SET_ECDH_AUTO,onoff,NULL) # define SSL_CTX_set1_sigalgs(ctx, slist, slistlen) \ SSL_CTX_ctrl(ctx,SSL_CTRL_SET_SIGALGS,slistlen,(int *)slist) # define SSL_CTX_set1_sigalgs_list(ctx, s) \ diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index e9a7b47668..d89cdfabab 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -4256,11 +4256,6 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) case SSL_CTRL_GET_SHARED_CURVE: return tls1_shared_curve(s, larg); -# ifndef OPENSSL_NO_EC - case SSL_CTRL_SET_ECDH_AUTO: - s->cert->ecdh_tmp_auto = larg; - return 1; -# endif #endif case SSL_CTRL_SET_SIGALGS: return tls1_set_sigalgs(s->cert, parg, larg, 0); @@ -4611,9 +4606,6 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) return tls1_set_curves_list(&ctx->tlsext_ellipticcurvelist, &ctx->tlsext_ellipticcurvelist_length, parg); - case SSL_CTRL_SET_ECDH_AUTO: - ctx->cert->ecdh_tmp_auto = larg; - return 1; #endif case SSL_CTRL_SET_SIGALGS: return tls1_set_sigalgs(ctx->cert, parg, larg, 0); diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 802b1141c1..0153b18f46 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -231,10 +231,6 @@ CERT *ssl_cert_dup(CERT *cert) ret->dh_tmp_auto = cert->dh_tmp_auto; #endif -#ifndef OPENSSL_NO_EC - ret->ecdh_tmp_auto = cert->ecdh_tmp_auto; -#endif - for (i = 0; i < SSL_PKEY_NUM; i++) { CERT_PKEY *cpk = cert->pkeys + i; CERT_PKEY *rpk = ret->pkeys + i; diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c index 58acec4235..d2139e1248 100644 --- a/ssl/ssl_ciph.c +++ b/ssl/ssl_ciph.c @@ -1415,8 +1415,6 @@ static int check_suiteb_cipher_list(const SSL_METHOD *meth, CERT *c, *prule_str = "ECDHE-ECDSA-AES256-GCM-SHA384"; break; } - /* Set auto ECDH parameter determination */ - c->ecdh_tmp_auto = 1; return 1; # else SSLerr(SSL_F_CHECK_SUITEB_CIPHER_LIST, diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c index ad20f4434c..ce52569ce9 100644 --- a/ssl/ssl_conf.c +++ b/ssl/ssl_conf.c @@ -268,48 +268,23 @@ static int cmd_Curves(SSL_CONF_CTX *cctx, const char *value) /* ECDH temporary parameters */ static int cmd_ECDHParameters(SSL_CONF_CTX *cctx, const char *value) { - int onoff = -1, rv = 1; - if (cctx->flags & SSL_CONF_FLAG_FILE) { - if (*value == '+') { - onoff = 1; - value++; - } - if (*value == '-') { - onoff = 0; - value++; - } - if (strcasecmp(value, "automatic") == 0) { - if (onoff == -1) - onoff = 1; - } else if (onoff != -1) - return 0; - } else if (cctx->flags & SSL_CONF_FLAG_CMDLINE) { - if (strcmp(value, "auto") == 0) - onoff = 1; - } + int rv = 1; + EC_KEY *ecdh; + int nid; - if (onoff != -1) { - if (cctx->ctx) - rv = SSL_CTX_set_ecdh_auto(cctx->ctx, onoff); - else if (cctx->ssl) - rv = SSL_set_ecdh_auto(cctx->ssl, onoff); - } else { - EC_KEY *ecdh; - int nid; - nid = EC_curve_nist2nid(value); - if (nid == NID_undef) - nid = OBJ_sn2nid(value); - if (nid == 0) - return 0; - ecdh = EC_KEY_new_by_curve_name(nid); - if (!ecdh) - return 0; - if (cctx->ctx) - rv = SSL_CTX_set_tmp_ecdh(cctx->ctx, ecdh); - else if (cctx->ssl) - rv = SSL_set_tmp_ecdh(cctx->ssl, ecdh); - EC_KEY_free(ecdh); - } + nid = EC_curve_nist2nid(value); + if (nid == NID_undef) + nid = OBJ_sn2nid(value); + if (nid == 0) + return 0; + ecdh = EC_KEY_new_by_curve_name(nid); + if (!ecdh) + return 0; + if (cctx->ctx) + rv = SSL_CTX_set_tmp_ecdh(cctx->ctx, ecdh); + else if (cctx->ssl) + rv = SSL_set_tmp_ecdh(cctx->ssl, ecdh); + EC_KEY_free(ecdh); return rv > 0; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index d598f91eb7..ea2acce96a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2010,7 +2010,7 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) unsigned long mask_k, mask_a, emask_k, emask_a; #ifndef OPENSSL_NO_EC int have_ecc_cert, ecdsa_ok, ecc_pkey_size; - int have_ecdh_tmp, ecdh_ok; + int ecdh_ok; X509 *x = NULL; EVP_PKEY *ecc_pkey = NULL; int pk_nid = 0, md_nid = 0; @@ -2036,9 +2036,6 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) dh_tmp = dh_tmp_export = 0; #endif -#ifndef OPENSSL_NO_EC - have_ecdh_tmp = c->ecdh_tmp_auto; -#endif cpk = &(c->pkeys[SSL_PKEY_RSA_ENC]); rsa_enc = pvalid[SSL_PKEY_RSA_ENC] & CERT_PKEY_VALID; rsa_enc_export = (rsa_enc && EVP_PKEY_size(cpk->privatekey) * 8 <= kl); @@ -2063,8 +2060,8 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) #ifdef CIPHER_DEBUG fprintf(stderr, - "rt=%d rte=%d dht=%d ecdht=%d re=%d ree=%d rs=%d ds=%d dhr=%d dhd=%d\n", - rsa_tmp, rsa_tmp_export, dh_tmp, have_ecdh_tmp, rsa_enc, + "rt=%d rte=%d dht=%d re=%d ree=%d rs=%d ds=%d dhr=%d dhd=%d\n", + rsa_tmp, rsa_tmp_export, dh_tmp, rsa_enc, rsa_enc_export, rsa_sign, dsa_sign, dh_rsa, dh_dsa); #endif @@ -2169,10 +2166,8 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) #endif #ifndef OPENSSL_NO_EC - if (have_ecdh_tmp) { - mask_k |= SSL_kECDHE; - emask_k |= SSL_kECDHE; - } + mask_k |= SSL_kECDHE; + emask_k |= SSL_kECDHE; #endif #ifndef OPENSSL_NO_PSK diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index a8789325a4..c1ae1c00ff 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1567,10 +1567,6 @@ typedef struct cert_st { DH *dh_tmp; DH *(*dh_tmp_cb) (SSL *ssl, int is_export, int keysize); int dh_tmp_auto; -# endif -# ifndef OPENSSL_NO_EC - /* Select ECDH parameters automatically */ - int ecdh_tmp_auto; # endif /* Flags related to certificates */ uint32_t cert_flags; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 693c265789..d4668b23b9 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1730,7 +1730,6 @@ int tls_construct_server_key_exchange(SSL *s) DH *dh = NULL, *dhp; #endif #ifndef OPENSSL_NO_EC - EC_KEY *ecdh = NULL, *ecdhp; unsigned char *encodedPoint = NULL; int encodedlen = 0; int curve_id = 0; @@ -1867,15 +1866,13 @@ int tls_construct_server_key_exchange(SSL *s) #ifndef OPENSSL_NO_EC if (type & (SSL_kECDHE | SSL_kECDHEPSK)) { const EC_GROUP *group; + EC_KEY *ecdh = NULL; - ecdhp = NULL; - if (s->cert->ecdh_tmp_auto) { - /* Get NID of appropriate shared curve */ - int nid = tls1_shared_curve(s, -2); - if (nid != NID_undef) - ecdhp = EC_KEY_new_by_curve_name(nid); - } - if (ecdhp == NULL) { + /* Get NID of appropriate shared curve */ + int nid = tls1_shared_curve(s, -2); + if (nid != NID_undef) + ecdh = EC_KEY_new_by_curve_name(nid); + if (ecdh == NULL) { al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, SSL_R_MISSING_TMP_ECDH_KEY); @@ -1888,18 +1885,6 @@ int tls_construct_server_key_exchange(SSL *s) goto err; } - /* Duplicate the ECDH structure. */ - if (ecdhp == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, ERR_R_ECDH_LIB); - goto err; - } - if (s->cert->ecdh_tmp_auto) - ecdh = ecdhp; - else if ((ecdh = EC_KEY_dup(ecdhp)) == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, ERR_R_ECDH_LIB); - goto err; - } - s->s3->tmp.ecdh = ecdh; if ((EC_KEY_get0_public_key(ecdh) == NULL) || (EC_KEY_get0_private_key(ecdh) == NULL) || diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6a9dc5db28..971aad381a 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -259,8 +259,8 @@ static const unsigned char ecformats_default[] = { TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2 }; -/* The client's default curves / the server's 'auto' curves. */ -static const unsigned char eccurves_auto[] = { +/* The default curves */ +static const unsigned char eccurves_default[] = { /* Prefer P-256 which has the fastest and most secure implementations. */ 0, 23, /* secp256r1 (23) */ /* Other >= 256-bit prime curves. */ @@ -438,13 +438,8 @@ static int tls1_get_curvelist(SSL *s, int sess, pcurveslen = s->tlsext_ellipticcurvelist_length; } if (!*pcurves) { - if (!s->server || s->cert->ecdh_tmp_auto) { - *pcurves = eccurves_auto; - pcurveslen = sizeof(eccurves_auto); - } else { - *pcurves = eccurves_all; - pcurveslen = sizeof(eccurves_all); - } + *pcurves = eccurves_default; + pcurveslen = sizeof(eccurves_default); } } @@ -877,19 +872,11 @@ 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 assume OK */ - if (s->cert->ecdh_tmp_auto) - return 1; - else - return 0; - } - if (s->cert->ecdh_tmp_auto) { - /* Need a shared curve */ - if (tls1_shared_curve(s, 0)) - return 1; - else - return 0; + return 1; } + /* Need a shared curve */ + if (tls1_shared_curve(s, 0)) + return 1; return 0; } # endif /* OPENSSL_NO_EC */ diff --git a/test/ssltest.c b/test/ssltest.c index 6455af3fbf..68d48d1d73 100644 --- a/test/ssltest.c +++ b/test/ssltest.c @@ -1475,7 +1475,6 @@ int main(int argc, char *argv[]) goto end; } - SSL_CTX_set_ecdh_auto(s_ctx, 1); SSL_CTX_set_tmp_ecdh(s_ctx, ecdh); SSL_CTX_set_options(s_ctx, SSL_OP_SINGLE_ECDH_USE); EC_KEY_free(ecdh); -- 2.34.1