When calling SSL_stream_reset on a QUIC stream object that has received
all data that is expected to be sent (i.e. when the sender has sent a
STREAM frame with the FIN bit set), we encounter the following segfault:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f0bd28 in ossl_quic_sstream_get_final_size (qss=0x0, final_size=0x0) at ssl/quic/quic_sstream.c:273
273 if (!qss->have_final_size)
(gdb) bt
0) 0x00007ffff7f0bd28 in ossl_quic_sstream_get_final_size (qss=0x0, final_size=0x0) at ssl/quic/quic_sstream.c:273
1) 0x00007ffff7ef65bf in quic_validate_for_write (xso=0x5555555efcb0, err=0x7fffffffd5e0) at ssl/quic/quic_impl.c:2513
2) 0x00007ffff7ef8ae3 in ossl_quic_stream_reset (ssl=0x5555555efcb0, args=0x0, args_len=0) at ssl/quic/quic_impl.c:3657
3) 0x00007ffff7ebdaa6 in SSL_stream_reset (s=0x5555555efcb0, args=0x0, args_len=0) at ssl/ssl_lib.c:7635
4) 0x0000555555557527 in build_request_set (
req_list=0x55555555ebd0 "neil1.txt neil2.txt neil3.txt neil4.txt neil5.txt neil6.txt neil7.txt neil8.txt neil9.txt neil10.txt neil11.txt neil12.txt neil13.txt neil14.txt neil15.txt neil16.txt neil17.txt neil18.txt neil19.txt "..., ssl=0x5555555b6f80)
at demos/guide/quic-hq-interop.c:545
5) 0x00005555555587b2 in main (argc=4, argv=0x7fffffffe568) at demos/guide/quic-hq-interop.c:941
This occurs because:
1) When the stream FIN bit is set, the quic stack frees the underlying
stream structures immediately within the QUIC stack
and
2) when SSL_stream_reset is called, the call stack indicates we call
quic_validate_for_write, which attempts to access the
xso->stream->sstream QUIC_SSTREAM object, which was already freed in
(1)
The fix I think is pretty straightforward. On receipt of a STREAM frame
with a FIN bit set, the QUIC stack sets the QUIC_STREAM object state to
QUIC_SSTREAM_STATE_DATA_RECVD, which means we can use that state to
simply assert that the stream is valid for write, which allows it to be
reset properly.
Fixes#25410
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25910)
In some cases a QUIC SSL_CONNECTION object needs to get hold of a reference
to the original SSL object as created by the user. We should keep a
reference to it.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25874)
SSL_poll indicates that a stream which has had the fin bit set on it,
should generate SSL_POLL_EVENT_R events, so that applications can detect
stream completion via SSL_read_ex and SSL_get_error returning
SSL_ERROR_ZERO_RETURN.
However, the quic polling code misses on this, as a client that
completely reads a buffer after receipt has its underlying stream buffer
freed, loosing the fin status
We can however detect stream completion still, as a stream which has
been finalized, and had all its data read will be in the
QUIC_RSTREAM_STATE_DATA_READ state, iff the fin bit was set.
Fix it by checking in test_poll_event_r for that state, and generating a
SSL_POLL_EVENT_R if its found to be true, so as to stay in line with the
docs.
Fixesopenssl/private#627
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Sasa Nedvedicky <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25399)
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)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23535)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23535)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23535)
There was really no need for this to be void and it made bugs very easy
to introduce accidentally, especially given that the free functions
needed to be passed a pointer to the pointer.
Also fix some bugs in the QUIC code detected immediately by this change.
.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23519)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23360)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23360)
Freeing the QUIC TLS object may make calls back into QUIC so we should
free it first.
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23256)
Fixes Coverity 1548383
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22606)
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/22569)
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22485)
Return SSL_SENT_SHUTDOWN and SSL_RECEIVED_SHUTDOWN with semantics
similar to TLS connections.
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22408)
Although many of the QUIC tests use fake time, the time we pass to the
ossl_crypto_condvar_wait_timeout() must be a real time.
Passing fake time was causing the QUIC tserver test to hang because
ossl_crypto_convar_wait_timeout() always timed out immediately and never
relinquished the CPU.
If using fake time we adjust the time to real time just before using it.
Fixes#22020
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22301)
An application may pass in a whole BIO chain via SSL_set_bio(). When we
free the BIO we should be using BIO_free_all() not BIO_free() like we do
with TLS.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22157)
TLS misconfiguration errors should be shown to the application to enable
diagnosis of the problem. Otherwise you just get a generical "internal
error" message.
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22066)
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22011)
SSL_get_error() may respond with some retry errors that are not IO related.
In particular SSL_ERROR_WANT_RETRY_VERIFY and SSL_ERROR_WANT_X509_LOOKUP.
These can occur during a TLS handshake. If they occur when a QUIC Connection
is performing a TLS handshake then we need to propagate these up to the QCSO.
We also handle SSL_ERROR_WANT_CLIENT_HELLO_CB. This one will only ever
occur on the server side which we don't currently support. However adding
the handling for it now is identical to all the other handling so including
it is no cost, and will be needed when we do add server support.
We are not concerned with SSL_ERROR_WANT_ASYNC or SSL_ERROR_WANT_ASYNC_JOB
since we do not support async operation with QUIC.
Fixesopenssl/project#199
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21922)
Raise errors when appropriate.
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21700)
In some cases where a FIN has been received but with no data quic_read_actual
was failing to raise SSL_ERROR_ZERO_RETURN. This meant that we could end up
blocking in SSL_read(_ex) for too long.
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21780)
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)
If we shutdown the QUIC connection then we should mark the underlying
TLS SSL object as shutdown as well. Otherwise any sessions are considered
unusable for resumption.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21591)
s_client calls SSL_has_pending() even before the connection has been
established. We expect it to return 0 in this case and not put any errors
on the stack.
We change things so that SSL_has_pending() always returns 0 if there is
no stream available.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21578)
A call to SSL_write() with a zero length buffer should still advance the
handshake. Applications (including s_client) may rely on this.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21578)
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: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21381)
We save the error state from the thread that encountered
a permanent error condition caused by system or internal
error to the QUIC_CHANNEL.
Then we restore it whenever we are returning to a user
call when protocol is shutdown.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21087)
We return SSL_ERROR_SYSCALL when network error is encountered.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21087)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
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/20061)
QUIC differs from TLS in this regard because it buffers the
data to be sent. TLS just encrypts the data to send in place.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21311)
Fix supplied by Hugo Landau.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21204)
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/21029)
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/21029)
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/21029)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20879)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20879)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20879)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20879)
We create setter functions for the msg_callback and msg_callback_arg so
that these values can be properly propagated to the QRX/QTX/TXP even
after the channel has been created.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20914)
At this stage we just support msg_callback on receipt of a datagram.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20914)
This refactors the error raising code in the APL to automatically raise
errors in the correct SSL object, either a QCSO or QSSO depending on the
circumstances. The QCTX structure's responsibilities are expanded to
facilitate this. This further drives towards a unified mechanism for
generic dispatch precondition checking and error raising.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20856)
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)
QUIC in single-stream mode could be used with a protocol where the
server writes first or the client writes first. This determines
whether the single stream would be client or server initiated,
which affects the stream ID allocated to the stream. We should support
both client-sends-first and server-sends-first application protocols.
Thus, defer default XSO creation until the point in time at which
we know whether a client-first or server-first application protocol
is being used. We do this by taking whether SSL_read() or SSL_write()
is called first as a cue.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
We switch to instantiating the QUIC_CHANNEL up front at QCSO
instantiation time. This creates the QUIC_STREAM_MAP early and makes it
easy for us to allocate streams prior to connection initiation. The role
(client or server) is determined at QCSO allocation time and cannot be
changed.
SSL_set_connect/accept_state() are still modelled but their usage must
be consistent with the chosen SSL_METHOD which dictates which role is
being used.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
In QUIC, we have an architectural need (in future, when we implement
0-RTT, etc.) to be able to create streams before we start connecting.
This requires we allocate a stream, including a stream ID, after
creating a QCSO but prior to connecting. However stream IDs are
dependent on whether the endpoint is in the client or server role,
therefore we must know whether we are going to be a client or server
before any pre-connection streams are created. Moreover, the originally
defined QUIC_client_method() and QUIC_server_method() functions heavily
implied the original plan was to have different SSL_METHODs for clients
and servers. Up until now we had been relying on
SSL_set_connect/accept_state() instead.
Solve these problems by basing client/server identity on whether
QUIC_server_method() is used (in future, when we support servers). This
ensures that once a QCSO is created its client/server identity are fixed
and cannot change, allowing pre-connection stream IDs, etc. to be
allocated.
Client/server uncertainty was the primary reason why QUIC_CHANNEL
creation was deferred until connection time up until now, so this
enables further refactoring to facilitate eager allocation of the
QUIC_CHANNEL at QCSO allocation time. This is important as allocating a
stream including its write buffers is hard without having the
QUIC_CHANNEL (which owns the QUIC_STREAM_MAP) in existence.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
We now refactor the interface between ssl_lib.c frontend functions and
the QUIC API Personality Layer so that the respective functions
comprising the interface use SSL object pointers rather than raw
QUIC_CONNECTION pointers. This is in preparation for stream support
since once streams are supported, calls to e.g. ossl_quic_write() may be
made on a QUIC_CONNECTION or a QUIC_XSO (that is, a stream object). Thus
we take a uniform approach across all functions comprising the interface
between the ssl_lib.c frontend and the QUIC API Personality Layer of
using SSL pointers always. This provides a uniform approach and
ensures that any function of the API personality layer can be easily
adapted to support being called on a stream object in the future.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
Uniform changes to all dispatch functions to use the new dispatch
functionality follows this commit. Separated into a core commit
and a commit containing the uniform pattern (monotonous) changes
for ease of review.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)
Neil Horman
Matt Caswell
Tomas Mraz
Hugo Landau
Richard Levitte
Pauli