From c14b1a050478cdc317c1eab57083671be0576d7b Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 24 Oct 2025 14:56:13 +0200 Subject: [PATCH] Validate cipher suite after HelloRetryRequest - Add validation to ensure the cipher suite in the ServerHello matches the one specified in the HelloRetryRequest. - test_TLSX_CA_NAMES_bad_extension: use the same ciphersuite in HRR and SH --- src/tls13.c | 12 ++++++ tests/api.c | 11 +++-- tests/api/test_tls13.c | 93 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 4 +- wolfssl/internal.h | 4 ++ 5 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index 5f4c8697a..7ee0a018d 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -5253,6 +5253,18 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* Set the cipher suite from the message. */ ssl->options.cipherSuite0 = input[args->idx++]; ssl->options.cipherSuite = input[args->idx++]; + if (*extMsgType == hello_retry_request) { + ssl->options.hrrCipherSuite0 = ssl->options.cipherSuite0; + ssl->options.hrrCipherSuite = ssl->options.cipherSuite; + } + else if (ssl->msgsReceived.got_hello_retry_request && + (ssl->options.hrrCipherSuite0 != ssl->options.cipherSuite0 || + ssl->options.hrrCipherSuite != ssl->options.cipherSuite)) { + WOLFSSL_MSG("Received ServerHello with different cipher suite than " + "HelloRetryRequest"); + WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); + return INVALID_PARAMETER; + } #ifdef WOLFSSL_DEBUG_TLS WOLFSSL_MSG("Chosen cipher suite:"); WOLFSSL_MSG(GetCipherNameInternal(ssl->options.cipherSuite0, diff --git a/tests/api.c b/tests/api.c index 02a8a3d8d..f0090c183 100644 --- a/tests/api.c +++ b/tests/api.c @@ -48134,9 +48134,8 @@ static int test_TLSX_CA_NAMES_bad_extension(void) EXPECT_DECLS; #if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) && \ - defined(OPENSSL_EXTRA) && defined(WOLFSSL_SHA384) && \ - defined(HAVE_NULL_CIPHER) && defined(HAVE_CHACHA) && \ - defined(HAVE_POLY1305) + defined(OPENSSL_EXTRA) && defined(BUILD_TLS_CHACHA20_POLY1305_SHA256) && \ + defined(HAVE_ECC) && !defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT) /* This test should only fail (with BUFFER_ERROR) when we actually try to * parse the CA Names extension. Otherwise it will return other non-related * errors. If CA Names will be parsed in more configurations, that should @@ -48144,7 +48143,7 @@ static int test_TLSX_CA_NAMES_bad_extension(void) WOLFSSL *ssl_c = NULL; WOLFSSL_CTX *ctx_c = NULL; struct test_memio_ctx test_ctx; - /* HRR + SH using TLS_DHE_PSK_WITH_NULL_SHA384 */ + /* HRR + SH using TLS_CHACHA20_POLY1305_SHA256 */ const byte shBadCaNamesExt[] = { 0x16, 0x03, 0x04, 0x00, 0x3f, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0xcf, 0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02, 0x1e, @@ -48155,7 +48154,7 @@ static int test_TLSX_CA_NAMES_bad_extension(void) 0x5c, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x03, 0xcf, 0x21, 0xad, 0x74, 0x00, 0x00, 0x83, 0x3f, 0x3b, 0x80, 0x01, 0xac, 0x65, 0x8c, 0x19, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x02, 0x00, 0x9e, 0x09, 0x1c, 0xe8, - 0xa8, 0x09, 0x9c, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00, + 0xa8, 0x09, 0x9c, 0x00, 0x13, 0x03, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00, 0x03, 0x3f, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x13, 0x05, 0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x00, 0x09, 0x00, 0x00, 0x0d, 0x00, 0x00, 0x11, 0x00, 0x00, 0x0d, 0x00, 0x2f, 0x00, 0x01, 0xff, @@ -48171,7 +48170,7 @@ static int test_TLSX_CA_NAMES_bad_extension(void) 0x5e, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x7f, 0xd0, 0x2d, 0xea, 0x6e, 0x53, 0xa1, 0x6a, 0xc9, 0xc8, 0x54, 0xef, 0x75, 0xe4, 0xd9, 0xc6, 0x3e, 0x74, 0xcb, 0x30, 0x80, 0xcc, 0x83, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0xc0, 0x5a, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00, + 0x00, 0xc0, 0x5a, 0x00, 0x13, 0x03, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00, 0x03, 0x03, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x53, 0x25, 0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x02, 0x05, 0x00, 0x00, 0x0d, 0x00, 0x00, 0x11, 0x00, 0x00, 0x0d, 0x00, 0x2f, 0x00, 0x06, 0x00, diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index e0aa9d734..ab4398dda 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2224,3 +2224,96 @@ int test_tls13_same_ch(void) #endif return EXPECT_RESULT(); } + +int test_tls13_hrr_different_cs(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_TLS13) && \ + defined(BUILD_TLS_AES_256_GCM_SHA384) && \ + defined(BUILD_TLS_CHACHA20_POLY1305_SHA256) && \ + defined(HAVE_ECC) && defined(HAVE_ECC384) + /* + * TLSv1.3 Record Layer: Handshake Protocol: Hello Retry Request + * Content Type: Handshake (22) + * Version: TLS 1.2 (0x0303) + * Length: 56 + * Handshake Protocol: Hello Retry Request + * Handshake Type: Server Hello (2) + * Length: 52 + * Version: TLS 1.2 (0x0303) + * Random: cf21ad74e59a6111be1d8c021e65b891c2a211167abb8c5e079e09e2c8a8339c (HelloRetryRequest magic) + * Session ID Length: 0 + * Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302) + * Compression Method: null (0) + * Extensions Length: 12 + * Extension: supported_versions (len=2) TLS 1.3 + * Extension: key_share (len=2) secp384r1 + * + */ + unsigned char hrr[] = { + 0x16, 0x03, 0x03, 0x00, 0x38, 0x02, 0x00, 0x00, 0x34, 0x03, 0x03, 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, 0x00, 0x13, 0x02, 0x00, 0x00, + 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00, + 0x18 + }; + /* + * TLSv1.3 Record Layer: Handshake Protocol: Server Hello + * Content Type: Handshake (22) + * Version: TLS 1.2 (0x0303) + * Length: 155 + * Handshake Protocol: Server Hello + * Handshake Type: Server Hello (2) + * Length: 151 + * Version: TLS 1.2 (0x0303) + * Random: 0101010101010101010101010101010101010101010101010101010101010101 + * Session ID Length: 0 + * Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303) + * Compression Method: null (0) + * Extensions Length: 111 + * Extension: key_share (len=101) secp384r1 + * Extension: supported_versions (len=2) TLS 1.3 + * + */ + unsigned char sh[] = { + 0x16, 0x03, 0x03, 0x00, 0x9b, 0x02, 0x00, 0x00, 0x97, 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, 0x00, 0x13, 0x03, 0x00, 0x00, + 0x6f, 0x00, 0x33, 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, 0x02, 0x03, 0x04 + }; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL *ssl_c = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL, + wolfTLSv1_3_client_method, NULL), 0); + + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (char*)hrr, + sizeof(hrr)), 0); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (char*)sh, + sizeof(sh)), 0); + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), INVALID_PARAMETER); + + wolfSSL_free(ssl_c); + wolfSSL_CTX_free(ctx_c); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index fb46c404d..42a19073b 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -31,6 +31,7 @@ int test_tls13_rpk_handshake(void); 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); #define TEST_TLS13_DECLS \ TEST_DECL_GROUP("tls13", test_tls13_apis), \ @@ -39,6 +40,7 @@ int test_tls13_same_ch(void); TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ - TEST_DECL_GROUP("tls13", test_tls13_same_ch) + TEST_DECL_GROUP("tls13", test_tls13_same_ch), \ + TEST_DECL_GROUP("tls13", test_tls13_hrr_different_cs) #endif /* WOLFCRYPT_TEST_TLS13_H */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index e88cc6e74..f19154063 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5100,6 +5100,10 @@ struct Options { byte processReply; /* nonblocking resume */ byte cipherSuite0; /* first byte, normally 0 */ byte cipherSuite; /* second byte, actual suite */ +#ifdef WOLFSSL_TLS13 + byte hrrCipherSuite0; /* first byte, normally 0 */ + byte hrrCipherSuite; /* second byte, actual suite */ +#endif byte hashAlgo; /* selected hash algorithm */ byte sigAlgo; /* selected sig algorithm */ byte peerHashAlgo; /* peer's chosen hash algo */