MEDIUM: mux-h1: Split h1_process_mux() to make code more readable
h1_process_mux() function was pretty huge a quite hard to debug. So, the
funcion is split in sub-functions. Each sub-function is responsible to a
part of the message (start-line, headers, payload, trailers...). We are
still relying on a HTTP parser to format the message to be sure to detect
errors. Functionnaly speaking, there is no change. But the code is now more
readable.
REGTESTS: h1_host_normalization : Add a barrier to not mix up log messages
Depending on the timing, time to time, the log messages can be mixed. A
client can start and be fully handled by HAProxy (including its log message)
before the log message of the previous client was emitted or received. To
fix the issue, a barrier was added to be sure to eval the "expect" rule on
logs before starting the next client.
Tim Duesterhus [Tue, 13 Jun 2023 13:15:47 +0000 (15:15 +0200)]
DOC: Attempt to fix dconv parsing error for tune.h2.fe.initial-window-size
It appears that dconv dislikes the "see also" part being on the same line as
the regular paragraph. The beginning of the line does not show up in the
rendered version.
Attempt to fix this by inserting an additional newline which is consistent with
other options.
MINOR: quic: Remove pool_zalloc() from qc_new_conn()
Missing initialization of largest packet number received during a keyupdate phase.
This prevented the keyupdate feature from working and made the keyupdate interop
tests to fail for all the clients.
Furthermore, ->flags from quic_tls_ctx was also not initialized. This could
also impact the keyupdate feature at least.
BUG/MINOR: quic: ticks comparison without ticks API use
Replace a "less than" comparison between two tick variable by a call to tick_is_lt()
in quic_loss_pktns(). This bug could lead to a wrong packet loss detection
when the loss time computed values could wrap. This is the case 20 seconds after
haproxy has started.
BUG/MEDIUM: mworker: increase maxsock with each new worker
In ticket #2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.
Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.
This patch fixes the issue by increasing the maxsock for each remaining
worker.
BUG/MINOR: quic: Missing transport parameters initializations
This bug was introduced by this commit:
MINOR: quic: Remove pool_zalloc() from qc_new_conn()
The transport parameters was not initialized. This leaded to a crash when
dumping the received ones from TRACE()s.
Also reset the lengths of the CIDs attached to a quic_conn struct to 0 value
to prevent them from being dumped from traces when not already initialized.
MINOR: quic: Remove pool_zalloc() from quic_dgram_parse()
Replace a call to pool_zalloc() by a call to pool_malloc() into quic_dgram_parse
to allocate quic_rx_packet struct objects.
Initialize almost all the members of quic_rx_packet struct.
->saddr is initialized by quic_rx_pkt_retrieve_conn().
->pnl and ->pn are initialized by qc_do_rm_hp().
->dcid and ->scid are initialized by quic_rx_pkt_parse() which calls
quic_packet_read_long_header() for a long packet. For a short packet,
only ->dcid will be initialized.
MINOR: quic: Remove pool_zalloc() from qc_conn_alloc_ssl_ctx()
pool_zalloc() is replaced by pool_alloc() into qc_conn_alloc_ssl_ctx() to allocate
a ssl_sock_ctx struct. ssl_sock_ctx struct member are all initiliazed to null values
excepted ->ssl which is initialized by the next statement: a call to qc_ssl_sess_init().
MINOR: quic: Remove pool_zalloc() from qc_new_conn()
qc_new_conn() is ued to initialize QUIC connections with quic_conn struct objects.
This function calls quic_conn_release() when it fails to initialize a connection.
quic_conn_release() is also called to release the memory allocated by a QUIC
connection.
Replace pool_zalloc() by pool_alloc() in this function and initialize
all quic_conn struct members which are referenced by quic_conn_release() to
prevent use of non initialized variables in this fonction.
The ebtrees, the lists attached to quic_conn struct must be initialized.
The tasks must be reset to their NULL default values to be safely destroyed
by task_destroy(). This is all the case for all the TLS cipher contexts
of the encryption levels (struct quic_enc_level) and those for the keyupdate.
The packet number spaces (struct quic_pktns) must also be initialized.
->prx_counters pointer must be initialized to prevent quic_conn_prx_cntrs_update()
from dereferencing this pointer.
->latest_rtt member of quic_loss struct must also be initialized. This is done
by quic_loss_init() called by quic_path_init().
BUG/MINOR: quic: Possible endless loop in quic_lstnr_dghdlr()
This may happen when the initilization of a new QUIC conn fails with qc_new_conn()
when receiving an Initial paquet. This is done after having allocated a CID with
new_quic_cid() called by quic_rx_pkt_retrieve_conn() which stays in the listener
connections tree without a QUIC connection attached to. Then when the listener
receives another Initial packet for the same CID, quic_rx_pkt_retrieve_conn()
returns NULL again (no QUIC connection) but with an thread ID already bound to the
connection, leading the datagram to be requeued in the same datagram handler thread
queue. And so on.
To fix this, the connection is created after having created the connection ID.
If this fails, the connection is deallocated.
During the race condition, when two different threads handle two datagrams for
the same connection, in addition to releasing the newer created connection ID,
the newer QUIC connection must also be released.
BUG/MINOR: quic: Possible crash in quic_conn_prx_cntrs_update()
quic_conn_prx_cntrs_update() may be called from quic_conn_release() with
NULL as value for ->prx_counters member. This is the case when qc_new_conn() fails
when allocating <buf_area>. In this case quic_conn_prx_cntrs_update() BUG_ON().
BUG/MINOR: quic: Missing initialization (packet number space probing)
->tx.pto_probe member of quic_pktns struct was not initialized by quic_pktns_init().
This bug never occured because all quic_pktns structs are attached to quic_conn
structs which are always pool_zalloc()'ed.
BUG/MINOR: namespace: missing free in netns_sig_stop()
On soft-stop, netns_sig_stop() function is called to purge the shared
namespace tree by iterating over each entries to free them.
However, once an entry is cleaned up and removed from the tree, the entry
itself isn't freed and this results into a minor leak when soft-stopping
because entry was allocated using calloc() in netns_store_insert() when
parsing the configuration.
This could be backported in every stable versions.
BUG/MINOR: server: inherit from netns in srv_settings_cpy()
When support for 'namespace' keyword was added for the 'default-server'
directive in 22f41a2 ("MINOR: server: Make 'default-server' support
'namespace' keyword."), we forgot to copy the attribute from the parent
to the newly created server.
This resulted in the 'namespace' keyword being parsed without errors when
used from a 'default-server' directive, but in practise the option was
simply ignored.
There's no need to duplicate the netns struct because it is stored in
a shared list, so copying the pointer does the job.
This patch partially fixes GH #2038 and should be backported to all
stable versions.
BUG/MINOR: quic: Address inversion in "show quic full"
The local address was dumped as "from" address by dump_quic_full() and
the peer address as "to" address. This patch fixes this issue.
Furthermore, to support the server side (QUIC client) to come, it is preferable
to stop using "from" and "to" labels to dump the local and peer addresses which
is confusing for a QUIC client which uses its local address as "from" address.
To mimic netstat, this is "Local Address" and "Foreign Address" which will
be displayed by "show quic" CLI command and "local_addr" and "foreign_addr"
for "show quic full" command to mention the local addresses and the peer
addresses.
This bug arrived with this commit which was supposed to fix another one:
BUG/MINOR: quic: Wrong Application encryption level selection when probing
The aim of this patch was to prevent the Application encryption to be selected
when probing leading to ACK only packets to be sent if the ack delay timer
had fired in the meantime, leading to crashes when no 01-RTT had been sent
because the ack range tree is empty in this case.
This statement is not correct (qc->pktns->flags & QUIC_FL_PKTNS_PROBE_NEEDED)
because qc->pktns is an array of packet number space. But it is equivalent
to (qc->pktns[QUIC_TLS_PKTNS_INITIAL].flags & QUIC_FL_PKTNS_PROBE_NEEDED).
That said, the patch mentionned above is not more useful since this following
which disable the ack time during the handshakes:
BUG/MINOR: quic: Do not use ack delay during the handshakes
This commit revert the first patch mentionned above.
BUG/MEDIUM: hlua: Use front SC to detect EOI in HTTP applets' receive functions
When an HTTP applet tries to get request data, we must take care to properly
detect the end of the message. It an empty HTX message with the SC_FL_EOI
flag set on the front SC. However, an issue was introduced during the SC
refactoring performed in the 2.8. The backend SC is tested instead of the
frontend one.
Because of this bug, the receive functions hang because the test on
SC_FL_EOI flag never succeeds. Of course, by checking the frontend SC (the
opposite SC to the one attached to the appctx), it works.
This patch should fix the issue #2180. It must be backported to the 2.8.
BUG/MINOR: proxy/server: free default-server on deinit
proxy default-server is a specific type of server that is not allocated
using new_server(): it is directly stored within the parent proxy
structure. However, since it may contain some default config options that
may be inherited by regular servers, it is also subject to dynamic members
(strings, structures..) that needs to be deallocated when the parent proxy
is cleaned up.
Unfortunately, srv_drop() may not be used directly from p->defsrv since
this function is meant to be used on regular servers only (those created
using new_server()).
To circumvent this, we're splitting srv_drop() to make a new function
called srv_free_params() that takes care of the member cleaning which
originally takes place in srv_drop(). This function is exposed through
server.h, so it may be called from outside server.c.
Thanks to this, calling srv_free_params(&p->defsrv) from free_proxy()
prevents any memory leaks due to dynamic parameters allocated when
parsing a default-server line from a proxy section.
This partially fixes GH #2173 and may be backported to 2.8.
[While it could also be relevant for other stable versions, the patch
won't apply due to architectural changes / name changes between 2.4 => 2.6
and then 2.6 => 2.8. Considering this is a minor fix that only makes
memory analyzers happy during deinit paths (at least for <= 2.8), it might
not be worth the trouble to backport them any further?]
BUG/MINOR: proxy: add missing interface bind free in free_proxy
bind->settings.interface hint is allocated when "interface" keyword
is specified on a bind line, but the string isn't explicitly freed in
proxy_free, resulting in minor memory leak on deinit paths when the
keyword is being used.
It partially fixes GH #2173 and may be backported to all stable versions.
[in 2.2 free_proxy did not exist so the patch must be applied directly
in deinit() function from haproxy.c]
BUG/MINOR: cfgparse-tcp: leak when re-declaring interface from bind line
When interface keyword is used multiple times within the same bind line,
the previous value isn't checked and is rewritten as-is, resulting in a
small memory leak.
Ensuring the interface name is first freed before assigning it to a new
value.
This may be backported to every stable versions.
[Note for 2.2, the fix must be performed in bind_parse_interface() from
proto_tcp.c, directly within the listener's loop, also ha_free() was
not available so free() must be used instead]
To prevent bogus matches, var() does not default to string type anymore
since 44c5ff6 ("MEDIUM: vars: make the var() sample fetch function really
return type ANY).
Thanks to the above fix, haproxy now returns an error if var() is used
within an ACL or IF condition and the matching type is not explicitly
set.
However, the documentation was not updated to reflect this change.
This partially fixes GH #2087 and must be backported up to 2.6.
Artur Pydo [Tue, 6 Jun 2023 09:49:59 +0000 (11:49 +0200)]
DOC: quic: fix misspelled tune.quic.socket-owner
Commit 511ddd5 introduced tune.quic.socket-owner parameter related to
QUIC socket behaviour. However it was misspelled in configuration.txt in
'bind' section as tune.quic.conn-owner.
REG-TESTS: stickiness: Delay haproxys start to properly resolv variables
Because of the commit 5cb8d7b8f ("BUG/MINOR: peers: Improve detection of
config errors in peers sections"), 2 scripts now report errors during
startup because some variables are not set and the remote peer server is
thus malformed. To perform a peer synchro between 2 haproxys in these
scripts, the startup must be delayed to properly resolve addresses.
In addidiotn, we must wait (2s) to be sure the connection between peers is
properly established. These scripts are now flagged as slow.
BUG/MINOR: peers: Improve detection of config errors in peers sections
There are several misuses in peers sections that are not detected during the
configuration parsing and that could lead to undefined behaviors or crashes.
First, only one listener is expected for a peers section. If several bind
lines or local peer definitions are used, an error is triggered. However, if
multiple addresses are set on the same bind line, there is no error while
only the last listener is properly configured. On the 2.8, there is no crash
but side effects are hardly predictable. On older version, HAProxy crashes
if an unconfigured listener is used.
Then, there is no check on remote peers name. It is unexpected to have same
name for several remote peers. There is now a test, performed during the
post-parsing, to verify all remote peer names are unique.
Finally, server parsing options for the peers sections are changed to be
sure a port is always defined, and not a port range or a port offset.
This patch fixes the issue #2066. It could be backported to all stable
versions.
BUG/MINOR: spoe: Only skip sending new frame after a receive attempt
When a SPOE appctx is processing frames in sync mode, we must only skip
sending a new frame if it is still waiting for a ACK frame after a receive
attempt. It was performed before the receive attempt. As a consequence, if
the ACK frame was received, the SPOE appctx did not try to process queued
messages immediately. This could increase the queue time and thus slow down
the processing time of the stream.
Thanks to Daniel Epperson for his help to diagnose the bug.
This patch must be backported to every stable versions.
Willy Tarreau [Fri, 2 Jun 2023 14:19:51 +0000 (16:19 +0200)]
BUG/MINOR: stream: do not use client-fin/server-fin with HTX
Historically the client-fin and server-fin timeouts were made to allow
a connection closure to be effective quickly if the last data were sent
down a socket and the client didn't close, something that can happen
when the peer's FIN is lost and retransmits are blocked by a firewall
for example. This made complete sense in 1.5 for TCP and HTTP in close
mode. But nowadays with muxes, it's not done at the right layer anymore
and even the description doesn't match what is being done, because what
happens is that the stream will abort the whole transfer after it's done
sending to the mux and this timeout expires.
We've seen in GH issue 2095 that this can happen with very short timeout
values, and while this didn't trigger often before, now that the muxes
(h2 & quic) properly report an end of stream before even the first
sc_conn_sync_recv(), it seems that it can happen more often, and have
two undesirable effects:
- logging a timeout when that's not the case
- aborting the request channel, hence the server-side conn, possibly
before it had a chance to be put back to the idle list, causing
this connection to be closed and not reusable.
Unfortunately for TCP (mux_pt) this remains necessary because the mux
doesn't have a timeout task. So here we're adding tests to only do
this through an HTX mux. But to be really clean we should in fact
completely drop all of this and implement these timeouts in the mux
itself.
This needs to be backported to 2.8 where the issue was discovered,
and maybe carefully to older versions, though that is not sure at
all. In any case, using a higher timeout or removing client-fin in
HTTP proxies is sufficient to make the issue disappear.
Willy Tarreau [Fri, 2 Jun 2023 06:35:50 +0000 (08:35 +0200)]
MINOR: stats: protect against future stats fields omissions
As seen in commits 33a4461fa ("BUG/MINOR: stats: Fix Lua's `get_stats`
function") and a46b142e8 ("BUG/MINOR: Missing stat_field_names (since f21d17bb)") it seems frequent to omit to update stats_fields[] when
adding a new ST_F_xxx entry. This breaks Lua's get_stats() and shows
a "(null)" in the header of "show stat", but that one is not detectable
to the naked eye anymore.
Let's add a reminder above the enum declaration about this, and a small
reg tests checking for the absence of "(null)". It was verified to fail
before the last patch above.
Tim Duesterhus [Thu, 1 Jun 2023 16:58:08 +0000 (18:58 +0200)]
BUG/MINOR: stats: Fix Lua's `get_stats` function
Lua's `get_stats` function stopped working in 4cfb0019e65bce79953164eddf54c1bbb61add62, due to the addition a new field
ST_F_PROTO without a corresponding entry in `stat_fields`.
Willy Tarreau [Wed, 31 May 2023 14:24:38 +0000 (16:24 +0200)]
[RELEASE] Released version 2.8.0
Released version 2.8.0 with the following main changes :
- MINOR: compression: Improve the way Vary header is added
- BUILD: makefile: search for SSL_INC/wolfssl before SSL_INC
- MINOR: init: pre-allocate kernel data structures on init
- DOC: install: add details about WolfSSL
- BUG/MINOR: ssl_sock: add check for ha_meth
- BUG/MINOR: thread: add a check for pthread_create
- BUILD: init: print rlim_cur as regular integer
- DOC: install: specify the minimum openssl version recommended
- CLEANUP: mux-quic: remove unneeded fields in qcc
- MINOR: mux-quic: remove nb_streams from qcc
- MINOR: quic: fix stats naming for flow control BLOCKED frames
- BUG/MEDIUM: mux-quic: only set EOI on FIN
- BUG/MEDIUM: threads: fix a tiny race in thread_isolate()
- DOC: config: fix rfc7239 converter examples
- DOC: quic: remove experimental status for QUIC
- CLEANUP: mux-quic: rename functions for mux_ops
- CLEANUP: mux-quic: rename internal functions
- BUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty
- DOC: config: Fix bind/server/peer documentation in the peers section
- BUILD: Makefile: use -pthread not -lpthread when threads are enabled
- CLEANUP: doc: remove 21 totally obsolete docs
- DOC: install: mention the common strict-aliasing warning on older compilers
- DOC: install: clarify a few points on the wolfSSL build method
- MINOR: quic: Add QUIC connection statistical counters values to "show quic"
- EXAMPLES: update the basic-config-edge file for 2.8
- MINOR: quic/cli: clarify the "show quic" help message
- MINOR: version: mention that it's LTS now.
Willy Tarreau [Wed, 31 May 2023 13:35:29 +0000 (15:35 +0200)]
DOC: install: clarify a few points on the wolfSSL build method
Let's make clear which commands goes into the wolfSSL directory and
which one in the haproxy directory. Also, let's add a paragraph in the
QUIC section explaining how to proceed with wolfSSL.
Willy Tarreau [Wed, 31 May 2023 13:27:01 +0000 (15:27 +0200)]
DOC: install: mention the common strict-aliasing warning on older compilers
In the errors and warnings section about common issues, it's useful to
mention the strict-aliasing warning that was happening with gcc-4.4 that
may still be found on old systems, especially since it will probably take
ages to build there and the warning is harmless.
Willy Tarreau [Wed, 31 May 2023 13:13:29 +0000 (15:13 +0200)]
CLEANUP: doc: remove 21 totally obsolete docs
These were docs for very old design thoughts or internal subsystems
which are now totally irrelevant and even misleading. Those with some
outdated ideas mixed with useful stuff were kept though.
Willy Tarreau [Wed, 31 May 2023 10:03:33 +0000 (12:03 +0200)]
BUILD: Makefile: use -pthread not -lpthread when threads are enabled
-pthread is normally the right way to enable threads, it involves -lpthread
at the end of the arguments, and also enables -D_REENTRANT=1. We normally
don't care about the subtle difference, but building with a static openssl
library that has threads enabled breaks because -lpthread is placed before
the SSL_LDFLAGS and openssl doesn't find pthread_atfork().
Let's change the flag to -pthread once for all, that's something we've
considered over the last decade without having a good reason to do it
since it didn't bring any value. Now at least it fixes a build issues,
this is a good reason. This doesn't need to be backported since it is
one of the consequences of the new more flexible build options in 2.8.
DOC: config: Fix bind/server/peer documentation in the peers section
Documentation about bind and server directives in the peers section was
retrieved from the proxy part but there are some limitations, especially for
the bind directive. And the same is true for the peer directive. It is
forbidden to have several listening addresses. Multiple addresses or port
range are not allowed.
Here, only the documentation is fixed. The configuration parsing will be
improved later to trigger errors on bad uses.
In addition, it is also specified that unix socket are supported.
This patch partially fixes the issue #2066. It should be backported to all
stable versions.
Willy Tarreau [Tue, 30 May 2023 13:42:35 +0000 (15:42 +0200)]
BUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty
There's a rare case where on long fat pipes, we can see the keep-alive
timeout trigger before the end of the transfer of the last large object,
and the connection closed a bit quickly after the end of the transfer
because a GOAWAY is queued. The data are not destroyed, except that
the WINDOW_UPDATES from the client arriving late while the last data
are being drained by the socket buffers may at some point trigger a
reset, and some clients might choke a bit too early on these. Let's
make sure we only arm the idle_start timestamp once the output buffer
is empty. Of course it will still not cover for the data pending in the
socket buffers but it will at least let those in the buffer leave in
peace. More elaborate options can be used to protect the data in the
kernel buffers, such as the one described in GH issue #5.
It's very likely that this old issue was emphasized by the following
commit in 2.6: 15a4733d5 ("BUG/MEDIUM: mux-h2: make use of http-request and keep-alive timeouts")
and the behavior probably changed again with this one in 2.8, which
was backported to 2.7 and scheduled for 2.6: d38d8c6cc ("BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout")
As such this patch should be backported to 2.6 after some observation
period.
Amaury Denoyelle [Tue, 30 May 2023 13:04:46 +0000 (15:04 +0200)]
CLEANUP: mux-quic: rename internal functions
This patch is similar to the previous one but for QUIC mux functions
used inside the mux code itself or application layer. Replace all
occurences of qc_* prefix by qcc_* or qcs_*. This should help to better
differentiate code between quic_conn and MUX.
Amaury Denoyelle [Tue, 30 May 2023 12:51:57 +0000 (14:51 +0200)]
CLEANUP: mux-quic: rename functions for mux_ops
Rename all QUIC mux function exposed through mux_ops structure. Use the
prefix qmux_* or qmux_strm_*. The objective is to remove qc_* prefix
which should only be used in quic_conn layer.
Willy Tarreau [Sat, 27 May 2023 11:45:01 +0000 (13:45 +0200)]
BUG/MEDIUM: threads: fix a tiny race in thread_isolate()
Aurélien found a tiny race in thread_isolate() that can allow a thread
that was running under isolation to continue running while another one
enters isolation. The reason is that the check for harmless is only
done before winning the CAS, but since the previously isolated thread
doesn't wait for !rdv_request in thread_release(), it can effectively
continue its activities while the next one believes it's isolated. A
proper solution consists in looping once again in thread_isolate() to
recheck (and wait) for all threads to be isolated once the CAS is won.
The issue was introduced in 2.7 by commit 598cf3f22 ("MAJOR: threads:
change thread_isolate to support inter-group synchronization") so the
fix needs to be backported there.
Amaury Denoyelle [Thu, 25 May 2023 13:02:24 +0000 (15:02 +0200)]
BUG/MEDIUM: mux-quic: only set EOI on FIN
Recently stconn flags were reviewed for QUIC mux to be conform with
other HTTP muxes. However, a mistake was made when dealing with a proper
stream FIN with both EOI and EOS set. This was done as RESET_STREAM
received after a FIN are ignored by QUIC mux and thus there is no
difference between EOI or EOI+EOS. However, analyzers may interpret EOS
as an interrupted request which result in a 400 HTTP error code.
To fix this, only set EOI on proper stream FIN. EOS is set when input is
interrupted (RESET_STREAM before FIN) or a STOP_SENDING is received
which prevent transfer to complete. In this last case, EOS must be
manually set too if FIN has been received before STOP_SENDING to go
directly from ERR_PENDING to final ERROR state.
Amaury Denoyelle [Thu, 25 May 2023 08:16:19 +0000 (10:16 +0200)]
MINOR: mux-quic: remove nb_streams from qcc
Remove nb_streams field from qcc. It was not used outside of a BUG_ON()
statement to ensure we never have a negative count of streams. However
this is already checked with other fields.
haproxy does not compile anymore on macOS+clang since 425d7ad ("MINOR:
init: pre-allocate kernel data structures on init"). This is due to
rlim_cur being printed uncasted using %lu format specifier, with rlim_cur
being stored as a rlim_t which is a typedef so its size may vary depending
on the system's architecture.
This is not the first time we need to dump rlim_cur in case of errors,
there are already multiple occurences in the init code. Everywhere this
happens, rlim is casted as a regular int and printed using the '%d'
format specifier, so we do the same here as well to fix the build issue.
No backport needed unless 425d7ad gets backported.
eaglegai [Fri, 26 May 2023 08:44:34 +0000 (16:44 +0800)]
BUG/MINOR: thread: add a check for pthread_create
preload_libgcc_s() use pthread_create to create a thread and then call
pthread_join to use it, but it doesn't check if the option is successful.
So add a check to aviod potential crash.
eaglegai [Fri, 26 May 2023 08:42:47 +0000 (16:42 +0800)]
BUG/MINOR: ssl_sock: add check for ha_meth
in __ssl_sock_init, BIO_meth_new may failed and return NULL if
OPENSSL_zalloc failed. in this case, ha_meth will be NULL, and then
crash happens in BIO_meth_set_write. So, we add a check for ha_meth.
Patrick Hemmer [Tue, 23 May 2023 17:02:08 +0000 (13:02 -0400)]
MINOR: init: pre-allocate kernel data structures on init
The Linux kernel maintains data structures to track a processes' open file
descriptors, and it expands these structures as necessary when FD usage grows
(at every FD=2^X starting at 64). However when threading is in use, during
expansion the kernel will pause (observed up to 47ms) while it waits for thread
synchronization (see https://bugzilla.kernel.org/show_bug.cgi?id=217366).
This change addresses the issue and avoids the random pauses by opening the
maximum file descriptor during initialization, so that expansion will not occur
while processing traffic.
Willy Tarreau [Thu, 25 May 2023 18:17:27 +0000 (20:17 +0200)]
BUILD: makefile: search for SSL_INC/wolfssl before SSL_INC
Building with an install of wolfssl and openssl side-by-side breaks
because for wolfssl we need the two include levels and since some
names are in common, this results in some files being found in the
original openssl tree. Let's swap the two include paths so that all
that is related to wolfssl is found there first when needed.
MINOR: compression: Improve the way Vary header is added
When a message is compressed, A "Vary" header is added with
"accept-encoding" value. However, a new header is always added, regardless
there is already a Vary header or not. In addition, if there is already a
Vary header, there is no check on values to be sure "accept-encoding" value
is not already there. So it is possible to have it twice.
To improve this part, we now test Vary header values and "accept-encoding"
is only added if it was not found. In addition, "accept-encoding" value is
appended to the last Vary header found, if any. Otherwise, a new header is
added.
Willy Tarreau [Wed, 24 May 2023 20:53:55 +0000 (22:53 +0200)]
[RELEASE] Released version 2.8-dev13
Released version 2.8-dev13 with the following main changes :
- DOC: add size format section to manual
- CLEANUP: mux-quic/h3: complete BUG_ON with comments
- MINOR: quic: remove return val of quic_aead_iv_build()
- MINOR: quic: use WARN_ON for encrypt failures
- BUG/MINOR: quic: handle Tx packet allocation failure properly
- MINOR: quic: fix alignment of oneline show quic
- MEDIUM: stconn/applet: Allow SF_SL_EOS flag alone
- MEDIUM: stconn: make the SE_FL_ERR_PENDING to ERROR transition systematic
- DOC: internal: add a bit of documentation for the stconn closing conditions
- DOC/MINOR: config: Fix typo in description for `ssl_bc` in configuration.txt
- BUILD: quic: re-enable chacha20_poly1305 for libressl
- MINOR: mux-quic: set both EOI EOS for stream fin
- MINOR: mux-quic: only set EOS on RESET_STREAM recv
- MINOR: mux-quic: report error on stream-endpoint earlier
- BUILD: makefile: fix build issue on GNU make < 3.82
- BUG/MINOR: mux-h2: Check H2_SF_BODY_TUNNEL on H2S flags and not demux frame ones
- MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame
- MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
- BUG/MEDIUM: mux-h2: Propagate termination flags when frontend SC is created
- DEV: add a Lua helper script for SSL keys logging
- CLEANUP: makefile: don't display a dummy features list without a target
- BUILD: makefile: do not erase build options for some build options
- MINOR: quic: Add low level traces (addresses, DCID)
- BUG/MINOR: quic: Wrong token length check (quic_generate_retry_token())
- BUG/MINOR: quic: Missing Retry token length on receipt
- MINOR: quic: Align "show quic" command help information
- CLEANUP: quic: Indentation fix quic_rx_pkt_retrieve_conn()
- CLEANUP: quic: Useless tests in qc_rx_pkt_handle()
- MINOR: quic: Add some counters at QUIC connection level
- MINOR: quic: Add a counter for sent packets
- MINOR: hlua: hlua_smp2lua_str() may LJMP
- MINOR: hlua: hlua_smp2lua() may LJMP
- MINOR: hlua: hlua_arg2lua() may LJMP
- DOC: hlua: document hlua_lua2arg() function
- DOC: hlua: document hlua_lua2smp() function
- BUG/MINOR: hlua: unsafe hlua_lua2smp() usage
- BUILD: makefile: commit the tiny FreeBSD makefile stub
- BUILD: makefile: fix build options when building tools first
- BUILD: ist: do not put a cast in an array declaration
- BUILD: ist: use the literal declaration for ist_lc/ist_uc under TCC
- BUILD: compiler: systematically set USE_OBSOLETE_LINKER with TCC
- DOC: install: update reference to known supported versions
- SCRIPTS: publish-release: update the umask to keep group write access
Willy Tarreau [Wed, 24 May 2023 20:32:46 +0000 (22:32 +0200)]
DOC: install: update reference to known supported versions
Gcc 13 is known to work, OpenSSL 3.1 and wolfSSL as well. Add a few
hints about build errors when using QUIC + OpenSSL and warnings about
the dramatic OpenSSL 3.x performance regression.
Willy Tarreau [Wed, 24 May 2023 19:37:06 +0000 (21:37 +0200)]
BUILD: compiler: systematically set USE_OBSOLETE_LINKER with TCC
TCC silently ignores the weak and section attributes, which ruins the
initcalls. Technically we're exactly in the same situation as with an
obsolete linker. Let's just automatically set the flag if TCC is
detected, this avoids surprises where the program compiles but does
not start.
Willy Tarreau [Wed, 24 May 2023 19:31:21 +0000 (21:31 +0200)]
BUILD: ist: use the literal declaration for ist_lc/ist_uc under TCC
TCC doesn't knoow about __attribute__((weak)), it silently ignores it.
We could add a "static" modifier there in this case but we already have
an alternate portable mode that is based on a slightly larger literal
for obsolete linkers (and non-ELF systems) which choke on weak. Let's
just add the test for tcc there and use it in this case.
It was written like this because it's expanded from the _IST_LC macro
but it's never used as-is, it's only used from ist_lc, which should be
the one containing the cast so that the macro only contains the list of
bytes that can be used in both places. And this assigns more consistent
roles to the lower and upper case macro/variable now, one is typed and
the other one not. No backport is needed.
Willy Tarreau [Wed, 24 May 2023 15:23:45 +0000 (17:23 +0200)]
BUILD: makefile: fix build options when building tools first
Due to the test on the target introduced by commit 9577a152b ("BUILD:
makefile: do not erase build options for some build options"), if a
tool (e.g. halog) is build first before haproxy after a clean or a
fresh source extraction, the .build_opts file does not exist and
"make" complains since there's no such target. Make sure to define
the empty target for all "else" blocks there. No backport is needed.
Willy Tarreau [Wed, 24 May 2023 15:06:30 +0000 (17:06 +0200)]
BUILD: makefile: commit the tiny FreeBSD makefile stub
The idea here is to try to detect the use of "make" instead of "gmake"
on FreeBSD. After having long tried, there's no way to construct a
condition that is common to both makefile languages and could serve as
a differentiator since there's simply no common word between the two
languages. However on FreeBSD (the main used BSD platform), "make" is
configured to look for BSDmakefile before the other ones. It allows us
to intercept it and explain to use gmake with an example of a roughly
converted make command line (we just strip "-J xx,xx" that systematically
gets inserted if "-j" is used). A few tricks are used, such as creating
a dummy target on the fly based on the requested one just to silence the
output, and always match "all" since it's used by default when no target
is specified. .DEFAULTS was initially used but finally dropped thanks to
this.
For example:
$ make -j$(getconf NPROCESSORS_ONLN) TARGET=freebsd USE_OPENSSL=1
Please use GNU make instead. It is often called gmake.
Example:
gmake -j 4 TARGET=freebsd USE_OPENSSL=1 all
It will often be sufficient to permit a copy-paste and to try again.
Note that the .gitignore was updated.
Fixing hlua_lua2smp() usage in hlua's code since it was assumed that
hlua_lua2smp() makes a standalone smp out of lua data, but it is not
the case.
This is especially true when dealing with lua strings (string is
extracted using lua_tolstring() which returns a pointer to lua string
memory location that may be reclaimed by lua at any time when no longer
used from lua's point of view). Thus, smp generated by hlua_lua2smp() may
only be used from the lua context where the call was initially made, else
it should be explicitly duplicated before exporting it out of lua's
context to ensure safe (standalone) usage.
Add ->sent_pkt counter to quic_conn struct to count the packet at QUIC connection
level. Then, when the connection is released, the ->sent_pkt counter value
is added to the one for the listener.
MINOR: quic: Add some counters at QUIC connection level
Add some statistical counters to quic_conn struct from quic_counters struct which
are used at listener level to handle them at QUIC connection level. This avoid
calling atomic functions. Furthermore this will be useful soon when a counter will
be added for the total number of packets which have been sent which will be very
often incremented.
Some counters were not added, espcially those which count the number of QUIC errors
by QUIC error types. Indeed such counters would be incremented most of the time
only one time at QUIC connection level.
Implement quic_conn_prx_cntrs_update() which accumulates the QUIC connection level
statistical counters to the listener level statistical counters.
CLEANUP: quic: Useless tests in qc_rx_pkt_handle()
There is no reason to test <qc> nullity at the end of this function because it is
clearly not null, furthermore the trace handle the case where <qc> is null.
MINOR: quic: Align "show quic" command help information
Align the "show quic" help information with all the others command help information.
Furthermore, makes this information match the management documentation.
BUG/MINOR: quic: Missing Retry token length on receipt
quic_retry_token_check() must decipher the token sent to and received back from
clients. This token is made of the token format byte, the ODCID prefixed by its one byte
length, the timestamp of its creation, and terminated by an AEAD TAG followed
by the salt used to derive the secret to cipher the token.
So, the length of these data must be between
2 + QUIC_ODCID_MINLEN + sizeof(uint32_t) + QUIC_TLS_TAG_LEN + QUIC_RETRY_TOKEN_SALTLEN
and
2 + QUIC_CID_MAXLEN + sizeof(uint32_t) + QUIC_TLS_TAG_LEN + QUIC_RETRY_TOKEN_SALTLEN.
This bug would never occur because the buffer supplied to quic_generate_retry_token()
to build a Retry token is large enough to embed such a token. Anyway, this patch
fixes quic_generate_retry_token() implementation.
There were two errors: this is the ODCID which is added to the token. Furthermore
the timestamp was not taken into an account.
Willy Tarreau [Wed, 24 May 2023 14:18:39 +0000 (16:18 +0200)]
BUILD: makefile: do not erase build options for some build options
One painfully annoying thing with the build options change detection
is that they get rebuild for about everything except when the build
target is exactly "reg-tests". But in practice every time reg tests
are run we end up having to experience a full rebuild because the
reg-tests script runs "make version" which is sufficient to refresh
the file.
There are two issues here. The first one is that we ought to skip all
targets that do not make use of the build options. This includes all
the tools such as "flags" for example, or utility targets like "tags",
"help" or "version". The second issue is that with most of these extra
targets we do not set the TARGET variable, and that one is used when
creating the build_opts file, so let's preserve the file when TARGET
is not set.
Now it's possible to re-run a make after a make reg-tests without having
to rebuild the whole project.
Willy Tarreau [Wed, 24 May 2023 13:59:04 +0000 (15:59 +0200)]
CLEANUP: makefile: don't display a dummy features list without a target
"make help" ends with a list of enabled/disabled features for TARGET '',
which makes no sense. Let's only display enabled/disabled features when
a target is set. It also removes visual pollution when users seek help.
Amaury Denoyelle [Wed, 24 May 2023 14:02:17 +0000 (16:02 +0200)]
DEV: add a Lua helper script for SSL keys logging
This script can be used through a http-request rules to log SSL keys for
traffic on a dedicated frontend. The resulting file can then be injected
into wireshark to decipher the corresponding network capture.
BUG/MEDIUM: mux-h2: Propagate termination flags when frontend SC is created
We must evaluate if EOS/EOI/ERR_PENDING/ERROR flags must be set on the SE
when the frontend SC is created because the rxbuf is transferred to the
steeam at this stage. It means the call to h2_rcv_buf() may be skipped on
some circumstances.
And indeed, it happens when HAproxy quickly replies, for instance because of
a deny rule. In this case, depending on the scheduling, the abort may block
the receive attempt from the SC. In this case if SE flags were not properly
set earlier, there is no way to terminate the request and the session may be
freezed.
For now, I can't explain why there is no timeout when this happens but it
remains an issue because here we should not rely on timeouts to close the
stream.
This patch relies on following commits:
* MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
* MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame
The issue was encountered on the 2.8 but it seems the bug exists since the
2.4. But it is probably a good idea to only backport the series to 2.7 only
and wait for a bug report on earlier versions.
MINOR: mux-h2: Add a function to propagate termination flags from h2s to SE
The function h2s_propagate_term_flags() was added to check the H2S state and
evaluate when EOI/EOS/ERR_PENDING/ERROR flags must be set on the SE. It is
not the only place where those flags are set. But it centralizes the synchro
between the H2 stream and the SC.
For now, this function is only used at the end of h2_rcv_buf(). But it will
be used to fix a bug.
MINOR: mux-h2: Set H2_SF_ES_RCVD flag when decoding the HEADERS frame
The flag H2_SF_ES_RCVD is set on the H2 stream when the ES flag is found in
a frame. On HEADERS frame, it was set in function processing the frame. It
is moved in the function decoding the frame. Fundamentally, this changes
nothing. But it will be useful to have this information earlier when a
client H2 stream is created.
BUG/MINOR: mux-h2: Check H2_SF_BODY_TUNNEL on H2S flags and not demux frame ones
In h2c_frt_stream_new(), H2_SF_BODY_TUNNEL flags was tested on demux frame
flags (h2c->dff) instead of the h2s flags. By chance, it is a noop test
becasue H2_SF_BODY_TUNNEL value, once converted to an int8_t, is 0.
Willy Tarreau [Wed, 24 May 2023 13:23:34 +0000 (15:23 +0200)]
BUILD: makefile: fix build issue on GNU make < 3.82
Thierry Fournier reported a build breakage with the ubiquitous make
3.81, LDFLAGS were ignored. This is caused by the declaration of the
collect_opt_flags macro that is defined with an "=" sign, something
that only appeared in 3.82 and that is not necessary. With it removed,
the build now works fine at least from 3.80 to 4.3.
No backport is needed since this makefile cleanup appeared in 2.8.
Amaury Denoyelle [Wed, 24 May 2023 12:43:43 +0000 (14:43 +0200)]
MINOR: mux-quic: report error on stream-endpoint earlier
A RESET_STREAM is emitted in several occasions :
- protocol error during HTTP/3.0 parsing
- STOP_SENDING reception
In both cases, if a stream-endpoint is attached we must set its ERR
flag. This was correctly done but after some delay as it was only when
the RESET_STREAM was emitted. Change this to set the ERR flag as soon as
one of the upper cases has been encountered. This should help to release
faster streams in error.
Amaury Denoyelle [Wed, 24 May 2023 08:49:44 +0000 (10:49 +0200)]
MINOR: mux-quic: only set EOS on RESET_STREAM recv
A recent review was done to rationalize ERR/EOS/EOI flags on stream
endpoint. A common definition for both H1/H2/QUIC mux have been written
in the following documentation :
./doc/internals/stconn-close.txt
In QUIC it is possible to close each channels of a stream independently
with RESET_STREAM and STOP_SENDING frames. When a RESET_STREAM is
received, it indicates that the peer has ended its transmission in an
abnormal way. However, it is still ready to receive.
Previously, on RESET_STREAM reception, QUIC MUX set the ERR flag on
stream-endpoint. However, according to the QUIC mechanism, it should be
instead EOS but this was impossible due to a BUG_ON() which prevents EOS
without EOI or ERR. This BUG_ON was only present because this case was
never used before the introduction of QUIC. It was removed in a recent
commit which allows us to now properly set EOS alone on RESET_STREAM
reception.
In practice, this change allows to continue to send data even after
RESET_STREAM reception. However, currently browsers always emit it with
a STOP_SENDING as this is used to abort the whole H3 streams. In the end
this will result in a stream-endpoint with EOS and ERR_PENDING/ERR
flags.
Amaury Denoyelle [Wed, 24 May 2023 08:48:52 +0000 (10:48 +0200)]
MINOR: mux-quic: set both EOI EOS for stream fin
A recent review was done to rationalize ERR/EOS/EOI flags on stream
endpoint. A common definition for both H1/H2/QUIC mux have been written
in the following documentation :
./doc/internals/stconn-close.txt
Always set EOS with EOI flag to conform to this specification. EOI is
set whenever the proper stream end has been encountered : with QUIC it
corresponds to a STREAM frame with FIN bit. At this step, RESET_STREAM
frames are ignored by QUIC MUX as allowed by RFC 9000. This means we can
always set EOS at the same time with EOI.
Mariam John [Mon, 22 May 2023 18:11:13 +0000 (13:11 -0500)]
DOC/MINOR: config: Fix typo in description for `ssl_bc` in configuration.txt
Fix a minor typo in the description of the `ssl_bc` sample fetch method described under
Section `7.3.4. Fetching samples at Layer 5` in configuration.txt. Changed `other` to `to`.
projects / thirdparty / haproxy.git / log
