The previous fix:
BUG/MEDIUM: peers: fix segfault using multiple bind on peers
Prevents to declare multiple listeners on a peers sections but if
peers protocol is extended to support this we could raise the bug
again.
Indeed, after allocating a new listener and adding it to a list the
code mistakenly re-configure the first element of the list instead
of the new added one, and the last one remains finally uninitialized.
The previous fix assure there is no more than one listener in this
list but this could be changed in futur.
This patch patch assures we configure and initialize the newly added
listener instead of the first one in the list.
This patch could be backported until version 2.0 to complete
BUG/MEDIUM: peers: fix segfault using multiple bind on peers
If multiple "bind" lines were present on the "peers" section, multiple
listeners were added to a list but the code mistakenly initialize
the first member and this first listener was re-configured instead of
the newly created one. The last one remains uninitialized causing a null
dereference a soon a connection is received.
In addition, the 'peers' sections and protocol are not currently designed to
handle multiple listeners.
This patch check if there is already a listener configured on the 'peers'
section when we want to create a new one. This is rising an error if
a listener is already present showing the file and line in the error
message.
To keep the file and line number of the previous listener available
for the error message, the 'bind_conf_uniq_alloc' function was modified
to keep the file/line data the struct 'bind_conf' was firstly
allocated (previously it was updated each time the 'bind_conf' was
reused).
This patch should be backported until version 2.0
resolvers_deinit() function is called on error, during post-parsing stage,
or on deinit, when HAProxy is stopped. It releases all entities: resolvers,
resolutions and SRV requests. There is no reason to defer the resolutions
release by moving them in the death_row list because this function is
terminal. And it is in fact a bug. Resolutions must not be released at the
end of the function because resolvers were already freed. However some
resolutions may still be attached to a reolver. Thus, when we try to remove
it from the resolver's tree, in resolv_reset_resolution(), this resolver was
already released.
So now, resolution are immediately released. It means there is no more
reason to track this function. calls to
enter_resolver_code()/leave_resolver_code() have been removed.
This patch should fix the issue #1680 and may be related to #1485. It must
be backported as far as 2.2.
We used to support both RTSP and HTTP protocol version names with and
without accept-invalid-http-request, but since this is based on the
characters themselves, any protocol made of chars {0-9/.HPRST} was
possible and not others. Now that such non-standard protocols are
restricted to accept-invalid-http-request, there's no reason for not
allowing other letters. With this patch, characters {0-9./A-Z} are
permitted when the option is set.
This patch hardens the verification of the HTTP/1.x version line
(i.e. the first line within an HTTP/1.x request) to verify that
the protocol name within the version actually reads "HTTP".
Previously protocols that superficially resembled the wire-format
of HTTP/1.x and having a 4-letter acronym as the protocol name, such
as RTSP would pass this check.
This patch fixes GitHub issue #540, it must be backported to all
supported versions. The legacy, non-HTX parser is affected as well,
a fix must be created for it as well.
Note that such protocols can still be used when option
accept-invalid-http-request is set.
In issue #1585 Coverity suspects a risk of multiply overflow when
calculating the SSL cache size, though in practice the cache is
limited to 2^32 anyway thus it cannot really happen. Nevertheless,
casting the operation should be sufficient to avoid marking it as a
false positive.
This reverts commit 118b2cbf84.
This patch was useful mainly for the docker image of QUIC interop to
have traces on stdout.
A better solution has been found by integrating this patch directly in
the qns repository which is used to build the docker image. Thus, this
hack is not require anymore in the main repository.
The wake handler detects if the frontend is closed. This can happen if
the proxy has been disabled individually or even on process soft-stop.
Before this patch, in this condition QCS instances were freed before
being detached from the cs_endpoint. This clearly violates the haproxy
connection architecture and cause a BUG_ON statement crash in cs_free().
To handle this properly, cs_endpoint is notified by setting RD_SH|WR_SH
on connection flags. The cs_endpoint will thus use the detach operation
which allows the QCS instance to be freed.
This code allows the soft-stop process to complete as soon as possible.
However, the client is not notified about the connection closing. It
should be done by emitting a H3 GOAWAY + CONNECTION_CLOSE. Sadly, this
is impossible at this stage because the listener sockets are closed so
the quic-conn cannot use it to emit new frames. At this stage the client
will most probably detect connection closing on its idle timeout
expiration.
Thus, to completely support proxy closing/soft-stop, important
architecture changes are required in QUIC socket management. This is
also linked with the reload feature.
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.
