Send the supported_groups extension in EE where applicable

The TLSv1.3 spec says that a server SHOULD send supported_groups in the
EE message if there is a group that it prefers to the one used in the
key_share. Clients MAY act on that. At the moment we don't do anything
with it on the client side, but that may change in the future.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 764ecea31a5c1d6b714fc56e8e5385a3ce289a75..e89e97f7f1241937e0014f3bcdf4e5fdcb1339d4 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2496,6 +2496,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_CONSTRUCT_STOC_SERVER_NAME             459
 # define SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET          460
 # define SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST          461
+# define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS        544
 # define SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP                462
 # define SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO        521
 # define SSL_F_TLS_GET_MESSAGE_BODY                       351

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index a845dae42194f694c89264fa3e68617e763c7653..18a38dfe475bb502cbaee881d0badf386e3df278 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -398,6 +398,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
      "tls_construct_stoc_session_ticket"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST),
      "tls_construct_stoc_status_request"},
+    {ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS),
+     "tls_construct_stoc_supported_groups"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP),
      "tls_construct_stoc_use_srtp"},
     {ERR_FUNC(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO),

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 847ff139a4bfa7b0263280412ac50c654969dcf9..8984577d4fe6d12a819e729f72e2192925e4fbc4 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -151,7 +151,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         TLSEXT_TYPE_supported_groups,
         SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
         NULL, tls_parse_ctos_supported_groups, NULL,
-        NULL /* TODO(TLS1.3): Need to add this */,
+        tls_construct_stoc_supported_groups,
         tls_construct_ctos_supported_groups, NULL
     },
 #else

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 7ba1aac8d4a966d4e354fcdd53de3745808faeb1..b12505c1f730e1731b753fc856050cfbdfd47068 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -868,6 +868,63 @@ int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
 }
 #endif
 
+int tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
+                                        unsigned int context, X509 *x,
+                                        size_t chainidx, int *al)
+{
+    const unsigned char *groups;
+    size_t numgroups, i, first = 1;
+
+    /* s->s3->group_id is non zero if we accepted a key_share */
+    if (s->s3->group_id == 0)
+        return 1;
+
+    /* Get our list of supported groups */
+    if (!tls1_get_curvelist(s, 0, &groups, &numgroups) || numgroups == 0) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    /* Copy group ID if supported */
+    for (i = 0; i < numgroups; i++, groups += 2) {
+        if (tls_curve_allowed(s, groups, SSL_SECOP_CURVE_SUPPORTED)) {
+            if (first) {
+                /*
+                 * Check if the client is already using our preferred group. If
+                 * so we don't need to add this extension
+                 */
+                if (s->s3->group_id == GET_GROUP_ID(groups, 0))
+                    return 1;
+
+                /* Add extension header */
+                if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
+                           /* Sub-packet for supported_groups extension */
+                        || !WPACKET_start_sub_packet_u16(pkt)
+                        || !WPACKET_start_sub_packet_u16(pkt)) {
+                    SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS,
+                           ERR_R_INTERNAL_ERROR);
+                    return 0;
+                }
+
+                first = 0;
+            }
+            if (!WPACKET_put_bytes_u8(pkt, groups[0])
+                || !WPACKET_put_bytes_u8(pkt, groups[1])) {
+                    SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS,
+                           ERR_R_INTERNAL_ERROR);
+                    return 0;
+                }
+        }
+    }
+
+    if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
+}
+
 int tls_construct_stoc_session_ticket(SSL *s, WPACKET *pkt,
                                       unsigned int context, X509 *x,
                                       size_t chainidx, int *al)

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 36d55343b82aa9793a65c65cf52f1941c950e598..33206c6ea96c9e7f2bf8e5e4fe56b5d91e3f9d32 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1957,9 +1957,7 @@ int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
         return 0;
 
     for (i = 0; i < num_groups; i++, groups += 2) {
-        unsigned int share_id = (groups[0] << 8) | (groups[1]);
-
-        if (group_id == share_id
+        if (group_id == GET_GROUP_ID(groups, 0)
                 && (!checkallow
                     || tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) {
             return 1;

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 3b9311e02f5b1e7065c7c51222c947a9b95f3f24..49a5ed5cefb3ed7e33d8fc403951af8e6a0d4df0 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -55,6 +55,9 @@ int statem_flush(SSL *s);
 
 typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
 
+#define GET_GROUP_ID(group, idx) \
+    (unsigned int)(((group)[(idx) * 2] << 8) | (group)[((idx) * 2) + 1])
+
 int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
                   size_t num_groups, int checkallow);
 int create_synthetic_message_hash(SSL *s);
@@ -230,6 +233,9 @@ int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context,
 int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
                                      X509 *x, size_t chainidx, int *al);
 #endif
+int tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
+                                        unsigned int context, X509 *x,
+                                        size_t chainidx, int *al);
 int tls_construct_stoc_session_ticket(SSL *s, WPACKET *pkt,
                                       unsigned int context, X509 *x,
                                       size_t chainidx, int *al);