Clean up the EC key/curve configuration handling.

Over the years OpenSSL grew multiple ways of being able to specify EC keys
(and/or curves) for use with ECDH and ECDHE key exchange. You could specify
a static EC key (SSL{_CTX,}_set_tmp_ecdh()), use that as a curve and
generate ephemeral keys (SSL_OP_SINGLE_ECDH_USE), provide the EC key via
a callback that was provided with insufficient information
(SSL{_CTX,}_set_tmp_ecdh_cb()) or enable automatic selection and generation
of EC keys via SSL{_CTX,}_set_ecdh_auto(). This complexity leads to
problems (like ECDHE not being enabled) and potential weird configuration
(like being able to do ECDHE without the ephemeral part...).

We no longer support ECDH and ECDHE can be disabled by removing ECDHE
ciphers from the cipher list. As such, permanently enable automatic EC
curve selection and generation, effectively disabling all of the
configuration knobs. The only exception is the
SSL{_CTX,}_set_tmp_ecdh() functions, which retain part of their previous
behaviour by configuring the curve of the given EC key as the only curve
being enabled. Everything else becomes a no-op.

ok beck@ doug@

7 files changed, 54 insertions, 120 deletions

diff --git a/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index 1abe01cd88..abebaa0fc4 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.154 2017/08/09 17:49:54 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.155 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1838,39 +1838,26 @@ _SSL_set_dh_auto(SSL *s, int state)
 static int
 _SSL_set_tmp_ecdh(SSL *s, EC_KEY *ecdh)
 {
+        const EC_GROUP *group;
+        int nid;
+
         if (!ssl_cert_inst(&s->cert)) {
                 SSLerror(s, ERR_R_MALLOC_FAILURE);
                 return 0;
         }
 
-        if (ecdh == NULL) {
-                SSLerror(s, ERR_R_PASSED_NULL_PARAMETER);
+        if (ecdh == NULL)
                 return 0;
-        }
-
-        if (!EC_KEY_up_ref(ecdh)) {
-                SSLerror(s, ERR_R_ECDH_LIB);
+        if ((group = EC_KEY_get0_group(ecdh)) == NULL)
                 return 0;
-        }
-
-        if (!(s->internal->options & SSL_OP_SINGLE_ECDH_USE)) {
-                if (!EC_KEY_generate_key(ecdh)) {
-                        EC_KEY_free(ecdh);
-                        SSLerror(s, ERR_R_ECDH_LIB);
-                        return 0;
-                }
-        }
-
-        EC_KEY_free(s->cert->ecdh_tmp);
-        s->cert->ecdh_tmp = ecdh;
 
-        return 1;
+        nid = EC_GROUP_get_curve_name(group);
+        return SSL_set1_groups(s, &nid, 1);
 }
 
 static int
 _SSL_set_ecdh_auto(SSL *s, int state)
 {
-        s->cert->ecdh_tmp_auto = state;
         return 1;
 }
 
@@ -2095,7 +2082,6 @@ ssl3_callback_ctrl(SSL *s, int cmd, void (*fp)(void))
                 return 1;
 
         case SSL_CTRL_SET_TMP_ECDH_CB:
-                s->cert->ecdh_tmp_cb = (EC_KEY *(*)(SSL *, int, int))fp;
                 return 1;
 
         case SSL_CTRL_SET_TLSEXT_DEBUG_CB:
@@ -2133,35 +2119,21 @@ _SSL_CTX_set_dh_auto(SSL_CTX *ctx, int state)
 static int
 _SSL_CTX_set_tmp_ecdh(SSL_CTX *ctx, EC_KEY *ecdh)
 {
-        EC_KEY *ecdh_tmp;
+        const EC_GROUP *group;
+        int nid;
 
-        if (ecdh == NULL) {
-                SSLerrorx(ERR_R_ECDH_LIB);
+        if (ecdh == NULL)
                 return 0;
-        }
-
-        if ((ecdh_tmp = EC_KEY_dup(ecdh)) == NULL) {
-                SSLerrorx(ERR_R_EC_LIB);
+        if ((group = EC_KEY_get0_group(ecdh)) == NULL)
                 return 0;
-        }
-        if (!(ctx->internal->options & SSL_OP_SINGLE_ECDH_USE)) {
-                if (!EC_KEY_generate_key(ecdh_tmp)) {
-                        EC_KEY_free(ecdh_tmp);
-                        SSLerrorx(ERR_R_ECDH_LIB);
-                        return 0;
-                }
-        }
 
-        EC_KEY_free(ctx->internal->cert->ecdh_tmp);
-        ctx->internal->cert->ecdh_tmp = ecdh_tmp;
-
-        return 1;
+        nid = EC_GROUP_get_curve_name(group);
+        return SSL_CTX_set1_groups(ctx, &nid, 1);
 }
 
 static int
 _SSL_CTX_set_ecdh_auto(SSL_CTX *ctx, int state)
 {
-        ctx->internal->cert->ecdh_tmp_auto = state;
         return 1;
 }
 
@@ -2347,8 +2319,6 @@ ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp)(void))
                 return 1;
 
         case SSL_CTRL_SET_TMP_ECDH_CB:
-                ctx->internal->cert->ecdh_tmp_cb =
-                    (EC_KEY *(*)(SSL *, int, int))fp;
                 return 1;
 
         case SSL_CTRL_SET_TLSEXT_SERVERNAME_CB:
diff --git a/src/lib/libssl/ssl.h b/src/lib/libssl/ssl.h
index dda5192c10..e816dec83c 100644
--- a/src/lib/libssl/ssl.h
+++ b/src/lib/libssl/ssl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl.h,v 1.129 2017/05/07 04:22:24 beck Exp $ */
+/* $OpenBSD: ssl.h,v 1.130 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -503,8 +503,6 @@ struct ssl_session_st {
 #define SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION   0x00010000L
 /* Disallow client initiated renegotiation. */
 #define SSL_OP_NO_CLIENT_RENEGOTIATION                  0x00020000L
-/* If set, always create a new key when using tmp_ecdh parameters */
-#define SSL_OP_SINGLE_ECDH_USE                          0x00080000L
 /* If set, always create a new key when using tmp_dh parameters */
 #define SSL_OP_SINGLE_DH_USE                            0x00100000L
 /* Set on servers to choose the cipher according to the server's
@@ -549,6 +547,7 @@ struct ssl_session_st {
 #define SSL_OP_PKCS1_CHECK_1                            0x0
 #define SSL_OP_PKCS1_CHECK_2                            0x0
 #define SSL_OP_SAFARI_ECDHE_ECDSA_BUG                   0x0
+#define SSL_OP_SINGLE_ECDH_USE                          0x0
 #define SSL_OP_SSLEAY_080_CLIENT_DH_BUG                 0x0
 #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG              0x0
 #define SSL_OP_TLS_BLOCK_PADDING_BUG                    0x0
diff --git a/src/lib/libssl/ssl_cert.c b/src/lib/libssl/ssl_cert.c
index 83a9f2e92d..174441c70e 100644
--- a/src/lib/libssl/ssl_cert.c
+++ b/src/lib/libssl/ssl_cert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_cert.c,v 1.64 2017/02/07 02:08:38 beck Exp $ */
+/* $OpenBSD: ssl_cert.c,v 1.65 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -242,8 +242,6 @@ ssl_cert_dup(CERT *cert)
                         goto err;
                 }
         }
-        ret->ecdh_tmp_cb = cert->ecdh_tmp_cb;
-        ret->ecdh_tmp_auto = cert->ecdh_tmp_auto;
 
         for (i = 0; i < SSL_PKEY_NUM; i++) {
                 if (cert->pkeys[i].x509 != NULL) {
diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index d933acb32d..bc8b56d3be 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.162 2017/08/09 22:24:25 jsing Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.163 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2042,7 +2042,7 @@ void
 ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher)
 {
         int              rsa_enc, rsa_sign, dh_tmp, dsa_sign;
-        int              have_ecc_cert, have_ecdh_tmp;
+        int              have_ecc_cert;
         unsigned long    mask_k, mask_a;
         X509            *x = NULL;
         CERT_PKEY       *cpk;
@@ -2053,9 +2053,6 @@ ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher)
         dh_tmp = (c->dh_tmp != NULL || c->dh_tmp_cb != NULL ||
             c->dh_tmp_auto != 0);
 
-        have_ecdh_tmp = (c->ecdh_tmp != NULL || c->ecdh_tmp_cb != NULL ||
-            c->ecdh_tmp_auto != 0);
-
         cpk = &(c->pkeys[SSL_PKEY_RSA_ENC]);
         rsa_enc = (cpk->x509 != NULL && cpk->privatekey != NULL);
         cpk = &(c->pkeys[SSL_PKEY_RSA_SIGN]);
@@ -2104,8 +2101,7 @@ ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher)
                         mask_a|=SSL_aECDSA;
         }
 
-        if (have_ecdh_tmp)
-                mask_k|=SSL_kECDHE;
+        mask_k |= SSL_kECDHE;
 
         c->mask_k = mask_k;
         c->mask_a = mask_a;
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index f98ce681a2..8ef2d01402 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.183 2017/08/09 22:24:25 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.184 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -993,8 +993,6 @@ typedef struct cert_st {
         int dh_tmp_auto;
 
         EC_KEY *ecdh_tmp;
-        EC_KEY *(*ecdh_tmp_cb)(SSL *ssl, int is_export, int keysize);
-        int ecdh_tmp_auto;
 
         CERT_PKEY pkeys[SSL_PKEY_NUM];
 
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 730d4ed1ad..575621a0ce 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.17 2017/05/07 04:22:24 beck Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.18 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1267,27 +1267,23 @@ ssl3_send_server_kex_dhe(SSL *s, CBB *cbb)
 static int
 ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
 {
-        CBB ecpoint;
-        unsigned char *data;
-        EC_KEY *ecdh = NULL, *ecdhp;
         const EC_GROUP *group;
+        const EC_POINT *pubkey;
+        unsigned char *data;
         int encoded_len = 0;
         int curve_id = 0;
         BN_CTX *bn_ctx = NULL;
+        EC_KEY *ecdh;
+        CBB ecpoint;
         int al;
 
-        ecdhp = s->cert->ecdh_tmp;
-        if (s->cert->ecdh_tmp_auto != 0) {
-                if (nid != NID_undef)
-                        ecdhp = EC_KEY_new_by_curve_name(nid);
-        } else if (ecdhp == NULL && s->cert->ecdh_tmp_cb != NULL) {
-                ecdhp = s->cert->ecdh_tmp_cb(s, 0,
-                    SSL_C_PKEYLENGTH(S3I(s)->hs.new_cipher));
-        }
-        if (ecdhp == NULL) {
-                al = SSL_AD_HANDSHAKE_FAILURE;
-                SSLerror(s, SSL_R_MISSING_TMP_ECDH_KEY);
-                goto f_err;
+        /*
+         * Only named curves are supported in ECDH ephemeral key exchanges.
+         * For supported named curves, curve_id is non-zero.
+         */
+        if ((curve_id = tls1_ec_nid2curve_id(nid)) == 0) {
+                SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
+                goto err;
         }
 
         if (S3I(s)->tmp.ecdh != NULL) {
@@ -1295,46 +1291,28 @@ ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
                 goto err;
         }
 
-        /* Duplicate the ECDH structure. */
-        if (s->cert->ecdh_tmp_auto != 0) {
-                ecdh = ecdhp;
-        } else if ((ecdh = EC_KEY_dup(ecdhp)) == NULL) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        S3I(s)->tmp.ecdh = ecdh;
-
-        if ((EC_KEY_get0_public_key(ecdh) == NULL) ||
-            (EC_KEY_get0_private_key(ecdh) == NULL) ||
-            (s->internal->options & SSL_OP_SINGLE_ECDH_USE)) {
-                if (!EC_KEY_generate_key(ecdh)) {
-                        SSLerror(s, ERR_R_ECDH_LIB);
-                        goto err;
-                }
+        if ((S3I(s)->tmp.ecdh = EC_KEY_new_by_curve_name(nid)) == NULL) {
+                al = SSL_AD_HANDSHAKE_FAILURE;
+                SSLerror(s, SSL_R_MISSING_TMP_ECDH_KEY);
+                goto f_err;
         }
+        ecdh = S3I(s)->tmp.ecdh;
 
-        if (((group = EC_KEY_get0_group(ecdh)) == NULL) ||
-            (EC_KEY_get0_public_key(ecdh)  == NULL) ||
-            (EC_KEY_get0_private_key(ecdh) == NULL)) {
+        if (!EC_KEY_generate_key(ecdh)) {
                 SSLerror(s, ERR_R_ECDH_LIB);
                 goto err;
         }
-
-        /*
-         * Only named curves are supported in ECDH ephemeral key exchanges.
-         * For supported named curves, curve_id is non-zero.
-         */
-        if ((curve_id = tls1_ec_nid2curve_id(
-            EC_GROUP_get_curve_name(group))) == 0) {
-                SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
+        if ((group = EC_KEY_get0_group(ecdh)) == NULL ||
+            (pubkey = EC_KEY_get0_public_key(ecdh)) == NULL ||
+            EC_KEY_get0_private_key(ecdh) == NULL) {
+                SSLerror(s, ERR_R_ECDH_LIB);
                 goto err;
         }
 
         /*
-         * Encode the public key. First check the size of encoding and
-         * allocate memory accordingly.
+         * Encode the public key.
          */
-        encoded_len = EC_POINT_point2oct(group, EC_KEY_get0_public_key(ecdh),
+        encoded_len = EC_POINT_point2oct(group, pubkey,
             POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL);
         if (encoded_len == 0) {
                 SSLerror(s, ERR_R_ECDH_LIB);
@@ -1360,8 +1338,8 @@ ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
                 goto err;
         if (!CBB_add_space(&ecpoint, &data, encoded_len))
                 goto err;
-        if (EC_POINT_point2oct(group, EC_KEY_get0_public_key(ecdh),
-            POINT_CONVERSION_UNCOMPRESSED, data, encoded_len, bn_ctx) == 0) {
+        if (EC_POINT_point2oct(group, pubkey, POINT_CONVERSION_UNCOMPRESSED,
+            data, encoded_len, bn_ctx) == 0) {
                 SSLerror(s, ERR_R_ECDH_LIB);
                 goto err;
         }
@@ -1431,7 +1409,7 @@ ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb)
 
         nid = tls1_get_shared_curve(s);
 
-        if (s->cert->ecdh_tmp_auto != 0 && nid == NID_X25519)
+        if (nid == NID_X25519)
                 return ssl3_send_server_kex_ecdhe_ecx(s, nid, cbb);
 
         return ssl3_send_server_kex_ecdhe_ecp(s, nid, cbb);
diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index ea44e7579a..42fd18fe2d 100644
--- a/src/lib/libssl/t1_lib.c
+++ b/src/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.123 2017/08/09 22:24:25 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.124 2017/08/10 17:18:38 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -611,18 +611,13 @@ tls1_check_ec_tmp_key(SSL *s)
         EC_KEY *ec = s->cert->ecdh_tmp;
         uint16_t curve_id;
 
-        if (s->cert->ecdh_tmp_auto != 0) {
-                /* Need a shared curve. */
-                if (tls1_get_shared_curve(s) != NID_undef)
-                        return (1);
-                return (0);
-        }
+        /* Need a shared curve. */
+        if (tls1_get_shared_curve(s) != NID_undef)
+                return (1);
 
-        if (ec == NULL) {
-                if (s->cert->ecdh_tmp_cb != NULL)
-                        return (1);
+        if (ec == NULL)
                 return (0);
-        }
+
         if (tls1_set_ec_id(&curve_id, NULL, ec) != 1)
                 return (0);