]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
10 months agoMINOR: Implements new log format of option tcplog clf
Nathan Wehrman [Tue, 13 Aug 2024 18:25:38 +0000 (11:25 -0700)] 
MINOR: Implements new log format of option tcplog clf

Some systems require log formats in the CLF format and that meant that I
could not send my logs for proxies in mode tcp to those servers.  This
implements a format that uses log variables that are compatble with TCP
mode frontends and replaces traditional HTTP values in the CLF format
to make them stand out. Instead of logging method and URI like this
"GET /example HTTP/1.1" it will log "TCP " and for a response code I
used "000" so it would be easy to separate from legitimate HTTP
traffic. Now your log servers that require a CLF format can see the
timings for TCP traffic as well as HTTP.

10 months agoDOC: lua: fix incorrect english in lua.txt
Nicolas CARPi [Tue, 13 Aug 2024 20:57:56 +0000 (22:57 +0200)] 
DOC: lua: fix incorrect english in lua.txt

This commit fixes some typos, grammatical errors and unusual english
such as "can not" instead of preferred "cannot".

10 months agoCI: QUIC Interop AWS-LC: enable chrome client
Ilia Shipitsin [Tue, 13 Aug 2024 19:11:29 +0000 (21:11 +0200)] 
CI: QUIC Interop AWS-LC: enable chrome client

chrome is important browser, let's enable it in AWS-LC weekly tests.
the only test supported by chrome is http3

10 months agoCI: modernize codespell action, switch to node 16
Ilia Shipitsin [Tue, 13 Aug 2024 19:11:30 +0000 (21:11 +0200)] 
CI: modernize codespell action, switch to node 16

The following actions uses node12 which is deprecated and will be forced
to run on node16: codespell-project/codespell-problem-matcher@v1. For
more info:
   https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default/

10 months agoCI: QUIC Interop LibreSSL: document chacha20 test status
Ilia Shipitsin [Tue, 13 Aug 2024 19:11:28 +0000 (21:11 +0200)] 
CI: QUIC Interop LibreSSL: document chacha20 test status

due to https://github.com/haproxy/haproxy/issues/2569 chacha20 is
disabled completely on LibreSSL. let's add a comment to not forget
enabling it

10 months agoMINOR: log: "drop" support for log-profile steps
Aurelien DARRAGON [Mon, 19 Aug 2024 16:06:19 +0000 (18:06 +0200)] 
MINOR: log: "drop" support for log-profile steps

It is now possible to use "drop" keyword for "on" lines under a
log-profile section to specify that no log at all should be emitted for
the specified step (setting an empty format was not sufficient to do so
because only the log payload would be empty, not the log header, thus the
log would still be emitted).

It may be useful to selectively disable logging at specific steps for a
given log target (since the log profile may be set on log directives):

log-profile myprof
  on request format "blabla" sd "custom sd"
  on response drop

New testcase was added to reg-tests/log/log_profiles.vtc

10 months agoMEDIUM: log: relax some checks and emit diag warnings instead in lf_expr_postcheck()
Aurelien DARRAGON [Tue, 13 Aug 2024 15:49:46 +0000 (17:49 +0200)] 
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.

10 months agoDOC: config: correct the table for option tcplog
Nathan Wehrman [Tue, 13 Aug 2024 17:36:28 +0000 (10:36 -0700)] 
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.

10 months agoMINOR: release-estimator: fix the shebang of the python script
William Lallemand [Tue, 13 Aug 2024 15:26:36 +0000 (17:26 +0200)] 
MINOR: release-estimator: fix the shebang of the python script

Fix the shebang of the python script to use /usr/bin/env, allowing to
call the script directly from a virtualenv with `./release-estimator.py`
without using the python3 install of the system.

10 months ago MINOR: release-estimator: add installation steps in README.md
William Lallemand [Tue, 13 Aug 2024 15:21:47 +0000 (17:21 +0200)] 
 MINOR: release-estimator: add installation steps in README.md

 Update the README.md with the dependencies and the installation steps
 with a python venv.

10 months agoMINOR: release-estimator: add requirements.txt
William Lallemand [Tue, 13 Aug 2024 15:12:59 +0000 (17:12 +0200)] 
MINOR: release-estimator: add requirements.txt

Add a requirements.txt file to install the release-estimator script.

10 months agoBUG/MINOR: release-estimator: fix relative scheme in CHANGELOG URL
William Lallemand [Tue, 13 Aug 2024 14:43:03 +0000 (16:43 +0200)] 
BUG/MINOR: release-estimator: fix relative scheme in CHANGELOG URL

The CHANGELOG URL which is parsed in the HTML now have a relative
scheme, which is incompatible with requests. This patch adds an https
scheme to the URL.

10 months agoCI: keep logs for failed QIUC Interop jobs
Ilia Shipitsin [Wed, 7 Aug 2024 11:02:29 +0000 (13:02 +0200)] 
CI: keep logs for failed QIUC Interop jobs

it might be useful to investigate logs of failed tests. to keep
artifacts small the following actions are taken
- only failed logs are kept
- logs retention is 6 days

10 months agoBUG/MINOR: pattern: pat_ref_set: return 0 if err was found
Valentine Krasnobaeva [Mon, 12 Aug 2024 17:21:00 +0000 (19:21 +0200)] 
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.

10 months agoBUG/MINOR: pattern: pat_ref_set: fix UAF reported by coverity
Valentine Krasnobaeva [Mon, 12 Aug 2024 13:32:00 +0000 (15:32 +0200)] 
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.

11 months agoBUG/MINOR: tools: make fgets_from_mem() stop at the end of the input
Willy Tarreau [Sun, 11 Aug 2024 12:44:28 +0000 (14:44 +0200)] 
BUG/MINOR: tools: make fgets_from_mem() stop at the end of the input

The memchr() used to look for the LF character must consider the end of
input, not just the output buffer size.

This was found by oss-fuzz:
   https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=71096

No backport is needed.

11 months agoCLEANUP: mworker/cli: clean up the mode handling
William Lallemand [Fri, 9 Aug 2024 15:41:36 +0000 (17:41 +0200)] 
CLEANUP: mworker/cli: clean up the mode handling

Cleanup the mode handling by refactoring the strings constant
that are written multiple times

11 months agoBUG/MINOR: h3: properly reject too long header responses
Amaury Denoyelle [Thu, 1 Aug 2024 16:20:08 +0000 (18:20 +0200)] 
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.

11 months agoMINOR: mux-quic: do not trace error in qcc_send_frames() on empty list
Amaury Denoyelle [Thu, 8 Aug 2024 07:30:40 +0000 (09:30 +0200)] 
MINOR: mux-quic: do not trace error in qcc_send_frames() on empty list

qcc_send_frames() can be called with an empty list and returns
immediately with an error code. This is convenience to be able to call
it in a while loop.

Remove the trace with "error" when this is the case and replacing it
with a less alarming "leaving on..." message. This should help debugging
when traces are active.

11 months agoMINOR: proto_uxst: copy errno in errmsg for syscalls
Valentine Krasnobaeva [Fri, 9 Aug 2024 11:54:43 +0000 (13:54 +0200)] 
MINOR: proto_uxst: copy errno in errmsg for syscalls

Let's copy errno in error messages, which we emit in cases when listen() or
connect() fail. This is helpful for debugging.

11 months agoBUG/MINOR: cfgparse: parse_cfg: fix null ptr dereference reported by coverity
Valentine Krasnobaeva [Fri, 9 Aug 2024 07:25:37 +0000 (09:25 +0200)] 
BUG/MINOR: cfgparse: parse_cfg: fix null ptr dereference reported by coverity

This commit fixes potential null ptr dereferences reported by coverity, see
more details about it in the issues #2676 and #2668.

'outline' ptr, which is initialized to NULL explicitly as a temporary buffer to
store split keywords may be in theory implicitly dereferenced in some corner
cases (which we haven't encountered yet with real world configurations) in
'if (!**args)'. parse_line() code, called before under some conditions
assigns: args[arg] = outline + outpos and outpos initial value is 0.

11 months agoBUG/MINOR: proto_uxst: delete fd from fdtab if listen() fails
Valentine Krasnobaeva [Fri, 9 Aug 2024 09:05:41 +0000 (11:05 +0200)] 
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.

11 months agoBUG/MINOR: mux-quic: do not send too big MAX_STREAMS ID
Amaury Denoyelle [Thu, 8 Aug 2024 10:04:47 +0000 (12:04 +0200)] 
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.

11 months agoMINOR: startup: fix unused value reported by coverity
Valentine Krasnobaeva [Thu, 8 Aug 2024 16:54:28 +0000 (18:54 +0200)] 
MINOR: startup: fix unused value reported by coverity

Unused 0 is assigned to ret, as it's rewritten by error code of read_cfg().
This issue was reported by coverity.

11 months agoMINOR: cfgparse: load_cfg_in_mem: fix null ptr dereference reported by coverity
Valentine Krasnobaeva [Thu, 8 Aug 2024 14:34:54 +0000 (16:34 +0200)] 
MINOR: cfgparse: load_cfg_in_mem: fix null ptr dereference reported by coverity

This helps to optimize a bit load_cfg_in_mem() and fixes the potential null ptr
dereference in fread() call. If (read_bytes + bytes_to_read) equals to initial
chunk_size (zero), realloc is never called, *cfg_content keeps its NULL value.

So, let's assure that initial number of bytes to read
(read_bytes + bytes_to_read) is stricly positive, when we enter into loop at
the first time.

11 months agoREGTESTS: mcli: test the pipelined commands on master CLI
William Lallemand [Thu, 8 Aug 2024 15:21:12 +0000 (17:21 +0200)] 
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.

11 months agoBUG/MEDIUM: mworker/cli: fix pipelined modes on master CLI
William Lallemand [Thu, 8 Aug 2024 15:09:15 +0000 (17:09 +0200)] 
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.

11 months agoMINOR: channel: implement ci_insert() function
William Lallemand [Thu, 8 Aug 2024 15:05:45 +0000 (17:05 +0200)] 
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'

11 months agoMINOR: proto_tcp: tcp_bind_listener: copy errno in errmsg
Valentine Krasnobaeva [Thu, 8 Aug 2024 09:25:34 +0000 (11:25 +0200)] 
MINOR: proto_tcp: tcp_bind_listener: copy errno in errmsg

Let's copy errno in errmsg produced by tcp_bind_listener if it fails in
a syscall(). This is helpful to debug issues, while binding listeners.

11 months agoBUG/MINOR: proto_tcp: keep error msg if listen() fails
Valentine Krasnobaeva [Wed, 7 Aug 2024 17:34:07 +0000 (19:34 +0200)] 
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.

11 months agoBUG/MINOR: proto_tcp: delete fd from fdtab if listen() fails
Valentine Krasnobaeva [Wed, 7 Aug 2024 17:31:09 +0000 (19:31 +0200)] 
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.

11 months ago[RELEASE] Released version 3.1-dev5 v3.1-dev5
Willy Tarreau [Wed, 7 Aug 2024 16:42:33 +0000 (18:42 +0200)] 
[RELEASE] Released version 3.1-dev5

Released version 3.1-dev5 with the following main changes :
    - BUG/MINOR: quic: Lack of precision when computing K (cubic only cc)
    - MEDIUM: ssl/quic: implement quic crypto with EVP_AEAD
    - MINOR: quic: rename confusing wording aes to hp
    - MEDIUM: quic: add key argument to header protection crypto functions
    - MEDIUM: quic: implement CHACHA20_POLY1305 for AWS-LC
    - MEDIUM: sink: assume sft appctx stickiness
    - MINOR: quic: delay Retry emission on quic-force-retry
    - MEDIUM: quic: implement quic-initial rules
    - MINOR: quic: support ACL for quic-initial rules
    - MINOR: quic: pass quic_dgram as obj_type for quic-initial rules
    - MINOR: quic: implement reject quic-initial action
    - MINOR: quic: implement send-retry quic-initial rules
    - BUG/MEDIUM: quic: fix invalid conn reject with CONNECTION_REFUSED
    - MEDIUM: h1: allow to preserve keep-alive on T-E + C-L
    - MINOR: quic: Add information to "show quic" for CUBIC cc.
    - MINOR: quic: Dump TX in flight bytes vs window values ratio.
    - BUG/MEDIUM: jwt: Clear SSL error queue on error when checking the signature
    - BUILD: cfgparse-quic: fix build error on Solaris due to missing netinet/in.h
    - MINOR: queue: add a function to check for TOCTOU after queueing
    - BUG/MEDIUM: queue: deal with a rare TOCTOU in assign_server_and_queue()
    - DOC: config: Add documentation about spop mode for backends
    - BUG/MEDIUM: stconn: Report error on SC on send if a previous SE error was set
    - BUG/MEDIUM: mux-pt/mux-h1: Release the pipe on connection error on sending path
    - BUILD: mux-pt: Use the right name for the sedesc variable
    - BUG/MINOR: stconn: bs.id and fs.id had their dependencies incorrect
    - BUG/MEDIUM: ssl: reactivate 0-RTT for AWS-LC
    - BUG/MEDIUM: ssl: 0-RTT initialized at the wrong place for AWS-LC
    - BUILD: ssl: replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC
    - BUG/MEDIUM: quic: prevent conn freeze on 0RTT undeciphered content
    - MINOR: tcp_sample: Move TCP low level sample fetch function to control layer
    - MINOR: quic: Define ->get_info() control layer callback for QUIC
    - MINOR: flags/mux-quic: decode qcc and qcs flags
    - BUG/MINOR: quic: fix fc_rtt/srtt values
    - BUG/MIONR: quic: fix fc_lost
    - BUG/MINOR: h1: do not forward h2c upgrade header token
    - BUG/MINOR: h2: reject extended connect for h2c protocol
    - BUG/MEDIUM: http-ana: Report error on write error waiting for the response
    - BUG/MEDIUM: h2: Only report early HTX EOM for tunneled streams
    - BUG/MEDIUM: mux-h2: Propagate term flags to SE on error in h2s_wake_one_stream
    - BUG/MEDIUM: peer: Notify the applet won't consume data when it waits for sync
    - BUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)
    - CI: add weekly QUIC Interop regression against AWS-LC
    - CI: harden NetBSD builds by ERR=1
    - BUG/MINOR: quic: Too short datagram during packet building failures (aws-lc only)
    - DEV: coccinelle: add a test to detect unchecked strdup()
    - BUG/MINOR: fcgi-app: handle a possible strdup() failure
    - BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once event miss and leak
    - MINOR: quic: convert qc_stream_desc release field to flags
    - MINOR: quic: implement function to check if STREAM is fully acked
    - BUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM
    - MINOR: quic: enforce ACK reception is handled in order
    - DOC: configuration: fix alphabetical ordering of {bs,fs}.aborted
    - MINOR: stconn: add a new pair of sf functions {bs,fs}.debug_str
    - MINOR: mux-h2: implement the debug string for logs
    - MINOR: mux-quic: define dump functions for QCC and QCS
    - MINOR: mux-quic: implement debug string for logs
    - MINOR: quic: dump quic_conn debug string for logs
    - MINOR: time: define tot_time structure
    - MINOR: mux-quic: measure QCS lifetime and its blocking state
    - BUG/MINOR: trace/quic: enable conn/session pointer recovery from quic_conn
    - BUG/MINOR: trace/quic: permit to lock on frontend/connect/session etc
    - BUG/MEDIUM: trace: fix null deref in lockon mechanism since TRACE_ENABLED()
    - BUG/MINOR: trace: automatically start in waiting mode with "start <evt>"
    - BUG/MINOR: trace/quic: make "qconn" selectable as a lockon criterion
    - BUG/MINOR: quic/trace: make quic_conn_enc_level_init() emit NEW not CLOSE
    - MINOR: trace: support setting the sink and level for all sources at once
    - MINOR: session/trace: enable very minimal session tracing
    - MEDIUM: trace: implement a "follow" mechanism
    - MINOR: trace: move the known trace context into a dedicated struct
    - MINOR: trace: add a per-source helper to pre-fill the context
    - MINOR: mux-h2: add a trace context filling helper
    - MINOR: mux-h1: add a trace context filling helper
    - MINOR: mux-quic: don't leave dangling pointer after freeing qcs->sd
    - MINOR: mux-quic: add a trace context filling helper
    - MINOR: mux-h1/trace: add a state trace on stream creation/upgrade
    - MINOR: mux-h2/trace: add a state trace on stream creation/destruction
    - MINOR: mux-h3/trace: add a state trace on stream creation/destruction
    - BUG/MINOR: quic: prevent freeze after early QCS closure
    - MINOR: server: ensure max_events_at_once > 0 in server_atomic_sync()
    - MINOR: cfgparse: add struct cfgfile to represent config in memory
    - REORG: tools: move list_append_word to cfgparse
    - MINOR: startup: adapt list_append_word to use cfgfile
    - MINOR: cfgparse: add load_cfg_in_mem
    - MINOR: cfgparse: load_cfg_in_mem: take in account file size
    - MINOR: tools: add fgets_from_mem
    - MEDIUM: startup: make read_cfg() return immediately on ENOMEM
    - MEDIUM: startup: load and parse configs from memory
    - MINOR: startup: rename readcfgfile in parse_cfg

11 months agoMINOR: startup: rename readcfgfile in parse_cfg
Valentine Krasnobaeva [Mon, 5 Aug 2024 08:04:03 +0000 (10:04 +0200)] 
MINOR: startup: rename readcfgfile in parse_cfg

As readcfgfile no longer opens configuration files and reads them with fgets,
but performs only the parsing of provided data, let's rename it to parse_cfg by
analogy with read_cfg in haproxy.c.

11 months agoMEDIUM: startup: load and parse configs from memory
Valentine Krasnobaeva [Wed, 7 Aug 2024 14:53:50 +0000 (16:53 +0200)] 
MEDIUM: startup: load and parse configs from memory

Let's call load_cfg_in_ram() helper for each configuration file to load it's
content in some area in memory. Adapt readcfgfile() parser function
respectively. In order to limit changes in its scope we give as an argument a
cfgfile structure, already filled in init_args() and in load_cfg_in_ram() with
file metadata and content.

Parser function (readcfgfile()) uses now fgets_from_mem() instead of standard
fgets from libc implementations.

SPOE filter parses its own configuration file, pointed by 'config' keyword in
the configuration already loaded in memory. So, let's allocate and fill for
this a supplementary cfgfile structure, which is not referenced in cfg_cfgfiles
list. This structure and the memory with content of SPOE filter configuration
are freed immediately in parse_spoe_flt(), when readcfgfile() returns.

HAProxy OpenTracing filter also uses its own configuration file. So, let's
follow the same logic as we do for SPOE filter.

11 months agoMEDIUM: startup: make read_cfg() return immediately on ENOMEM
Valentine Krasnobaeva [Mon, 5 Aug 2024 08:03:52 +0000 (10:03 +0200)] 
MEDIUM: startup: make read_cfg() return immediately on ENOMEM

This commit prepares read_cfg() to call load_cfg_in_mem() helper in order to
load configuration files in memory. Before, read_cfg() calls the parser for all
files from cfg_cfgfiles list and cumulates parser's errors and memprintf's
errors in for_each loop. memprintf's errors did not stop this loop and were
accounted just after.

Now, as we plan to load configuration files in memory, we stop the loop, if
memprintf() fails, and we show appropraite error message with ha_alert. Then
process terminates. So not all cumulated syntax-related errors will be shown
before exit in this case and we has to stop, because we run out of memory.

If we can't open the current file or we fail to allocate a memory to store
some configuration line, the previous behaviour is kept, process emits
appropriate alert message and exits.

If parser returns some syntax-related error on the current file, the previous
behaviour is kept as well. We cumulate such errors for all parsed files and we
check them just after the loop. All syntax-related errors for all files is
shown then  as before in ha_alert messages line by line during the startup.
Then process will exit with 1.

As now cfg_cfgfiles list contains many pointers to some memory areas with
configuration files content and this content could be big, it's better to
free the list explicitly, when parsing was finished. So, let's change
read_cfg() to return some integer value to its caller init(), and let's perform
the free  routine at a caller level, as cfg_cfgfiles list was initialized and
initially filled at this level.

11 months agoMINOR: tools: add fgets_from_mem
Valentine Krasnobaeva [Mon, 5 Aug 2024 08:03:46 +0000 (10:03 +0200)] 
MINOR: tools: add fgets_from_mem

Add fgets_from_mem() helper to read lines from configuration files, stored now
as memory chunks. In order to limit changes in the first-level parser code
(readcfgfile()), it is better to reimplement the standard fgets, i.e. to
have a fgets, which can read the serialized data line by line from some memory
area, instead of file stream, and can keep the same behaviour as libc
implementations fgets.

11 months agoMINOR: cfgparse: load_cfg_in_mem: take in account file size
Valentine Krasnobaeva [Wed, 7 Aug 2024 14:31:25 +0000 (16:31 +0200)] 
MINOR: cfgparse: load_cfg_in_mem: take in account file size

Let's take in account the given file size, when its reported via stat.

It's very convenient for large configuration files, as this allows to
perform only the one memory allocation call for precisely needeed file size.
This also allows to perform only the one call to fread().

We need to provide to fread() file_stat.st_size + 1 to be able to grab EOF.
Like this it sets feof(f)=1 flag and this allows to exit from the loop
immediately, just after fread call.

If /dev/stdin or /dev/null is provided as a file, we continue to read the
configuration chunk by chunk, stat doesn't report the size.

11 months agoMINOR: cfgparse: add load_cfg_in_mem
Valentine Krasnobaeva [Mon, 5 Aug 2024 08:03:39 +0000 (10:03 +0200)] 
MINOR: cfgparse: add load_cfg_in_mem

Add load_cfg_in_mem() helper, which allows to store the content of a given file
in memory.

11 months agoMINOR: startup: adapt list_append_word to use cfgfile
Valentine Krasnobaeva [Wed, 7 Aug 2024 16:20:43 +0000 (18:20 +0200)] 
MINOR: startup: adapt list_append_word to use cfgfile

list_append_word() helper was used before only to chain configuration file names
in a list. As now we start to use cfgfile structure which represents entire file
in memory and its metadata, let's adapt this helper to use this structure and
let's rename it to list_append_cfgfile().

Adapt functions, which process configuration files and directories to use
cfgfile structure and list_append_cfgfile() instead of wordlist.

11 months agoREORG: tools: move list_append_word to cfgparse
Valentine Krasnobaeva [Wed, 7 Aug 2024 16:12:48 +0000 (18:12 +0200)] 
REORG: tools: move list_append_word to cfgparse

Let's move list_append_word to cfgparse.c as it is used only to fill
cfg_cfgfiles list with configuration file names.

11 months agoMINOR: cfgparse: add struct cfgfile to represent config in memory
Valentine Krasnobaeva [Mon, 5 Aug 2024 08:03:00 +0000 (10:03 +0200)] 
MINOR: cfgparse: add struct cfgfile to represent config in memory

This and following commits serve to prepare loading configuration files in
memory, before parsing them, as we may need to parse some parts of
configuration in different moments of the startup sequence. This is a case of
the new master-worker initialization process. Here we need to read at first
only the global and the program sections and only after some steps
(forking worker, etc) the rest of the configuration.

Add a new structure cfgfile to keep configuration files metadata and content,
loaded somewhere in a memory. Instances of filled cfgfile structures could be
chained in a list, as the order in which they were loaded is important.

11 months agoMINOR: server: ensure max_events_at_once > 0 in server_atomic_sync()
Aurelien DARRAGON [Wed, 7 Aug 2024 16:04:08 +0000 (18:04 +0200)] 
MINOR: server: ensure max_events_at_once > 0 in server_atomic_sync()

In 8f1fd96 ("BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once
event miss and leak"), we added a comment saying that
tune.events.max-events-at-once is assumed to be strictly positive.

It is so because the keyword parser forces values between 1 and 10000:
we don't want less than 1 because it wouldn't make any sense, and 10k
max because beyond that we could create contention in server_atomic_sync()

Now as the above commit implements a do..while it heavily relies on the
fact that the budget is at least 1. Upon soft-stop, we break away from
the loop without decrementing the budget. With all that in mind, it is
safe to assume that the 'remain' counter will only fall to 0 if the task
runs out of budget while doing work, in which case the task still exists
and must be rescheduled.

As seen in GH #2667 this assumption was ambiguous, so let's make it
official by adding a pair of BUG_ON() that make it explicit that it
works because remain 'cannot' be 0 unless the entire budget was
consumed.

No backport needed.

11 months agoBUG/MINOR: quic: prevent freeze after early QCS closure
Amaury Denoyelle [Wed, 7 Aug 2024 16:01:51 +0000 (18:01 +0200)] 
BUG/MINOR: quic: prevent freeze after early QCS closure

A connection freeze may occur if a QCS is released before transmitting
any data. This can happen when an error is detected early by the stream,
for example during HTTP response headers encoding, forcing the whole
connection closure.

In this case, a connection error is registered by the QUIC MUX to the
lower layer. MUX is then release and xprt layer is notified to prepare
CONNECTION_CLOSE emission. However, this is prevented because quic_conn
streams tree is not empty as it contains the qc_stream_desc previously
attached to the failed QCS instance. The connection will freeze until
QUIC idle timeout.

This situation is caused by an omission during qc_stream_desc release
operation. In the described situation, qc_stream_desc current buffer is
empty and can thus by removed, which is the purpose of this patch. This
unblocks this previously failed situation, with qc_stream_desc removal
from quic_conn tree.

This issue can be reproduced by modifying H3/QPACK code to return an
early error during HEADERS response processing.

This must be backported up to 2.6, after a period of observation.

11 months agoMINOR: mux-h3/trace: add a state trace on stream creation/destruction
Willy Tarreau [Wed, 7 Aug 2024 13:36:17 +0000 (15:36 +0200)] 
MINOR: mux-h3/trace: add a state trace on stream creation/destruction

Logging below the developer level doesn't always yield very convenient
traces as we don't know well where streams are allocated nor released.
Let's just make that more explicit by using state-level traces for these
important steps.

11 months agoMINOR: mux-h2/trace: add a state trace on stream creation/destruction
Willy Tarreau [Wed, 7 Aug 2024 13:35:30 +0000 (15:35 +0200)] 
MINOR: mux-h2/trace: add a state trace on stream creation/destruction

Logging below the developer level doesn't always yield very convenient
traces as we don't know well where streams are allocated nor released.
Let's just make that more explicit by using state-level traces for these
important steps.

11 months agoMINOR: mux-h1/trace: add a state trace on stream creation/upgrade
Willy Tarreau [Wed, 7 Aug 2024 13:31:56 +0000 (15:31 +0200)] 
MINOR: mux-h1/trace: add a state trace on stream creation/upgrade

Logging below the developer level doesn't always yield very convenient
traces as we don't know well where streams are allocated nor released.
Let's just make that more explicit by using state-level traces. Note that
h1s destruction was already logged as closing connection or switching
to idle mode.

11 months agoMINOR: mux-quic: add a trace context filling helper
Willy Tarreau [Tue, 6 Aug 2024 17:01:30 +0000 (19:01 +0200)] 
MINOR: mux-quic: add a trace context filling helper

This helper is able to find a connection, a session, a stream, or a
frontend from its args.

11 months agoMINOR: mux-quic: don't leave dangling pointer after freeing qcs->sd
Willy Tarreau [Tue, 6 Aug 2024 16:59:39 +0000 (18:59 +0200)] 
MINOR: mux-quic: don't leave dangling pointer after freeing qcs->sd

In qcs_free() we're calling a few other functions after releasing
qcs->sd. None of them make use of it for now but with traces that
will change. Make sure to clear qcs->sd after releasing it.

11 months agoMINOR: mux-h1: add a trace context filling helper
Willy Tarreau [Tue, 6 Aug 2024 16:42:00 +0000 (18:42 +0200)] 
MINOR: mux-h1: add a trace context filling helper

This helper is able to find a connection, a session, a stream, a
frontend or a backend from its args.

11 months agoMINOR: mux-h2: add a trace context filling helper
Willy Tarreau [Tue, 6 Aug 2024 16:33:42 +0000 (18:33 +0200)] 
MINOR: mux-h2: add a trace context filling helper

This helper is able to find a connection, a session, a stream, a
frontend or a backend from its args.

Note that this required to always make sure that h2s->sess is reset on
allocation because it's normally initialized later for backend streams,
and producing traces between the two could pre-fill a bad pointer in
the trace_ctx.

11 months agoMINOR: trace: add a per-source helper to pre-fill the context
Willy Tarreau [Tue, 6 Aug 2024 16:09:18 +0000 (18:09 +0200)] 
MINOR: trace: add a per-source helper to pre-fill the context

Now sources which want to do it can provide a helper that can pre-fill
some fields in the context based on their knowledge (e.g. mux streams).

11 months agoMINOR: trace: move the known trace context into a dedicated struct
Willy Tarreau [Tue, 6 Aug 2024 16:04:31 +0000 (18:04 +0200)] 
MINOR: trace: move the known trace context into a dedicated struct

We now have a trace_ctx to hold the sess, conn, qc, stream and so on.
This will allow us to pass it across layers so that other helpers can
help fill them.

Ideally it should be passed as an argument to __trace_enabled() by
__trace() so that it can be passed back to the trace callback. But
it seems that trace callbacks are smart enough to figure all their
info when they need them.

11 months agoMEDIUM: trace: implement a "follow" mechanism
Willy Tarreau [Tue, 6 Aug 2024 14:44:33 +0000 (16:44 +0200)] 
MEDIUM: trace: implement a "follow" mechanism

With "follow" from one source to another, it becomes possible for a
source to automatically follow another source's tracked pointer. The
best example is the session:
  - the "session" source is enabled and has a "lockon session"
    -> its lockon_ptr is equal to the session when valid
  - other sources (h1,h2,h3 etc) are configured for "follow session"
    and will then automatically check if session's lockon_ptr matches
    its own session, in which case tracing will be enabled for that
    trace (no state change).

It's not necessary to start/pause/stop traces when using this, only
"follow" followed by a source with lockon enabled is needed. Some
combinations might work better than others. At the moment the session
is almost never known from the backend, but this may improve.

The meta-source "all" is supported for the follower so that all sources
will follow the tracked one.

11 months agoMINOR: session/trace: enable very minimal session tracing
Willy Tarreau [Tue, 6 Aug 2024 14:12:11 +0000 (16:12 +0200)] 
MINOR: session/trace: enable very minimal session tracing

By having traces at the session level, it becomes possible to start
traces on session creation and pause them on session end. Doing so
will soon open new possibilties to synchronize multiple traces.

11 months agoMINOR: trace: support setting the sink and level for all sources at once
Willy Tarreau [Tue, 6 Aug 2024 17:18:43 +0000 (19:18 +0200)] 
MINOR: trace: support setting the sink and level for all sources at once

It's extremely painful to have to set "trace <src> sink buf1" for all
sources, then to do the same for "level developer" (for example). Let's
have a possibility via a meta-source "all" to apply the change to all
sources at once. This currently supports level and sink, which are not
dependent on the source, this is a good start.

11 months agoBUG/MINOR: quic/trace: make quic_conn_enc_level_init() emit NEW not CLOSE
Willy Tarreau [Tue, 6 Aug 2024 13:22:50 +0000 (15:22 +0200)] 
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.

11 months agoBUG/MINOR: trace/quic: make "qconn" selectable as a lockon criterion
Willy Tarreau [Tue, 6 Aug 2024 13:21:00 +0000 (15:21 +0200)] 
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.

11 months agoBUG/MINOR: trace: automatically start in waiting mode with "start <evt>"
Willy Tarreau [Tue, 6 Aug 2024 09:45:54 +0000 (11:45 +0200)] 
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.

11 months agoBUG/MEDIUM: trace: fix null deref in lockon mechanism since TRACE_ENABLED()
Willy Tarreau [Tue, 6 Aug 2024 09:32:10 +0000 (11:32 +0200)] 
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.

11 months agoBUG/MINOR: trace/quic: permit to lock on frontend/connect/session etc
Willy Tarreau [Tue, 6 Aug 2024 09:09:03 +0000 (11:09 +0200)] 
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.

11 months agoBUG/MINOR: trace/quic: enable conn/session pointer recovery from quic_conn
Willy Tarreau [Tue, 6 Aug 2024 09:07:13 +0000 (11:07 +0200)] 
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.

11 months agoMINOR: mux-quic: measure QCS lifetime and its blocking state
Amaury Denoyelle [Wed, 31 Jul 2024 16:43:55 +0000 (18:43 +0200)] 
MINOR: mux-quic: measure QCS lifetime and its blocking state

Reuse newly defined tot_time structure to measure various values related
to a QCS lifetime.

First, a timer is used to comptabilize the total QCS lifetime. Then, two
other timers are used to account the total time during which Tx from
stream layer to MUX is blocked, either on lack of buffer or due to
flow-control.

These three timers are reported in qmux_dump_qcs_info(). Thus, they are
available in traces and for QUIC MUX debug string sample.

11 months agoMINOR: time: define tot_time structure
Amaury Denoyelle [Wed, 7 Aug 2024 12:50:26 +0000 (14:50 +0200)] 
MINOR: time: define tot_time structure

Define a new utility type tot_time. Its purpose is to be able to account
elapsed time accross multiple periods. Functions are defined to easily
start and stop measures, and return the current value.

11 months agoMINOR: quic: dump quic_conn debug string for logs
Amaury Denoyelle [Thu, 1 Aug 2024 09:35:04 +0000 (11:35 +0200)] 
MINOR: quic: dump quic_conn debug string for logs

Define a new xprt_ops callback named dump_info. This can be used to
extend MUX debug string with infos from the lower layer.

Implement dump_info for QUIC stack. For now, only minimal info are
reported : bytes in flight and size of the sending window. This should
allow to detect if the congestion controller is fine. These info are
reported via QUIC MUX debug string sample.

11 months agoMINOR: mux-quic: implement debug string for logs
Amaury Denoyelle [Wed, 31 Jul 2024 15:28:24 +0000 (17:28 +0200)] 
MINOR: mux-quic: implement debug string for logs

Implement MUX_SCTL_DBG_STR for QUIC MUX. This returns info for the
current QCS and QCC instances, reusing qmux_dump_qc{c,s}_info functions
already used for traces, and the connection flags.

This stream operation is useful for debug string sample support.

11 months agoMINOR: mux-quic: define dump functions for QCC and QCS
Amaury Denoyelle [Wed, 31 Jul 2024 15:27:33 +0000 (17:27 +0200)] 
MINOR: mux-quic: define dump functions for QCC and QCS

Extract trace code to dump QCC and QCS instances into dedicated
functions named qmux_dump_qc{c,s}_info(). This will allow to easily
print QCC/QCS infos outside of traces.

11 months agoMINOR: mux-h2: implement the debug string for logs
Willy Tarreau [Tue, 30 Jul 2024 17:33:07 +0000 (19:33 +0200)] 
MINOR: mux-h2: implement the debug string for logs

Now it permits to have this for a front and a back:

<134>Jul 30 19:32:53 haproxy[24405]: 127.0.0.1:64860 [30/Jul/2024:19:32:53.732] test2 test2/s1 0/0/0/0/0 200 130 - - ---- 2/1/0/0/0 0/0 "GET /blah HTTP/2.0"  h2s.id=1 .st=CLO .flg=0x7003 .rxbuf=0@(nil)+0/0 .sc=0x1e03fb0(.flg=0x00034482 .app=0x1e04020) .sd=0x1e03f30(.flg=0x50405601) .subs=(nil) h2c.st0=FRH .err=0 .maxid=1 .lastid=-1 .flg=0x100e00 .nbst=0 .nbsc=1, .glitches=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=1 .orph_cnt=0 .sub=1 .dsi=1 .dbuf=0@(nil)+0/0 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] .task=(nil) conn.flg=0x80000300
<134>Jul 30 19:32:53 haproxy[24405]: 127.0.0.1:65246 [30/Jul/2024:19:32:53.732] test1 test1/s1 0/0/0/0/0 200 130 - - ---- 2/1/0/0/0 0/0 "GET /blah HTTP/1.1"  h2s.id=1 .st=CLO .flg=0x7003 .rxbuf=0@(nil)+0/0 .sc=0x1dfc7b0(.flg=0x0006d01b .app=0x1c65fe0) .sd=0x1dfc820(.flg=0x1040ca01) .subs=(nil) h2c.st0=FRH .err=0 .maxid=1 .lastid=-1 .flg=0x108e00 .nbst=0 .nbsc=1, .glitches=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=1 .orph_cnt=0 .sub=1 .dsi=1 .dbuf=0@(nil)+0/0 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] .task=(nil) conn.flg=0x000300

Just with this in the front and back proxies respectively:
  log-format "$HAPROXY_HTTP_LOG_FMT %[bs.debug_str(15)]"
  log-format "$HAPROXY_HTTP_LOG_FMT %[fs.debug_str(15)]"

For now the mux only implements muxs, muxc, conn. Xprt is ignored.

11 months agoMINOR: stconn: add a new pair of sf functions {bs,fs}.debug_str
Willy Tarreau [Tue, 30 Jul 2024 15:57:37 +0000 (17:57 +0200)] 
MINOR: stconn: add a new pair of sf functions {bs,fs}.debug_str

These are passed to the underlying mux to retrieve debug information
at the mux level (stream/connection) as a string that's meant to be
added to logs.

The API is quite complex just because we can't pass any info to the
bottom function. So we construct a union and pass the argument as an
int, and expect the callee to fill that with its buffer in return.

Most likely the mux->ctl and ->sctl API should be reworked before
the release to simplify this.

The functions take an optional argument that is a bit mask of the
layers to dump:
  muxs=1
  muxc=2
  xprt=4
  conn=8
  sock=16

The default (0) logs everything available.

11 months agoDOC: configuration: fix alphabetical ordering of {bs,fs}.aborted
Willy Tarreau [Wed, 7 Aug 2024 11:51:52 +0000 (13:51 +0200)] 
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).

11 months agoMINOR: quic: enforce ACK reception is handled in order
Amaury Denoyelle [Tue, 6 Aug 2024 14:30:42 +0000 (16:30 +0200)] 
MINOR: quic: enforce ACK reception is handled in order

Add a new BUG_ON() in qc-stream_desc_ack(). It ensures that
acknowledgement are always notify in-order. This is because out-of-order
ACKs cannot be handled by qc_stream_desc layer which does not support
gap in STREAM sent data.

Prior to this fix, out-of-order ACKs are simply ignored without any
error. This currently cannot happen thanks to careful
qc_stream_desc_ack() invokation. If this assumption is broken in the
future by inatteion, this would cause loss of ACK notification which
will prevent qc_stream_desc release.

11 months agoBUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM
Amaury Denoyelle [Mon, 5 Aug 2024 16:58:49 +0000 (18:58 +0200)] 
BUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM

STREAM frames have dedicated handling on retransmission. A special check
is done to remove data already acked in case of duplicated frames, thus
only unacked data are retransmitted.

This handling is faulty in case of an empty STREAM frame with FIN set.
On retransmission, this frame does not cover any unacked range as it is
empty and is thus discarded. This may cause the transfer to freeze with
the client waiting indefinitely for the FIN notification.

To handle retransmission of empty FIN STREAM frame, qc_stream_desc layer
have been extended. A new flag QC_SD_FL_WAIT_FOR_FIN is set by MUX QUIC
when FIN has been transmitted. If set, it prevents qc_stream_desc to be
freed until FIN is acknowledged. On retransmission side,
qc_stream_frm_is_acked() has been updated. It now reports false if
FIN bit is set on the frame and qc_stream_desc has QC_SD_FL_WAIT_FOR_FIN
set.

This must be backported up to 2.6. However, this modifies heavily
critical section for ACK handling and retransmission. As such, it must
be backported only after a period of observation.

This issue can be reproduced by using the following socat command as
server to add delay between the response and connection closure :
  $ socat TCP-LISTEN:<port>,fork,reuseaddr,crlf SYSTEM:'echo "HTTP/1.1 200 OK"; echo ""; sleep 1;'

On the client side, ngtcp2 can be used to simulate packet drop. Without
this patch, connection will be interrupted on QUIC idle timeout or
haproxy client timeout with ERR_DRAINING on ngtcp2 :
  $ ngtcp2-client --exit-on-all-streams-close -r 0.3 <host> <port> "http://<host>:<port>/?s=32o"

Alternatively to ngtcp2 random loss, an extra haproxy patch can also be
used to force skipping the emission of the empty STREAM frame :

diff --git a/include/haproxy/quic_tx-t.h b/include/haproxy/quic_tx-t.h
index efbdfe687..1ff899acd 100644
--- a/include/haproxy/quic_tx-t.h
+++ b/include/haproxy/quic_tx-t.h
@@ -26,6 +26,8 @@ extern struct pool_head *pool_head_quic_cc_buf;
 /* Flag a sent packet as being probing with old data */
 #define QUIC_FL_TX_PACKET_PROBE_WITH_OLD_DATA (1UL << 5)

+#define QUIC_FL_TX_PACKET_SKIP_SENDTO (1UL << 6)
+
 /* Structure to store enough information about TX QUIC packets. */
 struct quic_tx_packet {
  /* List entry point. */
diff --git a/src/quic_tx.c b/src/quic_tx.c
index 2f199ac3c..2702fc9b9 100644
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -318,7 +318,7 @@ static int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
  tmpbuf.size = tmpbuf.data = dglen;

  TRACE_PROTO("TX dgram", QUIC_EV_CONN_SPPKTS, qc);
- if (!skip_sendto) {
+ if (!skip_sendto && !(first_pkt->flags & QUIC_FL_TX_PACKET_SKIP_SENDTO)) {
  int ret = qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0, gso);
  if (ret < 0) {
  if (gso && ret == -EIO) {
@@ -354,6 +354,7 @@ static int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
  qc->cntrs.sent_bytes_gso += ret;
  }
  }
+ first_pkt->flags &= ~QUIC_FL_TX_PACKET_SKIP_SENDTO;

  b_del(buf, dglen + QUIC_DGRAM_HEADLEN);
  qc->bytes.tx += tmpbuf.data;
@@ -2066,6 +2067,17 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
  continue;
  }

+ switch (cf->type) {
+ case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
+ if (!cf->stream.len && (qc->flags & QUIC_FL_CONN_TX_MUX_CONTEXT)) {
+ TRACE_USER("artificially drop packet with empty STREAM frame", QUIC_EV_CONN_TXPKT, qc);
+ pkt->flags |= QUIC_FL_TX_PACKET_SKIP_SENDTO;
+ }
+ break;
+ default:
+ break;
+ }
+
  quic_tx_packet_refinc(pkt);
  cf->pkt = pkt;
  }

11 months agoMINOR: quic: implement function to check if STREAM is fully acked
Amaury Denoyelle [Tue, 6 Aug 2024 15:36:56 +0000 (17:36 +0200)] 
MINOR: quic: implement function to check if STREAM is fully acked

When a STREAM frame is retransmitted, a check is performed to remove
range of data already acked from it. This is useful when STREAM frames
are duplicated and splitted to cover different data ranges. The newly
retransmitted frame contains only unacked data.

This process is performed similarly in qc_dup_pkt_frms() and
qc_build_frms(). Refactor the code into a new function named
qc_stream_frm_is_acked(). It returns true if frame data are already
fully acked and retransmission can be avoided. If only a partial range
of data is acknowledged, frame content is updated to only cover the
unacked data.

This patch does not have any functional change. However, it simplifies
retransmission for STREAM frames. Also, it will be reused to fix
retransmission for empty STREAM frames with FIN set from the following
patch :
  BUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM

As such, it must be backported prior to it.

11 months agoMINOR: quic: convert qc_stream_desc release field to flags
Amaury Denoyelle [Mon, 5 Aug 2024 16:52:27 +0000 (18:52 +0200)] 
MINOR: quic: convert qc_stream_desc release field to flags

qc_stream_desc had a field <release> used as a boolean. Convert it with
a new <flags> field and QC_SD_FL_RELEASE value as equivalent.

The purpose of this patch is to be able to extend qc_stream_desc by
adding newer flags values. This patch is required for the following
patch
  BUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM

As such, it must be backported prior to it.

11 months agoBUG/MEDIUM: server/addr: fix tune.events.max-events-at-once event miss and leak
Aurelien DARRAGON [Tue, 6 Aug 2024 12:29:56 +0000 (14:29 +0200)] 
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.

11 months agoBUG/MINOR: fcgi-app: handle a possible strdup() failure
Ilia Shipitsin [Mon, 5 Aug 2024 18:59:09 +0000 (20:59 +0200)] 
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.

11 months agoDEV: coccinelle: add a test to detect unchecked strdup()
Ilia Shipitsin [Mon, 5 Aug 2024 18:59:09 +0000 (20:59 +0200)] 
DEV: coccinelle: add a test to detect unchecked strdup()

The coccinelle test "unchecked-strdup.cocci" detects various cases of
unchecked strdup().

11 months agoBUG/MINOR: quic: Too short datagram during packet building failures (aws-lc only)
Frederic Lecaille [Mon, 5 Aug 2024 11:40:51 +0000 (13:40 +0200)] 
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.

11 months agoCI: harden NetBSD builds by ERR=1
Ilia Shipitsin [Sun, 21 Jul 2024 11:52:08 +0000 (13:52 +0200)] 
CI: harden NetBSD builds by ERR=1

Add ERR=1 build option to the NetBSD build from github.

11 months agoCI: add weekly QUIC Interop regression against AWS-LC
Ilia Shipitsin [Sat, 3 Aug 2024 13:43:36 +0000 (15:43 +0200)] 
CI: add weekly QUIC Interop regression against AWS-LC

currently only quic-go and picoquic clients are enabled.
Tests will be run weekly.

11 months agoBUG/MINOR: quic: Too shord datagram during O-RTT handshakes (aws-lc only)
Frederic Lecaille [Fri, 2 Aug 2024 09:05:45 +0000 (11:05 +0200)] 
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.

11 months agoBUG/MEDIUM: peer: Notify the applet won't consume data when it waits for sync
Christopher Faulet [Thu, 1 Aug 2024 15:09:06 +0000 (17:09 +0200)] 
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.

11 months agoBUG/MEDIUM: mux-h2: Propagate term flags to SE on error in h2s_wake_one_stream
Christopher Faulet [Thu, 1 Aug 2024 14:22:41 +0000 (16:22 +0200)] 
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.

11 months agoBUG/MEDIUM: h2: Only report early HTX EOM for tunneled streams
Christopher Faulet [Thu, 1 Aug 2024 14:01:50 +0000 (16:01 +0200)] 
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.

11 months agoBUG/MEDIUM: http-ana: Report error on write error waiting for the response
Christopher Faulet [Thu, 1 Aug 2024 13:42:09 +0000 (15:42 +0200)] 
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.

11 months agoBUG/MINOR: h2: reject extended connect for h2c protocol
Amaury Denoyelle [Thu, 1 Aug 2024 13:52:56 +0000 (15:52 +0200)] 
BUG/MINOR: h2: reject extended connect for h2c protocol

This commit prevents forwarding of an HTTP/2 Extended CONNECT when "h2c"
or "h2" token is set as targetted protocol. Contrary to the previous
commit which deals with HTTP/1 mux, this time the request is rejected
and a RESET_STREAM is reported to the client.

This must be backported up to 2.4 after a period of observation.

11 months agoBUG/MINOR: h1: do not forward h2c upgrade header token
Amaury Denoyelle [Thu, 1 Aug 2024 13:35:06 +0000 (15:35 +0200)] 
BUG/MINOR: h1: do not forward h2c upgrade header token

haproxy supports tunnel establishment through HTTP Upgrade mechanism.
Since the following commit, extended CONNECT is also supported for
HTTP/2 both on frontend and backend side.

  commit 9bf957335e2c385b74901481f7a89c9565dfce53
  MEDIUM: mux_h2: generate Extended CONNECT from htx upgrade

As specified by HTTP/2 rfc, "h2c" can be used by an HTTP/1.1 client to
request an upgrade to HTTP/2. In haproxy, this is not supported so it
silently ignores this. However, Connection and Upgrade headers are
forwarded as-is on the backend side.

If using HTTP/1 on the backend side and the server supports this upgrade
mechanism, haproxy won't be able to parse the HTTP response. If using
HTTP/2, mux backend tries to incorrectly convert the request to an
Extended CONNECT with h2c protocol, which may also prevent the response
to be transmitted.

To fix this, flag HTTP/1 request with "h2c" or "h2" token in an upgrade
header. On converting the header list to HTX, the upgrade header is
skipped if any of this token is present and the H1_MF_CONN_UPG flag is
removed.

This issue can easily be reproduced using curl --http2 argument to
connect to an HTTP/1 frontend.

This must be backported up to 2.4 after a period of observation.

11 months agoBUG/MIONR: quic: fix fc_lost
Amaury Denoyelle [Thu, 1 Aug 2024 09:29:58 +0000 (11:29 +0200)] 
BUG/MIONR: quic: fix fc_lost

Control layer callback get_info has recently been implemented for QUIC.
However, fc_lost always returned 0. This is because quic_get_info() does
not use the correct input argument value to identify lost value.

This does not need to be backported.

11 months agoBUG/MINOR: quic: fix fc_rtt/srtt values
Amaury Denoyelle [Thu, 1 Aug 2024 09:25:15 +0000 (11:25 +0200)] 
BUG/MINOR: quic: fix fc_rtt/srtt values

QUIC has recently implement get_info callback to return RTT/sRTT values.
However, it uses milliseconds, contrary to TCP which uses microseconds.
This cause smp fetch functions to return invalid values. Fix this by
converting QUIC values to microseconds.

This does not need to be backported.

11 months agoMINOR: flags/mux-quic: decode qcc and qcs flags
Amaury Denoyelle [Wed, 31 Jul 2024 15:58:02 +0000 (17:58 +0200)] 
MINOR: flags/mux-quic: decode qcc and qcs flags

Decode QUIC MUX connection and stream elements via qcc_show_flags() and
qcs_show_flags(). Flags definition have been moved outside of USE_QUIC
to ease compilation of flags binary.

11 months agoMINOR: quic: Define ->get_info() control layer callback for QUIC
Frederic Lecaille [Tue, 30 Jul 2024 14:32:48 +0000 (16:32 +0200)] 
MINOR: quic: Define ->get_info() control layer callback for QUIC

This low level callback may be called by several sample fetches for
frontend connections like "fc_rtt", "fc_rttvar" etc.
Define this callback for QUIC protocol as pointer to quic_get_info().
This latter supports these sample fetches:
   "fc_lost", "fc_reordering", "fc_rtt" and "fc_rttvar".

Update the documentation consequently.

11 months agoMINOR: tcp_sample: Move TCP low level sample fetch function to control layer
Frederic Lecaille [Tue, 30 Jul 2024 13:21:43 +0000 (15:21 +0200)] 
MINOR: tcp_sample: Move TCP low level sample fetch function to control layer

Add ->get_info() new control layer callback definition to protocol struct to
retreive statiscal counters information at transport layer (TCPv4/TCPv6) identified by
an integer into a long long int.
Move the TCP specific code from get_tcp_info() to the tcp_get_info() control layer
function (src/proto_tcp.c) and define it as  the ->get_info() callback for
TCPv4 and TCPv6.
Note that get_tcp_info() is called for several TCP sample fetches.
This patch is useful to support some of these sample fetches for QUIC and to
keep the code simple and easy to maintain.

11 months agoBUG/MEDIUM: quic: prevent conn freeze on 0RTT undeciphered content
Amaury Denoyelle [Mon, 29 Jul 2024 08:42:50 +0000 (10:42 +0200)] 
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.

11 months agoBUILD: ssl: replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC
William Lallemand [Tue, 30 Jul 2024 13:51:59 +0000 (15:51 +0200)] 
BUILD: ssl: replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC

Replace USE_OPENSSL_AWSLC by OPENSSL_IS_AWSLC in the code source, so we
won't need to set USE_OPENSSL_AWSLC in the Makefile on the long term.

11 months agoBUG/MEDIUM: ssl: 0-RTT initialized at the wrong place for AWS-LC
William Lallemand [Tue, 30 Jul 2024 12:54:44 +0000 (14:54 +0200)] 
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)

11 months agoBUG/MEDIUM: ssl: reactivate 0-RTT for AWS-LC
William Lallemand [Mon, 29 Jul 2024 13:42:47 +0000 (15:42 +0200)] 
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.

11 months agoBUG/MINOR: stconn: bs.id and fs.id had their dependencies incorrect
Willy Tarreau [Tue, 30 Jul 2024 15:54:26 +0000 (17:54 +0200)] 
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>
11 months agoBUILD: mux-pt: Use the right name for the sedesc variable
Christopher Faulet [Tue, 30 Jul 2024 08:43:55 +0000 (10:43 +0200)] 
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.

11 months agoBUG/MEDIUM: mux-pt/mux-h1: Release the pipe on connection error on sending path
Christopher Faulet [Tue, 30 Jul 2024 06:41:03 +0000 (08:41 +0200)] 
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.

11 months agoBUG/MEDIUM: stconn: Report error on SC on send if a previous SE error was set
Christopher Faulet [Mon, 29 Jul 2024 15:48:16 +0000 (17:48 +0200)] 
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.

11 months agoDOC: config: Add documentation about spop mode for backends
Christopher Faulet [Mon, 29 Jul 2024 15:25:37 +0000 (17:25 +0200)] 
DOC: config: Add documentation about spop mode for backends

The SPOE was refactored. Now backends referenced by a SPOE filter must use
the spop mode to be able to use the spop multiplexer for server connections.
The "spop" mode was added in the list of supported mode for backends.