Commit Graph

16997 Commits

Author SHA1 Message Date
Miroslav Zagorac
220a1ad33b Revert "MINOR: opentracing: change the scope of the variable 'ot.uuid' from 'sess' to 'txn'"
This reverts commit 560c7b874a.

The ot.uuid variable should have the 'sess' scope because it is created
when an OpenTracing filter is attached to a stream.  After that, the
stream processing is started and on that occasion the contexts for the
variables that have the range 'txn' and 'req' are initialized.  This
means that we cannot use variables with the specified scopes before that
point.

This patch must be backported in 2.5.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
e6f76f0663 CLEANUP: opentracing: removed unused function flt_ot_var_get()
The flt_ot_var_get() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.

This patch must be backported as far as 2.4.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
e40a34cea1 CLEANUP: opentracing: removed unused function flt_ot_var_unset()
The flt_ot_var_unset() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.

This patch must be backported as far as 2.4.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
b51e94b0b3 DOC: opentracing: corrected comments in function descriptions
Several comments in the function descriptions have been corrected.

This patch must be backported as far as 2.4.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
9154e00f70 EXAMPLES: opentracing: refined shell scripts for testing filter performance
When calling the 'basename' command, the argument ${0} is enclosed in
quotation marks.  This is necessary if the path of the executable script
(contained in that argument) has "non-standard" elements, such as space
and the like.

Also, in the script 'test-speed.sh' the function sh_exit() has been added
for easier printing of messages at the end of execution.

This patch must be backported as far as 2.4.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
9964ad8436 BUG/BUILD: opentracing: fixed OT_DEFINE variable setting
In case of using parameter 'OT_USE_VARS=1', the value of the OT_DEFINE
variable is set incorrectly (i.e. the previous value was deleted and a
new one set instead of adding new content).

This patch must be backported in 2.5.
2022-04-08 16:31:33 +02:00
Miroslav Zagorac
728627f932 BUG/MINOR: opentracing: setting the return value in function flt_ot_var_set()
Function flt_ot_var_set() did not check whether the variable was
successfully set or not.  In case of failure, the value -1 is returned.

This patch must be backported as far as 2.4.
2022-04-08 16:31:33 +02:00
Frédéric Lécaille
8c7927c6dd MINOR: quic_tls: Make key update use of reusable cipher contexts
We modify the key update feature implementation to support reusable cipher contexts
as this is done for the other cipher contexts for packet decryption and encryption.
To do so we attach a context to the quic_tls_kp struct and initialize it each time
the underlying secret key is updated. Same thing when we rotate the secrets keys,
we rotate the contexts as the same time.
2022-04-08 15:38:29 +02:00
Frédéric Lécaille
3dfd4c4b0d MINOR: quic: Add short packet key phase bit values to traces
This is useful to diagnose key update related issues.
2022-04-08 15:38:29 +02:00
Frédéric Lécaille
9688a8df49 CLEANUP: quic: Do not set any cipher/group from ssl_quic_initial_ctx()
These settings are potentially cancelled by others setting initialization shared
with SSL sock bindings. This will have to be clarified when we will adapt the
QUIC bindings configuration.
2022-04-08 15:38:29 +02:00
Frédéric Lécaille
f2f4a4eee5 MINOR: quic_tls: Stop hardcoding cipher IV lengths
For QUIC AEAD usage, the number of bytes for the IVs is always 12.
2022-04-08 15:38:29 +02:00
Frédéric Lécaille
f4605748f4 MINOR: quic_tls: Add reusable cipher contexts to QUIC TLS contexts
Add ->ctx new member field to quic_tls_secrets struct to store the cipher context
for each QUIC TLS context TX/RX parts.
Add quic_tls_rx_ctx_init() and quic_tls_tx_ctx_init() functions to initialize
these cipher context for RX and TX parts respectively.
Make qc_new_isecs() call these two functions to initialize the cipher contexts
of the Initial secrets. Same thing for ha_quic_set_encryption_secrets() to
initialize the cipher contexts of the subsequent derived secrets (ORTT, Handshake,
1RTT).
Modify quic_tls_decrypt() and quic_tls_encrypt() to always use the same cipher
context without allocating it each time they are called.
2022-04-08 15:38:29 +02:00
Frédéric Lécaille
82851bd3cb BUG/MEDIUM: quic: Possible crash from quic_free_arngs()
All quic_arng_node objects are allocated from "pool_head_quic_arng" memory pool.
They must be deallocated calling pool_free().
2022-04-08 15:38:29 +02:00
Willy Tarreau
9cc88c3075 BUG/MINOR: quic: set the source not the destination address on accept()
When a QUIC connection is accepted, the wrong field is set from the
client's source address, it's the destination instead of the source.
No backport needed.
2022-04-08 14:43:27 +02:00
Amaury Denoyelle
8038821c88 BUG/MEDIUM: mux-quic: properly release conn-stream on detach
On qc_detach(), the qcs must cleared the conn-stream context and set its
cs pointer to NULL. This prevents the qcs to point to a dangling
reference.

Without this, a SEGFAULT may occurs in qc_wake_some_streams() when
accessing an already detached conn-stream instance through a qcs.

Here is the SEGFAULT observed on haproxy.org.
 Program terminated with signal 11, Segmentation fault.
 1234                            else if (qcs->cs->data_cb->wake) {
 (gdb) p qcs.cs.data_cb
 $1 = (const struct data_cb *) 0x0

This can happens since the following patch :
 commit fe035eca3a
 MEDIUM: mux-quic: report errors on conn-streams
2022-04-08 12:09:23 +02:00
Amaury Denoyelle
9c3955c98c CLEANUP: mux-quic: remove uneeded TODO in qc_detach
The stream mux buffering has been reworked since the introduction of the
struct qc_stream_desc. A qcs is now able to quickly release its buffer
to the quic-conn.
2022-04-08 12:08:50 +02:00
Christopher Faulet
114e759d5d BUG/MEDIUM: http-act: Don't replace URI if path is not found or invalid
For replace-path, replace-pathq and replace-uri actions, we must take care
to not match on the selected element if it is not defined.

regex_exec_match2() function expects to be called with a defined
subject. However, if the request path is invalid or not found, the function
is called with a NULL subject, leading to a crash when compiled without the
PRCE/PCRE2 support.

For instance the following rules crashes HAProxy on a CONNECT request:

  http-request replace-path /short/(.) /\1

This patch must be backported as far as 2.0.
2022-04-08 10:45:31 +02:00
Christopher Faulet
21ac0eec28 BUG/MEDIUM: http-conv: Fix url_enc() to not crush const samples
url_enc() encodes an input string by calling encode_string(). To do so, it
adds a trailing '\0' to the sample string. However it never restores the
sample string at the end. It is a problem for const samples. The sample
string may be in the middle of a buffer. For instance, the HTTP headers
values are concerned.

However, instead of modifying the sample string, it is easier to rely on
encode_chunk() function. It does the same but on a buffer.

This patch must be backported as far as 2.2.
2022-04-08 10:12:59 +02:00
Christopher Faulet
dca3b5b2c6 BUG/MINOR: http_client: Don't add input data on an empty request buffer
When compiled in debug mode, a BUG_ON triggers an error when the payload is
fully transfered from the http-client buffer to the request channel
buffer. In fact, when channel_add_input() is called, the request buffer is
empty. So an error is reported when those data are directly forwarded,
because we try to add some output data on a buffer with no data.

To fix the bug, we must be sure to call channel_add_input() after the data
transfer.

The bug was introduced by the commit ccc7ee45f ("MINOR: httpclient: enable
request buffering"). So, this patch must be backported if the above commit
is backported.
2022-04-07 11:04:39 +02:00
Christopher Faulet
2eb5243e7f BUG/MEDIUM: mux-h1: Set outgoing message to DONE when payload length is reached
When a message is sent, we can switch it state to MSG_DONE if all the
announced payload was processed. This way, if the EOM flag is not set on the
message when the last expected data block is processed, the message can
still be set to MSG_DONE state.

This bug is related to the previous ones. There is a design issue with the
HTX since the 2.4. When the EOM HTX block was replaced by a flag, I tried
hard to be sure the flag is always set with the last HTX block on a
message. It works pretty well for all messages received from a client or a
server. But for internal messages, it is not so simple. Because applets
cannot always properly handle the end of messages. So, there are some cases
where the EOM flag is set on an empty message.

As a workaround, for chunked messages, we can add an EOT HTX block. It does
the trick. But for messages with a content-length, there is no empty DATA
block. Thus, the only way to be sure the end of the message was reached in
this case is to detect it in the H1 multiplexr.

We already count the amount of data processed when the payload length is
announced. Thus, we must only switch the message in DONE state when last
bytes of the payload are received. Or when the EOM flag is received of
course.

This patch must be backported as far as 2.4.
2022-04-07 11:04:07 +02:00
Christopher Faulet
be69cbdafe BUG/MEDIUM: promex: Be sure to never set EOM flag on an empty HTX message
It is the same bug than "BUG/MEDIUM: stats: Be sure to never set EOM flag on
an empty HTX message". We must be sure to set the EOM flag on a non empyt
message to be sure the mux on the client will properly handle
shutdowns. Otherwise, it may miss the end of the message and considers any
shutdown as an abort.

This patch must be backported as far as 2.4. On previous version there is
still the EOM HTX block.
2022-04-07 11:04:07 +02:00
Christopher Faulet
f89af9c205 BUG/MEDIUM: hlua: Don't set EOM flag on an empty HTX message in HTTP applet
In a lua HTTP applet, when the script is finished, we must be sure to not
set the EOM on an empty message. Otherwise, because there is no data to
send, the mux on the client side may miss the end of the message and
consider any shutdown as an abort.

See "UG/MEDIUM: stats: Be sure to never set EOM flag on an empty HTX
message" for details.

This patch must be backported as far as 2.4. On previous version there is
still the EOM HTX block.
2022-04-07 11:04:07 +02:00
Christopher Faulet
08b45cb8fd BUG/MEDIUM: stats: Be sure to never set EOM flag on an empty HTX message
During the last call to the stats I/O handle, it is possible to have nothing
to dump. It may happen for many reasons. For instance, all remaining proxies
are disabled or they don't match the specified scope. In HTML or in JSON, it
is not really an issue because there is a footer. So there are still some
data to push in the response channel buffer. In CSV, it is a problem because
there is no footer. It means it is possible to finish the response with no
payload at all in the HTX message. Thus, the EOM flag may be added on an
empty message. When this happens, a shutdown is performed on an empty HTX
message. Because there is nothing to send, the mux on the client side is not
notified that the message was properly finished and interprets the shutdown
as an abort.

The response is chunked. So an abort at this stage means the last CRLF is
never sent to the client. All data were sent but the message is invalid
because the response chunking is not finished. If the reponse is compressed,
because of a similar bug in the comppression filter, the compression is also
aborted and the content is truncated because some data a lost in the
compression filter.

It is design issue with the HTX. It must be addressed. And there is an
opportunity to do so with the recent conn-stream refactoring. It must be
carefully evaluated first. But it is possible. In the means time and to also
fix stable versions, to workaround the bug, a end-of-trailer HTX block is
systematically added at the end of the message when the EOM flag is set if
the HTX message is empty. This way, there are always some data to send when
the EOM flag is set.

Note that with a H2 client, it is only a problem when the response is
compressed.

This patch should fix the issue #1478. It must be backported as far as
2.4. On previous versions there is still the EOM block.
2022-04-07 11:04:07 +02:00
Christopher Faulet
d057960769 BUG/MINOR: fcgi-app: Don't add C-L header on response to HEAD requests
In the FCGI app, when a full response is received, if there is no
content-length and transfer-encoding headers, a content-length header is
automatically added. This avoid, as far as possible to chunk the
response. This trick was added because, most of time, scripts don"t add
those headers.

But this should not be performed for response to HEAD requests. Indeed, in
this case, there is no payload. If the payload size is not specified, we
must not added it by hand. Otherwise, a "content-length: 0" will always be
added while it is not the real payload size (unknown at this stage).

This patch should solve issue #1639. It must be backported as far as 2.2.
2022-04-07 11:04:07 +02:00
Amaury Denoyelle
b515b0af1d MEDIUM: quic: report closing state for the MUX
Define a new API to notify the MUX from the quic-conn when the
connection is about to be closed. This happens in the following cases :
- on idle timeout
- on CONNECTION_CLOSE emission or reception

The MUX wake callback is called on these conditions. The quic-conn
QUIC_FL_NOTIFY_CLOSE is set to only report once. On the MUX side,
connection flags CO_FL_SOCK_RD_SH|CO_FL_SOCK_WR_SH are set to interrupt
future emission/reception.

This patch is the counterpart to
  "MEDIUM: mux-quic: report CO_FL_ERROR on send".
Now the quic-conn is able to report its closing, which may be translated
by the MUX into a CO_FL_ERROR on the connection for the upper layer.
This allows the MUX to properly react to the QUIC closing mechanism for
both idle-timeout and closing/draining states.
2022-04-07 10:37:45 +02:00
Amaury Denoyelle
fe035eca3a MEDIUM: mux-quic: report errors on conn-streams
Complete the error reporting. For each attached streams, if CO_FL_ERROR
is set, mark them with CS_FL_ERR_PENDING|CS_FL_ERROR. This will notify
the upper layer to trigger streams detach and release of the MUX.

This reporting is implemented in a new function qc_wake_some_streams(),
called by qc_wake(). This ensures that a lower-layer error is quickly
reported to the individual streams.
2022-04-07 10:37:45 +02:00
Amaury Denoyelle
d97fc804f9 MEDIUM: mux-quic: report CO_FL_ERROR on send
Mark the connection with CO_FL_ERROR on qc_send() if the socket Tx is
closed. This flag is used by the upper layer to order a close on the
MUX. This requires to check CO_FL_ERROR in qcc_is_dead() to process to
immediate MUX free when set.

The qc_wake() callback has been completed. Most notably, it now calls
qc_send() to report a possible CO_FL_ERROR. This is useful because
qc_wake() is called by the quic-conn on imminent closing.

Note that for the moment the error flag can never be set because the
quic-conn does not report when the Tx socket is closed. This will be
implemented in a following patch.
2022-04-07 10:35:34 +02:00
Amaury Denoyelle
c933780f1e MINOR: mux-quic: centralize send operations in qc_send
Regroup all features related to sending in qc_send(). This will be
useful when qc_send() will be called outside of the io-cb.

Currently, flow-control frames generation is now automatically
integrated in qc_send().
2022-04-07 10:23:10 +02:00
Amaury Denoyelle
198d35f9c6 MINOR: mux-quic: define is_active app-ops
Add a new app layer operation is_active. This can be used by the MUX to
check if the connection can be considered as active or not. This is used
inside qcc_is_dead as a first check.

For example on HTTP/3, if there is at least one bidir client stream
opened the connection is active. This explicitly ignore the uni streams
used for control and qpack as they can never be closed during the
connection lifetime.
2022-04-07 10:23:10 +02:00
Amaury Denoyelle
06890aaa91 MINOR: mux-quic: adjust timeout to accelerate closing
Improve timeout handling on the MUX. When releasing a stream, first
check if the connection can be considered as dead and should be freed
immediatly. This allows to liberate resources faster when possible.

If the connection is still active, ensure there is no attached
conn-stream before scheduling the timeout. To do this, add a nb_cs field
in the qcc structure.
2022-04-07 10:23:10 +02:00
Amaury Denoyelle
846cc046ae MINOR: mux-quic: factorize conn-stream attach
Provide a new function qc_attach_cs. This must be used by the app layer
when a conn-stream can be instantiated. This will simplify future
development.
2022-04-07 10:10:23 +02:00
Amaury Denoyelle
c9acc31018 BUG/MINOR: fix memleak on quic-conn streams cleaning
When freeing a quic-conn, the streams resources attached to it must be
cleared. This code is already implemented but the streams buffer was not
deallocated.

Fix this by using the function qc_stream_desc_free. This existing
function centralize all operations to properly free all streams
elements, attached both to the MUX and the quic-conn.

This fixes a memory leak which can happen for each released connection.
2022-04-07 10:10:23 +02:00
Amaury Denoyelle
6057b4090e CLEANUP: mux-quic: remove unused QC_CF_CC_RECV
This flag was used to notify the MUX about a CONNECTION_CLOSE frame
reception. It is now unused on the MUX side and can be removed. A new
mechanism to detect quic-conn closing will be soon implemented.
2022-04-07 10:10:23 +02:00
Amaury Denoyelle
e0be573c1b CLEANUP: quic: use static qualifer on quic_close
quic_close can be used through xprt-ops and can thus be kept as a static
symbol.
2022-04-07 10:10:22 +02:00
Amaury Denoyelle
db71e3bd09 BUG/MEDIUM: quic: ensure quic-conn survives to the MUX
Rationalize the lifetime of the quic-conn regarding with the MUX. The
quic-conn must not be freed if the MUX is still allocated.

This simplify the MUX code when accessing the quic-conn and removed
possible segfaults.

To implement this, if the quic-conn timer expired, the quic-conn is
released only if the MUX is not allocated. Else, the quic-conn is
flagged with QUIC_FL_CONN_EXP_TIMER. The MUX is then responsible
to call quic_close() which will free the flagged quic-conn.
2022-04-07 10:10:22 +02:00
Frédéric Lécaille
59bf255806 MINOR: quic: Add closing connection state
New received packets after sending CONNECTION_CLOSE frame trigger a new
CONNECTION_CLOSE frame to be sent. Each time such a frame is sent we
increase the number of packet required to send another CONNECTION_CLOSE
frame.
Rearm only one time the idle timer when sending a CONNECTION_CLOSE frame.
2022-04-06 15:52:35 +02:00
Frédéric Lécaille
47756809fb MINOR: quic: Add draining connection state.
As soon as we receive a CONNECTION_CLOSE frame, we must stop sending packets.
We add QUIC_FL_CONN_DRAINING connection flag to do so.
2022-04-06 15:52:35 +02:00
William Lallemand
eb0d4c40ac BUG/MINOR: httpclient: end callback in applet release
In case an error provokes the release of the applet, we will never call
the end callback of the httpclient.

In the case of a lua script, it would mean that the lua task will never
be waked up after a yield, letting the lua script stuck forever.

Fix the issue by moving the callback from the end of the iohandler to
the applet release function.

Must be backported in 2.5.
2022-04-06 14:19:36 +02:00
William Lallemand
71abad050a MEDIUM: httpclient: enable l7-retry
Enable the layer-7 retry in the httpclient. This way the client will
retry upon a connection error or a timeout.
2022-04-06 11:50:10 +02:00
William Lallemand
ccc7ee45f9 MINOR: httpclient: enable request buffering
The request buffering is required for doing l7 retry. The IO handler of
the httpclient need to be rework for that.

This patch change the IO handler so it copies partially the data instead
of swapping buffer. This is needed because the b_xfer won't never work
if the destination buffer is not empty, which is the case when
buffering.
2022-04-06 11:43:01 +02:00
Remi Tricot-Le Breton
e8041fe8bc BUG/MINOR: ssl/cli: Remove empty lines from CLI output
There were empty lines in the output of "show ssl ca-file <cafile>" and
"show ssl crl-file <crlfile>" commands when an empty line should only
mark the end of the output. This patch adds a space to those lines.

This patch should be backported to 2.5.
2022-04-05 16:53:37 +02:00
William Lallemand
80296b4bd5 BUG/MINOR: ssl: handle X509_get_default_cert_dir() returning NULL
ssl_store_load_locations_file() is using X509_get_default_cert_dir()
when using '@system-ca' as a parameter.

This function could return a NULL if OpenSSL was built with a
X509_CERT_DIR set to NULL, this is uncommon but let's fix this.

No backport needed, 2.6 only.

Fix issue #1637.
2022-04-05 10:19:30 +02:00
Nikola Sale
0dbf03871f MINOR: sample: converter: Add add_item convertor
This new converter is similar to the concat converter and can be used to
build new variables made of a succession of other variables but the main
difference is that it does the checks if adding a delimiter makes sense as
wouldn't be the case if e.g the current input sample is empty. That
situation would require 2 separate rules using concat converter where the
first rule would have to check if the current sample string is empty before
adding a delimiter. This resolves GitHub Issue #1621.
2022-04-04 07:30:58 +02:00
William Lallemand
34107800dd DOC: configuration: add the ca-file changes
Add the documentation about the directory support and @system-ca for the
"ca-file" directive.
2022-04-01 23:52:50 +02:00
William Lallemand
c6b1763dcd MINOR: ssl: ca-file @system-ca loads the system trusted CA
The new parameter "@system-ca" to the ca-file directives loads the
trusted CA in the directory returned by X509_get_default_cert_dir().
2022-04-01 23:52:50 +02:00
William Lallemand
4f6ca32217 BUG/MINOR: ssl: continue upon error when opening a directory w/ ca-file
Previous patch was accidentaly breaking upon an error when itarating
through a CA directory. This is not the expected behavior, the function
must start processing the other files after the warning.
2022-04-01 23:52:50 +02:00
William Lallemand
87fd994727 MEDIUM: ssl: allow loading of a directory with the ca-file directive
This patch implements the ability to load a certificate directory with
the "ca-file" directive.

The X509_STORE_load_locations() API does not allow to cache a directory
in memory at startup, it only references the directory to allow a lookup
of the files when needed. But that is not compatible with the way
HAProxy works, without any access to the filesystem.

The current implementation loads every ".pem", ".crt", ".cer", and
".crl" available in the directory which is what is done when using
c_rehash and X509_STORE_load_locations(). Those files are cached in the
same X509_STORE referenced by the directory name. When looking at "show ssl
ca-file", everything will be shown in the same entry.

This will eventually allow to load more easily the CA of the system,
which could already be done with "ca-file /etc/ssl/certs" in the
configuration.

Loading failure intentionally emit a warning instead of an alert,
letting HAProxy starts when one of the files can't be loaded.

Known limitations:

- There is a bug in "show ssl ca-file", once the buffer is full, the
iohandler is not called again to output the next entries.

- The CLI API is kind of limited with this, since it does not allow to
  add or remove a entry in a particular ca-file. And with a lot of
  CAs you can't push them all in a buffer. It probably needs a "add ssl
  ca-file" like its done with the crt-list.

Fix issue #1476.
2022-04-01 20:36:38 +02:00
Willy Tarreau
fcce0e1e09 OPTIM: hpack: read 32 bits at once when possible.
As suggested in the comment, it's possible to read 32 bits at once in
big-endian order, and now we have the functions to do this, so let's
do it. This reduces the code on the fast path by 31 bytes on x86, and
more importantly performs single-operation 32-bit reads instead of
playing with shifts and additions.
2022-04-01 17:29:06 +02:00
Willy Tarreau
17b4687a89 CLEANUP: hpack: be careful about integer promotion from uint8_t
As reported in issue #1635, there's a subtle sign change when shifting
a uint8_t value to the left because integer promotion first turns any
smaller type to signed int *even if it was unsigned*. A warning was
reported about uint8_t shifted left 24 bits that couldn't fit in int
due to this.

It was verified that the emitted code didn't change, as expected, but
at least this allows to silence the code checkers. There's no need to
backport this.
2022-04-01 17:29:06 +02:00
Frédéric Lécaille
eb2a2da67c BUG/MINOR: quic: Missing TX packet deallocations
Ensure all TX packets are deallocated. There may be remaining ones which
will never be acknowledged or deemed lost.
2022-04-01 16:26:06 +02:00