This change introduces a new public API symbol: SSL_get_peer_addr().
The change is QUIC-only, there are no changes for TLS connections
- API: add peer address query for QUIC connections
* Internal: declare/implement ossl_quic_get_peer_addr(SSL*, BIO_ADDR*)
* Public: declare/implement SSL_get_peer_addr(SSL*, BIO_ADDR*)
Rationale:
- Allow applications to retrieve the remote UDP tuple for QUIC sessions
(e.g., logging, access control, diagnostics)
Provided documentation and test cases for SSL_get_peer_addr().
Set peer via channel API on new-conn.
- In ch_on_new_conn_common(), BIO_ADDR_copy(&ch->cur_peer_addr, peer)
was replaced with ossl_quic_channel_set_peer_addr(ch, peer) so
addressed_mode is enabled at connection bring-up.
Dropped redundant peer detection in create_qc_from_incoming_conn()
The peer address is now propagated in ch_on_new_conn_common() via
ossl_quic_channel_set_peer_addr(), so the channel is already in
"addressed" mode. This also avoids querying the (unconnected) server
UDP BIO, reduces duplication, and simplifies the accept path. All
regression tests pass.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28690)
Github issue #28501 reported an odd condition in which a double free was
occuring when a given thread was popping entries of its error stack.
It was hypothesized that, because a few places in the quic stack save
error state to a shared structure (ch->err_state, port->error_state,
qtls->error_state), that multiple threads may attempt to mutate the
shared structure during error save/restore in parallel.
Investigation showed that all paths which led to such mutations were
done under lock, so that shouldn't occur.
Except for one case, which this PR addresses.
In ossl_quic_conn_stream_conclude, we unlock our protecting mutex, prior
to calling QUIC_RAISE_NON_NORMAL_ERROR. If that function is called with
an reason code of SHUTDOWN, it attempts to restore the channel error
state. Given that the lock was released first, this creates a small
race condition in which two threads may manipulate the shared error
state in the channel struct in parallel.
According to the reporter, applying this patch prevents the reported
error from occuring again.
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28642)
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)
ossl_quic_free_token_store doesn't call CRYPTO_FREE_REF on the
hdl->reference object, which could lead to memory leaks on platforms
that don't support atomics (where the call to CRYPTO_NEW_REF allocates a
mutex as part of its function. It wasn't caught before because all the
platforms we do ci on support threads.
Fixes#28241
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28247)
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)
Run-checker merge / run-checker (enable-weak-ssl-ciphers) (push) Waiting to runDetails
Run-checker merge / run-checker (enable-zlib) (push) Waiting to runDetails
Run-checker merge / run-checker (no-dso) (push) Waiting to runDetails
Run-checker merge / run-checker (no-dynamic-engine) (push) Waiting to runDetails
Run-checker merge / run-checker (no-ec2m enable-fips) (push) Waiting to runDetails
Run-checker merge / run-checker (no-engine no-shared) (push) Waiting to runDetails
Run-checker merge / run-checker (no-err) (push) Waiting to runDetails
Run-checker merge / run-checker (no-filenames) (push) Waiting to runDetails
Run-checker merge / run-checker (no-integrity-only-ciphers) (push) Waiting to runDetails
Run-checker merge / run-checker (no-module) (push) Waiting to runDetails
Run-checker merge / run-checker (no-ocsp) (push) Waiting to runDetails
Run-checker merge / run-checker (no-pinshared) (push) Waiting to runDetails
Run-checker merge / run-checker (no-srp) (push) Waiting to runDetails
Run-checker merge / run-checker (no-srtp) (push) Waiting to runDetails
Run-checker merge / run-checker (no-ts) (push) Waiting to runDetails
Run-checker merge / jitter (push) Waiting to runDetails
Run-checker merge / threads_sanitizer_atomic_fallback (push) Waiting to runDetails
Windows GitHub CI / shared (map[arch:win32 config:--strict-warnings no-fips os:windows-2022]) (push) Waiting to runDetails
Windows GitHub CI / shared (map[arch:win64 config:enable-fips no-thread-pool no-quic os:windows-2022]) (push) Waiting to runDetails
Windows GitHub CI / shared (map[arch:win64 config:enable-fips os:windows-2019]) (push) Waiting to runDetails
Windows GitHub CI / plain (windows-2022) (push) Waiting to runDetails
Windows GitHub CI / minimal (windows-2019) (push) Waiting to runDetails
Windows GitHub CI / cygwin (windows-2019, map[arch:win64 config:-DCMAKE_C_COMPILER=gcc --strict-warnings enable-demos no-fips]) (push) Waiting to runDetails
Windows Compression GitHub CI / zstd (push) Waiting to runDetails
Windows Compression GitHub CI / brotli (push) Waiting to runDetails
If a peer address hasn't been set on a quic channel yet, we will not
yield a token from our hashtable of available tokens. Fail the
get_peer_token lookup in that event
Fixes#27608
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27610)
Calling SSL_accept() was raising two errors on the stack if you passed
the wrong object type. Similarly SSL_get_error() was adding an error to
the stack if the wrong object type was passed and returning the wrong
result.
We also ensure SSL_set_accept_state() and SSL_set_connect_state() don't
raise spurious errors since these are void functions.
Fixes#27347Fixes#27348
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27351)
The CWM might prevent a stream from being writeable. We should not report
a stream as writeable if there is no credit.
Fixes#27312
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/27319)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27264)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27091)
It is no longer static.
Also add it to libssl only with quic enabled.
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26882)
We've not implemented it yet, and don't need it for MVP, so move the
TODO's to QUIC FUTURE and remove the docs for it.
Fixesopenssl/project#1074
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26593)
I cloned a copy of fnv1a_hash from hashtable.c. Deduplicate that so we
have common source code.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
@sashan and I were discussing the usefulness of the public facing api
for NEW_TOKEN support, and he has concerns over its usefulness and our
being stuck with it if we need to make changes later. Given that it is
a convience api for using multiple CTX-es to share a cache, its fine if
we remove it for now, as that seems like a less common use case.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
Replace it with SSL_TOKEN_STORE and make the structure opaque in the
public api
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
we use this struct internally to track computed tokens, we may as well
use it when fetching those tokens, as it allows the removeal of the QTOK
type
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
we use get0 to get a token store, but set to set it. Since the latter
takes a refcount, change that to set1. Also rename the interal quic
functions to match.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
Forgot to free the CRYPTO_REF when freeing a token
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
closer reading of RFC 9000 indicates that a NEW_TOKEN token can be
(re)used repeatedly.
so instead of creating a use once and discard pattern in the token api.
Let the tokens stick around until they are replaced with a new token
from the server. To do this, we need to ref count the tokens so that we
don't accidentally free them while a given client is waiting to send an
initial frame making use of them.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
This will make it easier to refcount them in a moment
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
We don't want to schedule the NEW_TOKEN frame until such time as the
handshake is complete, otherwise we risk giving a token to validate a
future connection to a peer we haven't decided to trust yet
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
Instead of copying the token thats store, return a pointer to it
along with a pointer to the token struct to free should we need to
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
the SSL_new_from_listener api creates a client SSL from a server
SSL_CTX context. Normally server contexts need no token cache, but once
we start using it as a client, that changes. Allocate one here when
needed
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
Start storing new tokens in our new cache
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26517)
These are either already implemented or not relevant for
the QUIC server MVP.
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26544)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
Not strictly needed
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
As per @sashan suggestion, try pre-creating user ssls with a NULL
listener
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
We have a chicken and egg problem.
Normally when we create a connection object in quic, we associate it
with a listener, and up the ref on the parent listener, which is fine.
However, now that we are pre-allocating user_ssl objects for incomming
connections we have a situation in which:
1) The pre-alocated connection object holds a ref on the listener
2) The application has no awareness of the quic connection object (and
so can't free it)
3) The freeing of the listener object never calls into the quic stack,
because its reference count may hold references from connections that
haven't been accepted yet
We could require that applications register a function for the
new_pending_conn callback, and track/free these pending connections, but
that seems like alot of extra unneeded work to place on the application
Instead:
a) add a quic_conn_st flag named accepted
b) When pre-allocating connections, clear the flag in (a) and _dont_
hold a reference to the parent listener
c) in SSL_accept_connection, set the accepted flag and reference the
listener
d) in ossl_quic_free drop the listener reference only if the accepted
flag is set
c) expressly free all user_ssl objects in ossl_quic_port_drop_incoming
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
SSL_new_from_listner() creates QUIC connection object (QCSO)
from listener. Caller can use the object retuned from
SSL_new_from_listener() to connect to remote QUIC server.
The QCSO created here shares engine/port with listener.
the change is covered by `test_ssl_new_from_listener()` in
test/quicapitest.c
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26138)
I had experimented with starting the ssl handshake during accept, and
forgot to remove it
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26178)
Now that we have the infrastructure to skip address validation, add a
public flag to SSL_new_listener and SSL_new_listener_from to allow the
skipping of address validation on selected quic listener SSL objects
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26114)
This got introduced by #595288251bb (QUIC APL: Ensure APL
functions use correct prologue)
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25659)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25416)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25416)
The ossl_quic_get_net_write_desired() and
ossl_quic_reactor_net_read_desired() implementations can be used by
listeners. But in that case there is no ctx.qc object present. Instead we
should use the reactor from ctx.obj which will work also for a listener.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25642)
Just because one connection has not started yet, it does not mean that
we should not tick the QUIC_ENGINE. There may be other connections that do
need ticking.
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25452)
Gustaf Neumann
Neil Horman
Alexandr Nedvedicky
Andrew Dinh
Tomas Mraz
Matt Caswell
Samson S. Kolge
openssl-machine
sashan
Hugo Landau