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.

BUG/MEDIUM: resolvers: detach servers without resolvers from the SRV request

All servers of a proxy using the same SRV name share the same SRV request
(srvrq), and since commit 81ba74ae5 ("BUG/MEDIUM: resolvers: Make 1st
server of a template take part to SRV resolution") they are all attached
to its list of available servers while parsing their address. But the
"resolvers" keyword is only known later, and resolvers_finalize_config()
simply skips servers without it, leaving them in the list without their
srvrq_check task nor their resolvers. If another server of the same proxy
uses the same SRV name with a "resolvers" keyword, the SRV request is
resolved, and resolv_check_response() may pick the server without
resolvers when assigning SRV records, then crashes dereferencing its NULL
srvrq_check (and would call resolv_link_resolution() with a NULL
resolvers otherwise).

This is easily reproduced with such a backend, which is accepted without
any warning, and a nameserver answering two SRV records:

    backend be
        server x _http._tcp.svc.test init-addr none
        server-template a 2 _http._tcp.svc.test resolvers dns init-addr none

The process segfaults on the first SRV response. The reg-test provided
with this patch does exactly this.

But detaching the server from the SRV request's list of available servers
is not enough: the pointer to the SRV request itself must also be dropped.
Otherwise "show servers state" keeps dumping the SRV record name for this
server, and on a reload with a server state file, srv_apply_state() re-adds
the server to the SRV request's named_servers tree before crashing while
scheduling its NULL srvrq_check task. The server could also be picked from
that tree again by any subsequent SRV response. Note that a server with an
SRV name but no resolvers will never be resolved: keeping its pointer to
the SRV request serves no purpose.

Thus, in resolvers_finalize_config(), let's fully detach such servers
from the SRV request, since they will never be resolved anyway. As a
consequence, their state is no longer applied on reload (the SRV record
reported in the state file can no longer match), which is the desired
behavior, and it is no longer forbidden to change their FQDN from the
stats socket.

This must be backported to all stable branches.

CI: github: install libcrypt-dev:i386 for the i686 build

Since Ubuntu 26.04, the i686 job needs libcrypt-dev:i386 to link
with -lcrypt. Without it, the build fails with:

/usr/bin/i686-linux-gnu-ld.bfd: cannot find -lcrypt: No such file or directory
collect2: error: ld returned 1 exit status

CI: temporarily disable AWS-LC 3.x (fips-2024-09-27)

AWS-LC 3.x fails to build with recent GCC/glibc due to a
-Wdiscarded-qualifiers error in OPENSSL_memchr (C23 made memchr
const-correct, and glibc 2.41+ applies this even in C11 mode).

The fix exists upstream (aws/aws-lc#3008) but has not been
cherry-picked to the fips-2024-09-27 branch yet.

See https://github.com/aws/aws-lc/issues/3759

CI: github: remove MIPS targets from cross-zoo.yml

Ubuntu 26.04 ("resolute") has dropped MIPS architecture support, so
cross-compiler packages (gcc-mips-linux-gnu, gcc-mips64-linux-gnuabi64,
gcc-mipsel-linux-gnu, and libc6-dev-*-cross) are no longer available in
the distribution repositories.

Remove the MIPS targets from the cross-zoo matrix to allow the workflow
to continue running on ubuntu-latest.

CI: github: switch development builds back to ubuntu-latest

It was changed by 50cf89a ("CI: bump development builds explicitely to
Ubuntu 24.04"), but it was just a matter of cleaning the cache.

Switch development builds back to ubuntu-latest (and ubuntu-latest-arm)
and update stable branches to ubuntu-26.04.

REGTEST: ocsp: make the ocsp_client_check test fast

The last part of the test, which checks what happens when the OCSP
responder is not running anymore, used to take about 3 seconds: the
connection to the responder is refused, and the httpclient performs its
retries with the usual 1 second delay before reporting the responder as
unreachable.

A previous attempt tried to split the test in two but it failed due to
the need to hard-code the listening port which appears in the certificate,
preventing the tests from running in parallel, so everything must remain
in the same file to remain sequential.

In addition, setting a shorter timeout in HAPROXY_TEST_TIMEOUT (e.g. 400)
made it systematically fail with a 504: the 'do-ocsp-request' action limits
the OCSP check to the http-request timeout, which it arms when the rule is
evaluated, while the server timeout of the clear listener which forwards
the client's request was started earlier. Both being set to the same value,
the listener always gave up first.

Let's perform this last check through a dedicated SSL frontend whose
http-request timeout is set to a fixed 50ms, which is the timeout being
tested and remains well below the clear listener's server timeout. The
other checks keep using HAPROXY_TEST_TIMEOUT, so that a real OCSP round
trip is never limited to 50ms. The whole check now reports a timeout,
whose code and reason are verified, and the whole test takes about 700ms
in multiple tests of 250ms each, instead of around 3.2s.

BUG/MEDIUM: flt-decomp: preserve pending output before decoding again

The HTTP no-input path tries to drain the decompressor's pending output,
then may call add_data() with an empty input even when the drain was only
partial. decomp_uslz_add_data() replaces drain and drain_len with the new
result, so this can discard output that has not yet been forwarded.

Only call the decoder again after the previous output has been fully
drained, as the TCP decompression path already does.

BUG/MINOR: mjson: fix out-of-bounds reads on malformed JSON paths

The path passed to mjson_find() is parsed without always staying within
its bounds, so a malformed path can read past its end. This is a
heap-buffer-overflow whenever the path is stored in an exactly sized
allocation, which is the case of the json_query converter's argument
since it is duplicated by my_strndup() at configuration parsing time.
Two code paths are involved.

plen1() and plen2() measure a path segment and skip two characters when
they see a backslash, so that an escaped '.' or '[' is not taken as a
separator. When the path ends with a backslash, the second skipped
character is the trailing zero and the loop then reads past the end of
the string. With:

  http-request set-var(txn.out) req.fhdr(json),json_query($.a\\)

(the config parser turns "\\" into a single backslash, so the path is
"$.a\" followed by the terminating zero). Every request carrying a JSON
object in the "json" header then makes plen1() read past the end of the
5-byte allocation. ASan reports a heap-buffer-overflow READ of size 1 in
plen1(), reached from sample_conv_json_query(). Paths without a
trailing backslash are not affected.

Then mjson_get_cb() locates the array index either by skipping a fixed 3
bytes from the path position when the index is 0, or by scanning
forward for a ']' with no bound otherwise. A truncated index breaks
both: for "$.a[" and "$.a[0", the fixed skip lands past the trailing
zero, and for "$.a[1", the scan walks past the end of the string until
it happens to meet a ']' further in the heap. Nothing validates the
path at configuration parsing time, so such a configuration passes
"haproxy -c" and only fails when the converter is evaluated on a
matching request.

These paths come from the configuration only: the JWE and ACME code
uses hardcoded paths or snprintf() with a bounded index, and the other
callers pass constant strings. So the bug is only reachable when the
configuration itself is malformed, which limits its severity.

Let's only skip the character following a backslash when it is not the
end of the string, in both plen1() and plen2(), and move the array index
parsing to a new skip_idx() helper. It only accepts a decimal number
immediately followed by a ']', and never scans past the end of the
string so that a truncated index is reported as not found. This is
slightly stricter than the previous code, which accepted "+1", "1 " and
"1x" as index 1 because mystrtod() stops at the first character that is
not part of the number, while "$.a[00]" is now recognized as index 0
instead of being skipped as a 3-byte suffix.

The plen1() and plen2() bug is still present in the upstream mjson
project.

This should be backported to all stable versions. The code came with
mjson in 2.4.

Co-authored-by: Martin Mirchev <mirchevmartin2203@gmail.com>

BUG/MINOR: mux-h2: only convert GET upgrade requests to Extended CONNECT

An H1 request carrying "Connection: upgrade" sent to an H2 server is
converted to an Extended CONNECT (RFC8441) whatever its method, while
RFC8441 only defines GET requests, and an Extended CONNECT received by
haproxy on the H2 side is always converted back to a GET. So through an
H2 hop (H1 -> haproxy -> h2 -> haproxy -> H1 server), a bodyless DELETE,
OPTIONS, POST or PUT request with an upgrade reaches the server as a GET
(bodies are rejected however). There's no direct implication to this but
it could at least be confusing, and maybe cause trouble in certain cases
(idempotent methods, HEAD etc).

Let's reject the conversion for methods other than GET, like it is
already done for upgrade requests with a payload since commit 001fb1a54
("BUG/MEDIUM: mux-h1/mux-h2: Reject upgrades with payload on H2 side
only"). This way we don't block them in H1 where they are supported
(and used as shown in the commit above).

The protocol_upgrade reg-test was extended to check that a bodyless
DELETE upgrade request is rejected instead of reaching the server
as a GET.

This could be backported to all stable versions.

BUG/MINOR: mux-h2: also emit the HPACK Dynamic Table Size Update before trailers

Commit 39a0a1e12 ("MEDIUM: h2/hpack: emit a Dynamic Table Size Update
after settings change") made the mux emit an HPACK Dynamic Table Size
Update at the beginning of the first header block following a change of
the peer's SETTINGS_HEADER_TABLE_SIZE (RFC7541#4.2). The previous commit
did the same for request HEADERS, but h2s_make_trailers() still never
emits it, on any side, so that if the first header block following the
change is a trailers block, a strict decoder will report a
COMPRESSION_ERROR and kill the connection.

It's quite unlikely to happen to be honest, because:
  - on the frontend, the client sends the SETTINGS frame before headers
    and has no real reason to change it afterwards ;
  - on the backend, it could theoretically happen but only on a large
    POST that receives a SETTINGS frame after the first HEADERS frame and
    acks it before the trailers HEADERS frame; it's not as if trailers to
    backends were that common anyway.

The update is now inserted before the encoded fields of the trailers when
at least one field is present. Otherwise an empty DATA frame is sent
instead of a HEADERS frame, and the update is left for the next header
block. The flags are only updated once the frame is committed, as in the
other functions.

This was verified on the backend with a server sending its SETTINGS frame
with a lower header table size after receiving a request's HEADERS frame:
the request's trailers now start with the update, which was not the case
before. The frontend case was only verified by code analysis, it uses the
same code.

Like previous patch, this could be backported to all stable versions, but
preferably only if found to be necessary. Let's just start with 3.3 and 3.4
only.

BUG/MEDIUM: mux-h2: also emit the HPACK dynamic table size update on requests

Commit 39a0a1e12 ("MEDIUM: h2/hpack: emit a Dynamic Table Size Update
after settings change") made the mux emit an HPACK Dynamic Table Size
Update at the beginning of the first header block following a change of
the peer's SETTINGS_HEADER_TABLE_SIZE (RFC7541#4.2), but only in
h2s_snd_fhdrs(), i.e. for responses sent to clients. H2_CF_SHTS_UPDATED is
also set on backend connections, but h2s_snd_bhdrs() never emits the
update. Indeed, by then the spec was still hesitant on this point and
we were not certain that this behavior would be kept, and even less
that it would be necessary at all for the backend side.

On the backend side this is almost certain to happen with servers which
advertise a lower header table size: the first request is usually sent
before the server's SETTINGS frame is received, and the next one is the
first header block after the change. Strict decoders (e.g. those based on
nghttp2 or python-h2) treat the missing update as a COMPRESSION_ERROR on
the first request sent after the server's SETTINGS was acknowledged,
which kills the connection.

This is reproducible with a python-h2 server announcing a lower header
table size: before the fix, the second request on the connection triggers
a COMPRESSION_ERROR ("Encoder did not shrink table size to within the
max") and fails, and a new connection is used for the third one; after
the fix, all requests are served over the same connection.

The update is now also emitted at the beginning of request HEADERS frames,
exactly as for responses on the frontend size, and the flags are only
updated once the frame is committed, as in h2s_snd_fhdrs().

Emitting a DTSU toward servers which did not expect one could be a concern
since some implementations were already met which do not like receiving
one (VTest in the case of the commit above). It is considered safe here
for the following reasons:

  - the update is only emitted after the server explicitly advertised
    SETTINGS_HEADER_TABLE_SIZE in its SETTINGS frame. Servers which do not
    advertise it will never see it, which is the same rule that has been
    applied to clients since 2.6.

  - nghttp2's encoder, hence curl, nghttpx, Apache's mod_proxy_http2 and
    every other client based on it, already does exactly this: on any
    SETTINGS_HEADER_TABLE_SIZE received, including one which does not
    change the value, it starts the next header block with an update
    (nghttp2_hd_deflate_change_table_size() unconditionally sets
    notify_table_size_change). This was verified on the wire with curl
    8.5.0 and nghttp2 1.59: a server advertising 4096 or 65536 receives
    an update to 4096 at the beginning of the next request. Thus any server
    advertising this setting is already routinely exposed to this opcode.

  - the only difference is the value: we always send zero since we don't
    use the dynamic table, while nghttp2 sends min(advertised, 4096). Zero
    is always valid since RFC7541#6.3 only requires the value not to
    exceed the advertised limit, and we've been sending this exact update
    to clients (including browsers) over the last 5 years.

This could be backported to all versions, but only if found necessary.
It would make sense to have it in 3.4 and 3.3 though.

BUG/MEDIUM: mux-h2: don't send RST_STREAM for backend streams whose HEADERS were not sent

On the backend side, h2_snd_buf() assigns the stream ID before calling
h2s_snd_bhdrs() to encode the HEADERS frame. If the encoding fails, the
the HEADERS frame is never sent and the stream is put in error, but an
RST_STREAM is sent for this ID. The same happens via h2_do_shutr()/
h2_do_shutw() when the stream is aborted while its HEADERS frame is
still blocked. But for the server, this stream is still idle, and the
H2 spec says that receiving an RST_STREAM on an idle stream must be
treated as a connection error, so a compliant server will then close
the connection with a GOAWAY(PROTOCOL_ERROR).

Commit f065d0009 ("BUG/MEDIUM: mux-h2: Don't send RST_STREAM frame for
streams with no ID") already addressed the case of streams without ID,
which was the first manifestation of this situation. Let's also skip
the RST_STREAM for backend streams which haven't sent their HEADERS
frame yet.

The protocol_upgrade reg-test, which expected the RST_STREAM on the idle
stream for requests which cannot be converted, was updated accordingly.

The issue has existed since outgoing streams were introduced in 1.9, so
the fix should be backported to all supported versions.

BUG/MINOR: mux-h2: unblock the demux when destroying the stream it was blocked on

When the demux is blocked on a stream whose rxbufs are full, it sets
H2_CF_DEM_SFULL and waits for h2_rcv_buf() to make room. If the upper
layer leaves at this moment, h2_detach() normally clears the demux
blocking flags when the stream is the one being demuxed. But if the
stream could not send its RST_STREAM (or ES) at shutdown time because the
mux buffer was full, h2_detach() leaves early to keep the stream as an
orphan, and skips that part. The stream is later destroyed by
h2_deferred_shut() once the frame can be sent, but h2s_destroy() only
unblocks the demux if it was waiting for rxbuf slots (H2_CF_DEM_RXBUF),
not if it was waiting for this stream's buffers to be drained
(H2_CF_DEM_SFULL). The demux thus remains blocked until the timeout
triggers.

This requires an upload on a stream whose upper layer aborts (e.g. server
timeout or error) while the client is not reading its responses fast
enough for the RST_STREAM to be sent. It was reproduced with a debug patch
simulating a full mux buffer when sending the RST_STREAM of the stream
the demux is blocked on, with an H2 client uploading to a server which
never reads (timeout server 2s), then sending a GET on another stream of
the same connection: the GET never gets a response, while it gets a 200
immediately with the fix.

The fix consists in also clearing H2_CF_DEM_SFULL in h2s_destroy() when
the destroyed stream is the one the demux is blocked on. The issue seems
to exist since orphaned streams are destroyed from h2_deferred_shut()
(1.9). The fix may be backported to all supported versions, though before
3.1 the rxbuf release code differs and the same test must be applied after
releasing the stream's rxbuf.

BUG/MEDIUM: mux-h2: serve all pending buffer allocations in the buffer callback

The demux buffer, the mux buffer and the streams' rxbufs all share the
same buffer_wait entry in the h2c, and the blocking causes are indicated
by the H2_CF_DEM_DALLOC, H2_CF_MUX_MALLOC and H2_CF_DEM_SALLOC flags.
However h2_buf_available() only serves the first cause it manages to
allocate for and returns 1, which dequeues the connection. If another
cause was pending, its flag remains set while the connection is not in
the queue anymore, so nothing will ever retry it:
  - a stale H2_CF_DEM_DALLOC prevents h2_recv() from receiving anymore;
  - a stale H2_CF_DEM_SALLOC blocks the demux forever;
  - a stale H2_CF_MUX_MALLOC prevents h2_send() from calling
    h2_process_mux(), so that window updates, pings and GOAWAY frames are
    not sent anymore, and streams blocked on the connection's flow
    control are never resumed.

This was reproduced with a debug patch faking both a demux buffer and a
mux buffer allocation failure on an H2 connection, then calling
offer_buffers() once 200ms later: a GET for a 200kB object forwarded to
a server stops after exactly 65535 bytes of DATA despite the client's
WINDOW_UPDATE frames, and stays there until the client gives up after
5s, while the whole object is received instantly once both causes are
served.

Let's serve all pending causes in a single call, wake the connection up
if any progress was made, and only accept to leave the queue once none
remain. This needs two simultaneous buffer allocation failures on the
same connection, or a single one combined with the issue fixed by commit
"BUG/MEDIUM: mux-h2: always return already allocated buffers in
h2_get_buf()", so it's rare. The problem exists since the buffer
allocators were merged in 1.9 by commit 44e973f50 ("MEDIUM: h2: use a
single buffer allocator"). The fix may be backported to all supported
versions after the aforementioned one.

BUG/MEDIUM: mux-h2: wake the connection up after a delayed mbuf allocation

When the mux buffer couldn't be allocated, H2_CF_MUX_MALLOC is set, the
streams that were trying to send are marked H2_SF_BLK_MROOM, and the
connection is queued in the buffer wait list. Once h2_buf_available()
manages to allocate the buffer, it clears the flag but only wakes the
connection up if the demux was also blocked (H2_CF_DEM_MROOM), which is
only the case for control frames (ACKs, WU, RST etc). When only streams
were blocked (e.g. a response HEADERS frame), nothing resumes them, and
they remain blocked until another event happens on the connection, or
until the timeout strikes.

This was reproduced with a debug patch faking a failed mbuf allocation
when sending the response HEADERS frame of an H2 request forwarded to
a server, and calling offer_buffers() 200ms later: the request hangs
until the client gives up after 5s, while it completes in 200ms once the
tasklet is woken up, since h2_send() resumes the streams waiting in the
send list.

This was introduced in 1.9 by commit 53216e7db ("MEDIUM: connections:
Don't directly mess with the polling from the upper layers.") which
removed the conn_xprt_want_send() call without replacing it. It requires
a buffer allocation failure to happen first so it's quite rare. This may
be backported to all supported versions.

BUG/MEDIUM: mux-h2: permit the buffer callback to allocate despite the queue

Since 3.0 with commit d1eb48a12 ("MEDIUM: dynbuf: refrain from offering
a buffer if more critical ones are waiting"), b_alloc() refuses to
allocate a buffer when the wait queue of the same or more critical levels
is not empty, unless DB_F_NOQUEUE is passed. But h2_buf_available() is
called by offer_buffers() while the h2c is still in the queue, and it
tries to allocate without this flag. For the demux buffer (DB_MUX_RX,
which is also used by h2_get_buf() for all other buffers and is the
least critical queue), the h2c's own presence in the queue makes the
allocation always fail. The connection thus remains in the queue until
it times out, and as long as it's there, every DB_MUX_RX allocation on
the same thread (all H2 demux and mux buffers, H1 input buffers) has to
be queued as well and can only be served by buffer releases, so that
new connections on a quiet thread stall as well.

This requires a buffer allocation failure in the first place (memory
shortage or tune.buffers.limit reached), which is rare, but then a single
one is enough. It was reproduced with a debug patch faking one dbuf
allocation failure and calling offer_buffers() 200ms later on a single
thread: the callback sees bufq_map=0x8 and b_may_alloc_for_crit(DB_MUX_RX)
returns 0, and two H2 requests and a subsequent H1 request all hang for
5s until the client gives up, while they're all served in 200ms once the
flag is passed.

Let's pass DB_F_NOQUEUE to the allocations made from the callback, since
by definition we're called because a buffer is being offered to us. This
must be backported to 3.0.

BUG/MEDIUM: mux-h2: always return already allocated buffers in h2_get_buf()

h2_get_buf() is used to get the demux buffer, the mux buffer ring's tail
and the streams' rxbufs. It refrains from calling b_alloc() when the
connection is already in the buffer wait queue, but it does so even when
the requested buffer is already allocated, in which case it returns NULL.
The consequence is that as soon as an H2 connection waits for any buffer
(e.g. a stream's rxbuf), all other users consider that their already
allocated buffer is missing: h2_recv() stops reading and sets
H2_CF_DEM_DALLOC although the demux buffer is allocated, frame emitters
set H2_CF_MUX_MALLOC although the mbuf has room, etc. The connection then
stops both receiving and sending until the buffer callback is called,
and these spurious flags confuse h2_buf_available() which only serves one
of them (see next fixes).

This can be observed by limiting the number of buffers with
tune.buffers.limit, holding them with incomplete HTTP/1 requests, then
sending an H2 request: once the stream's rxbuf allocation fails, the
connection reports a demux buffer allocation failure on the next recv
attempt (seen with debug traces).

Let's always return an already allocated buffer. This has been there
since the merge of the buffer allocators in 1.9 with commit 44e973f50
("MEDIUM: h2: use a single buffer allocator"), so this fix may be
backported to all supported versions.

CLEANUP: mux-h2: clarify that bptr is not NULL when calling to h2_get_buf()

In h2_frt_transfer_data(), we first check h2s_rxbuf_tail() and if it's
NULL, we get out. A bit later we pass that function's return to
h2_get_buf(), which dereferences it. It's OK since it was checked. But
depending on optimization levels and tiny extra fixes, gcc-9.5 (at least)
sometimes thinks it sees a possible null-deref and emits a warning. Let's
just use the known safe version of h2s_rxbuf_tail() since we know for sure
that the pointer is valid so that the compiler is no longer confused. A
comment was added at the top of the function to clarify that the pointer
may not be NULL.

REGTESTS: cli: check uninterruptible mode

Tests wait command running in uninterruptible mode. The first check
ensures that the limit on concurrent commands cannot be exceeded. The
second check tests the "kill" command.

MINOR: cli: add "kill" CLI command

Implements a new "kill" admin CLI command. This allows stopping other
running commands.

This patch is implemented in two parts. First, the "kill" parser, which
loops over CLI jobs and sets the CLI_CMD_FL_KILLED flag on the targeted
ones.

The other change is in cli_io_handler() so that each targeted job reacts
to the CLI_CMD_FL_KILLED flag. If set, normal CLI flow is interrupted
and the io_release handler is called. Any release pending message is
suppressed and "Killed." is displayed to notify the user.

Currently, "kill" can only stop uninterruptible commands. This must
still be specified as a mandatory argument to support future extensions
of the command.

MINOR: sink: add timeout argument for "show events"

Define a new "-u" argument for the CLI show events command. This allows
activating the uninterruptible execution mode for
cli_io_handler_show_ring(). The inactivity delay is directly derived
from the configured stats timeout.

MINOR: ring: support inactivity timeout in show ring CLI handler

This patch implements uninterruptible mode of execution when monitoring
a ring via the CLI. The handler continues to run despite the detection
of the CLI input closure.

The command is still stopped if a send error occurs while reporting new
events. Also, the socket timeout is reused as an inactivity timeout,
refreshed each time new events are detected. The command is stopped if
there is nothing to report during this delay.

MINOR: cli: implement uninterruptible mode for wait

Define a new argument "-u" for the CLI wait command. This allows
executing wait in uninterruptible mode, which is convenient when using
utilities such as socat.

The new argument is parsed in cli_parse_wait(). If present, the command
will be started as an uninterruptible CLI job and will be accounted for
as such, unless the uninterruptible jobs limit is reached.

MINOR: cli: implement basic mechanism for uninterruptible jobs

Interactive CLI commands will be interrupted when the CLI input
is disconnected. This is a safety measure to ensure a long running
command won't consume unnecessarily a CLI connection slot without any
means of shutting it down.

A disconnection is detected by the closure of the remote write side as
this is the only way to detect it when there is nothing to send.
However, this causes issues with utilities such as socat running with a
timeout which perform a half-close on their write channel when the
command has been sent. For example, it is currently impossible to
execute "wait" with socat, unless the special option "ignoreeof" is set.

Introduce a new concept of CLI uninterruptible jobs. This mode of
execution will only be available to long-running commands with a
dedicated argument. Under this mode of execution, the execution will
continue despite the remote write channel being already closed.

A limit of concurrent uninterruptible jobs is set to 3 to ensure they
won't exhaust the default CLI maxconn of 10.

MINOR: cli: store active CLI sessions in a global list

Implement a global list of the currently active CLI sessions. This is
sound as since only one CLI frontend exists per HAProxy instance (via
"stats socket" for a worker, or alternatively a master CLI via -S for a
master process).

Each session is appended in the CLI_ST_INIT state at the beginning of
cli_io_handler() and removed in cli_release_handler().

This will allow to implement new CLI commands which can interact with
other CLI sessions.

MINOR: cli: factorize CLI session reset

Define a new helper cli_reset_cmd() which is used to reset the CLI
context between each executed command. This will become useful to
introduce new advanced CLI mechanisms.

BUG/MEDIUM: ssl: don't use MSG_MORE for kTLS control records

ha_ssl_write() also applies the application's SEND_MORE hint to kTLS
control records. Linux rejects TLS_SET_RECORD_TYPE with MSG_MORE with
EINVAL. If application data are still buffered, the failed close_notify
leaves the response tail unsent when the socket closes.
Fix that by just not using MSG_MORE for control records.

This should be backported up to 3.3.

BUG/MEDIUM: mux-h2: also consider the fctl_list as pending data for the timeout

The previous commit 87cf2a6b ("BUG/MEDIUM: mux-h2: consider the send list
as pending data for the timeout") was incomplete: a stream blocked on the
connection window (H2_SF_BLK_MFCTL) may also be orphaned by h2_detach()
after its stream connector left, in which case it sits on the fctl_list
and not on the send_list. Nothing of its frame was emitted yet (typically
an ES/RST frame via a WANT_SHUTW, or the remaining fast-forward bytes of
its iobuf), so br_data() on the mux buffer can be zero.

Such a connection was still considered idle and, on a backend connection,
was armed with no timeout at all while not being reusable: the orphan is
still in streams_by_id so it is not moved to the idle list either. It
then relied on the peer to send a WINDOW_UPDATE to ever wake up, which a
stalled peer never does.

The blocked_list is deliberately not considered: streams blocked on their
own stream window (H2_SF_BLK_SFCTL) are never orphaned by h2_detach(), so
this list is always empty once the last stream connector detached.

This matches the original h2c_may_expire() from before commit 3439583dd
("MEDIUM: mux-h2: slightly relax timeout management rules") which
considered both lists. mux-fcgi is not affected, it has no flow-control
window and no fctl list.

This could be backported as far as 2.6, along with 87cf2a6b.

Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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

In fcgi_detach(), the test deciding whether the connection may be moved to
an idle list checks FCGI_CF_EOS, FCGI_CF_ERR_PENDING, FCGI_CF_ERROR and
FCGI_CF_KEEP_CONN, but not the connection state. A connection that already
switched to FCGI_CS_CLOSED may thus be inserted in the session server list
or in the server's idle pool, while fcgi_avail_streams() reports no
available stream for it. The stream immediately before, in the same
function, does test FCGI_CS_CLOSED.

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

Note that fcgi_conn_is_dead() must not be used here: it also requires
FCGI_CF_KEEP_CONN to be cleared or FCGI_CF_ERROR to be set, so it does not
cover every FCGI_CS_CLOSED connection, and the non-private branch below
clears conn->owner before parking the connection.

This may be backported to all supported versions.

MINOR: mux-fcgi: check for a dead connection before refreshing the timeout

fcgi_detach() refreshed the connection timeout right after the stream was
destroyed, then evaluated fcgi_conn_is_dead() only at the end of the
function, on the paths that did not already return. So a connection about to
be released was first armed and requeued via task_queue(), then
fcgi_release() cleared its task context and woke it up again for nothing.

Let's evaluate fcgi_conn_is_dead() first, right after the stream is
destroyed, and return early when the connection must be released. The check
that closed the function is then redundant and is removed, and the timeout
is only refreshed for connections that survive.

This only reorders statements: fcgi_conn_update_timeout() writes the task's
expiration date, the FCGI_CF_IDL_PING-like state and the timer queue, none
of which fcgi_conn_is_dead() reads. It also matches the structure mux-h2
just got. The move is safe with regards to the idle lists, since the
branches below only set conn->owner, the session and idle list entries and
the transport idle state, which the predicate does not look at either.

Note that this was only correct once the previous commit removed the
conn->owner test from fcgi_conn_is_dead(): the non-private branch below
clears conn->owner before parking the connection, so evaluating the
predicate first would otherwise report every connection as dead.

This relates to GitHub issue #3558.

BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is dead

fcgi_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.

That assumption no longer holds. Idle backend connections are stored in the
server, reachable through a hash, so they are no longer attached to a
session and may legitimately have conn->owner NULL while still being
perfectly usable. This is exactly the situation mux-h2 hit: the first stream
on a fresh connection sets conn->owner, but a later stream picking that
connection from the idle pool does not, so the connection is destroyed the
next time its last stream detaches instead of being returned to the
pool. Commit a0541f5d2 ("BUG/MEDIUM: mux-h2: ignore conn->owner when
deciding if a connection is dead") removed the test from h2c_is_dead() for
that reason; do the same here.

The test was also actively harmful in fcgi_detach(): the non-private branch
clears conn->owner and then calls fcgi_conn_is_dead(), so the predicate was
true by construction at that point, which would have prevented returning the
connection to the server's idle pool at all.  It only worked because
detach() tests the flags and FCGI_CF_KEEP_CONN first and never reaches the
tail for such a connection.

Note that mux-fcgi has no reverse-connection support, so unlike mux-h2 there
is no conn_is_reverse() companion test to remove.

This should be backported wherever a0541f5d2 was backported, so as far as 2.8.

BUG/MEDIUM: mux-fcgi: detect an early connection error during the handshake

fcgi_conn_is_dead() treats a connection with FCGI_CF_ERROR as dead, but not
one with FCGI_CF_ERR_PENDING. The latter is set by fcgi_send() when the
transport reports an error, and is only turned into FCGI_CF_ERROR once
FCGI_CF_EOS is also set, i.e. once a read0 was seen. A connection whose
transport failed while the GET_VALUES handshake was still in progress
carries FCGI_CF_ERR_PENDING alone, and is therefore considered alive, so
nothing closes it: fcgi_send() bails out immediately on the flag,
fcgi_recv() is not allowed to run either (fcgi_recv_allowed() refuses when
the wait event is subscribed for SEND only... in practice nothing makes
progress any more), and h_detach()'s idle-list branch excludes it so it is
not parked either. It is only released when the idle-connection purge
reaches it.

This is the same situation mux-h2 fixed in commit 21fb6bdab ("BUG/MEDIUM:
mux-h2: Be able to detect connection error during handshake"), which reports
a connection as dead when H2_CF_ERR_PENDING is set before any frame was
received. Let's do the same here: a connection with FCGI_CF_ERR_PENDING
whose state is still below FCGI_CS_RECORD_H has not exchanged a single FCGI
record yet, so it cannot be reused and is dropped.

The state threshold differs from mux-h2: there, H2_CS_FRAME_H means "waiting
for the next frame header" and is reached before any frame is processed,
while FCGI_CS_RECORD_H is the state where records may be exchanged, the
three states below it covering only the GET_VALUES handshake.

This should be backported to all supported versions.

BUG/MEDIUM: mux-fcgi: arm the connection timeout only when data remains

The commit "BUG/MEDIUM: mux-fcgi: do not enforce the timeout on connections
with streams" made the connection timeout inactive while stream connectors
are attached, since the stream layer is then responsible for timeouts. It
has to become active again once the last stream detaches, which detach()
does not do in all cases: when the stream is blocked waiting for room in the
mux buffer, the function returns early without touching the timeout, and
when the connection is moved to an idle list, it returns before the refresh
at the end of the function.

In both cases the connection stays without any timeout. When a client aborts
a large POST to a FastCGI server that does not read it, the connection with
its full mux buffer is put in the server's idle list and is only closed
later by the purge of the idle connections, instead of after "timeout
server".

Arming "timeout server" on every connection without a stream is wrong
though: a connection with nothing left to send is idle and reusable, and
belongs to the idle connection pool for its lifetime. Arming the server
timeout there closes it while it sits in the idle list, defeating connection
reuse, unlike idle backend H1 and H2 connections which are left without a
mux timeout.

So the update is moved to fcgi_conn_update_timeout(), which now tells the
cases apart, and is called once right after the stream is destroyed, before
the connection may be moved to an idle list. The refresh that used to sit at
the end of the function is dropped: everything between the two points only
manipulates session and idle lists, so the second call recomputed the very
same expiration date.

  - streams attached: no connection timeout, the stream layer knows
    whether it waits for the server or for the client;
  - nothing attached, but data pending in the mux buffer or streams still
    queued in the send list (blocked on a missing mux buffer, their bytes
    still in their own buffer): the regular data timeout, so that the
    data gets a chance to be drained and the connection closed after it;
  - nothing attached, nothing to send, connection closing: the shutdown
    timeout, otherwise nothing would close it;
  - nothing attached, nothing to send, connection alive: no timeout, it
    is an idle connection.

tick_add_ifset() replaces tick_add() so an unset timeout cannot arm an
already expired timer.

It is related to GitHub issue #3558 and should be backported along with the
mentionned commit to all supported versions.

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

h2_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, so a connection detached with data still queued in
the mux buffer is parked without any timeout and nothing drains it. The
timeout used to be refreshed for idle-ping only, which left every other
case aside.

The refresh is now performed right after h2s_destroy(), before the
connection may be given away, and the redundant 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.
h2c_update_timeout() already returns early when there is no task, so no
guard is needed at the call site.

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
h2_release() clears its task context. This only reorders statements:
h2c_update_timeout() touches neither the stream map nor the flags and
states h2c_is_dead() looks at, so the predicate gives the same result at
both places. Since a dead connection now returns early, the
"!h2c_is_dead(h2c)" test that guarded the idle-list branches, and the
h2c_is_dead() check that closed the function, are both redundant and are
removed. Commit 59c599f3f ("BUG/MEDIUM: mux-h2: make sure not to move a
dead connection to idle") is thus enforced structurally rather than by a
duplicated test.

This could be backported as far as 2.6.

BUG/MEDIUM: mux-h2: consider the send list as pending data for the timeout

When no stream connector is attached anymore, h2c_update_timeout() only
looks at the mux buffer to decide whether the connection still has data
to send, in which case it must be armed with the regular data timeout.
Streams may however still be queued in the send list while their data
sits in their own iobuf: they are blocked on a missing mux buffer, so
br_data() on the mux buffer is zero even though some bytes remain to be
sent. Such a connection is then considered idle and armed with neither
the data timeout nor anything else.

Let's also consider the send list in the condition, as it was part of
the original h2c_may_expire() before commit 3439583dd ("MEDIUM: mux-h2:
slightly relax timeout management rules") reduced it to the attached
stream count, with only br_data() being put back afterwards. This
matches what fcgi_conn_update_timeout() does.

This could be backported as far as 2.6.

BUILD: debug: fix _COUNT undefinied with USE_OBSOLETE_LINKER

We recently introduced the COUNT() macro with commit 29925e59a7 ("MINOR:
debug: add a COUNT() macro which doesn't take a condition"). However it
relies on a _COUNT() macro that is not defined when USE_OBSOLETE_LINKER
is used, causing build failures visible in the CI with ASAN builds.
Let's just add it. No backport needed.

BUG/MEDIUM: compression: use a trash chunk as large as the buffer for the output

Since commit a0b37fb47 ("BUG/MEDIUM: compression: fix incompatibility
between compression and large-buffers"), slz_encode() writes directly into
the output chunk passed by the compression filter, which is the global
trash, sized like regular buffers. But with large buffers, a DATA block may
be larger than this trash. The input is then cut to the trash size by the
test added in commit f82ace414 ("MEDIUM: compression: Be sure to never
compress more than a chunk at once"), leaving no room for the deflater's
overhead. On incompressible data, the first call emits the input plus 5
bytes for the stored block plus the 10 bytes of the gzip header, which
overflows the trash:

  FATAL: bug condition "b->data + count > b->size" matched at include/haproxy/buf.h:395
  #0 b_add (b=..., count=32783) at include/haproxy/buf.h:395
  #1 comp_rfc195x_add_data (..., in_len=32768, out=...) at src/compression.c:404

This was seen in production on a PDF served with 32 kB buffers and
"http-response wait-for-body use-large-buffer", where the body formed a
37 kB DATA block. Without DEBUG_STRICT=2, the overflow goes unnoticed.

The output replaces the input in the HTX, so the right size for the output
chunk is the one of the buffer it comes from. It is now obtained with
get_best_trash_chunk(), which returns a large trash chunk for large buffers
and the regular one otherwise. htx_compression_buffer_init() still ensures
that the HTX has the room for the deflater's overhead, so a DATA block
plus this overhead always fits in such a chunk. A side effect is that the
data of a large buffer are now compressed in a single call, as intended by
the use of large buffers for compression.

The reg-test compression/large_buffers.vtc reproduces the crash with
unpatched versions built with DEBUG_STRICT=2.

This must be backported wherever a0b37fb47 is backported.

DOC: minor: update VTest2 URL everywhere

In commit 27e1ec8ca97 ("CI: vtest: move the vtest2 URL to
vinyl-cache.org"), the VTest2 URL was updated for CI.

Also update the URLs in the docs.

Must be backported to all branches including 2.6 and 2.8.

BUG/MINOR: server: fix a leak when completing a named default-server

A named default-server may be defined over several lines, each line
completing the previous one, as documented and as exercised by
reg-tests/server/from_keyword.vtc. In _srv_parse_init(), the name is
duplicated with strdup() before being looked up in defsrv_by_name.
When the lookup finds the existing default-server, the copy is neither
stored nor freed, so every line after the first one leaks it.

The leak only happens at configuration load and is limited to the name
length plus one byte per line, so the impact is minimal, but it is
reported by LeakSanitizer, e.g. "haproxy -c" built with
-fsanitize=address on a configuration defining the same
"default-server name other ..." on several lines.

Fix it by freeing the copy when an existing default-server is found,
since that one already owns its name. The code path is already covered
by reg-tests/server/from_keyword.vtc, so no new regression test is
added.

This was introduced with named default-servers by commit 507f790af
("MEDIUM: proxy: implement named default-server") in 3.5-dev4, so no
backport is needed.

This fixes GitHub issue #3562.

BUG/MINOR: applet: accept a temporarily owned tid in appctx_init()

appctx_init() makes sure the appctx is initialized on the thread it is bound
to, and checks it with a BUG_ON() which tolerates an unbound task (tid ==
-1), since an appctx may be created without a target thread and left to
whichever thread handles it first. That tolerance was added by commit
4f7c26cbb ("BUG/MINOR: applet: Don't trigger BUG_ON if the tid is not on
appctx init") for 2.8, at a time when a task's thread ID was either a real
thread number or -1.

Since commit 8b6d8f5e4 ("MINOR: tasks: Add __task_get_new_tid_field()"), the
scheduler encodes the temporary ownership of a shared task differently: when
a thread takes ownership of a task whose tid is -1, it sets -2 - <thread>
instead, so that no other thread can grab it before it is queued or
run. That value is still "not bound" but designates a specific owner, and it
is what task_wakeup() and task_queue() now set through
__task_get_new_tid_field(). The task code already knows about it:
__tasklet_wakeup_here() carries the very same triple test, and
task_set_thread() has an explicit early return for "t->tid == -2 - tid" when
the requested thread is -1.

appctx_init() did not, so it would trip the BUG_ON() on a task that happens
to be in that transient state, instead of adopting the current thread as
task_set_thread() is about to do. Let's accept -2 - tid here too, so that
the three states handled by task_set_thread() are all accepted: unbound,
bound to the current thread, or temporarily owned by the current thread.

This is not reachable today: every appctx is created with an explicit target
thread (appctx_new_here() or appctx_new_on() with a valid thread number), so
its task tid is never -1 and never -2 - tid, and appctx_new_anywhere()
currently has no caller. It is nonetheless a latent bug: any future user of
appctx_new_anywhere(), which is the intended way to start an appctx on a
thread subset, would hit the BUG_ON() as soon as the task is woken up before
being initialized.

The "-2 - tid" encoding was introduced in 3.5-dev1, so no backport needed.

CLEANUP: debug/tree-wide: replace COUNT_IF(1, ...) with COUNT()

This patch replaces 28 occurrences of COUNT_IF(1, msg) with COUNT(msg).
This makes the counters output more readable, the code as well, and even
saves a few bytes (148) in the executable.

MINOR: debug: add a COUNT() macro which doesn't take a condition

Most of the COUNT_IF() calls are placed in locations where they're not
part of a condition, but we just want to count one pass at a specific
location (e.g. branch of an "if" or after a label, etc). In such cases
it's systematically written as COUNT_IF(1, ...), but it's ugly, adds
bytes in the code, and needlessly clobbers the output.

Let's introduce a new variant COUNT() which doesn't require a condition
and only counts the number of passes. This is possible since COUNT_GLITCH
was introduced and already unlocked the ability to write messages without
conditions, so let's just exploit this.

BUILD: http: fix build of macro http_harden() with clang

clang doesn't seem to like that COUNT_IF() returns an expression that
produces a result that is not used:

    CC      src/http_htx.o
  src/http_htx.c:2076:16: warning: expression result unused [-Wunused-value]
                  if (unlikely(http_harden(HTTP_HRD_DEL_ALL_TLR))) {
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/http.h:137:3: note: expanded from macro 'http_harden'
                  COUNT_IF(1, "hardening " #flag " condition met");             \
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/bug.h:485:31: note: expanded from macro 'COUNT_IF'
  #  define COUNT_IF(cond, ...) _COUNT_IF   (cond, __FILE__, __LINE__, __VA_ARGS__)
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/bug.h:388:7: note: expanded from macro '_COUNT_IF'
          }) : 0)
               ^
  include/haproxy/compiler.h:351:40: note: expanded from macro 'unlikely'
  #define unlikely(x) (__builtin_expect((x) != 0, 0))
                                         ^
  src/http_htx.c:2076:16: warning: expression result unused [-Wunused-value]
                  if (unlikely(http_harden(HTTP_HRD_DEL_ALL_TLR))) {
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/http.h:139:4: note: expanded from macro 'http_harden'
                          COUNT_IF(1, "hardening " #flag " enforced");          \
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/bug.h:485:31: note: expanded from macro 'COUNT_IF'
  #  define COUNT_IF(cond, ...) _COUNT_IF   (cond, __FILE__, __LINE__, __VA_ARGS__)
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/haproxy/bug.h:388:7: note: expanded from macro '_COUNT_IF'
          }) : 0)
               ^
  include/haproxy/compiler.h:351:40: note: expanded from macro 'unlikely'
  #define unlikely(x) (__builtin_expect((x) != 0, 0))
                                         ^
  2 warnings generated.

Just placing a (void) in front of COUNT_IF() is sufficient to silence it.
No backport is needed, as this is only in 3.5.

BUG/MEDIUM: quic: fix loss time selection in quic_loss_pktns()

quic_loss_pktns() must return the packet number space with the earliest
loss time. But an unset loss time (TICK_ETERNITY, i.e. 0) could win the
selection and hide a real loss time.

Indeed, since a55acf993 ("BUG/MINOR: quic: ticks comparison without
ticks API use"), the candidate loss time is compared with
tick_is_lt(). An unset candidate (0) then beats any armed loss time T
whenever (0 - T) wraps to a negative value, that is to say whenever
T lies in the low half of the 32-bit tick circle. c7c87641 ("MINOR:
quic: Wrong packet number space selection in quic_loss_pktns()") had
already fixed the same problem for the naive comparison in 2021, but
the guard was inverted by 1f6cf181 in the same year.

The bug is not a rare calendar coincidence: clock.c forces the internal
time to wrap BOOT_TIME_WRAP_SEC (20) seconds after boot, after which
now_ms starts over from zero and lives in the low half of its 32-bit
circle for about 24.9 days. Thus, on every instance restarted less
than ~25 days ago, the loss time of the Initial packet number space
could be discarded. The Initial packets in flight were then never
declared lost, their loss time was never honored, and the PTO branch
kept arming probe requests on both Initial and Handshake spaces
without end, as long as the Handshake packets never reached the
client: with the Handshake probe flag cleared without emitting
(quic_tx.c), the server entered a tight timer loop sending padded
Initial PINGs over and over without ever retransmitting the Handshake
CRYPTO data. A conforming client breaks this loop within about one
second by sending the anti-deadlock Handshake probe it must arm when
it holds Handshake keys but no Handshake packet of its own was
acknowledged; a loop lasting minutes was only observed with a client
missing this behavior.

Fix the selection by considering only the spaces with an armed loss
time. If none is armed, the function returns the first space of the
list, and both callers already check tick_isset() on the returned
loss time, so nothing else changes.

Thank you to @jbevemyr for having reported this issue in GH #3552.

This could be backported as far as 2.6 where the tick_is_lt() call
was introduced.

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

The FCGI connection task is rearmed with "timeout server" on each pass
in fcgi_process(), and fcgi_timeout_task() kills the connection when it
expires, even when streams are still attached to it. With a slow
client, HAProxy stops reading from the FastCGI server once the buffers
are full, so nothing happens on the FCGI connection for a while, and
the connection is killed in the middle of the response. The response is
truncated, the log reports "SD--", and the FastCGI server gets a reset
while it is blocked in write().

Whether the stream waits for the server or for the client is only known
by the stream layer, which already enforces "timeout server" when the
server does not answer. So, as done for the H2 mux by commit c2ea47fb1
("BUG/MEDIUM: mux-h2: do not enforce timeout on long connections", for
GitHub issue #308), the connection timeout is now only enforced when no
stream connector is attached anymore: fcgi_conn_may_expire() mirrors
h2c_may_expire(), fcgi_process() leaves the task unarmed while streams
are attached, and fcgi_timeout_task() does not kill the connection in
this case.

A FastCGI server which does not answer at all still gets a 504 after
"timeout server", reported as "sH--", as before.

This fixes GitHub issue #3558. It should be backported to all supported
versions.

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

BUG/MEDIUM: filters: avoid NULL block access in decompression

The decompression filter may end up with a NULL block pointer when
processing an empty request. If no replacement block is provided,
htx_get_next_blk() is called with this NULL pointer, causing HAProxy
to crash.

Only advance to the next block if the current block pointer is valid.

This was introduced 3.5-dev, no backport needed.

MEDIUM: h1/violations: restrict valid version strings to "1.0" and "1.1"

The "invalid-version-string" HTTP violation only permitted to use
versions which are not made of exactly one digit, a dot, and one digit.
But the only versions where such a version is advertised are 1.0 and
1.1, and after them no other version is advertised. This means that the
only valid tokens are exactly 1.0 and 1.1. There's no point in accepting
"0.9", "1.2" nor "2.0" for example, and while haproxy handles them
carefully, some components only look at the second digit, or apply "&1"
to it resulting in 1.2 becoming 1.0 etc. Better be strict on this and
block these ones unless the violation is explicitly accepted. No
visible effect is expected given that the vast majority of servers and
proxies already do not accept other versions.