From 357e27342e9bbe8d45e8be079a11588e7905fc55 Mon Sep 17 00:00:00 2001 From: Frederik Wedel-Heinen Date: Mon, 28 Oct 2024 08:32:42 +0100 Subject: [PATCH] Support DEFAULT keyword and '-' prefix in SSL_CTX_set1_groups_list() Fixes #25790 Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/25813) --- CHANGES.md | 7 +++ doc/man3/SSL_CTX_set1_curves.pod | 6 ++- ssl/t1_lib.c | 83 +++++++++++++++++++++++++++----- test/sslapitest.c | 47 ++++++++++++++++++ 4 files changed, 130 insertions(+), 13 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 77e8fe3b04..1fa8a26212 100644 --- 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 1ebabfb481..578c3c0e5e 100644 --- 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 to B groups in the array B. The array consist of all NIDs of supported groups. @@ -83,7 +83,9 @@ B, B, B, B, B, B, B and B. 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. diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 4e4671d013..e6afc7199b 100644 --- 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 c7730d9bdb..ff151390fe 100644 --- 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);