]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoMINOR: resolvers: shut the warning when "default" resolvers is implicit
William Lallemand [Wed, 24 Aug 2022 12:50:32 +0000 (14:50 +0200)] 
MINOR: resolvers: shut the warning when "default" resolvers is implicit

Shut the connect() warning of resolvers_finalize_config() when the
configuration was not emitted manually.

This shuts the warning for the "default" resolvers which is created
automatically for the httpclient.

Must be backported in 2.6.

2 years agoREGTESTS: Fix prometheus script to perform HTTP health-checks
Christopher Faulet [Wed, 24 Aug 2022 10:17:31 +0000 (12:17 +0200)] 
REGTESTS: Fix prometheus script to perform HTTP health-checks

TCP Health-checks are enabled on server "s2". However it expects to receive
an HTTP requests. So HAProxy configuration must be changed to perform HTTP
health-checks instead. Otherwise, depending on the timing, an error can be
triggered if a check is performed before the end of the script.

This scripts never failed because TCP_QUICKACK was disabled, adding some
latency on health-checks. But since the last fix, it is an issue.

This patch should be backported as far as 2.4.

2 years agoBUG/MINOR: tcpcheck: Disable QUICKACK only if data should be sent after connect
Christopher Faulet [Wed, 24 Aug 2022 09:38:03 +0000 (11:38 +0200)] 
BUG/MINOR: tcpcheck: Disable QUICKACK only if data should be sent after connect

It is only a real problem for agent-checks when there is no agent string to
send. The condition to disable TCP_QUICKACK was only based on the action
type following the connect one. But it is not always accurate. indeed, for
agent-checks, there is always a SEND action. But if there is no "agent-send"
string defined, nothing is sent. In this case, this adds 200ms of latency
with no reason.

To fix the bug, a flag is now used on the CONNECT action to instruct there
are data that should be sent after the connect. For health-checks, this flag
is set if the action following the connect is a SEND action. For
agent-checks, it is set if an "agent-send" string is defined.

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

2 years agoBUG/MINOR: mworker: does not create the "default" resolvers in wait mode
William Lallemand [Wed, 24 Aug 2022 09:15:08 +0000 (11:15 +0200)] 
BUG/MINOR: mworker: does not create the "default" resolvers in wait mode

When doing a re-exec, the master was creating a "default" resolvers,
which could result in a warning emitted because the "default" resolvers
of the configuration file is not available anymore.

Skip the creating of the "default" resolvers in wait mode, this is not
useful in the master.

Must be backported as far as 2.6.

2 years agoBUG/MINOR: resolvers: return the correct value in resolvers_finalize_config()
William Lallemand [Wed, 24 Aug 2022 07:58:31 +0000 (09:58 +0200)] 
BUG/MINOR: resolvers: return the correct value in resolvers_finalize_config()

Patch c31577f ("MEDIUM: resolvers: continue startup if network is
unavailable") was not working correctly. Indeed
resolvers_finalize_config() was returning a ERR type, but a postparser
is supposed to return 0 or 1.

The return value was never right, however it was only a problem since c31577f.

Must be backported in every stable branch.

2 years agoBUILD: tcp_sample: fix build of get_tcp_info() on OpenBSD
Brad Smith [Sat, 13 Aug 2022 02:23:13 +0000 (22:23 -0400)] 
BUILD: tcp_sample: fix build of get_tcp_info() on OpenBSD

The build on OpenBSD is broken since commit 5c83e3a15 ("MINOR: tcp_sample:
clarifying samples support per os, for further expansion."), hence it
only affects 2.7 and 2.6.

It looks like this changed things in such a way that if TCP_INFO is added
but the OS is not added to the list of OS's it will not build.

Extend support for get_tcp_info to OpenBSD.

This must be backported to 2.6.

2 years agoMEDIUM: peers: limit the number of updates sent at once
Willy Tarreau [Tue, 19 Jul 2022 18:17:38 +0000 (20:17 +0200)] 
MEDIUM: peers: limit the number of updates sent at once

As seen in GH issue #1770, peers synchronization do not cope well with
very large buffers because by default the only two reasons for stopping
the processing of updates is either that the end was reached or that
the buffer is full. This can cause high latencies, and even rightfully
trigger the watchdog when the operations are numerous and slowed down
by competition on the stick-table lock.

This patch introduces a limit to the number of messages one may send
at once, which now defaults to 200, regardless of the buffer size. This
means taking and releasing the lock up to 400 times in a row, which is
costly enough to let some other parts work.

After some observation this could be backported to 2.6. If so, however,
previous commits "BUG/MEDIUM: applet: fix incorrect check for abnormal
return condition from handler" and "BUG/MINOR: applet: make the call_rate
only count the no-progress calls" must be backported otherwise the call
rate might trigger the looping protection.

2 years agoBUG/MINOR: applet: make the call_rate only count the no-progress calls
Willy Tarreau [Tue, 19 Jul 2022 18:36:15 +0000 (20:36 +0200)] 
BUG/MINOR: applet: make the call_rate only count the no-progress calls

This is very similar to what we did in commit 6c539c4b8 ("BUG/MINOR:
stream: make the call_rate only count the no-progress calls"), it's
better to only count the call rate with no progress than to count all
calls and try to figure if there's no progress, because a fast running
applet might once satisfy the whole condition and trigger the bug. This
typically happens when artificially limiting the number of messages sent
at once by an applet, but could happen with plenty of highly interactive
applets.

This patch could be backported to stable versions if there are any
indications that it might be useful there.

2 years agoBUG/MEDIUM: applet: fix incorrect check for abnormal return condition from handler
Willy Tarreau [Tue, 23 Aug 2022 07:01:30 +0000 (09:01 +0200)] 
BUG/MEDIUM: applet: fix incorrect check for abnormal return condition from handler

We have quite numerous checks for abnormal applet handler behavior which
are supposed to trigger the loop protection. However, consecutive to
commit 15252cd9c ("MEDIUM: stconn: move the RXBLK flags to the stream
connector") that was merged into 2.6-dev12, one flag was incorrectly
renamed, and the check for an applet waiting for a buffer that is present
mistakenly turned to a check for missing room in the buffer. This erroneous
test could mistakenly trigger on applets that perform intensive I/Os doing
small exchanges each (e.g. cache, peers or HTTP client) if the load would
be sustained (>100k iops). For the cache this could represent higher than
13 Gbps on an object at least 1.6 GB large for example, which is quite
unlikely but theoretically possible.

This fix needs to be backported to 2.6.

2 years agoMINOR: quic: Replace MT_LISTs by LISTs for RX packets.
Frédéric Lécaille [Tue, 23 Aug 2022 15:45:52 +0000 (17:45 +0200)] 
MINOR: quic: Replace MT_LISTs by LISTs for RX packets.

Replace ->rx.pqpkts quic_enc_level struct member MT_LIST by an LIST.
Same thing for ->list quic_rx_packet struct member MT_LIST.
Update the code consequently. This was a reminisence of the multithreading
support (several threads by connection).

Must be backported to 2.6

2 years agoBUG/MINOR: quic: Safer QUIC frame builders
Frédéric Lécaille [Tue, 23 Aug 2022 15:40:09 +0000 (17:40 +0200)] 
BUG/MINOR: quic: Safer QUIC frame builders

Do not rely on the fact the callers of qc_build_frm() handle their
buffer passed to function the correct way (without leaving garbage).
Make qc_build_frm() update the buffer passed as argument only if
the frame it builds is well formed.

As far as I sse, there is no such callers which does not handle
carefully such buffers.

Must be backported to 2.6.

2 years agoBUG/MINOR: quic: Wrong list_for_each_entry() use when building packets from qc_do_bui...
Frédéric Lécaille [Tue, 23 Aug 2022 09:42:48 +0000 (11:42 +0200)] 
BUG/MINOR: quic: Wrong list_for_each_entry() use when building packets from qc_do_build_pkt()

This is list_for_each_entry_safe() which must be used if we want to delete elements
inside its code block. This could explain that some frames which were not built were added
to packets with a NULL ->pkt member.

Thank you to Tristan for having reported this issue through backtraces in GH #1808

Must be backported to 2.6.

2 years agoBUG/MINOR: quix: Memleak for non in flight TX packets
Frédéric Lécaille [Mon, 22 Aug 2022 16:47:51 +0000 (18:47 +0200)] 
BUG/MINOR: quix: Memleak for non in flight TX packets

First, these packets must not be inserted in the tree of TX packets.
They are never explicitely acknowledged (for instance an ACK only
packet will never be acknowledged). Furthermore, if taken into an account
these packets may uselessly disturb the congestion control. We do not care
if they are lost or not. Furthermore as the ->in_fligh_len member value is null
they were not released by qc_release_lost_pkts() which rely on these values
to decide to release the allocated memory for such packets.

Must be backported to 2.6.

2 years agoREGTESTS: launch http_reuse_always in mworker mode
William Lallemand [Mon, 22 Aug 2022 10:59:50 +0000 (12:59 +0200)] 
REGTESTS: launch http_reuse_always in mworker mode

We don't have enough tests with the mworker mode, and even less that
have no master CLI (-S) configured. Let's run this one with -W, it
shouldn't have any impact.

VTest won't be able to catch a lot of things for now, but that's a first
step.

2 years agoBUG/MAJOR: mworker: fix infinite loop on master with no proxies.
Emeric Brun [Mon, 22 Aug 2022 08:25:11 +0000 (10:25 +0200)] 
BUG/MAJOR: mworker: fix infinite loop on master with no proxies.

The master is re-exec with an empty proxies list if no master CLI is
configured.

This results in infinite loop since last patch:
3b68b602 ("BUG/MAJOR: log-forward: Fix log-forward proxies not fully initialized")

This patch avoid to loop again on log-forward proxies list if empty.

This patch should be backported until v2.3

2 years agoMINOR: cpu-map: remove obsolete diag warning about combined ranges
Willy Tarreau [Mon, 22 Aug 2022 08:46:13 +0000 (10:46 +0200)] 
MINOR: cpu-map: remove obsolete diag warning about combined ranges

We used to emit a diag warning in case ranges were used both with the
process and thread part of a thread spec. Now with groups it's not
longer a problem, so let's just kill this warning.

2 years agoBUG/MEDIUM: cpu-map: fix thread 1's affinity affecting all threads
Willy Tarreau [Mon, 22 Aug 2022 08:38:00 +0000 (10:38 +0200)] 
BUG/MEDIUM: cpu-map: fix thread 1's affinity affecting all threads

Since 2.7-dev2 with commit 5b09341c02 ("MEDIUM: cpu-map: replace the
process number with the thread group number"), the thread group has
replaced the process number in the "cpu-map" directive. In part due to
a design limit in 2.4 and 2.5, a special case was made of thread 1 in
commit bda7c1decd ("MEDIUM: config: simplify cpu-map handling"), because
there was no other location to store a single-threaded setup's mask by
then. The combination of the two resulted in a problem with thread
groups, by which as soon as one line exhibiting thread number 1 alone
was found in a config, the mask would be applied to all threads in the
group.

The loop was reworked to avoid this obsolete special case, and was
factored for better legibility. One obsolete comment about nbproc
was also removed. No backport is needed.

2 years agoBUG/MINOR: mux-quic: Fix memleak on QUIC stream buffer for unacknowledged data
Frédéric Lécaille [Sat, 20 Aug 2022 16:59:36 +0000 (18:59 +0200)] 
BUG/MINOR: mux-quic: Fix memleak on QUIC stream buffer for unacknowledged data

Some clients send CONNECTION_CLOSE frame without acknowledging the STREAM
data haproxy has sent. In this case, when closing the connection if
there were remaining data in QUIC stream buffers, they were not released.

Add a <closing> boolean option to qc_stream_desc_free() to force the
stream buffer memory releasing upon closing connection.

Thank you to Tristan for having reported such a memory leak issue in GH #1801.

Must be backported to 2.6.

2 years ago[RELEASE] Released version 2.7-dev4 v2.7-dev4
Willy Tarreau [Sat, 20 Aug 2022 13:56:31 +0000 (15:56 +0200)] 
[RELEASE] Released version 2.7-dev4

Released version 2.7-dev4 with the following main changes :
    - BUG/MEDIUM: quic: Wrong packet length check in qc_do_rm_hp()
    - MINOR: quic: Too much useless traces in qc_build_frms()
    - BUG/MEDIUM: quic: Missing AEAD TAG check after removing header protection
    - MINOR: quic: Replace pool_zalloc() by pool_malloc() for fake datagrams
    - MINOR: debug: make the mem_stats section aligned to void*
    - MINOR: debug: store and report the pool's name in struct mem_stats
    - MINOR: debug: also store the function name in struct mem_stats
    - MINOR: debug/memstats: automatically determine first column size
    - MINOR: debug/memstats: permit to pass the size to free()
    - CLEANUP: mux-quic: remove loop on sending frames
    - MINOR: quic: replace custom buf on Tx by default struct buffer
    - MINOR: quic: release Tx buffer on each send
    - MINOR: quic: refactor datagram commit in Tx buffer
    - MINOR: quic: skip sending if no frame to send in io-cb
    - BUG/MINOR: mux-quic: open stream on STOP_SENDING
    - BUG/MINOR: quic: fix crash on handshake io-cb for null next enc level
    - BUG/MEDIUM: quic: always remove the connection from the accept list on close
    - BUG/MEDIUM: poller: use fd_delete() to release the poller pipes
    - BUG/MEDIUM: task: relax one thread consistency check in task_unlink_wq()
    - MEDIUM: quic: xprt traces rework
    - BUILD: stconn: fix build warning at -O3 about possible null sc
    - MINOR: quic: Remove useless lock for RX packets
    - BUG/MINOR: quic: Possible infinite loop in quic_build_post_handshake_frames()
    - CLEANUP: quic: Remove trailing spaces
    - MINOR: mux-quic: adjust enter/leave traces
    - MINOR: mux-quic: define protocol error traces
    - CLEANUP: mux-quic: adjust traces level
    - MINOR: mux-quic: define new traces
    - BUG/MEDIUM: mux-quic: fix crash due to invalid trace arg
    - BUG/MEDIUM: quic: Possible use of uninitialized <odcid> variable in qc_lstnr_params_init()
    - BUG/MEDIUM: ring: fix too lax 'size' parser
    - BUG/MEDIUM: quic: Wrong use of <token_odcid> in qc_lsntr_pkt_rcv()
    - BUILD: ring: forward-declare struct appctx to avoid a build warning
    - MINOR: ring: support creating a ring from a linear area
    - MINOR: ring: add support for a backing-file
    - DEV: haring: add a simple utility to read file-backed rings
    - DEV: haring: support remapping LF in contents with CR VT
    - BUG/MINOR: quic: memleak on wrong datagram receipt
    - BUILD: sink: replace S_IRUSR, S_IWUSR with their octal value
    - MINOR: ring: archive a previous file-backed ring on startup
    - BUG/MINOR: mux-quic: fix crash with traces in qc_detach()
    - BUG/MINOR: quic: MIssing check when building TX packets
    - BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()
    - MINOR: memprof: export the minimum definitions for memory profiling
    - MINOR: pool/memprof: report pool alloc/free in memory profiling
    - MINOR: pools/memprof: store and report the pool's name in each bin
    - MINOR: chunk: inline alloc_trash_chunk()
    - MINOR: stick-table: Add table_expire() and table_idle() new converters
    - CLEANUP: exclude haring with .gitignore
    - MINOR: quic: adjust quic_frame flag manipulation
    - MINOR: h3: report error on control stream close
    - MINOR: qpack: report error on enc/dec stream close
    - BUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control
    - MINOR: mux-quic: adjust traces on stream init
    - MINOR: mux-quic: add missing args on some traces
    - MINOR: quic: refactor application send
    - BUG/MINOR: quic: do not notify MUX on frame retransmit
    - BUG/MEDIUM: http-ana: fix crash or wrong header deletion by http-restrict-req-hdr-names
    - BUG/MINOR: quic: Missing initializations for ducplicated frames.
    - BUG/MEDIUM: quic: fix crash on MUX send notification
    - REORG: h2: extract cookies concat function in http_htx
    - REGTESTS: add test for HTTP/2 cookies concatenation
    - MEDIUM: h3: concatenate multiple cookie headers
    - MINOR: applet: add a function to reset the svcctx of an applet
    - BUG/MEDIUM: cli: always reset the service context between commands
    - BUG/MEDIUM: mux-h2: do not fiddle with ->dsi to indicate demux is idle
    - MINOR: mux-h2/traces: report transition to SETTINGS1 before not after
    - MINOR: mux-h2: make streams know if they need to send more data
    - BUG/MINOR: mux-h2: send a CANCEL instead of ES on truncated writes
    - BUG/MINOR: quic: Possible crashes when dereferencing ->pkt quic_frame struct member
    - MINOR: quic: Add frame addresses to QUIC_EV_CONN_PRSAFRM event traces
    - BUG/MINOR: quic: Wrong splitted duplicated frames handling
    - MINOR: quic: Add the QUIC connection to mux traces
    - MINOR: quic: Trace fix in qc_release_frm()
    - BUG/MAJOR: log-forward: Fix log-forward proxies not fully initialized
    - BUG/MAJOR: log-forward: Fix ssl layer not initialized on bind even if configured
    - MINOR: quic: Add reusable cipher contexts for header protection
    - BUG/MINOR: ssl/cli: error when the ca-file is empty
    - MINOR: ssl: handle ca-file appending in cafile_entry
    - MINOR: ssl/cli: implement "add ssl ca-file"

2 years agoMINOR: ssl/cli: implement "add ssl ca-file"
William Lallemand [Fri, 29 Jul 2022 15:50:58 +0000 (17:50 +0200)] 
MINOR: ssl/cli: implement "add ssl ca-file"

In ticket #1805 an user is impacted by the limitation of size of the CLI
buffer when updating a ca-file.

This patch allows a user to append new certificates to a ca-file instead
of trying to put them all with "set ssl ca-file"

The implementation use a new function ssl_store_dup_cafile_entry() which
duplicates a cafile_entry and its X509_STORE.

ssl_store_load_ca_from_buf() was modified to take an apped parameter so
we could share the function for "set" and "add".

2 years agoMINOR: ssl: handle ca-file appending in cafile_entry
William Lallemand [Fri, 29 Jul 2022 15:08:02 +0000 (17:08 +0200)] 
MINOR: ssl: handle ca-file appending in cafile_entry

In order to be able to append new CA in a cafile_entry,
ssl_store_load_ca_from_buf() was reworked and a "append" parameter was
added.

The function is able to keep the previous X509_STORE which was already
present in the cafile_entry.

2 years agoBUG/MINOR: ssl/cli: error when the ca-file is empty
William Lallemand [Thu, 18 Aug 2022 13:53:02 +0000 (15:53 +0200)] 
BUG/MINOR: ssl/cli: error when the ca-file is empty

"set ssl ca-file" does not return any error when a ca-file is empty or
only contains comments. This could be a problem is the file was
malformated and did not contain any PEM header.

It must be backported as far as 2.5.

2 years agoMINOR: quic: Add reusable cipher contexts for header protection
Frédéric Lécaille [Fri, 19 Aug 2022 16:18:13 +0000 (18:18 +0200)] 
MINOR: quic: Add reusable cipher contexts for header protection

Implement quic_tls_rx_hp_ctx_init() and quic_tls_tx_hp_ctx_init() to initiliaze
such header protection cipher contexts for each RX and TX parts and for each
packet number spaces, only one time by connection.
Make qc_new_isecs() call these two functions to initialize the cipher contexts
of the Initial secrets. Same thing for ha_quic_set_encryption_secrets() to
initialize the cipher contexts of the subsequent derived secrets (ORTT, 1RTT,
Handshake).
Modify qc_do_rm_hp() and quic_apply_header_protection() to reuse these
cipher contexts.
Note that there is no need to modify the key update for the header protection.
The header protection secrets are never updated.

2 years agoBUG/MAJOR: log-forward: Fix ssl layer not initialized on bind even if configured
Emeric Brun [Fri, 19 Aug 2022 13:16:26 +0000 (15:16 +0200)] 
BUG/MAJOR: log-forward: Fix ssl layer not initialized on bind even if configured

Since commit 2071a99df ("MINOR: listener/ssl: set the SSL xprt layer only
once the whole config is known") the xprt is initialized for ssl directly
from a generic funtion used to parse bind args.

But the 'bind' lines from 'log-forward' sections were forgotten in commit
55f0f7bb5 ("MINOR: config: use the new bind_parse_args_list() to parse a
"bind" line").

This patch re-works 'log-forward' section parsing to use the generic
function to parse bind args and fix the issue.

Since the generic way to parse was introduced in 2.6, this patch
should be backported as far as this version.

2 years agoBUG/MAJOR: log-forward: Fix log-forward proxies not fully initialized
Emeric Brun [Thu, 18 Aug 2022 13:53:21 +0000 (15:53 +0200)] 
BUG/MAJOR: log-forward: Fix log-forward proxies not fully initialized

Some initialisation for log forward proxies was missing such
as ssl configuration on 'log-forward's 'bind' lines.

After the loop on the proxy initialization code for proxies present
in the main proxies list, this patch force to loop again on this code
for proxies present in the log forward proxies list.

Those two lists should be merged. This will be part of a global
re-work of proxy initialization including peers proxies and resolver
proxies.

This patch was made in first attempt to fix the bug and to facilitate
the backport on older branches waiting for a cleaner re-work on proxies
initialization on the dev branch.

This patch should be backported as far as 2.3.

2 years agoMINOR: quic: Trace fix in qc_release_frm()
Frédéric Lécaille [Fri, 19 Aug 2022 10:08:13 +0000 (12:08 +0200)] 
MINOR: quic: Trace fix in qc_release_frm()

This wrong trace came with this commit:
  "BUG/MINOR: quic: Possible crashes when dereferencing ->pkt quic_frame struct member"
In qc_release_frm() we mark frames as acked. Nothing to see with references
to frames.

Thank you to Willy for having caught this one.

Must be backported to 2.6 as these traces arrived with a bug fix to be backported
to 2.6.

2 years agoMINOR: quic: Add the QUIC connection to mux traces
Frédéric Lécaille [Fri, 19 Aug 2022 10:02:29 +0000 (12:02 +0200)] 
MINOR: quic: Add the QUIC connection to mux traces

This should help for debugging purpose.

Should be backported to 2.6

2 years agoBUG/MINOR: quic: Wrong splitted duplicated frames handling
Frédéric Lécaille [Fri, 19 Aug 2022 07:32:14 +0000 (09:32 +0200)] 
BUG/MINOR: quic: Wrong splitted duplicated frames handling

When duplicated frames are splitted, we must propagate this information
to the new allocated frame and add a reference to this new frame
to the reference list of the original frame.

Must be backported to 2.6

2 years agoMINOR: quic: Add frame addresses to QUIC_EV_CONN_PRSAFRM event traces
Frédéric Lécaille [Fri, 19 Aug 2022 07:23:15 +0000 (09:23 +0200)] 
MINOR: quic: Add frame addresses to QUIC_EV_CONN_PRSAFRM event traces

This should be useful to diagnose some issues.

Should be backported to 2.6.

2 years agoBUG/MINOR: quic: Possible crashes when dereferencing ->pkt quic_frame struct member
Frédéric Lécaille [Thu, 18 Aug 2022 15:21:19 +0000 (17:21 +0200)] 
BUG/MINOR: quic: Possible crashes when dereferencing ->pkt quic_frame struct member

This was done at several places. First in qc_requeue_nacked_pkt_tx_frms.
This aim of this function is, if needed, to requeue all the TX frames of a lost
<pkt> packet passed as argument and detach them from this packet they have been
sent from. They are possible cases where the frm->pkt quic_frame struct member could
be NULL, as a result of a duplication of an original frame by qc_dup_pkt_frms(). This
function adds the duplicated frame to the original frame reference list:
        LIST_APPEND(&origin->reflist, &dup_frm->ref);
But, in this function, the packet which contains the frame is the one which is passed
as argument (for debug purpose). So let us prefer using this variable.
Also do not dereference this ->pkt quic_frame member in qc_release_frm() and
qc_frm_unref() and add a trace to catch the frame with a null ->pkt member.
They are logically frames which have not already been sent.

Thank you to Tristan for having reported such crashes in GH #1808.

Must be backported to 2.6

2 years agoBUG/MINOR: mux-h2: send a CANCEL instead of ES on truncated writes
Willy Tarreau [Thu, 18 Aug 2022 14:12:15 +0000 (16:12 +0200)] 
BUG/MINOR: mux-h2: send a CANCEL instead of ES on truncated writes

If a POST upload is cancelled after having advertised a content-length,
or a response body is truncated after a content-length, we're not allowed
to send ES because in this case the total body length must exactly match
the advertised value. Till now that's what we were doing, and that was
causing the other side (possibly haproxy) to respond with an RST_STREAM
PROTOCOL_ERROR due to "ES on DATA frame before content-length".

We can behave a bit cleaner here. Let's detect that we haven't sent
everything, and send an RST_STREAM(CANCEL) instead, which is designed
exactly for this purpose.

This patch could be backported to older versions but only a little bit
of exposure to make sure it doesn't wake up a bad behavior somewhere.
It relies on the following previous commit:

   "MINOR: mux-h2: make streams know if they need to send more data"

2 years agoMINOR: mux-h2: make streams know if they need to send more data
Willy Tarreau [Thu, 18 Aug 2022 14:03:51 +0000 (16:03 +0200)] 
MINOR: mux-h2: make streams know if they need to send more data

H2 streams do not even know if they are expected to send more data or
not, which is problematic when closing because we don't know if we're
closing too early or not. Let's start by adding a new stream flag
"H2_SF_MORE_HTX_DATA" to indicate this on the tx path.

2 years agoMINOR: mux-h2/traces: report transition to SETTINGS1 before not after
Willy Tarreau [Thu, 18 Aug 2022 13:30:41 +0000 (15:30 +0200)] 
MINOR: mux-h2/traces: report transition to SETTINGS1 before not after

Traces indicating "switching to XXX" generally apply before the transition
so that the current connection state is visible in the trace. SETTINGS1
was incorrect in this regard, with the trace being emitted after. Let's
fix this.

No need to backport this, as this is purely cosmetic.

2 years agoBUG/MEDIUM: mux-h2: do not fiddle with ->dsi to indicate demux is idle
Willy Tarreau [Thu, 18 Aug 2022 09:19:57 +0000 (11:19 +0200)] 
BUG/MEDIUM: mux-h2: do not fiddle with ->dsi to indicate demux is idle

When switching to H2_CS_FRAME_H, we do not want to present the previous
frame's state, flags, length etc in traces, or we risk to confuse the
analysis, making the reader think that the header information presented
is related to the new frame header being analysed. A naive approach could
have consisted in simply relying on the current parser state (FRAME_H
being that state), but traces are emitted before switching the state,
so traces cannot rely on this.

This was initially addressed by commit 73db434f7 ("MINOR: h2/trace: report
the frame type when known") which used to set dsi to -1 when the connection
becomes idle again, but was accidentally broken by commit 5112a603d
("BUG/MAJOR: mux_h2: Don't consume more payload than received for skipped
frames") which moved dsi after calling the trace function.

But in both cases there's problem with this approach. If an RST or WU frame
cannot be uploaded due to a busy mux, and at the same time we complete
processing on a perfect end of frame with no single new frame header, we
can leave the demux loop with dsi=-1 and with RST or WU to be sent, and
these ones will be sent for stream ID -1. This is what was reported in
github issue #1830. This can be reproduced with a config chaining an h1->h2
proxy to an empty h2 frontend, and uploading a large body such as below:

  $ (printf "POST / HTTP/1.1\r\nContent-length: 1000000000\r\n\r\n";
     cat /dev/zero) |  nc 0 4445 > /dev/null

This shows that we must never affect ->dsi which must always remain valid,
and instead we should set "something else". That something else could be
served by the demux frame type, but that one also needs to be preserved
for the RST_STREAM case. Instead, let's just add a connection flag to say
that the demuxing is in progress. This will be set once a new demux header
is set and reset after the end of a frame. This way the trace subsystem
can know that dft/dfl must not be displayed, without affecting the logic
relying on such values.

Given that the commits above are old and were backported to 1.8, this
new one also needs to be backported as far as 1.8.

Many thanks to David le Blanc (@systemmonkey42) for spotting, reporting,
capturing and analyzing this bug; his work permitted to quickly spot the
problem.

2 years agoBUG/MEDIUM: cli: always reset the service context between commands
Willy Tarreau [Thu, 18 Aug 2022 16:04:37 +0000 (18:04 +0200)] 
BUG/MEDIUM: cli: always reset the service context between commands

Erwan Le Goas reported that chaining certain commands on the CLI would
systematically crash the process; for example, "show version; show sess".
This happened since the conversion of cli context to appctx->svcctx,
because if applet_reserve_svcctx() is called a first time for a tiny
context, it's allocated in-situ, and later a keyword that wants a
larger one will see that it's not null and will reuse it and will
overwrite the end of the first one's context.

What is missing is a reset of the svcctx when looping back to
CLI_ST_GETREQ.

This needs to be backported to 2.6, and relies on previous commit
"MINOR: applet: add a function to reset the svcctx of an applet".

2 years agoMINOR: applet: add a function to reset the svcctx of an applet
Willy Tarreau [Thu, 18 Aug 2022 16:13:34 +0000 (18:13 +0200)] 
MINOR: applet: add a function to reset the svcctx of an applet

The CLI needs to reset the svcctx between commands, and there was nothing
done to handle this. Let's add appctx_reset_svcctx() to do that, it's the
closing equivalent of appctx_reserve_svcctx().

This will have to be backported to 2.6 as it will be used by a subsequent
patch to fix a bug.

2 years agoMEDIUM: h3: concatenate multiple cookie headers
Amaury Denoyelle [Wed, 17 Aug 2022 16:02:47 +0000 (18:02 +0200)] 
MEDIUM: h3: concatenate multiple cookie headers

As specified by RFC 9114, multiple cookie headers must be concatenated
into a single entry before passing it to a HTTP/1.1 connection. To
implement this, reuse the same function as already used for HTTP/2
module.

This should answer to feature requested in github issue #1818.

2 years agoREGTESTS: add test for HTTP/2 cookies concatenation
Amaury Denoyelle [Wed, 17 Aug 2022 14:34:13 +0000 (16:34 +0200)] 
REGTESTS: add test for HTTP/2 cookies concatenation

Write a regtest to test RFC 7540 compliance in regards to multiple
cookie headers concatenation.

2 years agoREORG: h2: extract cookies concat function in http_htx
Amaury Denoyelle [Wed, 17 Aug 2022 14:33:53 +0000 (16:33 +0200)] 
REORG: h2: extract cookies concat function in http_htx

As specified by RFC 7540, multiple cookie headers are merged in a single
entry before passing it to a HTTP/1.1 connection. This step is
implemented during headers parsing in h2 module.

Extract this code in the generic http_htx module. This will allow to
reuse it quickly for HTTP/3 implementation which has the same
requirement for cookie headers.

2 years agoBUG/MEDIUM: quic: fix crash on MUX send notification
Amaury Denoyelle [Wed, 17 Aug 2022 14:33:13 +0000 (16:33 +0200)] 
BUG/MEDIUM: quic: fix crash on MUX send notification

MUX notification on TX has been edited recently : it will be notified
only when sending its own data, and not for example on retransmission by
the quic-conn layer. This is subject of the patch :
  b29a1dc2f4a334c1c7fea76c59abb4097422c05c
  BUG/MINOR: quic: do not notify MUX on frame retransmit

A new flag QUIC_FL_CONN_RETRANS_LOST_DATA has been introduced to
differentiate qc_send_app_pkts invocation by MUX and directly by the
quic-conn layer in quic_conn_app_io_cb(). However, this is a first
problem as internal quic-conn layer usage is not limited to
retransmission. For example for NEW_CONNECTION_ID emission.

Another problem much important is that send functions are also called
through quic_conn_io_cb() which has not been protected from MUX
notification. This could probably result in crash when trying to notify
the MUX.

To fix both problems, quic-conn flagging has been inverted : when used
by the MUX, quic-conn is flagged with QUIC_FL_CONN_TX_MUX_CONTEXT. To
improve the API, MUX must now used qc_send_mux which ensure the flag is
set. qc_send_app_pkts is now static and can only be used by the
quic-conn layer.

This must be backported wherever the previously mentionned patch is.

2 years agoBUG/MINOR: quic: Missing initializations for ducplicated frames.
Frédéric Lécaille [Thu, 18 Aug 2022 06:20:47 +0000 (08:20 +0200)] 
BUG/MINOR: quic: Missing initializations for ducplicated frames.

When duplication frames in qc_dup_pkt_frms(), ->pkt member was not correctly
initialized (copied from the original frame). This could not have any impact
because this member is initialized whe the frame is added to a packet.
This was also the case for ->flags.
Also replace the pool_zalloc() call by a call to pool_alloc().

Must be backported to 2.6.

2 years agoBUG/MEDIUM: http-ana: fix crash or wrong header deletion by http-restrict-req-hdr...
Mateusz Malek [Wed, 17 Aug 2022 12:22:09 +0000 (14:22 +0200)] 
BUG/MEDIUM: http-ana: fix crash or wrong header deletion by http-restrict-req-hdr-names

When using `option http-restrict-req-hdr-names delete`, HAproxy may
crash or delete wrong header after receiving request containing multiple
forbidden characters in single header name; exact behavior depends on
number of request headers, number of forbidden characters and position
of header containing them.

This patch fixes GitHub issue #1822.

Must be backported as far as 2.2 (buggy feature got included in 2.2.25,
2.4.18 and 2.5.8).

2 years agoBUG/MINOR: quic: do not notify MUX on frame retransmit
Amaury Denoyelle [Fri, 12 Aug 2022 12:30:04 +0000 (14:30 +0200)] 
BUG/MINOR: quic: do not notify MUX on frame retransmit

On STREAM emission, quic-conn notifies MUX through a callback named
qcc_streams_sent_done(). This also happens on retransmission : in this
case offset are examined and notification is ignored if already seen.

However, this behavior has slightly changed since
  e53b489826ba9760a527b461095402ca05d2b6be
  BUG/MEDIUM: mux-quic: fix server chunked encoding response

Indeed, if offset diff is NULL, frame is now not ignored. This is to
support FIN notification with a final empty STREAM frame. A side-effect
of this is that if the last stream frame is retransmitted, it won't be
ignored in qcc_streams_sent_done().

In most cases, this side-effect is harmless as qcs instance will soon be
freed after being closed. But if qcs is still alive, this will cause a
BUG_ON crash as it is considered as locally closed.

This bug depends on delay condition and seems to be extremely rare. But
it might be the reason for a crash seen on interop with s2n client on
http3 testcase :

FATAL: bug condition "qcs->st == QC_SS_CLO" matched at src/mux_quic.c:372
  call trace(16):
  | 0x558228912b0d [b8 01 00 00 00 c6 00 00]: main-0x1c7878
  | 0x558228917a70 [48 8b 55 d8 48 8b 45 e0]: qcc_streams_sent_done+0xcf/0x355
  | 0x558228906ff1 [e9 29 05 00 00 48 8b 05]: main-0x1d3394
  | 0x558228907cd9 [48 83 c4 10 85 c0 0f 85]: main-0x1d26ac
  | 0x5582289089c1 [48 83 c4 50 85 c0 75 12]: main-0x1d19c4
  | 0x5582288f8d2a [48 83 c4 40 48 89 45 a0]: main-0x1e165b
  | 0x5582288fc4cc [89 45 b4 83 7d b4 ff 74]: qc_send_app_pkts+0xc6/0x1f0
  | 0x5582288fd311 [85 c0 74 12 eb 01 90 48]: main-0x1dd074
  | 0x558228b2e4c1 [48 c7 c0 d0 60 ff ff 64]: run_tasks_from_lists+0x4e6/0x98e
  | 0x558228b2f13f [8b 55 80 29 c2 89 d0 89]: process_runnable_tasks+0x7d6/0x84c
  | 0x558228ad9aa9 [8b 05 75 16 4b 00 83 f8]: run_poll_loop+0x80/0x48c
  | 0x558228ada12f [48 8b 05 aa c5 20 00 48]: main-0x256
  | 0x7ff01ed2e609 [64 48 89 04 25 30 06 00]: libpthread:+0x8609
  | 0x7ff01e8ca163 [48 89 c7 b8 3c 00 00 00]: libc:clone+0x43/0x5e

To reproduce it locally, code was artificially patched to produce
retransmission and avoid qcs liberation.

In order to fix this and avoid future class of similar problem, the best
way is to not call qcc_streams_sent_done() to notify MUX for
retranmission. To implement this, we test if any of
QUIC_FL_CONN_RETRANS_OLD_DATA or the new flag
QUIC_FL_CONN_RETRANS_LOST_DATA is set. A new wrapper
qc_send_app_retransmit() has been added to set the new flag as a
complement to already existing qc_send_app_probing().

This must be backported up to 2.6.

2 years agoMINOR: quic: refactor application send
Amaury Denoyelle [Wed, 17 Aug 2022 08:08:16 +0000 (10:08 +0200)] 
MINOR: quic: refactor application send

Adjust qc_send_app_pkts function : remove <old_data> arg and provide a
new wrapper function qc_send_app_probing() which should be used instead
when probing with old data.

This simplifies the interface of the default function, most notably for
the MUX which does not interfer with retransmission.
QUIC_FL_CONN_RETRANS_OLD_DATA flag is set/unset directly in the wrapper
qc_send_app_probing().

At the same time, function documentation has been updated to clarified
arguments and return values.

This commit will be useful for the next patch to differentiate MUX and
retransmission send context. As a consequence, the current patch should
be backported wherever the next one will be.

2 years agoMINOR: mux-quic: add missing args on some traces
Amaury Denoyelle [Thu, 11 Aug 2022 16:35:55 +0000 (18:35 +0200)] 
MINOR: mux-quic: add missing args on some traces

Complete some MUX traces by adding qcc or qcs instance as arguments when
this is possible. This will be useful when several connections are
interleaved.

2 years agoMINOR: mux-quic: adjust traces on stream init
Amaury Denoyelle [Tue, 16 Aug 2022 09:13:45 +0000 (11:13 +0200)] 
MINOR: mux-quic: adjust traces on stream init

Adjust traces on qcc_init_stream_remote() : replace "opening" by
"initializing" to avoid confusion with traces dealing with OPEN stream
state.

2 years agoBUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control
Amaury Denoyelle [Tue, 16 Aug 2022 09:29:08 +0000 (11:29 +0200)] 
BUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control

Emit STREAM_LIMIT_ERROR if a client tries to open an unidirectional
stream with an ID greater than the value specified by our flow-control
limit. The code is similar to the bidirectional stream opening.

MAX_STREAMS_UNI emission is not implement for the moment and is left as
a TODO. This should not be too urgent for the moment : in HTTP/3, a
client has only a limited use for unidirectional streams (H3 control
stream + 2 QPACK streams). This is covered by the value provided by
haproxy in transport parameters.

This patch has been tagged with BUG as it should have prevented last
crash reported on github issue #1808 when opening a new unidirectional
streams with an invalid ID. However, it is probably not the main cause
of the bug contrary to the patch
  commit 11a6f4007b908b49ecd3abd5cd10fba177f07c11
  BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()

This must be backported up to 2.6.

2 years agoMINOR: qpack: report error on enc/dec stream close
Amaury Denoyelle [Tue, 16 Aug 2022 15:42:47 +0000 (17:42 +0200)] 
MINOR: qpack: report error on enc/dec stream close

As specified by RFC 9204, encoder and decoder streams must not be
closed. If the peer behaves incorrectly and closes one of them, emit a
H3_CLOSED_CRITICAL_STREAM connection error.

To implement this, QPACK stream decoding API has been slightly adjusted.
Firstly, fin parameter is passed to notify about FIN STREAM bit.
Secondly, qcs instance is passed via unused void* context. This allows
to use qcc_emit_cc_app() function to report a CONNECTION_CLOSE error.

2 years agoMINOR: h3: report error on control stream close
Amaury Denoyelle [Tue, 16 Aug 2022 15:16:47 +0000 (17:16 +0200)] 
MINOR: h3: report error on control stream close

As specified by RFC 9114 the control stream must not be closed. If the
peer behaves incorrectly and closes it, emit a H3_CLOSED_CRITICAL_STREAM
connection error.

2 years agoMINOR: quic: adjust quic_frame flag manipulation
Amaury Denoyelle [Tue, 16 Aug 2022 12:41:57 +0000 (14:41 +0200)] 
MINOR: quic: adjust quic_frame flag manipulation

Replace a plain '=' operator by '|=' when setting quic_frame
QUIC_FL_TX_FRAME_LOST flag.

For the moment, this change has no impact as only two exclusive flags
are defined for quic_frame. On the edited code path we are certain that
QUIC_FL_TX_FRAME_ACKED is not set due to a previous if statement, so a
plain equal or a binary OR is strictly identical.

This change will be useful if new flags are defined for quic_frame in
the future. These new flags won't be resetted automatically thanks to
binary OR without explictly intended, which otherwise could easily lead
to new bugs.

2 years agoCLEANUP: exclude haring with .gitignore
Amaury Denoyelle [Tue, 16 Aug 2022 09:13:59 +0000 (11:13 +0200)] 
CLEANUP: exclude haring with .gitignore

haring is a new utility to decode file-backed rings. It is compiled in
dev/ directory and so the binary should be specified in .gitignore to
not clutter git status output.

2 years agoMINOR: stick-table: Add table_expire() and table_idle() new converters
Frédéric Lécaille [Tue, 16 Aug 2022 16:11:25 +0000 (18:11 +0200)] 
MINOR: stick-table: Add table_expire() and table_idle() new converters

table_expire() returns the expiration delay for a stick-table entry associated
to an input sample. Its counterpart table_idle() returns the time the entry
remained idle since the last time it was updated.
Both converters may take a default value as second argument which is returned
when the entry is not present.

2 years agoMINOR: chunk: inline alloc_trash_chunk()
Willy Tarreau [Wed, 17 Aug 2022 08:45:22 +0000 (10:45 +0200)] 
MINOR: chunk: inline alloc_trash_chunk()

This function is responsible for all calls to pool_alloc(trash), whose
total size can be huge. As such it's quite a pain that it doesn't provide
more hints about its users. However, since the function is tiny, it fully
makes sense to inline it, the code is less than 0.1% larger with this.
This way we can now detect where the callers are via "show profiling",
e.g.:

       0 1953671           0 32071463136| 0x59960f main+0x10676f p_free(-16416) [pool=trash]
       0       1           0       16416| 0x59960f main+0x10676f p_free(-16416) [pool=trash]
 1953672       0 32071479552           0| 0x599561 main+0x1066c1 p_alloc(16416) [pool=trash]
       0  976835           0 16035723360| 0x576ca7 http_reply_to_htx+0x447/0x920 p_free(-16416) [pool=trash]
       0       1           0       16416| 0x576ca7 http_reply_to_htx+0x447/0x920 p_free(-16416) [pool=trash]
  976835       0 16035723360           0| 0x576a5d http_reply_to_htx+0x1fd/0x920 p_alloc(16416) [pool=trash]
       1       0       16416           0| 0x576a5d http_reply_to_htx+0x1fd/0x920 p_alloc(16416) [pool=trash]

2 years agoMINOR: pools/memprof: store and report the pool's name in each bin
Willy Tarreau [Wed, 17 Aug 2022 07:35:16 +0000 (09:35 +0200)] 
MINOR: pools/memprof: store and report the pool's name in each bin

Storing the pointer to the pool along with the stats is quite useful as
it allows to report the name. That's what we're doing here. We could
store it in place of another field but that's not convenient as it would
require to change all functions that manipulate counters. Thus here we
store one extra field, as well as some padding because the struct turns
56 bytes long, thus better go to 64 directly. Example of output from
"show profiling memory":

      2      0       48         0|  0x4bfb2c ha_quic_set_encryption_secrets+0xcc/0xb5e p_alloc(24) [pool=quic_tls_iv]
      0  55252        0  10608384|  0x4bed32 main+0x2beb2 free(-192)
     15      0     2760         0|  0x4be855 main+0x2b9d5 p_alloc(184) [pool=quic_frame]
      1      0     1048         0|  0x4be266 ha_quic_add_handshake_data+0x2b6/0x66d p_alloc(1048) [pool=quic_crypto]
      3      0      552         0|  0x4be142 ha_quic_add_handshake_data+0x192/0x66d p_alloc(184) [pool=quic_frame]
  31276      0  6755616         0|  0x4bb8f9 quic_sock_fd_iocb+0x689/0x69b p_alloc(216) [pool=quic_dgram]
      0  31424        0   6787584|  0x4bb7f3 quic_sock_fd_iocb+0x583/0x69b p_free(-216) [pool=quic_dgram]
    152      0    32832         0|  0x4bb4d9 quic_sock_fd_iocb+0x269/0x69b p_alloc(216) [pool=quic_dgram]

2 years agoMINOR: pool/memprof: report pool alloc/free in memory profiling
Willy Tarreau [Wed, 17 Aug 2022 07:12:53 +0000 (09:12 +0200)] 
MINOR: pool/memprof: report pool alloc/free in memory profiling

Pools are being used so well that it becomes difficult to profile their
usage via the regular memory profiling. Let's add new entries for pools
there, named "p_alloc" and "p_free" that correspond to pool_alloc() and
pool_free(). Ideally it would be nice to only report those that fail
cache lookups but that's complicated, particularly on the free() path
since free lists are released in clusters to the shared pools.

It's worth noting that the alloc_tot/free_tot fields can easily be
determined by multiplying alloc_calls/free_calls by the pool's size, and
could be better used to store a pointer to the pool itself. However it
would require significant changes down the code that sorts output.

If this were to cause a measurable slowdown, an alternate approach could
consist in using a different value of USE_MEMORY_PROFILING to enable pools
profiling. Also, this profiler doesn't depend on intercepting regular malloc
functions, so we could also imagine enabling it alone or the other one alone
or both.

Tests show that the CPU overhead on QUIC (which is already an extremely
intensive user of pools) jumps from ~7% to ~10%. This is quite acceptable
in most deployments.

2 years agoMINOR: memprof: export the minimum definitions for memory profiling
Willy Tarreau [Wed, 17 Aug 2022 06:53:36 +0000 (08:53 +0200)] 
MINOR: memprof: export the minimum definitions for memory profiling

Right now it's not possible to feed memory profiling info from outside
activity.c, so let's export the function and move the enum and struct
to the include file.

2 years agoBUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()
Frédéric Lécaille [Tue, 16 Aug 2022 12:48:59 +0000 (14:48 +0200)] 
BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()

This bug came with this big commit:
     "MEDIUM: quic: xprt traces rework"

This is the <ret> variable value which must be returned by most of the xprt functions.
This leaded packets which could not be decrypted to be parsed, with weird frames
to be parsed as found by Tristan in GH #1808.

To be backported where the commit above was backported.

2 years agoBUG/MINOR: quic: MIssing check when building TX packets
Frédéric Lécaille [Tue, 16 Aug 2022 10:03:00 +0000 (12:03 +0200)] 
BUG/MINOR: quic: MIssing check when building TX packets

When building an ack-eliciting frame only packet, if we did not manage to add
at least one such a frame to the packet, we did not notify the caller about
the fact the packet is empty. This could lead the caller to believe
everything was ok and make it endlessly try to build packet again and again.
This issue was amplified by the recent changes where a while(1) loop has been
added to qc_send_app_pkt() which calls qc_do_build_pkt() through qc_prep_app_pkts()
until we could not prepare packets. Before this recent change, I guess only
one empty packet was sent.
This patch checks that non empty packets could be built by qc_do_build_pkt()
and makes this function return an error if this was the case. Also note that
such an issue could happened only when the packet building was limited by
the congestion control.

Thank you to Tristan for having reported this issue in GH #1808.

Must be backported to 2.6.

2 years agoBUG/MINOR: mux-quic: fix crash with traces in qc_detach()
Amaury Denoyelle [Fri, 12 Aug 2022 13:56:21 +0000 (15:56 +0200)] 
BUG/MINOR: mux-quic: fix crash with traces in qc_detach()

qc_detach() is used to free a qcs as notified by sedesc. If there is no
more stream active and the connection is considered as dead, it will
then be freed. This prevent to dereference qcc in TRACE macro. Else this
will cause a crash.

Use a different code-path on release for qc_detach() to fix this bug.

This will fix the last occurence of crash on github issue #1808.

This has been introduced by recent QUIC MUX traces rework. Thus, it does
not need to be backport.

2 years agoMINOR: ring: archive a previous file-backed ring on startup
Willy Tarreau [Fri, 12 Aug 2022 13:38:20 +0000 (15:38 +0200)] 
MINOR: ring: archive a previous file-backed ring on startup

In order to ensure that an instant restart of the process will not wipe
precious debugging information, and to leave time for an admin to archive
a copy of a ring, now upon startup, any previously existing file will be
renamed with the extra suffix ".bak", and any previously existing file
with suffix ".bak" will be removed.

2 years agoBUILD: sink: replace S_IRUSR, S_IWUSR with their octal value
Willy Tarreau [Fri, 12 Aug 2022 13:03:12 +0000 (15:03 +0200)] 
BUILD: sink: replace S_IRUSR, S_IWUSR with their octal value

The build broke on freebsd with S_IRUSR undefined after commit 0b8e9ceb1
("MINOR: ring: add support for a backing-file"). Maybe another include
is needed there, but the point is that we really don't care about these
symbolic names, file modes are more readable as 0600 than via these
cryptic names anyway, so let's go back to 0600. This will teach me not
to try to make things too clean.

No backport is needed.

2 years agoBUG/MINOR: quic: memleak on wrong datagram receipt
Frédéric Lécaille [Fri, 12 Aug 2022 09:55:20 +0000 (11:55 +0200)] 
BUG/MINOR: quic: memleak on wrong datagram receipt

There was a missing pool_free() call for such datagrams. As far as I see
there is no leak on valid datagram receipt.

Must be backported to 2.6.

2 years agoDEV: haring: support remapping LF in contents with CR VT
Willy Tarreau [Fri, 12 Aug 2022 10:09:41 +0000 (12:09 +0200)] 
DEV: haring: support remapping LF in contents with CR VT

Some traces may contain LF characters which are quite cumbersome to
deal with using the common tools. Given that the utility still has
access to the raw traces and knows where the delimiters are, let's
offer the possibility to remap LF characters to a different sequence.

Here we're using CR VT which will have the same visual appearance but
will remain on the same line for grep etc. This behavior is enabled by
the -l option. It's not enabled by default because it's 50% slower due
to content processing.

2 years agoDEV: haring: add a simple utility to read file-backed rings
Willy Tarreau [Fri, 12 Aug 2022 09:23:59 +0000 (11:23 +0200)] 
DEV: haring: add a simple utility to read file-backed rings

With the ability to back a memory ring into an mmapped file, it makes
sense to be able to dump these files. That's what this utility does.
The entire ring is dumped to stdout. It's well suited to large dumps,
it converts roughly 6 GB of logs per second.

The utility is really meant for developers at the moment. It might
evolve into a more general tool but at the moment it's still possible
that it might need to be run under gdb to process certain crash dumps.

Also at the moment it must not be used on a ring being actively written
to or it will dump garbage.

The code is made so that we can envision later to attach to a live
ring and dump live contents, but this requires that the utility is
built with the exact same options (threads etc), and that the file
is opened read-write. For now these parts have been commented out,
waiting for a reasonably balanced and non-intrusive solution to be
found (e.g. signals must be intercepted so that the tool cannot
leave the ring with a watcher present).

If it is detected that the memory layout of the ring struct differs,
a warning is emitted. At the end, if an error occurs, a warning is
printed as well (this does happen when the process is not cleanly
stopped, but it indicates the end was reached).

2 years agoMINOR: ring: add support for a backing-file
Willy Tarreau [Thu, 11 Aug 2022 14:38:20 +0000 (16:38 +0200)] 
MINOR: ring: add support for a backing-file

This mmaps a file which will serve as the backing-store for the ring's
contents. The idea is to provide a way to retrieve sensitive information
(last logs, debugging traces) even after the process stops and even after
a possible crash. Right now this was possible by connecting to the CLI
and dumping the contents of the ring live, but this is not handy and
consumes quite a bit of resources before it is needed.

With a backing file, the ring is effectively RAM-mapped file, so that
contents stored there are the same as those found in the file (the OS
doesn't guarantee immediate sync but if the process dies it will be OK).

Note that doing that on a filesystem backed by a physical device is a
bad idea, as it will induce slowdowns at high loads. It's really
important that the device is RAM-based.

Also, this may have security implications: if the file is corrupted by
another process, the storage area could be corrupted, causing haproxy
to crash or to overwrite its own memory. As such this should only be
used for debugging.

2 years agoMINOR: ring: support creating a ring from a linear area
Willy Tarreau [Fri, 12 Aug 2022 05:50:43 +0000 (07:50 +0200)] 
MINOR: ring: support creating a ring from a linear area

Instead of allocating two parts, one for the ring struct itself and
one for the storage area, ring_make_from_area() will arrange the two
inside the same memory area, with the storage starting immediately
after the struct. This will allow to store a complete ring state in
shared memory areas for example.

2 years agoBUILD: ring: forward-declare struct appctx to avoid a build warning
Willy Tarreau [Fri, 12 Aug 2022 07:31:16 +0000 (09:31 +0200)] 
BUILD: ring: forward-declare struct appctx to avoid a build warning

When using ring.h standalone it emits warnings about appctx. Let's
forward-declare it.

2 years agoBUG/MEDIUM: quic: Wrong use of <token_odcid> in qc_lsntr_pkt_rcv()
Frédéric Lécaille [Thu, 11 Aug 2022 16:54:26 +0000 (18:54 +0200)] 
BUG/MEDIUM: quic: Wrong use of <token_odcid> in qc_lsntr_pkt_rcv()

This commit was not complete:
  "BUG/MEDIUM: quic: Possible use of uninitialized <odcid>
variable in qc_lstnr_params_init()"
<token_odcid> should have been directly passed to qc_lstnr_params_init()
without dereferencing it to prevent haproxy to have new chances to crash!

Must be backported to 2.6.

2 years agoBUG/MEDIUM: ring: fix too lax 'size' parser
Willy Tarreau [Thu, 11 Aug 2022 14:12:11 +0000 (16:12 +0200)] 
BUG/MEDIUM: ring: fix too lax 'size' parser

It took me a while to figure why a ring declared with "size 1M" was causing
strange effects in a ring, it's just because it's parsed as "1", which is
smaller than the default 16384 size and errors are silently ignored.

This commit tries to address this the best possible way without breaking
existing configs that would work by accident, by warning that the size is
ignored if it's smaller than the current one, and by printing the parsed
size instead of the input string in warnings and errors. This way if some
users have "size 10000" or "size 100k" it will continue to work as 16kB
like today but they will now be aware of it.

In addition the error messages were a bit poor in context in that they
only provided line numbers. The ring name was added to ease locating the
problem.

As the issue was present since day one and was introduced in 2.2 with
commit 99c453df9d ("MEDIUM: ring: new section ring to declare custom ring
buffers."), it could make sense to backport this as far as 2.2, but with
2.2 being quite old now it doesn't seem very reasonable to start emitting
new config warnings in config that apparently worked well.

Thus it looks more reasonable to backport this as far as 2.4.

2 years agoBUG/MEDIUM: quic: Possible use of uninitialized <odcid> variable in qc_lstnr_params_i...
Frédéric Lécaille [Thu, 11 Aug 2022 15:24:38 +0000 (17:24 +0200)] 
BUG/MEDIUM: quic: Possible use of uninitialized <odcid> variable in qc_lstnr_params_init()

When receiving a token into a client Initial packet without a cluster secret defined
by configuration, the <odcid> variable used to parse the ODCID from the token
could be used without having been initialized. Such a packet must be dropped. So
the sufficient part of this patch is this check:

+             }
+             else if (!global.cluster_secret && token_len) {
+                    /* Impossible case: a token was received without configured
+                    * cluster secret.
+                    */
+                    TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT,
+                    NULL, NULL, NULL, qv);
+                    goto drop;
              }

Take the opportunity of this patch to rework and make it more readable this part
of code where such a packet must be dropped removing the <check_token> variable.
When an ODCID is parsed from a token, new <token_odcid> new pointer variable
is set to the address of the parsed ODCID. This way, is not set but used it will
make crash haproxy. This was not always the case with an uninitialized local
variable.

Adapt the API to used such a pointer variable: <token> boolean variable is removed
from qc_lstnr_params_init() prototype.

This must be backported to 2.6.

2 years agoBUG/MEDIUM: mux-quic: fix crash due to invalid trace arg
Amaury Denoyelle [Thu, 11 Aug 2022 16:22:22 +0000 (18:22 +0200)] 
BUG/MEDIUM: mux-quic: fix crash due to invalid trace arg

Traces argument were incorrectly used in qcs_free(). A qcs was specified
as first arg instead of a connection. This will lead to a crash if
developer qmux traces are activated. This is now fixed.

This bug has been introduced with QUIC MUX traces rework. No need to
backport.

2 years agoMINOR: mux-quic: define new traces
Amaury Denoyelle [Wed, 10 Aug 2022 14:58:01 +0000 (16:58 +0200)] 
MINOR: mux-quic: define new traces

Add new traces to help debugging on QUIC MUX. Most notable, the
following functions are now traced :
* qcc_emit_cc
* qcs_free
* qcs_consume
* qcc_decode_qcs
* qcc_emit_cc_app
* qcc_install_app_ops
* qcc_release_remote_stream
* qcc_streams_sent_done
* qc_init

2 years agoCLEANUP: mux-quic: adjust traces level
Amaury Denoyelle [Wed, 10 Aug 2022 14:42:35 +0000 (16:42 +0200)] 
CLEANUP: mux-quic: adjust traces level

Change default devel level for some traces in QUIC MUX:
* proto : used to notify about reception/emission of frames
* state : modification of internal state of connection or streams
* data : detailled information about transfer and flow-control

2 years agoMINOR: mux-quic: define protocol error traces
Amaury Denoyelle [Wed, 10 Aug 2022 14:39:54 +0000 (16:39 +0200)] 
MINOR: mux-quic: define protocol error traces

Replace devel traces with error level on all errors situation. Also a
new event QMUX_EV_PROTO_ERR is used. This should help to detect invalid
situations quickly.

2 years agoMINOR: mux-quic: adjust enter/leave traces
Amaury Denoyelle [Wed, 10 Aug 2022 14:14:32 +0000 (16:14 +0200)] 
MINOR: mux-quic: adjust enter/leave traces

Improve MUX traces by adding some missing enter/leave trace points. In
some places, early function returns have been replaced by a goto
statement.

2 years agoCLEANUP: quic: Remove trailing spaces
Frédéric Lécaille [Thu, 11 Aug 2022 10:14:07 +0000 (12:14 +0200)] 
CLEANUP: quic: Remove trailing spaces

This spaces have come with this commit:
  "MEDIUM: quic: xprt traces rework".

2 years agoBUG/MINOR: quic: Possible infinite loop in quic_build_post_handshake_frames()
Frédéric Lécaille [Thu, 11 Aug 2022 10:04:07 +0000 (12:04 +0200)] 
BUG/MINOR: quic: Possible infinite loop in quic_build_post_handshake_frames()

This loop is due to the fact that we do not select the next node before
the conditional "continue" statement. Furthermore the condition and the
"continue" statement may be removed after replacing eb64_first() call
by eb64_lookup_ge(): we are sure this condition may not be satisfied.
Add some comments: this function initializes connection IDs with sequence
number 1 upto <max> non included.
Take the opportunity of this patch to remove a "return" wich broke this
traces rule: for any function, do not call TRACE_ENTER() without TRACE_LEAVE()!
Add also TRACE_ERROR() for any encoutered errors.

Must be backported to 2.6

2 years agoMINOR: quic: Remove useless lock for RX packets
Frédéric Lécaille [Thu, 11 Aug 2022 09:40:01 +0000 (11:40 +0200)] 
MINOR: quic: Remove useless lock for RX packets

This lock was there be able to handle the RX packets for a connetion
from several threads. This is no more needed since a QUIC connection
is always handled by the same thread.

May be backported to 2.6

2 years agoBUILD: stconn: fix build warning at -O3 about possible null sc
Willy Tarreau [Thu, 11 Aug 2022 11:56:42 +0000 (13:56 +0200)] 
BUILD: stconn: fix build warning at -O3 about possible null sc

gcc-6.x and 7.x emit build warnings about sc possibly being null upon
return from sc_detach_endp(). This actually is not the case and the
compiler is a little bit overzealous there, but there exists code
paths that can make this analysis non-trivial so let's at least add
a similar BUG_ON() to let both the compiler and the deverloper know
this doesn't happen.

This should be backported to 2.6.

2 years agoMEDIUM: quic: xprt traces rework
Frédéric Lécaille [Wed, 10 Aug 2022 15:56:45 +0000 (17:56 +0200)] 
MEDIUM: quic: xprt traces rework

Add a least as much as possible TRACE_ENTER() and TRACE_LEAVE() calls
to any function. Note that some functions do not have any access to the
a quic_conn argument when  receiving or parsing datagram at very low level.

2 years agoBUG/MEDIUM: task: relax one thread consistency check in task_unlink_wq()
Willy Tarreau [Wed, 10 Aug 2022 16:03:11 +0000 (18:03 +0200)] 
BUG/MEDIUM: task: relax one thread consistency check in task_unlink_wq()

While testing the fix for the previous issue related to reloads with
hard_stop_after, I've met another one which could spuriously produce:

  FATAL: bug condition "t->tid >= 0 && t->tid != tid" matched at include/haproxy/task.h:266

In 2.3-dev2, we've added more consistency checks for a number of bug-
inducing programming errors related to the tasks, via commit e5d79bccc
("MINOR: tasks/debug: add a few BUG_ON() to detect use of wrong timer
queue"), and this check comes from there.

The problem that happens here is that when hard-stop-after is set, we
can abort the current thread even if there are still ongoing checks
(or connections in fact). In this case some tasks are present in a
thread's wait queue and are thus bound exclusively to this thread.

During deinit(), the collect and cleanup of all memory areas also
stops servers and kills their check tasks. And calling task_destroy()
does in turn call task_unlink_wq()... except that it's called from
thread 0 which doesn't match the initially planned thread number.

Several approaches are possible. One of them would consist in letting
threads perform their own cleanup (tasks, pools, FDs, etc). This would
possibly be even faster since done in parallel, but some corner cases
might be way more complicated (e.g. who will kill a check's task, or
what to do with a task found in a local wait queue or run queue, and
what about other consistency checks this could violate?).

Thus for now this patches takes an easier and more conservative
approach consisting in admitting that when the process is stopping,
this rule is not necessarily valid, and to let thread 0 collect all
other threads' garbage.

As such this patch can be backpoted to 2.4.

2 years agoBUG/MEDIUM: poller: use fd_delete() to release the poller pipes
Willy Tarreau [Wed, 10 Aug 2022 15:08:17 +0000 (17:08 +0200)] 
BUG/MEDIUM: poller: use fd_delete() to release the poller pipes

The poller pipes needed to communicate between multiple threads are
allocated in init_pollers_per_thread() and released in
deinit_pollers_per_thread(). The former adds them via fd_insert()
so that they are known, but the former only closes them using a
regular close().

This asymmetry represents a problem, because we have in the fdtab[]
an entry for something that may disappear when one thread leaves, and
since these FD numbers are very low, there is a very high likelihood
that they are immediately reassigned to another thread trying to
connect() to a server or just sending a health check. In this case,
the other thread is going to fd_insert() the fd and the recently
added consistency checks will notive that ->owner is not NULL and
will crash. We just need to use fd_delete() here to match fd_insert().

Note that this test was added in 2.7-dev2 by commit 36d9097cf
("MINOR: fd: Add BUG_ON checks on fd_insert()") which was backported
to 2.4 as a safety measure (since it allowed to catch particularly
serious issues). The patch in itself isn't wrong, it just revealed
a long-dormant bug (been there since 1.9-dev1, 4 years ago). As such
the current patch needs to be backported wherever the commit above
is backported.

Many thanks to Christian Ruppert for providing detailed traces in
github issue #1807 and Cedric Paillet for bringing his complementary
analysis that helped to understand the required conditions for this
issue to happen (fast health checks @100ms + randomly long connections
~7s + fast reloads every second + hard-stop-after 5s were necessary
on the dev's machine to trigger it from time to time).

2 years agoBUG/MEDIUM: quic: always remove the connection from the accept list on close
Willy Tarreau [Wed, 10 Aug 2022 05:26:27 +0000 (07:26 +0200)] 
BUG/MEDIUM: quic: always remove the connection from the accept list on close

Fred managed to reproduce a crash showing a corrupted accept_list when
firing thousands of concurrent picoquicdemo clients to a same instance.
It may happen if the connection was placed into the accept_list and
immediately closed before being processed (e.g. on error or t/o ?).

In any case the quic_conn_release() function should always detach a
connection to be deleted from any list, like it does for other lists,
so let's add an MT_LIST_DELETE() here.

This should be backported to 2.6.

2 years agoBUG/MINOR: quic: fix crash on handshake io-cb for null next enc level
Amaury Denoyelle [Tue, 9 Aug 2022 15:52:52 +0000 (17:52 +0200)] 
BUG/MINOR: quic: fix crash on handshake io-cb for null next enc level

When arriving at the handshake completion, next encryption level will be
null on quic_conn_io_cb(). Thus this must be check this before
dereferencing it via qc_need_sending() to prevent a crash.

This was reproduced quickly when browsing over a local nextcloud
instance through QUIC with firefox.

This has been introduced in the current dev with quic-conn Tx
refactoring. No need to backport it.

2 years agoBUG/MINOR: mux-quic: open stream on STOP_SENDING
Amaury Denoyelle [Tue, 9 Aug 2022 15:36:38 +0000 (17:36 +0200)] 
BUG/MINOR: mux-quic: open stream on STOP_SENDING

Considered a stream as opened when receiving a STOP_SENDING frame as the
first frame on the stream.

This patch is tagged as BUG because a BUG_ON may occur if only a
STOP_SENDING frame has been received for a frame. This will reset the
stream in respect with RFC9000 but internally it is considered invalid
transition to reset an idle stream.

To fix this, simply use qcs_idle_open() on STOP_SENDING parsing
function. This will mark the stream as OPEN before resetting it.

This was detected on haproxy.org with the following backtrace :

FATAL: bug condition "qcs->st == QC_SS_IDLE" matched at
src/mux_quic.c:383
  call trace(12):
  |       0x490dd3 [b8 01 00 00 00 c6 00 00]: main-0x1d0633
  |       0x4975b8 [48 8b 85 58 ff ff ff 8b]: main-0x1c9e4e
  |       0x497df4 [48 8b 45 c8 48 89 c7 e8]: main-0x1c9612
  |       0x49934c [48 8b 45 c8 48 89 c7 e8]: main-0x1c80ba
  |       0x6b3475 [48 8b 05 54 1b 3a 00 64]: run_tasks_from_lists+0x45d/0x8b2
  |       0x6b4093 [29 c3 89 d8 89 45 d0 83]: process_runnable_tasks+0x7c9/0x824
  |       0x660bde [8b 05 fc b3 4f 00 83 f8]: run_poll_loop+0x74/0x430
  |       0x6611de [48 8b 05 7b a6 40 00 48]: main-0x228
  | 0x7f66e4fb2ea5 [64 48 89 04 25 30 06 00]: libpthread:+0x7ea5
  | 0x7f66e455ab0d [48 89 c7 e8 5b 72 fc ff]: libc:clone+0x6d/0x86

Stream states have been implemented in the current dev tree. Thus, this
patch does not need to be backported.

2 years agoMINOR: quic: skip sending if no frame to send in io-cb
Amaury Denoyelle [Mon, 8 Aug 2022 16:15:24 +0000 (18:15 +0200)] 
MINOR: quic: skip sending if no frame to send in io-cb

Check on quic_conn_io_cb() if sending is required. This allows to skip
over Tx buffer allocation if not needed.

To implement this, we check if frame lists on current and next
encryption level are empty. We also need to check if there is no need to
send ACK, PROBE or CONNECTION_CLOSE. This has been isolated in a new
function qc_need_sending() which may be reuse in some other functions in
the future.

2 years agoMINOR: quic: refactor datagram commit in Tx buffer
Amaury Denoyelle [Mon, 8 Aug 2022 13:38:57 +0000 (15:38 +0200)] 
MINOR: quic: refactor datagram commit in Tx buffer

This is the final patch on quic-conn Tx refactor. Extend the function
which is used to write a datagram header to save at the same time
written buffer data. This makes sense as the two operations are used at
the same occasion when a pre-written datagram is comitted.

2 years agoMINOR: quic: release Tx buffer on each send
Amaury Denoyelle [Mon, 8 Aug 2022 14:07:30 +0000 (16:07 +0200)] 
MINOR: quic: release Tx buffer on each send

Complete refactor of quic-conn Tx buffer. The buffer is now released
on every send operation completion. This should help to reduce memory
footprint as now Tx buffers are allocated and released on demand.

To simplify allocation/free of quic-conn Tx buffer, two static functions
are created named qc_txb_alloc() and qc_txb_release().

2 years agoMINOR: quic: replace custom buf on Tx by default struct buffer
Amaury Denoyelle [Thu, 4 Aug 2022 14:19:57 +0000 (16:19 +0200)] 
MINOR: quic: replace custom buf on Tx by default struct buffer

On first prototype version of QUIC, emission was multithreaded. To
support this, a custom thread-safe ring-buffer has been implemented with
qring/cbuf.

Now the thread model has been adjusted : a quic-conn is always used on
the same thread and emission is not multi-threaded. Thus, qring/cbuf
usage can be replace by a standard struct buffer.

The code has been simplified even more as for now buffer is always
drained after a prepare/send invocation. This is the case since a
datagram is always considered as sent even on sendto() error. BUG_ON
statements guard are here to ensure that this model is always valid.
Thus, code to handle data wrapping and consume too small contiguous
space with a 0-length datagram is removed.

2 years agoCLEANUP: mux-quic: remove loop on sending frames
Amaury Denoyelle [Thu, 4 Aug 2022 08:11:12 +0000 (10:11 +0200)] 
CLEANUP: mux-quic: remove loop on sending frames

qc_send_app_pkts() has now a while loop implemented which allows to send
all possible frames even if the send buffer is full between packet
prepare and send. This is present since commit :
  dc07751ed7ebad10f49081d28a9a5ae785f53d76
  MINOR: quic: Send packets as much as possible from qc_send_app_pkts()

This means we can remove code from the MUX which implement this at the
upper layer. This is useful to simplify qc_send_frames() function.

As mentionned commit is subject to backport, this commit should be
backported as well to 2.6.

2 years agoMINOR: debug/memstats: permit to pass the size to free()
Willy Tarreau [Tue, 9 Aug 2022 07:08:18 +0000 (09:08 +0200)] 
MINOR: debug/memstats: permit to pass the size to free()

Right now the free() call is not intercepted since all this is done
using macros and that would break a lot of stuff. Instead a __free()
macro was provided but never used. In addition it used to only report
a zero size, which is not very convenient.

With this patch comes a better solution. Instead it provides a new
will_free() macro that can be prepended before a call to free(). It
only keeps the counters up to date, and also supports being passed a
size. The pool_free_area() command now uses it, which finally allows
the stats to look correct:

  pool-os.h:38   MALLOC  size:   5802127832  calls:   3868044  size/call:   1500
  pool-os.h:47     FREE  size:   5800041576  calls:   3867444  size/call:   1499

The few other places directly calling free() could now be instrumented to
use this and to pass the correct sizeof() when known.

2 years agoMINOR: debug/memstats: automatically determine first column size
Willy Tarreau [Tue, 9 Aug 2022 06:51:08 +0000 (08:51 +0200)] 
MINOR: debug/memstats: automatically determine first column size

The first column's width may vary a lot depending on outputs, and it's
annoying to have large empty columns on small names and mangled large
columns that are not yet large enough. In order to overcome this, this
patch adds a width field to the memstats applet's context, and this
width is calculated the first time the function is entered, by estimating
the width of all lines that will be dumped. This is simple enough and
does the job well. If in the future some filtering criteria are added,
it will still be possible to perform a single pass on everything
depending on the desired output format.

2 years agoMINOR: debug: also store the function name in struct mem_stats
Willy Tarreau [Tue, 9 Aug 2022 06:40:08 +0000 (08:40 +0200)] 
MINOR: debug: also store the function name in struct mem_stats

The calling function name is now stored in the structure, and it's
reported when the "all" argument is passed. The first column is
significantly enlarged because some names are really wide :-(

2 years agoMINOR: debug: store and report the pool's name in struct mem_stats
Willy Tarreau [Tue, 9 Aug 2022 06:15:27 +0000 (08:15 +0200)] 
MINOR: debug: store and report the pool's name in struct mem_stats

Let's add a generic "extra" pointer to the struct mem_stats to store
context-specific information. When tracing pool_alloc/pool_free, we
can now store a pointer to the pool, which allows to report the pool
name on an extra column. This significantly improves tracing
capabilities.

Example:

  proxy.c:1598      CALLOC   size:  28832  calls:  4     size/call:  7208
  dynbuf.c:55       P_FREE   size:  32768  calls:  2     size/call:  16384  buffer
  quic_tls.h:385    P_FREE   size:  34008  calls:  1417  size/call:  24     quic_tls_iv
  quic_tls.h:389    P_FREE   size:  34008  calls:  1417  size/call:  24     quic_tls_iv
  quic_tls.h:554    P_FREE   size:  34008  calls:  1417  size/call:  24     quic_tls_iv
  quic_tls.h:558    P_FREE   size:  34008  calls:  1417  size/call:  24     quic_tls_iv
  quic_tls.h:562    P_FREE   size:  34008  calls:  1417  size/call:  24     quic_tls_iv
  quic_tls.h:401    P_ALLOC  size:  34080  calls:  1420  size/call:  24     quic_tls_iv
  quic_tls.h:403    P_ALLOC  size:  34080  calls:  1420  size/call:  24     quic_tls_iv
  xprt_quic.c:4060  MALLOC   size:  45376  calls:  5672  size/call:  8
  quic_sock.c:328   P_ALLOC  size:  46440  calls:  215   size/call:  216    quic_dgram

2 years agoMINOR: debug: make the mem_stats section aligned to void*
Willy Tarreau [Tue, 9 Aug 2022 06:09:24 +0000 (08:09 +0200)] 
MINOR: debug: make the mem_stats section aligned to void*

Not specifying the alignment will let the linker choose it, and it turns
out that it will not necessarily be the same size as the one chosen for
struct mem_stats, as can be seen if any new fields are added there. Let's
enforce an alignment to void* both for the section and for the structure.

2 years agoMINOR: quic: Replace pool_zalloc() by pool_malloc() for fake datagrams
Frédéric Lécaille [Mon, 8 Aug 2022 19:10:58 +0000 (21:10 +0200)] 
MINOR: quic: Replace pool_zalloc() by pool_malloc() for fake datagrams

These fake datagrams are only used by the low level I/O handler. They
are not provided to the "by connection" datagram handlers. This
is why they are not MT_LIST_APPEND()ed to the listner RX buffer list
(see &quic_dghdlrs[cid_tid].dgrams in quic_lstnr_dgram_dispatch().
Replace the call to pool_zalloc() to by the lighter call to pool_malloc()
and initialize only the ->buf and ->length members. This is safe because
only these fields are inspected by the low level I/O handler.

2 years agoBUG/MEDIUM: quic: Missing AEAD TAG check after removing header protection
Frédéric Lécaille [Mon, 8 Aug 2022 16:41:16 +0000 (18:41 +0200)] 
BUG/MEDIUM: quic: Missing AEAD TAG check after removing header protection

After removing the packet header protection, we can check the packet is long
enough to contain a 16 bytes length AEAD TAG (at this end of the packet).
This test was missing.

Must be backported to 2.6.

2 years agoMINOR: quic: Too much useless traces in qc_build_frms()
Frédéric Lécaille [Mon, 8 Aug 2022 14:09:46 +0000 (16:09 +0200)] 
MINOR: quic: Too much useless traces in qc_build_frms()

These traces about the available room into the packet currently built and
its payload length could be displayed for each STREAM frame, even for
those which have no chance to be embedded into a packet leading to
very traces to be displayed from a connection with a lot of stream.
This was revealed by traces provide by Tristan in GH #1808

May be backported to 2.6.

2 years agoBUG/MEDIUM: quic: Wrong packet length check in qc_do_rm_hp()
Frédéric Lécaille [Mon, 8 Aug 2022 08:28:07 +0000 (10:28 +0200)] 
BUG/MEDIUM: quic: Wrong packet length check in qc_do_rm_hp()

When entering this function, we first check the packet length is not too short.
But this was done against the datagram lenght in place of the packet length.
This could lead to the header protection to be removed using data past
the end of the packet (without buffer overflow).

Use the packet length in place of the datagram length which is at <end>
address passed as parameter to this function. As the packet length
is already stored in ->len packet struct member, this <end> parameter is no
more useful.

Must be backported to 2.6.

2 years ago[RELEASE] Released version 2.7-dev3 v2.7-dev3
Willy Tarreau [Sun, 7 Aug 2022 15:28:59 +0000 (17:28 +0200)] 
[RELEASE] Released version 2.7-dev3

Released version 2.7-dev3 with the following main changes :
    - BUILD: makefile: Fix install(1) handling for OpenBSD/NetBSD/Solaris/AIX
    - BUG/MEDIUM: tools: avoid calling dlsym() in static builds (try 2)
    - MINOR: resolvers: resolvers_destroy() deinit and free a resolver
    - BUG/MINOR: resolvers: shut off the warning for the default resolvers
    - BUG/MINOR: ssl: allow duplicate certificates in ca-file directories
    - BUG/MINOR: tools: fix statistical_prng_range()'s output range
    - BUG/MINOR: quic: do not send CONNECTION_CLOSE_APP in initial/handshake
    - BUILD: debug: Add braces to if statement calling only CHECK_IF()
    - BUG/MINOR: fd: Properly init the fd state in fd_insert()
    - BUG/MEDIUM: fd/threads: fix incorrect thread selection in wakeup broadcast
    - MINOR: init: load OpenSSL error strings
    - MINOR: ssl: enhance ca-file error emitting
    - BUG/MINOR: mworker/cli: relative pid prefix not validated anymore
    - BUG/MAJOR: mux_quic: fix invalid PROTOCOL_VIOLATION on POST data overlap
    - BUG/MEDIUM: mworker: proc_self incorrectly set crashes upon reload
    - BUILD: add detection for unsupported compiler models
    - BUG/MEDIUM: stconn: Only reset connect expiration when processing backend side
    - BUG/MINOR: backend: Fallback on RR algo if balance on source is impossible
    - BUG/MEDIUM: master: force the thread count earlier
    - BUG/MAJOR: poller: drop FD's tgid when masks don't match
    - DEBUG: fd: detect possibly invalid tgid in fd_insert()
    - BUG/MINOR: sockpair: wrong return value for fd_send_uxst()
    - MINOR: sockpair: move send_fd_uxst() error message in caller
    - Revert "BUG/MINOR: peers: set the proxy's name to the peers section name"
    - DEBUG: fd: split the fd check
    - MEDIUM: resolvers: continue startup if network is unavailable
    - BUG/MINOR: fd: always remove late updates when freeing fd_updt[]
    - MINOR: cli: emit a warning when _getsocks was used more than once
    - BUG/MINOR: mworker: PROC_O_LEAVING used but not updated
    - Revert "MINOR: cli: emit a warning when _getsocks was used more than once"
    - MINOR: cli: warning on _getsocks when socket were closed
    - BUG/MEDIUM: mux-quic: fix missing EOI flag to prevent streams leaks
    - MINOR: quic: Congestion control architecture refactoring
    - MEDIUM: quic: Cubic congestion control algorithm implementation
    - MINOR: quic: New "quic-cc-algo" bind keyword
    - BUG/MINOR: quic: loss time limit variable computed but not used
    - MINOR: quic: Stop looking for packet loss asap
    - BUG/MAJOR: quic: Useless resource intensive loop qc_ackrng_pkts()
    - MINOR: quic: Send packets as much as possible from qc_send_app_pkts()
    - BUG/MEDIUM: queue/threads: limit the number of entries dequeued at once
    - MAJOR: threads/plock: update the embedded library
    - MINOR: thread: provide an alternative to pthread's rwlock
    - DEBUG: tools: provide a tree dump function for ebmbtrees as well
    - MINOR: ebtree: add ebmb_lookup_shorter() to pursue lookups
    - BUG/MEDIUM: pattern: only visit equivalent nodes when skipping versions
    - BUG/MINOR: mux-quic: prevent crash if conn released during IO callback
    - CLEANUP: mux-quic: remove useless app_ops is_active callback
    - BUG/MINOR: mux-quic: do not free conn if attached streams
    - MINOR: mux-quic: save proxy instance into qcc
    - MINOR: mux-quic: use timeout server for backend conns
    - MEDIUM: mux-quic: adjust timeout refresh
    - MINOR: mux-quic: count in-progress requests
    - MEDIUM: mux-quic: implement http-keep-alive timeout
    - MINOR: peers: Add a warning about incompatible SSL config for the local peer
    - MINOR: peers: Use a dedicated reconnect timeout when stopping the local peer
    - BUG/MEDIUM: peers: limit reconnect attempts of the old process on reload
    - BUG/MINOR: peers: Use right channel flag to consider the peer as connected
    - BUG/MEDIUM: dns: Properly initialize new DNS session
    - BUG/MINOR: backend: Don't increment conn_retries counter too early
    - MINOR: server: Constify source server to copy its settings
    - REORG: server: Export srv_settings_cpy() function
    - BUG/MEDIUM: proxy: Perform a custom copy for default server settings
    - BUG/MINOR: quic: Missing in flight ack eliciting packet counter decrement
    - BUG/MEDIUM: quic: Floating point exception in cubic_root()
    - MINOR: h3: support HTTP request framing state
    - MINOR: mux-quic: refresh timeout on frame decoding
    - MINOR: mux-quic: refactor refresh timeout function
    - MEDIUM: mux-quic: implement http-request timeout
    - BUG/MINOR: quic: Avoid sending truncated datagrams
    - BUG/MINOR: ring/cli: fix a race condition between the writer and the reader
    - BUG/MEDIUM: sink: Set the sink ref for forwarders created during ring parsing
    - BUG/MINOR: sink: fix a race condition between the writer and the reader
    - BUG/MINOR: quic: do not reject datagrams matching minimum permitted size
    - MINOR: quic: Add two new stats counters for sendto() errors
    - BUG/MINOR: quic: Missing Initial packet dropping case
    - MINOR: quic: explicitely ignore sendto error
    - BUG/MINOR: quic: adjust errno handling on sendto
    - BUG/MEDIUM: quic: break out of the loop in quic_lstnr_dghdlr
    - MINOR: threads: report the number of thread groups in build options
    - MINOR: config: automatically preset MAX_THREADS based on MAX_TGROUPS
    - BUILD: SSL: allow to pass additional configure args to QUICTLS
    - CI: enable weekly "m32" builds on x86_64
    - CLEANUP: assorted typo fixes in the code and comments
    - BUG/MEDIUM: fix DH length when EC key is used
    - REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
    - REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
    - REGTESTS: ssl: fix grep invocation to use extended regex in ssl_generate_certificate.vtc
    - BUILD: cfgparse: always defined _GNU_SOURCE for sched.h and crypt.h