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)

diff --git a/CHANGES.md b/CHANGES.md
index 77e8fe3b04ed19f68f01a8dafacc20872461458c..1fa8a26212559c91249298be1e157143058da488 100644 (file)
--- a/CHANGES.md
+++ b/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`.
 

diff --git a/doc/man3/SSL_CTX_set1_curves.pod b/doc/man3/SSL_CTX_set1_curves.pod
index 1ebabfb4815f2907673af43471167c8f9a6455b8..578c3c0e5eb7a7732bb7c40aeb637e0d15a2bbe7 100644 (file)
--- a/doc/man3/SSL_CTX_set1_curves.pod
+++ b/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>.

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 4e4671d01302a0667fb716a4db2d761b1c0f0768..e6afc7199b59902a795480d6669c1a8d5eb08244 100644 (file)
--- a/ssl/t1_lib.c
+++ b/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;
 }
 

diff --git a/test/sslapitest.c b/test/sslapitest.c
index c7730d9bdb5478db40536daecb5f18ada72f72ea..ff151390fe2d256e00762a54518bfd0b1588967f 100644 (file)
--- a/test/sslapitest.c
+++ b/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);