Commit Graph

21758 Commits

Author SHA1 Message Date
Frédéric Lécaille
028a55a1d0 MINOR: quic: Add a max window parameter to congestion control algorithms
Add a new ->max_cwnd member to bind_conf struct to store the maximum
congestion control window value for each QUIC binding.
Modify the "quic-cc-algo" keyword parsing to add an optional parameter
to its value: the maximum congestion window value between parentheses
as follows:

      ex: quic-cc-algo cubic(10m)

This value must be bounded, greater than 10k and smaller than 1g.
2023-11-13 17:53:18 +01:00
Frédéric Lécaille
840af0928b BUG/MEDIUM: quic: Non initialized CRYPTO data stream deferencing
This bug arrived with this commit:
   BUG/MINOR: quic: Useless use of non-contiguous buffer for in order CRYPTO data

Before this commit qc->cstream was tested before entering qc_treat_rx_crypto_frms().
This patch restablishes this behavior. Furthermore, it simplyfies
qc_ssl_provide_all_quic_data() which is a little bit ugly: the CRYPTO data
frame may be freed asap in the list_for_each_entry_safe() block after
having store its data pointer and length in local variables.
Also interrupt the CRYPTO data process as soon as qc_ssl_provide_quic_data()
or qc_treat_rx_crypto_frms() fail.

No need to be backported.
2023-11-13 16:00:25 +01:00
William Lallemand
59b313832a REGTESTS: startup: -conf-OK requires -V with current VTest
Current version of VTest tests the output of "haproxy -c" instead of the
return code. Since we don't output anymore when the configuration is
valid, this broke the test. (a06f621).

This fixes the issue by adding the -V when doing a -conf-OK. But this
must fixed in VTest.
2023-11-13 14:57:26 +01:00
Christopher Faulet
cb560bf3d7 DOC: config: Fix name for tune.disable-zero-copy-forwarding global param
"disable-" prefix was missing. This param was correctly named in the list of
supported keywords in the global section, but not in the keyword
description.

No backport needed.
2023-11-13 14:31:14 +01:00
Amaury Denoyelle
954b5b756a BUG/MEDIUM: quic: fix FD for quic_cc_conn
Since following commit, quic_conn closes its owned socket before
transition to quic_cc_conn for closing state. This allows to save FDs as
quic_cc_conn could use the listener socket for their I/O.

  commit 150c0da889
  MEDIUM: quic: release conn socket before using quic_cc_conn

This patch is incomplete as it removes initialization of <fd> member for
quic_cc_conn. Thus, if sending is done on closing state, <fd> value is
undefined which in most cases will result in a crash. Fix this by simply
initializing <fd> member with qc_init_fd() in qc_new_cc_conn().

This bug should fix recent issue from #2095. Thanks to Tristan for its
reporting and then testing of this patch.

No need to backport.
2023-11-13 11:55:07 +01:00
Amaury Denoyelle
78d244e9f7 BUG/MINOR: quic: fix decrement of half_open counter on qc alloc failure
Half open counter is used to comptabilize QUIC connections waiting for
address validation. It was recently reworked to adjust its scope. With
each decrement operation, a BUG_ON() was added to ensure the counter
never wraps.

This BUG_ON() could be triggered if an allocation fails for one of
quic_conn members in qc_new_conn(). This is because half open counter is
incremented at the end of qc_new_conn(). However, in case of alloc
failure, quic_conn_release() is called immediately to ensure the counter
is decremented if a connection is freed before peer address has been
validated.

To fix this, increment half open counter early in qc_new_conn() prior to
every quic_conn members allocations.

This issue was reproduced using -dMfail argument.

This issue has been introduced by
  commit 278808915b
  MINOR: quic: reduce half open counters scope

No need to backport.
2023-11-13 11:16:41 +01:00
Amaury Denoyelle
92da3accfd BUG/MINOR: quic: fix crash on qc_new_conn alloc failure
A new counter was recently introduced to comptabilize the current number
of active QUIC handshakes. This counter is stored on the listener
instance.

This counter is incremented at the beginning of qc_new_conn() to check
if limit is not reached prior to quic_conn allocation. If quic_conn or
one of its inner member allocation fails, special care is taken to
decrement the counter as the connection instance is released. However,
it relies on <l> variable which is initialized too late to cover
pool_head_quic_conn allocation failure.

To fix this, simply initialize <l> at the beginning of qc_new_conn().

This issue was reproduced using -dMfail argument.

This issue was introduced by the following commit
  commit 3df6a60113
  MEDIUM: quic: limit handshake per listener

No need to backport.
2023-11-13 11:16:41 +01:00
Aurelien DARRAGON
76acde9107 BUG/MINOR: log: keep the ref in dup_logger()
This bug was introduced with 969e212 ("MINOR: log: add dup_logsrv() helper
function")

When duplicating an existing log entry, we must take care to inherit from
its original ->ref if it is set, because not doing so would make 28ac0999
("MINOR: log: Keep the ref when a log server is copied to avoid duplicate entries")
ineffective given that global log directives will lose their original
reference when duplicated resursively (at least twice), which is what
happens when global log directives are first inherited to defaults which
are then inherited to a regular proxy at the end of the chain.

This can be easily reproduced using the following configuration:

   |global
   |  log stdout format raw local0
   |
   |defaults
   |  log global
   |
   |frontend test
   |  log global
   |  ...

Logs from "test" proxy will be duplicated because test incorrectly
inherited from global "log" directives twice, which 28ac0999 would
normally detect and prevent.

No backport needed unless 969e212 gets backported.
2023-11-13 11:06:05 +01:00
Christopher Faulet
33a1fc883a BUG/MINOR: sample: Fix bytes converter if offset is bigger than sample length
When the bytes converter was improved to be able to use variables (915e48675
["MEDIUM: sample: Enhances converter "bytes" to take variable names as
arguments"]), the behavior of the sample slightly change. A failure is
reported if the given offset is bigger than the sample length. Before, a
empty binary sample was returned.

This patch fixes the converter to restore the original behavior. The
function was also refactored to properly handle failures by removing
SMP_F_MAY_CHANGE flag. Because the converter now handles variables, the
conversion to an integer may fail. In this case SMP_F_MAY_CHANGE flag must
be removed to be sure the caller will not retry.

This patch should fix the issue #2335. No backport needed except if commit
above is backported.
2023-11-13 11:06:05 +01:00
William Lallemand
a06f6212c9 MEDIUM: startup: 'haproxy -c' is quiet when valid
MODE_CHECK does not output "Configuration file is valid" by default
anymore. To display this message the -V option must be used with -c.

However the warning and errors are still output by default if they
exist.

This allows to clean the output of the systemd unit file with is doing a
-c.
2023-11-13 09:59:34 +01:00
Willy Tarreau
cf07cb96be BUG/MEDIUM: proxy: always initialize the default settings after init
The proxy's initialization is rather odd. First, init_new_proxy() is
called to zero all the lists and certain values, except those that can
come from defaults, which are initialized by proxy_preset_defaults().
The default server settings are also only set there.

This results in these settings not to be set for a number of internal
proxies that do not explicitly call proxy_preset_defaults() after
allocation, such as sink and log forwarders.

This was revealed by last commit 79aa63823 ("MINOR: server: always
initialize pp_tlvs for default servers") which crashes in log parsers
when applied to certain proxies which did not initialize their default
servers.

In theory this should be backported, however it would be desirable to
wait a bit before backporting it, in case certain parts would rely on
these elements not being initialized.
2023-11-13 09:17:05 +01:00
Willy Tarreau
79aa638238 MINOR: server: always initialize pp_tlvs for default servers
In commit 6f4bfed3a ("MINOR: server: Add parser support for
set-proxy-v2-tlv-fmt") a suspicious check for a NULL srv_tlv was placed
in the list_for_each_entry(), that should not be needed. In practice,
it's caused by the list head not being initialized, hence the first
element is NULL, as shown by Alexander's reproducer below which crashes
if the test in the loop is removed:

  backend dummy
    default-server send-proxy-v2 set-proxy-v2-tlv-fmt(0xE1) %[fc_pp_tlv(0xE1)]
    server dummy_server 127.0.0.1:2319

The right place to initialize this field is proxy_preset_defaults().
We'd really need a function to initialize a server :-/

The check in the loop was removed. No backport is needed.
2023-11-13 08:53:28 +01:00
Frdric Lcaille
dfda884633 BUG/MINOR: quic: Useless use of non-contiguous buffer for in order CRYPTO data
This issue could be reproduced with a TLS client certificate verificatio to
generate enough CRYPTO data between the client and haproxy and with dev/udp/udp-perturb
as network perturbator. Haproxy could crash thanks to a BUG_ON() call as soon as
in disorder data were bufferized into a non-contiguous buffer.

There is no need to pass a non NULL non-contiguous to qc_ssl_provide_quic_data()
from qc_ssl_provide_all_quic_data() which handles in order CRYPTO data which
have not been bufferized. If not, the first call to qc_ssl_provide_quic_data()
to process the first block of in order data leads the non-contiguous buffer
head to be advanced to a wrong offset, by <len> bytes which is the length of the
in order CRYPTO frame. This is detected by a BUG_ON() as follows:

FATAL: bug condition "ncb_ret != NCB_RET_OK" matched at src/quic_ssl.c:620
  call trace(11):
  | 0x5631cc41f3cc [0f 0b 8b 05 d4 df 48 00]: qc_ssl_provide_quic_data+0xca7/0xd78
  | 0x5631cc41f6b2 [89 45 bc 48 8b 45 b0 48]: qc_ssl_provide_all_quic_data+0x215/0x576
  | 0x5631cc3ce862 [48 8b 45 b0 8b 40 04 25]: quic_conn_io_cb+0x19a/0x8c2
  | 0x5631cc67f092 [e9 1b 02 00 00 83 45 e4]: run_tasks_from_lists+0x498/0x741
  | 0x5631cc67fb51 [89 c2 8b 45 e0 29 d0 89]: process_runnable_tasks+0x816/0x879
  | 0x5631cc625305 [8b 05 bd 0c 2d 00 83 f8]: run_poll_loop+0x8b/0x4bc
  | 0x5631cc6259c0 [48 8b 05 b9 ac 29 00 48]: main-0x2c6
  | 0x7fa6c34a2ea7 [64 48 89 04 25 30 06 00]: libpthread:+0x7ea7
  | 0x7fa6c33c2a2f [48 89 c7 b8 3c 00 00 00]: libc:clone+0x3f/0x5a

Thank you to @Tristan971 for having reported this issue in GH #2095.

No need to backport.
2023-11-10 18:16:14 +01:00
Aurelien DARRAGON
078ebde870 CLEANUP: sink: useless leftover in sink_add_srv()
Removing a useless leftover which has been introduced with 31e8a003a5
("MINOR: sink: function to add new sink servers")
2023-11-10 17:49:57 +01:00
Aurelien DARRAGON
2694621151 CLEANUP: sink: bad indent in sink_new_from_logger()
Fixing bad indent in sink_new_from_logger() which was recently introduced
2023-11-10 17:49:57 +01:00
Aurelien DARRAGON
d710dfbacc BUG/MINOR: sink: don't learn srv port from srv addr
Since 04276f3d ("MEDIUM: server: split the address and the port into two
different fields") we should not use srv->addr to store server's port
and rely on srv->svc_port instead.

For sink servers, we correctly set >svc_port upon server creation but
we didn't use it when initializing address for the connection.

As a result, FQDN resolution will not work properly with sink servers.
Hopefully, this used to work by accident because sink servers were
resolved using the PA_O_RESOLVE flag in str2sa_range(), which made the
srv->addr contain the port in addition to the address.

But this will fail to work when FQDN resolution is postponed because only
->svc_port will contain the proper server port upon resolution.

For instance, FQDN resolution with servers from log backends (which are
resolved as regular servers, that is, without the PA_O_RESOLVE) will fail
to work because of this.

This may be backported as far as 2.2 even though the bug didn't have
noticeable effects for versions below 2.9

[In 2.2, sink_forward_session_init() didn't exist it should be applied in
 sink_forward_session_create()]
2023-11-10 17:49:57 +01:00
Aurelien DARRAGON
64e0b63442 BUG/MEDIUM: server: invalid address (post)parsing checks
This bug was introduced with 29b76ca ("BUG/MEDIUM: server/log: "mode log"
after server keyword causes crash ")

Indeed, we cannot safely rely on addr_proto being set when str2sa_range()
returns in parse_server() (even if SRV_PARSE_PARSE_ADDR is set), because
proto lookup might be bypassed when FQDN addresses are involved.

Unfortunately, the above patch wrongly assumed that proto would always
be set when SRV_PARSE_PARSE_ADDR was passed to parse_server() (so when
str2sa_range() was called), resulting in invalid postparsing checks being
performed, which could as well lead to crashes with log backends
("mode log" set) because some postparsing init was skipped as a result of
proto not being set and this wasn't expected later in the init code.

To fix this, we now make use of the previous patch to perform server's
address compatibility checks on hints that are always set when
str2sa_range() succesfully returns.

For log backend, we're also adding a complementary test to check if the
address family is of expected type, else we report an error, plus we're
moving the postinit logic in log api since _srv_check_proxy_mode() is
only meant to check proxy mode compatibility and we were abusing it.

This patch depends on:
 - "MINOR: tools: make str2sa_range() directly return type hints"

No backport required unless 29b76ca gets backported.
2023-11-10 17:49:57 +01:00
Aurelien DARRAGON
12582eb8e5 MINOR: tools: make str2sa_range() directly return type hints
str2sa_range() already allows the caller to provide <proto> in order to
get a pointer on the protocol matching with the string input thanks to
5fc9328a ("MINOR: tools: make str2sa_range() directly return the protocol")

However, as stated into the commit message, there is a trick:
   "we can fail to return a protocol in case the caller
    accepts an fqdn for use later. This is what servers do and in this
    case it is valid to return no protocol"

In this case, we're unable to return protocol because the protocol lookup
depends on both the [proto type + xprt type] and the [family type] to be
known.

While family type might not be directly resolved when fqdn is involved
(because family type might be discovered using DNS queries), proto type
and xprt type are already known. As such, the caller might be interested
in knowing those address related hints even if the address family type is
not yet resolved and thus the matching protocol cannot be looked up.

Thus in this patch we add the optional net_addr_type (custom type)
argument to str2sa_range to enable the caller to check the protocol type
and transport type when the function succeeds.
2023-11-10 17:49:57 +01:00
Christopher Faulet
ebf90ca550 BUG/MEDIUM: applet: Remove appctx from buffer wait list on release
For now, the appctx is removed from the buffer wait list when it is
freed. However, when it is released, it is not necessarily freed
immediately. But it is detached from the SC. If it is still registered in
the buffer wait list, it could then be woken up to get a buffer. At this
stage it is totally unexpected, especially because we must access the SC.

The fix is obvious, the appctx must be removed from the buffer wait list on
release.

Note this bug exists because the appctx was moved at the mux level.

This patch must be backported as far as 2.6.
2023-11-10 17:49:57 +01:00
Willy Tarreau
9530e7dcd3 DOC: config: use the word 'backend' instead of 'proxy' in 'track' description
User @nwehrman reported in issue #2328 that the used of "proxy" instead
of "backend" in the argument of the "track" server keyword is confusing.
Admittedly, all other places in the doc use "backend/server" instead of
"proxy/server", so let's update it for the sake of consistency.
2023-11-10 16:29:02 +01:00
Amaury Denoyelle
150c0da889 MEDIUM: quic: release conn socket before using quic_cc_conn
After emission/reception of a CONNECTION_CLOSE, a connection enters the
CLOSING state. In this state, only minimal exchanges occurs as only the
packets which containted the CONNECTION_CLOSE frame can be reemitted. In
conformance with the RFC, most resources are released and quic_conn
instance is converted to the lighter quic_cc_conn.

Push further this optimization by closing quic_conn socket FD before
switching to a quic_cc_conn. This means that quic_cc_conn will rely on
listener socket for its send/recv operation. This should not impact
performance as as stated input/output are minimal on this state.

This patch should improve FD consumption as prior to this a socket FD
was kept during the closing delay which could cause maxsock to be
reached for other connections.

Note that fd member is kept in QUIC_CONN_COMMON and not removed from
quic_cc_conn. This is because quic_cc_conn relies on qc_snd_buf() which
access this field.

As a side-effect to this change, jobs accounting for quic_conn is also
updated. quic_cc_conn instances are now not counted as jobs. Indeed, the
main objective of jobs is to prevent haproxy process to be stopped with
data truncation. However, this relies on the connection to uses its
owned socket as the listener socket is shut down inconditionaly on
shutdown.

A consequence of the jobs handling change is that haproxy process will
be closed if only quic_cc_conn instances are present, thus preventing to
respect the closing state. In case of a reload, if a client missed a
CONNECTION_CLOSE frame just before process shutdown, it will probably
received a Stateless Reset on sending retry.

This change is considered safe as, for now, haproxy only emits
CONNECTION_CLOSE on error conditions (such as protocol violation or
timeout). It is considered as expected to suffer from data truncation
from this. However, if connection closing is reused by haproxy to
implement clean shutdown, it should be necessary to delay
CONNECTION_CLOSE frame emission to ensure no data truncation happens
here.
2023-11-10 15:27:45 +01:00
Amaury Denoyelle
f549eb2b34 MEDIUM: quic: respect closing state even on soft-stop
Prior to this patch, a special condition was set when idle timer was
rearmed for closing connections during haproxy process stopping. In this
case, the timeout was ditched and the idle task woken up immediatly.

The objective was to release quickly closing connections to not prevent
the process stopping to be too long. However, it is not conform with RFC
9000 recommandations and may cause some clients to miss a
CONNECTION_CLOSE in case of a packet loss.

A recent fix was set to use a shorter timeout for closing state. Now a
connection should only be left in this state for one second or less.
This reduces greatly the importance of stopping special condition. Thus,
this patch removes it completely.
2023-11-10 15:26:03 +01:00
Amaury Denoyelle
75e36c57f0 BUG/MINOR: quic: remove dead code in error path
In quic_rx_pkt_retrieve_conn(), err label is now only used if qc is
NULL. Thus, condition on qc can be removed.

No need to backport.

This issue was reported by coverity on github.
This should fix issue #2338.
2023-11-10 15:26:03 +01:00
Willy Tarreau
0a7ab7067f OPTIM: mux-h2: don't allocate more buffers per connections than streams
When an H2 mux works with a slow downstream connection and without the
mux-mux mode, it is possible that a single stream will allocate all 32
buffers in the connection. This is not desirable at all because 1) it
brings no value, and 2) it allocates a lot of memory per connection,
which, in addition to using a lot of memory, tends to degrade performance
due to cache thrashing.

This patch improves the situation by refraining from sending data frames
over a connection when more mbufs than streams are allocated. On a test
featuring 10k connections each with a single stream reading from the
cache, this patch reduces the RAM usage from ~180k buffers to ~20k bufs,
and improves the bandwidth. This may even be backported later to recent
versions to improve memory usage. Note however that it is efficient only
when combined with e16762f8a ("OPTIM: mux-h2: call h2_send() directly
from h2_snd_buf()"), and tends to slightly reduce the single-stream
performance without it, so in case of a backport, the two need to be
considered together.
2023-11-09 17:24:00 +01:00
Willy Tarreau
a13f8425f0 MINOR: task/debug: make task_queue() and task_schedule() possible callers
It's common to see process_stream() being woken up by wake_expired_tasks
in the profiling output, without knowing which timeout was set to cause
this. By making it possible to record the call places of task_queue()
and task_schedule(), and by making wake_expired_tasks() explicitly not
replace it, we'll be able to know which task_queue() or task_schedule()
was triggered for a given wakeup.

For example below:
  process_stream                51200   311.4ms   6.081us   34.59s    675.6us <- run_tasks_from_lists@src/task.c:659 task_queue
  process_stream                19227   70.00ms   3.640us   9.813m    30.62ms <- sc_notify@src/stconn.c:1136 task_wakeup
  process_stream                 6414   102.3ms   15.95us   8.093m    75.70ms <- stream_new@src/stream.c:578 task_wakeup

It's visible that it's the run_tasks_from_lists() which in fact applies
on the task->expire returned by the ->process() function itself.
2023-11-09 17:24:00 +01:00
Willy Tarreau
0eb0914dba MINOR: task/debug: explicitly support passing a null caller to wakeup functions
This is used for tracing and profiling. By permitting to have a NULL
caller, we allow a caller to explicitly pass zero to state that the
current caller must not be replaced. This will soon be used by
wake_expired_tasks() to avoid replacing a caller in the expire loop.
2023-11-09 17:24:00 +01:00
Amaury Denoyelle
4dee110f56 BUG/MINOR: quic: fix retry token check inconsistency
A client may send multiple INITIAL packets if ClientHello is too big for
only one. In case a Retry token is used, the client must reuse it for
every INITIAL packets.

On the haproxy server side, there was an inconsistency to handle these
packets depending on the socket mode :
* when using listener socket, token is always revalidated.
* when using connection socket, token check is bypassed. This is because
  quic_conn instance is known through its socket and thus
  quic_rx_pkt_retrieve_conn() is not necessary.

RFC 9000 does not seems to mandate retry token validation after the
first INITIAL packet per connection. Thus, this patch chooses to bypass
the check every time the connection instance is known, as this indicates
that a previous token was already validated.

This should be backported up to 2.7.
2023-11-09 16:57:37 +01:00
Amaury Denoyelle
bb28215d9b MEDIUM: quic: define an accept queue limit
QUIC connections are pushed manually into a dedicated listener queue
when they are ready to be accepted. This happens after handshake
finalization or on 0-RTT packet reception. Listener is then woken up to
dequeue them with listener_accept().

This patch comptabilizes the number of connections currently stored in
the accept queue. If reaching a certain limit, INITIAL packets are
dropped on reception to prevent further QUIC connections allocation.
This should help to preserve system resources.

This limit is automatically derived from the listener backlog. Half of
its value is reserved for handshakes and the other half for accept
queues. By default, backlog is equal to maxconn which guarantee that
there can't be no more than maxconn connections in handshake or waiting
to be accepted.
2023-11-09 16:24:00 +01:00
Amaury Denoyelle
3df6a60113 MEDIUM: quic: limit handshake per listener
Implement a limit per listener for concurrent number of QUIC
connections. When reached, INITIAL packets for new connections are
automatically dropped until the number of handshakes is reduced.

The limit value is automatically based on listener backlog, which itself
defaults to maxconn.

This feature is important to ensure CPU and memory resources are not
consume if too many handshakes attempt are started in parallel.

Special care is taken if a connection is released before handshake
completion. In this case, counter must be decremented. This forces to
ensure that member <qc.state> is set early in qc_new_conn() before any
quic_conn_release() invocation.
2023-11-09 16:23:52 +01:00
Amaury Denoyelle
278808915b MINOR: quic: reduce half open counters scope
Accounting is implemented for half open connections which represent QUIC
connections waiting for handshake completion. When reaching a certain
limit, Retry mechanism is automatically activated prior to instantiate
new connections.

The issue with this behavior is that two notions are mixed : QUIC
connection handshake phase and Retry which is mechanism against
amplification attacks. As such, only peer address validation should be
taken into account to activate Retry protection.

This patch chooses to reduce the scope of half_open_conn. Now only
connection waiting to validate the peer address are now accounted for.
Most notably, connections instantiated with a validated Retry token
check are not accounted.

One impact of this patch is that it should prevent to activate Retry
mechanism too early, in particular in case if multiple handshakes are
too slow. Another limitation should be implemented to protect against
this scenario.
2023-11-09 16:23:52 +01:00
Amaury Denoyelle
d38bb7f8a7 MEDIUM: quic: adjust address validation
When a new QUIC connection is created, server considers peer address as
not yet validated. The server must limit its sending up to 3 times the
content already received. This is a defensive measure to avoid flooding
a remote host victim of address spoofing.

This patch adjust the condition to consider the peer address as
validated. Two conditions are now considered :
* successful handling of a received HANDSHAKE packet. This was already
  done before although implemented in a different way.
* validation of a Retry token. This was not considered prior this patch
  despite RFC recommandation.

This patch also adjusts how a connection is internally labelled as using
a validated peer address. Before, above conditions were checked via
quic_peer_validated_addr(). Now, a flag QUIC_FL_CONN_PEER_VALIDATED_ADDR
is set to labelled this. It already existed prior this patch but was
only used for quic_cc_conn. This should now be more explicit.
2023-11-09 16:23:52 +01:00
Christopher Faulet
3a051ca0c8 BUG/MEDIUM: mux-h1: Exit early if fast-forward is not supported by opposite SC
The commit 4be0c7c65 ("MEDIUM: stconn/muxes: Loop on data fast-forwarding to
forward at least a buffer") introduced a regression. In h1_fastfwd(), if
data fast-forwarding is not supported by the opposite SC, we must exit
without calling se_donn_ff(). Otherwise a BUG_ON() will be triggered because
the opposite mux has no .done_fastfwd() callback function.

No backport needed.
2023-11-09 15:18:43 +01:00
William Lallemand
3ac3a06963 MEDIUM: mworker: -W is mandatory when using -S
Defining a master CLI without the master-worker mode emits a warning
since version 1.8. This patch enforce the behavior by forbiding the
usage of the -S option without the master-worker mode.
2023-11-09 15:07:15 +01:00
William Lallemand
705a72fd19 DOC: management: -q is quiet all the time
The documentation about -q seems wrong, it does not output messages
after the startup, it disables all messages. It was always quiet with
the stdio_quiet() function.

Must be backported in all stable versions.
2023-11-09 14:39:11 +01:00
William Lallemand
da24b462c3 MEDIUM: errors: move the MODE_QUIET test in print_message()
Move the MODE_QUIET and MODE_VERBOSE test in print_message() so we
always output in the startup-logs even with MODE_QUIET.

ha_warning(), ha_alert() and ha_notice() does not check the MODE_QUIET
and MODE_VERBOSE anymore, it is done before doing the fprintf() in
print_message().
2023-11-09 14:39:11 +01:00
William Lallemand
59d699c0c4 MINOR: errors: does not check MODE_STARTING for log emission
ha_alert(), ha_warning() and ha_notice() shouldn't check MODE_STARTING
for log emission. Let's remove the check.

This shouldn't do much since the stdio_quiet() function mute the output
in main().
2023-11-09 14:39:11 +01:00
William Lallemand
b959b752f9 MINOR: errors: ha_alert() and ha_warning() uses warn_exec_path()
Move the code to display the haproxy version and path during starting
mode, which is called by the first ha_alert() or ha_warning().
2023-11-09 14:39:11 +01:00
Christopher Faulet
78021ee9ef BUG/MEDIUM: stconn: Don't update stream expiration date if already expired
The commit 08d7169f4 ("MINOR: stconn: Don't queue stream task in past in
sc_notify()") tried to fix issues with epiration date set in past for the
stream in sc_notify(). However it remains some cases where the stream
expiration date may already be expired before recomputing it. This happens
when an event is reported by the mux exactly when a timeout is triggered. In
this case, depending on the scheduling, the SC may be woken up before the
stream. For these cases, we fall into the BUG_ON() preventing to queue in
the past.

So, it remains unexpected to queue a task in the past. The BUG_ON() is
correct at this place. We must just avoid to recompute the stream expiration
date if it is already expired. At worst, the stream will be woken up for
nothing. But it is not really a big deal because it will only happen on
timeouts from time to time. It is so sporadic that we can ignore it from a
performance point of view.

This patch must be backpoted to 2.8. Be careful to remove the BUG_ON() on
the 2.8.
2023-11-09 12:08:59 +01:00
Frédéric Lécaille
0016dbaef4 BUG/MEDIUM: quic: Possible crashes during secrets allocations (heavy load)
This bug could be reproduced with -dMfail option and detected by libasan.
During the TLS secrets allocations, when failed, quic_tls_ctx_secs_free()
is called. It resets the already initialized secrets. Some were detected
as initialized when not, or with a non initialized length, which leads
to big "memset(0)" detected by libsasan.

Ensure that all the secrets are really initialized with correct lengths.

No need to be backported.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
819690303d BUG/MEDIUM: quic: Avoid some crashes upon TX packet allocation failures
If a TX packet cannot be allocated (by qc_build_pkt()), as it can be coalesced
to another one, this leads the TX buffer to have remaining not sent prepared data.
Then haproxy crashes upon a BUG_ON() triggered by the next call to qc_txb_release().
This may happen only during handshakes.

To fix this, qc_build_pkt() returns a new -3 error to dected such allocation
failures followed which is for now on followed by a call to qc_purge_txbuf() to
send the TX prepared data and purge the TX buffer.

Must be backported as far as 2.6.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
b21e08cbd2 BUG/MEDIUM: quic: Possible crashes when sending too short Initial packets
This may happen during handshakes when Handshake packets cannot be coalesced
to a first Initial packet because of TX frame allocation failures (from
qc_build_frms()). This leads too short (not padded) Initial packets to be sent.
This is detected by a BUG_ON() in qc_send_ppkts().

To avoid this an Handshake packet without ack-eliciting frames which should have
been built by qc_build_frms() is built.

Must be backported as far as 2.6.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
c78cb49a3b BUG/MEDIUM: quic: Avoid trying to send ACK frames from an empty ack ranges tree
This may happen upon ack ranges allocation failures (from quic_update_ack_ranges_list().
This can lead to empty trees of ack ranges to be used to build ACK frames which
is not good at all. Furthermore this is detected by a BUG_ON() (in qc_do_build_pkt()).

To avoid this, simply update the acknowledgemen state of the connection only if
quic_update_ack_ranges_list() succeeds, as it fails only in case of memory
allocation failures.

Must be backported as far as 2.6.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
4e3b28e8b6 BUG/MEDIUM: quic: Too short Initial packet sent (enc. level allocation failed)
If the Handshake encryption level could not be allocated, this could lead
to Initial packets to be sent because no Handshake CRYPTO frames were generated.

Furthermore in such an allocation failure case, the connection should be closed
as soon as possible. This is done making ha_quic_set_encryption_secrets() return
0 upon an encryption level allocation failure.

Also fix a typo in the trace in relation to this allocation failure.

No need to be backported.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
4cf784f38e MINOR: quic: Avoid zeroing frame structures
Do not initialize anymore ->type of quic_frame structures which leads
to the others to be zeroed.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
f1be725474 CLEANUP: quic: Indentation fix in qc_do_build_pkt()
Modification without any functional impact.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
7ecf4b34b9 BUG/MINOR: quic: idle timer task requeued in the past
When the idle timer expired with a still present mux, this task was not freed
and even requeued with a timer in the past.

Fix this issue calling task_destroy() in this case. As the task is freed,
its handler must return NULL setting local <t> variable to NULL in every cases.

Also ensure that this timer task is not armed again after having been released
with a <return> statement when this is the case from qc_idle_timer_do_rearm().

Must be backported as far as 2.6.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
b48abf0beb MINOR: quic: Add idle timer task pointer to traces
Helpful to detect if this timer was freed or not.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
4cfae3ac01 MINOR: quic: release the TLS context asap from quic_conn_release()
This was no reason not to release as soon as possible the TLS/SSL QUIC connection
context from quic_conn_release() before allocating a "closing connection" connection
(quic_cc_conn struct).
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
3a8dd48e30 MEDIUM: quic: Heavy task mode with non contiguously bufferized CRYPTO data
This patch sets the handshake task in heavy task mode when receiving in disorder
CRYPTO data which results in in order bufferized CRYPTO data. This is done
thanks to a non-contiguous buffer and from qc_handle_crypto_frm() after having
potentially bufferized CRYPTO data in this buffer.
qc_treat_rx_crypto_frms() is no more called from qc_treat_rx_pkts() but instead
this is where the task is set in heavy task mode. Consequently,
this is the job of qc_ssl_provide_all_quic_data() to call directly
qc_treat_rx_crypto_frms() to provide the in order bufferized CRYPTO data to the
TLS stack. As this function releases the non-contiguous buffer for the CRYPTO
data, if possible, there is no need to do that from qc_treat_rx_crypto_frms()
anymore.
2023-11-09 10:32:31 +01:00
Frédéric Lécaille
94d20be138 MEDIUM: quic: Heavy task mode during handshake
Add a new pool for the CRYPTO data frames received in order.
Add ->rx.crypto_frms list to each encryption level to store such frames
when they are received in order from qc_handle_crypto_frm().
Also set the handshake task (qc_conn_io_cb()) in heavy task mode from
this function after having received such frames. When this task
detects that it is set in heavy mode, it calls qc_ssl_provide_all_quic_data()
newly implemented function to provide the CRYPTO data to the TLS task.
Modify quic_conn_enc_level_uninit() to release these CRYPTO frames
when releasing the encryption level they are in relation with.
2023-11-09 10:32:31 +01:00