From 2f37ab38fd537a636da2845c49f554974985ffae Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 27 Mar 2026 12:55:04 +0100 Subject: [PATCH] DTLS Bucket improvements - test_wolfSSL_DTLS_fragment_buckets: rewrite to use Expect framework - Correctly handle buckets between other buckets that don't touch - Fix DTLS fragment combine when data overlaps cur from the left --- src/internal.c | 20 +++--- tests/api.c | 185 ++++++++++++++++++++++++++++++------------------- 2 files changed, 125 insertions(+), 80 deletions(-) diff --git a/src/internal.c b/src/internal.c index ccc010acf7..19d7e99973 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9701,8 +9701,9 @@ static DtlsFragBucket* DtlsMsgCombineFragBuckets(DtlsMsg* msg, } else { /* data -> cur. memcpy as much possible as its faster. */ - XMEMMOVE(newBucket->buf + dataSz, cur->buf, - cur->m.m.sz - (offsetEnd - cur->m.m.offset)); + word32 skipInCur = offsetEnd - cur->m.m.offset; + XMEMMOVE(newBucket->buf + dataSz, cur->buf + skipInCur, + cur->m.m.sz - skipInCur); XMEMCPY(newBucket->buf, data, dataSz); } } @@ -9863,22 +9864,25 @@ int DtlsMsgSet(DtlsMsg* msg, word32 seq, word16 epoch, const byte* data, byte ty msg->fragBucketListCount++; } } - else if (prev == NULL && fragOffsetEnd < cur->m.m.offset) { - /* This is the new first fragment we have received */ + else if (fragOffsetEnd < cur->m.m.offset) { + /* Fragment is entirely before cur with a gap */ + DtlsFragBucket** prev_next; if (msg->fragBucketListCount >= DTLS_FRAG_POOL_SZ) { WOLFSSL_ERROR_VERBOSE(DTLS_TOO_MANY_FRAGMENTS_E); return DTLS_TOO_MANY_FRAGMENTS_E; } - msg->fragBucketList = DtlsMsgCreateFragBucket(fragOffset, data, + prev_next = prev != NULL + ? &prev->m.m.next : &msg->fragBucketList; + *prev_next = DtlsMsgCreateFragBucket(fragOffset, data, fragSz, heap); - if (msg->fragBucketList != NULL) { - msg->fragBucketList->m.m.next = cur; + if (*prev_next != NULL) { + (*prev_next)->m.m.next = cur; msg->bytesReceived += fragSz; msg->fragBucketListCount++; } else { /* reset on error */ - msg->fragBucketList = cur; + *prev_next = cur; } } else { diff --git a/tests/api.c b/tests/api.c index 91315cf464..0459c09fd9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -28829,7 +28829,8 @@ static int test_wolfSSL_DtlsUpdateWindow(void) static int DFB_TEST(WOLFSSL* ssl, word32 seq, word32 len, word32 f_offset, word32 f_len, word32 f_count, byte ready, word32 bytesReceived) { - DtlsMsg* cur; + EXPECT_DECLS; + DtlsMsg* cur = NULL; static byte msg[100]; static byte msgInit = 0; @@ -28841,40 +28842,34 @@ static int DFB_TEST(WOLFSSL* ssl, word32 seq, word32 len, word32 f_offset, } /* Sanitize test parameters */ - if (len > sizeof(msg)) - return -1; - if (f_offset + f_len > sizeof(msg)) - return -1; + ExpectIntLE(len, sizeof(msg)); + ExpectIntLE(f_offset + f_len, sizeof(msg)); - DtlsMsgStore(ssl, 0, seq, msg + f_offset, len, certificate, f_offset, f_len, NULL); + if (EXPECT_SUCCESS()) + DtlsMsgStore(ssl, 0, seq, msg + f_offset, len, certificate, f_offset, f_len, NULL); - if (ssl->dtls_rx_msg_list == NULL) - return -100; + ExpectNotNull(ssl->dtls_rx_msg_list); - if ((cur = DtlsMsgFind(ssl->dtls_rx_msg_list, 0, seq)) == NULL) - return -200; - if (cur->fragBucketListCount != f_count) - return -300; - if (cur->ready != ready) - return -400; - if (cur->bytesReceived != bytesReceived) - return -500; + ExpectNotNull(cur = DtlsMsgFind(ssl->dtls_rx_msg_list, 0, seq)); + ExpectIntEQ(cur->fragBucketListCount, f_count); + ExpectIntEQ(cur->ready, ready); + ExpectIntEQ(cur->bytesReceived, bytesReceived); if (ready) { - if (cur->fragBucketList != NULL) - return -600; - if (XMEMCMP(cur->fullMsg, msg, cur->sz) != 0) - return -700; + ExpectNull(cur->fragBucketList); + ExpectBufEQ(cur->fullMsg, msg, cur->sz); } else { DtlsFragBucket* fb; - if (cur->fragBucketList == NULL) - return -800; - for (fb = cur->fragBucketList; fb != NULL; fb = fb->m.m.next) { - if (XMEMCMP(fb->buf, msg + fb->m.m.offset, fb->m.m.sz) != 0) - return -900; - } + ExpectNotNull(cur->fragBucketList); + for (fb = cur != NULL ? cur->fragBucketList : NULL; + EXPECT_SUCCESS() && fb != NULL; fb = fb->m.m.next) + ExpectBufEQ(fb->buf, msg + fb->m.m.offset, fb->m.m.sz); } - return 0; + if (EXPECT_FAIL()) { + printf("Test parameters: seq %u len %u f_offset %u f_len %u f_count %u ready %u bytesReceived %u\n", + seq, len, f_offset, f_len, f_count, ready, bytesReceived); + } + return EXPECT_RESULT(); } static int test_wolfSSL_DTLS_fragment_buckets(void) @@ -28884,68 +28879,114 @@ static int test_wolfSSL_DTLS_fragment_buckets(void) XMEMSET(ssl, 0, sizeof(*ssl)); - ExpectIntEQ(DFB_TEST(ssl, 0, 100, 0, 100, 0, 1, 100), 0); /* 0-100 */ + EXPECT_TEST(DFB_TEST(ssl, 0, 100, 0, 100, 0, 1, 100)); /* 0-100 */ - ExpectIntEQ(DFB_TEST(ssl, 1, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */ - ExpectIntEQ(DFB_TEST(ssl, 1, 100, 20, 20, 1, 0, 40), 0); /* 20-40 */ - ExpectIntEQ(DFB_TEST(ssl, 1, 100, 40, 20, 1, 0, 60), 0); /* 40-60 */ - ExpectIntEQ(DFB_TEST(ssl, 1, 100, 60, 20, 1, 0, 80), 0); /* 60-80 */ - ExpectIntEQ(DFB_TEST(ssl, 1, 100, 80, 20, 0, 1, 100), 0); /* 80-100 */ + EXPECT_TEST(DFB_TEST(ssl, 1, 100, 0, 20, 1, 0, 20)); /* 0-20 */ + EXPECT_TEST(DFB_TEST(ssl, 1, 100, 20, 20, 1, 0, 40)); /* 20-40 */ + EXPECT_TEST(DFB_TEST(ssl, 1, 100, 40, 20, 1, 0, 60)); /* 40-60 */ + EXPECT_TEST(DFB_TEST(ssl, 1, 100, 60, 20, 1, 0, 80)); /* 60-80 */ + EXPECT_TEST(DFB_TEST(ssl, 1, 100, 80, 20, 0, 1, 100)); /* 80-100 */ /* Test all permutations of 3 regions */ /* 1 2 3 */ - ExpectIntEQ(DFB_TEST(ssl, 2, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */ - ExpectIntEQ(DFB_TEST(ssl, 2, 100, 30, 30, 1, 0, 60), 0); /* 30-60 */ - ExpectIntEQ(DFB_TEST(ssl, 2, 100, 60, 40, 0, 1, 100), 0); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 2, 100, 0, 30, 1, 0, 30)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 2, 100, 30, 30, 1, 0, 60)); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 2, 100, 60, 40, 0, 1, 100)); /* 60-100 */ /* 1 3 2 */ - ExpectIntEQ(DFB_TEST(ssl, 3, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */ - ExpectIntEQ(DFB_TEST(ssl, 3, 100, 60, 40, 2, 0, 70), 0); /* 60-100 */ - ExpectIntEQ(DFB_TEST(ssl, 3, 100, 30, 30, 0, 1, 100), 0); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 3, 100, 0, 30, 1, 0, 30)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 3, 100, 60, 40, 2, 0, 70)); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 3, 100, 30, 30, 0, 1, 100)); /* 30-60 */ /* 2 1 3 */ - ExpectIntEQ(DFB_TEST(ssl, 4, 100, 30, 30, 1, 0, 30), 0); /* 30-60 */ - ExpectIntEQ(DFB_TEST(ssl, 4, 100, 0, 30, 1, 0, 60), 0); /* 0-30 */ - ExpectIntEQ(DFB_TEST(ssl, 4, 100, 60, 40, 0, 1, 100), 0); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 4, 100, 30, 30, 1, 0, 30)); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 4, 100, 0, 30, 1, 0, 60)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 4, 100, 60, 40, 0, 1, 100)); /* 60-100 */ /* 2 3 1 */ - ExpectIntEQ(DFB_TEST(ssl, 5, 100, 30, 30, 1, 0, 30), 0); /* 30-60 */ - ExpectIntEQ(DFB_TEST(ssl, 5, 100, 60, 40, 1, 0, 70), 0); /* 60-100 */ - ExpectIntEQ(DFB_TEST(ssl, 5, 100, 0, 30, 0, 1, 100), 0); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 5, 100, 30, 30, 1, 0, 30)); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 5, 100, 60, 40, 1, 0, 70)); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 5, 100, 0, 30, 0, 1, 100)); /* 0-30 */ /* 3 1 2 */ - ExpectIntEQ(DFB_TEST(ssl, 6, 100, 60, 40, 1, 0, 40), 0); /* 60-100 */ - ExpectIntEQ(DFB_TEST(ssl, 6, 100, 0, 30, 2, 0, 70), 0); /* 0-30 */ - ExpectIntEQ(DFB_TEST(ssl, 6, 100, 30, 30, 0, 1, 100), 0); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 6, 100, 60, 40, 1, 0, 40)); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 6, 100, 0, 30, 2, 0, 70)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 6, 100, 30, 30, 0, 1, 100)); /* 30-60 */ /* 3 2 1 */ - ExpectIntEQ(DFB_TEST(ssl, 7, 100, 60, 40, 1, 0, 40), 0); /* 60-100 */ - ExpectIntEQ(DFB_TEST(ssl, 7, 100, 30, 30, 1, 0, 70), 0); /* 30-60 */ - ExpectIntEQ(DFB_TEST(ssl, 7, 100, 0, 30, 0, 1, 100), 0); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 7, 100, 60, 40, 1, 0, 40)); /* 60-100 */ + EXPECT_TEST(DFB_TEST(ssl, 7, 100, 30, 30, 1, 0, 70)); /* 30-60 */ + EXPECT_TEST(DFB_TEST(ssl, 7, 100, 0, 30, 0, 1, 100)); /* 0-30 */ /* Test overlapping regions */ - ExpectIntEQ(DFB_TEST(ssl, 8, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */ - ExpectIntEQ(DFB_TEST(ssl, 8, 100, 20, 10, 1, 0, 30), 0); /* 20-30 */ - ExpectIntEQ(DFB_TEST(ssl, 8, 100, 70, 10, 2, 0, 40), 0); /* 70-80 */ - ExpectIntEQ(DFB_TEST(ssl, 8, 100, 20, 30, 2, 0, 60), 0); /* 20-50 */ - ExpectIntEQ(DFB_TEST(ssl, 8, 100, 40, 60, 0, 1, 100), 0); /* 40-100 */ + EXPECT_TEST(DFB_TEST(ssl, 8, 100, 0, 30, 1, 0, 30)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl, 8, 100, 20, 10, 1, 0, 30)); /* 20-30 */ + EXPECT_TEST(DFB_TEST(ssl, 8, 100, 70, 10, 2, 0, 40)); /* 70-80 */ + EXPECT_TEST(DFB_TEST(ssl, 8, 100, 20, 30, 2, 0, 60)); /* 20-50 */ + EXPECT_TEST(DFB_TEST(ssl, 8, 100, 40, 60, 0, 1, 100)); /* 40-100 */ /* Test overlapping multiple regions */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 30, 5, 2, 0, 25), 0); /* 30-35 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 40, 5, 3, 0, 30), 0); /* 40-45 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 50, 5, 4, 0, 35), 0); /* 50-55 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 60, 5, 5, 0, 40), 0); /* 60-65 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 70, 5, 6, 0, 45), 0); /* 70-75 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 30, 25, 4, 0, 55), 0); /* 30-55 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 55, 15, 2, 0, 65), 0); /* 55-70 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 75, 25, 2, 0, 90), 0); /* 75-100 */ - ExpectIntEQ(DFB_TEST(ssl, 9, 100, 10, 25, 0, 1, 100), 0); /* 10-35 */ - - ExpectIntEQ(DFB_TEST(ssl, 10, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */ - ExpectIntEQ(DFB_TEST(ssl, 10, 100, 30, 20, 2, 0, 40), 0); /* 30-50 */ - ExpectIntEQ(DFB_TEST(ssl, 10, 100, 0, 40, 1, 0, 50), 0); /* 0-40 */ - ExpectIntEQ(DFB_TEST(ssl, 10, 100, 50, 50, 0, 1, 100), 0); /* 10-35 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 0, 20, 1, 0, 20)); /* 0-20 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 30, 5, 2, 0, 25)); /* 30-35 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 40, 5, 3, 0, 30)); /* 40-45 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 50, 5, 4, 0, 35)); /* 50-55 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 60, 5, 5, 0, 40)); /* 60-65 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 70, 5, 6, 0, 45)); /* 70-75 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 30, 25, 4, 0, 55)); /* 30-55 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 55, 15, 2, 0, 65)); /* 55-70 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 75, 25, 2, 0, 90)); /* 75-100 */ + EXPECT_TEST(DFB_TEST(ssl, 9, 100, 10, 25, 0, 1, 100)); /* 10-35 */ + + EXPECT_TEST(DFB_TEST(ssl,10, 100, 0, 20, 1, 0, 20)); /* 0-20 */ + EXPECT_TEST(DFB_TEST(ssl,10, 100, 30, 20, 2, 0, 40)); /* 30-50 */ + EXPECT_TEST(DFB_TEST(ssl,10, 100, 0, 40, 1, 0, 50)); /* 0-40 */ + EXPECT_TEST(DFB_TEST(ssl,10, 100, 50, 50, 0, 1, 100)); /* 50-100 */ + + /* Test region between other regions */ + EXPECT_TEST(DFB_TEST(ssl,11, 100, 0, 20, 1, 0, 20)); /* 0-20 */ + EXPECT_TEST(DFB_TEST(ssl,11, 100, 80, 20, 2, 0, 40)); /* 80-100 */ + EXPECT_TEST(DFB_TEST(ssl,11, 100, 40, 20, 3, 0, 60)); /* 40-60 */ + EXPECT_TEST(DFB_TEST(ssl,11, 100, 20, 20, 2, 0, 80)); /* 20-40 */ + EXPECT_TEST(DFB_TEST(ssl,11, 100, 60, 20, 0, 1, 100)); /* 60-80 */ + + /* Test gap before first bucket (prev==NULL in gap-before branch) */ + EXPECT_TEST(DFB_TEST(ssl,12, 100, 50, 20, 1, 0, 20)); /* 50-70 */ + EXPECT_TEST(DFB_TEST(ssl,12, 100, 0, 20, 2, 0, 40)); /* 0-20 gap before first */ + EXPECT_TEST(DFB_TEST(ssl,12, 100, 20, 30, 1, 0, 70)); /* 20-50 bridges gap */ + EXPECT_TEST(DFB_TEST(ssl,12, 100, 70, 30, 0, 1, 100)); /* 70-100 */ + + /* Test fragment after message is already complete (ready early return) */ + EXPECT_TEST(DFB_TEST(ssl,13, 100, 0,100, 0, 1, 100)); /* 0-100 complete */ + EXPECT_TEST(DFB_TEST(ssl,13, 100, 0, 50, 0, 1, 100)); /* 0-50 dup on ready */ + + /* Test combine where next bucket is larger than cur (chosenBucket=&next) */ + EXPECT_TEST(DFB_TEST(ssl,14, 100, 0, 10, 1, 0, 10)); /* 0-10 */ + EXPECT_TEST(DFB_TEST(ssl,14, 100, 30, 50, 2, 0, 60)); /* 30-80 */ + EXPECT_TEST(DFB_TEST(ssl,14, 100, 5, 30, 1, 0, 80)); /* 5-35 next>cur */ + EXPECT_TEST(DFB_TEST(ssl,14, 100, 80, 20, 0, 1, 100)); /* 80-100 */ + + /* Test super fragment covering all existing buckets */ + EXPECT_TEST(DFB_TEST(ssl,15, 100, 10, 10, 1, 0, 10)); /* 10-20 */ + EXPECT_TEST(DFB_TEST(ssl,15, 100, 30, 10, 2, 0, 20)); /* 30-40 */ + EXPECT_TEST(DFB_TEST(ssl,15, 100, 60, 10, 3, 0, 30)); /* 60-70 */ + EXPECT_TEST(DFB_TEST(ssl,15, 100, 0,100, 0, 1, 100)); /* 0-100 super frag */ + + /* Test exact duplicate fragment */ + EXPECT_TEST(DFB_TEST(ssl,16, 100, 20, 40, 1, 0, 40)); /* 20-60 */ + EXPECT_TEST(DFB_TEST(ssl,16, 100, 20, 40, 1, 0, 40)); /* 20-60 exact dup */ + EXPECT_TEST(DFB_TEST(ssl,16, 100, 0, 20, 1, 0, 60)); /* 0-20 */ + EXPECT_TEST(DFB_TEST(ssl,16, 100, 60, 40, 0, 1, 100)); /* 60-100 */ + + /* Test combine bridging two buckets (combineNext, cur->data) */ + EXPECT_TEST(DFB_TEST(ssl,17, 100, 0, 30, 1, 0, 30)); /* 0-30 */ + EXPECT_TEST(DFB_TEST(ssl,17, 100, 60, 20, 2, 0, 50)); /* 60-80 */ + EXPECT_TEST(DFB_TEST(ssl,17, 100, 20, 45, 1, 0, 80)); /* 20-65 bridge */ + EXPECT_TEST(DFB_TEST(ssl,17, 100, 80, 20, 0, 1, 100)); /* 80-100 */ + + /* Test progressive left-extension with partial overlaps */ + EXPECT_TEST(DFB_TEST(ssl,18, 100, 70, 30, 1, 0, 30)); /* 70-100 */ + EXPECT_TEST(DFB_TEST(ssl,18, 100, 50, 30, 1, 0, 50)); /* 50-80 extend left */ + EXPECT_TEST(DFB_TEST(ssl,18, 100, 30, 30, 1, 0, 70)); /* 30-60 extend left */ + EXPECT_TEST(DFB_TEST(ssl,18, 100, 0, 40, 0, 1, 100)); /* 0-40 complete left */ DtlsMsgListDelete(ssl->dtls_rx_msg_list, ssl->heap); ssl->dtls_rx_msg_list = NULL; ssl->dtls_rx_msg_list_sz = 0; - return EXPECT_RESULT(); }