DOC: config: explain the effect of the weights on random's draws

"balance random" draws the servers on the consistent hash ring, where a
server gets 16 points per unit of weight. With the default weight of 1,
the shares of the ring are uneven (between about 2% and 4% for 32
servers, computed from the ring), so some servers are drawn twice as
often as others. Using the same higher weight for all servers, such as
100, makes the shares almost even.

Measured on 32 servers with 4 slow ones (with the peak EWMA option under
review, which compares the response times of the drawn servers), raising
the weight of all servers from 1 to 100 brought the share of the
requests sent to the slow servers from 2.0% to 1.5%, the same as a
uniform draw.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Aleksandar Lazic <al-haproxy@none.at>

MEDIUM: backend: avoid picking the same server twice in random draws

Aleksandar Lazic suggested [1] that the random() distribution could
be improved to avoid picking the same server twice, comparing it with
itself and choosing it, possibly even if suboptimal. His mesurements
showed that a slow server was picked more often than desired.

It turns out that we don't need extra draws to avoid this situation,
because get_server_rnd() uses chash_get_server_hash(), which supports
a server to avoid (normally used for redispatch, but usable here as
well to avoid picking the same server twice), so we can pass there the
pointer to the currently chosen best server and count on the function
to pick another one.

Before the patch, a config with 1 slow server responding in 50ms,
and N-1 fast servers responding in 5ms would give this distribution
for 100k requests at 100 concurrency:

  4 srv: req rate= 11.1k/s    8 srv: req rate = 15.8k/s
    srv  max  total             srv  max  total
     s0   63   8525              s0   29   2248
     s1   27  28296              s1   17  13874
     s2   28  31049              s2   17  13869
     s3   29  32130              s3   18  13996
                                 s4   18  14316
                                 s5   17  14325
                                 s6   16  13758
                                 s7   16  13614

Now with the patch:

  4 srv: req rate= 14.6k/s    8 srv: req rate = 16.6k/s
    srv  max  total             srv  max  total
     s0   29   3572              s0   17   1669
     s1   28  31380              s1   15  14007
     s2   29  32513              s2   16  14010
     s3   29  32535              s3   15  14059
                                 s4   16  14205
                                 s5   16  14215
                                 s6   16  13968
                                 s7   15  13867

It's clear that the distribution is much better and follows the ratios
of response times, with little to no difference in maximum server load,
thus being much closer to a leastconn algo. The average rate is also
better, +33% for 4 servers and +5% for 8.

Note that in order to avoid wasting CPU cycles in degenerate farm cases,
the two previous patches are strongly recommended:

  OPTIM: lb-chash: don't look for more servers when there is only one
  OPTIM: lb-chash: limit the scan to few consecutive nodes

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

OPTIM: lb-chash: limit the scan to few consecutive nodes

Strongly imbalanced configs such as one server of weight 256 and another
one of weight 1 can result in high CPU usage with consistent hashing when
the first attempted server is the one to be avoided, or when affinity is
set to maxconn or maxqueue, because in this case, the frequent server may
appear a lot of consecutive times, on average 256 after a node before
meeting the other server, and if terribly unlucky, 4096 times.

Also in properly balanced farms, all servers are more or less well
distributed, so that after visiting a number of them we've seen them
all.

This patch takes this into account to avoid too long scans of the whole
tree caused by such imbalances. What it does is:
  - limit the search of another server to 3 consecutive nodes of the
    same server. If this happens, the tree is severely imbalanced so
    there's no point looking for another server: not only will this
    take a lot of time, but it may even result in a lot of traffic
    being transferred to it at ratios well above the weights ratios ;

  - limit the search of an eligible server to twice the number of servers
    in the farm, considering that once we've witnessed 2N server changes,
    it's fairly unlikely that a remaining server is worth being considered
    beyond that point (as a reminder, each server is present 16 times).

For the last point we could also have decided to stop based on the
distance, e.g. 1/8 of the whole spectrum, but it would be more
probabilistic and there it remains understandable.

OPTIM: lb-chash: don't look for more servers when there is only one

The chash_get_server_hash() function never makes a special case for a
single server. Worse, when asked to avoid a specific one and it is the
only one, or when the chosen server is not eligible, the function will
run over all its positions at the end. Thus having a single server in
a config with a weight 256 and asking to avoid it burns a lot of CPU.

Let's just test for this case after picking the first server. If the
total servers count is 1, we can just leave.

BUG/MEDIUM: mux_quic: fix Rx flow control increment on RESET_STREAM

Since the following patch, on RESET_STREAM frame reception, its final
size field is used to account for non-received data in the connection
flow control.

  1b2bd2aeef08209664585e4b3f230744835257d9
  BUG/MINOR: mux_quic: report conn error on RESET_STREAM fctl violation

This addition is not correct as it uses the read offset of the QCS
instance instead of the maximum offset already checked in flow control.
Over time, there is a risk to report a flow control violation despite
legitimate client usage.

Fix this by using the proper QCS field in the flow control calculation
check. Note that accounted data for MAX_DATA emission is still based on
the QCS read offset though.

This must be backported up to 2.8, after the above-mentioned patch.

BUG/MINOR: log: don't let an LF in a forwarded message create new events

A message received by a log-forward section over UDP or over TCP with
octet counting may contain CR or LF bytes. They are forwarded as is to
line-based outputs (rings, tcp@ servers, stdout/stderr), where each LF
starts a new event. A sender can then forge complete syslog lines with
any host name and program, which also defeats "option host".

syslog_process_message() now replaces every CR and LF but the trailing
ones with a space before parsing, so each received message gives one
line.

This should be backported to all stable branches.

BUG/MINOR: http-ana: don't emit a network-path Location for a "/" redirect prefix

With the special prefix "/" (redirect prefix /, or server ... redir /),
or a log-format prefix which evaluates to an empty string, the Location
header is the request path alone. A request for "//evil.example/x" or
"/\evil.example/x" then gives "Location: //evil.example/x", which
browsers resolve to another host: an open redirect on a rule which is
only meant to keep the client on the same site.

When nothing is written before the path and it starts with "//" or "/\",
"/." is now written first. "/.//evil.example/x" resolves to the same
path on the same host. Prefixes which write something are unchanged.

The reg-test redirect-self-path.vtc covers it.

This should be backported to all stable branches.

BUG/MINOR: sample: reject a \0 byte in raw string fetches

These fetches return a string pointing to the raw client data, which may
contain a \0 byte, while string consumers rely on it not happening
(37204a71a). The pattern tree lookup used by case-sensitive "-m str"
matches and by maps stops at this byte, so "user\0admin" matches "user",
while the next hop sees the whole value. A rule based on these fetches
can then be bypassed.

req.ssl_sni and req.rdp_cookie now fail on such a value, and
req.ssl_alpn and req.body_param skip it and report the next one.
Legitimate values never contain a \0.

The reg-test str_embedded_nul.vtc covers it.

This should be backported along with 37204a71a.

BUG/MINOR: sample: fix host_only and port_only on empty input

sample_conv_host_only() and sample_conv_port_only() scan backwards from
area + data - 1. With an empty sample whose area is NULL, as returned by
req.body for a request without a body, this pointer is (char *)-1, the
first dereference faults and the worker dies (SIGSEGV, also in release
builds). A configuration applying these converters to req.body can thus
be stopped by a plain "GET /".

Both converters now return early on an empty input, with an empty string
and 0 respectively.

This should be backported to 2.8 and later.

BUG/MINOR: h3: prevent missing request FIN on empty trailers

Since the following patch, HTTP/3 trailers are only decoded when the
stream HTX buffer is empty, with previous data already consumed by the
upper layer. This is the simplest reason to ensure there is always
enough space for trailers decoding.

  8020cefc30ceadf61b45130609b90fdbb0741840
  BUG/MEDIUM: h3: prevent conn failure on trailers decoding without space

However, this check is also performed for an empty trailers frame, which
is unnecessary and thus suboptimal. Worse, QCS demux is blocked but when
it is woken up there is nothing left to decode as the empty trailers
frame header was already consumed and the demuxer starts on a new frame.
The trailer frame and its FIN signal are thus dropped.

To fix this, simply bypass the check on HTX buffer if the trailers frame
is empty. EOM is then reported and the rest of the frame parsing is
skipped.

Note that h3_resp_headers_to_htx() has a similar check. However, an
HTTP/3 headers frame cannot be empty as it must contain mandatory
pseudo-headers, thus an empty headers frame is rejected earlier.

This must be backported up to 2.8.

BUG/MINOR: quic: remove CID on BE conn alloc failure

A QUIC backend connection is instantiated in qc_conn_init() via
xprt_quic. First, a random CID is generated and inserted into the tree.
On success, the quic-conn instance is allocated.

If the allocation fails, the CID is freed but not removed from the
global CID tree. This may cause a UAF issue later. Fix this by removing
the CID from the tree prior to freeing it.

This must be backported up to 3.3.

BUG/MINOR: cache: skip already resolved filters when resolving caches

This is a latent bug with no visible impact today: when a proxy uses
several caches, resolving the second cache's name reads the first cache's
structure as if it were a string.

post_check_cache() resolves the caches one after the other, and for each
of them walks the cache filter configurations of all the proxies with
cache_postcheck_flt(), which compares the cache's name with cconf->c.name.
But c is a union: once a configuration is resolved, CACHE_FLT_INIT is set
and c.cache points to the cache, whose structure is then compared as a
string with the name of the next cache. This currently reads the first
bytes of the cache's first tree root, which are NULL at boot, so it never
matches, but any change to the layout of struct cache or to the moment
this runs could make it match or read past the structure.

The configurations already resolved, i.e. flagged with CACHE_FLT_INIT, are
now skipped.

This patch may be backported. If so, it must be adapted because this part
was reworked on 3.5.

BUG/MEDIUM: mux-h1: Report EOS when a demux error is handled by the app layer

Since 30ca6e01 ("BUG/MEDIUM: mux-h1: Don't close the connection on a payload
parsing error"), a demux error is reported as EOI on the SE when the stream
connector is still attached, leaving the app layer handle the error via the
htx flags and the termination events. But this is not enough for consumers
waiting for more data, such as health checks: tcpcheck_eval_recv() loops
while neither EOS nor ERROR is reported, and the mux does not provide any
data anymore after a demux error. Thus a health check waits until its
timeout and reports an L7TOUT instead of an immediate invalid response.
This is especially annoying because a server returning random data instead
of a valid HTTP response is reported as a timeout.

To fix the bug, SE_FL_EOS is now also reported on the SE. It does not carry
the error itself, which is still reported via the termination events and
consumed via the HTX flags. It only reports that no more data will be
received, which is true because the demux error is sticky. This is also
what mux-h2 does (h2s_propagate_term_flags()). The 400-response path in
streams is preserved because the close forwarding is gated by SC_FL_NOHALF,
which is never set on the front stream connector.

This fix must be backported as far as 2.6, but without 30ca6e01, which is
not needed on older versions. There, the demux error is always reported as
EOI on the SE and SE_FL_ERROR is never set. On 3.0 and 2.8, SE_FL_EOS is
only reported when there is no stream connector attached to the h1s, it
must also be reported when the stream connector is still attached. On 2.6,
SE_FL_EOS is never reported, it must be set in addition to SE_FL_EOI.

BUG/MEDIUM: fd: Mark a fd as dead on early close

Commit ad082111a04b4387f8aa76e386a2f340f9583bd2 made it so
conn_ctrl_close() will close a fd, if no xprt was created, because
nobody else would do it for us. But this is not entirely true, some code
paths, such as the checks, if connection failed, will take care of
closing the fd already, and doing it in conn_ctrl_close() will lead to a
second close, that may close an unrelated fd.
Fix that by changing the fd to DEAD_FD_MAGIC so that conn_ctrl_close()
knows the fd has already been closed, and don't have to do it.
It would probably be cleaner to just remove those calls to close(), and
always let conn_ctrl_close() do it, but for that we'd have to make sure
conn_ctrl_close() is always called, and that close to the release, I'd
rather go the safer way.

This should be backported to all supported versions, when
ad082111a04b4387f8aa76e386a2f340f9583bd2 has been backported already.

This should fix github issue #3573.

BUG/MINOR: init: fix missing error on no listener on config check

Since the conversion of the list of proxies, sinks and logs from a plain
linked list to a full struct list type, config check would not report an
error anymore if no listener is present.

This is caused because iterations are now performed via
list_for_each_entry() without producing a final NULL iterator value. Fix
this by using a plain boolean set during the loop and check its value at
the end instead of inspecting the iterator itself.

This should fix github issue #3567.

No need to backport.

BUG/MINOR: qmux: remove invalid BUG_ON() on empty record

In QMux, a BUG_ON() was present in receive path to report about a
realign needed but not implemented. This is not necessary anymore as
realign is now implemented.

Worst, BUG_ON() could be triggered when receiving an empty record. Such
records are not (yet?) prohibited by the spec. Simply remove the
BUG_ON() to avoid any issue with it.

This must be backported up to 3.4.

BUG/MINOR: quic: fix ACK_ECN parsing after optional ack ranges

QUIC acknowledgements are conveyed in ACK and ACK_ECN frame types. The
latter shares a similar structure, but also contains 3 varints at the
end related to explicit congestion notification.

The following patch differentiates ACK and ACK_ECN to ensure the latter
is correctly parsed.

  7aa839296de9e6d330c6a952ae2197aa49db1c66
  BUG/MEDIUM: quic: fix ACK ECN frame parsing

However, ACK_ECN frame parsing is incorrect if the optional ack range
field is present. This patch fixes it by restoring a common parsing
function for ACK and ACK_ECN, which extracts only the first fields prior
to the optional ack ranges. ECN fields are now parsed later, at the end
of qc_parse_ack_frm(), after the optional ack ranges.

This must be backported up to 2.6.

REGTESTS: cache: cover the object lifetime with a single script

The fixes about the object lifetime have no dedicated script each: the
reg-test reg-tests/cache/lifetime.vtc covers the whole lifetime
computation:

  - the lifetime comes from s-maxage, max-age, Expires, or falls back to
    the cache's max-age when the response has a validator (heuristic
    lifetime);
  - a null lifetime, written as "max-age=0", "max-age=00", a quoted
    "max-age=\"0\"" or a null s-maxage, prevents the response from being
    served from the cache;
  - the Age received from the server reduces a relative lifetime but not
    an absolute one given by Expires;
  - "Age: 0" is a valid value, it does not prevent the response from being
    cached;
  - the Age of a copy delivered by the cache stays below the lifetime
    announced by the server;
  - the Age header of a stored response is still forwarded to the client
    the response is delivered to, and the copies carry a single, freshly
    computed Age header.

All the checks are made from the response headers, without any delay: the
X-Age-Relative header is set by an http-after-response rule on the
responses delivered from the cache whose Age reaches the max-age
announced by the server, which can only happen when the entry lifetime
was not reduced by the Age. X-Age-Count reports the number of Age headers
to check the forwarded response and the copies.

The script fails when any of the covered fixes is reverted, and it runs
in a few milliseconds.

BUG/MINOR: cache: do not ignore null lifetimes not written as "max-age=0"

A response carrying "Cache-Control: max-age=00", "max-age=\"0\"" or
"s-maxage=00, max-age=60" is delivered from the cache for the whole
max-age of the cache (60 seconds by default) or for the max-age, while
its null lifetime means it must not be reused without revalidation.
Clients thus receive outdated contents. Such forms are unusual but valid
(RFC 9111#1.2.2 and #5.2 allow leading zeroes and quoted values).

http_check_response_for_cacheability() only recognizes the exact forms
"max-age=0" and "s-maxage=0" as null lifetimes. Other forms are taken as
freshness information, and then http_calc_maxage() only considers values
strictly greater than zero, so a null value is ignored and the next
source of lifetime is used: max-age after s-maxage, Expires, or the
cache's max-age.

http_calc_maxage() now uses s-maxage and max-age as soon as they were
found, including when they are null, and the store action does not store
a response whose remaining lifetime is null, since it would already be
expired. This last check also avoids storing responses with an Expires
date in the past, which were stored expired.

This patch may be backported to all supported versions. It depends on
the previous fixes to http_calc_maxage() ("BUG/MINOR: cache: reduce the
max-age lifetime by the response's Age" and "BUG/MINOR: cache: compare
the Age to the heuristic lifetime when none is given"), but only for
context, the change is easy to adapt.

BUG/MINOR: cache: compare the Age to the heuristic lifetime when none is given

A response carrying a validator (Last-Modified or ETag) but no explicit
lifetime (no max-age, s-maxage nor Expires) is normally stored for the
cache's max-age. But as soon as such a response carries an Age header
with a non-null value, which is the case of all responses coming through
an upstream cache, it is never stored, which only affects performance and
the load of the servers.

When the response has no explicit lifetime, http_calc_maxage() returns the
cache's max-age but leaves the true max age untouched, i.e. zero. The
store action then refuses the response because its Age is greater than
this "true max age" of zero, as if it was stale.

When no explicit lifetime is found, the cache's max-age is used as a
heuristic lifetime (RFC 9111#4.2.2), so it is now also reported as the
true max age, and as a lifetime relative to the response's generation,
like max-age, so that the Age is deducted from it by the previous fix. A
response whose Age exceeds the cache's max-age is still refused.

This patch may be backported to all supported versions, along with the
previous one ("BUG/MINOR: cache: reduce the max-age lifetime by the
response's Age") on which it depends.

BUG/MINOR: cache: reduce the max-age lifetime by the response's Age

A response received with "Cache-Control: max-age=3" and "Age: 2" is kept
fresh in the cache for 3 seconds, and delivered with an Age reaching 4 or
5 seconds, while it only had one second of freshness left. In general,
responses coming through upstream caches are kept fresh for as long as
their Age in excess, at most the cache's max-age, which delivers stale
contents to clients, against RFC 9111#4.2. Clients may see outdated
versions of resources for longer than what the origin server specified.

The expiration date of an entry is computed as the reception date plus
the lifetime returned by http_calc_maxage(), capped by the cache's
max-age. The Age header is parsed afterwards, only to refuse responses
whose Age already exceeds their lifetime, and to compute the Age header of
the copies. But a lifetime given by max-age or s-maxage counts from the
generation of the response at the origin, so the time already spent in
upstream caches, given by Age, must be deducted from it. A lifetime given
by Expires is an absolute date and must not be reduced.

http_calc_maxage() now also reports through a new argument whether the
lifetime comes from s-maxage or max-age, and in this case the effective
lifetime is reduced to what remains after deducing the Age.

This patch may be backported to all supported versions.

BUG/MINOR: cache: do not eat the request while waiting for a buffer

The cache applet's I/O handler leaves through the "exit" label when it
cannot allocate its input or output buffer, when one of these buffers is
still pending or when the output buffer is full. But "exit" eats the whole
request with b_reset(&appctx->inbuf), and the request can still be needed on
the next call. For instance, in the HTX_CACHE_HEADER state, the handler
reads the request start line to know whether it is a HEAD request.

The input buffer is empty at this point, and appctx_release_buffers() also
frees it while it has no data, so http_get_stline() may return NULL and the
process crashes. This requires the output buffer allocation to fail exactly
after the request was received, which is rare since the buffers reserve
covers most transient failures, hence a latent bug.

Add a dedicated "wait" label for these paths: it flushes the pending HTX
message to the output buffer and returns without touching the request.  The
"exit" label is kept for the states where the request must be consumed:
EOS/error, fast-forward blocked by the consumer, and the end of the
response.

This was introduced when the applet got its own buffers by commit 863417292
("MAJOR: cache: Update HTTP cache applet to handle its own buffers") in
3.0-dev3, so the fix should be backported to 3.0.

A debug patch forcing a single output buffer allocation failure on the
first call in the HTX_CACHE_HEADER state reproduced the crash with ASAN
on the htx_sl_req_meth() line; with this fix, the GET and HEAD requests
all succeed and the process stays alive.

BUG/MINOR: cache: recognize null qvalues with decimals in Accept-Encoding

With "process-vary on", a client refusing an encoding with a null weight
written with decimals, such as "Accept-Encoding: gzip;q=0.0, deflate", may
be delivered a cached gzip-encoded response, that it explicitly refused
and may not be able to decode. Only the client issuing such a request is
affected, it receives an encoding it cannot process.

The accept-encoding normalizer builds a bitmap of the accepted encodings,
from which the ones with a null weight are removed. A null weight was only
detected when the parameter was exactly "q=0", while RFC 9110#12.4.2
defines a qvalue as ( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] ),
so "q=0.", "q=0.0", "q=0.00" and "q=0.000" are null weights as well. They
were considered as non-null, the encoding was accepted and a cached
response with this encoding matched the request's secondary key.

The weight is now considered null when "q=0" is only followed by an
optional dot and zeroes.

The reg-test reg-tests/cache/vary_accept_encoding.vtc is extended: it
stores a gzip response then sends requests refusing gzip with "q=0.0" or
"q=0.000". It fails without the fix since the cached gzip response is
delivered.

This patch may be backported to all supported versions.

BUG/MINOR: cache: do not remove the Age header from the forwarded response

When a response carrying an Age header is stored in the cache, the header
is removed from the response itself, so the client which triggered the
storage receives the object without its Age header, as if it had just
been produced by the origin server, while it may have been sitting in an
upstream cache for a long time. This client may then keep it fresh for
longer than allowed. Worse, when the storage finally fails after this
point (headers too large, unknown content-encoding with Vary, no room in
the cache...), the header is lost although nothing was stored. Copies
delivered from the cache are not affected, they carry a freshly computed
Age header.

The Age header was removed with http_remove_header() from the live
response, just so that the headers copied into the cache entry do not
contain it, since the applet adds its own when delivering the object.
Instead of altering the response, the Age headers are now simply skipped
when the headers are copied into the entry. The ETag offset is computed
from the copied data so it remains correct. This also removes all the Age
headers from the stored copy and not only the first one, so a copy is now
always delivered with a single Age header.

Thi patch fix may be backported to all supported versions. In versions
before 3.5, the removal is done unconditionally (no hints-only mode) but
the fix is the same.

BUG/MINOR: cache: do not refuse to store responses with "Age: 0"

Responses carrying an "Age: 0" header are never stored in the cache. This
header is commonly emitted by an upstream cache or CDN delivering an object
it just fetched, so in a setup chaining caches, the haproxy cache simply
never stores these objects and all requests for them reach the servers,
which only affects performance and the load of the servers.

The Age value is parsed with strl2llrc() and only accepted when strictly
positive, any other case jumping to the "out" label as an invalid value.
Zero is a perfectly valid Age value (RFC 9111#5.1), it is not stale and
needs no special treatment, so it is now accepted like positive values.
Negative or non-numerical values are still considered invalid.

This patch may be backported to all supported versions.

BUG/MEDIUM: filters: order the implicit filters around the explicit ones

In the legacy filter mode, flt_reorder_implicit_conf() reordered the
implicitly declared core filters but pinned the explicitly declared core
filters in their declaration position, treating them as segment
boundaries. An implicit filter bucketed after an explicit core filter
ended up in a wrong position: with an explicit "filter compression"
declaration followed by cache rules, the implicit cache-store filter
was placed after the compression filters, and the configuration checks
reported bogus errors:

    proxy 'li': unable to enable the compression filter on response
                before any cache filter.
    proxy 'li': unable to enable the compression filter before the
                cache 'c'.

while this configuration is valid: the implicit cache filter must be
placed at its canonical position, before the explicit compression
filters.

Explicitly declared core filters are now placed in the "any other
filter" class, like any explicitly declared filter: their manual
placement among the explicit filters is the user's responsibility, and
only the implicitly declared core filters are moved to their canonical
position. The placement policy is implemented in flt_order_class(),
which returns the "any other filter" class for an explicit core filter.

The order checks reporting conflicts between the compression,
decompression and cache filters are updated accordingly: an order
error is only reported when the filters involved are explicitly
declared. A filter that is automatically ordered never conflicts with
an explicitly declared one, since the reordering places them correctly
relative to each other.

No backport needed.

BUG/MEDIUM: cache: forbid cache rules in defaults sections

A "cache-use" or "cache-store" rule declared in a defaults section has
never worked: the implicit cache-store filter it creates is attached to
the defaults section itself, where the "filter" directive is not
supported. The filter is never resolved, and the rule inherited by the
proxies references a configuration pointing at an unresolved cache name
instead of a cache, which leads to crashes or silently broken caching
depending on the filter mode of the proxies.

Supporting these rules with the legacy filters would require a deep
rework of the rule-to-filter binding, hard to backport. So, until the
new filter API covers this case, the rules are simply forbidden in a
defaults section, with an explicit alert:

    [ALERT] : Proxy 'd': 'cache-use' rules are not supported in a defaults section.

This patch must be backported to all supported versions.

BUG/MEDIUM: cache: do not create a second applet when two cache-use rules hit

A configuration with two "http-request cache-use" rules that may both find
the requested object, for example two caches both fed by unconditional
"cache-store" rules, or a cache-use rule in a defaults section that is
repeated in the proxy, leaks memory and cache space on every request that
hits both of them. Each such request leaks one appctx and its task (about
1kB on x86_64) and leaves the row of the first cache's entry retained
forever. The memory grows with the number of requests until the process is
restarted, which affects the system owner, and the rows retained this way
can never be evicted anymore: once their entry expires, their blocks are
lost and the cache progressively shrinks, until it cannot store anything.
Any client may trigger this by simply requesting cached objects. With
2000 requests, "show pools" reports 2016 appctx used and "show cache"
reports a refcount of 2000 on the entry, which stays like this after all
the streams are gone.

The cache-use action is not a final one. On a hit, it sets s->target to
the cache applet and creates the applet on the back stream connector with
sc_applet_create(), after retaining the entry and detaching its row. The
next cache-use rule is still evaluated and, when it also hits, it calls
sc_applet_create() again on the same stream connector. The new appctx
takes the same sedesc, so the first one is not referenced anymore: it was
woken up once, found no request in its input buffer and waits for one
forever. Its release callback is never called, so the entry it retained is
never released and its row never reattached to the avail list.

Let's simply ignore the cache-use rule when the stream already has a
target: the response will be produced by the applet that is already
there. This is consistent with the stats applet that is only set up when
s->target is NULL. An alternative would be to make the action final on a
hit, but this would change the evaluation of the following rules for all
users. Note that "use-service" has the same weakness, it creates its
applet without checking whether one is already attached, so a cache hit
followed by a matching use-service rule could leak the cache applet the
same way. This one is left as is since use-service is a final action and
mixing it with a cache hit is not a sensible configuration.

This patch should be backported to all supported versions. In older versions,
http_action_req_cache_use() may register the applet with a different
function (si_register_handler(), stream_int_register_handler()) but the
fix remains the same: return ACT_RET_CONT at the beginning of the function
when s->target is set.

BUG/MEDIUM: cache: fix "show cache" crash/skip with several caches

With "show cache" cli command, a crash could be experienced when several
caches are configured. This happened when iterating on the second cache in
the list. Indded the cache tree array was not reset when the dump of a cache
was finished, leading to computing the cache tree index against the wrong
cache tree and to lock the wrong cache tree.

This is a regression introduced by the resumable CLI output.

Fix it by clearing ctx->cache_tree when the dump of a cache finishes: the
pointer must only be non-NULL while the dump is paused on the cache it
belongs to.

This patch must be backported to all supported versions.

BUG/MAJOR: cache: do not mix variants varying on different headers

With "process-vary on", a variant of a resource stored for one client may
be delivered to other clients it was not meant for. This happens when the
server does not always vary on the same headers for a given resource,
which is very common: CORS implementations typically add "Vary: origin"
only when the request carries an Origin header. If a first variant varying
only on accept-encoding is stored, then a client sending
"Origin: https://evil.example" gets a response varying on
"accept-encoding, origin" with "Access-Control-Allow-Origin:
https://evil.example", this response is stored with a null hash for the
origin, and is then delivered from the cache to every client whatever its
Origin header. Any client can thus poison the cache for all the other
clients of the resource, breaking the CORS checks of legitimate sites
(availability of the service for these users) or, with Referer, mixing
contents meant for different referrers. The same may happen without any
malicious intent.

When an entry already exists for the primary key, cache-use builds the
request's secondary key only for the headers of the first entry's
signature, the hashes of the other headers being zero-filled. It then
compares this key with each variant. On a miss, the same key is kept in
the transaction and reduced with the response's signature by
cache-store, which expects it to be complete, so the new variant is
stored with zeroes for the headers that were not part of the first
signature. Subsequent lookups build their keys the same way, so the
zeroes match whatever the request's Origin or Referer.

The fix consists in always building the complete secondary key on lookup,
as is already done when there is no entry yet, and in reducing it to the
signature of each variant before comparing it to the variant's key, which
is how the variant's key was built when it was stored. This is done by a
new function, secondary_key_entry_cmp(), used by get_secondary_entry(). A
variant varying on Origin is thus only delivered to requests carrying the
same Origin, and variants varying on different headers can all be found.
On the store side, get_secondary_entry() is passed the key already
reduced to the response's signature, so reducing it again to the
signature of an existing variant does not change the comparison for a
variant with the same signature. Note that a request with a duplicate
Origin or Referer header now never uses the entries of a resource which
has variants, as it already happened when the resource had no entry yet.

The reg-test reg-tests/cache/vary_signature_change.vtc reproduces the
scenario above with an "evil" and a "good" origin and checks that each
variant is delivered to the right requests only. Without the fix, the
request with the good origin receives the evil variant.

This exists since the Vary support was introduced by commit 1785f3dd9
("MEDIUM: cache: Add the Vary header support") in 2.4-dev2, so the fix
must be backported to all supported versions. The severity is set to
MAJOR because this is a cache poisoning that any client can trigger, but
it requires "process-vary on", which is not the default.

BUG/MEDIUM: mux-fcgi: put BE conn back in idle list after deferred shut

When a stream cannot send its FCGI_ABORT record or its end of stream at
detach time because the mux buffer is busy or full, fcgi_detach() leaves
early to keep the stream as an orphan. This skips the idle/avail list
management performed at the end of the function. The orphan is later
destroyed by fcgi_deferred_shut() once the record can be sent, but nothing
makes the connection idle or available again at this point. The timeout is
not refreshed either.

If the orphan was the last stream of the connection, the connection is then
in no idle list, it is not released and it remains accounted in
srv->curr_used_conns. Its timeout is the one armed when the stream was
orphaned, so the connection is closed after "timeout server" instead of
being reused, and it is wasted in the mean time.

To fix the issue, the idle/avail list management is extracted from
fcgi_detach() into a new fcgi_may_idle_conn() function, which is now also
called from fcgi_deferred_shut() after the orphan was destroyed and the
timeout was refreshed, as fcgi_detach() does.

In fcgi_deferred_shut(), the stream's session cannot be used, since it may
have been released in the mean time. The connection owner is used instead,
but only if the connection is still attached to its session, because
session_free() resets conn->owner only in this case. For a private
connection, if there is no session anymore when the last stream leaves, the
connection can no longer be reused and it is destroyed.

The orphans destroyed from fcgi_strm_wake_one_stream() are not covered here.
This mostly happens on connection errors or shutdowns, in which case the
connection is released anyway.

This is the same fix as the one applied on mux-h2 by the commit
"BUG/MEDIUM: mux-h2: put BE conn back in idle list after deferred shut".

This could be backported to all supported versions, after the recent
fcgi_detach() fixes: a167d39a5 ("BUG/MEDIUM: mux-fcgi: arm the connection
timeout only when data remains"), 170a6469c ("BUG/MEDIUM: mux-fcgi: ignore
conn->owner when deciding if a connection is dead"), b77b044d3 ("MINOR:
mux-fcgi: check for a dead connection before refreshing the timeout") and
2acf116bf ("MINOR: mux-fcgi: don't add a closed connection to an idle list
on detach").

MINOR: mux-spop: don't add a closing connection to an idle list on detach

In spop_detach(), the test deciding whether the connection may be moved to
an idle list checks SPOP_CF_RCVD_SHUT, SPOP_CF_ERR_PENDING and
SPOP_CF_ERROR, but not the connection state. A connection that already
switched to SPOP_CS_ERROR, SPOP_CS_CLOSING or SPOP_CS_CLOSED may thus be
inserted in the session server list or in the server's idle pool, while
spop_avail_streams() reports no available stream for it.

This can only happen with a private connection, since srv_add_to_idle_list()
already rejects connections without available streams. It is not harmful
either, since session_get_conn() also requires an available stream, so such
a connection is never reused. But it is still wrong to park it: it prevents
the session from dropping it and forces a useless idle counter increment.

This is the same fix as the one applied on mux-fcgi by commit 2acf116bf
("MINOR: mux-fcgi: don't add a closed connection to an idle list on
detach"). Unlike FCGI, all states from SPOP_CS_ERROR are excluded, which
matches the test performed by spop_avail_streams().

This may be backported to 3.1.

BUG/MEDIUM: mux-spop: refresh the connection timeout before parking it idle

spop_detach() refreshes the connection timeout in two places only: in the
early return taken when the stream is blocked, and at the very end of the
function. The branches that move the connection to an idle list (the
session's private list or the server's idle pool) return before reaching
the tail. Since the timeout is disabled while streams are attached, a
connection detached with data still queued in the mux buffer is thus parked
without any timeout and nothing drains it. It is only released by the purge
of the idle connections.

The refresh is now performed right after spop_strm_destroy(), before the
connection may be given away, and the call at the end of the function is
dropped: everything between the two points only manipulates session and
idle lists, so it recomputed the very same expiration date.
spop_conn_update_timeout() already returns early when there is no task, so
no guard is needed at the call site. Note that an idle connection without
pending data now also gets the default timeout when it is parked, as it is
already the case each time it is processed by spop_process().

The dead connection check is moved ahead of the refresh, so that a
connection about to be released is not armed and requeued just before
spop_release() clears its task context. This only reorders statements:
spop_conn_update_timeout() touches neither the stream tree nor the flags
and states spop_conn_is_dead() looks at. And since the previous commit
removed the conn->owner test from spop_conn_is_dead(), clearing conn->owner
in the non-private branch below has no effect on the predicate.

This is the same fix as the one applied on mux-h2 by commit 5e7bb2063
("BUG/MEDIUM: mux-h2: refresh the connection timeout before parking it
idle") and on mux-fcgi by commits a167d39a5 ("BUG/MEDIUM: mux-fcgi: arm the
connection timeout only when data remains") and b77b044d3 ("MINOR:
mux-fcgi: check for a dead connection before refreshing the timeout").

This should be backported to 3.1 with the previous commit.

MINOR: mux-spop: ignore conn->owner when deciding if a connection is dead

spop_conn_is_dead() reports a connection as dead when it has no stream left
and conn->owner is NULL, on the assumption that an ownerless connection has
nobody left to take care of it. This no longer holds since idle backend
connections are stored in the server and may legitimately have conn->owner
NULL while still being usable. The same test was removed from h2c_is_dead()
by commit a0541f5d2 ("BUG/MEDIUM: mux-h2: ignore conn->owner when deciding
if a connection is dead") and from fcgi_conn_is_dead() by commit 170a6469c
("BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is
dead").

In the SPOP mux, spop_conn_is_dead() is only called at the end of
spop_detach(), which is never reached for an idle connection moved to an
idle list, and the stream test makes the predicate false as long as streams
remain. So the test has no visible effect for now. But it prevents the dead
connection check from being performed before the idle list management, the
non-private branch clearing conn->owner before parking the connection. So
let's remove it.

This may be backported to 3.1.

BUG/MEDIUM: mux-spop: do not enforce the timeout on connections with streams

spop_conn_update_timeout() leaves the connection task unarmed while stream
connectors are attached, since the timeouts are then enforced by the upper
layer. But spop_attach() does not touch the task, so a connection which got
a timeout while it had no stream, for instance because of pending output
data or because of the default timeout applied to connections without
stream, keeps it once a new stream is attached. When it strikes,
spop_timeout_task() kills the connection and aborts the streams attached
to it, while nothing indicates that the agent is not responding.

As done for the H2 mux by commit c2ea47fb1 ("BUG/MEDIUM: mux-h2: do not
enforce timeout on long connections") and for the FCGI mux by commit
6e3bad2f3 ("BUG/MEDIUM: mux-fcgi: do not enforce the timeout on connections
with streams"), spop_timeout_task() now does not kill the connection when
stream connectors are still attached, it only disarms the timeout.

This should be backported to 3.1.

BUG/MEDIUM: mux-h2: put BE conn back in idle list after deferred shut

When a backend stream cannot send its RST_STREAM or ES at detach time
because the mux buffer is busy or full, or because the connection window
is exhausted, h2_detach() leaves early to keep the stream as an orphan.
This skips the idle/avail list management performed at the end of the
function. The orphan is later destroyed by h2_deferred_shut() once the
frame can be sent, but nothing makes the connection idle or available
again at this point.

If the orphan was the last stream of the connection, the connection is
then in no idle list, it is not released and it remains accounted in
srv->curr_used_conns. Its timeout is not armed either, because a backend
connection without stream only relies on idle-ping. So it cannot be
reused and it stays there until the server closes it or an error occurs,
holding an FD and preventing the server from being deleted.

To fix the issue, the idle/avail list management is extracted from
h2_detach() into a new h2_may_idle_conn() function, which is now also
called from h2_deferred_shut() after the orphan was destroyed and the
timeout was refreshed, as h2_detach() does.

In h2_deferred_shut(), the stream's session cannot be used, since it may
have been released in the mean time. The connection owner is used instead,
but only if the connection is still attached to its session, because
session_free() resets conn->owner only in this case. For a private
connection, if there is no session anymore when the last stream leaves,
the connection can no longer be reused and it is destroyed.

The orphans destroyed from h2s_wake_one_stream() are not covered here.
This mostly happens on connection errors or shutdowns, in which case the
connection is released anyway.

This bug exists since orphaned streams are destroyed from
h2_deferred_shut() with backend connections reuse. It could be backported
to all supported versions. Note that 5e7bb2063 ("BUG/MEDIUM: mux-h2:
refresh the connection timeout before parking it idle") reworked the
h2_detach() tail, so it should be backported first. However, it could be
good to wait for a period of observation first.

BUG/MINOR: tcpcheck: fix out-of-bounds reads on short MySQL error packets

tcpcheck_mysql_expect_packet() only verifies that the packet header is
present before reading inside it.

This covers the 3 length bytes and the sequence id, but not the packet type
byte read at offset+4 right after. Then the whole packet is checked against
the buffer, which only guarantees that 4 bytes past offset were received.

When the packet announces a length lower than 4, the error branch below is
entered even though the bytes it reads were never received: it reads the
error code at offset+5 and offset+6 and uses offset+7 as the start of the
message body, while the body length is computed as b_data()-offset-7, which
wraps around when b_data() is lower than offset+7. A 5-byte response, i.e. a
packet announcing plen == 0 followed by a 0xff byte, is enough to trigger
this.

The wrapped length does not lead to a copy today because chunk_istcat()
refuses to append more than b_room(), but the reads still happen and the
error code and status reported are garbage.

Thus, first make sure the packet type byte is available before reading it,
and then reject error packets announcing less than 4 payload bytes, the
minimum for the packet type, a 2-byte error code and at least one byte of
description. As a consequence, such packets are now reported as a Layer7
invalid response instead of a Layer7 wrong status with a bogus error code,
and a response carrying only the 4 header bytes is now considered incomplete
and waits for more data instead of being parsed from bytes that were never
received. Truncated responses keep waiting for more data as before.

This bug is present since 2.2, so it should be backported to all
maintained branches.

Co-authored-by: QiuYitai <74493355+QiuYitai@users.noreply.github.com>

[RELEASE] Released version 3.5-dev12

Released version 3.5-dev12 with the following main changes :
    - CLEANUP: quic: compute the STREAM frame end offset once in qc_stream_frm_is_acked()
    - CLEANUP: quic: fix and complete the comments in quic_retransmit.c
    - CLEANUP: quic: remove a redundant test in the fast retransmission lookups
    - CLEANUP: quic: use QUIC_PACKET_PN_MAXLEN instead of the magic 4
    - CLEANUP: quic: remove the dead encryption level variable in qc_prep_hdshk_fast_retrans()
    - CLEANUP: quic: drop the intermediate packet tree pointer in the fast retransmissions
    - CLEANUP: quic: remove the duplicate qc_prep_hdshk_fast_retrans() prototype
    - REGTESTS: protobuf: cover the ungrpc message realignment
    - BUG/MINOR: fd: allow fd_write_frag_line() to run before fdtab allocation
    - BUG/MINOR: http/hardening: don't count unregistered hdr/tlr too early
    - MINOR: http/hardening: turn it to a macro and count matches with COUNT_IF()
    - BUG/MEDIUM: stream: reset analyse_exp when analysers are stripped on error
    - BUG/MINOR: http-ana: also set FINAL on http-response actions on server error
    - BUG/MEDIUM: spoe: stop waiting on a dead producer channel
    - BUG/MINOR: tcpcheck: check the number of "hdr" in http-check send
    - MINOR: tcpcheck: add post-80-sha2 mode to mysql-check for caching_sha2_password
    - BUG/MINOR: stick-table: reject invalid gpc/gpt array indexes
    - MEDIUM: h1/violations: restrict valid version strings to "1.0" and "1.1"
    - BUG/MEDIUM: filters: avoid NULL block access in decompression
    - BUG/MEDIUM: mux-fcgi: do not enforce the timeout on connections with streams
    - BUG/MEDIUM: quic: fix loss time selection in quic_loss_pktns()
    - BUILD: http: fix build of macro http_harden() with clang
    - MINOR: debug: add a COUNT() macro which doesn't take a condition
    - CLEANUP: debug/tree-wide: replace COUNT_IF(1, ...) with COUNT()
    - BUG/MINOR: applet: accept a temporarily owned tid in appctx_init()
    - BUG/MINOR: server: fix a leak when completing a named default-server
    - DOC: minor: update VTest2 URL everywhere
    - BUG/MEDIUM: compression: use a trash chunk as large as the buffer for the output
    - BUILD: debug: fix _COUNT undefinied with USE_OBSOLETE_LINKER
    - BUG/MEDIUM: mux-h2: consider the send list as pending data for the timeout
    - BUG/MEDIUM: mux-h2: refresh the connection timeout before parking it idle
    - BUG/MEDIUM: mux-fcgi: arm the connection timeout only when data remains
    - BUG/MEDIUM: mux-fcgi: detect an early connection error during the handshake
    - BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is dead
    - MINOR: mux-fcgi: check for a dead connection before refreshing the timeout
    - MINOR: mux-fcgi: don't add a closed connection to an idle list on detach
    - BUG/MEDIUM: mux-h2: also consider the fctl_list as pending data for the timeout
    - BUG/MEDIUM: ssl: don't use MSG_MORE for kTLS control records
    - MINOR: cli: factorize CLI session reset
    - MINOR: cli: store active CLI sessions in a global list
    - MINOR: cli: implement basic mechanism for uninterruptible jobs
    - MINOR: cli: implement uninterruptible mode for wait
    - MINOR: ring: support inactivity timeout in show ring CLI handler
    - MINOR: sink: add timeout argument for "show events"
    - MINOR: cli: add "kill" CLI command
    - REGTESTS: cli: check uninterruptible mode
    - CLEANUP: mux-h2: clarify that bptr is not NULL when calling to h2_get_buf()
    - BUG/MEDIUM: mux-h2: always return already allocated buffers in h2_get_buf()
    - BUG/MEDIUM: mux-h2: permit the buffer callback to allocate despite the queue
    - BUG/MEDIUM: mux-h2: wake the connection up after a delayed mbuf allocation
    - BUG/MEDIUM: mux-h2: serve all pending buffer allocations in the buffer callback
    - BUG/MINOR: mux-h2: unblock the demux when destroying the stream it was blocked on
    - BUG/MEDIUM: mux-h2: don't send RST_STREAM for backend streams whose HEADERS were not sent
    - BUG/MEDIUM: mux-h2: also emit the HPACK dynamic table size update on requests
    - BUG/MINOR: mux-h2: also emit the HPACK Dynamic Table Size Update before trailers
    - BUG/MINOR: mux-h2: only convert GET upgrade requests to Extended CONNECT
    - BUG/MINOR: mjson: fix out-of-bounds reads on malformed JSON paths
    - BUG/MEDIUM: flt-decomp: preserve pending output before decoding again
    - REGTEST: ocsp: make the ocsp_client_check test fast
    - CI: github: switch development builds back to ubuntu-latest
    - CI: github: remove MIPS targets from cross-zoo.yml
    - CI: temporarily disable AWS-LC 3.x (fips-2024-09-27)
    - CI: github: install libcrypt-dev:i386 for the i686 build
    - BUG/MEDIUM: resolvers: detach servers without resolvers from the SRV request
    - BUG/MEDIUM: resolvers: don't ignore records not matching a resolve-net family
    - BUG/MEDIUM: resolvers: skip the data of ignored additional records
    - BUG/MINOR: resolvers: reject a null or negative resolve_retries value
    - BUG/MINOR: resolvers: reject null "timeout resolve" and "timeout retry" values
    - BUG/MINOR: resolvers: preserve nameserver identity on "clear counters all"
    - BUG/MINOR: resolvers: don't dump a nameserver twice in "show resolvers"
    - BUG/MINOR: resolvers: Always init a new resolution's list element
    - BUG/MINOR: resolvers: count nameserver timeouts
    - BUG/MEDIUM: resolvers: reject servers sharing an SRV record with different resolvers
    - BUG/MINOR: resolvers: check the SRV expiration date again under the lock
    - OPTIM: resolvers: only walk duplicates when looking up an existing record
    - BUG/MINOR: resolvers: warn about ignored arguments on datagram nameservers
    - BUG/MINOR: resolvers: fix leaks on do-resolve's fetch availability error
    - BUG/MINOR: resolvers: report a timeout when a TCP fallback gets no response
    - BUG/MINOR: resolvers: use truncated SRV responses when no TCP fallback is possible
    - BUG/MEDIUM: resolvers: properly skip root-named authority and additional records
    - BUG/MINOR: resolvers: compute a name's wire length by walking its labels
    - BUG/MINOR: quic/ssl: record TLS version bounds for certificate selection
    - CI: github: pin ARM development builds to Ubuntu 26.04
    - BUG/MINOR: cfgparse: detect unsupported crypt() hashes on Debian/Ubuntu
    - DOC: configuration: clarify normalize-uri version availability
    - BUG/MINOR: ssl/ckch: fix an invalid free() in the domains/ips error paths
    - BUG/MINOR: tools: ha_freearray() must also free the array
    - CI: github: re-enable AWS-LC 3.x (fips-2024-09-27)
    - BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame
    - BUG/MINOR: quic: reject RETIRE_CONNECTION_ID for a CID not sent to the peer
    - BUG/MINOR: ssl: skip the cipher workaround with AWS-LC API 38 and newer
    - BUILD: Makefile: force $(shell command -v ...) to run through the shell
    - BUG/MINOR: quic: reject invalid stream ID after MUX closure
    - BUG/MEDIUM: quic: limit reject after MUX closure to once per stream
    - BUG/MINOR: quic-be: fix Initial token length check on backend connections
    - BUG/MINOR: mux_quic: remove dangling qc->qcc ref on MUX init failure
    - MINOR: http: add <need_eot> to trailers parsing function
    - MINOR: mux_quic: refactor qcs_http_handle_standalone_fin()
    - BUG/MINOR: h3: fix missing EOM after trailers without FIN
    - BUG/MINOR: h3: close connection on response start-line alloc failure
    - BUG/MINOR: h3: fix error code on incomplete request
    - BUG/MEDIUM: h3: reject standalone FIN on invalid request/response
    - BUG/MEDIUM: threads: don't read lock history entries that don't fit in ulong
    - BUG/MINOR: quic-be: drop long header packets with unsupported version
    - BUG/MINOR: stick-table: release server_key on any stksess destruction
    - BUG/MINOR: acl: check the presence of an argument to "-u"
    - BUG/MINOR: server: don't report dyncookie collision on same IP servers
    - SCRIPTS: create-release: do not match the word "version" isolated
    - MEDIUM: sched: fix the default runqueue-depth
    - DOC: security: clarify how to send proper reports
    - DOC: threat-model: add a few more entries for stuff that should be public
    - DOC: configuration: add a section on secure filtering of HTTP requests
    - MINOR: sample: add SMP_USE__MAY_ALIAS to mark fetches returning aliasable contents
    - MINOR: http-fetch: mark URI-based sample fetches with SMP_USE__MAY_ALIAS
    - MINOR: acl: add acl_cond_find_positive_use() to find positive terms by usage
    - MINOR: http-act: mark http-request access control actions with KWF_ACCESS_CTRL
    - MINOR: http-rules: accept "no-warn" before the condition of access control rules
    - REGTESTS: add a no-warn directive on deny rules using a path
    - MEDIUM: http-rules: report access control rules relying on aliasable contents
    - MINOR: http: add a global setting to decide on warnings for http rules
    - DEBUG: stream: do not emit debug messages for internal proxies
    - MINOR: mux-quic: add an sctl() callback to the application ops
    - MINOR: mux: add sctl MUX_SCTL_GET_HDNG_FOUND/FIXED to report hardening results
    - MINOR: http-fetch: add {fs,bs}.hdng.{found,fixed} to report hardening results
    - MINOR: http-conv: add hdng_rej_name to name the HTTP hardening reject rule
    - REGTESTS: http-messaging: test the reporting of HTTP hardening results

REGTESTS: http-messaging: test the reporting of HTTP hardening results

This new test checks fs.hdng.found, fs.hdng.fixed, bs.hdng.found and
bs.hdng.fixed from a log-format, as well as the hdng_rej_name converter.
The same anomalies (underscore in a header name, unregistered header,
CTL in a trailer) are sent:
  - through the "trusted" profile on both sides: everything passes, and
    only "found" reports them;
  - in a request to a "strict" profile which deletes the two headers and
    rejects the trailer: the 400 is logged with the three rules in "fixed"
    and hdng_rej_name only names the reject. The trailers are sent apart so
    that the stream exists when they are rejected;
  - in a response from a server using the "strict" profile: same with a 502
    on the backend side;
  - in an H2 request with only the deleted headers: nothing is named.

MINOR: http-conv: add hdng_rej_name to name the HTTP hardening reject rule

The hdng_rej_name converter takes a mask of HTTP_HRD_* flags such as returned
by fs.hdng.fixed or bs.hdng.fixed, keeps only the "reject" rules, and
returns the name of the lowest one, as found in http_hardening_kws[]. Since
a reject stops the processing, at most one is normally present. Other rules
are ignored, and the converter fails when no reject rule remains, so that
nothing is logged when nothing was rejected:

    log-format "... cli_hdng=%[fs.hdng.found,htonl,hex],%[fs.hdng.fixed,hdng_rej_name] ..."

MINOR: http-fetch: add {fs,bs}.hdng.{found,fixed} to report hardening results

These new sample fetches return, for the frontend or backend mux stream,
the mask of HTTP hardening conditions that were met (found) and of the
rules that were enforced (fixed). They rely on the new mux stream controls
MUX_SCTL_GET_HDNG_FOUND and MUX_SCTL_GET_HDNG_FIXED, and fail when the mux
doesn't support them. The main purpose is to log them, e.g.:

    log-format "... cli_hdng=%[fs.hdng.found,htonl,hex] ..."

Note that a request rejected before the stream is created cannot be
reported this way since it is handled and logged by the mux alone.

The doc also mentions that the bit numbers are subject to change.

MINOR: mux: add sctl MUX_SCTL_GET_HDNG_FOUND/FIXED to report hardening results

The H1, H2 and H3 streams store in their http_hdng_chk what HTTP hardening
conditions were met (found) and what rules were enforced (fixed). This adds
two new mux stream controls to retrieve them as uint32_t, so that they can
be reported by sample fetches and logged:

  - MUX_SCTL_GET_HDNG_FOUND returns hdng_chk.found
  - MUX_SCTL_GET_HDNG_FIXED returns hdng_chk.fixed

They're implemented in the H1 and H2 muxes. The QUIC mux passes them to
the application layer's sctl() after checking that it's H3, and h3_sctl()
implements them. Other muxes return -1 as they don't support hardening.

MINOR: mux-quic: add an sctl() callback to the application ops

The QUIC mux will need to pass some stream controls to the application
layer, which holds information that the mux doesn't know about (e.g. the
HTTP hardening results of the H3 stream). This adds a new sctl() callback
to qcc_app_ops, which takes the qcs, the mux_sctl_type and an output
pointer, and works like mux->sctl(), returning -1 when not supported.

For now, h3_sctl() doesn't implement anything and the mux doesn't call it
yet.

DEBUG: stream: do not emit debug messages for internal proxies

The debug mode that is triggered via "haproxy -d" remains helpful in
unit tests during development, typically to see HTTP headers being
processed. However, for historic reasons it doesn't make any difference
between service connections and maintenance connections such as peers
or CLI, making it close to useless in environments involving one of
these.

This patch just eliminates logs for sessions coming from an internal
proxy, since this is not something users are expected to deal with nor
want to observe. Thus, it finally becomes possible to use "haproxy -d"
while a monitoring script polls the CLI or when peers are configured.

MINOR: http: add a global setting to decide on warnings for http rules

The default choice of "notice" as level to emit a warning for an
http-request rule relying on unreliable processing may not always be
the best choice. This commit adds a new global "on-unreliable-rules"
directives which takes 3 parameters, "notice" (the default), "warn"
and "ignore". Choosing "warn" will emit a warning instead of a notice,
so that a config using "zero-warning" will detect them. This might be
helpful after a config was sanitized in order to make sure no new
warnings are introduced. And "ignore" may be used to silence all
messages when migrating a config.

MEDIUM: http-rules: report access control rules relying on aliasable contents

Rules such as "http-request deny if { path_beg /admin }" are regularly
reported as bypassable using another form of the same request (case,
%-encoding, "//", "/./", "../", backslashes etc). This is expected, since
these aliases depend on what lies behind haproxy, and the only reliable
way to filter requests is to reject what was not explicitly accepted.

Now when an http-request rule using an action marked KWF_ACCESS_CTRL
(auth, deny, reject, silent-drop, tarpit) has a condition which may be
true because of a match on a sample fetch marked SMP_USE__MAY_ALIAS, it
is reported at boot. Only terms making the condition true when they
match are considered (non-negated after "if", negated after "unless"),
so that allow-lists are not reported. Converters are not considered, as
most of them do not handle all aliases.

Some rules are legitimately used as an extra filter (e.g. blocking %2E or
"<script" in the query string as a complement to other protections). For
these ones, the "no-warn" keyword placed right before "if" or "unless"
excludes the rule from the report. It also indicates to future readers
that the rule is known not to be reliable by itself. It is not made a
warning so as not to cause problems to configs running in zero-warning
mode.

The documentation was updated accordingly.

REGTESTS: add a no-warn directive on deny rules using a path

Historically a number of reg-tests were built with action "deny" and
deny_status followed by a path match in order to distinguish client
requests in the tests. These ones will soon emit warnings, so let's
add a no-warn directive to them.

MINOR: http-rules: accept "no-warn" before the condition of access control rules

The http-request actions marked with KWF_ACCESS_CTRL (auth, deny, reject,
silent-drop, tarpit) now accept the "no-warn" keyword placed right before
"if" or "unless". It is removed from the arguments before the action's
parser is called so that the parsers don't have to know about it. It is
only looked up for these actions so that it cannot be confused with an
argument value of another action (e.g. a header value).

It has no effect for now. It will be used to exclude a rule from the
upcoming report of conditions relying on aliasable contents. It comes
separately so that it can be backported to stable versions along with
the previous commit (KWF_ACCESS_CTRL), allowing configurations which
use it to still parse after a rollback, and so that the reg-tests can
be updated before the report is introduced.

MINOR: http-act: mark http-request access control actions with KWF_ACCESS_CTRL

This new keyword flag indicates that an action is used to block traffic,
and as such that the reliability of its condition matters. It is set on
the "deny", "tarpit", "reject", "auth" and "silent-drop" http-request
actions. It will be used to detect such rules relying on a deny-list of
aliasable contents. "return" is deliberately not marked, as it's used
for many other purposes (e.g. serving local contents).

MINOR: acl: add acl_cond_find_positive_use() to find positive terms by usage

This function looks for a term of a condition which makes the condition
true when it matches (a non-negated term after "if", or a negated one
after "unless") and which involves a sample fetch having certain
SMP_USE_* bits. It returns the first matching ACL and keyword, in the
same way as acl_cond_kw_conflicts() does. The combination of terms is
not considered, a single positive term is enough.

It will be used to detect deny-lists built on aliasable contents.

MINOR: http-fetch: mark URI-based sample fetches with SMP_USE__MAY_ALIAS

The path, the query string and the parameters they convey may be written
in many equivalent forms (case, %-encoding, "+" vs "%20", "/./", "../",
"//", backslashes etc), depending on what lies behind haproxy. Let's mark
the fetches returning or depending on these contents with
SMP_USE__MAY_ALIAS: base, base32, base32+src, baseq, capture.req.uri,
path, pathq, query, req.body_param, url, url32, url32+src, url_param,
urlp and urlp_val.

The host part is left aside for now (url_ip, url_port, Host header), as
it's much less commonly used to deny access, and that it's generally
used the same way to route the traffic.

MINOR: sample: add SMP_USE__MAY_ALIAS to mark fetches returning aliasable contents

Some sample fetches return contents which may be written in multiple
equivalent forms that are interpreted the same way by the next hop, for
example the path with its %-encoding, its case, or its "/./" and "../"
segments. Matching such contents against a pattern to deny requests is
unreliable, and we'll want to be able to detect such constructs at boot.

This adds SMP_USE__MAY_ALIAS, which is not a source but a property of the
fetched contents. It is placed in the ->use field so that it doesn't
require a new field in all fetch keywords, and since this field is
already or'ed into the ACLs and conditions, it will be possible to
quickly check if a condition involves such a fetch.

The double underscore marks the values which are properties of the
contents and not sources. They're not mapped to any SMP_SRC_* value, so
they're ignored when computing the validity domain. They are allocated
from the topmost bit downwards, leaving all the room to the sources
below, and a build time check verifies that the sources never reach
bit 31.

DOC: configuration: add a section on secure filtering of HTTP requests

Deny rules matching a path, an URL or a query string are regularly
reported as bypassable using alternate encodings or aliases (case,
repeated slashes, "." and ".." segments, %-encoding, "%u" encoding,
backslashes, path parameters etc). This is expected: HTTP has many
aliases and which ones apply depends on what is behind haproxy, so no
normalization performed here can be correct in the general case.

The only reliable approach is an allow-list: accept what is explicitly
recognized and reject everything else. Let's document this in a new
section 7.5, starting with a table of aliases of a trivially denied path
to make the problem obvious, explain why it cannot be solved by code,
show how to write allow-list rules with "deny unless" or with a variable
set by each accepting rule and checked once at the end, and list the
patterns worth spotting when reviewing a configuration.

Link: https://www.mail-archive.com/haproxy@formilux.org/msg37732.html

DOC: threat-model: add a few more entries for stuff that should be public

Some reports still contain well-known claims that need to be re-explained
and would be better described publicly, particularly because they come
with zero real risk. Let's clarify a few such points.

DOC: security: clarify how to send proper reports

Due to the advent of AI, reports are becoming extremely common the the
security list and seem to be competing in a race to bad quality, wasting
everyone's time and nerves. So let's clarify a few points on the expected
format and expected behavior.

Also let's fix 2 lines that were ruined by the create-release script.

MEDIUM: sched: fix the default runqueue-depth

The scheduler's run queue depth had been empirically limited to around
280/sqrt(nbhread) when the scheduler was shared between all threads and
the number of threads was very low (4-16), because by then it looked
like a good balance between contention and latency.

Nowadays, each thread has its own scheduler and the number of threads
has exploded, but this calculus remains in place. It causes a noticeable
scheduling overhead on large systems because the number of tasks each
scheduler handles at once can become ridiculously low (e.g. 14 for 256
threads), forcing some users to work around this by forcing the value
in the configuration.

Let's get rid of this outdated calculation and keep the fixed default
value of 280 that was once considered correct and that is still applied
to a single thread, since each scheduler continues to run on a single
thread. Nothing prevents changing the default again later but at least
now the value in defaults.h corresponds to the one we're running on by
default.

SCRIPTS: create-release: do not match the word "version" isolated

It happens that some parts of security.txt were ruined, where anything
following the word "version" till the end of a line was replaced with
the current branch. Let's make sure we only match version chars after
it.

This can be backported.

BUG/MINOR: server: don't report dyncookie collision on same IP servers

When using "cookie dynamic", we detect cookie hash collisions and report
them. However, the test didn't take into account the fact that servers
could have the same addresses (e.g. server templates, DNS updates in
progress etc), and could produce lots of warning that the user couldn't
do anything with.

This patch adds an address check before reporting the warning: if two
servers have the same address, regardless of the cause, it is expected
that they will have the same cookie, so there's no point emitting a
warning.

In addition, there was no info about the servers causing the problem.
The only message was to ask to change the proxy's secret key. Let's
dump the server names so that the user may also correlate what's
happening with reports from other sources, or may even just delete or
readdress one of the offending servers if relevnt.

This can be backported to stable versions. Thanks to Nathan Wehrman
for the report.

BUG/MINOR: acl: check the presence of an argument to "-u"

In 1.5-dev23, support for specifying an ACL unique ID was added via
commit 3534d881bc ("MINOR: pattern/acl: Each pattern of each acl can
be load with specified id"). It turns out that if the argument is
missing, we read past the end and can segfault.

This can be backported to all versions. Thanks to Zlatko Bratkovic
for reporting this bug.

BUG/MINOR: stick-table: release server_key on any stksess destruction

A stick-table storing server_key holds a reference on a dictionary entry
for each of its entries. This reference is released by stksess_free()
only, while expiry (process_table_expire()), eviction
(stktable_trash_oldest()) and explicit deletion (__stksess_kill(), also
used by "clear table") destroy the session through __stksess_free()
directly. The dictionary entry is then never freed: it stays in
server_key_dict with a refcount of 1 and no owner, and memory grows
linearly with the number of distinct values that went through the table.

Move the release into __stksess_free() so that every destruction path
releases it, and leave stksess_free() as a simple wrapper.

This must be backported to all stable versions.

BUG/MINOR: quic-be: drop long header packets with unsupported version

On backend connections, quic_rx_pkt_parse() accepts long header packets
with an unsupported version and Version Negotiation packets, as these
checks only apply to listeners. Such packets keep the
QUIC_PACKET_TYPE_UNKNOWN type, which maps to no encryption level, and hit
BUG_ON(!qel || !pktns) in qc_enc_level_alloc() from qc_try_rm_hp(), or a
NULL dereference without DEBUG_STRICT.

No malicious peer is required: a server which does not support the
version selected by haproxy responds with a Version Negotiation packet
(RFC 9000 6.1). A malicious server or an on-path attacker may also send a
packet with an unsupported version.

Drop these packets on backend connections, as required by RFC 9000 5.2.1,
since version negotiation is not supported on the backend side. To do
so, export quic_version_VN_reserved and compare the packet version to it
instead of testing a null version number.

This should be backported up to 3.3, where it was introduced by 43d88a44f
("MINOR: quic-be: Datagrams and packet parsing support").

Signed-off-by: William Dauchy <wdauchy@gmail.com>

BUG/MEDIUM: threads: don't read lock history entries that don't fit in ulong

The per-thread lock history is stored in the "ulong lock_history" field,
with 8 bits per lock operation. On 32-bit systems only 4 entries fit,
but the dump code in ha_thread_dump_one() always read 8 of them,
shifting the value by up to 56, which is undefined behavior with a
32-bit operand. Recent clang versions may generate bad code that will
lead to a crash when using "show threads" on 32bits machines.

Let's keep the field an ulong and only iterate over the entries that
fit in it. On 64-bit nothing changes, and on 32-bit the last 4 entries
are dumped, which is all the field stores anyway.

This should be backported up to 3.2.

BUG/MEDIUM: h3: reject standalone FIN on invalid request/response

HTTP/3 parser has a special code path to handle a standalone FIN.
However for a request stream there was no check to ensure that a valid
request/response was received before reporting the closure.

This would result in an empty HTX buffer with EOM set. On the MUX level,
QCS instance would remain accounted in QCC <nb_hreq> counter, which
could result in unnecessary memory consumption.

Fix this rejecting a request stream closed with a standalone FIN without
a valid request or response received. The stream read is aborted and
write is reset using error code H3_REQUEST_INCOMPLETE or
H3_MESSAGE_ERROR depending on the proxy side. This allows to quickly
release the QCS instance.

This must be backported up to 2.6.

BUG/MINOR: h3: fix error code on incomplete request

HTTP/3 parser validates that a request has been received when FIN is
signalled by the peer on a request stream. FIN signal may be conveyed
with an extension frame. In this case, if no header was received, the
request is invalid and the stream is closed with an error.

This patch simply fixes the reported error code to H3_REQUEST_INCOMPLETE
as recommended by RFC 9114 when the issue occurs on the frontend side.
H3_MESSAGE_ERROR is kept on the backend side as there is no specific
error code defined for an incomplete response.

This must be backported up to 2.8. Ensure enum is adapted for 2.8 as the
name changed after.

BUG/MINOR: h3: close connection on response start-line alloc failure

On QUIC backend side, correct the error handling when HTX start-line
cannot be allocated during HTTP/3 response parsing.

Previously, a stream error would be reported with H3_MESSAGE_ERROR. Now,
h3_resp_headers_to_htx() only returns a default negative value without
any explicit error code. This results in a connection level error using
H3_INTERNAL_ERROR, generated at the end of h3_rcv_buf().

This pattern is already used on request parsing. It is necessary to
close the connection asap on memory exhaustion while differentiating
between a peer protocol violation and an haproxy internal error.

This must be backported up to 3.3.

BUG/MINOR: h3: fix missing EOM after trailers without FIN

In HTTP/3, it is possible to receive a trailers frame without STREAM FIN
set. It is forbidden for the peer to emit any other HEADERS or DATA
frame, however it is possible to close the stream via an empty STREAM
FIN, or even using HTTP/3 extension frames which must be ignored by a
parser. In this case, EOT will be inserted without EOM flag. If it is
consumed by the stream layer prior to the final EOM reporting, a second
EOT will be inserted, resulting in an invalid message rejected by the
MUX from the other side.

To prevent this situation, disable EOT emission in case EOM cannot be
set. This is implemented via the new argument recently added to
http_trailers_to_htx().

Now that EOT trailers can be voluntarily delayed, it is necessary that
it will be inserted, which htx_set_eom() does not perform if the HTX
message is not empty. This could result with EOM set on the last HTX
headers block instead. Thus, HTTP/3 parser forces EOT emission when
trailers were already parsed, via a new argument on
qcs_http_handle_standalone_fin().

This must be backported up to 2.8. This relies on the previous
refactoring patch. Also note that http_trailers_to_htx() only exist in
master branch, as previously EOT block was added directly by h3 parser
in h3_trailers_to_htx().

MINOR: mux_quic: refactor qcs_http_handle_standalone_fin()

Add htx_to_buf() in err label for qcs_http_handle_standalone_fin(). The
objective is to simplify modification of the function body for future
fixes. This will be reused first to prevent a possible double EOT after
HTTP/3 trailers parsing.

MINOR: http: add <need_eot> to trailers parsing function

Adds an extra argument to http_trailers_to_htx() to specify if EOT block
is necessary. Most of this times this will be set to true by callers.
However, it is necessary to skip it if caller does not want to report
EOM immediately after.

For now, every parsers set this new argument to true. This will change
for HTTP/3 parser as FIN can be received later.

BUG/MINOR: mux_quic: remove dangling qc->qcc ref on MUX init failure

During QUIC MUX init, the newly allocated QCC reference is stored in the
underlying quic_conn instance. If MUX init fails later, the reference is
not reset despite QCC being freed, and there is a risk that the
transport layer uses the dangling pointer.

Fix this by delaying the transport layer registration until every
fallible operations in MUX init has succeeded.

This must be backported up to 2.8.

BUG/MINOR: quic-be: fix Initial token length check on backend connections

The check rejecting server Initial packets with a non-zero token length
tests pkt->token_len before it is set, so it never matches. Test the
parsed <token_len> value instead, as required by RFC 9000 17.2.2.

This should be backported up to 3.3, where it was introduced by 43d88a44f
("MINOR: quic-be: Datagrams and packet parsing support").

Signed-off-by: William Dauchy <wdauchy@gmail.com>

BUG/MEDIUM: quic: limit reject after MUX closure to once per stream

This patch is a follow-up to the previous one. The objective is to limit
the number of allocated frames after MUX closure for STREAM handling.

Ensures that each stream is closed only once. This is performed by
incrementing the ID stored in the quic_conn structure each time a stream
is rejected. If a non-contiguous ID is received, all intermediary
streams are rejected as well.

Along with the previous patch which ensures that stream IDs higher than
the flow control limit cause an immediate closure, this guarantees that
frame allocation for stream rejection after MUX closure remains low and
bounded.

This must be backported up to 2.6 after a period of observation.

BUG/MINOR: quic: reject invalid stream ID after MUX closure

QUIC transport layer may remain for some time after MUX closure, until
all sent frames have been acknowledged. In the meantime, if the client
opens a new stream, an application layer callback is used to reject it.
With HTTP/3, this produces a STOP_SENDING + RESET_STREAM.

Previously, there was no limit on the queued frames in this case. This
could cause unnecessary memory consumption until the connection is
finally closed.

Address this issue by ensuring the client won't try to open new streams
with invalid ID. First, if the ID corresponds to a server initiated
stream, the connection is closed with error STREAM_STATE.

Another check is performed to ensure that the stream ID is not beyond
the limit advertised last by MUX layer. This requires two new fields for
uni and bidi types in quic_conn, updated during qcc_release() similarly
to the already existing max fields. In case of a violation, the
connection is immediately closed with transport error STREAM_LIMIT.

This must be backported up to 2.6 after a period of observation.

BUILD: Makefile: force $(shell command -v ...) to run through the shell

$(shell) commands with no metacharacter and not starting with a shell
builtin may be execvp()'d directly instead of being run through the
shell. "command" is a shell builtin that only appears in make's own
builtin list since version 4.3, so with make <= 4.2 the USE_MINIDEBUG
probe failed with "Command not found" and silently disabled the
embedded symbol table. Adding "2>/dev/null || :" brings metacharacters
which force the shell path on all make versions.

This should fix build on older distributions such as Ubuntu 18.04.

BUG/MINOR: ssl: skip the cipher workaround with AWS-LC API 38 and newer

The crt-list cipher workaround reparses the ciphers and ciphersuites
strings on each connection after SSL_set_SSL_CTX(). AWS-LC restored
context-based cipher inheritance in 5.14.0, so these calls are redundant
with a library providing the fixed context-switching behavior.

Limit the workaround to AWSLC_API_VERSION values greater than 30 and
below 38. Keep the string setters for older API versions and preserve
the API 30 exclusion, where cipher lists already follow the SSL_CTX.
This deliberately retains the workaround for API 37, including 5.14.0,
since that API version also identifies earlier, affected releases.

This follows 38398d2476 ("BUG/MEDIUM: ssl: crt-list ciphers ignored with
AWS-LC") and relates to GitHub issue #3500 and aws/aws-lc#3614.

This should be backported as far as 3.0. The code is in src/ssl_sock.c
before 3.1.

BUG/MINOR: quic: reject RETIRE_CONNECTION_ID for a CID not sent to the peer

A RETIRE_CONNECTION_ID frame for a sequence number below
<next_cid_seq_num> is accepted even if the corresponding
NEW_CONNECTION_ID frame has not been sent yet: the counter counts
allocated CIDs, not sent ones. A client can thus retire a CID it
cannot know about, for example the one allocated while processing a
previous RETIRE_CONNECTION_ID frame of the same packet. This is
forbidden by RFC 9000 19.16.

This adds <max_sent_cid_seq_num>, updated when a packet containing
NEW_CONNECTION_ID frame(s) is sent, and rejects such
RETIRE_CONNECTION_ID frames with a connection error of type
PROTOCOL_VIOLATION, as required by RFC 9000 19.16.

Must be backported as far as 2.8.

BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame

A NEW_CONNECTION_ID frame built by HAProxy only stores pointers to the
CID and stateless reset token of its quic_connection_id. When a
RETIRE_CONNECTION_ID frame is received, the CID is freed at once, while
a NEW_CONNECTION_ID frame for it may still be queued, in flight, or be
duplicated later for a retransmission. A client can trigger it by
retiring sequence numbers 1 and 2 in the same packet: the second frame
frees the CID allocated by the first one, whose frame is not sent yet.
The frame is then built from freed memory. In a default build, the pool
cache gives the object back to the next CID of the same connection, so
two frames carry the same CID. Without the pool cache, freed heap memory
is sent to the peer, and ASan reports a heap-use-after-free.

struct qf_new_connection_id now stores the CID and the token instead of
pointers, and quic_connection_id_to_frm_cpy() and the frame duplication
copy them. The parser copies them as well and rejects a CID length above
20 (RFC 9000 section 19.15). quic_frame does not grow, since its union
already holds a larger member.

This should be backported to 2.8 and later.

CI: github: re-enable AWS-LC 3.x (fips-2024-09-27)

This reverts commit 9fc468fe5d90e155cd86ed6b51430fedc04e02bf.

The OPENSSL_memchr build fix for 3.x was backported in the branch so we
can re-enable the CI. https://github.com/aws/aws-lc/issues/3759.

BUG/MINOR: tools: ha_freearray() must also free the array

ha_freearray() frees each string of a NULL-terminated array and then
sets the array pointer to NULL, but never frees the array itself.

Its only caller is ckch_conf_parse(), which uses it to release the
previous value when a PARSE_TYPE_ARRAY_SUBSTR keyword ("domains", "ips")
is specified twice on the same line, as in:

    crt-store s
      load crt site.pem domains "a.example,b.example" domains "c.example"

The strings are freed but the array of the first value is leaked
(24 bytes here, reported by LeakSanitizer with "haproxy -c -d" on a
build with -fsanitize=address). The leak only happens when parsing a
line with a duplicated keyword, which also emits an "already specified,
overwriting" warning, so the impact is minimal.

Fix it by freeing the array with ha_free(), which also sets the pointer
to NULL as before. With the patch, the same configuration with either
"domains" or "ips" repeated reports no leak.

Also accept a NULL array argument without dereferencing it. An argument
pointing to a NULL array remains safe, and non-NULL arrays are still fully
freed and their pointer cleared.

Fixes issue #3569.

This was introduced by commit 96b1f1fd2 ("MINOR: tools: ha_freearray()
frees an array of string") together with its use in commit 9c0c05b7b
("BUG/MINOR: ssl/ckch: always ha_freearray() the previous entry during
parsing"), both in 3.2-dev16, so this must be backported to 3.2.

BUG/MINOR: ssl/ckch: fix an invalid free() in the domains/ips error paths

The "domains" and "ips" keywords are stored as NULL-terminated arrays of
strings. When an allocation fails while one of these arrays is built,
both ckch_conf_parse() and ckchs_dup() release it with:

while (r && *r) {
char *prev = *r;
r++;
free(prev);
}
free(r);

The loop advances r to the terminating NULL entry, so free() is called
on r + n instead of the start of the array. When the failing my_strndup()
or strdup() is not the first one, glibc aborts the process with
"free(): invalid pointer" (ASan reports "attempting free on address
which was not malloc()-ed").

ckch_conf_parse() is reached from the configuration ("crt-store",
"crt-list") and from "add ssl crt-list" on the CLI. ckchs_dup() is
reached at runtime from "set ssl cert" on the CLI, from Lua
CertCache.set() and from the ACME scheduler when a certificate with at
least two domains or IPs is renewed, so the whole process is killed. In
both cases an allocation failure is needed.

Fix it by freeing the entries by index and then the array itself,
leaving r unchanged. ha_freearray() cannot be used here since it does
not free the array.

The issue was reproduced by failing the malloc() of the second domain
with an LD_PRELOAD shim: without the patch both paths abort, with it
"haproxy -c" reports "out of memory" and the CLI answers "Can't update
@s/site.pem!" while the process keeps running.

The ckch_conf_parse() part was introduced by commit 2e8acf54d ("BUG/MINOR:
ssl/ckch: leak in error path") and the ckchs_dup() part by commit
5b85b81d8 ("MINOR: ssl/ckch: handle ckch_conf in ckchs_dup() and
ckch_conf_clean()"), both in 3.2-dev, so this must be backported to 3.2.

This fixes GitHub issue #3561.

DOC: configuration: clarify normalize-uri version availability

The normalize-uri documentation can be read as saying that URI
normalization is only available in HAProxy 2.4. Clarify that the
experimental technical preview has been available since HAProxy 2.4.

Reported by Nick Ramirez in issue #3533.

Must be backported in all stable branches.

BUG/MINOR: cfgparse: detect unsupported crypt() hashes on Debian/Ubuntu

crypt() and crypt_r() may return NULL or error tokens when
encountering unsupported hash types. On Debian/Ubuntu (libxcrypt),
unsupported hashes return strings starting with '*' (e.g. '*0', '*1').
On musl, crypt() does not produce error tokens for unknown hashes, so
detection is not possible there.

Capture the return value of crypt() and check for NULL or a '*'
prefix, ensuring unsupported hash types are detected at config
validation time on platforms that report them.

Fixes isse #3568.

Must be backported to all stable branches.

CI: github: pin ARM development builds to Ubuntu 26.04

The ubuntu-latest-arm runner label does not exist, preventing ARM
development jobs from starting. Use ubuntu-26.04-arm instead, matching
the stable branch builds.

BUG/MINOR: quic/ssl: record TLS version bounds for certificate selection

QUIC restricts its SSL context to TLS 1.3 but leaves the bind's
ssl_methods bounds unset. Without an explicit ssl-max-ver, the client
hello callback therefore fails its TLS 1.3 check and selects RSA instead
of ECDSA when both certificates match the SNI.

Record the effective min/max bounds as ssl_sock_initial_ctx() does for
TCP binds. This fixes issue #3566.

This should be backported as far as 2.8.

BUG/MINOR: resolvers: compute a name's wire length by walking its labels

After decoding a name, resolv_read_name() computes the number of bytes
the name occupies in the message by scanning it byte by byte until it
finds either a zero byte or a byte with the two upper bits set, which it
takes for the terminating zero or a compression pointer. But labels may
contain any byte, so a label containing a zero byte or a byte >= 0xc0
stops the scan in the middle of the name, and the rest of the message is
parsed from this wrong position, generally resulting in the response
being considered invalid.

Such bytes are found for example in names using raw UTF-8 characters
instead of their IDNA form, which do-resolve may be asked to resolve:
"caf\xc3\xa9.test" is correctly sent in the query, but the valid response
is rejected because the question name is believed to end at 0xc3 (seen
as a pointer), and do-resolve fails while plain names work.

Let's walk the labels instead, as the first loop already validated them.

This patch may be backported to stable branches.

BUG/MEDIUM: resolvers: properly skip root-named authority and additional records

resolv_read_name() returns zero both on error and when the name is the
root (a single zero byte), since it returns the length of the decoded
name. When parsing the authority and additional records of an SRV
response, resolv_validate_dns_response() handles a zero length by moving
to the next record without advancing the read pointer. For a root name,
all remaining records are then parsed again from the same place and
ignored, and for an error, the following records are parsed from an
unknown position.

The most common root-named record is the EDNS OPT pseudo-record, which is
placed in the additional section, and which RFC 6891 allows anywhere in
it. When a server places it before the glue records, all of them are
ignored, and the servers' addresses must be resolved with extra A/AAAA
queries. An attempt to make this an error in commit fefce297a
("BUG/MEDIUM: dns: fix long loops in additional records parse on name
failure") had to be reverted by commit 091768ab3 because it rejected all
responses containing an OPT record.

This can be reproduced with an SRV response whose additional section
starts with an OPT record followed by the A glue records: A queries are
sent for all targets instead of using the glue records. The srv_glue
reg-test was extended to cover this case.

Let's explicitly detect the root name, which is then processed as any
other name (the OPT record being ignored and skipped as an unsupported
type). On real errors, since we can't know where the next record starts,
let's stop parsing the remaining authority and additional records instead
of parsing garbage, but still keep the answers.

This patch should be backported to all stable branches, after the previous
fix ("BUG/MEDIUM: resolvers: skip the data of ignored additional records")
which is needed for the OPT record to be skipped.

BUG/MINOR: resolvers: use truncated SRV responses when no TCP fallback is possible

Commit bc20f8c0b ("MEDIUM: resolvers: retry truncated UDP responses over
TCP") states that a truncated SRV response can still be partially
processed when no TCP fallback can be initiated for it. However for a
nameserver having both a UDP and a TCP server, the UDP response is
validated with allow_truncated_srv=0 before knowing whether the fallback
can be queued, so when resolv_send_tcp_fallback() fails (e.g. the TCP
request ring is full), the truncated SRV response is treated as an error
instead of being used, and the servers are not updated.

This was reproduced with a debug patch making resolv_send_tcp_fallback()
always fail, and a nameserver answering the SRV query with a truncated
response still carrying the SRV record and its glue: the server remained
without address before this fix, and gets 127.0.0.1 after.

Let's validate the response again allowing truncated SRV responses when
the fallback could not be queued. This is safe since the validation stops
before parsing any record for truncated responses, and rebuilds the
header and question information from scratch.

This was introduced in 3.5-dev6 by the commit above, no backport is
needed.

BUG/MINOR: resolvers: report a timeout when a TCP fallback gets no response

When a truncated UDP response triggers a TCP fallback, the resolution's
status is first set to RSLV_STATUS_OTHER, then the fallback query is
queued. If the TCP query never gets a response (e.g. the nameserver
refuses TCP connections or doesn't respond over TCP), the resolution
eventually fails on timeout, but process_resolvers() only sets the
status to RSLV_STATUS_TIMEOUT when no response at all was received, which
is not the case since the truncated UDP response was counted. The
requesters are then notified of an "other" error: servers are put into
maintenance for an "unspecified DNS error", and the "hold other" period
applies instead of "hold timeout".

This can be reproduced with a nameserver answering all UDP queries with
the TC flag set and refusing TCP, and a server using this name with
"hold other 100ms" and "hold timeout 100ms": the server goes down for
"unspecified DNS error" instead of "DNS timeout status".

Let's set the status to RSLV_STATUS_TIMEOUT when the fallback is queued.
Any subsequent response will update it as usual, and if none comes, it
is the right status to report.

This was introduced in 3.5-dev6 by commit bc20f8c0b ("MEDIUM: resolvers:
retry truncated UDP responses over TCP"), no backport is needed.

BUG/MINOR: resolvers: fix leaks on do-resolve's fetch availability error

When the sample fetch used by a do-resolve rule is not available in the
rule's context (e.g. "res.hdr(x)" in an http-request rule),
resolv_parse_do_resolve() releases the sample expression with free()
instead of release_sample_expr(), leaking its arguments and converters,
and returns without releasing the variable name, resolvers id and
options it already allocated, contrary to the other error paths.

This only happens on configuration errors, after which the process
exits, but it's visible with valgrind:

    backend b
        http-request do-resolve(txn.ip,r) res.hdr(x)

reports 349 bytes definitely lost in 4 blocks before this fix, 0 after.

While this patch may be backported to all stable version, it is harmless.
So no reason to backport it.

BUG/MINOR: resolvers: warn about ignored arguments on datagram nameservers

Since TCP nameservers were supported, the "nameserver" line accepts server
keywords after the address (e.g. "ssl", "verify"), which are parsed by
parse_server() for "tcp@" addresses. For datagram nameservers these
arguments are silently ignored. A typo such as forgetting the "tcp@"
prefix on "nameserver ns1 1.2.3.4:853 ssl verify none" thus results in
plain DNS being sent over UDP to the DoT port (and, since commit bc20f8c0b
("MEDIUM: resolvers: retry truncated UDP responses over TCP"), over plain
TCP for fallback queries) without any notice.

Let's emit a warning when arguments follow the address of a datagram
nameserver. A warning rather than an error is used so as not to break
existing configurations.

This patch may be backported to stable branches.

OPTIM: resolvers: only walk duplicates when looking up an existing record

When parsing answer records, resolv_validate_dns_response() looks up the
record's hash key in the answer tree to detect duplicates, then walks the
following nodes with eb32_next(), which doesn't stop at the end of the
duplicates of this key. When the key already exists but the record
differs (i.e. on a hash collision), all the remaining nodes of the tree
are compared, which is quadratic in the number of records for crafted
responses of colliding records (up to ~4000 records per 64kB TCP
response).

In normal operation the lookup returns NULL for new records and the first
node is the duplicate for repeated ones, so no difference could be
measured (60 resolutions of 4000 distinct A records over TCP cost the
same CPU time before and after). Still, let's use eb32_next_dup() to only
walk the nodes sharing the looked up key, as intended.

No backport is needed.

BUG/MINOR: resolvers: check the SRV expiration date again under the lock

resolv_srvrq_expire_task() checks that the server's SRV record is expired
before taking the resolvers' lock and cleaning the server up. But the
resolvers' thread may assign an SRV record to this server between the two
(resolv_check_response() resets the task's expiration date under the
lock), in which case the server is then cleaned up and put down right
after being assigned a valid record, until the next SRV response.

This task is only scheduled when a server state file assigns an SRV
target to a server at boot, so the window is very narrow and only exists
once, and this was found by code review.

Let's check the expiration date again once the lock is held.

This patch may be backported to stable branches.

BUG/MEDIUM: resolvers: reject servers sharing an SRV record with different resolvers

All servers of a proxy using the same SRV name share the same SRV request
(srvrq), which is linked to the resolvers section of the first server
found with a "resolvers" keyword. When another server of the same proxy
uses the same SRV name with a different resolvers section, the SRV
responses are processed by the first section under its own lock, and
resolv_check_response() updates all attached servers from there, calling
srv_update_fqdn(), resolv_link_resolution() and _resolv_unlink_resolution()
on servers whose resolutions belong to the other section, without holding
this other section's lock, while its task may run on another thread. This
can corrupt this section's resolution lists. In addition, the second
section's settings (timeouts, nameservers) are silently ignored for the
SRV record.

Such a configuration is accepted without any warning:

    backend be
        server-template a 2 _http._tcp.svc.test resolvers r1 init-addr none
        server-template c 2 _http._tcp.svc.test resolvers r2 init-addr none

Since there's no valid use case for this, let's report an error when a
server uses an SRV record already used with another resolvers section in
the same proxy.

The SRV request sharing was introduced in 1.8 by commit 67957bd59
("MAJOR: dns: Refactor the DNS code"). This should be backported to all
stable branches.

BUG/MINOR: resolvers: count nameserver timeouts

The per-nameserver "timeout" counter, reported by "show resolvers", the
stats and Prometheus, and documented as the number of times the server
did not answer in time, has never been incremented since it was
introduced in 1.6, so it always reports zero, even when a nameserver
never responds.

Individual responses are not tracked per nameserver for a given query,
so we cannot know which nameservers did not respond when only some did.
However when a try expires without any response at all, all nameservers
the query was sent to failed to respond in time. Let's count a timeout
for each of them in this case. It may undercount timeouts of some
nameservers when others respond, but it's never wrong anymore when
none responds, which is by far the most useful case.

This can be verified with a nameserver which never responds and
"resolve_retries 2": after 3 seconds, "show resolvers" reports
"sent: 5" and "timeout: 0" before this patch, and "timeout: 4" after.

This patch may be backported to stable branches.

BUG/MINOR: resolvers: Always init a new resolution's list element

resolv_pick_resolution() allocates a new resolution with pool_zalloc(),
then duplicates the hostname into it. If strdup() fails, the resolution
is released with resolv_free_resolution(), which ends with
LIST_DEL_INIT(&resolution->list). But this list element was not
initialized yet (it's only appended to the wait list once the hostname
was duplicated), so it contains NULL pointers which are dereferenced,
crashing the process instead of reporting the allocation failure.

This can only happen when running out of memory. Let's initialize the
list element with the other fields.

This was introduced in 3.3 by commit 6cf2401ed ("BUG/MEDIUM: resolvers:
Make resolution owns its hostname_dn value"), so this must be backported
wherever this commit was backported (so as far as 2.8).

BUG/MINOR: resolvers: don't dump a nameserver twice in "show resolvers"

When the output buffer is full, "show resolvers" yields and resumes from
the nameserver saved in its context. But this one is only updated after a
nameserver was successfully dumped, so the dump resumes from the last
nameserver already dumped, which appears twice in the output, instead of
the one which could not be dumped.

This can be reproduced with 40 nameservers in a section and
"tune.bufsize 4096": "show resolvers" lists 43 nameservers, ns13, ns25
and ns37 appearing twice.

Let's save the nameserver being dumped before trying to dump it instead.

This patch should be backported to all stable branches.

BUG/MINOR: resolvers: preserve nameserver identity on "clear counters all"

The resolvers statistics module stores the nameserver's id, its parent
resolvers' id and the nameserver's puid in the counters structure, next
to the counters themselves. "clear counters all" resets all counters by
copying the module's template over them in resolv_stats_clear_counters(),
which also clears these fields. As a result, "show stat domain resolvers"
reports empty "id" and "pid" fields for all nameservers afterwards, and
ns_puid becomes zero, so that server address changes performed by a DNS
resolution are then attributed to the wrong nameserver
(SERVER_INETADDR_UPDATER_DNS_RESOLVER(counters->ns_puid)).

This can be reproduced with any resolvers section:

  > clear counters all
  > show stat domain resolvers
  # id,pid,sent,...
  ,,0,0,...

Let's restore these fields after the reset, as done when allocating the
counters.

This patch should be backported to all stable branches.

BUG/MINOR: resolvers: reject null "timeout resolve" and "timeout retry" values

Both timeouts are parsed with parse_time_err() which accepts zero. But
"timeout resolve" is the delay between two resolutions of a same name,
so with a null value, a resolution is restarted as soon as it completes,
from the same task wakeup, and the resolvers task spins at 100% CPU
flooding the nameservers. With a null "timeout retry", all retries of a
resolution are sent at once without waiting for any response, which
defeats their purpose.

This can be reproduced with "timeout resolve 0ms" (which doesn't even
emit the "suspiciously small" warning thanks to the unit) and a server
resolving a name answered by the nameserver: 68225 queries were sent in
2 seconds, versus 2 with "timeout resolve 1s".

Let's reject null values for these two timeouts.

This patch should be backported to all stable branches.

BUG/MINOR: resolvers: reject a null or negative resolve_retries value

The resolve_retries value is parsed with atoi() and never checked. When
a resolution is started, its try counter is initialized to this value and
immediately decremented for the first query, and the resolution is only
considered as failed once the counter reaches zero on a timeout. With a
value of zero (or a negative one, or a non-numeric one which atoi() turns
into zero), the counter starts negative and never reaches zero, so the
resolution never fails: requesters are never notified of timeouts or
errors (servers keep their stale address forever), and queries are sent
every "timeout retry" forever instead of waiting "timeout resolve"
between resolutions, which with a small "timeout retry" floods the
nameserver.

This can be reproduced with "resolve_retries 0", "timeout retry 200ms",
"timeout resolve 1s" and a nameserver which never responds: 20 queries
are sent in 4s, versus 4 with "resolve_retries 1".

Let's reject values lower than 1 at parsing time, since a resolution
always needs at least one query anyway. Configurations with such values
were not working as expected and will now be rejected with an explicit
error.

This patch should be backported to all stable branches.

BUG/MEDIUM: resolvers: skip the data of ignored additional records

When parsing the additional records of an SRV response, records which are
not A or AAAA, or whose family is not accepted by dns-accept-family, are
ignored, but resolv_validate_dns_response() then moves on to the next
record without skipping the ignored record's data. The next record's
name is thus read from the middle of the ignored one's data, which
usually fails and makes all remaining additional records be ignored as
well (resolv_read_name() fails or returns an empty name, and the loop
continues at the same place), or may even produce bogus records if the
data happens to look like a valid record.

The glue records carrying the SRV targets' addresses are then lost, and
the servers' addresses have to be resolved with extra A/AAAA queries.
Since commit c84c15d39 ("BUG/MINOR: resolvers: Apply dns-accept-family
setting on additional records") this also concerns AAAA records when
IPv6 is not accepted, which is the default on hosts without IPv6
connectivity (dns-accept-family defaults to "auto"), so any SRV response
with AAAA glue records placed before the A ones is affected there.

This can be reproduced with "dns-accept-family ipv4" and an SRV response
whose additional section contains an AAAA record for the first target
followed by A records for all targets: the A records are never used and
A queries are sent for all targets.

Let's skip the record's data when ignoring it.

The problem has existed for unknown record types since the additional
records parsing was introduced. This must be backported to all stable
branches.

BUG/MEDIUM: resolvers: don't ignore records not matching a resolve-net family

When selecting an IP address in a response, resolv_get_ip_from_response()
walks over the "resolve-net" networks to give a better score to records
belonging to one of them, and skips networks of another address family
than the record's. Commit 3023e9819 ("BUG/MINOR: resolvers: Restore
round-robin selection on records in DNS answers") changed the outer loop
into a do/while loop with a "next" label, and turned all "continue"
statements into "goto next", including the one skipping networks of the
other family in this inner loop. As a result, as soon as a network of
another family is met before a matching one, the whole record is ignored
instead of just this network, and no address may be found at all.

This is easily reproduced with:

    server s1 www.test:80 resolvers dns resolve-prefer ipv4 \
                          resolve-net 2001:db8::/32,10.0.0.0/8

and a nameserver returning A 127.0.0.1: the server never gets an address
and remains in maintenance, while it works with "resolve-net 10.0.0.0/8".

The same commit also changed the "continue" skipping the owner server
itself in the list of servers attached to the record, into a "goto next".
It's currently harmless because the function returns early when the
owner is attached to any record, but it's wrong as well, so let's restore
it too.

This must be backported wherever the commit above was backported, so as
far as 2.8.