Repository navigation
Conversation
With WOLFSSL_DTLS compiled in, InitSSL pointed IOCB_ReadCtx and IOCB_WriteCtx at ssl->buffers.dtlsCtx for every WOLFSSL, TLS ones included. EmbedReceive and EmbedSend read the socket as *(int*)ctx, and the first member of WOLFSSL_DTLS_CTX is the zeroed peer address, so a TLS object that never had an fd set did its I/O on fd 0 instead of failing on -1. When stdin is an idle socket, an accept or connect without an fd blocks forever; test_tls13_apis hits this through wolfSSL_read_early_data on a server with no fd. Choose the default IO context in one place for builds with and without DTLS. NetX, Mynewt and GNRC keep their port context, and the socket default is &ssl->rfd/&ssl->wfd, or the DTLS context when the method is DTLS. Mynewt and GNRC objects in builds with DTLS now get the port context their I/O callbacks expect instead of the DTLS one. The new test checks the default contexts of a TLS and a DTLS object, and puts a socket on fd 0 to check that a client with no fd, and one with only a write fd, fail with SOCKET_ERROR_E without writing to or reading from fd 0.
The OpenSSL compat wolfSSL_set_wfd only stored the fd in ssl->wfd and pointed IOCB_WriteCtx at it, also for DTLS objects. EmbedSendTo casts the write context to WOLFSSL_DTLS_CTX, so it read the socket, the peer address and the sendto callback from the WOLFSSL fields that follow wfd. wolfSSL_set_rfd handled DTLS but did not reset dtlsCtx.connected, and neither checked ssl for NULL. Make both wrappers around wolfSSL_set_read_fd and wolfSSL_set_write_fd, which select the DTLS context and fill it in.
|
There was a problem hiding this comment.
🟡 Changes recommended
The regression test lacks a client PSK callback, causing PSK-only DTLS 1.2 builds with TLS 1.3 enabled to return SUITES_ERROR instead of the expected WOLFSSL_ERROR_WANT_READ.
1 open issue
What changed in this PR
This PR corrects wolfSSL’s default I/O contexts so TLS objects without file descriptors fail rather than access stdin, while preserving DTLS and platform-specific contexts.
Changes:
- Select default I/O contexts according to the protocol and platform.
- Delegate OpenSSL-compatible read/write descriptor setters to the DTLS-aware implementations.
- Add regression tests for unset TLS descriptors and DTLS descriptor setters.
| File | Description |
|---|---|
tests/api/test_tls.h |
Registers the unset-descriptor regression test. |
tests/api/test_tls.c |
Checks default contexts and prevents unintended stdin I/O. |
tests/api/test_dtls.h |
Registers the DTLS descriptor-setter test. |
tests/api/test_dtls.c |
Tests DTLS handshake initiation through compatibility setters. |
src/ssl.c |
Reuses DTLS-aware descriptor setters. |
src/internal.c |
Selects protocol-appropriate default I/O contexts. |
Review effort: Balanced
|
@claude review |
|
@wolfSSL-Fenrir-bot review |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11956
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): tests/api/test_dtls.h, tests/api/test_tls.h
Coverage: 2 of 4 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_dtls.c, tests/api/test_tls.c
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It changes the default I/O context for every WOLFSSL object in DTLS-enabled builds, which includes --enable-all, so a maintainer should still look at it before merge.
What was reviewed:
InitSSL: NetX, Mynewt and GNRC still get their port contexts. Without those ports, TLS objects default to&ssl->rfd/&ssl->wfd, both initialized to -1 just above, and only methods withDTLS_MAJORgetdtlsCtx.wolfSSL_set_rfd/wolfSSL_set_wfdnow wrapwolfSSL_set_read_fd/wolfSSL_set_write_fd. On DTLS objects,set_wfdnow updatesdtlsCtx.wfd. Both setters now also NULL-check and resetdtlsCtx.connected, the same aswolfSSL_set_fd.- Both new tests restore fd 0, close their sockets on every path, and use C90 declarations.
Extended reasoning...
The change touches the default I/O callback context setup in InitSSL (src/internal.c), the OpenSSL-compat fd setters in src/ssl.c, and adds two API tests. It doesn't touch crypto, auth or parsing, but it changes the behavior of the core I/O path for every SSL object in DTLS-enabled builds. The diff is small, and in the configurations I traced it looks correct. I deferred because this changes a library-wide default. There is also an inline Copilot comment on the new DTLS test whose content and resolution I can't see.
|
Jenkins retest this please |
With
WOLFSSL_DTLScompiled in,InitSSLset the default IO context of everyWOLFSSL, TLS included, to&ssl->buffers.dtlsCtx.EmbedReceive/EmbedSendread that as*(int*)ctx, and the first member of the DTLS context is zero, so a TLS object with no fd did its I/O on fd 0 (stdin) instead of failing. With stdin as an idle socket,test_tls13_apishangs inwolfSSL_read_early_data.&ssl->rfd/&ssl->wfd, or the DTLS context only for DTLS methods. NetX, Mynewt and GNRC keep their port context, also in DTLS builds.wolfSSL_set_rfd/wolfSSL_set_wfdnow wrapwolfSSL_set_read_fd/wolfSSL_set_write_fd.set_wfdon a DTLS object pointed the write context atssl->wfd, whichEmbedSendTothen read as aWOLFSSL_DTLS_CTX.Testing
test_tls_unset_fd_no_stdin_io: puts a socket on fd 0 and checks a client with no fd, and one with only a write fd, fail withSOCKET_ERROR_Ewithout touching fd 0; checks the default contexts of TLS and DTLS objects.test_dtls_set_rfd_wfd: DTLS handshake start overSSL_set_rfd/SSL_set_wfdon a datagram socketpair.make checkpasses with default,--enable-dtls,--enable-all, and--enable-all --enable-cryptocb --enable-asynccrypt-sw.