When writing to a blocking quic stream, we sometimes get duplicate
transmitted data. This occurs when a call to quic_write_blocking has to
wait for space to become available in the ring buffer. When we do a
wait, the call sets *written to the value returned in args.total_written
as filled out by the calls to block_until_pred->quic_write_again.
However, the value there is based on the amount we requested, which is
only the remaining data that we didn't append in xso_sstream_write. So
if we call quic_write_blocking with a buffer of length X, and initially
append Y bytes, and write the remainig X-Y bytes via a block_until_pred
call, then *written will return with the value X-Y, even though we wrote
the full X bytes to the ring buffer.
Fix it by recording the initial amount appended into *written, and then
add the args.total_written value if we have to wait on more space
Fixesopenssl/project#924
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26023)
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)
Otherwise we will calculate an incorrect header
size for higher stream ids and won't fit the
frame into the packet.
Fixes#25417
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25928)
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)
Fixes#25625
Several error paths return 0 directly instead of going to err to clean
up the objects.
CLA: trivial
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25636)
The quic implementation defined a set of LIST_* macros for list
manipulation, which conflicts with the generally support BSD api found
in the queue.h system header. While this isn't normally a problem, A
report arrived indicating that MacOSX appears to implicitly include
queue.h from another system header which causes definition conflicts.
As the openssl macros are internal only, it seems the most sensible
thing to do is place them in a well known namespace for our library to
avoid the conflict, so add an OSSL_ prefix to all our macros
Fixes#25516
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/25519)
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)
Found by running the checkpatch.pl Linux script to enforce coding style.
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22097)
Return value of function 'ossl_quic_rxfc_on_retire', called at
quic_stream_map.c:767, is not checked, but it is usually checked
for this function.
CLA: trivial
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24794)
The QUIC test server was using incorrectly formatted ALPN data. With the
previous implementation of SSL_select_next_proto this went unnoticed. With
the new stricter implemenation it was failing.
Follow on from CVE-2024-5535
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24716)
Signed-off-by: fanqiaojun <fanqiaojun@yeah.net>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24128)
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: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24008)
(cherry picked from commit 1a4b029af5)
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)
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)
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: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23517)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23517)
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)
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)
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)
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)
Neil Horman
Tomas Mraz
Matt Caswell
Niels Dossche
pohsingwu
Dimitri Papadopoulos
MrRurikov
Amir Mohammadi
Hugo Landau
Richard Levitte
Bernd Edlinger