From f798a585d9dc57f7c42a90e693d8f0aa8a241e52 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 25 Sep 2025 16:56:51 +0200 Subject: [PATCH] Abort connection if we are about to send the same CH --- src/ssl.c | 6 +++++ src/tls.c | 6 ++++- src/tls13.c | 15 +++++++++++++ tests/api/test_tls13.c | 50 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls13.h | 4 +++- wolfssl/internal.h | 6 +++++ 6 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 71cbdd17c..fafa64ff1 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -14149,6 +14149,12 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, ssl->options.haveSessionId = 0; ssl->options.tls = 0; ssl->options.tls1_1 = 0; + #ifdef WOLFSSL_TLS13 + #ifdef WOLFSSL_SEND_HRR_COOKIE + ssl->options.hrrSentCookie = 0; + #endif + ssl->options.hrrSentKeyShare = 0; + #endif #ifdef WOLFSSL_DTLS ssl->options.dtlsStateful = 0; #endif diff --git a/src/tls.c b/src/tls.c index 7ce76aacc..6d63cf618 100644 --- a/src/tls.c +++ b/src/tls.c @@ -7318,9 +7318,11 @@ static int TLSX_Cookie_Parse(WOLFSSL* ssl, const byte* input, word16 length, if (length - idx != len) return BUFFER_E; - if (msgType == hello_retry_request) + if (msgType == hello_retry_request) { + ssl->options.hrrSentCookie = 1; return TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 1, &ssl->extensions); + } /* client_hello */ extension = TLSX_Find(ssl->extensions, TLSX_COOKIE); @@ -10173,6 +10175,8 @@ int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length, if (length != OPAQUE16_LEN) return BUFFER_ERROR; + ssl->options.hrrSentKeyShare = 1; + /* The data is the named group the server wants to use. */ ato16(input, &group); diff --git a/src/tls13.c b/src/tls13.c index 149ed574f..19b7aced0 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -5635,6 +5635,21 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } else { + /* https://datatracker.ietf.org/doc/html/rfc8446#section-4.1.4 + * Clients MUST abort the handshake with an + * "illegal_parameter" alert if the HelloRetryRequest would not result + * in any change in the ClientHello. + */ + /* Check if the HRR contained a cookie or a keyshare */ + if (!ssl->options.hrrSentKeyShare +#ifdef WOLFSSL_SEND_HRR_COOKIE + && !ssl->options.hrrSentCookie +#endif + ) { + SendAlert(ssl, alert_fatal, illegal_parameter); + return DUPLICATE_MSG_E; + } + ssl->options.tls1_3 = 1; ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE; diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 3121608e2..aff381854 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2141,3 +2141,53 @@ int test_tls13_early_data(void) return EXPECT_RESULT(); } + +/* Check that the client won't send the same CH after a HRR. An HRR without + * a KeyShare or a Cookie extension will trigger the error. */ +int test_tls13_same_ch(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_TLS13) && defined(WOLFSSL_AES_128) && \ + defined(HAVE_AESGCM) && !defined(NO_SHA256) && \ + /* middlebox compat requires that the session ID is echoed */ \ + !defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT) + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL *ssl_c = NULL; + struct test_memio_ctx test_ctx; + /* Transport Layer Security + * TLSv1.3 Record Layer: Handshake Protocol: Hello Retry Request + * Content Type: Handshake (22) + * Version: TLS 1.2 (0x0303) + * Length: 50 + * Handshake Protocol: Hello Retry Request + * Handshake Type: Server Hello (2) + * Length: 46 + * Version: TLS 1.2 (0x0303) + * Random: cf21ad74e59a6111be1d8c021e65b891c2a211167abb8c5e079e09e2c8a8339c (HelloRetryRequest magic) + * Session ID Length: 0 + * Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301) + * Compression Method: null (0) + * Extensions Length: 6 + * Extension: supported_versions (len=2) TLS 1.3 */ + unsigned char hrr[] = { + 0x16, 0x03, 0x03, 0x00, 0x32, 0x02, 0x00, 0x00, 0x2e, 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, 0x01, 0x00, 0x00, + 0x06, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04 + }; + + 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(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), DUPLICATE_MSG_E); + + 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 10997ab39..fb46c404d 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -30,6 +30,7 @@ int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); int test_tls13_pq_groups(void); int test_tls13_early_data(void); +int test_tls13_same_ch(void); #define TEST_TLS13_DECLS \ TEST_DECL_GROUP("tls13", test_tls13_apis), \ @@ -37,6 +38,7 @@ int test_tls13_early_data(void); TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder), \ 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_early_data), \ + TEST_DECL_GROUP("tls13", test_tls13_same_ch) #endif /* WOLFCRYPT_TEST_TLS13_H */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 82876b27c..543871d1f 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5139,6 +5139,12 @@ struct Options { #if defined(HAVE_DANE) word16 useDANE:1; #endif /* HAVE_DANE */ +#ifdef WOLFSSL_TLS13 +#ifdef WOLFSSL_SEND_HRR_COOKIE + word16 hrrSentCookie:1; /* HRR sent with cookie */ +#endif + word16 hrrSentKeyShare:1; /* HRR sent with key share */ +#endif word16 disableRead:1; #ifdef WOLFSSL_DTLS byte haveMcast; /* using multicast ? */