Don't insert the acme account key in the ckchs_tree anymore. ckch_store
are not made to only include a private key. CLI operations are not
possible with them either. That doesn't make much sense to keep it that
way until we rework the ckch_store.
Thanks for previous changes, it is now possible to remove the <extra> field
from the HTX structure. HTX_FL_ALTERED_PAYLOAD flag is also removed because
it is now unsued.
When data are sent to the consumer, the known output payload length is
updated using the known input payload length value and this last one is then
reset. se_fwd_kip() function is used for this purpose.
Set <kip> value when data are transfer to the upper layer, in h3_rcv_buf().
The difference between the known length of the payload before and after a
parsing loop is added to <kip> value. When a content-length is specified in
the message, the h3s <body_len> field is used. Otherwise, it is the h3s
<data_len> field.
Set <kip> value when data are transfer to the upper layer, in h2_rcv_buf().
The new <body_len> filed of the H2S is used to increment <kip> value and
then it is reset. The patch relies on the previous one ("MINOR: mux-h2: Save
the known length of the payload").
Before, the <body_len> H2S field was only use for verity the annonced
content-lenght value was respected. Now, this field is used for all
messages. Messages with a content-length are still handled the same way.
<body_len> is set to the content-length value and decremented by the size of
each DATA frame. For other messages, the value is initialized to ULLONG_MAX
and still decremented by the size of each DATA frame. This change is
mandatory to properly define the known input payload length value of the
sedesc.
Set <kip> value during the response parsing. The difference between the body
length before and after a parsing loop is added. The patch relies on the
previous one ("MINOR: h1-htx: Increment body len when parsing a payload with
no xfer length").
Set <kip> value during the message parsing. The difference between the body
length before and after a parsing loop is added. The patch relies on the
previous one ("MINOR: h1-htx: Increment body len when parsing a payload with
no xfer length").
In the H1 parseur, the body length was only incremented when the transfer
length was known. So when the content-length was specified or when the
transfer-encoding value was set to "chunk".
Now for messages with unknown transfer length, it is also incremented. It is
mandatory to be able to remove the extra field from the HTX message.
For now, the HTX extra value is used to specify the known part, in bytes, of
the HTTP payload we will receive. It may concerne the full payload if a
content-length is specified or the current chunk for a chunk-encoded
message. The main purpose of this value is to be used on the opposite side
to be able to announce chunks bigger than a buffer. It can also be used to
check the validity of the payload on the sending path, to properly detect
too big or too short payload.
However, setting this information in the HTX message itself is not really
appropriate because the information is lost when the HTX message is consumed
and the underlying buffer released. So the producer must take care to always
add it in all HTX messages. it is especially an issue when the payload is
altered by a filter.
So to fix this design issue, the information will be moved in the sedesc. It
is a persistent area to save the information. In addition, to avoid the
ambiguity between what the producer say and what the consumer see, the
information will be splitted in two fields. In this patch, the fields are
added:
* kip : The known input payload length
* kop : The known output payload lenght
The producer will be responsible to set <kip> value. The stream will be
responsible to decrement <kip> and increment <kop> accordingly. And the
consumer will be responsible to remove consumed bytes from <kop>.
QC_SF_UNKNOWN_PL_LENGTH flag is set on the qcs to know a payload of message
has an unknown length and not send a RESET_STREAM on shutdown. This flag was
based on the HTX extra field value. However, it is not necessary. When
headers are processed, before sending them, it is possible to check the HTX
start-line to know if the length of the payload is known or not.
So let's do so and don't use anymore the HTX extra field for this purpose.
In the continuity of https://github.com/orgs/haproxy/discussions/3146,
we must also enable abortonclose by default for TLS listeners so as not
to needlessly compute TLS handshakes on dead connections. The change is
very small (just set the default value to 1 in the TLS code when neither
the option nor its opposite were set).
It may possibly cause some TLS handshakes to start failing with 3.3 in
certain legacy environments (e.g. TLS health-checks performed using only
a client hello and closing afterwards), and in this case it is sufficient
to disable the option using "no option abortonclose" in either the
affected frontend or the "defaults" section it derives from.
With this function we can now pass the desired default value for the
abortonclose option when neither the option nor its opposite were set.
Let's also take this opportunity for using it directly from the HTTP
analyser since there's no point in re-checking the proxy's mode there.
As discussed on https://github.com/orgs/haproxy/discussions/3146 and on
the mailing list, there's a marked preference for having abortonclose
enabled by default when relevant. The point being that with todays'
internet, the large majority of requests sent with a closed input
channel are aborted requests, and that it's pointless to waste resources
processing them.
This patch now considers both "option abortonclose" and its opposite
"no option abortonclose" to figure whether abortonclose is enabled or
disabled in a backend. When neither are set (thus not even inherited
from a defaults section), then it considers the proxy's mode, and HTTP
mode implies abortonclose by default.
This may make some legacy services fail starting with 3.3. In this case
it will be sufficient to add "no option abortonclose" in either the
affected backend or the defaults section it derives from. But for
internet-facing proxies it's better to stay with the option enabled.
In order to prepare for changing the way abortonclose works, let's
replace the direct flag check with a similarly named function
(proxy_abrt_close) which returns the on/off status of the directive
for the proxy. For now it simply reflects the flag's state.
By default when building an H2 request, vtest sets the END_STREAM flag
on the HEADERS frame. This is problematic with the websocket and proto
upgrade tests since we're using CONNECT, because it immediately closes
afterwards, which does not correspond to what we're testing. Doing this
in abortonclose mode rightfully produces an error. Let's fix the test
so as not to set the flag on the HEADERS frame. However, doing so means
we'll receive a window update that we must also accept. Now the test
works both with and without abortonclose.
Tests with abortonclose showed a bug with this test where the client
would close the stream immediately after sending the request, without
waiting for the response, causing some random failures on the server
side.
The "abortonclose" option was recently deprecated in frontends because its
action was essentially limited to the backend part (queuing etc). But in
3.3 we started to support it for TLS on frontends, though it would only
work when placed in a defaults section. Let's officially support it in
frontends, and take this opportunity to clarify the documentation on this
topic, which was incomplete regarding frontend and TLS support. Now the
doc tries to better cover the different use cases.
The patchbot stopped on a previous ultra-rare forced push due to wanting
the user's name and e-mail before proceeding. We don't want merges nor
rebases anyway, only to reset the tree to the next one, so let's do that.
'wait-for-body' action set analyse_exp date for the channel to the
configured time. However, when the action is finished, it does not reset
it. It is an issue for some following actions, like 'pause', that also rely
on this date.
To fix the issue, we must take care to reset the analyse_exp date to
TICK_ETERNITY when the 'wait-for-body' action is finished.
This patch should fix the issue #3147. It must be backported to all stable
versions.
Since 9561b9fb6 ("BUG/MINOR: sink: add tempo between 2 connection
attempts for sft servers"), there is a possibility that the tempo we use
to schedule the task expiry may point to TICK_ETERNITY as we add ticks to
tempo with a simple addition that doesn't take care of potential wrapping.
When this happens (although relatively rare, since now_ms only wraps every
49.7 days, but a forced wrap occurs 20 seconds after haproxy is started
so it is more likely to happen there), the process_sink_forward() task
expiry being set to TICK_ETERNITY, it may never be called again, this
is especially true if the ring section only contains a single server.
To fix the issue, we must use tick_add() helper function to set the tempo
value and this way we ensure that the value will never be TICK_ETERNITY.
It must be backported everywhere 9561b9fb6 was backported (up to 2.6
it seems).
In connect_server(), only avoid creating a mux when we're reusing a
connection, if that connection already has one. We can reuse a
connection with no mux, if we made a first attempt at connecting to the
server and it failed before we could create the mux (or during the mux
creation). The connection will then be reused when trying again.
This fixes a bug where a stream could stall if the first connection
attempt failed before the mux creation. It is easy to reproduce by
creating random memory allocation failure with -dmFail.
This was introduced by commit 4aaf0bfbced22d706af08725f977dcce9845d340,
and thus does not need any backport as long as that commit is not
backported.
Released version 3.3-dev9 with the following main changes :
- BUG/MINOR: acl: Fix error message about several '-m' parameters
- MINOR: server: Parse sni and pool-conn-name expressions in a dedicated function
- BUG/MEDIUM: server: Use sni as pool connection name for SSL server only
- BUG/MINOR: server: Update healthcheck when server settings are changed via CLI
- OPTIM: backend: Don't set SNI for non-ssl connections
- OPTIM: proto_rhttp: Don't set SNI for non-ssl connections
- OPTIM: tcpcheck: Don't set SNI and ALPN for non-ssl connections
- BUG/MINOR: tcpcheck: Don't use sni as pool-conn-name for non-SSL connections
- MEDIUM: server/ssl: Base the SNI value to the HTTP host header by default
- MEDIUM: httpcheck/ssl: Base the SNI value on the HTTP host header by default
- OPTIM: tcpcheck: Reorder tcpchek_connect structure fields to fill holes
- REGTESTS: ssl: Add a script to test the automatic SNI selection
- MINOR: quic: add useful trace about padding params values
- BUG/MINOR: quic: too short PADDING frame for too short packets
- BUG/MINOR: cpu_topo: work around a small bug in musl's CPU_ISSET()
- BUG/MEDIUM: ssl: Properly initialize msg_controllen.
- MINOR: quic: SSL session reuse for QUIC
- BUG/MEDIUM: proxy: fix crash with stop_proxy() called during init
- MINOR: stats-file: use explicit unsigned integer bitshift for user slots
- CLEANUP: quic: fix typo in quic_tx trace
- TESTS: quic: add unit-tests for QUIC TX part
- MINOR: quic: restore QUIC_HP_SAMPLE_LEN constant
- REGTESTS: ssl: Fix the script about automatic SNI selection
- BUG/MINOR: pools: Fix the dump of pools info to deal with buffers limitations
- MINOR: pools: Don't dump anymore info about pools when purge is forced
- BUG/MINOR: quic: properly support GSO on backend side
- BUG/MEDIUM: mux-h2: Reset MUX blocking flags when a send error is caught
- BUG/MEDIUM: mux-h2; Don't block reveives in H2_CS_ERROR and H2_CS_ERROR2 states
- BUG/MEDIUM: mux-h2: Restart reading when mbuf ring is no longer full
- BUG/MINOR: mux-h2: Remove H2_CF_DEM_DFULL flags when the demux buffer is reset
- BUG/MEDIUM: mux-h2: Report RST/error to app-layer stream during 0-copy fwding
- BUG/MEDIUM: mux-h2: Reinforce conditions to report an error to app-layer stream
- BUG/MINOR: hq-interop: adjust parsing/encoding on backend side
- OPTIM: check: do not delay MUX for ALPN if SSL not active
- BUG/MEDIUM: checks: fix ALPN inheritance from server
- BUG/MINOR: check: ensure checks are compatible with QUIC servers
- MINOR: check: reject invalid check config on a QUIC server
- MINOR: debug: report the process id in warnings and panics
- DEBUG: stream: count the number of passes in the connect loop
- MINOR: debug: report the number of loops and ctxsw for each thread
- MINOR: debug: report the time since last wakeup and call
- DEBUG: peers: export functions that use locks
- MINOR: stick-table: permit stksess_new() to temporarily allocate more entries
- MEDIUM: stick-tables: relax stktable_trash_oldest() to only purge what is needed
- MEDIUM: stick-tables: give up on lock contention in process_table_expire()
- MEDIUM: stick-tables: don't wait indefinitely in stktable_add_pend_updates()
- MEDIUM: peers: don't even try to process updates under contention
- BUG/MEDIUM: h1: Allow reception if we have early data
- BUG/MEDIUM: ssl: create the mux immediately on early data
- MINOR: ssl: Add a flag to let it known we have an ALPN negociated
- MINOR: ssl: Use the new flag to know when the ALPN has been set.
- MEDIUM: server: Introduce the concept of path parameters
- CLEANUP: backend: clarify the role of the init_mux variable in connect_server()
- CLEANUP: backend: invert the condition to start the mux in connect_server()
- CLEANUP: backend: simplify the complex ifdef related to 0RTT in connect_server()
- CLEANUP: backend: clarify the cases where we want to use early data
- MEDIUM: server: Make use of the stored ALPN stored in the server
- BUILD: ssl: address a recent build warning when QUIC is enabled
- BUG/MINOR: activity: fix reporting of task latency
- MINOR: activity: indicate the number of calls on "show tasks"
- MINOR: tools: don't emit "+0" for symbol names which exactly match known ones
- BUG/MEDIUM: stick-tables: don't loop on non-expirable entries
- DEBUG: stick-tables: export stktable_add_pend_updates() for better reporting
- BUG/MEDIUM: ssl: Fix a crash when using QUIC
- BUG/MEDIUM: ssl: Fix a crash if we failed to create the mux
- MEDIUM: dns: bind the nameserver sockets to the initiating thread
- MEDIUM: resolvers: make the process_resolvers() task single-threaded
- BUG/MINOR: stick-table: make sure never to miss a process_table_expire update
- MEDIUM: stick-table: move process_table_expire() to a single thread
- MEDIUM: peers: move process_peer_sync() to a single thread
- BUG/MAJOR: stream: Force channel analysis on successful synchronous send
- MINOR: quic: get rid of ->target quic_conn struct member
- MINOR: quic-be: make SSL/QUIC objects use their own indexes (ssl_qc_app_data_index)
- MINOR: quic: display build warning for compat layer on recent OpenSSL
- DOC: quic: clarifies limited-quic support
- BUG/MINOR: acme: null pointer dereference upon allocation failure
- BUG/MEDIUM: jws: return size_t in JWS functions
- BUG/MINOR: ssl: Potential NULL deref in trace macro
- BUG/MINOR: ssl: Fix potential NULL deref in trace callback
- BUG/MINOR: ocsp: prototype inconsistency
- MINOR: ocsp: put internal functions as static ones
- MINOR: ssl: set functions as static when no protypes in the .h
- BUILD: ssl: functions defined but not used
- BUG/MEDIUM: resolvers: Properly cache do-resolv resolution
- BUG/MINOR: resolvers: Restore round-robin selection on records in DNS answers
- MINOR: activity: don't report the lat_tot column for show profiling tasks
- MINOR: activity: add a new lkw_avg column to show profiling stats
- MINOR: activity: collect time spent waiting on a lock for each task
- MINOR: thread: add a lock level information in the thread_ctx
- MINOR: activity: add a new lkd_avg column to show profiling stats
- MINOR: activity: collect time spent with a lock held for each task
- MINOR: activity: add a new mem_avg column to show profiling stats
- MINOR: activity: collect CPU time spent on memory allocations for each task
- MINOR: activity/memory: count allocations performed under a lock
- DOC: proxy-protocol: Add TLS group and sig scheme TLVs
- BUG/MEDIUM: resolvers: Test for empty tree when getting a record from DNS answer
- BUG/MEDIUM: resolvers: Make resolution owns its hostname_dn value
- BUG/MEDIUM: resolvers: Accept to create resolution without hostname
- BUG/MEDIUM: resolvers: Wake resolver task up whne unlinking a stream requester
- BUG/MINOR: ocsp: Crash when updating CA during ocsp updates
- Revert "BUG/MINOR: ocsp: Crash when updating CA during ocsp updates"
- BUG/MEDIUM: http_ana: fix potential NULL deref in http_process_req_common()
- MEDIUM: log/proxy: store log-steps selection using a bitmask, not an eb tree
- BUG/MINOR: ocsp: Crash when updating CA during ocsp updates
- BUG/MINOR: resolvers: always normalize FQDN from response
- BUILD: makefile: implement support for running a command in range
- IMPORT: cebtree: import version 0.5.0 to support duplicates
- MEDIUM: migrate the patterns reference to cebs_tree
- MEDIUM: guid: switch guid to more compact cebuis_tree
- MEDIUM: server: switch addr_node to cebis_tree
- MEDIUM: server: switch conf.name to cebis_tree
- MEDIUM: server: switch the host_dn member to cebis_tree
- MEDIUM: proxy: switch conf.name to cebis_tree
- MEDIUM: stktable: index table names using compact trees
- MINOR: proxy: add proxy_get_next_id() to find next free proxy ID
- MINOR: listener: add listener_get_next_id() to find next free listener ID
- MINOR: server: add server_get_next_id() to find next free server ID
- CLEANUP: server: use server_find_by_id() when looking for already used IDs
- MINOR: server: add server_index_id() to index a server by its ID
- MINOR: listener: add listener_index_id() to index a listener by its ID
- MINOR: proxy: add proxy_index_id() to index a proxy by its ID
- MEDIUM: proxy: index proxy ID using compact trees
- MEDIUM: listener: index listener ID using compact trees
- MEDIUM: server: index server ID using compact trees
- CLEANUP: server: slightly reorder fields in the struct to plug holes
- CLEANUP: proxy: slightly reorganize fields to plug some holes
- CLEANUP: backend: factor the connection lookup loop
- CLEANUP: server: use eb64_entry() not ebmb_entry() to convert an eb64
- MINOR: server: pass the server and thread to srv_migrate_conns_to_remove()
- CLEANUP: backend: use a single variable for removed in srv_cleanup_idle_conns()
- MINOR: connection: pass the thread number to conn_delete_from_tree()
- MEDIUM: connection: move idle connection trees to ceb64
- MEDIUM: connection: reintegrate conn_hash_node into connection
- CLEANUP: tools: use the item API for the file names tree
- CLEANUP: vars: use the item API for the variables trees
- BUG/MEDIUM: pattern: fix possible infinite loops on deletion
- CI: scripts: add support for git in openssl builds
- CI: github: add an OpenSSL + ECH job
- CI: scripts: mkdir BUILDSSL_TMPDIR
- Revert "BUG/MEDIUM: pattern: fix possible infinite loops on deletion"
- BUG/MEDIUM: pattern: fix possible infinite loops on deletion (try 2)
- CLEANUP: log: remove deadcode in px_parse_log_steps()
- MINOR: counters: document that tg shared counters are tied to shm-stats-file mapping
- DOC: internals: document the shm-stats-file format/mapping
- IMPORT: ebtree: delete unusable ebpttree.c
- IMPORT: eb32/eb64: reorder the lookup loop for modern CPUs
- IMPORT: eb32/eb64: use a more parallelizable check for lack of common bits
- IMPORT: eb32: drop the now useless node_bit variable
- IMPORT: eb32/eb64: place an unlikely() on the leaf test
- IMPORT: ebmb: optimize the lookup for modern CPUs
- IMPORT: eb32/64: optimize insert for modern CPUs
- IMPORT: ebtree: only use __builtin_prefetch() when supported
- IMPORT: ebst: use prefetching in lookup() and insert()
- IMPORT: ebtree: Fix UB from clz(0)
- IMPORT: ebtree: add a definition of offsetof()
- IMPORT: ebtree: replace hand-rolled offsetof to avoid UB
- MINOR: listener: add the "cc" bind keyword to set the TCP congestion controller
- MINOR: server: add the "cc" keyword to set the TCP congestion controller
- BUG/MEDIUM: ring: invert the length check to avoid an int overflow
- MINOR: trace: don't call strlen() on the thread-id numeric encoding
- MINOR: trace: don't call strlen() on the function's name
- OPTIM: sink: reduce contention on sink_announce_dropped()
- OPTIM: sink: don't waste time calling sink_announce_dropped() if busy
- CLEANUP: ring: rearrange the wait loop in ring_write()
- OPTIM: ring: always relax in the ring lock and leader wait loop
- OPTIM: ring: check the queue's owner using a CAS on x86
- OPTIM: ring: avoid reloading the tail_ofs value before the CAS in ring_write()
- BUG/MEDIUM: sink: fix unexpected double postinit of sink backend
- MEDIUM: stats: consider that shared stats pointers may be NULL
- BUG/MEDIUM: http-client: Fix the test on the response start-line
- MINOR: acme: acme-vars allow to pass data to the dpapi sink
- MINOR: acme: check acme-vars allocation during escaping
- BUG/MINOR: acme/cli: wrong description for "acme challenge_ready"
- CI: move VTest preparation & friends to dedicated composite action
- BUG/MEDIUM: stick-tables: Don't let table_process_entry() handle refcnt
- BUG/MINOR: compression: Test payload size only if content-length is specified
- BUG/MINOR: pattern: Properly flag virtual maps as using samples
- BUG/MINOR: acme: possible overflow on scheduling computation
- BUG/MINOR: acme: possible overflow in acme_will_expire()
- CLEANUP: acme: acme_will_expire() uses acme_schedule_date()
- BUG/MINOR: pattern: Fix pattern lookup for map with opt@ prefix
- CI: scripts: build curl with ECH support
- CI: github: add curl+ech build into openssl-ech job
- BUG/MEDIUM: ssl: ca-file directory mode must read every certificates of a file
- MINOR: acme: provider-name for dpapi sink
- BUILD: acme: fix false positive null pointer dereference
- MINOR: backend: srv_queue helper
- MINOR: backend: srv_is_up converter
- BUILD: halog: misleading indentation in halog.c
- CI: github: build halog on the vtest job
- BUG/MINOR: acme: don't unlink from acme_ctx_destroy()
- BUG/MEDIUM: acme: cfg_postsection_acme() don't init correctly acme sections
- MINOR: acme: implement "reuse-key" option
- ADMIN: haproxy-dump-certs: implement a certificate dumper
- ADMIN: dump-certs: don't update the file if it's up to date
- ADMIN: dump-certs: create files in a tmpdir
- ADMIN: dump-certs: fix lack of / in -p
- ADMIN: dump-certs: use same error format as haproxy
- ADMIN: reload: add a synchronous reload helper
- BUG/MEDIUM: acme: free() of i2d_X509_REQ() with AWS-LC
- ADMIN: reload: introduce verbose and silent mode
- ADMIN: reload: introduce -vv mode
- MINOR: mt_list: Implement MT_LIST_POP_LOCKED()
- BUG/MEDIUM: stick-tables: Make sure not to free a pending entry
- MINOR: sched: let's permit to share the local ctx between threads
- MINOR: sched: pass the thread number to is_sched_alive()
- BUG/MEDIUM: wdt: improve stuck task detection accuracy
- MINOR: ssl: add the ssl_bc_sni sample fetch function to retrieve backend SNI
- MINOR: rawsock: introduce CO_RFL_TRY_HARDER to detect closures on complete reads
- MEDIUM: ssl: don't always process pending handshakes on closed connections
- MEDIUM: servers: Schedule the server requeue target on creation
- MEDIUM: fwlc: Make it so fwlc_srv_reposition works with unqueued srv
- BUG/MEDIUM: fwlc: Handle memory allocation failures.
- DOC: config: clarify some known limitations of the json_query() converter
- BUG/CRITICAL: mjson: fix possible DoS when parsing numbers
- BUG/MINOR: h2: forbid 'Z' as well in header field names checks
- BUG/MINOR: h3: forbid 'Z' as well in header field names checks
- BUG/MEDIUM: resolvers: break an infinite loop in resolv_get_ip_from_response()
The fix in 3023e98199 ("BUG/MINOR: resolvers: Restore round-robin
selection on records in DNS answers") still contained an issue not
addressed f6dfbbe870 ("BUG/MEDIUM: resolvers: Test for empty tree
when getting a record from DNS answer"). Indeed, if the next element
is the same as the first one, then we can end up with an endless loop
because the test at the end compares the next pointer (possibly null)
with the end one (first).
Let's move the null->first transition at the end. This must be
backported where the patches above were backported (3.2 for now).
The current tests in _h3_handle_hdr() and h3_trailers_to_htx() check
for an interval between 'A' and 'Z' for letters in header field names
that should be forbidden, but mistakenly leave the 'Z' out of the
forbidden range, resulting in it being implicitly valid.
This has no real consequences but should be fixed for the sake of
protocol validity checking.
This must be backported to all relevant versions.
The current tests in h2_make_htx_request(), h2_make_htx_response()
and h2_make_htx_trailers() check for an interval between 'A' and 'Z'
for letters in header field names that should be forbidden, but
mistakenly leave the 'Z' out of the forbidden range, resulting in it
being implicitly valid.
This has no real consequences but should be fixed for the sake of
protocol validity checking.
This must be backported to all relevant versions.
Mjson comes with its own strtod() implementation for portability
reasons and probably also because many generic strtod() versions as
provided by operating systems do not focus on resource preservation
and may call malloc(), which is not welcome in a parser.
The strtod() implementation used here apparently originally comes from
https://gist.github.com/mattn/1890186 and seems to have purposely
omitted a few parts that were considered as not needed in this context
(e.g. skipping white spaces, or setting errno). But when subject to the
relevant test cases of the designated file above, the current function
provides the same results.
The aforementioned implementation uses pow() to calculate exponents,
but mjson authors visibly preferred not to introduce a libm dependency
and replaced it with an iterative loop in O(exp) time. The problem is
that the exponent is not bounded and that this loop can take a huge
amount of time. There's even an issue already opened on mjson about
this: https://github.com/cesanta/mjson/issues/59. In the case of
haproxy, fortunately, the watchdog will quickly stop a runaway process
but this remains a possible denial of service.
A first approach would consist in reintroducing pow() like in the
original implementation, but if haproxy is built without Lua nor
51Degrees, -lm is not used so this will not work everywhere.
Anyway here we're dealing with integer exponents, so an easy alternate
approach consists in simply using shifts and squares, to compute the
exponent in O(log(exp)) time. Not only it doesn't introduce any new
dependency, but it turns out to be even faster than the generic pow()
(85k req/s per core vs 83.5k on the same machine).
This must be backported as far as 2.4, where mjson was introduced.
Many thanks to Oula Kivalo for reporting this issue.
CVE-2025-11230 was assigned to this issue.
Oula Kivalo reported that different JSON libraries may process duplicate
keys differently and that most JSON libraries usually decode the stream
before extracting keys, while the current mjson implementation decodes the
contents during extraction instead. Let's document this point so that
users are aware of the limitations and do not rely on the current behavior
and do not use it for what it's not made for (e.g. content sanitization).
This is also the case for jwt_header_query(), jwt_payload_query() and
jwt_verify(), which already refer to this converter for specificities.
Properly handle memory allocation failures, by checking the return value
for pool_alloc(), and if it fails, make sure that the caller will take
it into account.
The only use of pool_alloc() in fwlc is to allocate the tree elements in
order to properly queue the server into the ebtree, so if that
allocation fails, just schedule the requeue tasklet, that will try
again, until it hopefully eventually succeeds.
This should be backported to 3.2.
This should fix github issue #3143.
Modify fwlc_srv_reposition() so that it does not assume that the server
was already queued, and so make it so it works even if s->tree_elt is
NULL.
While the server will usually be queued, there is an unlikely
possibility that when the server attempted to get queued when it got up,
it failed due to a memory allocation failure, and it just expect the
server_requeue tasklet to run to take care of that later.
This should be backported to 3.2.
This is part of an attempt to fix github issue #3143
On creation, schedule the server requeue once it's been created.
It is possible that when the server went up, it tried to queue itself
into the lb specific code, failed to do so, and expect the tasklet to
run to take care of that.
This should be backported to 3.2.
This is part of an attempt to fix github issue #3143.
If a client aborts a pending SSL connection for whatever reason (timeout
etc) and the listen queue is large, it may inflict a severe load to a
frontend which will spend the CPU creating new sessions then killing the
connection. This is similar to HTTP requests aborted just after being
sent, except that asymmetric crypto is way more expensive.
Unfortunately "option abortonclose" has no effect on this, because it
only applies at a higher level.
This patch ensures that handshakes being received on a frontend having
"option abortonclose" set will be checked for a pending close, and if
this is the case, then the connection will be aborted before the heavy
calculations. The principle is to use recv(MSG_PEEK) to detect the end,
and to destroy the pending handshake data before returning to the SSL
library so that it cannot start computing, notices the error and stops.
We don't do it without abortonclose though, because this can be used for
health checks from other haproxy nodes or even other components which
just want to see a handshake succeed.
This is in relation with GH issue #3124.
Normally, when reading a full buffer, or exactly the requested size, it
is not really possible to know if the peer had closed immediately after,
and usually we don't care. There's a problematic case, though, which is
with SSL: the SSL layer reads in small chunks of a few bytes, and can
consume a client_hello this way, then start computation without knowing
yet that the client has aborted. In order to permit knowing more, we now
introduce a new read flag, CO_RFL_TRY_HARDER, which says that if we've
read up to the permitted limit and the flag is set, then we attempt one
extra byte using MSG_PEEK to detect whether the connection was closed
immediately after that content or not. The first use case will obviously
be related to SSL and client_hello, but it might possibly also make sense
on HTTP responses to detect a pending FIN at the end of a response (e.g.
if a close was already advertised).
Sometimes in order to debug certain difficult situations it can be useful
to know what SNI was configured on a connection going to a server, for
example to match it against what the server saw or to detect cases where
a server would route on SNI instead of Host. This sample fetch function
simply retrieves the SNI configured on the backend connection, if any.
The fact that the watchdog timer measures the execution time from the
last return from the poller tends to amplify the impact of multiple
bad tasks, and may explain some of the panics reported by Felipe and
Ricardo in GH issues #3084, #3092 and #3101. The problem is that we
check the time if we see that the scheduler appears not to be moving
anymore, but one situation may still arise and catch a bad task:
- one slow task takes so long a time that it triggers the watchdog
twice, emitting a warning the second time (~200ms). The scheduler
is rightfully marked as stuck.
- then it completes and the scheduler is no longer stuck. Many other
tasks run in turn, they all take quite some time but not enough to
trigger a warning. But collectively their cost adds up.
- then a task takes more than the warning time (100ms), and causes
the total execution time to cross the second. The watchdog is
called, sees that we've spend more than 1 second since we left the
poller, and marks the thread as stuck.
- the task is not finished, the watchdog is called again, sees more
than one second with a stuck thread and panics 100ms later.
The total time away from the poller is indeed more than one second,
which is very bad, but no single task caused this individually, and
while the warnings are OK, the watchdog should not panic in this case.
This patch revisits the approach to store the moment the scheduler was
marked as stuck in the wdt context. The idea is that this date will be
used to detect warnings and panics. And by doing so and exploiting the
new is_sched_alive(thr), we can greatly simplify the mechanism so that
the signal handling thread does the strict minimum (mark the scheduler
as possibly stuck and update the stuck_start date), and only bounces to
the reporting thread if the scheduler made no progress since last call.
This means that without even doing computations in the handing thread,
we can continue to avoid all bounces unless a warning is required. Then
when the reporting thread is signaled, it will check the dates from the
last moment the scheduler was marked, and will decide to warn or panic.
The panic decision continues to pass via a TH_FL_STUCK flag to probe the
code so that exceptionally slow code (e.g. live cert generation etc) can
still find a way to avoid the panic if absolutely certain that things
are still moving.
This means that now we have the guarantee that panics will only happen
if a given task spends more than one full second not moving, and that
warnings will be issued for other calls crossing the warn delay boundary.
This was tested using artificially slow operations, and all combinations
which individually took less than a second only resulted in floods of
warnings even if the total reported time in the warning was much higher,
while those above one second provoked the panic.
One improvement could consist in reporting the time since last stuck
in the thread dumps to differentiate the individual task from the whole
set.
This needs to be backported to 3.2 along with the two previous patches:
MINOR: sched: let's permit to share the local ctx between threads
MINOR: sched: pass the thread number to is_sched_alive()
Now it will be possible to query any thread's scheduler state, not
only the current one. This aims at simplifying the watchdog checks
for reported threads. The operation is now a simple atomic xchg.
The watchdog timer has to go through complex operations due to not being
able to check if another thread's scheduler is still ticking. This is
simply because the scheduler status is marked as thread-local while it
could in fact also be an array. Let's do that (and align the array to
avoid false sharing) so that it's now possible to check any scheduler's
status.
There is a race condition, an entry can be free'd by stksess_kill()
between the time stktable_add_pend_updates() gets the entry from the
mt_list, and the time it adds it to the ebtree.
To prevent this, use the newly implemented MT_LIST_POP_LOCKED() to keep
the stksess locked until it is added to the tree. That way,
__stksess_kill() will wait until we're done with it.
This should be backported to 3.2.
