The given size must be the size of the destination buffer, not the size of the
(binary) address representation.
This fixes GitHub issue #1599.
The bug was introduced in 92149f9a82 which is in
2.4+. The fix must be backported there.
If QUIC support is enabled both branches of the ternary conditional are
identical, upsetting Coverity. Move the full conditional into the non-QUIC
preprocessor branch to make the code more clear.
This resolves GitHub issue #1710.
Released version 2.6-dev11 with the following main changes :
- CI: determine actual LibreSSL version dynamically
- BUG/MEDIUM: ncbuf: fix null buffer usage
- MINOR: ncbuf: fix warnings for testing build
- MEDIUM: http-ana: Add a proxy option to restrict chars in request header names
- MEDIUM: ssl: Delay random generator initialization after config parsing
- MINOR: ssl: Add 'ssl-propquery' global option
- MINOR: ssl: Add 'ssl-provider' global option
- CLEANUP: Add missing header to ssl_utils.c
- CLEANUP: Add missing header to hlua_fcn.c
- CLEANUP: Remove unused function hlua_get_top_error_string
- BUILD: fix build warning on solaris based systems with __maybe_unused.
- MINOR: tools: add get_exec_path implementation for solaris based systems.
- BUG/MINOR: ssl: Fix crash when no private key is found in pem
- CLEANUP: conn-stream: Remove cs_applet_shut declaration from header file
- MINOR: applet: Prepare appctx to own the session on frontend side
- MINOR: applet: Let the frontend appctx release the session
- MINOR: applet: Change return value for .init callback function
- MINOR: stream: Export stream_free()
- MINOR: applet: Add appctx_init() helper fnuction
- MINOR: applet: Add a function to finalize frontend appctx startup
- MINOR: applet: Add function to release appctx on error during init stage
- MEDIUM: dns: Refactor dns appctx creation
- MEDIUM: spoe: Refactor SPOE appctx creation
- MEDIUM: lua: Refactor cosocket appctx creation
- MEDIUM: httpclient: Refactor http-client appctx creation
- MINOR: sink: Add a ref to sink in the sink_forward_target structure
- MEDIUM: sink: Refactor sink forwarder appctx creation
- MINOR: peers: Add a ref to peers section in the peer structure
- MEDIUM: peers: Refactor peer appctx creation
- MINOR: applet: Add API to start applet on a thread subset
- MEDIUM: applet: Add support for async appctx startup on a thread subset
- MINOR: peers: Track number of applets run by thread
- MEDIUM: peers: Balance applets across threads
- MINOR: conn-stream/applet: Stop setting appctx as the endpoint context
- CLEANUP: proxy: Remove dead code when parsing "http-restrict-req-hdr-names" option
- REGTESTS: abortonclose: Fix some race conditions
- MINOR: ssl: Add 'ssl-provider-path' global option
- CLEANUP: http_ana: Make use of the return value of stream_generate_unique_id()
- BUG/MINOR: spoe: Fix error handling in spoe_init_appctx()
- CLEANUP: peers: Remove unreachable code in peer_session_create()
- CLEANUP: httpclient: Remove useless test on ss_dst in httpclient_applet_init()
- BUG/MEDIUM: quic: fix Rx buffering
- OPTIM: quic: realign empty Rx buffer
- BUG/MINOR: ncbuf: fix ncb_is_empty()
- MINOR: ncbuf: refactor ncb_advance()
- BUG/MINOR: mux-quic: update session's idle delay before stream creation
- MINOR: h3: do not wait a complete frame for demuxing
- MINOR: h3: flag demux as full on HTX full
- MEDIUM: mux-quic: implement recv on io-cb
- MINOR: mux-quic: remove qcc_decode_qcs() call in XPRT
- MINOR: mux-quic: reorganize flow-control frames emission
- MINOR: mux-quic: implement MAX_STREAM_DATA emission
- MINOR: mux-quic: implement MAX_DATA emission
- BUG/MINOR: mux-quic: support nul buffer with qc_free_ncbuf()
- MINOR: mux-quic: free RX buf if empty
- BUG/MEDIUM: config: Reset outline buffer size on realloc error in readcfgfile()
- BUG/MINOR: check: Reinit the buffer wait list at the end of a check
- MEDIUM: check: No longer shutdown the connection in .wake callback function
- REORG: check: Rename and export I/O callback function
- MEDIUM: check: Use the CS to handle subscriptions for read/write events
- BUG/MINOR: quic: break for error on sendto
- MINOR: quic: abort on unlisted errno on sendto()
- MINOR: quic: detect EBADF on sendto()
- BUG/MEDIUM: quic: fix initialization for local/remote TPs
- CLEANUP: quic: adjust comment/coding style for TPs init
- BUG/MINOR: cfgparse: abort earlier in case of allocation error
- MINOR: quic: Dump initial derived secrets
- MINOR: quic_tls: Add quic_tls_derive_retry_token_secret()
- MINOR: quic_tls: Add quic_tls_decrypt2() implementation
- MINOR: quic: Retry implementation
- MINOR: cfgparse: Update for "cluster-secret" keyword for QUIC Retry
- MINOR: quic: Move quic_lstnr_dgram_dispatch() out of xprt_quic.c
- BUILD: stats: Missing headers inclusions from stats.h
- MINOR: quic_stats: Add a new stats module for QUIC
- MINOR: quic: Attach proxy QUIC stats counters to the QUIC connection
- BUG/MINOR: quic: Fix potential memory leak during QUIC connection allocations
- MINOR: quic: QUIC stats counters handling
- MINOR: quic: Add tune.quic.retry-threshold keyword
- MINOR: quic: Dynamic Retry implementation
- MINOR: quic/mux-quic: define CONNECTION_CLOSE send API
- MINOR: mux-quic: emit FLOW_CONTROL_ERROR
- MINOR: mux-quic: emit STREAM_LIMIT_ERROR
- MINOR: mux-quic: close connection on error if different data at offset
- BUG/MINOR: peers: fix error reporting of "bind" lines
- CLEANUP: config: improve address parser error report for unmatched protocols
- CLEANUP: config: provide cleare hints about unsupported QUIC addresses
- MINOR: protocol: replace ctrl_type with xprt_type and clarify it
- MINOR: listener: provide a function to process all of a bind_conf's arguments
- MINOR: config: use the new bind_parse_args_list() to parse a "bind" line
- CLEANUP: listener: add a comment about what the BC_SSL_O_* flags are for
- MINOR: listener: add a new "options" entry in bind_conf
- CLEANUP: listener: replace all uses of bind_conf->is_ssl with BC_O_USE_SSL
- CLEANUP: listener: replace bind_conf->generate_cers with BC_O_GENERATE_CERTS
- CLEANUP: listener: replace bind_conf->quic_force_retry with BC_O_QUIC_FORCE_RETRY
- CLEANUP: listener: store stream vs dgram at the bind_conf level
- MINOR: listener: detect stream vs dgram conflict during parsing
- MINOR: listener: set the QUIC xprt layer immediately after parsing the args
- MINOR: listener/ssl: set the SSL xprt layer only once the whole config is known
- MINOR: connection: add flag MX_FL_FRAMED to mark muxes relying on framed xprt
- MINOR: config: detect and report mux and transport incompatibilities
- MINOR: listener: automatically select a QUIC mux with a QUIC transport
- MINOR: listener: automatically enable SSL if a QUIC transport is found
- BUG/MINOR: quic: Fixe a typo in qc_idle_timer_task()
- BUG/MINOR: quic: Missing <conn_opening> stats counter decrementation
- BUILD/MINOR: cpuset fix build for FreeBSD 13.1
- CI: determine actual OpenSSL version dynamically
the cpuset api changes done fir the future 14 release had been
backported to the 13.1 release so changing the cpuset api of choice
condition change accordingly.
When we receive a CONNECTION_CLOSE frame, we should decrement this counter
if the handshake state was not successful and if we have not received
a TLS alert from the TLS stack.
When a bind line is configured without the "ssl" keyword, a warning is
emitted and a crash happens at runtime:
bind quic4@:4449 crt rsa+dh2048.pem alpn h3 allow-0rtt
[WARNING] (17867) : config : Proxy 'decrypt': A certificate was specified but SSL was not enabled on bind 'quic4@:4449' at [quic-mini.cfg:24] (use 'ssl').
Let's automatically turn SSL on when QUIC is detected, as it doesn't
exist without SSL anyway. It solves the runtime issue, and also makes
sure it is not possible to accidentally configure a quic listener with
no certificate since the error is detected via the SSL checks.
A warning is emitted in this case, to encourage the user to fix the
configuration so that it remains reviewable.
When no mux protocol is configured on a bind line with "proto", and the
transport layer is QUIC, right now mux_h1 is being used, leading to a
crash.
Now when the transport layer of the bind line is already known as being
QUIC, let's automatically try to configure the QUIC mux, so that users
do not have to enter "proto quic" all the time while it's the only
supported option. this means that the following line now works:
bind quic4@:4449 ssl crt rsa+dh2048.pem alpn h3 allow-0rtt
Till now, placing "proto h1" or "proto h2" on a "quic" bind or placing
"proto quic" on a TCP line would parse fine but would crash when traffic
arrived. The reason is that there's a strong binding between the QUIC
mux and QUIC transport and that they're not expected to be called with
other types at all.
Now that we have the mux's type and we know the type of the protocol used
on the bind conf, we can perform such checks. This now returns:
[ALERT] (16978) : config : frontend 'decrypt' : stream-based MUX protocol 'h2' is incompatible with framed transport of 'bind quic4@:4448' at [quic-mini.cfg:27].
[ALERT] (16978) : config : frontend 'decrypt' : frame-based MUX protocol 'quic' is incompatible with stream transport of 'bind :4448' at [quic-mini.cfg:29].
This config tightening is only tagged MINOR since while such a config,
despite not reporting error, cannot work at all so even if it breaks
experimental configs, they were just waiting for a single connection
to crash.
In order to be able to check compatibility between muxes and transport
layers, we'll need a new flag to tag muxes that work on framed transport
layers like QUIC. Only QUIC has this flag now.
We used to preset XPRT_SSL on bind_conf->xprt when parsing the "ssl"
keyword, which required to be careful about what QUIC could have set
before, and which makes it impossible to consider the whole line to
set all options.
Now that we have the BC_O_USE_SSL option on the bind_conf, it becomes
easier to set XPRT_SSL only once the bind_conf's args are parsed.
It used to be set when parsing the listeners' addresses but this comes
with some difficulties in that other places have to be careful not to
replace it (e.g. the "ssl" keyword parser).
Now we know what protocols a bind_conf line relies on, we can set it
after having parsed the whole line.
Now that we have a function to parse all bind keywords, and that we
know what types of sock-level and xprt-level protocols a bind_conf
is using, it's easier to centralize the check for stream vs dgram
conflict by putting it directly at the end of the args parser. This
way it also works for peers, provides better precision in the report,
and will also allow to validate transport layers. The check was even
extended to detect inconsistencies between xprt layer (which were not
covered before). It can even detect that there are two incompatible
"bind" lines in a single peers section.
Let's collect the set of xprt-level and sock-level dgram/stream protocols
seen on a bind line and store that in the bind_conf itself while they're
being parsed. This will make it much easier to detect incompatibilities
later than the current approch which consists in scanning all listeners
in post-parsing.
There is no way to store useful info there, yet there's about one entry
per boolean. Let's add an "options" attribute which will collect various
options.
In practice, even the BC_O_SSL_* flags and a few info such as strict_sni
could move there.
This now makes sure that both the peers' "bind" line and the regular one
will use the exact same parser with the exact same behavior. Note that
the parser applies after the address and that it could be factored
further, since the peers one still does quite a bit of duplicated work.
The "bind" parsing code was duplicated for the peers section and as a
result it wasn't kept updated, resulting in slightly different error
behavior (e.g. errors were not freed, warnings were emitted as alerts)
Let's first unify it into a new dedicated function that properly reports
and frees the error.
There's been some great confusion between proto_type, ctrl_type and
sock_type. It turns out that ctrl_type was improperly chosen because
it's not the control layer that is of this or that type, but the
transport layer, and it turns out that the transport layer doesn't
(normally) denaturate the underlying control layer, except for QUIC
which turns dgrams to streams. The fact that the SOCK_{DGRAM|STREAM}
set of values was used added to the confusion.
Let's replace it with xprt_type which reuses the later introduced
PROTO_TYPE_* values, and update the comments to explain which one
works at what level.
Just trying "quic4@:4433" with USE_QUIC not set rsults in such a cryptic
error:
[ALERT] (14610) : config : parsing [quic-mini.cfg:44] : 'bind' : unsupported protocol family 2 for address 'quic4@:4433'
Let's at least add the stream and datagram statuses to indicate what was
being looked for:
[ALERT] (15252) : config : parsing [quic-mini.cfg:44] : 'bind' : unsupported stream protocol for datagram family 2 address 'quic4@:4433'
Still not very pretty but gives a little bit more info.
In case the str2listener() parser reports a generic error with no message
when parsing the argument of a "bind" statement in a "peers" section, the
reported error indicates an invalid address on the empty arg. This has
existed since 2.0 with commit 355b2033e ("MINOR: cfgparse: SSL/TLS binding
in "peers" sections."), so this must be backported till 2.0.
As specified by the RFC reception of different STREAM data for the same
offset should be treated with a CONNECTION_CLOSE with error
PROTOCOL_VIOLATION.
Use ncbuf API to detect this case : if add operation fails with
NCB_RET_DATA_REJ with add mode NCB_ADD_COMPARE.
Send a CONNECTION_CLOSE on reception of a STREAM frame for a STREAM id
exceeding the maximum value enforced. Only implemented for bidirectional
streams for the moment.
Send a CONNECTION_CLOSE if the peer emits more data than authorized by
our flow-control. This is implemented for both stream and connection
level.
Fields have been added in qcc/qcs structures to differentiate received
offsets for limit enforcing with consumed offsets for sending of
MAX_DATA/MAX_STREAM_DATA frames.
Define an API to easily set a CONNECTION_CLOSE. This will mainly be
useful for the MUX when an error is detected which require to close the
whole connection.
On the MUX side, a new flag is added when a CONNECTION_CLOSE has been
prepared. This will disable add future send operations.
We rely on <conn_opening> stats counter and tune.quic.retry_threshold
setting to dynamically start sending Retry packets. We continue to send such packets
when "quic-force-retry" setting is set. The difference is when we receive tokens.
We check them regardless of this setting because the Retry could have been
dynamically started. We must also send Retry packets when we receive Initial
packets without token if the dynamic Retry threshold was reached but only for connection
which are not currently opening or in others words for Initial packets without
connection already instantiated. Indeed, we must not send Retry packets for all
Initial packets without token. For instance a client may have already sent an
Initial packet without receiving Retry packet because the Retry feature was not
started, then the Retry starts on exeeding the threshold value due to others
connections, then finally our client decide to send another Initial packet
(to ACK Initial CRYPTO data for instance). It does this without token. So, for
this already existing connection we must not send a Retry packet.
This QUIC specific keyword may be used to set the theshold, in number of
connection openings, beyond which QUIC Retry feature will be automatically
enabled. Its default value is 100.
First commit to handle the QUIC stats counters. There is nothing special to say
except perhaps for ->conn_openings which is a gauge to count the number of
connection openings. It is incremented after having instantiated a quic_conn
struct, then decremented when the handshake was successful (handshake completed
state) or failed or when the connection timed out without reaching the handshake
completed state.
Move the code which finalizes the QUIC connections initialisations after
having called qc_new_conn() into this function to benefit from its
error handling to release the memory allocated for QUIC connections
the initialization of which could not be finalized.
If we add a new stats module to C source files including only
stats.h we get these errors:
include/haproxy/stats.h:39:31: error: array type has incomplete element type
‘struct name_desc’
39 | extern const struct name_desc stat_fields[];
include/haproxy/stats.h:55:50: warning: ‘struct listener’ declared inside
parameter list will not be visible outside of this definition or declaration
55 | int stats_fill_li_stats(struct proxy *px, struct listener *l, int flags,
name_desc struct is defined in tools-t.h and listener struct in listner-t.h.
Here is the format of a token:
- format (1 byte)
- ODCID (from 9 up 21 bytes)
- creation timestamp (4 bytes)
- salt (16 bytes)
A format byte is required to distinguish the Retry token from others sent in
NEW_TOKEN frames.
The Retry token is ciphered after having derived a strong secret from the cluster secret
and generated the AEAD AAD, as well as a 16 bytes long salt. This salt is
added to the token. Obviously it is not ciphered. The format byte is not
ciphered too.
The AAD are built by quic_generate_retry_token_aad() which concatenates the version,
the client SCID and the IP address and port. We had to implement quic_saddr_cpy()
to copy the IP address and port to the AAD buffer. Only the Retry SCID is generated
on our side to build a Retry packet, the others fields come from the first packet
received by the client. It must reuse this Retry SCID in response to our Retry packet.
So, we have not to store it on our side. Everything is offloaded to the client (stateless).
quic_generate_retry_token() must be used to generate a Retry packet. It calls
quic_pkt_encrypt() to cipher the token.
quic_generate_retry_check() must be used to check the validity of a Retry token.
It is able to decipher a token which arrives into an Initial packet in response
to a Retry packet. It calls parse_retry_token() after having deciphered the token
to store the ODCID into a local quic_cid struct variable. Finally this ODCID may
be stored into the transport parameter thanks to qc_lstnr_params_init().
The Retry token lifetime is 10 seconds. This lifetime is also checked by
quic_generate_retry_check(). If quic_generate_retry_check() fails, the received
packet is dropped without anymore packet processing at this time.
This function does exactly the same thing as quic_tls_decrypt(), except that
it does reuse its input buffer as output buffer. This is needed
to decrypt the Retry token without modifying the packet buffer which
contains this token. Indeed, this would prevent us from decryption
the packet itself as the token belong to the AEAD AAD for the packet.
This function must be used to derive strong secrets from a non pseudo-random
secret (cluster-secret setting in our case) and an IV. First it call
quic_hkdf_extract_and_expand() to do that for a temporary strong secret (tmpkey)
then two calls to quic_hkdf_expand() reusing this strong temporary secret
to derive the final strong secret and IV.
In issue #1563, Coverity reported a very interesting issue about a
possible UAF in the config parser if the config file ends in with a
very large line followed by an empty one and the large one causes an
allocation failure.
The issue essentially is that we try to go on with the next line in case
of allocation error, while there's no point doing so. If we failed to
allocate memory to read one config line, the same may happen on the next
one, and blatantly dropping it while trying to parse what follows it. In
the best case, subsequent errors will be incorrect due to this prior error
(e.g. a large ACL definition with many patterns, followed by a reference of
this ACL).
Let's just immediately abort in such a condition where there's no recovery
possible.
This may be backported to all versions once the issue is confirmed to be
addressed.
Thanks to Ilya for the report.
The local and remote TPs were both processed through the same function
quic_transport_params_init(). This caused the remote TPs to be
overwritten with values configured for our local usage.
Change this by reserving quic_transport_params_init() only for our local
TPs. Remote TPs are simply initialized via
quic_dflt_transport_params_cpy().
This bug could result in a connection closed in error by the client due
to a violation of its TPs. For example, curl client closed the
connection after receiving too many CONNECTION_ID due to an invalid
active_connection_id value used.
If an unlisted errno is reported, abort the process. If a crash is
reported on this condition, we must determine if the error code is a
bug, should interrupt emission on the fd or if we can retry the syscall.
If sendto returns an error, we should not retry the call and break from
the sending loop. An exception is made for EINTR which allows to retry
immediately the syscall.
This bug caused an infinite loop reproduced when the process is in the
closing state by SIGUSR1 but there is still QUIC data emission left.
Instead of using the health-check to subscribe to read/write events, we now
rely on the conn-stream. Indeed, on the server side, the conn-stream's
endpoint is a multiplexer. Thus it seems appropriate to handle subscriptions
for read/write events the same way than for the streams. Of course, the I/O
callback function is not the same. We use srv_chk_io_cb() instead of
cs_conn_io_cb().
