Skip to content

Commit d900037

Browse files
FrauschiFrauschi
committed
Incorporate review comments
* Move new test to the test_wolfSSL_dtls_stateless_xxx group * Move cookie parsing to stateful DTLS processing * Remove unnecessary redundant cookie parsing logic
1 parent 45fa35f commit d900037

5 files changed

Lines changed: 162 additions & 160 deletions

File tree

‎src/dtls.c‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -295,42 +295,6 @@ static int CheckDtlsCookie(const WOLFSSL* ssl, WolfSSL_CH* ch,
295295
return ret;
296296
}
297297

298-
#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
299-
static int ParseCookieExt(WOLFSSL* ssl, WolfSSL_CH* ch)
300-
{
301-
int ret = 0;
302-
word16 len;
303-
const byte* cookie = NULL;
304-
305-
if (ch->cookieExt.size < OPAQUE16_LEN + 1)
306-
return BUFFER_E;
307-
ato16(ch->cookieExt.elements, &len);
308-
if (ch->cookieExt.size - OPAQUE16_LEN != len)
309-
return BUFFER_E;
310-
cookie = ch->cookieExt.elements + OPAQUE16_LEN;
311-
ret = TlsCheckCookie(ssl, cookie, len);
312-
if (ret < 0)
313-
return ret;
314-
len = (word16)ret;
315-
316-
/* Cookie Data = Hash Len | Hash | CS | KeyShare Group (optional) */
317-
318-
/* Check if the cookie has a key share group */
319-
if (len > cookie[0] + 4) {
320-
word16 keyShareGroup = 0;
321-
ato16(cookie + 3 + cookie[0], &keyShareGroup);
322-
323-
/* The key share group in the cookie is the group selected by the
324-
* server in the HelloRetryRequest. Hence, the client must use this
325-
* group in the second ClientHello.
326-
*/
327-
ssl->hrr_keyshare_group = keyShareGroup;
328-
}
329-
330-
return 0;
331-
}
332-
#endif /* WOLFSSL_DTLS13 && WOLFSSL_SEND_HRR_COOKIE */
333-
334298
static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch,
335299
byte isFirstCHFrag)
336300
{
@@ -1077,10 +1041,6 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz,
10771041
e = Dtls13GetEpoch(ssl, ssl->keys.curEpoch64);
10781042
if (e != NULL)
10791043
XMEMSET(e->window, 0xFF, sizeof(e->window));
1080-
1081-
ret = ParseCookieExt(ssl, &ch);
1082-
if (ret != 0)
1083-
return ret;
10841044
}
10851045
else
10861046
#endif

‎src/tls.c‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7248,6 +7248,55 @@ static int TLSX_Cookie_Write(Cookie* cookie, byte* output, byte msgType,
72487248
return 0;
72497249
}
72507250

7251+
#if defined(WOLFSSL_DTLS13) && defined (WOLFSSL_SEND_HRR_COOKIE)
7252+
/* Extract the key share group from the cookie and store it in the
7253+
* ssl session for later checks.
7254+
*
7255+
* ssl The SSL/TLS object.
7256+
* returns 0 on success and other values indicate failure.
7257+
*/
7258+
static int TLSX_Cookie_RestoreHrrGroup(WOLFSSL* ssl)
7259+
{
7260+
TLSX* extension;
7261+
Cookie* cookie;
7262+
#ifndef NO_SHA256
7263+
byte macSz = WC_SHA256_DIGEST_SIZE;
7264+
#elif defined(WOLFSSL_SHA384)
7265+
byte macSz = WC_SHA384_DIGEST_SIZE;
7266+
#elif defined(WOLFSSL_TLS13_SHA512)
7267+
byte macSz = WC_SHA512_DIGEST_SIZE;
7268+
#elif defined(WOLFSSL_SM3)
7269+
byte macSz = WC_SM3_DIGEST_SIZE;
7270+
#else
7271+
#error "No digest to available to use with HMAC for cookies."
7272+
#endif /* NO_SHA */
7273+
7274+
extension = TLSX_Find(ssl->extensions, TLSX_COOKIE);
7275+
if (extension == NULL)
7276+
return 0;
7277+
7278+
cookie = (Cookie*)extension->data;
7279+
if (cookie == NULL)
7280+
return 0;
7281+
7282+
/* Cookie Data = Hash Len | Hash | CS | KeyShare Group (optional) | MAC */
7283+
7284+
/* Check if the cookie has a key share group */
7285+
if (cookie->data[0] + 4 + macSz < cookie->len) {
7286+
word16 keyShareGroup = 0;
7287+
ato16(cookie->data + 3 + cookie->data[0], &keyShareGroup);
7288+
7289+
/* The key share group in the cookie is the group selected by the
7290+
* server in the HelloRetryRequest. Hence, the client must use this
7291+
* group in the second ClientHello.
7292+
*/
7293+
ssl->hrr_keyshare_group = keyShareGroup;
7294+
}
7295+
7296+
return 0;
7297+
}
7298+
#endif /* WOLFSSL_DTLS13 && WOLFSSL_SEND_HRR_COOKIE */
7299+
72517300
/* Parse the Cookie extension.
72527301
* In messages: ClientHello and HelloRetryRequest.
72537302
*
@@ -7290,12 +7339,21 @@ static int TLSX_Cookie_Parse(WOLFSSL* ssl, const byte* input, word16 length,
72907339
extension = TLSX_Find(ssl->extensions, TLSX_COOKIE);
72917340
if (extension == NULL) {
72927341
#ifdef WOLFSSL_DTLS13
7293-
if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version))
7342+
if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)) {
7343+
int ret = 0;
72947344
/* Allow a cookie extension with DTLS 1.3 because it is possible
72957345
* that a different SSL instance sent the cookie but we are now
72967346
* receiving it. */
7297-
return TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 0,
7298-
&ssl->extensions);
7347+
ret = TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 0,
7348+
&ssl->extensions);
7349+
#if defined(WOLFSSL_SEND_HRR_COOKIE)
7350+
if (ret == 0 && ssl->options.dtlsStateful) {
7351+
/* Try to extract a HRR key share group from the cookie */
7352+
ret = TLSX_Cookie_RestoreHrrGroup(ssl);
7353+
}
7354+
#endif
7355+
return ret;
7356+
}
72997357
else
73007358
#endif
73017359
{

‎tests/api.c‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27783,6 +27783,105 @@ static int test_wolfSSL_dtls_stateless_downgrade(void)
2778327783
}
2778427784
#endif /* !defined(NO_OLD_TLS) */
2778527785

27786+
/* DTLS stateless API handling multiple CHs with different HRR groups */
27787+
static int test_wolfSSL_dtls_stateless_hrr_group(void)
27788+
{
27789+
EXPECT_DECLS;
27790+
size_t i;
27791+
struct {
27792+
method_provider client_meth;
27793+
method_provider server_meth;
27794+
} params[] = {
27795+
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_DTLS13)
27796+
{ wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method },
27797+
#endif
27798+
#if !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_DTLS)
27799+
{ wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method },
27800+
#endif
27801+
};
27802+
for (i = 0; i < XELEM_CNT(params) && !EXPECT_FAIL(); i++) {
27803+
WOLFSSL_CTX *ctx_s = NULL, *ctx_c = NULL;
27804+
WOLFSSL *ssl_s = NULL, *ssl_c = NULL, *ssl_c2 = NULL;
27805+
struct test_memio_ctx test_ctx;
27806+
int groups_1[] = {
27807+
WOLFSSL_ECC_SECP256R1,
27808+
WOLFSSL_ECC_SECP384R1,
27809+
WOLFSSL_ECC_SECP521R1
27810+
};
27811+
int groups_2[] = {
27812+
WOLFSSL_ECC_SECP384R1,
27813+
WOLFSSL_ECC_SECP521R1
27814+
};
27815+
char hrrBuf[1000];
27816+
int hrrSz = sizeof(hrrBuf);
27817+
27818+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
27819+
27820+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
27821+
params[i].client_meth, params[i].server_meth), 0);
27822+
27823+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c2, NULL,
27824+
params[i].client_meth, params[i].server_meth), 0);
27825+
27826+
27827+
wolfSSL_SetLoggingPrefix("server");
27828+
wolfSSL_dtls_set_using_nonblock(ssl_s, 1);
27829+
27830+
/* Set groups and disable key shares. This ensures that only the given
27831+
* groups are in the SupportedGroups extension and that an empty key
27832+
* share extension is sent in the initial ClientHello of each session.
27833+
* This triggers the server to send a HelloRetryRequest with the first
27834+
* group in the SupportedGroups extension selected. */
27835+
wolfSSL_SetLoggingPrefix("client1");
27836+
ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups_1, 3), WOLFSSL_SUCCESS);
27837+
ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c), WOLFSSL_SUCCESS);
27838+
27839+
wolfSSL_SetLoggingPrefix("client2");
27840+
ExpectIntEQ(wolfSSL_set_groups(ssl_c2, groups_2, 2), WOLFSSL_SUCCESS);
27841+
ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c2), WOLFSSL_SUCCESS);
27842+
27843+
/* Start handshake, send first ClientHello */
27844+
wolfSSL_SetLoggingPrefix("client1");
27845+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
27846+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
27847+
27848+
/* Read first ClientHello, send HRR with WOLFSSL_ECC_SECP256R1 */
27849+
wolfSSL_SetLoggingPrefix("server");
27850+
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0);
27851+
ExpectIntEQ(test_memio_copy_message(&test_ctx, 1, hrrBuf, &hrrSz, 0), 0);
27852+
ExpectIntGT(hrrSz, 0);
27853+
test_memio_clear_buffer(&test_ctx, 1);
27854+
27855+
/* Send second ClientHello */
27856+
wolfSSL_SetLoggingPrefix("client2");
27857+
ExpectIntEQ(wolfSSL_connect(ssl_c2), -1);
27858+
ExpectIntEQ(wolfSSL_get_error(ssl_c2, -1), WOLFSSL_ERROR_WANT_READ);
27859+
27860+
/* Read second ClientHello, send HRR now with WOLFSSL_ECC_SECP384R1 */
27861+
wolfSSL_SetLoggingPrefix("server");
27862+
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0);
27863+
test_memio_clear_buffer(&test_ctx, 1);
27864+
27865+
/* Complete first handshake with WOLFSSL_ECC_SECP256R1 */
27866+
wolfSSL_SetLoggingPrefix("client1");
27867+
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, hrrBuf, hrrSz), 0);
27868+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
27869+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
27870+
27871+
wolfSSL_SetLoggingPrefix("server");
27872+
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), WOLFSSL_SUCCESS);
27873+
27874+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
27875+
27876+
wolfSSL_free(ssl_s);
27877+
wolfSSL_free(ssl_c);
27878+
wolfSSL_free(ssl_c2);
27879+
wolfSSL_CTX_free(ctx_s);
27880+
wolfSSL_CTX_free(ctx_c);
27881+
}
27882+
return EXPECT_RESULT();
27883+
}
27884+
2778627885
#endif /* defined(WOLFSSL_DTLS) && !defined(WOLFSSL_NO_TLS12) && \
2778727886
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)*/
2778827887

@@ -33223,6 +33322,7 @@ TEST_CASE testCases[] = {
3322333322
#if !defined(NO_OLD_TLS)
3322433323
TEST_DECL(test_wolfSSL_dtls_stateless_downgrade),
3322533324
#endif /* !defined(NO_OLD_TLS) */
33325+
TEST_DECL(test_wolfSSL_dtls_stateless_hrr_group),
3322633326
#endif /* ! NO_RSA */
3322733327
#endif /* defined(WOLFSSL_DTLS) && !defined(WOLFSSL_NO_TLS12) && \
3322833328
* !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) */

‎tests/api/test_dtls.c‎

Lines changed: 0 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -2567,117 +2567,3 @@ int test_dtls13_min_rtx_interval(void)
25672567
#endif
25682568
return EXPECT_RESULT();
25692569
}
2570-
2571-
/* DTLS stateless API handling multiple CHs with different HRR groups */
2572-
int test_dtls_stateless_hrr_group(void)
2573-
{
2574-
EXPECT_DECLS;
2575-
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS)
2576-
size_t i;
2577-
struct {
2578-
method_provider client_meth;
2579-
method_provider server_meth;
2580-
} params[] = {
2581-
#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_DTLS13)
2582-
{ wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method },
2583-
#endif
2584-
#if !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_DTLS)
2585-
{ wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method },
2586-
#endif
2587-
};
2588-
XMEMSET(&test_memio_wolfio_ctx, 0, sizeof(test_memio_wolfio_ctx));
2589-
for (i = 0; i < XELEM_CNT(params) && !EXPECT_FAIL(); i++) {
2590-
WOLFSSL_CTX *ctx_s = NULL, *ctx_c = NULL;
2591-
WOLFSSL *ssl_s = NULL, *ssl_c1 = NULL, *ssl_c2 = NULL;
2592-
struct test_memio_ctx test_ctx;
2593-
int groups_1[] = {
2594-
WOLFSSL_ECC_SECP256R1,
2595-
WOLFSSL_ECC_SECP384R1,
2596-
WOLFSSL_ECC_SECP521R1
2597-
};
2598-
int groups_2[] = {
2599-
WOLFSSL_ECC_SECP384R1,
2600-
WOLFSSL_ECC_SECP521R1
2601-
};
2602-
char hrrBuf[1000];
2603-
int hrrSz = sizeof(hrrBuf);
2604-
2605-
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
2606-
2607-
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c1, &ssl_s,
2608-
params[i].client_meth, params[i].server_meth), 0);
2609-
2610-
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c2, NULL,
2611-
params[i].client_meth, params[i].server_meth), 0);
2612-
2613-
test_memio_wolfio_ctx.test_ctx = &test_ctx;
2614-
test_memio_wolfio_ctx.ssl_s = ssl_s;
2615-
/* Large number to error out if any syscalls are called with it */
2616-
test_memio_wolfio_ctx.fd = 6000;
2617-
XMEMSET(&test_memio_wolfio_ctx.peer_addr, 0,
2618-
sizeof(test_memio_wolfio_ctx.peer_addr));
2619-
test_memio_wolfio_ctx.peer_addr.ss_family = AF_INET;
2620-
2621-
wolfSSL_SetLoggingPrefix("server");
2622-
wolfSSL_dtls_set_using_nonblock(ssl_s, 1);
2623-
wolfSSL_SetRecvFrom(ssl_s, test_memio_wolfio_recvfrom);
2624-
/* Restore default functions */
2625-
wolfSSL_SSLSetIORecv(ssl_s, EmbedReceiveFrom);
2626-
ExpectIntEQ(wolfSSL_set_read_fd(ssl_s, test_memio_wolfio_ctx.fd),
2627-
WOLFSSL_SUCCESS);
2628-
2629-
/* Set groups and disable key shares. This ensures that only the given
2630-
* groups are in the SupportedGroups extension and that an empty key
2631-
* share extension is sent in the initial ClientHello of each session.
2632-
* This triggers the server to send a HelloRetryRequest with the first
2633-
* group in the SupportedGroups extension selected. */
2634-
wolfSSL_SetLoggingPrefix("client1");
2635-
ExpectIntEQ(wolfSSL_set_groups(ssl_c1, groups_1, 3), WOLFSSL_SUCCESS);
2636-
ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c1), WOLFSSL_SUCCESS);
2637-
2638-
wolfSSL_SetLoggingPrefix("client2");
2639-
ExpectIntEQ(wolfSSL_set_groups(ssl_c2, groups_2, 2), WOLFSSL_SUCCESS);
2640-
ExpectIntEQ(wolfSSL_NoKeyShares(ssl_c2), WOLFSSL_SUCCESS);
2641-
2642-
/* Start handshake, send first ClientHello */
2643-
wolfSSL_SetLoggingPrefix("client1");
2644-
ExpectIntEQ(wolfSSL_connect(ssl_c1), -1);
2645-
ExpectIntEQ(wolfSSL_get_error(ssl_c1, -1), WOLFSSL_ERROR_WANT_READ);
2646-
2647-
/* Read first ClientHello, send HRR with WOLFSSL_ECC_SECP256R1 */
2648-
wolfSSL_SetLoggingPrefix("server");
2649-
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0);
2650-
ExpectIntEQ(test_memio_copy_message(&test_ctx, 1, hrrBuf, &hrrSz, 0), 0);
2651-
ExpectIntGT(hrrSz, 0);
2652-
test_memio_clear_buffer(&test_ctx, 1);
2653-
2654-
/* Send second ClientHello */
2655-
wolfSSL_SetLoggingPrefix("client2");
2656-
ExpectIntEQ(wolfSSL_connect(ssl_c2), -1);
2657-
ExpectIntEQ(wolfSSL_get_error(ssl_c2, -1), WOLFSSL_ERROR_WANT_READ);
2658-
2659-
/* Read second ClientHello, send HRR now with WOLFSSL_ECC_SECP384R1 */
2660-
wolfSSL_SetLoggingPrefix("server");
2661-
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), 0);
2662-
test_memio_clear_buffer(&test_ctx, 1);
2663-
2664-
/* Complete first handshake with WOLFSSL_ECC_SECP256R1 */
2665-
wolfSSL_SetLoggingPrefix("client1");
2666-
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, hrrBuf, hrrSz), 0);
2667-
ExpectIntEQ(wolfSSL_connect(ssl_c1), -1);
2668-
ExpectIntEQ(wolfSSL_get_error(ssl_c1, -1), WOLFSSL_ERROR_WANT_READ);
2669-
2670-
wolfSSL_SetLoggingPrefix("server");
2671-
ExpectIntEQ(wolfDTLS_accept_stateless(ssl_s), WOLFSSL_SUCCESS);
2672-
2673-
ExpectIntEQ(test_memio_do_handshake(ssl_c1, ssl_s, 10, NULL), 0);
2674-
2675-
wolfSSL_free(ssl_s);
2676-
wolfSSL_free(ssl_c1);
2677-
wolfSSL_free(ssl_c2);
2678-
wolfSSL_CTX_free(ctx_s);
2679-
wolfSSL_CTX_free(ctx_c);
2680-
}
2681-
#endif
2682-
return EXPECT_RESULT();
2683-
}

‎tests/api/test_dtls.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ int test_dtls_memio_wolfio_stateless(void);
5050
int test_dtls_mtu_fragment_headroom(void);
5151
int test_dtls_mtu_split_messages(void);
5252
int test_dtls13_min_rtx_interval(void);
53-
int test_dtls_stateless_hrr_group(void);
5453

5554
#define TEST_DTLS_DECLS \
5655
TEST_DECL_GROUP("dtls", test_dtls12_basic_connection_id), \
@@ -80,6 +79,5 @@ int test_dtls_stateless_hrr_group(void);
8079
TEST_DECL_GROUP("dtls", test_dtls_mtu_fragment_headroom), \
8180
TEST_DECL_GROUP("dtls", test_dtls_mtu_split_messages), \
8281
TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio_stateless), \
83-
TEST_DECL_GROUP("dtls", test_dtls13_min_rtx_interval), \
84-
TEST_DECL_GROUP("dtls", test_dtls_stateless_hrr_group)
82+
TEST_DECL_GROUP("dtls", test_dtls13_min_rtx_interval)
8583
#endif /* TESTS_API_DTLS_H */

0 commit comments

Comments
 (0)