Open
Conversation
- Copy TLS 1.3 traffic secrets and DTLS 1.3 epoch/cipher state to the write-dup side in DupSSL so key updates can be performed. - Delegate KeyUpdate responses from the read side to the write side via the shared WriteDup struct, for both peer-initiated and local key updates. - Delegate DTLS 1.3 ACK sending from the read side to the write side. - Track DTLS 1.3 KeyUpdate ACKs: write side records the in-flight KeyUpdate epoch/seq, read side sets keyUpdateAcked when the matching ACK arrives. - Delegate post-handshake certificate authentication (CertificateRequest processing) from the read side to the write side, transferring transcript hashes, cert context, and signature parameters. - Reset prevSent/plainSz to prevent stale values from SendData to think that data was already sent. - Refactor FreeHandshakeHashes into Free_HS_Hashes for reuse. - Move DTLS 1.3 epoch initialization earlier in InitSSL so the write-dup early-return path has valid epoch state. - Add tests for write dup with all protocol versions, key update, post-handshake auth, and WANT_WRITE recovery. - Add --enable-all --enable-writedup to CI os-check matrix.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds DTLS 1.3/TLS 1.3 write-dup (DupSSL) support so the read-side can delegate post-handshake work (KeyUpdate responses, DTLS13 ACK sending, post-handshake auth) to the write-side, along with new tests and CI coverage.
Changes:
- Extend
WriteDupshared state to transfer DTLS13 ACKs, track KeyUpdate ACKs, and delegate post-handshake auth state. - Update TLS13/DTLS13 processing to delegate write-required actions from read-dup to write-dup.
- Add/expand API tests for write-dup across protocol versions, WANT_WRITE recovery paths, key updates, and post-handshake auth; enable writedup config in CI.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/internal.h |
Adds new write-dup shared fields/flags and new internal function declarations used by delegation paths. |
src/internal.c |
Refactors handshake-hash freeing for reuse and moves DTLS13 epoch init earlier for write-dup early returns. |
src/tls13.c |
Delegates TLS 1.3 KeyUpdate response and post-handshake CertificateRequest handling to write-dup side. |
src/dtls13.c |
Delegates DTLS 1.3 ACK work to write-dup and tracks KeyUpdate ACK arrival; exposes RTX removal helper. |
src/ssl.c |
Copies TLS13 secrets/DTLS13 epoch state into dup; implements write-side execution of delegated work. |
tests/api.c |
Adds/expands tests for write-dup across TLS/DTLS versions, key updates, post-handshake auth, WANT_WRITE. |
.github/workflows/os-check.yml |
Adds --enable-all --enable-writedup to CI matrix to exercise the new feature. |
Comments suppressed due to low confidence (3)
src/internal.c:1
Free_HS_Hashesis declared asWOLFSSL_LOCALinwolfssl/internal.h, but the definition here omitsWOLFSSL_LOCAL, which can cause a linkage/visibility mismatch depending on howWOLFSSL_LOCALis defined. Update the definition to match the header's storage/visibility macro. Also, settinghsHashes = NULLat the end has no effect on the caller (pass-by-value), so either remove that line or change the API to takeHS_Hashes**if caller-nullification is required.
src/internal.c:1Free_HS_Hashesis declared asWOLFSSL_LOCALinwolfssl/internal.h, but the definition here omitsWOLFSSL_LOCAL, which can cause a linkage/visibility mismatch depending on howWOLFSSL_LOCALis defined. Update the definition to match the header's storage/visibility macro. Also, settinghsHashes = NULLat the end has no effect on the caller (pass-by-value), so either remove that line or change the API to takeHS_Hashes**if caller-nullification is required.
tests/api.c:1EXCHANGE_DATAis defined multiple times in this file with very similar bodies (including WANT_WRITE variants). This increases the chance of tests diverging unintentionally when the exchange logic needs updating. Consider using a small helper function (or a single parameterized macro) that takes the relevantsslhandles/test context flags to centralize the exchange behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| #endif /* WOLFSSL_DEBUG_TLS */ | ||
|
|
||
| static void Dtls13RtxRemoveRecord(WOLFSSL* ssl, w64wrapper epoch, | ||
| void Dtls13RtxRemoveRecord(WOLFSSL* ssl, w64wrapper epoch, |
Comment on lines
+11834
to
+11836
| } | ||
| ssl->keys.keyUpdateRespond = 0; | ||
| return 0; |
Comment on lines
+2722
to
+2736
| #ifdef HAVE_WRITE_DUP | ||
| /* The read side cannot encrypt. Transfer the seenRecords list to the | ||
| * shared WriteDup struct so the write side sends the ACK instead. */ | ||
| if (ssl->dupWrite != NULL && ssl->dupSide == READ_DUP_SIDE) { | ||
| if (wc_LockMutex(&ssl->dupWrite->dupMutex) == 0) { | ||
| struct Dtls13RecordNumber** tail = | ||
| (struct Dtls13RecordNumber**)&ssl->dupWrite->sendAckList; | ||
| while (*tail != NULL) | ||
| tail = &(*tail)->next; | ||
| *tail = ssl->dtls13Rtx.seenRecords; | ||
| ssl->dtls13Rtx.seenRecords = NULL; | ||
| ssl->dupWrite->sendAcks = 1; | ||
| wc_UnLockMutex(&ssl->dupWrite->dupMutex); | ||
| } | ||
| } |
Comment on lines
+2864
to
+2872
| if (wc_LockMutex(&ssl->dupWrite->dupMutex) == 0) { | ||
| if (ssl->dupWrite->keyUpdateWaiting && | ||
| w64Equal(epoch, ssl->dupWrite->keyUpdateEpoch) && | ||
| w64Equal(seq, ssl->dupWrite->keyUpdateSeq)) { | ||
| ssl->dupWrite->keyUpdateAcked = 1; | ||
| ssl->dupWrite->keyUpdateWaiting = 0; | ||
| } | ||
| wc_UnLockMutex(&ssl->dupWrite->dupMutex); | ||
| } |
Comment on lines
+2555
to
+2557
| if (ssl->dupWrite->keyUpdateAcked) { | ||
| Dtls13RtxRemoveRecord(ssl, ssl->dupWrite->keyUpdateEpoch, | ||
| ssl->dupWrite->keyUpdateSeq); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
write-dup side in DupSSL so key updates can be performed.
the shared WriteDup struct, for both peer-initiated and local key
updates.
KeyUpdate epoch/seq, read side sets keyUpdateAcked when the matching
ACK arrives.
processing) from the read side to the write side, transferring
transcript hashes, cert context, and signature parameters.
that data was already sent.
write-dup early-return path has valid epoch state.
post-handshake auth, and WANT_WRITE recovery.