Support DEFAULT keyword and '-' prefix in SSL_CTX_set1_groups_list()

Fixes #25790

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25813)

CHANGES.md

@@ -30,6 +30,13 @@ OpenSSL 3.5
 
 ### Changes between 3.4 and 3.5 [xx XXX xxxx]
 
+* Support DEFAULT keyword and '-' prefix in SSL_CTX_set1_groups_list().
+  SSL_CTX_set1_groups_list() now supports the DEFAULT keyword which sets the
+  available groups to the default selection. The '-' prefix allows the calling
+  application to remove a group from the selection.
+
+  *Frederik Wedel-Heinen*
+
  * Updated the default encryption cipher for the `req`, `cms`, and `smime` applications
    from `des-ede3-cbc` to `aes-256-cbc`.
 

doc/man3/SSL_CTX_set1_curves.pod

@@ -40,7 +40,7 @@ least one group in the list. A number of these functions identify groups via a
 unique integer NID value. However, support for some groups may be added by
 external providers. In this case there will be no NID assigned for the group.
 When setting such groups applications should use the "list" form of these
-functions (i.e. SSL_CTX_set1_groups_list() and SSL_set1_groups_list).
+functions (i.e. SSL_CTX_set1_groups_list() and SSL_set1_groups_list()).
 
 SSL_CTX_set1_groups() sets the supported groups for B<ctx> to B<glistlen>
 groups in the array B<glist>. The array consist of all NIDs of supported groups.
@@ -83,7 +83,9 @@ B<brainpoolP256r1tls13>, B<brainpoolP384r1tls13>, B<brainpoolP512r1tls13>,
 B<ffdhe2048>, B<ffdhe3072>, B<ffdhe4096>, B<ffdhe6144> and B<ffdhe8192>. Support
 for other groups may be added by external providers, however note the discussion
 on TLS 1.3 selection criteria above. If a group name is preceded with the C<?>
-character, it will be ignored if an implementation is missing.
+character, it will be ignored if an implementation is missing. The "DEFAULT"
+group name is used to select the default selection of groups. Group names that
+are preceded with the C<-> character will be removed from the selected groups.
 
 SSL_set1_groups() and SSL_set1_groups_list() are similar except they set
 supported groups for the SSL structure B<ssl>.

ssl/t1_lib.c

@@ -1046,24 +1046,72 @@ static int gid_cb(const char *elem, int len, void *arg)
     uint16_t gid = 0;
     char etmp[GROUP_NAME_BUFFER_LENGTH];
     int ignore_unknown = 0;
+    int remove_group = 0;
+    int found_group = 0;
+    int add_default_groups = 0;
+    size_t groups_to_add = 0;
 
     if (elem == NULL)
         return 0;
-    if (elem[0] == '?') {
-        ignore_unknown = 1;
-        ++elem;
-        --len;
+
+    while (((elem[0] == '-' && !remove_group) || (elem[0] == '?' && !ignore_unknown))
+           && len > 0) {
+        if (elem[0] == '-') {
+            remove_group = 1;
+            ++elem;
+            --len;
+        }
+        if (elem[0] == '?') {
+            ignore_unknown = 1;
+            ++elem;
+            --len;
+        }
     }
-    if (garg->gidcnt == garg->gidmax) {
+
+    if (len == strlen("DEFAULT") && OPENSSL_strncasecmp("DEFAULT", elem, len) == 0)
+        add_default_groups = 1;
+
+    if (add_default_groups)
+        groups_to_add = garg->ctx->ext.supported_groups_default_len;
+    else if (!remove_group)
+        groups_to_add = 1;
+
+    if (groups_to_add > garg->gidmax - garg->gidcnt) {
+        size_t list_increment = groups_to_add > GROUPLIST_INCREMENT ? groups_to_add
+                                                                    : GROUPLIST_INCREMENT;
         uint16_t *tmp =
             OPENSSL_realloc(garg->gid_arr,
-                            (garg->gidmax + GROUPLIST_INCREMENT) * sizeof(*garg->gid_arr));
+                            (garg->gidmax + list_increment) * sizeof(*garg->gid_arr));
+
         if (tmp == NULL)
             return 0;
-        garg->gidmax += GROUPLIST_INCREMENT;
+
+        garg->gidmax += list_increment;
         garg->gid_arr = tmp;
     }
-    if (len > (int)(sizeof(etmp) - 1))
+
+    if (add_default_groups) {
+        size_t j;
+
+        for (j = 0; j < garg->ctx->ext.supported_groups_default_len; j++) {
+            gid = garg->ctx->ext.supported_groups_default[j];
+            found_group = 0;
+
+            for (i = 0; i < garg->gidcnt; i++) {
+                if (garg->gid_arr[i] == gid) {
+                    found_group = 1;
+                    break;
+                }
+            }
+
+            if (!found_group)
+                garg->gid_arr[garg->gidcnt++] = gid;
+        }
+
+        return 1;
+    }
+
+    if (len > (int) (sizeof(etmp) - 1))
         return 0;
     memcpy(etmp, elem, len);
     etmp[len] = 0;
@@ -1073,12 +1121,25 @@ static int gid_cb(const char *elem, int len, void *arg)
         /* Unknown group - ignore, if ignore_unknown */
         return ignore_unknown;
     }
+
     for (i = 0; i < garg->gidcnt; i++)
         if (garg->gid_arr[i] == gid) {
-            /* Duplicate group - ignore */
-            return 1;
+            found_group = 1;
+            break;
         }
-    garg->gid_arr[garg->gidcnt++] = gid;
+
+    if (found_group && remove_group) {
+        size_t j;
+
+        for (j = i + 1; j < garg->gidcnt; j++)
+            garg->gid_arr[j - 1] = garg->gid_arr[j];
+
+        garg->gidcnt--;
+    }
+
+    if (!found_group && !remove_group)
+        garg->gid_arr[garg->gidcnt++] = gid;
+
     return 1;
 }
 

test/sslapitest.c

@@ -9772,6 +9772,52 @@ static int test_unknown_sigalgs_groups(void)
     return ret;
 }
 
+static int test_configuration_of_groups(void)
+{
+    int ret = 0;
+    SSL_CTX *ctx = NULL;
+#if (!defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH))
+    size_t default_groups_len;
+#endif
+
+    if (!TEST_ptr(ctx = SSL_CTX_new_ex(libctx, NULL, TLS_server_method())))
+        goto end;
+
+#if (!defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH))
+    default_groups_len = ctx->ext.supported_groups_default_len;
+
+    if (!TEST_size_t_gt(default_groups_len, 0)
+        || !TEST_int_gt(SSL_CTX_set1_groups_list(ctx, "DEFAULT"), 0)
+        || !TEST_size_t_eq(ctx->ext.supportedgroups_len, default_groups_len))
+        goto end;
+#endif
+
+#if (!defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH))
+    if (!TEST_int_gt(SSL_CTX_set1_groups_list(ctx, "DEFAULT:-?P-256"), 0)
+# if !defined(OPENSSL_NO_EC)
+        || !TEST_size_t_eq(ctx->ext.supportedgroups_len, default_groups_len - 1)
+# else
+        || !TEST_size_t_eq(ctx->ext.supportedgroups_len, default_groups_len)
+# endif
+        )
+        goto end;
+#endif
+
+#if !defined(OPENSSL_NO_EC)
+    if (!TEST_int_gt(SSL_CTX_set1_groups_list(ctx, "?P-256:?P-521:-?P-256"), 0)
+        || !TEST_size_t_eq(ctx->ext.supportedgroups_len, 1)
+        || !TEST_int_eq(ctx->ext.supportedgroups[0], OSSL_TLS_GROUP_ID_secp521r1)
+        )
+        goto end;
+#endif
+
+    ret = 1;
+
+end:
+    SSL_CTX_free(ctx);
+    return ret;
+}
+
 #if !defined(OPENSSL_NO_EC) \
     && (!defined(OSSL_NO_USABLE_TLS1_3) || !defined(OPENSSL_NO_TLS1_2))
 /*
@@ -12627,6 +12673,7 @@ int setup_tests(void)
 #endif
     ADD_ALL_TESTS(test_servername, 10);
     ADD_TEST(test_unknown_sigalgs_groups);
+    ADD_TEST(test_configuration_of_groups);
 #if !defined(OPENSSL_NO_EC) \
     && (!defined(OSSL_NO_USABLE_TLS1_3) || !defined(OPENSSL_NO_TLS1_2))
     ADD_ALL_TESTS(test_sigalgs_available, 6);