From 406f5033c60775e69a8aff26868b7daef1d5f2db Mon Sep 17 00:00:00 2001 From: sebastian-carpenter Date: Fri, 20 Mar 2026 12:05:10 -0600 Subject: [PATCH] verify ciphersuite in CH2 matches HRR --- src/tls13.c | 26 ++++++++- tests/api/test_tls13.c | 123 +++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 2 + 3 files changed, 149 insertions(+), 2 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index c4864394838..3192771ade7 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6635,8 +6635,10 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie) hrrIdx += ssl->session->sessionIDSz; } - /* Cipher Suite */ + /* Restore the cipher suite from the cookie. */ + ssl->options.hrrCipherSuite0 = cookieData[idx]; hrr[hrrIdx++] = cookieData[idx++]; + ssl->options.hrrCipherSuite = cookieData[idx]; hrr[hrrIdx++] = cookieData[idx++]; /* Compression not supported in TLS v1.3. */ @@ -7350,6 +7352,21 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } #endif + /* Verify the cipher suite is the same as what was chosen in HRR. + * got_client_hello == 2 covers the stateful path. + * cookieGood covers the stateless DTLS path. */ + if ((ssl->msgsReceived.got_client_hello == 2 +#ifdef WOLFSSL_SEND_HRR_COOKIE + || ssl->options.cookieGood +#endif + ) && + (ssl->options.cipherSuite0 != ssl->options.hrrCipherSuite0 || + ssl->options.cipherSuite != ssl->options.hrrCipherSuite)) { + WOLFSSL_MSG("Cipher suite in second ClientHello does not match " + "HelloRetryRequest"); + ERROR_OUT(INVALID_PARAMETER, exit_dch); + } + /* Advance state and proceed */ ssl->options.asyncState = TLS_ASYNC_VERIFY; } /* case TLS_ASYNC_BUILD || TLS_ASYNC_DO */ @@ -7657,7 +7674,9 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) * 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. */ + * server. + * + * Similar logic holds for the hrrCipherSuite. */ if (extMsgType == hello_retry_request #if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE) && (!ssl->options.dtls || ssl->options.dtlsStateful) @@ -7669,6 +7688,9 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) if (kse != NULL) ssl->hrr_keyshare_group = kse->group; } + + ssl->options.hrrCipherSuite0 = ssl->options.cipherSuite0; + ssl->options.hrrCipherSuite = ssl->options.cipherSuite; } #ifdef WOLFSSL_SEND_HRR_COOKIE diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 4650fa0946d..34ddcc94c81 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2542,6 +2542,129 @@ int test_tls13_hrr_different_cs(void) return EXPECT_RESULT(); } +/* Server-side complement to test_tls13_hrr_different_cs: the client sends a + * different cipher suite in CH2 than what the server selected in the HRR. */ +int test_tls13_ch2_different_cs(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER) && \ + defined(BUILD_TLS_AES_256_GCM_SHA384) && \ + defined(BUILD_TLS_AES_128_GCM_SHA256) && \ + defined(HAVE_ECC) && defined(HAVE_ECC384) + /* + * First ClientHello: cipher suite TLS_AES_256_GCM_SHA384 (0x1302), + * empty key_share, secp384r1 in supported_groups. This triggers the + * server to send a HelloRetryRequest selecting TLS_AES_256_GCM_SHA384 + * and requesting a secp384r1 key share. + */ + /* + * TLSv1.3 Record Layer: Handshake Protocol: Client Hello + * Content Type: Handshake (22) + * Version: TLS 1.2 (0x0303) + * Length: 110 + * Handshake Protocol: Client Hello + * Handshake Type: Client Hello (1) + * Length: 106 + * Version: TLS 1.2 (0x0303) + * Random: 0101010101010101010101010101010101010101010101010101010101010101 + * Session ID Length: 32 + * Session ID: 0303030303030303030303030303030303030303030303030303030303030303 + * Cipher Suites Length: 2 + * Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302) + * Compression Methods Length: 1 + * Compression Method: null (0) + * Extensions Length: 31 + * Extension: supported_groups (len=4) secp384r1 (0x0018) + * Extension: signature_algorithms (len=6) rsa_pkcs1_sha256 (0x0401), + * rsa_pss_rsae_sha256 (0x0804) + * Extension: key_share (len=2) client_shares length=0 (empty) + * Extension: supported_versions (len=3) TLS 1.3 (0x0304) + */ + unsigned char ch1[] = { + 0x16, 0x03, 0x03, 0x00, 0x6e, 0x01, 0x00, 0x00, 0x6a, 0x03, 0x03, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x20, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x00, 0x02, 0x13, 0x02, 0x01, 0x00, 0x00, 0x1f, + 0x00, 0x0a, 0x00, 0x04, 0x00, 0x02, 0x00, 0x18, 0x00, 0x0d, 0x00, 0x06, + 0x00, 0x04, 0x04, 0x01, 0x08, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00, 0x00, + 0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04 + }; + /* + * TLSv1.3 Record Layer: Handshake Protocol: Client Hello + * Content Type: Handshake (22) + * Version: TLS 1.2 (0x0303) + * Length: 211 + * Handshake Protocol: Client Hello + * Handshake Type: Client Hello (1) + * Length: 207 + * Version: TLS 1.2 (0x0303) + * Random: 0101010101010101010101010101010101010101010101010101010101010101 + * Session ID Length: 32 + * Session ID: 0303030303030303030303030303030303030303030303030303030303030303 + * Cipher Suites Length: 2 + * Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301) + * Compression Methods Length: 1 + * Compression Method: null (0) + * Extensions Length: 132 + * Extension: supported_groups (len=4) secp384r1 (0x0018) + * Extension: signature_algorithms (len=6) rsa_pkcs1_sha256 (0x0401), + * rsa_pss_rsae_sha256 (0x0804) + * Extension: key_share (len=103) + * client_shares length: 101 + * KeyShareEntry: group secp384r1 (0x0018), key_exchange length: 97 + * key_exchange: 04 || X(48) || Y(48) (uncompressed P-384 point) + * Extension: supported_versions (len=3) TLS 1.3 (0x0304) + */ + unsigned char ch2[] = { + 0x16, 0x03, 0x03, 0x00, 0xd3, 0x01, 0x00, 0x00, 0xcf, 0x03, 0x03, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x20, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, + 0x03, 0x03, 0x03, 0x03, 0x00, 0x02, 0x13, 0x01, 0x01, 0x00, 0x00, 0x84, + 0x00, 0x0a, 0x00, 0x04, 0x00, 0x02, 0x00, 0x18, 0x00, 0x0d, 0x00, 0x06, + 0x00, 0x04, 0x04, 0x01, 0x08, 0x04, 0x00, 0x33, 0x00, 0x67, 0x00, 0x65, + 0x00, 0x18, 0x00, 0x61, 0x04, 0x53, 0x3e, 0xe5, 0xbf, 0x40, 0xec, 0x2d, + 0x67, 0x98, 0x8b, 0x77, 0xf3, 0x17, 0x48, 0x9b, 0xb6, 0xdf, 0x95, 0x29, + 0x25, 0xc7, 0x09, 0xfc, 0x03, 0x81, 0x11, 0x1a, 0x59, 0x56, 0xf2, 0xd7, + 0x58, 0x11, 0x0e, 0x59, 0xd3, 0xd7, 0xc1, 0x72, 0x9e, 0x2c, 0x0d, 0x70, + 0xea, 0xf7, 0x73, 0xe6, 0x12, 0x01, 0x16, 0x42, 0x6d, 0xe2, 0x43, 0x6a, + 0x2f, 0x5f, 0xdd, 0x7f, 0xe5, 0x4f, 0xaf, 0x95, 0x2b, 0x04, 0xfd, 0x13, + 0xf5, 0x16, 0xce, 0x62, 0x7f, 0x89, 0xd2, 0x01, 0x9d, 0x4c, 0x87, 0x96, + 0x95, 0x9e, 0x43, 0x33, 0xc7, 0x06, 0x5b, 0x49, 0x6c, 0xa6, 0x34, 0xd5, + 0xdc, 0x63, 0xbd, 0xe9, 0x1f, 0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04 + }; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s, + NULL, wolfTLSv1_3_server_method), 0); + + /* Server reads CH1, sends HRR, then waits for CH2 */ + ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, (char*)ch1, + sizeof(ch1)), 0); + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + + /* Server must reject CH2 because the cipher suite changed from the HRR */ + ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, (char*)ch2, + sizeof(ch2)), 0); + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), INVALID_PARAMETER); + + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + #if defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER) && \ defined(HAVE_ECC) /* Called when writing. */ diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index a67860108c0..d8b95a41906 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -32,6 +32,7 @@ int test_tls13_pq_groups(void); int test_tls13_early_data(void); int test_tls13_same_ch(void); int test_tls13_hrr_different_cs(void); +int test_tls13_ch2_different_cs(void); int test_tls13_sg_missing(void); int test_tls13_ks_missing(void); int test_tls13_duplicate_extension(void); @@ -51,6 +52,7 @@ int test_tls13_derive_keys_no_key(void); TEST_DECL_GROUP("tls13", test_tls13_early_data), \ TEST_DECL_GROUP("tls13", test_tls13_same_ch), \ TEST_DECL_GROUP("tls13", test_tls13_hrr_different_cs), \ + TEST_DECL_GROUP("tls13", test_tls13_ch2_different_cs), \ TEST_DECL_GROUP("tls13", test_tls13_sg_missing), \ TEST_DECL_GROUP("tls13", test_tls13_ks_missing), \ TEST_DECL_GROUP("tls13", test_tls13_duplicate_extension), \