Skip to content

Commit a21542c

Browse files
JacobBarthelmehJacobBarthelmeh
authored
Merge pull request #8689 from julek-wolfssl/gh/8666
dtls13: send acks with correct record number order
2 parents c414071 + 43c564d commit a21542c

5 files changed

Lines changed: 100 additions & 5 deletions

File tree

‎src/dtls13.c‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ static Dtls13RecordNumber* Dtls13NewRecordNumber(w64wrapper epoch,
718718
return rn;
719719
}
720720

721-
static int Dtls13RtxAddAck(WOLFSSL* ssl, w64wrapper epoch, w64wrapper seq)
721+
int Dtls13RtxAddAck(WOLFSSL* ssl, w64wrapper epoch, w64wrapper seq)
722722
{
723723
Dtls13RecordNumber* rn;
724724

@@ -728,12 +728,28 @@ static int Dtls13RtxAddAck(WOLFSSL* ssl, w64wrapper epoch, w64wrapper seq)
728728
if (wc_LockMutex(&ssl->dtls13Rtx.mutex) == 0)
729729
#endif
730730
{
731+
/* Find location to insert new record */
732+
Dtls13RecordNumber** prevNext = &ssl->dtls13Rtx.seenRecords;
733+
Dtls13RecordNumber* cur = ssl->dtls13Rtx.seenRecords;
734+
735+
for (; cur != NULL; prevNext = &cur->next, cur = cur->next) {
736+
if (w64Equal(cur->epoch, epoch) && w64Equal(cur->seq, seq)) {
737+
/* already in list. no duplicates. */
738+
return 0;
739+
}
740+
else if (w64LT(epoch, cur->epoch)
741+
|| (w64Equal(epoch, cur->epoch)
742+
&& w64LT(seq, cur->seq))) {
743+
break;
744+
}
745+
}
746+
731747
rn = Dtls13NewRecordNumber(epoch, seq, ssl->heap);
732748
if (rn == NULL)
733749
return MEMORY_E;
734750

735-
rn->next = ssl->dtls13Rtx.seenRecords;
736-
ssl->dtls13Rtx.seenRecords = rn;
751+
*prevNext = rn;
752+
rn->next = cur;
737753
#ifdef WOLFSSL_RW_THREADED
738754
wc_UnLockMutex(&ssl->dtls13Rtx.mutex);
739755
#endif
@@ -2522,7 +2538,7 @@ static int Dtls13GetAckListLength(Dtls13RecordNumber* list, word16* length)
25222538
return 0;
25232539
}
25242540

2525-
static int Dtls13WriteAckMessage(WOLFSSL* ssl,
2541+
int Dtls13WriteAckMessage(WOLFSSL* ssl,
25262542
Dtls13RecordNumber* recordNumberList, word32* length)
25272543
{
25282544
word16 msgSz, headerLength;

‎tests/api.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67812,6 +67812,7 @@ TEST_CASE testCases[] = {
6781267812
TEST_DECL(test_wolfSSL_inject),
6781367813
TEST_DECL(test_wolfSSL_dtls_cid_parse),
6781467814
TEST_DECL(test_dtls13_epochs),
67815+
TEST_DECL(test_dtls13_ack_order),
6781567816
TEST_DECL(test_ocsp_status_callback),
6781667817
TEST_DECL(test_ocsp_basic_verify),
6781767818
TEST_DECL(test_ocsp_response_parsing),

‎tests/api/test_dtls.c‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,3 +646,77 @@ int test_dtls13_epochs(void) {
646646
return EXPECT_RESULT();
647647
}
648648

649+
int test_dtls13_ack_order(void)
650+
{
651+
EXPECT_DECLS;
652+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
653+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
654+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
655+
struct test_memio_ctx test_ctx;
656+
unsigned char readBuf[50];
657+
word32 length = 0;
658+
/* struct {
659+
* uint64 epoch;
660+
* uint64 sequence_number;
661+
* } RecordNumber;
662+
* Big endian */
663+
unsigned char expected_output[] = {
664+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
665+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
666+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
667+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
668+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
669+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
670+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
671+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
672+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
673+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
674+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
675+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
676+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
677+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04,
678+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
679+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06,
680+
};
681+
682+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
683+
684+
/* Get a populated DTLS object */
685+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
686+
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
687+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
688+
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
689+
/* Clear the buffer of any extra messages */
690+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
691+
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), -1);
692+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
693+
ExpectIntEQ(test_ctx.c_len, 0);
694+
ExpectIntEQ(test_ctx.s_len, 0);
695+
696+
/* Add seen records */
697+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 2)), 0);
698+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 0)), 0);
699+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 1)), 0);
700+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 4)), 0);
701+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 2), w64From32(0, 0)), 0);
702+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 6)), 0);
703+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 3), w64From32(0, 6)), 0);
704+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 2), w64From32(0, 1)), 0);
705+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 2), w64From32(0, 2)), 0);
706+
ExpectIntEQ(Dtls13RtxAddAck(ssl_c, w64From32(0, 2), w64From32(0, 2)), 0);
707+
ExpectIntEQ(Dtls13WriteAckMessage(ssl_c, ssl_c->dtls13Rtx.seenRecords,
708+
&length), 0);
709+
/* N * RecordNumber + 2 extra bytes for length */
710+
ExpectIntEQ(length, sizeof(expected_output) + 2);
711+
ExpectNotNull(mymemmem(ssl_c->buffers.outputBuffer.buffer,
712+
ssl_c->buffers.outputBuffer.bufferSize, expected_output,
713+
sizeof(expected_output)));
714+
715+
716+
wolfSSL_free(ssl_c);
717+
wolfSSL_CTX_free(ctx_c);
718+
wolfSSL_free(ssl_s);
719+
wolfSSL_CTX_free(ctx_s);
720+
#endif
721+
return EXPECT_RESULT();
722+
}

‎tests/api/test_dtls.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,6 @@ int test_dtls12_basic_connection_id(void);
2626
int test_dtls13_basic_connection_id(void);
2727
int test_wolfSSL_dtls_cid_parse(void);
2828
int test_dtls13_epochs(void);
29+
int test_dtls13_ack_order(void);
2930

3031
#endif /* TESTS_API_DTLS_H */

‎wolfssl/internal.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6578,7 +6578,7 @@ WOLFSSL_LOCAL int TLSv1_3_Capable(WOLFSSL* ssl);
65786578
WOLFSSL_LOCAL void FreeHandshakeResources(WOLFSSL* ssl);
65796579
WOLFSSL_LOCAL void ShrinkInputBuffer(WOLFSSL* ssl, int forcedFree);
65806580
WOLFSSL_LOCAL void ShrinkOutputBuffer(WOLFSSL* ssl);
6581-
WOLFSSL_LOCAL byte* GetOutputBuffer(WOLFSSL* ssl);
6581+
WOLFSSL_TEST_VIS byte* GetOutputBuffer(WOLFSSL* ssl);
65826582

65836583
WOLFSSL_LOCAL int CipherRequires(byte first, byte second, int requirement);
65846584
WOLFSSL_LOCAL int VerifyClientSuite(word16 havePSK, byte cipherSuite0,
@@ -7066,7 +7066,10 @@ WOLFSSL_LOCAL int Dtls13ReconstructEpochNumber(WOLFSSL* ssl, byte epochBits,
70667066
w64wrapper* epoch);
70677067
WOLFSSL_LOCAL int Dtls13ReconstructSeqNumber(WOLFSSL* ssl,
70687068
Dtls13UnifiedHdrInfo* hdrInfo, w64wrapper* out);
7069+
WOLFSSL_TEST_VIS int Dtls13WriteAckMessage(WOLFSSL* ssl,
7070+
Dtls13RecordNumber* recordNumberList, word32* length);
70697071
WOLFSSL_LOCAL int SendDtls13Ack(WOLFSSL* ssl);
7072+
WOLFSSL_TEST_VIS int Dtls13RtxAddAck(WOLFSSL* ssl, w64wrapper epoch, w64wrapper seq);
70707073
WOLFSSL_LOCAL int Dtls13RtxProcessingCertificate(WOLFSSL* ssl, byte* input,
70717074
word32 inputSize);
70727075
WOLFSSL_LOCAL int Dtls13HashHandshake(WOLFSSL* ssl, const byte* input,

0 commit comments

Comments
 (0)