]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
11 months agoBUG/MEDIUM: server/addr: fix tune.events.max-events-at-once event miss and leak
Aurelien DARRAGON [Tue, 6 Aug 2024 12:29:56 +0000 (14:29 +0200)] 
BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once event miss and leak

An issue has been introduced with cd99440 ("BUG/MAJOR: server/addr: fix
a race during server addr:svc_port updates").

Indeed, in the above commit we implemented the atomic_sync task which is
responsible for consuming pending server events to apply the changes
atomically. For now only server's addr updates are concerned.

To prevent the task from causing contention, a budget was assigned to it.
It can be controlled with the global tunable
'tune.events.max-events-at-once': the task may not process more than this
number of events at once.

However, a bug was introduced with this budget logic: each time the task
has to be interrupted because it runs out of budget, we reschedule the
task to finish where it left off, but the current event which was already
removed from the queue wasn't processed yet. This means that this pending
event (each tune.events.max-events-at-once) is effectively lost.

When the atomic_sync task deals with large number of concurrent events,
this bug has 2 known consequences: first a server's addr/port update
will be lost every 'tune.events.max-events-at-once'. This can of course
cause reliability issues because if the event is not republished
periodically, the server could stay in a stale state for indefinite amount
of time. This is the case when the DNS server flaps for instance: some
servers may not come back UP after the incident as described in GH #2666.

Another issue is that the lost event was not cleaned up, resulting in a
small memory leak. So in the end, it means that the bug is likely to
cause more and more degradation over time until haproxy is restarted.

As a workaround, 'tune.events.max-events-at-once' may be set to the
maximum number of events expected per batch. Note however that this value
cannot exceed 10 000, otherwise it could cause the watchdog to trigger due
to the task being busy for too long and preventing other threads from
making any progress. Setting higher values may not be optimal for common
workloads so it should only be used to mitigate the bug while waiting for
this fix.

Since tune.events.max-events-at-once defaults to 100, this bug only
affects configs that involve more than 100 servers whose addr:port
properties are likely to be updated at the same time (batched updates
from cli, lua, dns..)

To fix the bug, we move the budget check after the current event is fully
handled. For that we went from a basic 'while' to 'do..while' loop as we
assume from the config that 'tune.events.max-events-at-once' cannot be 0.
While at it, we reschedule the task once thread isolation ends (it was not
required to perform the reschedule while under isolation) to give the hand
back faster to waiting threads.

This patch should be backported up to 2.9 with cd99440. It should fix
GH #2666.

11 months agoBUG/MINOR: fcgi-app: handle a possible strdup() failure
Ilia Shipitsin [Mon, 5 Aug 2024 18:59:09 +0000 (20:59 +0200)] 
BUG/MINOR: fcgi-app: handle a possible strdup() failure

This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to 2.2.

11 months agoDEV: coccinelle: add a test to detect unchecked strdup()
Ilia Shipitsin [Mon, 5 Aug 2024 18:59:09 +0000 (20:59 +0200)] 
DEV: coccinelle: add a test to detect unchecked strdup()

The coccinelle test "unchecked-strdup.cocci" detects various cases of
unchecked strdup().

11 months agoBUG/MINOR: quic: Too short datagram during packet building failures (aws-lc only)
Frederic Lecaille [Mon, 5 Aug 2024 11:40:51 +0000 (13:40 +0200)] 
BUG/MINOR: quic: Too short datagram during packet building failures (aws-lc only)

This issue was reported by Ilya (@Chipitsine) when building haproxy against
aws-lc in GH #2663 where handshakeloss and handshakecorruption interop tests could
lead haproxy to crash after having built too short datagrams:

FATAL: bug condition "first_pkt->type == QUIC_PACKET_TYPE_INITIAL && (first_pkt->flags & (1UL << 0)) && length < 1200" matched at src/quic_tx.c:163
call trace(13):
| 0x55f4ee4dcc02 [ba d9 00 00 00 48 8d 35]: main-0x195bf2
| 0x55f4ee4e3112 [83 3d 2f 16 35 00 00 0f]: qc_send+0x11f3/0x1b5d
| 0x55f4ee4e9ab4 [85 c0 0f 85 00 f6 ff ff]: quic_conn_io_cb+0xab1/0xf1c
| 0x55f4ee6efa82 [48 c7 c0 f8 55 ff ff 64]: run_tasks_from_lists+0x173/0x9c2
| 0x55f4ee6f05d3 [8b 7d a0 29 c7 85 ff 0f]: process_runnable_tasks+0x302/0x6e6
| 0x55f4ee671bb7 [83 3d 86 72 44 00 01 0f]: run_poll_loop+0x6e/0x57b
| 0x55f4ee672367 [48 8b 1d 22 d4 1d 00 48]: main-0x48d
| 0x55f4ee6755e0 [b8 00 00 00 00 e8 08 61]: main+0x2dec/0x335d

This could happen after Handshake packet building failures which follow a successful
Initial packet into the same datagram. In this case, the datagram could be emitted
with a too short length (<1200 bytes).

To fix this, store the datagram only if the first packet is not an Initial packet
or if its length is big enough (>=1200 bytes).

Must be backported as far as 2.6.

11 months agoCI: harden NetBSD builds by ERR=1
Ilia Shipitsin [Sun, 21 Jul 2024 11:52:08 +0000 (13:52 +0200)] 
CI: harden NetBSD builds by ERR=1

Add ERR=1 build option to the NetBSD build from github.

11 months agoCI: add weekly QUIC Interop regression against AWS-LC
Ilia Shipitsin [Sat, 3 Aug 2024 13:43:36 +0000 (15:43 +0200)] 
CI: add weekly QUIC Interop regression against AWS-LC

currently only quic-go and picoquic clients are enabled.
Tests will be run weekly.

11 months agoBUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)
Frederic Lecaille [Fri, 2 Aug 2024 09:05:45 +0000 (11:05 +0200)] 
BUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)

By "aws-lc only", one means that this bug was first revealed by aws-lc stack.
This does not mean it will not appeared for new versions of other TLS stacks which
have never revealed this bug.

This bug was reported by Ilya (@chipitsine) in GH #2657 where some QUIC interop
tests (resumption, zerortt) could lead to crash with haproxy compiled against
aws-lc TLS stack. These crashed were triggered by this BUG_ON() which detects
that too short datagrams with at least one ack-eliciting Initial packet inside
could be built.

  <0>2024-07-31T15:13:42.562717+02:00 [01|quic|5|quic_tx.c:739] qc_prep_pkts():
  next encryption level : qc@0x61d000041080 idle_timer_task@0x60d000006b80 flags=0x6000058

  FATAL: bug condition "first_pkt->type == QUIC_PACKET_TYPE_INITIAL && (first_pkt->flags & (1UL << 0)) && length < 1200" matched at src/quic_tx.c:163
  call trace(12):
  | 0x563ea447bc02 [ba d9 00 00 00 48 8d 35]: main-0x1958ce
  | 0x563ea4482703 [e9 73 fe ff ff ba 03 00]: qc_send+0x17e4/0x1b5d
  | 0x563ea4488ab4 [85 c0 0f 85 00 f6 ff ff]: quic_conn_io_cb+0xab1/0xf1c
  | 0x563ea468e6f9 [48 c7 c0 f8 55 ff ff 64]: run_tasks_from_lists+0x173/0x9c2
  | 0x563ea468f24a [8b 7d a0 29 c7 85 ff 0f]: process_runnable_tasks+0x302/0x6e6
  | 0x563ea4610893 [83 3d aa 65 44 00 01 0f]: run_poll_loop+0x6e/0x57b
  | 0x563ea4611043 [48 8b 1d 46 c7 1d 00 48]: main-0x48d
  | 0x7f64d05fb609 [64 48 89 04 25 30 06 00]: libpthread:+0x8609
  | 0x7f64d0520353 [48 89 c7 b8 3c 00 00 00]: libc:clone+0x43/0x5e

That said everything was correctly done by qc_prep_ptks() to prevent such a case.
But this relied on the hypothesis that the list of encryption levels it used
was always built in the same order as follows for 0-RTT sessions:

    initial, early-data, handshake, application

But this order is determined but the order the TLS stack derives the secrets
for these encryption levels. For aws-lc, this order is not the same but
as follows:

    initial, handshake, application, early-data

During 0-RTT sessions, the server may have to build three ack-eliciting packets
(with CRYPTO data inside) to reply to the first client packet: initial, hanshake,
application. qc_prep_pkts() adds a PADDING frame to the last built packet
for the last encryption level in the list. But after application level encryption,
there is early-data encryption level. This prevented qc_prep_pkts() to build
a padded applicaiton level last packet to send a 1200-bytes datagram.

To fix this, always insert early-data encryption level after the initial
encryption level into the encryption levels list when initializing this encryption
level from quic_conn_enc_level_init().

Must be backported as far as 2.9.

11 months agoBUG/MEDIUM: peer: Notify the applet won't consume data when it waits for sync
Christopher Faulet [Thu, 1 Aug 2024 15:09:06 +0000 (17:09 +0200)] 
BUG/MEDIUM: peer: Notify the applet won't consume data when it waits for sync

When the peer applet is waiting for a synchronisation with the global sync
task, we must notify it won't consume data. Otherwise, if some data are
already waiting in the input buffer, the applet will be woken up in loop and
this wil trigger the watchdog. Once synchronized, the applet is woken up. In
that case, the peer applet must indicate it is going to consume data again.

This patch should fix the issue #2656. It must be backported to 3.0.

11 months agoBUG/MEDIUM: mux-h2: Propagate term flags to SE on error in h2s_wake_one_stream
Christopher Faulet [Thu, 1 Aug 2024 14:22:41 +0000 (16:22 +0200)] 
BUG/MEDIUM: mux-h2: Propagate term flags to SE on error in h2s_wake_one_stream

When a stream is explicitly woken up by the H2 conneciton, if an error
condition is detected, the corresponding error flag is set on the SE. So
SE_FL_ERROR or SE_FL_ERR_PENDING, depending if the end of stream was
reported or not.

However, there is no attempt to propagate other termination flags. We must
be sure to properly set SE_FL_EOI and SE_FL_EOS when appropriate to be able
to switch a pending error to a fatal error.

Because of this bug, the SE remains with a pending error and no end of
stream, preventing the applicative stream to trully abort it. It means on
some abort scenario, it is possible to block a stream infinitely.

This patch must be backported at least as far as 2.8. No bug was observed on
older versions while the same code is inuse.

11 months agoBUG/MEDIUM: h2: Only report early HTX EOM for tunneled streams
Christopher Faulet [Thu, 1 Aug 2024 14:01:50 +0000 (16:01 +0200)] 
BUG/MEDIUM: h2: Only report early HTX EOM for tunneled streams

For regular H2 messages, the HTX EOM flag is synonymous the end of input. So
SE_FL_EOI flag must also be set on the stream-endpoint descriptor. However,
there is an exception. For tunneled streams, the end of message is reported
on the HTX message just after the headers. But in that case, no end of input
is reported on the SE.

But here, there is a bug. The "early" EOM is also report on the HTX messages
when there is no payload (for instance a content-length set to 0). If there
is no ES flag on the H2 HEADERS frame, it is an unexpected case. Because for
the applicative stream and most probably for the opposite endpoint, the
message is considered as finihsed. It is switched in its DONE state (or the
equivalent on the endpoint). But, if an extra H2 frame with the ES flag is
received, a TRAILERS frame or an emtpy DATA frame, an extra EOT HTX block is
pushed to carry the HTX EOM flag. So an extra HTX block is emitted for a
regular HTX message. It is totally invalid, it must never happen.

Because it is an undefined behavior, it is difficult to predict the result.
But it definitly prevent the applicative stream to properly handle aborts
and errors because data remain blocked in the channel buffer. Indeed, the
end of the message was seen, so no more data are forwarded.

It seems to be an issue for 2.8 and upper. Harder to evaluate for older
versions.

This patch must be backported as far as 2.4.

11 months agoBUG/MEDIUM: http-ana: Report error on write error waiting for the response
Christopher Faulet [Thu, 1 Aug 2024 13:42:09 +0000 (15:42 +0200)] 
BUG/MEDIUM: http-ana: Report error on write error waiting for the response

When we are waiting for the server response, if an error is pending on the
frontend side (a write error on client), it is handled as an abort and all
regular response analyzers are removed, except the one responsible to
release the filters, if any. However, while it is handled as an abort, the
error is not reported, as usual, via http_reply_and_close() function. It is
an issue because in that, the channels buffers are not reset.

Because of this bug, it is possible to block a stream infinitely. The
request side is waiting for the response side and the response side is
blocked because filters must be released and this cannot be done because
data remain blocked in channels buffers.

So, in that case, calling http_reply_and_close() with no message is enough
to unblock the stream.

This patch must be backported as far as 2.8.

11 months agoBUG/MINOR: h2: reject extended connect for h2c protocol
Amaury Denoyelle [Thu, 1 Aug 2024 13:52:56 +0000 (15:52 +0200)] 
BUG/MINOR: h2: reject extended connect for h2c protocol

This commit prevents forwarding of an HTTP/2 Extended CONNECT when "h2c"
or "h2" token is set as targetted protocol. Contrary to the previous
commit which deals with HTTP/1 mux, this time the request is rejected
and a RESET_STREAM is reported to the client.

This must be backported up to 2.4 after a period of observation.

11 months agoBUG/MINOR: h1: do not forward h2c upgrade header token
Amaury Denoyelle [Thu, 1 Aug 2024 13:35:06 +0000 (15:35 +0200)] 
BUG/MINOR: h1: do not forward h2c upgrade header token

haproxy supports tunnel establishment through HTTP Upgrade mechanism.
Since the following commit, extended CONNECT is also supported for
HTTP/2 both on frontend and backend side.

  commit 9bf957335e2c385b74901481f7a89c9565dfce53
  MEDIUM: mux_h2: generate Extended CONNECT from htx upgrade

As specified by HTTP/2 rfc, "h2c" can be used by an HTTP/1.1 client to
request an upgrade to HTTP/2. In haproxy, this is not supported so it
silently ignores this. However, Connection and Upgrade headers are
forwarded as-is on the backend side.

If using HTTP/1 on the backend side and the server supports this upgrade
mechanism, haproxy won't be able to parse the HTTP response. If using
HTTP/2, mux backend tries to incorrectly convert the request to an
Extended CONNECT with h2c protocol, which may also prevent the response
to be transmitted.

To fix this, flag HTTP/1 request with "h2c" or "h2" token in an upgrade
header. On converting the header list to HTX, the upgrade header is
skipped if any of this token is present and the H1_MF_CONN_UPG flag is
removed.

This issue can easily be reproduced using curl --http2 argument to
connect to an HTTP/1 frontend.

This must be backported up to 2.4 after a period of observation.

11 months agoBUG/MIONR: quic: fix fc_lost
Amaury Denoyelle [Thu, 1 Aug 2024 09:29:58 +0000 (11:29 +0200)] 
BUG/MIONR: quic: fix fc_lost

Control layer callback get_info has recently been implemented for QUIC.
However, fc_lost always returned 0. This is because quic_get_info() does
not use the correct input argument value to identify lost value.

This does not need to be backported.

11 months agoBUG/MINOR: quic: fix fc_rtt/srtt values
Amaury Denoyelle [Thu, 1 Aug 2024 09:25:15 +0000 (11:25 +0200)] 
BUG/MINOR: quic: fix fc_rtt/srtt values

QUIC has recently implement get_info callback to return RTT/sRTT values.
However, it uses milliseconds, contrary to TCP which uses microseconds.
This cause smp fetch functions to return invalid values. Fix this by
converting QUIC values to microseconds.

This does not need to be backported.

11 months agoMINOR: flags/mux-quic: decode qcc and qcs flags
Amaury Denoyelle [Wed, 31 Jul 2024 15:58:02 +0000 (17:58 +0200)] 
MINOR: flags/mux-quic: decode qcc and qcs flags

Decode QUIC MUX connection and stream elements via qcc_show_flags() and
qcs_show_flags(). Flags definition have been moved outside of USE_QUIC
to ease compilation of flags binary.

11 months agoMINOR: quic: Define ->get_info() control layer callback for QUIC
Frederic Lecaille [Tue, 30 Jul 2024 14:32:48 +0000 (16:32 +0200)] 
MINOR: quic: Define ->get_info() control layer callback for QUIC

This low level callback may be called by several sample fetches for
frontend connections like "fc_rtt", "fc_rttvar" etc.
Define this callback for QUIC protocol as pointer to quic_get_info().
This latter supports these sample fetches:
   "fc_lost", "fc_reordering", "fc_rtt" and "fc_rttvar".

Update the documentation consequently.

11 months agoMINOR: tcp_sample: Move TCP low level sample fetch function to control layer
Frederic Lecaille [Tue, 30 Jul 2024 13:21:43 +0000 (15:21 +0200)] 
MINOR: tcp_sample: Move TCP low level sample fetch function to control layer

Add ->get_info() new control layer callback definition to protocol struct to
retreive statiscal counters information at transport layer (TCPv4/TCPv6) identified by
an integer into a long long int.
Move the TCP specific code from get_tcp_info() to the tcp_get_info() control layer
function (src/proto_tcp.c) and define it as  the ->get_info() callback for
TCPv4 and TCPv6.
Note that get_tcp_info() is called for several TCP sample fetches.
This patch is useful to support some of these sample fetches for QUIC and to
keep the code simple and easy to maintain.

11 months agoBUG/MEDIUM: quic: prevent conn freeze on 0RTT undeciphered content
Amaury Denoyelle [Mon, 29 Jul 2024 08:42:50 +0000 (10:42 +0200)] 
BUG/MEDIUM: quic: prevent conn freeze on 0RTT undeciphered content

Received QUIC packets are stored in quic_conn Rx buffer after header
protection removal in qc_rx_pkt_handle(). These packets are then removed
after quic_conn IO handler via qc_treat_rx_pkts().

If HP cannot be removed, packets are still copied into quic_conn Rx
buffer. This can happen if encryption level TLS keys are not yet
available. The packet remains in the buffer until HP can be removed and
its content processed.

An issue occurs if client emits a 0-RTT packet but haproxy does not have
the shared secret, for example after a haproxy process restart. In this
case, the packet is copied in quic_conn Rx buffer but its HP won't ever
be removed. This prevents the buffer to be purged. After some time, if
the client has emitted enough packets, Rx buffer won't have any space
left and received packets are dropped. This will cause the connection to
freeze.

To fix this, remove any 0-RTT buffered packets on handshake completion.
At this stage, 0-RTT packets are unnecessary anymore. The client is
expected to reemit its content in 1-RTT packet which are properly
deciphered.

This can easily reproduce with HTTP/3 POST requests or retrieving a big
enough object, which will fill the Rx buffer with ACK frames. Here is a
picoquic command to provoke the issue on haproxy startup :

$ picoquicdemo -Q -v 00000001 -a h3 <hostname> 20443 "/?s=1g"

Note that allow-0rtt must be present on the bind line to trigger the
issue. Else haproxy will reject any 0-RTT packets.

This must be backported up to 2.6.

This could be one of the reason for github issue #2549 but it's unsure
for now.

11 months agoBUILD: ssl: replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC
William Lallemand [Tue, 30 Jul 2024 13:51:59 +0000 (15:51 +0200)] 
BUILD: ssl: replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC

Replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC in the code source, so we
won't need to set USE_OPENSSL_AWSLC in the Makefile on the long term.

11 months agoBUG/MEDIUM: ssl: 0-RTT initialized at the wrong place for AWS-LC
William Lallemand [Tue, 30 Jul 2024 12:54:44 +0000 (14:54 +0200)] 
BUG/MEDIUM: ssl: 0-RTT initialized at the wrong place for AWS-LC

Revert patch fcc8255 "MINOR: ssl_sock: Early data disabled during
SSL_CTX switching (aws-lc)". The patch was done in the wrong callback
which is never built for AWS-LC, and applies options on the SSL_CTX
instead of the SSL, which should never be done elsewhere than in the
configuration parsing.

This was probably triggered by successfully linking haproxy against
AWS-LC without using USE_OPENSSL_AWSLC.

The patch also reintroduced SSL_CTX_set_early_data_enabled() in the
ssl_quic_initial_ctx() and ssl_sock_initial_ctx(). So the initial_ctx
does have the right setting, but it still needs to be applied to the
selected SSL_CTX in the clienthello, because we need it on the selected
SSL_CTX.

Must be backported to 3.0. (ssl_clienthello.c part was in ssl_sock.c)

11 months agoBUG/MEDIUM: ssl: reactivate 0-RTT for AWS-LC
William Lallemand [Mon, 29 Jul 2024 13:42:47 +0000 (15:42 +0200)] 
BUG/MEDIUM: ssl: reactivate 0-RTT for AWS-LC

Then reactivate HAVE_SSL_0RTT and HAVE_SSL_0RTT_QUIC for AWS-LC, which
were wrongly deactivated in f5353f2c ("MINOR: ssl: add HAVE_SSL_0RTT
constant").

Must be backported to 3.0.

11 months agoBUG/MINOR: stconn: bs.id and fs.id had their dependencies incorrect
Willy Tarreau [Tue, 30 Jul 2024 15:54:26 +0000 (17:54 +0200)] 
BUG/MINOR: stconn: bs.id and fs.id had their dependencies incorrect

The backend depends on the response and the frontend on the request, not
the other way around. In addition, they used to depend on L6 (hence
contents in the channel buffers) while they should only depend on L5
(permanent info known in the mux).

This came in 2.9 with commit 24059615a7 ("MINOR: Add sample fetches to
get the frontend and backend stream ID") so this can be backported there.

(cherry picked from commit 61dd0156c82ea051779e6524cad403871c31fc5a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
11 months agoBUILD: mux-pt: Use the right name for the sedesc variable
Christopher Faulet [Tue, 30 Jul 2024 08:43:55 +0000 (10:43 +0200)] 
BUILD: mux-pt: Use the right name for the sedesc variable

A typo was introduced in 760d26a86 ("BUG/MEDIUM: mux-pt/mux-h1: Release the
pipe on connection error on sending path"). The sedesc variable is 'sd', not
'se'.

This patch must be backported with the commit above.

11 months agoBUG/MEDIUM: mux-pt/mux-h1: Release the pipe on connection error on sending path
Christopher Faulet [Tue, 30 Jul 2024 06:41:03 +0000 (08:41 +0200)] 
BUG/MEDIUM: mux-pt/mux-h1: Release the pipe on connection error on sending path

When data are sent using the kernel splicing, if a connection error
occurred, the pipe must be released. Indeed, in that case, no more data can
be sent and there is no reason to not release the pipe. But it is in fact an
issue for the stream because the channel will appear are not empty. This may
prevent the stream to be released. This happens on 2.8 when a filter is also
attached on it. On 2.9 and upper, it seems there is not issue. But it is
hard to be sure and the current patch remains valid is all cases. On 2.6 and
lower, the code is not the same and, AFAIK, there is no issue.

This patch must be backported to 2.8. However, on 2.8, there is no zero-copy
data forwarding. The patch must be adapted. There is no done_ff/resume_ff
callback functions for muxes. The pipe must released in sc_conn_send() when
an error flag is set on the SE, after the call to snd_pipe callback
function.

11 months agoBUG/MEDIUM: stconn: Report error on SC on send if a previous SE error was set
Christopher Faulet [Mon, 29 Jul 2024 15:48:16 +0000 (17:48 +0200)] 
BUG/MEDIUM: stconn: Report error on SC on send if a previous SE error was set

When a send on a connection is performed, if a SE error (or a pending error)
was already reported earlier, we leave immediately. No send is performed.
However, we must be sure to report the error at the SC level if necessary.
Indeed, the SE error may have been reported during the zero-copy data
forwarding. So during receive on the opposite side. In that case, we may
have missed the opportunity to report it at the SC level.

The patch must be backported as far as 2.8.

11 months agoDOC: config: Add documentation about spop mode for backends
Christopher Faulet [Mon, 29 Jul 2024 15:25:37 +0000 (17:25 +0200)] 
DOC: config: Add documentation about spop mode for backends

The SPOE was refactored. Now backends referenced by a SPOE filter must use
the spop mode to be able to use the spop multiplexer for server connections.
The "spop" mode was added in the list of supported mode for backends.

11 months agoBUG/MEDIUM: queue: deal with a rare TOCTOU in assign_server_and_queue()
Willy Tarreau [Mon, 22 Jul 2024 06:29:28 +0000 (08:29 +0200)] 
BUG/MEDIUM: queue: deal with a rare TOCTOU in assign_server_and_queue()

After checking that a server or backend is full, it remains possible
to call pendconn_add() just after the last pending requests finishes, so
that there's no more connection on the server for very low maxconn (typ 1),
leaving new ones in queue till the timeout.

The approach depends on where the request was queued, though:
  - when queued on a server, we can simply detect that we may dequeue
    pending requests and wake them up, it will wake our request and
    that's fine. This needs to be done in srv_redispatch_connect() when
    the server is set.

  - when queued on a backend, it means that all servers are done with
    their requests. It means that all servers were full before the
    check and all were empty after. In practice this will only concern
    configs with less servers than threads. It's where the issue was
    first spotted, and it's very hard to reproduce with more than one
    server. In this case we need to load-balance again in order to find
    a spare server (or even to fail). For this, we call the newly added
    dedicated function pendconn_must_try_again() that tells whether or
    not a blocked pending request was dequeued and needs to be retried.

This should be backported along with pendconn_must_try_again() to all
stable versions, but with extreme care because over time the queue's
locking evolved.

11 months agoMINOR: queue: add a function to check for TOCTOU after queueing
Willy Tarreau [Fri, 26 Jul 2024 17:24:33 +0000 (19:24 +0200)] 
MINOR: queue: add a function to check for TOCTOU after queueing

There's a rare TOCTOU case that happens from time to time with maxconn 1
and multiple threads. Between the moment we see the queue full and the
moment we queue a request, it's possible that the last request on the
server or proxy ended and that no other one is left to offer it its place.

Given that all this code path is performance-critical and we cannot afford
to increase the lock duration, better recheck for the condition after
queueing. For this we need to be able to check for the condition and
cleanly dequeue a request. That's what this patch provides via the new
function pendconn_must_try_again(). It will catch more requests than
absolutely needed though it will catch them all. It may find that around
1/1000 of requests are at risk, though testing shows that in practice,
it's around 1 per million that really gets stuck (other ones benefit
from timing and finishing late requests). Maybe in the future some
conditions might be refined but it's harmless.

What happens to such requests is that they're dequeued and their pendconn
freed, so that the caller can decide to try to LB or queue them again. For
now the function is not used, it's just added separately for easier tracking.

11 months agoBUILD: cfgparse-quic: fix build error on Solaris due to missing netinet/in.h
Willy Tarreau [Sun, 28 Jul 2024 12:59:23 +0000 (14:59 +0200)] 
BUILD: cfgparse-quic: fix build error on Solaris due to missing netinet/in.h

Since commit 35470d518 ("MINOR: quic: activate UDP GSO for QUIC if
supported"), Solaris build fails due to netinet/udp.h being included
without netinet/in.h. Adding it is sufficient to fix the problem. No
backport is needed.

11 months agoBUG/MEDIUM: jwt: Clear SSL error queue on error when checking the signature
Christopher Faulet [Fri, 26 Jul 2024 14:47:15 +0000 (16:47 +0200)] 
BUG/MEDIUM: jwt: Clear SSL error queue on error when checking the signature

When the signature included in a JWT is verified, if an error occurred, one
or more SSL errors are queued and never cleared. These errors may be then
caught by the SSL stack and a fatal SSL error may be erroneously reported
during a SSL received or send.

So we must take care to clear the SSL error queue when the signature
verification failed.

This patch should fix issue #2643. It must be backported as far as 2.6.

11 months agoMINOR: quic: Dump TX in flight bytes vs window values ratio.
Frederic Lecaille [Fri, 26 Jul 2024 14:30:57 +0000 (16:30 +0200)] 
MINOR: quic: Dump TX in flight bytes vs window values ratio.

Display the ratio of the numbers of bytes in flight by packet number spaces
versus the current window values in percent.

11 months agoMINOR: quic: Add information to "show quic" for CUBIC cc.
Frederic Lecaille [Fri, 26 Jul 2024 14:06:27 +0000 (16:06 +0200)] 
MINOR: quic: Add information to "show quic" for CUBIC cc.

Add ->state_cli() new callback to quic_cc_algo struct to define a
function called by the "show quic (cc|full)" commands to dump some information
about the congestion algorithm internal state currently in use by the QUIC
connections.

Implement this callback for CUBIC algorithm to dump its internal variables:
   - K: (the time to reach the cubic curve inflexion point),
   - last_w_max: the last maximum window value reached before intering
     the last recovery period. This is also the window value at the
     inflexion point of the cubic curve,
   - wdiff: the difference between the current window value and last_w_max.
     So negative before the inflexion point, and positive after.

11 months agoMEDIUM: h1: allow to preserve keep-alive on T-E + C-L
Willy Tarreau [Fri, 26 Jul 2024 13:14:58 +0000 (15:14 +0200)] 
MEDIUM: h1: allow to preserve keep-alive on T-E + C-L

In 2.5-dev9, commit 631c7e866 ("MEDIUM: h1: Force close mode for invalid
uses of T-E header") enforced a recently arrived new security rule in the
HTTP specification aiming at preventing a class of content-smuggling
attacks involving HTTP/1.0 agents. It consists in handling the very rare
T-E + C-L requests or responses in close mode.

It happens it does have an impact of a rare few and very old clients
(probably running insecure TLS stacks by the way) that continue to send
both with their POST requests. The impact is that for each and every
request they'll have to reconnect, possibly negotiating a full TLS
handshake that becomes harmful to the machine in terms of CPU computation.

This commit adds a new option "h1-do-not-close-on-insecure-transfer-encoding"
that does exactly what it says, it just asks not to close on such messages,
even though the message continues to be sanitized and C-L dropped. It means
that the risk is only between the sender and haproxy, which is limited, and
might be the only acceptable solution for such environments having to deal
with broken implementations.

The cases are so rare that it should not need to be backported, or in the
worst case, to the latest LTS if there is any demand.

11 months agoBUG/MEDIUM: quic: fix invalid conn reject with CONNECTION_REFUSED
Amaury Denoyelle [Fri, 26 Jul 2024 13:24:35 +0000 (15:24 +0200)] 
BUG/MEDIUM: quic: fix invalid conn reject with CONNECTION_REFUSED

quic-initial rules were implemented just recently. For some actions, a
new flags field was added in quic_dgram structure. This is used to
report the result of the rules execution.

However, this flags field was left uninitialized. Depending on its
value, it may close the connection to be wrongly rejected via
CONNECTION_REFUSED. Fix this by properly set flags value to 0.

No need to backport.

11 months agoMINOR: quic: implement send-retry quic-initial rules
Amaury Denoyelle [Mon, 22 Jul 2024 11:29:04 +0000 (13:29 +0200)] 
MINOR: quic: implement send-retry quic-initial rules

Define a new quic-initial "send-retry" rule. This allows to force the
emission of a Retry packet on an initial without token instead of
instantiating a new QUIC connection.

11 months agoMINOR: quic: implement reject quic-initial action
Amaury Denoyelle [Fri, 19 Jul 2024 15:39:04 +0000 (17:39 +0200)] 
MINOR: quic: implement reject quic-initial action

Define a new quic-initial action named "reject". Contrary to dgram-drop,
the client is notified of the rejection by a CONNECTION_CLOSE with
CONNECTION_REFUSED error code.

To be able to emit the necessary CONNECTION_CLOSE frame, quic_conn is
instantiated, contrary to dgram-drop action. quic_set_connection_close()
is called immediatly after qc_new_conn() which prevents the handshake
startup.

11 months agoMINOR: quic: pass quic_dgram as obj_type for quic-initial rules
Amaury Denoyelle [Fri, 19 Jul 2024 14:04:22 +0000 (16:04 +0200)] 
MINOR: quic: pass quic_dgram as obj_type for quic-initial rules

To extend quic-initial rules, pass quic_dgram instance to argument for
the various actions. As such, quic_dgram is now supported as an obj_type
and can be used in session origin field.

11 months agoMINOR: quic: support ACL for quic-initial rules
Amaury Denoyelle [Fri, 19 Jul 2024 14:05:15 +0000 (16:05 +0200)] 
MINOR: quic: support ACL for quic-initial rules

Add ACL condition support for quic-initial rules. This requires the
extension of quic_parse_quic_initial() to parse an extra if/unless
block.

Only layer4 client samples are allowed to be used with quic-initial
rules. However, due to the early execution of quic-initial rules prior
to any connection instantiation, some samples are non supported.

To be able to use the 4 described samples, a dummy session is
instantiated before quic-initial rules execution. Its src and dst fields
are set from the received datagram values.

11 months agoMEDIUM: quic: implement quic-initial rules
Amaury Denoyelle [Thu, 18 Jul 2024 16:25:43 +0000 (18:25 +0200)] 
MEDIUM: quic: implement quic-initial rules

Implement a new set of rules labelled as quic-initial.

These rules as specific to QUIC. They are scheduled to be executed early
on Initial packet parsing, prior a new QUIC connection instantiation.
Contrary to tcp-request connection, this allows to reject traffic
earlier, most notably by avoiding unnecessary QUIC SSL handshake
processing.

A new module quic_rules is created. Its main function
quic_init_exec_rules() is called on Initial packet parsing in function
quic_rx_pkt_retrieve_conn().

For the moment, only "accept" and "dgram-drop" are valid actions. Both
are final. The latter drops silently the Initial packet instead of
allocating a new QUIC connection.

11 months agoMINOR: quic: delay Retry emission on quic-force-retry
Amaury Denoyelle [Fri, 19 Jul 2024 15:37:52 +0000 (17:37 +0200)] 
MINOR: quic: delay Retry emission on quic-force-retry

Currently, quic Retry packets are emitted for two different reasons
after processing an Initial without token :
- quic-force-retry is set on bind-line
- an abnormal number of half-open connection is currently detected

Previously, these two conditions were checked separately in different
functions during datagram parsing. Uniformize this by moving
quic-force-retry check in quic_rx_pkt_retrieve_conn() along the second
condition check.

The purpose of this patch is to uniformize datagram parsing stages. It
is necessary to implement quic-initial rules in
quic_rx_pkt_retrieve_conn() prior to any Retry emission. This prevents
to emit unnecessary Retry if an Initial is subject to a reject rule.

11 months agoMEDIUM: sink: assume sft appctx stickiness
Aurelien DARRAGON [Wed, 24 Jul 2024 13:57:04 +0000 (15:57 +0200)] 
MEDIUM: sink: assume sft appctx stickiness

As mentioned in b40d804 ("MINOR: sink: add some comments about sft->appctx
usage in applet handlers"), there are few places in the code where it
looks like we assumed that the applet callbacks such as
sink_forward_session_init() or sink_forward_io_handler() could be
executing an appctx whose sft is detached from the appctx
(appctx != sft->appctx).

In practise this should not be happening since an appctx sticks to the
same thread its entire lifetime, and the only times sft->appctx is
effectively assigned is during the session/appctx creation (in
process_sink_forward()) or release.

Thus if sft->appctx wouldn't point to the appctx that the sft was bound
to after appctx creation, it would probably indicate a bug rather than
an expected condition. To further emphasize that and prevent the
confusion, and since 3.1-dev4 was released, let's remove such checks and
instead add a BUG_ON to ensure this never happens.

In _sink_forward_io_handler(), the "hard_close" label was removed since
there are no more uses for it (no hard errors may be caught from the
function for now)

11 months agoMEDIUM: quic: implement CHACHA20_POLY1305 for AWS-LC
William Lallemand [Thu, 25 Jul 2024 08:54:10 +0000 (10:54 +0200)] 
MEDIUM: quic: implement CHACHA20_POLY1305 for AWS-LC

With AWS-LC, the aead part is covered by the EVP_AEAD API which
provides the correct EVP_aead_chacha20_poly1305(), however for header
protection it does not provides an EVP_CIPHER for chacha20.

This patch implements exceptions in the header protection code and use
EVP_CIPHER_CHACHA20 and EVP_CIPHER_CTX_CHACHA20 placeholders so we can
use the CRYPTO_chacha_20() primitive manually instead of the EVP_CIPHER
API.

This requires to check if we are using EVP_CIPHER_CTX_CHACHA20 when
doing EVP_CIPHER_CTX_free().

11 months agoMEDIUM: quic: add key argument to header protection crypto functions
William Lallemand [Thu, 25 Jul 2024 08:33:29 +0000 (10:33 +0200)] 
MEDIUM: quic: add key argument to header protection crypto functions

In order to prepare the code for using Chacha20 with the EVP_AEAD API,
both quic_tls_hp_decrypt() and quic_tls_hp_encrypt() need an extra key
argument.

Indeed Chacha20 does not exists as an EVP_CIPHER in AWS-LC, so the key
won't be embedded into the EVP_CIPHER_CTX, so we need an extra parameter
to use it.

11 months agoMINOR: quic: rename confusing wording aes to hp
William Lallemand [Thu, 18 Jul 2024 13:03:54 +0000 (15:03 +0200)] 
MINOR: quic: rename confusing wording aes to hp

Some of the crypto functions used for headers protection in QUIC are
named with an "aes" name even thought they are not used for AES
encryption only.

This patch renames these "aes" to "hp" so it is clearer.

11 months agoMEDIUM: ssl/quic: implement quic crypto with EVP_AEAD
William Lallemand [Wed, 10 Jul 2024 08:28:27 +0000 (10:28 +0200)] 
MEDIUM: ssl/quic: implement quic crypto with EVP_AEAD

The QUIC crypto is using the EVP_CIPHER API in order to achieve
authenticated encryption, this was the API which was used with OpenSSL.
With libraries that inspires from BoringSSL (libreSSL and AWS-LC), the
AEAD algorithms are implemented using the EVP_AEAD API.

This patch converts the call to the EVP_CIPHER API when called in the
contex of AEAD cryptography for QUIC.

The patch defines some QUIC_AEAD macros that can be either EVP_CIPHER or
EVP_AEAD depending on the library.

This was mainly done for AWS-LC but this could be useful for other
libraries. This should finally allow to use CHACHA20_POLY1305 with
AWS-LC.

This patch allows to use the following ciphers with the EVP_AEAD API:
- TLS1_3_CK_AES_128_GCM_SHA256
- TLS1_3_CK_AES_256_GCM_SHA384

AWS-LC does not implement TLS1_3_CK_AES_128_CCM_SHA256 and
TLS1_3_CK_CHACHA20_POLY1305_SHA256 requires some hack for headers
protection which will come in another patch.

11 months agoBUG/MINOR: quic: Lack of precision when computing K (cubic only cc)
Frederic Lecaille [Wed, 24 Jul 2024 14:16:26 +0000 (16:16 +0200)] 
BUG/MINOR: quic: Lack of precision when computing K (cubic only cc)

K cubic variable is stored in ms. But it was a formula with the second as unit
for the window difference parameter which was used to compute K without
considering the loss of information. Then the result was converted in ms (K *= 1000).
This leaded to a lack of precision and multiples of 1000 as values.

To fix this, use the same formula but with the window difference in ms as parameter
passed to the cubic function and remove the conversion.

Must be backported as far as 2.6.

11 months ago[RELEASE] Released version 3.1-dev4 v3.1-dev4
Willy Tarreau [Wed, 24 Jul 2024 16:20:24 +0000 (18:20 +0200)] 
[RELEASE] Released version 3.1-dev4

Released version 3.1-dev4 with the following main changes :
    - MINOR: limits: prepare to keep limits in one place
    - REORG: fd: move raise_rlim_nofile to limits
    - CLEANUP: fd: rm struct rlimit definition
    - REORG: global: move rlim_fd_*_at_boot in limits
    - MINOR: haproxy: prepare to move limits-related code
    - REORG: haproxy: move limits handlers to limits
    - MINOR: limits: add is_any_limit_configured
    - CLEANUP: quic: remove obsolete comment on send
    - MINOR: quic: extend detection of UDP API OS features
    - MINOR: quic: activate UDP GSO for QUIC if supported
    - MINOR: quic: define quic_cc_path MTU as constant
    - MINOR: quic: add GSO parameter on quic_sock send API
    - MAJOR: quic: support GSO when encoding datagrams
    - MEDIUM: quic: implement GSO fallback mechanism
    - MINOR: quic: add counters of sent bytes with and without GSO
    - BUG/MEDIUM: bwlim: Be sure to never set the analyze expiration date in past
    - CLEANUP: proto: rename TID affinity callbacks
    - CLEANUP: quic: rename TID affinity elements
    - BUG/MINOR: limits: fix license type in limits.h
    - BUG/MINOR: session: Eval L4/L5 rules defined in the default section
    - CLEANUP: stconn: Fix a typo in comments for SE_ABRT_SRC_*
    - MEDIUM: spoe: Remove fragmentation support
    - MEDIUM: spoe: Remove async mode support
    - MINOR: spoe: Use only a global engine-id per agent
    - MINOR: spoe: Remove debugging
    - MAJOR: spoe: Remove idle applets and pipelining support
    - MINOR: spoe: Remove the dedicated SPOE applet task
    - MEDIUM: proxy/spoe: Add a SPOP mode
    - MEDIUM: applet: Add a .shut callback function for applets
    - MINOR: connection: No longer include stconn type header in connection-t.h
    - MINOR: stconn: Use a dedicated function to get the opposite sedesc
    - MINOR: spoe: Rename some flags and constant to use SPOP prefix
    - MINOR: spoe: Dynamically alloc the message list per event of an agent
    - MINOR: spoe: Move all stuff regarding the filter/applet in the C file
    - MINOR: spoe: Move spoe_str_to_vsn() into the header file
    - MEDIUM: mux-spop: Introduce the SPOP multiplexer
    - MEDIUM: check/spoe: Use SPOP multiplexer to perform SPOP health-checks
    - MAJOR: spoe: Rewrite SPOE applet to use the SPOP mux
    - CLEANUP: spoe: Uniformize function definitions
    - MINOR: spoe: Add internal sample fetch to retrieve the SPOE engine ID
    - MEDIUM: spoe: Set a specific name for the connection pool of SPOP servers
    - MINOR: backend: Remove test on HTX streams to reuse idle connections on connect
    - MEDIUM: spoe: Force the reuse 'always' mode for SPOP backends
    - MINOR: mux-spop: Use a dedicated function to update the SPOP connection timeout
    - MAJOR: mux-spop: Make the SPOP connections reusable
    - MINOR: stats-html: Display reuse ratio for spop connections
    - MEDIUM: spoe: Directly xfer NOTIFY frame when SPOE applet is created
    - MEDIUM: spoe: Directly receive ACK frame in the SPOE context buffer
    - MEDIUM: mux-spop/spoe: Save negociated max-frame-size value in the mux
    - MINOR: spoe: Remove the spop version from the SPOE appctx context
    - MEDIUM: mux-spop: Add checks on received frames
    - MEDIUM: mux-spop: Announce the pipeling support if possible
    - MEDIUM: spoe: Forward SPOE context error to the SPOE applet
    - MEDIUM: spoe: Make the SPOE applet use its own buffers
    - DOC: spoe: Update SPOE documentation to reflect recent refactoring
    - BUILD: mux-spop: fix build failure on gcc 4-10 and clang
    - MINOR: fd: don't scan the full fdtab on all threads
    - MINOR: server: better mt_list usage for node migration (prev_deleted handling)
    - BUG/MINOR: do not close uninit FD in quic_test_socketops()
    - BUG/MEDIUM: debug/cli: fix "show threads" crashing with low thread counts
    - MINOR: debug: prepare feed_post_mortem_late
    - CLEANUP: debug: fix indents in debug_parse_cli_show_dev
    - MINOR: debug: store runtime uid/gid in postmortem
    - MINOR: debug: keep runtime capabilities in post_mortem
    - MINOR: debug: use LIM2A to show limits
    - MINOR: debug: prepare to show runtime limits
    - MINOR: debug: keep runtime limits in postmortem
    - DOC: install: don't reference removed CPU arg
    - BUG/MEDIUM: ssl_sock: fix deadlock in ssl_sock_load_ocsp() on error path
    - BUG/MAJOR: mux-h2: force a hard error upon short read with pending error
    - MEDIUM: sink: start applets asynchronously
    - OPTIM: sink: balance applets accross threads
    - MEDIUM: ocsp: fix ocsp when the chain is loaded from 'issuers-chain-path'
    - MEDIUM: ssl: add extra_chain to ckch_data
    - MINOR: ssl: change issuers-chain for show_cert_detail()
    - REGTESTS: ssl: test the issuers-chain-path keyword
    - DOC: configuration: issuers-chain-path not compatible with OCSP
    - DOC: configuration: issuers-chain-path is compatible with OCSP
    - BUG/MEDIUM: startup: fix zero-warning mode
    - BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char (2)
    - MINOR: cfgparse-global: move mode's keywords in cfg_kw_list
    - MINOR: cfgparse-global: move no<poller_name> in cfg_kw_list
    - DOC: config: improve the http-keep-alive section
    - BUG/MINOR: stick-table: fix crash for src_inc_gpc() without stkcounter
    - BUG/MINOR: server: Don't warn fallback IP is used during init-addr resolution
    - BUG/MINOR: cli: Atomically inc the global request counter between CLI commands
    - MINOR: stream: Add a pointer to set the parent stream
    - MINOR: vars: Fill a description instead of hash and scope when a name is parsed
    - MINOR: vars: Use a description to set/unset a variable instead of its hash and scope
    - MEDIUM: vars: Be able to parse parent scopes for variables
    - MINOR: vars: Use a variable description to get variables of a specific scope
    - MEDIUM: vars: Be able to retrieve variable of the parent stream, if any
    - MEDIUM: spoe: Set the parent stream for SPOE streams
    - BUG/MINOR: quic: Non optimal first datagram.
    - DOC: config: Add a dedicated section about variables
    - DOC: config: Add info about variable scopes referencing the parent stream
    - DOC: config: Explicitly state the SPOE streams have a usable parent stream
    - MINOR: quic: Avoid cc priv buffer overflow.
    - MINOR: spoe: Add a function to validate a version is supported
    - MINOR: spoe: export the list of SPOP error reasons
    - MEDIUM: spoe/tcpcheck: Reintroduce SPOP check as a customized tcp-check
    - REGTESTS: check/spoe: Re-enable the script performing SPOP health-checks
    - BUG/MEDIUM: sink: properly init applet under sft lock
    - MINOR: sink: unify and sink_forward_io_handler() and sink_forward_oc_io_handler()
    - MINOR: sink: Remove useless test on SE_FL_SHR/SHW flags
    - MINOR: sink: merge sink_forward_io_handler() with sink_forward_oc_io_handler()
    - MINOR: sink: add some comments about sft->appctx usage in applet handlers
    - MINOR: sink: distinguish between hard and soft close in _sink_forward_io_handler()
    - MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface
    - MINOR: ring: count processed messages in ring_dispatch_messages()
    - MINOR: sink: add processed events counter in sft
    - MEDIUM: sink: "max-reuse" support for sink servers
    - OPTIM: sink: consider threads' current load when rebalancing applets

11 months agoOPTIM: sink: consider threads' current load when rebalancing applets
Aurelien DARRAGON [Mon, 22 Jul 2024 15:52:18 +0000 (17:52 +0200)] 
OPTIM: sink: consider threads' current load when rebalancing applets

In c454296f0 ("OPTIM: sink: balance applets accross threads"), we already
made sure to balance applets accross threads by picking a random thread
to spawn the new applet.

Also, thanks to the previous commit, we also have the ability to destroy
the applet when a certain amount of messages were processed to help
distribute the load during runtime.

Let's improve that by trying up to 3 different threads in the hope to pick
a non-overloaded one in the best scenario, and the least over loaded one
in the worst case. This should help to better distribute the load over
multiple threads when high loads are expected.

Logic was greatly inspired from thread migration logic used by server
health checks, but it was simpliflied for sink's use case.

11 months agoMEDIUM: sink: "max-reuse" support for sink servers
Aurelien DARRAGON [Mon, 22 Jul 2024 14:55:30 +0000 (16:55 +0200)] 
MEDIUM: sink: "max-reuse" support for sink servers

Thanks to the previous commit, it is now possible to know how many events
were processed for a given sft/server sink pair. As mentioned in commit
c454296 ("OPTIM: sink: balance applets accross threads"), let's provide
the ability to restart a server connection when a certain amount of events
were processed to help better balance the load over multiple threads.

For this, we make use the of "max-reuse" server keyword which was only
relevant under "http" context so far. Under sink context, "max-reuse"
corresponds to the number of times the tcp connection can be reused
for sending messages, which in fact means that "max-reuse + 1" is the
number of events (ie: messages) that are allowed to be sent using the
same tcp server connection: when this threshold is met, the connection
will be destroyed and a new one will be created on a random thread.
The value is not strict: it is the minimum value above which the
connection may be destroyed since the value is checked after
ring_dispatch_messages() which may process multiple messages at once.

By default, no limit is enforced (the connection will be reused for as
long as it is available).

The documentation was updated accordingly.

11 months agoMINOR: sink: add processed events counter in sft
Aurelien DARRAGON [Mon, 22 Jul 2024 13:24:26 +0000 (15:24 +0200)] 
MINOR: sink: add processed events counter in sft

Add a new struct member to sft structure named e_processed in order to
track the total number of events processed by sft applets.

sink_forward_oc_io_handler() and sink_forward_io_handler() now make use
of ring_dispatch_messages() optional value added in the previous commit
in order to increase the number of processed events.

11 months agoMINOR: ring: count processed messages in ring_dispatch_messages()
Aurelien DARRAGON [Mon, 22 Jul 2024 09:17:08 +0000 (11:17 +0200)] 
MINOR: ring: count processed messages in ring_dispatch_messages()

ring_dispatch_messages() now takes an optional argument <processed> which
must point to a size_t counter when provided.

When provided, the value is updated to the number of messages processed
by the function.

11 months agoMEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface
Aurelien DARRAGON [Tue, 23 Jul 2024 15:16:58 +0000 (17:16 +0200)] 
MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface

Given that sink applets are responsible for conveying messages from the
ring to the tcp server endpoint, there are no protocol timeout or errors
expected there, it is an unidirectional flow of data over TCP.

As such, NOLINGER flag which was inherited from peers applet, see
dbd026792 ("BUG/MEDIUM: peers: set NOLINGER on the outgoing stream
interface") is not desirable under sink context:

The reason why we have the NOLINGER flag set is to ensure the connection
is closed right away and avoid 60s TIME_WAIT delay on closed sockets.
The downside is that messages sent right before closing the socket are
not guaranteed to make it to the server because closing with NOLINGER
flag set will result in RST packet being emitted right away, which could
prevent in-flight messages from being properly delivered.

Unlike peers applets, the only cases were sink applets are expected to
close the connection are upon unexpected error or upon stopping, which are
relatively rare events. Thanks to previous commit, ERROR flag is already
set in case of error, so the use of NOLINGER is not mandatory for the
RST to be sent. Now for the stopping case, it only happens once in the
process lifetime so it's acceptable to close the socket using EOS+EOI
flags without the NOLINGER option set.

So in our case, it is preferable to ensure messages get properly delivered
knowning that closed sockets should be piling up in TIME_WAIT, this means
removing the NOLINGER flag on the outgoing stream interface for sink
applets. It is a prerequisite for upcoming patches in order to cleanly
shut the applet during runtime without risking to send the RST packet
before all pending messages were sent to the endpoint.

11 months agoMINOR: sink: distinguish between hard and soft close in _sink_forward_io_handler()
Aurelien DARRAGON [Tue, 23 Jul 2024 17:18:32 +0000 (19:18 +0200)] 
MINOR: sink: distinguish between hard and soft close in _sink_forward_io_handler()

Aborting the socket on soft-stop is not the same as aborting it due to
unexpected error. As such, let's leverage the granularity offered by
sedesc flags to better reflect the situation: abort during soft-stop is
handled as a soft close thanks to EOI+EOS flags, while abort due to
unexpected error is handled as hard error thanks to ERROR+EOS flags.

Thanks to this change, hard error will always emit RST packet even if
the NOLINGER option wasn't set on the socket.

11 months agoMINOR: sink: add some comments about sft->appctx usage in applet handlers
Aurelien DARRAGON [Wed, 24 Jul 2024 15:42:31 +0000 (17:42 +0200)] 
MINOR: sink: add some comments about sft->appctx usage in applet handlers

There seem to be an ambiguity in the code where sft->appctx would differ
from the appctx that was assigned to it upon appctx creation.

In practise, it doesn't seem this could be happening. Adding a few notes
to come back to this later and try to see if we can remove this
ambiguity.

11 months agoMINOR: sink: merge sink_forward_io_handler() with sink_forward_oc_io_handler()
Aurelien DARRAGON [Tue, 23 Jul 2024 17:12:28 +0000 (19:12 +0200)] 
MINOR: sink: merge sink_forward_io_handler() with sink_forward_oc_io_handler()

Now that sink_forward_oc_io_handler() and sink_forward_io_handler() were
unified again thanks to the previous commit, let's take a chance to merge
code that is common to both functions in order to ease code maintenance.

Let's add _sink_forward_io_handler() internal function which takes the
applet and a message handler as argument: sink_forward_io_handler() and
sink_forward_oc_io_handler() leverage this internal function by passing
the correct message handler for the desired format.

11 months agoMINOR: sink: Remove useless test on SE_FL_SHR/SHW flags
Aurelien DARRAGON [Tue, 23 Jul 2024 17:02:29 +0000 (19:02 +0200)] 
MINOR: sink: Remove useless test on SE_FL_SHR/SHW flags

Re-apply dcd917d972 ("MINOR: applet: Remove uselelss test on SE_FL_SHR/SHW
flags") for sink_forward_oc_io_handler() function as it was probably
overlooked given that sink_forward_oc_io_handler() and
sink_forward_io_handler() follow the same logic.

11 months agoMINOR: sink: unify and sink_forward_io_handler() and sink_forward_oc_io_handler()
Aurelien DARRAGON [Tue, 23 Jul 2024 16:57:35 +0000 (18:57 +0200)] 
MINOR: sink: unify and sink_forward_io_handler() and sink_forward_oc_io_handler()

In a739dc2 ("MEDIUM: sink: Use the sedesc to report and detect end of
processing"), we added a drain after close in sink_forward_oc_io_handler()
by the use of "goto out".

However, since we perform a close, there is no reason to drain data from
the socket. Moreover, before the patch there was no drain and nothing
mentioned the fact that that the drain was added on purpose. Lastly,
sink_forward_io_handler() and sink_forward_oc_io_handler() functions are
strictly identical when in comes to processing logic, and the drain was
only added in sink_forward_oc_io_handler() and not in
sink_forward_io_handler().

As such, it's pretty safe to assume that the drain is not needed here
and was added as accident. So in this patch we remove it in an attempt
to unify sink_forward_io_handler() and sink_forward_oc_io_handler()
functions like it was already the case before.

11 months agoBUG/MEDIUM: sink: properly init applet under sft lock
Aurelien DARRAGON [Wed, 24 Jul 2024 13:45:50 +0000 (15:45 +0200)] 
BUG/MEDIUM: sink: properly init applet under sft lock

Since 09d69eacf8 ("MEDIUM: sink: start applets asynchronously") the applet
is no longer initialized under the sft lock while it was the case before.

At first it doesn't seem to be an issue, but if we look closer at
sink_forward_session_init(), we can see that sft->appctx is assigned
while it can be accessed at the same time from sink_init_forward().

Let's restore the old guarantees by performing the .init under the sft
lock.

No backport needed unless 09d69eacf8 is.

11 months agoREGTESTS: check/spoe: Re-enable the script performing SPOP health-checks
Christopher Faulet [Mon, 22 Jul 2024 17:08:48 +0000 (19:08 +0200)] 
REGTESTS: check/spoe: Re-enable the script performing SPOP health-checks

Thanks to previous patches, it is now possible to re-enable the test on SPOP
health-checks support.

11 months agoMEDIUM: spoe/tcpcheck: Reintroduce SPOP check as a customized tcp-check
Christopher Faulet [Mon, 22 Jul 2024 17:00:42 +0000 (19:00 +0200)] 
MEDIUM: spoe/tcpcheck: Reintroduce SPOP check as a customized tcp-check

To be able to retrieve accurrate errors when a SPOP health-check is
performed, a customized tcp-check is used. Indeed, it is not possible to
rely on the SPOP multiplexer for now because the check is performed at the
mux connection layer and the error, if any, cannot be retrieved by the
health-check. A L4 success or error is reported.

To fix this issue and restore the previous behavior, a customized tcp-check
is created. The connection is forced to use the PT multiplexer. An hardcoded
message is sent and a customer handler is used to decode the SPOA response.
This way, it is possible to parse the response and return an accurrate
status code.

11 months agoMINOR: spoe: export the list of SPOP error reasons
Christopher Faulet [Mon, 22 Jul 2024 16:57:00 +0000 (18:57 +0200)] 
MINOR: spoe: export the list of SPOP error reasons

The strings representing the human-readable version for SPOP errors are now
exported. It is now an array of IST to ease manipulation.

11 months agoMINOR: spoe: Add a function to validate a version is supported
Christopher Faulet [Mon, 22 Jul 2024 16:55:28 +0000 (18:55 +0200)] 
MINOR: spoe: Add a function to validate a version is supported

spoe_check_vsn() function can now be used to check if a version, converted
to an integer, via spoe_str_to_vsn() for instance, is supported. To do so,
the list of all supported version is now exported.

11 months agoMINOR: quic: Avoid cc priv buffer overflow.
Frederic Lecaille [Wed, 24 Jul 2024 09:07:19 +0000 (11:07 +0200)] 
MINOR: quic: Avoid cc priv buffer overflow.

Add two initcall callback with BUG_ON_HOT() to newro and cubic modules to
ensure there is no buffer overflow when accessing the private data of
these congestion control algorithm state structures. This is to ensure
that further modifications about these data structures will not
lead to surprises. At this time there is no possible buffer overflow.

11 months agoDOC: config: Explicitly state the SPOE streams have a usable parent stream
Christopher Faulet [Fri, 19 Jul 2024 14:29:26 +0000 (16:29 +0200)] 
DOC: config: Explicitly state the SPOE streams have a usable parent stream

It is explicitly mentionned in the configuration manual that the parent of a
SPOE stream is the filtered stream. It means variables of the filtered
stream are usable from the SPOE stream.

11 months agoDOC: config: Add info about variable scopes referencing the parent stream
Christopher Faulet [Fri, 19 Jul 2024 14:28:52 +0000 (16:28 +0200)] 
DOC: config: Add info about variable scopes referencing the parent stream

It is now possible for a stream to have a parent and it is also possible to
retrieve variables defined in the parent stream context. To do so, some
extra scopes were introduced. The section 2.8. was updated accordingly.

11 months agoDOC: config: Add a dedicated section about variables
Christopher Faulet [Fri, 19 Jul 2024 14:27:38 +0000 (16:27 +0200)] 
DOC: config: Add a dedicated section about variables

The variables in the HAProxy configuration are now described in a dedicated
section. Instead of repeating the same description everywhere a variable
name can be used, the section 2.8. is now referenced.

11 months agoBUG/MINOR: quic: Non optimal first datagram.
Frederic Lecaille [Fri, 19 Jul 2024 14:06:55 +0000 (16:06 +0200)] 
BUG/MINOR: quic: Non optimal first datagram.

This bug arrived with this commit:

     b068e758f MINOR: quic: simplify rescheduling for handshake

This commit introduced a bad side effect. Haproxy always replied by an ACK-only
datagram when it received the first client Initial packet. Then it handled
the CRYPTO data insided. And finally, it sent its own CRYPTO data. This broke
the packet coalescing rule whose aim is to optimally build and send as more
as QUIC packets by datagram.

To fix this, simply partially reverts this commit, to make the low level I/O
task return again if some CRYPTO were received. This will delay the
acknowledgement which will be sent with the CRYPTO data from the same
datagram again.

Must be backported to 3.0.

11 months agoMEDIUM: spoe: Set the parent stream for SPOE streams
Christopher Faulet [Wed, 17 Jul 2024 15:06:00 +0000 (17:06 +0200)] 
MEDIUM: spoe: Set the parent stream for SPOE streams

When a SPOE applet is created to send a message to an agent, the parent of
the associated stream is set to the one filtered. And the relationship
between the streams is removed when the applet is released or when the
processing on main stream is finished.

In the mean time, it is possible to get variables of the parent stream from
the SPOE one. It is not a huge change but this will be amazingly useful. For
instance, it is now possible to be sticky on a server using a critera of the
main streem. Here is an example using the client source address:

  listen http
    bind *:80
    tcp-request content set-var(txn.client_src) src
    filter spoe engine {SPOE-NAME} config /{SPOE-CONFIG}
    http-request send-spoe-group {SPOE-NAME} {SPOE-MSG}
    server www 127.0.0.1:8000

  backend spoe-backend
    mode spop
    timeout server 10s

    stick-table type ip size 200k expire 30m
    stick on var(ptxn.client_src)

    server srv1 ...
    server srv2 ...
    server srv3 ...
    server srv4 ...

Of course, the feature is not limited to stick-tables. Everywhere variables
are used, it is now possible to get the value set on the parent stream from
the SPOE stream.

11 months agoMEDIUM: vars: Be able to retrieve variable of the parent stream, if any
Christopher Faulet [Wed, 17 Jul 2024 14:54:45 +0000 (16:54 +0200)] 
MEDIUM: vars: Be able to retrieve variable of the parent stream, if any

It is now possible to retrieved the value of a variable using the parent
stream or the parent session instead of the current one. It remains
forbidden to set or unset this value. The sample fetch used to store the
result is a local copy. So it may be safely altered by a converter without
changing the value of the original variable.

Note that for now, the parent of a stream is never set. So this part is not
really used. This will change with the SPOE.

11 months agoMINOR: vars: Use a variable description to get variables of a specific scope
Christopher Faulet [Wed, 17 Jul 2024 14:53:20 +0000 (16:53 +0200)] 
MINOR: vars: Use a variable description to get variables of a specific scope

Now a variable description is retrieved when a variable is parsed, we can
use it to get the variable value. It is mandatory to be able to know the
parent stream, if any, must be used, instead of the current one.

11 months agoMEDIUM: vars: Be able to parse parent scopes for variables
Christopher Faulet [Wed, 17 Jul 2024 13:57:55 +0000 (15:57 +0200)] 
MEDIUM: vars: Be able to parse parent scopes for variables

Add session/stream scopes related to the parent. To do so, "psess", "ptxn",
"preq" or "pres" must be used instead of tranditionnal scopes (without the
first "p"). the "proc" scope is not concerned by this change because it is
not linked to a stream. When such scopes are used, a specific flags is added
on the variable description during the variable parsing.

For now, theses scopes are parsed and the variable description is updated
accordingly. But at the end, any operation on the variable value fails.

11 months agoMINOR: vars: Use a description to set/unset a variable instead of its hash and scope
Christopher Faulet [Wed, 17 Jul 2024 14:38:13 +0000 (16:38 +0200)] 
MINOR: vars: Use a description to set/unset a variable instead of its hash and scope

Now a variable description is retrieved when a variable is parsed, we can
use it to set or unset the variable value. It is mandatory to be able to
know the parent stream, if any, must be used, instead of the current one.

11 months agoMINOR: vars: Fill a description instead of hash and scope when a name is parsed
Christopher Faulet [Wed, 17 Jul 2024 13:56:40 +0000 (15:56 +0200)] 
MINOR: vars: Fill a description instead of hash and scope when a name is parsed

A variable description is now used to parse a variable and extract its name
and its scope. It is mandatory to be able to add some flags on the variable
when it is evaluated (set or get). Among other things, this will be used to
know the parent stream, if any, must be used, instead of the current one.

11 months agoMINOR: stream: Add a pointer to set the parent stream
Christopher Faulet [Tue, 16 Jul 2024 13:55:57 +0000 (15:55 +0200)] 
MINOR: stream: Add a pointer to set the parent stream

A pointer to a parent stream was added in the stream structure. For now,
this pointer is never set, but the idea is to have an access to a stream
environment from another one from the moment there is a parent/child
relationship betwee these streams.

Concretely, for now, there is nothing to formalize this relationship.

11 months agoBUG/MINOR: cli: Atomically inc the global request counter between CLI commands
Christopher Faulet [Tue, 16 Jul 2024 12:42:20 +0000 (14:42 +0200)] 
BUG/MINOR: cli: Atomically inc the global request counter between CLI commands

The global request counter is used to set the stream id (s->uniq_id). It is
incremented at different places. And it must be atomically incremented
because it is a global value. However, in the analyer dealing with CLI
command response, this was not the case. It is now fixed.

This patch must be backported to all stable versions.

11 months agoBUG/MINOR: server: Don't warn fallback IP is used during init-addr resolution
Christopher Faulet [Thu, 18 Jul 2024 07:52:46 +0000 (09:52 +0200)] 
BUG/MINOR: server: Don't warn fallback IP is used during init-addr resolution

When a fallback IP address is provided in the list of methods to use to
resolve the server address, a warning is emitted if previous methods
failed. The aim is to inform this address will be used for the
server. However, it is valid use-case. It is the expected behavior. There is
no reason to emit a warning. Having a message during HAProxy startup to
inform the fallback IP address will be used is probably a good idea. But it
should be a notice not a warning. Otherwise, checking the configuration
validity will always failed, just like starting HAProxy in zero-warning
mode while the option was set on purpose.

This patch should fix the issue #2627. It must be backported to all stable
versions.

11 months agoBUG/MINOR: stick-table: fix crash for src_inc_gpc() without stkcounter
Amaury Denoyelle [Thu, 18 Jul 2024 12:48:55 +0000 (14:48 +0200)] 
BUG/MINOR: stick-table: fix crash for src_inc_gpc() without stkcounter

Since 2.5, an array of GPC is provided to replace legacy gpc0/gpc1.
src_inc_gpc is a sample fetch which is used to increment counters in
this array.

A crash occurs if src_inc_gpc is used without any previous track-sc
rule. This is caused by an error in smp_fetch_sc_inc_gpc(). When
temporary stick counter is created via smp_create_src_stkctr(), table
pointer arg value used is not correct : it points to the counter ID
instead of the table argument. To fix this, use the proper sample fetch
second arg.

This can be reproduced with the following config :
  acl mark src_inc_gpc(0,<table>) -m bool
  tcp-request connection accept if mark

This should be backported up to 2.6.

11 months agoDOC: config: improve the http-keep-alive section
Willy Tarreau [Wed, 17 Jul 2024 15:25:40 +0000 (17:25 +0200)] 
DOC: config: improve the http-keep-alive section

Nathan Wehrman suggested this add-on to try to better explain the
interactions between http-keep-alive and other timeouts, and the
impacts on protocols (HTTP/1, HTTP/2 etc).

11 months agoMINOR: cfgparse-global: move no<poller_name> in cfg_kw_list
Valentine Krasnobaeva [Thu, 18 Jul 2024 08:51:23 +0000 (10:51 +0200)] 
MINOR: cfgparse-global: move no<poller_name> in cfg_kw_list

This commit continues to clean up cfg_parse_global() and to prepare the
refactoring of master-worker mode. Master, after forking a worker, enters in
its wait polling loop to catch signals and to provide master CLI. So, some
poller types could be disabled for master process it as well.

11 months agoMINOR: cfgparse-global: move mode's keywords in cfg_kw_list
Valentine Krasnobaeva [Tue, 16 Jul 2024 16:02:53 +0000 (18:02 +0200)] 
MINOR: cfgparse-global: move mode's keywords in cfg_kw_list

This commit cleans up cfg_parse_global() and prepares the config parser for
master-worker mode refactoring, where daemon and master-worker fork() calls
will happen very early in init().

So, the config in such case should be read twice:
 - at first: only some keywords in the global section for the mode discovery
   and everything, which is related to master process by opportunity;
 - at second: except the master process, all other keywords would be parsed;

11 months agoBUILD: tree-wide: cast arguments to tolower/toupper to unsigned char (2)
Aurelien DARRAGON [Thu, 18 Jul 2024 11:20:04 +0000 (13:20 +0200)] 
BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char (2)

Fix build warning on NetBSD by reapplying f278eec37a ("BUILD: tree-wide:
cast arguments to tolower/toupper to unsigned char").

This should fix issue #2551.

11 months agoBUG/MEDIUM: startup: fix zero-warning mode
Valentine Krasnobaeva [Wed, 17 Jul 2024 16:40:41 +0000 (18:40 +0200)] 
BUG/MEDIUM: startup: fix zero-warning mode

Let's check the second time a global counter of "ha_warning" messages, if
zero-warning is set. And let's do this just before forking. At this moment we
are sure, that we've already done all init operations, where we could emit
"ha_warning", and we still have stderr fd opened.

Even with the second check, we could lost some late and rare warnings
about failing to drop supplementary groups and about re-enabling core dumps.
Notes about this are added into 'zero-warning' keyword description.

11 months agoDOC: configuration: issuers-chain-path is compatible with OCSP
William Lallemand [Wed, 17 Jul 2024 16:20:43 +0000 (18:20 +0200)] 
DOC: configuration: issuers-chain-path is compatible with OCSP

Since patch f3dfd95a ("MEDIUM: ocsp: fix ocsp when the chain is loaded
from 'issuers-chain-path'") the OCSP features are compatible with
'issuers-chain-path'.

11 months agoDOC: configuration: issuers-chain-path not compatible with OCSP
William Lallemand [Wed, 17 Jul 2024 15:46:16 +0000 (17:46 +0200)] 
DOC: configuration: issuers-chain-path not compatible with OCSP

State that issuers-chain-path is not compatible with OCSP features.

Must be backported in every stable version.

11 months agoREGTESTS: ssl: test the issuers-chain-path keyword
William Lallemand [Wed, 17 Jul 2024 09:51:31 +0000 (11:51 +0200)] 
REGTESTS: ssl: test the issuers-chain-path keyword

Add a reg-test which test the completion of the issuers-chain-path
keyword

Note that it could be interesting to have the loading of a .ocsp
combined with this, but our pki for OCSP tests lacks
the SubjectKeyIdentifier extensions.

11 months agoMINOR: ssl: change issuers-chain for show_cert_detail()
William Lallemand [Wed, 17 Jul 2024 09:37:04 +0000 (11:37 +0200)] 
MINOR: ssl: change issuers-chain for show_cert_detail()

Since data->chain is now completed when loading the files, we don't need
to use ssl_get0_issuer_chain() anywhere else in the code.

data->chain will always be completed once the files are loaded, but we
can't know from show_cert_detail() from what chain file it was completed.
That's why the extra_chain pointer was added to dump the chain file.

11 months agoMEDIUM: ssl: add extra_chain to ckch_data
William Lallemand [Wed, 17 Jul 2024 11:32:43 +0000 (13:32 +0200)] 
MEDIUM: ssl: add extra_chain to ckch_data

The extra_chain member is a pointer to the 'issuers-chain-path' file
that completed the chain.

This is useful to get what chain file was used.

11 months agoMEDIUM: ocsp: fix ocsp when the chain is loaded from 'issuers-chain-path'
Valentine Krasnobaeva [Thu, 11 Jul 2024 15:46:56 +0000 (17:46 +0200)] 
MEDIUM: ocsp: fix ocsp when the chain is loaded from 'issuers-chain-path'

This fixes OCSP, when issuer chain is in a separate PEM file. This is a
case of issuers-chain-path keyword, which points to folder that contains only
PEM with RootCA and IntermediateCA.

Before this patch, the chain from 'issuers-chain-path' was applied
directly to the SSL_CTX without being applied to the data->chain
structure. This would work for SSL traffic, but every tests done with
data->chain would fail, OCSP included, because the chain would be NULL.

This patch moves the loading of the chain from
ssl_sock_load_cert_chain(), which is the function that applies the chain
to the SSL_CTX, to ssl_sock_load_pem_into_ckch() which is the function
that loads the files into the ckch_data structure.

Fixes issue #2635 but it changes thing on the CLI, so that's not
backportable.

11 months agoOPTIM: sink: balance applets accross threads
Aurelien DARRAGON [Wed, 17 Jul 2024 14:19:12 +0000 (16:19 +0200)] 
OPTIM: sink: balance applets accross threads

Most of the time all sink applets (which are responsible for relaying
messages from the ring to the tcp servers endpoints) would end up being
assigned to the first available thread (tid:0), resulting in excessive CPU
usage on a single thread when multiple sink servers were defined (no
matter if they were defined over multiple "ring" sections) and significant
message load was pushed through them over the ring API.

This patch is similar to 34e4085f ("MEDIUM: peers: Balance applets across
threads") but for sinks. We use a slightly different approach, which is to
elect a random thread instead of picking the one with leasts applets. This
proves to be already sufficient to alleviate the issue.

In the case we want to have a better load distribution we should consider
breaking existing connections to reestablish them on a new thread when we
find out that they start monopolizing a cpu thread (ie: after a certain
amount of messages for instance). Also check tcpchecks migrating model for
inspiration.

This patch depends on the previous one ("MEDIUM: sink: start applets
asynchronously").

11 months agoMEDIUM: sink: start applets asynchronously
Aurelien DARRAGON [Wed, 17 Jul 2024 13:12:54 +0000 (15:12 +0200)] 
MEDIUM: sink: start applets asynchronously

Since d9c1d33fa1 ("MEDIUM: applet: Add support for async appctx startup
on a thread subset"), it is now possible to delay appctx's init: for that
it is required that the .init callback is defined on the applet.

When the applet will be processed on the first run, applet API will
automatically finish the applet initialization. Thus we explicitly
call appctx_wakeup() on the applet to schedule it for initial run
instead of calling appctx_init() ourselves.

This is done in prevision of the next patch in order to be able to
schedule the applet on a different thread from the one executing
sink_forward_session_create() function.

Note: 'out_free_appctx' label was removed since it is no longer used.

11 months agoBUG/MAJOR: mux-h2: force a hard error upon short read with pending error
Willy Tarreau [Tue, 16 Jul 2024 14:38:31 +0000 (16:38 +0200)] 
BUG/MAJOR: mux-h2: force a hard error upon short read with pending error

A risk of truncated packet was addressed in 2.9 by commit 19fb19976f
("BUG/MEDIUM: mux-h2: Only Report H2C error on read error if demux
buffer is empty") by ignoring CO_FL_ERROR after a recv() call as long
as some data remained present in the buffer. However it has a side
effect due to the fact that some frame processors only deal with full
frames, for example, HEADERS. The side effect is that an incomplete
frame will not be processed and will remain in the buffer, preventing
the error from being taken into account, so the I/O handler wakes up
the H2 parser to handle the error, and that one just subscribes for
more data, and this loops forever wasting CPU cycles.

Note that this only happens with errors at the SSL layer exclusively,
otherwise we'd have a read0 pending that would properly be detected:

  conn->flags = CO_FL_XPRT_TRACKED | CO_FL_ERROR | CO_FL_XPRT_READY | CO_FL_CTRL_READY
  conn->err_code = CO_ERR_SSL_FATAL
  h2c->flags  = H2_CF_ERR_PENDING | H2_CF_WINDOW_OPENED | H2_CF_MBUF_HAS_DATA | H2_CF_DEM_IN_PROGRESS | H2_CF_DEM_SHORT_READ

The condition to report the error in h2_recv() needs to be refined, so
that connection errors are taken into account either when the buffer is
empty, or when there's an incomplete frame, since we're certain it will
never be completed. We're certain to enter that function because
H2_CF_DEM_SHORT_READ implies too short a frame, and earlier there's a
protocol check to validate that no frame size is larger than bufsize,
hence a H2_CF_DEM_SHORT_READ implies there's some room left in the
buffer and we're allowed to try to receive.

The condition to reproduce the bug seems super hard to meet but was
observed once by Patrick Hemmer who had the reflex to capture lots of
information that allowed to explain the problem. In order to reproduce
it, the SSL code had to be significantly modified to alter received
contents at very empiric places, but that was sufficient to reproduce
it and confirm that the current patch works as expected.

The bug was tagged MAJOR because when it triggers there's no other
solution to get rid of it but to restart the process. However given how
hard it is to trigger on a lab, it does not seem very likely to occur
in field.

This needs to be backported to 2.9.

11 months agoBUG/MEDIUM: ssl_sock: fix deadlock in ssl_sock_load_ocsp() on error path
Valentine Krasnobaeva [Mon, 15 Jul 2024 12:57:05 +0000 (14:57 +0200)] 
BUG/MEDIUM: ssl_sock: fix deadlock in ssl_sock_load_ocsp() on error path

We could run under heavy load in containers or on premises and some automatic
tool in parallel could use CLI to check OCSP updates statuses or to upload new
OCSP responses. So, calloc() to store OCSP update callback arguments may fail
and ocsp_tree_lock need to be unlocked, when exiting due to this failure.

This needs to be backported in all stable versions until v2.4.0 included.

11 months agoDOC: install: don't reference removed CPU arg
Lukas Tribus [Tue, 16 Jul 2024 17:47:50 +0000 (17:47 +0000)] 
DOC: install: don't reference removed CPU arg

Remove reference to the removed CPU= build argument in commit 018443b8a1
("BUILD: makefile: get rid of the CPU variable").

This should be backported to 3.0.

11 months agoMINOR: debug: keep runtime limits in postmortem
Valentine Krasnobaeva [Sun, 14 Jul 2024 14:58:02 +0000 (16:58 +0200)] 
MINOR: debug: keep runtime limits in postmortem

It's usefull to keep runtime limits (fd and RAM) in postmortem and show them in
debug_parse_cli_show_dev(). Runtime limits are fed in feed_post_mortem_late(),
as we are sure that at this moment that all configuration was parsed and
all applied limits were alredy adjusted.

11 months agoMINOR: debug: prepare to show runtime limits
Valentine Krasnobaeva [Sat, 13 Jul 2024 11:23:46 +0000 (13:23 +0200)] 
MINOR: debug: prepare to show runtime limits

This is a preparation patch to extend postmortem in order to store runtime
limits. No need to perform getrlimit() in feed_post_mortem(), as we do this
in the very beginning of main() and we store initial fd limits in global
'rlim_fd_cur_at_boot' and 'rlim_fd_max_at_boot' variables.

11 months agoMINOR: debug: use LIM2A to show limits
Valentine Krasnobaeva [Fri, 12 Jul 2024 15:55:15 +0000 (17:55 +0200)] 
MINOR: debug: use LIM2A to show limits

It is more handy to use LIM2A in debug_parse_cli_show_dev(), as it allows to
show a custom string ("unlimited"), if a given limit value equals to 0.

normalize_rlim() handler is needed to convert properly RLIM_INFINITY to zero,
with the respect of type sizes, as rlim_t is always 4 bytes on 32bit and
64bit arch.

11 months agoMINOR: debug: keep runtime capabilities in post_mortem
Valentine Krasnobaeva [Sun, 14 Jul 2024 13:20:02 +0000 (15:20 +0200)] 
MINOR: debug: keep runtime capabilities in post_mortem

Let's extend postmortem to keep process runtime capabilities. This information
is gathered in feed_post_mortem_late(), as it is called just before
run_poll_loop() and we are sure at this moment, that all configuration
settings were successfully applied.

11 months agoMINOR: debug: store runtime uid/gid in postmortem
Valentine Krasnobaeva [Fri, 12 Jul 2024 15:50:18 +0000 (17:50 +0200)] 
MINOR: debug: store runtime uid/gid in postmortem

Let's extend post_mortem to store runtime process uid and gid.
This information is fed in feed_post_mortem_late(), just before calling
run_poll_loop(). Like this we are sure that all configuration settings were
successfully applied.

11 months agoCLEANUP: debug: fix indents in debug_parse_cli_show_dev
Valentine Krasnobaeva [Sun, 14 Jul 2024 13:54:43 +0000 (15:54 +0200)] 
CLEANUP: debug: fix indents in debug_parse_cli_show_dev

Fix indents in debug_parse_cli_show_dev() to avoid useless conflicts in case of
future changes in this function or git-bisect.