Remove SSL_{CTX_}set_ecdh_auto() and always enable ECDH

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>

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]

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_set1_curves_list, SSL_get1_curves, SSL_get_shared_curve - EC supported curve functions
-SSL_CTX_set_ecdh_auto, SSL_set_ecdh_auto - 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<ctx> to B<clistlen>
@@ -52,11 +48,6 @@ most applications will only be interested in the first shared curve
 so B<n> is normally set to zero. If the value B<n> 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<ctx> or B<ssl> to B<onoff>. If B<onoff> 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_CTX_set1_curves(), SSL_CTX_set1_curves_list(), SSL_set1_curves() and
-SSL_set1_curves_list(), SSL_CTX_set_ecdh_auto() and SSL_set_ecdh_auto()
+SSL_set1_curves_list(), return 1 for success and 0 for failure.
-return 1 for success and 0 for failure.
 
 SSL_get1_curves() returns the number of curves, which may be zero.
 

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) \

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);

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;

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,

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;
+    int rv = 1;
-    if (cctx->flags & SSL_CONF_FLAG_FILE) {
+    EC_KEY *ecdh;
-        if (*value == '+') {
+    int nid;
-            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;
-    }
 
-    if (onoff != -1) {
+    nid = EC_curve_nist2nid(value);
-        if (cctx->ctx)
+    if (nid == NID_undef)
-            rv = SSL_CTX_set_ecdh_auto(cctx->ctx, onoff);
+        nid = OBJ_sn2nid(value);
-        else if (cctx->ssl)
+    if (nid == 0)
-            rv = SSL_set_ecdh_auto(cctx->ssl, onoff);
+        return 0;
-    } else {
+    ecdh = EC_KEY_new_by_curve_name(nid);
-        EC_KEY *ecdh;
+    if (!ecdh)
-        int nid;
+        return 0;
-        nid = EC_curve_nist2nid(value);
+    if (cctx->ctx)
-        if (nid == NID_undef)
+        rv = SSL_CTX_set_tmp_ecdh(cctx->ctx, ecdh);
-            nid = OBJ_sn2nid(value);
+    else if (cctx->ssl)
-        if (nid == 0)
+        rv = SSL_set_tmp_ecdh(cctx->ssl, ecdh);
-            return 0;
+    EC_KEY_free(ecdh);
-        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;
 }

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",
+            "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, have_ecdh_tmp, rsa_enc,
+            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;
-        mask_k |= SSL_kECDHE;
+    emask_k |= SSL_kECDHE;
-        emask_k |= SSL_kECDHE;
-    }
 #endif
 
 #ifndef OPENSSL_NO_PSK

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;

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;
+        /* Get NID of appropriate shared curve */
-        if (s->cert->ecdh_tmp_auto) {
+        int nid = tls1_shared_curve(s, -2);
-            /* Get NID of appropriate shared curve */
+        if (nid != NID_undef)
-            int nid = tls1_shared_curve(s, -2);
+            ecdh = EC_KEY_new_by_curve_name(nid);
-            if (nid != NID_undef)
+        if (ecdh == NULL) {
-                ecdhp = EC_KEY_new_by_curve_name(nid);
-        }
-        if (ecdhp == 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) ||

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. */
+/* The default curves */
-static const unsigned char eccurves_auto[] = {
+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_default;
-                *pcurves = eccurves_auto;
+            pcurveslen = sizeof(eccurves_default);
-                pcurveslen = sizeof(eccurves_auto);
-            } else {
-                *pcurves = eccurves_all;
-                pcurveslen = sizeof(eccurves_all);
-            }
         }
     }
 
@@ -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 */
+        return 1;
-        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;
     }
+    /* Need a shared curve */
+    if (tls1_shared_curve(s, 0))
+        return 1;
     return 0;
 }
 # endif                         /* OPENSSL_NO_EC */

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);