haproxy-3.0.git
5 months agoMINOR: epoll: permit to mask certain specific events
Willy Tarreau [Mon, 27 Jan 2025 14:41:26 +0000 (15:41 +0100)]
MINOR: epoll: permit to mask certain specific events

A few times in the past we've seen cases where epoll was caught reporting
a wrong event that caused trouble (e.g. spuriously reporting HUP or RDHUP
after a successful connect()). The new tune.epoll.mask-events directive
permits to mask events such as ERR, HUP and RDHUP and convert them to IN
events that are processed by the regular receive path. This should help
better diagnose and troubleshoot issues such as this one, as well as rule
out such a cause when similar issues are reported:

   https://github.com/haproxy/haproxy/issues/2368
   https://www.spinics.net/lists/netdev/msg876470.html

It should be harmless to backport this if necessary.

(cherry picked from commit 7fa70da06db3e69a61e4451a82a4f980b40b4d0c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit cf4d3fc9aec7bb16191cad2678554b631553a99d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 months agoMINOR: fd: add a generation number to file descriptors
Willy Tarreau [Thu, 30 Jan 2025 15:25:40 +0000 (16:25 +0100)]
MINOR: fd: add a generation number to file descriptors

This patch adds a counter of close() on file descriptors in the fdtab.
The goal is to better detect if reported events concern the current or
a previous file descriptor. For now the counter is only added, and is
showed in "show fd" as "gen". We're reusing unused space at the end of
the struct. If it's needed for something more important later, this
patch can be reverted.

(cherry picked from commit d155924efe453faa0ea5b5e62e6a3e2bf7ee7358)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3f646022e0d7421a7f105e9aab1546fd7263ce7d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 months agoDEBUG: fd: add a counter of takeovers of an FD since it was last opened
Willy Tarreau [Thu, 30 Jan 2025 14:59:11 +0000 (15:59 +0100)]
DEBUG: fd: add a counter of takeovers of an FD since it was last opened

That's essentially in order to help with debugging strange cases like
the occasional epoll issues/races, by keeping a counter of how many
times an FD was taken over since last inserted. The room is available
so let's use it. If it's needed later, this patch can easily be reverted.
The counter is also reported in "show fd" as "tkov".

(cherry picked from commit 44ac7a7e731537c9388a141b69cff7074afe2376)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e8c657a034d5ff4ad9648370faea5b9f1039d6fb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 months agoBUG/MEDIUM: mux-fcgi: Properly handle read0 on partial records
Christopher Faulet [Mon, 27 Jan 2025 14:18:14 +0000 (15:18 +0100)]
BUG/MEDIUM: mux-fcgi: Properly handle read0 on partial records

A Read0 event could be ignored by the FCGI multiplexer if it is blocked on a
partial record. Instead of handling the event, it remained blocked, waiting
for the end of the record.

To fix the issue, the same solution than the H2 multiplexer is used. Two
flags are introduced. The first one, FCGI_CF_END_REACHED, is used to
acknowledge a read0. This flag is set when a read0 was received AND the FCGI
multiplexer must handle it. The second one, FCGI_CF_DEM_SHORT_READ, is set
when the demux is interrupted on a partial record. A short read and a read0
lead to set the FCGI_CF_END_REACHED flag.

With these changes, the FCGI mux should be able to properly handle read0 on
partial records.

This patch should be backported to all stable versions after a period of
observation.

(cherry picked from commit 5f927f603aae4614f82c73ff6c2e3324ecfdf506)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e20714440249497e99ef7b45c47ac4439d5af803)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

5 months agoBUG/MINOR: stktable: invalid use of stkctr_set_entry() with mixed table types
Aurelien DARRAGON [Tue, 24 Dec 2024 15:28:35 +0000 (16:28 +0100)]
BUG/MINOR: stktable: invalid use of stkctr_set_entry() with mixed table types

Some actions such as "sc0_get_gpc0" (using smp_fetch_sc_stkctr()
internally) can take an optional table name as parameter to perform the
lookup on a different table from the tracked one but using the key from
the tracked entry. It is done by leveraging the stktable_lookup() function
which was originally meant to perform intra-table lookups.

Calling sc0_get_gpc0() with a different table name will result in
stktable_lookup() being called to perform lookup using a stktsess from
a different table. While it is theorically fine, it comes with a pitfall:
both tables (the one from where the stktsess originates and the actual
target table) should rely on the exact same key type and length.

Failure to do so actually results in undefined behavior, because the key
type and/or length from one table is used to perform the lookup in
another table, while the underlying lookup API expects explicit type and
key length.

For instance, consider the below example:

  peers testpeers
    bind 127.0.0.1:10001
    server localhost

    table test type binary len 1 size 100k expire 1h store gpc0
    table test2 type string size 100k expire 1h store gpc0

  listen test_px
    mode http
    bind 0.0.0.0:8080
    http-request track-sc0 bin(AA) table testpeers/test
    http-request track-sc1 str(ok) table testpeers/test2
    log-format "%[sc0_get_gpc0(testpeers/test2)]"
    log stdout format raw local0

    server s1 git.haproxy.org:80

Performing a curl request to localhost:8080 will cause unitialized reads
because string "ok" from test2 table will be compared as a string against
"AA" binary sample which is not NULL terminated:

==2450742== Conditional jump or move depends on uninitialised value(s)
==2450742==    at 0x484F238: strlen (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2450742==    by 0x27BCE6: stktable_lookup (stick_table.c:539)
==2450742==    by 0x281470: smp_fetch_sc_stkctr (stick_table.c:3580)
==2450742==    by 0x283083: smp_fetch_sc_get_gpc0 (stick_table.c:3788)
==2450742==    by 0x2A805C: sample_process (sample.c:1376)

So let's prevent that by adding some comments in stktable_set_entry()
func description, and by adding a check in smp_fetch_sc_stkctr() to ensure
both source stksess and target table share the same key properties.

While it could be relevant to backport this in all stable versions, it is
probably safer to wait for some time before doing so, to ensure that no
existing configs rely on this ambiguity because the fact that the target
table and source stksess entry need to share the same key type and length
is not explicitly documented.

(cherry picked from commit 5bbdd14f56a2a672e8fef4449e4143b0f0642812)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit f9023cbd1791e5a9828a85c23d829e6732ea2ca8)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

5 months agoDEBUG: stream: Add debug counters to track some client/server aborts
Christopher Faulet [Tue, 22 Oct 2024 14:46:34 +0000 (16:46 +0200)]
DEBUG: stream: Add debug counters to track some client/server aborts

Not all aborts are tracked for now but only those a bit ambiguous. Mainly,
aborts during the data forwarding are concerned. Those triggered during the
request or the response analysis are easier to analyze with the stream
termination state.

(cherry picked from commit c8aecc393bd125e476469d2b6159a1af542c78ae)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 months agoBUG/MINOR: quic: do not crash on CRYPTO ncbuf alloc failure
Amaury Denoyelle [Fri, 18 Apr 2025 16:02:48 +0000 (18:02 +0200)]
BUG/MINOR: quic: do not crash on CRYPTO ncbuf alloc failure

To handle out-of-order received CRYPTO frames, a ncbuf instance is
allocated. This is done via the helper quic_get_ncbuf().

Buffer allocation was improperly checked. In case b_alloc() fails, it
crashes due to a BUG_ON(). Fix this by removing it. The function now
returns NULL on allocation failure, which is already properly handled in
its caller qc_handle_crypto_frm().

This should fix the last reported crash from github issue #2935.

This must be backported up to 2.6.

(cherry picked from commit 4309a6fbf80240b0880c5adf091f0075c3bcd53f)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 1c913c47cad4b00c9fef726b32b1a4240161a89e)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

6 months agoBUG/MEDIUM: hlua: fix hlua_applet_{http,tcp}_fct() yield regression (lost data)
Aurelien DARRAGON [Thu, 17 Apr 2025 12:21:20 +0000 (14:21 +0200)]
BUG/MEDIUM: hlua: fix hlua_applet_{http,tcp}_fct() yield regression (lost data)

Jacques Heunis from bloomberg reported on the mailing list [1] that
with haproxy 2.8 up to master, yielding from a Lua tcp service while
data was still buffered inside haproxy would eat some data which was
definitely lost.

He provided the reproducer below which turned out to be really helpful:

  global
      log stdout format raw local0 info
      lua-load haproxy_yieldtest.lua

  defaults
      log global
      timeout connect         10s
      timeout client          1m
      timeout server          1m

  listen echo
      bind *:9090
      mode tcp
      tcp-request content use-service lua.print_input

haproxy_yieldtest.lua:

  core.register_service("print_input", "tcp", function(applet)
      core.Info("Start printing input...")
      while true do
          local inputs = applet:getline()
          if inputs == nil or string.len(inputs) == 0 then
              core.Info("closing input connection")
              return
          end
          core.Info("Received line: "..inputs)
          core.yield()
      end
  end)

And the script below:

  #!/usr/bin/bash
  for i in $(seq 1 9999); do
      for j in $(seq 1 50); do
          echo "${i}_foo_${j}"
      done
      sleep 2
  done

Using it like this:
  ./test_seq.sh | netcat localhost 9090

We can clearly see the missing data for every "foo" burst (every 2
seconds), as they are holes in the numbering.

Thanks to the reproducer, it was quickly found that only versions
>= 2.8 were affected, and that in fact this regression was introduced
by commit 31572229e ("MEDIUM: hlua/applet: Use the sedesc to report and
detect end of processing")

In fact in 31572229e 2 mistakes were made during the refaco.
Indeed, both in hlua_applet_tcp_fct() (which is involved in the reproducer
above) and hlua_applet_http_fct(), the request (buffer) is now
systematically consumed when returning from the function, which wasn't the
case prior to this commit: when HLUA_E_AGAIN is returned, it means a
yield was requested and that the processing is not done yet, thus we
should not consume any data, like we did prior to the refacto.

Big thanks to Jacques who did a great job reproducing and reporting this
issue on the mailing list.

[1]: https://www.mail-archive.com/haproxy@formilux.org/msg45778.html

It should be backported up to 2.8 with commit 31572229e

(cherry picked from commit b81ab159a613323a0e8cd625d138955acfcbb666)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit ea97524977950be1162b022058f1dbcaa16167d3)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: h3: reject request URI with invalid characters
Amaury Denoyelle [Wed, 16 Apr 2025 13:27:03 +0000 (15:27 +0200)]
BUG/MINOR: h3: reject request URI with invalid characters

Ensure that the HTX start-line generated after parsing an HTTP/3 request
does not contain any invalid character, i.e. control or whitespace
characters.

Note that for now path is used directly as URI. Thus, the check is
performed directly over it. A patch will change this to generate an
absolute-form URI in most cases, but it won't be backported to avoid
configuration breaking in stable versions.

This must be backported up to 2.6.

(cherry picked from commit 1faa1285aacaaacd2c2e062d87f04aa481843862)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 3c1914822d9b98f244c3e603c050a19cc29eed36)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: h3: reject invalid :path in request
Amaury Denoyelle [Wed, 16 Apr 2025 09:17:20 +0000 (11:17 +0200)]
BUG/MINOR: h3: reject invalid :path in request

RFC 9114 specifies some requirements for :path pseudo-header when using
http or https scheme. This commit enforces this by rejecting a request
if needed. Thus, path cannot be empty, and it must either start with a
'/' character or contains only '*'.

This must be backported up to 2.6.

(cherry picked from commit fc28fe7191701251115e817a7c4a673b88e49f65)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit cb5ff8cf01212b321b15f24a3f28b68154774955)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: h3: filter upgrade connection header
Amaury Denoyelle [Wed, 16 Apr 2025 09:20:42 +0000 (11:20 +0200)]
BUG/MINOR: h3: filter upgrade connection header

As specified in RFC 9114, connection headers required special care in
HTTP/3. When a request is received with connection headers, the stream
is immediately closed. Conversely, when translating the response from
HTX, such headers are not encoded but silently ignored.

However, "upgrade" was not listed in connection headers. This commit
fixes this by adding a check on it both on request parsing and response
encoding.

This must be backported up to 2.6.

(cherry picked from commit 6403bfbce8ea54ba83e23d34c5d52ff10fa7fe22)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 95749cdc2b7a90ba956e546b7c314e3a37dd971e)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MEDIUM: h3: trim whitespaces in header value prior to QPACK encoding
Amaury Denoyelle [Wed, 16 Apr 2025 15:29:41 +0000 (17:29 +0200)]
BUG/MEDIUM: h3: trim whitespaces in header value prior to QPACK encoding

This commit does a similar job than the previous one, but it acts now on
the response path. Any leading or trailing whitespaces characters from a
HTX block header value are removed, prior to the header encoding via
QPACK.

This must be backported up to 2.6.

(cherry picked from commit bd3587574dae43cc019d8e9998e0996c75550de4)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit a4f7bfa029ada954c8fae75c1426555a3a4012d4)
[ada: ctx adjt]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MEDIUM: h3: trim whitespaces when parsing headers value
Amaury Denoyelle [Wed, 16 Apr 2025 13:47:42 +0000 (15:47 +0200)]
BUG/MEDIUM: h3: trim whitespaces when parsing headers value

Remove any leading and trailing whitespace from header field values
prior to inserting a new HTX header block. This is done when parsing a
HEADERS frame, both as headers and trailers.

This must be backported up to 2.6.

(cherry picked from commit a17e5b27c03230cc65ebedc6dc28dbfce5181c79)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit c76eff8ecd4853960e57f081fcfddbd87b8d666d)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: sink: add tempo between 2 connection attempts for sft servers (2)
Aurelien DARRAGON [Wed, 16 Apr 2025 19:59:04 +0000 (21:59 +0200)]
BUG/MINOR: sink: add tempo between 2 connection attempts for sft servers (2)

This is a follow up patch for 9e02e4d5 ("BUG/MINOR: sink: add tempo
between 2 connection attempts for sft servers").

When the patch was backported to 3.0, due to context mismatch it had to
be manually adjusted but a mistake was made. Indeed, the fix is a no-op
given that the last_conn is always set to TICK_ETERNITY (which translates
to 0) that corresponds to the starting value (sft is calloced) where the
applet is allowed to be created instantly. Instead, we must set last_conn
to now_ms as it was done in the upstream patch.

It should be backported everywhere 9e02e4d5 was.

This commit has no upstream id because it is specific to the 3.0

6 months agoBUG/MEDIUM: http-ana: Report 502 from req analyzer only during rsp forwarding
Christopher Faulet [Tue, 15 Apr 2025 06:18:48 +0000 (08:18 +0200)]
BUG/MEDIUM: http-ana: Report 502 from req analyzer only during rsp forwarding

A server abort must be handled by the request analyzers only when the
response forwarding was already started. Otherwise, it it the responsability
of the response analyzer to detect this event. L7-retires and conditions to
decide to silently close a client conneciotn are handled by this analyzer.

Because a reused server connections closed too early could be detected at
the wrong place, it was possible to get a 502/SH instead of a silent close,
preventing the client to safely retries its request.

Thanks to this patch, we are able to silently close the client connection in
this case and eventually to perform a L7 retry.

This patch must be backported as far as 2.8.

(cherry picked from commit d160046e2c9caae7deff5d59abc9694b6e664446)
[ada: ctx adjt since COUNT_IF counters are still there in 3.1]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit c0d92e80821a4af654c6a4230fa9ab24b35f9ee4)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: http-ana: Properly detect client abort when forwarding the response
Christopher Faulet [Tue, 15 Apr 2025 05:54:19 +0000 (07:54 +0200)]
BUG/MINOR: http-ana: Properly detect client abort when forwarding the response

During the response payload forwarding, if the back SC is closed, we try to
figure out if it is because of a client abort or a server abort. However,
the condition was not accurrate, especially when abortonclose option is
set. Because of this issue, a server abort may be reported (SD-- in logs)
instead of a client abort (CD-- in logs).

The right way to detect a client abort when we try to forward the response
is to test if the back SC was shut down (SC_FL_SHUT_DOWN flag set) AND
aborted (SC_FL_ABRT_DONE flag set). When these both flags are set, it means
the back connection underwent the shutdown, which should be converted to a
client abort at this stage.

This patch should be backported as far as 2.8. It should fix last strange SD
report in the issue #2749.

(cherry picked from commit c672b2a297158bcd673feab2fd366709f9fc3d4f)
[ada: ctx adjt because COUNT_IF counters are still there in 3.1]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit d99407770db42ac27f488093e9b419cd0839e48f)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoDOC: config: add the missing "profiling.memory" to the global kw index
Willy Tarreau [Mon, 14 Apr 2025 16:28:09 +0000 (18:28 +0200)]
DOC: config: add the missing "profiling.memory" to the global kw index

It was in the description but not in the index. This can be backported to
all versions where it applies.

(cherry picked from commit 640a6998046c4a94d3fb59469b1cdcccedbeb01a)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit ab7fb86e3cf7cde3f1a381c1324a75a7f2ce75c9)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: hlua: fix invalid errmsg use in hlua_init()
Aurelien DARRAGON [Thu, 10 Apr 2025 15:35:53 +0000 (17:35 +0200)]
BUG/MINOR: hlua: fix invalid errmsg use in hlua_init()

errmsg is used with memprintf and friends, thus it must be NULL
initialized before being passed to memprintf, else invalid read will
occur.

However in hlua_init() the errmsg value isn't initialized, let's fix that

This is really minor because it would only cause issue on error paths,
yet it may be backported to all stable versions, just in case.

(cherry picked from commit ea3c96369f4a5def90888c9207cd88010d473eb4)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 50838c51a97113d0c9b95d22b4312c9a80a0409d)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: backend: do not use the source port when hashing clientip
Willy Tarreau [Wed, 9 Apr 2025 08:57:54 +0000 (10:57 +0200)]
BUG/MINOR: backend: do not use the source port when hashing clientip

The server's "usesrc" keyword supports among other options "client"
and "clientip". The former means we bind to the client's IP and port
to connect to the server, while the latter means we bind to its IP
only. It's done in two steps, first alloc_bind_address() retrieves
the IP address and port, and second, tcp_connect_server() decides
to either bind to the IP only or IP+port.

The problem comes with idle connection pools, which hash all the
parameters: the hash is calculated before (and ideally withouy) calling
tcp_connect_server(), and it considers the whole struct sockaddr_storage
for the hash, except that both client and clientip entirely fill it with
the client's address. This means that both client and clientip make use
of the source port in the hash calculation, making idle connections
almost not reusable when using "usesrc clientip" while they should for
clients coming from the same source. A work-around is to force the
source port to zero using "tcp-request session set-src-port int(0)" but
it's ugly.

Let's fix this by properly zeroing the port for AF_INET/AF_INET6 addresses.

This can be backported to 2.4. Thanks to Sebastien Gross for providing a
reproducer for this problem.

(cherry picked from commit 7b6df86a834883f27f6f7d18d0caa8a6ea128c14)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 277e88cb78bb371fb8dbb1fff24a06e8a6de85d1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MEDIUM: sample: fix risk of overflow when replacing multiple regex back-refs
Willy Tarreau [Mon, 7 Apr 2025 13:30:43 +0000 (15:30 +0200)]
BUG/MEDIUM: sample: fix risk of overflow when replacing multiple regex back-refs

Aleandro Prudenzano of Doyensec and Edoardo Geraci of Codean Labs
reported a bug in sample_conv_regsub(), which can cause replacements
of multiple back-references to overflow the temporary trash buffer.

The problem happens when doing "regsub(match,replacement,g)": we're
replacing every occurrence of "match" with "replacement" in the input
sample, which requires a length check. For this, a max is applied, so
that a replacement may not use more than the remaining length in the
buffer. However, the length check is made on the replaced pattern and
not on the temporary buffer used to carry the new string. This results
in the remaining size to be usable for each input match, which can go
beyond the temporary buffer size if more than one occurrence has to be
replaced with something that's larger than the remaining room.

The fix proposed by Aleandro and Edoardo is the correct one (check on
"trash" not "output"), and is the one implemented in this patch.

While it is very unlikely that a config will replace multiple short
patterns each with a larger one in a request, this possibility cannot
be entirely ruled out (e.g. mask a known, short IP address using
"XXX.XXX.XXX.XXX").  However when this happens, the replacement pattern
will be static, and not be user-controlled, which is why this patch is
marked as medium.

The bug was introduced in 2.2 with commit 07e1e3c93e ("MINOR: sample:
regsub now supports backreferences"), so it must be backported to all
versions.

Special thanks go to Aleandro and Edoardo for reporting this bug with
a simple reproducer and a fix.

(cherry picked from commit 3e3b9eebf871510aee36c3a3336faac2f38c9559)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit db87c8d9fe621539531f6f915ba9e1755a2a26cb)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoBUG/MINOR: log: fix CBOR encoding with LOG_VARTEXT_START() + lf_encode_chunk()
Aurelien DARRAGON [Mon, 7 Apr 2025 10:00:03 +0000 (12:00 +0200)]
BUG/MINOR: log: fix CBOR encoding with LOG_VARTEXT_START() + lf_encode_chunk()

There have been some reports that using %HV logformat alias with CBOR
encoder would produce invalid CBOR payload according to some CBOR
implementations such as "cbor.me". Indeed, with the below log-format:

  log-format "%{+cbor}o %(protocol)HV"

And the resulting CBOR payload:

  BF6870726F746F636F6C7F48485454502F312E31FFFF

cbor.me would complain with: "bytes/text mismatch (ASCII-8BIT != UTF-8) in
streaming string") error message.

It is due to the version string being first announced as text, while CBOR
encoder actually encodes it as byte string later when lf_encode_chunk()
is used.

In fact it affects all patterns combining LOG_VARTEXT_START() with
lf_encode_chunk() which means  %HM, %HU, %HQ, %HPO and %HP are also
affected. To fix the issue, in _lf_encode_bytes() (which is
lf_encode_chunk() helper), we now check if we are inside a VARTEXT (we
can tell it if ctx->in_text is true), in which case we consider that we
already announced the current data as regular text so we keep the same
type to encode the bytes from the chunk to prevent inconsistencies.

It should be backported in 3.0

(cherry picked from commit 9e8444b730e9b24e73f1f78d269f5001a34bd98d)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 6c042450a4465e0dea7dafbe43157f2346444a70)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

6 months agoCLEANUP: log: adjust _lf_cbor_encode_byte() comment
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>

6 months agoBUG/MINOR: hlua_fcn: fix potential UAF with Queue:pop_wait()
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>

6 months agoMINOR: task: add thread safe notification_new and notification_wake variants
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>

6 months agoTESTS: Fix build for filltab25.c
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>

6 months agoBUG/MEDIUM: stream: Fix a possible freeze during a forced shut on a stream
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>

6 months agoBUG/MEDIUM: backend: fix reuse with set-dst/set-dst-port
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>

6 months agoBUG/MINOR: backend: do not overwrite srv dst address on reuse
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>

6 months agoBUG/MINOR: rhttp: fix incorrect dst/dst_port values
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>

6 months agoBUILD: compiler: undefine the CONCAT() macro if already defined
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>

6 months agoDOC: config: fix two missing "content" in "tcp-request" examples
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>

6 months agoBUG/MINOR: log: fix gcc warn about truncating NUL terminator while init char arrays
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>

6 months agoBUG/MINOR: mux-quic: remove extra BUG_ON() in _qcc_send_stream()
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>

6 months agoBUG/MEDIUM: mux-quic: fix crash on RS/SS emission if already close local
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>

6 months agoBUG/MEDIUM: peers: prevent learning expiration too far in futur from unsync node
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>

6 months agoBUG/MINOR: peers: fix expire learned from a peer not converted from ms to ticks
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>

6 months agoMINOR: log: support "raw" logformat node typecast
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>

7 months ago[RELEASE] Released version 3.0.9 v3.0.9
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()

7 months agoBUILD: 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>

7 months agoMINOR: tools: teach resolve_sym_name() a few more common symbols
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>

7 months agoMINOR: tools: ease the declaration of known symbols in resolve_sym_name()
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>

7 months agoMINOR: tools: improve symbol resolution without dl_addr
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>

7 months agoMINOR: cli: export cli_io_handler() to ease symbol resolution
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>

7 months agoMINOR: tinfo: split the signal handler report flags into 3
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>

7 months agoBUILD: tools: silence a build warning when USE_THREAD=0
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>

7 months agoMINOR: tools: use only opportunistic symbols resolution
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>

7 months agoMINOR: tools: resolve main() only once in resolve_sym_name()
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>

7 months agoMINOR: compiler: add a new __decl_thread_var() macro to declare local variables
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>

7 months agoMINOR: compiler: add a simple macro to concatenate resolved strings
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>

7 months agoBUG/MEDIUM: stream: don't use localtime in dumps from a signal handler
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>

7 months agoBUG/MEDIUM: hlua/cli: fix cli applet UAF in hlua_applet_wakeup()
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>

7 months agoBUG/MINOR: http-check: Don't pretend a C-L heeader is set before adding it
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>

7 months agoBUG/MINOR: ssl_crtlist: handle a possible strdup() failure
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>

7 months agoBUG/MINOR: namespace: handle a possible strdup() failure
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>

7 months agoBUG/MINOR: stats: fix capabilities and hide settings for some generic metrics
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>

7 months agoMINOR: cfgparse/peers: provide more info when ignoring invalid "peer" or "server...
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>

7 months agoBUG/MINOR: server: dont return immediately from parse_server() when skipping checks
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>

7 months agoBUG/MINOR: cfgparse/peers: properly handle ignored local peer case
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>

7 months agoBUG/MINOR: cfgparse/peers: fix inconsistent check for missing peer server
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>

7 months agoBUG/MEIDUM: startup: return to initial cwd only after check_config_validity()
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>

7 months agoBUG/MINOR: log: set proper smp size for balance log-hash
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>

7 months agoCLEANUP: log: removing "log-balance" references
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>

7 months agoBUG/MINOR: server: check for either proxy-protocol v1 or v2 to send hedaer
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>

7 months agoCLEANUP: h3: fix documentation of h3_rcv_buf()
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>

7 months agoBUG/MINOR: h3: do not report transfer as aborted on preemptive response
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>

7 months agoBUG/MINOR: h2: always trim leading and trailing LWS in header values
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>

7 months agoBUG/MEDIUM: stream: use non-blocking freq_ctr calls from the stream dumper
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>

7 months agoMINOR: freq_ctr: provide non-blocking read functions
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>

7 months agoBUG/MEDIUM: stream: never allocate connection addresses from signal handler
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>

7 months agoMINOR: tinfo: add a new thread flag to indicate a call from a sig handler
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>

7 months agoBUG/MINOR: mux-h1: always make sure h1s->sd exists in h1_dump_h1s_info()
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>

7 months agoMINOR: clock: always use atomic ops for global_now_ms
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>

7 months agoBUG/MINOR: sink: add tempo between 2 connection attempts for sft servers
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>

7 months agoBUG/MINOR: cfgparse: fix NULL ptr dereference in cfg_parse_peers
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>

7 months agoBUG/MINOR: stats-json: Define JSON_INT_MAX as a signed integer
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>

7 months agoBUG/MINOR: flt-trace: Support only one name option
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>

7 months agoBUG/MINOR: auth: Fix a leak on error path when parsing user's groups
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>

7 months agoBUG/MINOR: config/userlist: Support one 'users' option for 'group' directive
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>

7 months agoBUG/MINOR: cli: Fix a possible infinite loop in _getsocks()
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>

7 months agoBUG/MINOR: cli: Fix memory leak on error for _getsocks command
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>

7 months agoBUG/MINOR: cli: Don't set SE flags from the cli applet
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>

7 months agoBUG/MINOR: tcp-rules: Don't forward close during tcp-response content rules eval
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>

7 months agoBUG/MINOR: mux-h2: Properly handle full or truncated HTX messages on shut
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>

7 months agoBUG/MINOR: mux-quic: prevent crash after MUX init failure
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>

7 months agoBUG/MINOR: quic: prevent crash on conn access after MUX init failure
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>

7 months agoBUG/MINOR: fcgi: Don't set the status to 302 if it is already set
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>

7 months agoBUG/MEDIUM: filters: Handle filters registered on data with no payload callback
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>

7 months agoBUG/MINOR: cli: Wait for the last ACK when FDs are xferred from the old worker
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>

7 months agoBUG/MINOR: ssl/cli: "show ssl crt-list" lacks sigals
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>

7 months agoBUG/MINOR: ssl/cli: "show ssl crt-list" lacks client-sigals
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>

7 months agoBUG/MINOR: quic: fix CRYPTO payload size calcul for encoding
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>

7 months agoBUG/MINOR: quic: reserve length field for long header encoding
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>

7 months agoBUG/MEDIUM: debug: close a possible race between thread dump and panic()
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>

7 months agoBUILD: ssl: more cleaner approach to WolfSSL without renegotiation
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>

7 months agoBUILD: ssl: allow to build without the renegotiation API of WolfSSL
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>

7 months agoBUG/MINOR: server: fix the "server-template" prefix memory leak
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>

7 months agoDOC: management: rename some last occurences from domain "dns" to "resolvers"
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>

7 months agoBUG/MEDIUM: thread: use pthread_self() not ha_pthread[tid] in set_affinity
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>

7 months agoMINOR: startup: adjust alert messages, when capabilities are missed
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>

7 months agoBUG/MINOR: cfgparse-tcp: relax namespace bind check
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>