From 60a6627f5352b4025f2b30fe2112c00416a259ae Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Tue, 17 Feb 2026 18:37:06 +1100 Subject: [PATCH] Fix group tuple handling in DEFAULT expansion Also fine-tune docs and add tests. Fixes: #30109 Fixes: CVE-2026-2673 Reviewed-by: Matt Caswell Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz MergeDate: Fri Mar 13 12:42:50 2026 (Merged from https://github.com/openssl/openssl/pull/30111) --- CHANGES.md | 12 +++ NEWS.md | 7 ++ doc/man3/SSL_CTX_set1_curves.pod | 123 +++++++++++++++++++++---------- ssl/t1_lib.c | 89 ++++++++++++---------- test/tls13groupselection_test.c | 37 ++++++++-- 5 files changed, 187 insertions(+), 81 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e5a0bed138..4579a0a6dc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -29,6 +29,17 @@ OpenSSL Releases OpenSSL 3.6 ----------- +### Changes between 3.6.1 and 3.6.2 [xx XXX xxxx] + + * Fixed loss of key agreement group tuple structure when the `DEFAULT` keyword + is used in the server-side configuration of the key-agreement group list. + This could result in accepting a less preferred than intended client + keyshare. + + ([CVE-2026-2673]) + + *Viktor Dukhovni* + ### Changes between 3.6.0 and 3.6.1 [27 Jan 2026] * Fixed Improper validation of PBMAC1 parameters in PKCS#12 MAC verification. @@ -21850,6 +21861,7 @@ ndif +[CVE-2026-2673]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-2673 [CVE-2026-22796]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-22796 [CVE-2026-22795]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-22795 [CVE-2025-69421]: https://www.openssl.org/news/vulnerabilities.html#CVE-2025-69421 diff --git a/NEWS.md b/NEWS.md index 2a382f39ad..28f233060d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -24,6 +24,12 @@ OpenSSL Releases OpenSSL 3.6 ----------- +### Major changes between OpenSSL 3.6.1 and OpenSSL 3.6.2 [under development] + + * Fixed loss of key agreement group tuple structure when the `DEFAULT` keyword + is used in the server-side configuration of the key-agreement group list. + ([CVE-2026-2673]) + ### Major changes between OpenSSL 3.6.0 and OpenSSL 3.6.1 [27 Jan 2026] OpenSSL 3.6.1 is a security patch release. The most severe CVE fixed in this @@ -2028,6 +2034,7 @@ OpenSSL 0.9.x * Support for various new platforms +[CVE-2026-2673]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-2673 [CVE-2026-22796]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-22796 [CVE-2026-22795]: https://www.openssl.org/news/vulnerabilities.html#CVE-2026-22795 [CVE-2025-69421]: https://www.openssl.org/news/vulnerabilities.html#CVE-2025-69421 diff --git a/doc/man3/SSL_CTX_set1_curves.pod b/doc/man3/SSL_CTX_set1_curves.pod index 352fe34347..bcf792023c 100755 --- a/doc/man3/SSL_CTX_set1_curves.pod +++ b/doc/man3/SSL_CTX_set1_curves.pod @@ -40,13 +40,13 @@ SSL_get1_curves, SSL_get_shared_curve, SSL_CTX_get0_implemented_groups For all of the functions below that set the supported groups there must be at 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. +unique integer B value. However, support for some groups may be added by +external providers. In this case there will be no B 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()). 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. +groups in the array B. The array consist of all B of supported groups. The supported groups for B include: B, B, @@ -73,20 +73,27 @@ B is set, the order of the elements in the array determines the selected group. Otherwise, the order is ignored and the client's order determines the selection. -For a TLS 1.3 server, the groups determine the selected group, but -selection is more complex. A TLS 1.3 client sends both a group list as well as a -predicted subset of groups. Choosing a group outside the predicted subset incurs -an extra roundtrip. However, in some situations, the most preferred group may -not be predicted. OpenSSL considers all supported groups in I to be comparable -in security and prioritizes avoiding roundtrips above either client or server -preference order. If an application uses an external provider to extend OpenSSL -with, e.g., a post-quantum algorithm, this behavior may allow a network attacker -to downgrade connections to a weaker algorithm. It is therefore recommended -to use SSL_CTX_set1_groups_list() with the ability to specify group tuples. +For a TLS 1.3 server, the groups determine the selected group, but selection is +more complex. +A TLS 1.3 client sends both a group list and predicted keyshares for a subset +of groups. +A server choosing a group outside the client's predicted subset incurs an extra +roundtrip. +However, in some situations, the most preferred group may not be predicted. + +When groups are specified via SSL_CTX_set1_groups() as a list of B +values, OpenSSL considers all supported groups in I to be comparable in +security and prioritises avoiding roundtrips above either client or server +preference order. +If an application uses an external provider to extend OpenSSL with, e.g., a +post-quantum algorithm, this behavior may allow a network attacker to downgrade +connections to a weaker algorithm. +It is therefore recommended to use SSL_CTX_set1_groups_list() instead, making +it possible to specify group tuples as described below. SSL_CTX_set1_groups_list() sets the supported groups for B to string I. In contrast to SSL_CTX_set1_groups(), the names of the -groups, rather than their NIDs, are used. +groups, rather than their B, are used. The commands below list the available groups for TLS 1.2 and TLS 1.3, respectively: @@ -102,30 +109,72 @@ The preferred group names are those defined by L. The I can be used to define several group tuples of comparable security -levels, and can specify which key shares should be sent by a client. -The specified list elements can optionally be ignored, if not implemented +levels, and can specify which predicted key shares should be sent by a client. +Group tuples are used by OpenSSL TLS servers to decide whether to request a +stronger keyshare than those predicted by sending a Hello Retry Request +(B) even if some of the predicted groups are supported. +OpenSSL clients ignore tuple boundaries, and pay attenion only to the overall +order of I elements and which groups are selected as predicted keyshares +as described below. + +The specified list elements can optionally be ignored if not implemented (listing unknown groups otherwise results in error). -It is also possible to specify the built-in default set of groups, and to explicitly -remove a group from that list. - -In its simplest form, the string I is just a colon separated list -of group names, for example "P-521:P-384:P-256:X25519:ffdhe2048". The first -group listed will also be used for the B sent by a client in a -TLSv1.3 B. For servers note the discussion above. The list should -be in order of preference with the most preferred group first. - -Group tuples of comparable security are defined by separating them from each -other by a tuple separator C. Keyshares to be sent by a client are specified -by prepending a C<*> to the group name, while any C<*> will be ignored by a -server. The following string I for example defines three tuples when -used on the server-side, and triggers the generation of three key shares -when used on the client-side: P-521:*P-256/*P-384/*X25519:P-384:ffdhe2048. - -If a group name is preceded with the C character, it will be ignored if an -implementation is missing. If a group name is preceded with the C<-> character, it -will be removed from the list of groups if present (including not sending a -key share for this group), ignored otherwise. The pseudo group name -C can be used to select the OpenSSL built-in default list of groups. +It is also possible to specify the built-in default set of groups, and to +explicitly remove a group from that list. + +In its simplest legacy form, the string I is just a colon separated list +of group names, for example "P-521:P-384:P-256:X25519:ffdhe2048". +The first group listed will in this case be used as the sole predicted +B sent by a client in a TLSv1.3 B. +The list should be in order of preference with the most preferred group first. + +A more expressive syntax supports definition of group tuples of comparable +security by separating them from each other with C characters. + +The predicted keyshares to be sent by clients can be explicitly specified by +adding a C<*> prefix to the associated group name. +These C<*> prefixes are ignored by servers. + +If a group name is prefixed with the C character, it will be ignored if an +implementation is missing. +Otherwise, listing an unknown group name will cause a failure to parse the +I. +Note that whether a group is known or not may depend on the OpenSSL version, +how OpenSSL was compiled and/or which providers are loaded. +Make sure you have the correct spelling of the group name and when in doubt +prefix it with a C to handle configurations in which it might nevertheless +be unknown. + +If a group name is prefixed with the C<-> character, it will be removed from +the list of groups specified up to that point. +It can be added again if specified later. +Removal of groups that have not been included earlier in the list is silently +ignored. + +The pseudo group name C can be used to select the OpenSSL built-in +default list of groups. +Prepending one or more groups to C using only C<:> separators prepends those +groups to the built-in default list's first tuple. +Additional tuples can be prepended by use of the C separator. +Appending a set of groups to C using only C<:> separators appends those +groups to the built-in default list's last tuple. +Additional tuples can be appended by use of the C separator. + +The B list selects B as one of the predicted keyshares. +In rare cases this can lead to failures or timeouts because the resulting +larger TLS Client Hello message may no longer fit in a single TCP segment and +firewall software may erroneously disrupt the TLS handshake. +If this is an issue or concern, prepending C without a C<*> +prefix leads to its occurrence in the default list to be ignored as a duplicate, +and along with that also the keyshare prediction. +The group will then only be selected by servers that specifically expect it, +after a Hello Retry Request (HRR). +Servers that specifically prefer B, are much less likely to be +found behind problematic firewalls. + +The following string I for example defines three tuples when used on the +server-side, and triggers the generation of three key shares when used on the +client-side: P-521:*P-256/*P-384/*X25519:P-384:ffdhe2048. For a TLS 1.3 client, all the groups in the string I are added to the supported groups extension of a C, in the order in which they are listed, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 9ece318950..9adbabe5ee 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -215,7 +215,7 @@ static const uint16_t suiteb_curves[] = { /* Group list string of the built-in pseudo group DEFAULT_SUITE_B */ #define SUITE_B_GROUP_NAME "DEFAULT_SUITE_B" -#define SUITE_B_GROUP_LIST "secp256r1:secp384r1", +#define SUITE_B_GROUP_LIST "?secp256r1:?secp384r1", struct provider_ctx_data_st { SSL_CTX *ctx; @@ -1248,8 +1248,8 @@ typedef struct { size_t ksidcnt; /* Number of key shares */ uint16_t *ksid_arr; /* The IDs of the key share groups (flat list) */ /* Variable to keep state between execution of callback or helper functions */ - size_t tuple_mode; /* Keeps track whether tuple_cb called from 'the top' or from gid_cb */ - int ignore_unknown_default; /* Flag such that unknown groups for DEFAULT[_XYZ] are ignored */ + int inner; /* Are we expanding a DEFAULT list */ + int first; /* First tuple of possibly nested expansion? */ } gid_cb_st; /* Forward declaration of tuple callback function */ @@ -1324,16 +1324,16 @@ static int gid_cb(const char *elem, int len, void *arg) for (i = 0; i < OSSL_NELEM(default_group_strings); i++) { if ((size_t)len == (strlen(default_group_strings[i].list_name)) && OPENSSL_strncasecmp(default_group_strings[i].list_name, elem, len) == 0) { + int saved_first; + /* * We're asked to insert an entire list of groups from a * DEFAULT[_XYZ] 'pseudo group' which we do by * recursively calling this function (indirectly via * CONF_parse_list and tuple_cb); essentially, we treat a DEFAULT * group string like a tuple which is appended to the current tuple - * rather then starting a new tuple. Variable tuple_mode is the flag which - * controls append tuple vs start new tuple. + * rather then starting a new tuple. */ - if (ignore_unknown || remove_group) return -1; /* removal or ignore not allowed here -> syntax error */ @@ -1354,15 +1354,17 @@ static int gid_cb(const char *elem, int len, void *arg) default_group_strings[i].group_string, strlen(default_group_strings[i].group_string)); restored_default_group_string[strlen(default_group_strings[i].group_string) + restored_prefix_index] = '\0'; - /* We execute the recursive call */ - garg->ignore_unknown_default = 1; /* We ignore unknown groups for DEFAULT_XYZ */ - /* we enforce group mode (= append tuple) for DEFAULT_XYZ group lists */ - garg->tuple_mode = 0; - /* We use the tuple_cb callback to process the pseudo group tuple */ + /* + * Append first tuple of result to current tuple, and don't + * terminate the last tuple until we return to a top-level + * tuple_cb. + */ + saved_first = garg->first; + garg->inner = garg->first = 1; retval = CONF_parse_list(restored_default_group_string, TUPLE_DELIMITER_CHARACTER, 1, tuple_cb, garg); - garg->tuple_mode = 1; /* next call to tuple_cb will again start new tuple */ - garg->ignore_unknown_default = 0; /* reset to original value */ + garg->inner = 0; + garg->first = saved_first; /* We don't need the \0-terminated string anymore */ OPENSSL_free(restored_default_group_string); @@ -1382,9 +1384,6 @@ static int gid_cb(const char *elem, int len, void *arg) if (len == 0) return -1; /* Seems we have prefxes without a group name -> syntax error */ - if (garg->ignore_unknown_default == 1) /* Always ignore unknown groups for DEFAULT[_XYZ] */ - ignore_unknown = 1; - /* Memory management in case more groups are present compared to initial allocation */ if (garg->gidcnt == garg->gidmax) { uint16_t *tmp = OPENSSL_realloc_array(garg->gid_arr, @@ -1520,7 +1519,7 @@ static int gid_cb(const char *elem, int len, void *arg) /* and update the book keeping for the number of groups in current tuple */ garg->tuplcnt_arr[garg->tplcnt]++; - /* We memorize if needed that we want to add a key share for the current group */ + /* We want to add a key share for the current group */ if (add_keyshare) garg->ksid_arr[garg->ksidcnt++] = gid; } @@ -1529,6 +1528,35 @@ done: return retval; } +static int grow_tuples(gid_cb_st *garg) +{ + if (garg->tplcnt == garg->tplmax) { + size_t newcnt = garg->tplmax + GROUPLIST_INCREMENT; + size_t *tmp = OPENSSL_realloc_array(garg->tuplcnt_arr, + newcnt, sizeof(*garg->tuplcnt_arr)); + + if (tmp == NULL) + return 0; + + garg->tplmax = newcnt; + garg->tuplcnt_arr = tmp; + } + return 1; +} + +static int close_tuple(gid_cb_st *garg) +{ + size_t gidcnt = garg->tuplcnt_arr[garg->tplcnt]; + + if (gidcnt == 0) + return 1; + if (!grow_tuples(garg)) + return 0; + + garg->tuplcnt_arr[++garg->tplcnt] = 0; + return 1; +} + /* Extract and process a tuple of groups */ static int tuple_cb(const char *tuple, int len, void *arg) { @@ -1542,17 +1570,9 @@ static int tuple_cb(const char *tuple, int len, void *arg) return 0; } - /* Memory management for tuples */ - if (garg->tplcnt == garg->tplmax) { - size_t *tmp = OPENSSL_realloc_array(garg->tuplcnt_arr, - garg->tplmax + GROUPLIST_INCREMENT, - sizeof(*garg->tuplcnt_arr)); - - if (tmp == NULL) - return 0; - garg->tplmax += GROUPLIST_INCREMENT; - garg->tuplcnt_arr = tmp; - } + if (garg->inner && !garg->first && !close_tuple(garg)) + return 0; + garg->first = 0; /* Convert to \0-terminated string */ restored_tuple_string = OPENSSL_malloc(len + 1 /* \0 */); @@ -1567,15 +1587,8 @@ static int tuple_cb(const char *tuple, int len, void *arg) /* We don't need the \o-terminated string anymore */ OPENSSL_free(restored_tuple_string); - if (garg->tuplcnt_arr[garg->tplcnt] > 0) { /* Some valid groups are present in current tuple... */ - if (garg->tuple_mode) { - /* We 'close' the tuple */ - garg->tplcnt++; - garg->tuplcnt_arr[garg->tplcnt] = 0; /* Next tuple is initialized to be empty */ - garg->tuple_mode = 1; /* next call will start a tuple (unless overridden in gid_cb) */ - } - } - + if (!garg->inner && !close_tuple(garg)) + return 0; return retval; } @@ -1606,8 +1619,6 @@ int tls1_set_groups_list(SSL_CTX *ctx, } memset(&gcb, 0, sizeof(gcb)); - gcb.tuple_mode = 1; /* We prepare to collect the first tuple */ - gcb.ignore_unknown_default = 0; gcb.gidmax = GROUPLIST_INCREMENT; gcb.tplmax = GROUPLIST_INCREMENT; gcb.ksidmax = GROUPLIST_INCREMENT; diff --git a/test/tls13groupselection_test.c b/test/tls13groupselection_test.c index 54e094464f..98c81cde07 100644 --- a/test/tls13groupselection_test.c +++ b/test/tls13groupselection_test.c @@ -40,6 +40,12 @@ typedef enum SERVER_RESPONSE { SH = 2 } SERVER_RESPONSE; +static const char *response_desc[] = { + "HRR", + "INIT", + "SH", +}; + static char *cert = NULL; static char *privkey = NULL; @@ -307,7 +313,23 @@ static const struct tls13groupselection_test_st tls13groupselection_tests[] = { { "*brainpoolP256r1:X25519", /* test 43 */ "X25519", SERVER_PREFERENCE, - NEGOTIATION_FAILURE, INIT } + NEGOTIATION_FAILURE, INIT }, + + /* DEFAULT retains tuple structure */ + { "*X25519:secp256r1", + "secp256r1:DEFAULT", /* test 44 */ + SERVER_PREFERENCE, + "secp256r1", HRR }, +#ifndef OPENSSL_NO_DH + { "*ffdhe2048:secp256r1", + "DEFAULT:ffdhe4096", /* test 45 */ + CLIENT_PREFERENCE, + "secp256r1", HRR }, + { "x25519:ffdhe2048:*ffdhe4096", + "DEFAULT:ffdhe4096", /* test 46 */ + SERVER_PREFERENCE, + "x25519", HRR }, +#endif }; static void server_response_check_cb(int write_p, int version, @@ -318,10 +340,12 @@ static void server_response_check_cb(int write_p, int version, enum SERVER_RESPONSE *server_response = (enum SERVER_RESPONSE *)arg; /* Prepare check for HRR */ const uint8_t *incoming_random = (uint8_t *)buf + 6; - const uint8_t magic_HRR_random[32] = { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, + const uint8_t magic_HRR_random[32] = { + 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, - 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C }; + 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C + }; /* Did a server hello arrive? */ if (write_p == 0 && /* Incoming data... */ @@ -450,13 +474,16 @@ static int test_groupnegotiation(const struct tls13groupselection_test_st *curre group_name_client = SSL_group_to_name(clientssl, negotiated_group_client); if (!TEST_int_eq(negotiated_group_client, negotiated_group_server)) goto end; - if (!TEST_int_eq((int)current_test_vector->expected_server_response, (int)server_response)) + if (!TEST_str_eq(response_desc[current_test_vector->expected_server_response], + response_desc[server_response])) goto end; if (TEST_str_eq(group_name_client, current_test_vector->expected_group)) ok = 1; } else { TEST_false_or_end(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)); - if (test_type == TEST_NEGOTIATION_FAILURE && !TEST_int_eq((int)current_test_vector->expected_server_response, (int)server_response)) + if (test_type == TEST_NEGOTIATION_FAILURE + && !TEST_str_eq(response_desc[current_test_vector->expected_server_response], + response_desc[server_response])) goto end; ok = 1; } -- 2.53.0