Commit Graph

18302 Commits

Author SHA1 Message Date
Christopher Faulet
871dd82117 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.
2022-08-24 11:59:04 +02:00
William Lallemand
6020c4e44e 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.
2022-08-24 11:28:29 +02:00
William Lallemand
866b88bc95 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.
2022-08-24 10:11:17 +02:00
Brad Smith
02fd3caa8f 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.
2022-08-24 05:23:13 +02:00
Willy Tarreau
8bd146d8af 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.
2022-08-23 20:19:11 +02:00
Willy Tarreau
df3cab1ca1 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.
2022-08-23 20:19:11 +02:00
Willy Tarreau
8a3f58280f 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.
2022-08-23 20:19:11 +02:00
Frédéric Lécaille
a2d8ad20a3 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
2022-08-23 17:55:02 +02:00
Frédéric Lécaille
b8047de11a 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.
2022-08-23 17:40:09 +02:00
Frédéric Lécaille
a8a6043240 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.
2022-08-23 12:06:40 +02:00
Frédéric Lécaille
da9c441886 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.
2022-08-22 19:06:08 +02:00
William Lallemand
16972e19d4 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.
2022-08-22 13:09:40 +02:00
Emeric Brun
8032a276ce 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
2022-08-22 13:09:29 +02:00
Willy Tarreau
f1cfd9bc97 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.
2022-08-22 10:46:13 +02:00
Willy Tarreau
3cd71acd06 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.
2022-08-22 10:38:00 +02:00
Frédéric Lécaille
ea4a5cbbdf 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.
2022-08-20 19:08:31 +02:00
Willy Tarreau
f53201940b [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"
2022-08-20 15:56:31 +02:00
William Lallemand
62c0b99e3b 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".
2022-08-19 19:58:53 +02:00
William Lallemand
d4774d3cfa 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.
2022-08-19 19:58:53 +02:00
William Lallemand
ec7eb59d20 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.
2022-08-19 19:56:53 +02:00
Frédéric Lécaille
86a53c5669 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.
2022-08-19 18:31:59 +02:00
Emeric Brun
a8942cd9c4 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.
2022-08-19 16:09:06 +02:00
Emeric Brun
3b68b60261 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.
2022-08-19 16:08:03 +02:00
Frédéric Lécaille
a846a17fde 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.
2022-08-19 12:15:05 +02:00
Frédéric Lécaille
e4c3074c00 MINOR: quic: Add the QUIC connection to mux traces
This should help for debugging purpose.

Should be backported to 2.6
2022-08-19 12:02:29 +02:00
Frédéric Lécaille
b827840b42 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
2022-08-19 10:10:43 +02:00
Frédéric Lécaille
2f16348d24 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.
2022-08-19 09:59:07 +02:00
Frédéric Lécaille
1ba25c244e 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
2022-08-19 09:58:28 +02:00
Willy Tarreau
473e0e54f5 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"
2022-08-19 08:03:53 +02:00
Willy Tarreau
4877045f1d 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.
2022-08-19 08:03:53 +02:00
Willy Tarreau
ed2b9d9f27 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.
2022-08-19 08:03:53 +02:00
Willy Tarreau
0f45871344 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.
2022-08-19 08:03:53 +02:00
Willy Tarreau
1addf8b777 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".
2022-08-18 18:16:36 +02:00
Willy Tarreau
1cc08a33e1 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.
2022-08-18 18:16:36 +02:00
Amaury Denoyelle
115ccce867 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.
2022-08-18 16:13:33 +02:00
Amaury Denoyelle
d23435df28 REGTESTS: add test for HTTP/2 cookies concatenation
Write a regtest to test RFC 7540 compliance in regards to multiple
cookie headers concatenation.
2022-08-18 16:13:33 +02:00
Amaury Denoyelle
2c5a7ee333 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.
2022-08-18 16:13:33 +02:00
Amaury Denoyelle
704675656b 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 :
  b29a1dc2f4
  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.
2022-08-18 11:33:22 +02:00
Frédéric Lécaille
4173a39c1f 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.
2022-08-18 10:28:31 +02:00
Mateusz Malek
4b85a963be 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).
2022-08-17 15:52:17 +02:00
Amaury Denoyelle
b29a1dc2f4 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
  e53b489826
  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.
2022-08-17 11:06:24 +02:00
Amaury Denoyelle
cc13047364 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.
2022-08-17 11:05:49 +02:00
Amaury Denoyelle
3baab744e7 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.
2022-08-17 11:05:47 +02:00
Amaury Denoyelle
fd79ddb2d6 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.
2022-08-17 11:05:46 +02:00
Amaury Denoyelle
bf3c208760 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 11a6f4007b
  BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()

This must be backported up to 2.6.
2022-08-17 11:05:19 +02:00
Amaury Denoyelle
26aa399d6b 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.
2022-08-17 11:04:53 +02:00
Amaury Denoyelle
6b02c6bb47 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.
2022-08-17 11:04:52 +02:00
Amaury Denoyelle
f372e744de 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.
2022-08-17 11:04:47 +02:00
Amaury Denoyelle
7b8f477da5 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.
2022-08-17 11:04:20 +02:00
Frdric Lcaille
bbeec37b31 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.
2022-08-17 10:52:15 +02:00