]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoMINOR: ssl/lua: CertCache.set() allows to update an SSL certificate file
William Lallemand [Wed, 30 Mar 2022 10:03:12 +0000 (12:03 +0200)] 
MINOR: ssl/lua: CertCache.set() allows to update an SSL certificate file

The CertCache.set() function allows to update an SSL certificate file
stored in the memory of the HAProxy process. This function does the same
as "set ssl cert" + "commit ssl cert" over the CLI.

This could be used to update the crt and key, as well as the OCSP, the
SCTL, and the OSCP issuer.

The implementation does yield every 10 ckch instances, the same way the
"commit ssl cert" do.

3 years agoMINOR: ssl: add "crt" in the cert_exts array
William Lallemand [Wed, 30 Mar 2022 10:01:32 +0000 (12:01 +0200)] 
MINOR: ssl: add "crt" in the cert_exts array

The cert_exts array does handle "crt" the default way, however
you might stil want to look for these extensions in the array.

3 years agoMINOR: ssl: export ckch_inst_rebuild()
William Lallemand [Wed, 30 Mar 2022 09:26:15 +0000 (11:26 +0200)] 
MINOR: ssl: export ckch_inst_rebuild()

ckch_inst_rebuild() will be needed to regenerate the ckch instances from
the lua code, we need to export it.

3 years agoMINOR: ssl: simplify the certificate extensions array
William Lallemand [Tue, 29 Mar 2022 08:44:23 +0000 (10:44 +0200)] 
MINOR: ssl: simplify the certificate extensions array

Simplify the "cert_exts" array which is used for the selection of the
parsing function depending on the extension.

It now uses a pointer to an array element instead of an index, which is
simplier for the declaration of the array.

This way also allows to have multiple extension using the same type.

3 years agoMINOR: ssl: move the cert_exts and the CERT_TYPE enum
William Lallemand [Thu, 24 Mar 2022 16:48:40 +0000 (17:48 +0100)] 
MINOR: ssl: move the cert_exts and the CERT_TYPE enum

Move the cert_exts declaration and the CERT_TYPE enum in the .h in order
to reuse them in another file.

3 years agoMINOR: ssl: split the cert commit io handler
William Lallemand [Tue, 29 Mar 2022 12:29:31 +0000 (14:29 +0200)] 
MINOR: ssl: split the cert commit io handler

Extract the code that replace the ckch_store and its dependencies into
the ckch_store_replace() function.

This function must be used under the global ckch lock.
It frees everything related to the old ckch_store.

3 years agoMEDIUM: httpclient/lua: be stricter with httpclient parameters
William Lallemand [Thu, 3 Mar 2022 14:33:12 +0000 (15:33 +0100)] 
MEDIUM: httpclient/lua: be stricter with httpclient parameters

Checks the argument passed to the httpclient send functions so we don't
add a mispelled argument.

3 years agoMINOR: services: alphabetically sort service names
Willy Tarreau [Wed, 30 Mar 2022 10:12:44 +0000 (12:12 +0200)] 
MINOR: services: alphabetically sort service names

Note that we cannot reuse dump_act_rules() because the output format
may be adjusted depending on the call place (this is also used from
haproxy -vv). The principle is the same however.

3 years agoMINOR: filters: alphabetically sort the list of filter names
Willy Tarreau [Wed, 30 Mar 2022 10:08:00 +0000 (12:08 +0200)] 
MINOR: filters: alphabetically sort the list of filter names

There are very few but they're registered from constructors, hence
in a random order. The scope had to be copied when retrieving the
next keyword. Note that this also has the effect of listing them
sorted in haproxy -vv.

3 years agoMINOR: cli: alphanumerically sort the dump of supported commands
Willy Tarreau [Wed, 30 Mar 2022 10:02:35 +0000 (12:02 +0200)] 
MINOR: cli: alphanumerically sort the dump of supported commands

Like for previous keyword classes, we're sorting the output. But this
time as it's not trivial to do it with multiple words, instead we're
proceeding like the help command, we sort them on their usage message
when present, and fall back to the first word of the command when there
is no usage message (e.g. "help" command).

3 years agoMINOR: acl: alphanumerically sort the ACL dump
Willy Tarreau [Wed, 30 Mar 2022 09:49:59 +0000 (11:49 +0200)] 
MINOR: acl: alphanumerically sort the ACL dump

The mechanism is similar to others. We take care of sorting on the
keyword only and not the fetch_kw which is not unique.

3 years agoMINOR: sample: alphanumerically sort sample & conv keyword dumps
Willy Tarreau [Wed, 30 Mar 2022 09:30:36 +0000 (11:30 +0200)] 
MINOR: sample: alphanumerically sort sample & conv keyword dumps

It's much more convenient to sort these keywords on output to detect
changes, and it's easy to do. The patch looks big but most of it is
only caused by an indent change in the loop, as "git diff -b" shows.

3 years agoMINOR: config: alphanumerically sort config keywords output
Willy Tarreau [Wed, 30 Mar 2022 09:21:32 +0000 (11:21 +0200)] 
MINOR: config: alphanumerically sort config keywords output

The output produced by dump_registered_keywords() really deserves to be
sorted in order to ease comparisons. The function now implements a tiny
sorting mechanism that's suitable for each two-level list, and makes
use of dump_act_rules() to dump rulesets. The code is not significantly
more complicated and some parts (e.g options) could even be factored.
The output is much more exploitable to detect differences now.

3 years agoMINOR: action: add a function to dump the list of actions for a ruleset
Willy Tarreau [Wed, 30 Mar 2022 09:19:22 +0000 (11:19 +0200)] 
MINOR: action: add a function to dump the list of actions for a ruleset

The new function dump_act_rules() now dumps the list of actions supported
by a ruleset. These actions are alphanumerically sorted first so that the
produced output is easy to compare.

3 years agoMINOR: tools: add strordered() to check whether strings are ordered
Willy Tarreau [Wed, 30 Mar 2022 08:02:56 +0000 (10:02 +0200)] 
MINOR: tools: add strordered() to check whether strings are ordered

When trying to sort sets of strings, it's often needed to required to
compare 3 strings to see if the chosen one fits well between the two
others. That's what this function does, in addition to being able to
ignore extremities when they're NULL (typically for the first iteration
for example).

3 years agoMINOR: sample: list registered sample converter functions
Willy Tarreau [Tue, 29 Mar 2022 14:59:49 +0000 (16:59 +0200)] 
MINOR: sample: list registered sample converter functions

Similar to the sample fetch keywords, let's also list the converter
keywords. They're much simpler since there's no compatibility matrix.
Instead the input and output types are listed. This is called by
dump_registered_keywords() for the "cnv" keywords class.

3 years agoMINOR: samples: add a function to list register sample fetch keywords
Willy Tarreau [Tue, 29 Mar 2022 14:51:29 +0000 (16:51 +0200)] 
MINOR: samples: add a function to list register sample fetch keywords

New function smp_dump_fetch_kw lists registered sample fetch keywords
with their compatibility matrix, mandatory and optional argument types,
and output types. It's called from dump_registered_keywords() with class
"smp".

3 years agoMINOR: acl: add a function to dump the list of known ACL keywords
Willy Tarreau [Tue, 29 Mar 2022 13:36:56 +0000 (15:36 +0200)] 
MINOR: acl: add a function to dump the list of known ACL keywords

New function acl_dump_kwd() dumps the registered ACL keywords and their
sample-fetch equivalent to stdout. It's called by dump_registered_keywords()
for keyword class "acl".

3 years agoMINOR: cli: add a new keyword dump function
Willy Tarreau [Tue, 29 Mar 2022 13:25:30 +0000 (15:25 +0200)] 
MINOR: cli: add a new keyword dump function

New function cli_list_keywords() scans the list of registered CLI keywords
and dumps them on stdout. It's now called from dump_registered_keywords()
for the class "cli".

Some keywords are valid for the master, they'll be suffixed with
"[MASTER]". Others are valid for the worker, they'll have "[WORKER]".
Those accessible only in expert mode will show "[EXPERT]" and the
experimental ones will show "[EXPERIM]".

3 years agoMINOR: services: extend list_services() to dump to stdout
Willy Tarreau [Tue, 29 Mar 2022 13:10:44 +0000 (15:10 +0200)] 
MINOR: services: extend list_services() to dump to stdout

When no output stream is passed, stdout is used with one entry per line,
and this is called from dump_registered_services() when passed the class
"svc".

3 years agoMINOR: filters: extend flt_dump_kws() to dump to stdout
Willy Tarreau [Tue, 29 Mar 2022 13:03:09 +0000 (15:03 +0200)] 
MINOR: filters: extend flt_dump_kws() to dump to stdout

When passing a NULL output buffer the function will now dump to stdout
with a more compact format that is more suitable for machine processing.

An entry was added to dump_registered_keyword() to call it when the
keyword class "flt" is requested.

3 years agoMINOR: config: add a function to dump all known config keywords
Willy Tarreau [Tue, 29 Mar 2022 13:02:44 +0000 (15:02 +0200)] 
MINOR: config: add a function to dump all known config keywords

All registered config keywords that are valid in the config parser are
dumped to stdout organized like the regular sections (global, listen,
etc). Some keywords that are known to only be valid in frontends or
backends will be suffixed with [FE] or [BE].

All regularly registered "bind" and "server" keywords are also dumped,
one per "bind" or "server" line. Those depending on ssl are listed after
the "ssl" keyword. Doing so required to export the listener and server
keyword lists that were static.

The function is called from dump_registered_keywords() for keyword
class "cfg".

3 years agoMINOR: management: add some basic keyword dump infrastructure
Willy Tarreau [Tue, 8 Mar 2022 15:01:40 +0000 (16:01 +0100)] 
MINOR: management: add some basic keyword dump infrastructure

It's difficult from outside haproxy to detect the supported keywords
and syntax. Interestingly, many of our modern keywords are enumerated
since they're registered from constructors, so it's not very hard to
enumerate most of them.

This patch creates some basic infrastructure to support dumping existing
keywords from different classes on stdout. The format will differ depending
on the classes, but the idea is that the output could easily be passed to
a script that generates some simple syntax highlighting rules, completion
rules for editors, syntax checkers or config parsers.

The principle chosen here is that if "-dK" is passed on the command-line,
at the end of the parsing the registered keywords will be dumped for the
requested classes passed after "-dK". Special name "help" will show known
classes, while "all" will execute all of them. The reason for doing that
after the end of the config processor is that it will also enumerate
internally-generated keywords, Lua or even those loaded from external
code (e.g. if an add-on is loaded using LD_PRELOAD). A typical way to
call this with a valid config would be:

    ./haproxy -dKall -q -c -f /path/to/config

If there's no config available, feeding /dev/null will also do the job,
though it will not be able to detect dynamically created keywords, of
course.

This patch also updates the management doc.

For now nothing but the help is listed, various subsystems will follow
in subsequent patches.

3 years agoBUG/MINOR: samples: add missing context names for sample fetch functions
Willy Tarreau [Tue, 29 Mar 2022 14:39:24 +0000 (16:39 +0200)] 
BUG/MINOR: samples: add missing context names for sample fetch functions

In 2.4, two commits added support for supporting sample fetch calls from
new config and CLI contexts, but these were not added to the visibile
names, which may possibly cause "(null)" to appear in some error messages.
The commit in question were:
  db5e0dbea ("MINOR: sample: add a new CLI_PARSER context for samples")
  f9a7a8fd8 ("MINOR: sample: add a new CFG_PARSER context for samples")

This patch needs to be backported where these are present (2.4 and above).

3 years agoBUG/MINOR: log: Initialize the list element when allocating a new log server
Christopher Faulet [Tue, 29 Mar 2022 12:17:09 +0000 (14:17 +0200)] 
BUG/MINOR: log: Initialize the list element when allocating a new log server

211ea252d ("BUG/MINOR: logs: fix logsrv leaks on clean exit") introduced a
regression because the list element of a new log server is not intialized. Thus
HAProxy crashes on error path when an invalid log server is released.

This patch shoud fix the issue #1636. It must be backported if the above commit
is backported. For now, it is 2.6-specific and no backport is needed.

3 years agoBUG/MEDIUM: mux-h1: Properly detect full buffer cases during message parsing
Christopher Faulet [Mon, 28 Mar 2022 14:19:02 +0000 (16:19 +0200)] 
BUG/MEDIUM: mux-h1: Properly detect full buffer cases during message parsing

When the destination buffer is full while there are still data to parse, the
h1s must be marked as congested to be able to restart the parsing
later. This work on headers and data parsing. But on trailers parsing, we
fail to do so when the buffer is full before to parse the trailers. In this
case, we skip the trailers parsing but the h1s is not marked as
congested. This is important to be sure to wake up the mux to restart the
parsing when some room is made in the buffer.

Because of this bug, the message processing may hang till a timeout is
triggered. Note that for 2.3 and 2.2, the EOM processing is buggy too, for
the same reason. It should be fixed too on these versions. On the 2.0, only
trailers parsing is affected.

This patch must be backported as far as 2.0. On 2.3 and 2.2, the EOM parsing
must be fixed too.

3 years agoBUG/MEDIUM: mux-fcgi: Properly handle return value of headers/trailers parsing
Christopher Faulet [Mon, 28 Mar 2022 13:37:01 +0000 (15:37 +0200)] 
BUG/MEDIUM: mux-fcgi: Properly handle return value of headers/trailers parsing

h1_parse_msg_hdrs() and h1_parse_msg_tlrs() may return negative values if
the parsing fails or if more space is needed in the destination buffer. When
h1-htx was changed, The H1 mux was updated accordingly but not the FCGI
mux. Thus if a negative value is returned, it is ignored and it is casted to
a size_t, leading to an integer overflow on the <ofs> value, used to know
the position in the RX buffer.

This patch must be backported as far as 2.2.

3 years agoDOC: reflect H2 timeout changes
Lukas Tribus [Sat, 26 Mar 2022 19:43:48 +0000 (20:43 +0100)] 
DOC: reflect H2 timeout changes

Reverts 75df9d7a7 ("DOC: explain HTTP2 timeout behavior") since H2
connections now respect "timeout http-keep-alive".

If commit 15a4733d5d ("BUG/MEDIUM: mux-h2: make use of http-request
and keep-alive timeouts") is backported, this DOC change needs to
be backported along with it.

3 years ago[RELEASE] Released version 2.6-dev4 v2.6-dev4
Willy Tarreau [Sat, 26 Mar 2022 07:31:33 +0000 (08:31 +0100)] 
[RELEASE] Released version 2.6-dev4

Released version 2.6-dev4 with the following main changes :
    - BUG/MEDIUM: httpclient: don't consume data before it was analyzed
    - CLEANUP: htx: remove unused co_htx_remove_blk()
    - BUG/MINOR: httpclient: consume partly the blocks when necessary
    - BUG/MINOR: httpclient: remove the UNUSED block when parsing headers
    - BUG/MEDIUM: httpclient: must manipulate head, not first
    - REGTESTS: fix the race conditions in be2hex.vtc
    - BUG/MEDIUM: quic: Blocked STREAM when retransmitted
    - BUG/MAJOR: quic: Possible crash with full congestion control window
    - BUG/MINOR: httpclient/lua: stuck when closing without data
    - BUG/MEDIUM: applet: Don't call .release callback function twice
    - BUG/MEDIUM: cli/debug: Properly get the stream-int in all debug I/O handlers
    - BUG/MEDIUM: sink: Properly get the stream-int in appctx callback functions
    - DEV: udp: switch parser to getopt() instead of positional arguments
    - DEV: udp: add support for random packet corruption
    - MINOR: server: export server_parse_sni_expr() function
    - BUG/MINOR: httpclient: send the SNI using the host header
    - BUILD: httpclient: fix build without SSL
    - BUG/MINOR: server/ssl: free the SNI sample expression
    - BUG/MINOR: logs: fix logsrv leaks on clean exit
    - MINOR: actions: add new function free_act_rule() to free a single rule
    - BUG/MINOR: tcp-rules: completely free incorrect TCP rules on error
    - BUG/MINOR: http-rules: completely free incorrect TCP rules on error
    - BUG/MINOR: httpclient: only check co_data() instead of HTTP_MSG_DATA
    - BUG/MINOR: httpclient: process the response when received before the end of the request
    - BUG/MINOR: httpclient: CF_SHUTW_NOW should be tested with channel_is_empty()
    - CI: github actions: switch to LibreSSL-3.5.1
    - BUG/MEDIUM: mux-h1: only turn CO_FL_ERROR to CS_FL_ERROR with empty ibuf
    - BUG/MEDIUM: stream-int: do not rely on the connection error once established
    - BUG/MEDIUM: trace: avoid race condition when retrieving session from conn->owner
    - MEDIUM: mux-h2: slightly relax timeout management rules
    - BUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts
    - BUG/MINOR: rules: Initialize the list element when allocating a new rule
    - BUG/MINOR: http-rules: Don't free new rule on allocation failure
    - DEV: coccinelle: Fix incorrect replacement in ist.cocci
    - CLEANUP: Reapply ist.cocci with `--include-headers-for-types --recursive-includes`
    - DEV: coccinelle: Add a new pattern to ist.cocci
    - CLEANUP: Reapply ist.cocci
    - REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+
    - MINOR: quic: Code factorization (TX buffer reuse)
    - CLEANUP: quic: "largest_acked_pn" pktns struc member moving
    - MEDIUM: quic: Limit the number of ACK ranges
    - MEDIUM: quic: Rework of the TX packets memory handling
    - BUG/MINOR: quic: Possible crash in parse_retry_token()
    - BUG/MINOR: quic: Possible leak in quic_build_post_handshake_frames()
    - BUG/MINOR: quic: Unsent frame because of qc_build_frms()
    - BUG/MINOR: mux-quic: Access to empty frame list from qc_send_frames()
    - BUG/MINOR: mux-quic: Missing I/O handler events initialization
    - BUG/MINOR: quic: Missing TX packet initializations
    - BUG/MINOR: quic: 1RTT packets ignored after mux was released
    - BUG/MINOR: quic: Incorrect peer address validation
    - BUG/MINOR: quic: Non initialized variable in quic_build_post_handshake_frames()
    - BUG/MINOR: quic: Wrong TX packet related counters handling
    - MEDIUM: mqtt: support mqtt_is_valid and mqtt_field_value converters for MQTTv3.1
    - DOC: config: Explictly add supported MQTT versions
    - MINOR: quic: Add traces about stream TX buffer consumption
    - MINOR: quic: Add traces in qc_set_timer() (scheduling)
    - CLEANUP: mux-quic: change comment style to not mess with git conflict
    - CLEANUP: mux-quic: adjust comment for coding-style
    - MINOR: mux-quic: complete trace when stream is not found
    - MINOR: mux-quic: add comments for send functions
    - MINOR: mux-quic: use shorter name for flow-control fields
    - MEDIUM: mux-quic: respect peer bidirectional stream data limit
    - MEDIUM: mux-quic: respect peer connection data limit
    - MINOR: mux-quic: support MAX_STREAM_DATA frame parsing
    - MINOR: mux-quic: support MAX_DATA frame parsing
    - BUILD: stream-int: avoid a build warning when DEBUG is empty
    - BUG/MINOR: quic: Wrong buffer length passed to generate_retry_token()
    - BUG/MINOR: tools: fix url2sa return value with IPv4
    - MINOR: mux-quic: convert fin on push-frame as boolean
    - BUILD: quic: add missing includes
    - REORG: quic: use a dedicated quic_loss.c
    - MINOR: mux-quic: declare the qmux trace module
    - MINOR: mux-quic: replace printfs by traces
    - MINOR: mux-quic: add trace event for frame sending
    - MINOR: mux-quic: add trace event for qcs_push_frame
    - MINOR: mux-quic: activate qmux traces on stdout via macro
    - BUILD: qpack: fix unused value when not using DEBUG_HPACK
    - CLEANUP: qpack: suppress by default stdout traces
    - CLEANUP: h3: suppress by default stdout traces
    - BUG/MINOR: tools: url2sa reads too far when no port nor path

3 years agoBUG/MINOR: tools: url2sa reads too far when no port nor path
William Lallemand [Fri, 25 Mar 2022 16:37:51 +0000 (17:37 +0100)] 
BUG/MINOR: tools: url2sa reads too far when no port nor path

url2sa() still have an unfortunate case where it reads 1 byte too far,
it happens when no port or path are specified in the URL, and could
crash if the byte after the URL is not allocated (mostly with ASAN).

This case is never triggered in old versions of haproxy because url2sa
is used with buffers which are way bigger than the URL. It is only
triggered with the httpclient.

Should be bacported in every stable branches.

3 years agoCLEANUP: h3: suppress by default stdout traces
Amaury Denoyelle [Fri, 25 Mar 2022 14:28:33 +0000 (15:28 +0100)] 
CLEANUP: h3: suppress by default stdout traces

H3_DEBUG definition is removed from h3.c similarly to the commit
  d96361b2703a6364c1116af76016f09807b4c65b
  CLEANUP: qpack: suppress by default stdout traces

Also, a plain fprintf in h3_snd_buf has been replaced to be conditional
to the H3_DEBUG definition.

These changes reduces the default output on stdout with QUIC traffic.

3 years agoCLEANUP: qpack: suppress by default stdout traces
Amaury Denoyelle [Fri, 25 Mar 2022 13:56:51 +0000 (14:56 +0100)] 
CLEANUP: qpack: suppress by default stdout traces

Remove the definition of DEBUG_HPACK on qpack-dec.c which forces the
QPACK decoding traces on stderr. Also change the name to use a dedicated
one for QPACK decoding as DEBUG_QPACK.

3 years agoBUILD: qpack: fix unused value when not using DEBUG_HPACK
Amaury Denoyelle [Fri, 25 Mar 2022 14:11:38 +0000 (15:11 +0100)] 
BUILD: qpack: fix unused value when not using DEBUG_HPACK

If the macro is not defined, some local variables are flagged as unused
by the compiler. Fix this by using the __maybe_unused attribute.

For now, the macro is defined in the qpack-dec.c. However, this will
change to not mess up the stderr output of haproxy with QUIC traffic.

3 years agoMINOR: mux-quic: activate qmux traces on stdout via macro
Amaury Denoyelle [Thu, 24 Mar 2022 16:14:52 +0000 (17:14 +0100)] 
MINOR: mux-quic: activate qmux traces on stdout via macro

This commit is similar to the following one :
  commit 118b2cbf8430a9513947c27a8403ff380e1dcaf2
  MINOR: quic: activate QUIC traces at compilation

If the macro ENABLE_QUIC_STDOUT_TRACES is defined, qmux traces are
outputted automatically on stdout. This is useful for the haproxy-qns
interop docker image.

3 years agoMINOR: mux-quic: add trace event for qcs_push_frame
Amaury Denoyelle [Fri, 25 Mar 2022 08:28:10 +0000 (09:28 +0100)] 
MINOR: mux-quic: add trace event for qcs_push_frame

Add a new qmux trace event QMUX_EV_QCS_PUSH_FRM. Its only purpose is to
display the meaningful result of a qcs_push_frame invocation.

A dedicated struct qcs_push_frm_trace_arg is defined to pass a series of
extra args for the trace output.

3 years agoMINOR: mux-quic: add trace event for frame sending
Amaury Denoyelle [Fri, 25 Mar 2022 08:09:40 +0000 (09:09 +0100)] 
MINOR: mux-quic: add trace event for frame sending

Define a new qmux event QMUX_EV_SEND_FRM. This allows to pass a
quic_frame as an extra argument. Depending on the frame type, a special
format can be used to log the frame content.

Currently this event is only used in qc_send_max_streams. Thus the
handler is only able to handle MAX_STREAMS frames.

3 years agoMINOR: mux-quic: replace printfs by traces
Amaury Denoyelle [Thu, 24 Mar 2022 16:10:00 +0000 (17:10 +0100)] 
MINOR: mux-quic: replace printfs by traces

Convert all printfs in the mux-quic code with traces.

Note that some meaningul printfs were not converted because they use
extra args in a format-string. This is the case inside qcs_push_frame
and qc_send_max_streams. A dedicated trace event should be implemented
for them to be able to display the extra arguments.

3 years agoMINOR: mux-quic: declare the qmux trace module
Amaury Denoyelle [Thu, 24 Mar 2022 15:09:16 +0000 (16:09 +0100)] 
MINOR: mux-quic: declare the qmux trace module

Declare a new trace module for mux-quic named qmux. It will be used to
convert all printf to regular traces. The handler qmux_trace can uses a
connection and a qcs instance as extra arguments.

3 years agoREORG: quic: use a dedicated quic_loss.c
Amaury Denoyelle [Thu, 24 Mar 2022 15:08:05 +0000 (16:08 +0100)] 
REORG: quic: use a dedicated quic_loss.c

Move all inline functions with trace from quic_loss.h to a dedicated
object file. This let to remove the TRACE_SOURCE macro definition
outside of the include file.

This change is required to be able to define another TRACE_SOUCE inside
the mux_quic.c for a dedicated trace module.

3 years agoBUILD: quic: add missing includes
Amaury Denoyelle [Thu, 24 Mar 2022 15:06:26 +0000 (16:06 +0100)] 
BUILD: quic: add missing includes

Complete the include list for the files quic_loss.h and quic_sock.c.

3 years agoMINOR: mux-quic: convert fin on push-frame as boolean
Amaury Denoyelle [Thu, 24 Mar 2022 17:23:29 +0000 (18:23 +0100)] 
MINOR: mux-quic: convert fin on push-frame as boolean

This is only useful to display a clear 0/1 value in the traces. This has
no impact beyond this cosmetic change.

3 years agoBUG/MINOR: tools: fix url2sa return value with IPv4
William Lallemand [Thu, 24 Mar 2022 20:59:03 +0000 (21:59 +0100)] 
BUG/MINOR: tools: fix url2sa return value with IPv4

Fix 8a91374 ("BUG/MINOR: tools: url2sa reads ipv4 too far") introduced a
regression in the value returned when parsing an ipv4 host.

Tthe consumed length is supposed to be as far as the first character of
the path, only its not computed correctly anymore and return the length
minus the size of the scheme.

Fixed the issue by reverting 'curr' and 'url' as they were before the
patch.

Must be backported in every stable branch where the 8a91374 patch was
backported.

3 years agoBUG/MINOR: quic: Wrong buffer length passed to generate_retry_token()
Frédéric Lécaille [Wed, 23 Mar 2022 13:09:09 +0000 (14:09 +0100)] 
BUG/MINOR: quic: Wrong buffer length passed to generate_retry_token()

After having consumed <i> bytes from <buf>, the remaining available room to be
passed to generate_retry_token() is sizeof(buf) - i.
This bug could be easily reproduced with quic-qo as client which chooses a random
value as ODCID length.

3 years agoBUILD: stream-int: avoid a build warning when DEBUG is empty
Willy Tarreau [Wed, 23 Mar 2022 10:11:31 +0000 (11:11 +0100)] 
BUILD: stream-int: avoid a build warning when DEBUG is empty

When no DEBUG_STRICT is enabled, we get this build warning:

  src/stream_interface.c: In function 'stream_int_chk_snd_conn':
  src/stream_interface.c:1198:28: warning: unused variable 'conn' [-Wunused-variable]
   1198 |         struct connection *conn = cs_conn(cs);
        |                            ^~~~

This was the result of the simplification of the code in commit
d1480cc8a ("BUG/MEDIUM: stream-int: do not rely on the connection error
once established") which removed the last user of this variable outside
of a BUG_ON().

If the patch above is backported, this one should be backported as well.

3 years agoMINOR: mux-quic: support MAX_DATA frame parsing
Amaury Denoyelle [Tue, 8 Mar 2022 15:23:03 +0000 (16:23 +0100)] 
MINOR: mux-quic: support MAX_DATA frame parsing

This commit is similar to the previous one but with MAX_DATA frames.
This allows to increase the connection level flow-control limit. If the
connection was blocked due to QC_CF_BLK_MFCTL flag, the flag is reseted.

3 years agoMINOR: mux-quic: support MAX_STREAM_DATA frame parsing
Amaury Denoyelle [Tue, 8 Mar 2022 09:39:55 +0000 (10:39 +0100)] 
MINOR: mux-quic: support MAX_STREAM_DATA frame parsing

Implement a MUX method to parse MAX_STREAM_DATA. If the limit is greater
than the previous one and the stream was blocked, the flag
QC_SF_BLK_SFCTL is removed.

3 years agoMEDIUM: mux-quic: respect peer connection data limit
Amaury Denoyelle [Tue, 8 Mar 2022 09:35:42 +0000 (10:35 +0100)] 
MEDIUM: mux-quic: respect peer connection data limit

This commit is similar to the previous one, but this time on the
connection level instead of the stream.

When the connection limit is reached, the connection is flagged with
QC_CF_BLK_MFCTL. This flag is checked in qc_send.

qcs_push_frame uses a new parameter which is used to not exceed the
connection flow-limit while calling it repeatdly over multiple streams
instance before transfering data to the transport layer.

3 years agoMEDIUM: mux-quic: respect peer bidirectional stream data limit
Amaury Denoyelle [Mon, 7 Mar 2022 14:47:02 +0000 (15:47 +0100)] 
MEDIUM: mux-quic: respect peer bidirectional stream data limit

Implement the flow-control max-streams-data limit on emission. We ensure
that we never push more than the offset limit set by the peer. When the
limit is reached, the stream is marked as blocked with a new flag
QC_SF_BLK_SFCTL to disable emission.

Currently, this is only implemented for bidirectional streams. It's
required to unify the sending for unidirectional streams via
qcs_push_frame from the H3 layer to respect the flow-control limit for
them.

3 years agoMINOR: mux-quic: use shorter name for flow-control fields
Amaury Denoyelle [Mon, 21 Mar 2022 16:13:32 +0000 (17:13 +0100)] 
MINOR: mux-quic: use shorter name for flow-control fields

Rename the fields used for flow-control in the qcc structure. The
objective is to have shorter name for better readability while keeping
their purpose clear. It will be useful when the flow-control will be
extended with new fields.

3 years agoMINOR: mux-quic: add comments for send functions
Amaury Denoyelle [Tue, 22 Mar 2022 14:10:29 +0000 (15:10 +0100)] 
MINOR: mux-quic: add comments for send functions

Add comments on qc_send and qcs_push_frame. Also adjust the return of
qc_send to reflect the total bytes sent. This has no impact as currently
the return value is not checked by the caller.

3 years agoMINOR: mux-quic: complete trace when stream is not found
Amaury Denoyelle [Tue, 22 Mar 2022 15:42:10 +0000 (16:42 +0100)] 
MINOR: mux-quic: complete trace when stream is not found

Display the ID of the stream not found. This will help to detect when we
received retransmitted frames for an already closed stream.

3 years agoCLEANUP: mux-quic: adjust comment for coding-style
Amaury Denoyelle [Mon, 7 Mar 2022 14:16:56 +0000 (15:16 +0100)] 
CLEANUP: mux-quic: adjust comment for coding-style

Replace single-line comment style by /* ... */ format which is the
standard for haproxy documentation.

3 years agoCLEANUP: mux-quic: change comment style to not mess with git conflict
Amaury Denoyelle [Fri, 11 Mar 2022 18:12:23 +0000 (19:12 +0100)] 
CLEANUP: mux-quic: change comment style to not mess with git conflict

Remove "=======" symbols from the MUX buffer diagram. This is useful to
not mess with git conflict markers when resolving a conflict.

3 years agoMINOR: quic: Add traces in qc_set_timer() (scheduling)
Frédéric Lécaille [Tue, 22 Mar 2022 14:37:41 +0000 (15:37 +0100)] 
MINOR: quic: Add traces in qc_set_timer() (scheduling)

This should be helpful to diagnose some issues: timer task not
run when it should run.

3 years agoMINOR: quic: Add traces about stream TX buffer consumption
Frédéric Lécaille [Tue, 22 Mar 2022 11:45:33 +0000 (12:45 +0100)] 
MINOR: quic: Add traces about stream TX buffer consumption

This will be helpful to diagnose STREAM blocking states.

3 years agoDOC: config: Explictly add supported MQTT versions
Christopher Faulet [Tue, 22 Mar 2022 08:41:11 +0000 (09:41 +0100)] 
DOC: config: Explictly add supported MQTT versions

This avoids any ambiguities on supported versions. This patch depends on
129579813 ("MEDIUM: mqtt: support mqtt_is_valid and mqtt_field_value
converters for MQTTv3.1").

It must be backported with the above commit.

3 years agoMEDIUM: mqtt: support mqtt_is_valid and mqtt_field_value converters for MQTTv3.1
Dhruv Jain [Mon, 21 Mar 2022 14:34:00 +0000 (20:04 +0530)] 
MEDIUM: mqtt: support mqtt_is_valid and mqtt_field_value converters for MQTTv3.1

In MQTTv3.1, protocol name is "MQIsdp" and protocol level is 3. The mqtt
converters(mqtt_is_valid and mqtt_field_value) did not work for clients on
mqttv3.1 because the mqtt_parse_connect() marked the CONNECT message invalid
if either the protocol name is not "MQTT" or the protocol version is other than
v3.1.1 or v5.0. To fix it, we have added the mqttv3.1 protocol name and version
as part of the checks.

This patch fixes the mqtt converters to support mqttv3.1 clients as well (issue #1600).
It must be backported to 2.4.

3 years agoBUG/MINOR: quic: Wrong TX packet related counters handling
Frédéric Lécaille [Mon, 21 Mar 2022 15:12:19 +0000 (16:12 +0100)] 
BUG/MINOR: quic: Wrong TX packet related counters handling

During the packet number space discarding, do no reset tx.in_flight counter
before decrement it from other variables.
Furthermore path prep_in_flight counter was not decremented.

3 years agoBUG/MINOR: quic: Non initialized variable in quic_build_post_handshake_frames()
Frédéric Lécaille [Mon, 21 Mar 2022 11:01:22 +0000 (12:01 +0100)] 
BUG/MINOR: quic: Non initialized variable in quic_build_post_handshake_frames()

<cid> could be accessed before being initialized.

3 years agoBUG/MINOR: quic: Incorrect peer address validation
Frédéric Lécaille [Mon, 21 Mar 2022 11:18:00 +0000 (12:18 +0100)] 
BUG/MINOR: quic: Incorrect peer address validation

We must consider the peer address as validated as soon as we received an
handshake packet. An ACK frame in handshake packet was too restrictive.
Rename the concerned flag to reflect this situation.

3 years agoBUG/MINOR: quic: 1RTT packets ignored after mux was released
Frédéric Lécaille [Mon, 21 Mar 2022 10:37:13 +0000 (11:37 +0100)] 
BUG/MINOR: quic: 1RTT packets ignored after mux was released

We must be able to handle 1RTT packets after the mux has terminated its job
(qc->mux_state == QC_MUX_RELEASED). So the condition (qc->mux_state != QC_MUX_READY)
in qc_qel_may_rm_hp() is not correct when we want to wait for the mux to be started.
Add a check in qc_parse_pkt_frms() to ensure is started before calling it. All
the STREAM frames will be ignored when the mux will be released.

3 years agoBUG/MINOR: quic: Missing TX packet initializations
Frédéric Lécaille [Mon, 21 Mar 2022 09:43:53 +0000 (10:43 +0100)] 
BUG/MINOR: quic: Missing TX packet initializations

The most important one is the ->flags member which leads to an erratic xprt behavior.
For instance a non ack-eliciting packet could be seen as ack-eliciting leading the
xprt to try to retransmit a packet which are not ack-eliciting. In this case, the
xprt does nothing and remains indefinitively in a blocking state.

3 years agoBUG/MINOR: mux-quic: Missing I/O handler events initialization
Frédéric Lécaille [Fri, 18 Mar 2022 21:49:22 +0000 (22:49 +0100)] 
BUG/MINOR: mux-quic: Missing I/O handler events initialization

This could lead to a mux erratic behavior. Sometimes the application layer could
not wakeup the mux I/O handler because it estimated it had already subscribed
to write events (see h3_snd_buf() end of implementation).

3 years agoBUG/MINOR: mux-quic: Access to empty frame list from qc_send_frames()
Frédéric Lécaille [Fri, 18 Mar 2022 17:38:19 +0000 (18:38 +0100)] 
BUG/MINOR: mux-quic: Access to empty frame list from qc_send_frames()

This was revealed by libasan when each time qc_send_frames() is run at the first
time:

=================================================================
==84177==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fbaaca2b3c8 at pc 0x560a4fdb7c2e bp 0x7fbaaca2b300 sp 0x7fbaaca2b2f8
READ of size 1 at 0x7fbaaca2b3c8 thread T6
    #0 0x560a4fdb7c2d in qc_send_frames src/mux_quic.c:473
    #1 0x560a4fdb83be in qc_send src/mux_quic.c:563
    #2 0x560a4fdb8a6e in qc_io_cb src/mux_quic.c:638
    #3 0x560a502ab574 in run_tasks_from_lists src/task.c:580
    #4 0x560a502ad589 in process_runnable_tasks src/task.c:883
    #5 0x560a501e3c88 in run_poll_loop src/haproxy.c:2675
    #6 0x560a501e4519 in run_thread_poll_loop src/haproxy.c:2846
    #7 0x7fbabd120ea6 in start_thread nptl/pthread_create.c:477
    #8 0x7fbabcb19dee in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfddee)

Address 0x7fbaaca2b3c8 is located in stack of thread T6 at offset 56 in frame
    #0 0x560a4fdb7f00 in qc_send src/mux_quic.c:514

  This frame has 1 object(s):
    [32, 48) 'frms' (line 515) <== Memory access at offset 56 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
Thread T6 created by T0 here:
    #0 0x7fbabd1bd2a2 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:214
    #1 0x560a5036f9b8 in setup_extra_threads src/thread.c:221
    #2 0x560a501e70fd in main src/haproxy.c:3457
    #3 0x7fbabca42d09 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: stack-buffer-overflow src/mux_quic.c:473 in qc_send_frames

3 years agoBUG/MINOR: quic: Unsent frame because of qc_build_frms()
Frédéric Lécaille [Fri, 18 Mar 2022 16:49:29 +0000 (17:49 +0100)] 
BUG/MINOR: quic: Unsent frame because of qc_build_frms()

There are non already identified rare cases where qc_build_frms() does not manage
to size frames to be encoded in a packet leading qc_build_frm() to fail to add
such frame to the packet to be built. In such cases we must move back such
frames to their origin frame list passed as parameter to qc_build_frms(): <frms>.
because they were added to the packet frame list (but not built). If this
this packet is not retransmitted, the frame is lost for ever! Furthermore we must
not modify the buffer.

3 years agoBUG/MINOR: quic: Possible leak in quic_build_post_handshake_frames()
Frédéric Lécaille [Fri, 18 Mar 2022 16:45:28 +0000 (17:45 +0100)] 
BUG/MINOR: quic: Possible leak in quic_build_post_handshake_frames()

Rework this function to leave the connection passed as parameter in the same state
it was before entering this function.

3 years agoBUG/MINOR: quic: Possible crash in parse_retry_token()
Frédéric Lécaille [Thu, 17 Mar 2022 15:22:02 +0000 (16:22 +0100)] 
BUG/MINOR: quic: Possible crash in parse_retry_token()

We must check the decoded length of this incoming data before copying into our
internal structure. This could lead to crashes.
Reproduced with such a packet captured from QUIC interop.
    {
    0xc5, 0x00, 0x00, 0x00, 0x01, 0x12, 0xf2, 0x65,
0x4d, 0x9d, 0x58, 0x90, 0x23, 0x7e, 0x67, 0xef,
0xf8, 0xef, 0x5b, 0x87, 0x48, 0xbe, 0xde, 0x7a, /* corrupted byte: 0x11, */
0x01, 0xdc, 0x41, 0xbf, 0xfb, 0x07, 0x39, 0x9f,
0xfd, 0x96, 0x67, 0x5f, 0x58, 0x03, 0x57, 0x74,
0xc7, 0x26, 0x00, 0x45, 0x25, 0xdc, 0x7f, 0xf1,
0x22, 0x1d,
}

3 years agoMEDIUM: quic: Rework of the TX packets memory handling
Frédéric Lécaille [Thu, 17 Mar 2022 10:28:10 +0000 (11:28 +0100)] 
MEDIUM: quic: Rework of the TX packets memory handling

The TX packet refcounting had come with the multithreading support but not only.
It is very useful to ease the management of the memory allocated for TX packets
with TX frames attached to. At some locations of the code we have to move TX
frames from a packet to a new one during retranmission when the packet has been
deemed as lost or not. When deemed lost the memory allocated for the paquet must
be released contrary to when its frames are retransmitted when probing (PTO).

For now on, thanks to this patch we handle the TX packets memory this way. We
increment the packet refcount when:
  - we insert it in its packet number space tree,
  - we attache an ack-eliciting frame to it.
And reciprocally we decrement this refcount when:
  - we remove an ack-eliciting frame from the packet,
  - we delete the packet from its packet number space tree.

Note that an optimization WOULD NOT be to fully reuse (without releasing its
memorya TX packet to retransmit its contents (its ack-eliciting frames). Its
information (timestamp, in flight length) to be processed by packet loss detection
and the congestion control.

3 years agoMEDIUM: quic: Limit the number of ACK ranges
Frédéric Lécaille [Tue, 15 Mar 2022 17:44:20 +0000 (18:44 +0100)] 
MEDIUM: quic: Limit the number of ACK ranges

When building a packet with an ACK frame, we store the largest acknowledged
packet number sent in this frame in the packet (quic_tx_packet struc).
When receiving an ack for such a packet we can purge the tree of acknowledged
packet number ranges from the range sent before this largest acknowledged
packet number.

3 years agoCLEANUP: quic: "largest_acked_pn" pktns struc member moving
Frédéric Lécaille [Tue, 15 Mar 2022 11:07:41 +0000 (12:07 +0100)] 
CLEANUP: quic: "largest_acked_pn" pktns struc member moving

This struct member stores the largest acked packet number which was received. It
is used to build (TX) packet. But this is confusing to store it in the tx packet
of the packet number space structure even if it is used to build and transmit
packets.

3 years agoMINOR: quic: Code factorization (TX buffer reuse)
Frédéric Lécaille [Mon, 14 Mar 2022 11:21:03 +0000 (12:21 +0100)] 
MINOR: quic: Code factorization (TX buffer reuse)

Add qc_may_reuse_cbuf() function used by qc_prep_pkts() and qc_prep_app_pkts().
Simplification of the factorized section code: there is no need to check there
is enough room to mark the end of the data in the TX buf. This is done by
the callers (qc_prep_pkts() and qc_prep_app_pkts()). Add a diagram to explain
the conditions which must be verified to be able to reuse a cbuf struct.

This should improve the QUIC stack implementation maintenability.

3 years agoREGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+
Tim Duesterhus [Fri, 11 Mar 2022 21:46:16 +0000 (22:46 +0100)] 
REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+

Introduced in:

0657b9338 MINOR: stream: add "last_rule_file" and "last_rule_line" samples

3 years agoCLEANUP: Reapply ist.cocci
Tim Duesterhus [Tue, 15 Mar 2022 12:11:08 +0000 (13:11 +0100)] 
CLEANUP: Reapply ist.cocci

This makes use of the newly added:

    - i.ptr = p;
    - i.len = strlen(i.ptr);
    + i = ist(p);

patch.

3 years agoDEV: coccinelle: Add a new pattern to ist.cocci
Tim Duesterhus [Tue, 15 Mar 2022 12:11:07 +0000 (13:11 +0100)] 
DEV: coccinelle: Add a new pattern to ist.cocci

This was previously ignored in "DEV: coccinelle: Fix incorrect replacement in ist.cocci",
but is now properly replaced by a simple `ist()` call.

3 years agoCLEANUP: Reapply ist.cocci with `--include-headers-for-types --recursive-includes`
Tim Duesterhus [Tue, 15 Mar 2022 12:11:06 +0000 (13:11 +0100)] 
CLEANUP: Reapply ist.cocci with `--include-headers-for-types --recursive-includes`

Previous uses of `ist.cocci` did not add `--include-headers-for-types` and
`--recursive-includes` preventing Coccinelle seeing `struct ist` members of
other structs.

Reapply the patch with proper flags to further clean up the use of the ist API.

The command used was:

    spatch -sp_file dev/coccinelle/ist.cocci -in_place --include-headers --include-headers-for-types --recursive-includes --dir src/

3 years agoDEV: coccinelle: Fix incorrect replacement in ist.cocci
Tim Duesterhus [Tue, 15 Mar 2022 12:11:05 +0000 (13:11 +0100)] 
DEV: coccinelle: Fix incorrect replacement in ist.cocci

We must not use `ist2()` if the value of `i.len` is derived from the value of
`i.ptr`:

    i.ptr = "foo";
    i.len = strlen(i.ptr);

3 years agoBUG/MINOR: http-rules: Don't free new rule on allocation failure
Christopher Faulet [Mon, 21 Mar 2022 07:21:19 +0000 (08:21 +0100)] 
BUG/MINOR: http-rules: Don't free new rule on allocation failure

If allocation of a new HTTP rule fails, we must not release it calling
free_act_rule(). The regression was introduced by the commit dd7e6c6dc
("BUG/MINOR: http-rules: completely free incorrect TCP rules on error").

This patch must only be backported if the commit above is backported. It should
fix the issues #1627, #1628 and #1629.

3 years agoBUG/MINOR: rules: Initialize the list element when allocating a new rule
Christopher Faulet [Mon, 21 Mar 2022 06:55:34 +0000 (07:55 +0100)] 
BUG/MINOR: rules: Initialize the list element when allocating a new rule

dd7e6c6dc ("BUG/MINOR: http-rules: completely free incorrect TCP rules on
error") and 388c0f2a6 ("BUG/MINOR: tcp-rules: completely free incorrect TCP
rules on error") introduced a regression because the list element of a new
rule is not intialized. Thus HAProxy crashes when an incorrect rule is
released.

This patch must be backported if above commits are backported. Note that
new_act_rule() only exists since the 2.5. It relies on the commit d535f807b
("MINOR: rules: add a new function new_act_rule() to allocate act_rules").

3 years agoBUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts
Willy Tarreau [Fri, 18 Mar 2022 14:57:34 +0000 (15:57 +0100)] 
BUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts

Christian Ruppert reported an issue explaining that it's not possible to
forcefully close H2 connections which do not receive requests anymore if
they continue to send control traffic (window updates, ping etc). This
will indeed refresh the timeout. In H1 we don't have this problem because
any single byte is part of the stream, so the control frames in H2 would
be equivalent to TCP acks in H1, that would not contribute to the timeout
being refreshed.

What misses from H2 is the use of http-request and keep-alive timeouts.
These were not implemented because initially it was hard to see how they
could map to H2. But if we consider the real use of the keep-alive timeout,
that is, how long do we keep a connection alive with no request, then it's
pretty obvious that it does apply to H2 as well. Similarly, http-request
may definitely be honored as soon as a HEADERS frame starts to appear
while there is no stream. This will also allow to deal with too long
CONTINUATION frames.

This patch moves the timeout update to a new function, h2c_update_timeout(),
which is in charge of this. It also adds an "idle_start" timestamp in the
connection, which is set when nb_cs reaches zero or when a headers frame
start to arrive, so that it cannot be delayed too long.

This patch should be backported to recent stable releases after some
observation time. It depends on previous patch "MEDIUM: mux-h2: slightly
relax timeout management rules".

3 years agoMEDIUM: mux-h2: slightly relax timeout management rules
Willy Tarreau [Fri, 18 Mar 2022 13:59:54 +0000 (14:59 +0100)] 
MEDIUM: mux-h2: slightly relax timeout management rules

The H2 timeout rules were arranged to cover complex situations In 2.1
with commit c2ea47fb1 ("BUG/MEDIUM: mux-h2: do not enforce timeout on
long connections").

It turns out that such rules while complex, do not perfectly cover all
use cases. The real intent is to say that as long as there are attached
streams, the connection must not timeout. Then once all these streams
have quit (possibly for timeout reasons) then the mux should take over
the management of timeouts.

We do have this nb_cs field which indicates the number of attached
streams, and it's updated even when leaving orphaned streams. So
checking it alone is sufficient to know whether it's the mux or the
streams that are in charge of the timeouts.

In its current state, this doesn't cause visible effects except that
it makes it impossible to implement more subtle parsing timeouts.

This would need to be backported as far as 2.0 along with the next
commit that will depend on it.

3 years agoBUG/MEDIUM: trace: avoid race condition when retrieving session from conn->owner
Willy Tarreau [Fri, 18 Mar 2022 16:37:20 +0000 (17:37 +0100)] 
BUG/MEDIUM: trace: avoid race condition when retrieving session from conn->owner

There's a rare race condition possible when trying to retrieve session from
a back connection's owner, that was fixed in 2.4 and described in commit
3aab17bd5 ("BUG/MAJOR: connection: reset conn->owner when detaching from
session list").

It also affects the trace code which does the same, so the same fix is
needed, i.e. check from conn->session_list that the connection is still
enlisted. It's visible when sending a few tens to hundreds of parallel
requests to an h2 backend and enabling traces in parallel.

This should be backported as far as 2.2 which is the oldest version
supporting traces.

3 years agoBUG/MEDIUM: stream-int: do not rely on the connection error once established
Willy Tarreau [Thu, 17 Mar 2022 15:19:09 +0000 (16:19 +0100)] 
BUG/MEDIUM: stream-int: do not rely on the connection error once established

Historically the stream-interface code used to check for connection
errors by itself. Later this was partially deferred to muxes, but
only once the mux is installed or the connection is at least in the
established state. But probably as a safety practice the connection
error tests remained.

The problem is that they are causing trouble on when a response received
from a mux is mixed with an error report. The typical case is an upload
that is interrupted by the server sending an error or redirect without
draining all data, causing an RST to be queued just after the data. In
this case the mux has the data, the CO_FL_ERROR flag is present on the
connection, and unfortunately the stream-interface refuses to retrieve
the data due to this flag, and return an error to the client.

It's about time to only rely on CS_FL_ERROR which is set by the mux, but
the stream-interface is still responsible for the connection during its
setup. However everywhere the CO_FL_ERROR is checked, CS_FL_ERROR is
also checked.

This commit addresses this by:
  - adding a new function si_is_conn_error() that checks the SI state
    and only reports the status of CO_FL_ERROR for states before
    SI_ST_EST.

  - eliminating all checks for CO_FL_ERORR in places where CS_FL_ERROR
    is already checked and either the presence of a mux was already
    validated or the stream-int's state was already checked as being
    SI_ST_EST or higher.

CO_FL_ERROR tests on the send() direction are also inappropriate as they
may cause the loss of pending data. Now this doesn't happen anymore and
such events are only converted to CS_FL_ERROR by the mux once notified of
the problem. As such, this must not cause the loss of any error event.

Now an early error reported on a backend mux doesn't prevent the queued
response from being read and forwarded to the client (the list of syscalls
below was trimmed and epoll_ctl is not represented):

  recvfrom(10, "POST / HTTP/1.1\r\nConnection: clo"..., 16320, 0, NULL, NULL) = 66
  sendto(11, "POST / HTTP/1.1\r\ntransfer-encodi"..., 47, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 47
  epoll_wait(3, [{events=EPOLLIN|EPOLLERR|EPOLLHUP|EPOLLRDHUP, data={u32=11, u64=11}}], 200, 15001) = 1
  recvfrom(11, "HTTP/1.1 200 OK\r\ncontent-length:"..., 16320, 0, NULL, NULL) = 57
  sendto(10, "HTTP/1.1 200 OK\r\ncontent-length:"..., 57, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 57
  epoll_wait(3, [{events=EPOLLIN|EPOLLERR|EPOLLHUP|EPOLLRDHUP, data={u32=11, u64=11}}], 200, 13001) = 1
  epoll_wait(3, [{events=EPOLLIN, data={u32=10, u64=10}}], 200, 13001) = 1
  recvfrom(10, "A\n0123456789\r\n0\r\n\r\n", 16320, 0, NULL, NULL) = 19
  shutdown(10, SHUT_WR)                   = 0
  close(11)                               = 0
  close(10)                               = 0

Above the server is an haproxy configured with the following:

   listen blah
        bind :8002
        mode http
        timeout connect 5s
        timeout client  5s
        timeout server  5s
        option httpclose
        option nolinger
        http-request return status 200 hdr connection close

And the client takes care of sending requests and data in two distinct
parts:

   while :; do
     ./dev/tcploop/tcploop 8001 C T S:"POST / HTTP/1.1\r\nConnection: close\r\nTransfer-encoding: chunked\r\n\r\n" P1 S:"A\n0123456789\r\n0\r\n\r\n" P R F;
   done

With this, a small percentage of the requests will reproduce the behavior
above. Note that this fix requires the following patch to be applied for
the test above to work:

  BUG/MEDIUM: mux-h1: only turn CO_FL_ERROR to CS_FL_ERROR with empty ibuf

This should be backported with after a few weeks of observation, and
likely one version at a time. During the backports, the patch might
need to be adjusted at each check of CO_FL_ERORR to follow the
principles explained above.

3 years agoBUG/MEDIUM: mux-h1: only turn CO_FL_ERROR to CS_FL_ERROR with empty ibuf
Willy Tarreau [Thu, 17 Mar 2022 16:10:36 +0000 (17:10 +0100)] 
BUG/MEDIUM: mux-h1: only turn CO_FL_ERROR to CS_FL_ERROR with empty ibuf

A connection-level error must not be turned to a stream-level error if there
are still pending data for that stream, otherwise it can cause the truncation
of the last pending data.

This must be backported to affected releases, at least as far as 2.4,
maybe further.

3 years agoCI: github actions: switch to LibreSSL-3.5.1
Ilya Shipitsin [Wed, 16 Mar 2022 07:10:47 +0000 (12:10 +0500)] 
CI: github actions: switch to LibreSSL-3.5.1

3 years agoBUG/MINOR: httpclient: CF_SHUTW_NOW should be tested with channel_is_empty()
William Lallemand [Thu, 17 Mar 2022 14:14:15 +0000 (15:14 +0100)] 
BUG/MINOR: httpclient: CF_SHUTW_NOW should be tested with channel_is_empty()

CF_SHUTW_NOW shouldn't be a condition alone to exit the io handler, it
must be tested with the emptiness of the response channel.

Must be backported to 2.5.

3 years agoBUG/MINOR: httpclient: process the response when received before the end of the request
William Lallemand [Thu, 17 Mar 2022 13:57:23 +0000 (14:57 +0100)] 
BUG/MINOR: httpclient: process the response when received before the end of the request

A server could reply a response with a shut before the end of the htx
transfer, in this case the httpclient would leave before computing the
received response.

This patch fixes the issue by calling the "process_data" label instead of
the "more" label which don't do the si_shut.

Must be bacported in 2.5.

3 years agoBUG/MINOR: httpclient: only check co_data() instead of HTTP_MSG_DATA
William Lallemand [Thu, 17 Mar 2022 13:45:46 +0000 (14:45 +0100)] 
BUG/MINOR: httpclient: only check co_data() instead of HTTP_MSG_DATA

Checking msg >= HTTP_MSG_DATA was useful to check if we received all the
data. However it does not work correctly in case of errors because we
don't reach this state, preventing to catch the error in the httpclient.

The consequence of this problem is that we don't get the status code of
the error response upon an error.

Fix the issue by only checking co_data().

Must be backported to 2.5.

3 years agoBUG/MINOR: http-rules: completely free incorrect TCP rules on error
Willy Tarreau [Thu, 17 Mar 2022 19:29:06 +0000 (20:29 +0100)] 
BUG/MINOR: http-rules: completely free incorrect TCP rules on error

When a http-request or http-response rule fails to parse, we currently
free only the rule without its contents, which makes ASAN complain.
Now that we have a new function for this, let's completely free the
rule. This relies on this commit:

  MINOR: actions: add new function free_act_rule() to free a single rule

It's probably not needed to backport this since we're on the exit path
anyway.

3 years agoBUG/MINOR: tcp-rules: completely free incorrect TCP rules on error
Willy Tarreau [Thu, 17 Mar 2022 19:26:54 +0000 (20:26 +0100)] 
BUG/MINOR: tcp-rules: completely free incorrect TCP rules on error

When a tcp-request or tcp-response rule fails to parse, we currently
free only the rule without its contents, which makes ASAN complain.
Now that we have a new function for this, let's completely free the
rule. Reg-tests are now completely OK with ASAN. This relies on this
commit:

  MINOR: actions: add new function free_act_rule() to free a single rule

It's probably not needed to backport this since we're on the exit path
anyway.

3 years agoMINOR: actions: add new function free_act_rule() to free a single rule
Willy Tarreau [Thu, 17 Mar 2022 19:23:43 +0000 (20:23 +0100)] 
MINOR: actions: add new function free_act_rule() to free a single rule

There was free_act_rules() that frees all rules from a head but nothing
to free a single rule. Currently some rulesets partially free their own
rules on parsing error, and we're seeing some regtests emit errors under
ASAN because of this.

Let's first extract the code to free a rule into its own function so
that it becomes possible to use it on a single rule.

3 years agoBUG/MINOR: logs: fix logsrv leaks on clean exit
Willy Tarreau [Thu, 17 Mar 2022 18:47:33 +0000 (19:47 +0100)] 
BUG/MINOR: logs: fix logsrv leaks on clean exit

Log servers are a real mess because:
  - entries are duplicated using memcpy() without their strings being
    reallocated, which results in these ones not being freeable every
    time.

  - a new field, ring_name, was added in 2.2 by commit 99c453df9
    ("MEDIUM: ring: new section ring to declare custom ring buffers.")
    but it's never initialized during copies, causing the same issue

  - no attempt is made at freeing all that.

Of course, running "haproxy -c" under ASAN quickly notices that and
dumps a core.

This patch adds the missing strdup() and initialization where required,
adds a new free_logsrv() function to cleanly free() such a structure,
calls it from the proxy when iterating over logsrvs instead of silently
leaking their file names and ring names, and adds the same logsrv loop
to the proxy_free_defaults() function so that we don't leak defaults
sections on exit.

It looks a bit entangled, but it comes as a whole because all this stuff
is inter-dependent and was missing.

It's probably preferable not to backport this in the foreseable future
as it may reveal other jokes if some obscure parts continue to memcpy()
the logsrv struct.

3 years agoBUG/MINOR: server/ssl: free the SNI sample expression
William Lallemand [Wed, 16 Mar 2022 16:48:19 +0000 (17:48 +0100)] 
BUG/MINOR: server/ssl: free the SNI sample expression

ASAN complains about the SNI expression not being free upon an haproxy
-c. Indeed the httpclient is now initialized with a sni expression and
this one is never free in the server release code.

Must be backported in 2.5 and could be backported in every stable
versions.

3 years agoBUILD: httpclient: fix build without SSL
William Lallemand [Wed, 16 Mar 2022 15:39:23 +0000 (16:39 +0100)] 
BUILD: httpclient: fix build without SSL

src/http_client.c: In function ‘httpclient_cfg_postparser’:
src/http_client.c:1065:8: error: unused variable ‘errmsg’ [-Werror=unused-variable]
 1065 |  char *errmsg = NULL;
      |        ^~~~~~
src/http_client.c:1064:6: error: unused variable ‘err_code’ [-Werror=unused-variable]
 1064 |  int err_code = 0;
      |      ^~~~~~~~

Fix the build of the httpclient without SSL, the problem was introduced
with previous patch 71e3158 ("BUG/MINOR: httpclient: send the SNI using
the host header")

Must be backported in 2.5 as well.

3 years agoBUG/MINOR: httpclient: send the SNI using the host header
William Lallemand [Wed, 16 Mar 2022 14:47:47 +0000 (15:47 +0100)] 
BUG/MINOR: httpclient: send the SNI using the host header

Generate an SNI expression which uses the Host header of the request.
This is mandatory for most of the SSL servers nowadays.

Must be backported in 2.5 with the previous patch which export
server_parse_sni_expr().

3 years agoMINOR: server: export server_parse_sni_expr() function
William Lallemand [Wed, 16 Mar 2022 14:44:42 +0000 (15:44 +0100)] 
MINOR: server: export server_parse_sni_expr() function

Export the server_parse_sni_expr() function in order to create a SNI
expression in a server which was not parsed from the configuration.

3 years agoDEV: udp: add support for random packet corruption
Willy Tarreau [Wed, 16 Mar 2022 14:07:51 +0000 (15:07 +0100)] 
DEV: udp: add support for random packet corruption

-c sets a corruption rate in % of the packets.
-o sets the start offset of the area to be corrupted.
-w sets the length of the area to be corrupted.

A single byte within that area will then be randomly XORed with a
random value.

3 years agoDEV: udp: switch parser to getopt() instead of positional arguments
Willy Tarreau [Wed, 16 Mar 2022 13:49:33 +0000 (14:49 +0100)] 
DEV: udp: switch parser to getopt() instead of positional arguments

In order to ease addition of new types of perturbations to udp-perturb,
let's first switch to getopt() and get rid of the positional arguments.
The random seed was already a conditional option of the rate which was
a conditional option as well. We add -r and -s for the rate and the
seed, and new options will follow.

3 years agoBUG/MEDIUM: sink: Properly get the stream-int in appctx callback functions
Christopher Faulet [Wed, 16 Mar 2022 09:01:26 +0000 (10:01 +0100)] 
BUG/MEDIUM: sink: Properly get the stream-int in appctx callback functions

The appctx owner is not a stream-interface anymore. It is now a
conn-stream. However, sink code was not updated accordingly. It is now
fixed.

It is 2.6-specific, no backport is needed.

3 years agoBUG/MEDIUM: cli/debug: Properly get the stream-int in all debug I/O handlers
Christopher Faulet [Wed, 16 Mar 2022 08:52:10 +0000 (09:52 +0100)] 
BUG/MEDIUM: cli/debug: Properly get the stream-int in all debug I/O handlers

The appctx owner is not a stream-interface anymore. It is now a conn-stream.
In the cli I/O handler for the command "debug dev fd", we still handle it as
a stream-interface. It is now fixed.

It is 2.6-specific, no backport is needed.

3 years agoBUG/MEDIUM: applet: Don't call .release callback function twice
Christopher Faulet [Tue, 15 Mar 2022 10:29:59 +0000 (11:29 +0100)] 
BUG/MEDIUM: applet: Don't call .release callback function twice

Since the CS/SI refactoring, the .release callback function may be called
twice. The first call when a shutdown for read or for write is performed.
The second one when the applet is detached from its conn-stream. The second
call must be guarded, just like the first one, to only be performed is the
stream-interface is not the in disconnected (SI_ST_DIS) or closed
(SI_ST_CLO) state.

To simplify the fix, we now always rely on si_applet_release() function.

It is 2.6-specific, no backport is needed.