Commit Graph

21004 Commits

Author SHA1 Message Date
Willy Tarreau
89c6b67a82 BUG/MEDIUM: pool: fix releasable pool calculation when overloaded
In 2.6-dev1, the method used to decide how many pool entries could be
released at once was revisited to support releases in batches. This was
done with commits 91a8e28f9 ("MINOR: pool: add a function to estimate
how many may be released at once") and 361e31e3f ("MEDIUM: pool: compute
the number of evictable entries once per pool").

The first commit takes care of the possible inconsistency between the
moment the allocated count and the used count are read, but unfortunately
fixed it the wrong way, by adjusting "used" to match "alloc" whenever it
was lower (i.e. almost always). This results in a nasty case which is that
as soon as the allocated value becomes higher than the estimated count of
needed entries, we end up returning pool->minavail, which causes very
small batches to be released, starting from commit 1513c5479 ("MEDIUM:
pools: release cached objects in batches").

The problem was further amplified in 2.9-dev3 with commit 7bf829ace
("MAJOR: pools: move the shared pool's free_list over multiple buckets")
because it now becomes possible for a thread to allocate from one bucket
and release into a few other different ones, causing an accumulation of
entries in that bucket.

The fix is trivial, simply adjust the alloc counter if the used one is
higher, before performing operations.

This must be backported to 2.6.
2023-11-08 17:12:49 +01:00
Christopher Faulet
5705a6e3b7 BUG/MEDIUM: freq-ctr: Don't report overshoot for long inactivity period
The function returning the excess of events over the current period for a
target frequency (the overshoot) has a flaw if the inactivity period is too
long. In this case, the result may overflow. Instead to be negative, a very
high positive value is returned.

This function is used by the bandwidth limitation filter. It means after a
long inactivity period, a huge burst may be detected while it should not.

In fact, the problem arise from the moment we're past the current period. In
this case, we should not report any overshoot and just get the number of
remaining events as usual.

This patch should be backported as far as 2.7.
2023-11-08 16:38:06 +01:00
Christopher Faulet
2c9c2f9d77 BUG/MINOR: mux-h1: Properly handle http-request and http-keep-alive timeouts
It is now the turn for the H1 mux to be fix to properly handle http-request
and http-keep-alive timeouts. It is quite surprising but it is broken since
the 2.2. For idle connections on client side, the smallest value between the
client timeout and the http-request/http-keep-alive timeout is used while
the client timeout should only be used if other ones are not defined. So, if
the client timeout is the smallest value, the keep-alive timeout is not
respected.

It is only an issue for idle client connections. The http-request timeout is
respected from the moment part of the next request was received.

This patch should fix the issue #2334. It must be backported as far as 2.2. But
be careful during the backports. The H1 mux had evolved a lot since the 2.2.
2023-11-08 16:38:06 +01:00
Aurelien DARRAGON
8dae361f35 MINOR: stktable/cli: support v6tov4 and v4tov6 conversions
Add a special treatment for the IPV4 and IPV6 cases in
table_process_entry_per_key() function so that input string is parsed
in best effort (STR to pseudo type ADDR): input format is first considered
over table type and then let smp_to_stkey() do the type conversion for us
when needed.

This patch heavily depends on:
- "MEDIUM: stktable/cli: simplify entry key handling"

And optionally depends on:
- 72514a44 ("MEDIUM: tools/ip: v4tov6() and v6tov4() rework")
2023-11-08 16:38:06 +01:00
Aurelien DARRAGON
0a47e6bccc MEDIUM: stktable/cli: simplify entry key handling
Make use of smp_to_stkey() in table_process_entry_per_key() to simplify
key handling and leverage auto type conversions from sample API.

One noticeable side effect is that integer input checks will be relaxed
given that c_str2int() sample conv is more permissible than the integrated
table_process_entry_per_key() integer parser.
2023-11-08 16:38:06 +01:00
Aurelien DARRAGON
c6826b9570 BUG/MINOR: stick-table/cli: Check for invalid ipv4 key
When an ipv4 key is used to filter a CLI command on a stick table
clear/set/show table ...), inetaddr_host+htonl combination was used
with no error checking.

Instead, we now use inet_pton(), which is what we use for ipv6 addresses
since b7c962b0c0 ("BUG/MINOR: stick-table/cli: Check for invalid ipv6 key")

Doing this allows us to easily check for parsing errors: we're trading off
some parsing efficience to better catch input errors and ensure we get
similar behavior between ipv4 and ipv6 addresses handling.

This patch may be backported to all supported versions.
2023-11-08 16:38:06 +01:00
Christopher Faulet
ba6ad4654e BUG/MINOR: mux-h1: Release empty ibuf during data fast-forwarding
We must take care to release H1 input buffer when it is emptied during the
fast-forwarding nego. Otherwise, it may be kept allocated for a while,
waiting for the next "normal" receive or the H1C release.

No backport needed.
2023-11-08 16:38:06 +01:00
Amaury Denoyelle
d434acd8bb MINOR: proto_reverse_connect: use connect timeout
Use backend connect timeout when a new connection is instantiated for
rhttp. This ensures that if connect operation fails after a certain
delay, reverse_connect listener task is woken up. This allows to free
the current connection and retry a new connect.

As a consequence of this change, rev_process() may be woken up even if
connection is not reported with CO_FL_ERROR. This happens if timeout
fired before any network reported issue. Connection freeing is adjusted
as in this case MUX instance is already allocated. Use destroy callback
to release MUX context prior to the connection itself.

This patch is really useful as a side measure for a haproxy bug
impacting connect with SSL for both backend connections and active
reverse connect. This is caused by the delayed allocation of MUX
allocation. Asynchronous connect error detected at the socket layer is
not notified to upper layers. Currently, only connect timeout allows to
release this failed connection.
2023-11-08 10:17:43 +01:00
Christopher Faulet
7d7df1cf0a BUG/MEDIUM: mux-h1: Be sure xprt support splicing to use it during fast-forward
The commit d6d4abdc3 ("BUILD: mux-h1: Fix build without kernel splicing
support") introduced a regression. The kernel support for the underlying
XPRT is no longer checked. So it is possible to enable the splicing for SSL
connection. This of course leads to a segfault.

This patch restore the test on the xprt rcv_pipe/snd_pipe functions.

This patch should fix a crash reported by Tristan in #2095
(#issuecomment-1788949014). No backport needed.
2023-11-07 18:23:00 +01:00
Amaury Denoyelle
6f9b65f952 BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure
QUIC connections are accounted inside global sslconns. As with QUIC
actconn, it suffered from a similar issue if an intermediary allocation
failed inside qc_new_conn().

Fix this similarly by moving increment operation inside qc_new_conn().
Increment and error path are now centralized and much easier to
validate.

The consequences are similar to the actconn fix : on memory allocation
global sslconns may wrap, this time blocking any future QUIC or SSL
connections on the process.

This must be backported up to 2.6.
2023-11-07 14:06:02 +01:00
Amaury Denoyelle
a7ba679fe7 BUG/MEDIUM: quic: fix actconn on quic_conn alloc failure
Since the following commit, quic_conn instances are accounted into
global actconn and compared against maxconn.

  commit 7735cf3854
  MEDIUM: quic: count quic_conn instance for maxconn

Increment is always done prior to real allocation to guarantee minimal
resource consumption. Special care is taken to ensure there will always
be one decrement operation for each increment. To help this, decrement
is centralized in quic_conn_release().

This behaves incorrectly in case of an intermediary allocation failure
inside qc_new_conn(). In this case, quic_conn_release() will decrement
actconn. Then, a NULL qc is returned in quic_rx_pkt_retrieve_conn()
which will also decrement the counter on its own error code path.

To properly fix this, actconn incrementation has been moved directly
inside qc_new_conn(). It is thus easier to cover every cases :
* if alloc failure before or on pool_head_quic_conn, actconn is
  decremented manually at the end of qc_new_conn()
* after this step, actconn will be decremented by quic_conn_release()
  either on intermediary alloc failure or on proper connection release

This bug happens on memory allocation failure so it should be rare.
However, its impact is not negligeable as if actconn counter is wrapped
it will block any future connection allocation for both QUIC and TCP.

One small downside of this change is that a CID is now always allocated
before quic_conn even if maxconn will be reached. However, this is
considered as of minor importance compared to a more robust code.

This must be backported up to 2.6.
2023-11-07 13:50:07 +01:00
Christopher Faulet
62812b2a1d DOC: stconn: Improve comments about lra and fsb usage
Recent fixes have shown <lra> and <fsb> uses were not prettu clear. So let's
try to improve documentation about these value. Especially when <lra> is
updated and how to used it.
2023-11-07 10:41:11 +01:00
Christopher Faulet
e5fe2013a9 CLEANUP: htx: Properly indent htx_reserve_max_data() function
Spaces were used instead of tabs to indent htx_reserve_max_data()
function. Let's reindent the whole function.
2023-11-07 10:41:11 +01:00
Christopher Faulet
c57af8ebcd BUG/MINOR: stconn: Sanitize report for read activity
When a EOS or EOI is detected on the endpoint and when the event is reported
at the SC level, a read activity must be reported. It is not really a big
deal because these flags already inhibit any read timeout. But it is
consistent with the <lra> comment. In addition, no read activity is reported
on abort. It is up-down event and it is not an event unblocking the
reads. So there is no reason to report a read activity.

This patch must be backported to 2.8.
2023-11-07 10:41:11 +01:00
Christopher Faulet
d247152ec2 BUG/MEDIUM: Don't apply a max value on room_needed in sc_need_room()
In sc_need_room(), we compute the maximum room that can be requested to
restarted reading to be sure to be able to unblock the SC. At worst when the
buffer is emptied. Here, the buffer reserve is considered but it is an issue.

Counting the reserve can lead to a wicked bug with the H1 multiplexer, when
small amount of data are found at the end of the HTX buffer. In this case,
to not wrap, the H1 mux requests more room. It is an optim to be able to
resync the buffer with the consumer side and to be able to perform zero-copy
transfers. However, if this amount of data is smaller than the reserve and
if the consumer is congested, we fall in a loop because the wrong value is
used to request more room. The H1 mux continues to pretend there is not
enough space in the buffer, while the effective requested value is lower
than the free space in the buffer. While the consumer is congested and does
not consume these data, the is no way to stop the loop.

We can fix the function by removing the buffer reserve from the
computation. But it remains a dangerous decision to apply a max value on
room_needed. It is safer to require the caller must set a correct value. For
now, it is true. But at the end, it is totally unexepected to wait for more
room than an empty buffer can contain.

This patch must be backported to 2.8.
2023-11-07 10:35:38 +01:00
Christopher Faulet
08d7169f42 MINOR: stconn: Don't queue stream task in past in sc_notify()
A task must never be queued in past. However, in sc_notify(), the stream
task, if not woken up, is queued. Thanks to previous fixes, the stream task
expiration date should be correct. But to prevent any issue, a BUG_ON() is
added to be sure it never happens. I guess a good idea could be to remove it
or change it to BUG_ON_HOT() for the final release.
2023-11-07 10:32:25 +01:00
Christopher Faulet
4a2660aa45 BUG/MEDIUM: stconn: Don't report rcv/snd expiration date if SC cannot epxire
When receive or send expiration date of a stream-connector is retrieved, we
now automatically check if it may expire. If not, TICK_ETERNITY is returned.

The expiration dates of the frontend and backend stream-connectors are used
to compute the stream expiration date. This operation is performed at 2
places: at the end of process_stream() and in sc_notify() if the stream is
not woken up.

With this patch, there is no special changes for process_stream() because it
was already handled. It make thing a little simpler. However, it fixes
sc_notify() by avoiding to erroneously compute an expiration date in
past. This highly reduce the stream wakeups when there is contention on the
consumer side.

The bug was introduced with the commit 8073094bf ("NUG/MEDIUM: stconn:
Always update stream's expiration date after I/O"). It was an error to
unconditionnaly set the stream expiration data, without testing blocking
conditions on both SC.

This patch must be backported to 2.8.
2023-11-07 10:30:01 +01:00
Christopher Faulet
141b489291 BUG/MEDIUM: stconn: Report send activity during mux-to-mux fast-forward
When data are directly forwarded from a mux to the opposite one, we must not
forget to report send activity when data are successfully sent or report a
blocked send with data are blocked. It is important because otherwise, if
the transfer is quite long, longer than the client or server timeout, an
error may be triggered because the write timeout is reached.

H1, H2 and PT muxes are concerned. To fix the issue, The done_fastword()
callback now returns the amount of data consummed. This way it is possible
to update/reset the FSB data accordingly.

No backport needed.
2023-11-07 10:30:01 +01:00
Tim Duesterhus
d7eaa0d553 CLEANUP: Re-apply xalloc_size.cocci (3)
This reapplies the xalloc_size.cocci patch across the whole `src/` tree.

see 16cc16dd82
see 63ee0e4c01
see 9fb57e8c17
2023-11-06 20:49:56 +01:00
Willy Tarreau
ff3dcb20f2 [RELEASE] Released version 2.9-dev9
Released version 2.9-dev9 with the following main changes :
    - DOC: internal: filters: fix reference to entities.pdf
    - BUG/MINOR: ssl: load correctly @system-ca when ca-base is define
    - MINOR: lua: Add flags to configure logging behaviour
    - MINOR: lua: change tune.lua.log.stderr default from 'on' to 'auto'
    - BUG/MINOR: backend: fix wrong BUG_ON for avail conn
    - BUG/MAJOR: backend: fix idle conn crash under low FD
    - MINOR: backend: refactor insertion in avail conns tree
    - DEBUG: mux-h2/flags: fix list of h2c flags used by the flags decoder
    - BUG/MEDIUM: server/log: "mode log" after server keyword causes crash
    - MINOR: connection: add conn_pr_mode_to_proto_mode() helper func
    - BUG/MEDIUM: server: "proto" not working for dynamic servers
    - MINOR: server: add helper function to detach server from proxy list
    - DEBUG: add a tainted flag when ha_panic() is called
    - DEBUG: lua: add tainted flags for stuck Lua contexts
    - DEBUG: pools: detect that malloc_trim() is in progress
    - BUG/MINOR: quic: do not consider idle timeout on CLOSING state
    - MINOR: frontend: implement a dedicated actconn increment function
    - BUG/MINOR: ssl: use a thread-safe sslconns increment
    - MEDIUM: quic: count quic_conn instance for maxconn
    - MEDIUM: quic: count quic_conn for global sslconns
    - BUG/MINOR: ssl: suboptimal certificate selection with TLSv1.3 and dual ECDSA/RSA
    - REGTESTS: ssl: update the filters test for TLSv1.3 and sigalgs
    - BUG/MINOR: mux-quic: fix early close if unset client timeout
    - BUG/MEDIUM: ssl: segfault when cipher is NULL
    - BUG/MINOR: tcpcheck: Report hexstring instead of binary one on check failure
    - MEDIUM: systemd: be more verbose about the reload
    - MINOR: sample: Add fetcher for getting all cookie names
    - BUG/MINOR: proto_reverse_connect: support SNI on active connect
    - MINOR: proxy/stktable: add resolve_stick_rule helper function
    - BUG/MINOR: stktable: missing free in parse_stick_table()
    - BUG/MINOR: cfgparse/stktable: fix error message on stktable_init() failure
    - MINOR: stktable: stktable_init() sets err_msg on error
    - MINOR: stktable: check if a type should be used as-is
    - MEDIUM: stktable/peers: "write-to" local table on peer updates
    - CI: github: update wolfSSL to 5.6.4
    - DOC: install: update the wolfSSL required version
    - MINOR: server: Add parser support for set-proxy-v2-tlv-fmt
    - MINOR: connection: Send out generic, user-defined server TLVs
    - BUG/MEDIUM: pattern: don't trim pools under lock in pat_ref_purge_range()
    - MINOR: mux-h2: always use h2_send() in h2_done_ff(), not h2_process()
    - OPTIM: mux-h2: call h2_send() directly from h2_snd_buf()
    - BUG/MINOR: server: remove some incorrect free() calls on null elements
2023-11-04 09:38:16 +01:00
Willy Tarreau
09eacb8b24 BUG/MINOR: server: remove some incorrect free() calls on null elements
In commit 6f4bfed3a ("MINOR: server: Add parser support for
set-proxy-v2-tlv-fmt") a few free() calls were made to an element on
error path when it was detected it was NULL. It doesn't have any
effect, however there was one case of use-after-free at the end of
srv_settings_cpy() that was caught by gcc due to attempting to free
the element after freeing its holder.

No backport is needed.
2023-11-04 08:56:01 +01:00
Willy Tarreau
e16762f8a8 OPTIM: mux-h2: call h2_send() directly from h2_snd_buf()
This allows to eliminate full buffers very quickly and to recycle them
much faster, resulting in higher transfer rates and lower memory usage
at the same time. We just wake the tasklet up if it succeeded so that
h2_process() and friends are called to finalize what needs to.

For regular buffer sizes, the performance level becomes quite close to
the one obtained with the zero-copy mechanism (zero-copy remains much
faster with non-default buffer sizes). The memory savings are huge with
default buffer size: at 64c * 100 streams on a single thread, we used
to forward 4.4 Gbps of traffic using 10400 buffers. After the change,
the performance reaches 5.9 Gbps with only 22-24 buffers, since they
are quickly recycled. That's asaving of 160 MB of RAM.

A concern was an increase in the number of syscalls but this is not
the case, the numbers remained exactly the same before and after.

Some experimentations were made to try to cork data and not send
incomplete buffers, and that always voided these changes. One
explanation might be that keeping a first buffer with only headers
frames is sufficient to prevent a zero-copy of the data coming in
a next snd_buf() call. This still needs to be studied anyway.
2023-11-04 08:34:23 +01:00
Willy Tarreau
0fa5adee3b MINOR: mux-h2: always use h2_send() in h2_done_ff(), not h2_process()
By calling h2_process(), the code would theoretically make it possible
for a synchronous ->wake() call to provoke an indirect call to h2_snd_buf()
while we're in h2_done_ff(), which could be quite bad. The current
conditions do not permit it right now but this could easily break by
accident. Better use h2_send() and wake the task up if needed. Precise
performance tests showed no change.
2023-11-04 08:12:17 +01:00
Willy Tarreau
58185669d8 BUG/MEDIUM: pattern: don't trim pools under lock in pat_ref_purge_range()
There's a subtle issue that results from pat_ref_purge_range() trying
to release memory. Since commit 0d93a8186 ("MINOR: pools: work around
possibly slow malloc_trim() during gc") that was backported to 2.3,
trim_all_pools() now protects itself against concurrent malloc() and
free() by isolating itself. The problem is that pat_ref_purge_range()
must be called under a lock, which is precisely what's done in
cli_io_handler_clear_map(). Thus during a clearing of a map, if
another thread tries to access or update an entry in the same map, it
will wait for the ref->lock to be released, and trim_all_pools() will
wait for all threads to be harmless, thus causing a deadlock. Note
that disabling memory trimming cannot work around the problem here
because it's tested only under isolation.

The solution here consists in moving the call to trim_all_pools() to
the caller, out of the lock.

This must be backported as far as 2.4.
2023-11-04 07:55:37 +01:00
Alexander Stephan
ce7501de79 MINOR: connection: Send out generic, user-defined server TLVs
To follow-up the implementation of the new set-proxy-v2-tlv-fmt
keyword in the server, the connection is updated to use the previously
allocated TLVs. If no value was specified, we send out an empty TLV.
As the feature is fully working with this commit, documentation and a
test for the server and default-server are added as well.
2023-11-04 04:56:59 +01:00
Alexander Stephan
6f4bfed3a2 MINOR: server: Add parser support for set-proxy-v2-tlv-fmt
This commit introduces a generic server-side parsing of type-value pair
arguments and allocation of a TLV list via a new keyword called
set-proxy-v2-tlv-fmt.

This allows to 1) forward any TLV type with the help of fc_pp_tlv,
2) generally, send out any TLV type and value via a log format expression.
To have this fully working the connection will need to be updated in
a follow-up commit to actually respect the new server TLV list.

default-server support has also been implemented.
2023-11-04 04:56:59 +01:00
William Lallemand
2d213b268e DOC: install: update the wolfSSL required version
WolfSSL 5.6.4 was released with a lot of fixes for HAProxy, update the
required version so all supported reg-tests are working.
2023-11-03 19:02:23 +01:00
William Lallemand
20726b43aa CI: github: update wolfSSL to 5.6.4
Update wolfSSL to the 5.6.4 released version.
2023-11-03 18:50:45 +01:00
Aurelien DARRAGON
5158c0ff69 MEDIUM: stktable/peers: "write-to" local table on peer updates
In this patch, we add the possibility to declare on a table definition
("table" in peer section, or "stick-table" in proxy section) that we
want the remote/peer updates on that table to be pushed on a local
haproxy table in addition to the source table.

Consider this example:

  |peers mypeers
  |        peer local 127.0.0.1:3334
  |        peer clust 127.0.0.1:3333
  |        table t1.local type string size 10m store server_id,server_key expire 30s
  |        table t1.clust type string size 10m store server_id,server_key write-to mypeers/t1.local expire 30s

With this setup, we consider haproxy uses t1.local as cache/local table
for read and write operations, and that t1.clust is a remote table
containing datas processed from t1.local and similar tables from other
haproxy peers in a cluster setup. The t1.clust table will be used to
refresh the local/cache one via the "write-to" statement.

What will happen, is that every time haproxy will see entry updates for
the t1.clust table: it will overwrite t1.local table with fresh data and
will update the entry expiration timer. If t1.local entry doesn't exist
yet (key doesn't exist), it will automatically create it. Note that only
types that cannot be used for arithmetic ops will be handled, and this
to prevent processed values from the remote table from interfering with
computations based on values from the local table. (ie: prevent
cumulative counters from growing indefinitely).

"write-to" will only push supported types if they both exist in the source
and the target table. Be careful with server_id and server_key storage
because they are often declared implicitly when referencing a table in
sticking rules but it is required to declare them explicitly for them to
be pushed between a remote and a local table through "write-to" option.

Also note that the "write-to" target table should have the same type as
the source one, and that the key length should be strictly equal,
otherwise haproxy will raise an error due to the tables being
incompatibles. A table that is already being written to cannot be used
as a source table for a "write-to" target.

Thanks to this patch, it will now be possible to use sticking rules in
peer cluster context by using a local table as a local cache which
will be automatically refreshed by one or multiple remote table(s).

This commit depends on:
 - "MINOR: stktable: stktable_init() sets err_msg on error"
 - "MINOR: stktable: check if a type should be used as-is"
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
db0cb54f81 MINOR: stktable: check if a type should be used as-is
stick table types now have an extra bit named 'as_is' that allows us to
check if such type should be used as-is or if it may be involved in
arithmetic operations such as counters. This can be useful since those
types are not common and may require specific handling.

e.g.: stktable_data_types[data_type].as_is will be set to 1 if the type
cannot be used in arithmetic operations.
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
b8c19f877a MINOR: stktable: stktable_init() sets err_msg on error
stktable_init() now sets err_msg when error occurs so that caller is able
to precisely report the cause of the failure.
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
b6a9eca88d BUG/MINOR: cfgparse/stktable: fix error message on stktable_init() failure
As a result of copy paste error in 1b8e68e ("MEDIUM: stick-table: Stop
handling stick-tables as proxies."), postparsing stktable_init() failures
were reported as such for named peer tables:

   "Proxy 'table_name': failed to initialize stick table."

Now they are correctly reported like this:

   "Parsing [file:line]: failed to initialize 'table_name' stick-table."

This should be backported to every stable versions.
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
6376fe9142 BUG/MINOR: stktable: missing free in parse_stick_table()
When "peers" keyword is encountered within a stick table definition,
peers.name hint gets replaced with a new copy of the provided name using
strdup(). However, there is no detection on whether the name was
previously set or not, so it is currently allowed to reuse the keyword
multiple time to overwrite previous value, but here we forgot to free
previous value for peers.name before assigning it to a new one.

This should be backported to every stable versions.
2023-11-03 17:30:30 +01:00
Aurelien DARRAGON
b9c0b039c8 MINOR: proxy/stktable: add resolve_stick_rule helper function
Simplify stick and store sticktable proxy rules postparsing by adding
a sticking rule entry resolve (postparsing) function.

This will ease code maintenance.
2023-11-03 17:30:30 +01:00
Amaury Denoyelle
d82a6d93e2 BUG/MINOR: proto_reverse_connect: support SNI on active connect
SNI may be specify on a server line for connecting to the remote host.
This requires to manually set it on the connection via
ssl_sock_set_servername().

This step was missing when a server line was used for active reverse
HTTP. Fix this by adding the missing ssl_sock_set_servername()
invocation inside new_reverse_conn().

Note that for the moment, no session is instantiated to carry active
reverse connection. A direct consequence of this is that SNI sample
retrieval may crash depending if it depends on session parameters. This
should be fixed by a later commit. In the meantime, this patch is
sufficient to support simple SNI value such as constant expressions.

No need to backport.
2023-11-03 11:11:44 +01:00
Ruei-Bang Chen
7a1ec235cd MINOR: sample: Add fetcher for getting all cookie names
This new fetcher can be used to extract the list of cookie names from
Cookie request header or from Set-Cookie response header depending on
the stream direction. There is an optional argument that can be used
as the delimiter (which is assumed to be the first character of the
argument) between cookie names. The default delimiter is comma (,).

Note that we will treat the Cookie request header as a semi-colon
separated list of cookies and each Set-Cookie response header as
a single cookie and extract the cookie names accordingly.
2023-11-03 09:57:06 +01:00
William Lallemand
e826bc3dfa MEDIUM: systemd: be more verbose about the reload
When the `haproxy -c` check during the reload fails, no error is output
in the logs, this can be quite bothersome to understand what's going on.

This patch removes the -q option on the check so we can see the error
with `journalctl -u haproxy` or `systemctl status haproxy`

This will change the behavior when the check works, and will display
"Configuration file is valid"

Note that in some case this test could be completely removed, because
the master process loads the configuration itself and is able to keep
the previous workers running when the reload failed. This is interesting
to disable the test when there are a lot of certificates of files to
load, to divide the reload time by 2.

No need to backport.
2023-10-31 18:59:29 +01:00
Christopher Faulet
c72ab1cc6d BUG/MINOR: tcpcheck: Report hexstring instead of binary one on check failure
When an expect rule failed for a tcp-check, information about the expect
rule is dumped in the report. For a check on a binary string, a hexstring is
used in the configuration but the decoded string is dumped. It is an problem
because it can contain special characters. And it is not really handy
because there is no correspondance with the config.

So, now, the hexstring is dumped in the report. This way, we are sure there
is no special characters and it is easy to find it in the configuration.

This patch shoudl solve the issue #2326. It must be backported as far as
2.2.
2023-10-31 08:02:44 +01:00
William Lallemand
e7bae7a0b6 BUG/MEDIUM: ssl: segfault when cipher is NULL
The patch which fixes the certificate selection uses
SSL_CIPHER_get_id() to skip the SCSV ciphers without checking if cipher
is NULL. This patch fixes the issue by skipping any NULL cipher in the
iteration.

Problem was reported in #2329.

Need to be backported where 23093c72f1 was
backported. No release was made with this patch so the severity is
MEDIUM.
2023-10-30 18:08:16 +01:00
Amaury Denoyelle
47ed1181f2 BUG/MINOR: mux-quic: fix early close if unset client timeout
When no client timeout is defined in the configuration, QCC timeout task
is never allocated. However, a NULL timeout task is also used as a
criteria in qcc_is_dead() to consider that the MUX instance should be
released as timeout stroke earlier.

This bug causes every connection to be closed by haproxy side with a
CONNECTION_CLOSE. This is notable when using several streams per
connection with only the first stream completed and the others failed.

To fix this, change timeout task allocation policy. It is now always
allocated. This means that if no timeout is defined, it will never be
run. This is not considered a waste of resource as no timeout in the
configuration is considered as an exception case. However, this has the
advantage to simplify the rest of the code which can now check for the
task instance without having an extra check on the timeout value.

This bug is labelled as minor as it only occurs if no timeout client is
defined which reports warning on startup as it may caused unexpected
behavior.

This bug should be backported up to 2.6.
2023-10-27 17:51:08 +02:00
William Lallemand
9496e7e888 REGTESTS: ssl: update the filters test for TLSv1.3 and sigalgs
Signature algorithms allows us to select the right certificates when
using TLSv1.3. This patch update the ssl_crt-list_filters.vtc regtest to
do more precise testing with TLSv1.3 in addition to TLSv1.2.

This allow us to test correctly bug #2300.

It could be backported to 2.8 with the previous fix for certificate
selection.
2023-10-26 19:23:04 +02:00
William Lallemand
23093c72f1 BUG/MINOR: ssl: suboptimal certificate selection with TLSv1.3 and dual ECDSA/RSA
When using TLSv1.3, the signature algorithms extension is used to chose
the right ECDSA or RSA certificate.

However there was an old test for previous version of TLS (< 1.3) which
was testing if the cipher is compatible with ECDSA when an ECDSA
signature algorithm is used. This test was relying on
SSL_CIPHER_get_auth_nid(cipher) == NID_auth_ecdsa to verify if the
cipher is still good.

Problem is, with TLSv1.3, all ciphersuites are compatible with any
authentication algorithm, but SSL_CIPHER_get_auth_nid(cipher) does not
return NID_auth_ecdsa, but NID_auth_any.

Because of this, with TLSv1.3 when both ECDSA and RSA certificates are
available for a domain, the ECDSA one is not chosen in priority.

This patch also introduces a test on the cipher IDs for the signaling
ciphersuites, because they would always return NID_auth_any, and are not
relevent for this selection.

This patch fixes issue #2300.

Must be backported in all stable versions.
2023-10-26 19:17:13 +02:00
Amaury Denoyelle
4a89dba6d5 MEDIUM: quic: count quic_conn for global sslconns
Similar to the previous commit which check for maxconn before allocating
a QUIC connection, this patch checks for maxsslconn at the same step.
This is necessary as a QUIC connection cannot run without a SSL context.

This should be backported up to 2.6. It relies on the following patch :
  "BUG/MINOR: ssl: use a thread-safe sslconns increment"
2023-10-26 15:35:58 +02:00
Amaury Denoyelle
7735cf3854 MEDIUM: quic: count quic_conn instance for maxconn
Increment actconn and check maxconn limit when a quic_conn is
instantiated. This is necessary because prior to this patch, quic_conn
instances where not counted. Global actconn was only incremented after
the handshake has been completed and the connection structure is
allocated.

The increment is done using increment_actconn() on INITIAL packet
parsing if a new connection is about to be created. If the limit is
reached, the allocation is cancelled and the INITIAL packet is dropped.

The decrement is done under quic_conn_release(). This means that
quic_cc_conn instances are not taken into account. This seems safe
enough because quic_cc_conn are only used for minimal usage.

The counterpart of this change is that maxconn must not be checked a
second time when listener_accept() is done over a QUIC connection. For
this, a new bind_conf flag BC_O_XPRT_MAXCONN is set for listeners when
maxconn is already counted by the lower layer. For the moment, it is
positionned only for QUIC listeners.

Without this patch, haproxy process could suffer from heavy memory/CPU
load if the number of concurrent handshake is high.

This patch is not considered a bug fix per-se. However, it has a major
benefit to protect against too many QUIC handshakes. As such, it should
be backported up to 2.6. For this, it relies on the following patch :
  "MINOR: frontend: implement a dedicated actconn increment function"
2023-10-26 15:35:56 +02:00
Amaury Denoyelle
350f8b0c07 BUG/MINOR: ssl: use a thread-safe sslconns increment
Each time a new SSL context is allocated, global.sslconns is
incremented. If global.maxsslconn is reached, the allocation is
cancelled.

This procedure was not entirely thread-safe due to the check and
increment operations conducted at different stage. This could lead to
global.maxsslconn slightly exceeded when several threads allocate SSL
context while sslconns is near the limit.

To fix this, use a CAS operation in a do/while loop. This code is
similar to the actconn/maxconn increment for connection.

A new function increment_sslconn() is defined for this operation. For
the moment, only SSL code is using it. However, it is expected that QUIC
will also use it to count QUIC connections as SSL ones.

This should be backported to all stable releases. Note that prior to the
2.6, sslconns was outside of global struct, so this commit should be
slightly adjusted.
2023-10-26 15:25:07 +02:00
Amaury Denoyelle
fffd435bbd MINOR: frontend: implement a dedicated actconn increment function
When a new frontend connection is instantiated, actconn global counter
is incremented. If global maxconn value is reached, the connection is
cancelled. This ensures that system limit are under control.

Prior to this patch, the atomic check/increment operations were done
directly into listener_accept(). Move them in a dedicated function
increment_actconn() in frontend module. This will be useful when QUIC
connections will be counted in actconn counter.
2023-10-26 15:18:48 +02:00
Amaury Denoyelle
fe29dba872 BUG/MINOR: quic: do not consider idle timeout on CLOSING state
When entering closing state, a QUIC connection is maintained during a
certain delay. The principle is to ensure the other peer has received
the CONNECTION_CLOSE frame. In case of packet duplication/reordering,
CONNECTION_CLOSE is reemitted.

QUIC RFC recommends to use at least 3 times the PTO value. However,
prior to this patch, haproxy used instead the max value between 3 times
the PTO and the connection idle timeout. In the default case, idle
timeout is set to 30s which is in most of the times largely superior to
the PTO. This has the downside of keeping the connection in memory for
too long whereas all resources could be released much earlier.

Fix this behavior by using 3 times the PTO on closing or draining state.
This value is limited up to 1s. This ensures that most of connections
are covered by this. If a connection runs with a very high RTT, it must
not impact the whole process and should be released in a reasonable
delay.

This should be backported up to 2.6.
2023-10-26 15:14:36 +02:00
Willy Tarreau
96bb99a87d DEBUG: pools: detect that malloc_trim() is in progress
Now when calling ha_panic() with a thread still under malloc_trim(),
we'll set a new tainted flag to easily report it, and the output
trace will report that this condition happened and will suggest to
use no-memory-trimming to avoid it in the future.
2023-10-25 15:48:02 +02:00
Willy Tarreau
26a6481f00 DEBUG: lua: add tainted flags for stuck Lua contexts
William suggested that since we can detect the presence of Lua in the
stack, let's combine it with stuck detection to set a new pair of flags
indicating a stuck Lua context and a stuck Lua shared context.

Now, executing an infinite loop in a Lua sample fetch function with
yield disabled crashes with tainted=0xe40 if loaded from a lua-load
statement, or tainted=0x640 from a lua-load-per-thread statement.

In addition, at the end of the panic dump, we can check if Lua was
seen stuck and emit recommendations about lua-load-per-thread and
the choice of dependencies depending on the presence of threads
and/or shared context.
2023-10-25 15:48:02 +02:00
Willy Tarreau
46bbb3a33b DEBUG: add a tainted flag when ha_panic() is called
This will make it easier to know that the panic function was called,
for the occasional case where the dump crashes and/or the stack is
corrupted and not much exploitable. Now at least it will be sufficient
to check the tainted value to know that someone called ha_panic(), and
it will also be usable to condition extra analysis.
2023-10-25 15:48:02 +02:00