From 7842bf30f433328a7f69fd9c5b2d8f250b51d409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 2 Mar 2026 17:50:22 +0100 Subject: [PATCH] Fix for DTLS1.3 HRR group handling When a server uses a HRR to negotiate the key exchange group to use, the selected group is advertised in the HRR key share extension. Furthermore, this group is also stored in the Cookie that is sent to the client. When the server receives the second CH, the group used in the key share extension MUST be the one of the HRR. For stateless DTLS servers, the handling of this check had a bug. The key share group of the HRR is stored in the ssl->hrr_keyshare_group variable and is checked against the received key share of the second CH. However, in the stateless server case, another CH message may be received inbetween the two CH message of the desired client, potentially overwriting the ssl->hrr_keyshare_group variable. This then causes handshake failures when the ssl->hrr_keyshare_group variable contains another group than the second CH message of the desired client. To fix this, the following changes are conducted: 1. Disable the ssl->hrr_keyshare_group check for stateless DTLS 1.3 servers. As long as the server is stateless, CHs from multiple clients may be received that individually cause HRRs with different groups. For each of these clients, the HRR group is properly stored in the cookie. 2. When a valid cookie is received from the client, the server becomes stateful. In this case, we now parse the cookie for a stored HRR group in the RestartHandshakeHashWithCookie() method. If present, we restore the ssl->hrr_keyshare_group variable to this group to ensure the error checks succeed. 3. Move the check of ssl->hrr_keyshare_group of the the KeyShare extension parsing logic into the general TLS1.3 ClientHello parsing after extension handling. This ensures that the order of the cookie and key share extensions does not matter. A new test is added to check for this behavior. --- src/tls.c | 14 ------- src/tls13.c | 38 ++++++++++++++++- tests/api.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 15 deletions(-) diff --git a/src/tls.c b/src/tls.c index 1cad29ab2c..dc39c5ed36 100644 --- a/src/tls.c +++ b/src/tls.c @@ -10216,20 +10216,6 @@ int TLSX_KeyShare_Parse_ClientHello(const WOLFSSL* ssl, offset += ret; } - if (ssl->hrr_keyshare_group != 0) { - /* - * https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.8 - * when sending the new ClientHello, the client MUST - * replace the original "key_share" extension with one containing only a - * new KeyShareEntry for the group indicated in the selected_group field - * of the triggering HelloRetryRequest - */ - if (seenGroupsCnt != 1 || seenGroups[0] != ssl->hrr_keyshare_group) { - WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); - return BAD_KEY_SHARE_DATA; - } - } - return 0; } diff --git a/src/tls13.c b/src/tls13.c index a464ce9006..d226535af9 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6502,6 +6502,8 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie) hrrIdx += 2; c16toa(OPAQUE16_LEN, hrr + hrrIdx); hrrIdx += 2; + /* Restore the HRR key share group from the cookie. */ + ato16(cookieData + idx, &ssl->hrr_keyshare_group); hrr[hrrIdx++] = cookieData[idx++]; hrr[hrrIdx++] = cookieData[idx++]; } @@ -7015,6 +7017,29 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } #endif +#ifdef HAVE_SUPPORTED_CURVES + if (ssl->hrr_keyshare_group != 0) { + /* + * https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.8 + * when sending the new ClientHello, the client MUST + * replace the original "key_share" extension with one containing only + * a new KeyShareEntry for the group indicated in the selected_group + * field of the triggering HelloRetryRequest. + */ + TLSX* extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); + if (extension != NULL) { + KeyShareEntry* kse = (KeyShareEntry*)extension->data; + /* Exactly one KeyShareEntry with the HRR group must be present. */ + if (kse == NULL || kse->next != NULL || + kse->group != ssl->hrr_keyshare_group) { + ERROR_OUT(BAD_KEY_SHARE_DATA, exit_dch); + } + } + else + ERROR_OUT(BAD_KEY_SHARE_DATA, exit_dch); + } +#endif + #if defined(HAVE_ECH) /* hash clientHelloInner to hsHashesEch independently since it can't include * the HRR */ @@ -7509,7 +7534,18 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) if (ret != 0) return ret; - if (extMsgType == hello_retry_request) { + /* When we send a HRR, we store the selected key share group to later check + * that the client uses the same group in the second ClientHello. + * + * In case of stateless DTLS, we do not store the group, however, as it is + * already stored in the cookie that is sent to the client. We later recover + * the group from the cookie to prevent storing a state in a stateless + * server. */ + if (extMsgType == hello_retry_request +#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE) + && (!ssl->options.dtls || ssl->options.dtlsStateful) +#endif + ) { TLSX* ksExt = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); if (ksExt != NULL) { KeyShareEntry* kse = (KeyShareEntry*)ksExt->data; diff --git a/tests/api.c b/tests/api.c index 0cda4e114a..f8e5c8edf0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -24180,6 +24180,12 @@ static word32 test_wolfSSL_dtls_stateless_HashWOLFSSL(const WOLFSSL* ssl) sslCopy.keys.dtls_peer_handshake_number = 0; XMEMSET(&sslCopy.alert_history, 0, sizeof(sslCopy.alert_history)); sslCopy.hsHashes = NULL; +#if !defined(WOLFSSL_NO_CLIENT_AUTH) && \ + ((defined(WOLFSSL_SM2) && defined(WOLFSSL_SM3)) || \ + (defined(HAVE_ED25519) && !defined(NO_ED25519_CLIENT_AUTH)) || \ + (defined(HAVE_ED448) && !defined(NO_ED448_CLIENT_AUTH))) + sslCopy.options.cacheMessages = 0; +#endif #ifdef WOLFSSL_ASYNC_IO #ifdef WOLFSSL_ASYNC_CRYPT sslCopy.asyncDev = NULL; @@ -24317,11 +24323,122 @@ static int test_wolfSSL_dtls_stateless(void) return TEST_SUCCESS; } + +/* DTLS stateless API handling multiple CHs with different HRR groups */ +static int test_wolfSSL_dtls_stateless_hrr_group(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_SEND_HRR_COOKIE) + size_t i; + word32 initHash; + struct { + method_provider client_meth; + method_provider server_meth; + } params[] = { +#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_DTLS13) + { wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method }, +#endif +#if !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_DTLS) + { wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method }, +#endif + }; + for (i = 0; i < XELEM_CNT(params) && !EXPECT_FAIL(); i++) { + WOLFSSL_CTX *ctx_s = NULL, *ctx_c = NULL; + WOLFSSL *ssl_s = NULL, *ssl_c = NULL, *ssl_c2 = NULL; + struct test_memio_ctx test_ctx; + int groups_1[] = { + WOLFSSL_ECC_SECP256R1, + WOLFSSL_ECC_SECP384R1, + WOLFSSL_ECC_SECP521R1 + }; + int groups_2[] = { + WOLFSSL_ECC_SECP384R1, + WOLFSSL_ECC_SECP521R1 + }; + char hrrBuf[1000]; + int hrrSz = sizeof(hrrBuf); + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + params[i].client_meth, params[i].server_meth), 0); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c2, NULL, + params[i].client_meth, params[i].server_meth), 0); + + + wolfSSL_SetLoggingPrefix("server"); + wolfSSL_dtls_set_using_nonblock(ssl_s, 1); + + initHash = test_wolfSSL_dtls_stateless_HashWOLFSSL(ssl_s); + + /* Set groups and disable key shares. This ensures that only the given + * groups are in the SupportedGroups extension and that an empty key + * share extension is sent in the initial ClientHello of each session. + * This triggers the server to send a HelloRetryRequest with the first + * group in the SupportedGroups extension selected. */ + wolfSSL_SetLoggingPrefix("client1"); + ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups_1, 3), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c), WOLFSSL_SUCCESS); + + wolfSSL_SetLoggingPrefix("client2"); + ExpectIntEQ(wolfSSL_set_groups(ssl_c2, groups_2, 2), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c2), WOLFSSL_SUCCESS); + + /* Start handshake, send first ClientHello */ + wolfSSL_SetLoggingPrefix("client1"); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + + /* Read first ClientHello, send HRR with WOLFSSL_ECC_SECP256R1 */ + wolfSSL_SetLoggingPrefix("server"); + ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0); + ExpectIntEQ(test_memio_copy_message(&test_ctx, 1, hrrBuf, &hrrSz, 0), 0); + ExpectIntGT(hrrSz, 0); + ExpectIntEQ(initHash, test_wolfSSL_dtls_stateless_HashWOLFSSL(ssl_s)); + test_memio_clear_buffer(&test_ctx, 1); + + /* Send second ClientHello */ + wolfSSL_SetLoggingPrefix("client2"); + ExpectIntEQ(wolfSSL_connect(ssl_c2), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c2, -1), WOLFSSL_ERROR_WANT_READ); + + /* Read second ClientHello, send HRR now with WOLFSSL_ECC_SECP384R1 */ + wolfSSL_SetLoggingPrefix("server"); + ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0); + ExpectIntEQ(initHash, test_wolfSSL_dtls_stateless_HashWOLFSSL(ssl_s)); + test_memio_clear_buffer(&test_ctx, 1); + + /* Complete first handshake with WOLFSSL_ECC_SECP256R1 */ + wolfSSL_SetLoggingPrefix("client1"); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, hrrBuf, hrrSz), 0); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + + wolfSSL_SetLoggingPrefix("server"); + ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), WOLFSSL_SUCCESS); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + wolfSSL_free(ssl_s); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_c2); + wolfSSL_CTX_free(ctx_s); + wolfSSL_CTX_free(ctx_c); + } +#endif /* WOLFSSL_SEND_HRR_COOKIE */ + return EXPECT_RESULT(); +} #else static int test_wolfSSL_dtls_stateless(void) { return TEST_SKIPPED; } + +static int test_wolfSSL_dtls_stateless_hrr_group(void) +{ + return TEST_SKIPPED; +} #endif /* WOLFSSL_DTLS13 && WOLFSSL_SEND_HRR_COOKIE && * HAVE_IO_TESTS_DEPENDENCIES && !SINGLE_THREADED */ @@ -33186,6 +33303,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_dtls_bad_record), /* Uses Assert in handshake callback. */ TEST_DECL(test_wolfSSL_dtls_stateless), + TEST_DECL(test_wolfSSL_dtls_stateless_hrr_group), TEST_DECL(test_generate_cookie), #ifndef NO_BIO