Skip to content

Commit ff819b0

Browse files
committed
Incorporate DTLS review comments
1 parent ce85d69 commit ff819b0

3 files changed

Lines changed: 22 additions & 6 deletions

File tree

src/dtls.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,8 @@ static int SendStatelessReplyDtls13(const WOLFSSL* ssl, WolfSSL_CH* ch)
829829
* extension is present. */
830830
TLSX* ksExt = TLSX_Find(parsedExts, TLSX_KEY_SHARE);
831831
KeyShareEntry* kse = (KeyShareEntry*)ksExt->data;
832-
if (WOLFSSL_NAMED_GROUP_IS_PQC(kse->group) ||
833-
WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(kse->group)) {
832+
if (kse != NULL && (WOLFSSL_NAMED_GROUP_IS_PQC(kse->group) ||
833+
WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(kse->group))){
834834
/* Allow fragmentation of the second ClientHello due to the
835835
* large PQC key share. */
836836
wolfSSL_dtls13_allow_ch_frag((WOLFSSL*)ssl, 1);

src/tls.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10272,7 +10272,19 @@ int TLSX_KeyShare_Parse_ClientHello(const WOLFSSL* ssl,
1027210272
offset += ret;
1027310273
}
1027410274

10275-
if (ssl->hrr_keyshare_group != 0 && seenGroupsCnt > 0) {
10275+
if (ssl->hrr_keyshare_group != 0) {
10276+
#ifdef WOLFSSL_DTLS
10277+
/* In case of stateless DTLSv1.3, the ssl->hrr_keyshare_group variable
10278+
* may already been set without further proceeding the handshake, so we
10279+
* never received the second ClientHello with the selected group.
10280+
* Hence, when we receive another ClientHello message with an empty key
10281+
* share, we do not consider that as an error here.
10282+
*/
10283+
if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version) &&
10284+
!ssl->options.dtlsStateful && seenGroupsCnt == 0) {
10285+
return 0;
10286+
}
10287+
#endif
1027610288
/*
1027710289
* https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.8
1027810290
* when sending the new ClientHello, the client MUST

tests/api.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29130,6 +29130,10 @@ static int test_dtls13_bad_epoch_ch(void)
2913029130
WOLFSSL *ssl_s = NULL;
2913129131
struct test_memio_ctx test_ctx;
2913229132
const int EPOCH_OFF = 3;
29133+
int groups[] = {
29134+
WOLFSSL_ECC_SECP256R1,
29135+
WOLFSSL_ECC_SECP384R1,
29136+
};
2913329137

2913429138
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
2913529139
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
@@ -29139,6 +29143,9 @@ static int test_dtls13_bad_epoch_ch(void)
2913929143
* with just one message */
2914029144
ExpectIntEQ(wolfSSL_disable_hrr_cookie(ssl_s), WOLFSSL_SUCCESS);
2914129145

29146+
/* Set client groups to traditional only to avoid CH fragmentation */
29147+
ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups, 2), WOLFSSL_SUCCESS);
29148+
2914229149
ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
2914329150
ExpectIntEQ(wolfSSL_get_error(ssl_c, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)),
2914429151
WOLFSSL_ERROR_WANT_READ);
@@ -29161,9 +29168,6 @@ static int test_dtls13_bad_epoch_ch(void)
2916129168
/* resend the CH */
2916229169
ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS);
2916329170

29164-
/* Re-enable HRR cookie to account for potential CH fragmentation */
29165-
ExpectIntEQ(wolfSSL_send_hrr_cookie(ssl_s, NULL, 0), WOLFSSL_SUCCESS);
29166-
2916729171
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
2916829172

2916929173
wolfSSL_free(ssl_c);

0 commit comments

Comments
 (0)