Correctly unpack client key shares.

Even if we're not processing/using the peer public key from the key share,
we still need to unpack it in order to parse the TLS extension correctly.
Resolves issues with TLSv1.3 clients talking to TLSv1.2 server.

ok tb@

2 files changed, 11 insertions, 14 deletions

diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index 46f30aa47e..58ba11954d 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.58 2020/01/30 17:09:23 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.59 2020/02/01 12:41:58 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1274,7 +1274,7 @@ tlsext_keyshare_client_build(SSL *s, CBB *cbb)
 int
 tlsext_keyshare_server_parse(SSL *s, CBS *cbs, int *alert)
 {
-        CBS client_shares;
+        CBS client_shares, key_exchange;
         uint16_t group;
 
         if (!CBS_get_u16_length_prefixed(cbs, &client_shares))
@@ -1285,6 +1285,8 @@ tlsext_keyshare_server_parse(SSL *s, CBS *cbs, int *alert)
                 /* Unpack client share. */
                 if (!CBS_get_u16(&client_shares, &group))
                         goto err;
+                if (!CBS_get_u16_length_prefixed(&client_shares, &key_exchange))
+                        return 0;
 
                 /*
                  * XXX support other groups later.
@@ -1295,7 +1297,7 @@ tlsext_keyshare_server_parse(SSL *s, CBS *cbs, int *alert)
                         continue;
 
                 if (!tls13_key_share_peer_public(S3I(s)->hs_tls13.key_share,
-                    group, &client_shares))
+                    group, &key_exchange))
                         goto err;
         }
 
@@ -1330,16 +1332,19 @@ tlsext_keyshare_server_build(SSL *s, CBB *cbb)
 int
 tlsext_keyshare_client_parse(SSL *s, CBS *cbs, int *alert)
 {
+        CBS key_exchange;
         uint16_t group;
 
         /* Unpack server share. */
         if (!CBS_get_u16(cbs, &group))
                 goto err;
+        if (!CBS_get_u16_length_prefixed(cbs, &key_exchange))
+                return 0;
 
         /* XXX - Handle other groups and verify that they're valid. */
 
         if (!tls13_key_share_peer_public(S3I(s)->hs_tls13.key_share,
-            group, cbs))
+            group, &key_exchange))
                 goto err;
 
         return 1;
diff --git a/src/lib/libssl/tls13_key_share.c b/src/lib/libssl/tls13_key_share.c
index 9a83b9f9f7..3fe38ecc37 100644
--- a/src/lib/libssl/tls13_key_share.c
+++ b/src/lib/libssl/tls13_key_share.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_key_share.c,v 1.1 2020/01/30 17:09:23 jsing Exp $ */
+/* $OpenBSD: tls13_key_share.c,v 1.2 2020/02/01 12:41:58 jsing Exp $ */
 /*
  * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
  *
@@ -161,22 +161,14 @@ int
 tls13_key_share_peer_public(struct tls13_key_share *ks, uint16_t group,
     CBS *cbs)
 {
-        CBS key_exchange;
-
         if (ks->group_id != group)
                 return 0;
 
-        if (!CBS_get_u16_length_prefixed(cbs, &key_exchange))
-                return 0;
-
         if (ks->nid == NID_X25519) {
-                if (!tls13_key_share_peer_public_x25519(ks, &key_exchange))
+                if (!tls13_key_share_peer_public_x25519(ks, cbs))
                         return 0;
         }
 
-        if (CBS_len(cbs) != 0)
-                return 0;
-
         return 1;
 }