]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
17 months agoDOC: promex: Add documentation about extra-counters
Christopher Faulet [Mon, 29 Jan 2024 15:55:01 +0000 (16:55 +0100)] 
DOC: promex: Add documentation about extra-counters

Now the feature is fully implementated, the README is updated accordingly.

17 months agoMEDIUM: promex: Dump listeners extra counters if requested
Christopher Faulet [Mon, 29 Jan 2024 15:42:02 +0000 (16:42 +0100)] 
MEDIUM: promex: Dump listeners extra counters if requested

Dump extra counter for the listeners. We loop on stats modules and dumped
all concerned counters. The module name is reported with the "mod" label.

17 months agoMEDIUM: promex: Dump servers extra counters if requested
Christopher Faulet [Mon, 29 Jan 2024 15:41:56 +0000 (16:41 +0100)] 
MEDIUM: promex: Dump servers extra counters if requested

Dump extra counter for the servers. We loop on stats modules and dumped all
concerned counters. The module name is reported with the "mod" label.

17 months agoMEDIUM: promex: Dump backends extra counters if requested
Christopher Faulet [Mon, 29 Jan 2024 15:41:42 +0000 (16:41 +0100)] 
MEDIUM: promex: Dump backends extra counters if requested

Dump extra counter for the backend. We loop on stats modules and dumped all
concerned counters. The module name is reported with the "mod" label.

17 months agoMEDIUM: promex: Dump frontends extra counters if requested
Christopher Faulet [Mon, 29 Jan 2024 15:41:24 +0000 (16:41 +0100)] 
MEDIUM: promex: Dump frontends extra counters if requested

Dump extra counter for the frontends. We loop on stats modules and dumped
all concerned counters. The module name is reported with the "mod" label.

17 months agoMINOR: promex: Add info in the promex context to dump extra counters
Christopher Faulet [Mon, 29 Jan 2024 15:40:41 +0000 (16:40 +0100)] 
MINOR: promex: Add info in the promex context to dump extra counters

The context of the promex applet was extended to support the dump of extra
counters. These counters are not dumped yet, but info to interrupt and
restart the dump are required. The stats module and the relative field
number for this module can now be saved.

In addition support for "extra-counters" parameter was added on the
query-string to dump these counters. Otherwise, no extra-counters are
dumped.

17 months agoMINOR: promex: Add a param to override the description when a metric is dumped
Christopher Faulet [Mon, 29 Jan 2024 15:37:35 +0000 (16:37 +0100)] 
MINOR: promex: Add a param to override the description when a metric is dumped

When a metric is dumped, it is now possible to specify a custom
description. We will add the support for extra counters. The list of these
counters is retrived dynamically. Thus the description must be dynamic
too. Note it was already possible to customize the metric name.

17 months agoMEDIUM: stats: Be able to access a specific field into a stats module
Christopher Faulet [Mon, 29 Jan 2024 15:35:19 +0000 (16:35 +0100)] 
MEDIUM: stats: Be able to access a specific field into a stats module

It is now possible to selectively retrieve extra counters from stats
modules. H1, H2, QUIC and H3 fill_stats() callback functions are updated to
return a specific counter.

17 months agoMINOR: stats: Be able to access to registered stats modules from anywhere
Christopher Faulet [Mon, 29 Jan 2024 15:34:23 +0000 (16:34 +0100)] 
MINOR: stats: Be able to access to registered stats modules from anywhere

The list of modules registered on the stats to expose extra counters is now
public. It is required to export these counters into the Prometheus
exporter.

17 months agoMEDIUM: tcp-act/backend: support for set-bc-{mark,tos} actions
Aurelien DARRAGON [Thu, 18 Jan 2024 17:51:26 +0000 (18:51 +0100)] 
MEDIUM: tcp-act/backend: support for set-bc-{mark,tos} actions

set-bc-{mark,tos} actions are pretty similar to set-fc-{mark,tos} to set
mark/tos on packets sent from haproxy to server: set-bc-{mark,tos} actions
act on the whole backend/srv connection: from connect() to connection
teardown, thus they may only be used before the connection to the server
is instantiated, meaning that they are only relevant for request-oriented
rules such as tcp-request or http-request rules. For now their use is
limited to content request rules, because tos and mark informations are
stored directly within the stream, thus it is required that the stream
already exists.

stream flags are used in combination with dedicated stream struct members
variables to pass 'tos' and 'mark' informations so that they are correctly
considered during stream connection assignment logic (prior to connecting
to actually connecting to the server)

'tos' and 'mark' fd sockopts are taken into account in conn hash
parameters for connection reuse mechanism.

The documentation was updated accordingly.

17 months agoMEDIUM: tcp-act: <expr> support for set-fc-{mark,tos} actions
Aurelien DARRAGON [Thu, 18 Jan 2024 16:17:41 +0000 (17:17 +0100)] 
MEDIUM: tcp-act: <expr> support for set-fc-{mark,tos} actions

In this patch we add the possibility to use sample expression as argument
for set-fc-{mark,tos} actions. To make it backward compatible with
previous behavior, during parsing we first try to parse the value as
as integer (decimal or hex notation), and then fallback to expr parsing
in case of failure.

The documentation was updated accordingly.

17 months agoMINOR: hlua: Rename set_{tos, mark} to set_fc_{tos, mark}
Aurelien DARRAGON [Mon, 15 Jan 2024 15:33:04 +0000 (16:33 +0100)] 
MINOR: hlua: Rename set_{tos, mark} to set_fc_{tos, mark}

This is a complementary patch to "MINOR: tcp-act: Rename "set-{mark,tos}"
to "set-fc-{mark,tos}"", but for the Lua API.

set_mark and set_tos were kept as aliases for set_fc_mark and set_fc_tos
but they were marked as deprecated.

Using this opportunity to reorder set_mark and set_tos by alphabetical
order.

17 months agoMINOR: tcp-act: Rename "set-{mark,tos}" to "set-fc-{mark,tos}"
Aurelien DARRAGON [Mon, 15 Jan 2024 15:08:40 +0000 (16:08 +0100)] 
MINOR: tcp-act: Rename "set-{mark,tos}" to "set-fc-{mark,tos}"

"set-mark" and "set-tos" only alter packets from haproxy to client
(frontend connection). Since we may add support for equivalent keywords
on server side, we rename them with an explicit name to prevent
confusions.

Thus, we rename:
 - "set-mark" to "set-fc-mark"
 - "set-tos" to "set-fc-tos"

"set-mark" and "set-tos" were kept as aliases (to "set-fc-mark" and
"set-fc-tos" respectively) for now to prevent config breakage, but they
have been marked as deprecated so they can be removed in future version.

17 months agoMINOR: tcp_act: fix alphabetical ordering of tcp request content actions
Aurelien DARRAGON [Thu, 1 Feb 2024 08:53:10 +0000 (09:53 +0100)] 
MINOR: tcp_act: fix alphabetical ordering of tcp request content actions

"set-src" and "set-src-port" were misplaced and incorrectly ordered in
tcp_req_cont_actions keyword list.

17 months agoOPTIM: connection: progressive hash for conn_calculate_hash()
Aurelien DARRAGON [Thu, 25 Jan 2024 09:35:39 +0000 (10:35 +0100)] 
OPTIM: connection: progressive hash for conn_calculate_hash()

Some CPU time is needlessly wasted in conn_calculate_hash(), because all
params are first copied into a temporary buffer before computing the
hash on the whole buffer. Instead, let's leverage the XXH progressive
hash update functions to avoid expensive memcpys.

17 months agoCLEANUP: connection: remove obsolete comment in header file
Aurelien DARRAGON [Thu, 18 Jan 2024 16:58:30 +0000 (17:58 +0100)] 
CLEANUP: connection: remove obsolete comment in header file

0x00000008 bit for CO_FL_* flags is no more unused since 8cc3fc73f1
("MINOR: connection: update rhttp flags usage"). Removing the comment
that says otherwise.

17 months agoMINOR: mux-quic: realign Tx buffer if possible
Amaury Denoyelle [Wed, 17 Jan 2024 15:01:00 +0000 (16:01 +0100)] 
MINOR: mux-quic: realign Tx buffer if possible

A major reorganization of QUIC MUX sending has been implemented. Now
data transfer occur over a single QCS buffer. This has improve
performance but at the cost of restrictions on snd_buf. Indeed, buffer
instances are now shared from stream callback snd_buf up to quic-conn
layer.

As such, snd_buf cannot manipulate freely already present data buffer.
In particular, realign has been completely removed by the previous
patches.

This commit reintroduces a partial realign support. This is only done if
the buffer contains only unsent data, via a new MUX function
qcc_realign_stream_txbuf() which is called during snd_buf.

17 months agoMEDIUM: mux-quic: properly handle conn Tx buf exhaustion
Amaury Denoyelle [Wed, 17 Jan 2024 14:15:55 +0000 (15:15 +0100)] 
MEDIUM: mux-quic: properly handle conn Tx buf exhaustion

This commit is a direct follow-up on the major rearchitecture of send
buffering. This patch implements the proper handling of connection pool
buffer temporary exhaustion.

The first step is to be able to differentiate a fatal allocation error
from a temporary pool exhaustion. This is done via a new output argument
on qcc_get_stream_txbuf(). For a fatal error, application protocol layer
will schedule the immediate connection closing. For a pool exhaustion,
QCC is flagged with QC_CF_CONN_FULL and stream sending process is
interrupted. QCS instance is also registered in a new list
<qcc.buf_wait_list>.

A new connection buffer can become available when all ACKs are received
for an older buffer. This process is taken in charge by quic-conn layer.
It uses qcc_notify_buf() function to clear QC_CF_CONN_FULL and to wake
up every streams registered on buf_wait_list to resume sending process.

17 months agoMEDIUM: mux-quic: release Tx buf on too small room
Amaury Denoyelle [Mon, 22 Jan 2024 16:03:41 +0000 (17:03 +0100)] 
MEDIUM: mux-quic: release Tx buf on too small room

This commit is a direct follow-up on the major rearchitecture of send
buffering. It allows application protocol to react if current QCS
sending buffer space is too small. In this case, the buffer can be
released to the quic-conn layer. This allows to allocate a new QCS
buffer and retry HTX parsing, unless connection buffer pool is already
depleted.

A new function qcc_release_stream_txbuf() serves as API for app protocol
to release the QCS sending buffer. This operation fails if there is
unsent data in it. In this case, MUX has to keep it to finalize transfer
of unsent data to quic-conn layer. QCS is thus flagged with
QC_SF_BLK_MROOM to interrupt snd_buf operation.

When all data are sent to the quic-conn layer, QC_SF_BLK_MROOM is
cleared via qcc_streams_sent_done() and stream layer is woken up to
restart snd_buf.

Note that a new function qcc_stream_can_send() has been defined. It
allows app proto to check if sending is currently blocked for the
current QCS. For now, it checks QC_SF_BLK_MROOM flag. However, it will
be extended to other conditions with the following patches.

17 months agoMEDIUM: mux-quic: simplify sending API
Amaury Denoyelle [Tue, 30 Jan 2024 10:23:48 +0000 (11:23 +0100)] 
MEDIUM: mux-quic: simplify sending API

The previous commit was a major rework for QUIC MUX sending process.
Following this, this patch cleans up a few elements that remains but can
be removed as they are duplicated.

Of notable changes, offset fields from QCS and QCC are removed. They are
both equivalent to flow control soft offsets.

A new function qcs_prep_bytes() is implemented. Its purpose is to return
the count of prepared data bytes not yet sent. It also replaces
qcs_need_sending().

17 months agoMAJOR: mux-quic: remove intermediary Tx buffer
Amaury Denoyelle [Tue, 16 Jan 2024 15:47:57 +0000 (16:47 +0100)] 
MAJOR: mux-quic: remove intermediary Tx buffer

Previously, QUIC MUX sending was implemented with data transfered along
two different buffer instances per stream.

The first QCS buffer was used for HTX blocks conversion into H3 (or
other application protocol) during snd_buf stream callback. QCS instance
is then registered for sending via qcc_io_cb().

For each sending QCS, data memcpy is performed from the first to a
secondary buffer. A STREAM frame is produced for each QCS based on the
content of their secondary buffer.

This model is useful for QUIC MUX which has a major difference with
other muxes : data must be preserved longer, even after sent to the
lower layer. Data references is shared with quic-conn layer which
implements retransmission and data deletion on ACK reception.

This double buffering stages was the first model implemented and remains
active until today. One of its major drawbacks is that it requires
memcpy invocation for every data transferred between the two buffers.
Another important drawback is that the first buffer was is allocated by
each QCS individually without restriction. On the other hand, secondary
buffers are accounted for the connection. A bottleneck can appear if
secondary buffer pool is exhausted, causing unnecessary haproxy
buffering.

The purpose of this commit is to completely break this model. The first
buffer instance is removed. Now, application protocols will directly
allocate buffer from qc_stream_desc layer. This removes completely the
memcpy invocation.

This commit has a lot of code modifications. The most obvious one is the
removal of <qcs.tx.buf> field. Now, qcc_get_stream_txbuf() returns a
buffer instance from qc_stream_desc layer. qcs_xfer_data() which was
responsible for the memcpy between the two buffers is also completely
removed. Offset fields of QCS and QCC are now incremented directly by
qcc_send_stream(). These values are used as boundary with flow control
real offset to delimit the STREAM frames built.

As this change has a big impact on the code, this commit is only the
first part to fully support single buffer emission. For the moment, some
limitations are reintroduced and will be fixed in the next patches :

* on snd_buf if QCS sent buffer in used has room but not enough for the
  application protocol to store its content
* on snd_buf if QCS sent buffer is NULL and allocation cannot succeeds
  due to connection pool exhaustion

One final important aspect is that extra care is necessary now in
snd_buf callback. The same buffer instance is referenced by both the
stream and quic-conn layer. As such, some operation such as realign
cannot be done anymore freely.

17 months agoMINOR: mux-quic: check fctl during STREAM frame build
Amaury Denoyelle [Wed, 24 Jan 2024 10:54:41 +0000 (11:54 +0100)] 
MINOR: mux-quic: check fctl during STREAM frame build

qcs_build_stream_frm() is responsible to generate a STREAM frame
pointing to the content of QCS TX buffer.

This patch moves send flow control overflow check from qcs_xfer_data()
to qcs_build_stream_frm(), i.e. from transfer between internal
QCS buffer and qc_stream_desc, to STREAM frame generation.

Flow control is both check at stream and connection level. For
connection flow control, as several frames are built before emission, an
accumulator is used as extra arguments to functions to account the total
length of already built frames.

This patch should not provide any functional changes. Its main purpose
is to prepare for the removal of QCS internal buffer.

17 months agoMINOR: mux-quic: remove unneeded sent-offset fields
Amaury Denoyelle [Tue, 9 Jan 2024 10:37:56 +0000 (11:37 +0100)] 
MINOR: mux-quic: remove unneeded sent-offset fields

Both QCS and QCC have their owned sent offset field. These fields store
the newest offset sent to the quic-conn layer. It is similar to QCS/QCC
flow control real offset. This patch removes them and replaces them by
the latter for code clarification.

MINOR: mux-quic: remove unneeded qcc.tx.sent_offsets field

This commit as a similar purpose as previous, except that it removes QCC
<sent_offsets> field, now equivalent to connection flow control real
offset.

17 months agoMEDIUM: mux-quic: limit conn flow control on snd_buf
Amaury Denoyelle [Wed, 18 Oct 2023 15:48:11 +0000 (17:48 +0200)] 
MEDIUM: mux-quic: limit conn flow control on snd_buf

This commit is a direct follow-up on the previous one. This time, it
deals with connection level flow control. Process is similar to stream
level : soft offset is incremented during snd_buf and real offset during
STREAM frame emission.

On MAX_DATA reception, both stream layer and QMUX is woken up if
necessary. One extra feature for conn level is the introduction of a new
QCC list to reference QCS instances. It will store instances for which
snd_buf callback has been interrupted on QCC soft offset reached. Every
stream instances is woken up on MAX_DATA reception if soft_offset is
unblocked.

17 months agoMEDIUM: mux-quic: limit stream flow control on snd_buf
Amaury Denoyelle [Wed, 18 Oct 2023 13:55:38 +0000 (15:55 +0200)] 
MEDIUM: mux-quic: limit stream flow control on snd_buf

This patch is the first of two to reimplement flow control emission
limits check. The objective is to account flow control earlier during
snd_buf stream callback. This should smooth transfers and prevent over
buffering on haproxy side if flow control limit is reached.

The current patch deals with stream level flow control. It reuses the
newly defined flow control type. Soft offset is incremented after HTX to
data conversion. If limit is reached, snd_buf is interrupted and stream
layer will subscribe on QCS.

On qcc_io_cb(), generation of STREAM frames is restricted as previously
to ensure to never surpass peer limits. Finally, flow control real
offset is incremented on lower layer send notification. Thus, it will
serve as a base offset for built STREAM frames. If limit is reached,
STREAM frames generation is suspended.

Each time QCS data flow control limit is reached, soft and real offsets
are reconsidered.

Finally, special care is used when flow control limit is incremented via
MAX_STREAM_DATA reception. If soft value is unblocked, stream layer
snd_buf is woken up. If real value is unblocked, qcc_io_cb() is
rescheduled.

17 months agoMINOR: mux-quic: define a flow control related type
Amaury Denoyelle [Wed, 3 Jan 2024 15:14:52 +0000 (16:14 +0100)] 
MINOR: mux-quic: define a flow control related type

Create a new module dedicated to flow control handling. It will be used
to implement earlier flow control update on snd_buf stream callback.

For the moment, only Tx part is implemented (i.e. limit set by the peer
that haproxy must respect for sending). A type quic_fctl is defined to
count emitted data bytes. Two offsets are used : a real one and a soft
one. The difference is that soft offset can be incremented beyond limit
unless it is already in excess.

Soft offset will be used for HTX to H3 parsing. As size of generated H3
is unknown before parsing, it allows to surpass the limit one time. Real
offset will be used during STREAM frame generation : this time the limit
must not be exceeded to prevent protocol violation.

17 months agoMINOR: mux-quic: prepare for earlier flow control update
Amaury Denoyelle [Wed, 10 Jan 2024 10:09:33 +0000 (11:09 +0100)] 
MINOR: mux-quic: prepare for earlier flow control update

Add a new argument to qcc_send_stream() to specify the count of sent
bytes.

For the moment this argument is unused. This commit is in fact a step to
implement earlier flow control update during stream layer snd_buf.

17 months agoBUG/MINOR: ssl/quic: fix 0RTT define
Amaury Denoyelle [Wed, 31 Jan 2024 15:20:00 +0000 (16:20 +0100)] 
BUG/MINOR: ssl/quic: fix 0RTT define

Previous patches have reorganize define definitions for SSL 0RTT
support. However a typo was introduced. This caused haproxy to disable
0RTT support announcement and report of an erroneous warning for no
support on the SSL library side when using quictls/openssl compat layer.

This was detected by using ngtcp2-client. No 0RTT packet were emitted by
the client due to haproxy missing support advertisement.

The faulty commit is the following one :
  commit 5c4519934708bfe6a26b9ad0cc93a8c5c87df112
  MEDIUM: ssl/quic: always compile the ssl_conf.early_data test

This must be backported wherever the above patch is.

17 months agoCLEANUP: h1: remove unused function h1_measure_trailers()
Willy Tarreau [Wed, 31 Jan 2024 14:21:06 +0000 (15:21 +0100)] 
CLEANUP: h1: remove unused function h1_measure_trailers()

This one stopped being used in 2.1 when HTX became mandatory,
let's drop it.

17 months agoBUG/MEDIUM: h1: always reject the NUL character in header values
Willy Tarreau [Wed, 31 Jan 2024 14:10:39 +0000 (15:10 +0100)] 
BUG/MEDIUM: h1: always reject the NUL character in header values

Ben Kallus kindly reported that we still hadn't blocked the NUL
character from header values as clarified in RFC9110 and that, even
though there's no known issure related to this, it may one day be
used to construct an attack involving another component.

Actually, both Christopher and I sincerely believed we had done it
prior to releasing 2.9, shame on us for missing that one and thanks
to Ben for the reminder!

The change was applied, it was confirmed to properly reject this NUL
byte from both header and trailer values, and it's still possible to
force it to continue to be supported using the usual pair of unsafe
"option accept-invalid-http-{request|response}" for those who would
like to keep it for whatever reason that wouldn't make sense.

This was tagged medium so that distros also remember to apply it as
a preventive measure.

It should progressively be backported to all versions down to 2.0.

17 months agoBUG/MINOR: h1-htx: properly initialize the err_pos field
Willy Tarreau [Wed, 31 Jan 2024 14:04:11 +0000 (15:04 +0100)] 
BUG/MINOR: h1-htx: properly initialize the err_pos field

Trailers are parsed using a temporary h1m struct, likely due to using
distinct h1 parser states. However, the err_pos field that's used to
decide whether or not to enfore option accept-invalid-http-request (or
response) was not initialized in this struct, resulting in using a
random value that may randomly accept or reject a few bad chars. The
impact is very limited in trailers (e.g. no message size is transmitted
there) but we must make sure that the option is respected, at least for
users facing the need for this option there.

The issue was introduced in 2.0 by commit 2d7c5395ed ("MEDIUM: htx:
Add the parsing of trailers of chunked messages"), and the code moved
from mux_h1.c to h1_htx.c in 2.1 with commit 4f0f88a9d0 ("MEDIUM:
mux-h1/h1-htx: move HTX convertion of H1 messages in dedicated file")
so the patch needs to be backported to all stable versions, and the
file adjusted for 2.0.

17 months agoDOC: httpclient: add dedicated httpclient section
Lukas Tribus [Tue, 30 Jan 2024 21:17:44 +0000 (21:17 +0000)] 
DOC: httpclient: add dedicated httpclient section

Move httpclient keywords into its own section and explain adding
an introductory paragraph.

Also see Github issue #2409

Should be backported to 2.6 ; but note that:
2.7 does not have httpclient.resolvers.disabled
2.6 does not have httpclient.retries and httpclient.timeout.connect

17 months agoMEDIUM: ssl/quic: always compile the ssl_conf.early_data test
William Lallemand [Mon, 29 Jan 2024 17:36:31 +0000 (18:36 +0100)] 
MEDIUM: ssl/quic: always compile the ssl_conf.early_data test

Always compile the test of the early_data variable in
"ssl_quic_initial_ctx", this way we can emit a warning about its support
or not.

The test was moved in a more simple preprocessor check which only checks
the new HAVE_SSL_0RTT_QUIC constant.

Could be backported to 2.9 with the 2 previous commits.
However AWS-LC must be excluded of HAVE_SSL_0RTT_QUIC in this version.

17 months agoMINOR: ssl: rename HA_OPENSSL_HAVE_0RTT_SUPPORT constant to HAVE_SSL_0RTT_QUIC
William Lallemand [Mon, 29 Jan 2024 17:26:19 +0000 (18:26 +0100)] 
MINOR: ssl: rename HA_OPENSSL_HAVE_0RTT_SUPPORT constant to HAVE_SSL_0RTT_QUIC

Rename the constant to be me more comprehensive.

17 months agoMINOR: ssl: add HAVE_SSL_0RTT constant
William Lallemand [Mon, 29 Jan 2024 17:17:04 +0000 (18:17 +0100)] 
MINOR: ssl: add HAVE_SSL_0RTT constant

Add the HAVE_SSL_0RTT constant which define if the SSL library supports
0RTT. Which is different from HA_OPENSSL_HAVE_0RTT_SUPPORT which was
used only in the context of QUIC

17 months agoBUG/MEDIUM: h1: Don't support LF only to mark the end of a chunk size
Christopher Faulet [Fri, 26 Jan 2024 15:30:53 +0000 (16:30 +0100)] 
BUG/MEDIUM: h1: Don't support LF only to mark the end of a chunk size

It is similar to the previous fix but for the chunk size parsing. But this
one is more annoying because a poorly coded application in front of haproxy
may ignore the last digit before the LF thinking it should be a CR. In this
case it may be out of sync with HAProxy and that could be exploited to
perform some sort or request smuggling attack.

While it seems unlikely, it is safer to forbid LF with CR at the end of a
chunk size.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

17 months agoBUG/MINOR: h1: Don't support LF only at the end of chunks
Christopher Faulet [Fri, 26 Jan 2024 15:23:51 +0000 (16:23 +0100)] 
BUG/MINOR: h1: Don't support LF only at the end of chunks

When the message is chunked, all chunks must ends with a CRLF. However, on
old versions, to support bad client or server implementations, the LF only
was also accepted. Nowadays, it seems useless and can even be considered as
an issue. Just forbid LF only at the end of chunks, it seems reasonnable.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

17 months agoCLEANUP: log: deinitialization of the log buffer in one function
Miroslav Zagorac [Tue, 30 Jan 2024 02:14:09 +0000 (03:14 +0100)] 
CLEANUP: log: deinitialization of the log buffer in one function

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.

17 months agoBUG/MEDIUM: quic: fix crash on invalid qc_stream_buf_free() BUG_ON
Amaury Denoyelle [Mon, 29 Jan 2024 08:18:08 +0000 (09:18 +0100)] 
BUG/MEDIUM: quic: fix crash on invalid qc_stream_buf_free() BUG_ON

A recent fix was introduced to ensure unsent data are deleted when a
QUIC MUX stream releases its qc_stream_desc instance. This is necessary
to ensure all used buffers will be liberated once all ACKs are received.
This is implemented by the following patch :

  commit ad6b13d3177945bf6a85d6dc5af80b8e34ea6191 (quic-dev/qns)
  BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf

Before this patch, buffer removal was done only on ACK reception. ACK
handling is only done in order from the oldest one. A BUG_ON() statement
is present to ensure this assertion remains valid.

This is however not true anymore since the above patch. Indeed, after
unsent data removal, the current buffer may be empty if it did not
contain yet any sent data. In this case, it is not the oldest buffer,
thus the BUG_ON() statement will be triggered.

To fix this, simply remove this BUG_ON() statement. It should not have
any impact as it is safe to remove buffers in any order.

Note that several conditions must be met to trigger this BUG_ON crash :
* a QUIC MUX stream is destroyed before transmitting all of its data
* several buffers must have been previously allocated for this stream so
  it happens only for transfers bigger than bufsize
* latency should be high enough to delay ACK reception

This must be backported wherever the above patch is (currently targetted
to 2.6).

17 months agoBUG/MEDIUM: qpack: allow 6xx..9xx status codes
Amaury Denoyelle [Mon, 29 Jan 2024 12:45:48 +0000 (13:45 +0100)] 
BUG/MEDIUM: qpack: allow 6xx..9xx status codes

HTTP status codes outside of 100..599 are considered invalid in HTTP
RFC9110. However, it is explicitely stated that range 600..999 is often
used for internal communication so in practice haproxy must be lenient
with it.

Before this patch, QPACK encoder rejected these values. This resulted in
a connection error. Fix this by extending the range of allowed values
from 100 to 999.

This is linked to github issue #2422. Once again, thanks to @yokim-git
for his help here.

This must be backported up to 2.6.

17 months agoBUG/MEDIUM: h3: do not crash on invalid response status code
Amaury Denoyelle [Mon, 29 Jan 2024 12:47:44 +0000 (13:47 +0100)] 
BUG/MEDIUM: h3: do not crash on invalid response status code

A crash occurs in h3_resp_headers_send() if an invalid response code is
received from the backend side. Fix this by properly flagging the
connection on error. This will cause a CONNECTION_CLOSE.

This should fix github issue #2422.

Big thanks to ygkim (@yokim-git) for his help and reactivity. Initially,
GDB reported an invalid code source location due to heavy functions
inlining inside h3_snd_buf(). The issue was found after using -Og flag.

This must be backported up to 2.6.

17 months agoMINOR: h3: add traces for stream sending function
Amaury Denoyelle [Mon, 29 Jan 2024 14:15:27 +0000 (15:15 +0100)] 
MINOR: h3: add traces for stream sending function

Replace h3_debug_printf() by real trace for functions used by stream
layer snd_buf callback. A new event type H3_EV_STRM_SEND is created for
the occasion.

This should be backported up to 2.6 to help investigate H3 issues on
stable releases. Note that h3_nego_ff/h3_done_ff definition are not
available from 2.8.

17 months agoBUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions
Olivier Houchard [Sat, 27 Jan 2024 21:58:29 +0000 (22:58 +0100)] 
BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions

It has been found that under some rare error circumstances,
SSL_do_handshake() could return with SSL_ERROR_WANT_READ without
even trying to call the read function, causing permanent wakeups
that prevent the process from sleeping.

It was established that this only happens if the retry flags are
not systematically cleared in both directions upon any I/O attempt,
but, given the lack of documentation on this topic, it is hard to
say if this rather strange behavior is expected or not, otherwise
why wouldn't the library always clear the flags by itself before
proceeding?

In addition, this only seems to affect OpenSSL 1.1.0 and above,
and does not affect wolfSSL nor aws-lc.

A bisection on haproxy showed that this issue was first triggered by
commit a8955d57ed ("MEDIUM: ssl: provide our own BIO."), which means
that OpenSSL's socket BIO does not have this problem. And this one
does always clear the flags before proceeding. So let's just proceed
the same way. It was verified that it properly fixes the problem,
does not affect other implementations, and doesn't cause any freeze
nor spurious wakeups either.

Many thanks to Valentín Gutiérrez for providing a network capture
showing the incident as well as a reproducer. This is GH issue #2403.

This patch needs to be backported to all versions that include the
commit above, i.e. as far as 2.0.

17 months agoDOC: configuration: clarify http-request wait-for-body
Thayne McCombs [Mon, 29 Jan 2024 05:07:32 +0000 (22:07 -0700)] 
DOC: configuration: clarify http-request wait-for-body

Make it more explicit what happens in the various scenarios that cause
HAProxy to stop waiting when "http-request wait-for-body" is used.

Also fix a couple of grammatical errors.

Fixes: #2410
Signed-Off-By: Thayne McCombs <astrothayne@gmail.com>
17 months ago[RELEASE] Released version 3.0-dev2 v3.0-dev2
Willy Tarreau [Fri, 26 Jan 2024 19:11:39 +0000 (20:11 +0100)] 
[RELEASE] Released version 3.0-dev2

Released version 3.0-dev2 with the following main changes :
    - MINOR: ot: logsrv struct becomes logger
    - MINOR: ssl: Update ssl_fc_curve/ssl_bc_curve to use SSL_get0_group_name
    - CLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec()
    - DEV: patchbot: produce a verdict for too long commit messages
    - CLEANUP: ssl: fix indentation in smp_fetch_ssl_fc_ec() (part 2)
    - CLEANUP: quic: Double quic_dgram_parse() prototype declaration.
    - BUG/MINOR: map: list-based matching potential ordering regression
    - REGTESTS: add a test to ensure map-ordering is preserved
    - DOC: config: fix typo about map_*_key converters
    - DOC: configuration: corrected description of keyword tune.ssl.ocsp-update.mindelay
    - MINOR: map: mapfile ordering also matters for tree-based match types
    - DEV: phash: add a trivial perfect hash generator for integers
    - OPTIM: http: simplify http_get_status_idx() using a hash
    - CLEANUP: http: avoid duplicating literals in find_http_meth()
    - MINOR: http: add infrastructure to choose status codes for err / fail
    - MEDIUM: http_act: check status codes against the bit fields for err/fail
    - MEDIUM: http: add the ability to redefine http-err-codes and http-fail-codes
    - CI: codespell: ignore some words in URLs
    - CI: codespell: add more words to whitelist
    - CLEANUP: fix spelling of "occured" in src/h3.c
    - BUILD: quic: missing include for quic_tp
    - BUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control
    - MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA selection
    - MEDIUM: ssl: generate '*' SNI filters for default certificates
    - MEDIUM: ssl: does not use default_ctx for 'generate-certificate' option
    - REORG: ssl: move 'generate-certificates' code to ssl_gencert.c
    - DOC: configuration: update configuration on how to have multiple default certs
    - MEDIUM: ssl: implements 'default-crt' keyword for bind Lines
    - CI: github: update wolfSSL to 5.6.6
    - DOC: INSTALL: require at least WolfSSL 5.6.6
    - DEV: h2: add support for multiple flags in mkhdr
    - DEV: h2: support hex-encoded data sequences in mkhdr
    - BUG/MINOR: mux-h2: also count streams for refused ones
    - BUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)
    - MINOR: vars: fix indentation in var_clear_buffer()
    - DOC: configuration: fix set-dst in actions keywords matrix
    - BUG/MEDIUM: mux-h2: refine connection vs stream error on headers
    - MINOR: mux-h2/traces: add a missing trace on connection WU with negative inc
    - MINOR: mux-h2: add a counter of "glitches" on a connection
    - MINOR: connection: add a new mux_ctl to report number of connection glitches
    - MINOR: mux-h2: implement MUX_CTL_GET_GLITCHES
    - MINOR: connection: add sample fetches to report per-connection glitches
    - BUILD: stick-table: fix build error on 32-bit platforms
    - MINOR: quic: Transport parameters encoding without version_information
    - MINOR: quic: Enable early data at SSL session level (aws-lc)
    - MINOR: ssl_sock: Early data disabled during SSL_CTX switching (aws-lc)
    - MINOR: quic: Correctly wait for the completion of handshakes with early data (aws-lc)
    - BUG/MEDIUM: cli: some err/warn msg dumps add LR into CSV output on stat's CLI
    - BUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs
    - BUILD: quic: fix build error when using the compatibility layer
    - BUILD: quic: Fix build error when building QUIC against wolfssl.
    - BUILD: quic: Fix build error when building QUIC against libressl.
    - BUG/MINOR: hlua: fix uninitialized var in hlua_core_get_var()
    - CLEANUP: hlua: fix indent, remove extra return in hlua_core_get_var()
    - BUG/MEDIUM: cache: Fix crash when deleting secondary entry
    - BUG/MINOR: quic: newreno QUIC congestion control algorithm no more available
    - CLEANUP: quic: Remove unused CUBIC_BETA_SCALE_FACTOR_SHIFT macro.
    - MINOR: quic: Stop hardcoding a scale shifting value (CUBIC_BETA_SCALE_FACTOR_SHIFT)
    - MINOR: quic: extract qc_stream_buf free in a dedicated function
    - BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf
    - CLEANUP: fix spelling of "elemt"
    - CI: extend spell check white list
    - CI: enable spell check on git push
    - BUILD: makefile: also define cmd_CXX to pretty-print C++ build commands
    - BUILD/MEDIUM: deviceatlas: addon build rework.
    - DOC: deviceatlas: update to be in line with the v3 api.
    - BUILD/MEDIUM: deviceatlas: updating the addon part.
    - BUILD: deviceatlas: remove unneeded depenency on libcurl / libzip
    - BUILD: deviceatlas: fix empty "-I" left on CFLAGS
    - Revert "CI: enable spell check on git push"

17 months agoRevert "CI: enable spell check on git push"
Willy Tarreau [Fri, 26 Jan 2024 18:58:14 +0000 (19:58 +0100)] 
Revert "CI: enable spell check on git push"

This reverts commit 413aa6e2e94a591d444ef9bd174a99b64eea8436.

It reports failures that neither the patch's author nor the committer
are able to check for before pushing, causing an excess of failure
reports that can hardly be acted upon. We need to find a better
solution, let's revert it for now.

17 months agoBUILD: deviceatlas: fix empty "-I" left on CFLAGS
Willy Tarreau [Fri, 26 Jan 2024 18:46:59 +0000 (19:46 +0100)] 
BUILD: deviceatlas: fix empty "-I" left on CFLAGS

The previous fix was incomplete, there was a leftover CURL_INC on the
CFLAGS which breaks Lua.

17 months agoBUILD: deviceatlas: remove unneeded depenency on libcurl / libzip
Willy Tarreau [Fri, 26 Jan 2024 18:36:11 +0000 (19:36 +0100)] 
BUILD: deviceatlas: remove unneeded depenency on libcurl / libzip

These were previously used for the "dadwsch" utility that's no longer
part of the addon, so we should not move the CFLAGS/LDFLAGS to the
global ones as this adds an undesired dependency on the libcurl and
libzip libs.

No backport is needed.

17 months agoBUILD/MEDIUM: deviceatlas: updating the addon part.
David Carlier [Thu, 25 Jan 2024 09:11:18 +0000 (09:11 +0000)] 
BUILD/MEDIUM: deviceatlas: updating the addon part.

- Reflecing the changes done in addons/deviceatlas/Makefile.inc.
 Enabling the cache feature and its disabling option as well.
- Now the `dadwsch` application is part of the API's package for more
general purposes, we remove it.
- Minor and transparent to user changes into da.c's workflow, also
making more noticeable some notices with appropriate logging levels.
- Adding support for the new `deviceatlas-cache-size` config keyword,
 a no-op when the cache support is disabled.
- Adding missing compilation units and relevant api updates to
the dummy library version.

17 months agoDOC: deviceatlas: update to be in line with the v3 api.
David Carlier [Thu, 25 Jan 2024 09:09:07 +0000 (09:09 +0000)] 
DOC: deviceatlas: update to be in line with the v3 api.

Reflecting here all the changes, no longer need to cater with
the legacy v2 neither.

17 months agoBUILD/MEDIUM: deviceatlas: addon build rework.
David Carlier [Thu, 25 Jan 2024 09:00:14 +0000 (09:00 +0000)] 
BUILD/MEDIUM: deviceatlas: addon build rework.

- Removing the legacy v2 support, which in turn suppress the need to set
a regex engine.
- Moving the options and addon into its distrinct build unit, cleaning up
the main one in the process.
- Adding a new option to disable the cache if desired or if
having a C++ toolchain is not a possibility.

17 months agoBUILD: makefile: also define cmd_CXX to pretty-print C++ build commands
Willy Tarreau [Fri, 26 Jan 2024 17:48:45 +0000 (18:48 +0100)] 
BUILD: makefile: also define cmd_CXX to pretty-print C++ build commands

Device Atlas' dummy lib will use a C++ file when built with cache
support, so for completeness we'll have to pretty-print it as well.
Let's define cmd_CXX.

17 months agoCI: enable spell check on git push
Ilya Shipitsin [Wed, 24 Jan 2024 13:26:16 +0000 (14:26 +0100)] 
CI: enable spell check on git push

17 months agoCI: extend spell check white list
Ilya Shipitsin [Wed, 24 Jan 2024 13:26:15 +0000 (14:26 +0100)] 
CI: extend spell check white list

siz - seen in src/debug.c
EXPERIM - seen in src/cli.c

17 months agoCLEANUP: fix spelling of "elemt"
Ilya Shipitsin [Wed, 24 Jan 2024 13:26:14 +0000 (14:26 +0100)] 
CLEANUP: fix spelling of "elemt"

17 months agoBUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf
Amaury Denoyelle [Fri, 26 Jan 2024 13:41:04 +0000 (14:41 +0100)] 
BUG/MEDIUM: quic: remove unsent data from qc_stream_desc buf

QCS instances use qc_stream_desc for data buffering on emission. On
stream reset, its Tx channel is closed earlier than expected. This may
leave unsent data into qc_stream_desc.

Before this patch, these unsent data would remain after QCS freeing.
This prevents the buffer to be released as no ACK reception will remove
them. The buffer is only freed when the whole connection is closed. As
qc_stream_desc buffer is limited per connection, this reduces the buffer
pool for other streams of the same connection. In the worst case if
several streams are resetted, this may completely freeze the transfer of
the remaining connection streams.

This bug was reproduced by reducing the connection buffer pool to a
single buffer instance by using the following global statement :

  tune.quic.frontend.conn-tx-buffers.limit 1.

Then a QUIC client is used which opens a stream for a large enough
object to ensure data are buffered. The client them emits a STOP_SENDING
before reading all data, which forces the corresponding QCS instance to
be resetted. The client then opens a new request but the transfer is
freezed due to this bug.

To fix this, adjust qc_stream_desc API. Add a new argument <final_size>
on qc_stream_desc_release() function. Its value is compared to the
currently buffered offset in latest qc_stream_desc buffer. If
<final_size> is inferior, it means unsent data are present in the
buffer. As such, qc_stream_desc_release() removes them to ensure the
buffer will finally be freed when all ACKs are received. It is also
possible that no data remains immediately, indicating that ACK were
already received. As such, buffer instance is immediately removed by
qc_stream_buf_free().

This must be backported up to 2.6. As this code section is known to
regression, a period of observation could be reserved before
distributing it on LTS releases.

17 months agoMINOR: quic: extract qc_stream_buf free in a dedicated function
Amaury Denoyelle [Fri, 26 Jan 2024 13:30:16 +0000 (14:30 +0100)] 
MINOR: quic: extract qc_stream_buf free in a dedicated function

On ACK reception, data are removed from buffer via qc_stream_desc_ack().
The buffer can be freed if no more data are left. A new slot is also
accounted in buffer connection pool. Extract this operation in a
dedicated private function qc_stream_buf_free().

This change should have no functional change. However it will be useful
for the next patch which needs to remove a buffer from another function.

This patch is necessary for the following bugfix. As such, it must be
backported with it up to 2.6.

17 months agoMINOR: quic: Stop hardcoding a scale shifting value (CUBIC_BETA_SCALE_FACTOR_SHIFT)
Frederic Lecaille [Thu, 25 Jan 2024 06:55:33 +0000 (07:55 +0100)] 
MINOR: quic: Stop hardcoding a scale shifting value (CUBIC_BETA_SCALE_FACTOR_SHIFT)

Very minor modification to replace a statement with an hardcoded value by a macro.

Should be backported as far as 2.6 to ease any further modification to come.

17 months agoCLEANUP: quic: Remove unused CUBIC_BETA_SCALE_FACTOR_SHIFT macro.
Frederic Lecaille [Thu, 25 Jan 2024 06:47:46 +0000 (07:47 +0100)] 
CLEANUP: quic: Remove unused CUBIC_BETA_SCALE_FACTOR_SHIFT macro.

This macro is not used and has a confusing name.

Should be backported as far as 2.6.

17 months agoBUG/MINOR: quic: newreno QUIC congestion control algorithm no more available
Frederic Lecaille [Thu, 25 Jan 2024 06:30:40 +0000 (07:30 +0100)] 
BUG/MINOR: quic: newreno QUIC congestion control algorithm no more available

There is a typo in the statement to initialize this variable when selecting
newreno as cc algo:
      const char *newreno = "newrno";
This would have happened if #defines had be used in place of several const char *
variables harcoded values.

Take the opportunity of this patch to use #defines for all the available cc algorithms.

Must be backported to 2.9.

17 months agoBUG/MEDIUM: cache: Fix crash when deleting secondary entry
Remi Tricot-Le Breton [Wed, 24 Jan 2024 15:56:39 +0000 (16:56 +0100)] 
BUG/MEDIUM: cache: Fix crash when deleting secondary entry

When a cache is "cold" and multiple clients simultaneously try to access
the same resource we must forward all the requests to the server. Next,
every "duplicated" response will be processed in http_action_store_cache
and we will try to cache every one of them regardless of whether this
response was already cached. In order to avoid having multiple entries
for a same primary key, the logic is then to first delete any
preexisting entry from the cache tree before storing the current one.
The actual previous response content will not be deleted yet though
because if the corresponding row is detached from the "avail" list it
might still be used by a cache applet if it actually performed a lookup
in the cache tree before the new response could be received.

This all means that we can end up using a valid row that references a
cache_entry that was already removed from the cache tree. This does not
pose any problem in regular caches (no 'vary' mechanism enabled) because
the applet only works on the data and not the 'cache_entry' information,
but in the "vary" context, when calling 'http_cache_applet_release' we
might call 'delete_entry' on the given entry which in turn tries to
iterate over all the secondary entries to find the right one in which
the secondary entry counter can be updated. We would then call
eb32_next_dup on an entry that was not in the tree anymore which ended
up crashing.

This crash was introduced by "48f81ec09 : MAJOR: cache: Delay cache
entry delete in reserve_hot function" which added the call to
"release_entry" in "http_cache_applet_release" that ended up crashing.

This issue was raised in GitHub #2417.
This patch must be backported to branch 2.9.

17 months agoCLEANUP: hlua: fix indent, remove extra return in hlua_core_get_var()
Aurelien DARRAGON [Wed, 24 Jan 2024 15:17:49 +0000 (16:17 +0100)] 
CLEANUP: hlua: fix indent, remove extra return in hlua_core_get_var()

This is cleanup patch to address cosmetic issues introduced in f034139bc0
("MINOR: lua: Allow reading "proc." scoped vars from LUA core.")

Also taking this opportunity to prefix the function with __LJMP to
indicate that it may longjump.

No backport needed.

17 months agoBUG/MINOR: hlua: fix uninitialized var in hlua_core_get_var()
Aurelien DARRAGON [Wed, 24 Jan 2024 15:10:55 +0000 (16:10 +0100)] 
BUG/MINOR: hlua: fix uninitialized var in hlua_core_get_var()

As raised by Coverity in GH #2223, f034139bc0 ("MINOR: lua: Allow reading
"proc." scoped vars from LUA core.") causes uninitialized reads due to
smp being passed to vars_get_by_name() without being initialized first.

Indeed, vars_get_by_name() tries to read smp->sess and smp->strm pointers.
As we're only interested in the PROC var scope, it is safe to call
vars_get_by_name() with sess and strm pointers set to NULL, thus we
simply memset smp prior to calling vars_get_by_name() to fix the issue.

This should be backported in 2.9 with f034139bc0.

17 months agoBUILD: quic: Fix build error when building QUIC against libressl.
Frederic Lecaille [Wed, 24 Jan 2024 14:37:40 +0000 (15:37 +0100)] 
BUILD: quic: Fix build error when building QUIC against libressl.

This previous commit was not sufficient to completely fix the building issue
in relation with the TLS stack 0-RTT support. LibreSSL was the last TLS
stack to refuse to compile because of undefined a QUIC specific function
for 0-RTT: SSL_set_quic_early_data_enabled().

To get rid of such compilation issues, define HA_OPENSSL_HAVE_0RTT_SUPPORT
only when building against TLS stack with 0-RTT support.

No need to backport.

17 months agoBUILD: quic: Fix build error when building QUIC against wolfssl.
Frederic Lecaille [Wed, 24 Jan 2024 13:36:41 +0000 (14:36 +0100)] 
BUILD: quic: Fix build error when building QUIC against wolfssl.

This commit:

    "MINOR: quic: Enable early data at SSL session level (aws-lc)

introduced a build error when using wolfssl as TLS stack
because it references unknown function wolfSSL_set_quic_early_data_enabled()
which is not defined in qc_set_quic_early_data_context() that must not be used
in this case. The compilation of this fonction was enabled for wolfssl when
it should not have by the mentionned commit.

No backport is needed.

17 months agoBUILD: quic: fix build error when using the compatibility layer
Willy Tarreau [Wed, 24 Jan 2024 09:46:11 +0000 (10:46 +0100)] 
BUILD: quic: fix build error when using the compatibility layer

Commit f783dd959b ("MINOR: quic: Enable early data at SSL session level
(aws-lc)") introduced a build error when using the openssl compat layer
because it references unknown function SSL_set_quic_early_data_context()
in qc_set_quic_early_data_context() that is not used in this case.

No backport is needed.

17 months agoBUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs
Willy Tarreau [Wed, 24 Jan 2024 09:31:05 +0000 (10:31 +0100)] 
BUG/MINOR: jwt: fix jwt_verify crash on 32-bit archs

The jwt_verify converter was added in 2.5 with commit 130e142ee2
("MEDIUM: jwt: Add jwt_verify converter to verify JWT integrity"). It
takes a string on input and returns an integer. It turns out that by
presetting the return value to zero before processing contents, while
the sample data is a union, it overwrites the beginning of the buffer
struct passed on input. On a 64-bit arch it's not an issue because it's
where the allocated size is stored and it's not used in the operation,
which explains why the regtest works. But on 32-bit, both the size and
the pointer are overwritten, causing a NULL pointer to be passed to
jwt_tokenize() which is not designed to support this, hence crashes.

Let's just use a temporary variable to hold the result and move the
output sample initialization to the end of the function.

This should be backported as far as 2.5.

17 months agoBUG/MEDIUM: cli: some err/warn msg dumps add LR into CSV output on stat's CLI
Emeric Brun [Tue, 23 Jan 2024 14:44:32 +0000 (15:44 +0100)] 
BUG/MEDIUM: cli: some err/warn msg dumps add LR into CSV output on stat's CLI

The initial purpose of CSV stats through CLI was to make it easely
parsable by scripts. But in some specific cases some error or warning
messages strings containing LF were dumped into cells of this CSV.

This made some parsing failure on several tools. In addition, if a
warning or message contains to successive LF, they will be dumped
directly but double LFs tag the end of the response on CLI and the
client may consider a truncated response.

This patch extends the 'csv_enc_append' and 'csv_enc' functions used
to format quoted string content according to RFC  with an additionnal
parameter to convert multi-lines strings to one line: CRs are skipped,
and LFs are replaced with spaces. In addition and optionally, it is
also possible to remove resulting trailing spaces.

The call of this function to fill strings into stat's CSV output is
updated to force this conversion.

This patch should be backported on all supported branches (issue was
already present in v2.0)

17 months agoMINOR: quic: Correctly wait for the completion of handshakes with early data (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 10:50:38 +0000 (11:50 +0100)] 
MINOR: quic: Correctly wait for the completion of handshakes with early data (aws-lc)

This patch impacts only the haproxy builds against aws-lc TLS stack (USE_OPENSSL_AWSLC).

As mentionned by the boringssl documentation, SSL_do_handshake() completes as soon
as ClientHello is processed and server flight sent (from the TLS stack to the
server endpoint I guess). Into QUIC, the completion has as side effect to discard
the Handshake packet number space. If this handshake completion is not deffered,
the Handshake level CRYPTO data will not be sent to the peer (because of the
assotiated packet number space discarding). According to the documentation,
SSL_in_early_data() may be used to do that. If it returns 1, this means that
the handshake is still in progress but has enough progressed to send half-RTT
data.

This patch is required to make the haproxy builds against aws-lc TLS stack support 0-RTT.

17 months agoMINOR: ssl_sock: Early data disabled during SSL_CTX switching (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 13:45:25 +0000 (14:45 +0100)] 
MINOR: ssl_sock: Early data disabled during SSL_CTX switching (aws-lc)

This patch impacts only haproxy when built against aws-lc TLS stack (OPENSSL_IS_AWSLC).

During the SSL_CTX switching from ssl_sock_switchctx_cbk() callback,
ssl_sock_switchctx_set() is called. This latter calls SSL_set_SSL_CTX()
whose aims is to change the SSL_CTX attached o an SSL object (TLS session).
But the aws-lc (or boringssl) implementation of this function copy
the "early data enabled" setting value (boolean) coming with the SSL_CTX object
into the SSL object. So, if not set in the SSL_CTX object this setting disabled
the one which has been set by configuration into the SSL object
(see qc_set_quic_early_data_enabled(), it calls SSL_set_early_data_enabled()
with an SSL object as parameter).

Fix this enabling the "early data enabled" setting into the SSL_CTX before
setting this latter into the SSL object.

This patch is required to make QUIC 0-RTT work with haproxy built against
aws-lc.

Note that, this patch should also help in early data support for TCP connections.

17 months agoMINOR: quic: Enable early data at SSL session level (aws-lc)
Frederic Lecaille [Tue, 23 Jan 2024 10:41:44 +0000 (11:41 +0100)] 
MINOR: quic: Enable early data at SSL session level (aws-lc)

This patch impacts only the haproxy build against aws-lc TLS stack (USE_OPENSSL_AWSLC).

Implement qc_set_quic_early_data_enabled() new function to enable
early data at session level. To make QUIC O-RTT work, a context string
must be set calling SSL_set_quic_early_data_context(). This is a
subset of the encoded transport parameters which is used for this.
Note that some application level settings should be also added (TODO).

This patch is required to make 0-RTT work for haproxy builds against aws-lc.

17 months agoMINOR: quic: Transport parameters encoding without version_information
Frederic Lecaille [Tue, 23 Jan 2024 10:28:45 +0000 (11:28 +0100)] 
MINOR: quic: Transport parameters encoding without version_information

Encode the version_information parameter only if the chosen version is provided
to quic_transport_params_encode() whose aim is to encode into a buffer all the
transport parameters passed as parameter (struct quic_params *p) in addition
to the version_information parameter.

This enables the support of transport parameters encoding without
the version_information transport parameter. This is useful for build against TLS stacks
as boringssl, aws-lc where a subset of the listener transport parameters
without version_information must be set as context string for acception
early data (see https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_set_quic_early_data_context).

This patch is required to make haproxy builds against aws-lc TLS stack
(USE_OPENSSL_AWSLC) support 0-RTT. Does not impact the others builds.

17 months agoBUILD: stick-table: fix build error on 32-bit platforms
Willy Tarreau [Sun, 21 Jan 2024 07:21:35 +0000 (08:21 +0100)] 
BUILD: stick-table: fix build error on 32-bit platforms

Commit 9b2717e7b ("MINOR: stktable: use {show,set,clear} table with ptr")
stores a pointer in a long long (64bit), which fails the cas to void* on
32-bit platforms:

  src/stick_table.c: In function 'table_process_entry_per_ptr':
  src/stick_table.c:5136:37: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
   5136 |         ts = stktable_lookup_ptr(t, (void *)ptr);

On all our supported platforms, longs and pointers are of the same size,
so let's just turn this to a ulong instead.

17 months agoMINOR: connection: add sample fetches to report per-connection glitches
Willy Tarreau [Wed, 17 Jan 2024 17:00:21 +0000 (18:00 +0100)] 
MINOR: connection: add sample fetches to report per-connection glitches

Now with fc_glitches and bc_glitches we can retrieve the number of
detected glitches on a front or back connection. On the backend it
can indicate a bug in a server that may induce frequent reconnections
hence CPU usage in TLS reconnections, and on the frontend it may
indicate an abusive client that may be trying to attack the stack
or to fingerprint it. Small non-zero values are definitely expected
and can be caused by network glitches for example, as well as rare
bugs in the other component (or maybe even in haproxy). These should
never be considered as alarming as long as they remain low (i.e.
much less than one per request). A reg-test is provided.

17 months agoMINOR: mux-h2: implement MUX_CTL_GET_GLITCHES
Willy Tarreau [Wed, 17 Jan 2024 16:21:29 +0000 (17:21 +0100)] 
MINOR: mux-h2: implement MUX_CTL_GET_GLITCHES

This reports the number of glitches on a connection.

17 months agoMINOR: connection: add a new mux_ctl to report number of connection glitches
Willy Tarreau [Wed, 17 Jan 2024 16:20:30 +0000 (17:20 +0100)] 
MINOR: connection: add a new mux_ctl to report number of connection glitches

MUX_CTL_GET_GLITCHES will report the non-negative number of clitches
observed on a connection, or -1 if not supported.

17 months agoMINOR: mux-h2: add a counter of "glitches" on a connection
Willy Tarreau [Wed, 17 Jan 2024 15:57:23 +0000 (16:57 +0100)] 
MINOR: mux-h2: add a counter of "glitches" on a connection

There are a lot of H2 events which are not invalid from a protocol
perspective but which are yet anomalies, especially when repeated. They
can come from bogus or really poorly implemlented clients, as well as
purposely built attacks, as we've seen in the past with various waves
of attempts at abusing H2 stacks.

In order to better deal with such situations, it would be nice to be
able to sort out what is correct and what is not. There's already the
HTTP error counter that may even be updated on a tracked connection,
but HTTP errors are something clearly defined while there's an entire
scope of gray area around it that should not fall into it.

This patch introduces the notion of "glitches", which normally correspond
to unexpected and temporary malfunction. And this is exactly what we'd
like to monitor. For example a peer is not misbehaving if a request it
sends fails to decode because due to HPACK compression it's larger than
a buffer, and for this reason such an event is reported as a stream error
and not a connection error. But this causes trouble nonetheless and should
be accounted for, especially to detect if it's repeated. Similarly, a
truncated preamble or settings frame may very well be caused by a network
hiccup but how do we know that in the logs? For such events, a glitch
counter is incremented on the connection.

For now a total of 41 locations were instrumented with this and the
counter is reported in the traces when not null, as well as in
"show sess" and "show fd". This was done using a new function,
"h2c_report_glitch()" so that it becomes easier to extend to more
advanced processing (applying thresholds, producing logs, escalating
to connection error, tracking etc).

A test with h2spec shows it reported in 8545 trace lines for 147 tests,
with some reaching value 3 in a same test (e.g. HPACK errors).

Some places were not instrumented, typically anything that can be
triggered on perfectly valid activity (received data after RST being
emitted, timeouts, etc). Some types of events were thought about,
such as INITIAL_WINDOW_SIZE after the first SETTINGS frame, too small
window update increments, etc. It just sounds too early to know if
those are currently being triggered by perfectly legit clients. Also
it's currently not incremented on timeouts so that we don't do that
repeatedly on short keep-alive timeouts, though it could make sense.
This may change in the future depending on how it's used. For now
this is not exposed outside of traces and debugging.

17 months agoMINOR: mux-h2/traces: add a missing trace on connection WU with negative inc
Willy Tarreau [Wed, 17 Jan 2024 15:56:18 +0000 (16:56 +0100)] 
MINOR: mux-h2/traces: add a missing trace on connection WU with negative inc

The test was performed but no trace emitted, which can complicate certain
diagnostics, so let's just add the trace for this rare case. It may safely
be backported though this is really not important.

17 months agoBUG/MEDIUM: mux-h2: refine connection vs stream error on headers
Willy Tarreau [Thu, 18 Jan 2024 16:01:45 +0000 (17:01 +0100)] 
BUG/MEDIUM: mux-h2: refine connection vs stream error on headers

Commit 7021a8c4d8 ("BUG/MINOR: mux-h2: also count streams for refused
ones") addressed stream counting issues on some error cases but not
completely correctly regarding the conn_err vs stream_err case. Indeed,
contrary to the initial analysis, h2c_dec_hdrs() can set H2_CS_ERROR
when facing some unrecoverable protocol errors, and it's not correct
to send it to strm_err which will only send the RST_STREAM frame and
the subsequent GOAWAY frame is in fact the result of the read timeout.

The difficulty behind this lies on the sequence of output validations
because h2c_dec_hdrs() returns two results at once:
  - frame processing status (done/incomplete/failed)
  - connection error status

The original ordering requires to write 2 exemplaries of the exact
same error handling code disposed differently, which the patch above
tried to factor to one. After careful inspection of h2c_dec_hdrs()
and its comments, it's clear that it always returns -1 on failure,
*including* connection errors. This means we can rearrange the test
to get rid of the missing data first, and immediately enter the
no-return zone where both the stream and connection errors can be
checked at the same place, making sure to consistently maintain
error counters. This is way better because we don't have to update
stream counters on the error path anymore. h2spec now passes the
test much faster.

This will need to be backported to the same branches as the commit
above, which was already backported to 2.9.

17 months agoDOC: configuration: fix set-dst in actions keywords matrix
Aurelien DARRAGON [Thu, 18 Jan 2024 14:13:05 +0000 (15:13 +0100)] 
DOC: configuration: fix set-dst in actions keywords matrix

Since d54e8f8107 ("DOC: config: reorganize actions into their own section")
dconv-generated shortlink for "set-dst" in actions keywords matrix is
broken.

This is due to trailing "<expr>" which should not be specified in the
matrix, but only in the actual keyword prototype and description.

This should be backported in 2.9 with d54e8f8107.

17 months agoMINOR: vars: fix indentation in var_clear_buffer()
Aurelien DARRAGON [Thu, 18 Jan 2024 09:59:23 +0000 (10:59 +0100)] 
MINOR: vars: fix indentation in var_clear_buffer()

Fix indentation in var_clear_buffer() since it is exclusively using
spaces.

Could be backported if a fix depends on it.

17 months agoBUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)
Frederic Lecaille [Tue, 16 Jan 2024 09:17:27 +0000 (10:17 +0100)] 
BUG/MEDIUM: quic: keylog callback not called (USE_OPENSSL_COMPAT)

This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT)
and it was introduced by this commit:

    BUG/MINOR: quic: Wrong keylog callback setting.

quic_tls_compat_keylog_callback() callback was no more set when the SSL keylog was
enabled by tune.ssl.keylog setting. This is the callback which sets the TLS secrets
into haproxy.

Set it again when the SSL keylog is not enabled by configuration.

Thank you to @Greg57070 for having reported this issue in GH #2412.

Must be backported as far as 2.8.

18 months agoBUG/MINOR: mux-h2: also count streams for refused ones
Willy Tarreau [Fri, 12 Jan 2024 17:36:57 +0000 (18:36 +0100)] 
BUG/MINOR: mux-h2: also count streams for refused ones

There are a few places where we can reject an incoming stream based on
technical errors such as decoded headers that are too large for the
internal buffers, or memory allocation errors. In this case we send
an RST_STREAM to abort the request, but the total stream counter was
not incremented. That's not really a problem, until one starts to try
to enforce a total stream limit using tune.h2.fe.max-total-streams,
and which will not count such faulty streams. Typically a client that
learns too large cookies and tries to replay them in a way that
overflows the maximum buffer size would be rejected and depending on
how they're implemented, they might retry forever.

This patch removes the stream count increment from h2s_new() and moves
it instead to the calling functions, so that it translates the decision
to process a new stream instead of a successfully decoded stream. The
result is that such a bogus client will now be blocked after reaching
the total stream limit.

This can be validated this way:

  global
        tune.h2.fe.max-total-streams 128
        expose-experimental-directives
        trace h2 sink stdout
        trace h2 level developer
        trace h2 verbosity complete
        trace h2 start now

  frontend h
        bind :8080
        mode http
        redirect location /

Sending this will fill frames with 15972 bytes of cookie headers that
expand to 16500 for storage+index once decoded, causing "message too large"
events:

  (dev/h2/mkhdr.sh -t p;dev/h2/mkhdr.sh -t s;
   for sid in {0..1000}; do
     dev/h2/mkhdr.sh  -t h -i $((sid*2+1)) -f es,eh \
       -R "828684410f7777772e6578616d706c652e636f6d \
           $(for i in {1..66}; do
             echo -n 60 7F 73 433d $(for j in {1..24}; do
               echo -n 2e313233343536373839; done);
            done) ";
   done) | nc 0 8080

Now it properly stops after sending 128 streams.

This may be backported wherever commit 983ac4397 ("MINOR: mux-h2:
support limiting the total number of H2 streams per connection") is
present, since without it, that commit is less effective.

18 months agoDEV: h2: support hex-encoded data sequences in mkhdr
Willy Tarreau [Fri, 12 Jan 2024 16:53:50 +0000 (17:53 +0100)] 
DEV: h2: support hex-encoded data sequences in mkhdr

For HPACK-encoded headers (particularly with huffman encoding), it's
really necessary to support hex sequences as they appear in RFC7541
examples, so let's support hex digit pairs with -R.

Now it's possible to do this to send GET https://www.example.com/ :

    (dev/h2/mkhdr.sh -t p; dev/h2/mkhdr.sh -t s;
     dev/h2/mkhdr.sh -t h -i 1 -f es,eh \
     -R '8286 8441 0f77 7777 2e65 7861 6d70 6c65 2e63 6f6d ') | nc 0 8080

18 months agoDEV: h2: add support for multiple flags in mkhdr
Willy Tarreau [Fri, 12 Jan 2024 16:32:45 +0000 (17:32 +0100)] 
DEV: h2: add support for multiple flags in mkhdr

The mkhdr script did not support passing multiple flags at once using
symbolic names. That's now done.

18 months agoDOC: INSTALL: require at least WolfSSL 5.6.6
William Lallemand [Fri, 12 Jan 2024 16:48:45 +0000 (17:48 +0100)] 
DOC: INSTALL: require at least WolfSSL 5.6.6

WolfSSL 5.6.6 introduces the equivalent of the clienthello callback, so
lets switch to this version.

18 months agoCI: github: update wolfSSL to 5.6.6
William Lallemand [Fri, 12 Jan 2024 16:48:22 +0000 (17:48 +0100)] 
CI: github: update wolfSSL to 5.6.6

Update wolfSSL to 5.6.6

18 months agoMEDIUM: ssl: implements 'default-crt' keyword for bind Lines
William Lallemand [Fri, 12 Jan 2024 16:32:48 +0000 (17:32 +0100)] 
MEDIUM: ssl: implements 'default-crt' keyword for bind Lines

The 'default-crt' bind keyword allows to specify multiples
default/fallback certificates, allowing one to have an RSA as well as an
ECDSA default.

18 months agoDOC: configuration: update configuration on how to have multiple default certs
William Lallemand [Fri, 12 Jan 2024 16:01:30 +0000 (17:01 +0100)] 
DOC: configuration: update configuration on how to have multiple default certs

HAProxy now allows to configure default certificates with SNI filters or
multi-cert bundle.

18 months agoREORG: ssl: move 'generate-certificates' code to ssl_gencert.c
William Lallemand [Fri, 12 Jan 2024 14:23:49 +0000 (15:23 +0100)] 
REORG: ssl: move 'generate-certificates' code to ssl_gencert.c

A lot of code specific to the 'generate-certificates' option was left in
ssl_sock.c.

Move the code to 'ssl_gencert.c' and 'ssl_gencert.h'

18 months agoMEDIUM: ssl: does not use default_ctx for 'generate-certificate' option
William Lallemand [Thu, 11 Jan 2024 14:10:33 +0000 (15:10 +0100)] 
MEDIUM: ssl: does not use default_ctx for 'generate-certificate' option

The 'generate-certificates' option does not need its dedicated SSL_CTX
*, it only needs the default SSL_CTX.

Use the default SSL_CTX found in the sni_ctx to generate certificates.

It allows to remove all the specific default_ctx initialization, as
well as the default_ssl_conf and 'default_inst'.

18 months agoMEDIUM: ssl: generate '*' SNI filters for default certificates
William Lallemand [Wed, 10 Jan 2024 15:07:17 +0000 (16:07 +0100)] 
MEDIUM: ssl: generate '*' SNI filters for default certificates

This patch follows the previous one about default certificate selection
("MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA
selection").

This patch generates '*" SNI filters for the first certificate of a
bind line, it will be used to match default certificates. Instead of
setting the default_ctx pointer in the bind line.

Since the filters are in the SNI tree, it allows to have multiple
default certificate and restore the ecdsa/rsa selection with a
multi-cert bundle.

This configuration:
   # foobar.pem.ecdsa and foobar.pem.rsa
   bind *:8443 ssl crt foobar.pem crt next.pem

will use "foobar.pem.ecdsa" and "foobar.pem.rsa" as default
certificates.

Note: there is still cleanup needed around default_ctx.

This was discussed in github issue #2392.

18 months agoMEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA selection
William Lallemand [Wed, 10 Jan 2024 13:05:59 +0000 (14:05 +0100)] 
MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA selection

This patch changes the default certificate mechanism.

Since the beginning of SSL in HAProxy, the default certificate was the first
certificate of a bind line. This allowed to fallback on this certificate
when no servername extension was sent by the server, or when no SAN nor
CN was available in the certificate.

When using a multi-certificate bundle (ecdsa+rsa), it was possible to
have both certificates as the fallback one, leting openssl chose the
right one. This was possible because a multi-certificate bundle
was generating a unique SSL_CTX for both certificates.

When the haproxy and openssl architecture evolved, we decided to
use multiple SSL_CTX for a multi-cert bundle, in order to simplify the
code and allow updates over the CLI.

However only one default_ctx was allowed, so we lost the ability to
chose between ECDSA and RSA for the default certificate.

This patch allows to use a '*' filter for a certificate, which allow to
lookup between multiple '*' filter, and have one in RSA and another one
in ECDSA. It replaces the default_ctx mechanism in the ClientHello
callback and use the standard algorithm to look for a default cert and
chose between ECDSA and RSA.

/!\ This patch breaks the automatic setting of the default certificate, which
will be introduce in the next patch. So the first certificate of a bind
line won't be used as a defaullt anymore.

To use this feature, one could use crt-list with '*' filters:

$ cat foo.crtlist
foobar.pem.rsa   *
foobar.pem.ecdsa *

In order to test the feature, it's easy to send a request without
the servername extension and use ECDSA or RSA compatible ciphers:

$ openssl s_client -connect localhost:8443 -tls1_2 -cipher ECDHE-RSA-AES256-GCM-SHA384
$ openssl s_client -connect localhost:8443 -tls1_2 -cipher ECDHE-ECDSA-AES256-GCM-SHA384

18 months agoBUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control
Amaury Denoyelle [Tue, 9 Jan 2024 10:42:08 +0000 (11:42 +0100)] 
BUG/MINOR: mux-quic: do not prevent non-STREAM sending on flow control

Data emitted by QUIC MUX is restrained by the peer flow control. This is
checked on stream and connection level inside qcc_io_send().

The connection level check was placed early in qcc_io_send() preambule.
However, this also prevents emission of other frames STOP_SENDING and
RESET_STREAM, until flow control limitation is increased by a received
MAX_DATA. Note that local flow control frame emission is done prior in
qcc_io_send() and so are not impacted.

In the worst case, if no MAX_DATA is received for some time, this could
delay significantly streams closure and resource free. However, this
should be rare as other peers should anticipate emission of MAX_DATA
before reaching flow control limit. In the end, this is also covered by
the MUX timeout so the impact should be minimal

To fix this, move the connection level check directly inside QCS sending
loop. Note that this could cause unnecessary looping when connection
flow control level is reached and no STOP_SENDING/RESET_STREAM are
needed.

This should be backported up to 2.6.

18 months agoBUILD: quic: missing include for quic_tp
Amaury Denoyelle [Wed, 10 Jan 2024 09:57:18 +0000 (10:57 +0100)] 
BUILD: quic: missing include for quic_tp

Add missing netinet/in.h required for in_addr/in6_addr types.

This should be backported up to 2.9.

18 months agoCLEANUP: fix spelling of "occured" in src/h3.c
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:11 +0000 (20:49 +0100)] 
CLEANUP: fix spelling of "occured" in src/h3.c

18 months agoCI: codespell: add more words to whitelist
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:10 +0000 (20:49 +0100)] 
CI: codespell: add more words to whitelist

"Collet" is "Yann Collet" - a developer of xxhash
"bu" is variable name in src/sock_unix.c
"htmp" is variable name in src/quic_retransmit.c

18 months agoCI: codespell: ignore some words in URLs
Ilya Shipitsin [Thu, 11 Jan 2024 19:49:09 +0000 (20:49 +0100)] 
CI: codespell: ignore some words in URLs

"trafic,ressources" are found in URIs, due to
https://github.com/codespell-project/actions-codespell/issues/55 we cannot use
wildcard for exclusion, let start with fixed list

18 months agoMEDIUM: http: add the ability to redefine http-err-codes and http-fail-codes
Willy Tarreau [Thu, 11 Jan 2024 11:06:49 +0000 (12:06 +0100)] 
MEDIUM: http: add the ability to redefine http-err-codes and http-fail-codes

The new global keywords "http-err-codes" and "http-fail-codes" allow to
redefine which HTTP status codes indicate a client-induced error or a
server error, as tracked by stick-table counters. This is only done
globally, though everything was done so that it could easily be extended
to a per-proxy mechanism if there was a real need for this (but it would
eat quite more RAM then).

A simple reg-test was added (http-err-fail.vtc).

18 months agoMEDIUM: http_act: check status codes against the bit fields for err/fail
Willy Tarreau [Wed, 10 Jan 2024 17:44:30 +0000 (18:44 +0100)] 
MEDIUM: http_act: check status codes against the bit fields for err/fail

This drops the hard-coded 4xx and 5xx status codes for err_cnt and
fail_cnt, in favor of the new bit fields that will soon be configurable.
There should be no difference at all since the bit fields are initialized
to the exact same sets (400-499 for err, 500-599 minus 501 and 505 for
fail).