Aurelien DARRAGON [Tue, 1 Apr 2025 17:32:58 +0000 (19:32 +0200)]
CLEANUP: log: adjust _lf_cbor_encode_byte() comment
_lf_cbor_encode_byte() comment was not updated in
c33b857df ("MINOR: log:
support true cbor binary encoding") to reflect the new behavior.
Indeed, binary form is now supported. Updating the comment that says
otherwise.
(cherry picked from commit
ce6951d6f9259f778c0271da54a615c892184691)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
3a715a4d6d2e8554015eac5d59c059275bc4c6e6)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Aurelien DARRAGON [Tue, 1 Apr 2025 09:01:45 +0000 (11:01 +0200)]
BUG/MINOR: hlua_fcn: fix potential UAF with Queue:pop_wait()
If Queue:pop_wait() excecuted from a stream context and pop_wait() is
aborted due to a Lua or ressource error, then the waiting object pointing
to the task will still be registered, so if the task eventually dissapears,
Queue:push() may try to wake invalid task pointer..
To prevent this bug from happening, we now rely on notification_* API to
deliver waiting signals. This way signals are properly garbage collected
when a lua context is destroyed.
It should be backported in 2.8 with
86fb22c55 ("MINOR: hlua_fcn: add Queue
class").
This patch depends on ("MINOR: task: add thread safe notification_new and
notification_wake variants")
(cherry picked from commit
c6fa061f22e0409a9c1e0dbe9d4bd9a30eff6ba1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
51de928f9eda86631ef627d2a750a02857ccc38b)
[ada: ctx adjt]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Aurelien DARRAGON [Tue, 1 Apr 2025 08:07:50 +0000 (10:07 +0200)]
MINOR: task: add thread safe notification_new and notification_wake variants
notification_new and notification_wake were historically meant to be
called by a single thread doing both the init and the wakeup for other
tasks waiting on the signals.
In this patch, we extend the API so that notification_new and
notification_wake have thread-safe variants that can safely be used with
multiple threads registering on the same list of events and multiple
threads pushing updates on the list.
(cherry picked from commit
b77b1a2c3ac70a719a2a06964e56a206ab9cc6ec)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
ad6e04a1b575a731b01913ff092352d958481775)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Olivier Houchard [Thu, 3 Apr 2025 14:20:11 +0000 (16:20 +0200)]
TESTS: Fix build for filltab25.c
Give a return type to main(), so that filltab25.c compiles with
modern compilers.
(cherry picked from commit
4715c557e98725e28faac7fce449f090e136ac0b)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
fb1086c101bddb9ab49bb09d9077802da37e80d8)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Christopher Faulet [Mon, 17 Mar 2025 13:49:45 +0000 (14:49 +0100)]
BUG/MEDIUM: stream: Fix a possible freeze during a forced shut on a stream
When a forced shutdown is performed on a stream, it is possible to freeze it
infinitly because it is performed in an unexpected way from process_stream()
point of view, especially when the stream is waiting for a server
connection. The events sequence is a bit complex but at the end the stream
remains blocked in turn-around state and no event are trriggered to unblock
it.
By trying to fix the issue, we considered it was safer to rethink the
feature. The idea is to quickly shutdown a stream to release resources. For
instance to be able to delete a server. So, instead of scheduling a
shutdown, it is more efficient to trigger an error and detach the stream
from the server, if neecessary. The same code than the one used to deal with
connection errors in back_handle_st_cer() is used.
This patch must be slowly backported as far as 2.6.
(cherry picked from commit
51611a5b702e6dbf2e5ac56cbcef211326414282)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
bf7dfa488dc3bf830179f13e1262386735a4be91)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Amaury Denoyelle [Mon, 31 Mar 2025 15:57:56 +0000 (17:57 +0200)]
BUG/MEDIUM: backend: fix reuse with set-dst/set-dst-port
On backend connection reuse, a hash is calculated from various
parameters, to ensure the selected connection match the requested
parameters. Notably, destination address is one of these parameters.
However, it is only taken into account if using a transparent server
(server address 0.0.0.0).
This may cause issue where an incorrect connection is reused, which is
not targetted to the correct destination address. This may be the case
if a set-dst/set-dst-port is used with a transparent proxy (proxy option
transparent).
The fix is simple enough. Destination address is now always used as
input to the connection reuse hash.
This must be backported up to 2.6. Note that for reverse HTTP to work,
it relies on the following patch, which ensures destination address
remains NULL in this case.
commit
e94baf6ca71cb2319610baa74dbf17b9bc602b18
BUG/MINOR: rhttp: fix incorrect dst/dst_port values
(cherry picked from commit
5fda64e87e7963fa65812e0583338191e4cc7c8b)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
b2d0abb7f22debf025e313b1943cda97294b3a16)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Amaury Denoyelle [Thu, 27 Mar 2025 16:06:06 +0000 (17:06 +0100)]
BUG/MINOR: backend: do not overwrite srv dst address on reuse
Previously, destination address of backend connection was systematically
always reassigned. However, this step is unnecessary on connection
reuse. Indeed, reuse should only be conducted with connection using the
same destination address matching the stream requirements.
This patch removes this unnecessary assignment. It is now only performed
when reuse cannot be conducted and a new connection is instantiated.
Functionnally speaking, this patch should not change anything in theory,
as reuse is performed in conformance with the destination address.
However, it appears that it was not always properly enforced. The
systematic assignment of the destination address hides these issues, so
it is now remove. The identified bogus cases will then be fixed in the
following patches.would
This should be backported up to all stable versions.
(cherry picked from commit
d7fa8e88c4cda6d115b05c65cded5acb4069ee97)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
00ada2c2806a007df77f0f4fa2aa85938ea0b19a)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Amaury Denoyelle [Mon, 31 Mar 2025 15:57:35 +0000 (17:57 +0200)]
BUG/MINOR: rhttp: fix incorrect dst/dst_port values
With a @rhttp server, connect is not possible, transfer is only possible
via idle connection reuse. The server does not have any network address.
Thus, it is unnecessary to allocate the stream destination address prior
to connection reuse. This patch adjusts this by fixing
alloc_dst_address() to take this into account.
Prior to this patch, alloc_dst_address() would incorrectly assimilate a
@rhttp server with a transparent proxy mode. Thus stream destination
address would be copied from the destination address. Connection adress
would then be rewrote with this incorrect value. This did not impact
connect or reuse as destination addr is only used in idle conn hash
calculation for transparent servers. However, it causes incorrect values
for dst/dst_port samples.
This should be backported up to 2.9.
(cherry picked from commit
c05bb8c967c395f88666ccc93c00726f59a690b6)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
711185ca396122e4bd19725ab6d13c3574ff2209)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Willy Tarreau [Wed, 2 Apr 2025 09:36:43 +0000 (11:36 +0200)]
BUILD: compiler: undefine the CONCAT() macro if already defined
As Ilya reported in issue #2911, the CONCAT() macro breaks on NetBSD
which defines its own as __CONCAT() (which is exactly the same). Let's
just undefine it before ours to fix the issue instead of renaming, but
keep ours so that we don't have doubts about what we're running with.
Note that the patch introducing this breaking change was backported
to 3.0.
(cherry picked from commit
4ec5509541e29956107011d45c971a9fe3a430f1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
bf3252c1d48a63cccee17793b1f24e01409eaf01)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Willy Tarreau [Wed, 2 Apr 2025 09:17:05 +0000 (11:17 +0200)]
DOC: config: fix two missing "content" in "tcp-request" examples
As reported by Uku Sõrmus in GitHub issue #2917, two "tcp-request" rules
in an example were mistakenly missing the "content" hook, rendering them
invalid.
This can be backported.
(cherry picked from commit
3de99a09192e07a511c4956692a9cff0b252a77f)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
63384d0eb05fe8543277f8bca2d874311e867211)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Valentine Krasnobaeva [Thu, 27 Mar 2025 09:16:03 +0000 (10:16 +0100)]
BUG/MINOR: log: fix gcc warn about truncating NUL terminator while init char arrays
gcc 15 throws such kind of warnings about initialization of some char arrays:
src/log.c:181:33: error: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (17 chars into 16 available) [-Werror=unterminated-string-initialization]
181 | const char sess_term_cond[16] = "-LcCsSPRIDKUIIII"; /* normal, Local, CliTo, CliErr, SrvTo, SrvErr, PxErr, Resource, Internal, Down, Killed, Up, -- */
| ^~~~~~~~~~~~~~~~~~
src/log.c:182:33: error: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (9 chars into 8 available) [-Werror=unterminated-string-initialization]
182 | const char sess_fin_state[8] = "-RCHDLQT"; /* cliRequest, srvConnect, srvHeader, Data, Last, Queue, Tarpit */
So, let's make it happy by not giving the sizes of these char arrays
explicitly, thus he can accomodate there NUL terminators.
Reported in GitHub issue #2910.
This should be backported up to 2.6.
(cherry picked from commit
44f98f1747e8b2ef400dafa249b3f70a2844e8fe)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
d77be2405b6cd5d0cc6c29953cf43315967f02d2)
[ada: patch was applied manually because of multiple context conflicts]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Amaury Denoyelle [Thu, 20 Mar 2025 17:10:56 +0000 (18:10 +0100)]
BUG/MINOR: mux-quic: remove extra BUG_ON() in _qcc_send_stream()
The following patch fixed a BUG_ON() which could be triggered if RS/SS
emission was scheduled after stream local closure.
7ee1279f4b8416435faba5cb93a9be713f52e4df
BUG/MEDIUM: mux-quic: fix crash on RS/SS emission if already close local
qcc_send_stream() was rewritten as a wrapper around an internal
_qcc_send_stream() used to bypass the faulty BUG_ON(). However, an extra
unnecessary BUG_ON() was added by mistake in _qcc_send_stream().
This should not cause any issue, as the BUG_ON() is only active if <urg>
argument is false, which is not the case for RS/SS emission. However,
this patch is labelled as a bug as this BUG_ON() is unnecessary and may
cause issues in the future.
This should be backported up to 2.8, after the above mentionned patch.
(cherry picked from commit
c5f8df8d55e6a85f72e415c63188345dc670e53b)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
ebeece75c2e138e7c7c111ee7f66a65e79d32ef3)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Amaury Denoyelle [Thu, 20 Mar 2025 15:01:16 +0000 (16:01 +0100)]
BUG/MEDIUM: mux-quic: fix crash on RS/SS emission if already close local
A BUG_ON() is present in qcc_send_stream() to ensure that emission is
never performed with a stream already closed locally. However, this
function is also used for RESET_STREAM/STOP_SENDING emission. No
protection exists to ensure that RS/SS is not scheduled after stream
local closure, which would result in this BUG_ON() crash.
This crash can be triggered with the following QUIC client sequence :
1. SS is emitted to open a new stream. QUIC-MUX schedules a RS emission
by and the stream is locally closed.
2. An invalid HTTP/3 request is sent on the same stream, for example
with duplicated pseudo-headers. The objective is to ensure
qcc_abort_stream_read() is called after stream closure, which results
in the following backtrace.
0x000055555566a620 in qcc_send_stream (qcs=0x7ffff0061420, urg=1, count=0) at src/mux_quic.c:1633
1633 BUG_ON(qcs_is_close_local(qcs));
[ ## gdb ## ] bt
#0 0x000055555566a620 in qcc_send_stream (qcs=0x7ffff0061420, urg=1, count=0) at src/mux_quic.c:1633
#1 0x000055555566a921 in qcc_abort_stream_read (qcs=0x7ffff0061420) at src/mux_quic.c:1658
#2 0x0000555555685426 in h3_rcv_buf (qcs=0x7ffff0061420, b=0x7ffff748d3f0, fin=0) at src/h3.c:1454
#3 0x0000555555668a67 in qcc_decode_qcs (qcc=0x7ffff0049eb0, qcs=0x7ffff0061420) at src/mux_quic.c:1315
#4 0x000055555566c76e in qcc_recv (qcc=0x7ffff0049eb0, id=12, len=0, offset=23, fin=0 '\000',
data=0x7fffe0049c1c "\366\r,\230\205\354\234\301;\2563\335\037k\306\334\037\260", <incomplete sequence \323>) at src/mux_quic.c:1901
#5 0x0000555555692551 in qc_handle_strm_frm (pkt=0x7fffe00484b0, strm_frm=0x7ffff00539e0, qc=0x7fffe0049220, fin=0 '\000') at src/quic_rx.c:635
#6 0x0000555555694530 in qc_parse_pkt_frms (qc=0x7fffe0049220, pkt=0x7fffe00484b0, qel=0x7fffe0075fc0) at src/quic_rx.c:980
#7 0x0000555555696c7a in qc_treat_rx_pkts (qc=0x7fffe0049220) at src/quic_rx.c:1324
#8 0x00005555556b781b in quic_conn_app_io_cb (t=0x7fffe0037f20, context=0x7fffe0049220, state=49232) at src/quic_conn.c:601
#9 0x0000555555d53788 in run_tasks_from_lists (budgets=0x7ffff748e2b0) at src/task.c:603
#10 0x0000555555d541ae in process_runnable_tasks () at src/task.c:886
#11 0x00005555559c39e9 in run_poll_loop () at src/haproxy.c:2858
#12 0x00005555559c41ea in run_thread_poll_loop (data=0x55555629fb40 <ha_thread_info+64>) at src/haproxy.c:3075
The proper solution is to not execute this BUG_ON() for RS/SS emission.
Indeed, it is valid and can be useful to emit these frames, even after
stream local closure.
To implement this, qcc_send_stream() has been rewritten as a mere
wrapper function around the new internal _qcc_send_stream(). The latter
is used only by QMUX for STREAM, RS and SS emission. Application layer
continue to use the original function for STREAM emission, with the
BUG_ON() still in place there.
This must be backported up to 2.8.
(cherry picked from commit
7ee1279f4b8416435faba5cb93a9be713f52e4df)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
c44b72d88a48f81a3b2fd1433f372226131a2e5a)
[ada: ctx adjt: since
3a00193b2 ("MINOR: mux-quic: split STREAM and RS/SS
emission") was not backported past 3.1, _qcc_send_stream() was based
on prior qcc_send_stream() implementation]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Emeric Brun [Thu, 3 Apr 2025 08:32:30 +0000 (10:32 +0200)]
BUG/MEDIUM: peers: prevent learning expiration too far in futur from unsync node
This patch sets the expire of the entry to the max value in
configuration if the value showed in the peer update message
is too far in futur.
This should be backported an all supported branches.
(cherry picked from commit
b02b8453d15dfe2c45d132484e381c27f63d2fb1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
7dfb6efbaf8c17a82be38208dc1e8ce693d21b74)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Emeric Brun [Thu, 3 Apr 2025 08:29:16 +0000 (10:29 +0200)]
BUG/MINOR: peers: fix expire learned from a peer not converted from ms to ticks
This is has now impact currently since MS_TO_TICKS macro does nothing
but it will prevent further bugs.
(cherry picked from commit
00461fbfbfd19e93af621658abcf7d1205f44182)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
d128753b6160f15f3b361e68fcbd3072c8ea681a)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Aurelien DARRAGON [Tue, 1 Apr 2025 18:25:08 +0000 (20:25 +0200)]
MINOR: log: support "raw" logformat node typecast
"raw" logformat node typecast is a special value (unlike str,bool,int..)
which tells haproxy to completely ignore logformat options (including
encoding ones) and force binary output for the current node only. It is
mainly intended for use with JSON or CBOR encoders in order to generate
nested CBOR or nested JSON by storing intermediate log-formats within
variables and assembling the final object in the parent log-format.
Example:
http-request set-var-fmt(txn.intermediate) "%{+json}o %(lower)[str(value)]"
log-format "%{+json}o %(upper)[str(value)] %(intermediate:raw)[var(txn.intermediate)]"
Would produce:
{"upper": "value", "intermediate": {"lower": "value"}}
(cherry picked from commit
423cca64b632771235f2633f5c2a8b9855317cf4)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit
cf4488acff80814fa26e40fab24c06162e54bef4)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Willy Tarreau [Thu, 20 Mar 2025 13:27:37 +0000 (14:27 +0100)]
[RELEASE] Released version 3.0.9
Released version 3.0.9 with the following main changes :
- BUG/MEDIUM: ssl: chosing correct certificate using RSA-PSS with TLSv1.3
- BUG/MEDIUM: mux-quic: do not attach on already closed stream
- MINOR: mux-quic: change return value of qcs_attach_sc()
- BUG/MINOR: mux-quic: handle closure of uni-stream
- BUG/MINOR: spoe: Check the shared waiting queue to shut applets during stopping
- BUG/MINOR: spoe: Allow applet creation when closing the last one during stopping
- BUG/MEDIUM: spoe: Don't wakeup idle applets in loop during stopping
- BUG/MEDIUM: fd: mark FD transferred to another process as FD_CLONED
- REGTESTS: Fix truncated.vtc to send 0-CRLF
- BUG/MEDIUM: htx: wrong count computation in htx_xfer_blks()
- DOC: htx: clarify <mark> parameter for htx_xfer_blks()
- DOC: option redispatch should mention persist options
- BUG/MEDIUM: server: properly initialize PROXY v2 TLVs
- TESTS: ist: fix wrong array size
- CI: github: fix h2spec.config proxy names
- BUG/MINOR: stream: fix age calculation in "show sess" output
- BUG/MINOR: cfgparse-tcp: relax namespace bind check
- MINOR: startup: adjust alert messages, when capabilities are missed
- BUG/MEDIUM: thread: use pthread_self() not ha_pthread[tid] in set_affinity
- DOC: management: rename some last occurences from domain "dns" to "resolvers"
- BUG/MINOR: server: fix the "server-template" prefix memory leak
- BUILD: ssl: allow to build without the renegotiation API of WolfSSL
- BUILD: ssl: more cleaner approach to WolfSSL without renegotiation
- BUG/MEDIUM: debug: close a possible race between thread dump and panic()
- BUG/MINOR: quic: reserve length field for long header encoding
- BUG/MINOR: quic: fix CRYPTO payload size calcul for encoding
- BUG/MINOR: ssl/cli: "show ssl crt-list" lacks client-sigals
- BUG/MINOR: ssl/cli: "show ssl crt-list" lacks sigals
- BUG/MINOR: cli: Wait for the last ACK when FDs are xferred from the old worker
- BUG/MEDIUM: filters: Handle filters registered on data with no payload callback
- BUG/MINOR: fcgi: Don't set the status to 302 if it is already set
- BUG/MINOR: quic: prevent crash on conn access after MUX init failure
- BUG/MINOR: mux-quic: prevent crash after MUX init failure
- BUG/MINOR: mux-h2: Properly handle full or truncated HTX messages on shut
- BUG/MINOR: tcp-rules: Don't forward close during tcp-response content rules eval
- BUG/MINOR: cli: Don't set SE flags from the cli applet
- BUG/MINOR: cli: Fix memory leak on error for _getsocks command
- BUG/MINOR: cli: Fix a possible infinite loop in _getsocks()
- BUG/MINOR: config/userlist: Support one 'users' option for 'group' directive
- BUG/MINOR: auth: Fix a leak on error path when parsing user's groups
- BUG/MINOR: flt-trace: Support only one name option
- BUG/MINOR: stats-json: Define JSON_INT_MAX as a signed integer
- BUG/MINOR: cfgparse: fix NULL ptr dereference in cfg_parse_peers
- BUG/MINOR: sink: add tempo between 2 connection attempts for sft servers
- MINOR: clock: always use atomic ops for global_now_ms
- BUG/MINOR: mux-h1: always make sure h1s->sd exists in h1_dump_h1s_info()
- MINOR: tinfo: add a new thread flag to indicate a call from a sig handler
- BUG/MEDIUM: stream: never allocate connection addresses from signal handler
- MINOR: freq_ctr: provide non-blocking read functions
- BUG/MEDIUM: stream: use non-blocking freq_ctr calls from the stream dumper
- BUG/MINOR: h2: always trim leading and trailing LWS in header values
- BUG/MINOR: h3: do not report transfer as aborted on preemptive response
- CLEANUP: h3: fix documentation of h3_rcv_buf()
- BUG/MINOR: server: check for either proxy-protocol v1 or v2 to send hedaer
- CLEANUP: log: removing "log-balance" references
- BUG/MINOR: log: set proper smp size for balance log-hash
- BUG/MEIDUM: startup: return to initial cwd only after check_config_validity()
- BUG/MINOR: cfgparse/peers: fix inconsistent check for missing peer server
- BUG/MINOR: cfgparse/peers: properly handle ignored local peer case
- BUG/MINOR: server: dont return immediately from parse_server() when skipping checks
- MINOR: cfgparse/peers: provide more info when ignoring invalid "peer" or "server" lines
- BUG/MINOR: stats: fix capabilities and hide settings for some generic metrics
- BUG/MINOR: namespace: handle a possible strdup() failure
- BUG/MINOR: ssl_crtlist: handle a possible strdup() failure
- BUG/MINOR: http-check: Don't pretend a C-L heeader is set before adding it
- BUG/MEDIUM: hlua/cli: fix cli applet UAF in hlua_applet_wakeup()
- BUG/MEDIUM: stream: don't use localtime in dumps from a signal handler
- MINOR: compiler: add a simple macro to concatenate resolved strings
- MINOR: compiler: add a new __decl_thread_var() macro to declare local variables
- MINOR: tools: resolve main() only once in resolve_sym_name()
- MINOR: tools: use only opportunistic symbols resolution
- BUILD: tools: silence a build warning when USE_THREAD=0
- MINOR: tinfo: split the signal handler report flags into 3
- MINOR: cli: export cli_io_handler() to ease symbol resolution
- MINOR: tools: improve symbol resolution without dl_addr
- MINOR: tools: ease the declaration of known symbols in resolve_sym_name()
- MINOR: tools: teach resolve_sym_name() a few more common symbols
- BUILD: tools: avoid a build warning on gcc-4.8 in resolve_sym_name()
Willy Tarreau [Fri, 14 Mar 2025 17:28:32 +0000 (18:28 +0100)]
BUILD: tools: avoid a build warning on gcc-4.8 in resolve_sym_name()
A build warning is emitted with gcc-4.8 in tools.c since commit
e920d73f59 ("MINOR: tools: improve symbol resolution without dl_addr")
because the compiler doesn't see that <size> is necessarily initialized.
Let's just preset it.
(cherry picked from commit
ed75148ca0479c7f87e1c149f0b5f0c8dd50eb3c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
19b041e30ad9be9f00588d83c7d36a405f1e9cca)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 13 Mar 2025 16:29:16 +0000 (17:29 +0100)]
MINOR: tools: teach resolve_sym_name() a few more common symbols
This adds run_poll_loop, run_tasks_from_lists, process_runnable_tasks,
ha_dump_backtrace and cli_io_handler which are fairly common in
backtraces. This will be less relative symbols when dladdr is not
usable.
(cherry picked from commit
4e09789644efa6c0dacfbd278618594590019a6e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
965df55f98ed70ec111fa5878eadc41938576c35)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 13 Mar 2025 15:46:10 +0000 (16:46 +0100)]
MINOR: tools: ease the declaration of known symbols in resolve_sym_name()
Let's have a macro that declares both the symbol and its name, it will
avoid the risk of introducing typos, and encourages adding more when
needed. The macro also takes an optional second argument to permit an
inline declaration of an extern symbol.
(cherry picked from commit
a3582a77f73a1c2ba84018ac361b4a763582ac44)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2a9991d23f5ba6d60c725fed3f0705c9b18bdd15)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 13 Mar 2025 16:21:24 +0000 (17:21 +0100)]
MINOR: tools: improve symbol resolution without dl_addr
When dl_addr is not usable or fails, better fall back to the closest
symbol among the known ones instead of providing everything relative
to main. Most often, the location of the function will give some hints
about what it can be. Thus now we can emit fct+0xXXX in addition to
main+0xXXX or main-0xXXX. We keep a margin of +256kB maximum after a
function for a match, which is around the maximum size met in an object
file, otherwise it becomes pointless again.
(cherry picked from commit
e920d73f598bd770a5dc861074268d1b2db4de59)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9df54d8c1b4de89df9b933d9f58c0c388fed4a5b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 13 Mar 2025 16:28:12 +0000 (17:28 +0100)]
MINOR: cli: export cli_io_handler() to ease symbol resolution
It's common to meet this function in backtraces, it's a bit annoying
that it's not resolved, so let's export it so that it becomes resolvable.
(cherry picked from commit
1e99efccef21394f0b5e5940001aaee8b3cd6b4b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a827c7ad60ae65cf4867cecace9bbdc1952dc4ba)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 24 Feb 2025 12:37:52 +0000 (13:37 +0100)]
MINOR: tinfo: split the signal handler report flags into 3
While signals are not recursive, one signal (e.g. wdt) may interrupt
another one (e.g. debug). The problem this causes is that when leaving
the inner handler, it removes the outer's flag, hence the protection
that comes with it. Let's just have 3 distinct flags for regular signals,
debug signal and watchdog signal. We add a 4th definition which is an
aggregate of the 3 to ease testing.
(cherry picked from commit
fb7874c286dbe1594837fc75b9e96783dfe160f5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
88b97519ab35e851888761960ac64dd10614ae86)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Mar 2025 17:11:14 +0000 (18:11 +0100)]
BUILD: tools: silence a build warning when USE_THREAD=0
The dladdr_lock that was added to avoid re-entering into dladdr is
conditioned by threads, but the way it's declared causes a build
warning if threads are disabled due to the insertion of a lone semi
colon in the variables block. Let's switch to __decl_thread_var()
for this.
This can be backported wherever commit
eb41d768f9 ("MINOR: tools:
use only opportunistic symbols resolution") is backported. It relies
on these previous two commits:
bb4addabb7 ("MINOR: compiler: add a simple macro to concatenate resolved strings")
69ac4cd315 ("MINOR: compiler: add a new __decl_thread_var() macro to declare local variables")
(cherry picked from commit
b61ed9babe879fd5603af4643c1d60c8ab48d096)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
55ee696ca143e4900249525c5a45141dc8994e1d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 21 Feb 2025 14:01:13 +0000 (15:01 +0100)]
MINOR: tools: use only opportunistic symbols resolution
As seen in issue #2861, dladdr_and_size() an be quite expensive and
will often hold a mutex in the underlying library. It becomes a real
problem when issuing lots of "show threads" or wdt warnings in parallel
because threads will queue up waiting for each other to finish, adding
to their existing latency that possibly caused the warning in the first
place.
Here we're taking a different approach. If the thread is not isolated
and not panicking, it's doing unimportant stuff like showing threads
or warnings. In this case we try to grab a lock, and if we fail because
another thread is already there, we just pretend we cannot resolve the
symbol. This is not critical because then we fall back to the already
used case which consists in writing "main+<offset>". In practice this
will almost never happen except in bad situations which could have
otherwise degenerated.
(cherry picked from commit
eb41d768f954d5c7360fd19ec69f9d707b900532)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9882abeada0eef928ebdb9f7377e13bafaceb564)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 21 Nov 2024 13:14:49 +0000 (14:14 +0100)]
MINOR: tools: resolve main() only once in resolve_sym_name()
resolv_sym_name() calls dladdr(main) for each symbol in order to compare
the first address with other symbols. But this is pointless and quite
expensive in outputs to "show profiling" for example. Let's just keep a
local copy and have a variable indicating if the resolution is needed/
in progress/done to save the value for subsequent calls.
(cherry picked from commit
a205a91bb3daa8a84fa6b429c13d39563add950f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Mar 2025 17:08:12 +0000 (18:08 +0100)]
MINOR: compiler: add a new __decl_thread_var() macro to declare local variables
__decl_thread() already exists but is more suited for struct members.
When using it in a variables block, it appends the final trailing
semi-colon which is a statement that ends the variable block. Better
clean this up and have one precisely for variable blocks. In this
case we can simply define an unused enum value that will consume the
semi-colon. That's what the new macro __decl_thread_var() does.
(cherry picked from commit
69ac4cd315f05c69f3c34f824ef4e7f3221966cf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f7f2c888b113a3789a4c794f3380c8696e3bbf31)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Mar 2025 17:06:55 +0000 (18:06 +0100)]
MINOR: compiler: add a simple macro to concatenate resolved strings
It's often useful to be able to concatenate strings after resolving
them (e.g. __FILE__, __LINE__ etc). Let's just have a CONCAT() macro
to do that, which calls _CONCAT() with the same arguments to make
sure the contents are resolved before being concatenated.
(cherry picked from commit
bb4addabb742f2305ad6667ed42ebb12f5df2af3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
af1e0b247e64678927793e9cd6e10907b0eebb27)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 24 Feb 2025 10:43:15 +0000 (11:43 +0100)]
BUG/MEDIUM: stream: don't use localtime in dumps from a signal handler
In issue #2861, Jarosaw Rzeszótko reported another issue with
"show threads", this time in relation with the conversion of a stream's
accept date to local time. Indeed, if the libc was interrupted in this
same function, it could have been interrupted with a lock held, then
it's no longer possible to dump the date, and we face a deadlock.
This is easy to reproduce with logging enabled.
Let's detect we come from a signal handler and do not try to resolve
the time to localtime in this case.
(cherry picked from commit
2e0bac90da0013513969224db94661dda88d7b98)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6cdac8a3512961fe6a62af27ac1a71d834daabe6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Aurelien DARRAGON [Wed, 19 Mar 2025 15:41:08 +0000 (16:41 +0100)]
BUG/MEDIUM: hlua/cli: fix cli applet UAF in hlua_applet_wakeup()
Recent commit
e5e36ce09 ("BUG/MEDIUM: hlua/cli: Fix lua CLI commands
to work with applet's buffers") revealed a bug in hlua cli applet handling
Indeed, playing with Willy's lua tetris script on the cli, a segfault
would be encountered when forcefully closing the session by sending a
CTRL+C on the terminal.
In fact the crash was caused by a UAF: while the cli applet was already
freed, the lua task responsible for waking it up would still point to it.
Thus hlua_applet_wakeup() could be called even if the applet didn't exist
anymore.
To fix the issue, in hlua_cli_io_release_fct() we must also free the hlua
task linked to the applet, like we already do for
hlua_applet_tcp_release() and hlua_applet_http_release().
While this bug exists on stable versions (where it should be backported
too for precaution), it only seems to be triggered starting with 3.0.
(cherry picked from commit
21601f4a27c4a1c8da0dbbfa22329ec1f927670e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
411e02e84eb36ca15292ad2337680dc986b57f34)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 3 Feb 2025 17:36:17 +0000 (18:36 +0100)]
BUG/MINOR: http-check: Don't pretend a C-L heeader is set before adding it
When a GET/HEAD/OPTIONS/DELETE healthcheck request was formatted, we claimed
there was a "content-length" header set even when there was no payload,
leading to actually send a "content-length: 0" header to the server. It was
unexpected and could be rejected by servers.
When a healthcheck request is sent we must take care to state there is a
"content-length" header when it is explicitly added.
This patch should fix the issue #2851. It must be backported as far as 2.9.
(cherry picked from commit
fad68cb16d7f99acd2b327ff2f8a4d9ab88a68d8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d5ca78b83d7bfddd107e3243a6ff785dc92b9157)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilia Shipitsin [Tue, 3 Dec 2024 16:13:05 +0000 (17:13 +0100)]
BUG/MINOR: ssl_crtlist: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
(cherry picked from commit
ce30bc17305e4ac2dec7d641eedcb301a237d863)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f13318e2d28bcbd0929ab9e494cc52803c5db7d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilia Shipitsin [Tue, 3 Dec 2024 16:10:21 +0000 (17:10 +0100)]
BUG/MINOR: namespace: handle a possible strdup() failure
This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to all supported branches.
(cherry picked from commit
abee5468509a77537015a7e5dc37268d4d564e03)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1a61a186ea032c676934dbe846655b78bd3be9d2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Aurelien DARRAGON [Thu, 13 Mar 2025 10:09:00 +0000 (11:09 +0100)]
BUG/MINOR: stats: fix capabilities and hide settings for some generic metrics
Performing a diff on stats output before vs after commit
66152526
("MEDIUM: stats: convert counters to new column definition") revealed
that some metrics were not properly ported to to the new API. Namely,
"lbtot", "cli_abrt" and "srv_abrt" are now exposed on frontend and
listeners while it was not the case before.
Also, "hrsp_other" is exposed even when "mode http" wasn't set on the
proxy.
In this patch we restore original behavior by fixing the capabilities
and hide settings.
As this could be considered as a minor regression (looking at the commit
message it doesn't seem intended), better tag this as a bug. It should be
backported in 3.0 with
66152526.
(cherry picked from commit
8311be5ac60c10fc4af56e3df79031358236bc14)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
5f4fe64ea7d250f0927e3f250515319b3588ae10)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Fri, 7 Mar 2025 08:30:47 +0000 (09:30 +0100)]
MINOR: cfgparse/peers: provide more info when ignoring invalid "peer" or "server" lines
Invalid (incomplete) "server" or "peer" lines under peers section are now
properly ignored. For completeness, in this patch we add some reports so
that the user knows that incomplete lines were ignored.
For an incomplete server line, since it is tolerated (see GH #565), we
only emit a diag warning.
For an incomplete peer line, we report a real warning, as it is not
expected to have a peer line without an address:port specified.
Also, 'newpeer == curpeers->local' check could be simplified since
we already have the 'local_peer' variable which tells us that the
parsed line refers to a local peer.
(cherry picked from commit
dbb25720dd7157e0f180d17486f10340f80a9fda)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
27a92cffb34d534c91e6b9deee886102f4b57b6c)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Fri, 7 Mar 2025 08:11:21 +0000 (09:11 +0100)]
BUG/MINOR: server: dont return immediately from parse_server() when skipping checks
If parse_server() is called under peers section parser, and the address
needs to be parsed but it is missing, we directly return from the function
However since
0fc136ce5b ("REORG: server: use parsing ctx for server
parsing"), parse_server() uses parsing ctx to emit warning/errors, and
the ctx must be reset before returning from the function, yet this early
return was overlooked. Because of that, any ha_{warning,alert..} message
reported after early return from parse_server() could cause messages to
have an extra "parsing [file:line]" info.
We fix that by ensuring parse_server() doesn't return without resetting
the parsing context.
It should be backported up to 2.6
(cherry picked from commit
a76b5358f0400568b641dc373cbd582875cd6aa6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d7e564cb81be444025f4fe72202a842718059da5)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Thu, 6 Mar 2025 08:29:05 +0000 (09:29 +0100)]
BUG/MINOR: cfgparse/peers: properly handle ignored local peer case
In
8ba10fea6 ("BUG/MINOR: peers: Incomplete peers sections should be
validated."), some checks were relaxed in parse_server(), and extra logic
was added in the peers section parser in an attempt to properly ignore
incomplete "server" or "peer" statement under peers section.
This was done in response to GH #565, the main intent was that haproxy
should already complain about incomplete peers section (ie: missing
localpeer).
However,
8ba10fea69 explicitly skipped the peer cleanup upon missing
srv association for local peers. This is wrong because later haproxy
code always assumes that peer->srv is valid. Indeed, we got reports
that the (invalid) config below would cause segmentation fault on
all stable versions:
global
localpeer 01JM0TEPAREK01FQQ439DDZXD8
peers my-table
peer 01JM0TEPAREK01FQQ439DDZXD8
listen dummy
bind localhost:8080
To fix the issue, instead of by-passing some cleanup for the local
peer, handle this case specifically by doing the regular peer cleanup
and reset some fields set on the curpeers and curpeers proxy because
of the invalid local peer (do as if the peer was not declared).
It should still comply with requirements from #565.
This patch should be backported to all stable versions.
(cherry picked from commit
054443dfb908521e30aa57335dbcb5f9cd6f7218)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6bd743efe66a2ce28b7bf125b7c145c30cb305f7)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Thu, 6 Mar 2025 08:05:23 +0000 (09:05 +0100)]
BUG/MINOR: cfgparse/peers: fix inconsistent check for missing peer server
In the "peers" section parser, right after parse_server() is called, we
used to check whether the curpeers->peers_fe->srv pointer was set or not
to know if parse_server() successfuly added a server to the peers proxy,
server that we can then associate to the new peer.
However the check is wrong, as curpeers->peers_fe->srv points to the
last added server, if a server was successfully added before the
failing one, we cannot detect that the last parse_server() didn't
add a server. This is known to cause bug with bad "peer"/"server"
statements.
To fix the issue, we save a pointer on the last known
curpeers->peers_fe->srv before parse_server() is called, and we then
compare the save with the pointer after parse_server(), if the value
didn't change, then parse_server() didn't add a server. This makes
the check consistent in all situations.
It should be backported to all stable versions.
(cherry picked from commit
2560ab892f355e958007b287946f787b578d3131)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
fb5130fc8f0d942acf2afd20a02f6d7b35eb5a35)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Valentine Krasnobaeva [Tue, 4 Mar 2025 10:04:01 +0000 (11:04 +0100)]
BUG/MEIDUM: startup: return to initial cwd only after check_config_validity()
In check_config_validity() we evaluate some sample fetch expressions
(log-format, server rules, etc). These expressions may use external files like
maps.
If some particular 'default-path' was set in the global section before, it's no
longer applied to resolve file pathes in check_config_validity(). parse_cfg()
at the end of config parsing switches back to the initial cwd.
This fixes the issue #2886.
This patch should be backported in all stable versions since 2.4.0, including
2.4.0.
(cherry picked from commit
e900ef987e4167cf0921e97b09059d757f2c0ea5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4d7fe11b70df072007eaa1633061e3581b9618fc)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Wed, 5 Mar 2025 11:01:34 +0000 (12:01 +0100)]
BUG/MINOR: log: set proper smp size for balance log-hash
result.data.u.str.size was set to size+1 to take into account terminating
NULL byte as per the comment. But this is wrong because the caller is free
to set size to just the right amount of bytes (without terminating NULL
byte). In fact all smp API functions will not read past str.data so there
is not risk about uninitialized reads, but this leaves an ambiguity for
converters that may use all the smp size to perform transformations, and
since we don't know about the "message" memory origin, we cannot assume
that its size may be greater than size. So we max it out to size just to
be safe.
This bug was not known to cause any issue, it was spotted during code
review. It should be backported in 2.9 with b30bd7a ("MEDIUM: log/balance:
support for the "hash" lb algorithm")
(cherry picked from commit
94a9b0f5deabd49020c8ff535a3404494345b399)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7e5d59e8bf5f44c57e2cd13427aef5288777fbd2)
[ad: ctx adjustement]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Wed, 5 Mar 2025 10:33:06 +0000 (11:33 +0100)]
CLEANUP: log: removing "log-balance" references
This is a complementary patch to
0e1f389fe9 ("DOC: config: removing
"log-balance" references"): we properly removed all log-balance
references in the doc but there remained some in the code, let's fix
that.
It could be backported in 2.9 with
0e1f389fe9
(cherry picked from commit
ddf66132f4d77e00c52c977d8a9e5c829965e7c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
24160712d38f54fe4114379ecbe034bcb25ee976)
[ad: ctx adjustment]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Mon, 3 Mar 2025 02:58:46 +0000 (03:58 +0100)]
BUG/MINOR: server: check for either proxy-protocol v1 or v2 to send hedaer
As reported in issue #2882, using "no-send-proxy-v2" on a server line does
not properly disable the use of proxy-protocol if it was enabled in a
default-server directive in combination with other PP options. The reason
for this is that the sending of a proxy header is determined by a test on
srv->pp_opts without any distinction, so disabling PPv2 while leaving other
options results in a PPv1 header to be sent.
Let's fix this by explicitly testing for the presence of either send-proxy
or send-proxy-v2 when deciding to send a proxy header.
This can be backported to all versions. Thanks to Andre Sencioles (@asenci)
for reporting the issue and testing the fix.
(cherry picked from commit
730641f7cad32bfff97875716efe4bd784bb006b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7b2212f5547ba4268a0de7657ec0b9ca18d40445)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Thu, 27 Feb 2025 10:28:07 +0000 (11:28 +0100)]
CLEANUP: h3: fix documentation of h3_rcv_buf()
Return value of h3_rcv_buf() is incorrectly documented. Indeed, it may
return a positive value to indicate that input bytes were converted into
HTX. This is especially important, as caller uses this value to consume
the reported data amount in QCS Rx buffer.
This should be backported up to 2.6. Note that on 2.8, h3_rcv_buf() was
named h3_decode_qcs().
(cherry picked from commit
0aa35289b3b51e09a5757c9991212ec416d281f2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
da6832fd2fb065cb32f030609b53fee87488aab7)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Thu, 27 Feb 2025 15:38:39 +0000 (16:38 +0100)]
BUG/MINOR: h3: do not report transfer as aborted on preemptive response
HTTP/3 specification allows a server to emit the entire response even if
only a partial request was received. In particular, this happens when
request STREAM FIN is delayed and transmitted in an empty payload frame.
In this case, qcc_abort_stream_read() was used by HTTP/3 layer to emit a
STOP_SENDING. Remaining received data were not transmitted to the stream
layer as they were simply discared. However, this prevents FIN
transmission to the stream layer. This causes the transfer to be
considered as prematurely closed, resulting in a cL-- log line status.
This is misleading to users which could interpret it as if the response
was not sent.
To fix this, disable STOP_SENDING emission on full preemptive reponse
emission. Rx channel is kept opened until the client closes it with
either a FIN or a RESET_STREAM. This ensures that the FIN signal can be
relayed to the stream layer, which allows the transfer to be reported as
completed.
This should be backported up to 2.9.
(cherry picked from commit
f6648d478b632bbd243ab374e24c02d566a4112b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
00177c6f78d37a7fdbf22d86f1eb7c7403035fbb)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Mon, 24 Feb 2025 08:17:22 +0000 (09:17 +0100)]
BUG/MINOR: h2: always trim leading and trailing LWS in header values
Annika Wickert reported some occasional disconnections between haproxy
and varnish when communicating over HTTP/2, with varnish complaining
about protocol errors while captures looked apparently normal. Nils
Goroll managed to reproduce this on varnish by injecting the capture of
the outgoing haproxy traffic and noticed that haproxy was forwarding a
header value containing a trailing space, which is now explicitly
forbidden since RFC9113.
It turns out that the only way for such a header to pass through haproxy
is to arrive in h2 and not be edited, in which case it will arrive in
HTX with its undesired spaces. Since the code dealing with HTX headers
always trims spaces around them, these are not observable in dumps, but
only when started in debug mode (-d). Conversions to/from h1 also drop
the spaces.
With this patch we trim LWS both on input and on output. This way we
always present clean headers in the whole stack, and even if some are
manually crafted by the configuration or Lua, they will be trimmed on
the output.
This must be backported to all stable versions.
Thanks to Annika for the helpful capture and Nils for the help with
the analysis on the varnish side!
(cherry picked from commit
bbf824933f71eca90b5f07a51fa93f4fa7ac2256)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
2c9bb6fe295f2ce34718b566593cdc67d5880a9f)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2025 17:23:23 +0000 (18:23 +0100)]
BUG/MEDIUM: stream: use non-blocking freq_ctr calls from the stream dumper
The stream dump function is called from signal handlers (warning, show
threads, panic). It makes use of read_freq_ctr() which might possibly
block if it tries to access a locked freq_ctr in the process of being
updated, e.g. by the current thread.
Here we're relying on the non-blocking API instead. It may return incorrect
values (typically smaller ones after resetting the curr counter) but at
least it will not block.
This needs to be backported to stable versions along with the previous
commit below:
MINOR: freq_ctr: provide non-blocking read functions
At least 3.1 is concerned as the warnings tend to increase the risk of
this situation appearing.
(cherry picked from commit
3c22fa315bcb2945d588cb64302f6ba5c89b382e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
124c6cdb13e0f7c03fb9232f429eb58f7d3c4742)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2025 17:21:56 +0000 (18:21 +0100)]
MINOR: freq_ctr: provide non-blocking read functions
Some code called by the debug handlers in the context of a signal handler
accesses to some freq_ctr and occasionally ends up on a locked one from
the same thread that is dumping it. Let's introduce a non-blocking version
that at least allows to return even if the value is in the process of being
updated, it's less problematic than hanging.
(cherry picked from commit
29e246a84ce27af63779d98b305ad53877ae9acc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dce43434eaf7a402148209c2f0d4371ae063c1fa)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2025 15:45:18 +0000 (16:45 +0100)]
BUG/MEDIUM: stream: never allocate connection addresses from signal handler
In __strm_dump_to_buffer(), we call conn_get_src()/conn_get_dst() to try
to retrieve the connection's IP addresses. But this function may be called
from a signal handler to dump a currently running stream, and if the
addresses were not allocated yet, a poll_alloc() will be performed while
we might possibly already be running pools code, resulting in pool list
corruption.
Let's just make sure we don't call these sensitive functions there when
called from a signal handler.
This must be backported at least to 3.1 and ideally all other versions,
along with this previous commit:
MINOR: tinfo: add a new thread flag to indicate a call from a sig handler
(cherry picked from commit
84d4c948fce4b31220bb9a30cbf676613bbbf4f2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
5f22fca932a1c3d5c92f30add31ffce81abdf758)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2025 15:26:24 +0000 (16:26 +0100)]
MINOR: tinfo: add a new thread flag to indicate a call from a sig handler
Signal handlers must absolutely not change anything, but some long and
complex call chains may look innocuous at first glance, yet result in
some subtle write accesses (e.g. pools) that can conflict with a running
thread being interrupted.
Let's add a new thread flag TH_FL_IN_SIG_HANDLER that is only set when
entering a signal handler and cleared when leaving them. Note, we're
speaking about real signal handlers (synchronous ones), not deferred
ones. This will allow some sensitive call places to act differently
when detecting such a condition, and possibly even to place a few new
BUG_ON().
(cherry picked from commit
ddd173355c9c7452ff6ec317c8be6195d25dba2a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
bc35f7763b413370f611c775e64acab469dead04)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2025 10:31:36 +0000 (11:31 +0100)]
BUG/MINOR: mux-h1: always make sure h1s->sd exists in h1_dump_h1s_info()
This function may be called from a signal handler during a warning,
a panic or a show thread. We need to be more cautious about what may
or may not be dereferenced since an h1s is not necessarily fully
initialized. Loops of "show threads" sometimes manage to crash when
dereferencing a null h1s->sd, so let's guard it and add a comment
remining about the unusual call place.
This can be backported to the relevant versions.
(cherry picked from commit
a56dfbdcb4cb3eb9ffd3db641efb3e5605a6c3f0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0ce82061f72bf7f0c1341cdca2f9e90f03437aa4)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Wed, 19 Feb 2025 10:42:04 +0000 (11:42 +0100)]
MINOR: clock: always use atomic ops for global_now_ms
global_now_ms is shared between threads so we must give hint to the
compiler that read/writes operations should be performed atomically.
Everywhere global_now_ms was used, atomic ops were used, except in
clock_update_global_date() where a read was performed without using
atomic op. In practise it is not an issue because on most systems
such reads should be atomic already, but to prevent any confusion or
potential bug on exotic systems, let's use an explicit _HA_ATOMIC_LOAD
there.
This may be backported up to 2.8
(cherry picked from commit
97a19517ffe3438562f80c314f5b6f3f27df7668)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
e5057a90eb505d3a8f98d655bf669e6943a62861)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Aurelien DARRAGON [Fri, 21 Feb 2025 09:51:01 +0000 (10:51 +0100)]
BUG/MINOR: sink: add tempo between 2 connection attempts for sft servers
When the connection for sink_forward_{oc}_applet fails or a previous one
is destroyed, the sft->appctx is instantly released.
However process_sink_forward_task(), which may run at any time, iterates
over all known sfts and tries to create sessions for orphan ones.
It means that instantly after sft->appctx is destroyed, a new one will
be created, thus a new connection attempt will be made.
It can be an issue with tcp log-servers or sink servers, because if the
server is unavailable, process_sink_forward() will keep looping without
any temporisation until the applet survives (ie: connection succeeds),
which results in unexpected CPU usage on the threads responsible for
that task.
Instead, we add a tempo logic so that a delay of 1second is applied
between two retries. Of course the initial attempt is not delayed.
This could be backported to all stable versions.
(cherry picked from commit
9561b9fb6964af325a10e7128b563114f144a3cb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0164f13cb144fff527b970a6f19175ccd627980c)
[ad: ctx adjustement due to missing commit
09d69eac]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Valentine Krasnobaeva [Thu, 20 Feb 2025 14:00:38 +0000 (15:00 +0100)]
BUG/MINOR: cfgparse: fix NULL ptr dereference in cfg_parse_peers
When "peers" keyword is followed by more than one argument and it's the first
"peers" section in the config, cfg_parse_peers() detects it and exits with
"ERR_ALERT|ERR_FATAL" err_code.
So, upper layer parser, parse_cfg(), continues and parses the next keyword
"peer" and then he tries to check the global cfg_peers, which should contain
"my_cluster". The global cfg_peers is still NULL, because after alerting a user
in alertif_too_many_args, cfg_parse_peers() exited.
peers my_cluster __some_wrong_data__
peer haproxy1 1.1.1.1 1000
In order to fix this, let's add ERR_ABORT, if "peers" keyword is followed by
more than one argument. Like this parse_cfg() will stops immediately and
terminates haproxy with "too many args for peers my_cluster..." alert message.
It's more reliable, than add checks "if (cfg_peers !=NULL)" in "peer"
subparser, as we may have many "peers" sections.
peers my_another_cluster
peer haproxy1 1.1.1.2 1000
peers my_cluster __some_wrong_data__
peer haproxy1 1.1.1.1 1000
In addition, for the example above, parse_cfg() will parse all configuration
until the end and only then terminates haproxy with the alert
"too many args...". Peer haproxy1 will be wrongly associated with
my_another_cluster.
This fixes the issue #2872.
This should be backported in all stable versions.
(cherry picked from commit
390df282c1ac4605273abfeb82c97fad205b7294)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f595db184bcdc7e8d3063f2f13fe41725ff11971)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 16:13:50 +0000 (17:13 +0100)]
BUG/MINOR: stats-json: Define JSON_INT_MAX as a signed integer
A JSON integer is defined in the range [-(2**53)+1, (2**53)-1]. Macro are used
to define the minimum and the maximum value, The minimum one is defined using
the maximum one. So JSON_INT_MAX must be defined as a signed integer value to
avoid wrong cast of JSON_INT_MIN.
It was reported by Coverity in #2841: CID 1587769.
This patch could be backported to all stable versions.
(cherry picked from commit
d48b5add889db1bf2f0fae4721abb46413303d33)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d0c16f3353c5e673abb6967a714f1ca22797dbff)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 16:01:08 +0000 (17:01 +0100)]
BUG/MINOR: flt-trace: Support only one name option
When a trace filter is defined, only one 'name' option is expected. But it
was not tested. Thus it was possible to set several names leading to a
memory leak.
It is now tested, and it is not allowed to redefine the trace filter name.
It was reported by Coverity in #2841: CID 1587768.
This patch could be backported to all stable versions.
(cherry picked from commit
b20e2c96cfea06776db3200614f6d94441f15c2d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e3e565a556ce960ac47f4495687a0bbda38ae160)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 15:52:17 +0000 (16:52 +0100)]
BUG/MINOR: auth: Fix a leak on error path when parsing user's groups
In a userlist section, when a user is parsed, if a specified group is not
found, an error is reported. In this case we must take care to release the
alredy built groups list.
It was reported by Coverity in #2841: CID 1587770.
This patch could be backported to all stable versions.
(cherry picked from commit
a7f513af9187444680e3e054e0a13f818aed3307)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
8074bb26d1bbcb225100329ada2498112222e512)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 15:21:20 +0000 (16:21 +0100)]
BUG/MINOR: config/userlist: Support one 'users' option for 'group' directive
When a group is defined in a userlist section, only one 'users' option is
expected. But it was not tested. Thus it was possible to set several options
leading to a memory leak.
It is now tested, and it is not allowed to redefine the users option.
It was reported by Coverity in #2841: CID 1587771.
This patch could be backported to all stable versions.
(cherry picked from commit
a1e14d2a8272511c29d9225a61c74dc89847287d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4c9c15e3fcf489b5fb609c6f0f1c9e06047c7cef)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 14:37:52 +0000 (15:37 +0100)]
BUG/MINOR: cli: Fix a possible infinite loop in _getsocks()
In _getsocks() functuoin, when we failed to set the unix socket in
non-blocking mode, a goto to "out" label led to loop infinitly. To fix the
issue, we must only let the function exit.
This patch should be backported to all stable versions.
(cherry picked from commit
75e8c8ed330f2ad1b6b33630efebb5041af3d5e9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
48022892602a61ed695aad552a0dcda4457308c2)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 14:30:30 +0000 (15:30 +0100)]
BUG/MINOR: cli: Fix memory leak on error for _getsocks command
Some errors in parse function of _getsocks commands were not properly handled
and immediately returned, leading to a memory leak on cmsgbuf and tmpbuf
buffers.
To fix the issue, instead of immediately return with -1, we jump to "out"
label. Returning 1 intead of -1 in that case is valid.
This was reported by Coverity in #2841: CIDs 1587773 and 1587772.
This patch should be backported as far as 2.4.
(cherry picked from commit
372cc696d44e6853b9f7920f1c2d965736029764)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e9e1fbb7c858735e1376b069a8db7ca3b98f58ce)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Thu, 6 Feb 2025 14:15:27 +0000 (15:15 +0100)]
BUG/MINOR: cli: Don't set SE flags from the cli applet
Since the CLI was updated to use the new applet API, it should no longer set
directly the SE flags. Instead, the corresponding applet flags must be set,
using the applet API (appet_set_*). It is true for the CLI I/O handler but also
for the commands parse function and I/O callback function.
This patch should be backported as far as 3.0.
(cherry picked from commit
7e927243b9e86cee13ea9c16077a8523e79e41bb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
80542e814a56a446fe0d06bc64dd0ab7625e4762)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Mon, 3 Feb 2025 14:31:57 +0000 (15:31 +0100)]
BUG/MINOR: tcp-rules: Don't forward close during tcp-response content rules eval
When the tcp-response content ruleset evaluation is delayed because of an
ACL condition, the close forwarding on the client side is not explicitly
blocked. So it is possible to close the client side before the end of the
response evaluation.
To fix the issue, this is now done in all cases where some data are
missing. Concretely, channel_dont_close() is called in "missing_data" goto
label.
Note it is only a theorical bug (or pending bug). It is not possible to
trigger it for now because an ACL cannot wait for more data when a close was
received. But the code remains a bit weak. It is safer this way. It is
especially mandatory for the "force yield" option that should be added soon.
This patch could be backported to all stable versions.
(cherry picked from commit
04bbfa4354800b893f03294bd02475a16d90ec85)
[wt: trivial ctx adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ba7be506130d5b6554f6db1fead55dde97bc14dd)
[ad: context issue due to undefined stream waiting_entity member]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Mon, 17 Feb 2025 17:48:17 +0000 (18:48 +0100)]
BUG/MINOR: mux-h2: Properly handle full or truncated HTX messages on shut
On shut, truncated HTX messages were not properly handled by the H2
multiplexer. Depending on how data were emitted, a chunked HTX message
without the 0-CRLF could be considered as full and an empty data with ES
flag set could be emitted instead of a RST_STREAM(CANCEL) frame.
In the H2 multiplexer, when a shut is performed, an HTX message is
considered as truncated if more HTX data are still expected. It is based on
the presence or not of the H2_SF_MORE_HTX_DATA flag on the H2 stream.
However, this flag is set or unset depending on the HTX extra field
value. This field is used to state how much data that must still be
transferred, based on the announced data length. For a message with a
content-length, this assumption is valid. But for a chunked message, it is
not true. Only the length of the current chunk is announced. So we cannot
rely on this field in that case to know if a message is full or not.
Instead, we must rely on the HTX start-line flags to know if more HTX data
are expected or not. If the xfer length is known (the HTX_SL_F_XFER_LEN flag
is set on the HTX start-line), it means that more data are always expected,
until the end of message is reached (the HTX_FL_EOM flag is set on the HTX
message). This is true for bodyless message because the end of message is
reported with the end of headers. This is also true for tunneled messages
because the end of message is received before switching the H2 stream in
tunnel mode.
This patch must be backported as far as 2.8.
(cherry picked from commit
b70921f2c19a0612a4f1e31ef16cf8c8b216fdf9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
5341a464d733e7a562f5de3b94343ef027f8f079)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Mon, 17 Feb 2025 09:54:41 +0000 (10:54 +0100)]
BUG/MINOR: mux-quic: prevent crash after MUX init failure
qmux_init() may fail for several reasons. In this case, connection
resources are freed and underlying and a CONNECTION_CLOSE will be
emitted via its quic_conn instance.
In case of qmux_init() failure, qcc_release() is used to clean up
resources, but QCC <conn> member is first resetted to NULL, as
connection released must be delayed. Some cleanup operations are thus
skipped, one of them is the resetting of <ctx> connection member to
NULL. This may cause a crash as <ctx> is a dangling pointer after QCC
release. One of the possible reproducer is to activate QMUX traces,
which will cause a segfault on the qmux_init() error leave trace.
To fix this, simply reset <ctx> to NULL manually on qmux_init() failure.
This must be backported up to 3.0.
(cherry picked from commit
2715dbe9d065d8700a8fba6e2605a451cfbb72b8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e02d842f091e79dd58a13c0c12c7d72affc131cd)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Mon, 17 Feb 2025 16:15:49 +0000 (17:15 +0100)]
BUG/MINOR: quic: prevent crash on conn access after MUX init failure
Initially, QUIC-MUX was responsible to reset quic_conn <conn> member to
NULL when MUX was released. This was performed via qcc_release().
However, qcc_release() is also used on qmux_init() failure. In this
case, connection must be freed via its session, so QCC <conn> member is
resetted to NULL prior to qcc_release(), which prevents quic_conn <conn>
member to also be resetted. As the connection is freed soon after,
quic_conn <conn> is a dangling pointer, which may cause crashes.
This bug should be very rare as first it implies that QUIC-MUX
initialization has failed (for example due to a memory alloc error).
Also, <conn> member is rarely used by quic_conn instance. In fact, the
only reproducible crash was done with QUIC traces activated, as in this
case connection is accessed via quic_conn under __trace_enabled()
function.
To fix this, detach connection from quic_conn via the XPRT layer instead
of the MUX. More precisely, this is performed via quic_close(). This
should ensure that it will always be conducted, either on normal
connection closure, but also after special conditions such as MUX init
failure.
This should be backported up to 2.6.
(cherry picked from commit
2cdc4695cb82fce46d67cef17300ec7cf978906e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
82e7d79e727b148afae59592931fd0191c5eb1c5)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Mon, 17 Feb 2025 15:37:47 +0000 (16:37 +0100)]
BUG/MINOR: fcgi: Don't set the status to 302 if it is already set
When a "Location" header was found in a FCGI response, the status code was
forced to 302. But it should only be performed if no status code was set
first.
So now, we take care to not override an already defined status code when the
"Location" header is found.
This patch should fix the issue #2865. It must backported to all stable
versions.
(cherry picked from commit
ca79ed5eefaa65fc82e1a8c1ec4308eaaadaebd1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
870bbce00c8fc1be660499a943f81dd26530a82b)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Mon, 17 Feb 2025 14:54:49 +0000 (15:54 +0100)]
BUG/MEDIUM: filters: Handle filters registered on data with no payload callback
An HTTP filter with no http_payload callback function may be registered on
data. In that case, this filter is obviously not called when some data are
received but it remains important to update its internal state to be sure to
keep it synchronized on the stream, especially its offet value. Otherwise,
the wrong calculation on the global offset may be performed in
flt_http_end(), leading to an integer overflow when data are moved from
input to output. This overflow triggers a BUG_ON() in c_adv().
The same is true for TCP filters with no tcp_payload callback function.
This patch must be backport to all stable versions.
(cherry picked from commit
34542d5ec29c89ec45b63107f9330f185f0bfd40)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6633abae2a17f969754504f2aca37bbfa12f3ca4)
[ad: remove reference to non existant members in 3.0]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Christopher Faulet [Mon, 17 Feb 2025 14:16:15 +0000 (15:16 +0100)]
BUG/MINOR: cli: Wait for the last ACK when FDs are xferred from the old worker
On reload, the new worker requests bound FDs to the old one. The old worker
sends them in message of at most 252 FDs. Each message is acknowledged by
the new worker. All messages sent or received by the old worker are handled
manually via sendmsg/recv syscalls. So the old worker must be sure consume
all the ACK replies. However, the last one was never consumed. So it was
considered as a command by the CLI applet. This issue was hidden since
recently. But it was the root cause of the issue #2862.
Note this last ack is also the first one when there are less than 252 FDs to
transfer.
This patch must be backported to all stable versions.
(cherry picked from commit
49b7bcf583261efedabad5ba15c4026f2e713c61)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4a4e00637418f5342e6cbae8898cb1907b442e87)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
William Lallemand [Wed, 12 Feb 2025 16:13:03 +0000 (17:13 +0100)]
BUG/MINOR: ssl/cli: "show ssl crt-list" lacks sigals
1d3c8223 ("MINOR: ssl: allow to change the server signature algorithm")
mplemented the sigals keyword in the crt-list but never the dump of the
keyword over the CLI.
Must be backported as far as 2.8.
(cherry picked from commit
5a7cbb8d8115770c4776836fdf727b26d490d2ad)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7b8ec84c40f08eb25840929f014dc47cde3c099e)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
William Lallemand [Wed, 12 Feb 2025 16:09:21 +0000 (17:09 +0100)]
BUG/MINOR: ssl/cli: "show ssl crt-list" lacks client-sigals
b6ae2aafde43 ("MINOR: ssl: allow to change the signature algorithm for
client authentication") implemented the client-sigals keyword in the
crt-list but never the dump of the keyword over the CLI.
Must be backported as far as 2.8.
(cherry picked from commit
037d2e5498917d2323a9ad748b9d97aaa688f351)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d114a5cf61696bf0f3dde91bdf5b9cc1f80c6256)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Tue, 11 Feb 2025 13:35:52 +0000 (14:35 +0100)]
BUG/MINOR: quic: fix CRYPTO payload size calcul for encoding
Function max_stream_data_size() is used to determine the payload length
of a CRYPTO frame. It takes into account that the CRYPTO length field is
a variable length integer.
Implemented calcul was incorrect as it reserved too much space as a
frame header. This error is mostly due because max_stream_data_size()
reuses max_available_room() which also reserve space for a variable
length integer. This results in CRYPTO frames shorter of 1 to 2 bytes
than the maximum achievable value, which produces in the end datagram
shorter than the MTU.
Fix max_stream_data_size() implementation. It is now merely a wrapper on
max_available_room(). This ensures that CRYPTO frame encoding is now
properly optimized to use the MTU available.
This should be backported up to 2.6.
(cherry picked from commit
e6a223542ae87880b3e8261daffbe362730f9e55)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
7cdb2bb00af5a44bab775f2a565cfb27682451f9)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Amaury Denoyelle [Tue, 11 Feb 2025 13:34:57 +0000 (14:34 +0100)]
BUG/MINOR: quic: reserve length field for long header encoding
Long header packets have a mandatory Length field, which contains the
size of Packet number and payload, encoded as a variable-length integer.
Its value can thus only be determined after the payload size is known,
which depends on the remaining buffer space after this variable-length
field.
Packet payload are encoded in two steps. First, a list of input frames
is processed until the packet buffer is full. CRYPTO and STREAM frames
payload can be splitted if need to fill the buffer. Real encoding is
then performed as a second stage operation, first with Length field,
then with the selected frames themselves.
Before this patch, no space was reserved in the buffer for Length field
when attaching the frames to the packet. This could result in a error as
the packet payload would be too large for the remaining space.
In practice, this issue was rarely encounted, mostly as a side-effect
from another issue linked to CRYPTO frame encoding. Indeed, a wrong
calculation is performed on CRYPTO splitting, which results in frame
payload shorter by a few bytes than expected. This however ensured there
would be always enough room for the Length field and payload during
encoding. As CRYPTO frames are the only big enough content emitted with
a Long header packet, this renders the current issue mostly non
reproducible.
Fix the original issue by reserving some space for Length field prior to
frame payload calculation, using a maximum value based on the remaining
room space. Packet length is then reduced if needed when encoding is
performed, which ensures there is always enough room for the selected
frames.
Note that the other issue impacting CRYPTO frame encoding is not yet
fixed. This could result in datagrams with Long header packets not
completely extended to the full MTU. The issue will be addressed in
another patch.
This should be backported up to 2.6.
(cherry picked from commit
63747452a39cff72a6bc7610da079afe01fb6dcc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3906b5a59c27ab58f1e8a6ca79c21565a400c2fd)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Willy Tarreau [Mon, 10 Feb 2025 16:59:40 +0000 (17:59 +0100)]
BUG/MEDIUM: debug: close a possible race between thread dump and panic()
The rework of the thread dumping mechanism in 2.8 with commit
9a6ecbd590
("MEDIUM: debug: simplify the thread dump mechanism") opened a small
race, which is that a thread in the process of dumping other ones may
block the other one from panicing while it's looping at the end of
ha_thread_dump_fill(), or any other sequence involving the currently
dumped one.
This was emphasized in 3.1 with commit
148eb5875f ("DEBUG: wdt: better
detect apparently locked up threads and warn about them") that allowed
to emit warnings about long-stuck threads, because in this case, what
happens is that sometimes a thread starts to emit a warning (or a set
of warnings), and while the warning is being awaited for, a panic
finally happens and interrupts either the dumping thread, which never
finishes and waits for the target's pointer to become NULL which will
never happen since it was supposed to do it itself, or the currently
dumped thread which could wait for the dumping thread to become ready
while this one has not released the former.
In order to address this, first we now make sure never to dump a thread
that is already in the process of dumping another one. We're adding a
new thread flag to know this situation, that is set in ha_thread_dump_fill()
and cleared in ha_thread_dump_done(). And similarly, we don't trigger
the watchdog on a thread waiting for another one to finish its dump,
as it's likely a case of warning (and maybe even a panic) that makes
them wait for each other and we don't want such cases to be reentrant.
Finally, we check in the main polling loop that the flag never accidentally
leaked (e.g. wrong flag manipulation) as this would be difficult to spot
with bad consequences.
This should be backported at least to 2.8, and should resolve github
issue #2860. Thanks to Chris Staite for the very informative backtrace
that exhibited the problem.
(cherry picked from commit
7ddcdff33fbe62c1f0a342c64152833897c7647e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4c080c2e83dd81c292da06aa42b07dfbc25ad166)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
William Lallemand [Tue, 28 Jan 2025 19:55:20 +0000 (20:55 +0100)]
BUILD: ssl: more cleaner approach to WolfSSL without renegotiation
Patch discussed in https://github.com/wolfSSL/wolfssl/issues/6834
When building Wolfssl without renegotiation options, WolfSSL still
defines the macros about it, which warns during the build.
This patch completes the previous one by undefining the macros so
haproxy could build without any warning.
(cherry picked from commit
b43e5d8c1692a0f15db4e621e3cff41158a47167)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
5fde007890b75ecfcb70fb77f8644dd68cdcca1b)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
William Lallemand [Tue, 28 Jan 2025 17:27:31 +0000 (18:27 +0100)]
BUILD: ssl: allow to build without the renegotiation API of WolfSSL
In ticket https://github.com/wolfSSL/wolfssl/issues/6834, it was
suggested to push --enable-haproxy within --enable-distro.
WolfSSL does not want to include the renegotiation support in
--enable-distro.
To achieve this, let haproxy build without SSL_renegotiate_pending()
when wolfssl does not define HAVE_SECURE_RENEGOCIATION or
HAVE_SERVER_RENEGOCIATION_INFO.
(cherry picked from commit
c6a8279cdfc3272e34feb256ed9e4601e0a104db)
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
(cherry picked from commit
76cb3e6a855d014ff6a344d81bc52c0df6acdff3)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
Dragan Dosen [Wed, 26 Feb 2025 21:56:41 +0000 (22:56 +0100)]
BUG/MINOR: server: fix the "server-template" prefix memory leak
The srv->tmpl_info.prefix was not freed in srv_free_params().
This could be backported to all stable versions.
(cherry picked from commit
0ae7a5d672f61cd4a949bf081b61857f6bbad476)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
5cc252ce6313d1f323783865c788b1e87a822e3e)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
Aurelien DARRAGON [Thu, 13 Mar 2025 09:51:57 +0000 (10:51 +0100)]
DOC: management: rename some last occurences from domain "dns" to "resolvers"
This is a complementary patch to
cf913c2f9 ("DOC: management: rename show
stats domain cli "dns" to "resolvers"). The doc still refered to the
legacy "dns" domain filter for stat command. Let's rename those occurences
to "resolvers".
It may be backported to all stable versions.
(cherry picked from commit
4c3eb60e7019d12734501ceb9358d2714bce8922)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
b794eb91aa57be36c90439a7f1574a757ad84e9d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Mar 2025 14:54:36 +0000 (15:54 +0100)]
BUG/MEDIUM: thread: use pthread_self() not ha_pthread[tid] in set_affinity
A bug was uncovered by the work on NUMA. It only triggers in the CI
with libmusl due to a race condition. What happens is that the call
to set_thread_cpu_affinity() is done very early in the polling loop,
and that it relies on ha_pthread[tid] instead of pthread_self(). The
problem is that ha_pthread[tid] is only set by the return from
pthread_create(), which might happen later depending on the number of
CPUs available to run the starting thread.
Let's just use pthread_self() here. ha_pthread[] is only used to send
signals between threads, there's no point in using it here.
This can be backported to 2.6.
(cherry picked from commit
12383fd9f5b3614dbffec6260b82659c3c5fd0df)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
77494a9104e6d3ad5d8ca24f2fb343ab2e4b1c13)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Valentine Krasnobaeva [Fri, 7 Mar 2025 12:42:27 +0000 (13:42 +0100)]
MINOR: startup: adjust alert messages, when capabilities are missed
CAP_SYS_ADMIN support was added, in order to access sockets in namespaces. So
let's adjust the alert at startup, where we check preserved capabilities from
global.last_checks. Let's mention here cap_sys_admin as well.
(cherry picked from commit
7d427134fe01e8af56dfa48c6d9e6ecc5defe562)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
77b340e17d3da9fc78ed113ebe98b2101c10994c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Damien Claisse [Fri, 20 Dec 2024 13:36:34 +0000 (13:36 +0000)]
BUG/MINOR: cfgparse-tcp: relax namespace bind check
Commit 5cbb278 introduced cap_sys_admin support, and enforced checks for
both binds and servers. However, when binding into a namespace, the bind
is done before dropping privileges. Hence, checking that we have
cap_sys_admin capability set in this case is not needed (and it would
decrease security to add it).
For users starting haproxy with other user than root and without
cap_sys_admin, bind should have already failed.
As a consequence, relax runtime check for binds into a namespace.
(cherry picked from commit
f0a07f834c001c5b505e84b0f0b103e530e87d1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
42086a672888f569e009149130fad7b19b3fd13d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 6 Mar 2025 17:56:13 +0000 (18:56 +0100)]
BUG/MINOR: stream: fix age calculation in "show sess" output
The "show sess" output reports an age that's based on the last byte of
the HTTP request instead of the stream creation date, due to a confusion
between logs->request_ts and the request_date sample fetch function. Most
of the time these are equal except when the request is not yet full for
any reason (e.g. wait-body). This explains why a few "show sess" could
report a few new streams aged by 99 days for example.
Let's perform the correct request timestamp calculation like the sample
fetch function does, by adding t_idle and t_handshake to the accept_ts.
Now the stream's age is correct and can be correctly used with the
"show sess older <age>" variant.
This issue was introduced in 2.9 and the fix can be backported to 3.0.
(cherry picked from commit
1cdf2869f6757946546a2ef102ce822e95de78f8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
17085371afad2728713a3cc409ce37f2137b27d8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 4 Mar 2025 10:44:03 +0000 (11:44 +0100)]
CI: github: fix h2spec.config proxy names
h2spec.config config file emitted a warning because the frontend name
has the same name as the backend.
(cherry picked from commit
588237ca6e6624f7c1162289a6a00cab3f10ac61)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9368b370eace1e65497b79d0df05a405f25156ea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 4 Mar 2025 09:47:08 +0000 (10:47 +0100)]
TESTS: ist: fix wrong array size
test_istzero() and test_istpad() has the wrong array size buf[] which
lacks the space for the '\0';
Could be backported in every stable branches.
(cherry picked from commit
ddd2c82a3521ad61398d24fa10f7483cd6518de8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
df98c512bc927cd5c6a061a93f11bb01aceb265e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Dragan Dosen [Wed, 26 Feb 2025 18:13:31 +0000 (19:13 +0100)]
BUG/MEDIUM: server: properly initialize PROXY v2 TLVs
The PROXY v2 TLVs were not properly initialized when defined with
"set-proxy-v2-tlv-fmt" keyword, which could have caused a crash when
validating the configuration or malfunction (e.g. when used in
combination with "server-template" and/or "default-server").
The issue was introduced with commit
6f4bfed3a ("MINOR: server: Add
parser support for set-proxy-v2-tlv-fmt").
This should be backported up to 2.9.
(cherry picked from commit
6838fe43a320cf090892451ee907967666b626e5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
77b48b2ae0dba907115c304ea78187a0f77e60c0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Lukas Tribus [Wed, 5 Feb 2025 07:42:15 +0000 (07:42 +0000)]
DOC: option redispatch should mention persist options
"option redispatch" remains vague in which cases a session would persist;
let's mention "option persist" and "force-persist" as an example so folks
don't draw the conclusion that this may be default.
Should be backported to stable branches.
(cherry picked from commit
5926fb78233ab4ba95b07db3815ee62f5cbc5082)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6d1a374721b2813a2ad61bf0e88e0e58a2a0c8b0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 31 Jan 2025 14:23:47 +0000 (15:23 +0100)]
DOC: htx: clarify <mark> parameter for htx_xfer_blks()
Clarify the fact that the first <mark> block is transferred before
stopping when using htx_xfer_blks()
(cherry picked from commit
c17e02923256fef61f74b986045b7eb8d1e126c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
338edaa1e9aeb0ee87a58fc9d62e75548f11566f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 31 Jan 2025 13:41:28 +0000 (14:41 +0100)]
BUG/MEDIUM: htx: wrong count computation in htx_xfer_blks()
When transfering blocks from an src to another dst htx representation,
htx_xfer_blks() decreases the size of each block removed from the <count>
value passed in parameter, so it can't transfer more than <count>. The
size must also contains the metadata, represented by a simple
sizeof(struct htk_blk).
However, the code was doing a sizeof(dstblk) instead of a
sizeof(*dstblk) which as the consequence of removing only a size_t from
count. Fortunately htx_blk size is 64bits, so that does not provoke any
problem in 64bits. But on 32bits architecture, the count value is not
decreased correctly and the function could try to transfer more blocks
than allowed by the count parameter.
Must be backported in every stable release.
(cherry picked from commit
c6390cdf9ce4e74540544207d6e3cfb31581eaea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3c47adcfdfbf1ba9231059773865df036ced8bea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 18 Feb 2025 06:31:51 +0000 (07:31 +0100)]
REGTESTS: Fix truncated.vtc to send 0-CRLF
When a chunked messages is sent, the 0-CRLF must be explicitely sent. Since
the begining, it is missing. Just add it.
(cherry picked from commit
b93e419750aa8921e76e1e58c57a9bf8eefea969)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e08ed2d43e2871f9e9d696d35f61c6352703a4b6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Feb 2025 15:25:13 +0000 (16:25 +0100)]
BUG/MEDIUM: fd: mark FD transferred to another process as FD_CLONED
The crappy epoll API stroke again with reloads and transferred FDs.
Indeed, when listening sockets are retrieved by a new worker from a
previous one, and the old one finally stops listening on them, it
closes the FDs. But in this case, since the sockets themselves were
not closed, epoll will not unregister them and will continue to
report new activity for these in the old process, which can only
observe, count an fd_poll_drop event and not unregister them since
they're not reachable anymore.
The unfortunate effect is that long-lasting old processes are woken
up at the same rate as the new process when accepting new connections,
and can waste a lot of CPU. Accept rates divided by 8 were observed on
a small test involving a slow transfer on 10 connections facing a reload
every second so that 10 processes were busy dealing with them while
another process was hammering the service with new connections.
Fortunately, years ago we implemented a flag FD_CLONED exactly for
similar purposes. Let's simply mark transferred FDs with FD_CLONED
so that the process knows that these ones require special treatment
and have to be manually unregistered before being closed. This does
the job fine, now old processes correctly unregister the FD before
closing it and no longer receive accept events for the new process.
This needs to be backported to all stable versions. It only affects
epoll, as usual, and this time in combination with transferred FDs
(typically reloads in master-worker mode). Thanks to Damien Claisse
for providing all detailed measurements and statistics allowing to
understand and reproduce the problem.
(cherry picked from commit
561319bd1ccaab5dd69bac0c19387670b02d484f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
12b7be07d9437639ee503873647cbf85d635dad4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 19 Feb 2025 10:39:59 +0000 (11:39 +0100)]
BUG/MEDIUM: spoe: Don't wakeup idle applets in loop during stopping
When HAproxy is stopping, idle applets are systematically woken up, to have a
change to close them. This was performed to properly handle the applets switched
to idle state after processing in synchronous mode. However, there was an issue
in async and pipelining modes. Applets remained idles while they had some free
slots to process messages. So these applets were not closed immediately and were
woken up in loop.
Now, we also take care to check that idle applets are not processing messages to
wake them up. The test is also moved after a processing attempt because it is
the right place to do so. There is no reason to perform this test systematically
at the end of the I/O handler.
This patch should be backported to all stable versions.
Christopher Faulet [Wed, 19 Feb 2025 10:33:02 +0000 (11:33 +0100)]
BUG/MINOR: spoe: Allow applet creation when closing the last one during stopping
When the last SPOE applet is closed, for any reason, if there are still some
pending messages, in the shared sending queueu or the shared waiting queue, a
new applet is automatically created to be able to pocess these messages. Howver,
this was not performed when HAProxy was stopping. But it was an error because it
was then possible to abort some messages and reported them on error with the
status code set to 256.
So, now, it is still possible to create applet, if necessary, when HAProxy is
stopping.
This patch should partially fix the issue #2868. It must be backported to all
stable versions.
Christopher Faulet [Wed, 19 Feb 2025 09:17:30 +0000 (10:17 +0100)]
BUG/MINOR: spoe: Check the shared waiting queue to shut applets during stopping
When HAProxy is stopped, we should take care to process all pending messages
and wait for ACK for already sent messages. However, the shared waiting
queue, used in async mode, was not checked. So it was possible to close some
applets too early and some messages could be reported on error, with the
status code set to 256.
So now, in stopping mode, before closing a SPOE applet, we check the shared
sending queue, the applet waiting queue and the shared waiting queue. If all
these queues are empty, the applet is closed.
The issue is minor. However, a simple workaround is to disable the async
mode by adding "no option async" in the spoe-agent section.
This patch should partially fix the issue #2868. It must be backported to
all stable versions.
Amaury Denoyelle [Fri, 3 Jan 2025 15:25:14 +0000 (16:25 +0100)]
BUG/MINOR: mux-quic: handle closure of uni-stream
This commit is a direct follow-up to the previous one. As already
described, a previous fix was merged to prevent streamdesc attach
operation on already completed QCS instances scheduled for purging. This
was implemented by skipping app proto decoding.
However, this has a bad side-effect for remote uni-directional stream.
If receiving a FIN stream frame on such a stream, it will considered as
complete because streamdesc are never attached to a uni stream. Due to
the mentionned new fix, this prevent analysis of this last frame for
every uni stream.
To fix this, do not skip anymore app proto decoding for completed QCS.
Update instead qcs_attach_sc() to transform it as a noop function if QCS
is already fully closed before streamdesc instantiation. However,
success return value is still used to prevent an invalid decoding error
report.
The impact of this bug should be minor. Indeed, HTTP3 and QPACK uni
streams are never closed by the client as this is invalid due to the
spec. The only issue was that this prevented QUIC MUX to close the
connection with error H3_ERR_CLOSED_CRITICAL_STREAM.
This must be backported along the previous patch, at least to 3.1, and
eventually to 2.8 if mentionned patches are merged there.
(cherry picked from commit
801e39e1ccfc7404354d6870e779f83351593df0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
85e3bf658163d97fd09ac63f6c997d230d32729b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 3 Jan 2025 15:16:45 +0000 (16:16 +0100)]
MINOR: mux-quic: change return value of qcs_attach_sc()
A recent fix was introduced to ensure that a streamdesc instance won't
be attached to an already completed QCS which is eligible to purging.
This was performed by skipping application protocol decoding if a QCS is
in such a state. Here is the patch responsible for this change.
caf60ac696a29799631a76beb16d0072f65eef12
BUG/MEDIUM: mux-quic: do not attach on already closed stream
However, this is too restrictive, in particular for unidirection stream
where no streamdesc is never attached. To fix this behavior, first
qcs_attach_sc() API has been modified. Instead of returning a streamdesc
instance, it returns either 0 on success or a negative error code.
There should be no functional changes with this patch. It is only to be
able to extend qcs_attach_sc() with the possibility of skipping
streamdesc instantiation while still keeping a success return value.
This should be backported wherever the above patch has been merged. For
the record, it was scheduled for immediate backport on 3.1, plus merging
on older releases up to 2.8 after a period of observation.
(cherry picked from commit
af00be8e0f7fe3942fac93ac32bfe7ae7f5b78c9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
1bcdf3c5f2528910930209dbbcfff2556d44c45a)
[cf: ctx adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 31 Dec 2024 15:06:32 +0000 (16:06 +0100)]
BUG/MEDIUM: mux-quic: do not attach on already closed stream
Due to QUIC packet reordering, a stream may be opened via a new
RESET_STREAM or STOP_SENDING frame. This would cause either Tx or Rx
channel to be immediately closed.
This can cause an issue with current QUIC MUX implementation with QCS
purging. QCS are inserted into QCC purge list when transfer could be
considered as completed. In most cases, this happens after full
request/response exchange. However, it can also happens after request
reception if RESET_STREAM/STOP_SENDING are received first.
A BUG_ON() crash will occur if a STREAM frame is received after. In this
case, streamdesc instance will be attached via qcs_attach_sc() to handle
the new request. However, QCS is already considered eligible to purging.
It could cause it to be released while its streamdesc instance remains.
A BUG_ON() crash detects this problem in qcc_purge_streams().
To fix this, extend qcc_decode_qcs() to skip app proto rcv_buf
invokation if QCS is considered completed. A similar condition was
already implemented when read was previously aborted after a
STOP_SENDING emission by QUIC MUX.
This crash was reproduced on haproxy.org. Here is the output of the
backtrace :
Core was generated by `./haproxy-dev -db -f /etc/haproxy/haproxy-current.cfg -sf 16495'.
Program terminated with signal SIGILL, Illegal instruction.
#0 0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661
2661 BUG_ON_HOT(!qcs_is_completed(qcs));
[Current thread is 1 (LWP 1457)]
[ ## gdb ## ] bt
#0 0x00000000004e442b in qcc_purge_streams (qcc=0x774cca0) at src/mux_quic.c:2661
#1 0x00000000004e4db7 in qcc_io_process (qcc=0x774cca0) at src/mux_quic.c:2744
#2 0x00000000004e5a54 in qcc_io_cb (t=0x7f71193940c0, ctx=0x774cca0, status=573504) at src/mux_quic.c:2886
#3 0x0000000000b4f792 in run_tasks_from_lists (budgets=0x7ffdcea1e670) at src/task.c:603
#4 0x0000000000b5012f in process_runnable_tasks () at src/task.c:883
#5 0x00000000007de4a3 in run_poll_loop () at src/haproxy.c:2771
#6 0x00000000007deb9f in run_thread_poll_loop (data=0x1335a00 <ha_thread_info>) at src/haproxy.c:2985
#7 0x00000000007dfd8d in main (argc=6, argv=0x7ffdcea1e958) at src/haproxy.c:3570
This BUG_ON() crash can only happen since 3.1 refactoring. Indeed, purge
list was only implemented on this version. As such, please backport it
on 3.1 immediately. However, a logic issue remains for older version as
a stream could be attached on a fully closed QCS. Thus, it should be
backported up to 2.8, this time after a period of observation.
(cherry picked from commit
caf60ac696a29799631a76beb16d0072f65eef12)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
ae6e84d0b238ddf91b53c86fafc047d11a93f93d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 7 Feb 2025 19:28:39 +0000 (20:28 +0100)]
BUG/MEDIUM: ssl: chosing correct certificate using RSA-PSS with TLSv1.3
The clienthello callback was written when TLSv1.3 was not yet out, and
signatures algorithm changed since then.
With TLSv1.2, the least significant byte was used to determine the
SignatureAlgorithm, which could be rsa(1), dsa(2), ecdsa(3).
https://datatracker.ietf.org/doc/html/rfc5246#section-7.4.1.4.1
This was used to chose which type of certificate to push to the client.
But TLSv1.3 changed that, and introduced new RSA-PSS algorithms that
does not have the least sinificant byte to 1.
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3
This would result in chosing the wrong certificate when an RSA an ECDSA
ones are in the configuration for the same SNI or default entry.
This patch fixes the issue by parsing bothe hash and signature field to
check the RSA-PSS signature scheme.
This must fix issue #2852.
This must be backported in every stable versions. The code was moved
from ssl_sock.c to ssl_clienthello in recent versions.
(cherry picked from commit
64502715968fb12f446fcb6432a5431d803d8935)
[wla: moved the code in ssl_sock.c]
Signed-off-by: William Lallemand <wlallemand@haproxy.com>
Christopher Faulet [Wed, 29 Jan 2025 14:38:32 +0000 (15:38 +0100)]
[RELEASE] Released version 3.0.8
Released version 3.0.8 with the following main changes :
- BUG/MEDIUM: stconn: Don't forward shut for SC in connecting state
- BUG/MINOR: stats: decrement srv refcount on stats-file release
- MINOR: list: define a watcher type
- BUG/MEDIUM: stats/server: use watcher to track server during stats dump
- BUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY
- BUG/MINOR: cli: cli_snd_buf: preserve \r\n for payload lines
- REGTESTS: ssl: add a PEM with mix of LF and CRLF line endings
- BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date
- BUG/MEDIUM: queues: Make sure we call process_srv_queue() when leaving
- BUG/MEDIUM: queues: Do not use pendconn_grab_from_px().
- DOC: config: add example for server "track" keyword
- DOC: config: reorder "tune.lua.*" keywords by alphabetical order
- DOC: config: add "tune.lua.burst-timeout" to the list of global parameters
- BUG/MINOR: h2/rhttp: fix HTTP2 conn counters on reverse
- BUG/MEDIUM: queue: Make process_srv_queue return the number of streams
- BUG/MINOR: stats: fix segfault caused by uninitialized value in "show schema json"
- DOC: config: add missing "track-sc0" in action keywords matrix
- MINOR: config: Alert about extra arguments for errorfile and errorloc
- BUG/MEDIUM: promex/resolvers: Don't dump metrics if no nameserver is defined
- BUG/MEDIUM: h1-htx: Properly handle bodyless messages
- MINOR: stconn: add a new pair of sf functions {bs,fs}.debug_str
- MINOR: mux-h2: implement the debug string for logs
- MINOR: mux-quic: define dump functions for QCC and QCS
- MINOR: mux-quic: implement debug string for logs
- MINOR: quic: dump quic_conn debug string for logs
- MINOR: time: define tot_time structure
- MINOR: mux-quic: measure QCS lifetime and its blocking state
- MINOR: mux-h1: Add support of the debug string for logs
- MINOR: sample: add the "when" converter to condition some expressions
- MINOR: arg: add an argument type for identifier
- MINOR: acl: export find_acl_default()
- MINOR: sample: extend the "when" converter to support an ACL
- CLEANUP: debug: make the BUG_ON() macros check the condition in the outer one
- MEDIUM: debug: add match counters for BUG_ON/WARN_ON/CHECK_IF
- MINOR: debug: add a new debug macro COUNT_IF()
- MINOR: debug: add "debug dev counters" to list code counters
- MINOR: debug/cli: replace "debug dev counters" with "debug counters"
- DEV: sock: Add a debug counter to track strange flag on fd during connect()
- MINOR: activity/memprofile: also monitor strdup() activity
- MINOR: activity/memprofile: monitor non-portable calls as well
- BUILD: activity/memprofile: fix a build warning in the posix_memalign handler
- BUG/MINOR: stktable: fix big-endian compatiblity in smp_to_stkey()
- BUG/MINOR: quic: reject NEW_TOKEN frames from clients
- BUG/MEDIUM: stktable: fix missing lock on some table converters
- BUG/MEDIUM: promex: Use right context pointers to dump backends extra-counters
- BUG/MAJOR: quic: reject too large CRYPTO frames
- BUG/MAJOR: log/sink: possible sink collision in sink_new_from_srv()
- BUG/MINOR: init: set HAPROXY_STARTUP_VERSION from the variable, not the macro
- BUG/MINOR: quic: ensure a detached coalesced packet can't access its neighbours
- MINOR: quic: Add a BUG_ON() on quic_tx_packet refcount
- BUILD: quic: Move an ASSUME_NONNULL() for variable which is not null
- BUG/MEDIUM: mux-h1: Properly close H1C if an error is reported before sending data
- BUG/MINOR: quic: do not increase congestion window if app limited
- BUG/MINOR: ssl: put ssl_sock_load_ca under SSL_NO_GENERATE_CERTIFICATES
- BUG/MINOR: stream: Properly handle "on-marked-up shutdown-backup-sessions"
Christopher Faulet [Mon, 27 Jan 2025 15:17:27 +0000 (16:17 +0100)]
BUG/MINOR: stream: Properly handle "on-marked-up shutdown-backup-sessions"
shutdown-backup-sessions action for on-marked-up directive does not work anymore
since the stream_shutdown() function was modified to be async-safe.
When stream_shutdown() was modified to be async-safe, dedicated task events were
added to map the reasons to shut a stream down. SF_ERR_DOWN was mapped to
TASK_F_EVT1 and SF_ERR_KILLED was mapped to TASK_F_EVT2. The reverse mapping was
performed by process_stream() to shut the stream with the appropriate reason.
However, SF_ERR_UP reason, used by shutdown-backup-sessions action to shut a
stream down because a preferred server became available, was not mapped in the
same way. So since commit
b8e3b0a18d ("BUG/MEDIUM: stream: make
stream_shutdown() async-safe"), this action is ignored and does not work
anymore.
To fix an issue, and being able to bakcport the fix, a third task event was
added. TASK_F_EVT3 is now mapped on SF_ERR_UP.
This patch should fix the issue #2848. It must be backported as far as 2.6.
(cherry picked from commit
0a52a75ef7151a187b42cae6d49a75b79380f211)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
07a8dd887f412078842e16b016fbd8dd61063306)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Valentine Krasnobaeva [Thu, 23 Jan 2025 12:46:46 +0000 (13:46 +0100)]
BUG/MINOR: ssl: put ssl_sock_load_ca under SSL_NO_GENERATE_CERTIFICATES
ssl_sock_load_ca and ssl_sock_free_ca definitions are compiled only, if
SSL_NO_GENERATE_CERTIFICATES is not set. In case, when we set this define and
build haproxy, linker throws an error. So, let's fix this.
This should be backported in all stable versions.
(cherry picked from commit
c987f30245023f3bdf4dbe5296ed39f2d8faa98b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
5d3f2aba73c73329bc41a167ce931e8d03e5f28b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 9 Jan 2025 16:26:37 +0000 (17:26 +0100)]
BUG/MINOR: quic: do not increase congestion window if app limited
Previously, congestion window was increased any time each time a new
acknowledge was received. However, it did not take into account the
window filling level. In a network condition with negligible loss, this
will cause the window to be incremented until the maximum value (by
default 480k), even though the application does not have enough data to
fill it.
In most cases, this issue is not noticeable. However, it may lead to
excessive memory consumption when a QUIC connection is suddendly
interrupted, as in this case haproxy will fill the window with
retransmission. It even has caused OOM crash when thousands of clients
were interrupted at once on a local network benchmark.
Fix this by first checking window level prior to every incrementation
via a new helper function quic_cwnd_may_increase(). It was arbitrarily
decided that the window must be at least 50% full when the ACK is
handled prior to increment it. This value is a good compromise to keep
window in check while still allowing fast increment when needed.
Note that this patch only concerns cubic and newreno algorithm. BBR has
already its notion of application limited which ensures the window is
only incremented when necessary.
This should be backported up to 2.6.
(cherry picked from commit
bbaa7aef7ba25480b842cda274e57d820680bb57)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
2596916866366099c2cce22b26387598e995b724)
[cf: ctx adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 23 Jan 2025 09:31:41 +0000 (10:31 +0100)]
BUG/MEDIUM: mux-h1: Properly close H1C if an error is reported before sending data
It is possible to have front H1 connections waiting for the client timeout
while they should be closed because a conneciton error was reported before
sebding an error message to the client. It is not a leak because the
connections are closed when the timeout expires but it is a waste of
ressources, especially if the client timeout is high.
When an early error message must be sent to the client, if an error was
already detected, no data are sent and the output buffer is released. At
this stage, the H1 connection is in CLOSING state and it must be
released. But because of a bug, this is not performed. The client timeout is
rearmed and the H1 connection is only closed when it expires.
To fix the issue, the condition to close a H1C must also be evaluated when
an error is detected before sending data.
It is only an issue with idle client connections, because there is no H1
stream in that case and the error message is generated by the mux itself.
This patch must be backported as far as 2.8.
(cherry picked from commit
b18e988e0dabbb0c382d2dfb5b8701670a386dde)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
b833cbcf08ca7c9263e3be9112de8c7478a87ace)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>