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.
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.
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.
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.
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.
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.
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.
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.
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.
Willy Tarreau [Tue, 10 Nov 2026 09:11:07 +0000 (10:11 +0100)]
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.
Tiburce GIROUX [Mon, 9 Nov 2026 18:13:37 +0000 (19:13 +0100)]
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:
(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>
Willy Tarreau [Wed, 7 Oct 2026 14:21:44 +0000 (16:21 +0200)]
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.
Willy Tarreau [Mon, 9 Nov 2026 19:14:45 +0000 (20:14 +0100)]
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.
Willy Tarreau [Mon, 9 Nov 2026 17:53:21 +0000 (17:53 +0000)]
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.
Willy Tarreau [Tue, 6 Oct 2026 21:22:06 +0000 (23:22 +0200)]
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.
Willy Tarreau [Tue, 6 Oct 2026 20:53:29 +0000 (22:53 +0200)]
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.
Willy Tarreau [Tue, 6 Oct 2026 20:33:55 +0000 (22:33 +0200)]
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.
Willy Tarreau [Tue, 6 Oct 2026 20:31:50 +0000 (22:31 +0200)]
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.
Willy Tarreau [Tue, 6 Oct 2026 20:29:00 +0000 (22:29 +0200)]
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.
Willy Tarreau [Tue, 6 Oct 2026 20:26:44 +0000 (22:26 +0200)]
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.
Willy Tarreau [Mon, 9 Nov 2026 16:49:11 +0000 (17:49 +0100)]
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.
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.
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.
Amaury Denoyelle [Thu, 29 Oct 2026 14:24:59 +0000 (15:24 +0100)]
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.
Amaury Denoyelle [Thu, 29 Oct 2026 14:38:59 +0000 (15:38 +0100)]
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.
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.
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.
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.
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.
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.
Willy Tarreau [Mon, 9 Nov 2026 10:12:01 +0000 (11:12 +0100)]
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.
Willy Tarreau [Sun, 8 Nov 2026 11:03:45 +0000 (12:03 +0100)]
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.
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.
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.
Willy Tarreau [Fri, 6 Nov 2026 15:17:00 +0000 (16:17 +0100)]
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.
Willy Tarreau [Fri, 6 Nov 2026 15:11:47 +0000 (16:11 +0100)]
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.
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>
Adam Crawford [Thu, 5 Nov 2026 23:32:47 +0000 (17:32 -0600)]
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.
Willy Tarreau [Thu, 5 Nov 2026 17:14:02 +0000 (18:14 +0100)]
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.
The index of the gpc, gpc_rate and gpt arrays is never checked where it
is parsed, so invalid values are silently accepted:
- the sc-inc-gpc, sc-set-gpt and sc-add-gpc actions parse it with
strtoul(), which accepts a sign and wraps "-1" to UINT_MAX, and
which returns 0 for an empty index, so "sc-inc-gpc(,0)" updates
gpc[0]. Values of 100 and above are accepted as well.
- the sample fetches and converters taking an index (sc_get_gpc,
sc_get_gpt, sc_gpc_rate, sc_inc_gpc, sc_clr_gpc, sc1_clr_gpc and the
src_* and table_* variants) take it as a signed integer without any
check, so sc_get_gpc(-1,0) or table_gpc(100,t) pass "haproxy -c" and
never return anything.
- on the CLI, "data.gpc[-1]" is taken as index 4294967295, so "set
table" reports "index out of range in this data array" instead of
rejecting the name as for "data.gpc[q]", and the "show table" and
"clear table" filters accept it and match no entry, as they do for
any index beyond the table's array.
Since the index is stored as an unsigned int, values above UINT_MAX are
even truncated: "sc-inc-gpc(4294967296,0)" increments gpc[0] and
sc_get_gpc(4294967296,0) returns gpc[0], and the same happens on the
CLI. There is no memory access issue since stktable_data_ptr_idx()
checks the index against the array size before use, but these errors
should be reported where the index is parsed. The documentation states
that <idx> is an integer between 0 and 99, which is
STKTABLE_MAX_DT_ARRAY_SIZE - 1.
This patch adds stktable_parse_array_idx(), which requires the index to
start with a digit and to be lower than STKTABLE_MAX_DT_ARRAY_SIZE, and
uses it in stktable_get_data_type_idx() and in the three action parsers.
The fetches and converters get a new argument check function which does
the same verification on their first argument. Finally, the CLI table
filters now reject an index beyond the array size of the table, as "set
table" already does. The table is not known when the actions, fetches
and converters are parsed, so they can only be checked against the
maximum array size.
This is the index counterpart of commit 1f1b701fe3 ("BUG/MINOR:
stick-table: reject negative stick counter IDs in sc-* actions").
Configurations using an invalid index are now rejected, but such an
index never worked, or silently used gpc[0]/gpt[0].
This should be backported to all stable versions where the gpc/gpt
arrays exist (2.5 and above). The CLI parts only apply to 3.2 and above
where stktable_get_data_type_idx() was added, and sc-add-gpc() appeared
in 2.8.
Alex Joseph [Tue, 3 Nov 2026 21:19:51 +0000 (21:19 +0000)]
MINOR: tcpcheck: add post-80-sha2 mode to mysql-check for caching_sha2_password
MySQL 8.0 made caching_sha2_password the default authentication plugin
and MySQL 8.4 disables mysql_native_password by default (it is removed in
9.0). None of the existing mysql-check modes works with a
caching_sha2_password check user, and each of them generates an aborted
connection on the server for every check:
- pre-41 / post-41 do not set CLIENT_PLUGIN_AUTH, so the server rejects
the handshake with error 1251 ("Client does not support
authentication protocol requested by server") and the server is
marked DOWN.
- post-80 announces mysql_native_password, so the server answers with
an AuthSwitchRequest (0xFE) to caching_sha2_password. The check
accepts it as a valid reply since it is not an error packet, and
reports the server UP. But the COM_QUIT pipelined after the handshake
response is read by the server as the auth switch reply, and the
handshake is aborted with error 1156 ("Got packets out of order").
In both cases the server counts a connection error for the HAProxy host
(Aborted_connects, and the host cache handshake errors when
skip_name_resolve is OFF, which is the MySQL default, and the HAProxy
address resolves forward and backward). Once the host reaches max_connect_errors
(100 by default), the server blocks it with ER_HOST_IS_BLOCKED (1129)
and refuses every connection from it, including proxied client traffic,
until FLUSH HOSTS is issued. With post-80 this happens while the check
reported the server as healthy.
This patch adds a "post-80-sha2" mode which sets CLIENT_PLUGIN_AUTH and
announces caching_sha2_password with an empty auth response. For a
password-less caching_sha2_password account the server authenticates in
a single round trip and replies with an OK packet, then the pipelined
COM_QUIT closes the session cleanly, so no connection error is recorded:
CREATE USER 'haproxy'@'%' IDENTIFIED WITH caching_sha2_password BY '';
option mysql-check user haproxy post-80-sha2
Tested against MySQL 8.4.13 (skip_name_resolve=OFF,
max_connect_errors=10) and Cloud SQL for MySQL 8.4.11 with a
caching_sha2_password check user, one check every 2s: with post-80-sha2
the check is L7OK and neither Aborted_connects nor the host cache error
counters move. With post-41 or post-80 every check adds one aborted
connection and one host cache handshake error, and the HAProxy host is
blocked (1129) after 10 checks, i.e. 20 seconds.
The documentation and the mysql-check regtest are updated accordingly.
Signed-off-by: Alex Joseph <alex.lurthu@gmail.com>
Martin Mirchev [Fri, 30 Oct 2026 05:32:30 +0000 (13:32 +0800)]
BUG/MINOR: tcpcheck: check the number of "hdr" in http-check send
parse_tcpcheck_send_http() collects the "hdr <name> <value>" arguments
of an "http-check send" rule into a stack array sized by
tune.http.maxhdr, followed by an IST_NULL terminator, but never checks
the number of headers against that size. When tune.http.maxhdr is
lowered and a rule sets as many headers as the limit, the terminator is
written past the end of the array, and each extra header writes one
more entry past it.
This happens while parsing the configuration. E.g. with:
global
tune.http.maxhdr 5
backend b
option httpchk
http-check send meth GET uri /health hdr X-H1 v1 hdr X-H2 v2 \
hdr X-H3 v3 hdr X-H4 v4 hdr X-H5 v5
server s1 127.0.0.1:1 check
"haproxy -c" crashes with a segfault on a release build, and ASan
reports a dynamic-stack-buffer-overflow WRITE in
parse_tcpcheck_send_http(). With the default value (101) it cannot be
reached because a config line is limited to 64 words.
Let's reject the rule with an error when there are too many headers,
keeping room for the terminator.
This should be backported to all stable versions. The code dates back to
2.2 and commit e5870d872b ("MAJOR: checks: Implement HTTP check using
tcp-check rules").
BUG/MEDIUM: spoe: stop waiting on a dead producer channel
spoe_process_event() interrupts a pending event processing when the frontend
connector reports an error or an early close: there is no point waiting for
an agent reply if the stream is being torn down from the client side.
The check was only made on the frontend connector. With an event bound to
the response channel (on-http-response), a server error reported while the
processing is pending leaves the SPOE context in SENDING_MSGS/ WAITING_ACK
with the processing timeout armed on res->analyse_exp, and nothing can wake
it up anymore.
Test the error on the producer of the channel the event is bound to: scb
in addition to scf for response-side events, the scf checks remaining
valid whatever the direction. For the response side, only SC_FL_ERROR is
relevant: an early server close with "option abortonclose" is a
request-side semantic which is already handled on the request path. The
context then concludes with the INTERRUPT error, which lets the response
analyser proceed and deliver whatever response was already received.
This is the SPOE part of github issue #3559: with a filter engine bound to
on-http-response, an H2 backend answering early and resetting its stream
while the request body is still being uploaded used to leave the stream
spinning in process_stream() until the watchdog killed the worker. The
send-spoe-group action is covered by the http-ana fix, as it is called from
the http-response rule engine.
BUG/MINOR: http-ana: also set FINAL on http-response actions on server error
http_res_get_intercept_rule() stops http-response actions from yielding once
the client side is dead: when the frontend connector reports an error or an
early close, the action is called with ACT_OPT_FINAL, so a yield is refused
and the caller receives an HTTP_RULE_RES_ERROR instead of being left waiting
forever.
The server side was missing: when the backend connector reports an error
while an http-response action (e.g. send-spoe-group) is yielding, the action
kept waiting for a wakeup that would only ever come from the server it just
lost, and the response was stuck in the channel until the client timed out.
Add the missing server-side condition: an SC_FL_ERROR on the backend
connector also sets ACT_OPT_FINAL, so http-response actions get a final
chance to conclude when the server dies. Combined with the previous commit,
the "server answered early and reset its stream while the action was
pending" case from github issue #3559 now lets the action conclude with an
error instead of hanging the response.
This patch should be backported to all supported versions.
BUG/MEDIUM: stream: reset analyse_exp when analysers are stripped on error
When the client or the server connector reports an error or a timeout that
no analyser handled, process_stream() strips all analysers from the
corresponding channel to let the error propagate, except the flt_end one.
When this happens while an analyser was yielding with the channel's
analyse_exp timer armed (e.g. an SPOE event or a Lua action waiting on an
external party), the stripped analysers can no longer manage nor reset this
timer. If it already expired, update_exp_and_leave() refuses to clear it as
long as the AN_*_FLT_END bit is kept for the two-channel filter
synchronisation, so t->expire is set to a past date, the task is requeued in
the past and process_stream() spins in a tight loop until the watchdog kills
the worker.
The request-side retry path (http_ana.c, http_req_retry) already resets
res->analyse_exp right after the same strip; do the same on both error
propagation paths of process_stream() so that an armed analyse timer can
never survive the analysers it was armed for.
This is the first commit of the series fixing github issue #3559. It must be
backported to all stable versions
Willy Tarreau [Thu, 5 Nov 2026 07:54:06 +0000 (08:54 +0100)]
MINOR: http/hardening: turn it to a macro and count matches with COUNT_IF()
Thanks to COUNT_IF() it is quite easy to count the number of times the
condition for a given hardening flag was met, and the number of times it
was enforced. Let's replace the function by a macro for this. In order
to avoid counting multiple times the same match (e.g. many CTL chars in
a header value), the macro takes care of checking both the found and the
rules flags. This way we have at least one found per enforcement, and at
least one per distinct match but not necessarily more if no enforcement
is configured.
Willy Tarreau [Thu, 5 Nov 2026 07:51:27 +0000 (08:51 +0100)]
BUG/MINOR: http/hardening: don't count unregistered hdr/tlr too early
In 3.5-dev11, the ability to remove unregistered headers and trailers
(X-*) was added with commits 4b9b6044b2 ("MINOR: http/hardening: add
"delete-unregistered-header-fields"") and b5fd464f0a ("MINOR:
http/hardening: add "delete-unregistered-trailer-fields""). There is
a small mistake there, which is the the http_harden() call is made
before checking if the situation was met, resulting in the flags being
set for each header/trailer. For now this doesn't have any impact but
it will when reporting is implemented. Let's fix that by only calling
http_harden() after the header syntax was checked. No backport is
needed.
BUG/MINOR: fd: allow fd_write_frag_line() to run before fdtab allocation
With traces enabled on the command line using "-dt ssl:developer:clean",
a trace event is emitted during the configuration check if an SSL bind
line uses "generate-certificates".
check_config_validity() calls ->prepare_bind_conf() for every bind line.
For SSL, this is ssl_sock_prepare_bind_conf(). When "generate-certificates"
is set, that function checks that a default certificate exists, and calls
ssl_sock_choose_sni_ctx() for this. This happens long before
init_pollers() is called, so fdtab is still NULL. fd_write_frag_line()
reads fdtab[fd].state to take the per-fd lock and to test FD_INITIALIZED,
and the process segfaults as soon as the trace is written to stderr.
fd_write_frag_line() has read fdtab since 931d8b79a (v2.1), but no trace
could be emitted that early at that time. The crash became possible with 105c1ca13 (v3.2-dev13), which added traces to ssl_sock_choose_sni_ctx();
ssl_sock_prepare_bind_conf() has called this function from the
configuration check since b80635a7e (v3.0).
At this stage the process is still single-threaded, so the per-fd lock is
not needed. Skip the fdtab accesses when fdtab is not allocated yet, and
call writev() directly.
Owais Lone [Tue, 3 Nov 2026 10:29:01 +0000 (10:29 +0000)]
REGTESTS: protobuf: cover the ungrpc message realignment
Regression test for 3a319e14c ("BUG/MINOR: sample: realign the ungrpc
converter on message boundaries").
grpc.vtc already walks a body of two gRPC messages, but the lookup
succeeds in its first message, so the cursor is left at the message end
and the bug does not show: that file passes both before and after the
fix. Reaching the defect needs a lookup that fails part-way through the
first message, which a length-delimited field whose wire type does not
match the requested one produces.
The body is 23 bytes and holds field 1 twice: length-delimited in the
first message, with a payload that reads as a gRPC header once the
cursor is misaligned, and as a varint 9999 in the second one.
ungrpc(1,int64) must return 9999. Before the fix it returned 1337, a
field of no message of the body.
Checked against 3a319e14c (passes) and its parent 1f114e4c4 (fails,
returning 1337).
CLEANUP: quic: remove the duplicate qc_prep_hdshk_fast_retrans() prototype
This function is declared in quic_retransmit.h, which is included both by
quic_tx.c and quic_rx.c, its only two callers. The second declaration in
quic_tx.h is therefore useless.
CLEANUP: quic: drop the intermediate packet tree pointer in the fast retransmissions
In qc_prep_fast_retrans() and qc_prep_hdshk_fast_retrans(), the local
<pkts> pointer is assigned once and then only used once, as argument of
eb64_first(). Dereference the packet number space tree directly instead,
which removes five lines without changing the behaviour.
CLEANUP: quic: remove the dead encryption level variable in qc_prep_hdshk_fast_retrans()
<qel> was assigned once to qc->iel and never modified afterwards, so
every "qel == iqel" test was always true. Use <iqel> directly for the
packet number space and for the pto_probe increment, test the
anti-amplification condition positively, and unwrap the requeue
condition.
The <start> label has had no goto target since this function stopped
looping over encryption levels, remove it.
CLEANUP: quic: use QUIC_PACKET_PN_MAXLEN instead of the magic 4
Both fast retransmission functions account for the packet number increase
when comparing the size of the packets to be retransmitted with the
anti-amplification limit. They use a plain 4 for this, while
quic_conn-t.h defines QUIC_PACKET_PN_MAXLEN for this exact purpose, as
already used in quic_tx.c for the header protection sample.
Replace the magic value in qc_prep_fast_retrans() and
qc_prep_hdshk_fast_retrans().
CLEANUP: quic: remove a redundant test in the fast retransmission lookups
qc_pkt_with_only_acked_frms() returns 1 for a packet without any frame,
since its loop is not entered and it falls back on the final return 1.
The LIST_ISEMPTY(&pkt->frms) test combined with it is therefore
redundant, both in qc_prep_fast_retrans() and in
qc_prep_hdshk_fast_retrans().
CLEANUP: quic: fix and complete the comments in quic_retransmit.c
Three comments are inaccurate:
- qc_prep_fast_retrans() is documented as taking a <qel> argument, but
it has been replaced by <pktns> for a while;
- the packet lookup loop of qc_prep_fast_retrans() is said to skip the
coalesced packets, while QUIC_FL_TX_PACKET_COALESCED is not tested
there at all. Only the Handshake lookup loop skips them;
- qc_pkt_with_only_acked_frms() is documented as returning 1 for a
packet made only of acknowledged frames, but it also returns 1 for a
packet without any frame. qc_prep_fast_retrans() relies on this to
skip the empty packets.
Three comment blocks are added where the intent is not obvious: the
<tmp> list used by qc_dup_pkt_frms() to prepend the duplicated frames
ahead of the pending ones, the <pkt> variable which is deliberately not
reset before the <start> label so that a second probe datagram is built
from the same frames when no other packet may be retransmitted, and the
Initial packet number space which may still be probed with a PING frame
when the anti-amplification limit allows to send a full-size datagram.
CLEANUP: quic: compute the STREAM frame end offset once in qc_stream_frm_is_acked()
The end offset of a STREAM frame, frm->offset + frm->len, was recomputed
four times in qc_stream_frm_is_acked(): once in the BUG_ON() and three
times in the offset comparisons. Store it in a local frm_end variable, as
already done for the FIN bit flag.
Willy Tarreau [Wed, 4 Nov 2026 22:22:17 +0000 (23:22 +0100)]
[RELEASE] Released version 3.5-dev11
Released version 3.5-dev11 with the following main changes :
- BUILD: sym: Fix build on recent glibc
- BUG/MINOR: checks/log: force implicit UDP checks as TCP for log backend
- CLEANUP: sym: factor out the short object name extraction
- MINOR: sym: use the symbol table to resolve a DSO name
- BUG/MINOR: errors: reset diagnostic buffers after thread cleanup
- BUG/MINOR: sample: fix NULL dereference in the ltime/utime converters
- BUILD: Makefile: extend the capabilities of EXTRA_MAKE
- MINOR: thread: add EXTRAS_LOCK so addons can name their locks
- BUG/MINOR: debug: fix "debug dev fd" crash on unknown address families
- BUILD: Makefile: allow external makefiles to provide build targets
- BUG/MEDIUM: ssl: skip TLS 1.3 suites in the FIPS TLS 1.2 cipher check
- REGTESTS: ssl: skip AWS-LC FIPS rejected crypto in ciphersuites/curves tests
- CI: github: add AWS-LC FIPS testing workflow
- BUILD: github: run FreeBSD CI on every branch
- CI: github: cache VTest binary in FreeBSD workflow
- BUG/MEDIUM: proxy: fix stick table expired entries cleanup on stopping
- BUG/MINOR: http-ana: fix response compression for applets
- DOC: configuration: Clarify wait-for-body doc for the response side
- MINOR: http-act: Add 'no-fail' option on 'wait-for-body' action
- BUG/MEDIUM: leastconn: reset the smallest key when backups take over
- CLEANUP: quic: remove duplicated key phase pointer assignments
- MINOR: quic: use the right trace event in quic_tls_key_update()
- CLEANUP: quic: remove duplicated rx offset initialization in quic_cstream_new()
- CLEANUP: quic: remove the unused tx part of struct quic_cstream
- CLEANUP: quic: remove unused tx crypto size and offset counters
- BUG/MINOR: quic: fix the header protection key length dump with AWS-LC
- CLEANUP: quic: remove the unused key parameter from quic_tls_decrypt()
- CLEANUP: quic: remove the unused count and flags from struct quic_tls_kp
- CLEANUP: quic: remove the stale initial_salt declaration
- CLEANUP: quic: remove duplicated quic_cstream_new() declaration
- CLEANUP: quic: remove the unneeded quic_rx.h include from quic_tls.c
- CLEANUP: quic: update the outdated comment of quic_tls_derive_keys()
- BUG/MINOR: sample: realign the ungrpc converter on message boundaries
- BUG/MEDIUM: compression: fix incompatibility between compression and large-buffers
- CLEANUP: tools: make inetaddr_host_lim*() usable on untrusted inputs
- BUG/MINOR: flt-http-comp: check the block returned by htx_replace_blk_value()
- BUG/MINOR: compression: pass the configured level to slz
- OPTIM: flt-http-comp: only flush the compressor after the last DATA block
- IMPORT: slz: update slz to version 1.4-dev4
- MINOR: compression: pass the content type to the algorithms' init function
- MINOR: flt-http-comp: size the compression reserve using SLZ_MAX_OVERHEAD()
- REGTESTS: compression: check hard to compress contents and nearly full buffers
- BUG/MEDIUM: mux-h1: don't drop pending output on a short content-length
- BUG/MINOR: filters: Reject id-less implicit instances of multi-instance classes
- BUG/MINOR: ssl/ckch: free the new instance when ckch_inst_rebuild() fails
- BUG/MEDIUM: ssl/cli: reset the ongoing transaction when "commit ssl cert" is interrupted
- BUG/MEDIUM: ssl/cli: don't unlock the certificate lock after a failed trylock
- BUG/MINOR: httpclient/lua: fix stack overread when copying the headers
- BUG/MINOR: httpclient/lua: raise an error on too many request headers
- BUG/MINOR: resolvers: validate TXT RDATA before reading
- MINOR: stream: add per-stream memory slots
- BUG/MINOR: haterm: fix the response body phase on short writes
- BUG/MEDIUM: haload: restore request accounting after user timeouts
- BUG/MEDIUM: ssl: disable kTLS on OpenSSL versions affected by a known bug
- DOC: config: add missing empty line before tune.ssl.ktls-unsafe
- BUG/MINOR: ist: make ist_find_range() accept non-zero min boundaries
- MINOR: ist: add ist_find_http_ctl() to match http CTL chars
- MINOR: ist: add ist_find_outside() to find offending characters
- MINOR: http: use ist_find_http_ctl() instead of ist_find_ctl()
- MINOR: h1: indicate the trailers state to h1_parse_msg_tlrs()
- MINOR: http: introduce the notion of http-hardening flags and sections
- MINOR: cfgparse/http: add a simple parser for new section "http-fingerprint"
- MINOR: http: support default hardening names "untrusted" and "trusted"
- MINOR: cfgparse: dump the "http-hardening" keywords with -dKcfg
- MINOR: cli/http: add the "show http-hardening" CLI command
- MINOR: listener: add the "http-hardening" bind keyword
- MINOR: server: add the "http-hardening" server keyword
- MINOR: tools: add is_ip_untrusted() to check for non-local addresses
- DOC: config: document the "http-hardening" section and server/bind options
- MINOR: http: define http_hdng_chk for ambient hardening checks to be run
- MINOR: mux-h1: pass the profile's HTTP hardening checks into the h1c and h1s
- MINOR: mux-h2: pass the profile's HTTP hardening checks into the h2c and h2s
- MINOR: h3: pass the HTTP hardening checks to the h3s and h3c
- MINOR: http: add the "set http-hardening" CLI command
- MINOR: http/hardening: implement hardening "reject-duplicate-host-header"
- MINOR: http/hardening: implement hardening "reject-lowercase-method"
- MINOR: http/hardening: implement hardening "reject-underscore-in-header-name"
- MINOR: http/hardening: implement hardening "reject-underscore-in-trailer-name"
- MINOR: http/harderning: implement "delete-underscore-in-header-name"
- MINOR: http/harderning: implement "delete-underscore-in-trailer-name"
- MINOR: http/hardening: add "delete-unregistered-header-fields"
- MINOR: http/hardening: add "delete-unregistered-trailer-fields"
- MINOR: http/hardening: add "delete-all-trailer-fields"
- MINOR: http/hardening: implement hardening "reject-header-trailer-folding"
- MINOR: h1: rearrange the header value parser to better deal with CTL chars
- MINOR: http: speed up http_header_has_forbidden_char() to make it work alone
- MINOR: http: don't call ist_find_http_ctl() before up http_header_has_forbidden_char()
- MEDIUM: http/hardening: implement hardening "reject-ctl-in-header-trailer"
- MINOR: http/hardening: implement hardening "reject-content-length-with-te"
- MINOR: http/hardening: implement hardening "reject-content-length-leading-zero"
- MINOR: http/hardening: add aliases "all-reject" and "all-delete"
- MEDIUM: http: enable a few important hardening rules on untrusted connections
- REGTESTS: http-messaging: test the http-hardening checks on HTTP/1 headers
- REGTESTS: http-messaging: test the http-hardening checks on HTTP/2 headers
- REGTESTS: http-messaging: test the http-hardening checks on trailers
- REGTESTS: http-messaging: test http-hardening profiles and their CLI
Willy Tarreau [Wed, 4 Nov 2026 17:11:10 +0000 (18:11 +0100)]
REGTESTS: http-messaging: test http-hardening profiles and their CLI
This is an LLM-generated reg-test covering hardening config and CLI.
This covers the declaration and selection of http-hardening profiles, using
a few checks that are easy to observe on HTTP/1 requests:
- the implicit "trusted" profile is the one used over the loopback when
no profile or "auto" is set on the bind line, and both implicit
profiles may be redefined;
- "from" inherits the settings of another profile, and keywords are
applied in order ("all on" followed by an exception);
- "all-reject" and "all-delete" only cover their own family;
- a server inherits its profile from default-server, and "auto" resets
it to "trusted";
- "show http-hardening" lists the profiles (implicit ones being marked
as such) and dumps one profile's options, and "set http-hardening"
changes an option by name or number, with immediate effect on both
bind and server lines, and its error messages;
- the configuration errors: undeclared or forward-referenced profile on
bind and server lines, reserved name "auto", duplicate profile, unknown
or self parent, unknown keyword with a suggestion, invalid value.
Since the "untrusted" profile now enables a few rejections by default, a
redefined "untrusted" profile starts with reject-lowercase-method, so the
test must expect a lower case method to be rejected there. This is now
checked explicitly, as well as the default contents of the implicit
"untrusted" and "trusted" profiles as reported by "show http-hardening",
and their inheritance by "from".
Willy Tarreau [Wed, 4 Nov 2026 17:09:18 +0000 (18:09 +0100)]
REGTESTS: http-messaging: test the http-hardening checks on trailers
This is an LLM-generated reg-test covering hardening checks on trailers.
These are tricky because we don't have anything beyond TCP mode with
content inspection to check them.
Each trailer-related check gets its own profile, applied to a bind line for
request trailers and to a server line for response trailers, with both
HTTP/1 and HTTP/2 on the receiving side. A profile enabling header checks
verifies that these ones do not affect trailers.
vtest cannot parse HTTP/1 trailers, so the request trailers are forwarded in
HTTP/1 to a TCP frontend of the same instance, which waits for the whole
chunked message, then switches to HTTP and replies with the list of fields
it received. Response trailers are delivered to HTTP/2 clients, and the
response body is waited for so that a rejected trailer results in a 502.
Covered: underscore in trailer name (reject and delete), unregistered
trailer fields, deletion of all trailers, folding, CTL and DEL in values.
Willy Tarreau [Wed, 4 Nov 2026 17:08:51 +0000 (18:08 +0100)]
REGTESTS: http-messaging: test the http-hardening checks on HTTP/2 headers
This is an LLM-generated reg-test covering HTTP/2 hardening checks.
This is the HTTP/2 counterpart of http_hardening_h1.vtc for the checks that
apply to HTTP/2 (folding and Transfer-Encoding cannot appear there). A
rejected request must cause a stream error, except for an invalid method
which results in a 400 like other method errors, and a rejected response a
502. It also checks that a Host header accompanying an identical :authority
is not considered as a duplicate Host header.
Willy Tarreau [Wed, 4 Nov 2026 17:07:55 +0000 (18:07 +0100)]
REGTESTS: http-messaging: test the http-hardening checks on HTTP/1 headers
This is an LLM-generated reg-test covering HTTP/1 hardening checks.
Each header-related check gets its own profile, applied to a bind line for
requests and to a server line for responses, and a profile with everything
disabled shows that each offending message is accepted by default. The
frontend replies itself with the method, the header names and the value
of a header it saw, so that deletions are observed after the H1 parser.
Covered: duplicate Host, lower case method, underscore in header name (reject,
delete, and reject having precedence over delete), unregistered header
fields, folding (after the colon, and after a value with SP or HT), CTL and
DEL in values (with HT, SP and obs-text still accepted, and CTL found past
the first words), Content-Length with Transfer-Encoding, and Content-Length
with leading zeroes. Rejected requests must return a 400 and rejected
responses a 502.
Willy Tarreau [Wed, 4 Nov 2026 17:17:48 +0000 (18:17 +0100)]
MEDIUM: http: enable a few important hardening rules on untrusted connections
For connections coming from non-local IP addresses, we now enable these
hardening rules by default, which should not cause any trouble to
existing applications and which are essentially used to build attacks
when seen in public:
This will cause error 400 or 502 depending on the side they are triggered.
In HTTP/1 an error will be captured. We still don't have a capture of what
matched but that's a start.
Willy Tarreau [Wed, 4 Nov 2026 15:21:34 +0000 (16:21 +0100)]
MINOR: http/hardening: add aliases "all-reject" and "all-delete"
These aliases group all reject-* and all delete-* respectively into one
alias, allowing to simplify general cleanups on certain boundaries, or
to fix issues in emergency.
Rejects requests and responses having a Content-Length header field
whose value contains a leading zero before a digit. Some client and
server applications tend to parse this as an octal value giving a
different body size (e.g. "0300" = 192). This can be used in attacks
trying to desynchronize a client or server from the rest of the
communication, and for example for cache poisionning attacks. These may
seldom be seen on legacy applications but not on standard compoennts
found on the internet, so it is generally safe to reject them on public
facing listeners.
Rejects requests and responses having both a Content-Length header
field and a Transfer-Encoding header field. While these are permitted
by HTTP/1.1 (Transfer-Encoding wins), it can cause problems when the
message first passed through an HTTP/1.0 gateway that does not
recognize Transfer-Encoding, and can desynchronize this gateway from
the rest of the communication. These are still seldom seen in legacy
client or server applications, sometimes in relation with compression
and/or caching. While HAProxy properly normalizes them, it is generally
safe to reject them on public facing listeners.
Rejects requests and responses which contain control characters in
their header or trailer field values (codes 0-8,10-31,127). These ones
are forbidden but have long experienced a certain tolerance among
implementations, which has encouraged some rare applications to use
them as record delimiters for example. No standard component produces
them on the public internet, except attacks. By default, HAProxy only
blocks the strictly required ones (0,10,13) and lets other ones pass,
which can make certain backend applications trigger connection errors.
As such, it is both safe and recommended to block them on public facing
listeners. See also the has_ctl() converter used for detecting them.
Willy Tarreau [Wed, 4 Nov 2026 10:59:00 +0000 (11:59 +0100)]
MINOR: http: don't call ist_find_http_ctl() before up http_header_has_forbidden_char()
Now that the function was sped up to skip non-offending bytes, we don't
need ist_find_ctl() anymore. And since it would block us from finding
certain chars, let's now get rid of it.
Willy Tarreau [Wed, 4 Nov 2026 09:58:53 +0000 (10:58 +0100)]
MINOR: http: speed up http_header_has_forbidden_char() to make it work alone
The function reads one char at a time and is systematically hidden behind
a first call to ist_find_http_ctl() to speed up the lookup of control chars.
Unfortunately, this construct doesn't permit to find other control chars
such as 0x7F. Better make http_header_has_forbidden_char() self-sustaining,
but for this we need to implement the speedup there. For this, we start
with a loop using is_char8_outside(0x20,0x7e) that allows to fly over most
non-offending bytes (SP, VCHAR). Only HT (9) and obs-text (0x80..0xFF) will
exit early, which is not a problem given that they are extremely rare in
header values. After that we continue the loop to search for offending
bytes starting from the block of 4 or 8 that triggered.
Willy Tarreau [Wed, 4 Nov 2026 07:52:44 +0000 (08:52 +0100)]
MINOR: h1: rearrange the header value parser to better deal with CTL chars
The header value parser only made a special case of NUL, CR and LF in
its core loop, which doesn't ease detection of other CTL chars. Let's
first slightly rearrange the loop to make the special case of chars below
0x20 first. The checks were only moved inside a new "if" block but nothing
was changed. Use diff -b to better see.
Rejects requests and responses which contain folded header fields or
trailer fields. Folding consists in having a single field span multiple
lines. This practice is obsoleted by modern HTTP standards, absolutely
not seen anymore in valid messages over the internet but might still be
present in a few antique internal services dealing with large cookies.
On untrusted networks, they are always associated with attacks trying
to inject headers into non-compliant components, and should be rejected
as a sign of break-in attempt. Note that HAProxy does not have problems
with them and systematically unfolds these messages before forwarding
them.
Deletes from requests and responses any trailer field, regardless of
its content or syntax. This is done after syntactic checks though. The
vast majority of public facing applications do not use trailers because
they are not reliable (intermediaries are allowed to drop or fold them)
and their presence on public networks is generally associated with
attack attempts. It is generally safe to drop them by default on public
facing listeners, and even on servers meant to deliver public contents,
in order to avoid any information leakage. Note that gRPC services use
them.
This is the same as for the headers, but this time for the trailers. Better
keep them separated as trailers are used even worse than headers these days.
Similarly as for the underscore, unwanted trailers are dropped early for
h1.
Deletes from requests and responses any header field whose name starts
with 'x-' (upper or lower case). These header fields are not registered
with IANA and are not used across the internet by standard components.
They may appear in untrusted requests in attacks trying to exploit some
backend framework weaknesses however, as well as in old browsers that
were trying to work around known server bugs (e.G. "X-Pad"). While it
would be risky to block such requests, it is safe and even recommended
to delete such headers from inbound internet requests. It is preferable
not to do it on responses however when applications uses such header
fields to report information to HAProxy.
This is the same as for the headers, but this time for the trailers. Better
keep them separated as trailers are used even worse than headers these days.
Note that in h1 we delete them when we detect them since we already have
the code for this, no need to index them and delete them later.
Deletes from requests and responses any header field whose name
contains an underscore character ('_' or \x5C). While this character is
perfectly permitted, some application frameworks which create automatic
variables already turn the dash ('-') into an underscore in variable
names, leading to an ambiguity with the underscore which can result in
the same variable name. This could be abused to bypass some
access-control policy for example. No standard HTTP header uses this,
so it is recommended to enable on untrusted listeners. Note that this
applies only after "reject-underscore-in-header-name" was accepted.
projects / haproxy.git / log