Christopher Faulet [Wed, 13 Dec 2023 14:36:52 +0000 (15:36 +0100)]
BUG/MEDIUM: mux-h2: Report too large HEADERS frame only when rxbuf is empty
During HEADERS frames decoding, if a frame is too large to fit in a buffer,
an internal error is reported and a RST_STREAM is emitted. On the other
hand, we wait to have an empty rxbuf to decode the frame because we cannot
retry a failed HPACK decompression.
When we are decoding headers, it is valid to return an error if dbuf buffer
is full because no data can be blocked in the rxbuf (which hosts the HTX
message).
However, during the trailers decoding, it is possible to have some data not
sent yet for the current stream in the rxbug and data for another stream
fully filling the dbuf buffer. In this case, we don't decode the trailers
but we must not return an error. We must wait to empty the rxbuf first.
Now, a HEADERS frame is considered as too large if the dbuf buffer is full
and if the rxbuf is empty (the HTX message to be accurate).
This patch should fix the issue #2382. It must be backported to all stable
versions.
(cherry picked from commit
682f73b4fa6d76aa0b5b743fe92777822884772d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 12 Dec 2023 13:04:35 +0000 (14:04 +0100)]
CLEANUP: mux-h1: Fix a trace message about C-L header addition
This fixes a cut-paste error on a trace message notifying a 'Content-Length'
header was added during the HTTP message formatting.
(cherry picked from commit
65ca4442401c527530f5b456a8ed47a35c224480)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 12 Dec 2023 12:56:05 +0000 (13:56 +0100)]
BUG/MEDIUM: mux-h1: Explicitly skip request's C-L header if not set originally
Commit
f89ba27caa ("BUG/MEDIUM: mux-h1; Ignore headers modifications about
payload representation") introduced a regression. The Content-Length is no
longer sent to the server for requests without payload but with a
'Content-Lnegth' header explicitly set to 0, like POST request with no
payload. It is of course unexpected. In some cases, depending on the server,
such requests are considered as invalid and a 411-Length-Required is returned.
The above commit is not directly responsible for the bug, it only reveals a too
lax condition to skip the 'Content-Length' header of bodyless requests. We must
only skip this header if none was originally found, during the parsing.
This patch should fix the issue #2386. It must be backported to 2.9.
(cherry picked from commit
966a18e2b485901bd986ebe65670dc4bfbfe65b5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 11 Dec 2023 20:55:32 +0000 (21:55 +0100)]
BUG/MEDIUM: mux-h1: Cound data from input buf during zero-copy forwarding
During zero-copy forwarding, we first try to forward data found in the input
buffer before trying to receive more data. These data must be removed from
the amount of data to forward (the cound variable).
Otherwise, on an internal retry, in h1_fastfwd(), we can be lead to read
more data than expected. It is especially a problem on the end of a
chunk. An error is erroneously reported because more data than announced are
received.
This patch should fix the issue #2382. It must be backported to 2.9.
(cherry picked from commit
eed1e8733c279cda0b6a2db574ead4e0238177f6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 11 Dec 2023 12:56:15 +0000 (13:56 +0100)]
BUG/MEDIUM: stconn: Block zero-copy forwarding if EOS/ERROR on consumer side
When the producer side (h1 for now) negociates with the consumer side to
perform a zero-copy forwarding, we now consider the consumer side as blocked
if it is closed and this was reported to the SE via a end-of-stream or a
(pending) error.
It is performed before calling ->nego_ff callback function, in se_nego_ff().
This way, all consumer are concerned automatically. The aim of this patch is
to fix an issue with the QUIC mux. Indeed, it is unexpected to send a frame
on an closed stream. This triggers a BUG_ON(). Other muxes are not affected
but it remains useless to try to send data if the stream is closed.
This patch should fix the issue #2372. It must be backported to 2.9.
(cherry picked from commit
2421c6fa7db5b06109a88bffa411c8ba8f948384)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frédéric Lécaille [Wed, 13 Dec 2023 10:45:43 +0000 (11:45 +0100)]
BUG/MEDIUM: quic: QUIC CID removed from tree without locking
This bug arrived with this commit:
BUG/MINOR: quic: Wrong RETIRE_CONNECTION_ID sequence number chec
Every connection ID manipulations against the by thread trees used to store the
connection IDs must be done under the trees locks. These trees are accessed by
the low level connection identification code.
When receiving a RETIRE_CONNECTION_ID frame, the concerned connection ID must
be deleted from the its underlying by thread tree but not without locking!
Add a WR lock around ebmb_delete() call to do so.
Must be backported as far as 2.7.
(cherry picked from commit
dd58dff1e64fa382d368958469506c296454fde6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 8 Dec 2023 13:34:32 +0000 (14:34 +0100)]
MINOR: version: mention that it's stable now
This version will be maintained up to around Q1 2025. The INSTALL file
also mentions it.
Aurelien DARRAGON [Thu, 7 Dec 2023 08:58:27 +0000 (09:58 +0100)]
BUG/MINOR: ext-check: cannot use without preserve-env
Since 1de44da ("MINOR: ext-check: add an option to preserve environment
variables"), it is now possible to provide an extra argument to
"external-check" directive. This allows to support the "preserve-env"
option which differs from the default behavior.
However a mistake was made, because the config parser doesn't allow the
default configuration anymore: using external-check without argument will
trigger an error:
'external-check' only supports 'preserve-env' as an argument, found ''.
This is due to as small mistake in the code that make the check
systematically report an error if the first argument is not equal to
"preserve-env". The check was modified so that the error is only reported
if the argument is provided, so that the default behavior is restored.
This should fix GH #2380 and should be backported on 2.9 and potentially
further (anywhere 1de44da is, because a note about an optional backport
up to the 2.6 was left in the original commit message)
(cherry picked from commit
63282f3bfbaee50a0716823bdc8c4251a47e6e39)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Fri, 8 Dec 2023 10:46:15 +0000 (11:46 +0100)]
BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions
Some regressions were introduced by
5fea59754b ("MEDIUM: map/acl:
Accelerate several functions using pat_ref_elt struct ->head list")
pat_ref_delete_by_id() fails to properly unlink and free the removed
reference because it bypasses the pat_ref_delete_by_ptr() made for
that purpose. This function is normally used everywhere the target
reference is set for removal, such as the pat_ref_delete() function
that matches pattern against a string. The call was probably skipped
by accident during the rewrite of the function.
With the above commit also comes another undesirable change:
both pat_ref_delete_by_id() and pat_ref_set_by_id() directly use the
<refelt> argument as a valid pointer (they do dereference it).
This is wrong, because <refelt> is unsafe and should be handled as an
ID, not a pointer (hence the function name). Indeed, the calling function
may directly pass user input from the CLI as <refelt> argument, so we must
first ensure that it points to a valid element before using it, else it is
probably invalid and we shouldn't touch it.
What this patch essentially does, is that it reverts pat_ref_set_by_id()
and pat_ref_delete_by_id() to pre
5fea59754b behavior. This seems like
it was the only optimization from the patch that doesn't apply.
Hopefully, after reviewing the changes with Fred, it seems that the 2
functions are only being involved in commands for manipulating maps or
acls on the cli, so the "missed" opportunity to improve their performance
shouldn't matter much. Nonetheless, if we wanted to speed up the reference
lookup by ID, we could consider adding an eb64 tree for that specific
purpose that contains all pattern references IDs (ie: pointers) so that
eb lookup functions may be used instead of linear list search.
The issue was raised by Marko Juraga as he failed to perform an an acl
removal by reference on the CLI on 2.9 which was known to work properly
on other versions.
It should be backported on 2.9.
Co-Authored-by: Frédéric Lécaille <flecaille@haproxy.com>
(cherry picked from commit
d7964c52ceb18a690d33d35b0c802660241d9ba9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 8 Dec 2023 10:55:15 +0000 (11:55 +0100)]
BUILD: ssl: update types in wolfssl cert selection callback
The types have changed in the PR for the wolfSSL_get_sigalg_info()
function, let's update them.
Must be backported in 2.9.
(cherry picked from commit
dbe9cea35b7a43a4907fa726e7a8afc31a6fbee3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 7 Dec 2023 20:12:02 +0000 (21:12 +0100)]
BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
This bug impacts only the OpenSSL QUIC compatibility module (USE_QUIC_OPENSSL_COMPAT).
This may happen only when the TLS stack has to be provided with more than 1024+1+5+16
bytes of CRYPTO data. In this case several TLS records have to be built in one
call to SSL_provide_quic_data(). A 5-bytes header is created at the head
of these records. This header is used as AAD to cipher the record. But
the length of this AAD was counted two times. One time here in
quic_tls_compat_create_record() (initialization):
adlen = quic_tls_compat_create_header(qc, rec, ad, 0);
and a second time here in the same function after quic_tls_tls_seal() return:
ret = aad_len + outlen;
This addition is useless. Note that this bug could be reproduced when haproxy has
to authenticate the client.
Thank you to @vifino for having reported this issue in GH #2381.
Must be backported to 2.8.
(cherry picked from commit
c075e4f2fc9e662459f7ab0ce8e13b70c059334a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Wed, 6 Dec 2023 10:15:01 +0000 (11:15 +0100)]
BUG/MINOR: mworker/cli: fix set severity-output support
"set severity-output" is one of these command that changes the appctx
state so the next commands are affected.
Unfortunately the master CLI works with pipelining and server close
mode, which means the connection between the master and the worker is
closed after each response, so for the next command this is a new appctx
state.
To fix the problem, 2 new flags are added ACCESS_MCLI_SEVERITY_STR and
ACCESS_MCLI_SEVERITY_NB which are used to prefix each command sent to
the worker with the right "set severity-output" command.
This patch fixes issue #2350.
It could be backported as far as 2.6.
(cherry picked from commit
1c1bb8ef2abeab2dc8e4857ec2fef2b19b472be7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 7 Dec 2023 14:00:58 +0000 (15:00 +0100)]
DOC: configuration: typo req.ssl_hello_type
rep_ssl_hello_type was renamed in res.ssl_hello_type a long time ago.
This patch fixes a typo where an example was renamed
"rep.ssl_hello_type" instead of "res.ssl_hello_type"
fixes issue #2377 and #2379.
Must be backported in all maintained versions.
(cherry picked from commit
906525b16bf53c156f8b559f8782a81f2b960c2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Wed, 6 Dec 2023 10:42:42 +0000 (11:42 +0100)]
BUG/MINOR: lua: Wrong OCSP CID after modifying an SSL certficate (LUA)
This bugfix is the same as the following one:
"BUG/MINOR: ssl_ckch: Wrong OCSP CID after modifying an SSL certficate"
where the OCSP CID had to be reset when updating a certificate.
Must be backported to 2.8.
(cherry picked from commit
917f7c74d3d30a3f7ac77fd6ea69cfcd529e53a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 14:38:29 +0000 (15:38 +0100)]
BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate
This bug could be reproduced with the "set ssl cert" CLI command to update
a certificate. The OCSP CID is duplicated by ckchs_dup() which calls
ssl_sock_copy_cert_key_and_chain(). It should be computed again by
ssl_sock_load_ocsp(). This may be accomplished resetting the new ckch OCSP CID
returned by ckchs_dup().
This bug may be in relation with GH #2319.
Must be backported to 2.8.
(cherry picked from commit
75f5977ff4ccf205ee386e4f4b0c384fb6a110d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 14:11:33 +0000 (15:11 +0100)]
MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback
This patch allows cli_io_handler_commit_cert() callback called upon
a "commit ssl cert ..." command to prefix the messages returned by the CLI
to the by the ones built by ha_warining(), ha_alert().
Should be interesting to backport this commit to 2.8.
(cherry picked from commit
456ba6e95f094c084c2513a594e59670b5eac2d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 13:50:40 +0000 (14:50 +0100)]
BUG/MINOR: ssl: Double free of OCSP Certificate ID
This bug could be reproduced loading several certificated from "bind" line:
with "server_ocsp.pem" as argument to "crt" setting and updating
the CDSA certificate with the RSA as follows:
echo -e "set ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa \
<<\n$(cat reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.rsa)\n" | socat - /tmp/stats
followed by an "commit ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa"
command. This could be detected by libasan as follows:
=================================================================
==507223==ERROR: AddressSanitizer: attempting double-free on 0x60200007afb0 in thread T3:
#0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
#1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)
#2 0x7fabc6af54e9 in ossl_asn1_primitive_free (/opt/quictls/lib/libcrypto.so.81.3+0xe14e9)
#3 0x7fabc6af5960 in ossl_asn1_template_free (/opt/quictls/lib/libcrypto.so.81.3+0xe1960)
#4 0x7fabc6af569f in ossl_asn1_item_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xe169f)
#5 0x7fabc6af58a4 in ASN1_item_free (/opt/quictls/lib/libcrypto.so.81.3+0xe18a4)
#6 0x46a159 in ssl_sock_free_cert_key_and_chain_contents src/ssl_ckch.c:723
#7 0x46aa92 in ckch_store_free src/ssl_ckch.c:869
#8 0x4704ad in cli_release_commit_cert src/ssl_ckch.c:1981
#9 0x962e83 in cli_io_handler src/cli.c:1140
#10 0xc1edff in task_run_applet src/applet.c:454
#11 0xaf8be9 in run_tasks_from_lists src/task.c:634
#12 0xafa2ed in process_runnable_tasks src/task.c:876
#13 0xa23c72 in run_poll_loop src/haproxy.c:3024
#14 0xa24aa3 in run_thread_poll_loop src/haproxy.c:3226
#15 0x7fabc69e7ea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7ea6)
#16 0x7fabc6907a2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfba2e)
0x60200007afb0 is located 0 bytes inside of 3-byte region [0x60200007afb0,0x60200007afb3)
freed by thread T3 here:
#0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
#1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)
previously allocated by thread T2 here:
#0 0x7fabc6fb573f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f)
#1 0x7fabc6ae8d77 in ASN1_STRING_set (/opt/quictls/lib/libcrypto.so.81.3+0xd4d77)
Thread T3 created by T0 here:
#0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
#1 0xc04f36 in setup_extra_threads src/thread.c:252
#2 0xa2761f in main src/haproxy.c:3917
#3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
Thread T2 created by T0 here:
#0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
#1 0xc04f36 in setup_extra_threads src/thread.c:252
#2 0xa2761f in main src/haproxy.c:3917
#3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
SUMMARY: AddressSanitizer: double-free ??:0 __interceptor_free
==507223==ABORTING
Aborted
The OCSP CID stored in the impacted ckch data were freed but not reset to NULL,
leading to a subsequent double free.
Must be backported to 2.8.
(cherry picked from commit
7dab3e8266b8de8604dbae0a90eade85c5c0e3f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 5 Dec 2023 15:15:30 +0000 (16:15 +0100)]
[RELEASE] Released version 2.9.0
Released version 2.9.0 with the following main changes :
- DOC: config: add missing colon to "bytes_out" sample fetch keyword (2)
- BUG/MINOR: cfgparse-listen: fix warning being reported as an alert
- DOC: config: add matrix entry for "max-session-srv-conns"
- DOC: config: fix monitor-fail typo
- DOC: config: add context hint for proxy keywords
- DEBUG: stream: Report lra/fsb values for front end back SC in stream dump
- REGTESTS: sample: Test the behavior of consecutive delimiters for the field converter
- BUG/MINOR: sample: Make the `word` converter compatible with `-m found`
- DOC: Clarify the differences between field() and word()
- BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event
- BUILD: http_htx: silence uninitialized warning on some gcc versions
- MINOR: acme.sh: don't use '*' in the filename for wildcard domain
- MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding
- MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-quic: Disable zero-copy forwarding for send by default
- DOC: config: update the reminder on the HTTP model and add some terminology
- DOC: config: add a few more differences between HTTP/1 and 2+
- DOC: config: clarify session vs stream
- DOC: config: fix typo abandonned -> abandoned
- DOC: management: fix two latest typos (optionally, exception)
- BUG/MEDIUM: peers: fix partial message decoding
- DOC: management: update stream vs session
Willy Tarreau [Tue, 5 Dec 2023 08:30:44 +0000 (09:30 +0100)]
DOC: management: update stream vs session
Indicate for some commands such as "show sess" that we now dump streams
and not sessions.
Christopher Faulet [Tue, 5 Dec 2023 08:21:38 +0000 (09:21 +0100)]
BUG/MEDIUM: peers: fix partial message decoding
peer_recv_msg() may return because the message is incomplete without
checking if a shutdown is pending for the SC. The function relies on
co_getblk() to detect shutdowns. However, the message length decoding may be
interrupted if the multi-bytes integer is incomplete. In this case, the SC
is not check for shutdowns.
When this happens, this leads to an appctx spinning loop.
This patch should fix the issue #2373. It must be backported to 2.8.
Willy Tarreau [Tue, 5 Dec 2023 03:04:50 +0000 (04:04 +0100)]
DOC: management: fix two latest typos (optionally, exception)
No backport needed, these were introduced by latest commits
3dd55fa13
("MINOR: mworker/cli: implement hard-reload over the master CLI") and
cef29d370 ("MINOR: trace: define simple -dt argument").
Willy Tarreau [Tue, 5 Dec 2023 03:02:25 +0000 (04:02 +0100)]
DOC: config: fix typo abandonned -> abandoned
No need to backport, it was introduced by recent commit
fafa34e5f ("DOC:
config: update the reminder on the HTTP model and add some terminology").
Willy Tarreau [Mon, 4 Dec 2023 17:57:12 +0000 (18:57 +0100)]
DOC: config: clarify session vs stream
Rename "session" to "stream" where relevant (termination states, queue,
and so on).
Willy Tarreau [Mon, 4 Dec 2023 17:34:19 +0000 (18:34 +0100)]
DOC: config: add a few more differences between HTTP/1 and 2+
Mention the lack of reason phrase and the existence of pseudo-headers for
the request and the response.
Willy Tarreau [Mon, 4 Dec 2023 17:16:52 +0000 (18:16 +0100)]
DOC: config: update the reminder on the HTTP model and add some terminology
It was really necessary to try to clear the confusion between sessions
and streams, so let's first lift a little bit the HTTP model part to
better consider new protocols, and explain what a stream is and how this
differs from the earlier sessions.
Christopher Faulet [Mon, 4 Dec 2023 14:36:01 +0000 (15:36 +0100)]
MINOR: mux-quic: Disable zero-copy forwarding for send by default
There is at least an bug for now in this part and it is still unstable. Thus
it is better to disable it for now by default. It can be enable by setting
tune.quic.zero-copy-fwd-send to 'on'.
Christopher Faulet [Mon, 4 Dec 2023 14:26:23 +0000 (15:26 +0100)]
MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding
tune.quic.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the QUIC mux only, for sends. For now, there
is no option to disable it for receives because it is not supported yet.
It is enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 14:18:49 +0000 (15:18 +0100)]
MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding
tune.h2.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the H2 mux only, for sends. For now, there is
no option to disable it for receives because it is not supported yet.
It is enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 14:06:06 +0000 (15:06 +0100)]
MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding
tune.h1.zero-copy-fwd-recv and tune.h1.zero-copy-fwd-send can now be used to
enable or disable the zero-copy fast-forwarding for the H1 mux only, for
receives or sends. Unlike the PT mux, there are 2 options here because
client and server sides can use difference muxes.
Both are enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 13:48:52 +0000 (14:48 +0100)]
MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding
tune.pt.zero-copy-forwarding parameter can now be used to enable or disable
the zero-copy fast-forwarding for the PT mux only. It is enabled ('on') by
default. It can be disabled by setting the parameter to 'off'. In this case,
this disables receive and send side.
Christopher Faulet [Mon, 4 Dec 2023 13:18:50 +0000 (14:18 +0100)]
MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding
Zero-copy fast-forwading feature is a quite new and is a bit sensitive.
There is an option to disable it globally. However, all protocols have not
the same maturity. For instance, for the PT multiplexer, there is nothing
really new. The zero-copy fast-forwading is only another name for the kernel
splicing. However, for the QUIC/H3, it is pretty new, not really optimized
and it will evolved. And soon, the support will be added for the cache
applet.
In this context, it is usefull to be able to enable/disable zero-copy
fast-forwading per-protocol and applet. And when it is applicable, on sends
or receives separately. So, instead of having one flag to disable it
globally, there is now a dedicated bitfield, global.tune.no_zero_copy_fwd.
William Lallemand [Mon, 4 Dec 2023 10:52:31 +0000 (11:52 +0100)]
MINOR: acme.sh: don't use '*' in the filename for wildcard domain
By default acme.sh uses the '*' character in the filename for wildcard.
That can be confusing within HAProxy since the * character in front of a
filename in the stat socket is used to specified an uncommitted
transaction.
This patch replace the '*' by a '_' in the filename. This is only done
when using the default filename, the name can still be forced with an
asterisk.
Willy Tarreau [Fri, 1 Dec 2023 14:29:44 +0000 (15:29 +0100)]
BUILD: http_htx: silence uninitialized warning on some gcc versions
Building on gcc 4.4 reports "start may be used uninitialized". This is
a classical case of dependency between two variables where the compiler
lost track of their initialization and doesn't know that if one is not
set, the other is. By just moving the second test in the else clause
of the assignment both fixes it and makes the code more efficient, and
this can be simplified as a ternary operator.
It's probably not needed to backport this, unless anyone reports build
warnings with more recent compilers (intermediary optimization levels
such as -O1 can sometimes trigger such warnings).
Aurelien DARRAGON [Fri, 1 Dec 2023 16:41:40 +0000 (17:41 +0100)]
BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event
It is possible that a server's addr family is temporarily set to AF_UNSPEC
even if we're certain to be in INET context (ipv4, ipv6).
Indeed, as soon as IP address resolving is involved, srv->addr family will
be set to AF_UNSPEC when the resolution fails (could happen at anytime).
However, _srv_event_hdl_prepare_inetaddr() wrongly assumed that it would
only be called with AF_INET or AF_INET6 families. Because of that, the
function will handle AF_UNSPEC address as an IPV6 address: not only
we could risk reading from an unititialized area, but we would then
propagate false information when publishing the event.
In this patch we make sure to properly handle the AF_UNSPEC family in
both the "prev" and the "next" part for SERVER_INETADDR event and that
every members are explicitly initialized.
This bug was introduced by
6fde37e046 ("MINOR: server/event_hdl: add
SERVER_INETADDR event"), no backport needed.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:18 +0000 (16:41 +0100)]
DOC: Clarify the differences between field() and word()
word() mentions that delimiters at the start and end are ignored, but
it does not mention that consecutive delimiters are merged.
May be backported as far as the patch applies.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:17 +0000 (16:41 +0100)]
BUG/MINOR: sample: Make the `word` converter compatible with `-m found`
Previously an expression like:
path,word(2,/) -m found
always returned `true`.
Bug exists since the `word` converter exists. That is:
c9a0f6d0232cf44d6b08d1964b9097a45a6c65f0
The same bug was previously fixed for the `field` converter in commit
4381d26edc03faa46401eb0fe82fd7be84be14fd.
The fix should be backported to 1.6+.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:16 +0000 (16:41 +0100)]
REGTESTS: sample: Test the behavior of consecutive delimiters for the field converter
This is in preparation of a follow-up patch to fix the word converter.
Christopher Faulet [Fri, 1 Dec 2023 10:16:53 +0000 (11:16 +0100)]
DEBUG: stream: Report lra/fsb values for front end back SC in stream dump
REX and WEX date are already reported. But if the corresponding SC cannot
expire on read or write, "<NEVER>" is reported instead. The same is reported
if no expiration date is set. It is not really convenient because we cannot
distinguish the two cases.
So, now, for each SC, read and wirte timer (rto/wto) are also reported in
the dump, based on .lra/.fsb dates and the current I/O timeout. The SC I/O
timeout is also reported.
Aurelien DARRAGON [Mon, 20 Nov 2023 16:53:17 +0000 (17:53 +0100)]
DOC: config: add context hint for proxy keywords
Add a small list of contexts where each proxy keyword is expected to be
employed. (Similar to the defaults/frontend/backend/listen compatibility
grid).
Aurelien DARRAGON [Thu, 30 Nov 2023 10:11:43 +0000 (11:11 +0100)]
DOC: config: fix monitor-fail typo
monitor-fail doesn't exist, but it was mentionned in the documentation.
Fixing it with "monitor fail" instead.
Aurelien DARRAGON [Wed, 29 Nov 2023 09:13:18 +0000 (10:13 +0100)]
DOC: config: add matrix entry for "max-session-srv-conns"
Following 4039329 ("DOC: config: specify supported sections for
"max-session-srv-conns"), "max-session-srv-conns" was also missing
from the proxy keyword matrix.
Aurelien DARRAGON [Wed, 29 Nov 2023 14:03:25 +0000 (15:03 +0100)]
BUG/MINOR: cfgparse-listen: fix warning being reported as an alert
Since
b40542000d ("MEDIUM: proxy: Warn about ambiguous use of named
defaults sections") we introduced a new error to prevent user from
having an ambiguous named default section in the config which is both
inherited explicitly using "from" and implicitly by another proxy due to
the default section being the last one defined.
However, despite the error message being presented as a warning the
err_code, the commit message and the documentation, it is actually
reported as a fatal error because ha_alert() was used in place of
ha_warning().
In this patch we make the code comply with the documentation and the
intended behavior by using ha_warning() to report the error message.
This should be backported up to 2.6.
Tim Duesterhus [Thu, 30 Nov 2023 19:15:32 +0000 (20:15 +0100)]
DOC: config: add missing colon to "bytes_out" sample fetch keyword (2)
This reapplies
1eb049dc677f2de950158615ed3d8306ee5102d6, as the change was
accidentally reverted in
5ef48e063ecf992646c7af374153f106050fb8ec.
Willy Tarreau [Thu, 30 Nov 2023 17:07:06 +0000 (18:07 +0100)]
[RELEASE] Released version 2.9-dev12
Released version 2.9-dev12 with the following main changes :
- BUG/MINOR: global: Fix tune.disable-(fast-forward/zero-copy-forwarding) options
- DOC: config: removing "log-balance" references
- MINOR: server/event_hdl: add SERVER_INETADDR event
- MINOR: tools: use const for read only pointers in ip{cmp,cpy}
- MINOR: server/ip: centralize server ip updates
- MINOR: backend: remove invalid mode test for "hash-balance-factor"
- Revert "MINOR: cfgparse-listen: warn when use-server rules is used in wrong mode"
- MINOR: proxy: add free_logformat_list() helper function
- MINOR: proxy: add free_server_rules() helper function
- MINOR: log/backend: prevent "use-server" rules use with LOG mode
- MINOR: log/balance: set lbprm tot_weight on server on queue/dequeue
- DOC: config: specify supported sections for "max-session-srv-conns"
- DOC: config: fix timeout check inheritance restrictions
- REGTESTS: connection: disable http_reuse_be_transparent.vtc if !TPROXY
- DOC: lua: add sticktable class reference from Proxy.stktable
- DOC: lua: fix Proxy.get_mode() output
- DOC: lua: add "syslog" to Proxy.get_mode() output
- MEDIUM: ssl: implement rsa/ecdsa selection with WolfSSL
- MINOR: ssl: replace 'trash.area' by 'servername' in ssl_sock_switchctx_cbk()
- MINOR: ssl: move certificate selection in a dedicate function
- MEDIUM: ssl: use ssl_sock_chose_sni_ctx() in the clienthello callback
- MINOR: mworker/cli: implement hard-reload over the master CLI
- BUG/MEDIUM: mux-h1: Properly ignore trailers when a content-length is announced
- MINOR: task/profiling: do not record task_drop_running() as a caller
- OPTIM: pattern: save memory and time using ebst instead of ebis
- BUILD: map: fix build warning
- MINOR: trace: define simple -dt argument
- MINOR: trace: parse level in a function
- MINOR: trace: parse verbosity in a function
- MINOR: trace: support -dt optional format
- OPTIM: mux-h2/zero-copy: don't allocate more buffers per connections than streams
- BUG/MINOR: quic: fix CONNECTION_CLOSE_APP encoding
- BUG/MEDIUM: stconn: Don't perform zero-copy FF if opposite SC is blocked
- BUG/MEDIUM: mux-h2: Remove H2_SF_NOTIFIED flag for H2S blocked on fast-forward
- CLEANUP: quic: Remove dead definitions/declarations
- REORG: quic: Move some QUIC CLI code to its C file
- REORG: quic: Add a new module to handle QUIC connection IDs
- REORG: quic: QUIC connection types header cleaning
- BUILD: quic: Missing RX header inclusions
- REORG: quic: Move CRYPTO data buffer defintions to QUIC TLS module
- REORG: quic: Move QUIC CRYPTO stream definitions/declarations to QUIC TLS
- REORG: quic: Move several inlined functions from quic_conn.h
- REORG: quic: Move QUIC SSL BIO method related functions to quic_ssl.c
- REORG: quic: Move the QUIC DCID parser to quic_sock.c
- REORG: quic: Rename some functions used upon ACK receipt
- REORG: quic: Move QUIC path definitions/declarations to quic_cc module
- REORG: quic: Move qc_handle_conn_migration() to quic_conn.c
- REORG: quic: Move quic_build_post_handshake_frames() to quic_conn module
- REORG: quic: Move qc_may_probe_ipktns() to quic_tls.h
- REORG: quic: Move qc_pkt_long() to quic_rx.h
- REORG: quic: Rename some (quic|qc)_conn* objects to quic_conn_closed
- REORG: quic: Move NEW_CONNECTION_ID frame builder to quic_cid
- REORG: quic: Move ncbuf related function from quic_rx to quic_conn
- REORG: quic: Add a new module for QUIC retry
- BUILD: quic: Several compiler warns fixes after retry module creation
- REORG: quic: Move qc_notify_send() to quic_conn
- REORG: quic: Add a new module for retransmissions
- REORG: quic: Remove qc_pkt_insert() implementation
- REORG: quic: Move quic_increment_curr_handshake() to quic_sock
- BUG/MINOR: cache: Remove incomplete entries from the cache when stream is closed
- MEDIUM: cli: allow custom pattern for payload
- CLEANUP: mworker/cli: use a label to return errors
- MINOR: mworker/cli: implements the customized payload pattern for master CLI
- DOC: management: add documentation about customized payload pattern
- BUG/MEDIUM: server/event_hdl: memory overrun in _srv_event_hdl_prepare_inetaddr()
- MINOR: event_hdl: add global tunables
- BUG/MAJOR: server/addr: fix a race during server addr:svc_port updates
- MEDIUM: log/balance: support FQDN for UDP log servers
- BUG/MINOR: compression: possible NULL dereferences in comp_prepare_compress_request()
- BUG/MEDIUM: master/cli: Properly pin the master CLI on thread 1 / group 1
- BUG/MEDIUM: mux-quic: Stop zero-copy FF during nego if input is not empty
- CLEANUP: log: Fix %rc comment in sess_build_logline()
- BUG/MINOR: h3: fix TRAILERS encoding
- BUG/MINOR: h3: always reject PUSH_PROMISE
- MINOR: h3: use correct error code for missing SETTINGS
- MINOR: http-fetch: Add a sample to retrieve the server status code
- DOC: config: Improve 'status' sample documentation
- MINOR: http-fetch: Add a sample to get the transaction status code
- MEDIUM: http-ana: Set termination state before returning haproxy response
- MINOR: stream: Expose session terminate state via a new sample fetch
- MINOR: stream: add a sample fetch to get the number of connection retries
- MINOR: stream: Expose the stream's uniq_id via a new sample fetch
- MINOR: muxes: Rename mux_ctl_type values to use MUX_CTL_ prefix
- MINOR: muxes: Add a callback function to send commands to mux streams
- MINOR: muxes: Implement ->sctl() callback for muxes and return the stream id
- MINOR: Add sample fetches to get the frontend and backend stream ID
- BUG/MEDIUM: cli: Don't look for payload pattern on empty commands
- DOC: config: Add argument for tune.lua.maxmem
- DOC: config: fix mention of request slot in http-response capture
- DOC: config: fix remaining mention of @reverse for attach-srv action
- DOC: config: fix missing characters in set-spoe-group action
- DOC: config: reorganize actions into their own section
- BUG/MINOR: acme.sh: update the deploy script
- MINOR: rhttp: mark reverse HTTP as experimental
- CLEANUP: quic_cid: remove unused listener arg
- BUG/MINOR: quic_tp: fix preferred_address decoding
- MINOR: quic_tp: use in_addr/in6_addr for preferred_address
- MINOR: acme.sh: use the master CLI for hot update
- DOC: config: move the cache-use and cache-store actions to the proper section
- DOC: config: fix alphabetical ordering of converter keywords
- DOC: config: add missing colon to "bytes_out" sample fetch keyword
- DOC: config: add an index of converter keywords
- DOC: config: add an index of sample fetch keywords
- BUG/MINOR: config: Stopped parsing upon unmatched environment variables
- DEBUG: unstatify a few functions that are often present in backtraces
- BUILD: server: shut a bogus gcc warning on certain ubuntu
Willy Tarreau [Thu, 30 Nov 2023 16:48:03 +0000 (17:48 +0100)]
BUILD: server: shut a bogus gcc warning on certain ubuntu
On ubuntu 20.04 and 22.04 with gcc 9.4 and 11.4 respectively, we get
the following warning:
src/server.c: In function 'srv_update_addr_port':
src/server.c:4027:3: warning: 'new_port' may be used uninitialized in this function [-Wmaybe-uninitialized]
4027 | _srv_event_hdl_prepare_inetaddr(&cb_data.addr, &s->addr, s->svc_port,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4028 | ((ip_change) ? &sa : &s->addr),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4029 | ((port_change) ? new_port : s->svc_port),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4030 | 1);
| ~~
It's clearly wrong, port_change only changes from 0 to anything else
*after* assigning new_port. Let's just preset new_port to zero instead
of trying to play smart with the compiler.
Willy Tarreau [Thu, 30 Nov 2023 16:04:16 +0000 (17:04 +0100)]
DEBUG: unstatify a few functions that are often present in backtraces
It's useful to be able to recognize certain functions that are often
present in backtraces as they call lower level functions, and for this
they must not be static. Let's remove "static" in front of these
functions:
sc_notify, sc_conn_recv, sc_conn_send, sc_conn_process,
sc_applet_process, back_establish, stream_update_both_sc,
httpclient_applet_io_handler, httpclient_applet_init,
httpclient_applet_release
Frédéric Lécaille [Thu, 30 Nov 2023 08:26:05 +0000 (09:26 +0100)]
BUG/MINOR: config: Stopped parsing upon unmatched environment variables
When an environment variable could not be matched by getenv(), the
current character to be parsed by parse_line() from <in> variable
is the trailing double quotes. If nothing is done in such a case,
this character is skipped by parse_line(), then the following spaces
are parsed as an empty argument.
To fix this, skip the double quotes character and the following spaces
to make <in> variable point to the next argument to be parsed.
Thank you to @sigint2 for having reported this issue in GH #2367.
Must be backported as far as 2.4.
Willy Tarreau [Thu, 30 Nov 2023 15:00:14 +0000 (16:00 +0100)]
DOC: config: add an index of sample fetch keywords
Now we're adding a table for each section, it allows to more easily
spot the list of available sample fetch functions and their types.
For now the arguments are not mentioned in the table because they'd
break indexing but they can be added back later.
Willy Tarreau [Thu, 30 Nov 2023 15:03:55 +0000 (16:03 +0100)]
DOC: config: add an index of converter keywords
The goal here is to have a centralized list of converters and
their in/out types.
Willy Tarreau [Thu, 30 Nov 2023 15:03:19 +0000 (16:03 +0100)]
DOC: config: add missing colon to "bytes_out" sample fetch keyword
The colon was missing between the keyword and the type, breaking
rendering and indexing.
Willy Tarreau [Thu, 30 Nov 2023 10:55:22 +0000 (11:55 +0100)]
DOC: config: fix alphabetical ordering of converter keywords
- rfc7239_* were misplaced and incorrectly ordered
- table_gpt was placed before some table_gpc*
- capture-req/res were misplaced
- htonl was misplaced
- upper/url_* were misplaced
- x509_v_err_str was misplaced
Let's fix these since poor ordering complicates their finding.
Willy Tarreau [Thu, 30 Nov 2023 15:25:58 +0000 (16:25 +0100)]
DOC: config: move the cache-use and cache-store actions to the proper section
Actions were grouped by previous commit
d54e8f810 ("DOC: config: reorganize
actions into their own section") but cache-use and cache-store were still
making reference to the cache section. This moves the text back to their
respective keywords in the actions section and leaves the example and an
explanation of how to use the keywords in the cache section.
William Lallemand [Thu, 30 Nov 2023 15:15:45 +0000 (16:15 +0100)]
MINOR: acme.sh: use the master CLI for hot update
DEPLOY_HAPROXY_MASTER_CLI allows to use the HAProxy master CLI
instead of a stats socket for DEPLOY_HAPROXY_HOT_UPDATE="yes"
The syntax of the master CLI is slightly different, a prefix with
the process number need to be added before any command.
This patch uses ${_cmdpfx} in front of every socat commands which is
filled when the master CLI is used.
Amaury Denoyelle [Thu, 30 Nov 2023 14:36:10 +0000 (15:36 +0100)]
MINOR: quic_tp: use in_addr/in6_addr for preferred_address
preferred_address is a transport parameter specify by the server. It
specified both an IPv4 and IPv6 address. These addresses were defined as
plain array in <struct tp_preferred_address>.
Convert these adressees to use the common types in_addr/in6_addr. With
this change, dumping of preferred_address is extended. It now displays
the addresses using inet_ntop() and CID value.
Amaury Denoyelle [Thu, 30 Nov 2023 13:01:51 +0000 (14:01 +0100)]
BUG/MINOR: quic_tp: fix preferred_address decoding
quic_transport_param_dec_pref_addr() is responsible to decode
preferred_address from received transport parameter. There was two
issues with this function :
* address and port location as defined in RFC were inverted for both
IPv4 and IPv6 during decoding
* an invalid check was done to ensure decoded CID length corresponds to
remaining buffer size. It did not take into account the final field
for stateless reset token.
These issues were never encountered as only server can emit
preferred_address transport parameter, so the impact of this bug is
invisible.
This should be backported up to 2.6.
Amaury Denoyelle [Thu, 30 Nov 2023 13:00:56 +0000 (14:00 +0100)]
CLEANUP: quic_cid: remove unused listener arg
retrieve_qc_conn_from_cid() requires listener as argument whereas it is
unused. This is an artifact from the old architecture where CID trees
where stored on listener instances instead of globally. Remove it to
better reflect this change.
Amaury Denoyelle [Thu, 30 Nov 2023 13:28:47 +0000 (14:28 +0100)]
MINOR: rhttp: mark reverse HTTP as experimental
Mark the reverse HTTP feature as experimental. This will allow to adjust
if needed the configuration mechanism with future developments without
maintaining retro-compatibility.
Concretely, each config directives linked to it now requires to specify
first global expose-experimental-directives before. This is the case for
the following directives :
- rhttp@ prefix uses in bind and server lines
- nbconn bind keyword
- attach-srv tcp rule
Each documentation section refering to these keywords are updated to
highlight this new requirement.
Note that this commit has duplicated on several places the code from the
global function check_kw_experimental(). This is because the latter only
work with cfg_keyword type. This is not adapted with bind_kw or
action_kw types. This should be improve in a future patch.
William Lallemand [Thu, 30 Nov 2023 13:15:06 +0000 (14:15 +0100)]
BUG/MINOR: acme.sh: update the deploy script
https://github.com/acmesh-official/acme.sh/pull/4581 was updated, this
patch update the haproxy repository with the update.
the following changes were done:
- sanitize the PEM to remove the '\n' (truncated certicate chain)
- shellcheck fixes
- socat format is directly used in the DEPLOY_HAPROXY_STATS_SOCKET variable
Willy Tarreau [Thu, 30 Nov 2023 08:28:39 +0000 (09:28 +0100)]
DOC: config: reorganize actions into their own section
The split of the rulesets from their respective actions has long been
overdue so it's time to do it because it has become extremely difficult
to add simple actions in the documentation, as well as it's hard to find
them.
This commit creates two new sections "4.3 Actions keywords matrix" and
"4.4 Alphabetically sorted actions reference" that enumerates all known
actions, with a check indicating for which rule sets they're valid. This
removes all the repetition, occurrences of "see http-request blah for
details" and significantly reduces the number of keywords listed in the
proxies section. This removes 2245 lines from the proxies section in
exchange of 1608 in these new sections.
Willy Tarreau [Thu, 30 Nov 2023 08:27:51 +0000 (09:27 +0100)]
DOC: config: fix missing characters in set-spoe-group action
It was written "Thaction" instead of "This action".
Willy Tarreau [Thu, 30 Nov 2023 08:26:38 +0000 (09:26 +0100)]
DOC: config: fix remaining mention of @reverse for attach-srv action
The new address is "rhttp@".
Willy Tarreau [Thu, 30 Nov 2023 08:24:21 +0000 (09:24 +0100)]
DOC: config: fix mention of request slot in http-response capture
It's response slot, not request slot.
Olivier Duclos [Wed, 29 Nov 2023 11:08:12 +0000 (12:08 +0100)]
DOC: config: Add argument for tune.lua.maxmem
Make it clear that tune.lua.maxmem expects a number.
Christopher Faulet [Wed, 29 Nov 2023 12:20:59 +0000 (13:20 +0100)]
BUG/MEDIUM: cli: Don't look for payload pattern on empty commands
A regression was introduced by commit
9431aa0bdf ("BUG/MEDIUM: cli: Don't
look for payload pattern on empty commands").
On empty commands (really empty or containing spaces and tabs), the number of
arguments set to 0. However we look for the payload pattern without checking
it. The result is an access at the index -1 in the argument array. It is of
course invalid.
To fix the issue, we just skip this part when there is no argument. Note that
the empty command is still sent to the worker.
This patch should solve the issue #2365. No backport needed.
Christopher Faulet [Tue, 28 Nov 2023 15:34:23 +0000 (16:34 +0100)]
MINOR: Add sample fetches to get the frontend and backend stream ID
"fc.id" and "bc.id" sample fetches can now be used to get, respectively, the
frontend or the backend stream ID. They rely on ->sctl() callback function
on the mux attached to the corresponding SC.
It means these sample fetches work only for connection, not applets, and
from the time a multiplexer is installed.
Christopher Faulet [Tue, 28 Nov 2023 14:12:51 +0000 (15:12 +0100)]
MINOR: muxes: Implement ->sctl() callback for muxes and return the stream id
All muxes now implements the ->sctl() callback function and are able to
return the stream ID. For the PT multiplexer, it is always 0. For the H1
multiplexer it is the request count for the current H1 connection (added for
this purpose). The FCGI, H2 and QUIC muxes, the stream ID is returned.
The stream ID is returned as a signed 64 bits integer.
Christopher Faulet [Tue, 28 Nov 2023 13:53:58 +0000 (14:53 +0100)]
MINOR: muxes: Add a callback function to send commands to mux streams
Just like the ->ctl() callback function, used to send commands to mux
connections, the ->sctl() callback function can now be used to send commands
to mux streams. The first command, MUX_SCTL_SID, is a way to request the mux
stream ID.
It will be implemented later for each mux.
Christopher Faulet [Tue, 28 Nov 2023 13:27:51 +0000 (14:27 +0100)]
MINOR: muxes: Rename mux_ctl_type values to use MUX_CTL_ prefix
Instead of the generic MUX_, we now use MUX_CTL_ prefix for all mux_ctl_type
value. This will avoid any ambiguities with other enums, especially with a
new one that will be added to get information on mux streams.
Christopher Faulet [Tue, 28 Nov 2023 20:01:18 +0000 (21:01 +0100)]
MINOR: stream: Expose the stream's uniq_id via a new sample fetch
"txn.id32" may now be used to get the stream's uniq_id. It is equivalent to
%rt in logs.
Christopher Faulet [Tue, 28 Nov 2023 12:59:59 +0000 (13:59 +0100)]
MINOR: stream: add a sample fetch to get the number of connection retries
"txn.conn_retries" can now be used to get the number of connection
retries. This value is only stable once the connection is fully
established. For HTTP sessions, L7-retries must also be passed.
Christopher Faulet [Tue, 28 Nov 2023 07:45:33 +0000 (08:45 +0100)]
MINOR: stream: Expose session terminate state via a new sample fetch
It is now possible to retrieve the session terminate state, using
"txn.sess_term_state". The sample fetch returns the 2-character session
termation state.
Of course, the result of this sample fetch is volatile. It is subject to
change. It is also most of time useless because no termation state is set
except at the end. It should only be useful in http-after-response rule
sets. It may also be used to customize the logs using a log-format
directive.
This patch should fix the issue #2221.
Christopher Faulet [Tue, 28 Nov 2023 07:38:45 +0000 (08:38 +0100)]
MEDIUM: http-ana: Set termination state before returning haproxy response
When, for any reason and at any step, HAProxy decides to interrupt an HTTP
transaction, it returns a dedicated responses to the client (possibly empty)
and it sets the stream flags used to produce the session termination state.
These both operation were performed in any order, depending on the code
path. Most of time, the HAPRoxy response is produced first.
With this patch, the stream flags for the termination state are now set
first. This way, these flags become visible from http-after-reponse rule
sets. Only errors when the HAProxy response is generated are reported later.
Christopher Faulet [Mon, 27 Nov 2023 17:49:47 +0000 (18:49 +0100)]
MINOR: http-fetch: Add a sample to get the transaction status code
It was possible get the status code in the HTTP response and the one
received from the server. Thanks to 'txn.status', it is now possible to get
the transaction status code. It is equivalent to '%ST' in log-format.
Most of time, it is the same than 'status', except if the status code of the
HTTP reply does not match the one used to interrupt the transaction. For
instance, an error file use mapped on 400 containing a 404.
Christopher Faulet [Mon, 27 Nov 2023 17:27:47 +0000 (18:27 +0100)]
DOC: config: Improve 'status' sample documentation
We clearly state the 'status' sample returns the status code the client will
receive, if no change happens on the HTTP response. This should avoid
ambiguities with the 'server-status' sample fetch.
Christopher Faulet [Mon, 27 Nov 2023 17:12:13 +0000 (18:12 +0100)]
MINOR: http-fetch: Add a sample to retrieve the server status code
The code returned by the "status" sample fetch is the one in the HTTP
response at the moment the sample is evaluated. It may be the status code in
the server response or the one of the HAProxy reply in case of error, deny,
redirect...
However, it could be handy to retrieve the status code returned by the
server, when a HTTP response was really received from it. It is the purpose
of the "server_status" sample fetch. The server status code itself is stored
in the HTTP txn.
Amaury Denoyelle [Tue, 28 Nov 2023 15:01:38 +0000 (16:01 +0100)]
MINOR: h3: use correct error code for missing SETTINGS
Each received HTTP/3 frame is checked to ensure it is valid given the
type of stream and its current status. This was implemented via
h3_is_frame_valid().
Previously, no distinction was made for error code, so every failure
triggered a CONNECTION_CLOSE_APP with code H3_FRAME_UNEXPECTED. However,
this function also ensures that the first frame received on control
frame is of type SETTINGS. If not, the error code to use is
H3_MISSING_SETTINGS.
To support this, adjust the function prototype. Instead of returning a
boolean, 0 is returned for success, or a HTTP/3 error code. The function
is renamed h3_check_frame_valid() to reflects the return type change.
This is not considered as a bug as previously the connection was
correctly closed on a missing SETTINGS, albeit with a non conform error
code. It's not deemed as sufficient to be backported.
Amaury Denoyelle [Tue, 28 Nov 2023 11:00:40 +0000 (12:00 +0100)]
BUG/MINOR: h3: always reject PUSH_PROMISE
The condition for checking PUSH_PROMISE was not correctly interpreted
from the RFC. Initially, it rejects such a frame for every stream
initiated from client side.
In fact, the RFC indicates that PUSH_PROMISE are never sent by a client.
Thus, it can be rejected in any case until HTTP/3 will be implemented on
the backend side.
This should be backported up to 2.6.
Amaury Denoyelle [Tue, 28 Nov 2023 14:59:38 +0000 (15:59 +0100)]
BUG/MINOR: h3: fix TRAILERS encoding
HTTP/3 trailers encoding was never working as intended. It's because
h3_trailers_to_htx() manipulate a newly allocated buffer instead of the
already existing channel one. Thus, HTX message handled by the stream
was incomplete as it lacked trailers and EOM.
Fix this by reusing the already allocated channel buffer in
h3_trailers_to_htx().
This bug was detected by simulating TRAILERS emission which generate
CL--- state due to missing request side termination signal. Its impact
is deemed as minimal as trailers are pretty infrequent for now in
HTTP/3.
This must be backported up to 2.7.
Christopher Faulet [Tue, 28 Nov 2023 10:21:40 +0000 (11:21 +0100)]
CLEANUP: log: Fix %rc comment in sess_build_logline()
%rq was used instead of %rc.
Christopher Faulet [Tue, 28 Nov 2023 21:24:45 +0000 (22:24 +0100)]
BUG/MEDIUM: mux-quic: Stop zero-copy FF during nego if input is not empty
When the producer negociate with the QUIC mux to perform a zero-copy
fast-forward, data in the input buffer are first transferred in the H3
buffer. However, after the transfer, if the input buffer is not empty, the
data fast-forwarding must be stopped. In this case, qmux_nego_ff() must
return 0.
No backport needed.
Christopher Faulet [Tue, 28 Nov 2023 20:50:21 +0000 (21:50 +0100)]
BUG/MEDIUM: master/cli: Properly pin the master CLI on thread 1 / group 1
A previous fix was pushed for that (
13fb7170be "BUG/MEDIUM: master/cli: Pin
the master CLI on the first thread of the group 1" ). Unfortunately, instead
of the master CLI, it is the sockpairs between the master and the workers
that were pinned to the first thread of the group 1. So the crash is still
there.
So, again, to fix the bug the master CLI is now pinned on the first thread
of the first group.
patch should fix the issue #2259 and must be backported to 2.8.
Aurelien DARRAGON [Tue, 28 Nov 2023 14:47:25 +0000 (15:47 +0100)]
BUG/MINOR: compression: possible NULL dereferences in comp_prepare_compress_request()
This bug was introduced in
ead43fe4f2 ("MEDIUM: compression: Make it so
we can compress requests as well.")
2 cases where not properly handled, resulting in 2 possible NULL
dereferences leading to crashes in the function at runtime:
- when the backend didn't define any compression options so its comp
pointer is NULL (ie: if only the frontend defines some comp options)
- when both the frontend and the backend didn't set a compression algo
but at least one of the two defined some other comp options (comp
pointer set)
For the first case, we added the missing checks to make sure we don't
read ->comp pointer if it is NULL.
For the second case, we properly return from the function if no
compression algo is defined, because there is no default value that could
be used as a fallback.
This should be backported to 2.8.
Aurelien DARRAGON [Thu, 9 Nov 2023 15:57:01 +0000 (16:57 +0100)]
MEDIUM: log/balance: support FQDN for UDP log servers
In previous log backend implementation, we created a pseudo log target
for each declared log server, and we made the log target's address point
to the actual server address to save some time and prevent unecessary
copies.
But this was done without knowing that when FQDN is involved (more broadly
when dns/resolution is involved), the "port" part of server addr should
not be relied upon, and we should explicitly use ->svc_port for that
purpose.
With that in mind and thanks to the previous commit, some changes were
required: we allocate a dedicated addr within the log target when target
is in DGRAM mode. The addr is first initialized with known values and it
is then updated automatically by _srv_set_inetaddr() during runtime.
(the change is atomic so readers don't need to worry about it)
addr from server "log target" (INET/DGRAM mode) is made of the combination
of server's address (lacking the port part) and server's svc_port.
Aurelien DARRAGON [Mon, 13 Nov 2023 17:14:24 +0000 (18:14 +0100)]
BUG/MAJOR: server/addr: fix a race during server addr:svc_port updates
For inet families (IP4/IP6), it is expected that server's addr/port might
be updated at runtime from DNS, cli or lua for instance.
Such updates were performed under the server's lock.
Unfortunately, most readers such as backend.c or sink.c perform the read
without taking server's lock because they can't afford slowing down their
processing for a type of event which is normally rare. But this could
result in bad values being read for the server addr:svc_port tuple (ie:
during connection etablishment) as a result of concurrent updates from
external components, which can obviously cause some undesirable effects.
Instead of slowing the readers down, as we consider server's addr changes
are relatively rare, we take another approach and try to update the
addr:port atomically by performing changes under full thread isolation
when a new change is requested. The changes are performed by a dedicated
task which takes care of isolating the current thread and doesn't depend
on other threads (independent code path) to protect against dead locks.
As such, server's addr:port changes will now be performed atomically, but
they will not be processed instantly, they will be translated to events
that the dedicated task will pick up from time to time to apply the
pending changes.
This bug existed for a very long time and has never been reported so
far. It was discovered by reading the code during the implementation
of log backend ("mode log" in backends). As it involves changes in
sensitive areas as well as thread isolation, it is probably not
worth considering backporting it for now, unless it is proven that it
will help to solve bugs that are actually encountered in the field.
This patch depends on:
- 24da4d3 ("MINOR: tools: use const for read only pointers in ip{cmp,cpy}")
- c886fb5 ("MINOR: server/ip: centralize server ip updates")
- event_hdl API (which was first seen on 2.8) +
683b2ae ("MINOR: server/event_hdl: add SERVER_INETADDR event") +
BUG/MEDIUM: server/event_hdl: memory overrun in _srv_event_hdl_prepare_inetaddr() +
"MINOR: event_hdl: add global tunables"
Note that the patch may be reworked so that it doesn't depend on
event_hdl API for older versions, the approach would remain the same:
this would result in a larger patch due to the need to manually
implement a global queue of pending updates with its dedicated task
responsible for picking updates and comitting them. An alternative
approach could consist in per-server, lock-protected, temporary
addr:svc_port storage dedicated to "updaters" were only the most
recent values would be kept. The sync task would then use them as
source values to atomically update the addr:svc_port members that the
runtime readers are actually using.
Aurelien DARRAGON [Fri, 24 Nov 2023 14:37:40 +0000 (15:37 +0100)]
MINOR: event_hdl: add global tunables
The local variable "event_hdl_async_max_notif_at_once" which was
introduced with the event_hdl API was left as is but with a TODO note
telling that we should make it a global tunable.
Well, we're doing this now. To prepare for upcoming tunables related to
event_hdl API, we add a dedicated struct named event_hdl_tune which is
globally exposed through the event_hdl header file so that it may be used
from everywhere. The struct is automatically initialized in
event_hdl_init() according to defaults.h.
"event_hdl_async_max_notif_at_once" now becomes
"event_hdl_tune.max_events_at_once" with it's dedicated
configuation keyword: "tune.events.max-events-at-once".
We're also taking this opportunity to raise the default value from 10
to 100 since it's seems quite reasonnable given existing async event_hdl
users.
The documentation was updated accordingly.
Aurelien DARRAGON [Sat, 25 Nov 2023 15:39:19 +0000 (16:39 +0100)]
BUG/MEDIUM: server/event_hdl: memory overrun in _srv_event_hdl_prepare_inetaddr()
As reported in GH #2358, #2359, #2360, #2361 and #2362: ipv6 address
handling may cause memory overrun due to struct in6_addr being handled
as sockaddr_in6 which is larger. Moreover, source variable wasn't properly
read from since the raw value was used as a pointer instead of pointing to
the actual variable's address.
This bug was introduced by
6fde37e046
("MINOR: server/event_hdl: add SERVER_INETADDR event")
Unfortunately for us, gcc didn't catch this and, this actually used to
"work" by accident since in6_addr struct is made of array so not passing
pointer explicitly still resolved to the proper starting address..
Hopefully this was caught by coverity so thanks to Ilya for that.
The fix is simple: we simply copy the whole in6_addr struct by accessing
it using a pointer and using the proper struct size for the copy.
William Lallemand [Mon, 27 Nov 2023 22:26:52 +0000 (23:26 +0100)]
DOC: management: add documentation about customized payload pattern
One can customize a payload pattern in order to change the way the
payload ends.
William Lallemand [Tue, 28 Nov 2023 16:57:21 +0000 (17:57 +0100)]
MINOR: mworker/cli: implements the customized payload pattern for master CLI
Implements the customized payload pattern for the master CLI.
The pattern is stored in the stream in char pcli_payload_pat[8].
The principle is basically the same as the CLI one, it looks for '<<'
then stores what's between '<<' and '\n', and look for it to exit the
payload mode.
William Lallemand [Tue, 28 Nov 2023 16:28:07 +0000 (17:28 +0100)]
CLEANUP: mworker/cli: use a label to return errors
Remove the returns in the function to end directly at the end label.
William Lallemand [Mon, 27 Nov 2023 15:04:20 +0000 (16:04 +0100)]
MEDIUM: cli: allow custom pattern for payload
The CLI payload syntax has some limitation, it can't handle payloads
with empty lines, which is a common problem when uploading a PEM file
over the CLI.
This patch implements a way to customize the ending pattern of the CLI,
so we can't look for other things than empty lines.
A char cli_payload_pat[8] is used in the appctx to store the customized
pattern. The pattern can't be more than 7 characters and can still empty
to match an empty line.
The cli_io_handler() identifies the pattern and stores it, and
cli_parse_request() identifies the end of the payload.
If the customized pattern between "<<" and "\n" is more than 7
characters, it is not considered as a pattern.
This patch only implements the parser for the 'stats socket', another
patch is needed for the 'master CLI'.
Remi Tricot-Le Breton [Tue, 28 Nov 2023 16:08:56 +0000 (17:08 +0100)]
BUG/MINOR: cache: Remove incomplete entries from the cache when stream is closed
When a stream is interrupted by the client before the full answer is
stored in the cache, we end up with an incomplete entry in the cache
that cannot be overwritten until it "naturally" expires. In such a case,
we call the cache filter's cache_store_strm_deinit callback without ever
calling cache_store_http_end which means that the 'complete' flag is
never set on the concerned cache_entry.
This patch adds a check on the 'complete' flag in the strm_deinit
callback and removes the entry from the cache if it is incomplete.
A way to exhibit this bug is to try to get the same "big" response on
multiple clients at the same time thanks to h2load for instance, and to
interrupt the client side before the answer can be fully stored in the
cache.
This patch can be backported up to 2.4 but it will need some rework
starting with branch 2.8 because of the latest cache changes.
Frédéric Lécaille [Tue, 28 Nov 2023 14:01:01 +0000 (15:01 +0100)]
REORG: quic: Move quic_increment_curr_handshake() to quic_sock
Move quic_increment_curr_handshake() from quic_conn.c to quic_sock.h to be inlined.
Also move all the inlined functions at the end of this header.
Frédéric Lécaille [Tue, 28 Nov 2023 13:52:22 +0000 (14:52 +0100)]
REORG: quic: Remove qc_pkt_insert() implementation
As this function does only a few things with a not very well chosen name,
remove it and replace it by the its statements at the unique location it
is called.
Frédéric Lécaille [Tue, 28 Nov 2023 13:27:33 +0000 (14:27 +0100)]
REORG: quic: Add a new module for retransmissions
Move several functions in relation with the retransmissions from TX part
(quic_tx.c) to quic_retransmit.c new C file.
Frédéric Lécaille [Tue, 28 Nov 2023 13:02:17 +0000 (14:02 +0100)]
REORG: quic: Move qc_notify_send() to quic_conn
Move qc_notify_send() from quic_tx.c to quic_conn.c. Note that it was already
exported from both quic_conn.h and quic_tx.h. Modify this latter header
to fix the duplication.
Frédéric Lécaille [Tue, 28 Nov 2023 10:37:44 +0000 (11:37 +0100)]
BUILD: quic: Several compiler warns fixes after retry module creation
Such a warning appeared after having added quic_retry.h which includes only
headers for types (quic_cid-t.h, clock-t.h...)
In file included from include/haproxy/quic_retry.h:12,
from src/quic_retry.c:5:
include/haproxy/quic_cid-t.h:26:26: error: field ‘seq_num’ has incomplete type
26 | struct eb64_node seq_num;
Frédéric Lécaille [Tue, 28 Nov 2023 10:25:04 +0000 (11:25 +0100)]
REORG: quic: Add a new module for QUIC retry
Add quic_retry.c new C file for the QUIC retry feature:
quic_saddr_cpy() moved from quic_tx.c,
quic_generate_retry_token_aad() moved from
quic_generate_retry_token() moved from
parse_retry_token() moved from
quic_retry_token_check() moved from
quic_retry_token_check() moved from
Frédéric Lécaille [Tue, 28 Nov 2023 08:09:36 +0000 (09:09 +0100)]
REORG: quic: Move ncbuf related function from quic_rx to quic_conn
Move quic_get_ncbuf() and quic_free_ncbuf() from quic_rx.c to quic_conn.h
as static inlined functions.
Frédéric Lécaille [Tue, 28 Nov 2023 07:59:15 +0000 (08:59 +0100)]
REORG: quic: Move NEW_CONNECTION_ID frame builder to quic_cid
Move qc_build_new_connection_id_frm() from quic_conn.c to quic_cid.c.
Also move quic_connection_id_to_frm_cpy() from quic_conn.h to quic_cid.h.
Frédéric Lécaille [Tue, 28 Nov 2023 07:29:12 +0000 (08:29 +0100)]
REORG: quic: Rename some (quic|qc)_conn* objects to quic_conn_closed
These objects could be confused with the ones defined by the congestion control
part (quic_cc.c).