1 files changed, 77 insertions, 8 deletions

diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index e4ba549814..14cf6fce84 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/lib/libssl/ssl_tlsext.c
@@ -1,8 +1,8 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.144 2024/03/27 10:44:17 beck Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.145 2024/03/27 22:27:09 beck Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
- * Copyright (c) 2018-2019 Bob Beck <beck@openbsd.org>
+ * Copyright (c) 2018-2019, 2024 Bob Beck <beck@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
@@ -1464,14 +1464,65 @@ tlsext_keyshare_client_build(SSL *s, uint16_t msg_type, CBB *cbb)
 static int
 tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 {
-        CBS client_shares, key_exchange;
+        const uint16_t *client_groups = NULL, *server_groups = NULL;
+        size_t client_groups_len = 0, server_groups_len = 0;
+        size_t i, j, client_groups_index;
+        int preferred_group_found = 0;
         int decode_error;
-        uint16_t group;
+        uint16_t group, client_preferred_group;
+        CBS client_shares, key_exchange;
+
+        /*
+         * RFC 8446 section 4.2.8:
+         *
+         * Each KeyShareEntry value MUST correspond to a group offered in the
+         * "supported_groups" extension and MUST appear in the same order.
+         * However, the values MAY be a non-contiguous subset of the
+         * "supported_groups".
+         */
+
+        if (!tlsext_extension_seen(s, TLSEXT_TYPE_supported_groups)) {
+                *alert = SSL_AD_ILLEGAL_PARAMETER;
+                return 0;
+        }
+        if (!tlsext_extension_processed(s, TLSEXT_TYPE_supported_groups)) {
+                *alert = SSL_AD_INTERNAL_ERROR;
+                return 0;
+        }
+
+        /*
+         * XXX similar to tls1_get_supported_group, but client pref
+         * only - consider deduping later.
+         */
+        /*
+         * We are now assured of at least one client group.
+         * Get the client and server group preference orders.
+         */
+        tls1_get_group_list(s, 0, &server_groups, &server_groups_len);
+        tls1_get_group_list(s, 1, &client_groups, &client_groups_len);
+
+        /*
+         * Find the group that is most preferred by the client that
+         * we also support.
+         */
+        for (i = 0; i < client_groups_len && !preferred_group_found; i++) {
+                if (!ssl_security_supported_group(s, client_groups[i]))
+                        continue;
+                for (j = 0; j < server_groups_len; j++) {
+                        if (server_groups[j] == client_groups[i]) {
+                                client_preferred_group = client_groups[i];
+                                preferred_group_found = 1;
+                                break;
+                        }
+                }
+        }
 
         if (!CBS_get_u16_length_prefixed(cbs, &client_shares))
                 return 0;
 
+        client_groups_index = 0;
         while (CBS_len(&client_shares) > 0) {
+                int client_sent_group;
 
                 /* Unpack client share. */
                 if (!CBS_get_u16(&client_shares, &group))
@@ -1480,9 +1531,21 @@ tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
                         return 0;
 
                 /*
-                 * XXX - check key exchange against supported groups from client.
-                 * XXX - check that groups only appear once.
+                 * Ensure the client share group was sent in supported groups,
+                 * and was sent in the same order as supported groups. The
+                 * supported groups has already been checked for duplicates.
                  */
+                client_sent_group = 0;
+                while (client_groups_index < client_groups_len) {
+                        if (group == client_groups[client_groups_index++]) {
+                                client_sent_group = 1;
+                                break;
+                        }
+                }
+                if (!client_sent_group) {
+                        *alert = SSL_AD_ILLEGAL_PARAMETER;
+                        return 0;
+                }
 
                 /*
                  * Ignore this client share if we're using earlier than TLSv1.3
@@ -1493,8 +1556,14 @@ tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
                 if (s->s3->hs.key_share != NULL)
                         continue;
 
-                /* XXX - consider implementing server preference. */
-                if (!tls1_check_group(s, group))
+                /*
+                 * Ignore this client share if it is not for the most client
+                 * preferred supported group. This avoids a potential downgrade
+                 * situation where the client sends a client share for something
+                 * less preferred, and we choose to to use it instead of
+                 * requesting the more preferred group.
+                 */
+                if (!preferred_group_found || group != client_preferred_group)
                         continue;
 
                 /* Decode and store the selected key share. */