Skip to content

dtls13: epoch mangament improvement and bug fixes #11016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rizlik wants to merge 1 commit into
base: master
Choose a base branch
from
+318 −9
Open

dtls13: epoch mangament improvement and bug fixes #11016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/dtls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,10 @@ int Dtls13ReconstructEpochNumber(WOLFSSL* ssl, byte epochBits,
if (!e->isValid)
continue;

/* consider only epoch that can decrypt */
if (e->side == ENCRYPT_SIDE_ONLY)
continue;

if (Dtls13GetEpochBits(e->epochNumber) != epochBits)
continue;

Expand Down Expand Up @@ -2399,19 +2403,27 @@ int Dtls13GetSeq(WOLFSSL* ssl, int order, word32* seq, byte increment)
static Dtls13Epoch* Dtls13NewEpochSlot(WOLFSSL* ssl)
{
Dtls13Epoch *e, *oldest = NULL;
w64wrapper oldestNumber;
w64wrapper oldestNumber, prevPeerEpoch;
int i;

oldestNumber = w64From32((word32)-1, (word32)-1);
oldest = NULL;

/* local peer advances the remote peer epoch when receiving KeyUpdate but
* the remote peer advances its own sending epoch only after receiving our
* ACK. Preserve peer epoch - 1 in case the ACK gets lost */
prevPeerEpoch = ssl->dtls13PeerEpoch;
if (!w64IsZero(prevPeerEpoch))
w64Decrement(&prevPeerEpoch);

for (i = 0; i < DTLS13_EPOCH_SIZE; ++i) {
e = &ssl->dtls13Epochs[i];
if (!e->isValid)
return e;

if (!w64Equal(e->epochNumber, ssl->dtls13Epoch) &&
!w64Equal(e->epochNumber, ssl->dtls13PeerEpoch) &&
!w64Equal(e->epochNumber, prevPeerEpoch) &&
w64LT(e->epochNumber, oldestNumber)) {
oldest = e;
oldestNumber = e->epochNumber;
Expand All @@ -2427,6 +2439,10 @@ static Dtls13Epoch* Dtls13NewEpochSlot(WOLFSSL* ssl)
WOLFSSL_MSG_EX("Delete epoch: %d", e->epochNumber);
#endif /* WOLFSSL_DEBUG_TLS */

/* invalidate dtls13DecryptEpoch if pointing to the evicted slot */
if (ssl->dtls13DecryptEpoch == e)
ssl->dtls13DecryptEpoch = NULL;

/* The slot we are reusing holds the previous epoch's symmetric keys, IVs,
* and sn-keys; use ForceZero so the wipe cannot be elided by the
* optimizer when the slot is later overwritten. */
Expand Down
20 changes: 14 additions & 6 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12456,9 +12456,6 @@ static int GetDtls13RecordHeader(WOLFSSL* ssl, word32* inOutIdx,
if (w64IsZero(epochNumber))
return SEQUENCE_ERROR;

if (ssl->dtls13DecryptEpoch == NULL)
return BAD_STATE_E;

#ifdef WOLFSSL_EARLY_DATA
if (w64Equal(epochNumber, w64From32(0x0, DTLS13_EPOCH_EARLYDATA)) &&
ssl->options.handShakeDone) {
Expand All @@ -12467,7 +12464,8 @@ static int GetDtls13RecordHeader(WOLFSSL* ssl, word32* inOutIdx,
}
#endif /* WOLFSSL_DTLS13 */

if (!w64Equal(ssl->dtls13DecryptEpoch->epochNumber, epochNumber)) {
if (ssl->dtls13DecryptEpoch == NULL ||
!w64Equal(ssl->dtls13DecryptEpoch->epochNumber, epochNumber)) {
ret = Dtls13SetEpochKeys(ssl, epochNumber, DECRYPT_SIDE_ONLY);
if (ret != 0)
return SEQUENCE_ERROR;
Expand Down Expand Up @@ -12615,8 +12613,18 @@ static int GetDtlsRecordHeader(WOLFSSL* ssl, word32* inOutIdx,
return SEQUENCE_ERROR;

w64Zero(&ssl->keys.curEpoch64);
if (!w64IsZero(ssl->dtls13DecryptEpoch->epochNumber))
Dtls13SetEpochKeys(ssl, ssl->keys.curEpoch64, DECRYPT_SIDE_ONLY);

/* no plaintext messages after the handshake is done */
if (ssl->options.handShakeDone)
return SEQUENCE_ERROR;

if (ssl->dtls13DecryptEpoch == NULL ||
!w64IsZero(ssl->dtls13DecryptEpoch->epochNumber)) {
ret = Dtls13SetEpochKeys(ssl, ssl->keys.curEpoch64,
DECRYPT_SIDE_ONLY);
if (ret != 0)
return SEQUENCE_ERROR;
}
}
#endif /* WOLFSSL_DTLS13 */

Expand Down
273 changes: 273 additions & 0 deletions tests/api/test_dtls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,3 +2033,276 @@ int test_dtls13_5_9_0_compat_empty_echo(void)
#endif
return EXPECT_RESULT();
}

#define TEST_DTLS13_KEY_UPDATE_ROUNDS DTLS13_EPOCH_SIZE

int test_dtls13_epoch_slot_reuse_replay(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
const char msg[] = "APP-DATA-REPLAY-CANARY";
const int msgLen = (int)sizeof(msg) - 1;
const w64wrapper retiredEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0);
const w64wrapper peerEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0 + 1);
char replay[512];
int replayLen = (int)sizeof(replay);
char readBuf[64];
int i;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

for (i = 0; i < 4; i++) {
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
}
ExpectIntEQ(test_ctx.c_len, 0);
ExpectIntEQ(test_ctx.s_len, 0);

/* Capture an application-data record sent in the peer's current epoch. */
ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
ExpectIntEQ(test_memio_copy_message(&test_ctx, 0, replay, &replayLen, 0), 0);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);

/* Control: while that epoch is still live, the replay window rejects it. */
ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, replay, replayLen), 0);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);

/* this avoid the client to request the server to send a keyUpdate back */
if (ssl_c != NULL)
ssl_c->keys.updateResponseReq = 1;
ExpectIntEQ(wolfSSL_update_keys(ssl_c), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
/* drop the ACK */
test_memio_clear_buffer(&test_ctx, 1);
ExpectTrue(w64Equal(ssl_s->dtls13PeerEpoch, peerEpoch));
ExpectTrue(w64Equal(ssl_c->dtls13Epoch, retiredEpoch));
ExpectIntEQ(ssl_c->dtls13WaitKeyUpdateAck, 1);

/* That still-in-flight epoch is what parks ssl->dtls13DecryptEpoch on the
* slot Dtls13NewEpochSlot() is about to consider for eviction. */
ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);
ExpectTrue(w64Equal(ssl_s->dtls13DecryptEpoch->epochNumber, retiredEpoch));

/* Server key updates. PeerEpoch, PeerEpoch - 1 and Epoch must be preserved */
for (i = 0; i < TEST_DTLS13_KEY_UPDATE_ROUNDS; i++) {
if (ssl_s != NULL)
ssl_s->keys.updateResponseReq = 1;
ExpectIntEQ(wolfSSL_update_keys(ssl_s), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
/* wolfSSL_update_keys() reports success without sending while an
* earlier KeyUpdate is unacked, so check the round actually closed. */
ExpectIntEQ(ssl_s->dtls13WaitKeyUpdateAck, 0);
ExpectTrue(w64Equal(ssl_s->dtls13Epoch,
w64From32(0, DTLS13_EPOCH_TRAFFIC0 + (word32)i + 1)));
}

ExpectNull(Dtls13GetEpoch(ssl_s, w64From32(0, 0)));
ExpectTrue(w64Equal(ssl_s->dtls13PeerEpoch, peerEpoch));
ExpectTrue(w64Equal(ssl_c->dtls13Epoch, retiredEpoch));

/* The retired epoch's record must not be delivered a second time. */
ExpectIntEQ(test_memio_inject_message(&test_ctx, 0, replay, replayLen), 0);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);

/* The peer is still transmitting in that epoch, so it must keep working.
* Dropping its traffic instead of the replay is not a fix. */
test_memio_clear_buffer(&test_ctx, 0);
ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}

int test_dtls13_epoch_slot_reuse_decrypt_epoch(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
const char msg[] = "APP-DATA-AFTER-SLOT-REUSE";
const int msgLen = (int)sizeof(msg) - 1;
const w64wrapper plaintextEpoch = w64From32(0, 0);
const w64wrapper trafficEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0);
const w64wrapper ownEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0 + 1);
const w64wrapper peerEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0 + 2);
byte plaintextRec[DTLS_RECORD_HEADER_SZ];
char readBuf[64];
int i;

XMEMSET(plaintextRec, 0, sizeof(plaintextRec));
plaintextRec[0] = handshake;
plaintextRec[1] = DTLS_MAJOR;
plaintextRec[2] = DTLSv1_2_MINOR;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

for (i = 0; i < 4; i++) {
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
}
ExpectIntEQ(test_ctx.c_len, 0);
ExpectIntEQ(test_ctx.s_len, 0);

ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);
ExpectTrue(w64Equal(ssl_s->dtls13DecryptEpoch->epochNumber, trafficEpoch));

if (ssl_s != NULL)
ssl_s->keys.updateResponseReq = 1;
ExpectIntEQ(wolfSSL_update_keys(ssl_s), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(ssl_s->dtls13WaitKeyUpdateAck, 0);
ExpectTrue(w64Equal(ssl_s->dtls13Epoch, ownEpoch));

for (i = 0; i < 2; i++) {
if (ssl_c != NULL)
ssl_c->keys.updateResponseReq = 1;
ExpectIntEQ(wolfSSL_update_keys(ssl_c), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(ssl_c->dtls13WaitKeyUpdateAck, 0);
}
ExpectTrue(w64Equal(ssl_s->dtls13Epoch, ownEpoch));
ExpectTrue(w64Equal(ssl_s->dtls13PeerEpoch, peerEpoch));
ExpectTrue(w64Equal(ssl_c->dtls13Epoch, peerEpoch));
ExpectNotNull(Dtls13GetEpoch(ssl_s, trafficEpoch));

if (ssl_s != NULL)
ssl_s->dtls13DecryptEpoch = Dtls13GetEpoch(ssl_s, trafficEpoch);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);

for (i = 0; ssl_s != NULL && i < 2 * DTLS13_EPOCH_SIZE &&
Dtls13GetEpoch(ssl_s, trafficEpoch) != NULL; i++) {
ExpectIntEQ(Dtls13NewEpoch(ssl_s,
w64From32(0, DTLS13_EPOCH_TRAFFIC0 + 4 + (word32)i),
ENCRYPT_SIDE_ONLY), 0);
}
ExpectNull(Dtls13GetEpoch(ssl_s, trafficEpoch));
ExpectNull(Dtls13GetEpoch(ssl_s, plaintextEpoch));
ExpectNull(ssl_s->dtls13DecryptEpoch);

ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
(const char*)plaintextRec, (int)sizeof(plaintextRec)), 0);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
ExpectNull(ssl_s->dtls13DecryptEpoch);

ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);
ExpectTrue(w64Equal(ssl_s->dtls13DecryptEpoch->epochNumber, peerEpoch));

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}

int test_dtls13_plaintext_ack_after_handshake(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
const char msg[] = "APP-DATA-AFTER-PLAINTEXT-ACK";
const int msgLen = (int)sizeof(msg) - 1;
const w64wrapper plaintextEpoch = w64From32(0, 0);
const w64wrapper trafficEpoch = w64From32(0, DTLS13_EPOCH_TRAFFIC0);
/* ahead of the receiving window of every epoch of this connection */
const word32 ackSeq = 1000;
/* DTLSPlaintext record holding an ACK with an empty record_numbers list */
byte plaintextAck[DTLS_RECORD_HEADER_SZ + OPAQUE16_LEN];
char readBuf[64];
int i;

XMEMSET(plaintextAck, 0, sizeof(plaintextAck));
plaintextAck[0] = ack;
plaintextAck[1] = DTLS_MAJOR;
plaintextAck[2] = DTLSv1_2_MINOR;
/* epoch (2 bytes) is 0, sequence number is the low 32 bits of the 48 bit
* field that follows it */
c32toa(ackSeq, plaintextAck + ENUM_LEN + VERSION_SZ + OPAQUE16_LEN +
OPAQUE16_LEN);
c16toa(OPAQUE16_LEN, plaintextAck + DTLS_RECORD_HEADER_SZ - LENGTH_SZ);

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

for (i = 0; i < 4; i++) {
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
}
ExpectIntEQ(test_ctx.c_len, 0);
ExpectIntEQ(test_ctx.s_len, 0);

ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);
ExpectTrue(w64Equal(ssl_s->dtls13DecryptEpoch->epochNumber, trafficEpoch));

/* The epoch 0 slot is still around at this point, so setting it as the
* decrypting epoch would succeed. An unprotected record received after the
* handshake must be dropped before that: no record of epoch 0 can be
* accepted anymore, and epoch 0 may well have been recycled already. */
ExpectNotNull(Dtls13GetEpoch(ssl_s, plaintextEpoch));
ExpectIntEQ(test_memio_inject_message(&test_ctx, 0,
(const char*)plaintextAck, (int)sizeof(plaintextAck)), 0);
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
ExpectNotNull(ssl_s->dtls13DecryptEpoch);
ExpectTrue(w64Equal(ssl_s->dtls13DecryptEpoch->epochNumber, trafficEpoch));

/* the connection must keep working */
ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgLen), msgLen);
XMEMSET(readBuf, 0, sizeof(readBuf));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen);
ExpectStrEQ(readBuf, msg);

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}
8 changes: 7 additions & 1 deletion tests/api/test_dtls13.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ int test_dtls13_no_session_id_echo(void);
int test_dtls13_5_9_0_compat(void);
int test_dtls13_5_9_0_compat_bad_echo(void);
int test_dtls13_5_9_0_compat_empty_echo(void);
int test_dtls13_epoch_slot_reuse_replay(void);
int test_dtls13_epoch_slot_reuse_decrypt_epoch(void);
int test_dtls13_plaintext_ack_after_handshake(void);

#define TEST_DTLS13_DECLS \
TEST_DECL_GROUP("dtls13", test_dtls13_bad_epoch_ch), \
Expand All @@ -80,6 +83,9 @@ int test_dtls13_5_9_0_compat_empty_echo(void);
TEST_DECL_GROUP("dtls13", test_dtls13_no_session_id_echo), \
TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat), \
TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_bad_echo), \
TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_empty_echo)
TEST_DECL_GROUP("dtls13", test_dtls13_5_9_0_compat_empty_echo), \
TEST_DECL_GROUP("dtls13", test_dtls13_epoch_slot_reuse_replay), \
TEST_DECL_GROUP("dtls13", test_dtls13_epoch_slot_reuse_decrypt_epoch), \
TEST_DECL_GROUP("dtls13", test_dtls13_plaintext_ack_after_handshake)

#endif /* TESTS_API_DTLS13_H */
Loading
Loading