]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoMEDIUM: mworker: seamless reload use the internal sockpairs
William Lallemand [Wed, 24 Nov 2021 17:45:37 +0000 (18:45 +0100)] 
MEDIUM: mworker: seamless reload use the internal sockpairs

With the master worker, the seamless reload was still requiring an
external stats socket to the previous process, which is a pain to
configure.

This patch implements a way to use the internal socketpair between the
master and the workers to transfer the sockets during the reload.
This way, the master will always try to transfer the socket, even
without any configuration.

The master will still reload with the -x argument, followed by the
sockpair@ syntax. ( ex -x sockpair@4 ). Which use the FD of internal CLI
to the worker.

3 years agoBUG/MINOR: lua: don't expose internal proxies
William Lallemand [Wed, 24 Nov 2021 15:14:24 +0000 (16:14 +0100)] 
BUG/MINOR: lua: don't expose internal proxies

Since internal proxies are now in the global proxy list, they are now
reachable from core.proxies, core.backends, core.frontends.

This patch fixes the issue by checking the PR_CAP_INT flag before
exposing them in lua, so the user can't have access to them.

This patch must be backported in 2.5.

3 years agoBUG/MINOR: httpclient: allow to replace the host header
William Lallemand [Wed, 24 Nov 2021 14:38:17 +0000 (15:38 +0100)] 
BUG/MINOR: httpclient: allow to replace the host header

This patch allows to replace the host header generated by the
httpclient instead of adding a new one, resulting in the server replying
an error 400.

The host header is now generated from the uri only if it wasn't found in
the list of headers.

Also add a new request in the VTC file to test this.

This patch must be backported in 2.5.

3 years agoBUG/MINOR: cache: Fix loop on cache entries in "show cache"
Christopher Faulet [Tue, 23 Nov 2021 15:03:05 +0000 (16:03 +0100)] 
BUG/MINOR: cache: Fix loop on cache entries in "show cache"

A regression was introduced in the commit da91842b6 ("BUG/MEDIUM: cache/cli:
make "show cache" thread-safe"). When cli_io_handler_show_cache() is called,
only one node is retrieved and is used to fill the output buffer in loop.
Once set, the "node" variable is never renewed. At the end, all nodes are
dumped but each one is duplicated several time into the output buffer.

This patch must be backported everywhere the above commit is. It means only
to 2.5 and 2.4.

3 years ago[RELEASE] Released version 2.6-dev0 v2.6-dev0
Willy Tarreau [Tue, 23 Nov 2021 14:50:11 +0000 (15:50 +0100)] 
[RELEASE] Released version 2.6-dev0

Released version 2.6-dev0 with the following main changes :
    - MINOR: version: it's development again

3 years agoMINOR: version: it's development again
Willy Tarreau [Tue, 23 Nov 2021 14:48:35 +0000 (15:48 +0100)] 
MINOR: version: it's development again

This essentially reverts 9dc4057df064.

3 years ago[RELEASE] Released version 2.5.0 v2.5.0
Willy Tarreau [Tue, 23 Nov 2021 14:40:21 +0000 (15:40 +0100)] 
[RELEASE] Released version 2.5.0

Released version 2.5.0 with the following main changes :
    - BUILD: SSL: add quictls build to scripts/build-ssl.sh
    - BUILD: SSL: add QUICTLS to build matrix
    - CLEANUP: sock: Wrap `accept4_broken = 1` into additional parenthesis
    - BUILD: cli: clear a maybe-unused  warning on some older compilers
    - BUG/MEDIUM: cli: make sure we can report a warning from a bind keyword
    - BUG/MINOR: ssl: make SSL counters atomic
    - CLEANUP: assorted typo fixes in the code and comments
    - BUG/MINOR: ssl: free correctly the sni in the backend SSL cache
    - MINOR: version: mention that it's stable now

3 years agoMINOR: version: mention that it's stable now
Willy Tarreau [Tue, 23 Nov 2021 14:38:10 +0000 (15:38 +0100)] 
MINOR: version: mention that it's stable now

This version will be maintained up to around Q1 2023. The INSTALL file
also mentions it.

3 years agoBUG/MINOR: ssl: free correctly the sni in the backend SSL cache
William Lallemand [Tue, 23 Nov 2021 14:15:09 +0000 (15:15 +0100)] 
BUG/MINOR: ssl: free correctly the sni in the backend SSL cache

__ssl_sock_load_new_ckch_instance() does not free correctly the SNI in
the session cache, it only frees the one in the current tid.

This bug was introduced with e18d4e8 ("BUG/MEDIUM: ssl: backend TLS
resumption with sni and TLSv1.3").

This fix must be backported where the mentionned commit was backported.
(all maintained versions).

3 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sat, 20 Nov 2021 18:11:12 +0000 (23:11 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 28th iteration of typo fixes

3 years agoBUG/MINOR: ssl: make SSL counters atomic
Willy Tarreau [Mon, 22 Nov 2021 16:46:13 +0000 (17:46 +0100)] 
BUG/MINOR: ssl: make SSL counters atomic

SSL counters were added with commit d0447a7c3 ("MINOR: ssl: add counters
for ssl sessions") in 2.4, but their updates were not atomic, so it's
likely that under significant loads they are not correct.

This needs to be backported to 2.4.

3 years agoBUG/MEDIUM: cli: make sure we can report a warning from a bind keyword
Willy Tarreau [Sat, 20 Nov 2021 19:10:41 +0000 (20:10 +0100)] 
BUG/MEDIUM: cli: make sure we can report a warning from a bind keyword

Since recent 2.5 commit c8cac04bd ("MEDIUM: listener: deprecate "process"
in favor of "thread" on bind lines"), the "process" bind keyword may
report a warning. However some parts like the "stats socket" parser
will call such bind keywords and do not expect to face warnings, so
this will instantly cause a fatal error to be reported. A concrete
effect is that "stats socket ... process 1" will hard-fail indicating
the keyword is deprecated and will be removed in 2.7.

We must relax this test, but the code isn't designed to report warnings,
it uses a single string and only supports reporting an error code (-1).

This patch makes a special case of the ERR_WARN code and uses ha_warning()
to report it, and keeps the rest of the existing error code for other
non-warning codes. Now "process" on the "stats socket" is properly
reported as a warning.

No backport is needed.

3 years agoBUILD: cli: clear a maybe-unused warning on some older compilers
Willy Tarreau [Sat, 20 Nov 2021 18:17:38 +0000 (19:17 +0100)] 
BUILD: cli: clear a maybe-unused  warning on some older compilers

The SHOW_TOT() and SHOW_AVG() macros used in cli_io_handler_show_activity()
produce a warning on gcc 4.7 on MIPS with threads disabled because the
compiler doesn't know that global.nbthread is necessarily non-null, hence
that at least one iteration is performed. Let's just change the loop for
a do {} while () that lets the compiler know it's always initialized. It
also has the tiny benefit of making the code shorter.

3 years agoCLEANUP: sock: Wrap `accept4_broken = 1` into additional parenthesis
Tim Duesterhus [Sat, 20 Nov 2021 13:39:47 +0000 (14:39 +0100)] 
CLEANUP: sock: Wrap `accept4_broken = 1` into additional parenthesis

This makes it clear to static analysis tools that this assignment is
intentional and not a mistyped comparison.

3 years agoBUILD: SSL: add QUICTLS to build matrix
Ilya Shipitsin [Thu, 18 Nov 2021 13:27:57 +0000 (18:27 +0500)] 
BUILD: SSL: add QUICTLS to build matrix

It also enables QUIC when QUICTLS is used.

3 years agoBUILD: SSL: add quictls build to scripts/build-ssl.sh
Ilya Shipitsin [Thu, 18 Nov 2021 13:27:56 +0000 (18:27 +0500)] 
BUILD: SSL: add quictls build to scripts/build-ssl.sh

script/build-ssl.sh is used mostly in CI, let us introduce QUIC
OpenSSL fork support

3 years ago[RELEASE] Released version 2.5-dev15 v2.5-dev15
Willy Tarreau [Fri, 19 Nov 2021 18:30:04 +0000 (19:30 +0100)] 
[RELEASE] Released version 2.5-dev15

Released version 2.5-dev15 with the following main changes :
    - BUG/MINOR: stick-table/cli: Check for invalid ipv6 key
    - CLEANUP: peers: Remove useless test on peer variable in peer_trace()
    - DOC: log: Add comments to specify when session's listener is defined or not
    - BUG/MEDIUM: mux-h1: Handle delayed silent shut in h1_process() to release H1C
    - REGTESTS: ssl_crt-list_filters: feature cmd incorrectly set
    - DOC: internals: document the list API
    - BUG/MINOR: h3: ignore unknown frame types
    - MINOR: quic: redirect app_ops snd_buf through mux
    - MEDIUM: quic: inspect ALPN to install app_ops
    - MINOR: quic: support hq-interop
    - MEDIUM: quic: send version negotiation packet on unknown version
    - BUG/MEDIUM: mworker: cleanup the listeners when reexecuting
    - DOC: internals: document the scheduler API
    - BUG/MINOR: quic: fix version negotiation packet generation
    - CLEANUP: ssl: fix wrong #else commentary
    - MINOR: config: support default values for environment variables
    - SCRIPTS: run-regtests: reduce the number of processes needed to check options
    - SCRIPT: run-regtests: avoid several calls to grep to test for features
    - SCRIPT: run-regtests: avoid calling awk to compute the version
    - REGTEST: set retries count to zero for all tests that expect at 503
    - REGTESTS: make tcp-check_min-recv fail fast
    - REGTESTS: extend the default I/O timeouts and make them overridable
    - BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3
    - BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found
    - REGTESTS: ssl: test the TLS resumption
    - BUILD: makefile: stop opening sub-shells for each and every command
    - BUILD: makefile: reorder objects by build time
    - BUG/MEDIUM: mux-h2: always process a pending shut read
    - MINOR: quic_sock: missing CO_FL_ADDR_TO_SET flag
    - MINOR: quic: Possible wrong connection identification
    - MINOR: quic: Correctly pad UDP datagrams
    - MINOR: quic: Support transport parameters draft TLS extension
    - MINOR: quic: Anti-amplification implementation
    - MINOR: quic: Wrong Initial packet connection initialization
    - MINOR: quic: Wrong ACK range building
    - MINOR: quic: Update some QUIC protocol errors
    - MINOR: quic: Send CONNECTION_CLOSE frame upon TLS alert
    - MINOR: quic: Wrong largest acked packet number parsing
    - MINOR: quic: Add minimalistic support for stream flow control frames
    - MINOR: quic: Wrong value for version negotiation packet 'Unused' field
    - MINOR: quic: Support draft-29 QUIC version
    - BUG/MINOR: quic: fix segfault on trace for version negotiation
    - BUG/MINOR: hq-interop: fix potential NULL dereference
    - BUILD: quic: fix potential NULL dereference on xprt_quic
    - DOC: lua: documentation about the httpclient API
    - BUG/MEDIUM: cache/cli: make "show cache" thread-safe
    - BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found
    - BUG/MINOR: shctx: do not look for available blocks when the first one is enough
    - MINOR: shctx: add a few BUG_ON() for consistency checks

3 years agoMINOR: shctx: add a few BUG_ON() for consistency checks
Willy Tarreau [Fri, 19 Nov 2021 16:47:18 +0000 (17:47 +0100)] 
MINOR: shctx: add a few BUG_ON() for consistency checks

The shctx code relies on sensitive conditions that are hard to infer
from the code itself, let's add some BUG_ON() to verify them. They
helped spot the previous bugs.

3 years agoBUG/MINOR: shctx: do not look for available blocks when the first one is enough
Willy Tarreau [Fri, 19 Nov 2021 16:42:49 +0000 (17:42 +0100)] 
BUG/MINOR: shctx: do not look for available blocks when the first one is enough

In shctx_row_reserve_hot() we only leave if we've found the exact
requested size instead of at least as large, as is documented. This
results in extra lookups and free calls in the avail loop while it is
not needed, and participates to seeing a negative data_len early as
spotted in previous bugs.

It doesn't seem to have any other impact however, but it's better to
backport it to stable branches.

3 years agoBUG/MEDIUM: shctx: leave the block allocator when enough blocks are found
Willy Tarreau [Fri, 19 Nov 2021 16:29:23 +0000 (17:29 +0100)] 
BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found

In shctx_row_reserve_hot(), a missing break allows the avail loop to
loop for a while after having allocated the required blocks, possibly
leading to the point where it could trigger the watchdog after checking
up to 2 million blocks. In addition, the extra iteration may leave one
block assigned with size zero at the head of the avail list, and mark
it as being an isolated chain of 1 block. It's unclear whether this
could have had other consequences.

There is a non-negligible chance that it addreses bugs #1451 and #1284,
as the pattern observed in the loop looks exactly the same as the one
reported there in the crashes.

It's only marked medium because it is extremely hard to trigger. Here
the conditions were reproduced when starting 4k connections at once
requesting objects of random sizes between 0 and 20k to store them into
a small 1MB cache. However the watchdog will never trigger in such a case
so one needs to instrument the functions.

Thanks to Sohaib Ahmad and @g0uZ for providing useful traces.

This will need to be backported to all stable branches.

3 years agoBUG/MEDIUM: cache/cli: make "show cache" thread-safe
Willy Tarreau [Fri, 19 Nov 2021 16:25:41 +0000 (17:25 +0100)] 
BUG/MEDIUM: cache/cli: make "show cache" thread-safe

The "show cache" command restarts from the previous node to look for a
duplicate key, but does this after having released the lock, so under
high write load, the node has many chances of having been reassigned
and the dereference of the node crashes after a few iterations. Since
the keys are unique anyway, there's no point looking for a dup, so
let's just continue from the next value.

This is only marked as medium as it seems to have been there for a
while, and discovering it that late simply means that nobody uses that
command, thus in practice it has a very limited impact on real users.

This should be backported to all stable versions.

3 years agoDOC: lua: documentation about the httpclient API
William Lallemand [Fri, 19 Nov 2021 15:02:44 +0000 (16:02 +0100)] 
DOC: lua: documentation about the httpclient API

The patch adds the documentation about the httpclient lua
implementation.

3 years agoBUILD: quic: fix potential NULL dereference on xprt_quic
Amaury Denoyelle [Fri, 19 Nov 2021 14:49:29 +0000 (15:49 +0100)] 
BUILD: quic: fix potential NULL dereference on xprt_quic

A warning is triggered by gcc9 on this code path, which is the compiler
version used by ubuntu20.04 on the github CI.

This is linked to github issue #1445.

3 years agoBUG/MINOR: hq-interop: fix potential NULL dereference
Amaury Denoyelle [Thu, 18 Nov 2021 13:40:26 +0000 (14:40 +0100)] 
BUG/MINOR: hq-interop: fix potential NULL dereference

Test return from htx_add_stline() and returns an error if NULL.

3 years agoBUG/MINOR: quic: fix segfault on trace for version negotiation
Amaury Denoyelle [Thu, 18 Nov 2021 13:38:00 +0000 (14:38 +0100)] 
BUG/MINOR: quic: fix segfault on trace for version negotiation

When receiving Initial packets for Version Negotiation, no quic_conn is
instantiated. Thus, on the final trace, the quic_conn dereferencement
must be tested before using it.

3 years agoMINOR: quic: Support draft-29 QUIC version
Frédéric Lécaille [Fri, 19 Nov 2021 13:32:52 +0000 (14:32 +0100)] 
MINOR: quic: Support draft-29 QUIC version

This is only to support quic-tracker test suite.

3 years agoMINOR: quic: Wrong value for version negotiation packet 'Unused' field
Frédéric Lécaille [Thu, 18 Nov 2021 12:54:43 +0000 (13:54 +0100)] 
MINOR: quic: Wrong value for version negotiation packet 'Unused' field

The seven less significant bits of the first byte must be arbitrary.
Without this fix, QUIC tracker "version_negotiation" test could not pass.

3 years agoMINOR: quic: Add minimalistic support for stream flow control frames
Frédéric Lécaille [Thu, 18 Nov 2021 09:57:18 +0000 (10:57 +0100)] 
MINOR: quic: Add minimalistic support for stream flow control frames

This simple patch add the parsing support for theses frames. But nothing is
done at this time about the streams or flow control concerned. This is only to
prevent some QUIC tracker or interop runner tests from failing for a reason
independant of their tested features.

3 years agoMINOR: quic: Wrong largest acked packet number parsing
Frédéric Lécaille [Wed, 17 Nov 2021 15:16:04 +0000 (16:16 +0100)] 
MINOR: quic: Wrong largest acked packet number parsing

When we have already received ACK frames with the same largest packet
number, this is not an error at all. In this case, we must continue
to parse the ACK current frame.

3 years agoMINOR: quic: Send CONNECTION_CLOSE frame upon TLS alert
Frédéric Lécaille [Wed, 17 Nov 2021 10:56:21 +0000 (11:56 +0100)] 
MINOR: quic: Send CONNECTION_CLOSE frame upon TLS alert

Add ->err member to quic_conn struct to store the connection errors.
This is the responsability of ->send_alert callback of SSL_QUIC_METHOD
struct to handle the TLS alert and consequently update ->err value.
At this time, when entering qc_build_pkt() we build a CONNECTION_CLOSE
frame close the connection when ->err value is not null.

3 years agoMINOR: quic: Update some QUIC protocol errors
Frédéric Lécaille [Tue, 16 Nov 2021 13:34:24 +0000 (14:34 +0100)] 
MINOR: quic: Update some QUIC protocol errors

Add QC_ERR_ prefix to the macro names for the protocol errors.
Add new ones which came with the last RFC drafts.

3 years agoMINOR: quic: Wrong ACK range building
Frédéric Lécaille [Tue, 16 Nov 2021 09:54:19 +0000 (10:54 +0100)] 
MINOR: quic: Wrong ACK range building

When adding a range, if no "lower" range was present in the ack range root for
the packet number space concerned, we did not check if the new added range could
overlap the next one. This leaded haproxy to crash when encoding negative integer
when building ACK frames.
This bug was revealed thanks to "multi_packet_client_hello" QUIC tracker
test which makes a client send two first Initial packets out of order.

3 years agoMINOR: quic: Wrong Initial packet connection initialization
Frédéric Lécaille [Mon, 15 Nov 2021 15:21:40 +0000 (16:21 +0100)] 
MINOR: quic: Wrong Initial packet connection initialization

->qc (QUIC connection) member of packet structure were badly initialized
when received as second Initial packet (from picoquic -Q for instance).
This leaded to corrupt the quic_conn structure with random behaviors
as size effects. This bug came with this commit:
   "MINOR: quic: Possible wrong connection identification"

3 years agoMINOR: quic: Anti-amplification implementation
Frédéric Lécaille [Wed, 10 Nov 2021 16:30:15 +0000 (17:30 +0100)] 
MINOR: quic: Anti-amplification implementation

A QUIC server MUST not send more than three times as many bytes as received
by clients before its address validation.

3 years agoMINOR: quic: Support transport parameters draft TLS extension
Frédéric Lécaille [Wed, 10 Nov 2021 08:24:22 +0000 (09:24 +0100)] 
MINOR: quic: Support transport parameters draft TLS extension

If we want to run quic-tracker against haproxy, we must at least
support the draft version of the TLS extension for the QUIC transport
parameters (0xffa5). quic-tracker QUIC version is draft-29 at this time.
We select this depending on the QUIC version. If draft, we select the
draft TLS extension.

3 years agoMINOR: quic: Correctly pad UDP datagrams
Frédéric Lécaille [Tue, 9 Nov 2021 13:12:12 +0000 (14:12 +0100)] 
MINOR: quic: Correctly pad UDP datagrams

UDP datagrams with Initial packet were padded only for the clients (haproxy
servers). But such packets MUST also be padded for the servers (haproxy
listeners). Furthere, for servers, only UDP datagrams containing ack-eliciting
Initial packet must be padded.

3 years agoMINOR: quic: Possible wrong connection identification
Frédéric Lécaille [Mon, 8 Nov 2021 16:01:46 +0000 (17:01 +0100)] 
MINOR: quic: Possible wrong connection identification

A client may send several Initial packets. This is the case for picoquic
with -Q option. In this case we must identify the connection of incoming
Initial packets thanks to the original destination connection ID.

3 years agoMINOR: quic_sock: missing CO_FL_ADDR_TO_SET flag
Frédéric Lécaille [Fri, 5 Nov 2021 10:40:50 +0000 (11:40 +0100)] 
MINOR: quic_sock: missing CO_FL_ADDR_TO_SET flag

When allocating destination addresses for QUIC connections we did not set
this flag which denotes these addresses have been set. This had as side
effect to prevent the H3 request results from being returned to the QUIC clients.

Note that this bug was revealed by this commit:
  "MEDIUM: backend: Rely on addresses at stream level to init server connection"

Thanks to Christopher for having found the real cause of this issue.

3 years agoBUG/MEDIUM: mux-h2: always process a pending shut read
Willy Tarreau [Fri, 19 Nov 2021 10:41:10 +0000 (11:41 +0100)] 
BUG/MEDIUM: mux-h2: always process a pending shut read

During 2.4-dev, an issue with partial frames was fixed with commit
3d4631fec ("BUG/MEDIUM: mux-h2: fix read0 handling on partial frames").
However this patch is not completely correct. It makes h2_recv() return
0 if the connection was shut for reads, but this not make h2_io_cb()
call h2_process(), so if there are any pending data left in the demux
buffer, they will never be processed, and the I/O callback will be
called in loops forever from the poller.

The correct return value there is 1, as is done at the end of the
function to report a pending read0.

This should definitely fix issue #1328. However even after a lot of
tests I couldn't manage to reproduce it, the conditions to enter that
situation are quite racy.

This must be backported to 2.0 since the fix above was merged into
2.0.21 and 2.2.9.

3 years agoBUILD: makefile: reorder objects by build time
Willy Tarreau [Fri, 19 Nov 2021 10:22:25 +0000 (11:22 +0100)] 
BUILD: makefile: reorder objects by build time

This is the usual pre-release reordering. It saves roughly one
second of build time at -O2 on my machine, which is always nice to
have.

3 years agoBUILD: makefile: stop opening sub-shells for each and every command
Willy Tarreau [Fri, 19 Nov 2021 09:23:36 +0000 (10:23 +0100)] 
BUILD: makefile: stop opening sub-shells for each and every command

We're spending ~8% of the total build time calling a shell to display
"CC" using the "echo" command! We don't really need this, as make also
knows a "$(info ...)" command to print a message. However there's a catch,
this command trims leading spaces, so we need to use an invisible space
using "$ ". Furthermore, in GNU make 3.80 and older, $(info) doesn't show
anything, so we only do that for 3.81 and above, older versions continue
to use echo.

This measurably speeds up build time especially at -O0 that developers
use most of the time for quick checks.

3 years agoREGTESTS: ssl: test the TLS resumption
William Lallemand [Wed, 17 Nov 2021 01:52:51 +0000 (02:52 +0100)] 
REGTESTS: ssl: test the TLS resumption

This test is able to check if the TLS resumption is working correctly
with TLSv1.2, TLSv1.3, with tickets and session cache.

3 years agoBUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found
William Lallemand [Thu, 18 Nov 2021 16:46:26 +0000 (17:46 +0100)] 
BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found

Since commit c2aae74 ("MEDIUM: ssl: Handle early data with OpenSSL
1.1.1"), the codepath of the clientHello callback changed, letting an
unknown SNI escape with a 'return 1' instead of passing through the
abort label.

An error was still emitted because the frontend continued the handshake
with the initial_ctx, which can't be used to achieve an handshake.
However, it had the ugly side effect of letting the request pass in the
case of a TLS resume. Which could be surprising when combining strict-sni
with the removing of a crt-list entry over the CLI for example. (like
its done in the ssl/new_del_ssl_crlfile.vtc reg-test).

This patch switches the code path of the allow_early and abort label, so
the default code path is the abort one, letting the clientHello returns
the correct SSL_AD_UNRECOGNIZED_NAME in case of errors.

Which means the client will now receive:

OpenSSL error[0x14094458] ssl3_read_bytes: tlsv1 unrecognized name

Instead of:

OpenSSL error[0x14094410] ssl3_read_bytes: sslv3 alert handshake failure

Which was the error emitted before HAProxy 1.8.

This patch must be carrefuly backported as far as 1.8 once we validated
its impact.

3 years agoBUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3
William Lallemand [Wed, 17 Nov 2021 01:59:21 +0000 (02:59 +0100)] 
BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3

When establishing an outboud connection, haproxy checks if the cached
TLS session has the same SNI as the connection we are trying to
resume.

This test was done by calling SSL_get_servername() which in TLSv1.2
returned the SNI. With TLSv1.3 this is not the case anymore and this
function returns NULL, which invalidates any outboud connection we are
trying to resume if it uses the sni keyword on its server line.

This patch fixes the problem by storing the SNI in the "reused_sess"
structure beside the session itself.

The ssl_sock_set_servername() now has a RWLOCK because this session
cache entry could be accessed by the CLI when trying to update a
certificate on the backend.

This fix must be backported in every maintained version, however the
RWLOCK only exists since version 2.4.

3 years agoREGTESTS: extend the default I/O timeouts and make them overridable
Willy Tarreau [Thu, 18 Nov 2021 16:46:22 +0000 (17:46 +0100)] 
REGTESTS: extend the default I/O timeouts and make them overridable

With the CI occasionally slowing down, we're starting to see again some
spurious failures despite the long 1-second timeouts. This reports false
positives that are disturbing and doesn't provide as much value as this
could. However at this delay it already becomes a pain for developers
to wait for the tests to complete.

This commit adds support for the new environment variable
HAPROXY_TEST_TIMEOUT that will allow anyone to modify the connect,
client and server timeouts. It was set to 5 seconds by default, which
should be plenty for quite some time in the CI. All relevant values
that were 200ms or above were replaced by this one. A few larger
values were left as they are special. One test for the set-timeout
action that used to rely on a fixed 1-sec value was extended to a
fixed 5-sec, as the timeout is normally not reached, but it needs
to be known to compare the old and new values.

3 years agoREGTESTS: make tcp-check_min-recv fail fast
Willy Tarreau [Thu, 18 Nov 2021 16:26:25 +0000 (17:26 +0100)] 
REGTESTS: make tcp-check_min-recv fail fast

This test was dependent on the check/server timeout to detect a failure,
which is not logical since we also use that one as an upper bound for
success in the second test, and that needlessly extends the test duration.
Let's make sur the timeout strikes immediately with only 1 ms timeout. Now
the total tests time is around 5.3-5.4s down from 8.7s in dev14. There is
still quite some room for improvement in the peers tests which all wait 2s
before starting but this will be more complicated.

3 years agoREGTEST: set retries count to zero for all tests that expect at 503
Willy Tarreau [Thu, 18 Nov 2021 16:15:59 +0000 (17:15 +0100)] 
REGTEST: set retries count to zero for all tests that expect at 503

Some tests expect a 503, typically those that check that wrong CA/CRL
will not be accepted between a server and a frontend. But such tests
tend to last very long simply because of the 1-second turn-around on
connection retries that happens during the failure. Let's properly set
the retries count to zero for these ones. One test purposely wants to
exhaust the retries so the retries was set to 1 instead.

3 years agoSCRIPT: run-regtests: avoid calling awk to compute the version
Willy Tarreau [Thu, 18 Nov 2021 14:32:16 +0000 (15:32 +0100)] 
SCRIPT: run-regtests: avoid calling awk to compute the version

For each test, the version number is evaluated using a call to awk,
which can be slow to start depending on the versions and OS. This is
only needed for a printf() call to keep only leading digits of each
component, multiply them by 1000 and pad them to 3 digits, something
that's clearly doable in plain shell in a portable way. This is what
this patch does, and it saves yet another 400 ms here on the full
test sequence.

3 years agoSCRIPT: run-regtests: avoid several calls to grep to test for features
Willy Tarreau [Thu, 18 Nov 2021 14:05:45 +0000 (15:05 +0100)] 
SCRIPT: run-regtests: avoid several calls to grep to test for features

grep is used in the arguments loops to check for features such as OPENSSL
or LUA or services like prometheus-exporter. Let's just look for the words
inside the list, which requires to prepend a delimitor at the beginning of
the list and add one at the end.

3 years agoSCRIPTS: run-regtests: reduce the number of processes needed to check options
Willy Tarreau [Thu, 18 Nov 2021 12:49:01 +0000 (13:49 +0100)] 
SCRIPTS: run-regtests: reduce the number of processes needed to check options

run-tegtests is starting to take a lot of time to spot which tests are
eligible, because for each test file a lot of "sed" sub-processes are
launched. This commit eliminates calls to sed by using the shell's
internal processing and parsing the VTC file only once. Instead of
extracting each option one by one from the file, all entries that look
like a valid option are passed to a single case/esac statement and their
value is extracted. Splitting into lists is simply done by adjusting the
IFS depending on the list's delimiter, which, contrary to the // pattern
modifier, is supported on every shell.

This was tested on both bash and dash, and the tests' execution time
dropped by 31% from 8.7 seconds to 6.0 seconds.

3 years agoMINOR: config: support default values for environment variables
Willy Tarreau [Thu, 18 Nov 2021 16:42:50 +0000 (17:42 +0100)] 
MINOR: config: support default values for environment variables

Sometimes it is really useful to be able to specify a default value for
an optional environment variable, like the ${name-value} construct in
shell. In fact we're really missing this for a number of settings in
reg tests, starting with timeouts.

This commit simply adds support for the common syntax above. Other
common forms like '+' to replace existing variables, or ':-' and ':+'
to act on empty variables, were not implemented at this stage, as they
are less commonly needed.

3 years agoCLEANUP: ssl: fix wrong #else commentary
William Lallemand [Thu, 18 Nov 2021 14:25:16 +0000 (15:25 +0100)] 
CLEANUP: ssl: fix wrong #else commentary

The else is not for boringSSL but for the lack of Client Hello callback.
Should have been changed in 1fc44d4 ("BUILD: ssl: guard Client Hello
callbacks with HAVE_SSL_CLIENT_HELLO_CB macro instead of openssl
version").

Could be backported in 2.4.

3 years agoBUG/MINOR: quic: fix version negotiation packet generation
Amaury Denoyelle [Thu, 18 Nov 2021 12:48:57 +0000 (13:48 +0100)] 
BUG/MINOR: quic: fix version negotiation packet generation

Fix wrong memcpy usage for source and connection ID in generated Version
Negotiation packet.

3 years agoDOC: internals: document the scheduler API
Willy Tarreau [Thu, 18 Nov 2021 10:26:28 +0000 (11:26 +0100)] 
DOC: internals: document the scheduler API

Another non-trivial part that is often needed. Exported functions
and flags available to applications were documented as well as some
restrictions and falltraps.

3 years agoBUG/MEDIUM: mworker: cleanup the listeners when reexecuting
William Lallemand [Thu, 18 Nov 2021 09:51:30 +0000 (10:51 +0100)] 
BUG/MEDIUM: mworker: cleanup the listeners when reexecuting

Previously, the cleanup of the listeners was done in mworker_loop(),
which was called once the configuration file was parsed. HAProxy was
switching in wait mode when the configuration failed to load, so no
listeners where created.

Since the latest change on the mworker mode, HAProxy switch to wait mode
after successfuly loading the configuration, without cleaning its
listeners, because it was done in mworker_loop, resulting in the master
not closing its listeners and keeping them. The master needs its
configuration to know which listeners it need to close, so that must be
done before the exec().

This patch fixes the problem by cleaning the listeners in the
mworker_reexec() function.

No backport needeed.

3 years agoMEDIUM: quic: send version negotiation packet on unknown version
Amaury Denoyelle [Wed, 10 Nov 2021 14:17:56 +0000 (15:17 +0100)] 
MEDIUM: quic: send version negotiation packet on unknown version

If the client announced a QUIC version not supported by haproxy, emit a
Version Negotiation Packet, according to RFC9000 6. Version Negotiation.

This is required to be able to use the framework for QUIC interop
testing from https://github.com/marten-seemann/quic-interop-runner. The
simulator checks that the server is available by sending packets to
force the emission of a Version Negotiation Packet.

3 years agoMINOR: quic: support hq-interop
Amaury Denoyelle [Fri, 12 Nov 2021 15:09:54 +0000 (16:09 +0100)] 
MINOR: quic: support hq-interop

Implement a new app_ops layer for quic interop. This layer uses HTTP/0.9
on top of QUIC. Implementation is minimal, with the intent to be able to
pass interoperability test suite from
https://github.com/marten-seemann/quic-interop-runner.

It is instantiated if the negotiated ALPN is "hq-interop".

3 years agoMEDIUM: quic: inspect ALPN to install app_ops
Amaury Denoyelle [Fri, 12 Nov 2021 10:23:29 +0000 (11:23 +0100)] 
MEDIUM: quic: inspect ALPN to install app_ops

Remove the hardcoded initialization of h3 layer on mux init. Now the
ALPN is looked just after the SSL handshake. The app layer is then
installed if the ALPN negotiation returned a supported protocol.

This required to add a get_alpn on the ssl_quic layer which is just a
call to ssl_sock_get_alpn() from ssl_sock. This is mandatory to be able
to use conn_get_alpn().

3 years agoMINOR: quic: redirect app_ops snd_buf through mux
Amaury Denoyelle [Fri, 12 Nov 2021 15:09:29 +0000 (16:09 +0100)] 
MINOR: quic: redirect app_ops snd_buf through mux

This change is required to be able to use multiple app_ops layer on top
of QUIC. The stream-interface will now call the mux snd_buf which is
just a proxy to the app_ops snd_buf function.

The architecture may be simplified in the structure to install the
app_ops on the stream_interface and avoid the detour via the mux layer
on the sending path.

3 years agoBUG/MINOR: h3: ignore unknown frame types
Amaury Denoyelle [Mon, 15 Nov 2021 14:52:55 +0000 (15:52 +0100)] 
BUG/MINOR: h3: ignore unknown frame types

When receiving an unknown h3 frame type, the frame must be discarded
silently and the processing of the remaing frames must continue. This is
according to the HTTP/3 draft34.

This issue was detected when using the quiche client which uses GREASE
frame to test interoperability.

3 years agoDOC: internals: document the list API
Willy Tarreau [Wed, 17 Nov 2021 14:30:04 +0000 (15:30 +0100)] 
DOC: internals: document the list API

This one is aging and not always trivial. Let's indicate what the
macros do and what traps not to fall into.

3 years agoREGTESTS: ssl_crt-list_filters: feature cmd incorrectly set
William Lallemand [Wed, 17 Nov 2021 01:49:16 +0000 (02:49 +0100)] 
REGTESTS: ssl_crt-list_filters: feature cmd incorrectly set

The feature cmd was incorrectly set to:

feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)' && 'openssl_version_atleast(1.1.1)'"

Which was incorrect since the quotes must surrendered the -cc argument.

Also the test requires openssl and does not work with libressl.

3 years agoBUG/MEDIUM: mux-h1: Handle delayed silent shut in h1_process() to release H1C
Christopher Faulet [Mon, 15 Nov 2021 13:51:37 +0000 (14:51 +0100)] 
BUG/MEDIUM: mux-h1: Handle delayed silent shut in h1_process() to release H1C

The commit a85c522d4 ("BUG/MINOR: mux-h1: Save shutdown mode if the shutdown
is delayed") revealed several hidden bugs in connection's shutdown
handling. One of them is about delayed silent shudown.

If outgoing data are not fully sent, we delayed the shutdown. However, in
h1_process(), only normal (or clean) shutdown are really detected. If a
silent (or dirty) shutdown is performed, the H1 connection is not
immediately released. Of course, in this situation, the client never
acknowledged the shutdown. Thus, the H1 connection remains open till the
client timeout.

This patch should fix the issues #1448 and #1453. It must be backported as
far as 2.0.

3 years agoDOC: log: Add comments to specify when session's listener is defined or not
Christopher Faulet [Mon, 15 Nov 2021 10:31:08 +0000 (11:31 +0100)] 
DOC: log: Add comments to specify when session's listener is defined or not

When a log message is emitted, The session's listener is always defined when
the session's owner is an inbound connection while it is undefined for a
health-check. It is not obvious. So, comments have been added to make it
clear.

This patch is related to the issue #1434.

3 years agoCLEANUP: peers: Remove useless test on peer variable in peer_trace()
Christopher Faulet [Mon, 15 Nov 2021 08:40:57 +0000 (09:40 +0100)] 
CLEANUP: peers: Remove useless test on peer variable in peer_trace()

A useless test on peer variable was reported by cppcheck in peer_trace().

This patch should fix the issue #1165.

3 years agoBUG/MINOR: stick-table/cli: Check for invalid ipv6 key
Christopher Faulet [Mon, 15 Nov 2021 08:17:25 +0000 (09:17 +0100)] 
BUG/MINOR: stick-table/cli: Check for invalid ipv6 key

When an ipv6 key is used to filter a CLI command on a stick table
(clear/set/show table ...), the return value of inet_pton() call must be
checked to be sure the key is valid.

This patch should fix the issue #1163. It should be backported to all
supported versions.

3 years ago[RELEASE] Released version 2.5-dev14 v2.5-dev14
Willy Tarreau [Sun, 14 Nov 2021 15:04:57 +0000 (16:04 +0100)] 
[RELEASE] Released version 2.5-dev14

Released version 2.5-dev14 with the following main changes :
    - DEV: coccinelle: Remove unused `expression e`
    - DEV: coccinelle: Add rule to use `istend()` where possible
    - CLEANUP: Apply ist.cocci
    - CLEANUP: Re-apply xalloc_size.cocci
    - CLEANUP: halog: make the default usage message fit in small screens
    - MINOR: h3/qpack: fix gcc11 warnings
    - MINOR: mux-quic: fix gcc11 warning
    - MINOR: h3: fix potential NULL dereference
    - MINOR: quic: Fix potential null pointer dereference
    - CLEANUP: halog: remove unused strl2ui()
    - OPTIM: halog: improve field parser speed for modern compilers
    - OPTIM: halog: skip fields 64 bits at a time when supported
    - DEV: coccinelle: Add rule to use `isttrim()` where possible
    - CLEANUP: Apply ist.cocci
    - DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_memcat()`
    - DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`
    - CLEANUP: Apply ist.cocci
    - CLEANUP: chunk: Remove duplicated chunk_Xcat implementation
    - CLEANUP: chunk: remove misleading chunk_strncat() function
    - BUG/MINOR: cache: properly ignore unparsable max-age in quotes
    - Revert "DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`"
    - DOC: stats: fix location of the text representation
    - DOC: internals: document the IST API
    - BUG/MINOR: httpclient/lua: rcv freeze when no request payload
    - BUG/MEDIUM: httpclient: channel_add_input() must use htx->data
    - MINOR: promex: backend aggregated server check status
    - DOC: config: Fix typo in ssl_fc_unique_id description
    - BUG/MINOR: http-ana: Apply stop to the current section for http-response rules
    - Revert "BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back"
    - DOC: config: Be more explicit in "allow" actions description
    - DOC: lua: Be explicit with the Reply object limits
    - MINOR: mux-h1: Slightly Improve H1 traces
    - BUG/MEDIUM: conn-stream: Don't reset CS flags on close
    - CLEANUP: mworker: remove any relative PID reference
    - MEDIUM: mworker: reexec in waitpid mode after successful loading
    - MINOR: mworker: clarify starting/failure messages
    - MINOR: mworker: only increment the number of reload in wait mode
    - MINOR: mworker: implement a reload failure counter
    - MINOR: mworker: ReloadFailed shown depending on failedreload
    - MINOR: mworker: change the way we set PROC_O_LEAVING
    - BUG/MINOR: mworker: doesn't launch the program postparser
    - DOC: management: edit the "show proc" example to show the current output
    - BUG/MEDIUM: httpclient/cli: free of unallocated hc->req.uri
    - REGTESTS: httpclient/lua: add greater body values
    - BUG/MINOR: mux-h2: Fix H2_CF_DEM_SHORT_READ value
    - BUG/MINOR: pools: don't mark ourselves as harmless in DEBUG_UAF mode
    - BUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent
    - BUILD: makefile: simplify detection of libatomic

3 years agoBUILD: makefile: simplify detection of libatomic
Willy Tarreau [Sun, 14 Nov 2021 14:28:38 +0000 (15:28 +0100)] 
BUILD: makefile: simplify detection of libatomic

We've had libatomic enabled on arm and aarch64 for some Raspberry PI
while usually it's not needed, but it was a bit arbitrary and in
issue #1455 it was reported that RISCV requires it for single-byte
atomics.

This changes the approach to detect the explicit requirement of
external functions for the builtins, as reported with *_LOCK_FREE=1.
If any of the atomics requires libatomic, it will be used. Older
compilers do not report any such atomic as they use sync_* instead
and will not match it nor include libatomic (which usually is not
present there).

On x86, the rules depend on -march. i386 uses LOCK_FREE=1 for all of
them. i486 uses it only for the 8-byte CAS and i586 doesn't require
it at all. For this reason, the build flags are used during the test.

This was tested with armv7, aarch64, mips, riscv, i

3 years agoBUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent
Willy Tarreau [Sun, 14 Nov 2021 12:42:17 +0000 (13:42 +0100)] 
BUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent

In 1.8 when muxes and conn_streams were introduced, the call to
conn_full_close() was replaced with a call to cs_close() which only
relied on shutr/shutw (commits 6978db35e ("MINOR: connection:
add cs_close() to close a conn_stream") and a553ae96f ("MEDIUM:
connection: replace conn_full_close() with cs_close()")). By then
this was fine, and the rare risk of non-idempotent calls was addressed
by the muxes implementing the functions (e.g. mux_pt).

Later with commit 325607397 ("MEDIUM: stream: do not forcefully close
the client connection anymore"), stream_free() started to call cs_close()
instead of forcibly closing the connection via conn_full_close(). At this
point this started to break idempotence because it was possible to emit
a shutw() (e.g. when option httpclose was set), then to have it called
agian upon stream_free() via cs_close(). By then it was not a problem
since only mux_pt would implement this and did check for idempotence.

When HTX was implemented and mux-h1/h2 offered support for shutw/shutr,
the idempotence changed a little bit because the last shutdown mode
(normal/silent) was recorded and used at the moment of closing. The
call to cs_close() uses the silent mode and will replace the current
one. This has an effect on data pending in the buffer if the FIN could
not be sent before cs_close(), because lingering may be disabled and
final data lost in the network stack.

Interestingly, during 2.4-dev3, this was addressed as the side effect
of an improvement by commit 3c82d8b32 ("MINOR: mux-h1: Rework how
shutdowns are handled"), where the H1 mux's shutdown function becomes
explicitly idempotent. However older versions (2.3 to 2.0) do not have
it.

This patch addresses the issue globally by making sure that cs_shutr()
and cs_shutw() are idempotent and cannot have their data truncated by
a late cs_close(). It fixes the truncation that is observed in 2.3 and
2.2 as described in issue #1450.

This must be backported as far as 2.0, along with commit f14d750bf
("BUG/MEDIUM: conn-stream: Don't reset CS flags on close") which it
depends on.

3 years agoBUG/MINOR: pools: don't mark ourselves as harmless in DEBUG_UAF mode
Willy Tarreau [Fri, 12 Nov 2021 09:26:18 +0000 (10:26 +0100)] 
BUG/MINOR: pools: don't mark ourselves as harmless in DEBUG_UAF mode

When haproxy is built with DEBUG_UAF=1, some particularly slow
allocation functions are used for each pool, and it was not uncommon
to see the watchdog trigger during performance tests. For this reason
the allocation functions were surrounded by a pair of thread_harmless
calls to mention that the function was waiting in slow syscalls. The
problem is that this also releases functions blocked in thread_isolate()
which can then start their work.

In order to protect against the accidental removal of a shared resource
in this situation, in 2.5-dev4 with commit ba3ab7907 ("MEDIUM: servers:
make the server deletion code run under full thread isolation") was added
thread_isolate_full() for functions which want to be totally protected
due to being manipulating some data.

But this is not sufficient, because there are still places where we
can allocate/free (thus sleep) under a lock, such as in long call
chains involving the release of an idle connection. In this case, if
one thread asks for isolation, one thread might hang in
pool_alloc_area_uaf() with a lock held (for example the conns_lock
when coming from conn_backend_get()->h1_takeover()->task_new()), with
another thread blocked on a lock waiting for that one to release it,
both keeping their bit clear in the thread_harmless mask, preventing
the first thread from being released, thus causing a deadlock.

In addition to this, it was already seen that the "show fd" CLI handler
could wake up during a pool_free_area_uaf() with an incompletely
released memory area while deleting a file descriptor, and be fooled
showing bad pointers, or during a pool_alloc() on another thread that
was in the process of registering a freshly allocated connection to a
new file descriptor.

One solution could consist in replacing all thread_isolate() calls by
thread_isolate_full() but then that makes thread_isolate() useless
and only shifts the problem by one slot.

A better approach could possibly consist in having a way to mark that
a thread is entering an extremely slow section. Such sections would
be timed so that this is not abused, and the bit would be used to
make the watchdog more patient. This would be acceptable as this would
only affect debugging.

The approach used here for now consists in removing the harmless bits
around the UAF allocator, thus essentially undoing commit 85b2cae63
("MINOR: pools: make the thread harmless during the mmap/munmap
syscalls").

This is marked as minor because nobody is expected to be running with
DEBUG_UAF outside of development or serious debugging, so this issue
cannot affect regular users. It must be backported to stable branches
that have thread_harmless_now() around the mmap() call.

3 years agoBUG/MINOR: mux-h2: Fix H2_CF_DEM_SHORT_READ value
Christopher Faulet [Wed, 10 Nov 2021 16:50:10 +0000 (17:50 +0100)] 
BUG/MINOR: mux-h2: Fix H2_CF_DEM_SHORT_READ value

The value for H2_CF_DEM_SHORT_READ flag is wrong. 2 bits are erroneously
set, 0x200 and 0x80000.  It is not an issue because both bits are not used
anywhere else.

The typo was introduced in the commit b5f7b5296 ("BUG/MEDIUM: mux-h2: Handle
remaining read0 cases on partial frames"). Thus this patch must also be
backported as far a 2.0.

3 years agoREGTESTS: httpclient/lua: add greater body values
William Lallemand [Wed, 10 Nov 2021 16:40:19 +0000 (17:40 +0100)] 
REGTESTS: httpclient/lua: add greater body values

Add greater body values and checks in order to check the behavior with
sizes greater than an haproxy buffer size.

3 years agoBUG/MEDIUM: httpclient/cli: free of unallocated hc->req.uri
William Lallemand [Wed, 10 Nov 2021 15:57:25 +0000 (16:57 +0100)] 
BUG/MEDIUM: httpclient/cli: free of unallocated hc->req.uri

httpclient_new() sets the hc->req.uri ist without duplicating its
memory, which is a problem since the string in the ist could be
inaccessible at some point. The API was made to use a ist which was
allocated dynamically, but httpclient_new() didn't do that, which result
in a crash when calling istfree().

This patch fixes the problem by doing an istdup()

Fix issue #1452.

3 years agoDOC: management: edit the "show proc" example to show the current output
William Lallemand [Wed, 10 Nov 2021 14:42:17 +0000 (15:42 +0100)] 
DOC: management: edit the "show proc" example to show the current output

The "show proc" output changed and it's time to update the example.

The output does not display the relative PID anymore since the nbproc
keyword disappeared, and it displays the number of failed reloads since
the last successful one.

3 years agoBUG/MINOR: mworker: doesn't launch the program postparser
William Lallemand [Wed, 10 Nov 2021 14:10:00 +0000 (15:10 +0100)] 
BUG/MINOR: mworker: doesn't launch the program postparser

When in wait mode, the mworker-prog postparser is launched, but
unfortunately the child structure doesn't contain all required
information to be able to launch the test.

This test is only required when doing a configuration parsing.

Must be backported as far as 2.0.

3 years agoMINOR: mworker: change the way we set PROC_O_LEAVING
William Lallemand [Wed, 10 Nov 2021 10:26:14 +0000 (11:26 +0100)] 
MINOR: mworker: change the way we set PROC_O_LEAVING

Since the wait mode is always used once we successfuly loaded the
configuration, every processes were marked as old workers.

To fix this, the PROC_O_LEAVING flag is set only on the processes which
have a number of reloads greater than the current processes.

3 years agoMINOR: mworker: ReloadFailed shown depending on failedreload
William Lallemand [Wed, 10 Nov 2021 09:57:18 +0000 (10:57 +0100)] 
MINOR: mworker: ReloadFailed shown depending on failedreload

The ReloadFailed prompt in the master CLI is shown only when
failedreloads > 0. It was previously using a check on the wait mode, but
we always use the wait mode now.

3 years agoMINOR: mworker: implement a reload failure counter
William Lallemand [Wed, 10 Nov 2021 09:49:06 +0000 (10:49 +0100)] 
MINOR: mworker: implement a reload failure counter

Implement a reload failure counter which counts the number of failure
since the last success. This counter is available in 'show proc' over
the master CLI.

3 years agoMINOR: mworker: only increment the number of reload in wait mode
William Lallemand [Tue, 9 Nov 2021 17:43:59 +0000 (18:43 +0100)] 
MINOR: mworker: only increment the number of reload in wait mode

Since the wait mode will be started in any case of succesful or failed
reload, change the way haproxy computes the number of reloads of the
processes.

3 years agoMINOR: mworker: clarify starting/failure messages
William Lallemand [Tue, 9 Nov 2021 17:16:47 +0000 (18:16 +0100)] 
MINOR: mworker: clarify starting/failure messages

Clarify the startup and reload messages:

On a successful configuration load, haproxy will emit "Loading success."
after successfuly forked the children.

When it didn't success to load the configuration it will emit "Loading failure!".

When trying to reload the master process, it will emit "Reloading
HAProxy".

3 years agoMEDIUM: mworker: reexec in waitpid mode after successful loading
William Lallemand [Tue, 9 Nov 2021 17:01:22 +0000 (18:01 +0100)] 
MEDIUM: mworker: reexec in waitpid mode after successful loading

Use the waitpid mode after successfully loading the configuration, this
way the memory will be freed in the master, and will preserve the memory.

This will be useful when doing a reload with a configuration which has
large maps or a lot of SSL certificates, avoiding an OOM because too
much memory was allocated in the master.

3 years agoCLEANUP: mworker: remove any relative PID reference
William Lallemand [Tue, 9 Nov 2021 14:25:31 +0000 (15:25 +0100)] 
CLEANUP: mworker: remove any relative PID reference

nbproc was removed, it's time to remove any reference to the relative
PID in the master-worker, since there can be only 1 current haproxy
process.

This patch cleans up the alerts and warnings emitted during the exit of
a process, as well as the "show proc" output.

3 years agoBUG/MEDIUM: conn-stream: Don't reset CS flags on close
Christopher Faulet [Wed, 10 Nov 2021 14:12:47 +0000 (15:12 +0100)] 
BUG/MEDIUM: conn-stream: Don't reset CS flags on close

cs_close() and cs_drain_and_close() are called to close a conn-stream.
cs_shutr() and cs_shutw() are called with the appropriate modes. But the
conn-stream is not released at this stage. However the flags are
reset. Thus, after a cs_close(), we loose shutdown flags. If cs_close() is
performed several times, it is a problem. And indeed, it is possible. On one
hand, the stream-interface may close the conn-stream. On the other end, the
stream always closes it when it is released.

It is a problem for the H1 multiplexer. Because the conn-stream flags are
reset, the shutr and shutw are performed twice. For a delayed shutdown, the
dirty mode is used instead of the normal one because the last call to
h1_shutw() overwrite H1C flags set by the first call. This leads to dirty
shutdowns while normal ones are required. At the end, it is possible to
truncate the messages.

This bug was revealed by the commit a85c522d4 ("BUG/MINOR: mux-h1: Save
shutdown mode if the shutdown is delayed").

This patch is related to the issue #1450. It must be backported as far as
2.0.

3 years agoMINOR: mux-h1: Slightly Improve H1 traces
Christopher Faulet [Wed, 10 Nov 2021 09:30:15 +0000 (10:30 +0100)] 
MINOR: mux-h1: Slightly Improve H1 traces

Connection and conn-stream pointers and flags are now dumped, if available,
in each trace messages. In addition, shutr and shutw mode is now reported.

3 years agoDOC: lua: Be explicit with the Reply object limits
Christopher Faulet [Tue, 9 Nov 2021 17:39:51 +0000 (18:39 +0100)] 
DOC: lua: Be explicit with the Reply object limits

In HTTP, when a lua action is evaluated, a reply object can be used to send
a response to the client and interrupt the transaction. This reply object is
converted into HTX and is limited to the response channel buffer. Its size,
once converted, cannot exceed the buffer size. There is no streaming at this
stage. However, this limitation was not documented.

Note that, for now, there is no easy way to know if the reply will fit or
not int the response channel buffer. Thus the reply must be reasonably
small. Otherwise a 500-Internal-Error message is returned.

This patch is related to the issue #1447. It may be backported as far as
2.2.

3 years agoDOC: config: Be more explicit in "allow" actions description
Christopher Faulet [Tue, 9 Nov 2021 16:58:12 +0000 (17:58 +0100)] 
DOC: config: Be more explicit in "allow" actions description

TCP/HTTP allow actions stop rules evaluation of the current section
only. Only the http-response description was accurate on this
point. Thus, the documentation is now explicit on this point for all
other concerned rulesets.

This patch may be backported, to all supported versions for tcp-request
and http-request documentation, and as far as 2.2 for http-after-response
documentation.

3 years agoRevert "BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on...
Christopher Faulet [Tue, 9 Nov 2021 16:48:39 +0000 (17:48 +0100)] 
Revert "BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back"

This reverts commit 597909f4e67866c4f3ecf77f95f2cd4556c0c638

http-after-response rules evaluation was changed to do the same that was
done for http-response, in the code. However, the opposite must be performed
instead. Only the rules of the current section must be stopped. Thus the
above commit is reverted and the http-response rules evaluation will be
fixed instead.

Note that only "allow" action is concerned. It is most probably an uncommon
action for an http-after-request rule.

This patch must be backported as far as 2.2 if the above commit was
backported.

3 years agoBUG/MINOR: http-ana: Apply stop to the current section for http-response rules
Christopher Faulet [Tue, 9 Nov 2021 15:33:25 +0000 (16:33 +0100)] 
BUG/MINOR: http-ana: Apply stop to the current section for http-response rules

A TCP/HTTP action can stop the rules evaluation. However, it should be
applied on the current section only. For instance, for http-requests rules,
an "allow" on a frontend must stop evaluation of rules defined in this
frontend. But the backend rules, if any, must still be evaluated.

For http-response rulesets, according the configuration manual, the same
must be true. Only "allow" action is concerned. However, since the
beginning, this action stops evaluation of all remaining rules, not only
those of the current section.

This patch may be backported to all supported versions. But it is not so
critical because the bug exists since a while. I doubt it will break any
existing configuration because the current behavior is
counterintuitive.

3 years agoDOC: config: Fix typo in ssl_fc_unique_id description
Christopher Faulet [Tue, 9 Nov 2021 13:23:36 +0000 (14:23 +0100)] 
DOC: config: Fix typo in ssl_fc_unique_id description

In ssl_fc_unique_id decription, threre is a reference to the wrong sample
fetch. ssl_bc_unique_id is used instead of ssl_fc_unique_id.

This patch should fix the issue #1449. It may be backported to all
supportted versions.

3 years agoMINOR: promex: backend aggregated server check status
William Dauchy [Sun, 7 Nov 2021 09:18:47 +0000 (10:18 +0100)] 
MINOR: promex: backend aggregated server check status

- add new metric: `haproxy_backend_agg_server_check_status`
  it counts the number of servers matching a specific check status
  this permits to exclude per server check status as the usage is often
  to rely on the total. Indeed in large setup having thousands of
  servers per backend the memory impact is not neglible to store the per
  server metric.
- realign promex_str_metrics array

quite simple implementation - we could improve it later by adding an
internal state to the prometheus exporter, thus to avoid counting at
every dump.

this patch is an attempt to close github issue #1312. It may bebackported
to 2.4 if requested.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
3 years agoBUG/MEDIUM: httpclient: channel_add_input() must use htx->data
William Lallemand [Mon, 8 Nov 2021 15:55:14 +0000 (16:55 +0100)] 
BUG/MEDIUM: httpclient: channel_add_input() must use htx->data

The httpclient uses channel_add_input() to notify the channel layer that
it must forward some data. This function was used with b_data(&req->buf)
which ask to send the size of a buffer (because of the HTX metadata
which fill the buffer completely).

This is wrong and will have the consequence of trying to send data that
doesn't exist, letting HAProxy looping at 100% CPU.

When using htx channel_add_input() must be used with the size of the htx
payload, and not the size of a buffer.

When sending the request payload it also need to sets the buffer size to
0, which is achieved with a htx_to_buf() when the htx payload is empty.

3 years agoBUG/MINOR: httpclient/lua: rcv freeze when no request payload
William Lallemand [Thu, 4 Nov 2021 08:45:58 +0000 (09:45 +0100)] 
BUG/MINOR: httpclient/lua: rcv freeze when no request payload

This patch fixes the receive part of the lua httpclient when no payload
was sent.

The lua task was not awoken once it jumped into
hlua_httpclient_rcv_yield(), which caused the lua client to freeze.

It works with a payload because the payload push is doing the wakeup.

A change in the state machine of the IO handler is also require to
achieve correctly the change from the REQ state to the RES state, it has
to detect if there is the right EOM flag in the request.

3 years agoDOC: internals: document the IST API
Willy Tarreau [Mon, 8 Nov 2021 15:48:54 +0000 (16:48 +0100)] 
DOC: internals: document the IST API

This one was missing. It should be easier to use now. It is obvious that
some functions are missing, and it looks like ist2str() and istpad() are
exactly the same.

3 years agoDOC: stats: fix location of the text representation
William Dauchy [Sat, 6 Nov 2021 11:30:43 +0000 (12:30 +0100)] 
DOC: stats: fix location of the text representation

`info_field_names` and `stat_field_names` no longer exist and have been
moved in stats.c
To avoid changing this comment, just mention the name of the new table
`info_fields` and `stat_fields`

Signed-off-by: William Dauchy <wdauchy@gmail.com>
3 years agoRevert "DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`"
Willy Tarreau [Mon, 8 Nov 2021 12:42:03 +0000 (13:42 +0100)] 
Revert "DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`"

This reverts commit b9656e48377a9e5359494bce6a413a9915c8f74b. It's
not needed anymore since 49b0482ed ("CLEANUP: chunk: remove misleading
chunk_strncat() function").

3 years agoBUG/MINOR: cache: properly ignore unparsable max-age in quotes
Willy Tarreau [Mon, 8 Nov 2021 11:09:27 +0000 (12:09 +0100)] 
BUG/MINOR: cache: properly ignore unparsable max-age in quotes

When "max-age" or "s-maxage" receive their values in quotes, the pointer
to the integer to be parsed is advanced by one, but the error pointer
check doesn't consider this advanced offset, so it will not match a
parse error such as max-age="a" and will take the value zero instead.

This probably needs to be backported, though it's unsure it has any
effect in the real world.

3 years agoCLEANUP: chunk: remove misleading chunk_strncat() function
Willy Tarreau [Mon, 8 Nov 2021 10:44:47 +0000 (11:44 +0100)] 
CLEANUP: chunk: remove misleading chunk_strncat() function

This function claims to perform an strncat()-like operation but it does
not, it always copies the indicated number of bytes, regardless of the
presence of a NUL character (what is currently done by chunk_memcat()).
Let's remove it and explicitly replace it with chunk_memcat().

3 years agoCLEANUP: chunk: Remove duplicated chunk_Xcat implementation
Tim Duesterhus [Mon, 8 Nov 2021 08:05:05 +0000 (09:05 +0100)] 
CLEANUP: chunk: Remove duplicated chunk_Xcat implementation

Delegate chunk_istcat, chunk_cat and chunk_strncat to the most generic
chunk_memcat.

3 years agoCLEANUP: Apply ist.cocci
Tim Duesterhus [Mon, 8 Nov 2021 08:05:04 +0000 (09:05 +0100)] 
CLEANUP: Apply ist.cocci

This is to make use of `chunk_istcat()`.

3 years agoDEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`
Tim Duesterhus [Mon, 8 Nov 2021 08:05:03 +0000 (09:05 +0100)] 
DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`

This replaces `chunk_strncat()` with `chunk_istcat()` if the parameters are the
ist's `.ptr` and `.len`.