Factor out/rewrite the ECDHE EC point key exchange code.

This reduces replication between the existing TLS client/server and allows
the code to soon be reused for TLSv1.3.

With feedback from inoguchi@ and tb@

ok inoguchi@ tb@

5 files changed, 188 insertions, 173 deletions

diff --git a/src/lib/libssl/Makefile b/src/lib/libssl/Makefile
index b38bb5f682..489c4fd217 100644
--- a/src/lib/libssl/Makefile
+++ b/src/lib/libssl/Makefile
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.60 2020/01/22 07:58:27 jsing Exp $
+# $OpenBSD: Makefile,v 1.61 2020/01/30 16:25:09 jsing Exp $
 
 .include <bsd.own.mk>
 .ifndef NOMAN
@@ -51,6 +51,7 @@ SRCS= \
         ssl_clnt.c \
         ssl_err.c \
         ssl_init.c \
+        ssl_kex.c \
         ssl_lib.c \
         ssl_methods.c \
         ssl_packet.c \
diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 22e02735c8..dfb1d7ddb6 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.62 2020/01/23 10:48:37 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.63 2020/01/30 16:25:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1263,56 +1263,27 @@ ssl3_get_server_kex_dhe(SSL *s, EVP_PKEY **pkey, CBS *cbs)
 static int
 ssl3_get_server_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, int nid, CBS *public)
 {
-        const EC_GROUP *group;
-        EC_GROUP *ngroup = NULL;
-        EC_POINT *point = NULL;
-        BN_CTX *bn_ctx = NULL;
         EC_KEY *ecdh = NULL;
         int ret = -1;
 
-        /*
-         * Extract the server's ephemeral ECDH public key.
-         */
-
+        /* Extract the server's ephemeral ECDH public key. */
         if ((ecdh = EC_KEY_new()) == NULL) {
                 SSLerror(s, ERR_R_MALLOC_FAILURE);
                 goto err;
         }
-
-        if ((ngroup = EC_GROUP_new_by_curve_name(nid)) == NULL) {
-                SSLerror(s, ERR_R_EC_LIB);
-                goto err;
-        }
-        if (EC_KEY_set_group(ecdh, ngroup) == 0) {
-                SSLerror(s, ERR_R_EC_LIB);
-                goto err;
-        }
-
-        group = EC_KEY_get0_group(ecdh);
-
-        if ((point = EC_POINT_new(group)) == NULL ||
-            (bn_ctx = BN_CTX_new()) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
-                goto err;
-        }
-
-        if (EC_POINT_oct2point(group, point, CBS_data(public),
-            CBS_len(public), bn_ctx) == 0) {
+        if (!ssl_kex_peer_public_ecdhe_ecp(ecdh, nid, public)) {
                 SSLerror(s, SSL_R_BAD_ECPOINT);
                 ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
                 goto err;
         }
 
-        EC_KEY_set_public_key(ecdh, point);
+        sc->peer_nid = nid;
         sc->peer_ecdh_tmp = ecdh;
         ecdh = NULL;
 
         ret = 1;
 
  err:
-        BN_CTX_free(bn_ctx);
-        EC_GROUP_free(ngroup);
-        EC_POINT_free(point);
         EC_KEY_free(ecdh);
 
         return (ret);
@@ -2049,87 +2020,37 @@ err:
 static int
 ssl3_send_client_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, CBB *cbb)
 {
-        const EC_GROUP *group = NULL;
-        const EC_POINT *point = NULL;
         EC_KEY *ecdh = NULL;
-        BN_CTX *bn_ctx = NULL;
-        unsigned char *key = NULL;
-        unsigned char *data;
-        size_t encoded_len;
-        int key_size = 0, key_len;
+        uint8_t *key = NULL;
+        size_t key_len = 0;
         int ret = -1;
         CBB ecpoint;
 
-        if ((group = EC_KEY_get0_group(sc->peer_ecdh_tmp)) == NULL ||
-            (point = EC_KEY_get0_public_key(sc->peer_ecdh_tmp)) == NULL) {
-                SSLerror(s, ERR_R_INTERNAL_ERROR);
-                goto err;
-        }
-
         if ((ecdh = EC_KEY_new()) == NULL) {
                 SSLerror(s, ERR_R_MALLOC_FAILURE);
                 goto err;
         }
 
-        if (!EC_KEY_set_group(ecdh, group)) {
-                SSLerror(s, ERR_R_EC_LIB);
+        if (!ssl_kex_generate_ecdhe_ecp(ecdh, sc->peer_nid))
                 goto err;
-        }
 
-        /* Generate a new ECDH key pair. */
-        if (!EC_KEY_generate_key(ecdh)) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        if ((key_size = ECDH_size(ecdh)) <= 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        if ((key = malloc(key_size)) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
-                goto err;
-        }
-        key_len = ECDH_compute_key(key, key_size, point, ecdh, NULL);
-        if (key_len <= 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-
-        /* Generate master key from the result. */
-        s->session->master_key_length =
-            tls1_generate_master_secret(s,
-                s->session->master_key, key, key_len);
-
-        encoded_len = EC_POINT_point2oct(group, EC_KEY_get0_public_key(ecdh),
-            POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL);
-        if (encoded_len == 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-
-        if ((bn_ctx = BN_CTX_new()) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
-                goto err;
-        }
-
-        /* Encode the public key. */
+        /* Encode our public key. */
         if (!CBB_add_u8_length_prefixed(cbb, &ecpoint))
                 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 (!ssl_kex_public_ecdhe_ecp(ecdh, &ecpoint))
                 goto err;
         if (!CBB_flush(cbb))
                 goto err;
 
+        if (!ssl_kex_derive_ecdhe_ecp(ecdh, sc->peer_ecdh_tmp, &key, &key_len))
+                goto err;
+        s->session->master_key_length = tls1_generate_master_secret(s,
+                s->session->master_key, key, key_len);
+
         ret = 1;
 
  err:
-        freezero(key, key_size);
-
-        BN_CTX_free(bn_ctx);
+        freezero(key, key_len);
         EC_KEY_free(ecdh);
 
         return (ret);
diff --git a/src/lib/libssl/ssl_kex.c b/src/lib/libssl/ssl_kex.c
new file mode 100644
index 0000000000..439c1702b3
--- /dev/null
+++ b/src/lib/libssl/ssl_kex.c
@@ -0,0 +1,141 @@
+/* $OpenBSD: ssl_kex.c,v 1.1 2020/01/30 16:25:09 jsing Exp $ */
+/*
+ * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <stdlib.h>
+
+#include <openssl/ec.h>
+#include <openssl/ecdh.h>
+
+#include "bytestring.h"
+
+int
+ssl_kex_generate_ecdhe_ecp(EC_KEY *ecdh, int nid)
+{
+        EC_GROUP *group;
+        int ret = 0;
+
+        if ((group = EC_GROUP_new_by_curve_name(nid)) == NULL)
+                goto err;
+
+        if (!EC_KEY_set_group(ecdh, group))
+                goto err;
+        if (!EC_KEY_generate_key(ecdh))
+                goto err;
+
+        ret = 1;
+
+ err:
+        EC_GROUP_free(group);
+
+        return ret;
+}
+
+int
+ssl_kex_public_ecdhe_ecp(EC_KEY *ecdh, CBB *cbb)
+{
+        const EC_GROUP *group;
+        const EC_POINT *point;
+        uint8_t *ecp;
+        size_t ecp_len;
+        int ret = 0;
+
+        if ((group = EC_KEY_get0_group(ecdh)) == NULL)
+                goto err;
+        if ((point = EC_KEY_get0_public_key(ecdh)) == NULL)
+                goto err;
+
+        if ((ecp_len = EC_POINT_point2oct(group, point,
+            POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL)) == 0)
+                goto err;
+        if (!CBB_add_space(cbb, &ecp, ecp_len))
+                goto err;
+        if ((EC_POINT_point2oct(group, point, POINT_CONVERSION_UNCOMPRESSED,
+            ecp, ecp_len, NULL)) == 0)
+                goto err;
+
+        ret = 1;
+
+ err:
+        return ret;
+}
+
+int
+ssl_kex_peer_public_ecdhe_ecp(EC_KEY *ecdh, int nid, CBS *cbs)
+{
+        EC_GROUP *group = NULL;
+        EC_POINT *point = NULL;
+        int ret = 0;
+
+        if ((group = EC_GROUP_new_by_curve_name(nid)) == NULL)
+                goto err;
+
+        if (!EC_KEY_set_group(ecdh, group))
+                goto err;
+
+        if ((point = EC_POINT_new(group)) == NULL)
+                goto err;
+        if (EC_POINT_oct2point(group, point, CBS_data(cbs), CBS_len(cbs),
+            NULL) == 0)
+                goto err;
+        if (!EC_KEY_set_public_key(ecdh, point))
+                goto err;
+
+        ret = 1;
+
+ err:
+        EC_GROUP_free(group);
+        EC_POINT_free(point);
+
+        return ret;
+}
+
+int
+ssl_kex_derive_ecdhe_ecp(EC_KEY *ecdh, EC_KEY *ecdh_peer,
+    uint8_t **shared_key, size_t *shared_key_len)
+{
+        const EC_POINT *point;
+        uint8_t *sk = NULL;
+        int sk_len = 0;
+        int ret = 0;
+
+        if (!EC_GROUP_check(EC_KEY_get0_group(ecdh), NULL))
+                goto err;
+        if (!EC_GROUP_check(EC_KEY_get0_group(ecdh_peer), NULL))
+                goto err;
+
+        if ((point = EC_KEY_get0_public_key(ecdh_peer)) == NULL)
+                goto err;
+
+        if ((sk_len = ECDH_size(ecdh)) <= 0)
+                goto err;
+        if ((sk = calloc(1, sk_len)) == NULL)
+                goto err;
+
+        if (ECDH_compute_key(sk, sk_len, point, ecdh, NULL) <= 0)
+                goto err;
+
+        *shared_key = sk;
+        *shared_key_len = sk_len;
+        sk = NULL;
+
+        ret = 1;
+
+ err:
+        freezero(sk, sk_len);
+
+        return ret;
+}
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 6c33ec9743..5ff6f39b45 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.257 2020/01/29 17:08:49 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.258 2020/01/30 16:25:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -872,6 +872,7 @@ typedef struct ssl3_state_internal_st {
                 DH *dh;
 
                 EC_KEY *ecdh; /* holds short lived ECDH key */
+                int ecdh_nid;
 
                 uint8_t *x25519;
 
@@ -1017,6 +1018,7 @@ typedef struct sess_cert_st {
         /* Obviously we don't have the private keys of these,
          * so maybe we shouldn't even use the CERT_PKEY type here. */
 
+        int peer_nid;
         DH *peer_dh_tmp;
         EC_KEY *peer_ecdh_tmp;
         uint8_t *peer_x25519_tmp;
@@ -1278,6 +1280,12 @@ int ssl3_get_client_certificate(SSL *s);
 int ssl3_get_client_key_exchange(SSL *s);
 int ssl3_get_cert_verify(SSL *s);
 
+int ssl_kex_generate_ecdhe_ecp(EC_KEY *ecdh, int nid);
+int ssl_kex_public_ecdhe_ecp(EC_KEY *ecdh, CBB *cbb);
+int ssl_kex_peer_public_ecdhe_ecp(EC_KEY *ecdh, int nid, CBS *cbs);
+int ssl_kex_derive_ecdhe_ecp(EC_KEY *ecdh, EC_KEY *ecdh_peer,
+    uint8_t **shared_key, size_t *shared_key_len);
+
 int tls1_new(SSL *s);
 void tls1_free(SSL *s);
 void tls1_clear(SSL *s);
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 6b49afe6a8..843d2ee249 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.70 2020/01/23 10:48:37 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.71 2020/01/30 16:25:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1347,12 +1347,7 @@ ssl3_send_server_kex_dhe(SSL *s, CBB *cbb)
 static int
 ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
 {
-        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;
@@ -1371,39 +1366,20 @@ ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
                 goto err;
         }
 
-        if ((S3I(s)->tmp.ecdh = EC_KEY_new_by_curve_name(nid)) == NULL) {
+        if ((S3I(s)->tmp.ecdh = EC_KEY_new()) == NULL) {
                 al = SSL_AD_HANDSHAKE_FAILURE;
                 SSLerror(s, SSL_R_MISSING_TMP_ECDH_KEY);
                 goto f_err;
         }
+        S3I(s)->tmp.ecdh_nid = nid;
         ecdh = S3I(s)->tmp.ecdh;
 
-        if (!EC_KEY_generate_key(ecdh)) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        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);
+        if (!ssl_kex_generate_ecdhe_ecp(ecdh, nid))
                 goto err;
-        }
 
         /*
          * Encode the public key.
-         */
-        encoded_len = EC_POINT_point2oct(group, pubkey,
-            POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL);
-        if (encoded_len == 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        if ((bn_ctx = BN_CTX_new()) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
-                goto err;
-        }
-
-        /*
+         *
          * Only named curves are supported in ECDH ephemeral key exchanges.
          * In this case the ServerKeyExchange message has:
          * [1 byte CurveType], [2 byte CurveName]
@@ -1416,25 +1392,16 @@ ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
                 goto err;
         if (!CBB_add_u8_length_prefixed(cbb, &ecpoint))
                 goto err;
-        if (!CBB_add_space(&ecpoint, &data, encoded_len))
+        if (!ssl_kex_public_ecdhe_ecp(ecdh, &ecpoint))
                 goto err;
-        if (EC_POINT_point2oct(group, pubkey, POINT_CONVERSION_UNCOMPRESSED,
-            data, encoded_len, bn_ctx) == 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
         if (!CBB_flush(cbb))
                 goto err;
 
-        BN_CTX_free(bn_ctx);
-
         return (1);
 
  f_err:
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
  err:
-        BN_CTX_free(bn_ctx);
-
         return (-1);
 }
 
@@ -1861,20 +1828,13 @@ ssl3_get_client_kex_dhe(SSL *s, CBS *cbs)
 static int
 ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs)
 {
-        unsigned char *key = NULL;
-        int key_size = 0, key_len;
-        EC_POINT *point = NULL;
-        BN_CTX *bn_ctx = NULL;
-        const EC_GROUP *group;
+        uint8_t *key = NULL;
+        size_t key_len = 0;
+        EC_KEY *ecdh_peer = NULL;
         EC_KEY *ecdh;
         CBS public;
         int ret = -1;
 
-        if (!CBS_get_u8_length_prefixed(cbs, &public))
-                goto err;
-        if (CBS_len(cbs) != 0)
-                goto err;
-
         /*
          * Use the ephemeral values we saved when generating the
          * ServerKeyExchange message.
@@ -1883,54 +1843,38 @@ ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs)
                 SSLerror(s, ERR_R_INTERNAL_ERROR);
                 goto err;
         }
-        group = EC_KEY_get0_group(ecdh);
 
         /*
          * Get client's public key from encoded point in the ClientKeyExchange
          * message.
          */
-        if ((bn_ctx = BN_CTX_new()) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
-                goto err;
-        }
-        if ((point = EC_POINT_new(group)) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
+        if (!CBS_get_u8_length_prefixed(cbs, &public))
                 goto err;
-        }
-        if (EC_POINT_oct2point(group, point, CBS_data(&public),
-            CBS_len(&public), bn_ctx) == 0) {
-                SSLerror(s, ERR_R_EC_LIB);
+        if (CBS_len(cbs) != 0)
                 goto err;
-        }
 
-        /* Compute the shared pre-master secret */
-        if ((key_size = ECDH_size(ecdh)) <= 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
-                goto err;
-        }
-        if ((key = malloc(key_size)) == NULL) {
-                SSLerror(s, ERR_R_MALLOC_FAILURE);
+        if ((ecdh_peer = EC_KEY_new()) == NULL)
                 goto err;
-        }
-        if ((key_len = ECDH_compute_key(key, key_size, point, ecdh,
-            NULL)) <= 0) {
-                SSLerror(s, ERR_R_ECDH_LIB);
+
+        if (!ssl_kex_peer_public_ecdhe_ecp(ecdh_peer, S3I(s)->tmp.ecdh_nid,
+            &public))
                 goto err;
-        }
 
-        /* Compute the master secret */
+        /* Derive the shared secret and compute master secret. */
+        if (!ssl_kex_derive_ecdhe_ecp(ecdh, ecdh_peer, &key, &key_len))
+                goto err;
         s->session->master_key_length = tls1_generate_master_secret(s,
             s->session->master_key, key, key_len);
 
         EC_KEY_free(S3I(s)->tmp.ecdh);
         S3I(s)->tmp.ecdh = NULL;
+        S3I(s)->tmp.ecdh_nid = NID_undef;
 
         ret = 1;
 
  err:
-        freezero(key, key_size);
-        EC_POINT_free(point);
-        BN_CTX_free(bn_ctx);
+        freezero(key, key_len);
+        EC_KEY_free(ecdh_peer);
 
         return (ret);
 }