]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoBUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty
Willy Tarreau [Tue, 30 May 2023 13:42:35 +0000 (15:42 +0200)] 
BUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty

There's a rare case where on long fat pipes, we can see the keep-alive
timeout trigger before the end of the transfer of the last large object,
and the connection closed a bit quickly after the end of the transfer
because a GOAWAY is queued. The data are not destroyed, except that
the WINDOW_UPDATES from the client arriving late while the last data
are being drained by the socket buffers may at some point trigger a
reset, and some clients might choke a bit too early on these. Let's
make sure we only arm the idle_start timestamp once the output buffer
is empty. Of course it will still not cover for the data pending in the
socket buffers but it will at least let those in the buffer leave in
peace. More elaborate options can be used to protect the data in the
kernel buffers, such as the one described in GH issue #5.

It's very likely that this old issue was emphasized by the following
commit in 2.6:
  15a4733d5 ("BUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts")

and the behavior probably changed again with this one in 2.8, which
was backported to 2.7 and scheduled for 2.6:
  d38d8c6cc ("BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout")

As such this patch should be backported to 2.6 after some observation
period.

2 years agoCLEANUP: mux-quic: rename internal functions
Amaury Denoyelle [Tue, 30 May 2023 13:04:46 +0000 (15:04 +0200)] 
CLEANUP: mux-quic: rename internal functions

This patch is similar to the previous one but for QUIC mux functions
used inside the mux code itself or application layer. Replace all
occurences of qc_* prefix by qcc_* or qcs_*. This should help to better
differentiate code between quic_conn and MUX.

This should be backported up to 2.7.

2 years agoCLEANUP: mux-quic: rename functions for mux_ops
Amaury Denoyelle [Tue, 30 May 2023 12:51:57 +0000 (14:51 +0200)] 
CLEANUP: mux-quic: rename functions for mux_ops

Rename all QUIC mux function exposed through mux_ops structure. Use the
prefix qmux_* or qmux_strm_*. The objective is to remove qc_* prefix
which should only be used in quic_conn layer.

This should be backported up to 2.7.

2 years agoDOC: quic: remove experimental status for QUIC
Amaury Denoyelle [Tue, 30 May 2023 12:36:40 +0000 (14:36 +0200)] 
DOC: quic: remove experimental status for QUIC

QUIC support can now be considered production-ready. As such, remove all
statements on the documentation concerning its experimental status.

Do not backport this one.

2 years agoDOC: config: fix rfc7239 converter examples
Aurelien DARRAGON [Tue, 30 May 2023 07:47:53 +0000 (09:47 +0200)] 
DOC: config: fix rfc7239 converter examples

Some rfc7239 converter examples were not working and thus were misleading.
Fixing rfc7239_n2nn and rfc7239_n2np usage examples.

As both converters were introduced in 2.8, no backport needed.

2 years agoBUG/MEDIUM: threads: fix a tiny race in thread_isolate()
Willy Tarreau [Sat, 27 May 2023 11:45:01 +0000 (13:45 +0200)] 
BUG/MEDIUM: threads: fix a tiny race in thread_isolate()

AurĂ©lien found a tiny race in thread_isolate() that can allow a thread
that was running under isolation to continue running while another one
enters isolation. The reason is that the check for harmless is only
done before winning the CAS, but since the previously isolated thread
doesn't wait for !rdv_request in thread_release(), it can effectively
continue its activities while the next one believes it's isolated. A
proper solution consists in looping once again in thread_isolate() to
recheck (and wait) for all threads to be isolated once the CAS is won.

The issue was introduced in 2.7 by commit 598cf3f22 ("MAJOR: threads:
change thread_isolate to support inter-group synchronization") so the
fix needs to be backported there.

2 years agoBUG/MEDIUM: mux-quic: only set EOI on FIN
Amaury Denoyelle [Thu, 25 May 2023 13:02:24 +0000 (15:02 +0200)] 
BUG/MEDIUM: mux-quic: only set EOI on FIN

Recently stconn flags were reviewed for QUIC mux to be conform with
other HTTP muxes. However, a mistake was made when dealing with a proper
stream FIN with both EOI and EOS set. This was done as RESET_STREAM
received after a FIN are ignored by QUIC mux and thus there is no
difference between EOI or EOI+EOS. However, analyzers may interpret EOS
as an interrupted request which result in a 400 HTTP error code.

To fix this, only set EOI on proper stream FIN. EOS is set when input is
interrupted (RESET_STREAM before FIN) or a STOP_SENDING is received
which prevent transfer to complete. In this last case, EOS must be
manually set too if FIN has been received before STOP_SENDING to go
directly from ERR_PENDING to final ERROR state.

This must be backported up to 2.7.

2 years agoMINOR: quic: fix stats naming for flow control BLOCKED frames
Amaury Denoyelle [Thu, 25 May 2023 08:36:04 +0000 (10:36 +0200)] 
MINOR: quic: fix stats naming for flow control BLOCKED frames

There was a misnaming in stats counter for *_BLOCKED frames in regard to
QUIC rfc convention. This patch fixes it to prevent future ambiguity :

- STREAMS_BLOCKED -> STREAM_DATA_BLOCKED
- STREAMS_DATA_BLOCKED_BIDI -> STREAMS_BLOCKED_BIDI
- STREAMS_DATA_BLOCKED_UNI -> STREAMS_BLOCKED_UNI

This should be backported up to 2.7.

2 years agoMINOR: mux-quic: remove nb_streams from qcc
Amaury Denoyelle [Thu, 25 May 2023 08:16:19 +0000 (10:16 +0200)] 
MINOR: mux-quic: remove nb_streams from qcc

Remove nb_streams field from qcc. It was not used outside of a BUG_ON()
statement to ensure we never have a negative count of streams. However
this is already checked with other fields.

This should be backported up to 2.7.

2 years agoCLEANUP: mux-quic: remove unneeded fields in qcc
Amaury Denoyelle [Thu, 25 May 2023 08:15:46 +0000 (10:15 +0200)] 
CLEANUP: mux-quic: remove unneeded fields in qcc

Remove fields from qcc structure which are unused.

This should be backported up to 2.7.

2 years agoDOC: install: specify the minimum openssl version recommended
William Lallemand [Fri, 26 May 2023 12:44:33 +0000 (14:44 +0200)] 
DOC: install: specify the minimum openssl version recommended

Specify 1.1.1 as the minimum openssl version with full keywords support
in haproxy configuration.

2 years agoBUILD: init: print rlim_cur as regular integer
Aurelien DARRAGON [Fri, 26 May 2023 12:04:18 +0000 (14:04 +0200)] 
BUILD: init: print rlim_cur as regular integer

haproxy does not compile anymore on macOS+clang since 425d7ad ("MINOR:
init: pre-allocate kernel data structures on init"). This is due to
rlim_cur being printed uncasted using %lu format specifier, with rlim_cur
being stored as a rlim_t which is a typedef so its size may vary depending
on the system's architecture.

This is not the first time we need to dump rlim_cur in case of errors,
there are already multiple occurences in the init code. Everywhere this
happens, rlim is casted as a regular int and printed using the '%d'
format specifier, so we do the same here as well to fix the build issue.

No backport needed unless 425d7ad gets backported.

2 years agoBUG/MINOR: thread: add a check for pthread_create
eaglegai [Fri, 26 May 2023 08:44:34 +0000 (16:44 +0800)] 
BUG/MINOR: thread: add a check for pthread_create

preload_libgcc_s() use pthread_create to create a thread and then call
pthread_join to use it, but it doesn't check if the option is successful.
So add a check to aviod potential crash.

2 years agoBUG/MINOR: ssl_sock: add check for ha_meth
eaglegai [Fri, 26 May 2023 08:42:47 +0000 (16:42 +0800)] 
BUG/MINOR: ssl_sock: add check for ha_meth

in __ssl_sock_init, BIO_meth_new may failed and return NULL if
OPENSSL_zalloc failed.  in this case, ha_meth  will be NULL, and then
crash happens in  BIO_meth_set_write.  So, we add a check for ha_meth.

2 years agoDOC: install: add details about WolfSSL
William Lallemand [Thu, 25 May 2023 15:17:29 +0000 (17:17 +0200)] 
DOC: install: add details about WolfSSL

Add details about WolfSSL compilation and support.

2 years agoMINOR: init: pre-allocate kernel data structures on init
Patrick Hemmer [Tue, 23 May 2023 17:02:08 +0000 (13:02 -0400)] 
MINOR: init: pre-allocate kernel data structures on init

The Linux kernel maintains data structures to track a processes' open file
descriptors, and it expands these structures as necessary when FD usage grows
(at every FD=2^X starting at 64). However when threading is in use, during
expansion the kernel will pause (observed up to 47ms) while it waits for thread
synchronization (see https://bugzilla.kernel.org/show_bug.cgi?id=217366).

This change addresses the issue and avoids the random pauses by opening the
maximum file descriptor during initialization, so that expansion will not occur
while processing traffic.

2 years agoBUILD: makefile: search for SSL_INC/wolfssl before SSL_INC
Willy Tarreau [Thu, 25 May 2023 18:17:27 +0000 (20:17 +0200)] 
BUILD: makefile: search for SSL_INC/wolfssl before SSL_INC

Building with an install of wolfssl and openssl side-by-side breaks
because for wolfssl we need the two include levels and since some
names are in common, this results in some files being found in the
original openssl tree. Let's swap the two include paths so that all
that is related to wolfssl is found there first when needed.

No backport is needed.

2 years agoMINOR: compression: Improve the way Vary header is added
Christopher Faulet [Thu, 25 May 2023 09:18:21 +0000 (11:18 +0200)] 
MINOR: compression: Improve the way Vary header is added

When a message is compressed, A "Vary" header is added with
"accept-encoding" value. However, a new header is always added, regardless
there is already a Vary header or not. In addition, if there is already a
Vary header, there is no check on values to be sure "accept-encoding" value
is not already there. So it is possible to have it twice.

To improve this part, we now test Vary header values and "accept-encoding"
is only added if it was not found. In addition, "accept-encoding" value is
appended to the last Vary header found, if any. Otherwise, a new header is
added.

2 years ago[RELEASE] Released version 2.8-dev13 v2.8-dev13
Willy Tarreau [Wed, 24 May 2023 20:53:55 +0000 (22:53 +0200)] 
[RELEASE] Released version 2.8-dev13

Released version 2.8-dev13 with the following main changes :
    - DOC: add size format section to manual
    - CLEANUP: mux-quic/h3: complete BUG_ON with comments
    - MINOR: quic: remove return val of quic_aead_iv_build()
    - MINOR: quic: use WARN_ON for encrypt failures
    - BUG/MINOR: quic: handle Tx packet allocation failure properly
    - MINOR: quic: fix alignment of oneline show quic
    - MEDIUM: stconn/applet: Allow SF_SL_EOS flag alone
    - MEDIUM: stconn: make the SE_FL_ERR_PENDING to ERROR transition systematic
    - DOC: internal: add a bit of documentation for the stconn closing conditions
    - DOC/MINOR: config: Fix typo in description for `ssl_bc` in configuration.txt
    - BUILD: quic: re-enable chacha20_poly1305 for libressl
    - MINOR: mux-quic: set both EOI EOS for stream fin
    - MINOR: mux-quic: only set EOS on RESET_STREAM recv
    - MINOR: mux-quic: report error on stream-endpoint earlier
    - BUILD: makefile: fix build issue on GNU make < 3.82
    - BUG/MINOR: mux-h2: Check H2_SF_BODY_TUNNEL on H2S flags and not demux frame ones
    - MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame
    - MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
    - BUG/MEDIUM: mux-h2: Propagate termination flags when frontend SC is created
    - DEV: add a Lua helper script for SSL keys logging
    - CLEANUP: makefile: don't display a dummy features list without a target
    - BUILD: makefile: do not erase build options for some build options
    - MINOR: quic: Add low level traces (addresses, DCID)
    - BUG/MINOR: quic: Wrong token length check (quic_generate_retry_token())
    - BUG/MINOR: quic: Missing Retry token length on receipt
    - MINOR: quic: Align "show quic" command help information
    - CLEANUP: quic: Indentation fix quic_rx_pkt_retrieve_conn()
    - CLEANUP: quic: Useless tests in qc_rx_pkt_handle()
    - MINOR: quic: Add some counters at QUIC connection level
    - MINOR: quic: Add a counter for sent packets
    - MINOR: hlua: hlua_smp2lua_str() may LJMP
    - MINOR: hlua: hlua_smp2lua() may LJMP
    - MINOR: hlua: hlua_arg2lua() may LJMP
    - DOC: hlua: document hlua_lua2arg() function
    - DOC: hlua: document hlua_lua2smp() function
    - BUG/MINOR: hlua: unsafe hlua_lua2smp() usage
    - BUILD: makefile: commit the tiny FreeBSD makefile stub
    - BUILD: makefile: fix build options when building tools first
    - BUILD: ist: do not put a cast in an array declaration
    - BUILD: ist: use the literal declaration for ist_lc/ist_uc under TCC
    - BUILD: compiler: systematically set USE_OBSOLETE_LINKER with TCC
    - DOC: install: update reference to known supported versions
    - SCRIPTS: publish-release: update the umask to keep group write access

2 years agoSCRIPTS: publish-release: update the umask to keep group write access
Willy Tarreau [Wed, 24 May 2023 20:48:14 +0000 (22:48 +0200)] 
SCRIPTS: publish-release: update the umask to keep group write access

This is to avoid the occasional error that arises when a release is
first done by another maintainer.

2 years agoDOC: install: update reference to known supported versions
Willy Tarreau [Wed, 24 May 2023 20:32:46 +0000 (22:32 +0200)] 
DOC: install: update reference to known supported versions

Gcc 13 is known to work, OpenSSL 3.1 and wolfSSL as well. Add a few
hints about build errors when using QUIC + OpenSSL and warnings about
the dramatic OpenSSL 3.x performance regression.

2 years agoBUILD: compiler: systematically set USE_OBSOLETE_LINKER with TCC
Willy Tarreau [Wed, 24 May 2023 19:37:06 +0000 (21:37 +0200)] 
BUILD: compiler: systematically set USE_OBSOLETE_LINKER with TCC

TCC silently ignores the weak and section attributes, which ruins the
initcalls. Technically we're exactly in the same situation as with an
obsolete linker. Let's just automatically set the flag if TCC is
detected, this avoids surprises where the program compiles but does
not start.

No backport is needed.

2 years agoBUILD: ist: use the literal declaration for ist_lc/ist_uc under TCC
Willy Tarreau [Wed, 24 May 2023 19:31:21 +0000 (21:31 +0200)] 
BUILD: ist: use the literal declaration for ist_lc/ist_uc under TCC

TCC doesn't knoow about __attribute__((weak)), it silently ignores it.
We could add a "static" modifier there in this case but we already have
an alternate portable mode that is based on a slightly larger literal
for obsolete linkers (and non-ELF systems) which choke on weak. Let's
just add the test for tcc there and use it in this case.

No backport is needed.

2 years agoBUILD: ist: do not put a cast in an array declaration
Willy Tarreau [Wed, 24 May 2023 19:27:39 +0000 (21:27 +0200)] 
BUILD: ist: do not put a cast in an array declaration

TCC is upset by the declaration looking like:

  const unsigned char ist_lc[256] __attribute__((weak)) = ((const unsigned char[256]){ ... });

It was written like this because it's expanded from the _IST_LC macro
but it's never used as-is, it's only used from ist_lc, which should be
the one containing the cast so that the macro only contains the list of
bytes that can be used in both places. And this assigns more consistent
roles to the lower and upper case macro/variable now, one is typed and
the other one not. No backport is needed.

2 years agoBUILD: makefile: fix build options when building tools first
Willy Tarreau [Wed, 24 May 2023 15:23:45 +0000 (17:23 +0200)] 
BUILD: makefile: fix build options when building tools first

Due to the test on the target introduced by commit 9577a152b ("BUILD:
makefile: do not erase build options for some build options"), if a
tool (e.g. halog) is build first before haproxy after a clean or a
fresh source extraction, the .build_opts file does not exist and
"make" complains since there's no such target. Make sure to define
the empty target for all "else" blocks there. No backport is needed.

2 years agoBUILD: makefile: commit the tiny FreeBSD makefile stub
Willy Tarreau [Wed, 24 May 2023 15:06:30 +0000 (17:06 +0200)] 
BUILD: makefile: commit the tiny FreeBSD makefile stub

The idea here is to try to detect the use of "make" instead of "gmake"
on FreeBSD. After having long tried, there's no way to construct a
condition that is common to both makefile languages and could serve as
a differentiator since there's simply no common word between the two
languages. However on FreeBSD (the main used BSD platform), "make" is
configured to look for BSDmakefile before the other ones. It allows us
to intercept it and explain to use gmake with an example of a roughly
converted make command line (we just strip "-J xx,xx" that systematically
gets inserted if "-j" is used). A few tricks are used, such as creating
a dummy target on the fly based on the requested one just to silence the
output, and always match "all" since it's used by default when no target
is specified. .DEFAULTS was initially used but finally dropped thanks to
this.

For example:

  $ make -j$(getconf NPROCESSORS_ONLN) TARGET=freebsd USE_OPENSSL=1
  Please use GNU make instead. It is often called gmake.
  Example:
    gmake  -j 4 TARGET=freebsd USE_OPENSSL=1  all

It will often be sufficient to permit a copy-paste and to try again.
Note that the .gitignore was updated.

2 years agoBUG/MINOR: hlua: unsafe hlua_lua2smp() usage
Aurelien DARRAGON [Wed, 17 May 2023 14:06:11 +0000 (16:06 +0200)] 
BUG/MINOR: hlua: unsafe hlua_lua2smp() usage

Fixing hlua_lua2smp() usage in hlua's code since it was assumed that
hlua_lua2smp() makes a standalone smp out of lua data, but it is not
the case.

This is especially true when dealing with lua strings (string is
extracted using lua_tolstring() which returns a pointer to lua string
memory location that may be reclaimed by lua at any time when no longer
used from lua's point of view). Thus, smp generated by hlua_lua2smp() may
only be used from the lua context where the call was initially made, else
it should be explicitly duplicated before exporting it out of lua's
context to ensure safe (standalone) usage.

This should be backported to all stable versions.

2 years agoDOC: hlua: document hlua_lua2smp() function
Aurelien DARRAGON [Wed, 17 May 2023 13:44:45 +0000 (15:44 +0200)] 
DOC: hlua: document hlua_lua2smp() function

Add some developer notes to hlua_lua2smp() function description since
it lacks some important infos, including a critical usage restriction.

2 years agoDOC: hlua: document hlua_lua2arg() function
Aurelien DARRAGON [Wed, 17 May 2023 13:33:59 +0000 (15:33 +0200)] 
DOC: hlua: document hlua_lua2arg() function

Add some developer notes to hlua_lua2arg() function description since
it lacks some important infos, including an usage restriction.

2 years agoMINOR: hlua: hlua_arg2lua() may LJMP
Aurelien DARRAGON [Wed, 17 May 2023 08:51:50 +0000 (10:51 +0200)] 
MINOR: hlua: hlua_arg2lua() may LJMP

Add LJMP hint to hlua_arg2lua() prototype since it relies on
functions (e.g.: lua_pushlstring()) which may raise lua memory errors.

2 years agoMINOR: hlua: hlua_smp2lua() may LJMP
Aurelien DARRAGON [Wed, 17 May 2023 08:44:47 +0000 (10:44 +0200)] 
MINOR: hlua: hlua_smp2lua() may LJMP

Add LJMP hint to hlua_smp2lua() prototype since it relies on
functions (e.g.: lua_pushstring()) which may raise lua memory errors.

2 years agoMINOR: hlua: hlua_smp2lua_str() may LJMP
Aurelien DARRAGON [Wed, 17 May 2023 08:38:50 +0000 (10:38 +0200)] 
MINOR: hlua: hlua_smp2lua_str() may LJMP

Add LJMP hint to hlua_smp2lua_str() prototype since it relies on
functions (e.g.: lua_pushstring()) which may raise lua memory errors.

2 years agoMINOR: quic: Add a counter for sent packets
FrĂ©dĂ©ric LĂ©caille [Wed, 24 May 2023 13:55:14 +0000 (15:55 +0200)] 
MINOR: quic: Add a counter for sent packets

Add ->sent_pkt counter to quic_conn struct to count the packet at QUIC connection
level. Then, when the connection is released, the ->sent_pkt counter value
is added to the one for the listener.

Must be backported to 2.7.

2 years agoMINOR: quic: Add some counters at QUIC connection level
FrĂ©dĂ©ric LĂ©caille [Wed, 24 May 2023 09:10:19 +0000 (11:10 +0200)] 
MINOR: quic: Add some counters at QUIC connection level

Add some statistical counters to quic_conn struct from quic_counters struct which
are used at listener level to handle them at QUIC connection level. This avoid
calling atomic functions. Furthermore this will be useful soon when a counter will
be added for the total number of packets which have been sent which will be very
often incremented.

Some counters were not added, espcially those which count the number of QUIC errors
by QUIC error types. Indeed such counters would be incremented most of the time
only one time at QUIC connection level.

Implement quic_conn_prx_cntrs_update() which accumulates the QUIC connection level
statistical counters to the listener level statistical counters.

Must be backported to 2.7.

2 years agoCLEANUP: quic: Useless tests in qc_rx_pkt_handle()
FrĂ©dĂ©ric LĂ©caille [Wed, 24 May 2023 08:24:42 +0000 (10:24 +0200)] 
CLEANUP: quic: Useless tests in qc_rx_pkt_handle()

There is no reason to test <qc> nullity at the end of this function because it is
clearly not null, furthermore the trace handle the case where <qc> is null.

Must be backported to 2.7.

2 years agoCLEANUP: quic: Indentation fix quic_rx_pkt_retrieve_conn()
FrĂ©dĂ©ric LĂ©caille [Wed, 24 May 2023 07:06:06 +0000 (09:06 +0200)] 
CLEANUP: quic: Indentation fix quic_rx_pkt_retrieve_conn()

Add missing spaces.

Must be backported to 2.7.

2 years agoMINOR: quic: Align "show quic" command help information
FrĂ©dĂ©ric LĂ©caille [Tue, 23 May 2023 09:36:49 +0000 (11:36 +0200)] 
MINOR: quic: Align "show quic" command help information

Align the "show quic" help information with all the others command help information.
Furthermore, makes this information match the management documentation.

Must be backported to 2.7.

2 years agoBUG/MINOR: quic: Missing Retry token length on receipt
FrĂ©dĂ©ric LĂ©caille [Mon, 15 May 2023 16:11:21 +0000 (18:11 +0200)] 
BUG/MINOR: quic: Missing Retry token length on receipt

quic_retry_token_check() must decipher the token sent to and received back from
clients. This token is made of the token format byte, the ODCID prefixed by its one byte
length, the timestamp of its creation, and terminated by an AEAD TAG followed
by the salt used to derive the secret to cipher the token.

So, the length of these data must be between
2 + QUIC_ODCID_MINLEN + sizeof(uint32_t) + QUIC_TLS_TAG_LEN + QUIC_RETRY_TOKEN_SALTLEN
and
2 + QUIC_CID_MAXLEN + sizeof(uint32_t) + QUIC_TLS_TAG_LEN + QUIC_RETRY_TOKEN_SALTLEN.

Must be backported to 2.7 and 2.6.

2 years agoBUG/MINOR: quic: Wrong token length check (quic_generate_retry_token())
FrĂ©dĂ©ric LĂ©caille [Mon, 15 May 2023 15:40:00 +0000 (17:40 +0200)] 
BUG/MINOR: quic: Wrong token length check (quic_generate_retry_token())

This bug would never occur because the buffer supplied to quic_generate_retry_token()
to build a Retry token is large enough to embed such a token. Anyway, this patch
fixes quic_generate_retry_token() implementation.

There were two errors: this is the ODCID which is added to the token. Furthermore
the timestamp was not taken into an account.

Must be backported to 2.6 and 2.7.

2 years agoMINOR: quic: Add low level traces (addresses, DCID)
FrĂ©dĂ©ric LĂ©caille [Fri, 12 May 2023 15:37:29 +0000 (17:37 +0200)] 
MINOR: quic: Add low level traces (addresses, DCID)

Add source and destination addresses to QUIC_EV_CONN_RCV trace event. This is
used by datagram/socket level functions (quic_sock.c).

Must be backported to 2.7.

2 years agoBUILD: makefile: do not erase build options for some build options
Willy Tarreau [Wed, 24 May 2023 14:18:39 +0000 (16:18 +0200)] 
BUILD: makefile: do not erase build options for some build options

One painfully annoying thing with the build options change detection
is that they get rebuild for about everything except when the build
target is exactly "reg-tests". But in practice every time reg tests
are run we end up having to experience a full rebuild because the
reg-tests script runs "make version" which is sufficient to refresh
the file.

There are two issues here. The first one is that we ought to skip all
targets that do not make use of the build options. This includes all
the tools such as "flags" for example, or utility targets like "tags",
"help" or "version". The second issue is that with most of these extra
targets we do not set the TARGET variable, and that one is used when
creating the build_opts file, so let's preserve the file when TARGET
is not set.

Now it's possible to re-run a make after a make reg-tests without having
to rebuild the whole project.

2 years agoCLEANUP: makefile: don't display a dummy features list without a target
Willy Tarreau [Wed, 24 May 2023 13:59:04 +0000 (15:59 +0200)] 
CLEANUP: makefile: don't display a dummy features list without a target

"make help" ends with a list of enabled/disabled features for TARGET '',
which makes no sense. Let's only display enabled/disabled features when
a target is set. It also removes visual pollution when users seek help.

2 years agoDEV: add a Lua helper script for SSL keys logging
Amaury Denoyelle [Wed, 24 May 2023 14:02:17 +0000 (16:02 +0200)] 
DEV: add a Lua helper script for SSL keys logging

This script can be used through a http-request rules to log SSL keys for
traffic on a dedicated frontend. The resulting file can then be injected
into wireshark to decipher the corresponding network capture.

2 years agoBUG/MEDIUM: mux-h2: Propagate termination flags when frontend SC is created
Christopher Faulet [Wed, 24 May 2023 09:34:45 +0000 (11:34 +0200)] 
BUG/MEDIUM: mux-h2: Propagate termination flags when frontend SC is created

We must evaluate if EOS/EOI/ERR_PENDING/ERROR flags must be set on the SE
when the frontend SC is created because the rxbuf is transferred to the
steeam at this stage. It means the call to h2_rcv_buf() may be skipped on
some circumstances.

And indeed, it happens when HAproxy quickly replies, for instance because of
a deny rule. In this case, depending on the scheduling, the abort may block
the receive attempt from the SC. In this case if SE flags were not properly
set earlier, there is no way to terminate the request and the session may be
freezed.

For now, I can't explain why there is no timeout when this happens but it
remains an issue because here we should not rely on timeouts to close the
stream.

This patch relies on following commits:

    * MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
    * MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame

The issue was encountered on the 2.8 but it seems the bug exists since the
2.4. But it is probably a good idea to only backport the series to 2.7 only
and wait for a bug report on earlier versions.

This patch should solve the issue #2147.

2 years agoMINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
Christopher Faulet [Wed, 24 May 2023 09:14:38 +0000 (11:14 +0200)] 
MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE

The function h2s_propagate_term_flags() was added to check the H2S state and
evaluate when EOI/EOS/ERR_PENDING/ERROR flags must be set on the SE. It is
not the only place where those flags are set. But it centralizes the synchro
between the H2 stream and the SC.

For now, this function is only used at the end of h2_rcv_buf(). But it will
be used to fix a bug.

2 years agoMINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame
Christopher Faulet [Wed, 24 May 2023 09:02:50 +0000 (11:02 +0200)] 
MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame

The flag H2_SF_ES_RCVD is set on the H2 stream when the ES flag is found in
a frame. On HEADERS frame, it was set in function processing the frame. It
is moved in the function decoding the frame. Fundamentally, this changes
nothing. But it will be useful to have this information earlier when a
client H2 stream is created.

2 years agoBUG/MINOR: mux-h2: Check H2_SF_BODY_TUNNEL on H2S flags and not demux frame ones
Christopher Faulet [Wed, 24 May 2023 09:44:53 +0000 (11:44 +0200)] 
BUG/MINOR: mux-h2: Check H2_SF_BODY_TUNNEL on H2S flags and not demux frame ones

In h2c_frt_stream_new(), H2_SF_BODY_TUNNEL flags was tested on demux frame
flags (h2c->dff) instead of the h2s flags.  By chance, it is a noop test
becasue H2_SF_BODY_TUNNEL value, once converted to an int8_t, is 0.

It is a 2.8-specific issue. No backport needed.

2 years agoBUILD: makefile: fix build issue on GNU make < 3.82
Willy Tarreau [Wed, 24 May 2023 13:23:34 +0000 (15:23 +0200)] 
BUILD: makefile: fix build issue on GNU make < 3.82

Thierry Fournier reported a build breakage with the ubiquitous make
3.81, LDFLAGS were ignored. This is caused by the declaration of the
collect_opt_flags macro that is defined with an "=" sign, something
that only appeared in 3.82 and that is not necessary. With it removed,
the build now works fine at least from 3.80 to 4.3.

No backport is needed since this makefile cleanup appeared in 2.8.

2 years agoMINOR: mux-quic: report error on stream-endpoint earlier
Amaury Denoyelle [Wed, 24 May 2023 12:43:43 +0000 (14:43 +0200)] 
MINOR: mux-quic: report error on stream-endpoint earlier

A RESET_STREAM is emitted in several occasions :
- protocol error during HTTP/3.0 parsing
- STOP_SENDING reception

In both cases, if a stream-endpoint is attached we must set its ERR
flag. This was correctly done but after some delay as it was only when
the RESET_STREAM was emitted. Change this to set the ERR flag as soon as
one of the upper cases has been encountered. This should help to release
faster streams in error.

This should be backported up to 2.7.

2 years agoMINOR: mux-quic: only set EOS on RESET_STREAM recv
Amaury Denoyelle [Wed, 24 May 2023 08:49:44 +0000 (10:49 +0200)] 
MINOR: mux-quic: only set EOS on RESET_STREAM recv

A recent review was done to rationalize ERR/EOS/EOI flags on stream
endpoint. A common definition for both H1/H2/QUIC mux have been written
in the following documentation :
 ./doc/internals/stconn-close.txt

In QUIC it is possible to close each channels of a stream independently
with RESET_STREAM and STOP_SENDING frames. When a RESET_STREAM is
received, it indicates that the peer has ended its transmission in an
abnormal way. However, it is still ready to receive.

Previously, on RESET_STREAM reception, QUIC MUX set the ERR flag on
stream-endpoint. However, according to the QUIC mechanism, it should be
instead EOS but this was impossible due to a BUG_ON() which prevents EOS
without EOI or ERR. This BUG_ON was only present because this case was
never used before the introduction of QUIC. It was removed in a recent
commit which allows us to now properly set EOS alone on RESET_STREAM
reception.

In practice, this change allows to continue to send data even after
RESET_STREAM reception. However, currently browsers always emit it with
a STOP_SENDING as this is used to abort the whole H3 streams. In the end
this will result in a stream-endpoint with EOS and ERR_PENDING/ERR
flags.

This should be backported up to 2.7.

2 years agoMINOR: mux-quic: set both EOI EOS for stream fin
Amaury Denoyelle [Wed, 24 May 2023 08:48:52 +0000 (10:48 +0200)] 
MINOR: mux-quic: set both EOI EOS for stream fin

A recent review was done to rationalize ERR/EOS/EOI flags on stream
endpoint. A common definition for both H1/H2/QUIC mux have been written
in the following documentation :
 ./doc/internals/stconn-close.txt

Always set EOS with EOI flag to conform to this specification. EOI is
set whenever the proper stream end has been encountered : with QUIC it
corresponds to a STREAM frame with FIN bit. At this step, RESET_STREAM
frames are ignored by QUIC MUX as allowed by RFC 9000. This means we can
always set EOS at the same time with EOI.

This should be backported up to 2.7.

2 years agoBUILD: quic: re-enable chacha20_poly1305 for libressl
Ilya Shipitsin [Sun, 21 May 2023 10:51:46 +0000 (12:51 +0200)] 
BUILD: quic: re-enable chacha20_poly1305 for libressl

this reverts d2be9d4c48b71b2132938dbfac36142cc7b8f7c4

LibreSSL implements EVP_chacha20_poly1305() with EVP_CIPHER for every
released version starting with 3.6.0

2 years agoDOC/MINOR: config: Fix typo in description for `ssl_bc` in configuration.txt
Mariam John [Mon, 22 May 2023 18:11:13 +0000 (13:11 -0500)] 
DOC/MINOR: config: Fix typo in description for `ssl_bc` in configuration.txt

Fix a minor typo in the description of the `ssl_bc` sample fetch method described under
Section `7.3.4. Fetching samples at Layer 5` in configuration.txt. Changed `other` to `to`.

2 years agoDOC: internal: add a bit of documentation for the stconn closing conditions
Willy Tarreau [Tue, 23 May 2023 13:59:19 +0000 (15:59 +0200)] 
DOC: internal: add a bit of documentation for the stconn closing conditions

The conditions where ERR, EOS and EOI are found are not always
crystal clear, and the fact that there's still a good bunch of
original ones dating from the early days and that seem to test for
non-existing cases doesn't help either.

After auditing the code base and projecting the 3 main muxes' stream
termination conditions, with Christopher and Amaury we could establish
the current flags matrix which indicates both what each combination
means for each mux and when it is set by each of them (or not set and
for what reason).

It should be sufficient to void doubts when adding code or when chasing
a bug.

It *must not* be backported because it is highly specific to the latest
2.8-dev.

2 years agoMEDIUM: stconn: make the SE_FL_ERR_PENDING to ERROR transition systematic
Willy Tarreau [Tue, 23 May 2023 14:08:22 +0000 (16:08 +0200)] 
MEDIUM: stconn: make the SE_FL_ERR_PENDING to ERROR transition systematic

During a code audit of the various situations that promote ERR_PENDING to
ERROR, it appeared that:
  - all muxes use se_fl_set_error() to set it, which chooses either based
    on EOI/EOS presence ;
  - EOI/EOS that arrive late after ERR_PENDING were not systematically
    upgraded to ERROR

This results in confusion about how such ERROR or ERR_PENDING ought to
be handled, which is not quite desirable.

This patch adds a test to se_fl_set() to detect if we're setting EOI or
EOS while ERR_PENDING is present, or the other way around so that any
sequence of EOI/EOS <-> ERR_PENDING results in ERROR being set. This
way there will no longer be possible situations where ERROR is missing
while the other ones are set.

2 years agoMEDIUM: stconn/applet: Allow SF_SL_EOS flag alone
Christopher Faulet [Tue, 23 May 2023 13:13:40 +0000 (15:13 +0200)] 
MEDIUM: stconn/applet: Allow SF_SL_EOS flag alone

During the refactoring on SC/SE flags, it was stated that SE_FL_EOS flag
should not be set without on of SE_FL_EOI or SE_FL_ERROR flags. In fact, it
is a problem for the QUIC/H3 multiplexer. When a RST_STREAM frame is
received, it means no more data will be received from the peer. And this
happens before the end of the message (RST_STREAM frame received after the
end of the message are ignored). At this stage, it is a problem to report an
error because from the QUIC point of view, it is valid. Data may still be
sent to the peer. If an error is reported, this will stop the data sending
too.

In the same idea, the H1 mulitplexer reports an error when the message is
truncated because of a read0. But only an EOS flag should be reported in
this case, not an error. Fundamentally, it is important to distinguish
errors from shuts for reads because some cases are valid. For instance a H1
client can choose to stop uploading data if it received the server response.

So, relax tests on SE flags by removing BUG_ON_HOT() on SE_FL_EOS flag. For
now, the abort will be handled in the HTTP analyzers.

2 years agoMINOR: quic: fix alignment of oneline show quic
Amaury Denoyelle [Mon, 22 May 2023 08:57:56 +0000 (10:57 +0200)] 
MINOR: quic: fix alignment of oneline show quic

Output of 'show quic' CLI in oneline mode was not correctly done. This
was caused both due to differing qc pointer size and ports length. Force
proper alignment by using maximum sizes as expected and complete with
blanks if needed.

This should be backported up to 2.7.

2 years agoBUG/MINOR: quic: handle Tx packet allocation failure properly
Amaury Denoyelle [Wed, 19 Apr 2023 13:56:30 +0000 (15:56 +0200)] 
BUG/MINOR: quic: handle Tx packet allocation failure properly

qc_prep_app_pkts() is responsible to built several new packets for
sending. It can fail due to memory allocation error. Before this patch,
the Tx buffer was released on error even if some packets were properly
generated.

With this patch, if an error happens on qc_prep_app_pkts(), we still try
to send already built packets if Tx buffer is not empty. The sending
loop is then interrupted and the Tx buffer is released with data
cleared.

This should be backported up to 2.7.

2 years agoMINOR: quic: use WARN_ON for encrypt failures
Amaury Denoyelle [Tue, 16 May 2023 16:23:37 +0000 (18:23 +0200)] 
MINOR: quic: use WARN_ON for encrypt failures

It is expected that quic_packet_encrypt() and
quic_apply_header_protection() never fails as encryption is done in
place. This allows to remove their return value.

This is useful to simplify error handling on sending path. An error can
only be encountered on the first steps when allocating a new packet or
copying its frame content. After a clear packet is successfully built,
no error is expected on encryption.

However, it's still unclear if our assumption that in-place encryption
function never fail. As such, a WARN_ON() statement is used if an error
is detected at this stage. Currently, it's impossible to properly manage
this without data loss as this will leave partially unencrypted data in
the send buffer. If warning are reported a solution will have to be
implemented.

This should be backported up to 2.7.

2 years agoMINOR: quic: remove return val of quic_aead_iv_build()
Amaury Denoyelle [Tue, 16 May 2023 16:11:01 +0000 (18:11 +0200)] 
MINOR: quic: remove return val of quic_aead_iv_build()

quic_aead_iv_build() should never fail unless we call it with buffers of
different size. This never happens in the code as every input buffers
are of size QUIC_TLS_IV_LEN.

Remove the return value and add a BUG_ON() to prevent future misusage.
This is especially useful to remove one error handling on the sending
patch via quic_packet_encrypt().

This should be backported up to 2.7.

2 years agoCLEANUP: mux-quic/h3: complete BUG_ON with comments
Amaury Denoyelle [Thu, 11 May 2023 14:55:30 +0000 (16:55 +0200)] 
CLEANUP: mux-quic/h3: complete BUG_ON with comments

Complete each useful BUG_ON statements with a comment to explain its
purpose. Also convert BUG_ON_HOT to BUG_ON as they should not have a
big impact.

This should be backported up to 2.7.

2 years agoDOC: add size format section to manual
Daniel Epperson [Mon, 15 May 2023 19:45:27 +0000 (12:45 -0700)] 
DOC: add size format section to manual

The manual refers to an HAProxy size format but does not define it.
This patch adds a section to the manual to define the HAProxy size
format.

2 years ago[RELEASE] Released version 2.8-dev12 v2.8-dev12
Christopher Faulet [Wed, 17 May 2023 15:10:12 +0000 (17:10 +0200)] 
[RELEASE] Released version 2.8-dev12

Released version 2.8-dev12 with the following main changes :
    - BUILD: mjson: Fix warning about unused variables
    - MINOR: spoe: Don't stop disabled proxies
    - BUG/MEDIUM: filters: Don't deinit filters for disabled proxies during startup
    - BUG/MINOR: hlua_fcn/queue: fix broken pop_wait()
    - BUG/MINOR: hlua_fcn/queue: fix reference leak
    - CLEANUP: hlua_fcn/queue: make queue:push() easier to read
    - BUG/MINOR: quic: Buggy acknowlegments of acknowlegments function
    - DEBUG: list: add DEBUG_LIST to purposely corrupt list heads after delete
    - MINOR: stats: report the total number of warnings issued
    - MINOR: stats: report the number of times the global maxconn was reached
    - BUG/MINOR: mux-quic: do not prevent shutw on error
    - BUG/MINOR: mux-quic: do not free frame already released by quic-conn
    - BUG/MINOR: mux-quic: no need to subscribe for detach streams
    - MINOR: mux-quic: add traces for stream wake
    - MINOR: mux-quic: do not send STREAM frames if already subscribe
    - MINOR: mux-quic: factorize send subscribing
    - MINOR: mux-quic: simplify return path of qc_send()
    - MEDIUM: quic: streamline error notification
    - MEDIUM: mux-quic: adjust transport layer error handling
    - MINOR: stats: report the listener's protocol along with the address in stats
    - BUG/MEDIUM: mux-fcgi: Never set SE_FL_EOS without SE_FL_EOI or SE_FL_ERROR
    - BUG/MEDIUM: mux-fcgi: Don't request more room if mux is waiting for more data
    - MINOR: stconn: Add a cross-reference between SE descriptor
    - BUG/MINOR: proxy: missing free in free_proxy for redirect rules
    - MINOR: proxy: add http_free_redirect_rule() function
    - BUG/MINOR: http_rules: fix errors paths in http_parse_redirect_rule()
    - CLEANUP: http_act: use http_free_redirect_rule() to clean redirect act
    - MINOR: tree-wide: use free_acl_cond() where relevant
    - CLEANUP: acl: discard prune_acl_cond() function
    - BUG/MINOR: cli: don't complain about empty command on empty lines
    - MINOR: cli: add an option to display the uptime in the CLI's prompt
    - MINOR: master/cli: also implement the timed prompt on the master CLI
    - MINOR: cli: make "show fd" identify QUIC connections and listeners
    - MINOR: httpclient: allow to disable the DNS resolvers of the httpclient
    - BUILD: debug: fix build issue on 32-bit platforms in "debug dev task"
    - MINOR: ncbuf: missing malloc checks in standalone code
    - DOC: lua: fix core.{proxies,frontends,backends} visibility
    - EXAMPLES: fix race condition in lua mailers script
    - BUG/MINOR: errors: handle malloc failure in usermsgs_put()
    - BUG/MINOR: log: fix memory error handling in parse_logsrv()
    - BUG/MINOR: quic: Wrong redispatch for external data on connection socket
    - MINOR: htx: add function to set EOM reliably
    - MINOR: mux-quic: remove dedicated function to handle standalone FIN
    - BUG/MINOR: mux-quic: properly handle buf alloc failure
    - BUG/MINOR: mux-quic: handle properly recv ncbuf alloc failure
    - BUG/MINOR: quic: do not alloc buf count on alloc failure
    - BUG/MINOR: mux-quic: differentiate failure on qc_stream_desc alloc
    - BUG/MINOR: mux-quic: free task on qc_init() app ops failure
    - MEDIUM: session/ssl: return the SSL error string during a SSL handshake error
    - CI: enable monthly Fedora Rawhide clang builds
    - MEDIUM: mworker/cli: does not disconnect the master CLI upon error
    - MINOR: stconn: Remove useless test on sedesc on detach to release the xref
    - MEDIUM: proxy: stop emitting logs for internal proxies when stopping
    - MINOR: ssl: add new sample ssl_c_r_dn
    - BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout
    - BUILD: ssl: ssl_c_r_dn fetches uses  functiosn only available since 1.1.1
    - BUG/MINOR: mux-quic: handle properly Tx buf exhaustion
    - BUG/MINOR: h3: missing goto on buf alloc failure
    - BUILD: ssl: get0_verified chain is available on libreSSL
    - BUG/MINOR: makefile: use USE_LIBATOMIC instead of USE_ATOMIC
    - MINOR: mux-quic: add trace to stream rcv_buf operation
    - MINOR: mux-quic: properly report end-of-stream on recv
    - MINOR: mux-quic: uninline qc_attach_sc()
    - BUG/MEDIUM: mux-quic: fix EOI for request without payload
    - MINOR: checks: make sure spread-checks is used also at boot time
    - BUG/MINOR: tcp-rules: Don't shortened the inspect-delay when EOI is set
    - REGTESTS: log: Reduce response inspect-delay for last_rule.vtc
    - DOC: config: Clarify conditions to shorten the inspect-delay for TCP rules
    - CLEANUP: server: remove useless tmptrash assigments in srv_update_status()
    - BUG/MINOR: server: memory leak in _srv_update_status_op() on server DOWN
    - CLEANUP: check; Remove some useless assignments to NULL
    - CLEANUP: stats: update the trash chunk where it's used
    - MINOR: clock: measure the total boot time
    - MINOR: stats: report the boot time in "show info"
    - BUG/MINOR: checks: postpone the startup of health checks by the boot time
    - MINOR: clock: provide a function to automatically adjust now_offset
    - BUG/MINOR: clock: automatically adjust the internal clock with the boot time
    - CLEANUP: fcgi-app; Remove useless assignment to NULL
    - REGTESTS: log: Reduce again response inspect-delay for last_rule.vtc
    - CI: drop Fedora m32 pipeline in favour of cross matrix
    - MEDIUM: checks: Stop scheduling healthchecks during stopping stage
    - MEDIUM: resolvers: Stop scheduling resolution during stopping stage
    - BUG/MINOR: hlua: SET_SAFE_LJMP misuse in hlua_event_runner()
    - BUG/MINOR: debug: fix pointer check in debug_parse_cli_task()

2 years agoBUG/MINOR: debug: fix pointer check in debug_parse_cli_task()
Aurelien DARRAGON [Mon, 15 May 2023 09:59:08 +0000 (11:59 +0200)] 
BUG/MINOR: debug: fix pointer check in debug_parse_cli_task()

Task pointer check in debug_parse_cli_task() computes the theoric end
address of provided task pointer to check if it is valid or not thanks to
may_access() helper function.

However, relative ending address is calculated by adding task size to 't'
pointer (which is a struct task pointer), thus it will result to incorrect
address since the compiler automatically translates 't + x' to
't + x * sizeof(*t)' internally (with sizeof(*t) != 1 here).

Solving the issue by using 'ptr' (which is the void * raw address) as
starting address to prevent automatic address scaling.

This was revealed by coverity, see GH #2157.

No backport is needed, unless 9867987 ("DEBUG: cli: add "debug dev task"
to show/wake/expire/kill tasks and tasklets") gets backported.

2 years agoBUG/MINOR: hlua: SET_SAFE_LJMP misuse in hlua_event_runner()
Aurelien DARRAGON [Mon, 15 May 2023 16:46:44 +0000 (18:46 +0200)] 
BUG/MINOR: hlua: SET_SAFE_LJMP misuse in hlua_event_runner()

When hlua_event_runner() pauses the subscription (ie: if the consumer
can't keep up the pace), hlua_traceback() is used to get the current
lua trace (running context) to provide some info to the user.

However, as hlua_traceback() may raise an error (__LJMP) is set, it is
used within a SET_SAFE_LJMP() / RESET_SAFE_LJMP() combination to ensure
lua errors are properly handled and don't result in unexpected behavior.

But the current usage of SET_SAFE_LJMP() within the function is wrong
since hlua_traceback() will run a second time (unprotected) if the
first (protected) attempt fails. This is undefined behavior and could
even lead to crashes.

Hopefully it is very hard to trigger this code path, thus we can consider
this as a minor bug.

Also using this as an opportunity to enhance the message report to make
it more meaningful to the user.

This should fix GH #2159.

It is a 2.8 specific bug, no backport needed unless c84899c636
("MEDIUM: hlua/event_hdl: initial support for event handlers") gets
backported.

2 years agoMEDIUM: resolvers: Stop scheduling resolution during stopping stage
Christopher Faulet [Tue, 16 May 2023 16:28:57 +0000 (18:28 +0200)] 
MEDIUM: resolvers: Stop scheduling resolution during stopping stage

When the process is stopping, the server resolutions are suspended. However
the task is still periodically woken up for nothing. If there is a huge
number of resolution, it may lead to a noticeable CPU consumption for no
reason.

To avoid this extra CPU cost, we stop to schedule the the resolution tasks
during the stopping stage. Of course, it is only true for server
resolutinos. Dynamic ones, via do-resolve actions, are not concerned. These
ones must still be triggered during stopping stage.

Concretly, during the stopping stage, the resolvers task is no longer
scheduled if there is no running resolutions. In this case, if a do-resolve
action is evaluated, the task is woken up.

This patch should partially solve the issue #2145.

2 years agoMEDIUM: checks: Stop scheduling healthchecks during stopping stage
Christopher Faulet [Tue, 16 May 2023 16:07:51 +0000 (18:07 +0200)] 
MEDIUM: checks: Stop scheduling healthchecks during stopping stage

When the process is stopping, the health-checks are suspended. However the
task is still periodically woken up for nothing. If there is a huge number
of health-checks and if they are woken up in same time, it may lead to a
noticeable CPU consumption for no reason.

To avoid this extra CPU cost, we stop to schedule the health-check tasks
when the proxy is disabled or stopped.

This patch should partially solve the issue #2145.

2 years agoCI: drop Fedora m32 pipeline in favour of cross matrix
Ilya Shipitsin [Sun, 14 May 2023 19:40:20 +0000 (21:40 +0200)] 
CI: drop Fedora m32 pipeline in favour of cross matrix

Fedora m32 monthly was introduced before cross matrix. Actually,
many of cross builds are 32 bit, no need to keep dedicated Fedora
definition

2 years agoREGTESTS: log: Reduce again response inspect-delay for last_rule.vtc
Christopher Faulet [Wed, 17 May 2023 09:08:51 +0000 (11:08 +0200)] 
REGTESTS: log: Reduce again response inspect-delay for last_rule.vtc

It was previously reduced from 10s to 1s but it remains too high, espeically
for the CI. It may be drastically reduced to 100ms. Idea is to just be sure
we will wait for the response before evaluating the TCP rules.

2 years agoCLEANUP: fcgi-app; Remove useless assignment to NULL
Christopher Faulet [Wed, 17 May 2023 07:40:14 +0000 (09:40 +0200)] 
CLEANUP: fcgi-app; Remove useless assignment to NULL

When the fcgi configuration is checked and fcgi rules are created, a useless
assignment to NULL is reported by Covertiy. Let's remove it.

This patch should fix the coverity report #2161.

2 years agoBUG/MINOR: clock: automatically adjust the internal clock with the boot time
Willy Tarreau [Tue, 16 May 2023 17:19:36 +0000 (19:19 +0200)] 
BUG/MINOR: clock: automatically adjust the internal clock with the boot time

This is a better and more general solution to the problem described in
this commit:

    BUG/MINOR: checks: postpone the startup of health checks by the boot time

Now we're updating the now_offset that is used to compute now_ms at the
few points where we update the ready date during boot. This ensures that
now_ms while being stable during all the boot process will be correct
and will start with the boot value right after the boot is finished. As
such the patch above is rolled back (we don't want to count the boot
time twice).

This must not be backported because it relies on the more flexible clock
architecture in 2.8.

2 years agoMINOR: clock: provide a function to automatically adjust now_offset
Willy Tarreau [Tue, 16 May 2023 17:01:55 +0000 (19:01 +0200)] 
MINOR: clock: provide a function to automatically adjust now_offset

Right now there's no way to enforce a specific value of now_ms upon
startup in order to compensate for the time it takes to load a config,
specifically when dealing with the health check startup. For this we'd
need to force the now_offset value to compensate for the last known
value of the current date. This patch exposes a function to do exactly
this.

2 years agoBUG/MINOR: checks: postpone the startup of health checks by the boot time
Willy Tarreau [Wed, 17 May 2023 07:01:22 +0000 (09:01 +0200)] 
BUG/MINOR: checks: postpone the startup of health checks by the boot time

When health checks are started at boot, now_ms could be off by the boot
time. In general it's not even noticeable, but with very large configs
taking up to one or even a few seconds to start, this can result in a
part of the servers' checks being scheduled slightly in the past. As
such all of them will start groupped, partially defeating the purpose of
the spread-checks setting. For example, this can cause a burst of
connections for the network, or an excess of CPU usage during SSL
handshakes, possibly even causing some timeouts to expire early.

Here in order to compensate for this, we simply add the known boot time
to the computed delay when scheduling the startup of checks. That's very
simple and particularly efficient. For example, a config with 5k servers
in 800 backends checked every 5 seconds, that was taking 3.8 seconds to
start used to show this distribution of health checks previously despite
the spread-checks 50:

   3690 08:59:25
    417 08:59:26
    213 08:59:27
     71 08:59:28
    428 08:59:29
    860 08:59:30
    918 08:59:31
    938 08:59:32
   1124 08:59:33
    904 08:59:34
    647 08:59:35
    890 08:59:36
    973 08:59:37
    856 08:59:38
    893 08:59:39
    154 08:59:40

Now with the fix it shows this:
    470 08:59:59
    929 09:00:00
    896 09:00:01
    937 09:00:02
    854 09:00:03
    827 09:00:04
    906 09:00:05
    863 09:00:06
    913 09:00:07
    873 09:00:08
    162 09:00:09

This should be backported to all supported versions. It depends on
this commit:

    MINOR: clock: measure the total boot time

For 2.8 where the internal clock is now totally independent on the human
one, an more generic fix will consist in simply updating now_ms to reflect
the startup time.

2 years agoMINOR: stats: report the boot time in "show info"
Willy Tarreau [Wed, 17 May 2023 07:05:20 +0000 (09:05 +0200)] 
MINOR: stats: report the boot time in "show info"

Just like we have the uptime in "show info", let's add the boot time.
It's trivial to collect as it's just the difference between the ready
date and the start date, and will allow users to monitor this element
in order to take action before it starts becoming problematic. Here
the boot time is reported in milliseconds, so this allows to even
observe sub-second anomalies in startup delays.

2 years agoMINOR: clock: measure the total boot time
Willy Tarreau [Wed, 17 May 2023 07:02:21 +0000 (09:02 +0200)] 
MINOR: clock: measure the total boot time

Some huge configs take a significant amount of time to start and this
can cause some trouble (e.g. health checks getting delayed and grouped,
process not responding to the CLI etc). For example, some configs might
start fast in certain environments and slowly in other ones just due to
the use of a wrong DNS server that delays all libc's resolutions. Let's
first start by measuring it by keeping a copy of the most recently known
ready date, once before calling check_config_validity() and then refine
it when leaving this function. A last call is finally performed just
before deciding to split between master and worker processes, and it covers
the whole boot. It's trivial to collect and even allows to get rid of a
call to clock_update_date() in function check_config_validity() that was
used in hope to better schedule future events.

2 years agoCLEANUP: stats: update the trash chunk where it's used
Willy Tarreau [Wed, 17 May 2023 06:43:15 +0000 (08:43 +0200)] 
CLEANUP: stats: update the trash chunk where it's used

When integrating the number of warnings in "show info" in 2.8 with commit
3c4a297d2 ("MINOR: stats: report the total number of warnings issued"),
the update of the trash buffer used by the Tainted flag got displaced
lower. There's no harm for now util someone adds a new metric requiring
a call to chunk_newstr() and gets both values merged. Let's move the
call to its location now.

2 years agoCLEANUP: check; Remove some useless assignments to NULL
Christopher Faulet [Wed, 17 May 2023 07:25:32 +0000 (09:25 +0200)] 
CLEANUP: check; Remove some useless assignments to NULL

In process_chk_conn(), some assignments to NULL are useless and are reported
by Coverity as unused value. while it is harmless, these assignments can be
removed.

This patch should fix the coverity report #2158.

2 years agoBUG/MINOR: server: memory leak in _srv_update_status_op() on server DOWN
Aurelien DARRAGON [Mon, 15 May 2023 16:03:35 +0000 (18:03 +0200)] 
BUG/MINOR: server: memory leak in _srv_update_status_op() on server DOWN

When server is transitionning from UP to DOWN, a log message is generated.
e.g.: "Server backend_name/server_name is DOWN")

However since f71e064 ("MEDIUM: server: split srv_update_status() in two
functions"), the allocated buffer tmptrash which is used to prepare the
log message is not freed after it has been used, resulting in a small
memory leak each time a server goes DOWN because of an operational
change.

This is a 2.8 specific bug, no backport needed unless the above commit
gets backported.

2 years agoCLEANUP: server: remove useless tmptrash assigments in srv_update_status()
Aurelien DARRAGON [Mon, 15 May 2023 15:38:44 +0000 (17:38 +0200)] 
CLEANUP: server: remove useless tmptrash assigments in srv_update_status()

Within srv_update_status subfunctions _op() and _adm(), each time tmptrash
is freed, we assign it to NULL to ensure it will not be reused.

However, within those functions it is not very useful given that tmptrash
is never checked against NULL except upon allocation through
alloc_trash_chunk(), which happens everytime a new log message is
generated, sent, and then freed right away, so there are no code paths
that could lead to tmptrash being checked for reuse (tmptrash is
systematically overwritten since all log messages are independant from
each other).

This was raised by coverity, see GH #2162.

2 years agoDOC: config: Clarify conditions to shorten the inspect-delay for TCP rules
Christopher Faulet [Tue, 16 May 2023 06:15:12 +0000 (08:15 +0200)] 
DOC: config: Clarify conditions to shorten the inspect-delay for TCP rules

Add a sentence to state when the inspect-delay is shortened for a TCP rule.

2 years agoREGTESTS: log: Reduce response inspect-delay for last_rule.vtc
Christopher Faulet [Tue, 16 May 2023 06:04:00 +0000 (08:04 +0200)] 
REGTESTS: log: Reduce response inspect-delay for last_rule.vtc

Because of the previous fix, log/last_rule.vtc script is failing. The
inspect-delay is no longer shorten when the end of the message is
reached. Thus WAIT_END acl is trully respected. 10s is too high and hit the
Vtext timeout, making the script fails.

2 years agoBUG/MINOR: tcp-rules: Don't shortened the inspect-delay when EOI is set
Christopher Faulet [Mon, 15 May 2023 15:31:26 +0000 (17:31 +0200)] 
BUG/MINOR: tcp-rules: Don't shortened the inspect-delay when EOI is set

A regression was introduced with the commit cb59e0bc3 ("BUG/MINOR:
tcp-rules: Stop content rules eval on read error and end-of-input").  We
should not shorten the inspect-delay when the EOI flag is set on the SC.

Idea of the inspect-delay is to wait a TCP rule is matching. It is only
interrupted if an error occurs, on abort or if the peer shuts down. It is
also interrupted if the buffer is full. This last case is a bit ambiguous
and discutable. It could be good to add ACLS, like "wait_complete" and
"wait_full" to do so. But for now, we only remove the test on SC_FL_EOI
flag.

This patch must be backported to all stable versions.

2 years agoMINOR: checks: make sure spread-checks is used also at boot time
Willy Tarreau [Wed, 17 May 2023 05:39:57 +0000 (07:39 +0200)] 
MINOR: checks: make sure spread-checks is used also at boot time

This makes use of spread-checks also for the startup of the check tasks.
This provides a smoother load on startup for uneven configurations which
tend to enable only *some* servers. Below is the connection distribution
per second of the SSL checks of a config with 5k servers spread over 800
backends, with a check inter of 5 seconds:

- default:
    682 08:00:50
    826 08:00:51
    773 08:00:52
   1016 08:00:53
    885 08:00:54
    889 08:00:55
    825 08:00:56
    773 08:00:57
   1016 08:00:58
    884 08:00:59
    888 08:01:00
    491 08:01:01

- with spread-checks 50:
    437 08:01:19
    866 08:01:20
    777 08:01:21
   1023 08:01:22
   1118 08:01:23
    923 08:01:24
    641 08:01:25
    859 08:01:26
    962 08:01:27
    860 08:01:28
    929 08:01:29
    909 08:01:30
    866 08:01:31
    849 08:01:32
    114 08:01:33

- with spread-checks 50 + this patch:
    680 08:01:55
    922 08:01:56
    962 08:01:57
    899 08:01:58
    819 08:01:59
    843 08:02:00
    916 08:02:01
    896 08:02:02
    886 08:02:03
    846 08:02:04
    903 08:02:05
    894 08:02:06
    178 08:02:07

The load is much smoother from the start, this can help initial health
checks succeed when many target the same overloaded server for example.
This could be backported as it should make border-line configs more
reliable across reloads.

2 years agoBUG/MEDIUM: mux-quic: fix EOI for request without payload
Amaury Denoyelle [Fri, 12 May 2023 16:16:31 +0000 (18:16 +0200)] 
BUG/MEDIUM: mux-quic: fix EOI for request without payload

When a full message is received for a stream, MUX is responsible to set
EOI flag. This was done through rcv_buf stream callback by checking if
QCS HTX buffer contained the EOM flag.

This is not correct for HTTP without body. In this case, QCS HTX buffer
is never used. Only a local HTX buffer is used to transfer headers just
as stream endpoint is created. As such, EOI is never transmitted to the
upper layer.

If the transfer occur without any issue, this does not seem to cause any
problem. However, in case the transfer is aborted, the stream is never
released which cause a memory leak and prevent the process soft-stop.

To fix this, also check if EOM is put by application layer during
headers conversion. If true, this is transferred through a new argument
to qc_attach_sc() MUX function which is responsible to set the EOI flag.

This issue was reproduced using h2load with hundred of connections.
h2load is interrupted with a SIGINT which causes streams to never be
closed on haproxy side.

This should be backported up to 2.6.

2 years agoMINOR: mux-quic: uninline qc_attach_sc()
Amaury Denoyelle [Mon, 15 May 2023 13:17:28 +0000 (15:17 +0200)] 
MINOR: mux-quic: uninline qc_attach_sc()

Uninline and move qc_attach_sc() function to implementation source file.
This will be useful for next commit to add traces in it.

This should be backported up to 2.7.

2 years agoMINOR: mux-quic: properly report end-of-stream on recv
Amaury Denoyelle [Mon, 15 May 2023 09:31:20 +0000 (11:31 +0200)] 
MINOR: mux-quic: properly report end-of-stream on recv

MUX is responsible to put EOS on stream when read channel is closed.
This happens if underlying connection is closed or a RESET_STREAM is
received. FIN STREAM is ignored in this case.

For connection closure, simply check for CO_FL_SOCK_RD_SH.

For RESET_STREAM reception, a new flag QC_CF_RECV_RESET has been
introduced. It is set when RESET_STREAM is received, unless we already
received all data. This is conform to QUIC RFC which allows to ignore a
RESET_STREAM in this case. During RESET_STREAM processing, input buffer
is emptied so EOS can be reported right away on recv_buf operation.

This should be backported up to 2.7.

2 years agoMINOR: mux-quic: add trace to stream rcv_buf operation
Amaury Denoyelle [Mon, 15 May 2023 09:35:45 +0000 (11:35 +0200)] 
MINOR: mux-quic: add trace to stream rcv_buf operation

Add traces to render each stream transition more explicit. Also, move
ERR_PENDING to ERROR transition after other stream flags are set, as
with the MUX H2 implementation. This is purely a cosmetic change and it
should have no functional impact.

This should be backported up to 2.7.

2 years agoBUG/MINOR: makefile: use USE_LIBATOMIC instead of USE_ATOMIC
Dragan Dosen [Mon, 15 May 2023 13:13:32 +0000 (15:13 +0200)] 
BUG/MINOR: makefile: use USE_LIBATOMIC instead of USE_ATOMIC

The issue was introduced with commit c108f37c2 ("BUILD: makefile:
rework 51D to split v3/v4"), and is also related to commit b16d9b58
("BUILD: makefile: never force -latomic, set USE_LIBATOMIC instead")
where USE_ATOMIC has been replaced.

2 years agoBUILD: ssl: get0_verified chain is available on libreSSL
William Lallemand [Mon, 15 May 2023 12:42:28 +0000 (14:42 +0200)] 
BUILD: ssl: get0_verified chain is available on libreSSL

Define HAVE_SSL_get0_verified_chain when it's using libreSSL >= 3.3.6.

2 years agoBUG/MINOR: h3: missing goto on buf alloc failure
Amaury Denoyelle [Mon, 15 May 2023 07:35:59 +0000 (09:35 +0200)] 
BUG/MINOR: h3: missing goto on buf alloc failure

The following patch introduced proper error management on buffer
allocation failure :
  0abde9dee69fe151f5f181a34e0782ef840abe53
  BUG/MINOR: mux-quic: properly handle buf alloc failure

However, when decoding an empty STREAM frame with just FIN bit set, this
was not done correctly. Indeed, there is a missing goto statement in
case of a NULL buffer check.

This was reported thanks to coverity analysis. This should fix github
issue #2163.

This must be backported up to 2.6.

2 years agoBUG/MINOR: mux-quic: handle properly Tx buf exhaustion
Amaury Denoyelle [Mon, 15 May 2023 11:56:46 +0000 (13:56 +0200)] 
BUG/MINOR: mux-quic: handle properly Tx buf exhaustion

Since the following patch
  commit 6c501ed23bea953518059117e7dd19e8d6cb6bd8
  BUG/MINOR: mux-quic: differentiate failure on qc_stream_desc alloc
it is not possible to check if Tx buf allocation failed due to a
configured limit exhaustion or a simple memory failure.

This patch fixes it as the condition was inverted. Indeed, if buf_avail
is null, this means that the limit has been reached. On the contrary
case, this is a real memory alloc failure. This caused the flag
QC_CF_CONN_FULL to not be properly used and may have caused disruption
on transfer with several streams or large data.

This was detected due to an abnormal error QUIC MUX traces. Also change
in consequence trace for limit exhaustion to be more explicit.

This must be backported up to 2.6.

2 years agoBUILD: ssl: ssl_c_r_dn fetches uses functiosn only available since 1.1.1
William Lallemand [Mon, 15 May 2023 10:05:55 +0000 (12:05 +0200)] 
BUILD: ssl: ssl_c_r_dn fetches uses  functiosn only available since 1.1.1

Fix the openssl build with older openssl version by disabling the new
ssl_c_r_dn fetch.

This also disable the ssl_client_samples.vtc file for OpenSSL version
older than 1.1.1

2 years agoBUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout
Willy Tarreau [Mon, 15 May 2023 09:28:48 +0000 (11:28 +0200)] 
BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout

Christopher found as part of the analysis of Tim's issue #1891 that commit
15a4733d5 ("BUG/MEDIUM: mux-h2: make use of http-request and keep-alive
timeouts") introduced in 2.6 incompletely addressed a timeout issue in the
H2 mux. The problem was that the http-keepalive and http-request timeouts
were not applied before it. With that commit they are now considered, but
if a GOAWAY is sent (or even attempted to be sent), then they are not used
anymore again, because the way the code is arranged consists in applying
the client-fin timeout (if set) to the current date, and falling back to
the client timeout, without considering the idle_start period. This means
that a config having a "timeout http-keepalive" would still not close the
connection quickly when facing a client that periodically sends PING,
PRIORITY or whatever other frame types.

In addition, after the GOAWAY was attempted to be sent, there was no check
for pending data in the output buffer, meaning that it would be possible
to truncate some responses in configs involving a very short client-fin
timeout.

Finally the spreading of the closures during the soft-stop brought in 2.6
by commit b5d968d9b ("MEDIUM: global: Add a "close-spread-time" option to
spread soft-stop on time window") didn't consider the particular case of
an idle "pre-connect" connection, which would also live long if a browser
failed to deliver a valid request for a long time.

All of this indicates that the conditions must be reworked so as not to
have that level of exclusion between conditions, but rather stick to the
rules from the doc that are already enforced on other muxes:
  - timeout client always applies if there are data pending, and is
    relative to each new I/O ;
  - timeout http-request applies before the first complete request and
    is relative to the entry in idle state ;
  - timeout http-keepalive applies between idle and the next complete
    request and is relative to the entry in idle state ;
  - timeout client-fin applies when in idle after a shut was sent (here
    the shut is the GOAWAY). The shut may only be considered as sent if
    the buffer is empty and the flags indicate that it was successfully
    sent (or failed) but not if it's still waiting for some room in the
    output buffer for example. This implies that this timeout may then
    lower the http-keepalive/http-request ones.

This is what this patch implements. Of course the client timeout still
applies as a fallback when all the ones above are not set or when their
conditions are not met.

It would seem reasoanble to backport this to 2.7 first, then only after
one or two releases to 2.6.

2 years agoMINOR: ssl: add new sample ssl_c_r_dn
Abhijeet Rastogi [Sun, 14 May 2023 03:04:45 +0000 (20:04 -0700)] 
MINOR: ssl: add new sample ssl_c_r_dn

This patch addresses #1514, adds the ability to fetch DN of the root
ca that was in the chain when client certificate was verified during SSL
handshake.

2 years agoMEDIUM: proxy: stop emitting logs for internal proxies when stopping
William Lallemand [Sun, 14 May 2023 21:23:36 +0000 (23:23 +0200)] 
MEDIUM: proxy: stop emitting logs for internal proxies when stopping

The HTTPCLIENT and the OCSP-UPDATE proxies are internal proxies, we
don't need to display logs of them stopping during the stopping of the
process.

This patch checks if a proxy has the flag PR_CAP_INT so it doesn't
display annoying messages.

2 years agoMINOR: stconn: Remove useless test on sedesc on detach to release the xref
Christopher Faulet [Mon, 15 May 2023 07:53:29 +0000 (09:53 +0200)] 
MINOR: stconn: Remove useless test on sedesc on detach to release the xref

When the SC is detached from the endpoint, the xref between the endpoints is
removed. At this stage, the sedesc cannot be undefined. So we can remove the
test on it.

This issue should fix the issue #2156. No backport needed.

2 years agoMEDIUM: mworker/cli: does not disconnect the master CLI upon error
William Lallemand [Sun, 14 May 2023 16:36:00 +0000 (18:36 +0200)] 
MEDIUM: mworker/cli: does not disconnect the master CLI upon error

In the proxy CLI analyzer, when pcli_parse_request returns -1, the
client was shut to prevent any problem with the master CLI.

This behavior is a little bit excessive and not handy at all in prompt
mode. For example one could have activated multiples mode, then have an
error which disconnect the CLI, and they would have to reconnect and
enter all the modes again.

This patch introduces the pcli_error() function, which only output an
error and flush the input buffer, instead of closing everything.

When encountering a parsing error, this function is used, and the prompt
is written again, without any disconnection.

2 years agoCI: enable monthly Fedora Rawhide clang builds
Ilya Shipitsin [Fri, 12 May 2023 17:26:49 +0000 (19:26 +0200)] 
CI: enable monthly Fedora Rawhide clang builds

that was temporarily disabled due to
https://github.com/haproxy/haproxy/issues/1868

we are unblocked, let us enable clang in matrix

2 years agoMEDIUM: session/ssl: return the SSL error string during a SSL handshake error
William Lallemand [Fri, 12 May 2023 15:13:46 +0000 (17:13 +0200)] 
MEDIUM: session/ssl: return the SSL error string during a SSL handshake error

SSL hanshake error were unable to dump the OpenSSL error string by
default, to do so it was mandatory to configure a error-log-format with
the ssl_fc_err fetch.

This patch implements the session_build_err_string() function which creates
the error log to send during session_kill_embryonic(), a special case is
made with CO_ER_SSL_HANDSHAKE which is able to dump the error string
with ERR_error_string().

Before:
    <134>May 12 17:14:04 haproxy[183151]: 127.0.0.1:49346 [12/May/2023:17:14:04.571] frt2/1: SSL handshake failure

After:
    <134>May 12 17:14:04 haproxy[183151]: 127.0.0.1:49346 [12/May/2023:17:14:04.571] frt2/1: SSL handshake failure (error:0A000418:SSL routines::tlsv1 alert unknown ca)

2 years agoBUG/MINOR: mux-quic: free task on qc_init() app ops failure
Amaury Denoyelle [Fri, 12 May 2023 14:29:48 +0000 (16:29 +0200)] 
BUG/MINOR: mux-quic: free task on qc_init() app ops failure

qc_init() is used to initialize a QUIC MUX instance. On failure, each
resources are released via a series of goto statements. There is one
issue if the app_ops.init callback fails. In this case, MUX task is not
freed.

This can cause a crash as the task is already scheduled. When the
handler will run, it will crash when trying to access qcc instance.

To fix this, properly destroy qcc task on fail_install_app_ops label.

The impact of this bug is minor as app_ops.init callback succeeds most
of the time. However, it may fail on allocation failure due to memory
exhaustion.

This may fix github issue #2154.

This must be backported up to 2.7.