Skip to content

Commit 73bea90

Browse files
authored
Merge pull request #10034 from sebastian-carpenter/GH-10016
verify ciphersuite in CH2 matches HRR
2 parents 328822b + 406f503 commit 73bea90

3 files changed

Lines changed: 149 additions & 2 deletions

File tree

src/tls13.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6616,8 +6616,10 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
66166616
hrrIdx += ssl->session->sessionIDSz;
66176617
}
66186618

6619-
/* Cipher Suite */
6619+
/* Restore the cipher suite from the cookie. */
6620+
ssl->options.hrrCipherSuite0 = cookieData[idx];
66206621
hrr[hrrIdx++] = cookieData[idx++];
6622+
ssl->options.hrrCipherSuite = cookieData[idx];
66216623
hrr[hrrIdx++] = cookieData[idx++];
66226624

66236625
/* Compression not supported in TLS v1.3. */
@@ -7331,6 +7333,21 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
73317333
}
73327334
#endif
73337335

7336+
/* Verify the cipher suite is the same as what was chosen in HRR.
7337+
* got_client_hello == 2 covers the stateful path.
7338+
* cookieGood covers the stateless DTLS path. */
7339+
if ((ssl->msgsReceived.got_client_hello == 2
7340+
#ifdef WOLFSSL_SEND_HRR_COOKIE
7341+
|| ssl->options.cookieGood
7342+
#endif
7343+
) &&
7344+
(ssl->options.cipherSuite0 != ssl->options.hrrCipherSuite0 ||
7345+
ssl->options.cipherSuite != ssl->options.hrrCipherSuite)) {
7346+
WOLFSSL_MSG("Cipher suite in second ClientHello does not match "
7347+
"HelloRetryRequest");
7348+
ERROR_OUT(INVALID_PARAMETER, exit_dch);
7349+
}
7350+
73347351
/* Advance state and proceed */
73357352
ssl->options.asyncState = TLS_ASYNC_VERIFY;
73367353
} /* case TLS_ASYNC_BUILD || TLS_ASYNC_DO */
@@ -7638,7 +7655,9 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType)
76387655
* In case of stateless DTLS, we do not store the group, however, as it is
76397656
* already stored in the cookie that is sent to the client. We later recover
76407657
* the group from the cookie to prevent storing a state in a stateless
7641-
* server. */
7658+
* server.
7659+
*
7660+
* Similar logic holds for the hrrCipherSuite. */
76427661
if (extMsgType == hello_retry_request
76437662
#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
76447663
&& (!ssl->options.dtls || ssl->options.dtlsStateful)
@@ -7650,6 +7669,9 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType)
76507669
if (kse != NULL)
76517670
ssl->hrr_keyshare_group = kse->group;
76527671
}
7672+
7673+
ssl->options.hrrCipherSuite0 = ssl->options.cipherSuite0;
7674+
ssl->options.hrrCipherSuite = ssl->options.cipherSuite;
76537675
}
76547676

76557677
#ifdef WOLFSSL_SEND_HRR_COOKIE

tests/api/test_tls13.c

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2542,6 +2542,129 @@ int test_tls13_hrr_different_cs(void)
25422542
return EXPECT_RESULT();
25432543
}
25442544

2545+
/* Server-side complement to test_tls13_hrr_different_cs: the client sends a
2546+
* different cipher suite in CH2 than what the server selected in the HRR. */
2547+
int test_tls13_ch2_different_cs(void)
2548+
{
2549+
EXPECT_DECLS;
2550+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
2551+
defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER) && \
2552+
defined(BUILD_TLS_AES_256_GCM_SHA384) && \
2553+
defined(BUILD_TLS_AES_128_GCM_SHA256) && \
2554+
defined(HAVE_ECC) && defined(HAVE_ECC384)
2555+
/*
2556+
* First ClientHello: cipher suite TLS_AES_256_GCM_SHA384 (0x1302),
2557+
* empty key_share, secp384r1 in supported_groups. This triggers the
2558+
* server to send a HelloRetryRequest selecting TLS_AES_256_GCM_SHA384
2559+
* and requesting a secp384r1 key share.
2560+
*/
2561+
/*
2562+
* TLSv1.3 Record Layer: Handshake Protocol: Client Hello
2563+
* Content Type: Handshake (22)
2564+
* Version: TLS 1.2 (0x0303)
2565+
* Length: 110
2566+
* Handshake Protocol: Client Hello
2567+
* Handshake Type: Client Hello (1)
2568+
* Length: 106
2569+
* Version: TLS 1.2 (0x0303)
2570+
* Random: 0101010101010101010101010101010101010101010101010101010101010101
2571+
* Session ID Length: 32
2572+
* Session ID: 0303030303030303030303030303030303030303030303030303030303030303
2573+
* Cipher Suites Length: 2
2574+
* Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
2575+
* Compression Methods Length: 1
2576+
* Compression Method: null (0)
2577+
* Extensions Length: 31
2578+
* Extension: supported_groups (len=4) secp384r1 (0x0018)
2579+
* Extension: signature_algorithms (len=6) rsa_pkcs1_sha256 (0x0401),
2580+
* rsa_pss_rsae_sha256 (0x0804)
2581+
* Extension: key_share (len=2) client_shares length=0 (empty)
2582+
* Extension: supported_versions (len=3) TLS 1.3 (0x0304)
2583+
*/
2584+
unsigned char ch1[] = {
2585+
0x16, 0x03, 0x03, 0x00, 0x6e, 0x01, 0x00, 0x00, 0x6a, 0x03, 0x03, 0x01,
2586+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2587+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2588+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x20, 0x03, 0x03, 0x03, 0x03,
2589+
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
2590+
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
2591+
0x03, 0x03, 0x03, 0x03, 0x00, 0x02, 0x13, 0x02, 0x01, 0x00, 0x00, 0x1f,
2592+
0x00, 0x0a, 0x00, 0x04, 0x00, 0x02, 0x00, 0x18, 0x00, 0x0d, 0x00, 0x06,
2593+
0x00, 0x04, 0x04, 0x01, 0x08, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00, 0x00,
2594+
0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04
2595+
};
2596+
/*
2597+
* TLSv1.3 Record Layer: Handshake Protocol: Client Hello
2598+
* Content Type: Handshake (22)
2599+
* Version: TLS 1.2 (0x0303)
2600+
* Length: 211
2601+
* Handshake Protocol: Client Hello
2602+
* Handshake Type: Client Hello (1)
2603+
* Length: 207
2604+
* Version: TLS 1.2 (0x0303)
2605+
* Random: 0101010101010101010101010101010101010101010101010101010101010101
2606+
* Session ID Length: 32
2607+
* Session ID: 0303030303030303030303030303030303030303030303030303030303030303
2608+
* Cipher Suites Length: 2
2609+
* Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
2610+
* Compression Methods Length: 1
2611+
* Compression Method: null (0)
2612+
* Extensions Length: 132
2613+
* Extension: supported_groups (len=4) secp384r1 (0x0018)
2614+
* Extension: signature_algorithms (len=6) rsa_pkcs1_sha256 (0x0401),
2615+
* rsa_pss_rsae_sha256 (0x0804)
2616+
* Extension: key_share (len=103)
2617+
* client_shares length: 101
2618+
* KeyShareEntry: group secp384r1 (0x0018), key_exchange length: 97
2619+
* key_exchange: 04 || X(48) || Y(48) (uncompressed P-384 point)
2620+
* Extension: supported_versions (len=3) TLS 1.3 (0x0304)
2621+
*/
2622+
unsigned char ch2[] = {
2623+
0x16, 0x03, 0x03, 0x00, 0xd3, 0x01, 0x00, 0x00, 0xcf, 0x03, 0x03, 0x01,
2624+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2625+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2626+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x20, 0x03, 0x03, 0x03, 0x03,
2627+
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
2628+
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
2629+
0x03, 0x03, 0x03, 0x03, 0x00, 0x02, 0x13, 0x01, 0x01, 0x00, 0x00, 0x84,
2630+
0x00, 0x0a, 0x00, 0x04, 0x00, 0x02, 0x00, 0x18, 0x00, 0x0d, 0x00, 0x06,
2631+
0x00, 0x04, 0x04, 0x01, 0x08, 0x04, 0x00, 0x33, 0x00, 0x67, 0x00, 0x65,
2632+
0x00, 0x18, 0x00, 0x61, 0x04, 0x53, 0x3e, 0xe5, 0xbf, 0x40, 0xec, 0x2d,
2633+
0x67, 0x98, 0x8b, 0x77, 0xf3, 0x17, 0x48, 0x9b, 0xb6, 0xdf, 0x95, 0x29,
2634+
0x25, 0xc7, 0x09, 0xfc, 0x03, 0x81, 0x11, 0x1a, 0x59, 0x56, 0xf2, 0xd7,
2635+
0x58, 0x11, 0x0e, 0x59, 0xd3, 0xd7, 0xc1, 0x72, 0x9e, 0x2c, 0x0d, 0x70,
2636+
0xea, 0xf7, 0x73, 0xe6, 0x12, 0x01, 0x16, 0x42, 0x6d, 0xe2, 0x43, 0x6a,
2637+
0x2f, 0x5f, 0xdd, 0x7f, 0xe5, 0x4f, 0xaf, 0x95, 0x2b, 0x04, 0xfd, 0x13,
2638+
0xf5, 0x16, 0xce, 0x62, 0x7f, 0x89, 0xd2, 0x01, 0x9d, 0x4c, 0x87, 0x96,
2639+
0x95, 0x9e, 0x43, 0x33, 0xc7, 0x06, 0x5b, 0x49, 0x6c, 0xa6, 0x34, 0xd5,
2640+
0xdc, 0x63, 0xbd, 0xe9, 0x1f, 0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04
2641+
};
2642+
WOLFSSL_CTX *ctx_s = NULL;
2643+
WOLFSSL *ssl_s = NULL;
2644+
struct test_memio_ctx test_ctx;
2645+
2646+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
2647+
ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s,
2648+
NULL, wolfTLSv1_3_server_method), 0);
2649+
2650+
/* Server reads CH1, sends HRR, then waits for CH2 */
2651+
ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, (char*)ch1,
2652+
sizeof(ch1)), 0);
2653+
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
2654+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
2655+
2656+
/* Server must reject CH2 because the cipher suite changed from the HRR */
2657+
ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, (char*)ch2,
2658+
sizeof(ch2)), 0);
2659+
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
2660+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), INVALID_PARAMETER);
2661+
2662+
wolfSSL_free(ssl_s);
2663+
wolfSSL_CTX_free(ctx_s);
2664+
#endif
2665+
return EXPECT_RESULT();
2666+
}
2667+
25452668
#if defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_SERVER) && \
25462669
defined(HAVE_ECC)
25472670
/* Called when writing. */

tests/api/test_tls13.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ int test_tls13_pq_groups(void);
3232
int test_tls13_early_data(void);
3333
int test_tls13_same_ch(void);
3434
int test_tls13_hrr_different_cs(void);
35+
int test_tls13_ch2_different_cs(void);
3536
int test_tls13_sg_missing(void);
3637
int test_tls13_ks_missing(void);
3738
int test_tls13_duplicate_extension(void);
@@ -51,6 +52,7 @@ int test_tls13_derive_keys_no_key(void);
5152
TEST_DECL_GROUP("tls13", test_tls13_early_data), \
5253
TEST_DECL_GROUP("tls13", test_tls13_same_ch), \
5354
TEST_DECL_GROUP("tls13", test_tls13_hrr_different_cs), \
55+
TEST_DECL_GROUP("tls13", test_tls13_ch2_different_cs), \
5456
TEST_DECL_GROUP("tls13", test_tls13_sg_missing), \
5557
TEST_DECL_GROUP("tls13", test_tls13_ks_missing), \
5658
TEST_DECL_GROUP("tls13", test_tls13_duplicate_extension), \

0 commit comments

Comments
 (0)