Amaury Denoyelle [Thu, 12 Nov 2026 17:48:14 +0000 (18:48 +0100)]
BUG/MINOR: quic: fix ACK_ECN parsing after optional ack ranges
QUIC acknowledgements are conveyed in ACK and ACK_ECN frame types. The
latter shares a similar structure, but also contains 3 varints at the
end related to explicit congestion notification.
The following patch differentiates ACK and ACK_ECN to ensure the latter
is correctly parsed.
However, ACK_ECN frame parsing is incorrect if the optional ack range
field is present. This patch fixes it by restoring a common parsing
function for ACK and ACK_ECN, which extracts only the first fields prior
to the optional ack ranges. ECN fields are now parsed later, at the end
of qc_parse_ack_frm(), after the optional ack ranges.
REGTESTS: cache: cover the object lifetime with a single script
The fixes about the object lifetime have no dedicated script each: the
reg-test reg-tests/cache/lifetime.vtc covers the whole lifetime
computation:
- the lifetime comes from s-maxage, max-age, Expires, or falls back to
the cache's max-age when the response has a validator (heuristic
lifetime);
- a null lifetime, written as "max-age=0", "max-age=00", a quoted
"max-age=\"0\"" or a null s-maxage, prevents the response from being
served from the cache;
- the Age received from the server reduces a relative lifetime but not
an absolute one given by Expires;
- "Age: 0" is a valid value, it does not prevent the response from being
cached;
- the Age of a copy delivered by the cache stays below the lifetime
announced by the server;
- the Age header of a stored response is still forwarded to the client
the response is delivered to, and the copies carry a single, freshly
computed Age header.
All the checks are made from the response headers, without any delay: the
X-Age-Relative header is set by an http-after-response rule on the
responses delivered from the cache whose Age reaches the max-age
announced by the server, which can only happen when the entry lifetime
was not reduced by the Age. X-Age-Count reports the number of Age headers
to check the forwarded response and the copies.
The script fails when any of the covered fixes is reverted, and it runs
in a few milliseconds.
BUG/MINOR: cache: do not ignore null lifetimes not written as "max-age=0"
A response carrying "Cache-Control: max-age=00", "max-age=\"0\"" or
"s-maxage=00, max-age=60" is delivered from the cache for the whole
max-age of the cache (60 seconds by default) or for the max-age, while
its null lifetime means it must not be reused without revalidation.
Clients thus receive outdated contents. Such forms are unusual but valid
(RFC 9111#1.2.2 and #5.2 allow leading zeroes and quoted values).
http_check_response_for_cacheability() only recognizes the exact forms
"max-age=0" and "s-maxage=0" as null lifetimes. Other forms are taken as
freshness information, and then http_calc_maxage() only considers values
strictly greater than zero, so a null value is ignored and the next
source of lifetime is used: max-age after s-maxage, Expires, or the
cache's max-age.
http_calc_maxage() now uses s-maxage and max-age as soon as they were
found, including when they are null, and the store action does not store
a response whose remaining lifetime is null, since it would already be
expired. This last check also avoids storing responses with an Expires
date in the past, which were stored expired.
This patch may be backported to all supported versions. It depends on
the previous fixes to http_calc_maxage() ("BUG/MINOR: cache: reduce the
max-age lifetime by the response's Age" and "BUG/MINOR: cache: compare
the Age to the heuristic lifetime when none is given"), but only for
context, the change is easy to adapt.
BUG/MINOR: cache: compare the Age to the heuristic lifetime when none is given
A response carrying a validator (Last-Modified or ETag) but no explicit
lifetime (no max-age, s-maxage nor Expires) is normally stored for the
cache's max-age. But as soon as such a response carries an Age header
with a non-null value, which is the case of all responses coming through
an upstream cache, it is never stored, which only affects performance and
the load of the servers.
When the response has no explicit lifetime, http_calc_maxage() returns the
cache's max-age but leaves the true max age untouched, i.e. zero. The
store action then refuses the response because its Age is greater than
this "true max age" of zero, as if it was stale.
When no explicit lifetime is found, the cache's max-age is used as a
heuristic lifetime (RFC 9111#4.2.2), so it is now also reported as the
true max age, and as a lifetime relative to the response's generation,
like max-age, so that the Age is deducted from it by the previous fix. A
response whose Age exceeds the cache's max-age is still refused.
This patch may be backported to all supported versions, along with the
previous one ("BUG/MINOR: cache: reduce the max-age lifetime by the
response's Age") on which it depends.
BUG/MINOR: cache: reduce the max-age lifetime by the response's Age
A response received with "Cache-Control: max-age=3" and "Age: 2" is kept
fresh in the cache for 3 seconds, and delivered with an Age reaching 4 or
5 seconds, while it only had one second of freshness left. In general,
responses coming through upstream caches are kept fresh for as long as
their Age in excess, at most the cache's max-age, which delivers stale
contents to clients, against RFC 9111#4.2. Clients may see outdated
versions of resources for longer than what the origin server specified.
The expiration date of an entry is computed as the reception date plus
the lifetime returned by http_calc_maxage(), capped by the cache's
max-age. The Age header is parsed afterwards, only to refuse responses
whose Age already exceeds their lifetime, and to compute the Age header of
the copies. But a lifetime given by max-age or s-maxage counts from the
generation of the response at the origin, so the time already spent in
upstream caches, given by Age, must be deducted from it. A lifetime given
by Expires is an absolute date and must not be reduced.
http_calc_maxage() now also reports through a new argument whether the
lifetime comes from s-maxage or max-age, and in this case the effective
lifetime is reduced to what remains after deducing the Age.
This patch may be backported to all supported versions.
BUG/MINOR: cache: do not eat the request while waiting for a buffer
The cache applet's I/O handler leaves through the "exit" label when it
cannot allocate its input or output buffer, when one of these buffers is
still pending or when the output buffer is full. But "exit" eats the whole
request with b_reset(&appctx->inbuf), and the request can still be needed on
the next call. For instance, in the HTX_CACHE_HEADER state, the handler
reads the request start line to know whether it is a HEAD request.
The input buffer is empty at this point, and appctx_release_buffers() also
frees it while it has no data, so http_get_stline() may return NULL and the
process crashes. This requires the output buffer allocation to fail exactly
after the request was received, which is rare since the buffers reserve
covers most transient failures, hence a latent bug.
Add a dedicated "wait" label for these paths: it flushes the pending HTX
message to the output buffer and returns without touching the request. The
"exit" label is kept for the states where the request must be consumed:
EOS/error, fast-forward blocked by the consumer, and the end of the
response.
This was introduced when the applet got its own buffers by commit 863417292
("MAJOR: cache: Update HTTP cache applet to handle its own buffers") in
3.0-dev3, so the fix should be backported to 3.0.
A debug patch forcing a single output buffer allocation failure on the
first call in the HTX_CACHE_HEADER state reproduced the crash with ASAN
on the htx_sl_req_meth() line; with this fix, the GET and HEAD requests
all succeed and the process stays alive.
BUG/MINOR: cache: recognize null qvalues with decimals in Accept-Encoding
With "process-vary on", a client refusing an encoding with a null weight
written with decimals, such as "Accept-Encoding: gzip;q=0.0, deflate", may
be delivered a cached gzip-encoded response, that it explicitly refused
and may not be able to decode. Only the client issuing such a request is
affected, it receives an encoding it cannot process.
The accept-encoding normalizer builds a bitmap of the accepted encodings,
from which the ones with a null weight are removed. A null weight was only
detected when the parameter was exactly "q=0", while RFC 9110#12.4.2
defines a qvalue as ( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] ),
so "q=0.", "q=0.0", "q=0.00" and "q=0.000" are null weights as well. They
were considered as non-null, the encoding was accepted and a cached
response with this encoding matched the request's secondary key.
The weight is now considered null when "q=0" is only followed by an
optional dot and zeroes.
The reg-test reg-tests/cache/vary_accept_encoding.vtc is extended: it
stores a gzip response then sends requests refusing gzip with "q=0.0" or
"q=0.000". It fails without the fix since the cached gzip response is
delivered.
This patch may be backported to all supported versions.
BUG/MINOR: cache: do not remove the Age header from the forwarded response
When a response carrying an Age header is stored in the cache, the header
is removed from the response itself, so the client which triggered the
storage receives the object without its Age header, as if it had just
been produced by the origin server, while it may have been sitting in an
upstream cache for a long time. This client may then keep it fresh for
longer than allowed. Worse, when the storage finally fails after this
point (headers too large, unknown content-encoding with Vary, no room in
the cache...), the header is lost although nothing was stored. Copies
delivered from the cache are not affected, they carry a freshly computed
Age header.
The Age header was removed with http_remove_header() from the live
response, just so that the headers copied into the cache entry do not
contain it, since the applet adds its own when delivering the object.
Instead of altering the response, the Age headers are now simply skipped
when the headers are copied into the entry. The ETag offset is computed
from the copied data so it remains correct. This also removes all the Age
headers from the stored copy and not only the first one, so a copy is now
always delivered with a single Age header.
Thi patch fix may be backported to all supported versions. In versions
before 3.5, the removal is done unconditionally (no hints-only mode) but
the fix is the same.
BUG/MINOR: cache: do not refuse to store responses with "Age: 0"
Responses carrying an "Age: 0" header are never stored in the cache. This
header is commonly emitted by an upstream cache or CDN delivering an object
it just fetched, so in a setup chaining caches, the haproxy cache simply
never stores these objects and all requests for them reach the servers,
which only affects performance and the load of the servers.
The Age value is parsed with strl2llrc() and only accepted when strictly
positive, any other case jumping to the "out" label as an invalid value.
Zero is a perfectly valid Age value (RFC 9111#5.1), it is not stale and
needs no special treatment, so it is now accepted like positive values.
Negative or non-numerical values are still considered invalid.
This patch may be backported to all supported versions.
BUG/MEDIUM: filters: order the implicit filters around the explicit ones
In the legacy filter mode, flt_reorder_implicit_conf() reordered the
implicitly declared core filters but pinned the explicitly declared core
filters in their declaration position, treating them as segment
boundaries. An implicit filter bucketed after an explicit core filter
ended up in a wrong position: with an explicit "filter compression"
declaration followed by cache rules, the implicit cache-store filter
was placed after the compression filters, and the configuration checks
reported bogus errors:
proxy 'li': unable to enable the compression filter on response
before any cache filter.
proxy 'li': unable to enable the compression filter before the
cache 'c'.
while this configuration is valid: the implicit cache filter must be
placed at its canonical position, before the explicit compression
filters.
Explicitly declared core filters are now placed in the "any other
filter" class, like any explicitly declared filter: their manual
placement among the explicit filters is the user's responsibility, and
only the implicitly declared core filters are moved to their canonical
position. The placement policy is implemented in flt_order_class(),
which returns the "any other filter" class for an explicit core filter.
The order checks reporting conflicts between the compression,
decompression and cache filters are updated accordingly: an order
error is only reported when the filters involved are explicitly
declared. A filter that is automatically ordered never conflicts with
an explicitly declared one, since the reordering places them correctly
relative to each other.
BUG/MEDIUM: cache: forbid cache rules in defaults sections
A "cache-use" or "cache-store" rule declared in a defaults section has
never worked: the implicit cache-store filter it creates is attached to
the defaults section itself, where the "filter" directive is not
supported. The filter is never resolved, and the rule inherited by the
proxies references a configuration pointing at an unresolved cache name
instead of a cache, which leads to crashes or silently broken caching
depending on the filter mode of the proxies.
Supporting these rules with the legacy filters would require a deep
rework of the rule-to-filter binding, hard to backport. So, until the
new filter API covers this case, the rules are simply forbidden in a
defaults section, with an explicit alert:
[ALERT] : Proxy 'd': 'cache-use' rules are not supported in a defaults section.
This patch must be backported to all supported versions.
BUG/MEDIUM: cache: do not create a second applet when two cache-use rules hit
A configuration with two "http-request cache-use" rules that may both find
the requested object, for example two caches both fed by unconditional
"cache-store" rules, or a cache-use rule in a defaults section that is
repeated in the proxy, leaks memory and cache space on every request that
hits both of them. Each such request leaks one appctx and its task (about
1kB on x86_64) and leaves the row of the first cache's entry retained
forever. The memory grows with the number of requests until the process is
restarted, which affects the system owner, and the rows retained this way
can never be evicted anymore: once their entry expires, their blocks are
lost and the cache progressively shrinks, until it cannot store anything.
Any client may trigger this by simply requesting cached objects. With
2000 requests, "show pools" reports 2016 appctx used and "show cache"
reports a refcount of 2000 on the entry, which stays like this after all
the streams are gone.
The cache-use action is not a final one. On a hit, it sets s->target to
the cache applet and creates the applet on the back stream connector with
sc_applet_create(), after retaining the entry and detaching its row. The
next cache-use rule is still evaluated and, when it also hits, it calls
sc_applet_create() again on the same stream connector. The new appctx
takes the same sedesc, so the first one is not referenced anymore: it was
woken up once, found no request in its input buffer and waits for one
forever. Its release callback is never called, so the entry it retained is
never released and its row never reattached to the avail list.
Let's simply ignore the cache-use rule when the stream already has a
target: the response will be produced by the applet that is already
there. This is consistent with the stats applet that is only set up when
s->target is NULL. An alternative would be to make the action final on a
hit, but this would change the evaluation of the following rules for all
users. Note that "use-service" has the same weakness, it creates its
applet without checking whether one is already attached, so a cache hit
followed by a matching use-service rule could leak the cache applet the
same way. This one is left as is since use-service is a final action and
mixing it with a cache hit is not a sensible configuration.
This patch should be backported to all supported versions. In older versions,
http_action_req_cache_use() may register the applet with a different
function (si_register_handler(), stream_int_register_handler()) but the
fix remains the same: return ACT_RET_CONT at the beginning of the function
when s->target is set.
BUG/MEDIUM: cache: fix "show cache" crash/skip with several caches
With "show cache" cli command, a crash could be experienced when several
caches are configured. This happened when iterating on the second cache in
the list. Indded the cache tree array was not reset when the dump of a cache
was finished, leading to computing the cache tree index against the wrong
cache tree and to lock the wrong cache tree.
This is a regression introduced by the resumable CLI output.
Fix it by clearing ctx->cache_tree when the dump of a cache finishes: the
pointer must only be non-NULL while the dump is paused on the cache it
belongs to.
This patch must be backported to all supported versions.
BUG/MAJOR: cache: do not mix variants varying on different headers
With "process-vary on", a variant of a resource stored for one client may
be delivered to other clients it was not meant for. This happens when the
server does not always vary on the same headers for a given resource,
which is very common: CORS implementations typically add "Vary: origin"
only when the request carries an Origin header. If a first variant varying
only on accept-encoding is stored, then a client sending
"Origin: https://evil.example" gets a response varying on
"accept-encoding, origin" with "Access-Control-Allow-Origin:
https://evil.example", this response is stored with a null hash for the
origin, and is then delivered from the cache to every client whatever its
Origin header. Any client can thus poison the cache for all the other
clients of the resource, breaking the CORS checks of legitimate sites
(availability of the service for these users) or, with Referer, mixing
contents meant for different referrers. The same may happen without any
malicious intent.
When an entry already exists for the primary key, cache-use builds the
request's secondary key only for the headers of the first entry's
signature, the hashes of the other headers being zero-filled. It then
compares this key with each variant. On a miss, the same key is kept in
the transaction and reduced with the response's signature by
cache-store, which expects it to be complete, so the new variant is
stored with zeroes for the headers that were not part of the first
signature. Subsequent lookups build their keys the same way, so the
zeroes match whatever the request's Origin or Referer.
The fix consists in always building the complete secondary key on lookup,
as is already done when there is no entry yet, and in reducing it to the
signature of each variant before comparing it to the variant's key, which
is how the variant's key was built when it was stored. This is done by a
new function, secondary_key_entry_cmp(), used by get_secondary_entry(). A
variant varying on Origin is thus only delivered to requests carrying the
same Origin, and variants varying on different headers can all be found.
On the store side, get_secondary_entry() is passed the key already
reduced to the response's signature, so reducing it again to the
signature of an existing variant does not change the comparison for a
variant with the same signature. Note that a request with a duplicate
Origin or Referer header now never uses the entries of a resource which
has variants, as it already happened when the resource had no entry yet.
The reg-test reg-tests/cache/vary_signature_change.vtc reproduces the
scenario above with an "evil" and a "good" origin and checks that each
variant is delivered to the right requests only. Without the fix, the
request with the good origin receives the evil variant.
This exists since the Vary support was introduced by commit 1785f3dd9
("MEDIUM: cache: Add the Vary header support") in 2.4-dev2, so the fix
must be backported to all supported versions. The severity is set to
MAJOR because this is a cache poisoning that any client can trigger, but
it requires "process-vary on", which is not the default.
BUG/MEDIUM: mux-fcgi: put BE conn back in idle list after deferred shut
When a stream cannot send its FCGI_ABORT record or its end of stream at
detach time because the mux buffer is busy or full, fcgi_detach() leaves
early to keep the stream as an orphan. This skips the idle/avail list
management performed at the end of the function. The orphan is later
destroyed by fcgi_deferred_shut() once the record can be sent, but nothing
makes the connection idle or available again at this point. The timeout is
not refreshed either.
If the orphan was the last stream of the connection, the connection is then
in no idle list, it is not released and it remains accounted in
srv->curr_used_conns. Its timeout is the one armed when the stream was
orphaned, so the connection is closed after "timeout server" instead of
being reused, and it is wasted in the mean time.
To fix the issue, the idle/avail list management is extracted from
fcgi_detach() into a new fcgi_may_idle_conn() function, which is now also
called from fcgi_deferred_shut() after the orphan was destroyed and the
timeout was refreshed, as fcgi_detach() does.
In fcgi_deferred_shut(), the stream's session cannot be used, since it may
have been released in the mean time. The connection owner is used instead,
but only if the connection is still attached to its session, because
session_free() resets conn->owner only in this case. For a private
connection, if there is no session anymore when the last stream leaves, the
connection can no longer be reused and it is destroyed.
The orphans destroyed from fcgi_strm_wake_one_stream() are not covered here.
This mostly happens on connection errors or shutdowns, in which case the
connection is released anyway.
This is the same fix as the one applied on mux-h2 by the commit
"BUG/MEDIUM: mux-h2: put BE conn back in idle list after deferred shut".
This could be backported to all supported versions, after the recent
fcgi_detach() fixes: a167d39a5 ("BUG/MEDIUM: mux-fcgi: arm the connection
timeout only when data remains"), 170a6469c ("BUG/MEDIUM: mux-fcgi: ignore
conn->owner when deciding if a connection is dead"), b77b044d3 ("MINOR:
mux-fcgi: check for a dead connection before refreshing the timeout") and 2acf116bf ("MINOR: mux-fcgi: don't add a closed connection to an idle list
on detach").
MINOR: mux-spop: don't add a closing connection to an idle list on detach
In spop_detach(), the test deciding whether the connection may be moved to
an idle list checks SPOP_CF_RCVD_SHUT, SPOP_CF_ERR_PENDING and
SPOP_CF_ERROR, but not the connection state. A connection that already
switched to SPOP_CS_ERROR, SPOP_CS_CLOSING or SPOP_CS_CLOSED may thus be
inserted in the session server list or in the server's idle pool, while
spop_avail_streams() reports no available stream for it.
This can only happen with a private connection, since srv_add_to_idle_list()
already rejects connections without available streams. It is not harmful
either, since session_get_conn() also requires an available stream, so such
a connection is never reused. But it is still wrong to park it: it prevents
the session from dropping it and forces a useless idle counter increment.
This is the same fix as the one applied on mux-fcgi by commit 2acf116bf
("MINOR: mux-fcgi: don't add a closed connection to an idle list on
detach"). Unlike FCGI, all states from SPOP_CS_ERROR are excluded, which
matches the test performed by spop_avail_streams().
BUG/MEDIUM: mux-spop: refresh the connection timeout before parking it idle
spop_detach() refreshes the connection timeout in two places only: in the
early return taken when the stream is blocked, and at the very end of the
function. The branches that move the connection to an idle list (the
session's private list or the server's idle pool) return before reaching
the tail. Since the timeout is disabled while streams are attached, a
connection detached with data still queued in the mux buffer is thus parked
without any timeout and nothing drains it. It is only released by the purge
of the idle connections.
The refresh is now performed right after spop_strm_destroy(), before the
connection may be given away, and the call at the end of the function is
dropped: everything between the two points only manipulates session and
idle lists, so it recomputed the very same expiration date.
spop_conn_update_timeout() already returns early when there is no task, so
no guard is needed at the call site. Note that an idle connection without
pending data now also gets the default timeout when it is parked, as it is
already the case each time it is processed by spop_process().
The dead connection check is moved ahead of the refresh, so that a
connection about to be released is not armed and requeued just before
spop_release() clears its task context. This only reorders statements:
spop_conn_update_timeout() touches neither the stream tree nor the flags
and states spop_conn_is_dead() looks at. And since the previous commit
removed the conn->owner test from spop_conn_is_dead(), clearing conn->owner
in the non-private branch below has no effect on the predicate.
This is the same fix as the one applied on mux-h2 by commit 5e7bb2063
("BUG/MEDIUM: mux-h2: refresh the connection timeout before parking it
idle") and on mux-fcgi by commits a167d39a5 ("BUG/MEDIUM: mux-fcgi: arm the
connection timeout only when data remains") and b77b044d3 ("MINOR:
mux-fcgi: check for a dead connection before refreshing the timeout").
This should be backported to 3.1 with the previous commit.
MINOR: mux-spop: ignore conn->owner when deciding if a connection is dead
spop_conn_is_dead() reports a connection as dead when it has no stream left
and conn->owner is NULL, on the assumption that an ownerless connection has
nobody left to take care of it. This no longer holds since idle backend
connections are stored in the server and may legitimately have conn->owner
NULL while still being usable. The same test was removed from h2c_is_dead()
by commit a0541f5d2 ("BUG/MEDIUM: mux-h2: ignore conn->owner when deciding
if a connection is dead") and from fcgi_conn_is_dead() by commit 170a6469c
("BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is
dead").
In the SPOP mux, spop_conn_is_dead() is only called at the end of
spop_detach(), which is never reached for an idle connection moved to an
idle list, and the stream test makes the predicate false as long as streams
remain. So the test has no visible effect for now. But it prevents the dead
connection check from being performed before the idle list management, the
non-private branch clearing conn->owner before parking the connection. So
let's remove it.
BUG/MEDIUM: mux-spop: do not enforce the timeout on connections with streams
spop_conn_update_timeout() leaves the connection task unarmed while stream
connectors are attached, since the timeouts are then enforced by the upper
layer. But spop_attach() does not touch the task, so a connection which got
a timeout while it had no stream, for instance because of pending output
data or because of the default timeout applied to connections without
stream, keeps it once a new stream is attached. When it strikes,
spop_timeout_task() kills the connection and aborts the streams attached
to it, while nothing indicates that the agent is not responding.
As done for the H2 mux by commit c2ea47fb1 ("BUG/MEDIUM: mux-h2: do not
enforce timeout on long connections") and for the FCGI mux by commit 6e3bad2f3 ("BUG/MEDIUM: mux-fcgi: do not enforce the timeout on connections
with streams"), spop_timeout_task() now does not kill the connection when
stream connectors are still attached, it only disarms the timeout.
BUG/MEDIUM: mux-h2: put BE conn back in idle list after deferred shut
When a backend stream cannot send its RST_STREAM or ES at detach time
because the mux buffer is busy or full, or because the connection window
is exhausted, h2_detach() leaves early to keep the stream as an orphan.
This skips the idle/avail list management performed at the end of the
function. The orphan is later destroyed by h2_deferred_shut() once the
frame can be sent, but nothing makes the connection idle or available
again at this point.
If the orphan was the last stream of the connection, the connection is
then in no idle list, it is not released and it remains accounted in
srv->curr_used_conns. Its timeout is not armed either, because a backend
connection without stream only relies on idle-ping. So it cannot be
reused and it stays there until the server closes it or an error occurs,
holding an FD and preventing the server from being deleted.
To fix the issue, the idle/avail list management is extracted from
h2_detach() into a new h2_may_idle_conn() function, which is now also
called from h2_deferred_shut() after the orphan was destroyed and the
timeout was refreshed, as h2_detach() does.
In h2_deferred_shut(), the stream's session cannot be used, since it may
have been released in the mean time. The connection owner is used instead,
but only if the connection is still attached to its session, because
session_free() resets conn->owner only in this case. For a private
connection, if there is no session anymore when the last stream leaves,
the connection can no longer be reused and it is destroyed.
The orphans destroyed from h2s_wake_one_stream() are not covered here.
This mostly happens on connection errors or shutdowns, in which case the
connection is released anyway.
This bug exists since orphaned streams are destroyed from
h2_deferred_shut() with backend connections reuse. It could be backported
to all supported versions. Note that 5e7bb2063 ("BUG/MEDIUM: mux-h2:
refresh the connection timeout before parking it idle") reworked the
h2_detach() tail, so it should be backported first. However, it could be
good to wait for a period of observation first.
BUG/MINOR: tcpcheck: fix out-of-bounds reads on short MySQL error packets
tcpcheck_mysql_expect_packet() only verifies that the packet header is
present before reading inside it.
This covers the 3 length bytes and the sequence id, but not the packet type
byte read at offset+4 right after. Then the whole packet is checked against
the buffer, which only guarantees that 4 bytes past offset were received.
When the packet announces a length lower than 4, the error branch below is
entered even though the bytes it reads were never received: it reads the
error code at offset+5 and offset+6 and uses offset+7 as the start of the
message body, while the body length is computed as b_data()-offset-7, which
wraps around when b_data() is lower than offset+7. A 5-byte response, i.e. a
packet announcing plen == 0 followed by a 0xff byte, is enough to trigger
this.
The wrapped length does not lead to a copy today because chunk_istcat()
refuses to append more than b_room(), but the reads still happen and the
error code and status reported are garbage.
Thus, first make sure the packet type byte is available before reading it,
and then reject error packets announcing less than 4 payload bytes, the
minimum for the packet type, a 2-byte error code and at least one byte of
description. As a consequence, such packets are now reported as a Layer7
invalid response instead of a Layer7 wrong status with a bogus error code,
and a response carrying only the 4 header bytes is now considered incomplete
and waits for more data instead of being parsed from bytes that were never
received. Truncated responses keep waiting for more data as before.
This bug is present since 2.2, so it should be backported to all
maintained branches.
Willy Tarreau [Thu, 12 Nov 2026 17:14:04 +0000 (18:14 +0100)]
[RELEASE] Released version 3.5-dev12
Released version 3.5-dev12 with the following main changes :
- CLEANUP: quic: compute the STREAM frame end offset once in qc_stream_frm_is_acked()
- CLEANUP: quic: fix and complete the comments in quic_retransmit.c
- CLEANUP: quic: remove a redundant test in the fast retransmission lookups
- CLEANUP: quic: use QUIC_PACKET_PN_MAXLEN instead of the magic 4
- CLEANUP: quic: remove the dead encryption level variable in qc_prep_hdshk_fast_retrans()
- CLEANUP: quic: drop the intermediate packet tree pointer in the fast retransmissions
- CLEANUP: quic: remove the duplicate qc_prep_hdshk_fast_retrans() prototype
- REGTESTS: protobuf: cover the ungrpc message realignment
- BUG/MINOR: fd: allow fd_write_frag_line() to run before fdtab allocation
- BUG/MINOR: http/hardening: don't count unregistered hdr/tlr too early
- MINOR: http/hardening: turn it to a macro and count matches with COUNT_IF()
- BUG/MEDIUM: stream: reset analyse_exp when analysers are stripped on error
- BUG/MINOR: http-ana: also set FINAL on http-response actions on server error
- BUG/MEDIUM: spoe: stop waiting on a dead producer channel
- BUG/MINOR: tcpcheck: check the number of "hdr" in http-check send
- MINOR: tcpcheck: add post-80-sha2 mode to mysql-check for caching_sha2_password
- BUG/MINOR: stick-table: reject invalid gpc/gpt array indexes
- MEDIUM: h1/violations: restrict valid version strings to "1.0" and "1.1"
- BUG/MEDIUM: filters: avoid NULL block access in decompression
- BUG/MEDIUM: mux-fcgi: do not enforce the timeout on connections with streams
- BUG/MEDIUM: quic: fix loss time selection in quic_loss_pktns()
- BUILD: http: fix build of macro http_harden() with clang
- MINOR: debug: add a COUNT() macro which doesn't take a condition
- CLEANUP: debug/tree-wide: replace COUNT_IF(1, ...) with COUNT()
- BUG/MINOR: applet: accept a temporarily owned tid in appctx_init()
- BUG/MINOR: server: fix a leak when completing a named default-server
- DOC: minor: update VTest2 URL everywhere
- BUG/MEDIUM: compression: use a trash chunk as large as the buffer for the output
- BUILD: debug: fix _COUNT undefinied with USE_OBSOLETE_LINKER
- BUG/MEDIUM: mux-h2: consider the send list as pending data for the timeout
- BUG/MEDIUM: mux-h2: refresh the connection timeout before parking it idle
- BUG/MEDIUM: mux-fcgi: arm the connection timeout only when data remains
- BUG/MEDIUM: mux-fcgi: detect an early connection error during the handshake
- BUG/MEDIUM: mux-fcgi: ignore conn->owner when deciding if a connection is dead
- MINOR: mux-fcgi: check for a dead connection before refreshing the timeout
- MINOR: mux-fcgi: don't add a closed connection to an idle list on detach
- BUG/MEDIUM: mux-h2: also consider the fctl_list as pending data for the timeout
- BUG/MEDIUM: ssl: don't use MSG_MORE for kTLS control records
- MINOR: cli: factorize CLI session reset
- MINOR: cli: store active CLI sessions in a global list
- MINOR: cli: implement basic mechanism for uninterruptible jobs
- MINOR: cli: implement uninterruptible mode for wait
- MINOR: ring: support inactivity timeout in show ring CLI handler
- MINOR: sink: add timeout argument for "show events"
- MINOR: cli: add "kill" CLI command
- REGTESTS: cli: check uninterruptible mode
- CLEANUP: mux-h2: clarify that bptr is not NULL when calling to h2_get_buf()
- BUG/MEDIUM: mux-h2: always return already allocated buffers in h2_get_buf()
- BUG/MEDIUM: mux-h2: permit the buffer callback to allocate despite the queue
- BUG/MEDIUM: mux-h2: wake the connection up after a delayed mbuf allocation
- BUG/MEDIUM: mux-h2: serve all pending buffer allocations in the buffer callback
- BUG/MINOR: mux-h2: unblock the demux when destroying the stream it was blocked on
- BUG/MEDIUM: mux-h2: don't send RST_STREAM for backend streams whose HEADERS were not sent
- BUG/MEDIUM: mux-h2: also emit the HPACK dynamic table size update on requests
- BUG/MINOR: mux-h2: also emit the HPACK Dynamic Table Size Update before trailers
- BUG/MINOR: mux-h2: only convert GET upgrade requests to Extended CONNECT
- BUG/MINOR: mjson: fix out-of-bounds reads on malformed JSON paths
- BUG/MEDIUM: flt-decomp: preserve pending output before decoding again
- REGTEST: ocsp: make the ocsp_client_check test fast
- CI: github: switch development builds back to ubuntu-latest
- CI: github: remove MIPS targets from cross-zoo.yml
- CI: temporarily disable AWS-LC 3.x (fips-2024-09-27)
- CI: github: install libcrypt-dev:i386 for the i686 build
- BUG/MEDIUM: resolvers: detach servers without resolvers from the SRV request
- BUG/MEDIUM: resolvers: don't ignore records not matching a resolve-net family
- BUG/MEDIUM: resolvers: skip the data of ignored additional records
- BUG/MINOR: resolvers: reject a null or negative resolve_retries value
- BUG/MINOR: resolvers: reject null "timeout resolve" and "timeout retry" values
- BUG/MINOR: resolvers: preserve nameserver identity on "clear counters all"
- BUG/MINOR: resolvers: don't dump a nameserver twice in "show resolvers"
- BUG/MINOR: resolvers: Always init a new resolution's list element
- BUG/MINOR: resolvers: count nameserver timeouts
- BUG/MEDIUM: resolvers: reject servers sharing an SRV record with different resolvers
- BUG/MINOR: resolvers: check the SRV expiration date again under the lock
- OPTIM: resolvers: only walk duplicates when looking up an existing record
- BUG/MINOR: resolvers: warn about ignored arguments on datagram nameservers
- BUG/MINOR: resolvers: fix leaks on do-resolve's fetch availability error
- BUG/MINOR: resolvers: report a timeout when a TCP fallback gets no response
- BUG/MINOR: resolvers: use truncated SRV responses when no TCP fallback is possible
- BUG/MEDIUM: resolvers: properly skip root-named authority and additional records
- BUG/MINOR: resolvers: compute a name's wire length by walking its labels
- BUG/MINOR: quic/ssl: record TLS version bounds for certificate selection
- CI: github: pin ARM development builds to Ubuntu 26.04
- BUG/MINOR: cfgparse: detect unsupported crypt() hashes on Debian/Ubuntu
- DOC: configuration: clarify normalize-uri version availability
- BUG/MINOR: ssl/ckch: fix an invalid free() in the domains/ips error paths
- BUG/MINOR: tools: ha_freearray() must also free the array
- CI: github: re-enable AWS-LC 3.x (fips-2024-09-27)
- BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame
- BUG/MINOR: quic: reject RETIRE_CONNECTION_ID for a CID not sent to the peer
- BUG/MINOR: ssl: skip the cipher workaround with AWS-LC API 38 and newer
- BUILD: Makefile: force $(shell command -v ...) to run through the shell
- BUG/MINOR: quic: reject invalid stream ID after MUX closure
- BUG/MEDIUM: quic: limit reject after MUX closure to once per stream
- BUG/MINOR: quic-be: fix Initial token length check on backend connections
- BUG/MINOR: mux_quic: remove dangling qc->qcc ref on MUX init failure
- MINOR: http: add <need_eot> to trailers parsing function
- MINOR: mux_quic: refactor qcs_http_handle_standalone_fin()
- BUG/MINOR: h3: fix missing EOM after trailers without FIN
- BUG/MINOR: h3: close connection on response start-line alloc failure
- BUG/MINOR: h3: fix error code on incomplete request
- BUG/MEDIUM: h3: reject standalone FIN on invalid request/response
- BUG/MEDIUM: threads: don't read lock history entries that don't fit in ulong
- BUG/MINOR: quic-be: drop long header packets with unsupported version
- BUG/MINOR: stick-table: release server_key on any stksess destruction
- BUG/MINOR: acl: check the presence of an argument to "-u"
- BUG/MINOR: server: don't report dyncookie collision on same IP servers
- SCRIPTS: create-release: do not match the word "version" isolated
- MEDIUM: sched: fix the default runqueue-depth
- DOC: security: clarify how to send proper reports
- DOC: threat-model: add a few more entries for stuff that should be public
- DOC: configuration: add a section on secure filtering of HTTP requests
- MINOR: sample: add SMP_USE__MAY_ALIAS to mark fetches returning aliasable contents
- MINOR: http-fetch: mark URI-based sample fetches with SMP_USE__MAY_ALIAS
- MINOR: acl: add acl_cond_find_positive_use() to find positive terms by usage
- MINOR: http-act: mark http-request access control actions with KWF_ACCESS_CTRL
- MINOR: http-rules: accept "no-warn" before the condition of access control rules
- REGTESTS: add a no-warn directive on deny rules using a path
- MEDIUM: http-rules: report access control rules relying on aliasable contents
- MINOR: http: add a global setting to decide on warnings for http rules
- DEBUG: stream: do not emit debug messages for internal proxies
- MINOR: mux-quic: add an sctl() callback to the application ops
- MINOR: mux: add sctl MUX_SCTL_GET_HDNG_FOUND/FIXED to report hardening results
- MINOR: http-fetch: add {fs,bs}.hdng.{found,fixed} to report hardening results
- MINOR: http-conv: add hdng_rej_name to name the HTTP hardening reject rule
- REGTESTS: http-messaging: test the reporting of HTTP hardening results
Willy Tarreau [Tue, 10 Nov 2026 10:55:41 +0000 (11:55 +0100)]
REGTESTS: http-messaging: test the reporting of HTTP hardening results
This new test checks fs.hdng.found, fs.hdng.fixed, bs.hdng.found and
bs.hdng.fixed from a log-format, as well as the hdng_rej_name converter.
The same anomalies (underscore in a header name, unregistered header,
CTL in a trailer) are sent:
- through the "trusted" profile on both sides: everything passes, and
only "found" reports them;
- in a request to a "strict" profile which deletes the two headers and
rejects the trailer: the 400 is logged with the three rules in "fixed"
and hdng_rej_name only names the reject. The trailers are sent apart so
that the stream exists when they are rejected;
- in a response from a server using the "strict" profile: same with a 502
on the backend side;
- in an H2 request with only the deleted headers: nothing is named.
Willy Tarreau [Tue, 10 Nov 2026 10:54:56 +0000 (11:54 +0100)]
MINOR: http-conv: add hdng_rej_name to name the HTTP hardening reject rule
The hdng_rej_name converter takes a mask of HTTP_HRD_* flags such as returned
by fs.hdng.fixed or bs.hdng.fixed, keeps only the "reject" rules, and
returns the name of the lowest one, as found in http_hardening_kws[]. Since
a reject stops the processing, at most one is normally present. Other rules
are ignored, and the converter fails when no reject rule remains, so that
nothing is logged when nothing was rejected:
Willy Tarreau [Tue, 10 Nov 2026 10:41:32 +0000 (11:41 +0100)]
MINOR: http-fetch: add {fs,bs}.hdng.{found,fixed} to report hardening results
These new sample fetches return, for the frontend or backend mux stream,
the mask of HTTP hardening conditions that were met (found) and of the
rules that were enforced (fixed). They rely on the new mux stream controls
MUX_SCTL_GET_HDNG_FOUND and MUX_SCTL_GET_HDNG_FIXED, and fail when the mux
doesn't support them. The main purpose is to log them, e.g.:
Willy Tarreau [Tue, 10 Nov 2026 10:19:20 +0000 (11:19 +0100)]
MINOR: mux: add sctl MUX_SCTL_GET_HDNG_FOUND/FIXED to report hardening results
The H1, H2 and H3 streams store in their http_hdng_chk what HTTP hardening
conditions were met (found) and what rules were enforced (fixed). This adds
two new mux stream controls to retrieve them as uint32_t, so that they can
be reported by sample fetches and logged:
They're implemented in the H1 and H2 muxes. The QUIC mux passes them to
the application layer's sctl() after checking that it's H3, and h3_sctl()
implements them. Other muxes return -1 as they don't support hardening.
Willy Tarreau [Tue, 10 Nov 2026 10:18:01 +0000 (11:18 +0100)]
MINOR: mux-quic: add an sctl() callback to the application ops
The QUIC mux will need to pass some stream controls to the application
layer, which holds information that the mux doesn't know about (e.g. the
HTTP hardening results of the H3 stream). This adds a new sctl() callback
to qcc_app_ops, which takes the qcs, the mux_sctl_type and an output
pointer, and works like mux->sctl(), returning -1 when not supported.
For now, h3_sctl() doesn't implement anything and the mux doesn't call it
yet.
Willy Tarreau [Thu, 12 Nov 2026 13:56:14 +0000 (14:56 +0100)]
DEBUG: stream: do not emit debug messages for internal proxies
The debug mode that is triggered via "haproxy -d" remains helpful in
unit tests during development, typically to see HTTP headers being
processed. However, for historic reasons it doesn't make any difference
between service connections and maintenance connections such as peers
or CLI, making it close to useless in environments involving one of
these.
This patch just eliminates logs for sessions coming from an internal
proxy, since this is not something users are expected to deal with nor
want to observe. Thus, it finally becomes possible to use "haproxy -d"
while a monitoring script polls the CLI or when peers are configured.
Willy Tarreau [Thu, 12 Nov 2026 10:33:23 +0000 (11:33 +0100)]
MINOR: http: add a global setting to decide on warnings for http rules
The default choice of "notice" as level to emit a warning for an
http-request rule relying on unreliable processing may not always be
the best choice. This commit adds a new global "on-unreliable-rules"
directives which takes 3 parameters, "notice" (the default), "warn"
and "ignore". Choosing "warn" will emit a warning instead of a notice,
so that a config using "zero-warning" will detect them. This might be
helpful after a config was sanitized in order to make sure no new
warnings are introduced. And "ignore" may be used to silence all
messages when migrating a config.
Willy Tarreau [Wed, 11 Nov 2026 13:55:02 +0000 (15:55 +0200)]
MEDIUM: http-rules: report access control rules relying on aliasable contents
Rules such as "http-request deny if { path_beg /admin }" are regularly
reported as bypassable using another form of the same request (case,
%-encoding, "//", "/./", "../", backslashes etc). This is expected, since
these aliases depend on what lies behind haproxy, and the only reliable
way to filter requests is to reject what was not explicitly accepted.
Now when an http-request rule using an action marked KWF_ACCESS_CTRL
(auth, deny, reject, silent-drop, tarpit) has a condition which may be
true because of a match on a sample fetch marked SMP_USE__MAY_ALIAS, it
is reported at boot. Only terms making the condition true when they
match are considered (non-negated after "if", negated after "unless"),
so that allow-lists are not reported. Converters are not considered, as
most of them do not handle all aliases.
Some rules are legitimately used as an extra filter (e.g. blocking %2E or
"<script" in the query string as a complement to other protections). For
these ones, the "no-warn" keyword placed right before "if" or "unless"
excludes the rule from the report. It also indicates to future readers
that the rule is known not to be reliable by itself. It is not made a
warning so as not to cause problems to configs running in zero-warning
mode.
Willy Tarreau [Thu, 12 Nov 2026 08:37:18 +0000 (09:37 +0100)]
REGTESTS: add a no-warn directive on deny rules using a path
Historically a number of reg-tests were built with action "deny" and
deny_status followed by a path match in order to distinguish client
requests in the tests. These ones will soon emit warnings, so let's
add a no-warn directive to them.
Willy Tarreau [Wed, 11 Nov 2026 11:05:02 +0000 (13:05 +0200)]
MINOR: http-rules: accept "no-warn" before the condition of access control rules
The http-request actions marked with KWF_ACCESS_CTRL (auth, deny, reject,
silent-drop, tarpit) now accept the "no-warn" keyword placed right before
"if" or "unless". It is removed from the arguments before the action's
parser is called so that the parsers don't have to know about it. It is
only looked up for these actions so that it cannot be confused with an
argument value of another action (e.g. a header value).
It has no effect for now. It will be used to exclude a rule from the
upcoming report of conditions relying on aliasable contents. It comes
separately so that it can be backported to stable versions along with
the previous commit (KWF_ACCESS_CTRL), allowing configurations which
use it to still parse after a rollback, and so that the reg-tests can
be updated before the report is introduced.
Willy Tarreau [Wed, 11 Nov 2026 10:49:20 +0000 (12:49 +0200)]
MINOR: http-act: mark http-request access control actions with KWF_ACCESS_CTRL
This new keyword flag indicates that an action is used to block traffic,
and as such that the reliability of its condition matters. It is set on
the "deny", "tarpit", "reject", "auth" and "silent-drop" http-request
actions. It will be used to detect such rules relying on a deny-list of
aliasable contents. "return" is deliberately not marked, as it's used
for many other purposes (e.g. serving local contents).
Willy Tarreau [Wed, 11 Nov 2026 10:42:38 +0000 (12:42 +0200)]
MINOR: acl: add acl_cond_find_positive_use() to find positive terms by usage
This function looks for a term of a condition which makes the condition
true when it matches (a non-negated term after "if", or a negated one
after "unless") and which involves a sample fetch having certain
SMP_USE_* bits. It returns the first matching ACL and keyword, in the
same way as acl_cond_kw_conflicts() does. The combination of terms is
not considered, a single positive term is enough.
It will be used to detect deny-lists built on aliasable contents.
Willy Tarreau [Wed, 11 Nov 2026 12:02:20 +0000 (14:02 +0200)]
MINOR: http-fetch: mark URI-based sample fetches with SMP_USE__MAY_ALIAS
The path, the query string and the parameters they convey may be written
in many equivalent forms (case, %-encoding, "+" vs "%20", "/./", "../",
"//", backslashes etc), depending on what lies behind haproxy. Let's mark
the fetches returning or depending on these contents with
SMP_USE__MAY_ALIAS: base, base32, base32+src, baseq, capture.req.uri,
path, pathq, query, req.body_param, url, url32, url32+src, url_param,
urlp and urlp_val.
The host part is left aside for now (url_ip, url_port, Host header), as
it's much less commonly used to deny access, and that it's generally
used the same way to route the traffic.
Willy Tarreau [Wed, 11 Nov 2026 11:31:41 +0000 (13:31 +0200)]
MINOR: sample: add SMP_USE__MAY_ALIAS to mark fetches returning aliasable contents
Some sample fetches return contents which may be written in multiple
equivalent forms that are interpreted the same way by the next hop, for
example the path with its %-encoding, its case, or its "/./" and "../"
segments. Matching such contents against a pattern to deny requests is
unreliable, and we'll want to be able to detect such constructs at boot.
This adds SMP_USE__MAY_ALIAS, which is not a source but a property of the
fetched contents. It is placed in the ->use field so that it doesn't
require a new field in all fetch keywords, and since this field is
already or'ed into the ACLs and conditions, it will be possible to
quickly check if a condition involves such a fetch.
The double underscore marks the values which are properties of the
contents and not sources. They're not mapped to any SMP_SRC_* value, so
they're ignored when computing the validity domain. They are allocated
from the topmost bit downwards, leaving all the room to the sources
below, and a build time check verifies that the sources never reach
bit 31.
Willy Tarreau [Wed, 11 Nov 2026 12:36:57 +0000 (12:36 +0000)]
DOC: configuration: add a section on secure filtering of HTTP requests
Deny rules matching a path, an URL or a query string are regularly
reported as bypassable using alternate encodings or aliases (case,
repeated slashes, "." and ".." segments, %-encoding, "%u" encoding,
backslashes, path parameters etc). This is expected: HTTP has many
aliases and which ones apply depends on what is behind haproxy, so no
normalization performed here can be correct in the general case.
The only reliable approach is an allow-list: accept what is explicitly
recognized and reject everything else. Let's document this in a new
section 7.5, starting with a table of aliases of a trivially denied path
to make the problem obvious, explain why it cannot be solved by code,
show how to write allow-list rules with "deny unless" or with a variable
set by each accepting rule and checked once at the end, and list the
patterns worth spotting when reviewing a configuration.
Willy Tarreau [Thu, 12 Nov 2026 16:12:03 +0000 (17:12 +0100)]
DOC: threat-model: add a few more entries for stuff that should be public
Some reports still contain well-known claims that need to be re-explained
and would be better described publicly, particularly because they come
with zero real risk. Let's clarify a few such points.
Willy Tarreau [Thu, 12 Nov 2026 15:37:55 +0000 (16:37 +0100)]
DOC: security: clarify how to send proper reports
Due to the advent of AI, reports are becoming extremely common the the
security list and seem to be competing in a race to bad quality, wasting
everyone's time and nerves. So let's clarify a few points on the expected
format and expected behavior.
Also let's fix 2 lines that were ruined by the create-release script.
Willy Tarreau [Thu, 12 Nov 2026 16:24:32 +0000 (17:24 +0100)]
MEDIUM: sched: fix the default runqueue-depth
The scheduler's run queue depth had been empirically limited to around
280/sqrt(nbhread) when the scheduler was shared between all threads and
the number of threads was very low (4-16), because by then it looked
like a good balance between contention and latency.
Nowadays, each thread has its own scheduler and the number of threads
has exploded, but this calculus remains in place. It causes a noticeable
scheduling overhead on large systems because the number of tasks each
scheduler handles at once can become ridiculously low (e.g. 14 for 256
threads), forcing some users to work around this by forcing the value
in the configuration.
Let's get rid of this outdated calculation and keep the fixed default
value of 280 that was once considered correct and that is still applied
to a single thread, since each scheduler continues to run on a single
thread. Nothing prevents changing the default again later but at least
now the value in defaults.h corresponds to the one we're running on by
default.
Willy Tarreau [Thu, 12 Nov 2026 15:47:56 +0000 (16:47 +0100)]
SCRIPTS: create-release: do not match the word "version" isolated
It happens that some parts of security.txt were ruined, where anything
following the word "version" till the end of a line was replaced with
the current branch. Let's make sure we only match version chars after
it.
Willy Tarreau [Thu, 12 Nov 2026 15:12:11 +0000 (16:12 +0100)]
BUG/MINOR: server: don't report dyncookie collision on same IP servers
When using "cookie dynamic", we detect cookie hash collisions and report
them. However, the test didn't take into account the fact that servers
could have the same addresses (e.g. server templates, DNS updates in
progress etc), and could produce lots of warning that the user couldn't
do anything with.
This patch adds an address check before reporting the warning: if two
servers have the same address, regardless of the cause, it is expected
that they will have the same cookie, so there's no point emitting a
warning.
In addition, there was no info about the servers causing the problem.
The only message was to ask to change the proxy's secret key. Let's
dump the server names so that the user may also correlate what's
happening with reports from other sources, or may even just delete or
readdress one of the offending servers if relevnt.
This can be backported to stable versions. Thanks to Nathan Wehrman
for the report.
Willy Tarreau [Thu, 12 Nov 2026 14:12:06 +0000 (15:12 +0100)]
BUG/MINOR: acl: check the presence of an argument to "-u"
In 1.5-dev23, support for specifying an ACL unique ID was added via
commit 3534d881bc ("MINOR: pattern/acl: Each pattern of each acl can
be load with specified id"). It turns out that if the argument is
missing, we read past the end and can segfault.
This can be backported to all versions. Thanks to Zlatko Bratkovic
for reporting this bug.
BUG/MINOR: stick-table: release server_key on any stksess destruction
A stick-table storing server_key holds a reference on a dictionary entry
for each of its entries. This reference is released by stksess_free()
only, while expiry (process_table_expire()), eviction
(stktable_trash_oldest()) and explicit deletion (__stksess_kill(), also
used by "clear table") destroy the session through __stksess_free()
directly. The dictionary entry is then never freed: it stays in
server_key_dict with a refcount of 1 and no owner, and memory grows
linearly with the number of distinct values that went through the table.
Move the release into __stksess_free() so that every destruction path
releases it, and leave stksess_free() as a simple wrapper.
William Dauchy [Thu, 12 Nov 2026 16:50:46 +0000 (17:50 +0100)]
BUG/MINOR: quic-be: drop long header packets with unsupported version
On backend connections, quic_rx_pkt_parse() accepts long header packets
with an unsupported version and Version Negotiation packets, as these
checks only apply to listeners. Such packets keep the
QUIC_PACKET_TYPE_UNKNOWN type, which maps to no encryption level, and hit
BUG_ON(!qel || !pktns) in qc_enc_level_alloc() from qc_try_rm_hp(), or a
NULL dereference without DEBUG_STRICT.
No malicious peer is required: a server which does not support the
version selected by haproxy responds with a Version Negotiation packet
(RFC 9000 6.1). A malicious server or an on-path attacker may also send a
packet with an unsupported version.
Drop these packets on backend connections, as required by RFC 9000 5.2.1,
since version negotiation is not supported on the backend side. To do
so, export quic_version_VN_reserved and compare the packet version to it
instead of testing a null version number.
This should be backported up to 3.3, where it was introduced by 43d88a44f
("MINOR: quic-be: Datagrams and packet parsing support").
Olivier Houchard [Thu, 12 Nov 2026 11:58:20 +0000 (12:58 +0100)]
BUG/MEDIUM: threads: don't read lock history entries that don't fit in ulong
The per-thread lock history is stored in the "ulong lock_history" field,
with 8 bits per lock operation. On 32-bit systems only 4 entries fit,
but the dump code in ha_thread_dump_one() always read 8 of them,
shifting the value by up to 56, which is undefined behavior with a
32-bit operand. Recent clang versions may generate bad code that will
lead to a crash when using "show threads" on 32bits machines.
Let's keep the field an ulong and only iterate over the entries that
fit in it. On 64-bit nothing changes, and on 32-bit the last 4 entries
are dumped, which is all the field stores anyway.
Amaury Denoyelle [Thu, 12 Nov 2026 14:40:59 +0000 (15:40 +0100)]
BUG/MEDIUM: h3: reject standalone FIN on invalid request/response
HTTP/3 parser has a special code path to handle a standalone FIN.
However for a request stream there was no check to ensure that a valid
request/response was received before reporting the closure.
This would result in an empty HTX buffer with EOM set. On the MUX level,
QCS instance would remain accounted in QCC <nb_hreq> counter, which
could result in unnecessary memory consumption.
Fix this rejecting a request stream closed with a standalone FIN without
a valid request or response received. The stream read is aborted and
write is reset using error code H3_REQUEST_INCOMPLETE or
H3_MESSAGE_ERROR depending on the proxy side. This allows to quickly
release the QCS instance.
Amaury Denoyelle [Thu, 12 Nov 2026 14:36:05 +0000 (15:36 +0100)]
BUG/MINOR: h3: fix error code on incomplete request
HTTP/3 parser validates that a request has been received when FIN is
signalled by the peer on a request stream. FIN signal may be conveyed
with an extension frame. In this case, if no header was received, the
request is invalid and the stream is closed with an error.
This patch simply fixes the reported error code to H3_REQUEST_INCOMPLETE
as recommended by RFC 9114 when the issue occurs on the frontend side.
H3_MESSAGE_ERROR is kept on the backend side as there is no specific
error code defined for an incomplete response.
This must be backported up to 2.8. Ensure enum is adapted for 2.8 as the
name changed after.
Amaury Denoyelle [Thu, 12 Nov 2026 14:31:14 +0000 (15:31 +0100)]
BUG/MINOR: h3: close connection on response start-line alloc failure
On QUIC backend side, correct the error handling when HTX start-line
cannot be allocated during HTTP/3 response parsing.
Previously, a stream error would be reported with H3_MESSAGE_ERROR. Now,
h3_resp_headers_to_htx() only returns a default negative value without
any explicit error code. This results in a connection level error using
H3_INTERNAL_ERROR, generated at the end of h3_rcv_buf().
This pattern is already used on request parsing. It is necessary to
close the connection asap on memory exhaustion while differentiating
between a peer protocol violation and an haproxy internal error.
Amaury Denoyelle [Wed, 28 Oct 2026 14:34:42 +0000 (15:34 +0100)]
BUG/MINOR: h3: fix missing EOM after trailers without FIN
In HTTP/3, it is possible to receive a trailers frame without STREAM FIN
set. It is forbidden for the peer to emit any other HEADERS or DATA
frame, however it is possible to close the stream via an empty STREAM
FIN, or even using HTTP/3 extension frames which must be ignored by a
parser. In this case, EOT will be inserted without EOM flag. If it is
consumed by the stream layer prior to the final EOM reporting, a second
EOT will be inserted, resulting in an invalid message rejected by the
MUX from the other side.
To prevent this situation, disable EOT emission in case EOM cannot be
set. This is implemented via the new argument recently added to
http_trailers_to_htx().
Now that EOT trailers can be voluntarily delayed, it is necessary that
it will be inserted, which htx_set_eom() does not perform if the HTX
message is not empty. This could result with EOM set on the last HTX
headers block instead. Thus, HTTP/3 parser forces EOT emission when
trailers were already parsed, via a new argument on
qcs_http_handle_standalone_fin().
This must be backported up to 2.8. This relies on the previous
refactoring patch. Also note that http_trailers_to_htx() only exist in
master branch, as previously EOT block was added directly by h3 parser
in h3_trailers_to_htx().
Add htx_to_buf() in err label for qcs_http_handle_standalone_fin(). The
objective is to simplify modification of the function body for future
fixes. This will be reused first to prevent a possible double EOT after
HTTP/3 trailers parsing.
Amaury Denoyelle [Wed, 28 Oct 2026 15:16:28 +0000 (16:16 +0100)]
MINOR: http: add <need_eot> to trailers parsing function
Adds an extra argument to http_trailers_to_htx() to specify if EOT block
is necessary. Most of this times this will be set to true by callers.
However, it is necessary to skip it if caller does not want to report
EOM immediately after.
For now, every parsers set this new argument to true. This will change
for HTTP/3 parser as FIN can be received later.
Amaury Denoyelle [Wed, 28 Oct 2026 10:20:11 +0000 (11:20 +0100)]
BUG/MINOR: mux_quic: remove dangling qc->qcc ref on MUX init failure
During QUIC MUX init, the newly allocated QCC reference is stored in the
underlying quic_conn instance. If MUX init fails later, the reference is
not reset despite QCC being freed, and there is a risk that the
transport layer uses the dangling pointer.
Fix this by delaying the transport layer registration until every
fallible operations in MUX init has succeeded.
William Dauchy [Sat, 7 Nov 2026 19:44:30 +0000 (20:44 +0100)]
BUG/MINOR: quic-be: fix Initial token length check on backend connections
The check rejecting server Initial packets with a non-zero token length
tests pkt->token_len before it is set, so it never matches. Test the
parsed <token_len> value instead, as required by RFC 9000 17.2.2.
This should be backported up to 3.3, where it was introduced by 43d88a44f
("MINOR: quic-be: Datagrams and packet parsing support").
Amaury Denoyelle [Thu, 12 Nov 2026 09:34:53 +0000 (10:34 +0100)]
BUG/MEDIUM: quic: limit reject after MUX closure to once per stream
This patch is a follow-up to the previous one. The objective is to limit
the number of allocated frames after MUX closure for STREAM handling.
Ensures that each stream is closed only once. This is performed by
incrementing the ID stored in the quic_conn structure each time a stream
is rejected. If a non-contiguous ID is received, all intermediary
streams are rejected as well.
Along with the previous patch which ensures that stream IDs higher than
the flow control limit cause an immediate closure, this guarantees that
frame allocation for stream rejection after MUX closure remains low and
bounded.
This must be backported up to 2.6 after a period of observation.
Amaury Denoyelle [Thu, 12 Nov 2026 09:34:04 +0000 (10:34 +0100)]
BUG/MINOR: quic: reject invalid stream ID after MUX closure
QUIC transport layer may remain for some time after MUX closure, until
all sent frames have been acknowledged. In the meantime, if the client
opens a new stream, an application layer callback is used to reject it.
With HTTP/3, this produces a STOP_SENDING + RESET_STREAM.
Previously, there was no limit on the queued frames in this case. This
could cause unnecessary memory consumption until the connection is
finally closed.
Address this issue by ensuring the client won't try to open new streams
with invalid ID. First, if the ID corresponds to a server initiated
stream, the connection is closed with error STREAM_STATE.
Another check is performed to ensure that the stream ID is not beyond
the limit advertised last by MUX layer. This requires two new fields for
uni and bidi types in quic_conn, updated during qcc_release() similarly
to the already existing max fields. In case of a violation, the
connection is immediately closed with transport error STREAM_LIMIT.
This must be backported up to 2.6 after a period of observation.
Olivier Houchard [Tue, 10 Nov 2026 15:48:50 +0000 (16:48 +0100)]
BUILD: Makefile: force $(shell command -v ...) to run through the shell
$(shell) commands with no metacharacter and not starting with a shell
builtin may be execvp()'d directly instead of being run through the
shell. "command" is a shell builtin that only appears in make's own
builtin list since version 4.3, so with make <= 4.2 the USE_MINIDEBUG
probe failed with "Command not found" and silently disabled the
embedded symbol table. Adding "2>/dev/null || :" brings metacharacters
which force the shell path on all make versions.
This should fix build on older distributions such as Ubuntu 18.04.
BUG/MINOR: ssl: skip the cipher workaround with AWS-LC API 38 and newer
The crt-list cipher workaround reparses the ciphers and ciphersuites
strings on each connection after SSL_set_SSL_CTX(). AWS-LC restored
context-based cipher inheritance in 5.14.0, so these calls are redundant
with a library providing the fixed context-switching behavior.
Limit the workaround to AWSLC_API_VERSION values greater than 30 and
below 38. Keep the string setters for older API versions and preserve
the API 30 exclusion, where cipher lists already follow the SSL_CTX.
This deliberately retains the workaround for API 37, including 5.14.0,
since that API version also identifies earlier, affected releases.
This follows 38398d2476 ("BUG/MEDIUM: ssl: crt-list ciphers ignored with
AWS-LC") and relates to GitHub issue #3500 and aws/aws-lc#3614.
This should be backported as far as 3.0. The code is in src/ssl_sock.c
before 3.1.
BUG/MINOR: quic: reject RETIRE_CONNECTION_ID for a CID not sent to the peer
A RETIRE_CONNECTION_ID frame for a sequence number below
<next_cid_seq_num> is accepted even if the corresponding
NEW_CONNECTION_ID frame has not been sent yet: the counter counts
allocated CIDs, not sent ones. A client can thus retire a CID it
cannot know about, for example the one allocated while processing a
previous RETIRE_CONNECTION_ID frame of the same packet. This is
forbidden by RFC 9000 19.16.
This adds <max_sent_cid_seq_num>, updated when a packet containing
NEW_CONNECTION_ID frame(s) is sent, and rejects such
RETIRE_CONNECTION_ID frames with a connection error of type
PROTOCOL_VIOLATION, as required by RFC 9000 19.16.
Gihyeon Ryu [Wed, 11 Nov 2026 10:00:11 +0000 (10:00 +0000)]
BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame
A NEW_CONNECTION_ID frame built by HAProxy only stores pointers to the
CID and stateless reset token of its quic_connection_id. When a
RETIRE_CONNECTION_ID frame is received, the CID is freed at once, while
a NEW_CONNECTION_ID frame for it may still be queued, in flight, or be
duplicated later for a retransmission. A client can trigger it by
retiring sequence numbers 1 and 2 in the same packet: the second frame
frees the CID allocated by the first one, whose frame is not sent yet.
The frame is then built from freed memory. In a default build, the pool
cache gives the object back to the next CID of the same connection, so
two frames carry the same CID. Without the pool cache, freed heap memory
is sent to the peer, and ASan reports a heap-use-after-free.
struct qf_new_connection_id now stores the CID and the token instead of
pointers, and quic_connection_id_to_frm_cpy() and the frame duplication
copy them. The parser copies them as well and rejects a CID length above
20 (RFC 9000 section 19.15). quic_frame does not grow, since its union
already holds a larger member.
BUG/MINOR: tools: ha_freearray() must also free the array
ha_freearray() frees each string of a NULL-terminated array and then
sets the array pointer to NULL, but never frees the array itself.
Its only caller is ckch_conf_parse(), which uses it to release the
previous value when a PARSE_TYPE_ARRAY_SUBSTR keyword ("domains", "ips")
is specified twice on the same line, as in:
crt-store s
load crt site.pem domains "a.example,b.example" domains "c.example"
The strings are freed but the array of the first value is leaked
(24 bytes here, reported by LeakSanitizer with "haproxy -c -d" on a
build with -fsanitize=address). The leak only happens when parsing a
line with a duplicated keyword, which also emits an "already specified,
overwriting" warning, so the impact is minimal.
Fix it by freeing the array with ha_free(), which also sets the pointer
to NULL as before. With the patch, the same configuration with either
"domains" or "ips" repeated reports no leak.
Also accept a NULL array argument without dereferencing it. An argument
pointing to a NULL array remains safe, and non-NULL arrays are still fully
freed and their pointer cleared.
Fixes issue #3569.
This was introduced by commit 96b1f1fd2 ("MINOR: tools: ha_freearray()
frees an array of string") together with its use in commit 9c0c05b7b
("BUG/MINOR: ssl/ckch: always ha_freearray() the previous entry during
parsing"), both in 3.2-dev16, so this must be backported to 3.2.
BUG/MINOR: ssl/ckch: fix an invalid free() in the domains/ips error paths
The "domains" and "ips" keywords are stored as NULL-terminated arrays of
strings. When an allocation fails while one of these arrays is built,
both ckch_conf_parse() and ckchs_dup() release it with:
The loop advances r to the terminating NULL entry, so free() is called
on r + n instead of the start of the array. When the failing my_strndup()
or strdup() is not the first one, glibc aborts the process with
"free(): invalid pointer" (ASan reports "attempting free on address
which was not malloc()-ed").
ckch_conf_parse() is reached from the configuration ("crt-store",
"crt-list") and from "add ssl crt-list" on the CLI. ckchs_dup() is
reached at runtime from "set ssl cert" on the CLI, from Lua
CertCache.set() and from the ACME scheduler when a certificate with at
least two domains or IPs is renewed, so the whole process is killed. In
both cases an allocation failure is needed.
Fix it by freeing the entries by index and then the array itself,
leaving r unchanged. ha_freearray() cannot be used here since it does
not free the array.
The issue was reproduced by failing the malloc() of the second domain
with an LD_PRELOAD shim: without the patch both paths abort, with it
"haproxy -c" reports "out of memory" and the CLI answers "Can't update
@s/site.pem!" while the process keeps running.
The ckch_conf_parse() part was introduced by commit 2e8acf54d ("BUG/MINOR:
ssl/ckch: leak in error path") and the ckchs_dup() part by commit 5b85b81d8 ("MINOR: ssl/ckch: handle ckch_conf in ckchs_dup() and
ckch_conf_clean()"), both in 3.2-dev, so this must be backported to 3.2.
DOC: configuration: clarify normalize-uri version availability
The normalize-uri documentation can be read as saying that URI
normalization is only available in HAProxy 2.4. Clarify that the
experimental technical preview has been available since HAProxy 2.4.
BUG/MINOR: cfgparse: detect unsupported crypt() hashes on Debian/Ubuntu
crypt() and crypt_r() may return NULL or error tokens when
encountering unsupported hash types. On Debian/Ubuntu (libxcrypt),
unsupported hashes return strings starting with '*' (e.g. '*0', '*1').
On musl, crypt() does not produce error tokens for unknown hashes, so
detection is not possible there.
Capture the return value of crypt() and check for NULL or a '*'
prefix, ensuring unsupported hash types are detected at config
validation time on platforms that report them.
CI: github: pin ARM development builds to Ubuntu 26.04
The ubuntu-latest-arm runner label does not exist, preventing ARM
development jobs from starting. Use ubuntu-26.04-arm instead, matching
the stable branch builds.
BUG/MINOR: quic/ssl: record TLS version bounds for certificate selection
QUIC restricts its SSL context to TLS 1.3 but leaves the bind's
ssl_methods bounds unset. Without an explicit ssl-max-ver, the client
hello callback therefore fails its TLS 1.3 check and selects RSA instead
of ECDSA when both certificates match the SNI.
Record the effective min/max bounds as ssl_sock_initial_ctx() does for
TCP binds. This fixes issue #3566.
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.
projects / haproxy.git / log