Handle TLSv1.3 key shares other than X25519 on the server side.

Previously we would only select an X25519 key share from the client,
ignoring any others. Change this so that we will select the first of the
key shares that matches one of our supported groups.

ok beck@ inoguchi@ tb@

2 files changed, 34 insertions, 16 deletions

diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index 4cb7c5bf90..a0e2f7320b 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.62 2020/02/18 16:12:14 tb Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.63 2020/04/21 17:06:16 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1288,13 +1288,27 @@ tlsext_keyshare_server_parse(SSL *s, CBS *cbs, int *alert)
                         return 0;
 
                 /*
-                 * XXX support other groups later.
-                 * XXX enforce group can only appear once.
+                 * XXX - check key exchange against supported groups from client.
+                 * XXX - check that groups only appear once.
                  */
-                if (S3I(s)->hs_tls13.key_share == NULL ||
-                    tls13_key_share_group(S3I(s)->hs_tls13.key_share) != group)
+
+                /*
+                 * Ignore this client share if we're using earlier than TLSv1.3
+                 * or we've already selected a key share.
+                 */
+                if (S3I(s)->hs_tls13.max_version < TLS1_3_VERSION)
+                        continue;
+                if (S3I(s)->hs_tls13.key_share != NULL)
                         continue;
 
+                /* XXX - consider implementing server preference. */
+                if (!tls1_check_curve(s, group))
+                        continue;
+
+                /* Decode and store the selected key share. */
+                S3I(s)->hs_tls13.key_share = tls13_key_share_new(group);
+                if (S3I(s)->hs_tls13.key_share == NULL)
+                        goto err;
                 if (!tls13_key_share_peer_public(S3I(s)->hs_tls13.key_share,
                     group, &key_exchange))
                         goto err;
diff --git a/src/lib/libssl/tls13_server.c b/src/lib/libssl/tls13_server.c
index 1aebf5840c..f3d21a7477 100644
--- a/src/lib/libssl/tls13_server.c
+++ b/src/lib/libssl/tls13_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.29 2020/04/17 17:16:53 jsing Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.30 2020/04/21 17:06:16 jsing Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -49,11 +49,6 @@ tls13_server_init(struct tls13_ctx *ctx)
         if ((s->session = SSL_SESSION_new()) == NULL)
                 return 0;
 
-        if ((ctx->hs->key_share = tls13_key_share_new_nid(NID_X25519)) == NULL)
-                return 0;
-        if (!tls13_key_share_generate(ctx->hs->key_share))
-                return 0;
-
         arc4random_buf(s->s3->server_random, SSL3_RANDOM_SIZE);
 
         return 1;
@@ -284,6 +279,14 @@ tls13_client_hello_recv(struct tls13_ctx *ctx, CBS *cbs)
         if (s->method->internal->version < TLS1_3_VERSION)
                 return 1;
 
+        /*
+         * If no matching key share was provided, we need to send a
+         * HelloRetryRequest, if matching security parameters exist.
+         */
+        if (ctx->hs->key_share == NULL)
+                ctx->handshake_stage.hs_type |= WITH_HRR;
+
+        /* XXX - check this is the correct point */
         tls13_record_layer_allow_ccs(ctx->rl, 1);
 
         return 1;
@@ -524,6 +527,12 @@ err:
 int
 tls13_server_hello_send(struct tls13_ctx *ctx, CBB *cbb)
 {
+        if (ctx->hs->key_share == NULL)
+                return 0;
+
+        if (!tls13_key_share_generate(ctx->hs->key_share))
+                return 0;
+
         if (!tls13_server_hello_build(ctx, cbb))
                 return 0;
 
@@ -542,11 +551,6 @@ tls13_server_hello_sent(struct tls13_ctx *ctx)
         SSL *s = ctx->ssl;
         int ret = 0;
 
-        /* XXX - handle other key share types. */
-        if (ctx->hs->key_share == NULL) {
-                /* XXX - alert. */
-                goto err;
-        }
         if (!tls13_key_share_derive(ctx->hs->key_share,
             &shared_key, &shared_key_len))
                 goto err;