Current QUIC stack may leave connection monitored by SSL_poll() to stale
during regular shutdown. The issue is triggered when ACK for client's
FIN gets delayed. The sequeance of operations to trigger
the stale of QUIC connection at client goes as follows:
- application calls SSL_shutdown() on connection,
the shutdown can not proceed, because bi-directional
stream must be flushed. The client awaits ACK from
server acknowledging reception of FIN on client's stream
- the stream object gets destroyed, because application
received all data from server.
- application updates poll set and passes to SSL_poll()
- ssl poll ticks the engine. Engine receives delayed ACK
and marks stream as flushed. At this point the SSL_shutdown()
operation may proceed given the application calls the
SSL_shutdown(). However there is no mechanism to make SSL_poll()
return so application is unable to proceed with its event
loop where SSL_shutdown() may get called.
This change introduces ossl_quic_channel_notify_flush_done() function
which notifies channel when all streams are flushed (all FINs got ACKed).
The first thing SSL_shudown() does it calls ossl_quic_stream_map_begin_shutdown_flush().
The function walks list of all streams attached to channel and notes how many
streams is missing ACK for their FIN. In our test case it finds one such stream.
Call to SSL_shutdown() returns and application destroys the SSL stream object
and updates a poll set.
SSL_poll() gets called. The QUIC stack (engine) gets ticked and reads data
from socket. It processes delayed ACK now. The ACK-manager updates the
stream notifying the server ACKs the FIN sent by client. The stream
is flushed now. Thw shutdown_flush_done() for stream gets called on
behalf of ACK manager.
The shutdown_flush_done() does two things:
- it marks stream as flushed
- it decrements the num_shutdown_flush counter initialized
be earlier call to ossl_quic_stream_map_begin_shutdown_flush()
called by SSL_shutdown()
The change here calls ossl_quic_channel_notify_flush_done() when
num_shutdown_flush reaches zero.
The ossl_quic_channel_notify_flush_done() then calls function
ossl_quic_channel_notify_flush_done(), which just moves the state
of the channel (connection) from active to terminating state.
The change of channel state is sufficent for SSL_poll() to
signal _EC event on connection.
Once application receives _EC event on connection it should
check the state of the channel/reason of error. In regular case
the error/channel state hints application to call SSL_shutdown()
so connection object can proceed with connection shutdown.
The SSL_shutdown() call done now moves channel to terminated
state. So the next call to SSL_poll() can signal _ECD which
tells application it's time to stop polling on SSL connection
object and destroy it.
Fixesopenssl/project#1291
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28116)
Introduces SSL_ACCEPT_STREAM_UNI and SSL_ACCEPT_STREAM_BIDI flags to SSL_accept_stream, allowing callers to specify whether to accept only unidirectional or bidirectional streams. Returns the first of its type from the queue
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27883)
and `ossl_quic_stream_has_send_buffer()` explicitly.
This small tweak allows to find those functions more easily.
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27438)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Release: yes
(cherry picked from commit 0ce7d1f355)
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24034)
For some of the items we add FUTURE/SERVER/TESTING/MULTIPATH
designation to indicate these do not need to be resolved
in QUIC MVP release.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21539)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)
This allows QUIC_STREAM objects to be deleted when they are no longer
needed.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
We need to get acknowledgement notifications for our STOP_SENDING and
STREAM_RESET frames as this information is needed to know when we can
delete a QUIC_STREAM object.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
Also use accept queue popping by the application as the retirement
event, i.e., as the cue to increase the limit.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19346)
Alexandr Nedvedicky
Andrew Dinh
Tomas Mraz
sashan
openssl-machine
Hugo Landau
Richard Levitte
Matt Caswell