Fix 3 SSL_poll bugs - #5
Open
mattcaswell wants to merge 131 commits into
Open
Conversation
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#22259)
Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from openssl#22259)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22260)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22260)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22260)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22261)
…n 5.9 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22416)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22416)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22273)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22273)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22273)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22366) Updated the logic in ssl_cipher_list_to_bytes to take account of the changes from PR#24161 Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24226)
DTLS 1.3 session id must not be sent by client unless it has a cached id. And DTLS 1.3 servers must not echo a session id from a client. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22366)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22366)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22366)
SSL_CONNECTION_IS_VERSION13 macro is used where appropriate. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22364)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#23375)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22362)
…ches ssl3_read_bytes and ssl3_write_bytes Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22360)
…ync with their tls counterparts. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22360)
…ite() for tls1.3 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22360)
…tes() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22360)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22380)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22401)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22936)
Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from openssl#22936)
…er() Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22376)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22378)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24293)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24293)
Fixing up two tests that are not putting skip's in a SKIP block. These were identified by a FreeBSD image. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> MergeDate: Thu May 7 12:27:29 2026 (Merged from openssl#31089)
Added an ossl-guid-dtlsv13 file to describe OpenSSL's DTLSv1.3 implementation. Fixes: openssl/project#1831 Fixes: openssl/project#1830 Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Thu May 21 08:50:49 2026 (Merged from openssl#31032)
Added a DTLS 1.3 demo based off of the SSL echo demo. The client takes input from the user and sends it to the server. The server echos that input back. There was a bug if the server retransmitted New Session tickets cause it didn't receive the ACK for the first set of New Session tickets. The changes outside of the demo application are to fix that bug. Fixes: openssl/project#1943 Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 27 08:02:57 2026 (Merged from openssl#31042)
DTLSv1_listen will not support DTLSv1.3. Instead its max version is DTLSv1.2 Adding DTLS SSL Listener that utilizes a similar DEMUX approach as QUIC and passes traffic through the URXE structure. This will establish full connection and an application can ask for a ready SSL connection. The DTLS SSL listener also supports HVR and HRR cookie validation. Fixes: openssl/project#1946 Reviewed-by: Tomas Mraz <tomas@openssl.foundation> Reviewed-by: Matt Caswell <matt@openssl.foundation> MergeDate: Mon Jul 6 09:29:09 2026 (Merged from openssl#31137)
Several DTLS 1.3 test cases driven through TLSProxy currently fail or hang. To keep the tree buildable and testable while the DTLS 1.3 work is under review, skip the affected sub-tests (or whole DTLS runs) with TODO(DTLS1.3) markers. These are to be addressed in a follow-up. Skipped for DTLS: - 70-test_tls13psk.t: "Remove sig algs", "Too many PSKs" and "Bad PSK with no handshake fallback" - 70-test_tls13cookie.t: the DTLS run - 70-test_sslrecords.t: "Empty app data in DTLS" - 70-test_tls13downgrade.t: the DTLS run - 70-test_tls13messages.t: the DTLS run Also skipped in 70-test_tls13messages.t: "HelloRequest rejected in TLSv1.3" and "HelloRequest ignored in (D)TLSv1.2". These fail for TLS as well as DTLS, so the failure is not proxy-specific and needs separate investigation. Assisted-by: Claude Code:claude-opus-4-8 Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Mon Jul 13 14:24:27 2026 (Merged from openssl#31896)
Listener-created DTLS connections could not complete a handshake: the peer address was applied to the initial plaintext record layers by SSL_set1_initial_peer_addr(), but the write record layer recreated when encryption is enabled started with no peer, so tls_retry_write_records() fell back from BIO_sendmmsg() to BIO_write() on the shared listener BIO and the bytes went nowhere. This adaptation was lost when the branch was rebased: the change was originally written when the new_record_layer record method callback took a peer argument, whereas current record layers set the peer via the separate set1_peer callback. Re-apply the peer to every DTLS record layer as it is created in ssl_set_new_record_layer(), mirroring the existing use_urxe handling. Fixes test_dtlsssllistener and test_dtls_multithread. Assisted-by: Claude Code:claude-opus-4-8 Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Mon Jul 13 14:24:18 2026 (Merged from openssl#31896)
Need to fix a deprecated API in dtlsecho. Also updated the sslecho usage information since it is no longer correct. Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Thu Jul 16 07:29:15 2026 (Merged from openssl#31935)
DTLSv1.3 suppport was added into SSL_stateless, but it doesn't work properly. The reason why it doesn't work properly is that it doesn't handle concurrent DTLS sessions being established at the same time. If you want something that allows DTLS 1.3 connections being establish at the same time use DTLS SSL Listener. Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed Jul 22 15:55:14 2026 (Merged from openssl#31364)
In DTLS 1.3 the handshake sequence number is not reset to 0 after the connection is establish. This means New Session tickets will not send out with a handshake seq of 0. Instead it will pick up where it left off. Fixes: openssl/project#2008 Assisted-by: Claude:claude-opus-4-7 Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Mon Jul 27 12:20:10 2026 (Merged from openssl#31978)
mattcaswell
force-pushed
the
fix-dtls-ssl-poll-bugs
branch
9 times, most recently
from
August 7, 2026 15:33
a333d53 to
449b499
Compare
Now when a buffer BIO is created for the initial DTLS handshake from an SSL listener messages are now buffered until a flush is called. Fixes: openssl/project#1982 Assisted-by: Claude:claude-opus-4-7 Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Jakub Zelenka <jakub.zelenka@openssl.foundation> MergeDate: Tue Aug 11 14:41:07 2026 (Merged from openssl#31979)
To prevent resource exhaustion we have to cap the number of pending connections. The default is 256 pending connections but there are APIs to configure the max number of pending connections. Fixes: openssl/project#1983 Fixes: openssl/project#2035 Assisted-by: Claude:claude-opus-4-7 Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Jakub Zelenka <jakub.zelenka@openssl.foundation> MergeDate: Tue Aug 11 14:50:11 2026 (Merged from openssl#31980)
Updating the CHANGES.md for all the changes added via the DTLS1.3 feature branch. Fixes: openssl/project#2039 Assisted-by: Claude:claude-opus-4-8 Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com> MergeDate: Tue Aug 11 14:52:30 2026 (Merged from openssl#32182)
The listener's notifier was signalled when the demux routed a datagram into a
connection's receive queue, but not when dtls_listener_drive_pending() moved
a completed connection onto the accept queue - even though the latter is
precisely the readiness event a thread waiting in SSL_poll() for
SSL_POLL_EVENT_IC is waiting for.
Usually the injection signal covers for this. Every connection reaching the
accept queue got there because a datagram was demuxed into its receive queue,
so the waiter is woken by that, ticks the listener during its own readout,
and finds the connection. The exception is the window in which the injection
and the queue push straddle a waiter registering, since signalling only
happens if there is a waiter at the time:
1. Accept thread A polls the listener for SSL_POLL_EVENT_IC. Its readout
ticks the listener, finds nothing, and it decides to block. It is not a
registered waiter yet.
2. Worker thread B polls one of its own connections, which also ticks the
listener. The pump reads a client's final ClientHello and injects it into
that pending connection's queue. There are no waiters, so nothing is
signalled.
3. A enters the blocking section. Its re-check runs without ticking, so it
sees only the accept queue, still empty, and blocks.
4. B's tick reaches dtls_listener_drive_pending(), which completes the
connection against the buffered ClientHello and pushes it onto the accept
queue.
With no signal at step 4, A sleeps on while a validated connection sits ready
to be accepted, until an unrelated datagram makes the socket readable again.
B consumed the only one in flight and the client is now waiting on the server,
so on a quiet listener that means waiting for the client to retransmit its
handshake.
Factor the signalling out of dtls_listener_packet_handler() into a helper and
call it from both places. Both call sites already hold the listener mutex,
which the helper requires.
Assisted-by: Claude Code:claude-opus-5
A DTLS listener added only the network socket to the poll set. Three things
followed from that. The notifier was not in the poll set, so a thread blocked
in SSL_poll() on the listener could not be woken by it. The thread never
called ossl_dtls_listener_enter_blocking_section(), so cur_blocking_waiters
stayed at zero - and since signalling is conditional on there being a waiter,
no other thread even attempted to signal. And there was no re-check after
registering, so readiness arising between the readout deciding nothing was
ready and the wait actually starting was lost.
Polling the socket alone is not sufficient, though not because a wakeup can
be missed outright. poll() reports whatever is currently sitting in the
socket buffer and returns immediately if there is any, so a thread cannot
miss a datagram just by being outside poll() when it arrives. What it can
miss is a datagram another thread has already taken. With several threads
polling the one shared socket that happens constantly: an arriving datagram
wakes all of them, only one gets it, and the rest find nothing. Any of them
can be the one that takes it, because SSL_read() on a connection pumps the
demux and SSL_poll() on a connection ticks the whole listener.
1. Accept thread A polls the listener for SSL_POLL_EVENT_IC. Its readout
ticks the listener, finds nothing, and it decides to block.
2. A client's final ClientHello lands on the shared socket.
3. Worker thread B, polling one of its own connections, ticks the listener
and is the one that takes the datagram. Its tick completes the pending
connection and pushes it onto the accept queue. Signalling is attempted,
but A never registered as a waiter, so nothing is signalled.
4. A reaches its poll, watching the socket alone. B drained it, so it is
empty. A sleeps with a validated connection sitting on the accept queue,
and nothing can wake it until an unrelated datagram arrives or its
deadline expires.
A's readout, back at step 1, would have found that connection had it run
after step 3 rather than before it.
Add the notifier FD to the poll set and bracket the wait with the enter/leave
blocking section calls, mirroring what the listener-based connection path
already does. As there, re-check readiness once inside the section, since it
is only from that point that a readiness event is guaranteed to make the
notifier readable, and abort blocking if the listener became ready in the
meantime. The re-check removes the dependency on that ordering; the notifier
covers anything arising after it.
Note that the signal raised when a connection is pushed onto the accept queue
is itself conditional on a registered waiter, so it does nothing for a thread
polling the listener until that thread registers here. The two changes are
complementary and neither is sufficient alone.
Assisted-by: Claude Code:claude-opus-5
mattcaswell
force-pushed
the
fix-dtls-ssl-poll-bugs
branch
from
August 12, 2026 07:33
de5e751 to
6298f57
Compare
When SSL_poll() decides to block it computes a wakeup deadline from the per-object event timeout, so that timer driven work is not delayed by the wait. This was done for QUIC objects but not for DTLS connections, whose timeout is the handshake retransmission timer. The consequence was that a poll with no user timeout would sleep until a datagram arrived, straight through the point at which the connection should have retransmitted. If the peer had itself lost the message we were waiting for, neither side would make progress. Fold SSL_get_event_timeout() into the deadline for DTLS connections as the QUIC branch already does. It reports the DTLS timer already, so no new plumbing is needed there. Waking at the deadline is only useful if something then services the timer. Unlike the QUIC case, where the readout ticks the reactor and that handles timeouts, the DTLS readout only pumped the listener's demux. Nothing retransmitted, and because an expired timer reports zero time remaining the recomputed deadline would be "now" on every subsequent wait, turning the sleep into a spin. Call SSL_handle_events() from the readout as well, which is what a caller polling without SSL_POLL_FLAG_NO_HANDLE_EVENTS is asking for, and which for a DTLS connection services that timer. Both of those are the calls the documentation asks new code to use: DTLSv1_get_timeout(3) and DTLSv1_handle_timeout(3) each record that SSL_get_event_timeout(3) and SSL_handle_events(3) respectively supersede all of their use cases. The test asserts its precondition through SSL_get_event_timeout() for the same reason, which has the side benefit of using the same call SSL_poll() uses to compute the deadline being tested. Assisted-by: Claude Code:claude-opus-5
dtls1_handle_timeout() fails the connection with SSLfatal() once dtls1_check_timeout_num() reports that DTLS1_TMO_ALERT_COUNT unanswered retransmissions have been sent. It returns at that point without reaching dtls1_start_timer(), so next_timeout is left holding a time in the past. Nothing re-arms or clears it afterwards, the connection being finished, so every later dtls1_get_timeout() computes a negative remaining time, clamps it to zero and reports the timeout as due immediately. A caller which waits on that timeout therefore never waits at all. The documented DTLSv1_get_timeout() plus select() loop turns into a busy loop on any connection which has given up retransmitting, consuming a core until the application gives up on the connection itself. Stop the timer instead. dtls1_stop_timer() zeroes next_timeout, which dtls1_get_timeout() already reports as "no timer running", so callers see no pending timeout and wait on whatever other deadline they have. The test drives this without waiting by forcing a retransmission timeout of 1ms, which is below the 15ms floor at which dtls1_get_timeout() treats a timer as already expired. The server therefore retransmits on every read attempt and exhausts its budget inside SSL_accept(). Assisted-by: Claude Code:claude-opus-5
poll_translate_ssl_dtls_conn() adds no file descriptor to the poll set when the connection's BIO cannot supply one, which is the case for the memory and datagram-pair BIOs the test suite is built on. Unlike the QUIC and DTLS listener paths it does not report that as an error, and rightly so: a DTLS connection with no pollable BIO has no readiness to wait for, but it still has a retransmission deadline to wake for, so a wait with nothing to watch is meaningful. The resulting empty descriptor set was then handed to the operating system, where the behaviour differs. poll() treats it as a plain sleep, so this worked on Unix. Windows' select() rejects a call with no descriptors, so ossl_rio_poll_builder_poll() returned failure and SSL_poll() failed with it. Sleep out the deadline explicitly rather than relying on the platform. With no descriptors and no deadline nothing could ever provide a wakeup, so that combination is reported as a failure instead of sleeping for ever. This has no test of its own: on any platform whose poll() already sleeps, the behaviour before and after is identical, so only the Windows CI builds distinguish them. Assisted-by: Claude Code:claude-opus-5
mattcaswell
force-pushed
the
fix-dtls-ssl-poll-bugs
branch
from
August 12, 2026 07:51
6298f57 to
501af84
Compare
Compare short_timer_cb_count with the matching macro. It is an unsigned int, so TEST_uint_gt() is the right one rather than TEST_size_t_gt().
mattcaswell
force-pushed
the
feature/dtls-1.3
branch
from
August 24, 2026 10:35
deb2f12 to
39a43f7
Compare
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
5 participants
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.
Checklist