From 1941fb2b35b82c81da71afb146d0c51c909c2bdb Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 15 Sep 2022 15:49:05 +0200 Subject: [PATCH] Keep a separate drop counter for each epoch --- src/dtls13.c | 20 +++++++++----------- src/internal.c | 8 +++----- src/ssl.c | 7 +------ tests/api.c | 20 +++++++++----------- wolfssl/internal.h | 8 +++++--- 5 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 18b392cc7..718e2bb05 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -2012,15 +2012,6 @@ int Dtls13NewEpoch(WOLFSSL* ssl, w64wrapper epochNumber, int side) return BAD_STATE_E; } -#ifndef WOLFSSL_TLS13_IGNORE_AEAD_LIMITS - /* We are updating the receiving keys for this connection. We can restart - * the failed decryption counter if we haven't reached the key update - * limit. */ - if ((side == ENCRYPT_AND_DECRYPT_SIDE || side == DECRYPT_SIDE_ONLY) && - w64IsZero(ssl->dtls13InvalidateBefore)) - w64Zero(&ssl->macDropCount); -#endif - Dtls13EpochCopyKeys(ssl, e, &ssl->keys, side); if (!e->isValid) { @@ -2691,13 +2682,20 @@ int Dtls13CheckAEADFailLimit(WOLFSSL* ssl) WOLFSSL_ERROR_VERBOSE(DECRYPT_ERROR); return DECRYPT_ERROR; } - if (w64GT(ssl->macDropCount, hardLimit)) { + if (ssl->dtls13DecryptEpoch == NULL) { + WOLFSSL_MSG("Dtls13CheckAEADFailLimit: ssl->dtls13DecryptEpoch should " + "not be NULL"); + WOLFSSL_ERROR_VERBOSE(BAD_STATE_E); + return BAD_STATE_E; + } + w64Increment(&ssl->dtls13DecryptEpoch->dropCount); + if (w64GT(ssl->dtls13DecryptEpoch->dropCount, hardLimit)) { /* We have reached the hard limit for failed decryptions. */ WOLFSSL_MSG("Connection exceeded hard AEAD limit"); WOLFSSL_ERROR_VERBOSE(DECRYPT_ERROR); return DECRYPT_ERROR; } - else if (w64GT(ssl->macDropCount, keyUpdateLimit)) { + else if (w64GT(ssl->dtls13DecryptEpoch->dropCount, keyUpdateLimit)) { WOLFSSL_MSG("Connection exceeded key update limit. Issuing key update"); /* If not waiting for a response then request a key update. */ if (!ssl->keys.updateResponseReq) { diff --git a/src/internal.c b/src/internal.c index 635a3b917..77b7a7414 100644 --- a/src/internal.c +++ b/src/internal.c @@ -18206,7 +18206,6 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx, int sniff) Dtls13SetOlderEpochSide(ssl, ssl->dtls13InvalidateBefore, ENCRYPT_SIDE_ONLY); w64Zero(&ssl->dtls13InvalidateBefore); - w64Zero(&ssl->macDropCount); } } #endif @@ -18799,15 +18798,14 @@ static WC_INLINE int VerifyMac(WOLFSSL* ssl, const byte* input, word32 msgSz, static int HandleDTLSDecryptFailed(WOLFSSL* ssl) { int ret = 0; -#if defined(WOLFSSL_DTLS_DROP_STATS) || \ - (defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS)) - w64Increment(&ssl->macDropCount); +#ifdef WOLFSSL_DTLS_DROP_STATS + ssl->macDropCount++; +#endif #if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS) /* Handle AEAD limits specified by the RFC for failed decryption */ if (IsAtLeastTLSv1_3(ssl->version)) ret = Dtls13CheckAEADFailLimit(ssl); -#endif #endif (void)ssl; diff --git a/src/ssl.c b/src/ssl.c index 27c988cb0..4d7bb73ce 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1534,7 +1534,7 @@ int wolfSSL_dtls_get_drop_stats(WOLFSSL* ssl, else { ret = WOLFSSL_SUCCESS; if (macDropCount != NULL) - *macDropCount = w64GetLow32(ssl->macDropCount); + *macDropCount = ssl->macDropCount; if (replayDropCount != NULL) *replayDropCount = ssl->replayDropCount; } @@ -3284,11 +3284,6 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl) /* CLIENT/SERVER: Reset peer authentication for full secure handshake. */ ssl->options.peerAuthGood = 0; -#ifdef WOLFSSL_DTLS_DROP_STATS - if (ssl->options.dtls) - w64Zero(&ssl->macDropCount); -#endif - #ifdef HAVE_SESSION_TICKET if (ret == WOLFSSL_SUCCESS) #endif diff --git a/tests/api.c b/tests/api.c index e320e9ad6..a4753972b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -55381,7 +55381,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) test_AEAD_get_limits(ssl, &hardLimit, &keyUpdateLimit, &sendLimit); w64Zero(&counter); - AssertTrue(w64Equal(ssl->macDropCount, counter)); + AssertTrue(w64Equal(Dtls13GetEpoch(ssl, ssl->dtls13Epoch)->dropCount, counter)); wolfSSL_SSLSetIORecv(ssl, test_AEAD_cbiorecv); @@ -55392,12 +55392,12 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf)); /* Should succeed since decryption failures are dropped */ AssertIntGT(ret, 0); - AssertTrue(w64Equal(ssl->macDropCount, counter)); + AssertTrue(w64Equal(Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount, counter)); } test_AEAD_fail_decryption = 1; - ssl->macDropCount = keyUpdateLimit; - w64Increment(&ssl->macDropCount); + Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount = keyUpdateLimit; + w64Increment(&Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount); /* 100 read calls should be enough to complete the key update */ w64Zero(&counter); for (i = 0; i < 100; i++) { @@ -55406,7 +55406,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) AssertIntGT(ret, 0); /* Epoch after one key update is 4 */ if (w64Equal(ssl->dtls13PeerEpoch, w64From32(0, 4)) && - w64Equal(ssl->macDropCount, counter)) { + w64Equal(Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount, counter)) { didReKey = 1; break; } @@ -55415,9 +55415,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) if (!w64IsZero(sendLimit)) { /* Test the sending limit for AEAD ciphers */ - Dtls13Epoch* e = Dtls13GetEpoch(ssl, ssl->dtls13Epoch); - AssertNotNull(e); - e->nextSeqNumber = sendLimit; + Dtls13GetEpoch(ssl, ssl->dtls13Epoch)->nextSeqNumber = sendLimit; test_AEAD_seq_num = 1; ret = wolfSSL_write(ssl, msgBuf, sizeof(msgBuf)); AssertIntGT(ret, 0); @@ -55430,7 +55428,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) AssertIntGT(ret, 0); /* Epoch after another key update is 5 */ if (w64Equal(ssl->dtls13Epoch, w64From32(0, 5)) && - w64Equal(ssl->macDropCount, counter)) { + w64Equal(Dtls13GetEpoch(ssl, ssl->dtls13Epoch)->dropCount, counter)) { didReKey = 1; break; } @@ -55439,8 +55437,8 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) } test_AEAD_fail_decryption = 2; - ssl->macDropCount = hardLimit; - w64Decrement(&ssl->macDropCount); + Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount = hardLimit; + w64Decrement(&Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch)->dropCount); /* Connection should fail with a DECRYPT_ERROR */ ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf)); AssertIntEQ(ret, WOLFSSL_FATAL_ERROR); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 54db08cf5..49e4c7779 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4651,6 +4651,10 @@ typedef struct Dtls13Epoch { w64wrapper nextSeqNumber; w64wrapper nextPeerSeqNumber; +#ifndef WOLFSSL_TLS13_IGNORE_AEAD_LIMITS + w64wrapper dropCount; /* Amount of records that failed decryption */ +#endif + word32 window[WOLFSSL_DTLS_WINDOW_WORDS]; /* key material for the epoch */ @@ -4912,10 +4916,8 @@ struct WOLFSSL { #ifdef WOLFSSL_MULTICAST void* mcastHwCbCtx; /* Multicast highwater callback ctx */ #endif /* WOLFSSL_MULTICAST */ -#if defined(WOLFSSL_DTLS_DROP_STATS) || defined(WOLFSSL_DTLS13) - w64wrapper macDropCount; -#endif #ifdef WOLFSSL_DTLS_DROP_STATS + word32 macDropCount; word32 replayDropCount; #endif /* WOLFSSL_DTLS_DROP_STATS */ #ifdef WOLFSSL_SRTP