Uh oh!
There was an error while loading. Please reload this page.
QUIC server: limit the number of pending connections - #32052
Conversation
5a94a72 to
3eeb801Comparec0345ba to
b3582adCompareUh oh!
There was an error while loading. Please reload this page.
| =item B<SSL_VALUE_QUIC_MAX_PENDING_CHANNELS> (listener object) | ||
| This sets the limit on channels (connection objects) which QUIC server can |
There was a problem hiding this comment.
I'd prefer not using the word channels at all as this is internal name. For the public API these are always referred as connections.
| #define DEFAULT_INIT_CONN_MAX_STREAMS 100 | ||
| #define DEFAULT_MAX_PENDING_CHANNELS 32 |
There was a problem hiding this comment.
Please note we have this PR for similar work in DTLS. #31980
There the default limit is 1000 (which sounds a little excessive). However 32 might be too low I think. What about 256?
There was a problem hiding this comment.
I was not aware of DTLS PR, thanks for pointing me at it.
I agree with 256. I will bump it to 256 and will be keeping an eye on #31980 to make sure both QUIC and DTLS will be using the same default.
t8m
commented
Jul 28, 2026
One more note - the DTLS listener adds also SSL_VALUE_DTLS_LISTENER_PENDING_TIMEOUT - is that something that could be useful for QUIC too? |
Sashan
commented
Jul 28, 2026
I'm not sure. if I understand the DTLS documentation correct the DTLS-SSL object enters accept queue after handshake completes, correct? the QUIC connection is accepted before TLS handshake is done, it's server's responsibility to keep calling SSL_handle_events()/SSL_read()/SSL_write() to finish handshake. if remote peer (client) stops to be responsive the server should find it out because there will be no answers to PING frames which can be sent on all QUIC encryption levels. The default timeout ( So it does not seem to be useful for QUIC. |
andrewkdinh
left a comment
There was a problem hiding this comment.
check_docs CI jobs is relevant
| if (!TEST_true(create_quic_conn_objects(cctx, sctx, &clientssl, &serverssl_listener))) | ||
| goto end; | ||
| testresult = SSL_set_feature_request_uint(serverssl_listener, |
There was a problem hiding this comment.
If we goto end after this, testresult will always be 1
| * initiate yet another connection. The connection must not be inserted | ||
| * to pending queue. The pending_connections must be 5. | ||
| */ | ||
| for (i = 0; i < 10; i++) { |
There was a problem hiding this comment.
This is going to read out of bound since extra_clients is only length 5
There was a problem hiding this comment.
Took a closer look at it. I'm using OSSL_NELEM() I've also itroduced PENDING_LIMIT constant. the extra_connections array is set to be PENDING_LIMIT * 2. also watchdog iterations is now 10 instead of 1000. 10 iterations should be enough to complete handshake. And we don't actually need to finish handshake we just need enough cycles to send a retry packet.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| This release of OpenSSL uses a default value of 25 milliseconds. This default | ||
| value may change between releases of OpenSSL. | ||
| =item B< SSL_VALUE_QUIC_MAX_PENDING_CONNS> (listener object) |
There was a problem hiding this comment.
Superfluous space before SSL_VALUE_QUIC_MAX_PENDING_CONNS.
| =item B< SSL_VALUE_QUIC_MAX_PENDING_CONNS> (listener object) | ||
| This sets the limit on channels (connection objects) which QUIC server can |
There was a problem hiding this comment.
Should it include some description like "Feature request value."?
There was a problem hiding this comment.
to be honest I don't know. I'm aware of 'Feature request value' phrase used in the manpage, but I'm not quite sure what it actually means. I prefer to use stuff/constructs I do understand. here I am in doubts. my understanding is the feauture request value is something local end tranmits to its remote peer when connection is being established. this max pending connections is local parameter only it is not transmitted to remote peer during handshake. therefore I have not added it here.
There was a problem hiding this comment.
I believe it refers to the fact that the class of the configuration value being SSL_VALUE_CLASS_FEATURE_REQUEST.
There was a problem hiding this comment.
It should not be SSL_VALUE_CLASS_FEATURE_REQUEST. It isn't negotiated. It should be SSL_VALUE_CLASS_GENERIC. The former is for features that take part in the negotiation process. Generic is for:
Values in this class do not participate in the feature negotiation process.
There was a problem hiding this comment.
thanks for clarification, there is one less mystery in my life now.
| =item B< SSL_VALUE_QUIC_MAX_PENDING_CONNS> (listener object) | ||
| This sets the limit on channels (connection objects) which QUIC server can | ||
| insert into list of pending connections. The pending connection is connection |
| =item B< SSL_VALUE_QUIC_MAX_PENDING_CONNS> (listener object) | ||
| This sets the limit on channels (connection objects) which QUIC server can | ||
| insert into list of pending connections. The pending connection is connection |
| This sets the limit on channels (connection objects) which QUIC server can | ||
| insert into list of pending connections. The pending connection is connection | ||
| which local application needs to accept (L<SSL_accept_connection(3)>) in order to retrieve |
| goto end; | ||
| ok = SSL_set_feature_request_uint(serverssl_listener, | ||
| SSL_VALUE_QUIC_MAX_PENDING_CONNS, 5); |
There was a problem hiding this comment.
What is the PENDING_LIMIT macro constant for, then?
| BIO *c_bio, *s_bio; | ||
| SSL *c_ssl, *s_ssl; |
There was a problem hiding this comment.
Why not
BIO *c_bio = NULL, *s_bio = NULL;
SSL *c_ssl = NULL, *s_ssl = NULL;
?
| for (i = 0; i < PENDING_LIMIT; i++) { | ||
| watchdog = 0; | ||
| done = 0; | ||
| while (!done && watchdog++ < 10) { |
There was a problem hiding this comment.
What 10 means here? Is it OSSL_NELEM(extra_clients), or something else?
There was a problem hiding this comment.
the loop needs to keep calling SSL I/O functions (SSL_handle_events(), SSL_connect()) on SSL objects to keep handshake operation going. it's like one loop iteration involves like request being transmitted to peer and eventual response back. one iteration of the loop performs like one step in protocol. 10 steps should be enough to get handshake done. handshake usually taks like 3 iterations if i remember correct.
There was a problem hiding this comment.
Ah, okay, then you can either add a separate macro constant for that (HANDLE_EVENTS_LOOP_LIMIT or something), ideally, so the intention is easier to parse, or just leave it as-is.
There was a problem hiding this comment.
I will do s/watchdog/handshake_step and s/10/HANDSHAKE_STEPS this will be better then current watchdog
| for (i = PENDING_LIMIT; i < OSSL_NELEM(extra_clients); i++) { | ||
| watchdog = 0; | ||
| done = 0; | ||
| while (!done && watchdog++ < 10) { |
There was a problem hiding this comment.
10 steps/ticks on connection to keep protocol working. there is no established pattern yet for this. at least I have not noticed one.
| while (!done && watchdog++ < 10) { | ||
| /* | ||
| * connections are never accepted by the server. The SSL_connect() | ||
| * for non-blocking client returns -1 to keep connect retrying | ||
| */ | ||
| if (!TEST_int_le(SSL_connect(extra_clients[i]), 0)) | ||
| goto end; | ||
| SSL_handle_events(serverssl_listener); | ||
| pending_connections = ossl_quic_port_get_num_incoming_channels(port); | ||
| done = (pending_connections == (i + 1)); | ||
| } |
There was a problem hiding this comment.
Why not factor it out in a separate function?
There was a problem hiding this comment.
the code is similar but not same. note calls to TEST_int_le() (line 3926, second for loop) vs. TEST_int_lt() line 3903, the first loop).
| if (port->max_pending_channels > 0 && ossl_list_incoming_ch_num(&port->incoming_channel_list) >= port->max_pending_channels) | ||
| goto undesirable; | ||
There was a problem hiding this comment.
do we want to do this check earlier in port_default_packet_handler? I ask because otherwise we might preform version negotiation with a client, and then just drop the connection later - i.e. the server is going to do extra work when its already over connection capacity. Like maybe do the check around line 1648, immediately after the check for port->allow_incoming ?
There was a problem hiding this comment.
I don't think so. I feel there are at least two reasons to keep code here as-is:
- we should do the check when we are dealing with INITIAL packet
- also the number of pending connection may decrease while we run version negotiation.
Currently, there is no limit for pending QUIC connections. The port default packet handler creates channel for every valid initial packet which does belong to existing channel (a.k.a. connection). The newly created channel is inserted to list of pending channels where it waits to be accepted by local application by call to SSL_accept_connection(3ossl). This change introduces a limit for pending connection. The pending queue is limited to 256 pending connections. Applications may change the limit by calling SSL_set_feature_request_uint(3ossl) on SSL server listener object with configurable value SSL_VALUE_QUIC_MAX_PENDING_CONNS. Fixes: CVE-2026-14456
Add the following helper functions: * create_quic_ctx_pair() - creates pair of SSL_CTX (server, client). * create_quic_conn_objects() - creates pair of SSL objects, client and listener. They both are 'connected' by BIO_dgram_pair. * create_quic_client() - creates SSL QUIC client object bound to BIO object provided by caller.
This is a regression test for CVE-2026-14456.
This pull request is ready to merge |
Currently, there is no limit for pending QUIC connections. The port default packet handler creates channel for every valid initial packet which does belong to existing channel (a.k.a. connection). The newly created channel is inserted to list of pending channels where it waits to be accepted by local application by call to SSL_accept_connection(3ossl). This change introduces a limit for pending connection. The pending queue is limited to 256 pending connections. Applications may change the limit by calling SSL_set_feature_request_uint(3ossl) on SSL server listener object with configurable value SSL_VALUE_QUIC_MAX_PENDING_CONNS. Fixes: CVE-2026-14456 Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:25 2026 (Merged from #32052)
Add the following helper functions: * create_quic_ctx_pair() - creates pair of SSL_CTX (server, client). * create_quic_conn_objects() - creates pair of SSL objects, client and listener. They both are 'connected' by BIO_dgram_pair. * create_quic_client() - creates SSL QUIC client object bound to BIO object provided by caller. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:27 2026 (Merged from #32052)
This is a regression test for CVE-2026-14456. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:30 2026 (Merged from #32052)
Currently, there is no limit for pending QUIC connections. The port default packet handler creates channel for every valid initial packet which does belong to existing channel (a.k.a. connection). The newly created channel is inserted to list of pending channels where it waits to be accepted by local application by call to SSL_accept_connection(3ossl). This change introduces a limit for pending connection. The pending queue is limited to 256 pending connections. Applications may change the limit by calling SSL_set_feature_request_uint(3ossl) on SSL server listener object with configurable value SSL_VALUE_QUIC_MAX_PENDING_CONNS. Fixes: CVE-2026-14456 Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:25 2026 (Merged from #32052) (cherry picked from commit 9416706)
Add the following helper functions: * create_quic_ctx_pair() - creates pair of SSL_CTX (server, client). * create_quic_conn_objects() - creates pair of SSL objects, client and listener. They both are 'connected' by BIO_dgram_pair. * create_quic_client() - creates SSL QUIC client object bound to BIO object provided by caller. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:27 2026 (Merged from #32052) (cherry picked from commit f20e513)
This is a regression test for CVE-2026-14456. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:30 2026 (Merged from #32052) (cherry picked from commit 0461a56)
Currently, there is no limit for pending QUIC connections. The port default packet handler creates channel for every valid initial packet which does belong to existing channel (a.k.a. connection). The newly created channel is inserted to list of pending channels where it waits to be accepted by local application by call to SSL_accept_connection(3ossl). This change introduces a limit for pending connection. The pending queue is limited to 256 pending connections. Applications may change the limit by calling SSL_set_feature_request_uint(3ossl) on SSL server listener object with configurable value SSL_VALUE_QUIC_MAX_PENDING_CONNS. Fixes: CVE-2026-14456 Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:25 2026 (Merged from #32052) (cherry picked from commit 9416706)
Add the following helper functions: * create_quic_ctx_pair() - creates pair of SSL_CTX (server, client). * create_quic_conn_objects() - creates pair of SSL objects, client and listener. They both are 'connected' by BIO_dgram_pair. * create_quic_client() - creates SSL QUIC client object bound to BIO object provided by caller. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:27 2026 (Merged from #32052) (cherry picked from commit f20e513)
This is a regression test for CVE-2026-14456. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:30 2026 (Merged from #32052) (cherry picked from commit 0461a56)
nhorman
commented
Aug 12, 2026
merged to master, 4.0, 3.6 and 3.5, thank you |
Currently, there is no limit for pending QUIC connections. The port default packet handler creates channel for every valid initial packet which does belong to existing channel (a.k.a. connection). The newly created channel is inserted to list of pending channels where it waits to be accepted by local application by call to SSL_accept_connection(3ossl). This change introduces a limit for pending connection. The pending queue is limited to 256 pending connections. Applications may change the limit by calling SSL_set_feature_request_uint(3ossl) on SSL server listener object with configurable value SSL_VALUE_QUIC_MAX_PENDING_CONNS. Fixes: CVE-2026-14456 Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:25 2026 (Merged from #32052) (cherry picked from commit 9416706) (cherry picked from commit 4084152)
Add the following helper functions: * create_quic_ctx_pair() - creates pair of SSL_CTX (server, client). * create_quic_conn_objects() - creates pair of SSL objects, client and listener. They both are 'connected' by BIO_dgram_pair. * create_quic_client() - creates SSL QUIC client object bound to BIO object provided by caller. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:27 2026 (Merged from #32052) (cherry picked from commit f20e513) (cherry picked from commit 50c55ee)
This is a regression test for CVE-2026-14456. Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Andrew Dinh <andrewd@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> MergeDate: Wed Aug 12 15:00:30 2026 (Merged from #32052) (cherry picked from commit 0461a56) (cherry picked from commit d446963)
Checklist