Commit Graph

7742 Commits

Author SHA1 Message Date
Willy Tarreau
4e65fc66f6 MAJOR: import: update mt_list to support exponential back-off (try #2)
This is the second attempt at importing the updated mt_list code (commit
59459ea3). The previous one was attempted with commit c618ed5ff4 ("MAJOR:
import: update mt_list to support exponential back-off") but revealed
problems with QUIC connections and was reverted.

The problem that was faced was that elements deleted inside an iterator
were no longer reset, and that if they were to be recycled in this form,
they could appear as busy to the next user. This was trivially reproduced
with this:

  $ cat quic-repro.cfg
  global
          stats socket /tmp/sock1 level admin
          stats timeout 1h
          limited-quic

  frontend stats
          mode http
          bind quic4@:8443 ssl crt rsa+dh2048.pem alpn h3
          timeout client 5s
          stats uri /

  $ ./haproxy -db -f quic-repro.cfg  &

  $ h2load -c 10 -n 100000 --npn h3 https://127.0.0.1:8443/
  => hang

This was purely an API issue caused by the simplified usage of the macros
for the iterator. The original version had two backups (one full element
and one pointer) that the user had to take care of, while the new one only
uses one that is transparent for the user. But during removal, the element
still has to be unlocked if it's going to be reused.

All of this sparked discussions with Fred and Aurélien regarding the still
unclear state of locking. It was found that the lock API does too much at
once and is lacking granularity. The new version offers a much more fine-
grained control allowing to selectively lock/unlock an element, a link,
the rest of the list etc.

It was also found that plenty of places just want to free the current
element, or delete it to do anything with it, hence don't need to reset
its pointers (e.g. event_hdl). Finally it appeared obvious that the
root cause of the problem was the unclear usage of the list iterators
themselves because one does not necessarily expect the element to be
presented locked when not needed, which makes the unlock easy to overlook
during reviews.

The updated version of the list presents explicit lock status in the
macro name (_LOCKED or _UNLOCKED suffixes). When using the _LOCKED
suffix, the caller is expected to unlock the element if it intends to
reuse it. At least the status is advertised. The _UNLOCKED variant,
instead, always unlocks it before starting the loop block. This means
it's not necessary to think about unlocking it, though it's obviously
not usable with everything. A few _UNLOCKED were used at obvious places
(i.e. where the element is deleted and freed without any prior check).

Interestingly, the tests performed last year on QUIC forwarding, that
resulted in limited traffic for the original version and higher bit
rate for the new one couldn't be reproduced because since then the QUIC
stack has gaind in efficiency, and the 100 Gbps barrier is now reached
with or without the mt_list update. However the unit tests definitely
show a huge difference, particularly on EPYC platforms where the EBO
provides tremendous CPU savings.

Overall, the following changes are visible from the application code:

  - mt_list_for_each_entry_safe() + 1 back elem + 1 back ptr
    => MT_LIST_FOR_EACH_ENTRY_LOCKED() or MT_LIST_FOR_EACH_ENTRY_UNLOCKED()
       + 1 back elem

  - MT_LIST_DELETE_SAFE() no longer needed in MT_LIST_FOR_EACH_ENTRY_UNLOCKED()
      => just manually set iterator to NULL however.
    For MT_LIST_FOR_EACH_ENTRY_LOCKED()
      => mt_list_unlock_self() (if element going to be reused) + NULL

  - MT_LIST_LOCK_ELT => mt_list_lock_full()
  - MT_LIST_UNLOCK_ELT => mt_list_unlock_full()

  - l = MT_LIST_APPEND_LOCKED(h, e);  MT_LIST_UNLOCK_ELT();
    => l=mt_list_lock_prev(h); mt_list_lock_elem(e); mt_list_unlock_full(e, l)
2024-07-09 16:46:38 +02:00
Amaury Denoyelle
19b8c1b7cd DEV: flags/quic: decode quic_conn flags
Decode quic_conn flags via qc_show_flags() function.

To support this, quic flags definition have been put outside of USE_QUIC
directive.
2024-07-08 09:38:35 +02:00
Amaury Denoyelle
95f624540b BUG/MEDIUM: quic: prevent crash on accept queue full
Handshake for quic_conn instances runs on a single non-chosen thread. On
completion, listener_accept() is performed to select the less loaded
thread before initializing connection instance. As such, quic_conn
instance is migrated to the thread with its upper connection.

In case accept queue is full, listener_accept() fallback to local accept
mode, which cause the connection to be assigned to the current thread.
However, this is not supported by QUIC as quic_conn instance is left on
the previously selected thread. In most cases, this will cause a
BUG_ON() due to a task manipulation from an outside thread.

To fix this, handle quic_conn thread rebind in multiple steps using the
new extended protocol API. Several operations have been moved from
qc_set_tid_affinity1() to newly defined qc_set_tid_affinity2(), in
particular CID TID update. This ensures that quic_conn instance is not
prematurely accessed on the new thread until accept queue push is
guaranteed to succeed.

qc_reset_tid_affinity() is also newly defined to reassign the newly
created tasks and tasklets to the current thread. This is necessary to
prevent the BUG_ON() crash described above.

This must be backported up to 2.8 after a period of observation. Note
that it depends on previous patch :
  MINOR: proto: extend connection thread rebind API
2024-07-04 17:28:56 +02:00
Amaury Denoyelle
1a43b9f32c MINOR: proto: extend connection thread rebind API
MINOR: listener: define callback for accept queue push

Extend API for connection thread rebind API by replacing single callback
set_affinity by three different ones. Each one of them is used at a
different stage of the operation :

* set_affinity1 is used similarly to previous set_affinity

* set_affinity2 is called directly from accept_queue_push_mp() when an
  entry has been found in accept ring. This operation cannot fail.

* reset_affinity is called after set_affinity1 in case of failure from
  accept_queue_push_mp() due to no space left in accept ring. This is
  necessary for protocols which must reconfigure resources before
  fallback on the current tid.

This patch does not have any functional changes. However, it will be
required to fix crashes for QUIC connections when accept queue ring is
full. As such, it must be backported with it.
2024-07-04 16:33:21 +02:00
Valentine Krasnobaeva
41275a6918 MEDIUM: init: set default for fd_hard_limit via DEFAULT_MAXFD
Let's provide a default value for fd_hard_limit, if it's not set in the
configuration. With this patch we could set some specific default via
compile-time variable DEFAULT_MAXFD as well. Hope, this will be helpfull for
haproxy package maintainers.

    make -j 8 TARGET=linux-glibc DEBUG=-DDEFAULT_MAXFD=50000

If haproxy is comipled without DEFAULT_MAXFD defined, the default will be set
to 1048576.

This is done to avoid killing the process by its watchdog, while it started
without any limitations in its configuration or in the command line and the
hard RLIMIT_NOFILE is extremely huge (~1000000000). We use in this case
compute_ideal_maxconn() to calculate maxconn and maxsock, maxsock defines the
size of internal fdtab, which becames very-very large as well. When
the process starts to simply loop over this fdtab (0(n)), this takes a lot of
time, so watchdog does it job.

To avoid this, maxconn now is always reduced to some reasonable value either
by explicit global.fd-hard-limit from configuration, or by its default. The
default may be changed at build-time and overwritten then by
global.fd-hard-limit at runtime. Explicit global.fd-hard-limit from the
configuration has always precedence over DEFAULT_MAXFD, if set.

Must be backported in all stable versions until v2.6.0, including v2.6.0.
2024-07-04 07:52:42 +02:00
Amaury Denoyelle
8550549cca REORG: quic: remove quic_cid_trees reference from proto_quic
Previous commit removed access/manipulation to QUIC CID global tree
outside of quic_cid module. This ensures that proper locking is always
performed.

This commit finalizes this cleanup by marking CID global tree as static
only to quic_cid source file. Initialization of this tree is removed
from proto_quic and now performed using dedicated initcalls
quic_alloc_global_cid_tree().

As a side change, complete CID global tree documentation, in particular
to explain CID global tree artificial splitting and ODCID handling.
Overall, the code is now clearer and safer.
2024-07-03 15:02:40 +02:00
Amaury Denoyelle
0a352ef08e MINOR: quic: remove access to CID global tree outside of quic_cid module
haproxy generates for each QUIC connection a set of CID. The peer must
reuse them as DCID for its emitted packet. On datagram reception, DCID
field serves as identifier to dispatch them on their correct thread.

These CIDs are stored in a global CID tree. Access to this data
structure must always be protected with CID_LOCK. This commit is a
refactoring to regroup all CID tree access in quic_cid module. Several
code parts are ajusted :

* quic_cid_insert() is extended to check for insertion race-condition.
  This is useful on quic_conn instantiation. Code where such race cannot
  happen can use unsafe _quic_cid_insert() instead.

* on RETIRE_CONNECTION_ID frame reception, existing quic_cid_delete()
  function is used.

* remove tree lookup from qc_check_dcid(), extracted in the new
  quic_cmp_cid_conn() function. Ultimately, the latter should be removed
  as CID lookup could be conducted on quic_conn owned tree without
  locking.
2024-07-03 15:02:40 +02:00
Amaury Denoyelle
a05fefe74d CLEANUP: quic: cleanup prototypes related to CIDs handling
Remove duplicated prototypes from quic_conn.h also present in
quic_cid.h. Also remove quic_derive_cid() prototype and mark it as
static.
2024-07-03 15:02:40 +02:00
Amaury Denoyelle
789d4abd73 BUG/MEDIUM: h3: ensure the ":method" pseudo header is totally valid
Ensure pseudo-header method is only constitued of valid characters
according to RFC 9110. If an invalid value is found, the request is
rejected and stream is resetted.

Previously only characters forbidden in headers were rejected (NUL/CR/LF),
but this is insufficient for :method, where some other forbidden chars
might be used to trick a non-compliant backend server into seeing a
different path from the one seen by haproxy. Note that header injection
is not possible though.

This must be backported up to 2.6.

Many thanks to Yuki Mogi of FFRI Security Inc for the detailed report
that allowed to quicky spot, confirm and fix the problem.
2024-06-28 14:36:30 +02:00
Willy Tarreau
290659ffd3 MINOR: activity: make the memory profiling hash size configurable at build time
The MEMPROF_HASH_BITS variable was set to 10 without a possibility to
change it (beyond patching the code). After seeing a few reports already
with "other" being listed and a list with close to 1024 entries, it looks
like it's about time to either increase the hash size, or at least make
it configurable for special cases. As a reminder, in order to remain
fast, the algorithm searches no more than 16 places after the hash, so
when a table is almost full, searches are long and new places are rare.

The present patch just makes it possible to redefine it by passing
"-DMEMPROF_HASH_BITS=11" or "-DMEMPROF_HASH_BITS=12" in CFLAGS, and
moves the definition to defaults.h to make it easier to find. Such
values should be way sufficient for the vast majority of use cases.
Maybe in the future we'd change the default. At least this version
should be backported to ease rebuilds, say, till 2.8 or so.
2024-06-27 18:01:27 +02:00
Valentine Krasnobaeva
5e06d45df7 REORG: init: encapsulate 'reload' sockpair and master CLI listeners creation
Let's encapsulate the logic of 'reload' sockpair and master CLI listeners
creation, used by master CLI into a separate function, as we needed this
only in master-worker runtime  mode. This makes the code of init() more
readable.
2024-06-27 16:08:42 +02:00
Christopher Faulet
ad946a704d MINOR: stick-table: Always decrement ref count before killing a session
Guarded functions to kill a sticky session, stksess_kill()
stksess_kill_if_expired(), may or may not decrement and test its reference
counter before really killing it. This depends on a parameter. If it is set
to non-zero value, the ref count is decremented and if it falls to zero, the
session is killed. Otherwise, if this parameter is equal to zero, the
session is killed, regardless the ref count value.

In the code, these functions are always called with a non-zero parameter and
the ref count is always decremented and tested. So, there is no reason to
still have a special case. Especially because it is not really easy to say
if it is supported or not. Does it mean it is possible to kill a sticky
session while it is still referenced somewhere ? probably not. So, does it
mean it is possible to kill a unreferenced session ? This case may be
problematic because the session is accessed outside of any lock and thus may
be released by another thread because it is unreferenced. Enlarging scope of
the lock to avoid any issue is possible but it is a bit of shame to do so
because there is no usage for now.

The best is to simplify the API and remove this case. Now, stksess_kill()
and stksess_kill_if_expired() functions always decrement and test the ref
count before killing a sticky session.
2024-06-26 15:05:06 +02:00
Christopher Faulet
9357873641 BUG/MEDIUM: stick-table: Decrement the ref count inside lock to kill a session
When we try to kill a session, the shard must be locked before decrementing
the ref count on the session. Otherwise, the ref count can fall to 0 and a
purge task (stktable_trash_oldest or process_table_expire) may release the
session before we have the opportunity to acquire the lock on the shard to
effectively kill the session. This could lead to a double free.

Here is the scenario:

    Thread 1                                 Thread 2

  sktsess_kill(ts)
    if (ATOMIC_DEC(&ts->ref_cnt) != 0)
        return
                   /* here the ref count is 0 */

                                       stktable_trash_oldest()
                                          LOCK(&sh_lock)
                                          if (!ATOMIC_LOAD(&ts->ref_cnf))
                                              __stksess_free(ts)
                                          UNLOCK(&sh_lock)

                  /* here the session was released */
    LOCK(&sh_lock)
    __stksess_free(ts)  <--- double free
    UNLOCK(&sh_lock)

The bug was introduced in 2.9 by the commit 7968fe3889 ("MEDIUM:
stick-table: change the ref_cnt atomically"). The ref count must be
decremented inside the lock for stksess_kill() and sktsess_kill_if_expired()
function.

This patch should fix the issue #2611. It must be backported as far as 2.9. On
the 2.9, there is no sharding. All the table is locked. The patch will have to
be adapted.
2024-06-26 12:05:37 +02:00
Frederic Lecaille
bc9821fd26 BUILD: Missing inclusion header for ssize_t type
Compilation issue detected as follows by gcc:

In file included from src/ncbuf.c:19:
src/ncbuf.c: In function 'ncb_write_off':
include/haproxy/bug.h:144:10: error: unknown type name 'ssize_t'
  144 |   extern ssize_t write(int, const void *, size_t); \
2024-06-26 10:17:09 +02:00
Willy Tarreau
2d27c80288 BUILD: debug: also declare strlen() in __ABORT_NOW()
Previous commit 8f204fa8ae ("MINOR: debug: print gdb hints when crashing")
broken on the CI where strlen() isn't known. Let's forward-declare it in
the __ABORT_NOW() functions, just like write(). No backport is needed.
2024-06-26 08:04:40 +02:00
Willy Tarreau
8f204fa8ae MINOR: debug: print gdb hints when crashing
To make bug reporting easier for users, when crashing, let's suggest
what to do. Typically when a BUG_ON() matches, only the current thread
is useful the vast majority of the time, while when the watchdog
triggers, all threads are interesting.

The messages are printed at the end after the dump. We may adjust these
with wiki links in the future is more detailed instructions are relevant.
2024-06-26 07:43:00 +02:00
Valentine Krasnobaeva
2cd52a88be MINOR: cli/debug: show dev: show capabilities
If haproxy compiled with Linux capabilities support, let's show process
capabilities before applying the configuration and at runtime in 'show dev'
command output. This maybe useful for debugging purposes. Especially in
cases, when process changes its UID and GID to non-priviledged or it
has started and run under non-priviledged UID and needed capabilities are
set by admin on the haproxy binary.
2024-06-26 07:38:21 +02:00
Valentine Krasnobaeva
0d79c9bedf MINOR: cli/debug: show dev: add cmdline and version
'show dev' command is very convenient to obtain haproxy debugging information,
while process is run in container. Let's extend its output with version and
cmdline. cmdline is useful in a way, as it shows absolute binary path and its
arguments, because sometimes the person, who is debugging failing container is
not the same, who has created and deployed it.

argc and argv are stored in the exported global structure, because
feed_post_mortem() is added as a post check function callback in the
post_check_list. So we can't simply change the signature of
feed_post_mortem(), without breaking other post check callbacks APIs.

Parsers are not supposed to modify argv, so we can safely bypass its pointer
to debug_parse_cli_show_dev(), without copying all argument stings somewhere
in the heap or on stack.
2024-06-26 07:38:21 +02:00
Valentine Krasnobaeva
fcf1a0bcf5 MINOR: capabilities: export capget and __user_cap_header_struct
To be able to show process capabilities before applying its configuration and
also at runtime in 'show dev' command output, we need to export the wrapper
around capget() syscall. It also seems more handy to place
__user_cap_header_struct in .data section and declare it as globally
accessible, as we always fill it with the same values. This avoids allocate
and fill these 8 bytes each time on the stack frame, when capget() or capset()
wrappers are called.
2024-06-26 07:38:21 +02:00
Aurelien DARRAGON
9d312212df BUG/MINOR: proxy: fix email-alert leak on deinit() (2nd try)
As shown in GH #2608 and ("BUG/MEDIUM: proxy: fix email-alert invalid
free"), simply calling free_email_alert() from free_proxy() is not the
right thing to do.

In this patch, we reuse proxy->email_alert.set memory space to introduce
proxy->email_alert.flags in order to support 2 flags:
PR_EMAIL_ALERT_SET (to mimic proxy->email_alert.set) and
PR_EMAIL_ALERT_RESOLVED (set once init_email_alert() was called on the
proxy to resolve email_alert.mailer pointer).

Thanks to PR_EMAIL_ALERT_RESOLVED flag, free_email_alert() may now
properly handle the freeing of proxy email_alert settings: if the RESOLVED
flag is set, then it means the .email_alert.mailers.name parsing hint was
replaced by the actual mailers pointer, thus no free should be attempted.

No backport needed: as described in ("BUG/MEDIUM: proxy: fix email-alert
invalid free"), this historical leak is not sensitive as it cannot be
triggered during runtime.. thus given that the fix is not backport-
friendly, it's not worth the trouble.
2024-06-17 19:37:29 +02:00
Aurelien DARRAGON
ee8be55942 REORG: mailers: move free_email_alert() to mailers.c
free_email_alert() was declared in cfgparse.c, but it should belong to
mailers.c instead.
2024-06-17 19:37:29 +02:00
William Lallemand
30a432d198 MINOR: ssl: activate sigalgs feature for AWS-LC
AWSLC lacks the SSL_CTX_set1_sigalgs_list define, however the function
exists, which disables the feature in HAProxy, even if we could have
build with it.

SSL_CTX_set1_client_sigalgs_list() is not available, though.

This patch introduce the define so the feature is enabled.
2024-06-17 17:40:49 +02:00
Aurelien DARRAGON
983513d901 DEBUG: hlua: distinguish burst timeout errors from exec timeout errors
hlua burst timeout was introduced in 58e36e5b1 ("MEDIUM: hlua: introduce
tune.lua.burst-timeout").

It is a safety measure that allows to detect when too much time is spent
on a single lua execution (between 2 interruptions/yields), meaning that
the current thread is not able to perform other tasks. Such scenario
should be avoided because it will cause thread contention which may have
negative performance impact and could cause the watchdog to trigger. When
the burst timeout is exceeded, the current Lua execution is aborted and a
timeout error is reported to the user.

Unfortunately, the same error is currently being reported for cumulative
(AKA execution) timeout and for burst timeout, which may be confusing to
the user.

Indeed, "execution timeout" error historically results from the current
hlua context exceeding the total (cumulative) time it's allowed to run.
It is set per lua context using the dedicated tunables:
 - tune.lua.session-timeout
 - tune.lua.task-timeout
 - tune.lua.service-timeout

We've already faced an user report where the user was able to trigger the
burst timeout and got "Lua task: execution timeout." error while the user
didn't set cumulative timeout. Thus the error was actually confusing
because it was indeed the burst timeout which was causing it due to the
use of cpu-intensive call from within the task without sufficient manual
"yield" keypoints around the cpu-intensive call to ensure it runs on a
dedicated scheduler cycle.

In this patch we make it so burst timeout related errors are reported as
"burst timeout" errors instead of "execution timeout" errors (which
in fact became the generic timeout errors catchall with 58e36e5b1).

To do this, hlua_timer_check() now returns a different value depending if
the exeeded timeout is the burst one or the cumulative one, which allows
us to return either HLUA_E_ETMOUT or HLUA_E_BTMOUT in hlua_ctx_resume().

It should improve the situation described in GH #2356 and may possibly be
backported with 58e36e5b1 to improve error reporting if it applies without
resistance.
2024-06-14 18:25:58 +02:00
William Lallemand
ee5aa4e5e6 BUILD: ssl: disable deprecated functions for AWS-LC 1.29.0
AWS-LC have a lot of functions that does nothing, which are now
deprecated and emits some warning.

This patch disables the following useless functions that emits a warning:
SSL_CTX_get_security_level(), SSL_CTX_set_tmp_dh_callback(),
ERR_load_SSL_strings(), RAND_keep_random_devices_open()

The list of deprecated functions is here:

https://github.com/aws/aws-lc/blob/main/docs/porting/functionality-differences.md
2024-06-14 10:41:36 +02:00
William Lallemand
7120c77b14 MEDIUM: ssl: support for ECDA+RSA certificate selection with AWS-LC
AWS-LC does not support the SSL_CTX_set_client_hello_cb() function from
OpenSSL which allows to analyze ciphers and signatures algorithm of the
ClientHello. However it supports the SSL_CTX_set_select_certificate_cb()
which allows the same thing but was the implementation from the
boringSSL side.

This patch uses the SSL_CTX_set_select_certificate_cb() as well as the
SSL_early_callback_ctx_extension_get() function to get the signature
algorithms.

This was successfully tested with openssl s_client as well as
testssl.sh.

This should allow to enable more reg-tests that depend on certificate
selection.

Require at least AWS-LC 1.22.0.
2024-06-13 19:36:40 +02:00
William Lallemand
5149cc4990 BUILD: ssl: fix build with wolfSSL
fix build with wolfSSL, broken since the reorg in src/ssl_clienthello.c
2024-06-13 17:01:45 +02:00
William Lallemand
4ced880d22 REORG: ssl: move the SNI selection code in ssl_clienthello.c
Move the code which is used to select the final certificate with the
clienthello callback. ssl_sock_client_sni_pool need to be exposed from
outside ssl_sock.c
2024-06-13 16:48:17 +02:00
William Lallemand
fc7c5d892b MINOR: ssl: add ssl_sock_bind_verifycbk() in ssl_sock.h
Add missing ssl_sock_bind_verifycbk() in ssl_sock.h
2024-06-13 16:48:17 +02:00
Aurelien DARRAGON
15e9c7da6b MINOR: log: add log-profile parsing logic
This patch implements prerequisite log-profile struct and parser logic.
It has no effect during runtime for now.

Logformat expressions provided in log-profile "steps" are postchecked
during postparsing for each proxy "log" directive that makes use of a
given profile. (this allows to ensure that the logformat expressions
used in the profile are compatible with proxy using them)
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
33f3bec7ee MINOR: log: add logger flags
Logger struct may benefit from having a "flags" struct member to set
or remove different logger states. For that, we reuse an existing
4 bytes hole in the logger struct to store a 2 bytes flags integer,
leaving the struct with a 2-bytes hole now.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
3102c89dde MINOR: log: provide proxy context to resolve_logger()
Prerequisite work for log-profiles, we need to know under which proxy
context the logger is being used. When the info is not available, (ie:
global section or log-forward section, <px> is set to NULL)
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
8f34320e15 MINOR: log: provide log origin in logformat expressions using '%OG'
'%OG' logformat alias may be used to report the log origin (when/where)
that triggered log generation using sess_build_logline().

Possible values are:
  - "sess_error": log was generated during session error handling
  - "sess_killed": log was generated during session abortion (killed
    embryonic session)
  - "txn_accept": log was generated right after frontend conn was accepted
  - "txn_request": log was generated after client request was received
  - "txn_connect": log was generated after backend connection establishment
  - "txn_response": log was generated during server response handling
  - "txn_close": log was generated at the final txn step, before closing
  - "unspec": unknown or not specified

Documentation was updated.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
b52862d401 MINOR: log: add log_orig_to_str() function
Get human readable string from log_orig enum members.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
2a91bd52ad MINOR: log: provide sending log context to process_send_log() when available
This is another prerequisite work in preparation for log-profiles: in this
patch we make process_send_log() aware of the log origin, primarily aiming
for sess and txn logging steps such as error, accept, connect, close, as
well as relevant sess and stream pointers.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
0b7a5a64eb MEDIUM: log/session: handle embryonic session log within sess_log()
Move the embryonic session logging logic down to sess_log() in preparation
for log-profiles because then log preferences will be set per logger and
not per proxy. Indeed, as each logger may come with its own log-profile
that possibly overrides proxy logformat preferences, the check will need
to be performed at a central place by lower sending functions.

To ensure the change doesn't break existing behavior, a dedicated
sess_log_embryonic() wrapper was added and is exclusively used by
session_kill_embryonic() to indicate that a special logging logic must
be performed under sess_log().

Also, thanks to this change, log-format-sd will now be taken into account
for legacy embryonic session logging.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
79a0a7b4d8 MINOR: session: expose session_embryonic_build_legacy_err() function
rename session_build_err_string() to session_embryonic_build_legacy_err()
and add new <out> buffer argument to the prototype. <out> will be used as
destination for the generated string instead of implicitly relying on the
trash buffer. Finally, expose the new function through the header file so
that it becomes usable from any source file.

The function is expected to be called with a session originating from
a connection and should not be used for applets.
2024-06-13 15:43:09 +02:00
Aurelien DARRAGON
ee288a4eef REORG: log: reorder send log helpers by dependency order
This commit looks messy, but all it does is reorganize send_log() helpers
by dependency order to remove the need of forward-declaring some of them.

Also, since they're all internal helpers, let's explicitly mark them as
static to prevent any misuse.
2024-06-13 15:43:09 +02:00
Amaury Denoyelle
88681681cc MINOR: quic: refactor qc_build_pkt() error handling
qc_build_pkt() error handling was difficult due to multiple error code
possible. Improve this by defining a proper enum to describe the various
error code. Also clean up ending labels inside qc_build_pkt().
2024-06-12 18:05:40 +02:00
Christopher Faulet
9748df29ff BUG/MEDIUM: mux-quic: Don't unblock zero-copy fwding if blocked during nego
The previous fix (792a645ec2 ["BUG/MEDIUM: mux-quic: Unblock zero-copy
forwarding if the txbuf can be released"]) introduced a regression. The
zero-copy data forwarding must only be unblocked if it was blocked by the
producer, after a successful negotiation.

It is important because during a negotiation, the consumer may be blocked
for another reason. Because of the flow control for instance. In that case,
there is not necessarily a TX buffer. And it unexpected to try to release an
unallocated TX buf.

In addition, the same may happen while a TX buf is still in-use. In that
case, it must also not be released. So testing the TX buffer is not the
right solution.

To fix the issue, a new IOBUF flag was added (IOBUF_FL_FF_WANT_ROOM). It
must be set by the producer if it is blocked after a sucessful negotiation
because it needs more room. In that case, we know a buffer was provided by
the consummer. In done_fastfwd() callback function, it is then possible to
safely unblock the zero-copy data forwarding if this flag is set.

This patch must be backported to 3.0 with the commit above.
2024-06-05 07:28:10 +02:00
Willy Tarreau
1eb0f22ee1 [RELEASE] Released version 3.1-dev0
Released version 3.1-dev0 with the following main changes :
    - MINOR: version: mention that it's development again
2024-05-29 15:00:02 +02:00
Willy Tarreau
555772e961 MINOR: version: mention that it's development again
This essentially reverts 2e42a19cde.
2024-05-29 14:59:19 +02:00
Willy Tarreau
2e42a19cde MINOR: version: mention that it's 3.0 LTS now.
The version will be maintained up to around Q2 2029. Let's
also update the INSTALL file to mention this.
2024-05-29 14:40:26 +02:00
Willy Tarreau
decb7c90df CLEANUP: ssl_sock: move dirty openssl-1.0.2 wrapper to openssl-compat
Valentine noticed this ugly SSL_CTX_get_tlsext_status_cb() macro
definition inside ssl_sock.c that is dedicated to openssl-1.0.2 only.
It would be better placed in openssl-compat.h, which is what this
patch does. It also addresses a missing pair of parenthesis and
removes an invalid extra semicolon.
2024-05-28 19:17:57 +02:00
Aurelien DARRAGON
435a9da267 MINOR: log: rename 'log-format tag' to 'log-format alias'
In 2.9 we started to introduce an ambiguity in the documentation by
referring to historical log-format variables ('%var') as log-format
tags in 739c4e5b1e ("MINOR: sample: accept_date / request_date return
%Ts / %tr timestamp values") and 454c372b60 ("DOC: configuration: add
sample fetches for timing events").

In fact, we've had this confusion between log-format tag and log-format
var for more than 10 years now, but in 2.9 it was the first time the
confusion was exposed in the documentation.

Indeed, both 'log-format variable' and 'log-format tag' actually refer
to the same feature (that is: '%B' and friends that can be used for
direct access to some log-oriented predefined fetches instead of using
%[expr] with generic sample expressions).

This feature was first implemented in 723b73ad75 ("MINOR: config: Parse
the string of the log-format config keyword") and later documented in
4894040fa ("DOC: log-format documentation"). At that time, it was clear
that we used to name it 'log-format variable'.

But later the same year, 'log-format tag' naming started to appear in
some commit messages (while still referring to the same feature), for
instance with ffc3fcd6d ("MEDIUM: log: report SSL ciphers and version
in logs using logformat %sslc/%sslv").

Unfortunately in 2.9 when we added (and documented) new log-format
variables we officially started drifting to the misleading 'log-format
tag' naming (perhaps because it was the most recent naming found for
this feature in git log history, or because the confusion has always
been there)

Even worse, in 3.0 this confusion led us to rename all 'var' occurrences
to 'tag' in log-format related code to unify the code with the doc.

Hopefully William quickly noticed that we made a mistake there, but
instead of reverting to historical naming (log-format variable), it was
decided that we must use a different name that is less confusing than
'tags' or 'variables' (tags and variables are keywords that are already
used to designate other features in the code and that are not very
explicit under log-format context today).

Now we refer to '%B' and friends as a logformat alias, which is
essentially a handy way to print some log oriented information in the
log string instead of leveraging '%[expr]' with generic sample expressions
made of fetches and converters. Of course, there are some subtelties, such
as a few log-format aliases that still don't have sample fetch equivalent
for historical reasons, and some aliases that may be a little faster than
their generic sample expression equivalents because most aliases are
pretty much hardcoded in the log building function. But in general
logformat aliases should be simply considered as an alternative to using
expressions (with '%[expr']')

Also, under log-format context, when we want to refer to either an alias
('%alias') or an expression ('%[expr]'), we should use the generic term
'logformat item', which in fact designates a single item within the
logformat string provided by the user. Indeed, a logformat item (whether
is is an alias or an expression) always starts with '%' and may accept
optional flags / arguments

Both the code and the documentation were updated in that sense, hopefully
this will clarify things and prevent future confusions.
2024-05-27 17:03:48 +02:00
Amaury Denoyelle
47168e217a MEDIUM: connection: use pool-conn-name instead of sni on reuse
Implement pool-conn-name support for idle connection reuse. It replaces
SNI as arbitrary identifier for connections in the idle pool. Thus,
every SNI reference in this context have been replaced.

Main change occurs in connect_server() where pool-conn-name sample fetch
is now prehash to generate idle connection identifier. SNI is now solely
used in the context of SSL for ssl_sock_set_servername().
2024-05-24 14:47:21 +02:00
Amaury Denoyelle
be4f89f2b2 MINOR: server: define pool-conn-name keyword
Define a new server keyword pool-conn-name. The purpose of this keyword
will be to identify connections inside the idle connections pool,
replacing SNI in case SSL is not wanted.

This keyword uses a sample expression argument. It thus can reuse
existing function parse_srv_expr() for parsing. In the future, it may be
necessary to define a keyword variant which uses a logformat for
extensability.

This patch only implement parsing. Argument is stored inside new server
field <pool_conn_name> and expression is generated in
_srv_parse_finalize() into <pool_conn_name_expr>.

If pool-conn-name is not set but SNI is, the latter is reused
automatically as pool-conn-name via _srv_parse_finalize(). This ensures
current reuse behavior remains compatible and idle connection reuse will
not mix connections with different SNIs by mistake.

Main usage will be for rhttp when SSL is not wanted between the two
haproxy instances. Previously, it was possible to use "sni" keyword even
without SSL on a server line which have a similar effect. However,
having a dedicated "pool-conn-name" keyword is deemed clearer. Besides,
it would allow for more complex configuration where pool-conn-name and
SNI are use in parallel with different values.
2024-05-24 14:36:31 +02:00
Amaury Denoyelle
91001422b4 MINOR: server: generalize sni expr parsing
Two functions exists for server sni sample expression parsing. This is
confusing so this commit aims at clarifying this.

Functions are renamed with the following identifiers. First function is
named parse_srv_expr() and can be used during parsing. Besides
expression parsing, it has ensure sample fetch validity in the context
of a server line.

Second function is renamed _parse_srv_expr() and is used internally by
parse_srv_expr(). It only implements sample parsing without extra
checks. It is already use for server instantiation derived from
server-template as checks were already performed. Also, it is now used
in http-client code as SNI is a fixed string.

Finally, both functions are generalized to remove any reference to SNI.
This will allow to reuse it to parse other server keywords which use an
expression. This will be the case for the future keyword pool-conn-name.
2024-05-24 14:36:31 +02:00
Amaury Denoyelle
5764bc50b5 BUG/MINOR: quic: adjust restriction for stateless reset emission
Review RFC 9000 and ensure restriction on Stateless reset are properly
enforced. After careful examination, several changes are introduced.

First, redefine minimal Stateless Reset emitted packet length to 21
bytes (5 random bytes + a token). This is the new default length used in
every case, unless received packet which triggered it is 43 bytes or
smaller.

Ensure every Stateless Reset packets emitted are at 1 byte shorter than
the received packet which triggered it. No Stateless reset will be
emitted if this falls under the above limit of 21 bytes. Thus this
should prevent looping issues.

This should be backported up to 2.6.
2024-05-24 14:36:31 +02:00
Amaury Denoyelle
45f40bac4c MEDIUM: config: prevent communication with privileged ports
This commit introduces a new global setting named
harden.reject_privileged_ports.{tcp|quic}. When active, communications
with clients which use privileged source ports are forbidden. Such
behavior is considered suspicious as it can be used as spoofing or
DNS/NTP amplication attack.

Value is configured per transport protocol. For each TCP and QUIC
distinct code locations are impacted by this setting. The first one is
in sock_accept_conn() which acts as a filter for all TCP based
communications just after accept() returns a new connection. The second
one is dedicated for QUIC communication in quic_recv(). In both cases,
if a privileged source port is used and setting is disabled, received
message is silently dropped.

By default, protection are disabled for both protocols. This is to be
able to backport it without breaking changes on stable release.

This should be backported as it is an interesting security feature yet
relatively simple to implement.
2024-05-24 14:36:31 +02:00
Aurelien DARRAGON
9d37c4b989 DEBUG: tools: add vma_set_name_id() helper
Just like vma_set_name() from 51a8f134e ("DEBUG: tools: add vma_set_name()
helper"), but also takes <id> as parameter to append "-$id" suffix after
the name in order to differentiate 2 areas that were named using the same
<type> and <name> combination.

example, using mmap + MAP_SHARED|MAP_ANONYMOUS:
  7364c4fff000-736508000000 rw-s 00000000 00:01 3540  [anon_shmem:type:name-id]
Another example, using mmap + MAP_PRIVATE|MAP_ANONYMOUS or using
glibc/malloc() above MMAP_THRESHOLD:
  7364c4fff000-736508000000 rw-s 00000000 00:01 3540  [anon:type:name-id]
2024-05-24 12:07:13 +02:00