BUG/MINOR: h1-htx: Don't flag response as bodyless when a tunnel is established

This reverts commit 225a4d02e1f6a12c0b4f3584949fad3339d71708.

When a 200-OK response is replied to a CONNECT request or a
101-Switching-protocol, a tunnel is considered as established between the
client and the server. However, we must not declare the reponse as
bodyless. Of course, there is no payload, but tunneled data are expected.

Because of this bug, the zero-copy forwarding is disabled on the server
side.

This patch must be backported as far as 2.9.

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

BUG/MAJOR: mux-h1: Wake SC to perform 0-copy forwarding in CLOSING state

When the mux is woken up on I/O events, if the zero-copy forwarding is
enabled, receives are blocked. In this case, the SC is woken up to be able
to perform 0-copy forwarding to the other side. This works well, except for
the H1C in CLOSING state.

Indeed, in that case, in h1_process(), the SC is not woken up because only
RUNNING H1 connections are considered. As consequence, the mux will ignore
connection closure. The H1 connection remains blocked, waiting for the
shutdown timeout. If no timeout is configured, the H1 connection is never
closed leading to a leak.

This patch should fix leak reported by Damien Claisse in the issue #2697. It
should be backported as far as 2.8.

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

BUG/MEDIUM: pattern: prevent UAF on reused pattern expr

Since c5959fd ("MEDIUM: pattern: merge same pattern"), UAF (leading to
crash) can be experienced if the same pattern file (and match method) is
used in two default sections and the first one is not referenced later in
the config. In this case, the first default section will be cleaned up.
However, due to an unhandled case in the above optimization, the original
expr which the second default section relies on is mistakenly freed.

This issue was discovered while trying to reproduce GH #2708. The issue
was particularly tricky to reproduce given the config and sequence
required to make the UAF happen. Hopefully, Github user @asmnek not only
provided useful informations, but since he was able to consistently
trigger the crash in his environment he was able to nail down the crash to
the use of pattern file involved with 2 named default sections. Big thanks
to him.

To fix the issue, let's push the logic from c5959fd a bit further. Instead
of relying on "do_free" variable to know if the expression should be freed
or not (which proved to be insufficient in our case), let's switch to a
simple refcounting logic. This way, no matter who owns the expression, the
last one attempting to free it will be responsible for freeing it.
Refcount is implemented using a 32bit value which fills a previous 4 bytes
structure gap:

        int                        mflags;               /*    80     4 */

        /* XXX 4 bytes hole, try to pack */

        long unsigned int          lock;                 /*    88     8 */
(output from pahole)

Even though it was not reproduced in 2.6 or below by @asmnek (the bug was
revealed thanks to another bugfix), this issue theorically affects all
stable versions (up to c5959fd), thus it should be backported to all
stable versions.

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

BUG/MINOR: pattern: prevent const sample from being tampered in pat_match_beg()

This is a complementary patch to a68affeaa ("BUG/MINOR: pattern: a sample
marked as const could be written"). Indeed the same logic from
pat_match_str() is used there, but we lack the check to ensure that the
sample is not const before writing data to it.

It could be backported to all stable versions.

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

BUG/MEDIUM: clock: detect and cover jumps during execution

After commit e8b1ad4c2 ("BUG/MEDIUM: clock: also update the date offset
on time jumps"), @firexinghe mentioned that the issue was still present
in their case. In fact it depends on the load, which affects the
probability that the time changes between two poll() calls vs that it
changes during poll(). The time correction code used to only deal with
the latter. But under load if it changes between two poll() calls, what
happens then is that before_poll is off, and after returning from poll(),
the date is within bounds defined by before_poll, so no correction is
applied.

After many tests, it turns out that the most reliable solution without
using CLOCK_MONOTONIC is to prevent before_poll from being earlier than
the previous after_poll (trivial), and to cover forward jumps, we need
to enforce a margin. Given that the watchdog kills a looping task within
2 seconds and that no sane setup triggers it, it seems that 2 seconds
remains a safe enough margin. This means that in the worst case, some
forward jumps of up to 2 seconds will not be corrected, leading to an
apparent fast time and low rates. But this is supposed to be an exceptional
event anyway (typically an admin or crontab running ntpdate).

For future versions, given that we now opportunistically call
now_mono_time() before and after poll(), that returns zero if not
supported, we could imagine relying on this one for the thread's local
time when it's non-null.

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

REGTESTS: fix random failures with wrong_ip_port_logging.vtc under load

This test has an expect rule for syslog that looks for [cC]D, to
indicate a client abort or timeout during the data phase. The purpose
was to say that when it fails it must be this, but the very low timeout
(1ms) still makes it prone to succeeding if the machine is highly loaded.

This has become more visible since commit e8b1ad4c2b ("BUG/MEDIUM: clock:
also update the date offset on time jumps") because the clock drift
adjustments are more systematic. Since this commit, running 50 such tests
at twice more than the number of CPUs in parallel is sufficient to yield
errors due to some lines appearing as succeeding:

   make reg-tests -- --j $((($(nproc)+1)*2)) --vtestparams -n50 reg-tests/log/wrong_ip_port_logging.vtc

It was observed that pauses up to 300ms were observed in epoll_wait() in
such circumstances, which were properly fixed by the time drift detection..
Another approach would consist in increasing the permitted margin during
which we don't fix the clock drift but that would not be logical since the
base time had really been awaited for.

This should be backported to all stable releases since the commit above
will trigger the issue more often.

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

DOC: configuration: place the HAPROXY_HTTP_LOG_FMT example on the correct line

When HAPROXY_HTTP_LOG_FMT was added by commit 537b9e7f36 ("MINOR: config:
add environment variables for default log format"), the example was placed
by accident after the clf log format instead of the HTTP log format,
causing a bit of confusion.

This can be backported to 2.8.

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

BUG/MINOR: quic: Too short datagram during packet building failures (aws-lc only)

This issue was reported by Ilya (@Chipitsine) when building haproxy against
aws-lc in GH #2663 where handshakeloss and handshakecorruption interop tests could
lead haproxy to crash after having built too short datagrams:

FATAL: bug condition "first_pkt->type == QUIC_PACKET_TYPE_INITIAL && (first_pkt->flags & (1UL << 0)) && length < 1200" matched at src/quic_tx.c:163
call trace(13):
| 0x55f4ee4dcc02 [ba d9 00 00 00 48 8d 35]: main-0x195bf2
| 0x55f4ee4e3112 [83 3d 2f 16 35 00 00 0f]: qc_send+0x11f3/0x1b5d
| 0x55f4ee4e9ab4 [85 c0 0f 85 00 f6 ff ff]: quic_conn_io_cb+0xab1/0xf1c
| 0x55f4ee6efa82 [48 c7 c0 f8 55 ff ff 64]: run_tasks_from_lists+0x173/0x9c2
| 0x55f4ee6f05d3 [8b 7d a0 29 c7 85 ff 0f]: process_runnable_tasks+0x302/0x6e6
| 0x55f4ee671bb7 [83 3d 86 72 44 00 01 0f]: run_poll_loop+0x6e/0x57b
| 0x55f4ee672367 [48 8b 1d 22 d4 1d 00 48]: main-0x48d
| 0x55f4ee6755e0 [b8 00 00 00 00 e8 08 61]: main+0x2dec/0x335d

This could happen after Handshake packet building failures which follow a successful
Initial packet into the same datagram. In this case, the datagram could be emitted
with a too short length (<1200 bytes).

To fix this, store the datagram only if the first packet is not an Initial packet
or if its length is big enough (>=1200 bytes).

Must be backported as far as 2.6.

(cherry picked from commit eb1a097a6687cd8b93d981eff3d7e257c448ff3e)
[fl: same patch wich <wrlen> replaced by <dlen> (datagram length)]
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

BUG/MINOR: quic: Crash from trace dumping SSL eary data status (AWS-LC)

This bug follows this patch:
     MINOR: quic: Add trace for QUIC_EV_CONN_IO_CB event.
where a new third variable was added to be dumped from QUIC_EV_CONN_IO_CB trace
event. The quic_trace() code did not reveal there was already another variable
passed as third argument but not dumped. This leaded to crash when dereferencing
a point to an int in place of a point to an SSL object.

This issue was reproduced only by handshakecorruption aws-lc interop test with
s2n-quic as client.

Note that this patch must be backported with this one:
     BUG/MEDIUM: quic: always validate sender address on 0-RTT
which depends on the commit mentionned above.

(cherry picked from commit db13df3d6e64e9993b90f048734538491082cbed)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

BUG/MEDIUM: quic: always validate sender address on 0-RTT

It has been reported by Wedl Michael, a student at the University of Applied
Sciences St. Poelten, a potential vulnerability into haproxy as described below.

An attacker could have obtained a TLS session ticket after having established
a connection to an haproxy QUIC listener, using its real IP address. The
attacker has not even to send a application level request (HTTP3). Then
the attacker could open a 0-RTT session with a spoofed IP address
trusted by the QUIC listen to bypass IP allow/block list and send HTTP3 requests.

To mitigate this vulnerability, one decided to use a token which can be provided
to the client each time it successfully managed to connect to haproxy. These
tokens may be reused for future connections to validate the address/path of the
remote peer as this is done with the Retry token which is used for the current
connection, not the next one. Such tokens are transported by NEW_TOKEN frames
which was not used at this time by haproxy.

So, each time a client connect to an haproxy QUIC listener with 0-RTT
enabled, it is provided with such a token which can be reused for the
next 0-RTT session. If no such a token is presented by the client,
haproxy checks if the session is a 0-RTT one, so with early-data presented
by the client. Contrary to the Retry token, the decision to refuse the
connection is made only when the TLS stack has been provided with
enough early-data from the Initial ClientHello TLS message and when
these data have been accepted. Hopefully, this event arrives fast enough
to allow haproxy to kill the connection if some early-data have been accepted
without token presented by the client.

quic_build_post_handshake_frames() has been modified to build a NEW_TOKEN
frame with this newly implemented token to be transported inside.

quic_tls_derive_retry_token_secret() was renamed to quic_do_tls_derive_token_secre()
and modified to be reused and derive the secret for the new token implementation.

quic_token_validate() has been implemented to validate both the Retry and
the new token implemented by this patch. When this is a non-retry token
which could not be validated, the datagram received is marked as requiring
a Retry packet to be sent, and no connection is created.

When the Initial packet does not embed any non-retry token and if 0-RTT is enabled
the connection is marked with this new flag: QUIC_FL_CONN_NO_TOKEN_RCVD. As soon
as the TLS stack detects that some early-data have been provided and accepted by
the client, the connection is marked to be killed (QUIC_FL_CONN_TO_KILL) from
ha_quic_add_handshake_data(). This is done calling qc_ssl_eary_data_accepted()
new function. The secret TLS handshake is interrupted as soon as possible returnin
0 from ha_quic_add_handshake_data(). The connection is also marked as
requiring a Retry packet to be sent (QUIC_FL_CONN_SEND_RETRY) from
ha_quic_add_handshake_data(). The the handshake I/O handler (quic_conn_io_cb())
knows how to behave: kill the connection after having sent a Retry packet.

About TLS stack compatibility, this patch is supported by aws-lc. It is
disabled for wolfssl which does not support 0-RTT at this time thanks
to HAVE_SSL_0RTT_QUIC.

This patch depends on these commits:

     MINOR: quic: Add trace for QUIC_EV_CONN_IO_CB event.
     MINOR: quic: Implement qc_ssl_eary_data_accepted().
     MINOR: quic: Modify NEW_TOKEN frame structure (qf_new_token struct)
     BUG/MINOR: quic: Missing incrementation in NEW_TOKEN frame builder
     MINOR: quic: Token for future connections implementation.
     MINOR: quic: Implement quic_tls_derive_token_secret().
     MINOR: tools: Implement ipaddrcpy().

Must be backported as far as 2.6.

(cherry picked from commit f627b9272bd8ffca6f2f898bfafc6bf0b84b7d46)
[fl: Add ->flags to quic_dgram struct (would arrive with quic_initial feature).
     Add QUIC_DGRAM_FL_ quic_dgram flags (would arrive with quic_initial feature).
     Modify quic_rx_pkt_retrieve_conn() to fix a compilation issue and correctly
     handle the "if (pkt->token_len) {}" else block to do so with quic_initial
     feature]
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: quic: Add trace for QUIC_EV_CONN_IO_CB event.

Dump the early data status from QUIC_EV_CONN_IO_CB trace event.
This is very helpful to know if the QUIC server has accepted the
early data received from clients.

(cherry picked from commit 8854cef03672235addec6f3baafcc44f0e0441f4)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: quic: Implement qc_ssl_eary_data_accepted().

This function is a wrapper around SSL_get_early_data_status() for
OpenSSL derived stack and SSL_early_data_accepted() boringSSL derived
stacks like AWS-LC. It returns true for a TLS server if it has
accepted the early data received from a client.

Also implement quic_ssl_early_data_status_str() which is dedicated to be used
for debugging purposes (traces). This function converts the enum returned
by the two function mentionned above to a human readable string.

(cherry picked from commit 609b1245610c8f3e219a1eff12d13559cae109cd)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: quic: Modify NEW_TOKEN frame structure (qf_new_token struct)

Modify qf_new_token structure to use a static buffer with QUIC_TOKEN_LEN
as size as defined by the token for future connections (quic_token.c).
Modify consequently the NEW_TOKEN frame parser (see quic_parse_new_token_frame()).
Also add comments to denote that the NEW_TOKEN parser function is used only by
clients and that its builder is used only by servers.

(cherry picked from commit e926378375bcf579daadea071c600651eb7dce0d)
[fl: remove useless openssl/chacha.h header inclusion when moving
     openssl-compat.h at the start of the header inclusions as expected by this
     patch]
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

BUG/MINOR: quic: Missing incrementation in NEW_TOKEN frame builder

quic_build_new_token_frame() is the function which is called to build
a NEW_TOKEN frame into a buffer. The position pointer for this buffer
was not updated, leading the NEW_TOKEN frame to be malformed.

Must be backported as far as 2.6.

(cherry picked from commit 76c80605a6cc7880e4184c52fb1a314367410271)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: quic: Token for future connections implementation.

There exist two sorts of token used by QUIC. They are both used to validate
the peer address (path validation). Retry are used for the current
connection the client want to open. This patch implement the other
sort of tokens which after having been received from a connection, may
be provided for the next connection from the same IP address to validate
it (or validate the network path between the client and the server).

The token generation is implemented by quic_generate_token(), and
the token validation by quic_token_chek(). The same method
is used as for Retry tokens to build such tokens to be reused for
future connections. The format is very simple: one byte for the format
identifier to distinguish these new tokens for the Retry token, followed
by a 32bits timestamps. As this part is ciphered with AEAD as cryptographic
algorithm, 16 bytes are needed for the AEAD tag. 16 more random bytes
are added to this token and a salt to derive the AEAD secret used
to cipher the token. In addition to this salt, this is the client IP address
which is used also as AAD to derive the AEAD secret. So, the length of
the token is fixed: 37 bytes.

(cherry picked from commit f5b09dc452f582eb876527fd28103bc29c51afad)
[fl: very minor Makefile modif to correctly add quic_token.o object to be compiled]
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MEDIUM: ssl/quic: implement quic crypto with EVP_AEAD

The QUIC crypto is using the EVP_CIPHER API in order to achieve
authenticated encryption, this was the API which was used with OpenSSL.
With libraries that inspires from BoringSSL (libreSSL and AWS-LC), the
AEAD algorithms are implemented using the EVP_AEAD API.

This patch converts the call to the EVP_CIPHER API when called in the
contex of AEAD cryptography for QUIC.

The patch defines some QUIC_AEAD macros that can be either EVP_CIPHER or
EVP_AEAD depending on the library.

This was mainly done for AWS-LC but this could be useful for other
libraries. This should finally allow to use CHACHA20_POLY1305 with
AWS-LC.

This patch allows to use the following ciphers with the EVP_AEAD API:
- TLS1_3_CK_AES_128_GCM_SHA256
- TLS1_3_CK_AES_256_GCM_SHA384

AWS-LC does not implement TLS1_3_CK_AES_128_CCM_SHA256 and
TLS1_3_CK_CHACHA20_POLY1305_SHA256 requires some hack for headers
protection which will come in another patch.

(cherry picked from commit 31c831e29b432f0a9958be63948e8f4cb278e9f8)
[fl: required to support NEW_TOKEN which depends on QUIC_AEAD_* definitions]
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: quic: Implement quic_tls_derive_token_secret().

This is function is similar to quic_tls_derive_retry_token_secret().
Its aim is to derive the secret used to cipher the token to be used
for future connections.

This patch renames quic_tls_derive_retry_token_secret() to a more
and reuses its code to produce a more generic one: quic_do_tls_derive_token_secret().
Two arguments are added to this latter to produce both quic_tls_derive_retry_token_secret()
and quic_tls_derive_token_secret() new function which calls
quic_do_tls_derive_token_secret().

(cherry picked from commit 74caa0eece1cc3a8b35f1d34674ea5f357819314)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

MINOR: tools: Implement ipaddrcpy().

Implement ipaddrcpy() new function to copy only the IP address from
a sockaddr_storage struct object into a buffer.

(cherry picked from commit fb7a0922038932a6b82f1827a0214c5d2e8da32e)
Signed-off-by: Frederic Lecaille <flecaille@haproxy.com>

BUG/MEDIUM: clock: also update the date offset on time jumps

In GH issue #2704, @swimlessbird and @xanoxes reported problems handling
time jumps. Indeed, since 2.7 with commit 4eaf85f5d9 ("MINOR: clock: do
not update the global date too often") we refrain from updating the global
offset in case it didn't change. But there's a catch: in case of a large
time jump, if the poller was interrupted, the local time remains the same
and we return immediately from there without updating the offset. It then
becomes incorrect regarding the "date" value, and upon subsequent call to
the poller, there's no way to detect a jump anymore so we apply the old,
incorrect offset and the date becomes wrong. Worse, going back to the
original time (then in the past), global_now_ns remains higher than the
local time and neither get updated anymore.

What is missing in practice is to immediately update the offset when
detecting a time jump. In an ideal world, the offset would be updated
upon every call, that's what was being done prior to commit above but
it's extremely CPU intensive on large systems. However we can perfectly
afford to update the offset every time we detect a time jump, as it's
not as common.

This needs to be backported as far as 2.8. Thanks to both participants
above for providing very helpful details.

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

BUILD: quic: 32bits build broken by wrong integer conversions for printf()

Since these commits the 32bits build is broken due to several errors as follow:

CC src/quic_cli.o
src/quic_cli.c: In function ‘dump_quic_full’:
src/quic_cli.c:285:94: error: format ‘%ld’ expects argument of type ‘long int’,
        but argument 5 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Werror=format=]
  285 |                         chunk_appendf(&trash, "  [initl] rx.ackrng=%-6zu tx.inflight=%-6zu(%ld%%)\n",
      |                                                                                            ~~^
      |                                                                                              |
      |                                                                                              long int
      |                                                                                            %lld
  286 |                                       pktns->rx.arngs.sz, pktns->tx.in_flight,
  287 |                                       pktns->tx.in_flight * 100 / qc->path->cwnd);
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                                 |
      |                                                                 uint64_t {aka long long unsigned int}

Replace several %ld by %llu with ull as printf conversion in quic_clic.c and a
%ld by %lld with (long long) as printf conversion in quic_cc_cubic.c.

Thank you to Ilya (@chipitsine) for having reported this issue in GH #2689.

Must be backported to 3.0.

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

BUG/MINOR: cfgparse-global: remove tune.fast-forward from common_kw_list

Remove tune.fast-forward from common_kw_list. It was replaced by
'tune.disable-fast-forward' and it's no longer present in "if..else if.."
parser from cfg_parse_global(). Otherwise, it may be shown as the best-match
keyword for some tune options, which is now wrong.

Should be backported in versions 2.9 and 3.0.

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

DOC: config: correct the table for option tcplog

option tcplog was reported as functional in the backend section in
error. This can be back ported as needed but it simply corrects
that.

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

BUG/MINOR: pattern: pat_ref_set: return 0 if err was found

pat_ref_set_elt() returns 0, if we are run out of memory or can't parse a new
map value. Any arror message emitted by pat_ref_set_elt() is saved in err
buffer, if its provided by caller. These error messages are cumulated during
the loop.

pat_ref_set() is used to update values in map, referred to the same given key.
If during the update pat_ref_set_elt() fails, let's retun 0 to caller
immediately. We have the same non-unique key and the same new value in each
loop. So it seems quite odd to cumulate the same error messages and print it in
CLI:

        > add map @1 mytest.map <<
        + 1.0.1.11 TestA
        + 1.0.1.11 TESTA
        + 1.0.1.11 test_a
        +

        > set map mytest.map 1.0.1.11 15
         unable to parse '15' unable to parse '15' unable to parse '15'.

cli_parse_set_map(), which calls pat_ref_set() to update map, will return only
one error message with this patch:

> set map mytest.map 1.0.1.11 15
 unable to parse '15'.

hlua_set_map() and http_action_set_map() don't provide error buffer and will
just exit on the first error.

This should be backported in all stable versions.

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

BUG/MINOR: pattern: pat_ref_set: fix UAF reported by coverity

memprintf() performs realloc and updates then the pointer to an output buffer,
where it has written the data. So free() is called on the previous buffer
address, if it was provided.

pat_ref_set_elt() uses memprintf() to write its error message as well as
pat_ref_set(). So, when we re-enter into the while loop the second time and
pat_ref_set_elt() has returned, the *err ptr (previous value of *merr) is
already freed by memprintf() from pat_ref_set_el().

'if (!found)' condition is false at this point, because we've found a node at
the first loop. So, the second memprintf(), in order to write error messages,
does again free(*err).

This should be backported in all stable versions.

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

BUG/MINOR: h3: properly reject too long header responses

When encoding HTX to HTTP/3 headers on the response path, a bunch of
ABORT_NOW() where used when buffer room was not enough. In most cases
this is safe as output buffer has just been allocated and so is empty at
the start of the function. However, with a header list longer than a
whole buffer, this would cause an unexpected crash.

Fix this by removing ABORT_NOW() statement with proper error return
path. For the moment, this would cause the whole connection to be close
rather than the stream only. This may be further improved in the future.

Also remove ABORT_NOW() when encoding frame length at the end of headers
or trailers encoding. Buffer room is sufficient as it was already
checked prior in the same function.

This should be backported up to 2.6. Special care should be handled
however as this code path has changed frequently :
* for 2.9 and older, the extra following statement must be inserted
  prior each newly added goto statement :
  h3c->err = H3_INTERNAL_ERROR;
* for 2.6, trailers support is not implemented. As such, related chunks
  should just be ignored when backporting.

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

BUG/MINOR: proto_uxst: delete fd from fdtab if listen() fails

This patch is done mostly as a safeguard in order not to trigger
BUG_ON(fdtab[fd].owner != NULL) check, if listen() will fail on UNIX domain
socket.

In uxst_bind_listener(), the pretty same logic of closing socket on error path
was kept, as it was in tcp_bind_listener() before. The use of fd_delete() was
not generalized, when the support of UNIX sock_stream protocol was implemented.
So, let's remove fd from fdtab on failure, instead of closing it. Otherwise,
uxst_bind_listener(), which could be called in loop for each receiver, will
obtain the same fd via socket() for the next receiver. Then, it will bind it
again and it will try to re-insert it in fdtab.

This can be backported to all stable versions.

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

BUG/MINOR: mux-quic: do not send too big MAX_STREAMS ID

QUIC stream IDs are expressed as QUIC variable integer which cover the
range for 0 to 2^62 - 1. As such, it is forbidden to send an ID for
MAX_STREAMS flow-control frame which would allow to overcome this value.

This patch fixes MAX_STREAMS emission to ensure sent value is valid.
This also ensures that the peer cannot open a stream with an invalid ID
as this would cause a flow-control violation instead.

This must be backported up to 2.6.

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

REGTESTS: mcli: test the pipelined commands on master CLI

A recent fix broke the pipelined command on the master CLI, this
reg-tests implement a simple test that allow to check its right
behavior.

This could be backported as far as 2.6.

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

BUG/MEDIUM: mworker/cli: fix pipelined modes on master CLI

Since commit 3d93ecc ("BUG/MAJOR: cli: Restore non-interactive mode
behavior with pipelined commands") and commit 598c7f16 ("BUG/MEDIUM:
cli: Warn if pipelined commands are delimited by a \n"), the pipelined
command on the master CLI are either broken or emit warnings depending
on which version.

The reason is that mode applied on the master CLI are saved on the in
the current CLI session, and then reinserted for each pipelined command,
however, these commande were inserted as new lines.

For example:

 "@1; expert-mode on; debug dev log foo; debug dev log bar"

 Would be sent as:

  "expert mode on\ndebug dev log foo"
  "expert mode on\ndebug dev log bar"

This patch fixes the issue by using the new ci_insert() function which
inserts a string instead of a newline, and the command are now suffixed
by ';' upon insertion allowing a correct pipelined command chain.

This must be backported with the previous commit introducing ci_insert()
in every stable version.

This is broken since the 3.0 version, but it emits a warning in every
version below, because 598c7f164 was backported.

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

MINOR: channel: implement ci_insert() function

ci_insert() is a function which allows to insert a string <str> of size
<len> at <pos> of the input buffer. This is the equivalent of
ci_insert_line2() but without inserting '\r\n'

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

BUG/MINOR: proto_tcp: keep error msg if listen() fails

If listen() fails, we need to keep the message about it, which is copied then
in errmsg buffer on the error path. This buffer is properly provided by the
caller (protocol_bind_all()) and reallocated if needed in memprintf(), but
it was deleted without being returned.

This can be backported to all stable versions.

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

BUG/MINOR: proto_tcp: delete fd from fdtab if listen() fails

If listen() fails, fd should be deleted from fdtab, not just closed. Otherwise,
sock_inet_bind_receiver(), which is called in loop for each receiver, will
obtain the same fd via socket() for the next receiver, registered in the
receivers list. Then, it will bind it again and it will try to re-insert it in
fdtab, and fd_insert() will trigger the BUG_ON(fdtab[fd].owner != NULL) check.

When tcp_bind_listener() code was implemented, the use of fd_delete() was
not generalized and this one remained overlooked.

This can be backported to all stable versions.

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

BUG/MINOR: quic/trace: make quic_conn_enc_level_init() emit NEW not CLOSE

The event emitted by this trace was of type CLOSE instead of NEW, which
would somtimes temporarily pause a started trace.

This can be backported to 3.0, probably 2.6.

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

BUG/MINOR: trace/quic: make "qconn" selectable as a lockon criterion

The test was was performed but there's no way to set the option! Let's
just add "qconn" to select the quic conn when the source supports it.

This can be backported at least to 3.0, probably 2.6.

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

BUG/MINOR: trace: automatically start in waiting mode with "start <evt>"

The doc clearly says that "start <evt>" should leave the trace in pause
mode until the indicated event appears. However it's not what's happening,
the state is not changed until one command uses "now", so it's typically
needed to configure the events with "start <evt>" then enable the waiting
mode using "pause now". This is counter-intuitive and does not match the
doc, so let's fix it so that "start <evt>" switches from stopped to waiting
as long as at least one event is enabled.

This can be backported to all versions.

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

BUG/MEDIUM: trace: fix null deref in lockon mechanism since TRACE_ENABLED()

When calling TRACE_ENABLED(), which is called by TRACE_PRINTF(), we pass
a NULL plockptr to __trace_enabled(). This argument is used when lockon
is active, and may update the pointer. This is an overlook which also
broke the lockon mechanism because now for calls from __trace(), it
dereferences a pointer pointing to NULL, and never updates it due to the
broken condition, so that trace() never sets up src->lockon_ptr.

The bug was introduced in 2.8 by commit 8f9a9704bb ("MINOR: trace: add a
TRACE_ENABLED() macro to determine if a trace is active"), so the fix must
be backported there.

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

BUG/MINOR: trace/quic: permit to lock on frontend/connect/session etc

These ones were not proposed in the list of trackable elements. Note
that this depends on previous commit:

    BUG/MINOR: trace/quic: enable conn/session pointer recovery from quic_conn

This should be backported to at least 3.0, maybe even 2.6.

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

BUG/MINOR: trace/quic: enable conn/session pointer recovery from quic_conn

In __trace_enabled(), a quic_conn was detected, but it was not possible
to derive the connection nor the session from it, which was quite limiting
in terms of ability to track a same instance.

This should be backported to at least 3.0, maybe even 2.6.

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

DOC: configuration: fix alphabetical ordering of {bs,fs}.aborted

These must be before {bs,fs}.id, not after. Should be backported wherever
068ce2d5d2 ("MINOR: stconn: Add samples to retrieve about stream aborts")
is (normally 3.0).

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

BUG/MINOR: fcgi-app: handle a possible strdup() failure

This defect was found by the coccinelle script "unchecked-strdup.cocci".
It can be backported to 2.2.

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

BUG/MEDIUM: peer: Notify the applet won't consume data when it waits for sync

When the peer applet is waiting for a synchronisation with the global sync
task, we must notify it won't consume data. Otherwise, if some data are
already waiting in the input buffer, the applet will be woken up in loop and
this wil trigger the watchdog. Once synchronized, the applet is woken up. In
that case, the peer applet must indicate it is going to consume data again.

This patch should fix the issue #2656. It must be backported to 3.0.

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

BUG/MEDIUM: mux-h2: Propagate term flags to SE on error in h2s_wake_one_stream

When a stream is explicitly woken up by the H2 conneciton, if an error
condition is detected, the corresponding error flag is set on the SE. So
SE_FL_ERROR or SE_FL_ERR_PENDING, depending if the end of stream was
reported or not.

However, there is no attempt to propagate other termination flags. We must
be sure to properly set SE_FL_EOI and SE_FL_EOS when appropriate to be able
to switch a pending error to a fatal error.

Because of this bug, the SE remains with a pending error and no end of
stream, preventing the applicative stream to trully abort it. It means on
some abort scenario, it is possible to block a stream infinitely.

This patch must be backported at least as far as 2.8. No bug was observed on
older versions while the same code is inuse.

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

BUG/MEDIUM: h2: Only report early HTX EOM for tunneled streams

For regular H2 messages, the HTX EOM flag is synonymous the end of input. So
SE_FL_EOI flag must also be set on the stream-endpoint descriptor. However,
there is an exception. For tunneled streams, the end of message is reported
on the HTX message just after the headers. But in that case, no end of input
is reported on the SE.

But here, there is a bug. The "early" EOM is also report on the HTX messages
when there is no payload (for instance a content-length set to 0). If there
is no ES flag on the H2 HEADERS frame, it is an unexpected case. Because for
the applicative stream and most probably for the opposite endpoint, the
message is considered as finihsed. It is switched in its DONE state (or the
equivalent on the endpoint). But, if an extra H2 frame with the ES flag is
received, a TRAILERS frame or an emtpy DATA frame, an extra EOT HTX block is
pushed to carry the HTX EOM flag. So an extra HTX block is emitted for a
regular HTX message. It is totally invalid, it must never happen.

Because it is an undefined behavior, it is difficult to predict the result.
But it definitly prevent the applicative stream to properly handle aborts
and errors because data remain blocked in the channel buffer. Indeed, the
end of the message was seen, so no more data are forwarded.

It seems to be an issue for 2.8 and upper. Harder to evaluate for older
versions.

This patch must be backported as far as 2.4.

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

BUG/MEDIUM: http-ana: Report error on write error waiting for the response

When we are waiting for the server response, if an error is pending on the
frontend side (a write error on client), it is handled as an abort and all
regular response analyzers are removed, except the one responsible to
release the filters, if any. However, while it is handled as an abort, the
error is not reported, as usual, via http_reply_and_close() function. It is
an issue because in that, the channels buffers are not reset.

Because of this bug, it is possible to block a stream infinitely. The
request side is waiting for the response side and the response side is
blocked because filters must be released and this cannot be done because
data remain blocked in channels buffers.

So, in that case, calling http_reply_and_close() with no message is enough
to unblock the stream.

This patch must be backported as far as 2.8.

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

BUG/MEDIUM: quic: prevent conn freeze on 0RTT undeciphered content

Received QUIC packets are stored in quic_conn Rx buffer after header
protection removal in qc_rx_pkt_handle(). These packets are then removed
after quic_conn IO handler via qc_treat_rx_pkts().

If HP cannot be removed, packets are still copied into quic_conn Rx
buffer. This can happen if encryption level TLS keys are not yet
available. The packet remains in the buffer until HP can be removed and
its content processed.

An issue occurs if client emits a 0-RTT packet but haproxy does not have
the shared secret, for example after a haproxy process restart. In this
case, the packet is copied in quic_conn Rx buffer but its HP won't ever
be removed. This prevents the buffer to be purged. After some time, if
the client has emitted enough packets, Rx buffer won't have any space
left and received packets are dropped. This will cause the connection to
freeze.

To fix this, remove any 0-RTT buffered packets on handshake completion.
At this stage, 0-RTT packets are unnecessary anymore. The client is
expected to reemit its content in 1-RTT packet which are properly
deciphered.

This can easily reproduce with HTTP/3 POST requests or retrieving a big
enough object, which will fill the Rx buffer with ACK frames. Here is a
picoquic command to provoke the issue on haproxy startup :

$ picoquicdemo -Q -v 00000001 -a h3 <hostname> 20443 "/?s=1g"

Note that allow-0rtt must be present on the bind line to trigger the
issue. Else haproxy will reject any 0-RTT packets.

This must be backported up to 2.6.

This could be one of the reason for github issue #2549 but it's unsure
for now.

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

BUG/MEDIUM: ssl: 0-RTT initialized at the wrong place for AWS-LC

Revert patch fcc8255 "MINOR: ssl_sock: Early data disabled during
SSL_CTX switching (aws-lc)". The patch was done in the wrong callback
which is never built for AWS-LC, and applies options on the SSL_CTX
instead of the SSL, which should never be done elsewhere than in the
configuration parsing.

This was probably triggered by successfully linking haproxy against
AWS-LC without using USE_OPENSSL_AWSLC.

The patch also reintroduced SSL_CTX_set_early_data_enabled() in the
ssl_quic_initial_ctx() and ssl_sock_initial_ctx(). So the initial_ctx
does have the right setting, but it still needs to be applied to the
selected SSL_CTX in the clienthello, because we need it on the selected
SSL_CTX.

Must be backported to 3.0. (ssl_clienthello.c part was in ssl_sock.c)

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

BUG/MEDIUM: ssl: reactivate 0-RTT for AWS-LC

Then reactivate HAVE_SSL_0RTT and HAVE_SSL_0RTT_QUIC for AWS-LC, which
were wrongly deactivated in f5353f2c ("MINOR: ssl: add HAVE_SSL_0RTT
constant").

Must be backported to 3.0.

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

BUG/MINOR: stconn: bs.id and fs.id had their dependencies incorrect

The backend depends on the response and the frontend on the request, not
the other way around. In addition, they used to depend on L6 (hence
contents in the channel buffers) while they should only depend on L5
(permanent info known in the mux).

This came in 2.9 with commit 24059615a7 ("MINOR: Add sample fetches to
get the frontend and backend stream ID") so this can be backported there.

(cherry picked from commit 61dd0156c82ea051779e6524cad403871c31fc5a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 376b147ffffef0a1f898d72d1d70f10f07d2e5a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: mux-pt: Use the right name for the sedesc variable

A typo was introduced in 760d26a86 ("BUG/MEDIUM: mux-pt/mux-h1: Release the
pipe on connection error on sending path"). The sedesc variable is 'sd', not
'se'.

This patch must be backported with the commit above.

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

BUG/MEDIUM: mux-pt/mux-h1: Release the pipe on connection error on sending path

When data are sent using the kernel splicing, if a connection error
occurred, the pipe must be released. Indeed, in that case, no more data can
be sent and there is no reason to not release the pipe. But it is in fact an
issue for the stream because the channel will appear are not empty. This may
prevent the stream to be released. This happens on 2.8 when a filter is also
attached on it. On 2.9 and upper, it seems there is not issue. But it is
hard to be sure and the current patch remains valid is all cases. On 2.6 and
lower, the code is not the same and, AFAIK, there is no issue.

This patch must be backported to 2.8. However, on 2.8, there is no zero-copy
data forwarding. The patch must be adapted. There is no done_ff/resume_ff
callback functions for muxes. The pipe must released in sc_conn_send() when
an error flag is set on the SE, after the call to snd_pipe callback
function.

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

BUG/MEDIUM: stconn: Report error on SC on send if a previous SE error was set

When a send on a connection is performed, if a SE error (or a pending error)
was already reported earlier, we leave immediately. No send is performed.
However, we must be sure to report the error at the SC level if necessary.
Indeed, the SE error may have been reported during the zero-copy data
forwarding. So during receive on the opposite side. In that case, we may
have missed the opportunity to report it at the SC level.

The patch must be backported as far as 2.8.

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

BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once event miss and leak

An issue has been introduced with cd99440 ("BUG/MAJOR: server/addr: fix
a race during server addr:svc_port updates").

Indeed, in the above commit we implemented the atomic_sync task which is
responsible for consuming pending server events to apply the changes
atomically. For now only server's addr updates are concerned.

To prevent the task from causing contention, a budget was assigned to it.
It can be controlled with the global tunable
'tune.events.max-events-at-once': the task may not process more than this
number of events at once.

However, a bug was introduced with this budget logic: each time the task
has to be interrupted because it runs out of budget, we reschedule the
task to finish where it left off, but the current event which was already
removed from the queue wasn't processed yet. This means that this pending
event (each tune.events.max-events-at-once) is effectively lost.

When the atomic_sync task deals with large number of concurrent events,
this bug has 2 known consequences: first a server's addr/port update
will be lost every 'tune.events.max-events-at-once'. This can of course
cause reliability issues because if the event is not republished
periodically, the server could stay in a stale state for indefinite amount
of time. This is the case when the DNS server flaps for instance: some
servers may not come back UP after the incident as described in GH #2666.

Another issue is that the lost event was not cleaned up, resulting in a
small memory leak. So in the end, it means that the bug is likely to
cause more and more degradation over time until haproxy is restarted.

As a workaround, 'tune.events.max-events-at-once' may be set to the
maximum number of events expected per batch. Note however that this value
cannot exceed 10 000, otherwise it could cause the watchdog to trigger due
to the task being busy for too long and preventing other threads from
making any progress. Setting higher values may not be optimal for common
workloads so it should only be used to mitigate the bug while waiting for
this fix.

Since tune.events.max-events-at-once defaults to 100, this bug only
affects configs that involve more than 100 servers whose addr:port
properties are likely to be updated at the same time (batched updates
from cli, lua, dns..)

To fix the bug, we move the budget check after the current event is fully
handled. For that we went from a basic 'while' to 'do..while' loop as we
assume from the config that 'tune.events.max-events-at-once' cannot be 0.
While at it, we reschedule the task once thread isolation ends (it was not
required to perform the reschedule while under isolation) to give the hand
back faster to waiting threads.

This patch should be backported up to 2.9 with cd99440. It should fix
GH #2666.

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

[RELEASE] Released version 3.0.4

Released version 3.0.4 with the following main changes :
    - MINOR: proto: extend connection thread rebind API
    - BUILD: listener: silence a build warning about unused value without threads
    - BUG/MEDIUM: quic: prevent crash on accept queue full
    - CLEANUP: proto: rename TID affinity callbacks
    - CLEANUP: quic: rename TID affinity elements
    - BUG/MINOR: session: Eval L4/L5 rules defined in the default section
    - BUG/MEDIUM: debug/cli: fix "show threads" crashing with low thread counts
    - DOC: install: don't reference removed CPU arg
    - BUG/MEDIUM: ssl_sock: fix deadlock in ssl_sock_load_ocsp() on error path
    - BUG/MAJOR: mux-h2: force a hard error upon short read with pending error
    - DOC: configuration: issuers-chain-path not compatible with OCSP
    - DOC: config: improve the http-keep-alive section
    - BUG/MINOR: stick-table: fix crash for src_inc_gpc() without stkcounter
    - BUG/MINOR: server: Don't warn fallback IP is used during init-addr resolution
    - BUG/MINOR: cli: Atomically inc the global request counter between CLI commands
    - BUG/MINOR: quic: Non optimal first datagram.
    - MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface
    - BUG/MINOR: quic: Lack of precision when computing K (cubic only cc)
    - BUG/MEDIUM: jwt: Clear SSL error queue on error when checking the signature
    - MINOR: quic: Dump TX in flight bytes vs window values ratio.
    - MINOR: quic: Add information to "show quic" for CUBIC cc.
    - MEDIUM: h1: allow to preserve keep-alive on T-E + C-L
    - MINOR: queue: add a function to check for TOCTOU after queueing
    - BUG/MEDIUM: queue: deal with a rare TOCTOU in assign_server_and_queue()
    - MEDIUM: init: set default for fd_hard_limit via DEFAULT_MAXFD (take #2)
    - BUG/MEDIUM: init: fix fd_hard_limit default in compute_ideal_maxconn
    - Revert "MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface"
    - MEDIUM: log: relax some checks and emit diag warnings instead in lf_expr_postcheck()
    - DOC: quic: fix default minimal value for max window size
    - MINOR: proxy: Add support of 429-Too-Many-Requests in retry-on status
    - BUG/MEDIUM: mux-h2: Set ES flag when necessary on 0-copy data forwarding
    - BUG/MEDIUM: stream: Prevent mux upgrades if client connection is no longer ready
    - BUG/MINIR: proxy: Match on 429 status when trying to perform a L7 retry
    - BUG/MEDIUM: mux-pt: Never fully close the connection on shutdown
    - BUG/MEDIUM: cli: Always release back endpoint between two commands on the mcli
    - BUG/MINOR: quic: unexploited retransmission cases for Initial pktns.
    - BUG/MEDIUM: mux-h1: Properly handle empty message when an error is triggered
    - MINOR: mux-h2: try to clear DEM_MROOM and MUX_MFULL at more places
    - BUG/MAJOR: mux-h2: always clear MUX_MFULL and DEM_MROOM when clearing the mbuf
    - BUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)
    - BUG/MINOR: Crash on O-RTT RX packet after dropping Initial pktns
    - BUG/MEDIUM: mux-pt: Fix condition to perform a shutdown for writes in mux_pt_shut()

BUG/MEDIUM: mux-pt: Fix condition to perform a shutdown for writes in mux_pt_shut()

A regression was introduced in the commit 76fa71f7a ("BUG/MEDIUM: mux-pt:
Never fully close the connection on shutdown") because of a typo on the
connection flags. CO_FL_SOCK_WR_SH flag must be tested to prevent a call to
conn_sock_shutw() and not CO_FL_SOCK_RD_SH.

Concretly, most of time, it is harmeless because shutdown for writes is
always performed before any shutdown for reads. Except in case describe by
the commit above. But it is not clear if it has an impact or not.

This patch must be backported with the commit above, so as far as 2.9.

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

BUG/MINOR: Crash on O-RTT RX packet after dropping Initial pktns

This bug arrived with this naive commit:

    BUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)

which omitted to consider the case where the Initial packet number space
could be discarded before receiving 0-RTT packets.

To fix this, append/insert the O-RTT (early-data) packet number space
into the encryption level list depending on the presence or not of
the Initial packet number space.

This issue was revealed when using aws-lc as TLS stack in GH #2701 issue.
Thank you to @Tristan971 for having reported this issue.

Must be backported where the commit mentionned above is supposed to be
backported: as far as 2.9.

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

BUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)

By "aws-lc only", one means that this bug was first revealed by aws-lc stack.
This does not mean it will not appeared for new versions of other TLS stacks which
have never revealed this bug.

This bug was reported by Ilya (@chipitsine) in GH #2657 where some QUIC interop
tests (resumption, zerortt) could lead to crash with haproxy compiled against
aws-lc TLS stack. These crashed were triggered by this BUG_ON() which detects
that too short datagrams with at least one ack-eliciting Initial packet inside
could be built.

  <0>2024-07-31T15:13:42.562717+02:00 [01|quic|5|quic_tx.c:739] qc_prep_pkts():
  next encryption level : qc@0x61d000041080 idle_timer_task@0x60d000006b80 flags=0x6000058

  FATAL: bug condition "first_pkt->type == QUIC_PACKET_TYPE_INITIAL && (first_pkt->flags & (1UL << 0)) && length < 1200" matched at src/quic_tx.c:163
  call trace(12):
  | 0x563ea447bc02 [ba d9 00 00 00 48 8d 35]: main-0x1958ce
  | 0x563ea4482703 [e9 73 fe ff ff ba 03 00]: qc_send+0x17e4/0x1b5d
  | 0x563ea4488ab4 [85 c0 0f 85 00 f6 ff ff]: quic_conn_io_cb+0xab1/0xf1c
  | 0x563ea468e6f9 [48 c7 c0 f8 55 ff ff 64]: run_tasks_from_lists+0x173/0x9c2
  | 0x563ea468f24a [8b 7d a0 29 c7 85 ff 0f]: process_runnable_tasks+0x302/0x6e6
  | 0x563ea4610893 [83 3d aa 65 44 00 01 0f]: run_poll_loop+0x6e/0x57b
  | 0x563ea4611043 [48 8b 1d 46 c7 1d 00 48]: main-0x48d
  | 0x7f64d05fb609 [64 48 89 04 25 30 06 00]: libpthread:+0x8609
  | 0x7f64d0520353 [48 89 c7 b8 3c 00 00 00]: libc:clone+0x43/0x5e

That said everything was correctly done by qc_prep_ptks() to prevent such a case.
But this relied on the hypothesis that the list of encryption levels it used
was always built in the same order as follows for 0-RTT sessions:

    initial, early-data, handshake, application

But this order is determined but the order the TLS stack derives the secrets
for these encryption levels. For aws-lc, this order is not the same but
as follows:

    initial, handshake, application, early-data

During 0-RTT sessions, the server may have to build three ack-eliciting packets
(with CRYPTO data inside) to reply to the first client packet: initial, hanshake,
application. qc_prep_pkts() adds a PADDING frame to the last built packet
for the last encryption level in the list. But after application level encryption,
there is early-data encryption level. This prevented qc_prep_pkts() to build
a padded applicaiton level last packet to send a 1200-bytes datagram.

To fix this, always insert early-data encryption level after the initial
encryption level into the encryption levels list when initializing this encryption
level from quic_conn_enc_level_init().

Must be backported as far as 2.9.

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

BUG/MAJOR: mux-h2: always clear MUX_MFULL and DEM_MROOM when clearing the mbuf

There exists an extremely tricky code path that was revealed in 3.0 by
the glitches feature, though it might theoretically have existed before.

TL;DR: a mux mbuf may be full after successfully sending GOAWAY, and
discard its remaining contents without clearing H2_CF_MUX_MFULL and
H2_CF_DEM_MROOM, then endlessly loop in h2_send(), until the watchdog
takes care of it.

What can happen is the following: Some data are received, h2_io_cb() is
called. h2_recv() is called to receive the incoming data. Then
h2_process() is called and in turn calls h2_process_demux() to process
input data. At some point, a glitch limit is reached and h2c_error() is
called to close the connection. The input frame was incomplete, so some
data are left in the demux buffer. Then h2_send() is called, which in
turn calls h2_process_mux(), which manages to queue the GOAWAY frame,
turning the state to H2_CS_ERROR2. The frame is sent, and h2_process()
calls h2_send() a last time (doing nothing) and leaves. The streams
are all woken up to notify about the error.

Multiple backend streams were waiting to be scheduled and are woken up
in turn, before their parents being notified, and communicate with the
h2 mux in zero-copy-forward mode, request a buffer via h2_nego_ff(),
fill it, and commit it with h2_done_ff(). At some point the mux's output
buffer is full, and gets flags H2_CF_MUX_MFULL.

The io_cb is called again to process more incoming data. h2_send() isn't
called (polled) or does nothing (e.g. TCP socket buffers full). h2_recv()
may or may not do anything (doesn't matter). h2_process() is called since
some data remain in the demux buf. It goes till the end, where it finds
st0 == H2_CS_ERROR2 and clears the mbuf. We're now in a situation where
the mbuf is empty and MFULL is still present.

Then it calls h2_send(), which doesn't call h2_process_mux() due to
MFULL, doesn't enter the for() loop since all buffers are empty, then
keeps sent=0, which doesn't allow to clear the MFULL flag, and since
"done" was not reset, it loops forever there.

Note that the glitches make the issue more reproducible but theoretically
it could happen with any other GOAWAY (e.g. PROTOCOL_ERROR). What makes
it not happen with the data produced on the parsing side is that we
process a single buffer of input at once, and there's no way to amplify
this to 30 buffers of responses (RST_STREAM, GOAWAY, SETTINGS ACK,
WINDOW_UPDATE, PING ACK etc are all quite small), and since the mbuf is
cleared upon every exit from h2_process() once the error was sent, it is
not possible to accumulate response data across multiple calls. And the
regular h2_snd_buf() path checks for st0 >= H2_CS_ERROR so it will not
produce any data there either.

Probably that h2_nego_ff() should check for H2_CS_ERROR before accepting
to deliver a buffer, but this needs to be carefully studied. In the mean
time the real problem is that the MFULL flag was kept when clearing the
buffer, making the two inconsistent.

Since it doesn't seem possible to trigger this sequence without the
zero-copy-forward mechanism, this fix needs to be backported as far as
2.9, along with previous commit "MINOR: mux-h2: try to clear DEM_MROOM
and MUX_MFULL at more places" which will strengthen the consistency
between these checks.

Many thanks to Annika Wickert for her detailed report that allowed to
diagnose this problem. CVE-2024-45506 was assigned to this problem.

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

MINOR: mux-h2: try to clear DEM_MROOM and MUX_MFULL at more places

The code leading to H2_CF_MUX_MFULL and H2_CF_DEM_MROOM being cleared
is quite complex and assumptions about its state are extremely difficult
when reading the code. There are indeed long sequences where the mux might
possibly be empty, still having the flag set until it reaches h2_send()
which will clear it after the last send. Even then it's not obviour whether
it's always guaranteed to release the flag when invoked in multiple passes.
Let's just simplify the conditionnn so that h2_send() does not depend on
"sent" anymore and that h2_timeout_task() doesn't leave the flags set on
the buffer on emptiness. While it doesn't seem to fix anything, it will
make the code more robust against future changes.

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

BUG/MEDIUM: mux-h1: Properly handle empty message when an error is triggered

When a 400/408/500/501 error is returned by the H1 multiplexer, we first try
to get the error message of the proxy before using the default one. This may
be configured to be mapped on /dev/null or on an empty file. In that case,
no message is emitted, as expected. But everything is handled as the error
was successfully sent.

However, there is an bug here. In h1_send_error() function, this case is not
properly handled. The flag H1C_F_ABRTED is not set on the H1 connection as it
should be and h1_close() function is not called, leaving the H1 connection in an
undefined state.

It is especially an issue when a "empty" 408-Request-Time-out error is emitted
while there are data blocked in the output buffer. In that case, the connection
remains openned until the client closes and a "cR--"/408 is logged repeatedly, every
time the client timeout is reached.

This patch must backported as far as 2.8.

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

BUG/MINOR: quic: unexploited retransmission cases for Initial pktns.

qc_prep_hdshk_fast_retrans() job is to pick some packets to be retransmitted
from Initial and Handshake packet number spaces. A packet may be coalesced to
a first one into the same datagram. When a coalesced packet is inspected for
retransmission, it is skipped if its length would make the total datagram length
it is attached to exceeding the anti-amplification limit. But in this case, the
first packet must be kept for the current retransmission. This is tracked by
this trace statemement:
    TRACE_PROTO("will probe Initial packet number space", QUIC_EV_CONN_SPPKTS, qc);
This was not the case because of the wrong "goto end" statement. This latter
must be run only if the Initial packet number space must not be probe with
the first packet found as coalesced to another one which must be skipped.

This bug was revealed by AWS-LC interop runner with handshakeloss and
handshakecorruption which always fail because this stack leads the server
to send more Initial packets.

Thank you to Ilya (@chipitsine) for this issue report in GH #2663.

Must be backported as far as 2.6.

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

BUG/MEDIUM: cli: Always release back endpoint between two commands on the mcli

When several commands are chained on the master CLI, the same client
connection is used. Because, it is a TCP connection, the mux PT is used. It
means there is no stream at the mux level. It is not possible to release the
applicative stream between each commands as for the HTTP. So, to work around
this limitation, between two commands, the master CLI is resetting the
stream. It does exactly what it was performed on HTTP to manage keep-alive
connections on old HAProxy versions.

But this part was copied from a code dealing with connection only while the
back endpoint can be an applet or a mux for the master cli. The previous fix
on the mux PT ("BUG/MEDIUM: mux-pt: Never fully close the connection on
shutdown") revealed a bug. Between two commands, the back endpoint was only
released if the connection's XPRT was closed. This works if the back
endpoint is an applet because there is no connection. But for commands sent
to a worker, a connection is used. At this stage, this only works if the
connection's XPRT is closed. Otherwise, the old endpoint is never detached
leading to undefined behavior on the next command execution (most probably a
crash).

Without the commit above, the connection's XPRT is always closed on
shutdown. It is no longer true. At this stage, we must inconditionnally
release the back endpoint by resetting the corresponding sedesc to fix the
bug.

This patch must be backported with the commit above in all stable
versions. On 2.4 and lower, it will need to be adapted.

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

BUG/MEDIUM: mux-pt: Never fully close the connection on shutdown

When a shutdown is reported to the mux (shutdown for reads or writes), the
connexion is immediately fully closed if the mux detects the connexion is
closed in both directions. Only the passthrough multiplexer is able to
perform this action at this stage because there is no stream and no internal
data. Other muxes perform a full connection close during the mux's release
stage. It was working quite well since recently. But, in theory, the bug is
quite old.

In fact, it seems possible for the lower layer to report an error on the
connection in same time a shutdown is performed on the mux. Depending on how
events are scheduled, the following may happen:

 1. An connection error is detected at the fd layer and a wakeup is
    scheduled on the mux to handle the event.

 2. A shutdown for writes is performed on the mux. Here the mux decides to
    fully close the connexion. If the xprt is not used to log info, it is
    released.

 3. The mux is finally woken up. It tries to retrieve data from the xprt
    because it is not awayre there was an error. This leads to a crash
    because of a NULL-deref.

By reading the code, it is not obvious. But it seems possible with SSL
connection when the handshake is rearmed. It happens when a
SSL_ERROR_WANT_WRITE is reported on a SSL_read() attempt or a
SSL_ERROR_WANT_READ on a SSL_write() attempt.

This bug is only visible if the XPRT is not used to log info. So it is no so
common.

This patch should fix the 2nd crash reported in the issue #2656. It must
first be backported as far as 2.9 and then slowly to all stable versions.

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

BUG/MINIR: proxy: Match on 429 status when trying to perform a L7 retry

Support for 429 was recently added to L7 retries (0d142e075 "MINOR: proxy:
Add support of 429-Too-Many-Requests in retry-on status"). But the
l7_status_match() function was not properly updated. The switch statement
must match the 429 status to be able to perform a L7 retry.

This patch must be backported if the commit above is backported. It is
related to #2687.

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

BUG/MEDIUM: stream: Prevent mux upgrades if client connection is no longer ready

If an early error occurred on the client connection, we must prevent any
multiplexer upgrades. Indeed, it is unexpected for a mux to be initialized
with no xprt. On a normal workflow it is impossible. So it is not an
issue. But if a mux upgrade is performed at the stream level, an early error
on the connection may have already been handled by the previous mux and the
connection may be already fully closed. If the mux upgrade is still
performed, a crash can be experienced.

It is possible to have a crash with an implicit TCP>HTTP upgrade if there is no
data in the input buffer. But it is also possible to get a crash with an
explicit "switch-mode http" rule.

It must be backported to all stable versions. In 2.2, the patch must be
applied directly in stream_set_backend() function.

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

BUG/MEDIUM: mux-h2: Set ES flag when necessary on 0-copy data forwarding

When DATA frames are sent via the 0-copy data forwarding, we must take care
to set the ES flag on the last DATA frame. It should be performed in
h2_done_ff() when IOBUF_FL_EOI flag was set by the producer. This flag is
here to know when the producer has reached the end of input. When this
happens, the h2s state is also updated. It is switched to "half-closed
local" or "closed" state depending on its previous state.

It is mainly an issue on uploads because the server may be blocked waiting
for the end of the request. A workaround is to disable the 0-copy forwarding
support the the H2 by setting "tune.h2.zero-copy-fwd-send" directive to off
in your global section.

This patch should fix the issue #2665. It must be backported as far as 2.9.

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

MINOR: proxy: Add support of 429-Too-Many-Requests in retry-on status

The "429" status can now be specified on retry-on directives. PR_RE_* flags
were updated to remains sorted.

This patch should fix the issue #2687. It is quite simple so it may safely
be backported to 3.0 if necessary.

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

DOC: quic: fix default minimal value for max window size

It is possible to override the default QUIC congestion algorithm on a
bind line. With the same setting, it is also possible to specify the
maximum congestion window size.

The parser rejects values outside of the range between 10k and 4g. This
is in contradiction with the documentation which specify 1k as the lower
value. Correct this value in the documentation.

This should be backported up to 2.9.

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

MEDIUM: log: relax some checks and emit diag warnings instead in lf_expr_postcheck()

With 7a21c3a ("MAJOR: log: implement proper postparsing for logformat
expressions") which finally made postparsing checks reliable, we started
to get report from users that couldn't start haproxy 3.0 with configs that
used to work in the past. The current situation is described in GH #2642.

While the checks are mostly relevant, it turns out there are not strictly
needed anymore from a technical point of view. Most of them were useful in
early logformat implementation to prevent runtime bugs due to the use of
an alias or fetch at runtime from an incompatible proxy. It's been a few
versions already that the code handling fetches and log aliases is robust
enough to support fetches/aliases used from the wrong context: all it
does is that the fetch/alias will silently fail if it's not available.

This can be proved by the fact that even if the postparsing checks were
partially broken in the past, it didn't cause runtime issues (at least
on recent haproxy versions).

Most of these checks can now be seen as configuration hints: when a check
triggers, it will indicate a configuration inconsistency in most cases,
but they are some corner cases where it is not possible to know at config
time if the conditions will be met for the alias/fetch to work properly..
so instead of failing with a hard error like we did so far, let's just be
more permissive and report our findings using "diag_warning": such
warnings are only emitted when haproxy is started with '-dD' cli option.

We also took this opportunity to improve messages clarity and make them
more precise (report the offending item instead of complaining about the
whole expression because of a single element).

With this patch, configs that used to start before 7a21c3a shouldn't
trigger hard errors anymore.

This may be backported in 3.0.

(cherry picked from commit 41ca89bc6fe96a660fea992643dbc2c0844a609e)
[ada: ctx adjt]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

Revert "MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface"

This reverts commit 514a3110f0cb33a04fa5bd786927ad98aebfe72b.

This one was backported by mistake, it wasn't meant to. It should not
harm anyway but better not backport stuff that doesn't need to.

Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: init: fix fd_hard_limit default in compute_ideal_maxconn

This commit fixes 41275a691 ("MEDIUM: init: set default for fd_hard_limit via
DEFAULT_MAXFD").

fd_hard_limit is taken in account implicitly via 'ideal_maxconn' value in
all maxconn adjustements, when global.rlimit_memmax is set:

MIN(global.maxconn, capped by global.rlimit_memmax, ideal_maxconn);

It also caps provided global.rlimit_nofile, if it couldn't be set as a current
process fd limit (see more details in the main() code).

So, lets set the default value for fd_hard_limit only, when there is no any
other haproxy-specific limit provided, i.e. rlimit_memmax, maxconn,
rlimit_nofile. Otherwise we may break users configs.

Please, note, that in master-worker mode, master does not need the
DEFAULT_MAXFD (1048576) as well, as we explicitly limit its maxconn to 100.

Must be backported in all stable versions until v2.6.0, including v2.6.0,
like the commit above.

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

MEDIUM: init: set default for fd_hard_limit via DEFAULT_MAXFD (take #2)

Let's provide a default value for fd_hard_limit, if it's not set in the
configuration. With this patch we could set some specific default via
compile-time variable DEFAULT_MAXFD as well. Hope, this will be helpfull for
haproxy package maintainers.

    make -j 8 TARGET=linux-glibc DEBUG=-DDEFAULT_MAXFD=50000

If haproxy is comipled without DEFAULT_MAXFD defined, the default will be set
to 1048576.

This is done to avoid killing the process by its watchdog, while it started
without any limitations in its configuration or in the command line and the
hard RLIMIT_NOFILE is extremely huge (~1000000000). We use in this case
compute_ideal_maxconn() to calculate maxconn and maxsock, maxsock defines the
size of internal fdtab, which becames very-very large as well. When
the process starts to simply loop over this fdtab (0(n)), this takes a lot of
time, so watchdog does it job.

To avoid this, maxconn now is always reduced to some reasonable value either
by explicit global.fd-hard-limit from configuration, or by its default. The
default may be changed at build-time and overwritten then by
global.fd-hard-limit at runtime. Explicit global.fd-hard-limit from the
configuration has always precedence over DEFAULT_MAXFD, if set.

Must be backported in all stable versions until v2.6.0, including v2.6.0.

(cherry picked from commit 41275a691839df5f8dc7cb9faa4e259fbb755d34)
[wt: the discussion around this patch came to an agreement on the list:
     https://www.mail-archive.com/haproxy@formilux.org/msg45098.html ]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: queue: deal with a rare TOCTOU in assign_server_and_queue()

After checking that a server or backend is full, it remains possible
to call pendconn_add() just after the last pending requests finishes, so
that there's no more connection on the server for very low maxconn (typ 1),
leaving new ones in queue till the timeout.

The approach depends on where the request was queued, though:
  - when queued on a server, we can simply detect that we may dequeue
    pending requests and wake them up, it will wake our request and
    that's fine. This needs to be done in srv_redispatch_connect() when
    the server is set.

  - when queued on a backend, it means that all servers are done with
    their requests. It means that all servers were full before the
    check and all were empty after. In practice this will only concern
    configs with less servers than threads. It's where the issue was
    first spotted, and it's very hard to reproduce with more than one
    server. In this case we need to load-balance again in order to find
    a spare server (or even to fail). For this, we call the newly added
    dedicated function pendconn_must_try_again() that tells whether or
    not a blocked pending request was dequeued and needs to be retried.

This should be backported along with pendconn_must_try_again() to all
stable versions, but with extreme care because over time the queue's
locking evolved.

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

MINOR: queue: add a function to check for TOCTOU after queueing

There's a rare TOCTOU case that happens from time to time with maxconn 1
and multiple threads. Between the moment we see the queue full and the
moment we queue a request, it's possible that the last request on the
server or proxy ended and that no other one is left to offer it its place.

Given that all this code path is performance-critical and we cannot afford
to increase the lock duration, better recheck for the condition after
queueing. For this we need to be able to check for the condition and
cleanly dequeue a request. That's what this patch provides via the new
function pendconn_must_try_again(). It will catch more requests than
absolutely needed though it will catch them all. It may find that around
1/1000 of requests are at risk, though testing shows that in practice,
it's around 1 per million that really gets stuck (other ones benefit
from timing and finishing late requests). Maybe in the future some
conditions might be refined but it's harmless.

What happens to such requests is that they're dequeued and their pendconn
freed, so that the caller can decide to try to LB or queue them again. For
now the function is not used, it's just added separately for easier tracking.

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

MEDIUM: h1: allow to preserve keep-alive on T-E + C-L

In 2.5-dev9, commit 631c7e866 ("MEDIUM: h1: Force close mode for invalid
uses of T-E header") enforced a recently arrived new security rule in the
HTTP specification aiming at preventing a class of content-smuggling
attacks involving HTTP/1.0 agents. It consists in handling the very rare
T-E + C-L requests or responses in close mode.

It happens it does have an impact of a rare few and very old clients
(probably running insecure TLS stacks by the way) that continue to send
both with their POST requests. The impact is that for each and every
request they'll have to reconnect, possibly negotiating a full TLS
handshake that becomes harmful to the machine in terms of CPU computation.

This commit adds a new option "h1-do-not-close-on-insecure-transfer-encoding"
that does exactly what it says, it just asks not to close on such messages,
even though the message continues to be sanitized and C-L dropped. It means
that the risk is only between the sender and haproxy, which is limited, and
might be the only acceptable solution for such environments having to deal
with broken implementations.

The cases are so rare that it should not need to be backported, or in the
worst case, to the latest LTS if there is any demand.

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

MINOR: quic: Add information to "show quic" for CUBIC cc.

Add ->state_cli() new callback to quic_cc_algo struct to define a
function called by the "show quic (cc|full)" commands to dump some information
about the congestion algorithm internal state currently in use by the QUIC
connections.

Implement this callback for CUBIC algorithm to dump its internal variables:
   - K: (the time to reach the cubic curve inflexion point),
   - last_w_max: the last maximum window value reached before intering
     the last recovery period. This is also the window value at the
     inflexion point of the cubic curve,
   - wdiff: the difference between the current window value and last_w_max.
     So negative before the inflexion point, and positive after.

(cherry picked from commit 76ff8afa2d9eb0206bc72f4e2f8ad230720dfb94)
[wt: adjusted ctx in quic_cli since no GSO]
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: quic: Dump TX in flight bytes vs window values ratio.

Display the ratio of the numbers of bytes in flight by packet number spaces
versus the current window values in percent.

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

BUG/MEDIUM: jwt: Clear SSL error queue on error when checking the signature

When the signature included in a JWT is verified, if an error occurred, one
or more SSL errors are queued and never cleared. These errors may be then
caught by the SSL stack and a fatal SSL error may be erroneously reported
during a SSL received or send.

So we must take care to clear the SSL error queue when the signature
verification failed.

This patch should fix issue #2643. It must be backported as far as 2.6.

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

BUG/MINOR: quic: Lack of precision when computing K (cubic only cc)

K cubic variable is stored in ms. But it was a formula with the second as unit
for the window difference parameter which was used to compute K without
considering the loss of information. Then the result was converted in ms (K *= 1000).
This leaded to a lack of precision and multiples of 1000 as values.

To fix this, use the same formula but with the window difference in ms as parameter
passed to the cubic function and remove the conversion.

Must be backported as far as 2.6.

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

MEDIUM: sink: don't set NOLINGER flag on the outgoing stream interface

Given that sink applets are responsible for conveying messages from the
ring to the tcp server endpoint, there are no protocol timeout or errors
expected there, it is an unidirectional flow of data over TCP.

As such, NOLINGER flag which was inherited from peers applet, see
dbd026792 ("BUG/MEDIUM: peers: set NOLINGER on the outgoing stream
interface") is not desirable under sink context:

The reason why we have the NOLINGER flag set is to ensure the connection
is closed right away and avoid 60s TIME_WAIT delay on closed sockets.
The downside is that messages sent right before closing the socket are
not guaranteed to make it to the server because closing with NOLINGER
flag set will result in RST packet being emitted right away, which could
prevent in-flight messages from being properly delivered.

Unlike peers applets, the only cases were sink applets are expected to
close the connection are upon unexpected error or upon stopping, which are
relatively rare events. Thanks to previous commit, ERROR flag is already
set in case of error, so the use of NOLINGER is not mandatory for the
RST to be sent. Now for the stopping case, it only happens once in the
process lifetime so it's acceptable to close the socket using EOS+EOI
flags without the NOLINGER option set.

So in our case, it is preferable to ensure messages get properly delivered
knowning that closed sockets should be piling up in TIME_WAIT, this means
removing the NOLINGER flag on the outgoing stream interface for sink
applets. It is a prerequisite for upcoming patches in order to cleanly
shut the applet during runtime without risking to send the RST packet
before all pending messages were sent to the endpoint.

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

BUG/MINOR: quic: Non optimal first datagram.

This bug arrived with this commit:

     b068e758f MINOR: quic: simplify rescheduling for handshake

This commit introduced a bad side effect. Haproxy always replied by an ACK-only
datagram when it received the first client Initial packet. Then it handled
the CRYPTO data insided. And finally, it sent its own CRYPTO data. This broke
the packet coalescing rule whose aim is to optimally build and send as more
as QUIC packets by datagram.

To fix this, simply partially reverts this commit, to make the low level I/O
task return again if some CRYPTO were received. This will delay the
acknowledgement which will be sent with the CRYPTO data from the same
datagram again.

Must be backported to 3.0.

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

BUG/MINOR: cli: Atomically inc the global request counter between CLI commands

The global request counter is used to set the stream id (s->uniq_id). It is
incremented at different places. And it must be atomically incremented
because it is a global value. However, in the analyer dealing with CLI
command response, this was not the case. It is now fixed.

This patch must be backported to all stable versions.

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

BUG/MINOR: server: Don't warn fallback IP is used during init-addr resolution

When a fallback IP address is provided in the list of methods to use to
resolve the server address, a warning is emitted if previous methods
failed. The aim is to inform this address will be used for the
server. However, it is valid use-case. It is the expected behavior. There is
no reason to emit a warning. Having a message during HAProxy startup to
inform the fallback IP address will be used is probably a good idea. But it
should be a notice not a warning. Otherwise, checking the configuration
validity will always failed, just like starting HAProxy in zero-warning
mode while the option was set on purpose.

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

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

BUG/MINOR: stick-table: fix crash for src_inc_gpc() without stkcounter

Since 2.5, an array of GPC is provided to replace legacy gpc0/gpc1.
src_inc_gpc is a sample fetch which is used to increment counters in
this array.

A crash occurs if src_inc_gpc is used without any previous track-sc
rule. This is caused by an error in smp_fetch_sc_inc_gpc(). When
temporary stick counter is created via smp_create_src_stkctr(), table
pointer arg value used is not correct : it points to the counter ID
instead of the table argument. To fix this, use the proper sample fetch
second arg.

This can be reproduced with the following config :
  acl mark src_inc_gpc(0,<table>) -m bool
  tcp-request connection accept if mark

This should be backported up to 2.6.

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

DOC: config: improve the http-keep-alive section

Nathan Wehrman suggested this add-on to try to better explain the
interactions between http-keep-alive and other timeouts, and the
impacts on protocols (HTTP/1, HTTP/2 etc).

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

DOC: configuration: issuers-chain-path not compatible with OCSP

State that issuers-chain-path is not compatible with OCSP features.

Must be backported in every stable version.

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

BUG/MAJOR: mux-h2: force a hard error upon short read with pending error

A risk of truncated packet was addressed in 2.9 by commit 19fb19976f
("BUG/MEDIUM: mux-h2: Only Report H2C error on read error if demux
buffer is empty") by ignoring CO_FL_ERROR after a recv() call as long
as some data remained present in the buffer. However it has a side
effect due to the fact that some frame processors only deal with full
frames, for example, HEADERS. The side effect is that an incomplete
frame will not be processed and will remain in the buffer, preventing
the error from being taken into account, so the I/O handler wakes up
the H2 parser to handle the error, and that one just subscribes for
more data, and this loops forever wasting CPU cycles.

Note that this only happens with errors at the SSL layer exclusively,
otherwise we'd have a read0 pending that would properly be detected:

  conn->flags = CO_FL_XPRT_TRACKED | CO_FL_ERROR | CO_FL_XPRT_READY | CO_FL_CTRL_READY
  conn->err_code = CO_ERR_SSL_FATAL
  h2c->flags  = H2_CF_ERR_PENDING | H2_CF_WINDOW_OPENED | H2_CF_MBUF_HAS_DATA | H2_CF_DEM_IN_PROGRESS | H2_CF_DEM_SHORT_READ

The condition to report the error in h2_recv() needs to be refined, so
that connection errors are taken into account either when the buffer is
empty, or when there's an incomplete frame, since we're certain it will
never be completed. We're certain to enter that function because
H2_CF_DEM_SHORT_READ implies too short a frame, and earlier there's a
protocol check to validate that no frame size is larger than bufsize,
hence a H2_CF_DEM_SHORT_READ implies there's some room left in the
buffer and we're allowed to try to receive.

The condition to reproduce the bug seems super hard to meet but was
observed once by Patrick Hemmer who had the reflex to capture lots of
information that allowed to explain the problem. In order to reproduce
it, the SSL code had to be significantly modified to alter received
contents at very empiric places, but that was sufficient to reproduce
it and confirm that the current patch works as expected.

The bug was tagged MAJOR because when it triggers there's no other
solution to get rid of it but to restart the process. However given how
hard it is to trigger on a lab, it does not seem very likely to occur
in field.

This needs to be backported to 2.9.

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

BUG/MEDIUM: ssl_sock: fix deadlock in ssl_sock_load_ocsp() on error path

We could run under heavy load in containers or on premises and some automatic
tool in parallel could use CLI to check OCSP updates statuses or to upload new
OCSP responses. So, calloc() to store OCSP update callback arguments may fail
and ocsp_tree_lock need to be unlocked, when exiting due to this failure.

This needs to be backported in all stable versions until v2.4.0 included.

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

DOC: install: don't reference removed CPU arg

Remove reference to the removed CPU= build argument in commit 018443b8a1
("BUILD: makefile: get rid of the CPU variable").

This should be backported to 3.0.

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

BUG/MEDIUM: debug/cli: fix "show threads" crashing with low thread counts

The "show threads" command introduced early in the 2.0 dev cycle uses
appctx->st1 to store its context (the number of the next thread to dump).
It goes back to an era where contexts were shared between the various
applets and the CLI's command handlers.

In fact it was already not good by then because st1 could possibly have
APPCTX_CLI_ST1_PAYLOAD (2) in it, that would make the dmup start at
thread 2, though it was extremely unlikely.

When contexts were finally cleaned up and moved to their own storage,
this one was overlooked, maybe due to using st1 instead of st2 like
most others. So it continues to rely on st1, and more recently some
new flags were appended, one of which is APPCTX_CLI_ST1_LASTCMD (16)
and is always there. This results in "show threads" to believe it must
start do dump from thread 16, and if this thread is not present, it can
simply crash the process. A tiny reproducer is:

  global
    nbthread 1
    stats socket /tmp/sock1 level admin mode 666

  $ socat /tmp/sock1 - <<< "show threads"

The fix for modern versions simply consists in assigning a context to
this command from the applet storage. We're using a single int, no need
for a struct, an int* will do it. That's valid till 2.6.

Prior to 2.6, better switch to appctx->ctx.cli.i0 or i1 which are all
properly initialized before the command is executed.

This must be backported to all stable versions.

Thanks to Andjelko Horvat for the report and the reproducer.

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

BUG/MINOR: session: Eval L4/L5 rules defined in the default section

It is possible to define TCP/HTTP rules in a named default section to
inherit from it in a proxy. However, there is an issue with L4/L5 rules.
Only the lists of the current frontend are checked to know if an eval must
be performed. Nothing is done for an empty list. Of course, the lists of the
default proxy must also be checked to be sure to not ignored default L4/L5
rules. It is now fixed.

This patch should fix the issue #2637. It must be backported as far as 2.6.

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

CLEANUP: quic: rename TID affinity elements

This commit is the renaming counterpart of the previous one, this time
for quic_conn module. Several elements related to TID affinity update
from quic_conn has been renamed : public functions, but also flag
renamed to QUIC_FL_CONN_TID_REBIND and trace event to
QUIC_EV_CONN_BIND_TID.

This should be backported with the same instruction as the previous
commit.

(cherry picked from commit 3be58fc720c406ce4f4dfc70b87662cef4838886)
[wt: dropped the BUG_ON() from quic_conn since bfdf145859d not backported]
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: proto: rename TID affinity callbacks

Since the following patch, protocol API to update a connection TID
affinity has been extended.
  commit 1a43b9f32c71267e3cb514aa70a13c75adb20742
  MINOR: proto: extend connection thread rebind API

The single callback set_affinity has been splitted in 3 different
functions which are called at different stages during listener_accept(),
depending on accept queue push success or not. However, the naming was
rendered confusing by the usage of function prefix 1 and 2.

Rename proto callback related to TID affinity update and use the
following names :

* bind_tid_prep
* bind_tid_commit
* bind_tid_reset

This commit should probably be backported at least up to 3.0 with the
above patch. This is because the fix was recently backported and it
would allow to keep changes minimal between the two versions. It could
even be backported up to 2.8 if there is no major conflict.

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

BUG/MEDIUM: quic: prevent crash on accept queue full

Handshake for quic_conn instances runs on a single non-chosen thread. On
completion, listener_accept() is performed to select the less loaded
thread before initializing connection instance. As such, quic_conn
instance is migrated to the thread with its upper connection.

In case accept queue is full, listener_accept() fallback to local accept
mode, which cause the connection to be assigned to the current thread.
However, this is not supported by QUIC as quic_conn instance is left on
the previously selected thread. In most cases, this will cause a
BUG_ON() due to a task manipulation from an outside thread.

To fix this, handle quic_conn thread rebind in multiple steps using the
new extended protocol API. Several operations have been moved from
qc_set_tid_affinity1() to newly defined qc_set_tid_affinity2(), in
particular CID TID update. This ensures that quic_conn instance is not
prematurely accessed on the new thread until accept queue push is
guaranteed to succeed.

qc_reset_tid_affinity() is also newly defined to reassign the newly
created tasks and tasklets to the current thread. This is necessary to
prevent the BUG_ON() crash described above.

This must be backported up to 2.8 after a period of observation. Note
that it depends on previous patch :
  MINOR: proto: extend connection thread rebind API

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

BUILD: listener: silence a build warning about unused value without threads

A variable introduced in commit 1a43b9f32c ("MINOR: proto: extend
connection thread rebind API") is not used without threads and causes a
build warning. Let's just mark it maybe_unused.

Since the commit above is tagged for backporting, this one will need to
be backported along with it.

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

MINOR: proto: extend connection thread rebind API

MINOR: listener: define callback for accept queue push

Extend API for connection thread rebind API by replacing single callback
set_affinity by three different ones. Each one of them is used at a
different stage of the operation :

* set_affinity1 is used similarly to previous set_affinity

* set_affinity2 is called directly from accept_queue_push_mp() when an
  entry has been found in accept ring. This operation cannot fail.

* reset_affinity is called after set_affinity1 in case of failure from
  accept_queue_push_mp() due to no space left in accept ring. This is
  necessary for protocols which must reconfigure resources before
  fallback on the current tid.

This patch does not have any functional changes. However, it will be
required to fix crashes for QUIC connections when accept queue ring is
full. As such, it must be backported with it.

(cherry picked from commit 1a43b9f32c71267e3cb514aa70a13c75adb20742)
[wt: backported for ease of maintenance, as suggested in commit
     9fbe8b0334 ("CLEANUP: proto: rename TID affinity callbacks") also
     marked for backporting]
Signed-off-by: Willy Tarreau <w@1wt.eu>

[RELEASE] Released version 3.0.3

Released version 3.0.3 with the following main changes :
    - BUG/MINOR: log: fix broken '+bin' logformat node option
    - DEBUG: hlua: distinguish burst timeout errors from exec timeout errors
    - REGTESTS: ssl: fix some regtests 'feature cmd' start condition
    - BUG/MEDIUM: proxy: fix email-alert invalid free
    - DOC: configuration: fix alphabetical order of bind options
    - DOC: management: document ptr lookup for table commands
    - BUG/MAJOR: quic: fix padding with short packets
    - SCRIPTS: git-show-backports: do not truncate git-show output
    - DOC: api/event_hdl: small updates, fix an example and add some precisions
    - BUG/MINOR: h3: fix crash on STOP_SENDING receive after GOAWAY emission
    - BUG/MINOR: mux-quic: fix crash on qcs SD alloc failure
    - BUG/MINOR: h3: fix BUG_ON() crash on control stream alloc failure
    - BUG/MINOR: quic: fix BUG_ON() on Tx pkt alloc failure
    - DEV: flags/show-fd-to-flags: adapt to recent versions
    - BUG/MINOR: hlua: report proper context upon error in hlua_cli_io_handler_fct()
    - BUG/MEDIUM: stick-table: Decrement the ref count inside lock to kill a session
    - DOC: configuration: add details about crt-store in bind "crt" keyword
    - BUG/MINOR: server: fix first server template name lookup UAF
    - MINOR: activity: make the memory profiling hash size configurable at build time
    - BUG/MEDIUM: server/dns: prevent DOWN/UP flap upon resolution timeout or error
    - BUG/MEDIUM: h3: ensure the ":method" pseudo header is totally valid
    - BUG/MEDIUM: h3: ensure the ":scheme" pseudo header is totally valid
    - BUG/MEDIUM: quic: fix race-condition in quic_get_cid_tid()
    - BUG/MINOR: quic: fix race condition in qc_check_dcid()
    - BUG/MINOR: quic: fix race-condition on trace for CID retrieval
    - BUG/MEDIUM: quic: fix possible exit from qc_check_dcid() without unlocking
    - BUG/MINOR: promex: Remove Help prefix repeated twice for each metric
    - BUG/MEDIUM: hlua/cli: Fix lua CLI commands to work with applet's buffers
    - DOC: configuration: more details about the master-worker mode
    - BUG/MEDIUM: server: fix race on server_atomic_sync()
    - BUG/MINOR: jwt: don't try to load files with HMAC algorithm
    - MEDIUM: init: set default for fd_hard_limit via DEFAULT_MAXFD
    - DOC: configuration: update maxconn description
    - BUG/MEDIUM: peers: Fix crash when syncing learn state of a peer without appctx
    - Revert "MEDIUM: init: set default for fd_hard_limit via DEFAULT_MAXFD"
    - BUG/MINOR: jwt: fix variable initialisation
    - BUG/MINOR: h1: Fail to parse empty transfer coding names
    - BUG/MINOR: h1: Reject empty coding name as last transfer-encoding value
    - BUG/MEDIUM: h1: Reject empty Transfer-encoding header
    - BUG/MEDIUM: spoe: Be sure to create a SPOE applet if none on the current thread
    - DEV: flags/quic: decode quic_conn flags
    - BUG/MEDIUM: bwlim: Be sure to never set the analyze expiration date in past

BUG/MEDIUM: bwlim: Be sure to never set the analyze expiration date in past

Every time a bandwidth limitation is evaluated on a channel, the analyze
expiration date is renewed, mainly based on the internal bandwidth
limitation filter expiration date. However, when the filter is called while
there is no data to filter, we skip all limitation computations to jump at
the end of the function. At this stage, the analyze expiration date is
renewed before exiting. But here the internal expiration date may be expired
and not reset.

To sum up, it is possible to set the analyze expiration date of a channel in
the past. It is unexpected and this could lead to a loop in process_stream.

To fix the issue, we just now take care to reset the internal expiration
date, if needed, before exiting.

This patch should fix the issue #2634. It must be backported as far as 2.8.

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

DEV: flags/quic: decode quic_conn flags

Decode quic_conn flags via qc_show_flags() function.

To support this, quic flags definition have been put outside of USE_QUIC
directive.

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

BUG/MEDIUM: spoe: Be sure to create a SPOE applet if none on the current thread

When a message is queued, waiting to be processed by a SPOE applet, there
are some heuristic to know if a new applet must be created or not. There are
2 conditions to skip the applet creation:

  1 - if there are enough idle applets on the current thread, or,

  2 - if the processing rate on the current thread is high enough to handle
      this new message

In the 2nd case, there is a flaw when the number of processed messages falls
to zero while the processing rate is still greater than zero. In that case,
we will skip the SPOE applet creation without taking care to check there is
at least one applet on the current thread.

So now, the conditions above to skip the SPOE applet creation are only
evaluated if there is at least one applet on the current thread.

This patch must be backported to every stable versions.

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

BUG/MEDIUM: h1: Reject empty Transfer-encoding header

The Transfer-Encoding headers list the transfer coding that have been
applied to the content in order to form the message body. It is a list of
tokens. And as specified by RFC 9110, a token cannot be empty. When several
coding names are specify as a comma-separated value, this case is properly
handled and an error is triggered. However, an empty header value will just
be skipped and no error is triggered. This could be an issue with some buggy
servers.

Now, empty Transfer-Encoding header are rejected too.

This patch must be backported as far as 2.6.

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