Skip to content

Commit c14b1a0

Browse files
julek-wolfssljulek-wolfssl
committed
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
1 parent df79b10 commit c14b1a0

5 files changed

Lines changed: 117 additions & 7 deletions

File tree

‎src/tls13.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5253,6 +5253,18 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
52535253
/* Set the cipher suite from the message. */
52545254
ssl->options.cipherSuite0 = input[args->idx++];
52555255
ssl->options.cipherSuite = input[args->idx++];
5256+
if (*extMsgType == hello_retry_request) {
5257+
ssl->options.hrrCipherSuite0 = ssl->options.cipherSuite0;
5258+
ssl->options.hrrCipherSuite = ssl->options.cipherSuite;
5259+
}
5260+
else if (ssl->msgsReceived.got_hello_retry_request &&
5261+
(ssl->options.hrrCipherSuite0 != ssl->options.cipherSuite0 ||
5262+
ssl->options.hrrCipherSuite != ssl->options.cipherSuite)) {
5263+
WOLFSSL_MSG("Received ServerHello with different cipher suite than "
5264+
"HelloRetryRequest");
5265+
WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER);
5266+
return INVALID_PARAMETER;
5267+
}
52565268
#ifdef WOLFSSL_DEBUG_TLS
52575269
WOLFSSL_MSG("Chosen cipher suite:");
52585270
WOLFSSL_MSG(GetCipherNameInternal(ssl->options.cipherSuite0,

‎tests/api.c‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48134,17 +48134,16 @@ static int test_TLSX_CA_NAMES_bad_extension(void)
4813448134
EXPECT_DECLS;
4813548135
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
4813648136
!defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) && \
48137-
defined(OPENSSL_EXTRA) && defined(WOLFSSL_SHA384) && \
48138-
defined(HAVE_NULL_CIPHER) && defined(HAVE_CHACHA) && \
48139-
defined(HAVE_POLY1305)
48137+
defined(OPENSSL_EXTRA) && defined(BUILD_TLS_CHACHA20_POLY1305_SHA256) && \
48138+
defined(HAVE_ECC) && !defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
4814048139
/* This test should only fail (with BUFFER_ERROR) when we actually try to
4814148140
* parse the CA Names extension. Otherwise it will return other non-related
4814248141
* errors. If CA Names will be parsed in more configurations, that should
4814348142
* be reflected in the macro guard above. */
4814448143
WOLFSSL *ssl_c = NULL;
4814548144
WOLFSSL_CTX *ctx_c = NULL;
4814648145
struct test_memio_ctx test_ctx;
48147-
/* HRR + SH using TLS_DHE_PSK_WITH_NULL_SHA384 */
48146+
/* HRR + SH using TLS_CHACHA20_POLY1305_SHA256 */
4814848147
const byte shBadCaNamesExt[] = {
4814948148
0x16, 0x03, 0x04, 0x00, 0x3f, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0xcf,
4815048149
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)
4815548154
0x5c, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x03, 0xcf, 0x21, 0xad, 0x74,
4815648155
0x00, 0x00, 0x83, 0x3f, 0x3b, 0x80, 0x01, 0xac, 0x65, 0x8c, 0x19, 0x2a,
4815748156
0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x02, 0x00, 0x9e, 0x09, 0x1c, 0xe8,
48158-
0xa8, 0x09, 0x9c, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
48157+
0xa8, 0x09, 0x9c, 0x00, 0x13, 0x03, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
4815948158
0x03, 0x3f, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x13, 0x05,
4816048159
0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x00, 0x09, 0x00, 0x00,
4816148160
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)
4817148170
0x5e, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x7f, 0xd0, 0x2d, 0xea, 0x6e,
4817248171
0x53, 0xa1, 0x6a, 0xc9, 0xc8, 0x54, 0xef, 0x75, 0xe4, 0xd9, 0xc6, 0x3e,
4817348172
0x74, 0xcb, 0x30, 0x80, 0xcc, 0x83, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00,
48174-
0x00, 0xc0, 0x5a, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
48173+
0x00, 0xc0, 0x5a, 0x00, 0x13, 0x03, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
4817548174
0x03, 0x03, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x53, 0x25,
4817648175
0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x02, 0x05, 0x00, 0x00,
4817748176
0x0d, 0x00, 0x00, 0x11, 0x00, 0x00, 0x0d, 0x00, 0x2f, 0x00, 0x06, 0x00,

‎tests/api/test_tls13.c‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2224,3 +2224,96 @@ int test_tls13_same_ch(void)
22242224
#endif
22252225
return EXPECT_RESULT();
22262226
}
2227+
2228+
int test_tls13_hrr_different_cs(void)
2229+
{
2230+
EXPECT_DECLS;
2231+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
2232+
defined(WOLFSSL_TLS13) && \
2233+
defined(BUILD_TLS_AES_256_GCM_SHA384) && \
2234+
defined(BUILD_TLS_CHACHA20_POLY1305_SHA256) && \
2235+
defined(HAVE_ECC) && defined(HAVE_ECC384)
2236+
/*
2237+
* TLSv1.3 Record Layer: Handshake Protocol: Hello Retry Request
2238+
* Content Type: Handshake (22)
2239+
* Version: TLS 1.2 (0x0303)
2240+
* Length: 56
2241+
* Handshake Protocol: Hello Retry Request
2242+
* Handshake Type: Server Hello (2)
2243+
* Length: 52
2244+
* Version: TLS 1.2 (0x0303)
2245+
* Random: cf21ad74e59a6111be1d8c021e65b891c2a211167abb8c5e079e09e2c8a8339c (HelloRetryRequest magic)
2246+
* Session ID Length: 0
2247+
* Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
2248+
* Compression Method: null (0)
2249+
* Extensions Length: 12
2250+
* Extension: supported_versions (len=2) TLS 1.3
2251+
* Extension: key_share (len=2) secp384r1
2252+
*
2253+
*/
2254+
unsigned char hrr[] = {
2255+
0x16, 0x03, 0x03, 0x00, 0x38, 0x02, 0x00, 0x00, 0x34, 0x03, 0x03, 0xcf,
2256+
0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02, 0x1e,
2257+
0x65, 0xb8, 0x91, 0xc2, 0xa2, 0x11, 0x16, 0x7a, 0xbb, 0x8c, 0x5e, 0x07,
2258+
0x9e, 0x09, 0xe2, 0xc8, 0xa8, 0x33, 0x9c, 0x00, 0x13, 0x02, 0x00, 0x00,
2259+
0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00,
2260+
0x18
2261+
};
2262+
/*
2263+
* TLSv1.3 Record Layer: Handshake Protocol: Server Hello
2264+
* Content Type: Handshake (22)
2265+
* Version: TLS 1.2 (0x0303)
2266+
* Length: 155
2267+
* Handshake Protocol: Server Hello
2268+
* Handshake Type: Server Hello (2)
2269+
* Length: 151
2270+
* Version: TLS 1.2 (0x0303)
2271+
* Random: 0101010101010101010101010101010101010101010101010101010101010101
2272+
* Session ID Length: 0
2273+
* Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303)
2274+
* Compression Method: null (0)
2275+
* Extensions Length: 111
2276+
* Extension: key_share (len=101) secp384r1
2277+
* Extension: supported_versions (len=2) TLS 1.3
2278+
*
2279+
*/
2280+
unsigned char sh[] = {
2281+
0x16, 0x03, 0x03, 0x00, 0x9b, 0x02, 0x00, 0x00, 0x97, 0x03, 0x03, 0x01,
2282+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2283+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
2284+
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00, 0x13, 0x03, 0x00, 0x00,
2285+
0x6f, 0x00, 0x33, 0x00, 0x65, 0x00, 0x18, 0x00, 0x61, 0x04, 0x53, 0x3e,
2286+
0xe5, 0xbf, 0x40, 0xec, 0x2d, 0x67, 0x98, 0x8b, 0x77, 0xf3, 0x17, 0x48,
2287+
0x9b, 0xb6, 0xdf, 0x95, 0x29, 0x25, 0xc7, 0x09, 0xfc, 0x03, 0x81, 0x11,
2288+
0x1a, 0x59, 0x56, 0xf2, 0xd7, 0x58, 0x11, 0x0e, 0x59, 0xd3, 0xd7, 0xc1,
2289+
0x72, 0x9e, 0x2c, 0x0d, 0x70, 0xea, 0xf7, 0x73, 0xe6, 0x12, 0x01, 0x16,
2290+
0x42, 0x6d, 0xe2, 0x43, 0x6a, 0x2f, 0x5f, 0xdd, 0x7f, 0xe5, 0x4f, 0xaf,
2291+
0x95, 0x2b, 0x04, 0xfd, 0x13, 0xf5, 0x16, 0xce, 0x62, 0x7f, 0x89, 0xd2,
2292+
0x01, 0x9d, 0x4c, 0x87, 0x96, 0x95, 0x9e, 0x43, 0x33, 0xc7, 0x06, 0x5b,
2293+
0x49, 0x6c, 0xa6, 0x34, 0xd5, 0xdc, 0x63, 0xbd, 0xe9, 0x1f, 0x00, 0x2b,
2294+
0x00, 0x02, 0x03, 0x04
2295+
};
2296+
WOLFSSL_CTX *ctx_c = NULL;
2297+
WOLFSSL *ssl_c = NULL;
2298+
struct test_memio_ctx test_ctx;
2299+
2300+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
2301+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
2302+
wolfTLSv1_3_client_method, NULL), 0);
2303+
2304+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
2305+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
2306+
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (char*)hrr,
2307+
sizeof(hrr)), 0);
2308+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
2309+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
2310+
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (char*)sh,
2311+
sizeof(sh)), 0);
2312+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
2313+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), INVALID_PARAMETER);
2314+
2315+
wolfSSL_free(ssl_c);
2316+
wolfSSL_CTX_free(ctx_c);
2317+
#endif
2318+
return EXPECT_RESULT();
2319+
}

‎tests/api/test_tls13.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ int test_tls13_rpk_handshake(void);
3131
int test_tls13_pq_groups(void);
3232
int test_tls13_early_data(void);
3333
int test_tls13_same_ch(void);
34+
int test_tls13_hrr_different_cs(void);
3435

3536
#define TEST_TLS13_DECLS \
3637
TEST_DECL_GROUP("tls13", test_tls13_apis), \
@@ -39,6 +40,7 @@ int test_tls13_same_ch(void);
3940
TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \
4041
TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \
4142
TEST_DECL_GROUP("tls13", test_tls13_early_data), \
42-
TEST_DECL_GROUP("tls13", test_tls13_same_ch)
43+
TEST_DECL_GROUP("tls13", test_tls13_same_ch), \
44+
TEST_DECL_GROUP("tls13", test_tls13_hrr_different_cs)
4345

4446
#endif /* WOLFCRYPT_TEST_TLS13_H */

‎wolfssl/internal.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5100,6 +5100,10 @@ struct Options {
51005100
byte processReply; /* nonblocking resume */
51015101
byte cipherSuite0; /* first byte, normally 0 */
51025102
byte cipherSuite; /* second byte, actual suite */
5103+
#ifdef WOLFSSL_TLS13
5104+
byte hrrCipherSuite0; /* first byte, normally 0 */
5105+
byte hrrCipherSuite; /* second byte, actual suite */
5106+
#endif
51035107
byte hashAlgo; /* selected hash algorithm */
51045108
byte sigAlgo; /* selected sig algorithm */
51055109
byte peerHashAlgo; /* peer's chosen hash algo */

0 commit comments

Comments
 (0)