Partial revert and reimplement "Enable brainpool curves for TLS1.3"

This partially reverts commit 0a10825a0 in order to reimplement it in a
simpler way in the next commit. The reverted aspects are all related to
the TLSv1.3 brainpool curves in the supported_groups extension. Rather
than special casing the handling of these curves we simply add new entries
to the groups table to represent them. They can then be handled without
any additional special casing. This makes the code simpler to maintain.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/19315)

7 files changed, 6 insertions, 82 deletions

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 0896be0232..df9ceb1383 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3632,11 +3632,8 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
                 int *cptr = parg;
 
                 for (i = 0; i < clistlen; i++) {
-                    uint16_t cid = SSL_CONNECTION_IS_TLS13(sc)
-                                   ? ssl_group_id_tls13_to_internal(clist[i])
-                                   : clist[i];
                     const TLS_GROUP_INFO *cinf
-                        = tls1_group_id_lookup(s->ctx, cid);
+                        = tls1_group_id_lookup(s->ctx, clist[i]);
 
                     if (cinf != NULL)
                         cptr[i] = tls1_group_id2nid(cinf->group_id, 1);
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 2416def0b0..a01cc8cc71 100644
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2736,8 +2736,6 @@ __owur int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL_CONNECTION *s);
 
 SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n);
 
-__owur uint16_t ssl_group_id_internal_to_tls13(uint16_t curve_id);
-__owur uint16_t ssl_group_id_tls13_to_internal(uint16_t curve_id);
 __owur const TLS_GROUP_INFO *tls1_group_id_lookup(SSL_CTX *ctx, uint16_t curve_id);
 __owur int tls1_group_id2nid(uint16_t group_id, int include_unknown);
 __owur uint16_t tls1_nid2group_id(int nid);
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 6dc21ad42f..47ad5110ab 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1409,7 +1409,7 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent)
                     group_id = pgroups[i];
 
                     if (check_in_list(s, group_id, clntgroups, clnt_num_groups,
-                                      2))
+                                      1))
                         break;
                 }
 
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 19f6561b17..18bcba036f 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -226,21 +226,6 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL_CONNECTION *s, WPACKET *pkt,
 
         if (tls_valid_group(s, ctmp, min_version, max_version, 0, &okfortls13)
                 && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
-#ifndef OPENSSL_NO_TLS1_3
-            int ctmp13 = ssl_group_id_internal_to_tls13(ctmp);
-
-            if (ctmp13 != 0 && ctmp13 != ctmp
-                    && max_version == TLS1_3_VERSION) {
-                if (!WPACKET_put_bytes_u16(pkt, ctmp13)) {
-                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                    return EXT_RETURN_FAIL;
-                }
-                tls13added++;
-                added++;
-                if (min_version == TLS1_3_VERSION)
-                    continue;
-            }
-#endif
             if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
                 SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                 return EXT_RETURN_FAIL;
@@ -646,7 +631,7 @@ static int add_key_share(SSL_CONNECTION *s, WPACKET *pkt, unsigned int curve_id)
     }
 
     /* Create KeyShareEntry */
-    if (!WPACKET_put_bytes_u16(pkt, ssl_group_id_internal_to_tls13(curve_id))
+    if (!WPACKET_put_bytes_u16(pkt, curve_id)
             || !WPACKET_sub_memcpy_u16(pkt, encoded_point, encodedlen)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         goto err;
@@ -699,9 +684,6 @@ EXT_RETURN tls_construct_ctos_key_share(SSL_CONNECTION *s, WPACKET *pkt,
         curve_id = s->s3.group_id;
     } else {
         for (i = 0; i < num_groups; i++) {
-            if (ssl_group_id_internal_to_tls13(pgroups[i]) == 0)
-                continue;
-
             if (!tls_group_allowed(s, pgroups[i], SSL_SECOP_CURVE_SUPPORTED))
                 continue;
 
@@ -1799,7 +1781,6 @@ int tls_parse_stoc_key_share(SSL_CONNECTION *s, PACKET *pkt,
         return 0;
     }
 
-    group_id = ssl_group_id_tls13_to_internal(group_id);
     if ((context & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) {
         const uint16_t *pgroups = NULL;
         size_t i, num_groups;
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 4f7321fd20..6a488a8737 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -642,7 +642,7 @@ int tls_parse_ctos_key_share(SSL_CONNECTION *s, PACKET *pkt,
          * we requested, and must be the only key_share sent.
          */
         if (s->s3.group_id != 0
-                && (ssl_group_id_tls13_to_internal(group_id) != s->s3.group_id
+                && (group_id != s->s3.group_id
                     || PACKET_remaining(&key_share_list) != 0)) {
             SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_KEY_SHARE);
             return 0;
@@ -664,8 +664,6 @@ int tls_parse_ctos_key_share(SSL_CONNECTION *s, PACKET *pkt,
         /* Cache the selected group ID in the SSL_SESSION */
         s->session->kex_group = group_id;
 
-        group_id = ssl_group_id_tls13_to_internal(group_id);
-
         if ((s->s3.peer_tmp = ssl_generate_param_group(s, group_id)) == NULL) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                    SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
@@ -1612,8 +1610,7 @@ EXT_RETURN tls_construct_stoc_key_share(SSL_CONNECTION *s, WPACKET *pkt,
         }
         if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
                 || !WPACKET_start_sub_packet_u16(pkt)
-                || !WPACKET_put_bytes_u16(pkt, ssl_group_id_internal_to_tls13(
-                                          s->s3.group_id))
+                || !WPACKET_put_bytes_u16(pkt, s->s3.group_id)
                 || !WPACKET_close(pkt)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             return EXT_RETURN_FAIL;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 07939ee960..8b34c11048 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -2222,15 +2222,9 @@ int check_in_list(SSL_CONNECTION *s, uint16_t group_id, const uint16_t *groups,
     if (groups == NULL || num_groups == 0)
         return 0;
 
-    if (checkallow == 1)
-        group_id = ssl_group_id_tls13_to_internal(group_id);
-
     for (i = 0; i < num_groups; i++) {
         uint16_t group = groups[i];
 
-        if (checkallow == 2)
-            group = ssl_group_id_tls13_to_internal(group);
-
         if (group_id == group
                 && (!checkallow
                     || tls_group_allowed(s, group, SSL_SECOP_CURVE_CHECK))) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index ab539513bf..dcd7b294a0 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -435,42 +435,6 @@ static uint16_t tls1_group_name2id(SSL_CTX *ctx, const char *name)
     return 0;
 }
 
-uint16_t ssl_group_id_internal_to_tls13(uint16_t curve_id)
-{
-    switch(curve_id) {
-    case OSSL_TLS_GROUP_ID_brainpoolP256r1:
-        return OSSL_TLS_GROUP_ID_brainpoolP256r1_tls13;
-    case OSSL_TLS_GROUP_ID_brainpoolP384r1:
-        return OSSL_TLS_GROUP_ID_brainpoolP384r1_tls13;
-    case OSSL_TLS_GROUP_ID_brainpoolP512r1:
-        return OSSL_TLS_GROUP_ID_brainpoolP512r1_tls13;
-    case OSSL_TLS_GROUP_ID_brainpoolP256r1_tls13:
-    case OSSL_TLS_GROUP_ID_brainpoolP384r1_tls13:
-    case OSSL_TLS_GROUP_ID_brainpoolP512r1_tls13:
-        return 0;
-    default:
-        return curve_id;
-    }
-}
-
-uint16_t ssl_group_id_tls13_to_internal(uint16_t curve_id)
-{
-    switch(curve_id) {
-    case OSSL_TLS_GROUP_ID_brainpoolP256r1:
-    case OSSL_TLS_GROUP_ID_brainpoolP384r1:
-    case OSSL_TLS_GROUP_ID_brainpoolP512r1:
-        return 0;
-    case OSSL_TLS_GROUP_ID_brainpoolP256r1_tls13:
-        return OSSL_TLS_GROUP_ID_brainpoolP256r1;
-    case OSSL_TLS_GROUP_ID_brainpoolP384r1_tls13:
-        return OSSL_TLS_GROUP_ID_brainpoolP384r1;
-    case OSSL_TLS_GROUP_ID_brainpoolP512r1_tls13:
-        return OSSL_TLS_GROUP_ID_brainpoolP512r1;
-    default:
-        return curve_id;
-    }
-}
-
 const TLS_GROUP_INFO *tls1_group_id_lookup(SSL_CTX *ctx, uint16_t group_id)
 {
     size_t i;
@@ -677,15 +641,8 @@ uint16_t tls1_shared_group(SSL_CONNECTION *s, int nmatch)
 
     for (k = 0, i = 0; i < num_pref; i++) {
         uint16_t id = pref[i];
-        uint16_t cid = id;
 
-        if (SSL_CONNECTION_IS_TLS13(s)) {
-            if (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE)
-                cid = ssl_group_id_internal_to_tls13(id);
-            else
-                cid = id = ssl_group_id_tls13_to_internal(id);
-        }
-        if (!tls1_in_list(cid, supp, num_supp)
+        if (!tls1_in_list(id, supp, num_supp)
                 || !tls_group_allowed(s, id, SSL_SECOP_CURVE_SHARED))
             continue;
         if (nmatch == k)