Code review fixes

- Mark old epochs as invalid so we don't attempt to decrypt with them
- Return a non-zero value if possible in unit tests
- Move Dtls13CheckAEADFailLimit to dtls13.c
- Reset state in processreply
This commit is contained in:
Juliusz Sosinowicz
2022-09-15 11:34:30 +02:00
parent 63ba2f7b8f
commit 67473bac28
4 changed files with 166 additions and 112 deletions

View File

@@ -1846,6 +1846,20 @@ struct Dtls13Epoch* Dtls13GetEpoch(WOLFSSL* ssl, w64wrapper epochNumber)
return NULL;
}
void Dtls13SetOlderEpochSide(WOLFSSL* ssl, w64wrapper epochNumber,
int side)
{
Dtls13Epoch* e;
int i;
for (i = 0; i < DTLS13_EPOCH_SIZE; ++i) {
e = &ssl->dtls13Epochs[i];
if (e->isValid && w64LT(e->epochNumber, epochNumber)) {
e->side = (byte)side;
}
}
}
static void Dtls13EpochCopyKeys(WOLFSSL* ssl, Dtls13Epoch* e, Keys* k, int side)
{
byte clientWrite, serverWrite;
@@ -2000,8 +2014,10 @@ int Dtls13NewEpoch(WOLFSSL* ssl, w64wrapper epochNumber, int side)
#ifndef WOLFSSL_TLS13_IGNORE_AEAD_LIMITS
/* We are updating the receiving keys for this connection. We can restart
* the failed decryption counter. */
if (side == ENCRYPT_AND_DECRYPT_SIDE || side == DECRYPT_SIDE_ONLY)
* 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
@@ -2018,6 +2034,14 @@ int Dtls13NewEpoch(WOLFSSL* ssl, w64wrapper epochNumber, int side)
e->side = ENCRYPT_AND_DECRYPT_SIDE;
}
/* Once handshake is done. Mark epochs older than the last one as encrypt
* only so that they can't be used for decryption. */
if (ssl->options.handShakeDone && (e->side == ENCRYPT_AND_DECRYPT_SIDE ||
e->side == DECRYPT_SIDE_ONLY)) {
w64Decrement(&epochNumber);
Dtls13SetOlderEpochSide(ssl, epochNumber, ENCRYPT_SIDE_ONLY);
}
return 0;
}
@@ -2624,4 +2648,66 @@ int wolfSSL_dtls13_has_pending_msg(WOLFSSL* ssl)
return ssl->dtls13Rtx.rtxRecords != NULL;
}
#ifndef WOLFSSL_TLS13_IGNORE_AEAD_LIMITS
/* Limits specified by
* https://www.rfc-editor.org/rfc/rfc9147.html#name-aead-limits
* We specify the limit by which we need to do a key update as the halfway point
* to the hard decryption fail limit. */
int Dtls13CheckAEADFailLimit(WOLFSSL* ssl)
{
w64wrapper keyUpdateLimit;
w64wrapper hardLimit;
switch (ssl->specs.bulk_cipher_algorithm) {
#if defined(BUILD_AESGCM) || (defined(HAVE_CHACHA) && defined(HAVE_POLY1305))
case wolfssl_aes_gcm:
case wolfssl_chacha:
hardLimit = DTLS_AEAD_AES_GCM_CHACHA_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_GCM_CHACHA_FAIL_KU_LIMIT;
break;
#endif
#ifdef HAVE_AESCCM
case wolfssl_aes_ccm:
if (ssl->specs.aead_mac_size == AES_CCM_8_AUTH_SZ) {
/* Limit is 2^7. The RFC recommends that
* "TLS_AES_128_CCM_8_SHA256 is not suitable for general use".
* We still should enforce the limit. */
hardLimit = DTLS_AEAD_AES_CCM_8_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_CCM_8_FAIL_KU_LIMIT;
}
else {
/* Limit is 2^23.5.
* Without the fraction is 11863283 (0x00B504F3)
* Half of this value is 5931641 (0x005A8279) */
hardLimit = DTLS_AEAD_AES_CCM_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_CCM_FAIL_KU_LIMIT;
}
break;
#endif
case wolfssl_cipher_null:
/* No encryption being done. The MAC check must have failed. */
return 0;
default:
WOLFSSL_MSG("Unrecognized ciphersuite for AEAD limit check");
WOLFSSL_ERROR_VERBOSE(DECRYPT_ERROR);
return DECRYPT_ERROR;
}
if (w64GT(ssl->macDropCount, 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)) {
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) {
ssl->dtls13DoKeyUpdate = 1;
ssl->dtls13InvalidateBefore = ssl->dtls13PeerEpoch;
w64Increment(&ssl->dtls13InvalidateBefore);
}
}
return 0;
}
#endif
#endif /* WOLFSSL_DTLS13 */

View File

@@ -82,7 +82,8 @@
* possible attack.
* WOLFSSL_TLS13_IGNORE_AEAD_LIMITS
* Ignore the AEAD limits for messages specified in the RFC. After
* reaching the limit, we initiate a key update.
* reaching the limit, we initiate a key update. We enforce the AEAD limits
* by default.
* https://www.rfc-editor.org/rfc/rfc8446#section-5.5
* https://www.rfc-editor.org/rfc/rfc9147.html#name-aead-limits
*/
@@ -18192,6 +18193,24 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx, int sniff)
return OUT_OF_ORDER_E;
}
#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS)
/* Check if we want to invalidate old epochs. If
* ssl->dtls13InvalidateBefore is set then we want to mark all old
* epochs as encrypt only. This is done when we detect too many failed
* decryptions. We do this here to confirm that the peer has updated its
* keys and we can stop using the old keys. */
if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)) {
if (!w64IsZero(ssl->dtls13InvalidateBefore) &&
w64Equal(ssl->keys.curEpoch64, ssl->dtls13InvalidateBefore)) {
Dtls13SetOlderEpochSide(ssl, ssl->dtls13InvalidateBefore,
ENCRYPT_SIDE_ONLY);
w64Zero(&ssl->dtls13InvalidateBefore);
w64Zero(&ssl->macDropCount);
}
}
#endif
#ifndef WOLFSSL_AEAD_ONLY
if (ssl->specs.cipher_type == block) {
if (ssl->options.tls1_1)
@@ -18776,98 +18795,24 @@ static WC_INLINE int VerifyMac(WOLFSSL* ssl, const byte* input, word32 msgSz,
return 0;
}
enum {
AEAD_LIMIT_OK,
AEAD_LIMIT_KEY_UPDATE,
AEAD_LIMIT_FAIL,
};
#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS)
/* Limits specified by
* https://www.rfc-editor.org/rfc/rfc9147.html#name-aead-limits
* We specify the limit by which we need to do a key update as the halfway point
* to the hard decryption fail limit. */
static int checkDTLS13AEADFailLimit(byte bulk_cipher_algorithm,
word16 aead_mac_size, w64wrapper dropped)
{
w64wrapper keyUpdateLimit;
w64wrapper hardLimit;
switch (bulk_cipher_algorithm) {
#if defined(BUILD_AESGCM) || (defined(HAVE_CHACHA) && defined(HAVE_POLY1305))
case wolfssl_aes_gcm:
case wolfssl_chacha:
hardLimit = DTLS_AEAD_AES_GCM_CHACHA_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_GCM_CHACHA_FAIL_KU_LIMIT;
break;
#endif
#ifdef HAVE_AESCCM
case wolfssl_aes_ccm:
if (aead_mac_size == AES_CCM_8_AUTH_SZ) {
/* Limit is 2^7. The RFC recommends that
* "TLS_AES_128_CCM_8_SHA256 is not suitable for general use".
* We still should enforce the limit. */
hardLimit = DTLS_AEAD_AES_CCM_8_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_CCM_8_FAIL_KU_LIMIT;
}
else {
/* Limit is 2^23.5.
* Without the fraction is 11863283 (0x00B504F3)
* Half of this value is 5931641 (0x005A8279) */
hardLimit = DTLS_AEAD_AES_CCM_FAIL_LIMIT;
keyUpdateLimit = DTLS_AEAD_AES_CCM_FAIL_KU_LIMIT;
}
break;
#endif
case wolfssl_cipher_null:
/* No encryption being done. The MAC failed must have failed. */
return 0;
default:
WOLFSSL_MSG("Unrecognized ciphersuite for AEAD limit check");
return AEAD_LIMIT_FAIL;
}
if (w64GT(dropped, hardLimit)) {
WOLFSSL_MSG("Connection exceeded hard AEAD limit");
return AEAD_LIMIT_FAIL;
}
else if (w64GT(dropped, keyUpdateLimit)) {
WOLFSSL_MSG("Connection exceeded key update limit. Issuing key update");
return AEAD_LIMIT_KEY_UPDATE;
}
return AEAD_LIMIT_OK;
}
#endif
#ifdef WOLFSSL_DTLS
static int HandleDTLSDecryptFailed(WOLFSSL* ssl)
{
ssl->options.processReply = doProcessInit;
ssl->buffers.inputBuffer.idx = ssl->buffers.inputBuffer.length;
int ret = 0;
#if defined(WOLFSSL_DTLS_DROP_STATS) || \
(defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS))
w64Increment(&ssl->macDropCount);
#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)) {
int ret = checkDTLS13AEADFailLimit(ssl->specs.bulk_cipher_algorithm,
ssl->specs.aead_mac_size, ssl->macDropCount);
if (ret == AEAD_LIMIT_KEY_UPDATE) {
/* If not waiting for a response then request a key update. */
if (!ssl->keys.updateResponseReq)
ssl->dtls13DoKeyUpdate = 1;
}
else if (ret == AEAD_LIMIT_FAIL) {
/* We have reached the hard limit for failed decryptions. */
WOLFSSL_ERROR_VERBOSE(DECRYPT_ERROR);
return DECRYPT_ERROR;
}
}
if (IsAtLeastTLSv1_3(ssl->version))
ret = Dtls13CheckAEADFailLimit(ssl);
#endif
#endif
(void)ssl;
WOLFSSL_MSG("DTLS: Ignoring failed decryption");
return 0;
return ret;
}
#endif
@@ -19180,8 +19125,12 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
#ifdef WOLFSSL_DTLS
/* If in DTLS mode, if the decrypt fails for any
* reason, pretend the datagram never happened. */
if (ssl->options.dtls)
if (ssl->options.dtls) {
ssl->options.processReply = doProcessInit;
ssl->buffers.inputBuffer.idx =
ssl->buffers.inputBuffer.length;
return HandleDTLSDecryptFailed(ssl);
}
#endif /* WOLFSSL_DTLS */
#ifdef WOLFSSL_EXTRA_ALERTS
if (!ssl->options.dtls)
@@ -19336,8 +19285,12 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
#ifdef WOLFSSL_DTLS
/* If in DTLS mode, if the decrypt fails for any
* reason, pretend the datagram never happened. */
if (ssl->options.dtls)
if (ssl->options.dtls) {
ssl->options.processReply = doProcessInit;
ssl->buffers.inputBuffer.idx =
ssl->buffers.inputBuffer.length;
return HandleDTLSDecryptFailed(ssl);
}
#endif /* WOLFSSL_DTLS */
#ifdef WOLFSSL_EARLY_DATA
if (ssl->options.tls1_3) {
@@ -19402,8 +19355,12 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
#ifdef WOLFSSL_DTLS
/* If in DTLS mode, if the decrypt fails for any
* reason, pretend the datagram never happened. */
if (ssl->options.dtls)
if (ssl->options.dtls) {
ssl->options.processReply = doProcessInit;
ssl->buffers.inputBuffer.idx =
ssl->buffers.inputBuffer.length;
return HandleDTLSDecryptFailed(ssl);
}
#endif /* WOLFSSL_DTLS */
#ifdef WOLFSSL_EXTRA_ALERTS
if (!ssl->options.dtls)
@@ -22196,6 +22153,16 @@ int SendData(WOLFSSL* ssl, const void* data, int sz)
byte comp[MAX_RECORD_SIZE + MAX_COMP_EXTRA];
#endif
#if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS)
if (IsAtLeastTLSv1_3(ssl->version)) {
ret = CheckTLS13AEADSendLimit(ssl);
if (ret != 0) {
ssl->error = ret;
return WOLFSSL_FATAL_ERROR;
}
}
#endif
#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls && ssl->options.tls1_3) {
byte isEarlyData = 0;
@@ -22232,16 +22199,6 @@ int SendData(WOLFSSL* ssl, const void* data, int sz)
}
#endif /* WOLFSSL_DTLS13 */
#if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_TLS13_IGNORE_AEAD_LIMITS)
if (IsAtLeastTLSv1_3(ssl->version)) {
ret = CheckTLS13AEADSendLimit(ssl);
if (ret != 0) {
ssl->error = ret;
return WOLFSSL_FATAL_ERROR;
}
}
#endif
#ifdef WOLFSSL_DTLS
if (ssl->options.dtls) {
buffSz = wolfSSL_GetMaxFragSize(ssl, sz - sent);

View File

@@ -55129,8 +55129,10 @@ static int test_wolfSSL_dtls_plaintext(void)
test_wolfSSL_client_server_nofail(&func_cb_client, &func_cb_server);
AssertTrue(func_cb_client.return_code);
AssertTrue(func_cb_server.return_code);
if (!func_cb_client.return_code)
return -1;
if (!func_cb_server.return_code)
return -2;
}
printf(resultFmt, passed);
@@ -55397,42 +55399,43 @@ static void test_AEAD_limit_client(WOLFSSL* ssl)
ssl->macDropCount = keyUpdateLimit;
w64Increment(&ssl->macDropCount);
/* 100 read calls should be enough to complete the key update */
w64Zero(&counter);
for (i = 0; i < 100; i++) {
/* Key update should be sent and negotiated */
ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf));
AssertIntGT(ret, 0);
/* Epoch after one key update is 4 */
if (w64Equal(ssl->dtls13Epoch, w64From32(0, 4))) {
if (w64Equal(ssl->dtls13PeerEpoch, w64From32(0, 4)) &&
w64Equal(ssl->macDropCount, counter)) {
didReKey = 1;
break;
}
}
AssertTrue(didReKey);
w64Zero(&counter);
AssertTrue(w64Equal(ssl->macDropCount, counter));
if (!w64IsZero(sendLimit)) {
/* Test the sending limit for AEAD ciphers */
didReKey = 0;
AssertNotNull(ssl->dtls13EncryptEpoch);
ssl->dtls13EncryptEpoch->nextSeqNumber = sendLimit;
Dtls13Epoch* e = Dtls13GetEpoch(ssl, ssl->dtls13Epoch);
AssertNotNull(e);
e->nextSeqNumber = sendLimit;
test_AEAD_seq_num = 1;
ret = wolfSSL_write(ssl, msgBuf, sizeof(msgBuf));
AssertIntGT(ret, 0);
didReKey = 0;
w64Zero(&counter);
/* 100 read calls should be enough to complete the key update */
for (i = 0; i < 100; i++) {
/* Key update should be sent and negotiated */
ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf));
AssertIntGT(ret, 0);
/* Epoch after another key update is 5 */
if (w64Equal(ssl->dtls13Epoch, w64From32(0, 5))) {
if (w64Equal(ssl->dtls13Epoch, w64From32(0, 5)) &&
w64Equal(ssl->macDropCount, counter)) {
didReKey = 1;
break;
}
}
AssertTrue(didReKey);
w64Zero(&counter);
AssertTrue(w64Equal(ssl->macDropCount, counter));
}
test_AEAD_fail_decryption = 2;
@@ -55463,9 +55466,10 @@ static void test_AEAD_limit_server(WOLFSSL* ssl)
if (test_AEAD_seq_num) {
/* We need to update the seq number so that we can understand the
* peer. Otherwise we will incorrectly interpret the seq number. */
AssertNotNull(ssl->dtls13DecryptEpoch);
ssl->dtls13DecryptEpoch->nextPeerSeqNumber = sendLimit;
test_AEAD_seq_num = 1;
Dtls13Epoch* e = Dtls13GetEpoch(ssl, ssl->dtls13PeerEpoch);
AssertNotNull(e);
e->nextPeerSeqNumber = sendLimit;
test_AEAD_seq_num = 0;
}
(void)wolfSSL_read(ssl, msgBuf, sizeof(msgBuf));
ret = wolfSSL_write(ssl, msgBuf, sizeof(msgBuf));
@@ -55490,8 +55494,10 @@ static int test_wolfSSL_dtls_AEAD_limit(void)
test_wolfSSL_client_server_nofail(&func_cb_client, &func_cb_server);
AssertTrue(func_cb_client.return_code);
AssertTrue(func_cb_server.return_code);
if (!func_cb_client.return_code)
return -1;
if (!func_cb_server.return_code)
return -2;
printf(resultFmt, passed);

View File

@@ -4931,6 +4931,7 @@ struct WOLFSSL {
Dtls13Epoch *dtls13DecryptEpoch;
w64wrapper dtls13Epoch;
w64wrapper dtls13PeerEpoch;
w64wrapper dtls13InvalidateBefore;
byte dtls13CurRL[DTLS_RECVD_RL_HEADER_MAX_SZ];
word16 dtls13CurRlLength;
@@ -5749,8 +5750,11 @@ WOLFSSL_API int wolfSSL_DtlsUpdateWindow(word16 cur_hi, word32 cur_lo,
#ifdef WOLFSSL_DTLS13
WOLFSSL_LOCAL struct Dtls13Epoch* Dtls13GetEpoch(WOLFSSL* ssl,
/* Use WOLFSSL_API to use this function in tests/api.c */
WOLFSSL_API struct Dtls13Epoch* Dtls13GetEpoch(WOLFSSL* ssl,
w64wrapper epochNumber);
WOLFSSL_LOCAL void Dtls13SetOlderEpochSide(WOLFSSL* ssl, w64wrapper epochNumber,
int side);
WOLFSSL_LOCAL int Dtls13NewEpoch(WOLFSSL* ssl, w64wrapper epochNumber,
int side);
WOLFSSL_LOCAL int Dtls13SetEpochKeys(WOLFSSL* ssl, w64wrapper epochNumber,
@@ -5802,6 +5806,7 @@ WOLFSSL_LOCAL int Dtls13HashHandshake(WOLFSSL* ssl, const byte* output,
WOLFSSL_LOCAL void Dtls13FreeFsmResources(WOLFSSL* ssl);
WOLFSSL_LOCAL int Dtls13RtxTimeout(WOLFSSL* ssl);
WOLFSSL_LOCAL int Dtls13ProcessBufferedMessages(WOLFSSL* ssl);
WOLFSSL_LOCAL int Dtls13CheckAEADFailLimit(WOLFSSL* ssl);
#endif /* WOLFSSL_DTLS13 */
#ifdef WOLFSSL_STATIC_EPHEMERAL