]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
17 months agoBUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs
Willy Tarreau [Wed, 24 Jan 2024 09:31:05 +0000 (10:31 +0100)] 
BUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs

The jwt_verify converter was added in 2.5 with commit 130e142ee2
("MEDIUM: jwt: Add jwt_verify converter to verify JWT integrity"). It
takes a string on input and returns an integer. It turns out that by
presetting the return value to zero before processing contents, while
the sample data is a union, it overwrites the beginning of the buffer
struct passed on input. On a 64-bit arch it's not an issue because it's
where the allocated size is stored and it's not used in the operation,
which explains why the regtest works. But on 32-bit, both the size and
the pointer are overwritten, causing a NULL pointer to be passed to
jwt_tokenize() which is not designed to support this, hence crashes.

Let's just use a temporary variable to hold the result and move the
output sample initialization to the end of the function.

This should be backported as far as 2.5.

17 months agoBUG/MEDIUM: cli: some err/warn msg dumps add LR into CSV output on stat's CLI
Emeric Brun [Tue, 23 Jan 2024 14:44:32 +0000 (15:44 +0100)] 
BUG/MEDIUM: cli: some err/warn msg dumps add LR into CSV output on stat's CLI

The initial purpose of CSV stats through CLI was to make it easely
parsable by scripts. But in some specific cases some error or warning
messages strings containing LF were dumped into cells of this CSV.

This made some parsing failure on several tools. In addition, if a
warning or message contains to successive LF, they will be dumped
directly but double LFs tag the end of the response on CLI and the
client may consider a truncated response.

This patch extends the 'csv_enc_append' and 'csv_enc' functions used
to format quoted string content according to RFC  with an additionnal
parameter to convert multi-lines strings to one line: CRs are skipped,
and LFs are replaced with spaces. In addition and optionally, it is
also possible to remove resulting trailing spaces.

The call of this function to fill strings into stat's CSV output is
updated to force this conversion.

This patch should be backported on all supported branches (issue was
already present in v2.0)

17 months agoMINOR: quic: Correctly wait for the completion of handshakes with early data (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 10:50:38 +0000 (11:50 +0100)] 
MINOR: quic: Correctly wait for the completion of handshakes with early data (aws-lc)

This patch impacts only the haproxy builds against aws-lc TLS stack (USE_OPENSSL_AWSLC).

As mentionned by the boringssl documentation, SSL_do_handshake() completes as soon
as ClientHello is processed and server flight sent (from the TLS stack to the
server endpoint I guess). Into QUIC, the completion has as side effect to discard
the Handshake packet number space. If this handshake completion is not deffered,
the Handshake level CRYPTO data will not be sent to the peer (because of the
assotiated packet number space discarding). According to the documentation,
SSL_in_early_data() may be used to do that. If it returns 1, this means that
the handshake is still in progress but has enough progressed to send half-RTT
data.

This patch is required to make the haproxy builds against aws-lc TLS stack support 0-RTT.

17 months agoMINOR: ssl_sock: Early data disabled during SSL_CTX switching (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 13:45:25 +0000 (14:45 +0100)] 
MINOR: ssl_sock: Early data disabled during SSL_CTX switching (aws-lc)

This patch impacts only haproxy when built against aws-lc TLS stack (OPENSSL_IS_AWSLC).

During the SSL_CTX switching from ssl_sock_switchctx_cbk() callback,
ssl_sock_switchctx_set() is called. This latter calls SSL_set_SSL_CTX()
whose aims is to change the SSL_CTX attached o an SSL object (TLS session).
But the aws-lc (or boringssl) implementation of this function copy
the "early data enabled" setting value (boolean) coming with the SSL_CTX object
into the SSL object. So, if not set in the SSL_CTX object this setting disabled
the one which has been set by configuration into the SSL object
(see qc_set_quic_early_data_enabled(), it calls SSL_set_early_data_enabled()
with an SSL object as parameter).

Fix this enabling the "early data enabled" setting into the SSL_CTX before
setting this latter into the SSL object.

This patch is required to make QUIC 0-RTT work with haproxy built against
aws-lc.

Note that, this patch should also help in early data support for TCP connections.

17 months agoMINOR: quic: Enable early data at SSL session level (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 10:41:44 +0000 (11:41 +0100)] 
MINOR: quic: Enable early data at SSL session level (aws-lc)

This patch impacts only the haproxy build against aws-lc TLS stack (USE_OPENSSL_AWSLC).

Implement qc_set_quic_early_data_enabled() new function to enable
early data at session level. To make QUIC O-RTT work, a context string
must be set calling SSL_set_quic_early_data_context(). This is a
subset of the encoded transport parameters which is used for this.
Note that some application level settings should be also added (TODO).

This patch is required to make 0-RTT work for haproxy builds against aws-lc.

17 months agoMINOR: quic: Transport parameters encoding without version_information
Frederic Lecaille [Tue, 23 Jan 2024 10:28:45 +0000 (11:28 +0100)] 
MINOR: quic: Transport parameters encoding without version_information

Encode the version_information parameter only if the chosen version is provided
to quic_transport_params_encode() whose aim is to encode into a buffer all the
transport parameters passed as parameter (struct quic_params *p) in addition
to the version_information parameter.

This enables the support of transport parameters encoding without
the version_information transport parameter. This is useful for build against TLS stacks
as boringssl, aws-lc where a subset of the listener transport parameters
without version_information must be set as context string for acception
early data (see https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_set_quic_early_data_context).

This patch is required to make haproxy builds against aws-lc TLS stack
(USE_OPENSSL_AWSLC) support 0-RTT. Does not impact the others builds.

17 months agoBUILD: stick-table: fix build error on 32-bit platforms
Willy Tarreau [Sun, 21 Jan 2024 07:21:35 +0000 (08:21 +0100)] 
BUILD: stick-table: fix build error on 32-bit platforms

Commit 9b2717e7b ("MINOR: stktable: use {show,set,clear} table with ptr")
stores a pointer in a long long (64bit), which fails the cas to void* on
32-bit platforms:

  src/stick_table.c: In function 'table_process_entry_per_ptr':
  src/stick_table.c:5136:37: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
   5136 |         ts = stktable_lookup_ptr(t, (void *)ptr);

On all our supported platforms, longs and pointers are of the same size,
so let's just turn this to a ulong instead.

17 months agoMINOR: connection: add sample fetches to report per-connection glitches
Willy Tarreau [Wed, 17 Jan 2024 17:00:21 +0000 (18:00 +0100)] 
MINOR: connection: add sample fetches to report per-connection glitches

Now with fc_glitches and bc_glitches we can retrieve the number of
detected glitches on a front or back connection. On the backend it
can indicate a bug in a server that may induce frequent reconnections
hence CPU usage in TLS reconnections, and on the frontend it may
indicate an abusive client that may be trying to attack the stack
or to fingerprint it. Small non-zero values are definitely expected
and can be caused by network glitches for example, as well as rare
bugs in the other component (or maybe even in haproxy). These should
never be considered as alarming as long as they remain low (i.e.
much less than one per request). A reg-test is provided.

17 months agoMINOR: mux-h2: implement MUX_CTL_GET_GLITCHES
Willy Tarreau [Wed, 17 Jan 2024 16:21:29 +0000 (17:21 +0100)] 
MINOR: mux-h2: implement MUX_CTL_GET_GLITCHES

This reports the number of glitches on a connection.

17 months agoMINOR: connection: add a new mux_ctl to report number of connection glitches
Willy Tarreau [Wed, 17 Jan 2024 16:20:30 +0000 (17:20 +0100)] 
MINOR: connection: add a new mux_ctl to report number of connection glitches

MUX_CTL_GET_GLITCHES will report the non-negative number of clitches
observed on a connection, or -1 if not supported.

17 months agoMINOR: mux-h2: add a counter of "glitches" on a connection
Willy Tarreau [Wed, 17 Jan 2024 15:57:23 +0000 (16:57 +0100)] 
MINOR: mux-h2: add a counter of "glitches" on a connection

There are a lot of H2 events which are not invalid from a protocol
perspective but which are yet anomalies, especially when repeated. They
can come from bogus or really poorly implemlented clients, as well as
purposely built attacks, as we've seen in the past with various waves
of attempts at abusing H2 stacks.

In order to better deal with such situations, it would be nice to be
able to sort out what is correct and what is not. There's already the
HTTP error counter that may even be updated on a tracked connection,
but HTTP errors are something clearly defined while there's an entire
scope of gray area around it that should not fall into it.

This patch introduces the notion of "glitches", which normally correspond
to unexpected and temporary malfunction. And this is exactly what we'd
like to monitor. For example a peer is not misbehaving if a request it
sends fails to decode because due to HPACK compression it's larger than
a buffer, and for this reason such an event is reported as a stream error
and not a connection error. But this causes trouble nonetheless and should
be accounted for, especially to detect if it's repeated. Similarly, a
truncated preamble or settings frame may very well be caused by a network
hiccup but how do we know that in the logs? For such events, a glitch
counter is incremented on the connection.

For now a total of 41 locations were instrumented with this and the
counter is reported in the traces when not null, as well as in
"show sess" and "show fd". This was done using a new function,
"h2c_report_glitch()" so that it becomes easier to extend to more
advanced processing (applying thresholds, producing logs, escalating
to connection error, tracking etc).

A test with h2spec shows it reported in 8545 trace lines for 147 tests,
with some reaching value 3 in a same test (e.g. HPACK errors).

Some places were not instrumented, typically anything that can be
triggered on perfectly valid activity (received data after RST being
emitted, timeouts, etc). Some types of events were thought about,
such as INITIAL_WINDOW_SIZE after the first SETTINGS frame, too small
window update increments, etc. It just sounds too early to know if
those are currently being triggered by perfectly legit clients. Also
it's currently not incremented on timeouts so that we don't do that
repeatedly on short keep-alive timeouts, though it could make sense.
This may change in the future depending on how it's used. For now
this is not exposed outside of traces and debugging.

17 months agoMINOR: mux-h2/traces: add a missing trace on connection WU with negative inc
Willy Tarreau [Wed, 17 Jan 2024 15:56:18 +0000 (16:56 +0100)] 
MINOR: mux-h2/traces: add a missing trace on connection WU with negative inc

The test was performed but no trace emitted, which can complicate certain
diagnostics, so let's just add the trace for this rare case. It may safely
be backported though this is really not important.

17 months agoBUG/MEDIUM: mux-h2: refine connection vs stream error on headers
Willy Tarreau [Thu, 18 Jan 2024 16:01:45 +0000 (17:01 +0100)] 
BUG/MEDIUM: mux-h2: refine connection vs stream error on headers

Commit 7021a8c4d8 ("BUG/MINOR: mux-h2: also count streams for refused
ones") addressed stream counting issues on some error cases but not
completely correctly regarding the conn_err vs stream_err case. Indeed,
contrary to the initial analysis, h2c_dec_hdrs() can set H2_CS_ERROR
when facing some unrecoverable protocol errors, and it's not correct
to send it to strm_err which will only send the RST_STREAM frame and
the subsequent GOAWAY frame is in fact the result of the read timeout.

The difficulty behind this lies on the sequence of output validations
because h2c_dec_hdrs() returns two results at once:
  - frame processing status (done/incomplete/failed)
  - connection error status

The original ordering requires to write 2 exemplaries of the exact
same error handling code disposed differently, which the patch above
tried to factor to one. After careful inspection of h2c_dec_hdrs()
and its comments, it's clear that it always returns -1 on failure,
*including* connection errors. This means we can rearrange the test
to get rid of the missing data first, and immediately enter the
no-return zone where both the stream and connection errors can be
checked at the same place, making sure to consistently maintain
error counters. This is way better because we don't have to update
stream counters on the error path anymore. h2spec now passes the
test much faster.

This will need to be backported to the same branches as the commit
above, which was already backported to 2.9.

17 months agoDOC: configuration: fix set-dst in actions keywords matrix
Aurelien DARRAGON [Thu, 18 Jan 2024 14:13:05 +0000 (15:13 +0100)] 
DOC: configuration: fix set-dst in actions keywords matrix

Since d54e8f8107 ("DOC: config: reorganize actions into their own section")
dconv-generated shortlink for "set-dst" in actions keywords matrix is
broken.

This is due to trailing "<expr>" which should not be specified in the
matrix, but only in the actual keyword prototype and description.

This should be backported in 2.9 with d54e8f8107.

17 months agoMINOR: vars: fix indentation in var_clear_buffer()
Aurelien DARRAGON [Thu, 18 Jan 2024 09:59:23 +0000 (10:59 +0100)] 
MINOR: vars: fix indentation in var_clear_buffer()

Fix indentation in var_clear_buffer() since it is exclusively using
spaces.

Could be backported if a fix depends on it.

17 months agoBUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)
Frederic Lecaille [Tue, 16 Jan 2024 09:17:27 +0000 (10:17 +0100)] 
BUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)

This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT)
and it was introduced by this commit:

    BUG/MINOR: quic: Wrong keylog callback setting.

quic_tls_compat_keylog_callback() callback was no more set when the SSL keylog was
enabled by tune.ssl.keylog setting. This is the callback which sets the TLS secrets
into haproxy.

Set it again when the SSL keylog is not enabled by configuration.

Thank you to @Greg57070 for having reported this issue in GH #2412.

Must be backported as far as 2.8.

18 months agoBUG/MINOR: mux-h2: also count streams for refused ones
Willy Tarreau [Fri, 12 Jan 2024 17:36:57 +0000 (18:36 +0100)] 
BUG/MINOR: mux-h2: also count streams for refused ones

There are a few places where we can reject an incoming stream based on
technical errors such as decoded headers that are too large for the
internal buffers, or memory allocation errors. In this case we send
an RST_STREAM to abort the request, but the total stream counter was
not incremented. That's not really a problem, until one starts to try
to enforce a total stream limit using tune.h2.fe.max-total-streams,
and which will not count such faulty streams. Typically a client that
learns too large cookies and tries to replay them in a way that
overflows the maximum buffer size would be rejected and depending on
how they're implemented, they might retry forever.

This patch removes the stream count increment from h2s_new() and moves
it instead to the calling functions, so that it translates the decision
to process a new stream instead of a successfully decoded stream. The
result is that such a bogus client will now be blocked after reaching
the total stream limit.

This can be validated this way:

  global
        tune.h2.fe.max-total-streams 128
        expose-experimental-directives
        trace h2 sink stdout
        trace h2 level developer
        trace h2 verbosity complete
        trace h2 start now

  frontend h
        bind :8080
        mode http
        redirect location /

Sending this will fill frames with 15972 bytes of cookie headers that
expand to 16500 for storage+index once decoded, causing "message too large"
events:

  (dev/h2/mkhdr.sh -t p;dev/h2/mkhdr.sh -t s;
   for sid in {0..1000}; do
     dev/h2/mkhdr.sh  -t h -i $((sid*2+1)) -f es,eh \
       -R "828684410f7777772e6578616d706c652e636f6d \
           $(for i in {1..66}; do
             echo -n 60 7F 73 433d $(for j in {1..24}; do
               echo -n 2e313233343536373839; done);
            done) ";
   done) | nc 0 8080

Now it properly stops after sending 128 streams.

This may be backported wherever commit 983ac4397 ("MINOR: mux-h2:
support limiting the total number of H2 streams per connection") is
present, since without it, that commit is less effective.

18 months agoDEV: h2: support hex-encoded data sequences in mkhdr
Willy Tarreau [Fri, 12 Jan 2024 16:53:50 +0000 (17:53 +0100)] 
DEV: h2: support hex-encoded data sequences in mkhdr

For HPACK-encoded headers (particularly with huffman encoding), it's
really necessary to support hex sequences as they appear in RFC7541
examples, so let's support hex digit pairs with -R.

Now it's possible to do this to send GET https://www.example.com/ :

    (dev/h2/mkhdr.sh -t p; dev/h2/mkhdr.sh -t s;
     dev/h2/mkhdr.sh -t h -i 1 -f es,eh \
     -R '8286 8441 0f77 7777 2e65 7861 6d70 6c65 2e63 6f6d ') | nc 0 8080

18 months agoDEV: h2: add support for multiple flags in mkhdr
Willy Tarreau [Fri, 12 Jan 2024 16:32:45 +0000 (17:32 +0100)] 
DEV: h2: add support for multiple flags in mkhdr

The mkhdr script did not support passing multiple flags at once using
symbolic names. That's now done.

18 months agoDOC: INSTALL: require at least WolfSSL 5.6.6
William Lallemand [Fri, 12 Jan 2024 16:48:45 +0000 (17:48 +0100)] 
DOC: INSTALL: require at least WolfSSL 5.6.6

WolfSSL 5.6.6 introduces the equivalent of the clienthello callback, so
lets switch to this version.

18 months agoCI: github: update wolfSSL to 5.6.6
William Lallemand [Fri, 12 Jan 2024 16:48:22 +0000 (17:48 +0100)] 
CI: github: update wolfSSL to 5.6.6

Update wolfSSL to 5.6.6

18 months agoMEDIUM: ssl: implements 'default-crt' keyword for bind Lines
William Lallemand [Fri, 12 Jan 2024 16:32:48 +0000 (17:32 +0100)] 
MEDIUM: ssl: implements 'default-crt' keyword for bind Lines

The 'default-crt' bind keyword allows to specify multiples
default/fallback certificates, allowing one to have an RSA as well as an
ECDSA default.

18 months agoDOC: configuration: update configuration on how to have multiple default certs
William Lallemand [Fri, 12 Jan 2024 16:01:30 +0000 (17:01 +0100)] 
DOC: configuration: update configuration on how to have multiple default certs

HAProxy now allows to configure default certificates with SNI filters or
multi-cert bundle.

18 months agoREORG: ssl: move 'generate-certificates' code to ssl_gencert.c
William Lallemand [Fri, 12 Jan 2024 14:23:49 +0000 (15:23 +0100)] 
REORG: ssl: move 'generate-certificates' code to ssl_gencert.c

A lot of code specific to the 'generate-certificates' option was left in
ssl_sock.c.

Move the code to 'ssl_gencert.c' and 'ssl_gencert.h'

18 months agoMEDIUM: ssl: does not use default_ctx for 'generate-certificate' option
William Lallemand [Thu, 11 Jan 2024 14:10:33 +0000 (15:10 +0100)] 
MEDIUM: ssl: does not use default_ctx for 'generate-certificate' option

The 'generate-certificates' option does not need its dedicated SSL_CTX
*, it only needs the default SSL_CTX.

Use the default SSL_CTX found in the sni_ctx to generate certificates.

It allows to remove all the specific default_ctx initialization, as
well as the default_ssl_conf and 'default_inst'.

18 months agoMEDIUM: ssl: generate '*' SNI filters for default certificates
William Lallemand [Wed, 10 Jan 2024 15:07:17 +0000 (16:07 +0100)] 
MEDIUM: ssl: generate '*' SNI filters for default certificates

This patch follows the previous one about default certificate selection
("MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA
selection").

This patch generates '*" SNI filters for the first certificate of a
bind line, it will be used to match default certificates. Instead of
setting the default_ctx pointer in the bind line.

Since the filters are in the SNI tree, it allows to have multiple
default certificate and restore the ecdsa/rsa selection with a
multi-cert bundle.

This configuration:
   # foobar.pem.ecdsa and foobar.pem.rsa
   bind *:8443 ssl crt foobar.pem crt next.pem

will use "foobar.pem.ecdsa" and "foobar.pem.rsa" as default
certificates.

Note: there is still cleanup needed around default_ctx.

This was discussed in github issue #2392.

18 months agoMEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA selection
William Lallemand [Wed, 10 Jan 2024 13:05:59 +0000 (14:05 +0100)] 
MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA selection

This patch changes the default certificate mechanism.

Since the beginning of SSL in HAProxy, the default certificate was the first
certificate of a bind line. This allowed to fallback on this certificate
when no servername extension was sent by the server, or when no SAN nor
CN was available in the certificate.

When using a multi-certificate bundle (ecdsa+rsa), it was possible to
have both certificates as the fallback one, leting openssl chose the
right one. This was possible because a multi-certificate bundle
was generating a unique SSL_CTX for both certificates.

When the haproxy and openssl architecture evolved, we decided to
use multiple SSL_CTX for a multi-cert bundle, in order to simplify the
code and allow updates over the CLI.

However only one default_ctx was allowed, so we lost the ability to
chose between ECDSA and RSA for the default certificate.

This patch allows to use a '*' filter for a certificate, which allow to
lookup between multiple '*' filter, and have one in RSA and another one
in ECDSA. It replaces the default_ctx mechanism in the ClientHello
callback and use the standard algorithm to look for a default cert and
chose between ECDSA and RSA.

/!\ This patch breaks the automatic setting of the default certificate, which
will be introduce in the next patch. So the first certificate of a bind
line won't be used as a defaullt anymore.

To use this feature, one could use crt-list with '*' filters:

$ cat foo.crtlist
foobar.pem.rsa   *
foobar.pem.ecdsa *

In order to test the feature, it's easy to send a request without
the servername extension and use ECDSA or RSA compatible ciphers:

$ openssl s_client -connect localhost:8443 -tls1_2 -cipher ECDHE-RSA-AES256-GCM-SHA384
$ openssl s_client -connect localhost:8443 -tls1_2 -cipher ECDHE-ECDSA-AES256-GCM-SHA384

18 months agoBUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control
Amaury Denoyelle [Tue, 9 Jan 2024 10:42:08 +0000 (11:42 +0100)] 
BUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control

Data emitted by QUIC MUX is restrained by the peer flow control. This is
checked on stream and connection level inside qcc_io_send().

The connection level check was placed early in qcc_io_send() preambule.
However, this also prevents emission of other frames STOP_SENDING and
RESET_STREAM, until flow control limitation is increased by a received
MAX_DATA. Note that local flow control frame emission is done prior in
qcc_io_send() and so are not impacted.

In the worst case, if no MAX_DATA is received for some time, this could
delay significantly streams closure and resource free. However, this
should be rare as other peers should anticipate emission of MAX_DATA
before reaching flow control limit. In the end, this is also covered by
the MUX timeout so the impact should be minimal

To fix this, move the connection level check directly inside QCS sending
loop. Note that this could cause unnecessary looping when connection
flow control level is reached and no STOP_SENDING/RESET_STREAM are
needed.

This should be backported up to 2.6.

18 months agoBUILD: quic: missing include for quic_tp
Amaury Denoyelle [Wed, 10 Jan 2024 09:57:18 +0000 (10:57 +0100)] 
BUILD: quic: missing include for quic_tp

Add missing netinet/in.h required for in_addr/in6_addr types.

This should be backported up to 2.9.

18 months agoCLEANUP: fix spelling of "occured" in src/h3.c
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:11 +0000 (20:49 +0100)] 
CLEANUP: fix spelling of "occured" in src/h3.c

18 months agoCI: codespell: add more words to whitelist
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:10 +0000 (20:49 +0100)] 
CI: codespell: add more words to whitelist

"Collet" is "Yann Collet" - a developer of xxhash
"bu" is variable name in src/sock_unix.c
"htmp" is variable name in src/quic_retransmit.c

18 months agoCI: codespell: ignore some words in URLs
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:09 +0000 (20:49 +0100)] 
CI: codespell: ignore some words in URLs

"trafic,ressources" are found in URIs, due to
https://github.com/codespell-project/actions-codespell/issues/55 we cannot use
wildcard for exclusion, let start with fixed list

18 months agoMEDIUM: http: add the ability to redefine http-err-codes and http-fail-codes
Willy Tarreau [Thu, 11 Jan 2024 11:06:49 +0000 (12:06 +0100)] 
MEDIUM: http: add the ability to redefine http-err-codes and http-fail-codes

The new global keywords "http-err-codes" and "http-fail-codes" allow to
redefine which HTTP status codes indicate a client-induced error or a
server error, as tracked by stick-table counters. This is only done
globally, though everything was done so that it could easily be extended
to a per-proxy mechanism if there was a real need for this (but it would
eat quite more RAM then).

A simple reg-test was added (http-err-fail.vtc).

18 months agoMEDIUM: http_act: check status codes against the bit fields for err/fail
Willy Tarreau [Wed, 10 Jan 2024 17:44:30 +0000 (18:44 +0100)] 
MEDIUM: http_act: check status codes against the bit fields for err/fail

This drops the hard-coded 4xx and 5xx status codes for err_cnt and
fail_cnt, in favor of the new bit fields that will soon be configurable.
There should be no difference at all since the bit fields are initialized
to the exact same sets (400-499 for err, 500-599 minus 501 and 505 for
fail).

18 months agoMINOR: http: add infrastructure to choose status codes for err / fail
Willy Tarreau [Wed, 10 Jan 2024 17:36:12 +0000 (18:36 +0100)] 
MINOR: http: add infrastructure to choose status codes for err / fail

At the moment, http_err_cnt and http_fail_cnt are incremented on a
well-defined set of status codes, which are checked at various places.
Over time, there have been some complains about 404, 401 or 407
triggering errors, or 500 triggering failures in SOAP environments
for example. With a small bit field that fits in a cache line we
can match the presence of a status code from 100 to 599, so that
remains cheap.

This patch adds two such bit fields, one per code class, and the
accompanying functions to set/clear/test the codes. The arrays are
preset at boot time. For now they are not used and it's not possible
to adjust them.

18 months agoCLEANUP: http: avoid duplicating literals in find_http_meth()
Willy Tarreau [Wed, 10 Jan 2024 10:28:28 +0000 (11:28 +0100)] 
CLEANUP: http: avoid duplicating literals in find_http_meth()

The function does the inverse of http_known_methods[], better rely on
that array with its indices, that makes the code clearer. Note that
we purposely don't use a loop because the compiler is able to build
an evaluation tree of the size checks and content checks that's very
efficient for the most common methods. Moving a few unimportant
entries even simplified the output code a little bit (they're now
groupped by size without changing anything for the first ones).

18 months agoOPTIM: http: simplify http_get_status_idx() using a hash
Willy Tarreau [Wed, 10 Jan 2024 15:34:48 +0000 (16:34 +0100)] 
OPTIM: http: simplify http_get_status_idx() using a hash

This function uses a large switch/case, but the problem is that due to the
numerous holes in the range, the compiler implemented a large jump table.
With a bit of experimentations, some trivial perfect-hash code works, and
since the number of entries is 19, it was enlarged to match the nearest
next power of two to avoid a large modulo operation, and fills the holes
with the default return value (HTTP_ERR_500). Jumping to 32 also results
in a lot of valid keys and allows us to pick small values, resulting in
very fast and compact code. The new function, despite keeping a 32-bytes
table, saves slightly more than 800 bytes of code+data compared to the
previous code, and avoids table jumps that affect the CPU's branch history.

Note that another simple hash worked fine and produced exactly 19 codes
(hence no need to pad holes): ((status * 8675725) >> 13) % 19

But it's still about 24 bytes larger in code to save 13 bytes of data
that are aligned anyway, and it was a bit more expensive so that was
definitely not worth it.

The validity of the table was verified with this test code added just after
it:

  __attribute__((constructor)) void http_hash_test(void)
  {
int i;
for (i = 0; i <= 600; i++)
printf("code %d => %d\n", i, http_get_status_idx(i));
exit(0);
  }

And starting haproxy |grep -vw 14 correctly shows all ordered values
(except 500 of course which is 14).

In case new codes would be added, just play again with dev/phash to
updated the table. As long as there are less than 32 effective entries
it will remain easy to update without having to modify phash.

18 months agoDEV: phash: add a trivial perfect hash generator for integers
Willy Tarreau [Wed, 10 Jan 2024 15:25:58 +0000 (16:25 +0100)] 
DEV: phash: add a trivial perfect hash generator for integers

We have a few places where we're checking many status codes. The sets
are so small that just a multiply, a shift and an optional xor are
sufficient to turn that into a smaller set. The program used to produce
them is hackish as it allows to easily fiddle with various operations
manually and experiment by brute-forcing a pair of integers for the
mul and the xor. It also supports producing an incomplete table that
gives an easier modulo operation. Let's commit it as-is so that it can
be reused later (e.g. if new status codes are introduced for example).

18 months agoMINOR: map: mapfile ordering also matters for tree-based match types
Aurelien DARRAGON [Thu, 11 Jan 2024 09:31:04 +0000 (10:31 +0100)] 
MINOR: map: mapfile ordering also matters for tree-based match types

Willy made me realize that tree-based matching may also suffer from
out-of-order mapfile loading, as opposed to what's being said in
b546bb6d ("BUG/MINOR: map: list-based matching potential ordering
regression") and the associated REGTEST.

Indeed, in case of duplicated keys, we want to be sure that only the key
that was first seen in the file will be returned (as long as it is not
removed). The above fix is still valid, and the list-based match regtest
will also prevent regressions for tree-based match since mapfile loading
logic is currently match-type agnostic.

But let's clarify that by making both the code comment and the regtest
more precise.

18 months agoDOC: configuration: corrected description of keyword tune.ssl.ocsp-update.mindelay
Miroslav Zagorac [Tue, 9 Jan 2024 19:55:47 +0000 (20:55 +0100)] 
DOC: configuration: corrected description of keyword tune.ssl.ocsp-update.mindelay

Deleted the text paragraph in the description of keyword
tune.ssl.ocsp-update.mindelay, which was added in the commit 5843237
"MINOR: ssl: Add global options to modify ocsp update min/max delay",
because it was a copy of the description of tune.ssl.ssl-ctx-cache-size.

18 months agoDOC: config: fix typo about map_*_key converters
Aurelien DARRAGON [Wed, 3 Jan 2024 09:53:58 +0000 (10:53 +0100)] 
DOC: config: fix typo about map_*_key converters

Fix a doc typo that was introduced with ca4758378 ("MINOR: map: add
map_*_key converters to provide the matching key").

No backport needed unless ca4758378 is.

18 months agoREGTESTS: add a test to ensure map-ordering is preserved
Aurelien DARRAGON [Mon, 8 Jan 2024 09:25:18 +0000 (10:25 +0100)] 
REGTESTS: add a test to ensure map-ordering is preserved

As shown in "BUG/MINOR: map: list-based matching potential ordering
regression", list-based matching types such as dom are affected by the
order in which elements are loaded from the map.

Since this is historical behavior and existing usages depend on it, we
add a test to prevent future regressions.

18 months agoBUG/MINOR: map: list-based matching potential ordering regression
Aurelien DARRAGON [Wed, 3 Jan 2024 10:54:03 +0000 (11:54 +0100)] 
BUG/MINOR: map: list-based matching potential ordering regression

An unexpected side-effect was introduced by 5fea597 ("MEDIUM: map/acl:
Accelerate several functions using pat_ref_elt struct ->head list")

The above commit tried to use eb tree API to manipulate elements as much
as possible in the hope to accelerate some functions.

Prior to 5fea597, pattern_read_from_file() used to iterate over all
elements from the map file in the same order they were seen in the file
(using list_for_each_entry) to push them in the pattern expression.

Now, since eb api is used to iterate over elements, the ordering is lost
very early.

This is known to cause behavior changes with existing setups (same conf
and map file) when compared with previous versions for some list-based
matching methods as described in GH #2400. For instance, the map_dom()
converter may return a different matching key from the one that was
returned by older haproxy versions.

For IP or STR matching, matching is based on tree lookups for better
efficiency, so in this case the ordering is lost at the name of
performance. The order in which they are loaded doesn't matter because
tree ordering is based on the content, it is not positional.

But with some other types, matching is based on list lookups (e.g.: dom),
and the order in which elements are pushed into the list can affect the
matching element that will be returned (in case of multiple matches, since
only the first matching element in the list will be returned).

Despite the documentation not officially stating that the file ordering
should be preserved for list-based matching methods, it's probably best
to be conservative here and stick to historical behavior. Moreover, there
was no performance benefit from using the eb tree api to iterate over
elements in pattern_read_from_file() since all elements are visited
anyway.

This should be backported to 2.9.

18 months agoCLEANUP: quic: Double quic_dgram_parse() prototype declaration.
Frédéric Lécaille [Wed, 10 Jan 2024 16:22:24 +0000 (17:22 +0100)] 
CLEANUP: quic: Double quic_dgram_parse() prototype declaration.

This function is defined in the RX part (quic_rx.c) and declared in quic_rx.h
header. This is its correct place.

Remove the useless declaration of this function in quic_conn.h.

Should be backported in 2.9 where this double declaration was introduced when
moving quic_dgram_parse() from quic_conn.c to quic_rx.c.

18 months agoCLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec() (part 2)
William Lallemand [Tue, 9 Jan 2024 14:24:29 +0000 (15:24 +0100)] 
CLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec() (part 2)

Fix indentation in smp_fetch_ssl_fc_ec() since it is using exclusively
spaces.

This should have been in previous 9a21b4b43 patch but was missed by
accident.

Could be backported if a fix depends on it.

18 months agoDEV: patchbot: produce a verdict for too long commit messages
Willy Tarreau [Tue, 9 Jan 2024 13:44:08 +0000 (14:44 +0100)] 
DEV: patchbot: produce a verdict for too long commit messages

Some rare commit messages area really too large because they contain
code excerpts in the message body or are release commits with their
changelog. In this case, instead of leaving an empty file that will
be silently ignored, let's produce an output message indicating that
the verdict is uncertain, with an explanation stating that there was
an error.

18 months agoCLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec()
William Lallemand [Tue, 9 Jan 2024 10:42:51 +0000 (11:42 +0100)] 
CLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec()

Fix indentation in smp_fetch_ssl_fc_ec() since it is using exclusively
spaces.

Could be backported if a fix depends on it.

18 months agoMINOR: ssl: Update ssl_fc_curve/ssl_bc_curve to use SSL_get0_group_name
Mariam John [Fri, 29 Dec 2023 17:14:41 +0000 (11:14 -0600)] 
MINOR: ssl: Update ssl_fc_curve/ssl_bc_curve to use SSL_get0_group_name

The function `smp_fetch_ssl_fc_ec` gets the curve name used during key
exchange. It currently uses the `SSL_get_negotiated_group`, available
since OpenSSLv3.0 to get the nid and derive the short name of the curve
from the nid. In OpenSSLv3.2, a new function, `SSL_get0_group_name` was
added that directly gives the curve name.

The function `smp_fetch_ssl_fc_ec` has been updated to use
`SSL_get0_group_name` if using OpenSSL>=3.2 and for versions >=3.0 and <
3.2 use the old SSL_get_negotiated_group to get the curve name. Another
change made is to normalize the return value, so that
`smp_fetch_ssl_fc_ec` returns curve name in uppercase.
(`SSL_get0_group_name` returns the curve name in lowercase and
`SSL_get_negotiated_group` + `OBJ_nid2sn` returns curve name in
uppercase).

Can be backported to 2.8.

18 months agoMINOR: ot: logsrv struct becomes logger
Miroslav Zagorac [Mon, 8 Jan 2024 11:55:11 +0000 (12:55 +0100)] 
MINOR: ot: logsrv struct becomes logger

Addition to commit 18da35c "MEDIUM: tree-wide: logsrv struct becomes logger",
when the OpenTracing filter is compiled in debug mode (using OT_DEBUG=1)
then logsrv should be changed to logger here as well.

This patch should be backported to branch 2.9.

18 months ago[RELEASE] Released version 3.0-dev1 v3.0-dev1
Willy Tarreau [Sat, 6 Jan 2024 13:09:35 +0000 (14:09 +0100)] 
[RELEASE] Released version 3.0-dev1

Released version 3.0-dev1 with the following main changes :
    - MINOR: channel: Use dedicated functions to deal with STREAMER flags
    - MEDIUM: applet: Handle channel's STREAMER flags on applets size
    - MINOR: applets: Use channel's field to compute amount of data received
    - MEDIUM: cache: Save body size of cached objects and track it on delivery
    - MEDIUM: cache: Add support for endp-to-endp fast-forwarding
    - MINOR: cache: Add global option to enable/disable zero-copy forwarding
    - MINOR: pattern: Use reference name as filename to read patterns from a file
    - MEDIUM: pattern: Add support for virtual and optional files for patterns
    - DOC: config: Add section about name format for maps and ACLs
    - DOC: management/lua: Update commands about map and acl
    - MINOR: promex: Add support for specialized front/back/li/srv metric names
    - MINOR: promex: Export active/backup metrics per-server
    - BUG/MINOR: ssl: Double free of OCSP Certificate ID
    - MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback
    - BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate
    - BUG/MINOR: lua: Wrong OCSP CID after modifying an SSL certficate (LUA)
    - DOC: configuration: typo req.ssl_hello_type
    - MINOR: hq-interop: add fastfwd support
    - CLEANUP: mux_quic: rename ffwd function with prefix qmux_strm_
    - MINOR: mux-quic: add traces for 0-copy/fast-forward
    - BUG/MINOR: mworker/cli: fix set severity-output support
    - CLEANUP: mworker/cli: add comments about pcli_find_and_exec_kw()
    - BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
    - BUILD: ssl: update types in wolfssl cert selection callback
    - MINOR: ssl: activate the certificate selection callback for WolfSSL
    - CI: github: switch to wolfssl git-c4b77ad for new PR
    - BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions
    - BUG/MINOR: ext-check: cannot use without preserve-env
    - CLEANUP: mux-quic: remove unused prototype
    - MINOR: mux-quic: clean up qcs Rx buffer allocation API
    - MINOR: mux-quic: clean up qcs Tx buffer allocation API
    - CLEANUP: mux-quic: clean up app ops callback definitions
    - MINOR: mux-quic: factorize QC_SF_UNKNOWN_PL_LENGTH set
    - MINOR: h3: complete traces for sending
    - MINOR: h3: adjust zero-copy sending related code
    - MINOR: hq-interop: use zero-copy to transfer single HTX data block
    - BUG/MEDIUM: quic: QUIC CID removed from tree without locking
    - BUG/MEDIUM: stconn: Block zero-copy forwarding if EOS/ERROR on consumer side
    - BUG/MEDIUM: mux-h1: Cound data from input buf during zero-copy forwarding
    - BUG/MEDIUM: mux-h1: Explicitly skip request's C-L header if not set originally
    - CLEANUP: mux-h1: Fix a trace message about C-L header addition
    - BUG/MEDIUM: mux-h2: Report too large HEADERS frame only when rxbuf is empty
    - BUG/MEDIUM: mux-quic: report early error on stream
    - DOC: config: add arguments to sample fetch methods in the table
    - DOC: config: also add arguments to the converters in the table
    - BUG/MINOR: resolvers: default resolvers fails when network not configured
    - SCRIPTS: mk-patch-list: produce a list of patches
    - DEV: patchbot: add the AI-based bot to pre-select candidate patches to backport
    - BUG/MEDIUM: mux-h2: Switch pending error to error if demux buffer is empty
    - BUG/MEDIUM: mux-h2: Only Report H2C error on read error if demux buffer is empty
    - BUG/MEDIUM: mux-h2: Don't report error on SE if error is only pending on H2C
    - BUG/MEDIUM: mux-h2: Don't report error on SE for closed H2 streams
    - DOC: config: Update documentation about local haproxy response
    - DEV: patchbot: use checked buttons as reference instead of internal table
    - DEV: patchbot: allow to show/hide backported patches
    - MINOR: h3: remove quic_conn only reference
    - BUG/MINOR: server: Use the configured address family for the initial resolution
    - MINOR: mux-quic: remove qcc_shutdown() from qcc_release()
    - MINOR: mux-quic: use qcc_release in case of init failure
    - MINOR: mux-quic: adjust error code in init failure
    - MINOR: h3: add traces for connection init stage
    - BUG/MINOR: h3: properly handle alloc failure on finalize
    - MINOR: h3: use INTERNAL_ERROR code for init failure
    - BUG/MAJOR: stconn: Disable zero-copy forwarding if consumer is shut or in error
    - MINOR: stats: store the parent proxy in stats ctx (http)
    - BUG/MEDIUM: stats: unhandled switching rules with TCP frontend
    - MEDIUM: proxy: set PR_O_HTTP_UPG on implicit upgrades
    - MINOR: proxy: monitor-uri works with tcp->http upgrades
    - OPTIM: server: eb lookup for server_find_by_name()
    - OPTIM: server: ebtree lookups for findserver_unique_* functions
    - MINOR: server/event_hdl: add server_inetaddr struct to facilitate event data usage
    - MINOR: server/event_hdl: update _srv_event_hdl_prepare_inetaddr prototype
    - BUG/MINOR: server/event_hdl: propagate map port info through inetaddr event
    - MINOR: server: ensure connection cleanup on server addr changes
    - CLEANUP: server/event_hdl: remove purge_conn hint in INETADDR event
    - MEDIUM: server: merge srv_update_addr() and srv_update_addr_port() logic
    - CLEANUP: server: remove unused server_parse_addr_change_request() function
    - CLEANUP: resolvers: remove duplicate func prototype
    - MINOR: resolvers: add unique numeric id to nameservers
    - MEDIUM: server: make server_set_inetaddr() updater serializable
    - MINOR: server/event_hdl: expose updater info through INETADDR event
    - MINOR: server: add dns hint in server_inetaddr_updater struct
    - MEDIUM: server/dns: clear RMAINT when addr resolves again
    - BUG/MINOR: server/dns: use server_set_inetaddr() to unset srv addr from DNS
    - BUG/MEDIUM: server/dns: perform svc_port updates atomically from SRV records
    - MEDIUM: peers: use server as stream target
    - CLEANUP: peers: remove unused sock_init_arg struct member
    - CLEANUP: peers: remove unused "proto" and "xprt" struct members
    - MINOR: peers: rely on srv->addr and remove peer->addr
    - DOC: config: add context hint for server keywords
    - MINOR: stktable: add table_process_entry helper function
    - MINOR: stktable: use {show,set,clear} table with ptr
    - MINOR: map: add map_*_key converters to provide the matching key
    - DOC: fix typo for fastfwd QUIC option
    - BUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission
    - MEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS
    - BUG/MINOR: mux-quic: disable fast-fwd if connection on error
    - BUG/MINOR: quic: Wrong keylog callback setting.
    - BUG/MINOR: quic: Missing call to TLS message callbacks
    - MINOR: h3: check connection error during sending
    - BUG/MINOR: h3: close connection on header list too big
    - BUG/MINOR: h3: close connection on sending alloc errors
    - BUG/MINOR: h3: disable fast-forward on buffer alloc failure
    - Revert "MINOR: mux-quic: Disable zero-copy forwarding for send by default"
    - MINOR: stktable: stktable_data_ptr() cannot fail in table_process_entry()
    - CLEANUP: assorted typo fixes in the code and comments
    - CI: use semantic version compare for determing "latest" OpenSSL
    - CLEANUP: server: remove ambiguous check in srv_update_addr_port()
    - CLEANUP: resolvers: remove unused RSLV_UPD_OBSOLETE_IP flag
    - CLEANUP: resolvers: remove some more unused RSLV_UDP flags
    - MEDIUM: server: simplify snr_set_srv_down() to prevent confusions
    - MINOR: backend: export get_server_*() functions
    - MINOR: tcpcheck: export proxy_parse_tcpcheck()
    - MEDIUM: udp: allow to retrieve the frontend destination address
    - MINOR: global: export a way to list build options
    - MINOR: debug: add features and build options to "show dev"
    - BUG/MINOR: server: fix server_find_by_name() usage during parsing
    - REGTESTS: check attach-srv out of order declaration
    - CLEANUP: quic: Remaining useless code into server part
    - BUILD: quic: Missing quic_ssl.h header protection
    - BUG/MEDIUM: h3: fix incorrect snd_buf return value
    - MINOR: h3: do not consider missing buf room as error on trailers
    - BUG/MEDIUM: stconn: Forward shutdown on write timeout only if it is forwardable
    - BUG/MEDIUM: stconn: Set fsb date if zero-copy forwarding is blocked during nego
    - BUG/MEDIUM: spoe: Never create new spoe applet if there is no server up
    - MINOR: mux-h2: support limiting the total number of H2 streams per connection
    - CLEANUP: mux-h2: remove the printfs from previous commit on h2 streams limit.
    - DEV: h2: add the ability to emit literals in mkhdr
    - DEV: h2: add the preface as well in supported output types
    - DEV: h2: support passing raw data for a frame
    - IMPORT: ebtree: implement and use flsnz_long() to count bits
    - IMPORT: ebtree: switch the sizes and offsets to size_t and ssize_t
    - IMPORT: ebtree: rework the fls macros to better deal with arch-specific ones
    - IMPORT: ebtree: make string_equal_bits turn back to unsigned char
    - IMPORT: ebtree: use unsigned ints for flznz()
    - IMPORT: ebtree: make string_equal_bits() return an unsigned

18 months agoIMPORT: ebtree: make string_equal_bits() return an unsigned
Willy Tarreau [Sat, 30 Dec 2023 16:24:06 +0000 (17:24 +0100)] 
IMPORT: ebtree: make string_equal_bits() return an unsigned

It used to return ssize_t for -1 but in fact we're using this -1 as
the largest possible value and the result is generally cast to signed
to check if the end was reached, so better make it clearly return an
unsigned value here.

This is cbtree commit e1e58a2b2ced2560d4544abaefde595273089704.
This is ebtree commit d7531a7475f8ba8e592342ef1240df3330d0ab47.

18 months agoIMPORT: ebtree: use unsigned ints for flznz()
Willy Tarreau [Sat, 30 Dec 2023 16:22:52 +0000 (17:22 +0100)] 
IMPORT: ebtree: use unsigned ints for flznz()

There's no reason to return signed values there. And it turns out that
the compiler manages to improve the performance by ~2%.

This is cbtree commit ab3fd53b8d6bbe15c196dfb4f47d552c3441d602.
This is ebtree commit 0ebb1d7411d947de55fa5913d3ab17d089ea865c.

18 months agoIMPORT: ebtree: make string_equal_bits turn back to unsigned char
Willy Tarreau [Sat, 30 Dec 2023 15:28:05 +0000 (16:28 +0100)] 
IMPORT: ebtree: make string_equal_bits turn back to unsigned char

With flsnz() instead of flsnz_long() we're now getting a better
performance on both x86 and ARM. The difference is that previously
we were relying on a function that was forcing the use of register
%eax for the 8-bit version and that was preventing the compiler
from keeping the code optimized. The gain is roughly 5% on ARM and
1% on x86.

This is cbtree commit 19cf39b2514bea79fed94d85e421e293be097a0e.
This is ebtree commit a9aaf2d94e2c92fa37aa3152c2ad8220a9533ead.

18 months agoIMPORT: ebtree: rework the fls macros to better deal with arch-specific ones
Willy Tarreau [Sat, 30 Dec 2023 14:37:17 +0000 (15:37 +0100)] 
IMPORT: ebtree: rework the fls macros to better deal with arch-specific ones

The definitions were a bit of a mess and there wasn't even a fall back to
__builtin_clz() on compilers supporting it. Now we instead define a macro
for each implementation that is set on an arch-dependent case by case,
and add the fall back ones only when not defined. This also allows the
flsnz8() to automatically fall back to the 32-bit arch-specific version
if available. This shows a consistent 33% speedup on arm for strings.

This is cbtree commit c6075742e8d0a6924e7183d44bd93dec20ca8049.
This is ebtree commit f452d0f83eca72f6c3484ccb138d341ed6fd27ed.

18 months agoIMPORT: ebtree: switch the sizes and offsets to size_t and ssize_t
Willy Tarreau [Sun, 17 Dec 2023 16:24:01 +0000 (17:24 +0100)] 
IMPORT: ebtree: switch the sizes and offsets to size_t and ssize_t

Let's use these in order to avoid 32-64 bit casts on 64 bit platforms.

This is cbtree commit e4f4c10fcb5719b626a1ed4f8e4e94d175468c34.
This is ebtree commit cc10507385c784d9a9e74ea9595493317d3da99e.

18 months agoIMPORT: ebtree: implement and use flsnz_long() to count bits
Willy Tarreau [Sun, 17 Dec 2023 15:43:25 +0000 (16:43 +0100)] 
IMPORT: ebtree: implement and use flsnz_long() to count bits

The asm code shows multiple conversions. Gcc has always been terribly
bad at dealing with chars, which are constantly converted to ints for
every operation and zero-extended after each operation. But here in
addition there are conversions before and after the flsnz(). Let's
just mark the variables as long and use flsnz_long() to process them
without any conversion. This shortens the code and makes it slightly
faster.

Note that the fls operations could make use of __builtin_clz() on
gcc 4.6 and above, and it would be useful to implement native support
for ARM as well.

This is cbtree commit 1f0f83ba26f2279c8bba0080a2e09a803dddde47.
This is ebtree commit 9c38dcae22a84f0b0d9c5a56facce1ca2ad0aaef.

18 months agoDEV: h2: support passing raw data for a frame
Willy Tarreau [Tue, 10 Oct 2023 15:09:38 +0000 (17:09 +0200)] 
DEV: h2: support passing raw data for a frame

With -r it's possible to pass raw data that will be interpreted by
printf so it even supports \x sequences. E.g. for a RST_STREAM, let's
just use \x00\x00\x00\x00.

18 months agoDEV: h2: add the preface as well in supported output types
Willy Tarreau [Tue, 10 Oct 2023 14:45:45 +0000 (16:45 +0200)] 
DEV: h2: add the preface as well in supported output types

It's annoying to always have to append it from a capture, let's also
support producing the preface.

18 months agoDEV: h2: add the ability to emit literals in mkhdr
Willy Tarreau [Tue, 10 Oct 2023 14:42:39 +0000 (16:42 +0200)] 
DEV: h2: add the ability to emit literals in mkhdr

It's sometimes needed to be able to build a headers frame, let's
add this by permitting to encode key/value pairs as literal headers.

18 months agoCLEANUP: mux-h2: remove the printfs from previous commit on h2 streams limit.
Willy Tarreau [Fri, 5 Jan 2024 18:17:41 +0000 (19:17 +0100)] 
CLEANUP: mux-h2: remove the printfs from previous commit on h2 streams limit.

After thinking about them all the time at the end, I managed to remove
them while editing the commit and to forget to push them :-(

18 months agoMINOR: mux-h2: support limiting the total number of H2 streams per connection
Willy Tarreau [Fri, 13 Oct 2023 16:11:59 +0000 (18:11 +0200)] 
MINOR: mux-h2: support limiting the total number of H2 streams per connection

This patch introduces a new setting: tune.h2.fe.max-total-streams. It
sets the HTTP/2 maximum number of total streams processed per incoming
connection. Once this limit is reached, HAProxy will send a graceful GOAWAY
frame informing the client that it will close the connection after all
pending streams have been closed. In practice, clients tend to close as fast
as possible when receiving this, and to establish a new connection for next
requests. Doing this is sometimes useful and desired in situations where
clients stay connected for a very long time and cause some imbalance inside a
farm. For example, in some highly dynamic environments, it is possible that
new load balancers are instantiated on the fly to adapt to a load increase,
and that once the load goes down they should be stopped without breaking
established connections. By setting a limit here, the connections will have
a limited lifetime and will be frequently renewed, with some possibly being
established to other nodes, so that existing resources are quickly released.

The default value is zero, which enforces no limit beyond those implied by
the protocol (2^30 ~= 1.07 billion). Values around 1000 were found to
already cause frequent enough connection renewal without causing any
perceptible latency to most clients. One notable exception here is h2load
which reports errors for all requests that were expected to be sent over
a given connection after it receives a GOAWAY. This is an already known
limitation: https://github.com/nghttp2/nghttp2/issues/981

The patch was made in two parts inside h2_frt_handle_headers():
  - the first one, at the end of the function, which verifies if the
    configured limit was reached and if it's needed to emit a GOAWAY ;

  - the second, just before decoding the stream frame, which verifies if
    a previously configured limit was ignored by the client, and closes
    the connection if this happens. Indeed, one reason for a connection
    to stay alive for too long definitely comes from a stupid bot that
    periodically fetches the same resource, scans lots of URLs or tries
    to brute-force something. These ones are more likely to just ignore
    the last stream ID advertised in GOAWAY than a regular browser, or
    a well-behaving client such as curl which respects it. So in order
    to make sure we can close the connection we need to enforce the
    advertised limit.

Note that a regular client will not face a problem with that because in
the worst case it will have max_concurrent_streams in flight and this
limit is taken into account when calculating the advertised last
acceptable stream ID.

Just a note: it may also be possible to move the first part above to
h2s_frt_stream_new() instead so that it's not processed for trailers,
though it doesn't seem to be more interesting, first because it has
two return points.

This is something that may be backported to 2.9 and 2.8 to offer more
control to those dealing with dynamic infrastructures, especially since
for now we cannot force a connection to be cleanly closed using rules
(e.g. github issues #946, #2146).

18 months agoBUG/MEDIUM: spoe: Never create new spoe applet if there is no server up
Christopher Faulet [Fri, 5 Jan 2024 16:06:48 +0000 (17:06 +0100)] 
BUG/MEDIUM: spoe: Never create new spoe applet if there is no server up

This test was already performed when a new message is queued into the
sending queue. However not when the last applet is released, in
spoe_release_appctx(). It is a quite old bug. It was introduced by commit
6f1296b5c7 ("BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the
last one is released").

Because of this bug, new SPOE applets may be created and quickly released
because there is no server up, in loop and while there is at least one
message in the sending queue, consuming all the CPU. It is pretty visible if
the processing timeout is high.

To fix the bug, conditions to create or not a SPOE applet are now
centralized in spoe_create_appctx(). The test about the max connections per
second and about number of active servers are moved in this function.

This patch must be backported to all stable versions.

18 months agoBUG/MEDIUM: stconn: Set fsb date if zero-copy forwarding is blocked during nego
Christopher Faulet [Fri, 5 Jan 2024 15:59:06 +0000 (16:59 +0100)] 
BUG/MEDIUM: stconn: Set fsb date if zero-copy forwarding is blocked during nego

During the zero-copy forwarding, if the consumer side reports it is blocked,
it means it is blocked on send. At the stream-connector level, the event
must be reported to be sure to set/update the fsb date. Otherwise, write
timeouts cannot be properly reported. If this happens when no other timeout
is armed, this freezes the stream.

This patch must be backported to 2.9.

18 months agoBUG/MEDIUM: stconn: Forward shutdown on write timeout only if it is forwardable
Christopher Faulet [Fri, 5 Jan 2024 15:48:40 +0000 (16:48 +0100)] 
BUG/MEDIUM: stconn: Forward shutdown on write timeout only if it is forwardable

The commit b9c87f8082 ("BUG/MEDIUM: stconn/stream: Forward shutdown on write
timeout") introduced a regression. In sc_cond_forward_shut(), the write
timeout is considered too early to forward the shutdown. In fact, it is
always considered, even if the shutdown is not forwardable yet. It is of
course unexpected. It is especially an issue when a write timeout is
encountered on server side during the connection establishment. In this
case, if shutdown is forwarded too early on the client side, the connection
is closed before the 503 error sending.

So the write timeout must indeed be considered to forward the shutdown to
the underlying layer, but only if the shutdown is forwardable. Otherwise, we
should do nothing.

This patch should fix the issue #2404. It must be backported as far as 2.2.

18 months agoMINOR: h3: do not consider missing buf room as error on trailers
Amaury Denoyelle [Thu, 4 Jan 2024 11:01:24 +0000 (12:01 +0100)] 
MINOR: h3: do not consider missing buf room as error on trailers

Improve h3_resp_trailers_send() return value to be similar with
h3_resp_data_send(). In particular, if QCS Tx buffer has not enough
space for trailer encoding, 0 is returned instead of an error value,
with QC_SF_BLK_MROOM set.

This unify HTTP/3 headers/data/trailers encoding functions. Negative
error codes are limited to fatal error which should cause a connection
closure. Not enough output buffer space is only a transient condition
which is reflect by the QC_SF_BLK_MROOM flag.

18 months agoBUG/MEDIUM: h3: fix incorrect snd_buf return value
Amaury Denoyelle [Thu, 4 Jan 2024 10:33:33 +0000 (11:33 +0100)] 
BUG/MEDIUM: h3: fix incorrect snd_buf return value

h3_resp_data_send() is used to transcode HTX data into H3 data frames.
If QCS Tx buffer is not aligned when first invoked, two separate frames
may be built, first until buffer end, then with remaining space in
front.

If buffer space is not enough for at least the H3 frame header, -1 is
returned with the flag QC_SF_BLK_MROOM set to await for more room. An
issue arises if this occurs for the second frame : -1 is returned even
though HTX data were properly transcoded and removed on the first step.
This causes snd_buf callback to return an incorrect value to the stream
layer, which in the end will corrupt the channel output buffer.

To fix this, stop considering that not enough remaining space is an
error case. Instead, return 0 if this is encountered for the first frame
or the HTX removed block size for the second one. As QC_SF_BLK_MROOM is
set, this will correctly interrupt H3 encoding. Label err is thus only
properly limited to fatal error which should cause a connection closure.
A new BUG_ON() has been added which should prevent similar issues in the
future.

This issue was detected using the following client :
 $ ngtcp2-client --no-quic-dump --no-http-dump --exit-on-all-streams-close \
   127.0.0.1 20443 -n2 "http://127.0.0.1:20443/?s=50k"

This triggers the following CHECK_IF statement. Note that it may be
necessary to disable fast forwarding to enforce snd_buf usage.

Thread 1 "haproxy" received signal SIGILL, Illegal instruction.
0x00005555558bc48a in co_data (c=0x5555561ed428) at include/haproxy/channel.h:130
130             CHECK_IF_HOT(c->output > c_data(c));
[ ## gdb ## ] bt
 #0  0x00005555558bc48a in co_data (c=0x5555561ed428) at include/haproxy/channel.h:130
 #1  0x00005555558c1d69 in sc_conn_send (sc=0x5555561f92d0) at src/stconn.c:1637
 #2  0x00005555558c2683 in sc_conn_io_cb (t=0x5555561f7f10, ctx=0x5555561f92d0, state=32832) at src/stconn.c:1824
 #3  0x000055555590c48f in run_tasks_from_lists (budgets=0x7fffffffdaa0) at src/task.c:596
 #4  0x000055555590cf88 in process_runnable_tasks () at src/task.c:876
 #5  0x00005555558aae3b in run_poll_loop () at src/haproxy.c:3049
 #6  0x00005555558ab57e in run_thread_poll_loop (data=0x555555d9fa00 <ha_thread_info>) at src/haproxy.c:3251
 #7  0x00005555558ad053 in main (argc=6, argv=0x7fffffffddd8) at src/haproxy.c:3948

In case CHECK_IF are not activated, it may cause crash or incorrect
transfers.

This was introduced by the following commit
  commit 2144d2418651c1f76b91cc1f6e745feecdefcb00
  BUG/MINOR: h3: close connection on sending alloc errors

This must be backported wherever the above patch is.

18 months agoBUILD: quic: Missing quic_ssl.h header protection
Frédéric Lécaille [Thu, 4 Jan 2024 12:56:44 +0000 (13:56 +0100)] 
BUILD: quic: Missing quic_ssl.h header protection

Such "#ifdef USE_QUIC" prepocessor statements are used by QUIC C header
to avoid inclusion of QUIC headers when the QUIC support is not enabled
(by USE_QUIC make variable). Furthermore, this allows inclusions of QUIC
header from C file without having to protect them with others "#ifdef USE_QUIC"
statements as follows:

   #ifdef USE_QUIC
   #include <a QUIC header>
   #include <another one QUIC header>
   #endif /* USE_QUIC */

So, here if this quic_ssl.h header was included by a C file, and compiled without
QUIC support, this will lead to build errrors as follows:

 In file included from <a C file...>:
        include/haproxy/quic_ssl.h:35:35: warning: ‘enum ssl_encryption_level_t’
        declared inside parameter list will not be visible outside of this
        definition or declaration

Should be backported to 2.9 to avoid such building issues to come.

18 months agoCLEANUP: quic: Remaining useless code into server part
Frédéric Lécaille [Thu, 4 Jan 2024 10:16:06 +0000 (11:16 +0100)] 
CLEANUP: quic: Remaining useless code into server part

Remove some QUIC definitions of members from server structure as the haproxy QUIC
stack does not support at all the server part (QUIC client) as this time.
Remove the statements in relation with their initializations.

This patch should be backported as far as 2.6 to save memory.

18 months agoREGTESTS: check attach-srv out of order declaration
Amaury Denoyelle [Tue, 2 Jan 2024 13:40:25 +0000 (14:40 +0100)] 
REGTESTS: check attach-srv out of order declaration

Previous patch fixed a regression which caused some config with
attach-srv to be rejected if the rule was declared before the target
server itself. To better detect this kind of error, mix the declaration
order in the corresponding regtest.

18 months agoBUG/MINOR: server: fix server_find_by_name() usage during parsing
Amaury Denoyelle [Tue, 2 Jan 2024 13:39:26 +0000 (14:39 +0100)] 
BUG/MINOR: server: fix server_find_by_name() usage during parsing

Since below commit, server_find_by_name() now search using
'used_server_id' proxy backend tree :
  4bcfe30414005323a8d4ab986bce92bb736b59df
  OPTIM: server: eb lookup for server_find_by_name()

This introduces a regression if server_find_by_name() is used via
check_config_validity() during post-parsing. Indeed, used_server_id tree
is populated at the same stage so it's possible to not found an existing
server. This can cause incorrect rejection of previously valid
configuration file.

To fix this, servers are now inserted in used_server_id tree during
parsing via parse_server(). This guarantees that server instances can be
retrieved during post parsing.

A known feature which uses server_find_by_name() during post parsing is
attach-srv tcp-rule used for reverse HTTP. Prior to the current fix, a
config was wrongly rejected if the rule was declared before the server
line.

This should not be backported unless the mentionned commit is.

18 months agoMINOR: debug: add features and build options to "show dev"
Willy Tarreau [Tue, 2 Jan 2024 10:08:04 +0000 (11:08 +0100)] 
MINOR: debug: add features and build options to "show dev"

The "show dev" CLI command is still missing useful elements such as the
build options, SSL version etc. Let's just add the build features and
the build options there so that it's possible to collect all of this
from a running process without having to start the executable with -vv.

This is still dumped all at once from the parsing function since the
output is small. If it were to grow, this would possibly require to be
reworked to support a context.

It might be helpful to backport this to 2.9 since it can help narrow
down certain issues.

18 months agoMINOR: global: export a way to list build options
Willy Tarreau [Tue, 2 Jan 2024 09:56:05 +0000 (10:56 +0100)] 
MINOR: global: export a way to list build options

The new function hap_get_next_build_opt() will iterate over the list of
build options. This will be used for debugging, so that the build options
can be retrieved from the CLI.

18 months agoMEDIUM: udp: allow to retrieve the frontend destination address
Dragan Dosen [Tue, 19 Dec 2023 11:46:01 +0000 (12:46 +0100)] 
MEDIUM: udp: allow to retrieve the frontend destination address

A new flag RX_F_PASS_PKTINFO is now available, whose purpose is to mark
that the destination address is about to be retrieved on some listeners.

The address can be retrieved from the first received datagram, and
relies on the IP_PKTINFO, IP_RECVDSTADDR and IPV6_RECVPKTINFO support.

18 months agoMINOR: tcpcheck: export proxy_parse_tcpcheck()
Dragan Dosen [Wed, 27 Dec 2023 13:35:58 +0000 (14:35 +0100)] 
MINOR: tcpcheck: export proxy_parse_tcpcheck()

Export proxy_parse_tcpcheck() in tcpcheck.h

18 months agoMINOR: backend: export get_server_*() functions
Dragan Dosen [Wed, 27 Dec 2023 13:08:08 +0000 (14:08 +0100)] 
MINOR: backend: export get_server_*() functions

This is in preparation for exposing more of the LB internals.

18 months agoMEDIUM: server: simplify snr_set_srv_down() to prevent confusions
Aurelien DARRAGON [Fri, 22 Dec 2023 13:17:41 +0000 (14:17 +0100)] 
MEDIUM: server: simplify snr_set_srv_down() to prevent confusions

snr_set_srv_down() (was formely known as snr_update_srv_status()), is
still too ambiguous because it's not clear whether we will be putting
the server under maintenance or not. This is mainly due to the fact that
the function behaves differently if has_no_ip is set or not.

By reviewing the function callers, it has now become clear that
snr_resolution_cb() is always calling the function with a valid resolution
so we only want to put the server under maintenance if we don't have a
valid IP address. On the other hand snr_resolution_error_cb() always
calls the function on error, with either no resolution (for SRV requests)
or with failing resolution (all cases except RSLV_STATUS_VALID), so in
this case we decide whether to put the server under maintenance case by
case (ie: expired? timeout?)

As a result, let's simplify snr_set_srv_down() so that it is only called
when the caller really thinks that the server should be put under
maintenance, which means always for snr_resolution_error_cb(), and only
if the resolution didn't yield usable ip for snr_resolution_cb().

18 months agoCLEANUP: resolvers: remove some more unused RSLV_UDP flags
Aurelien DARRAGON [Thu, 28 Dec 2023 11:13:48 +0000 (12:13 +0100)] 
CLEANUP: resolvers: remove some more unused RSLV_UDP flags

RSLV_UPD_CNAME and RSLV_UPD_NAME_ERROR flags have now become useless since
3cf7f987 ("MINOR: dns: proper domain name validation when receiving DNS
response") as they are never set, but we forgot to remove them.

18 months agoCLEANUP: resolvers: remove unused RSLV_UPD_OBSOLETE_IP flag
Aurelien DARRAGON [Thu, 28 Dec 2023 10:50:05 +0000 (11:50 +0100)] 
CLEANUP: resolvers: remove unused RSLV_UPD_OBSOLETE_IP flag

RSLV_UPD_OBSOLETE_IP was introduced with commit a8c6db8d2 ("MINOR: dns:
Cache previous DNS answers.") but the commit didn't make any use of it,
and today the flag is still unused. Since we have no valid use for it,
better remove it to prevent confusions.

18 months agoCLEANUP: server: remove ambiguous check in srv_update_addr_port()
Aurelien DARRAGON [Fri, 22 Dec 2023 13:42:22 +0000 (14:42 +0100)] 
CLEANUP: server: remove ambiguous check in srv_update_addr_port()

A leftover check was left by recent patch series about server
addr:svc_port propagation: a check on (msg) being set was performed
in srv_update_addr_port(), but msg is always set, so the check is not
needed and confuses coverity (See GH #2399)

18 months agoCI: use semantic version compare for determing "latest" OpenSSL
Ilya Shipitsin [Fri, 29 Dec 2023 22:31:39 +0000 (23:31 +0100)] 
CI: use semantic version compare for determing "latest" OpenSSL

currently "openssl-3.2.0-beta1" wins over "openssl-3.2.0" due to
string comparision. let's switch to semantic version compare

18 months agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Fri, 29 Dec 2023 19:05:20 +0000 (20:05 +0100)] 
CLEANUP: assorted typo fixes in the code and comments

This is 38th iteration of typo fixes

18 months agoMINOR: stktable: stktable_data_ptr() cannot fail in table_process_entry()
Aurelien DARRAGON [Thu, 28 Dec 2023 08:48:56 +0000 (09:48 +0100)] 
MINOR: stktable: stktable_data_ptr() cannot fail in table_process_entry()

In table_process_entry(), stktable_data_ptr() result is dereferenced
without checking if it's NULL first, which may happen when bad inputs
are provided to the function.

However, data_type and ts arguments were already checked prior to calling
the function, so we know for sure that stktable_data_ptr() will never
return NULL in this case.

However some static code analyzers such as Coverity are being confused
because they think that the result might possibly be NULL.
(See GH #2398)

To make it explicit that we always provide good inputs and expect valid
result, let's switch to the __stktable_data_ptr() unsafe function.

18 months agoRevert "MINOR: mux-quic: Disable zero-copy forwarding for send by default"
Amaury Denoyelle [Fri, 22 Dec 2023 15:26:07 +0000 (16:26 +0100)] 
Revert "MINOR: mux-quic: Disable zero-copy forwarding for send by default"

This reverts commit 18f2ccd244f555348b0d5920e3b71286f1e7b04b.

Found issues related to QUIC fast-forward were resolved (see github
issue #2372). Reenable it by default. If any issue arises, it can be
disabled using the global statement :
  tune.quit.zero-copy-fwd-send off

This can be backported to 2.9, but only after a sensible period of
observation.

18 months agoBUG/MINOR: h3: disable fast-forward on buffer alloc failure
Amaury Denoyelle [Fri, 22 Dec 2023 15:07:10 +0000 (16:07 +0100)] 
BUG/MINOR: h3: disable fast-forward on buffer alloc failure

If QCS Tx buffer cannot be allocated in nego_ff callback, disable
fast-forward for this connection and return immediately. If snd_buf is
later finally used but still no buffer can being allocated, the
connection will be closed on error.

This should fix coverity reported in github issue #2390.

This should be backported up to 2.9.

18 months agoBUG/MINOR: h3: close connection on sending alloc errors
Amaury Denoyelle [Fri, 22 Dec 2023 08:00:13 +0000 (09:00 +0100)] 
BUG/MINOR: h3: close connection on sending alloc errors

When encoding new HTTP/3 frames, QCS Tx buffer must be allocated if
currently NULL. Previously, allocation failure was not properly checked,
leaving the connection in an unspecified state, or worse risking a
crash.

Fix this by setting <h3c.err> to H3_INTERNAL_ERROR each time the
allocation fails. This will stop sending and close the connection. In
the future, it may be better to put the connection on pause waiting for
allocation to succeed but this is too complicated to implement for now
in a reliable way.

Along with the current change, return of all HTX parsing functions
(h3_resp_*_send) were set to a negative value in case of error. A new
BUG_ON() in h3_snd_buf() ensures that if such a value is returned,
either a connection error is register (via <h3c.err>) or buffer is
temporarily full (flag QC_SF_BLK_MROOM).

This should fix github issue #2389.

This should be backported up to 2.6. Note that qcc_get_stream_txbuf()
does not exist in 2.9 and below. mux_get_buf() is its equivalent. An
explicit check b_is_null(&qcs.tx.buf) should be used there.

18 months agoBUG/MINOR: h3: close connection on header list too big
Amaury Denoyelle [Thu, 21 Dec 2023 16:42:43 +0000 (17:42 +0100)] 
BUG/MINOR: h3: close connection on header list too big

When parsing a HTX response, if too many headers are present, stop
sending and close the connection with error code H3_INTERNAL_ERROR.

Previously, no error was reported despite the interruption of header
parsing. This cause an infinite loop. However, this is considered as
minor as it happens on the response path from backend side.

This should be backported up to 2.6.
It relies on previous commit
  "MINOR: h3: check connection error during sending".

18 months agoMINOR: h3: check connection error during sending
Amaury Denoyelle [Fri, 22 Dec 2023 10:45:54 +0000 (11:45 +0100)] 
MINOR: h3: check connection error during sending

If an error occurs during HTX to H3 encoding, h3_snd_buf() should be
interrupted. This commit add this possibility by checking for <h3c.err>
member value. If non null, sending loop is stopped and an error is
reported using qcc_set_error().

This commit does not change any behavior for now, as <h3c.err> is never
set during sending. However, this will change in future commits, most
notably to reject too many headers or handle buffer allocation failure.
As such, this commit should be backported along the following fixes.
Note that in 2.6 qcc_set_error() does not exist and must be replaced by
qcc_emit_cc_app().

18 months agoBUG/MINOR: quic: Missing call to TLS message callbacks
Frédéric Lécaille [Thu, 21 Dec 2023 15:11:35 +0000 (16:11 +0100)] 
BUG/MINOR: quic: Missing call to TLS message callbacks

This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT).

The TLS capture of information from client hello enabled by
tune.ssl.capture-buffer-size could not work with USE_QUIC_OPENSSL_COMPAT. This
is due to the fact the callback set for this feature was replaced by
quic_tls_compat_msg_callback(). In fact this called must be registered by
ssl_sock_register_msg_callback() as this done for the TLS client hello capture.
A call to this function appends the function passed as parameter to a list of
callbacks to be called when the TLS stack parse a TLS message.
quic_tls_compat_msg_callback() had to be modified to return if it is called
for a non-QUIC TLS session.

Must be backported to 2.8.

18 months agoBUG/MINOR: quic: Wrong keylog callback setting.
Frédéric Lécaille [Thu, 21 Dec 2023 13:14:22 +0000 (14:14 +0100)] 
BUG/MINOR: quic: Wrong keylog callback setting.

This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT).

To make this module works, quic_tls_compat_keylog_callback() function must be
set as keylog callback, or at least be called by another keylog callback.
This is what SSL_CTX_keylog() was supposed to do. In addition to export the TLS
secrets via sample fetches this latter also calls quic_tls_compat_keylog_callback()
when compiled with USE_QUIC_OPENSSL_COMPAT defined.

Before this patch, SSL_CTX_keylog() was replaced by quic_tls_compat_keylog_callback()
and the TLS secret were no more exported by sample fetches.

Must be backported to 2.8.

18 months agoBUG/MINOR: mux-quic: disable fast-fwd if connection on error
Amaury Denoyelle [Thu, 21 Dec 2023 10:15:19 +0000 (11:15 +0100)] 
BUG/MINOR: mux-quic: disable fast-fwd if connection on error

Add a check on nego_ff to ensure connection is not on error. If this is
the case, fast-forward is disable to prevent unnecessary sending. If
snd_buf is latter called, stconn will be notified of the error to
interrupt the stream.

This check is necessary to ensure snd_buf and nego_ff are consistent.
Note that previously, if fast-forward was conducted even on connection
error, no sending would occur as qcc_io_send() also check these flags.
However, there is a risk that stconn is never notified of the error
status, thus it is considered as a bug.

Its impact is minimal for now as fast-forward is disable by default on
QUIC. By fixing it, it should be possible to reactive it soon.

This should be backported up to 2.9.

18 months agoMEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS
Amaury Denoyelle [Tue, 19 Dec 2023 10:25:18 +0000 (11:25 +0100)] 
MEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS

Previously, if snd_buf operation was conducted despite QCS already
locally closed, the input buffer was silently dropped. This situation
could happen if a RESET_STREAM was emitted butemission not reported to
the stream layer. Resetting silently the buffer ensure QUIC MUX remain
compliant with RFC 9000 which forbid emission after RESET_STREAM.

Since previous commit, it is now ensured that RESET_STREAM sending will
always be reported to stream-layer. Thus, there is no need anymore to
silently reset the buffer. A BUG_ON() statement is added to ensure this
assumption will remain valid.

The new code is deemed cleaner as it does not hide a missing error
notification on the stconn-layer. Previously, if an error was missing,
sending would continue unnecessarily with a false success status
reported for the stream.

Note that the BUG_ON() statement was also added into nego_ff callback.
This is necessary to ensure both sending path remains consistent.

This patch is labelled as MEDIUM as issues were already encountered in
snd_buf/nego_ff implementation and it's not easy to cover all occurences
during test. If the BUG_ON() is triggered without any apparent
stream-layer issue, this commit should be reverted.

18 months agoBUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission
Amaury Denoyelle [Tue, 19 Dec 2023 10:22:28 +0000 (11:22 +0100)] 
BUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission

On RESET_STREAM emission, the stream Tx channel is closed. This event
must be reported to stream-conn layer to interrupt future send
operations.

Previously, se_fl_set_error() was manually invocated before/after
qcc_reset_stream(). Change this by moving se_fl_set_error() invocation
into the latter. This ensures that notification won't be forget, most
notably in HTTP/3 layer.

In most cases, behavior should be identical as both functions were
called together unless not necessary. However, there is one exception
which could cause a RESET_STREAM emission without error notification :
this happens on H3 trailers parsing error. All other H3 errors happen
before the stream-layer creation and thus the error is notified on
stream creation. This regression has been caused by the following patch :

  152beeec34baed98ad4c186454ddb25e4c496b50
  MINOR: mux-quic: report error on stream-endpoint earlier

Thus it should be backported up to 2.7.

Note that the case described above did not cause any crash or protocol
error. This is because currently MUX QUIC snd_buf operation silently
reset buffer on transmission if QCS is already closed locally. This will
however be removed in a future commit so the current patch is necessary
to prevent an invalid behavior.

18 months agoDOC: fix typo for fastfwd QUIC option
Amaury Denoyelle [Thu, 21 Dec 2023 10:30:39 +0000 (11:30 +0100)] 
DOC: fix typo for fastfwd QUIC option

Replace prefix 'tune.quit.' by 'tune.quic.'.

This should be backported up to 2.9.

18 months agoMINOR: map: add map_*_key converters to provide the matching key
Aurelien DARRAGON [Tue, 19 Dec 2023 10:49:05 +0000 (11:49 +0100)] 
MINOR: map: add map_*_key converters to provide the matching key

All map_*_ converters now have an additional output type: key. Such
converters will return the matched entry's key (as found in the map file)
as a string instead of the value.

Consider this example map file:
 |example.com value1
 |haproxy value2

With the above map file:

str(test.example.com/url),map_dom_key(file.map) will return "example.com"
str(running haproxy),map_sub_key(file.map) will return "haproxy"

This should address GH #1446.

18 months agoMINOR: stktable: use {show,set,clear} table with ptr
Aurelien DARRAGON [Mon, 18 Dec 2023 14:37:25 +0000 (15:37 +0100)] 
MINOR: stktable: use {show,set,clear} table with ptr

This patchs adds support for optional ptr (0xffff form) instead of key
argument to match against existing sticktable entries, ie: if the key is
empty or cannot be matched on the cli due to incompatible characters.
Lookup is performed using a linear search so it will be slower than key
search which relies on eb tree lookup.

Example:

set table mytable key mykey data.gpc0 1

show table mytable
> 0x7fbd00032bd8: key=mykey use=0 exp=86373242 shard=0 gpc0=1

clear table mytable ptr 0x7fbd00032bd8

This patchs depends on:
 - "MINOR: stktable: add table_process_entry helper function"

It should solve GH #2118

18 months agoMINOR: stktable: add table_process_entry helper function
Aurelien DARRAGON [Mon, 18 Dec 2023 13:40:44 +0000 (14:40 +0100)] 
MINOR: stktable: add table_process_entry helper function

Only keep key-related logic in table_process_entry_per_key() function,
and then use table_process_entry() function that takes an entry pointer
as argument to process the entry.

18 months agoDOC: config: add context hint for server keywords
Aurelien DARRAGON [Tue, 21 Nov 2023 11:03:57 +0000 (12:03 +0100)] 
DOC: config: add context hint for server keywords

Add a small list of contexts where each server keyword is expected to be
employed.

This should NOT be backported.

18 months agoMINOR: peers: rely on srv->addr and remove peer->addr
Aurelien DARRAGON [Thu, 30 Nov 2023 16:41:58 +0000 (17:41 +0100)] 
MINOR: peers: rely on srv->addr and remove peer->addr

Similarly to the previous commit, we get rid of unused peer member.

peer->addr was only used to save a copy of the sever's addr at parsing
time. But instead of relying on an intermediate variable, we can actually
use server's address directly when initiating the peer session.

As with other streams created from server's settings (tcp/http, log, ring),
we should rely on srv->svc_port for the port part of the address. This
shouldn't change anything for peers since the address is fully resolved
at parsing time and runtime changes are not supported, but this should
help to make the code future-proof.

18 months agoCLEANUP: peers: remove unused "proto" and "xprt" struct members
Aurelien DARRAGON [Thu, 30 Nov 2023 16:24:39 +0000 (17:24 +0100)] 
CLEANUP: peers: remove unused "proto" and "xprt" struct members

peer->proto and peer->xprt struct members are now pure legacy: they are
only set during parsing but never used afterwards.

This is due to commit 02efedac ("MINOR: peers: now remove the remote
connection setup code") which made some cleanup in the past, but the
unused proto and xprt members were probably left unused by mistake.

Since we don't have valid uses for them, we remove them.

Also, peer_xprt() helper function was removed since it was related to
peer->xprt struct member.

18 months agoCLEANUP: peers: remove unused sock_init_arg struct member
Aurelien DARRAGON [Fri, 1 Dec 2023 08:46:22 +0000 (09:46 +0100)] 
CLEANUP: peers: remove unused sock_init_arg struct member

Since be0688c6 ("MEDIUM: stream_interface: remove the si->init"),
sock_init_arg is completely useless (set but never used later), thus
we remove it.