Skip to content

Commit fb4c401

Browse files
douzzerdouzzer
authored
Merge pull request #10090 from julek-wolfssl/zd/21421
Improve DTLS bucket logic
2 parents 14dbba7 + 2f37ab3 commit fb4c401

2 files changed

Lines changed: 125 additions & 80 deletions

File tree

‎src/internal.c‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9701,8 +9701,9 @@ static DtlsFragBucket* DtlsMsgCombineFragBuckets(DtlsMsg* msg,
97019701
}
97029702
else {
97039703
/* data -> cur. memcpy as much possible as its faster. */
9704-
XMEMMOVE(newBucket->buf + dataSz, cur->buf,
9705-
cur->m.m.sz - (offsetEnd - cur->m.m.offset));
9704+
word32 skipInCur = offsetEnd - cur->m.m.offset;
9705+
XMEMMOVE(newBucket->buf + dataSz, cur->buf + skipInCur,
9706+
cur->m.m.sz - skipInCur);
97069707
XMEMCPY(newBucket->buf, data, dataSz);
97079708
}
97089709
}
@@ -9863,22 +9864,25 @@ int DtlsMsgSet(DtlsMsg* msg, word32 seq, word16 epoch, const byte* data, byte ty
98639864
msg->fragBucketListCount++;
98649865
}
98659866
}
9866-
else if (prev == NULL && fragOffsetEnd < cur->m.m.offset) {
9867-
/* This is the new first fragment we have received */
9867+
else if (fragOffsetEnd < cur->m.m.offset) {
9868+
/* Fragment is entirely before cur with a gap */
9869+
DtlsFragBucket** prev_next;
98689870
if (msg->fragBucketListCount >= DTLS_FRAG_POOL_SZ) {
98699871
WOLFSSL_ERROR_VERBOSE(DTLS_TOO_MANY_FRAGMENTS_E);
98709872
return DTLS_TOO_MANY_FRAGMENTS_E;
98719873
}
9872-
msg->fragBucketList = DtlsMsgCreateFragBucket(fragOffset, data,
9874+
prev_next = prev != NULL
9875+
? &prev->m.m.next : &msg->fragBucketList;
9876+
*prev_next = DtlsMsgCreateFragBucket(fragOffset, data,
98739877
fragSz, heap);
9874-
if (msg->fragBucketList != NULL) {
9875-
msg->fragBucketList->m.m.next = cur;
9878+
if (*prev_next != NULL) {
9879+
(*prev_next)->m.m.next = cur;
98769880
msg->bytesReceived += fragSz;
98779881
msg->fragBucketListCount++;
98789882
}
98799883
else {
98809884
/* reset on error */
9881-
msg->fragBucketList = cur;
9885+
*prev_next = cur;
98829886
}
98839887
}
98849888
else {

‎tests/api.c‎

Lines changed: 113 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -28829,7 +28829,8 @@ static int test_wolfSSL_DtlsUpdateWindow(void)
2882928829
static int DFB_TEST(WOLFSSL* ssl, word32 seq, word32 len, word32 f_offset,
2883028830
word32 f_len, word32 f_count, byte ready, word32 bytesReceived)
2883128831
{
28832-
DtlsMsg* cur;
28832+
EXPECT_DECLS;
28833+
DtlsMsg* cur = NULL;
2883328834
static byte msg[100];
2883428835
static byte msgInit = 0;
2883528836

@@ -28841,40 +28842,34 @@ static int DFB_TEST(WOLFSSL* ssl, word32 seq, word32 len, word32 f_offset,
2884128842
}
2884228843

2884328844
/* Sanitize test parameters */
28844-
if (len > sizeof(msg))
28845-
return -1;
28846-
if (f_offset + f_len > sizeof(msg))
28847-
return -1;
28845+
ExpectIntLE(len, sizeof(msg));
28846+
ExpectIntLE(f_offset + f_len, sizeof(msg));
2884828847

28849-
DtlsMsgStore(ssl, 0, seq, msg + f_offset, len, certificate, f_offset, f_len, NULL);
28848+
if (EXPECT_SUCCESS())
28849+
DtlsMsgStore(ssl, 0, seq, msg + f_offset, len, certificate, f_offset, f_len, NULL);
2885028850

28851-
if (ssl->dtls_rx_msg_list == NULL)
28852-
return -100;
28851+
ExpectNotNull(ssl->dtls_rx_msg_list);
2885328852

28854-
if ((cur = DtlsMsgFind(ssl->dtls_rx_msg_list, 0, seq)) == NULL)
28855-
return -200;
28856-
if (cur->fragBucketListCount != f_count)
28857-
return -300;
28858-
if (cur->ready != ready)
28859-
return -400;
28860-
if (cur->bytesReceived != bytesReceived)
28861-
return -500;
28853+
ExpectNotNull(cur = DtlsMsgFind(ssl->dtls_rx_msg_list, 0, seq));
28854+
ExpectIntEQ(cur->fragBucketListCount, f_count);
28855+
ExpectIntEQ(cur->ready, ready);
28856+
ExpectIntEQ(cur->bytesReceived, bytesReceived);
2886228857
if (ready) {
28863-
if (cur->fragBucketList != NULL)
28864-
return -600;
28865-
if (XMEMCMP(cur->fullMsg, msg, cur->sz) != 0)
28866-
return -700;
28858+
ExpectNull(cur->fragBucketList);
28859+
ExpectBufEQ(cur->fullMsg, msg, cur->sz);
2886728860
}
2886828861
else {
2886928862
DtlsFragBucket* fb;
28870-
if (cur->fragBucketList == NULL)
28871-
return -800;
28872-
for (fb = cur->fragBucketList; fb != NULL; fb = fb->m.m.next) {
28873-
if (XMEMCMP(fb->buf, msg + fb->m.m.offset, fb->m.m.sz) != 0)
28874-
return -900;
28875-
}
28863+
ExpectNotNull(cur->fragBucketList);
28864+
for (fb = cur != NULL ? cur->fragBucketList : NULL;
28865+
EXPECT_SUCCESS() && fb != NULL; fb = fb->m.m.next)
28866+
ExpectBufEQ(fb->buf, msg + fb->m.m.offset, fb->m.m.sz);
2887628867
}
28877-
return 0;
28868+
if (EXPECT_FAIL()) {
28869+
printf("Test parameters: seq %u len %u f_offset %u f_len %u f_count %u ready %u bytesReceived %u\n",
28870+
seq, len, f_offset, f_len, f_count, ready, bytesReceived);
28871+
}
28872+
return EXPECT_RESULT();
2887828873
}
2887928874

2888028875
static int test_wolfSSL_DTLS_fragment_buckets(void)
@@ -28884,68 +28879,114 @@ static int test_wolfSSL_DTLS_fragment_buckets(void)
2888428879

2888528880
XMEMSET(ssl, 0, sizeof(*ssl));
2888628881

28887-
ExpectIntEQ(DFB_TEST(ssl, 0, 100, 0, 100, 0, 1, 100), 0); /* 0-100 */
28882+
EXPECT_TEST(DFB_TEST(ssl, 0, 100, 0, 100, 0, 1, 100)); /* 0-100 */
2888828883

28889-
ExpectIntEQ(DFB_TEST(ssl, 1, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */
28890-
ExpectIntEQ(DFB_TEST(ssl, 1, 100, 20, 20, 1, 0, 40), 0); /* 20-40 */
28891-
ExpectIntEQ(DFB_TEST(ssl, 1, 100, 40, 20, 1, 0, 60), 0); /* 40-60 */
28892-
ExpectIntEQ(DFB_TEST(ssl, 1, 100, 60, 20, 1, 0, 80), 0); /* 60-80 */
28893-
ExpectIntEQ(DFB_TEST(ssl, 1, 100, 80, 20, 0, 1, 100), 0); /* 80-100 */
28884+
EXPECT_TEST(DFB_TEST(ssl, 1, 100, 0, 20, 1, 0, 20)); /* 0-20 */
28885+
EXPECT_TEST(DFB_TEST(ssl, 1, 100, 20, 20, 1, 0, 40)); /* 20-40 */
28886+
EXPECT_TEST(DFB_TEST(ssl, 1, 100, 40, 20, 1, 0, 60)); /* 40-60 */
28887+
EXPECT_TEST(DFB_TEST(ssl, 1, 100, 60, 20, 1, 0, 80)); /* 60-80 */
28888+
EXPECT_TEST(DFB_TEST(ssl, 1, 100, 80, 20, 0, 1, 100)); /* 80-100 */
2889428889

2889528890
/* Test all permutations of 3 regions */
2889628891
/* 1 2 3 */
28897-
ExpectIntEQ(DFB_TEST(ssl, 2, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */
28898-
ExpectIntEQ(DFB_TEST(ssl, 2, 100, 30, 30, 1, 0, 60), 0); /* 30-60 */
28899-
ExpectIntEQ(DFB_TEST(ssl, 2, 100, 60, 40, 0, 1, 100), 0); /* 60-100 */
28892+
EXPECT_TEST(DFB_TEST(ssl, 2, 100, 0, 30, 1, 0, 30)); /* 0-30 */
28893+
EXPECT_TEST(DFB_TEST(ssl, 2, 100, 30, 30, 1, 0, 60)); /* 30-60 */
28894+
EXPECT_TEST(DFB_TEST(ssl, 2, 100, 60, 40, 0, 1, 100)); /* 60-100 */
2890028895
/* 1 3 2 */
28901-
ExpectIntEQ(DFB_TEST(ssl, 3, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */
28902-
ExpectIntEQ(DFB_TEST(ssl, 3, 100, 60, 40, 2, 0, 70), 0); /* 60-100 */
28903-
ExpectIntEQ(DFB_TEST(ssl, 3, 100, 30, 30, 0, 1, 100), 0); /* 30-60 */
28896+
EXPECT_TEST(DFB_TEST(ssl, 3, 100, 0, 30, 1, 0, 30)); /* 0-30 */
28897+
EXPECT_TEST(DFB_TEST(ssl, 3, 100, 60, 40, 2, 0, 70)); /* 60-100 */
28898+
EXPECT_TEST(DFB_TEST(ssl, 3, 100, 30, 30, 0, 1, 100)); /* 30-60 */
2890428899
/* 2 1 3 */
28905-
ExpectIntEQ(DFB_TEST(ssl, 4, 100, 30, 30, 1, 0, 30), 0); /* 30-60 */
28906-
ExpectIntEQ(DFB_TEST(ssl, 4, 100, 0, 30, 1, 0, 60), 0); /* 0-30 */
28907-
ExpectIntEQ(DFB_TEST(ssl, 4, 100, 60, 40, 0, 1, 100), 0); /* 60-100 */
28900+
EXPECT_TEST(DFB_TEST(ssl, 4, 100, 30, 30, 1, 0, 30)); /* 30-60 */
28901+
EXPECT_TEST(DFB_TEST(ssl, 4, 100, 0, 30, 1, 0, 60)); /* 0-30 */
28902+
EXPECT_TEST(DFB_TEST(ssl, 4, 100, 60, 40, 0, 1, 100)); /* 60-100 */
2890828903
/* 2 3 1 */
28909-
ExpectIntEQ(DFB_TEST(ssl, 5, 100, 30, 30, 1, 0, 30), 0); /* 30-60 */
28910-
ExpectIntEQ(DFB_TEST(ssl, 5, 100, 60, 40, 1, 0, 70), 0); /* 60-100 */
28911-
ExpectIntEQ(DFB_TEST(ssl, 5, 100, 0, 30, 0, 1, 100), 0); /* 0-30 */
28904+
EXPECT_TEST(DFB_TEST(ssl, 5, 100, 30, 30, 1, 0, 30)); /* 30-60 */
28905+
EXPECT_TEST(DFB_TEST(ssl, 5, 100, 60, 40, 1, 0, 70)); /* 60-100 */
28906+
EXPECT_TEST(DFB_TEST(ssl, 5, 100, 0, 30, 0, 1, 100)); /* 0-30 */
2891228907
/* 3 1 2 */
28913-
ExpectIntEQ(DFB_TEST(ssl, 6, 100, 60, 40, 1, 0, 40), 0); /* 60-100 */
28914-
ExpectIntEQ(DFB_TEST(ssl, 6, 100, 0, 30, 2, 0, 70), 0); /* 0-30 */
28915-
ExpectIntEQ(DFB_TEST(ssl, 6, 100, 30, 30, 0, 1, 100), 0); /* 30-60 */
28908+
EXPECT_TEST(DFB_TEST(ssl, 6, 100, 60, 40, 1, 0, 40)); /* 60-100 */
28909+
EXPECT_TEST(DFB_TEST(ssl, 6, 100, 0, 30, 2, 0, 70)); /* 0-30 */
28910+
EXPECT_TEST(DFB_TEST(ssl, 6, 100, 30, 30, 0, 1, 100)); /* 30-60 */
2891628911
/* 3 2 1 */
28917-
ExpectIntEQ(DFB_TEST(ssl, 7, 100, 60, 40, 1, 0, 40), 0); /* 60-100 */
28918-
ExpectIntEQ(DFB_TEST(ssl, 7, 100, 30, 30, 1, 0, 70), 0); /* 30-60 */
28919-
ExpectIntEQ(DFB_TEST(ssl, 7, 100, 0, 30, 0, 1, 100), 0); /* 0-30 */
28912+
EXPECT_TEST(DFB_TEST(ssl, 7, 100, 60, 40, 1, 0, 40)); /* 60-100 */
28913+
EXPECT_TEST(DFB_TEST(ssl, 7, 100, 30, 30, 1, 0, 70)); /* 30-60 */
28914+
EXPECT_TEST(DFB_TEST(ssl, 7, 100, 0, 30, 0, 1, 100)); /* 0-30 */
2892028915

2892128916
/* Test overlapping regions */
28922-
ExpectIntEQ(DFB_TEST(ssl, 8, 100, 0, 30, 1, 0, 30), 0); /* 0-30 */
28923-
ExpectIntEQ(DFB_TEST(ssl, 8, 100, 20, 10, 1, 0, 30), 0); /* 20-30 */
28924-
ExpectIntEQ(DFB_TEST(ssl, 8, 100, 70, 10, 2, 0, 40), 0); /* 70-80 */
28925-
ExpectIntEQ(DFB_TEST(ssl, 8, 100, 20, 30, 2, 0, 60), 0); /* 20-50 */
28926-
ExpectIntEQ(DFB_TEST(ssl, 8, 100, 40, 60, 0, 1, 100), 0); /* 40-100 */
28917+
EXPECT_TEST(DFB_TEST(ssl, 8, 100, 0, 30, 1, 0, 30)); /* 0-30 */
28918+
EXPECT_TEST(DFB_TEST(ssl, 8, 100, 20, 10, 1, 0, 30)); /* 20-30 */
28919+
EXPECT_TEST(DFB_TEST(ssl, 8, 100, 70, 10, 2, 0, 40)); /* 70-80 */
28920+
EXPECT_TEST(DFB_TEST(ssl, 8, 100, 20, 30, 2, 0, 60)); /* 20-50 */
28921+
EXPECT_TEST(DFB_TEST(ssl, 8, 100, 40, 60, 0, 1, 100)); /* 40-100 */
2892728922

2892828923
/* Test overlapping multiple regions */
28929-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */
28930-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 30, 5, 2, 0, 25), 0); /* 30-35 */
28931-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 40, 5, 3, 0, 30), 0); /* 40-45 */
28932-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 50, 5, 4, 0, 35), 0); /* 50-55 */
28933-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 60, 5, 5, 0, 40), 0); /* 60-65 */
28934-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 70, 5, 6, 0, 45), 0); /* 70-75 */
28935-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 30, 25, 4, 0, 55), 0); /* 30-55 */
28936-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 55, 15, 2, 0, 65), 0); /* 55-70 */
28937-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 75, 25, 2, 0, 90), 0); /* 75-100 */
28938-
ExpectIntEQ(DFB_TEST(ssl, 9, 100, 10, 25, 0, 1, 100), 0); /* 10-35 */
28939-
28940-
ExpectIntEQ(DFB_TEST(ssl, 10, 100, 0, 20, 1, 0, 20), 0); /* 0-20 */
28941-
ExpectIntEQ(DFB_TEST(ssl, 10, 100, 30, 20, 2, 0, 40), 0); /* 30-50 */
28942-
ExpectIntEQ(DFB_TEST(ssl, 10, 100, 0, 40, 1, 0, 50), 0); /* 0-40 */
28943-
ExpectIntEQ(DFB_TEST(ssl, 10, 100, 50, 50, 0, 1, 100), 0); /* 10-35 */
28924+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 0, 20, 1, 0, 20)); /* 0-20 */
28925+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 30, 5, 2, 0, 25)); /* 30-35 */
28926+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 40, 5, 3, 0, 30)); /* 40-45 */
28927+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 50, 5, 4, 0, 35)); /* 50-55 */
28928+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 60, 5, 5, 0, 40)); /* 60-65 */
28929+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 70, 5, 6, 0, 45)); /* 70-75 */
28930+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 30, 25, 4, 0, 55)); /* 30-55 */
28931+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 55, 15, 2, 0, 65)); /* 55-70 */
28932+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 75, 25, 2, 0, 90)); /* 75-100 */
28933+
EXPECT_TEST(DFB_TEST(ssl, 9, 100, 10, 25, 0, 1, 100)); /* 10-35 */
28934+
28935+
EXPECT_TEST(DFB_TEST(ssl,10, 100, 0, 20, 1, 0, 20)); /* 0-20 */
28936+
EXPECT_TEST(DFB_TEST(ssl,10, 100, 30, 20, 2, 0, 40)); /* 30-50 */
28937+
EXPECT_TEST(DFB_TEST(ssl,10, 100, 0, 40, 1, 0, 50)); /* 0-40 */
28938+
EXPECT_TEST(DFB_TEST(ssl,10, 100, 50, 50, 0, 1, 100)); /* 50-100 */
28939+
28940+
/* Test region between other regions */
28941+
EXPECT_TEST(DFB_TEST(ssl,11, 100, 0, 20, 1, 0, 20)); /* 0-20 */
28942+
EXPECT_TEST(DFB_TEST(ssl,11, 100, 80, 20, 2, 0, 40)); /* 80-100 */
28943+
EXPECT_TEST(DFB_TEST(ssl,11, 100, 40, 20, 3, 0, 60)); /* 40-60 */
28944+
EXPECT_TEST(DFB_TEST(ssl,11, 100, 20, 20, 2, 0, 80)); /* 20-40 */
28945+
EXPECT_TEST(DFB_TEST(ssl,11, 100, 60, 20, 0, 1, 100)); /* 60-80 */
28946+
28947+
/* Test gap before first bucket (prev==NULL in gap-before branch) */
28948+
EXPECT_TEST(DFB_TEST(ssl,12, 100, 50, 20, 1, 0, 20)); /* 50-70 */
28949+
EXPECT_TEST(DFB_TEST(ssl,12, 100, 0, 20, 2, 0, 40)); /* 0-20 gap before first */
28950+
EXPECT_TEST(DFB_TEST(ssl,12, 100, 20, 30, 1, 0, 70)); /* 20-50 bridges gap */
28951+
EXPECT_TEST(DFB_TEST(ssl,12, 100, 70, 30, 0, 1, 100)); /* 70-100 */
28952+
28953+
/* Test fragment after message is already complete (ready early return) */
28954+
EXPECT_TEST(DFB_TEST(ssl,13, 100, 0,100, 0, 1, 100)); /* 0-100 complete */
28955+
EXPECT_TEST(DFB_TEST(ssl,13, 100, 0, 50, 0, 1, 100)); /* 0-50 dup on ready */
28956+
28957+
/* Test combine where next bucket is larger than cur (chosenBucket=&next) */
28958+
EXPECT_TEST(DFB_TEST(ssl,14, 100, 0, 10, 1, 0, 10)); /* 0-10 */
28959+
EXPECT_TEST(DFB_TEST(ssl,14, 100, 30, 50, 2, 0, 60)); /* 30-80 */
28960+
EXPECT_TEST(DFB_TEST(ssl,14, 100, 5, 30, 1, 0, 80)); /* 5-35 next>cur */
28961+
EXPECT_TEST(DFB_TEST(ssl,14, 100, 80, 20, 0, 1, 100)); /* 80-100 */
28962+
28963+
/* Test super fragment covering all existing buckets */
28964+
EXPECT_TEST(DFB_TEST(ssl,15, 100, 10, 10, 1, 0, 10)); /* 10-20 */
28965+
EXPECT_TEST(DFB_TEST(ssl,15, 100, 30, 10, 2, 0, 20)); /* 30-40 */
28966+
EXPECT_TEST(DFB_TEST(ssl,15, 100, 60, 10, 3, 0, 30)); /* 60-70 */
28967+
EXPECT_TEST(DFB_TEST(ssl,15, 100, 0,100, 0, 1, 100)); /* 0-100 super frag */
28968+
28969+
/* Test exact duplicate fragment */
28970+
EXPECT_TEST(DFB_TEST(ssl,16, 100, 20, 40, 1, 0, 40)); /* 20-60 */
28971+
EXPECT_TEST(DFB_TEST(ssl,16, 100, 20, 40, 1, 0, 40)); /* 20-60 exact dup */
28972+
EXPECT_TEST(DFB_TEST(ssl,16, 100, 0, 20, 1, 0, 60)); /* 0-20 */
28973+
EXPECT_TEST(DFB_TEST(ssl,16, 100, 60, 40, 0, 1, 100)); /* 60-100 */
28974+
28975+
/* Test combine bridging two buckets (combineNext, cur->data) */
28976+
EXPECT_TEST(DFB_TEST(ssl,17, 100, 0, 30, 1, 0, 30)); /* 0-30 */
28977+
EXPECT_TEST(DFB_TEST(ssl,17, 100, 60, 20, 2, 0, 50)); /* 60-80 */
28978+
EXPECT_TEST(DFB_TEST(ssl,17, 100, 20, 45, 1, 0, 80)); /* 20-65 bridge */
28979+
EXPECT_TEST(DFB_TEST(ssl,17, 100, 80, 20, 0, 1, 100)); /* 80-100 */
28980+
28981+
/* Test progressive left-extension with partial overlaps */
28982+
EXPECT_TEST(DFB_TEST(ssl,18, 100, 70, 30, 1, 0, 30)); /* 70-100 */
28983+
EXPECT_TEST(DFB_TEST(ssl,18, 100, 50, 30, 1, 0, 50)); /* 50-80 extend left */
28984+
EXPECT_TEST(DFB_TEST(ssl,18, 100, 30, 30, 1, 0, 70)); /* 30-60 extend left */
28985+
EXPECT_TEST(DFB_TEST(ssl,18, 100, 0, 40, 0, 1, 100)); /* 0-40 complete left */
2894428986

2894528987
DtlsMsgListDelete(ssl->dtls_rx_msg_list, ssl->heap);
2894628988
ssl->dtls_rx_msg_list = NULL;
2894728989
ssl->dtls_rx_msg_list_sz = 0;
28948-
2894928990
return EXPECT_RESULT();
2895028991
}
2895128992

0 commit comments

Comments
 (0)