]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoBUG/MEDIUM: stick-table: fix a race condition when updating the expiration task
Willy Tarreau [Mon, 14 Nov 2022 17:02:44 +0000 (18:02 +0100)] 
BUG/MEDIUM: stick-table: fix a race condition when updating the expiration task

Pierre Cheynier reported a rare crash that can affect stick-tables. When
a entry is created, the stick-table's expiration date is updated. But if
at exactly the same time the expiration task runs, it finishes by updating
its expiration timer without any protection, which may collide with the
call to task_queue() in another thread. In this case, it sometimes happens
that the first test for TICK_ETERNITY in task_queue() passes, then the
"expire" field is reset, then the BUG_ON() triggers, like below:

  FATAL: bug condition "task->expire == 0" matched at src/task.c:279
    call trace(13):
    |       0x649d86 [c6 04 25 01 00 00 00 00]: __task_queue+0xc6/0xce
    |       0x596bef [eb 90 ba 03 00 00 00 be]: stktable_requeue_exp+0x1ef/0x258
    |       0x596c87 [48 83 bb 90 00 00 00 00]: stktable_touch_with_exp+0x27/0x312
    |       0x563698 [48 8b 4c 24 18 4c 8b 4c]: stream_process_counters+0x3a8/0x6a2
    |       0x569344 [49 8b 87 f8 00 00 00 48]: process_stream+0x3964/0x3b4f
    |       0x64a80b [49 89 c7 e9 23 ff ff ff]: run_tasks_from_lists+0x3ab/0x566
    |       0x64ad66 [29 44 24 14 8b 7c 24 14]: process_runnable_tasks+0x396/0x71e
    |       0x6184b2 [83 3d 47 b3 a6 00 01 0f]: run_poll_loop+0x92/0x4ff
    |       0x618acf [48 8b 1d aa 20 7d 00 48]: main+0x1877ef
    | 0x7fc7d6ec1e45 [64 48 89 04 25 30 06 00]: libpthread:+0x7e45
    | 0x7fc7d6c9e4af [48 89 c7 b8 3c 00 00 00]: libc:clone+0x3f/0x5a

This one is extremely difficult to reproduce in practice, but adding a
printf() in process_table_expire() before assigning the value, while
running with an expire delay of 1ms helps a lot and may trigger the
crash in less than one minute on a 8-thread machine. Interestingly,
depending on the sequencing, this bug could also have made a table fail
to expire if the expire field got reset after the last update but before
the call to task_queue(). It would require to be quite unlucky so that
the table is never touched anymore after the race though.

The solution taken by this patch is to take the table's lock when
updating its expire value in stktable_requeue_exp(), enclosing the call
to task_queue(), and to update the task->expire field while still under
the lock in process_table_expire(). Note that thanks to previous changes,
taking the table's lock for the update in stktable_requeue_exp() costs
almost nothing since we now have the guarantee that this is not done more
than 1000 times a second.

Since process_table_expire() sets the timeout after returning from
stktable_trash_expired() which just released the lock, the two functions
were merged so that the task's expire field is updated while still under
the lock. Note that this heavily depends on the two previous patches
below:

  CLEANUP: stick-table: remove the unused table->exp_next
  OPTIM: stick-table: avoid atomic ops in stktable_requeue_exp() when possible

This is a bit complicated due to the fact that in 2.7 some parts were
made lockless. In 2.6 and older, the second part (the merge of the
two functions) will be sufficient since the task_queue() call was
already performed under the table's lock, and the patches above are
not needed.

This needs to be backported as far as 1.8 scrupulously following
instructions above.

2 years agoOPTIM: stick-table: avoid atomic ops in stktable_requeue_exp() when possible
Willy Tarreau [Mon, 14 Nov 2022 16:54:07 +0000 (17:54 +0100)] 
OPTIM: stick-table: avoid atomic ops in stktable_requeue_exp() when possible

Since the task's time resolution is the millisecond we know there
will not be more than 1000 useful updates per second, so there's no
point in doing a CAS and a task_queue() for each call, better first
check if we're going to change the date. Now we're certain not to
perform such operations more than 1000 times a second for a given
table.

The loop was modified because this improvement will also be used to fix a
bug later.

2 years agoCLEANUP: stick-table: remove the unused table->exp_next
Willy Tarreau [Mon, 14 Nov 2022 16:33:02 +0000 (17:33 +0100)] 
CLEANUP: stick-table: remove the unused table->exp_next

The ->exp_next field of the stick-table was probably useful in 1.5 but
it currently only carries a copy of what the future value of the table's
task's expire value will be, while it's systematically copied over there
immediately after being assigned. As such it provides exactly a local
variable. Let's remove it, as it costs atomic operations.

2 years agoBUG/MINOR: ssl: Fix potential overflow
Remi Tricot-Le Breton [Mon, 14 Nov 2022 14:15:52 +0000 (15:15 +0100)] 
BUG/MINOR: ssl: Fix potential overflow

Coverity raised a potential overflow issue in these new functions that
work on unsigned long long objects. They were added in commit 9b25982
"BUG/MEDIUM: ssl: Verify error codes can exceed 63".

This patch needs to be backported alongside 9b25982.

2 years agoBUG/MINOR: ssl: crt-ignore-err memory leak with 'all' parameter
William Lallemand [Mon, 14 Nov 2022 10:36:11 +0000 (11:36 +0100)] 
BUG/MINOR: ssl:  crt-ignore-err memory leak with 'all' parameter

Only allocate "str" if the parameter is not "all" in order to avoid any
memory leak.

No backport needed.

2 years agoCLEANUP: ssl: remove printf in bind_parse_ignore_err
William Lallemand [Mon, 14 Nov 2022 10:34:07 +0000 (11:34 +0100)] 
CLEANUP: ssl: remove printf in bind_parse_ignore_err

Remove debug printf.

No backport needed.

2 years agoBUILD: prometheus: use __fallthrough in promex_dump_metrics() and IO handler()
Willy Tarreau [Mon, 14 Nov 2022 06:37:45 +0000 (07:37 +0100)] 
BUILD: prometheus: use __fallthrough in promex_dump_metrics() and IO handler()

This avoids 11 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: stconn: use __fallthrough in various shutw() functions
Willy Tarreau [Mon, 14 Nov 2022 06:36:42 +0000 (07:36 +0100)] 
BUILD: stconn: use __fallthrough in various shutw() functions

This avoids 7 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: compression: use __fallthrough in comp_http_payload()
Willy Tarreau [Mon, 14 Nov 2022 06:36:05 +0000 (07:36 +0100)] 
BUILD: compression: use __fallthrough in comp_http_payload()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: map: use __fallthrough in cli_io_handler_*()
Willy Tarreau [Mon, 14 Nov 2022 06:35:24 +0000 (07:35 +0100)] 
BUILD: map: use __fallthrough in cli_io_handler_*()

This avoids 6 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: vars: use __fallthrough in var_accounting_{diff,add}()
Willy Tarreau [Mon, 14 Nov 2022 06:33:48 +0000 (07:33 +0100)] 
BUILD: vars: use __fallthrough in var_accounting_{diff,add}()

This avoids 6 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: h1_htx: use __fallthrough in h1_parse_chunk()
Willy Tarreau [Mon, 14 Nov 2022 06:32:50 +0000 (07:32 +0100)] 
BUILD: h1_htx: use __fallthrough in h1_parse_chunk()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: http_act: use __fallthrough in parse_http_del_header()
Willy Tarreau [Mon, 14 Nov 2022 06:32:04 +0000 (07:32 +0100)] 
BUILD: http_act: use __fallthrough in parse_http_del_header()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: check: use __fallthrough in __health_adjust()
Willy Tarreau [Mon, 14 Nov 2022 06:31:36 +0000 (07:31 +0100)] 
BUILD: check: use __fallthrough in __health_adjust()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: logs: use __fallthrough in build_log_header()
Willy Tarreau [Mon, 14 Nov 2022 06:22:57 +0000 (07:22 +0100)] 
BUILD: logs: use __fallthrough in build_log_header()

This avoids 4 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: spoe: use __fallthrough in spoe_handle_appctx()
Willy Tarreau [Mon, 14 Nov 2022 06:22:14 +0000 (07:22 +0100)] 
BUILD: spoe: use __fallthrough in spoe_handle_appctx()

This avoids two build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: acl: use __fallthrough in parse_acl_expr()
Willy Tarreau [Mon, 14 Nov 2022 06:21:22 +0000 (07:21 +0100)] 
BUILD: acl: use __fallthrough in parse_acl_expr()

This avoids two build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: args: use __fallthrough in make_arg_list()
Willy Tarreau [Mon, 14 Nov 2022 06:20:54 +0000 (07:20 +0100)] 
BUILD: args: use __fallthrough in make_arg_list()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: tools: use __fallthrough in url_decode()
Willy Tarreau [Mon, 14 Nov 2022 06:20:09 +0000 (07:20 +0100)] 
BUILD: tools: use __fallthrough in url_decode()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: hash: use __fallthrough in hash_djb2()
Willy Tarreau [Mon, 14 Nov 2022 06:18:42 +0000 (07:18 +0100)] 
BUILD: hash: use __fallthrough in hash_djb2()

This avoids 5 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: peers: use __fallthrough in peer_io_handler()
Willy Tarreau [Mon, 14 Nov 2022 06:13:22 +0000 (07:13 +0100)] 
BUILD: peers: use __fallthrough in peer_io_handler()

This avoids 7 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: stats: use __fallthrough in stats_dump_proxy_to_buffer()
Willy Tarreau [Mon, 14 Nov 2022 06:12:05 +0000 (07:12 +0100)] 
BUILD: stats: use __fallthrough in stats_dump_proxy_to_buffer()

This avoids 12 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: tcpcheck: use __fallthrough in check_proxy_tcpcheck()
Willy Tarreau [Mon, 14 Nov 2022 06:10:33 +0000 (07:10 +0100)] 
BUILD: tcpcheck: use __fallthrough in check_proxy_tcpcheck()

This avoids 1 build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: stream: use __fallthrough in stats_dump_full_strm_to_buffer()
Willy Tarreau [Mon, 14 Nov 2022 06:09:39 +0000 (07:09 +0100)] 
BUILD: stream: use __fallthrough in stats_dump_full_strm_to_buffer()

This avoids 1 build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: hlua: use __fallthrough in hlua_post_init_state()
Willy Tarreau [Mon, 14 Nov 2022 06:08:28 +0000 (07:08 +0100)] 
BUILD: hlua: use __fallthrough in hlua_post_init_state()

This avoids 5 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: ssl: use __fallthrough in cli_io_handler_tlskeys_files()
Willy Tarreau [Mon, 14 Nov 2022 06:34:43 +0000 (07:34 +0100)] 
BUILD: ssl: use __fallthrough in cli_io_handler_tlskeys_files()

This avoids two build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: ssl: use __fallthrough in cli_io_handler_commit_{cert,cafile_crlfile}()
Willy Tarreau [Mon, 14 Nov 2022 06:05:31 +0000 (07:05 +0100)] 
BUILD: ssl: use __fallthrough in cli_io_handler_commit_{cert,cafile_crlfile}()

This avoids 8 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: ssl/crt-list: use __fallthrough in cli_io_handler_add_crtlist()
Willy Tarreau [Mon, 14 Nov 2022 06:03:16 +0000 (07:03 +0100)] 
BUILD: ssl/crt-list: use __fallthrough in cli_io_handler_add_crtlist()

This avoids 3 build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: quic: use __fallthrough in quic_connect_server()
Willy Tarreau [Mon, 14 Nov 2022 06:02:00 +0000 (07:02 +0100)] 
BUILD: quic: use __fallthrough in quic_connect_server()

This avoids one build warning when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: sample: use __fallthrough in smp_is_rw() and smp_dup()
Willy Tarreau [Mon, 14 Nov 2022 05:59:59 +0000 (06:59 +0100)] 
BUILD: sample: use __fallthrough in smp_is_rw() and smp_dup()

This avoids three build warnings when preprocessing happens before compiling
with gcc >= 7.

2 years agoBUILD: compiler: define a __fallthrough statement for switch/case
Willy Tarreau [Sun, 13 Nov 2022 11:21:22 +0000 (12:21 +0100)] 
BUILD: compiler: define a __fallthrough statement for switch/case

When the code is preprocessed first and compiled later, such as when
built under distcc, a lot of fallthrough warnings are emitted because
the preprocessor has already stripped the comments.

As an alternative, a "fallthrough" attribute was added with the same
compilers as those which started to emit those warnings. However it's
not portable to older compilers. Let's just define a __fallthrough
statement that corresponds to this attribute on supported compilers
and only switches to the classical empty do {} while (0) on other ones.

This way the code will support being cleaned up using __fallthrough.

2 years agoBUILD: compiler: add a default definition for __has_attribute()
Willy Tarreau [Sun, 13 Nov 2022 10:39:18 +0000 (11:39 +0100)] 
BUILD: compiler: add a default definition for __has_attribute()

It happens that gcc since 5.x has this macro which is only mentioned
once in the doc, associated with __builtin_has_attribute(). Clang had
it at least since 3.0. In addition it validates #ifdef when present,
so it's easy to detect it. Here we're providing a fallback to another
macro __has_attribute_<name> so that it's possible to define that macro
to the value 1 for older compilers when the attribute is supported.

2 years agoBUILD: compiler: add a macro to detect if another one is set and equals 1
Willy Tarreau [Sun, 13 Nov 2022 10:32:45 +0000 (11:32 +0100)] 
BUILD: compiler: add a macro to detect if another one is set and equals 1

In order to simplify compiler-specific checks, we'll need to check if some
attributes exist. In order to ease declarations, we'll only focus on those
that exist and will set them to 1. Let's first add a macro aimed at doing
this. Passed a macro name in argument, it will return 1 if the macro is
defined and equals 1, otherwise it will return 0. This is based on the
concatenation of the macro's value with a name to form the name of a macro
which contains one comma, resulting in some other macros arguments being
shifted by one when the macro is defined. As such it's only a matter of
pushing both a 1 and a 0 and picking the correct argument to see the
desired one. It was verified to work since at least gcc-3.4 so it should
be portable enough.

2 years agoIMPORT: slz: define and use a __fallthrough statement for switch/case
Willy Tarreau [Sun, 13 Nov 2022 12:06:02 +0000 (13:06 +0100)] 
IMPORT: slz: define and use a __fallthrough statement for switch/case

When the code is preprocessed first and compiled later, such as when
built under distcc, the "fall through" comments are dropped and warnings
are emitted. Let's use the alternative "fallthrough" attribute instead,
that is supported by versions of gcc and clang that also produce this
warning.

This is libslz upstream commit 0fdf8ae218f3ecb0b7f22afd1a6b35a4f94053e2

2 years agoIMPORT: slz: mention the potential header in slz_finish()
Dridi Boukelmoune [Wed, 30 Mar 2022 05:58:23 +0000 (07:58 +0200)] 
IMPORT: slz: mention the potential header in slz_finish()

There may be 2 or 10 bytes sent respectively for zlib and gzip.

This is libslz upstream commit de1cac155ac730ba0491a6c866a510760c01fa9b

2 years agoIMPORT: slz: declare len to fix debug build when optimal match is enabled
Willy Tarreau [Mon, 13 Dec 2021 08:07:05 +0000 (09:07 +0100)] 
IMPORT: slz: declare len to fix debug build when optimal match is enabled

Building with -DFIND_OPTIMAL_MATCH would fail on undeclared "len".
This one likely vanished in some cleanup.

This is libslz upstream commit 1ea20360715e1ad0cd81db83fa4361310716b8cc

2 years agoIMPORT: xxhash: update xxHash to version 0.8.1
Willy Tarreau [Sun, 13 Nov 2022 12:21:56 +0000 (13:21 +0100)] 
IMPORT: xxhash: update xxHash to version 0.8.1

This is the latest released version and a minor update on top of the
current one (0.8.0). It addresses a few build issues (some for which
patches were already backported), and particularly the fallthrough
issue by using an attribute instead of a comment.

2 years agoCI: emit the compiler's version in the build reports
Willy Tarreau [Mon, 14 Nov 2022 10:03:18 +0000 (11:03 +0100)] 
CI: emit the compiler's version in the build reports

Some occasional builds fail only on a specific platform and being able
to figure the exact compiler version used there is crucial. It's not
easy to guess from the rest of the output, so let's add it before the
platform-specific defines, which suit the same needs.

2 years agoBUILD: debug: remove unnecessary quotes in HA_WEAK() calls
Willy Tarreau [Sun, 13 Nov 2022 11:14:10 +0000 (12:14 +0100)] 
BUILD: debug: remove unnecessary quotes in HA_WEAK() calls

HA_WEAK() is supposed to take a symbol in argument, not a string, since
the asm statements it produces already quote the argument. Having it
quoted twice doesn't work on older compilers and was the only reason
why DEBUG_MEM_STATS didn't work on older compilers.

2 years agoBUILD: ssl_utils: fix build on gcc versions before 8
Willy Tarreau [Mon, 14 Nov 2022 07:11:32 +0000 (08:11 +0100)] 
BUILD: ssl_utils: fix build on gcc versions before 8

Commit 960fb74ca ("MEDIUM: ssl: {ca,crt}-ignore-err can now use error
constant name") provided a very convenient way to initialize only desired
macros. Unfortunately with gcc versions older than 8, it breaks with:

  src/ssl_utils.c:473:12: error: initializer element is not constant

because it seems that the compiler cannot resolve strings to constants
at build time.

This patch takes a different approach, it stores the value of the macro
as a string and this string is converted to integer at boot time. This
way it works everywhere.

2 years agoBUG/MINOR: ssl: bind_conf is uncorrectly accessed when using QUIC
William Lallemand [Thu, 10 Nov 2022 15:45:24 +0000 (16:45 +0100)] 
BUG/MINOR: ssl: bind_conf is uncorrectly accessed when using QUIC

Since commit 9b2598 ("BUG/MEDIUM: ssl: Verify error codes can exceed
63"), the ca_ignerr_bitfield and crt_ignerr_bietfield are incorrecly
accessed from __objt_listener(conn->target)->bind_conf which is not
avaiable from QUIC. The bind_conf variable was mistakenly replaced.

This patch fixes the issue by using again the bind_conf variable.

Must be backported where 9b2598 was backported.

2 years agoMINOR: server: clear prefix on stderr logs after add server
Amaury Denoyelle [Thu, 10 Nov 2022 14:16:49 +0000 (15:16 +0100)] 
MINOR: server: clear prefix on stderr logs after add server

cli_parse_add_server() is the CLI handler for 'add server' command. This
functions uses usermsgs_ctx to retrieve logs messages from internal
ha_alert() calls and display it at the end of the handler.

At the beginning of the handler, stderr prefix is defined to "CLI" via
usermsgs_clr() function. However, this is not resetted at the end. This
causes inconsistency for stderr output :
1. each ha_alert() invocation will reuse "CLI" prefix if 'add server'
   command was executed before, even in non-CLI context
2. usermsgs_ctx is thread local, so this is only true if this runs on
   the same thread as 'add server' handler.

To fix this, ensure that "CLI" prefix is now resetted after
cli_parse_add_server(). This is done thanks to the addition to
cli_umsg()/cli_umsgerr() functions.

This can be backported up to 2.5 if we prefer to ensure output
consistency at the risk of changing stderr behaviors in stable versions.
In this case, the previous commit should be backported before :
  MINOR: cli: define usermsgs print context

2 years agoMINOR: cli: define usermsgs print context
Amaury Denoyelle [Thu, 10 Nov 2022 13:24:51 +0000 (14:24 +0100)] 
MINOR: cli: define usermsgs print context

CLI 'add server' handler relies on usermsgs_ctx to display errors in
internal function on CLI output. This may be also extended to other
handlers.

However, to not clutter stderr from another contextes, usermsgs_ctx must
be resetted when it is not needed anymore. This operation cannot be
conducted in the CLI parse handler as display is conducted after it.

To achieve this, define new CLI states CLI_ST_PRINT_UMSG /
CLI_ST_PRINT_UMSGERR. Their principles is nearly identical to states for
dynamic messages printing.

2 years agoCLEANUP: cli: rename dynamic error printing state
Amaury Denoyelle [Thu, 10 Nov 2022 10:47:36 +0000 (11:47 +0100)] 
CLEANUP: cli: rename dynamic error printing state

Rename CLI_ST_PRINT_FREE to CLI_ST_PRINT_DYNERR.

Most notably, this highlights that this is reserved to error printing.

This is done to ensure consistency between CLI_ST_PRINT/CLI_ST_PRINT_DYN
and CLI_ST_PRINT_ERR/CLI_ST_PRINT_DYNERR. The name is also consistent
with the function cli_dynerr() which activates it.

2 years agoMINOR: ssl: x509_v_err_str converter transforms an integer to a X509_V_ERR name
William Lallemand [Thu, 3 Nov 2022 17:56:37 +0000 (18:56 +0100)] 
MINOR: ssl: x509_v_err_str converter transforms an integer to a X509_V_ERR name

The x509_v_err_str converter transforms a numerical X509 verify error
to its constant name.

2 years agoMEDIUM: ssl: {ca,crt}-ignore-err can now use error constant name
William Lallemand [Thu, 3 Nov 2022 15:31:50 +0000 (16:31 +0100)] 
MEDIUM: ssl: {ca,crt}-ignore-err can now use error constant name

The ca-ignore-err and crt-ignore-err directives are now able to use the
openssl X509_V_ERR constant names instead of the numerical values.

This allow a configuration to survive an OpenSSL upgrade, because the
numerical ID can change between versions. For example
X509_V_ERR_INVALID_CA was 24 in OpenSSL 1 and is 79 in OpenSSL 3.

The list of errors must be updated when a new major OpenSSL version is
released.

2 years agoBUG/MEDIUM: ssl: Verify error codes can exceed 63
Remi Tricot-Le Breton [Thu, 10 Nov 2022 09:48:58 +0000 (10:48 +0100)] 
BUG/MEDIUM: ssl: Verify error codes can exceed 63

The CRT and CA verify error codes were stored in 6 bits each in the
xprt_st field of the ssl_sock_ctx meaning that only error code up to 63
could be stored. Likewise, the ca-ignore-err and crt-ignore-err options
relied on two unsigned long longs that were used as bitfields for all
the ignored error codes. On the latest OpenSSL1.1.1 and with OpenSSLv3
and newer, verify errors have exceeded this value so these two storages
must be increased. The error codes will now be stored on 7 bits each and
the ignore-err bitfields are replaced by a big enough array and
dedicated bit get and set functions.

It can be backported on all stable branches.

[wla: let it be tested a little while before backport]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2 years agoCI: enable QUIC for LibreSSL builds
Ilya Shipitsin [Wed, 2 Nov 2022 06:59:37 +0000 (11:59 +0500)] 
CI: enable QUIC for LibreSSL builds

since LibreSSL-3.6.x supports QUIC, let us enable it

2 years agoCI: switch to the "latest" LibreSSL
Ilya Shipitsin [Wed, 2 Nov 2022 06:57:03 +0000 (11:57 +0500)] 
CI: switch to the "latest" LibreSSL

LibreSSL-3.6.0 had some regression, it was fixed in 3.6.1, let us
switch back to the latest LibreSSL available

2 years agoBUG/MINOR: ssl: ocsp structure not freed properly in case of error
Remi Tricot-Le Breton [Thu, 3 Nov 2022 14:16:49 +0000 (15:16 +0100)] 
BUG/MINOR: ssl: ocsp structure not freed properly in case of error

In case of error, the ocsp item might already be in the ocsp certificate
tree but simply freed instead of destroyed through ssl_sock_free_ocsp.

This patch can be backported to all stable versions.

2 years agoBUG/MINOR: ssl: Memory leak of AUTHORITY_KEYID struct when loading issuer
Remi Tricot-Le Breton [Thu, 3 Nov 2022 14:16:48 +0000 (15:16 +0100)] 
BUG/MINOR: ssl: Memory leak of AUTHORITY_KEYID struct when loading issuer

When calling ssl_get0_issuer_chain, if akid is not NULL but its keyid
is, then the AUTHORITY_KEYID is not freed.

This patch can be backported to all stable branches.

2 years agoBUG/MINOR: ssl: Memory leak of DH BIGNUM fields
Remi Tricot-Le Breton [Thu, 3 Nov 2022 14:16:47 +0000 (15:16 +0100)] 
BUG/MINOR: ssl: Memory leak of DH BIGNUM fields

When running HAProxy with OpenSSLv3, the two BIGNUMs used to build our
own DH parameters are not freed. It was not necessary previously because
ownership of those parameters was transferred to OpenSSL through the
DH_set0_pqg call.

This patch should be backported to 2.6.

2 years agoBUG/MINOR: httpclient: fixed memory allocation for the SSL ca_file
Miroslav Zagorac [Wed, 2 Nov 2022 15:11:50 +0000 (16:11 +0100)] 
BUG/MINOR: httpclient: fixed memory allocation for the SSL ca_file

The memory for the SSL ca_file was allocated only once (in the function
httpclient_create_proxy()) and that pointer was assigned to each created
proxy that the HTTP client uses.  This would not be a problem if this
memory was not freed in each individual proxy when it was deinitialized
in the function ssl_sock_free_srv_ctx().

  Memory allocation:
    src/http_client.c, function httpclient_create_proxy():
      1277: if (!httpclient_ssl_ca_file)
      1278: httpclient_ssl_ca_file = strdup("@system-ca");
      1280: srv_ssl->ssl_ctx.ca_file = httpclient_ssl_ca_file;

  Memory deallocation:
    src/ssl_sock.c, function ssl_sock_free_srv_ctx():
      5613: ha_free(&srv->ssl_ctx.ca_file);

This should be backported to version 2.6.

2 years agoCLEANUP: ssl: remove dead code in ssl_sock_load_pem_into_ckch()
William Lallemand [Sun, 30 Oct 2022 18:00:06 +0000 (19:00 +0100)] 
CLEANUP: ssl: remove dead code in ssl_sock_load_pem_into_ckch()

Commit 432cd1a ("MEDIUM: ssl: be stricter about chain error")
introduced some dead code, let's remove it.

Should fix issue #1909.

2 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sat, 29 Oct 2022 04:34:32 +0000 (09:34 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 32nd iteration of typo fixes

2 years agoCI: add monthly gcc cross compile jobs
Ilya Shipitsin [Tue, 18 Oct 2022 14:13:45 +0000 (19:13 +0500)] 
CI: add monthly gcc cross compile jobs

Build only gcc cross compile jobs are added with monthly run to catch
rare errors, mostly 32bit <--> 64bit

2 years agoBUG/MINOR: quic: fix race condition on datagram purging
Amaury Denoyelle [Tue, 25 Oct 2022 09:38:21 +0000 (11:38 +0200)] 
BUG/MINOR: quic: fix race condition on datagram purging

Each datagram is received by a random thread and dispatch to its
destination thread linked to the connection. Then, the datagram is
handled by the connection thread. Once this is done, datagram buffer
pointer is atomically set to NULL to mark it as consumed.

Consumed datagrams are purged before recvfrom() invocation on random
receiver threads. The check for NULL buffer must thus be done
atomically. This was not the case before this patch, which may have
triggered race conditions.

This bug has been introduced by commit
  91b2305ad79bb7086840797b6e98bd791992444f
  MINOR: quic: implement datagram cleanup for quic_receiver_buf

This should be backported up to 2.6 after previously mentionned commit.

2 years agoMINOR: quic: add counter for interrupted reception
Amaury Denoyelle [Thu, 27 Oct 2022 15:56:27 +0000 (17:56 +0200)] 
MINOR: quic: add counter for interrupted reception

Add a new counter "quic_rxbuf_full". It is incremented each time
quic_sock_fd_iocb() is interrupted on full buffer.

This should help to debug github issue #1903. It is suspected that
QUIC receiver buffers are full which in turn cause quic_sock_fd_iocb()
to be called repeatedly resulting in a high CPU consumption.

2 years agoMINOR: ssl: dump the SSL string error when SSL_CTX_use_PrivateKey() failed.
William Lallemand [Thu, 27 Oct 2022 12:41:07 +0000 (14:41 +0200)] 
MINOR: ssl: dump the SSL string error when SSL_CTX_use_PrivateKey() failed.

Display the OpenSSL reason error string when SSL_CTX_use_PrivateKey()
failed.

2 years agoBUG/MINOR: log: fixing bug in tcp syslog_io_handler Octet-Counting
Aurelien DARRAGON [Wed, 26 Oct 2022 21:40:08 +0000 (23:40 +0200)] 
BUG/MINOR: log: fixing bug in tcp syslog_io_handler Octet-Counting

syslog_io_handler does specific treatment to handle syslog tcp octet
counting:

Logic was good, but a sneaky mistake prevented
rfc-6587 octet counting from working properly.

trash.area was used as an input buffer.
It does not make sense here since it is uninitialized.
Compilation was unaffected because trash is a thread
local "global" variable.

buf->area should definitely be used instead.

This should be backported as far as 2.4.

2 years agoBUG/MINOR: quic: fix subscribe operation
Amaury Denoyelle [Wed, 28 Sep 2022 13:15:51 +0000 (15:15 +0200)] 
BUG/MINOR: quic: fix subscribe operation

Subscribing was not properly designed between quic-conn and quic MUX
layers. Align this as with in other haproxy components : <subs> field is
moved from the MUX to the quic-conn structure. All mention of qcc MUX is
cleaned up in quic_conn_subscribe()/quic_conn_unsubscribe().

Thanks to this change, ACK reception notification has been simplified.
It's now unnecessary to check for the MUX existence before waking it.
Instead, if <subs> quic-conn field is set, just wake-up the upper layer
tasklet without mentionning MUX. This should probably be extended to
other part in quic-conn code.

This should be backported up to 2.6.

2 years agoMINOR: quic: remove unnecessary quic_session_accept()
Amaury Denoyelle [Thu, 29 Sep 2022 16:31:24 +0000 (18:31 +0200)] 
MINOR: quic: remove unnecessary quic_session_accept()

A specialized listener accept was previously used for QUIC. This is now
unneeded and we can revert to the default one session_accept_fd().

One change of importance is that the call order between
conn_xprt_start() and conn_complete_session() is now reverted to the
default one. This means that MUX instance is now NULL during
qc_xprt_start() and its app-ops layer cannot be set here. This operation
has been delayed to qc_init() to prevent a segfault.

This should be backported up to 2.6.

2 years agoBUG/MAJOR: stick-table: don't process store-response rules for applets
Christopher Faulet [Tue, 25 Oct 2022 14:45:38 +0000 (16:45 +0200)] 
BUG/MAJOR: stick-table: don't process store-response rules for applets

The commit bc7c207f74 ("BUG/MAJOR: stick-tables: do not try to index a
server name for applets") tried to catch applets case when we tried to index
the server name. However, there is still an issue. The applets are
unconditionally casted to servers and this bug exists since a while. it's
just luck if it doesn't crash.

Now, when store rules are processed, we skip the rule if the stream's target
is not a server or, of course, if it is a server but the "non-stick" option
is set. However, we still take care to release the sticky session.

This patch must be backported to all stable versions.

2 years agoMEDIUM: ssl: be stricter about chain error
William Lallemand [Tue, 25 Oct 2022 13:55:13 +0000 (15:55 +0200)] 
MEDIUM: ssl: be stricter about chain error

The error check on certificate chain was ignoring all decoding error,
silently ignoring some errors.

This patch fixes the issue by being stricter on errors when reading the
chain, this is a change of behavior, it could break existing setup that
has a wrong chain.

2 years agoMINOR: ssl: add the SSL error string before the chain
William Lallemand [Tue, 25 Oct 2022 13:53:01 +0000 (15:53 +0200)] 
MINOR: ssl: add the SSL error string before the chain

Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
 descriptive errror are emitted. This one is for the certificate before
trying to load the complete chain.

2 years agoMINOR: ssl: add the SSL error string when failing to load a certificate
William Lallemand [Tue, 25 Oct 2022 10:31:39 +0000 (12:31 +0200)] 
MINOR: ssl: add the SSL error string when failing to load a certificate

Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
descriptive errror are emitted.

Example:
[ALERT]    (1264006) : config : parsing [ssl_default_server.cfg:51] : 'bind /tmp/ssl.sock' in section 'listen' : unable to load certificate chain from file 'reg-tests/ssl//common.pem': ASN no PEM Header Error

2 years agoBUG/MINOR: sink: Set default connect/server timeout for implicit ring buffers
Christopher Faulet [Mon, 24 Oct 2022 13:53:01 +0000 (15:53 +0200)] 
BUG/MINOR: sink: Set default connect/server timeout for implicit ring buffers

Ring buffers may be implicitly created from log declarations when "tcp@",
"tcp6@", "tcp4@" or "uxst@" prefixes are used. These ring buffers rely on
unconfigurable proxies. While connect and server timeouts should be defined for
explicit ring buffers, it is no possible for implicit ones. However, a default
value must be set and TICK_ETERNITY is not an acceptable one.

Thus, now "1s" is set for the connect timeout and "5s" is set for server one.

This patch may be backported as far as 2.4.

2 years agoBUG/MINOR: sink: Only use backend capability for the sink proxies
Christopher Faulet [Mon, 24 Oct 2022 13:10:18 +0000 (15:10 +0200)] 
BUG/MINOR: sink: Only use backend capability for the sink proxies

When a ring section is parsed, a proxy is created. For now, it has the
frontend (PR_CAP_FE) and the internal (PR_CAP_INT) capabilities, in addition
to the expected backend capability (PR_CAP_BE).

PR_CAP_INT capability was added to silent warning triggered because of
PR_CAP_FE capability. Indeed, Because the proxy is declared as a frontend,
warnings about missing bind lines and missing client timeout should be
triggered during the configuration parsing. These warnings are inhibited
because PR_CAP_INT capability is set. It is an issue on the 2.4 because
PR_CAP_INT capability does not exist. So warnings are always emitted.

But the true bug is that these proxies should not have PR_CAP_FE and
PR_CAP_INT capabilities. Removing these capabilities is enough to remove any
warnings on the 2.4, with no regression on higher versions. However, it may
be a good idea to eval if a dedicated frontend for sinks should be added or
not. This way, a true frontend would be used to start the sink applets. In
addition, proxies capabilities/modes have to be reviewed to have a less
ambiguous API. For instance a dedicate mode for sinks (PR_MODE_SINK ?) may
be added. Finally, it could be very nice to have all proxies in the same
list, including internal ones.

This patch should fix the issue #1900. It must be backported as far as 2.4.

2 years agoMINOR: peers: handle multiple resync requests using shards
Emeric Brun [Mon, 24 Oct 2022 08:04:59 +0000 (10:04 +0200)] 
MINOR: peers: handle multiple resync requests using shards

We considered the resync process is finished if a full resync request
is ended receiving the "resync-finish" message. But in the case of
"shards" each node declared with a "shard" has only a partial view
of the table. And the resync process is ended whereas the original
peer tables content contains only a "shard" of the full content.

This patch allow to retrieve the entire tables requesting a resync
from all different "shards".

To do so we don't commit the end of a resync process receiving a
"resync-finish" if the node is part of "shard", we only flag this
peer and all peers using the same shard as "notup2date" as if we
received a "resync-partial" message, and we re-schedule a request
of a resync as it is done receiving a "resync-partial" message.

Doing this the peers flagged "notup2date" won't be addressed for the
next resync request round and the next resync request will be send to
a shard not yet requested.

Receving a "resync-finish" message we also check if all peers using
"shards" are flagged "notup2date". It meens that all peers have been
addressed and we can considered the resync process is now finished.

Note also that the "resync request" scheduler already handle a timeout
and if we are not able to retrieve a full resync after a delay. The
resync process is ended.

This patch should be backported in all versions handling "shard"
on peer lines.

2 years agoMINOR: peers: Support for peer shards
Frédéric Lécaille [Mon, 17 Oct 2022 12:58:19 +0000 (14:58 +0200)] 
MINOR: peers: Support for peer shards

Add "shards" new keyword for "peers" section to configure the number
of peer shards attached to such secions. This impact all the stick-tables
attached to the section.
Add "shard" new "server" parameter to configure the peers which participate to
all the stick-tables contents distribution. Each peer receive the stick-tables updates
only for keys with this shard value as distribution hash. The "shard" value
is stored in ->shard new server struct member.
cfg_parse_peers() which is the function which is called to parse all
the lines of a "peers" section is modified to parse the "shards" parameter
stored in ->nb_shards new peers struct member.
Add srv_parse_shard() new callback into server.c to pare the "shard"
parameter.
Implement stksess_getkey_hash() to compute the distribution hash for a
stick-table key as the 64-bits xxhash of the key concatenated to the stick-table
name. This function is called by stksess_setkey_shard(), itself
called by the already implemented function which create a new stick-table
key (stksess_new()).
Add ->idlen new stktable struct member to store the stick-table name length
to not have to compute it each time a stick-table key hash is computed.

2 years agoMINOR: quic: display unknown error sendto counter on stat page
Amaury Denoyelle [Mon, 24 Oct 2022 08:44:34 +0000 (10:44 +0200)] 
MINOR: quic: display unknown error sendto counter on stat page

This patch complete the previous incomplete commit. The new counter
sendto_err_unknown is now displayed on stats page/CLI show stats.

This is related to github issue #1903.

This should be backported up to 2.6.

2 years agoMINOR: quic: do not crash on unhandled sendto error
Amaury Denoyelle [Mon, 24 Oct 2022 08:03:33 +0000 (10:03 +0200)] 
MINOR: quic: do not crash on unhandled sendto error

Remove ABORT_NOW() statement on unhandled sendto error. Instead use a
dedicated counter sendto_err_unknown to report these cases.

If we detect increment of this counter, strace can be used to detect
errno value :
  $ strace -p $(pidof haproxy) -f -e trace=sendto -Z

This should be backported up to 2.6.

This should help to debug github issue #1903.

2 years agoBUG/MEDIUM: compression: handle rewrite errors when updating response headers
Christopher Faulet [Mon, 24 Oct 2022 06:39:29 +0000 (08:39 +0200)] 
BUG/MEDIUM: compression: handle rewrite errors when updating response headers

When an HTTP response is compressed by HAProxy, the headers are updated.
However it is possible to encounter a rewrite error because the buffer is
full. In this case, the compression is aborted. Thus, we must be sure to
leave the response in a valid state.

For now, it is an issue because the "Content-Encoding" header is added
before all other headers manipulations. So if the compression is aborted on
error, the "Content-Encoding" header may remain while the payload is not
compressed.

So now, we take care to leave with a valid response on error by reordering
the headers manipulations. It is too painful to really rollback all changes,
especially for an edge case.

This patch should be backported as far as 2.0. Note that on the 2.0, the
legacy HTTP part is also concerned.

2 years agoBUG/MINOR: mux-quic: complete flow-control for uni streams
Amaury Denoyelle [Fri, 21 Oct 2022 15:02:18 +0000 (17:02 +0200)] 
BUG/MINOR: mux-quic: complete flow-control for uni streams

Max stream data was not enforced and respect for local/remote uni
streams. Previously, qcs instances incorrectly reused the limit defined
from bidirectional ones.

This is now fixed. Two fields are added in qcc structure connection :
* value for local flow control to enforce on remote uni streams
* value for remote flow control to respect on local uni streams

These two values can be reused to properly initialized msd field of a
qcs instance in qcs_new(). The rest of the code is similar.

This must be backported up to 2.6.

2 years agoMINOR: list: adding MT_LIST_APPEND_LOCKED macro
Aurelien DARRAGON [Thu, 20 Oct 2022 15:37:51 +0000 (17:37 +0200)] 
MINOR: list: adding MT_LIST_APPEND_LOCKED macro

adding a new mt macro: MT_LIST_APPEND_LOCKED.

This macro may be used to append an item to an existing
list, like MT_LIST_APPEND.

But here the item will be forced into locked/busy state
prior to appending, so that it is already referenced
in the list while still preventing concurrent accesses
until we decide to unlock it.

The macro returns a struct mt_list "np", that is needed
at unlock time using regular MT_LIST_UNLOCK_ELT macro.

2 years agoDOC/MINOR: list: fixing MT_LIST_LOCK_ELT macro documentation
Aurelien DARRAGON [Thu, 20 Oct 2022 07:42:23 +0000 (09:42 +0200)] 
DOC/MINOR: list: fixing MT_LIST_LOCK_ELT macro documentation

MT_LIST_LOCK_ELT macro was documented with an ambiguous
usage restriction, implying that concurrent list deletion
was not supported.

But it seems that either the code has evolved, or the comment is
wrong because the locking behavior implemented here is exactly
the same one used in MT_LIST_DELETE, and no such restriction is
described for MT_LIST_DELETE.

I made some tests to make sure concurrent MT_LIST_DELETE (or deletion
from mt_list_for_each_entry_safe) don't cause unexepected results.

At the present time, this macro is not used, this fix only
targets upcoming developments that might rely on this.

No backport needed.

2 years agoMINOR: list: fixing typo in MT_LIST_LOCK_ELT
Aurelien DARRAGON [Thu, 20 Oct 2022 07:19:30 +0000 (09:19 +0200)] 
MINOR: list: fixing typo in MT_LIST_LOCK_ELT

A minor typo was made in MT_LIST_LOCK_ELT, preventing
haproxy from compiling if MT_LIST_LOCK_ELT is
used in the code.

Today, the macro is unused, and that's the reason why
the typo has remained unnoticed for such a long time.

Fixing it so it can be used in upcoming developments.

No backport required.

2 years agoMINOR: mworker/cli: does no try to dump the startup-logs w/o USE_SHM_OPEN
William Lallemand [Fri, 21 Oct 2022 12:03:29 +0000 (14:03 +0200)] 
MINOR: mworker/cli: does no try to dump the startup-logs w/o USE_SHM_OPEN

When haproxy is compiled without USE_SHM_OPEN, does not try to dump the
startup-logs in the "reload" output, because it won't show anything
interesting.

2 years agoCLEANUP: mworker/cli: rename the status function to loadstatus
William Lallemand [Fri, 21 Oct 2022 12:00:05 +0000 (14:00 +0200)] 
CLEANUP: mworker/cli: rename the status function to loadstatus

clarify the name of the IO handler which show the reload status.

2 years agoDOC: lua: add a note about compression w/ httpclient
William Lallemand [Fri, 21 Oct 2022 09:48:24 +0000 (11:48 +0200)] 
DOC: lua: add a note about compression w/ httpclient

Decompression is not supported by the httpclient.

2 years agoBUILD: Makefile: add "USE_SHM_OPEN" on the linux-musl target
William Lallemand [Fri, 21 Oct 2022 08:35:37 +0000 (10:35 +0200)] 
BUILD: Makefile: add "USE_SHM_OPEN" on the linux-musl target

The startup-logs with the shm works correctly with Alpine and Musl,
enable the feature by default for the linux-musl target.

2 years agoCI: github: dump the backtrace of coredumps in the alpine container
William Lallemand [Thu, 20 Oct 2022 13:01:01 +0000 (15:01 +0200)] 
CI: github: dump the backtrace of coredumps in the alpine container

This patch allows to show the backtrace of a coredump produced in the
alpine/musl jobs.

It activates some option required by the containers to allow the
production of coredump, set a shared directory so the kernel could dump
the coredump within the container. Some debug packages were also added.

2 years agoREGTESTS: httpclient/lua: test the lua task timeout with the httpclient
William Lallemand [Thu, 20 Oct 2022 09:23:02 +0000 (11:23 +0200)] 
REGTESTS: httpclient/lua: test the lua task timeout with the httpclient

Test the httpclient when the lua action timeout. The lua timeout is
reached before the httpclient is ended. This test that the httpclient
are correctly cleaned when destroying the hlua context.

Must be backported as far as 2.5.

2 years agoBUG/MEDIUM: httpclient: check if the httpclient was released in the IO handler
William Lallemand [Thu, 20 Oct 2022 16:36:03 +0000 (18:36 +0200)] 
BUG/MEDIUM: httpclient: check if the httpclient was released in the IO handler

Upon a applet_release(), the applet can be scheduled again and a call to
the IO handler is still possible. When the struct httpclient is already
free the IO handler could try to access it.

This patch fixes the issue by setting svcctx to NULL in the
applet_release, and checking its value in the IO handler.

Must be backported as far as 2.5.

2 years agoBUG/MEDIUM: httpclient/lua: crash when the lua task timeout before the httpclient
William Lallemand [Thu, 20 Oct 2022 08:57:28 +0000 (10:57 +0200)] 
BUG/MEDIUM: httpclient/lua: crash when the lua task timeout before the httpclient

When the lua task finished  before the httpclient that are associated to
it, there is a risk that the httpclient try to task_wakeup() the lua
task which does not exist anymore.

To fix this issue the httpclient used in a lua task are stored in a
list, and the httpclient are destroyed at the end of the lua task.

Must be backported in 2.5 and 2.6.

2 years agoBUG/MINOR: ring: Properly parse connect timeout
Christopher Faulet [Wed, 19 Oct 2022 14:26:21 +0000 (16:26 +0200)] 
BUG/MINOR: ring: Properly parse connect timeout

The connect timeout in a ring section was not properly parsed. Thus, it was
never set and the server timeout may be overwritten, depending on the
directives order. The first char of the keyword must be tested, not the
third one.

This patch is related to the issue #1900. But it does not fix the issue. It
must be backported as far as 2.4.

2 years agoBUG/MINOR: log: Preserve message facility when the log target is a ring buffer
Christopher Faulet [Wed, 19 Oct 2022 06:16:48 +0000 (08:16 +0200)] 
BUG/MINOR: log: Preserve message facility when the log target is a ring buffer

When a ring is used as log target, the original facility, if any, must be
preserved. The default facility must only be used if there no facility was
found in the incoming log message.

This patch should fix the issue #1901. It must be backported as far as 2.4.

2 years agoMINOR: quic: extend Retry token check function
Amaury Denoyelle [Mon, 17 Oct 2022 09:13:07 +0000 (11:13 +0200)] 
MINOR: quic: extend Retry token check function

On Initial packet reception, token is checked for validity through
quic_retry_token_check() function. However, some related parts were left
in the parent function quic_rx_pkt_retrieve_conn(). Move this code
directly into quic_retry_token_check() to facilitate its call in various
context.

The API of quic_retry_token_check() has also been refactored. Instead of
working on a plain char* buffer, it now uses a quic_rx_packet instance.
This helps to reduce the number of parameters.

This change will allow to check Retry token even if data were received
with a FD-owned quic-conn socket. Indeed, in this case,
quic_rx_pkt_retrieve_conn() call will probably be skipped.

This should be backported up to 2.6.

2 years agoMINOR: quic: refactor packet drop on reception
Amaury Denoyelle [Mon, 17 Oct 2022 10:04:49 +0000 (12:04 +0200)] 
MINOR: quic: refactor packet drop on reception

Sometimes, a packet is dropped on reception. Several goto statements are
used, mostly to increment a proxy drop counter or drop silently the
packet. However, this labels are interleaved. Re-arrang goto labels to
simplify this process :
* drop label is used to drop a packet with counter incrementation. This
  is the default method.
* drop_silent is the next label which does the same thing but skip the
  counter incrementation. This is useful when we do not need to report
  the packet dropping operation.

This should be backported up to 2.6.

2 years agoMINOR: quic: split and rename qc_lstnr_pkt_rcv()
Amaury Denoyelle [Wed, 19 Oct 2022 13:37:44 +0000 (15:37 +0200)] 
MINOR: quic: split and rename qc_lstnr_pkt_rcv()

This change is the following of qc_lstnr_pkt_rcv() refactoring. This
function has finally been split into several ones.

The first half is renamed quic_rx_pkt_parse(). This function is
responsible to parse a QUIC packet header and calculate the packet
length.

QUIC connection retrieval has been extracted and is now called directly
by quic_lstnr_dghdlr().

The second half of qc_lstnr_pkt_rcv() is renamed to qc_rx_pkt_handle().
This function is responsible to copy a QUIC packet content to a
quic-conn receive buffer.

A third function named qc_rx_check_closing() is responsible to detect if
the connection is already in closing state. As this requires to drop the
whole datagram, it seems justified to be in a separate function.

This change has no functional impact. It is part of a refactoring series
on qc_lstnr_pkt_rcv(). The objective is to facilitate the integration of
FD-owned quic-conn socket patches.

This should be backported up to 2.6.

2 years agoMINOR: quic: extract connection retrieval
Amaury Denoyelle [Wed, 19 Oct 2022 13:28:44 +0000 (15:28 +0200)] 
MINOR: quic: extract connection retrieval

Simplify qc_lstnr_pkt_rcv() by extracting code responsible to retrieve
the quic-conn instance. This code is put in a dedicated function named
quic_rx_pkt_retrieve_conn(). This new function could be skipped if a
FD-owned quic-conn socket is used.

The first traces of qc_lstnr_pkt_rcv() have been clean up as qc instance
is always NULL here : thus qc parameter can be removed without any
change.

This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.

This should be backported up to 2.6.

2 years agoMINOR: quic: define first packet flag
Amaury Denoyelle [Wed, 19 Oct 2022 15:14:28 +0000 (17:14 +0200)] 
MINOR: quic: define first packet flag

Received packets treatment has some difference regarding if this is the
first one or not of the encapsulating datagram. Previously, this was set
via a function argument. Simplify this by defining a new Rx packet flag
named QUIC_FL_RX_PACKET_DGRAM_FIRST.

This change does not have functional impact. It will simplify API when
qc_lstnr_pkt_rcv() is broken into several functions : their number of
arguments will be reduced thanks to this patch.

This should be backported up to 2.6.

2 years agoMINOR: quic: extend pn_offset field from quic_rx_packet
Amaury Denoyelle [Mon, 17 Oct 2022 16:05:26 +0000 (18:05 +0200)] 
MINOR: quic: extend pn_offset field from quic_rx_packet

pn_offset field was only set if header protection cannot be removed.
Extend the usage of this field : it is now set everytime on packet
parsing in qc_lstnr_pkt_rcv().

This change helps to clean up API of Rx functions by removing
unnecessary variables and function argument.

This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.

This should be backported up to 2.6.

2 years agoMINOR: quic: add version field on quic_rx_packet
Amaury Denoyelle [Mon, 17 Oct 2022 16:05:18 +0000 (18:05 +0200)] 
MINOR: quic: add version field on quic_rx_packet

Add a new field version on quic_rx_packet structure. This is set on
header parsing in qc_lstnr_pkt_rcv() function.

This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.

This should be backported up to 2.6.

2 years agoBUG/MINOR: quic: fix buffer overflow on retry token generation
Amaury Denoyelle [Tue, 18 Oct 2022 09:05:02 +0000 (11:05 +0200)] 
BUG/MINOR: quic: fix buffer overflow on retry token generation

When generating a Retry token, client CID is used as encryption input.
The client must reuse the same CID when emitting the token in a new
Initial packet.

A memory overflow can occur on quic_generate_retry_token() depending on
the size of client CID. This is because space reserved for <aad> only
accounted for QUIC_HAP_CID_LEN (size of haproxy owned generated CID).
However, the client CID size only depends on client parameter and is
instead limited to QUIC_CID_MAXLEN as specified in RFC9000.

This was reproduced with ngtcp2 and haproxy built with ASAN. Here is the error
log :
  ==14964==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffee228cee at pc 0x7ffff785f427 bp 0x7fffee2289e0 sp 0x7fffee228188
  WRITE of size 17 at 0x7fffee228cee thread T5
      #0 0x7ffff785f426 in __interceptor_memcpy /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
      #1 0x555555906ea7 in quic_generate_retry_token_aad src/quic_conn.c:5452
      #2 0x555555907e72 in quic_retry_token_check src/quic_conn.c:5577
      #3 0x55555590d01e in qc_lstnr_pkt_rcv src/quic_conn.c:6103
      #4 0x5555559190fa in quic_lstnr_dghdlr src/quic_conn.c:7179
      #5 0x555555eb0abf in run_tasks_from_lists src/task.c:590
      #6 0x555555eb285f in process_runnable_tasks src/task.c:855
      #7 0x555555d9118f in run_poll_loop src/haproxy.c:2853
      #8 0x555555d91f88 in run_thread_poll_loop src/haproxy.c:3042
      #9 0x7ffff709f8fc  (/usr/lib/libc.so.6+0x868fc)
      #10 0x7ffff7121a5f  (/usr/lib/libc.so.6+0x108a5f)

This must be backported up to 2.6.

2 years agoBUILD: quic: Fix build for m68k cross-compilation
Frédéric Lécaille [Tue, 18 Oct 2022 09:57:01 +0000 (11:57 +0200)] 
BUILD: quic: Fix build for m68k cross-compilation

Fix several warinings as this one:

src/qmux_trace.c:80:45: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ {aka ‘const long long unsigned int’} [-Werror=format=]
   80 |    chunk_appendf(&trace_buf, " qcs=%p .id=%lu .st=%s",
      |                                           ~~^
      |                                             |
      |                                             long unsigned int
      |                                           %llu
   81 |                  qcs, qcs->id,
      |                       ~~~~~~~
      |                          |
      |                          uint64_t {aka const long long unsigned int}
compilation terminated due to -Wfatal-errors.

Cast remaining uint64_t variables as ullong with %llu as printf format and size_t
others as ulong with %lu as printf format.

Thank you to Ilya for having reported this issue in GH #1899.

Must be backported to 2.6

2 years agoBUILD: ssl_sock: fix null dereference for QUIC build
Amaury Denoyelle [Mon, 17 Oct 2022 16:46:49 +0000 (18:46 +0200)] 
BUILD: ssl_sock: fix null dereference for QUIC build

A previous commit tries to fix uninitialized GCC warning on ssl code for
QUIC build. See the fix here :
  48e46f98ccf97427995eb41c6f28cc38705bdd7e
  BUILD: ssl_sock: bind_conf uninitialized in ssl_sock_bind_verifycbk()

However, this is incomplete as it still reports possible NULL
dereference on ctx variable (GCC v12.2.0). Here is the compilation
result :

  src/ssl_sock.c: In function ‘ssl_sock_bind_verifycbk’:
  src/ssl_sock.c:1739:12: error: potential null pointer dereference [-Werror=null-dereference]
   1739 |         ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
        |

To fix this, remove check on qc which can also never happens and replace
it with a BUG_ON. This seems to satisfy GCC on my machine.

This must be backported up to 2.6.

2 years agoBUG/MEDIUM: httpclient: segfault when the httpclient parser fails
Thierry Fournier [Mon, 10 Oct 2022 10:46:38 +0000 (12:46 +0200)] 
BUG/MEDIUM: httpclient: segfault when the httpclient parser fails

If the uri is unexpected ("/" in place of "http://xxx/"), some parsing
function fails. The failure is not handled.

This patch handle these errors. Note: the return code is boolean, maybe
we can return more precise error for Lua reporting ?

Must be backported in 2.6.

2 years agoBUILD: scripts: disable tests build on QuicTLS build
Ilya Shipitsin [Sat, 15 Oct 2022 04:55:49 +0000 (09:55 +0500)] 
BUILD: scripts: disable tests build on QuicTLS build

during CI builds QuicTLS is not cached, let us speed it up by
disabling tests build. Doing so saves ~40s out of 3m40.

2 years agoBUILD: quic: QUIC mux build fix for 32-bit build
Frédéric Lécaille [Fri, 14 Oct 2022 20:10:50 +0000 (22:10 +0200)] 
BUILD: quic: QUIC mux build fix for 32-bit build

Thank you to Ilya for having reported this issue in GH #1897

Must be backported to 2.6.