Skip to content

Commit c1c1929

Browse files
rizlikrizlik
committed
dtls13: move Dtls13NewEpoch into DeriveTls13Keys
Dlts13NewEpoch saves the keys currently derived in the ssl object. Moving Dtls13NewEpoch inside DeriveTls13Keys avoid the risk of using the wrong keys when creating a new Epoch. This fixes at least he following scenario: - Client has encryption epoch != 2 in the handshake (eg. due to rtx) - Client derives traffic0 keys after receiving server Finished message - Client set encryption epoch to 2 again to send the Finished message, this override the traffic key computed - Client creates the new epoch with the wrong key
1 parent 0bac2c2 commit c1c1929

5 files changed

Lines changed: 107 additions & 52 deletions

File tree

‎src/dtls13.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2618,19 +2618,16 @@ static int Dtls13RtxIsTrackedByRn(const Dtls13RtxRecord* r, w64wrapper epoch,
26182618
static int Dtls13KeyUpdateAckReceived(WOLFSSL* ssl)
26192619
{
26202620
int ret;
2621-
w64Increment(&ssl->dtls13Epoch);
2622-
2623-
/* Epoch wrapped up */
2624-
if (w64IsZero(ssl->dtls13Epoch))
2625-
return BAD_STATE_E;
26262621

26272622
ret = DeriveTls13Keys(ssl, update_traffic_key, ENCRYPT_SIDE_ONLY, 1);
26282623
if (ret != 0)
26292624
return ret;
26302625

2631-
ret = Dtls13NewEpoch(ssl, ssl->dtls13Epoch, ENCRYPT_SIDE_ONLY);
2632-
if (ret != 0)
2633-
return ret;
2626+
w64Increment(&ssl->dtls13Epoch);
2627+
2628+
/* Epoch wrapped up */
2629+
if (w64IsZero(ssl->dtls13Epoch))
2630+
return BAD_STATE_E;
26342631

26352632
return Dtls13SetEpochKeys(ssl, ssl->dtls13Epoch, ENCRYPT_SIDE_ONLY);
26362633
}

‎src/tls13.c‎

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,9 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
14711471
byte key_dig[MAX_PRF_DIG];
14721472
#endif
14731473
int provision;
1474+
#ifdef WOLFSSL_DTLS13
1475+
w64wrapper epochNumber;
1476+
#endif
14741477

14751478
#if defined(WOLFSSL_RENESAS_TSIP_TLS)
14761479
ret = tsip_Tls13DeriveKeys(ssl, secret, side);
@@ -1626,6 +1629,34 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
16261629
ret = Dtls13DeriveSnKeys(ssl, provision);
16271630
if (ret != 0)
16281631
return ret;
1632+
1633+
switch (secret) {
1634+
case early_data_key:
1635+
epochNumber = w64From32(0, DTLS13_EPOCH_EARLYDATA);
1636+
break;
1637+
case handshake_key:
1638+
epochNumber = w64From32(0, DTLS13_EPOCH_HANDSHAKE);
1639+
break;
1640+
case traffic_key:
1641+
case no_key:
1642+
epochNumber = w64From32(0, DTLS13_EPOCH_TRAFFIC0);
1643+
break;
1644+
case update_traffic_key:
1645+
if (side == ENCRYPT_SIDE_ONLY) {
1646+
epochNumber = ssl->dtls13Epoch;
1647+
}
1648+
else if (side == DECRYPT_SIDE_ONLY) {
1649+
epochNumber = ssl->dtls13PeerEpoch;
1650+
}
1651+
else {
1652+
return BAD_STATE_E;
1653+
}
1654+
w64Increment(&epochNumber);
1655+
break;
1656+
}
1657+
ret = Dtls13NewEpoch(ssl, epochNumber, side);
1658+
if (ret != 0)
1659+
return ret;
16291660
}
16301661

16311662
#endif /* WOLFSSL_DTLS13 */
@@ -4083,15 +4114,6 @@ static int WritePSKBinders(WOLFSSL* ssl, byte* output, word32 idx)
40834114
if ((ret = SetKeysSide(ssl, ENCRYPT_SIDE_ONLY)) != 0)
40844115
return ret;
40854116

4086-
#ifdef WOLFSSL_DTLS13
4087-
if (ssl->options.dtls) {
4088-
ret = Dtls13NewEpoch(
4089-
ssl, w64From32(0x0, DTLS13_EPOCH_EARLYDATA), ENCRYPT_SIDE_ONLY);
4090-
if (ret != 0)
4091-
return ret;
4092-
}
4093-
#endif /* WOLFSSL_DTLS13 */
4094-
40954117
}
40964118
#endif
40974119

@@ -6296,17 +6318,6 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz,
62966318
return ret;
62976319

62986320
ssl->keys.encryptionOn = 1;
6299-
6300-
#ifdef WOLFSSL_DTLS13
6301-
if (ssl->options.dtls) {
6302-
ret = Dtls13NewEpoch(ssl,
6303-
w64From32(0x0, DTLS13_EPOCH_EARLYDATA),
6304-
DECRYPT_SIDE_ONLY);
6305-
if (ret != 0)
6306-
return ret;
6307-
}
6308-
#endif /* WOLFSSL_DTLS13 */
6309-
63106321
ssl->earlyData = process_early_data;
63116322
}
63126323
else
@@ -7604,11 +7615,6 @@ static int SendTls13EncryptedExtensions(WOLFSSL* ssl)
76047615
w64wrapper epochHandshake = w64From32(0, DTLS13_EPOCH_HANDSHAKE);
76057616
ssl->dtls13Epoch = epochHandshake;
76067617

7607-
ret = Dtls13NewEpoch(
7608-
ssl, epochHandshake, ENCRYPT_AND_DECRYPT_SIDE);
7609-
if (ret != 0)
7610-
return ret;
7611-
76127618
ret = Dtls13SetEpochKeys(
76137619
ssl, epochHandshake, ENCRYPT_AND_DECRYPT_SIDE);
76147620
if (ret != 0)
@@ -11194,11 +11200,6 @@ static int SendTls13Finished(WOLFSSL* ssl)
1119411200
ssl->dtls13Epoch = epochTraffic0;
1119511201
ssl->dtls13PeerEpoch = epochTraffic0;
1119611202

11197-
ret = Dtls13NewEpoch(
11198-
ssl, epochTraffic0, ENCRYPT_AND_DECRYPT_SIDE);
11199-
if (ret != 0)
11200-
return ret;
11201-
1120211203
ret = Dtls13SetEpochKeys(
1120311204
ssl, epochTraffic0, ENCRYPT_AND_DECRYPT_SIDE);
1120411205
if (ret != 0)
@@ -11236,11 +11237,6 @@ static int SendTls13Finished(WOLFSSL* ssl)
1123611237
ssl->dtls13Epoch = epochTraffic0;
1123711238
ssl->dtls13PeerEpoch = epochTraffic0;
1123811239

11239-
ret = Dtls13NewEpoch(
11240-
ssl, epochTraffic0, ENCRYPT_AND_DECRYPT_SIDE);
11241-
if (ret != 0)
11242-
return ret;
11243-
1124411240
ret = Dtls13SetEpochKeys(
1124511241
ssl, epochTraffic0, ENCRYPT_AND_DECRYPT_SIDE);
1124611242
if (ret != 0)
@@ -11440,10 +11436,6 @@ static int DoTls13KeyUpdate(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
1144011436
if (ssl->options.dtls) {
1144111437
w64Increment(&ssl->dtls13PeerEpoch);
1144211438

11443-
ret = Dtls13NewEpoch(ssl, ssl->dtls13PeerEpoch, DECRYPT_SIDE_ONLY);
11444-
if (ret != 0)
11445-
return ret;
11446-
1144711439
ret = Dtls13SetEpochKeys(ssl, ssl->dtls13PeerEpoch, DECRYPT_SIDE_ONLY);
1144811440
if (ret != 0)
1144911441
return ret;
@@ -12859,11 +12851,6 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
1285912851
ssl->dtls13Epoch = epochHandshake;
1286012852
ssl->dtls13PeerEpoch = epochHandshake;
1286112853

12862-
ret = Dtls13NewEpoch(
12863-
ssl, epochHandshake, ENCRYPT_AND_DECRYPT_SIDE);
12864-
if (ret != 0)
12865-
return ret;
12866-
1286712854
ret = Dtls13SetEpochKeys(
1286812855
ssl, epochHandshake, ENCRYPT_AND_DECRYPT_SIDE);
1286912856
if (ret != 0)

‎tests/api.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68274,6 +68274,7 @@ TEST_CASE testCases[] = {
6827468274
TEST_DECL(test_wolfSSL_inject),
6827568275
TEST_DECL(test_wolfSSL_dtls_cid_parse),
6827668276
TEST_DECL(test_dtls13_epochs),
68277+
TEST_DECL(test_dtls_rtx_across_epoch_change),
6827768278
TEST_DECL(test_dtls13_ack_order),
6827868279
TEST_DECL(test_dtls_version_checking),
6827968280
TEST_DECL(test_ocsp_status_callback),

‎tests/api/test_dtls.c‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,3 +1313,72 @@ int test_records_span_network_boundaries(void)
13131313
}
13141314
#endif /* defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
13151315
!defined(WOLFSSL_NO_TLS12) */
1316+
1317+
int test_dtls_rtx_across_epoch_change(void)
1318+
{
1319+
EXPECT_DECLS;
1320+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
1321+
defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS)
1322+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
1323+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
1324+
struct test_memio_ctx test_ctx;
1325+
1326+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
1327+
1328+
/* Setup DTLS contexts */
1329+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
1330+
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method),
1331+
0);
1332+
1333+
/* CH0 */
1334+
wolfSSL_SetLoggingPrefix("client:");
1335+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
1336+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), SSL_ERROR_WANT_READ);
1337+
1338+
/* SH */
1339+
wolfSSL_SetLoggingPrefix("server:");
1340+
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
1341+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), SSL_ERROR_WANT_READ);
1342+
1343+
/* CH1 */
1344+
wolfSSL_SetLoggingPrefix("client:");
1345+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
1346+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), SSL_ERROR_WANT_READ);
1347+
1348+
wolfSSL_SetLoggingPrefix("server:");
1349+
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
1350+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), SSL_ERROR_WANT_READ);
1351+
1352+
/* we should have now SH ... FINISHED messages in the buffer*/
1353+
ExpectIntGE(test_ctx.c_msg_count, 2);
1354+
1355+
/* now let's drop everything but the SH */
1356+
while (test_ctx.c_msg_count > 1 && EXPECT_SUCCESS()) {
1357+
ExpectIntEQ(test_memio_drop_message(&test_ctx, 1, test_ctx.c_msg_count - 1), 0);
1358+
}
1359+
1360+
/* Read the SH */
1361+
wolfSSL_SetLoggingPrefix("client:");
1362+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
1363+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), SSL_ERROR_WANT_READ);
1364+
1365+
/* trigger client timeout */
1366+
ExpectIntEQ(wolfSSL_dtls_got_timeout(ssl_c), WOLFSSL_SUCCESS);
1367+
/* this should have triggered a rtx */
1368+
ExpectIntGT(test_ctx.s_msg_count, 0);
1369+
1370+
/* finish the handshake */
1371+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
1372+
1373+
/* Test communication works correctly */
1374+
ExpectIntEQ(test_dtls_communication(ssl_s, ssl_c), TEST_SUCCESS);
1375+
1376+
/* Cleanup */
1377+
wolfSSL_free(ssl_c);
1378+
wolfSSL_CTX_free(ctx_c);
1379+
wolfSSL_free(ssl_s);
1380+
wolfSSL_CTX_free(ctx_s);
1381+
#endif /* defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
1382+
defined(WOLFSSL_DTLS13) */
1383+
return EXPECT_RESULT();
1384+
}

‎tests/api/test_dtls.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,5 @@ int test_dtls13_longer_length(void);
3636
int test_dtls13_short_read(void);
3737
int test_records_span_network_boundaries(void);
3838
int test_dtls_record_cross_boundaries(void);
39+
int test_dtls_rtx_across_epoch_change(void);
3940
#endif /* TESTS_API_DTLS_H */

0 commit comments

Comments
 (0)