Make SSL_{CTX}_set_tmp_ecdh() call SSL_{CTX_}set1_curves()

SSL_{CTX}_set_tmp_ecdh() allows to set 1 EC curve and then tries to use it.  On
the other hand SSL_{CTX_}set1_curves() allows you to set a list of curves, but
only when SSL_{CTX_}set_ecdh_auto() was called to turn it on.

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

diff --git a/CHANGES b/CHANGES
index b365cb0ad1f4a5522f1debc9d47365adad5315f6..21d95f18c8436aef8a365101a82fc5c5a251a2ba 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -13,6 +13,10 @@
      pages. This work was developed in partnership with Intel Corp.
      [Matt Caswell]
 
+  *) 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]
+
   *) 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]

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 0df228e50a1f5587986a5e33c4149953082e8937..e9a7b47668bc20ed47e9d4334d77aa0db4d060b6 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -4072,27 +4072,24 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
 #ifndef OPENSSL_NO_EC
     case SSL_CTRL_SET_TMP_ECDH:
         {
-            EC_KEY *ecdh = NULL;
+            const EC_GROUP *group = NULL;
+            int nid;
 
             if (parg == NULL) {
                 SSLerr(SSL_F_SSL3_CTRL, ERR_R_PASSED_NULL_PARAMETER);
-                return (ret);
-            }
-            if (!EC_KEY_up_ref((EC_KEY *)parg)) {
-                SSLerr(SSL_F_SSL3_CTRL, ERR_R_ECDH_LIB);
-                return (ret);
+                return 0;
             }
-            ecdh = (EC_KEY *)parg;
-            if (!(s->options & SSL_OP_SINGLE_ECDH_USE)) {
-                if (!EC_KEY_generate_key(ecdh)) {
-                    EC_KEY_free(ecdh);
-                    SSLerr(SSL_F_SSL3_CTRL, ERR_R_ECDH_LIB);
-                    return (ret);
-                }
+            group = EC_KEY_get0_group((const EC_KEY *)parg);
+            if (group == NULL) {
+                SSLerr(SSL_F_SSL3_CTRL, EC_R_MISSING_PARAMETERS);
+                return 0;
             }
-            EC_KEY_free(s->cert->ecdh_tmp);
-            s->cert->ecdh_tmp = ecdh;
-            ret = 1;
+            nid = EC_GROUP_get_curve_name(group);
+            if (nid == NID_undef)
+                return 0;
+            return tls1_set_curves(&s->tlsext_ellipticcurvelist,
+                                   &s->tlsext_ellipticcurvelist_length,
+                                   &nid, 1);
         }
         break;
 #endif                          /* !OPENSSL_NO_EC */
@@ -4522,28 +4519,24 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
 #ifndef OPENSSL_NO_EC
     case SSL_CTRL_SET_TMP_ECDH:
         {
-            EC_KEY *ecdh = NULL;
+            const EC_GROUP *group = NULL;
+            int nid;
 
             if (parg == NULL) {
-                SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_ECDH_LIB);
+                SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_PASSED_NULL_PARAMETER);
                 return 0;
             }
-            ecdh = EC_KEY_dup((EC_KEY *)parg);
-            if (ecdh == NULL) {
-                SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_EC_LIB);
+            group = EC_KEY_get0_group((const EC_KEY *)parg);
+            if (group == NULL) {
+                SSLerr(SSL_F_SSL3_CTX_CTRL, EC_R_MISSING_PARAMETERS);
                 return 0;
             }
-            if (!(ctx->options & SSL_OP_SINGLE_ECDH_USE)) {
-                if (!EC_KEY_generate_key(ecdh)) {
-                    EC_KEY_free(ecdh);
-                    SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_ECDH_LIB);
-                    return 0;
-                }
-            }
-
-            EC_KEY_free(cert->ecdh_tmp);
-            cert->ecdh_tmp = ecdh;
-            return 1;
+            nid = EC_GROUP_get_curve_name(group);
+            if (nid == NID_undef)
+                return 0;
+            return tls1_set_curves(&ctx->tlsext_ellipticcurvelist,
+                                   &ctx->tlsext_ellipticcurvelist_length,
+                                   &nid, 1);
         }
         /* break; */
 #endif                          /* !OPENSSL_NO_EC */

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 45b1d164bab57d24ce368a0c66f3e29d8610a30c..802b1141c1dd6d39577ea989f1758feab945f3df 100644 (file)
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -232,13 +232,6 @@ CERT *ssl_cert_dup(CERT *cert)
 #endif
 
 #ifndef OPENSSL_NO_EC
-    if (cert->ecdh_tmp) {
-        ret->ecdh_tmp = EC_KEY_dup(cert->ecdh_tmp);
-        if (ret->ecdh_tmp == NULL) {
-            SSLerr(SSL_F_SSL_CERT_DUP, ERR_R_EC_LIB);
-            goto err;
-        }
-    }
     ret->ecdh_tmp_auto = cert->ecdh_tmp_auto;
 #endif
 
@@ -394,9 +387,6 @@ void ssl_cert_free(CERT *c)
 #ifndef OPENSSL_NO_DH
     DH_free(c->dh_tmp);
 #endif
-#ifndef OPENSSL_NO_EC
-    EC_KEY_free(c->ecdh_tmp);
-#endif
 
     ssl_cert_clear_certs(c);
     OPENSSL_free(c->conf_sigalgs);

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 9343e7dff2f5ee4eeed4531e88d5a25963c77b50..d598f91eb79c5d66ec6c96cc0fb9701ca44cc03f 100644 (file)
--- 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_auto);
+    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;

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 2da47f1d35227a68c0d4f4b63b8e8c4995dbf6c4..a8789325a4a0582dd3c5b8cd0f977db1801357ed 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1569,7 +1569,6 @@ typedef struct cert_st {
     int dh_tmp_auto;
 # endif
 # ifndef OPENSSL_NO_EC
-    EC_KEY *ecdh_tmp;
     /* Select ECDH parameters automatically */
     int ecdh_tmp_auto;
 # endif

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index fb6410635085f575f298031367dd2ba2c487770c..693c26578954cc9bbe84aa89eb22079faba8cbfb 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1868,7 +1868,7 @@ int tls_construct_server_key_exchange(SSL *s)
     if (type & (SSL_kECDHE | SSL_kECDHEPSK)) {
         const EC_GROUP *group;
 
-        ecdhp = cert->ecdh_tmp;
+        ecdhp = NULL;
         if (s->cert->ecdh_tmp_auto) {
             /* Get NID of appropriate shared curve */
             int nid = tls1_shared_curve(s, -2);

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 951be10d2df1a8aba3a6cc712c2f08c84e5be585..6a9dc5db28343315c2edd059bbaeeacc9690a5c2 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -507,8 +507,9 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
 }
 
 /*-
- * Return |nmatch|th shared curve or NID_undef if there is no match.
- * For nmatch == -1, return number of  matches
+ * For nmatch >= 0, return the NID of the |nmatch|th shared curve or NID_undef
+ * if there is no match.
+ * For nmatch == -1, return number of matches
  * For nmatch == -2, return the NID of the curve to use for
  * an EC tmp key, or NID_undef if there is no match.
  */
@@ -842,11 +843,18 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int set_ee_md)
 }
 
 # ifndef OPENSSL_NO_EC
-/* Check EC temporary key is compatible with client extensions */
+/*
+ * tls1_check_ec_tmp_key - Check EC temporary key compatiblity
+ * @s: SSL connection
+ * @cid: Cipher ID we're considering using
+ *
+ * Checks that the kECDHE cipher suite we're considering using
+ * is compatible with the client extensions.
+ *
+ * Returns 0 when the cipher can't be used or 1 when it can.
+ */
 int tls1_check_ec_tmp_key(SSL *s, unsigned long cid)
 {
-    unsigned char curve_id[2];
-    EC_KEY *ec = s->cert->ecdh_tmp;
 #  ifdef OPENSSL_SSL_DEBUG_BROKEN_PROTOCOL
     /* Allow any curve: not just those peer supports */
     if (s->cert->cert_flags & SSL_CERT_FLAG_BROKEN_PROTOCOL)
@@ -857,6 +865,7 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid)
      * curves permitted.
      */
     if (tls1_suiteb(s)) {
+        unsigned char curve_id[2];
         /* Curve to check determined by ciphersuite */
         if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)
             curve_id[1] = TLSEXT_curve_P_256;
@@ -871,18 +880,8 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid)
         /* If auto assume OK */
         if (s->cert->ecdh_tmp_auto)
             return 1;
-        /* Otherwise check curve is acceptable */
-        else {
-            unsigned char curve_tmp[2];
-            if (!ec)
-                return 0;
-            if (!tls1_set_ec_id(curve_tmp, NULL, ec))
-                return 0;
-            if (!curve_tmp[0] || curve_tmp[1] == curve_id[1])
-                return 1;
+        else
             return 0;
-        }
-
     }
     if (s->cert->ecdh_tmp_auto) {
         /* Need a shared curve */
@@ -891,17 +890,7 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid)
         else
             return 0;
     }
-    if (!ec) {
-        return 0;
-    }
-    if (!tls1_set_ec_id(curve_id, NULL, ec))
-        return 0;
-/* Set this to allow use of invalid curves for testing */
-#  if 0
-    return 1;
-#  else
-    return tls1_check_ec_key(s, curve_id, NULL);
-#  endif
+    return 0;
 }
 # endif                         /* OPENSSL_NO_EC */
 

diff --git a/test/ssltest.c b/test/ssltest.c
index 68d48d1d739b6c4f649689eb0ff03c271b653df7..6455af3fbf4dfcc8de1db349f927b2aebd0da2a3 100644 (file)
--- a/test/ssltest.c
+++ b/test/ssltest.c
@@ -1475,6 +1475,7 @@ 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);