BUG/MEDIUM: h1: Don't support LF only to mark the end of a chunk size

It is similar to the previous fix but for the chunk size parsing. But this
one is more annoying because a poorly coded application in front of haproxy
may ignore the last digit before the LF thinking it should be a CR. In this
case it may be out of sync with HAProxy and that could be exploited to
perform some sort or request smuggling attack.

While it seems unlikely, it is safer to forbid LF with CR at the end of a
chunk size.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

(cherry picked from commit 4837e998920cbd4e43026e0a638b8ebd71c8018f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: h1: Don't support LF only at the end of chunks

When the message is chunked, all chunks must ends with a CRLF. However, on
old versions, to support bad client or server implementations, the LF only
was also accepted. Nowadays, it seems useless and can even be considered as
an issue. Just forbid LF only at the end of chunks, it seems reasonnable.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

(cherry picked from commit 7b737da8258ebdd84e702a2d65cfd3c423f8e96d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: quic: fix crash on invalid qc_stream_buf_free() BUG_ON

A recent fix was introduced to ensure unsent data are deleted when a
QUIC MUX stream releases its qc_stream_desc instance. This is necessary
to ensure all used buffers will be liberated once all ACKs are received.
This is implemented by the following patch :

  commit ad6b13d3177945bf6a85d6dc5af80b8e34ea6191 (quic-dev/qns)
  BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf

Before this patch, buffer removal was done only on ACK reception. ACK
handling is only done in order from the oldest one. A BUG_ON() statement
is present to ensure this assertion remains valid.

This is however not true anymore since the above patch. Indeed, after
unsent data removal, the current buffer may be empty if it did not
contain yet any sent data. In this case, it is not the oldest buffer,
thus the BUG_ON() statement will be triggered.

To fix this, simply remove this BUG_ON() statement. It should not have
any impact as it is safe to remove buffers in any order.

Note that several conditions must be met to trigger this BUG_ON crash :
* a QUIC MUX stream is destroyed before transmitting all of its data
* several buffers must have been previously allocated for this stream so
  it happens only for transfers bigger than bufsize
* latency should be high enough to delay ACK reception

This must be backported wherever the above patch is (currently targetted
to 2.6).

(cherry picked from commit a13989f109033e7139c58fdc78c35ef02576d788)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: qpack: allow 6xx..9xx status codes

HTTP status codes outside of 100..599 are considered invalid in HTTP
RFC9110. However, it is explicitely stated that range 600..999 is often
used for internal communication so in practice haproxy must be lenient
with it.

Before this patch, QPACK encoder rejected these values. This resulted in
a connection error. Fix this by extending the range of allowed values
from 100 to 999.

This is linked to github issue #2422. Once again, thanks to @yokim-git
for his help here.

This must be backported up to 2.6.

(cherry picked from commit 7d22c4956c53895e5347d84d43f2a70cbdc01bef)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: h3: do not crash on invalid response status code

A crash occurs in h3_resp_headers_send() if an invalid response code is
received from the backend side. Fix this by properly flagging the
connection on error. This will cause a CONNECTION_CLOSE.

This should fix github issue #2422.

Big thanks to ygkim (@yokim-git) for his help and reactivity. Initially,
GDB reported an invalid code source location due to heavy functions
inlining inside h3_snd_buf(). The issue was found after using -Og flag.

This must be backported up to 2.6.

(cherry picked from commit 5d2fe1871a1ec4ec68a8ed262f4526e02e8e9fc1)
[wt: dropped H3_EV_TX_FRAME from trace flags]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: h3: add traces for stream sending function

Replace h3_debug_printf() by real trace for functions used by stream
layer snd_buf callback. A new event type H3_EV_STRM_SEND is created for
the occasion.

This should be backported up to 2.6 to help investigate H3 issues on
stable releases. Note that h3_nego_ff/h3_done_ff definition are not
available from 2.8.

(cherry picked from commit df5cf9123fff25eaa8693b97e5c219827bd45581)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions

It has been found that under some rare error circumstances,
SSL_do_handshake() could return with SSL_ERROR_WANT_READ without
even trying to call the read function, causing permanent wakeups
that prevent the process from sleeping.

It was established that this only happens if the retry flags are
not systematically cleared in both directions upon any I/O attempt,
but, given the lack of documentation on this topic, it is hard to
say if this rather strange behavior is expected or not, otherwise
why wouldn't the library always clear the flags by itself before
proceeding?

In addition, this only seems to affect OpenSSL 1.1.0 and above,
and does not affect wolfSSL nor aws-lc.

A bisection on haproxy showed that this issue was first triggered by
commit a8955d57ed ("MEDIUM: ssl: provide our own BIO."), which means
that OpenSSL's socket BIO does not have this problem. And this one
does always clear the flags before proceeding. So let's just proceed
the same way. It was verified that it properly fixes the problem,
does not affect other implementations, and doesn't cause any freeze
nor spurious wakeups either.

Many thanks to Valentín Gutiérrez for providing a network capture
showing the incident as well as a reproducer. This is GH issue #2403.

This patch needs to be backported to all versions that include the
commit above, i.e. as far as 2.0.

(cherry picked from commit 1ad19917213fac57ee37e581b0ef137e36c6309d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DOC: configuration: clarify http-request wait-for-body

Make it more explicit what happens in the various scenarios that cause
HAProxy to stop waiting when "http-request wait-for-body" is used.

Also fix a couple of grammatical errors.

Fixes: #2410
Signed-Off-By: Thayne McCombs <astrothayne@gmail.com>
(cherry picked from commit 001bddf48c5e646466bd322817e8b4dc78c3760f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf

QCS instances use qc_stream_desc for data buffering on emission. On
stream reset, its Tx channel is closed earlier than expected. This may
leave unsent data into qc_stream_desc.

Before this patch, these unsent data would remain after QCS freeing.
This prevents the buffer to be released as no ACK reception will remove
them. The buffer is only freed when the whole connection is closed. As
qc_stream_desc buffer is limited per connection, this reduces the buffer
pool for other streams of the same connection. In the worst case if
several streams are resetted, this may completely freeze the transfer of
the remaining connection streams.

This bug was reproduced by reducing the connection buffer pool to a
single buffer instance by using the following global statement :

  tune.quic.frontend.conn-tx-buffers.limit 1.

Then a QUIC client is used which opens a stream for a large enough
object to ensure data are buffered. The client them emits a STOP_SENDING
before reading all data, which forces the corresponding QCS instance to
be resetted. The client then opens a new request but the transfer is
freezed due to this bug.

To fix this, adjust qc_stream_desc API. Add a new argument <final_size>
on qc_stream_desc_release() function. Its value is compared to the
currently buffered offset in latest qc_stream_desc buffer. If
<final_size> is inferior, it means unsent data are present in the
buffer. As such, qc_stream_desc_release() removes them to ensure the
buffer will finally be freed when all ACKs are received. It is also
possible that no data remains immediately, indicating that ACK were
already received. As such, buffer instance is immediately removed by
qc_stream_buf_free().

This must be backported up to 2.6. As this code section is known to
regression, a period of observation could be reserved before
distributing it on LTS releases.

(cherry picked from commit ad6b13d3177945bf6a85d6dc5af80b8e34ea6191)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: quic: extract qc_stream_buf free in a dedicated function

On ACK reception, data are removed from buffer via qc_stream_desc_ack().
The buffer can be freed if no more data are left. A new slot is also
accounted in buffer connection pool. Extract this operation in a
dedicated private function qc_stream_buf_free().

This change should have no functional change. However it will be useful
for the next patch which needs to remove a buffer from another function.

This patch is necessary for the following bugfix. As such, it must be
backported with it up to 2.6.

(cherry picked from commit 1da5787db44547590da0f339b797d728f04a01fd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: quic: Stop hardcoding a scale shifting value (CUBIC_BETA_SCALE_FACTOR_SHIFT)

Very minor modification to replace a statement with an hardcoded value by a macro.

Should be backported as far as 2.6 to ease any further modification to come.

(cherry picked from commit 96385f40b508a395fcd802a9dff13e968c12433d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: quic: Remove unused CUBIC_BETA_SCALE_FACTOR_SHIFT macro.

This macro is not used and has a confusing name.

Should be backported as far as 2.6.

(cherry picked from commit 574cf3fe0050a1ef36e1cca4af59a95316d67053)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: quic: newreno QUIC congestion control algorithm no more available

There is a typo in the statement to initialize this variable when selecting
newreno as cc algo:
      const char *newreno = "newrno";
This would have happened if #defines had be used in place of several const char *
variables harcoded values.

Take the opportunity of this patch to use #defines for all the available cc algorithms.

Must be backported to 2.9.

(cherry picked from commit b9a163e7e1ded0dc9cda78a9da652f14f424605a)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: cache: Fix crash when deleting secondary entry

When a cache is "cold" and multiple clients simultaneously try to access
the same resource we must forward all the requests to the server. Next,
every "duplicated" response will be processed in http_action_store_cache
and we will try to cache every one of them regardless of whether this
response was already cached. In order to avoid having multiple entries
for a same primary key, the logic is then to first delete any
preexisting entry from the cache tree before storing the current one.
The actual previous response content will not be deleted yet though
because if the corresponding row is detached from the "avail" list it
might still be used by a cache applet if it actually performed a lookup
in the cache tree before the new response could be received.

This all means that we can end up using a valid row that references a
cache_entry that was already removed from the cache tree. This does not
pose any problem in regular caches (no 'vary' mechanism enabled) because
the applet only works on the data and not the 'cache_entry' information,
but in the "vary" context, when calling 'http_cache_applet_release' we
might call 'delete_entry' on the given entry which in turn tries to
iterate over all the secondary entries to find the right one in which
the secondary entry counter can be updated. We would then call
eb32_next_dup on an entry that was not in the tree anymore which ended
up crashing.

This crash was introduced by "48f81ec09 : MAJOR: cache: Delay cache
entry delete in reserve_hot function" which added the call to
"release_entry" in "http_cache_applet_release" that ended up crashing.

This issue was raised in GitHub #2417.
This patch must be backported to branch 2.9.

(cherry picked from commit 6b695123327de3fd0843462504ce88293d7eda6d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: hlua: fix uninitialized var in hlua_core_get_var()

As raised by Coverity in GH #2223, f034139bc0 ("MINOR: lua: Allow reading
"proc." scoped vars from LUA core.") causes uninitialized reads due to
smp being passed to vars_get_by_name() without being initialized first.

Indeed, vars_get_by_name() tries to read smp->sess and smp->strm pointers.
As we're only interested in the PROC var scope, it is safe to call
vars_get_by_name() with sess and strm pointers set to NULL, thus we
simply memset smp prior to calling vars_get_by_name() to fix the issue.

This should be backported in 2.9 with f034139bc0.

(cherry picked from commit 564addcb727bfb3dd46507ec824f11c20c6bb861)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit e41638af33d76660220ce2e3ed613e8a24fb6e55)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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)

(cherry picked from commit ef02dba7bcf4cf41aaf08fa5cabc8dd2d7537f63)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 87b74697cd58f7780f3585f70a1966553192830f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit e1c8bfd0ed960d3b3dec39e78ad75bec117912d0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 62ef7966f0312e31e6fb8fc9e572e0e907e28558)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: h3: fix checking on NULL Tx buffer

The following patch was backported to handle gracefully Tx buffer
allocation failure.

  BUG/MINOR: h3: close connection on sending alloc errors

However, the backport is not correct. Indeed, mux_get_buf() returns a
pointer to <qcs.tx.buf> field which is always true. Instead, an explicit
check using b_is_null() must be done instead.

This must be backported up to 2.6.

[RELEASE] Released version 2.9.3

Released version 2.9.3 with the following main changes :
    - BUILD: quic: missing include for quic_tp
    - BUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control
    - BUG/MINOR: mux-h2: also count streams for refused ones
    - BUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)

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.

(cherry picked from commit 0eaf42a2a47f2ee73045e48274ed98e00aa44dba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 7021a8c4d8bd8b280d04f481a153a1487deb08d1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 333f2cababa36f4289493647fed4f7dce0babc77)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit c121fcef30559ee9bcf0aeb1634c48f87ef4b22c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[RELEASE] Released version 2.9.2

Released version 2.9.2 with the following main changes :
    - BUG/MINOR: resolvers: default resolvers fails when network not configured
    - DOC: config: Update documentation about local haproxy response
    - BUG/MINOR: server: Use the configured address family for the initial resolution
    - 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
    - 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
    - DOC: fix typo for fastfwd QUIC option
    - BUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission
    - 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
    - MINOR: h3: add traces for connection init stage
    - BUG/MINOR: h3: properly handle alloc failure on finalize
    - BUG/MINOR: h3: close connection on sending alloc errors
    - BUG/MINOR: h3: disable fast-forward on buffer alloc failure
    - CI: use semantic version compare for determing "latest" OpenSSL
    - MINOR: global: export a way to list build options
    - MINOR: debug: add features and build options to "show dev"
    - 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
    - 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
    - MINOR: ot: logsrv struct becomes logger
    - MINOR: ssl: Update ssl_fc_curve/ssl_bc_curve to use SSL_get0_group_name
    - CLEANUP: quic: Double quic_dgram_parse() prototype declaration.
    - BUG/MINOR: map: list-based matching potential ordering regression
    - REGTESTS: add a test to ensure map-ordering is preserved
    - DOC: configuration: corrected description of keyword tune.ssl.ocsp-update.mindelay

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.

(cherry picked from commit f6ab0446fba1f87b1a703acf98ca4587609bd122)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 1088f0b969e18ee5a8fe829d2071507ddef0d05e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit b546bb6d67145db0ebd14c79d72199064208dc2f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 37d5a26cc5a5ebe1abeb2836f89424e33408a186)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 25da2174c62b536b00bc5b0c0bb64ae95fd826b9)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 86298c6913fabb5dba48aeaa9befcd957e2e8ccf)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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).

(cherry picked from commit 983ac4397d2a6fe94c9d4960dcc8272d87801958)
[wt: also removed the debugging printfs killed in 3.0 by e19334a343]
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 72c23bd4cd3ee3a79955fbdd601206a9fa7d19eb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 7cc41514226a2072b538c859cd83dc01de0aeb3f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 7eb7ae283557d8467403b9db35605861a0870add)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 14673fe54d91af3ce28ee2d94ca77f626eb1a7ea)
[wt: adjusted context in h3_resp_data_send]
Signed-off-by: Willy Tarreau <w@1wt.eu>

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: â\80\98enum ssl_encryption_level_tâ\80\99
        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.

(cherry picked from commit fd178ccdb0e0c22ce784d53762bef33b5709bdaa)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 860028db47ac2ad38de56c7711ce872245c70b83)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 0627f470df366e9dadf604f5f473e2dc2693200f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit 9d869b10dea0f4234eb14787f658d6b64a0a4021)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit afba58f21e7c938b793858180b3bd8d8fd6aef5d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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

(cherry picked from commit e6d0b87f7f18cd07163687f15b086eb86ed57957)
Signed-off-by: Willy Tarreau <w@1wt.eu>

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.

(cherry picked from commit cfa6d4cdd058ea112c461349a39d095ba96825e4)
[cf: qcc_get_stream_txbuf() does not exist. Result of mux_get_buf() is tested
     instead]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 2144d2418651c1f76b91cc1f6e745feecdefcb00)
[cf: H3_EV_TX_FRAME removed from trace messages because it does not exist]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: h3: properly handle alloc failure on finalize

If H3 control stream Tx buffer cannot be allocated, return a proper
errur through h3_finalize(). This will cause the emission of a
CONNECTION_CLOSE with error H3_INTERNAL_ERROR and closure of the whole
connection.

This should be backported up to 2.6. Note that 2.9 has some difference
which will cause conflict. The main one is that qcc_get_stream_txbuf()
does not exist in this version. Instead the check in h3_control_send()
should be made after mux_get_buf(). Finally, it may be useful to first
pick previous commit (MINOR: h3: add traces for connection init stage)
to improve context similarity.

(cherry picked from commit 7a3602a1f55dacda5a669865e04474f5d503bab7)
[cf: H3_EV_TX_FRAME removed from trace messages because it does not exist]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: h3: add traces for connection init stage

Add traces H3_EV_H3C_NEW. These are used for h3_init() and h3_finalize()
functions.

(cherry picked from commit a2dbd6d916ed06d7c7b3db5a8dc77bc01c13d2c0)
[cf: CTX adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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".

(cherry picked from commit d077f7ccf48424fa40eea29c69283ec23732c96d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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().

(cherry picked from commit 642016ce033aeb5ccc09b81b98db8046eefd2e2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 10e96fcd177e858284252d413aaf16af06474beb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit b26f6fb0cb358ef1df15e21151906e4c2e9f4364)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 19f4f4d890bc28bbe4b850e1ced9be399f59fafb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

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.

(cherry picked from commit 0a69750a98bbf4c8d9ab2c84e3890956116dbfa0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: fix typo for fastfwd QUIC option

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

This should be backported up to 2.9.

(cherry picked from commit c62487fd08c14efa9dcb89a34f470c2cee8b7a66)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: server/event_hdl: propagate map port info through inetaddr event

server addr:svc_port updates during runtime might set or clear the
SRV_F_MAPPORTS flag. Unfortunately, the flag update is still directly
performed by srv_update_addr_port() function while the addr:svc_port
update is being scheduled for atomic update. Given that existing readers
don't take server's lock to read addr:svc_port, they also check the
SRV_F_MAPPORTS flag right after without the lock.

So we could cause the readers to incorrectly interpret the svc_port from
the server struct because the mapport information is not published
atomically, resulting in inconsistencies between svc_port / mapport flag.
(MAPPORTS flag causes svc_port to be used differently by the reader)

To fix this, we publish the mapport information within the INETADDR server
event and we let the task responsible for updating server's addr and port
position or clear the flag depending on the mapport hint.

This patch depends on:
 - MINOR: server/event_hdl: add server_inetaddr struct to facilitate event data usage
 - MINOR: server/event_hdl: update _srv_event_hdl_prepare_inetaddr prototype

This should be backported in 2.9 with 683b2ae01 ("MINOR: server/event_hdl:
add SERVER_INETADDR event")

(cherry picked from commit 545e72546c0d729fd3de0f753adc905dd0b13908)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: server/event_hdl: update _srv_event_hdl_prepare_inetaddr prototype

Slightly change _srv_event_hdl_prepare_inetaddr() function prototype to
reduce the input arguments by learning some settings directly from the
server. Also taking this opportunity to make the function static inline
since it's relatively simple and not meant to be used directly.

(cherry picked from commit 4e50c31eab9a45bf82c5225fbaf456a8da983ce9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: server/event_hdl: add server_inetaddr struct to facilitate event data usage

event_hdl_cb_data_server_inetaddr struct had some anonymous structs
defined in it, making it impossible to pass as a function argument and
harder to maintain since changes must be performed at multiple places
at once. So instead we define a storage struct named server_inetaddr
that helps to save addr:port server information in INET context.

(cherry picked from commit 14893a6a00f24e14dcca5baa335a7f9d78dafc80)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: stats: unhandled switching rules with TCP frontend

Consider the following configuration:

  backend back
    mode http

  frontend front
    mode tcp
    bind localhost:8080
    stats enable
    stats uri /stats
    tcp-request content switch-mode http if FALSE
    use_backend back

Firing a request to /stats on the haproxy process started with the above
configuration will cause a segfault in http_handle_stats().

The cause for the crash is that in this case, the upgrade doesn't simply
switches to HTTP mode, but also changes the stream backend (changing from
the frontend itself to the targeted HTTP backend).

However, there is an inconsitency in the stats logic between the check
for the stats URI and the actual handling of the stats page.

Indeed, http_stats_check_uri() checks uri parameters from the proxy
undergoing the http analyzers, whereas http_handle_stats() uses s->be
instead.

During stream analysis, from the frontend perspective: s->be defaults to
the frontend. But if the frontend is in TCP mode and the stream is
upgraded to HTTP via backend switching rules, then s->be will be assigned
to the actual HTTP-capable backend in stream_set_backend().

What this means is that when the http analyzer first checks if the current
URI matches the one from the "stats uri" directive, it will check against
the "stats uri" directive from the frontend, but later since the stats
handlers reads the uri from s->be it wil actually use the value from the
backend and the previous safety checks are thus garbage, resulting in
unexpected behavior. (In our test case since the backend didn't define
"stats uri" it is set to NULL, and http_handle_stats() dereferences it)

To fix this, we should ensure that prechecks and actual stats processing
always rely on the same proxy source for stats config directives.

This is what is done in this patch, thanks to the previous commit, since
we can make sure that the stat applet will use ->http_px as its parent
proxy. So here we simply propagate the current proxy being analyzed
through all the stats processing functions.

This patch depends on:
 - MINOR: stats: store the parent proxy in stats ctx (http)

It should be backported up to 2.4.
For 2.4: the fix is less trivial since stats ctx was directly stored
within the applet struct at that time, so this alternative patch must be
used instead (without "MINOR: stats: store the parent proxy in stats ctx
(http)" dependency):

    diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h
    index 014e01ed9..1d9a63359 100644
    --- a/include/haproxy/applet-t.h
    +++ b/include/haproxy/applet-t.h
    @@ -121,6 +121,7 @@ struct appctx {
       * keep the grouped together and avoid adding new ones.
       */
      struct {
    + struct proxy *http_px;  /* parent proxy of the current applet (only relevant for HTTP applet) */
      void *obj1;             /* context pointer used in stats dump */
      void *obj2;             /* context pointer used in stats dump */
      uint32_t domain;        /* set the stats to used, for now only proxy stats are supported */
    diff --git a/src/http_ana.c b/src/http_ana.c
    index b557da89d..1025d7711 100644
    --- a/src/http_ana.c
    +++ b/src/http_ana.c
    @@ -63,8 +63,8 @@ static enum rule_result http_req_restrict_header_names(struct stream *s, struct
     static void http_manage_client_side_cookies(struct stream *s, struct channel *req);
     static void http_manage_server_side_cookies(struct stream *s, struct channel *res);

    -static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *backend);
    -static int http_handle_stats(struct stream *s, struct channel *req);
    +static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *px);
    +static int http_handle_stats(struct stream *s, struct channel *req, struct proxy *px);

     static int http_handle_expect_hdr(struct stream *s, struct htx *htx, struct http_msg *msg);
     static int http_reply_100_continue(struct stream *s);
    @@ -428,7 +428,7 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
      }

      /* parse the whole stats request and extract the relevant information */
    - http_handle_stats(s, req);
    + http_handle_stats(s, req, px);
      verdict = http_req_get_intercept_rule(px, &px->uri_auth->http_req_rules, s);
      /* not all actions implemented: deny, allow, auth */

    @@ -3959,16 +3959,16 @@ void http_check_response_for_cacheability(struct stream *s, struct channel *res)

     /*
      * In a GET, HEAD or POST request, check if the requested URI matches the stats uri
    - * for the current backend.
    + * for the current proxy.
      *
      * It is assumed that the request is either a HEAD, GET, or POST and that the
      * uri_auth field is valid.
      *
      * Returns 1 if stats should be provided, otherwise 0.
      */
    -static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *backend)
    +static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *px)
     {
    - struct uri_auth *uri_auth = backend->uri_auth;
    + struct uri_auth *uri_auth = px->uri_auth;
      struct htx *htx;
      struct htx_sl *sl;
      struct ist uri;
    @@ -4003,14 +4003,14 @@ static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct p
      * s->target which is supposed to already point to the stats applet. The caller
      * is expected to have already assigned an appctx to the stream.
      */
    -static int http_handle_stats(struct stream *s, struct channel *req)
    +static int http_handle_stats(struct stream *s, struct channel *req, struct proxy *px)
     {
      struct stats_admin_rule *stats_admin_rule;
      struct stream_interface *si = &s->si[1];
      struct session *sess = s->sess;
      struct http_txn *txn = s->txn;
      struct http_msg *msg = &txn->req;
    - struct uri_auth *uri_auth = s->be->uri_auth;
    + struct uri_auth *uri_auth = px->uri_auth;
      const char *h, *lookup, *end;
      struct appctx *appctx;
      struct htx *htx;
    @@ -4020,6 +4020,7 @@ static int http_handle_stats(struct stream *s, struct channel *req)
      memset(&appctx->ctx.stats, 0, sizeof(appctx->ctx.stats));
      appctx->st1 = appctx->st2 = 0;
      appctx->ctx.stats.st_code = STAT_STATUS_INIT;
    + appctx->ctx.stats.http_px = px;
      appctx->ctx.stats.flags |= uri_auth->flags;
      appctx->ctx.stats.flags |= STAT_FMT_HTML; /* assume HTML mode by default */
      if ((msg->flags & HTTP_MSGF_VER_11) && (txn->meth != HTTP_METH_HEAD))
    diff --git a/src/stats.c b/src/stats.c
    index d1f3daa98..1f0b2bff7 100644
    --- a/src/stats.c
    +++ b/src/stats.c
    @@ -2863,9 +2863,9 @@ static int stats_dump_be_stats(struct stream_interface *si, struct proxy *px)
      return stats_dump_one_line(stats, stats_count, appctx);
     }

    -/* Dumps the HTML table header for proxy <px> to the trash for and uses the state from
    - * stream interface <si> and per-uri parameters <uri>. The caller is responsible
    - * for clearing the trash if needed.
    +/* Dumps the HTML table header for proxy <px> to the trash and uses the state from
    + * stream interface <si>. The caller is responsible for clearing the trash if
    + * needed.
      */
     static void stats_dump_html_px_hdr(struct stream_interface *si, struct proxy *px)
     {
    @@ -3015,17 +3015,19 @@ static void stats_dump_html_px_end(struct stream_interface *si, struct proxy *px
      * input buffer. Returns 0 if it had to stop dumping data because of lack of
      * buffer space, or non-zero if everything completed. This function is used
      * both by the CLI and the HTTP entry points, and is able to dump the output
    - * in HTML or CSV formats. If the later, <uri> must be NULL.
    + * in HTML or CSV formats.
      */
     int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
    -        struct proxy *px, struct uri_auth *uri)
    +        struct proxy *px)
     {
      struct appctx *appctx = __objt_appctx(si->end);
    - struct stream *s = si_strm(si);
      struct channel *rep = si_ic(si);
      struct server *sv, *svs; /* server and server-state, server-state=server or server->track */
      struct listener *l;
    + struct uri_auth *uri = NULL;

    + if (appctx->ctx.stats.http_px)
    + uri = appctx->ctx.stats.http_px->uri_auth;
      chunk_reset(&trash);

      switch (appctx->ctx.stats.px_st) {
    @@ -3045,7 +3047,7 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
      break;

      /* match '.' which means 'self' proxy */
    - if (strcmp(scope->px_id, ".") == 0 && px == s->be)
    + if (strcmp(scope->px_id, ".") == 0 && px == appctx->ctx.stats.http_px)
      break;
      scope = scope->next;
      }
    @@ -3227,10 +3229,16 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
     }

     /* Dumps the HTTP stats head block to the trash for and uses the per-uri
    - * parameters <uri>. The caller is responsible for clearing the trash if needed.
    + * parameters from the parent proxy. The caller is responsible for clearing
    + * the trash if needed.
      */
    -static void stats_dump_html_head(struct appctx *appctx, struct uri_auth *uri)
    +static void stats_dump_html_head(struct appctx *appctx)
     {
    + struct uri_auth *uri;
    +
    + BUG_ON(!appctx->ctx.stats.http_px);
    + uri = appctx->ctx.stats.http_px->uri_auth;
    +
      /* WARNING! This must fit in the first buffer !!! */
      chunk_appendf(&trash,
                    "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01 Transitional//EN\"\n"
    @@ -3345,17 +3353,21 @@ static void stats_dump_html_head(struct appctx *appctx, struct uri_auth *uri)
     }

     /* Dumps the HTML stats information block to the trash for and uses the state from
    - * stream interface <si> and per-uri parameters <uri>. The caller is responsible
    - * for clearing the trash if needed.
    + * stream interface <si> and per-uri parameters from the parent proxy. The caller
    + * is responsible for clearing the trash if needed.
      */
    -static void stats_dump_html_info(struct stream_interface *si, struct uri_auth *uri)
    +static void stats_dump_html_info(struct stream_interface *si)
     {
      struct appctx *appctx = __objt_appctx(si->end);
      unsigned int up = (now.tv_sec - start_date.tv_sec);
      char scope_txt[STAT_SCOPE_TXT_MAXLEN + sizeof STAT_SCOPE_PATTERN];
      const char *scope_ptr = stats_scope_ptr(appctx, si);
    + struct uri_auth *uri;
      unsigned long long bps = (unsigned long long)read_freq_ctr(&global.out_32bps) * 32;

    + BUG_ON(!appctx->ctx.stats.http_px);
    + uri = appctx->ctx.stats.http_px->uri_auth;
    +
      /* Turn the bytes per second to bits per second and take care of the
       * usual ethernet overhead in order to help figure how far we are from
       * interface saturation since it's the only case which usually matters.
    @@ -3629,8 +3641,7 @@ static void stats_dump_json_end()
      * a pointer to the current server/listener.
      */
     static int stats_dump_proxies(struct stream_interface *si,
    -                              struct htx *htx,
    -                              struct uri_auth *uri)
    +                              struct htx *htx)
     {
      struct appctx *appctx = __objt_appctx(si->end);
      struct channel *rep = si_ic(si);
    @@ -3650,7 +3661,7 @@ static int stats_dump_proxies(struct stream_interface *si,
      px = appctx->ctx.stats.obj1;
      /* skip the disabled proxies, global frontend and non-networked ones */
      if (!px->disabled && px->uuid > 0 && (px->cap & (PR_CAP_FE | PR_CAP_BE))) {
    - if (stats_dump_proxy_to_buffer(si, htx, px, uri) == 0)
    + if (stats_dump_proxy_to_buffer(si, htx, px) == 0)
      return 0;
      }

    @@ -3666,14 +3677,12 @@ static int stats_dump_proxies(struct stream_interface *si,
     }

     /* This function dumps statistics onto the stream interface's read buffer in
    - * either CSV or HTML format. <uri> contains some HTML-specific parameters that
    - * are ignored for CSV format (hence <uri> may be NULL there). It returns 0 if
    - * it had to stop writing data and an I/O is needed, 1 if the dump is finished
    - * and the stream must be closed, or -1 in case of any error. This function is
    - * used by both the CLI and the HTTP handlers.
    + * either CSV or HTML format. It returns 0 if it had to stop writing data and
    + * an I/O is needed, 1 if the dump is finished and the stream must be closed,
    + * or -1 in case of any error. This function is used by both the CLI and the
    + * HTTP handlers.
      */
    -static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *htx,
    -      struct uri_auth *uri)
    +static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *htx)
     {
      struct appctx *appctx = __objt_appctx(si->end);
      struct channel *rep = si_ic(si);
    @@ -3688,7 +3697,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht

      case STAT_ST_HEAD:
      if (appctx->ctx.stats.flags & STAT_FMT_HTML)
    - stats_dump_html_head(appctx, uri);
    + stats_dump_html_head(appctx);
      else if (appctx->ctx.stats.flags & STAT_JSON_SCHM)
      stats_dump_json_schema(&trash);
      else if (appctx->ctx.stats.flags & STAT_FMT_JSON)
    @@ -3708,7 +3717,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht

      case STAT_ST_INFO:
      if (appctx->ctx.stats.flags & STAT_FMT_HTML) {
    - stats_dump_html_info(si, uri);
    + stats_dump_html_info(si);
      if (!stats_putchk(rep, htx, &trash))
      goto full;
      }
    @@ -3733,7 +3742,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht
      case STATS_DOMAIN_PROXY:
      default:
      /* dump proxies */
    - if (!stats_dump_proxies(si, htx, uri))
    + if (!stats_dump_proxies(si, htx))
      return 0;
      break;
      }
    @@ -4112,11 +4121,14 @@ static int stats_process_http_post(struct stream_interface *si)
     static int stats_send_http_headers(struct stream_interface *si, struct htx *htx)
     {
      struct stream *s = si_strm(si);
    - struct uri_auth *uri = s->be->uri_auth;
    + struct uri_auth *uri;
      struct appctx *appctx = __objt_appctx(si->end);
      struct htx_sl *sl;
      unsigned int flags;

    + BUG_ON(!appctx->ctx.stats.http_px);
    + uri = appctx->ctx.stats.http_px->uri_auth;
    +
      flags = (HTX_SL_F_IS_RESP|HTX_SL_F_VER_11|HTX_SL_F_XFER_ENC|HTX_SL_F_XFER_LEN|HTX_SL_F_CHNK);
      sl = htx_add_stline(htx, HTX_BLK_RES_SL, flags, ist("HTTP/1.1"), ist("200"), ist("OK"));
      if (!sl)
    @@ -4166,11 +4178,14 @@ static int stats_send_http_redirect(struct stream_interface *si, struct htx *htx
     {
      char scope_txt[STAT_SCOPE_TXT_MAXLEN + sizeof STAT_SCOPE_PATTERN];
      struct stream *s = si_strm(si);
    - struct uri_auth *uri = s->be->uri_auth;
    + struct uri_auth *uri;
      struct appctx *appctx = __objt_appctx(si->end);
      struct htx_sl *sl;
      unsigned int flags;

    + BUG_ON(!appctx->ctx.stats.http_px);
    + uri = appctx->ctx.stats.http_px->uri_auth;
    +
      /* scope_txt = search pattern + search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */
      scope_txt[0] = 0;
      if (appctx->ctx.stats.scope_len) {
    @@ -4263,7 +4278,7 @@ static void http_stats_io_handler(struct appctx *appctx)
      }

      if (appctx->st0 == STAT_HTTP_DUMP) {
    - if (stats_dump_stat_to_buffer(si, res_htx, s->be->uri_auth))
    + if (stats_dump_stat_to_buffer(si, res_htx))
      appctx->st0 = STAT_HTTP_DONE;
      }

    @@ -4888,6 +4903,7 @@ static int cli_parse_show_stat(char **args, char *payload, struct appctx *appctx

      appctx->ctx.stats.scope_str = 0;
      appctx->ctx.stats.scope_len = 0;
    + appctx->ctx.stats.http_px = NULL; // not under http context
      appctx->ctx.stats.flags = STAT_SHNODE | STAT_SHDESC;

      if ((strm_li(si_strm(appctx->owner))->bind_conf->level & ACCESS_LVL_MASK) >= ACCESS_LVL_OPER)
    @@ -4954,7 +4970,7 @@ static int cli_io_handler_dump_info(struct appctx *appctx)
      */
     static int cli_io_handler_dump_stat(struct appctx *appctx)
     {
    - return stats_dump_stat_to_buffer(appctx->owner, NULL, NULL);
    + return stats_dump_stat_to_buffer(appctx->owner, NULL);
     }

     static int cli_io_handler_dump_json_schema(struct appctx *appctx)

(cherry picked from commit 64b7d8e173d169c31aff1cfc8495dc5e14fa1454)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: stats: store the parent proxy in stats ctx (http)

Some HTTP related stats functions need to know the parent proxy, mainly
to get a pointer on the related uri_auth set by the proxy or to check
scope settings.

The current design (probably historical as only the http context existed
by then) took the other approach: it propagates the uri pointer from the
http context deep down the calling stack up to the relevant functions.
For non-http contexts (cli), the pointer is set to NULL.

Doing so is not very pretty and not easy to maintain. Moreover, there were
still some places in the code were the uri pointer was learned directly
from the stream proxy because the argument was not available as argument
from those functions. This is error-prone, because if one day we decide to
change the source proxy in the parent function, we might still have some
functions down the stack that ignore the top most argument and still do
on their own, and we'll probably end up with inconsistencies.

So in this patch, we take a safer approach: the caller responsible for
creating the stats applet should set the http_px pointer so that any stats
function running under the applet that needs to know if it's running in
http context or needs to access parent proxy info may do so thanks to
the dedicated ctx->http_px pointer.

(cherry picked from commit ef9d692544f0bd961bfa5e6a4569447921bf1137)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: stconn: Disable zero-copy forwarding if consumer is shut or in error

A regression was introduced by commit 2421c6fa7d ("BUG/MEDIUM: stconn: Block
zero-copy forwarding if EOS/ERROR on consumer side"). When zero-copy
forwarding is inuse and the consumer side is shut or in error, we declare it
as blocked and it is woken up. The idea is to handle this state at the
stream-connector level. However this definitly blocks receives on the
producer side. So if the mux is unable to close by itself, but instead wait
the peer to shut, this can lead to a wake up loop. And indeed, with the
passthrough multiplexer this may happen.

To fix the issue and prevent any loop, instead of blocking the zero-copy
forwarding, we now disable it. This way, the stream-connector on producer
side will fallback on classical receives and will be able to handle peer
shutdown properly. In addition, the wakeup of the consumer side was
removed. This will be handled, if necessary, by sc_notify().

This patch should fix the issue #2395. It must be backported to 2.9.

(cherry picked from commit 123a9e7d83213bcc0478840bab2270113275fb0a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: server: Use the configured address family for the initial resolution

A regression was introduced by the commit c886fb58eb ("MINOR: server/ip:
centralize server ip updates"). The configured address family is lost when the
server address is initialized during the startup, for the resolution based on
the libc or based on the server state-file. Thus, "ipv4@" and "ipv6@" prefixed
are ignored.

To fix the bug, we take care to use the configured address family before calling
str2ip2() in srv_apply_lastaddr() and srv_apply_via_libc() functions.

This patch should fix the issue #2393. It must be backported to 2.9.

(cherry picked from commit 3811c1de25bd8b05fb4684614883a0f04f304978)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: Update documentation about local haproxy response

Documentation about 'L' state in the termination state was outdated. Today,
not only the request may be intercepted, but also the response.
Documentation about 'L' must be more generic.

However, documentation about possible 2-letter termination states was also
extended to add 'LC' and 'LH' in the list. And 'LR' was adapted too.

This patch should fix the issue #2384. It may be backported to every stable
versions. Note that on 2.8 and lowers, we talk about session and not stream.

(cherry picked from commit 0a203c1d3f359396e66679991f5c84460fae80e2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: resolvers: default resolvers fails when network not configured

Bug #1740 was opened again, this time a user is complaining about the
"can't create socket for nameserver". This can happen if the resolv.conf
file contains a class of address which was not configured on the
machine, for example IPv6.

The fix does the same as b10b1196b ("MINOR: resolvers: shut the warning
when "default" resolvers is implicit"), and uses the
"resolvers->conf.implicit" variable to emit the error.

Though it is not needed to convert the explicit behavior with a
ERR_WARN, because this is supposed to be an unrecoverable error, unlike
the connect().

Should fix issue #1740.

Must be backported were b10b1196b was backported. (as far as 2.6)

(cherry picked from commit 0d2ebb53f752502137479c9e753a96046d3254ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>

[RELEASE] Released version 2.9.1

Released version 2.9.1 with the following main changes :
    - 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
    - BUG/MINOR: mworker/cli: fix set severity-output support
    - BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
    - BUILD: ssl: update types in wolfssl cert selection callback
    - BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions
    - BUG/MINOR: ext-check: cannot use without preserve-env
    - MINOR: version: mention that it's stable now
    - 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

DOC: config: also add arguments to the converters in the table

Now that dconv supports linking to keywords with arguments from tables,
let's mention the arguments in the summary table of converters.

(cherry picked from commit c5bde03a0acac4263b7ff373f819b680d9a52102)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: add arguments to sample fetch methods in the table

Now that dconv supports linking to keywords with arguments from tables,
let's mention the arguments in the summary tables of sample fetch methods.

(cherry picked from commit 0d261dd13a9d475e3bf9d7bae07d156fa3f5c5a6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-quic: report early error on stream

On STOP_SENDING reception, an error is notified to the stream layer as
no more data can be responded. However, this is not done if the stream
instance is not allocated (already freed for example).

The issue occurs if STOP_SENDING is received and the stream instance is
instantiated after it. It happens if a STREAM frame is received after it
with H3 HEADERS, which is valid in QUIC protocol due to UDP packet
reordering. In this case, stream layer is never notified about the
underlying error. Instead, reponse buffers are silently purged by the
MUX in qmux_strm_snd_buf().

This is suboptimal as there is no point in exchanging data from the
server if it cannot be eventually transferred back to the client.
However, aside from this consideration, no other issue occured. However,
this is not the case with QUIC mux-to-mux implementation. Now, if
mux-to-mux is used, qmux_strm_snd_buf() is bypassed and response if
transferred via nego_ff/done_ff callbacks. However, these functions did
not checked if QCS is already locally closed. This causes a crash when
qcc_send_stream() is called via done_ff.

To fix this crash, there is several approach, one of them would be to
adjust nego_ff/done_ff QUIC callbacks. However, another method has been
chosen. Now stream layer is flagged on error just after its
instantiation if the stream is already locally closed. This ensures that
mux-to-mux won't try to emit data as se_nego_ff() check if the opposide
SD is not on error before continuing.

Note that an alternative solution could be to not instantiate at all
stream layer if QCS is already locally closed. This is the most optimal
solution as it reduce unnecessary allocations and task processing.
However, it's not easy to implement so the easier bug fix has been
chosen for the moment.

This patch is labelled as MEDIUM as it can change behavior of all QCS
instances, wheter mux-to-mux is used or not, and thus could reveal other
architecture issues.

This should fix latest crash occurence on github issue #2392.

It should be backported up to 2.6, until a necessary period of
observation.

(cherry picked from commit af297f19f6ff954b739cde3fa76f99f665b556bd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: mux-h2: Report too large HEADERS frame only when rxbuf is empty

During HEADERS frames decoding, if a frame is too large to fit in a buffer,
an internal error is reported and a RST_STREAM is emitted. On the other
hand, we wait to have an empty rxbuf to decode the frame because we cannot
retry a failed HPACK decompression.

When we are decoding headers, it is valid to return an error if dbuf buffer
is full because no data can be blocked in the rxbuf (which hosts the HTX
message).

However, during the trailers decoding, it is possible to have some data not
sent yet for the current stream in the rxbug and data for another stream
fully filling the dbuf buffer. In this case, we don't decode the trailers
but we must not return an error. We must wait to empty the rxbuf first.

Now, a HEADERS frame is considered as too large if the dbuf buffer is full
and if the rxbuf is empty (the HTX message to be accurate).

This patch should fix the issue #2382. It must be backported to all stable
versions.

(cherry picked from commit 682f73b4fa6d76aa0b5b743fe92777822884772d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: mux-h1: Fix a trace message about C-L header addition

This fixes a cut-paste error on a trace message notifying a 'Content-Length'
header was added during the HTTP message formatting.

(cherry picked from commit 65ca4442401c527530f5b456a8ed47a35c224480)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: mux-h1: Explicitly skip request's C-L header if not set originally

Commit f89ba27caa ("BUG/MEDIUM: mux-h1; Ignore headers modifications about
payload representation") introduced a regression. The Content-Length is no
longer sent to the server for requests without payload but with a
'Content-Lnegth' header explicitly set to 0, like POST request with no
payload. It is of course unexpected. In some cases, depending on the server,
such requests are considered as invalid and a 411-Length-Required is returned.

The above commit is not directly responsible for the bug, it only reveals a too
lax condition to skip the 'Content-Length' header of bodyless requests. We must
only skip this header if none was originally found, during the parsing.

This patch should fix the issue #2386. It must be backported to 2.9.

(cherry picked from commit 966a18e2b485901bd986ebe65670dc4bfbfe65b5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: mux-h1: Cound data from input buf during zero-copy forwarding

During zero-copy forwarding, we first try to forward data found in the input
buffer before trying to receive more data. These data must be removed from
the amount of data to forward (the cound variable).

Otherwise, on an internal retry, in h1_fastfwd(), we can be lead to read
more data than expected. It is especially a problem on the end of a
chunk. An error is erroneously reported because more data than announced are
received.

This patch should fix the issue #2382. It must be backported to 2.9.

(cherry picked from commit eed1e8733c279cda0b6a2db574ead4e0238177f6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: stconn: Block zero-copy forwarding if EOS/ERROR on consumer side

When the producer side (h1 for now) negociates with the consumer side to
perform a zero-copy forwarding, we now consider the consumer side as blocked
if it is closed and this was reported to the SE via a end-of-stream or a
(pending) error.

It is performed before calling ->nego_ff callback function, in se_nego_ff().
This way, all consumer are concerned automatically. The aim of this patch is
to fix an issue with the QUIC mux. Indeed, it is unexpected to send a frame
on an closed stream. This triggers a BUG_ON(). Other muxes are not affected
but it remains useless to try to send data if the stream is closed.

This patch should fix the issue #2372. It must be backported to 2.9.

(cherry picked from commit 2421c6fa7db5b06109a88bffa411c8ba8f948384)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: quic: QUIC CID removed from tree without locking

This bug arrived with this commit:

   BUG/MINOR: quic: Wrong RETIRE_CONNECTION_ID sequence number chec

Every connection ID manipulations against the by thread trees used to store the
connection IDs must be done under the trees locks. These trees are accessed by
the low level connection identification code.

When receiving a RETIRE_CONNECTION_ID frame, the concerned connection ID must
be deleted from the its underlying by thread tree but not without locking!
Add a WR lock around ebmb_delete() call to do so.

Must be backported as far as 2.7.

(cherry picked from commit dd58dff1e64fa382d368958469506c296454fde6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: version: mention that it's stable now

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

BUG/MINOR: ext-check: cannot use without preserve-env

Since 1de44da ("MINOR: ext-check: add an option to preserve environment
variables"), it is now possible to provide an extra argument to
"external-check" directive. This allows to support the "preserve-env"
option which differs from the default behavior.

However a mistake was made, because the config parser doesn't allow the
default configuration anymore: using external-check without argument will
trigger an error:
  'external-check' only supports 'preserve-env' as an argument, found ''.

This is due to as small mistake in the code that make the check
systematically report an error if the first argument is not equal to
"preserve-env". The check was modified so that the error is only reported
if the argument is provided, so that the default behavior is restored.

This should fix GH #2380 and should be backported on 2.9 and potentially
further (anywhere 1de44da is, because a note about an optional backport
up to the 2.6 was left in the original commit message)

(cherry picked from commit 63282f3bfbaee50a0716823bdc8c4251a47e6e39)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions

Some regressions were introduced by 5fea59754b ("MEDIUM: map/acl:
Accelerate several functions using pat_ref_elt struct ->head list")

pat_ref_delete_by_id() fails to properly unlink and free the removed
reference because it bypasses the pat_ref_delete_by_ptr() made for
that purpose. This function is normally used everywhere the target
reference is set for removal, such as the pat_ref_delete() function
that matches pattern against a string. The call was probably skipped
by accident during the rewrite of the function.

With the above commit also comes another undesirable change:
both pat_ref_delete_by_id() and pat_ref_set_by_id() directly use the
<refelt> argument as a valid pointer (they do dereference it).

This is wrong, because <refelt> is unsafe and should be handled as an
ID, not a pointer (hence the function name). Indeed, the calling function
may directly pass user input from the CLI as <refelt> argument, so we must
first ensure that it points to a valid element before using it, else it is
probably invalid and we shouldn't touch it.

What this patch essentially does, is that it reverts pat_ref_set_by_id()
and pat_ref_delete_by_id() to pre 5fea59754b behavior. This seems like
it was the only optimization from the patch that doesn't apply.

Hopefully, after reviewing the changes with Fred, it seems that the 2
functions are only being involved in commands for manipulating maps or
acls on the cli, so the "missed" opportunity to improve their performance
shouldn't matter much. Nonetheless, if we wanted to speed up the reference
lookup by ID, we could consider adding an eb64 tree for that specific
purpose that contains all pattern references IDs (ie: pointers) so that
eb lookup functions may be used instead of linear list search.

The issue was raised by Marko Juraga as he failed to perform an an acl
removal by reference on the CLI on 2.9 which was known to work properly
on other versions.

It should be backported on 2.9.

Co-Authored-by: Frédéric Lécaille <flecaille@haproxy.com>
(cherry picked from commit d7964c52ceb18a690d33d35b0c802660241d9ba9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: ssl: update types in wolfssl cert selection callback

The types have changed in the PR for the wolfSSL_get_sigalg_info()
function, let's update them.

Must be backported in 2.9.

(cherry picked from commit dbe9cea35b7a43a4907fa726e7a8afc31a6fbee3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: quic: Possible buffer overflow when building TLS records

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

This may happen only when the TLS stack has to be provided with more than 1024+1+5+16
bytes of CRYPTO data. In this case several TLS records have to be built in one
call to SSL_provide_quic_data(). A 5-bytes header is created at the head
of these records. This header is used as AAD to cipher the record. But
the length of this AAD was counted two times. One time here in
quic_tls_compat_create_record() (initialization):

 adlen = quic_tls_compat_create_header(qc, rec, ad, 0);

and a second time here in the same function after quic_tls_tls_seal() return:

     ret = aad_len + outlen;

This addition is useless. Note that this bug could be reproduced when haproxy has
to authenticate the client.

Thank you to @vifino for having reported this issue in GH #2381.

Must be backported to 2.8.

(cherry picked from commit c075e4f2fc9e662459f7ab0ce8e13b70c059334a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mworker/cli: fix set severity-output support

"set severity-output" is one of these command that changes the appctx
state so the next commands are affected.

Unfortunately the master CLI works with pipelining and server close
mode, which means the connection between the master and the worker is
closed after each response, so for the next command this is a new appctx
state.

To fix the problem, 2 new flags are added ACCESS_MCLI_SEVERITY_STR and
ACCESS_MCLI_SEVERITY_NB which are used to prefix each command sent to
the worker with the right "set severity-output" command.

This patch fixes issue #2350.

It could be backported as far as 2.6.

(cherry picked from commit 1c1bb8ef2abeab2dc8e4857ec2fef2b19b472be7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: configuration: typo req.ssl_hello_type

rep_ssl_hello_type was renamed in res.ssl_hello_type a long time ago.

This patch fixes a typo where an example was renamed
"rep.ssl_hello_type" instead of "res.ssl_hello_type"

fixes issue #2377 and #2379.

Must be backported in all maintained versions.

(cherry picked from commit 906525b16bf53c156f8b559f8782a81f2b960c2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: lua: Wrong OCSP CID after modifying an SSL certficate (LUA)

This bugfix is the same as the following one:
    "BUG/MINOR: ssl_ckch: Wrong OCSP CID after modifying an SSL certficate"
where the OCSP CID had to be reset when updating a certificate.

Must be backported to 2.8.

(cherry picked from commit 917f7c74d3d30a3f7ac77fd6ea69cfcd529e53a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate

This bug could be reproduced with the "set ssl cert" CLI command to update
a certificate. The OCSP CID is duplicated by ckchs_dup() which calls
ssl_sock_copy_cert_key_and_chain(). It should be computed again by
ssl_sock_load_ocsp(). This may be accomplished resetting the new ckch OCSP CID
returned by ckchs_dup().

This bug may be in relation with GH #2319.

Must be backported to 2.8.

(cherry picked from commit 75f5977ff4ccf205ee386e4f4b0c384fb6a110d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback

This patch allows cli_io_handler_commit_cert() callback called upon
a "commit ssl cert ..." command to prefix the messages returned by the CLI
to the by the ones built by ha_warining(), ha_alert().

Should be interesting to backport this commit to 2.8.

(cherry picked from commit 456ba6e95f094c084c2513a594e59670b5eac2d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: ssl: Double free of OCSP Certificate ID

This bug could be reproduced loading several certificated from "bind" line:
with "server_ocsp.pem" as argument to "crt" setting and updating
the CDSA certificate with the RSA as follows:

echo -e "set ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa \
     <<\n$(cat reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.rsa)\n" | socat - /tmp/stats
followed by an "commit ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa"
command. This could be detected by libasan as follows:

=================================================================
==507223==ERROR: AddressSanitizer: attempting double-free on 0x60200007afb0 in thread T3:
    #0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
    #1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)
    #2 0x7fabc6af54e9 in ossl_asn1_primitive_free (/opt/quictls/lib/libcrypto.so.81.3+0xe14e9)
    #3 0x7fabc6af5960 in ossl_asn1_template_free (/opt/quictls/lib/libcrypto.so.81.3+0xe1960)
    #4 0x7fabc6af569f in ossl_asn1_item_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xe169f)
    #5 0x7fabc6af58a4 in ASN1_item_free (/opt/quictls/lib/libcrypto.so.81.3+0xe18a4)
    #6 0x46a159 in ssl_sock_free_cert_key_and_chain_contents src/ssl_ckch.c:723
    #7 0x46aa92 in ckch_store_free src/ssl_ckch.c:869
    #8 0x4704ad in cli_release_commit_cert src/ssl_ckch.c:1981
    #9 0x962e83 in cli_io_handler src/cli.c:1140
    #10 0xc1edff in task_run_applet src/applet.c:454
    #11 0xaf8be9 in run_tasks_from_lists src/task.c:634
    #12 0xafa2ed in process_runnable_tasks src/task.c:876
    #13 0xa23c72 in run_poll_loop src/haproxy.c:3024
    #14 0xa24aa3 in run_thread_poll_loop src/haproxy.c:3226
    #15 0x7fabc69e7ea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7ea6)
    #16 0x7fabc6907a2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfba2e)

0x60200007afb0 is located 0 bytes inside of 3-byte region [0x60200007afb0,0x60200007afb3)
freed by thread T3 here:
    #0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
    #1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)

previously allocated by thread T2 here:
    #0 0x7fabc6fb573f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f)
    #1 0x7fabc6ae8d77 in ASN1_STRING_set (/opt/quictls/lib/libcrypto.so.81.3+0xd4d77)

Thread T3 created by T0 here:
    #0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
    #1 0xc04f36 in setup_extra_threads src/thread.c:252
    #2 0xa2761f in main src/haproxy.c:3917
    #3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)

Thread T2 created by T0 here:
    #0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
    #1 0xc04f36 in setup_extra_threads src/thread.c:252
    #2 0xa2761f in main src/haproxy.c:3917
    #3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)

SUMMARY: AddressSanitizer: double-free ??:0 __interceptor_free
==507223==ABORTING
Aborted

The OCSP CID stored in the impacted ckch data were freed but not reset to NULL,
leading to a subsequent double free.

Must be backported to 2.8.

(cherry picked from commit 7dab3e8266b8de8604dbae0a90eade85c5c0e3f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[RELEASE] Released version 2.9.0

Released version 2.9.0 with the following main changes :
    - DOC: config: add missing colon to "bytes_out" sample fetch keyword (2)
    - BUG/MINOR: cfgparse-listen: fix warning being reported as an alert
    - DOC: config: add matrix entry for "max-session-srv-conns"
    - DOC: config: fix monitor-fail typo
    - DOC: config: add context hint for proxy keywords
    - DEBUG: stream: Report lra/fsb values for front end back SC in stream dump
    - REGTESTS: sample: Test the behavior of consecutive delimiters for the field converter
    - BUG/MINOR: sample: Make the `word` converter compatible with `-m found`
    - DOC: Clarify the differences between field() and word()
    - BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event
    - BUILD: http_htx: silence uninitialized warning on some gcc versions
    - MINOR: acme.sh: don't use '*' in the filename for wildcard domain
    - MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding
    - MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding
    - MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding
    - MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding
    - MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding
    - MINOR: mux-quic: Disable zero-copy forwarding for send by default
    - DOC: config: update the reminder on the HTTP model and add some terminology
    - DOC: config: add a few more differences between HTTP/1 and 2+
    - DOC: config: clarify session vs stream
    - DOC: config: fix typo abandonned -> abandoned
    - DOC: management: fix two latest typos (optionally, exception)
    - BUG/MEDIUM: peers: fix partial message decoding
    - DOC: management: update stream vs session

DOC: management: update stream vs session

Indicate for some commands such as "show sess" that we now dump streams
and not sessions.

BUG/MEDIUM: peers: fix partial message decoding

peer_recv_msg() may return because the message is incomplete without
checking if a shutdown is pending for the SC. The function relies on
co_getblk() to detect shutdowns. However, the message length decoding may be
interrupted if the multi-bytes integer is incomplete. In this case, the SC
is not check for shutdowns.

When this happens, this leads to an appctx spinning loop.

This patch should fix the issue #2373. It must be backported to 2.8.

DOC: management: fix two latest typos (optionally, exception)

No backport needed, these were introduced by latest commits 3dd55fa13
("MINOR: mworker/cli: implement hard-reload over the master CLI") and
cef29d370 ("MINOR: trace: define simple -dt argument").

DOC: config: fix typo abandonned -> abandoned

No need to backport, it was introduced by recent commit fafa34e5f ("DOC:
config: update the reminder on the HTTP model and add some terminology").

DOC: config: clarify session vs stream

Rename "session" to "stream" where relevant (termination states, queue,
and so on).

DOC: config: add a few more differences between HTTP/1 and 2+

Mention the lack of reason phrase and the existence of pseudo-headers for
the request and the response.

DOC: config: update the reminder on the HTTP model and add some terminology

It was really necessary to try to clear the confusion between sessions
and streams, so let's first lift a little bit the HTTP model part to
better consider new protocols, and explain what a stream is and how this
differs from the earlier sessions.

MINOR: mux-quic: Disable zero-copy forwarding for send by default

There is at least an bug for now in this part and it is still unstable. Thus
it is better to disable it for now by default. It can be enable by setting
tune.quic.zero-copy-fwd-send to 'on'.

MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding

tune.quic.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the QUIC mux only, for sends. For now, there
is no option to disable it for receives because it is not supported yet.

It is enabled ('on') by default.

MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding

tune.h2.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the H2 mux only, for sends. For now, there is
no option to disable it for receives because it is not supported yet.

It is enabled ('on') by default.

MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding

tune.h1.zero-copy-fwd-recv and tune.h1.zero-copy-fwd-send can now be used to
enable or disable the zero-copy fast-forwarding for the H1 mux only, for
receives or sends. Unlike the PT mux, there are 2 options here because
client and server sides can use difference muxes.

Both are enabled ('on') by default.

MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding

tune.pt.zero-copy-forwarding parameter can now be used to enable or disable
the zero-copy fast-forwarding for the PT mux only. It is enabled ('on') by
default. It can be disabled by setting the parameter to 'off'. In this case,
this disables receive and send side.

MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding

Zero-copy fast-forwading feature is a quite new and is a bit sensitive.
There is an option to disable it globally. However, all protocols have not
the same maturity. For instance, for the PT multiplexer, there is nothing
really new. The zero-copy fast-forwading is only another name for the kernel
splicing. However, for the QUIC/H3, it is pretty new, not really optimized
and it will evolved. And soon, the support will be added for the cache
applet.

In this context, it is usefull to be able to enable/disable zero-copy
fast-forwading per-protocol and applet. And when it is applicable, on sends
or receives separately. So, instead of having one flag to disable it
globally, there is now a dedicated bitfield, global.tune.no_zero_copy_fwd.

MINOR: acme.sh: don't use '*' in the filename for wildcard domain

By default acme.sh uses the '*' character in the filename for wildcard.
That can be confusing within HAProxy since the * character in front of a
filename in the stat socket is used to specified an uncommitted
transaction.

This patch replace the '*' by a '_' in the filename.  This is only done
when using the default filename, the name can still be forced with an
asterisk.