Dtls13: Fix handshake hangs on WANT_WRITE I/O error #9330
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a handshake hang issue in DTLS 1.3 that occurred when a WANT_WRITE I/O error was encountered. The core fix ensures the internal buffer index pointer is updated before error handling, preventing infinite loops. Additionally, the PR corrects bugs in test infrastructure where client/server force-want-write flags were swapped, and adds comprehensive test coverage for non-blocking I/O scenarios.
Key Changes:
- Moved buffer index update in
_Dtls13HandshakeRecvto occur before error checking - Fixed swapped client/server
forceWantWritereferences in test I/O callbacks - Added two new test cases to verify proper handling of WANT_WRITE conditions during DTLS 1.3 handshakes
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dtls13.c | Fixes buffer index update timing to prevent handshake hangs on WANT_WRITE errors |
| tests/utils.c | Corrects swapped client/server force-want-write flag assignments in I/O callback |
| tests/api/test_dtls.h | Declares two new test functions for DTLS 1.3 WANT_WRITE scenarios |
| tests/api/test_dtls.c | Implements comprehensive tests for HRR and every-write WANT_WRITE cases |
| tests/api.c | Fixes incorrect side parameter in test_memio_simulate_want_write calls |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
🛟 Devin Lifeguard found 1 likely issues in this PR
@rizlik |
|
retest this please |
There was a problem hiding this comment.
Don't you think it would be easier to just pass in word32* inOutIdx to _Dtls13HandshakeRecv? This way we have to remember to set processedSize in all branches.
| ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, sizeof(readBuf)), msgLen); | ||
| ExpectStrEQ(readBuf, msg); | ||
|
|
||
| XMEMSET(readBuf, 0, sizeof(readBuf)); |
There was a problem hiding this comment.
You should XMEMSET before both uses.
| while ((!hs_complete[0] || !hs_complete[1]) && rounds < 10) { | ||
| rounds++; | ||
| for (side = 0; side < 2; side++) { | ||
| ssl = (side == 0) ? ssl_s : ssl_c; | ||
| if (hs_complete[side]) | ||
| continue; | ||
|
|
||
| test_memio_simulate_want_write(&test_ctx, side, 1); | ||
| ret = wolfSSL_negotiate(ssl); | ||
| if (ret != WOLFSSL_SUCCESS) { | ||
| err = wolfSSL_get_error(ssl, ret); | ||
| if (err == WOLFSSL_ERROR_WANT_WRITE) { | ||
| test_memio_simulate_want_write(&test_ctx, side, 0); | ||
| ret = wolfSSL_negotiate(ssl); | ||
| } | ||
| } | ||
| if (ret == WOLFSSL_SUCCESS) { | ||
| hs_complete[side] = 1; | ||
| continue; | ||
| } | ||
| else { | ||
| ExpectIntEQ(ret, -1); | ||
| ExpectIntEQ(wolfSSL_get_error(ssl, ret), | ||
| WOLFSSL_ERROR_WANT_READ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Do you think test_memio_do_handshake could be re-used if you used callbacks that would return WANT_WRITE every second call?
2 participants
Description
The internal buffer index pointer wasn't incremented in case of error inside
_Dtls13HandshakeRecv, causing an hangs.Fixes zd#20686
Added tests to increase coverage from non-blocking I/O.