Skip to content

Commit 8bddeb1

Browse files
julek-wolfssljulek-wolfssl
committed
DTLS sequence number and cookie fixes
- dtls: check that the cookie secret is not emtpy - Dtls13DoDowngrade -> Dtls13ClientDoDowngrade - dtls: generate both 1.2 and 1.3 cookie secrets in case we downgrade - dtls: setup sequence numbers for downgrade - add dtls downgrade sequence number check test Fixes ZD17314
1 parent 851f059 commit 8bddeb1

4 files changed

Lines changed: 107 additions & 11 deletions

File tree

‎src/dtls.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ int DtlsIgnoreError(int err)
114114
case SOCKET_ERROR_E:
115115
case WANT_READ:
116116
case WANT_WRITE:
117+
case COOKIE_ERROR:
117118
return 0;
118119
default:
119120
return 1;
@@ -207,6 +208,13 @@ static int CreateDtls12Cookie(const WOLFSSL* ssl, const WolfSSL_CH* ch,
207208
{
208209
int ret;
209210
Hmac cookieHmac;
211+
212+
if (ssl->buffers.dtlsCookieSecret.buffer == NULL ||
213+
ssl->buffers.dtlsCookieSecret.length == 0) {
214+
WOLFSSL_MSG("Missing DTLS 1.2 cookie secret");
215+
return COOKIE_ERROR;
216+
}
217+
210218
ret = wc_HmacInit(&cookieHmac, ssl->heap, ssl->devId);
211219
if (ret == 0) {
212220
ret = wc_HmacSetKey(&cookieHmac, DTLS_COOKIE_TYPE,

‎src/internal.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7456,15 +7456,14 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
74567456

74577457
#if defined(WOLFSSL_DTLS) && !defined(NO_WOLFSSL_SERVER)
74587458
if (ssl->options.dtls && ssl->options.side == WOLFSSL_SERVER_END) {
7459-
if (!IsAtLeastTLSv1_3(ssl->version)) {
7460-
ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
7461-
if (ret != 0) {
7462-
WOLFSSL_MSG("DTLS Cookie Secret error");
7463-
return ret;
7464-
}
7459+
/* Initialize both in case we allow downgrading. */
7460+
ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
7461+
if (ret != 0) {
7462+
WOLFSSL_MSG("DTLS Cookie Secret error");
7463+
return ret;
74657464
}
74667465
#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
7467-
else {
7466+
if (IsAtLeastTLSv1_3(ssl->version)) {
74687467
ret = wolfSSL_send_hrr_cookie(ssl, NULL, 0);
74697468
if (ret != WOLFSSL_SUCCESS) {
74707469
WOLFSSL_MSG("DTLS1.3 Cookie secret error");

‎src/tls13.c‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3546,6 +3546,12 @@ int CreateCookieExt(const WOLFSSL* ssl, byte* hash, word16 hashSz,
35463546
return BAD_FUNC_ARG;
35473547
}
35483548

3549+
if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
3550+
ssl->buffers.tls13CookieSecret.length == 0) {
3551+
WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
3552+
return COOKIE_ERROR;
3553+
}
3554+
35493555
/* Cookie Data = Hash Len | Hash | CS | KeyShare Group */
35503556
cookie[cookieSz++] = (byte)hashSz;
35513557
XMEMCPY(cookie + cookieSz, hash, hashSz);
@@ -4693,7 +4699,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
46934699
}
46944700

46954701
#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_NO_CLIENT)
4696-
static int Dtls13DoDowngrade(WOLFSSL* ssl)
4702+
static int Dtls13ClientDoDowngrade(WOLFSSL* ssl)
46974703
{
46984704
int ret;
46994705
if (ssl->dtls13ClientHello == NULL)
@@ -5099,7 +5105,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
50995105
if (ssl->options.dtls) {
51005106
ssl->chVersion.minor = DTLSv1_2_MINOR;
51015107
ssl->version.minor = DTLSv1_2_MINOR;
5102-
ret = Dtls13DoDowngrade(ssl);
5108+
ret = Dtls13ClientDoDowngrade(ssl);
51035109
if (ret != 0)
51045110
return ret;
51055111
}
@@ -5193,7 +5199,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
51935199
if (ssl->options.dtls) {
51945200
ssl->chVersion.minor = DTLSv1_2_MINOR;
51955201
ssl->version.minor = DTLSv1_2_MINOR;
5196-
ret = Dtls13DoDowngrade(ssl);
5202+
ret = Dtls13ClientDoDowngrade(ssl);
51975203
if (ret != 0)
51985204
return ret;
51995205
}
@@ -5266,7 +5272,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
52665272

52675273
#ifdef WOLFSSL_DTLS13
52685274
if (ssl->options.dtls) {
5269-
ret = Dtls13DoDowngrade(ssl);
5275+
ret = Dtls13ClientDoDowngrade(ssl);
52705276
if (ret != 0)
52715277
return ret;
52725278
}
@@ -6321,6 +6327,12 @@ int TlsCheckCookie(const WOLFSSL* ssl, const byte* cookie, word16 cookieSz)
63216327
byte cookieType = 0;
63226328
byte macSz = 0;
63236329

6330+
if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
6331+
ssl->buffers.tls13CookieSecret.length == 0) {
6332+
WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
6333+
return COOKIE_ERROR;
6334+
}
6335+
63246336
#if !defined(NO_SHA) && defined(NO_SHA256)
63256337
cookieType = SHA;
63266338
macSz = WC_SHA_DIGEST_SIZE;
@@ -6695,6 +6707,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
66956707
* wolfSSL_accept_TLSv13 when changing this one. */
66966708
if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie &&
66976709
!ssl->options.dtlsStateful) {
6710+
DtlsSetSeqNumForReply(ssl);
66986711
ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0, NULL);
66996712
if (ret != 0 || !ssl->options.dtlsStateful) {
67006713
*inOutIdx += helloSz;

‎tests/api.c‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68547,6 +68547,81 @@ static int test_dtls_dropped_ccs(void)
6854768547
#endif
6854868548
return EXPECT_RESULT();
6854968549
}
68550+
68551+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \
68552+
&& !defined(WOLFSSL_NO_TLS12)
68553+
static int test_dtls_seq_num_downgrade_check_num(byte* ioBuf, int ioBufLen,
68554+
byte seq_num)
68555+
{
68556+
EXPECT_DECLS;
68557+
DtlsRecordLayerHeader* dtlsRH;
68558+
byte sequence_number[8];
68559+
68560+
XMEMSET(&sequence_number, 0, sizeof(sequence_number));
68561+
68562+
ExpectIntGE(ioBufLen, sizeof(*dtlsRH));
68563+
dtlsRH = (DtlsRecordLayerHeader*)ioBuf;
68564+
ExpectIntEQ(dtlsRH->type, handshake);
68565+
ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR);
68566+
ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR);
68567+
sequence_number[7] = seq_num;
68568+
ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number,
68569+
sizeof(sequence_number)), 0);
68570+
68571+
return EXPECT_RESULT();
68572+
}
68573+
#endif
68574+
68575+
/*
68576+
* Make sure that we send the correct sequence number after a HelloVerifyRequest
68577+
* and after a HelloRetryRequest. This is testing the server side as it is
68578+
* operating statelessly and should copy the sequence number of the ClientHello.
68579+
*/
68580+
static int test_dtls_seq_num_downgrade(void)
68581+
{
68582+
EXPECT_DECLS;
68583+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \
68584+
&& !defined(WOLFSSL_NO_TLS12)
68585+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
68586+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
68587+
struct test_memio_ctx test_ctx;
68588+
68589+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
68590+
68591+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
68592+
wolfDTLSv1_2_client_method, wolfDTLS_server_method), 0);
68593+
68594+
/* CH1 */
68595+
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
68596+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
68597+
ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.s_buff,
68598+
test_ctx.s_len, 0), TEST_SUCCESS);
68599+
/* HVR */
68600+
ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
68601+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
68602+
ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.c_buff,
68603+
test_ctx.c_len, 0), TEST_SUCCESS);
68604+
/* CH2 */
68605+
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
68606+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
68607+
ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.s_buff,
68608+
test_ctx.s_len, 1), TEST_SUCCESS);
68609+
/* Server first flight */
68610+
ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
68611+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
68612+
ExpectIntEQ(test_dtls_seq_num_downgrade_check_num(test_ctx.c_buff,
68613+
test_ctx.c_len, 1), TEST_SUCCESS);
68614+
68615+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
68616+
68617+
wolfSSL_free(ssl_c);
68618+
wolfSSL_CTX_free(ctx_c);
68619+
wolfSSL_free(ssl_s);
68620+
wolfSSL_CTX_free(ctx_s);
68621+
#endif
68622+
return EXPECT_RESULT();
68623+
}
68624+
6855068625
/**
6855168626
* Make sure we don't send RSA Signature Hash Algorithms in the
6855268627
* CertificateRequest when we don't have any such ciphers set.
@@ -70649,6 +70724,7 @@ TEST_CASE testCases[] = {
7064970724
TEST_DECL(test_dtls_client_hello_timeout_downgrade),
7065070725
TEST_DECL(test_dtls_client_hello_timeout),
7065170726
TEST_DECL(test_dtls_dropped_ccs),
70727+
TEST_DECL(test_dtls_seq_num_downgrade),
7065270728
TEST_DECL(test_certreq_sighash_algos),
7065370729
TEST_DECL(test_revoked_loaded_int_cert),
7065470730
TEST_DECL(test_dtls_frag_ch),

0 commit comments

Comments
 (0)