Willy Tarreau [Sun, 13 Nov 2022 11:21:22 +0000 (12:21 +0100)]
BUILD: compiler: define a __fallthrough statement for switch/case
When the code is preprocessed first and compiled later, such as when
built under distcc, a lot of fallthrough warnings are emitted because
the preprocessor has already stripped the comments.
As an alternative, a "fallthrough" attribute was added with the same
compilers as those which started to emit those warnings. However it's
not portable to older compilers. Let's just define a __fallthrough
statement that corresponds to this attribute on supported compilers
and only switches to the classical empty do {} while (0) on other ones.
This way the code will support being cleaned up using __fallthrough.
Willy Tarreau [Sun, 13 Nov 2022 10:39:18 +0000 (11:39 +0100)]
BUILD: compiler: add a default definition for __has_attribute()
It happens that gcc since 5.x has this macro which is only mentioned
once in the doc, associated with __builtin_has_attribute(). Clang had
it at least since 3.0. In addition it validates #ifdef when present,
so it's easy to detect it. Here we're providing a fallback to another
macro __has_attribute_<name> so that it's possible to define that macro
to the value 1 for older compilers when the attribute is supported.
Willy Tarreau [Sun, 13 Nov 2022 10:32:45 +0000 (11:32 +0100)]
BUILD: compiler: add a macro to detect if another one is set and equals 1
In order to simplify compiler-specific checks, we'll need to check if some
attributes exist. In order to ease declarations, we'll only focus on those
that exist and will set them to 1. Let's first add a macro aimed at doing
this. Passed a macro name in argument, it will return 1 if the macro is
defined and equals 1, otherwise it will return 0. This is based on the
concatenation of the macro's value with a name to form the name of a macro
which contains one comma, resulting in some other macros arguments being
shifted by one when the macro is defined. As such it's only a matter of
pushing both a 1 and a 0 and picking the correct argument to see the
desired one. It was verified to work since at least gcc-3.4 so it should
be portable enough.
Willy Tarreau [Sun, 13 Nov 2022 12:06:02 +0000 (13:06 +0100)]
IMPORT: slz: define and use a __fallthrough statement for switch/case
When the code is preprocessed first and compiled later, such as when
built under distcc, the "fall through" comments are dropped and warnings
are emitted. Let's use the alternative "fallthrough" attribute instead,
that is supported by versions of gcc and clang that also produce this
warning.
Willy Tarreau [Sun, 13 Nov 2022 12:21:56 +0000 (13:21 +0100)]
IMPORT: xxhash: update xxHash to version 0.8.1
This is the latest released version and a minor update on top of the
current one (0.8.0). It addresses a few build issues (some for which
patches were already backported), and particularly the fallthrough
issue by using an attribute instead of a comment.
Willy Tarreau [Mon, 14 Nov 2022 10:03:18 +0000 (11:03 +0100)]
CI: emit the compiler's version in the build reports
Some occasional builds fail only on a specific platform and being able
to figure the exact compiler version used there is crucial. It's not
easy to guess from the rest of the output, so let's add it before the
platform-specific defines, which suit the same needs.
Willy Tarreau [Sun, 13 Nov 2022 11:14:10 +0000 (12:14 +0100)]
BUILD: debug: remove unnecessary quotes in HA_WEAK() calls
HA_WEAK() is supposed to take a symbol in argument, not a string, since
the asm statements it produces already quote the argument. Having it
quoted twice doesn't work on older compilers and was the only reason
why DEBUG_MEM_STATS didn't work on older compilers.
Willy Tarreau [Mon, 14 Nov 2022 07:11:32 +0000 (08:11 +0100)]
BUILD: ssl_utils: fix build on gcc versions before 8
Commit 960fb74ca ("MEDIUM: ssl: {ca,crt}-ignore-err can now use error
constant name") provided a very convenient way to initialize only desired
macros. Unfortunately with gcc versions older than 8, it breaks with:
src/ssl_utils.c:473:12: error: initializer element is not constant
because it seems that the compiler cannot resolve strings to constants
at build time.
This patch takes a different approach, it stores the value of the macro
as a string and this string is converted to integer at boot time. This
way it works everywhere.
BUG/MINOR: ssl: bind_conf is uncorrectly accessed when using QUIC
Since commit 9b2598 ("BUG/MEDIUM: ssl: Verify error codes can exceed
63"), the ca_ignerr_bitfield and crt_ignerr_bietfield are incorrecly
accessed from __objt_listener(conn->target)->bind_conf which is not
avaiable from QUIC. The bind_conf variable was mistakenly replaced.
This patch fixes the issue by using again the bind_conf variable.
Amaury Denoyelle [Thu, 10 Nov 2022 14:16:49 +0000 (15:16 +0100)]
MINOR: server: clear prefix on stderr logs after add server
cli_parse_add_server() is the CLI handler for 'add server' command. This
functions uses usermsgs_ctx to retrieve logs messages from internal
ha_alert() calls and display it at the end of the handler.
At the beginning of the handler, stderr prefix is defined to "CLI" via
usermsgs_clr() function. However, this is not resetted at the end. This
causes inconsistency for stderr output :
1. each ha_alert() invocation will reuse "CLI" prefix if 'add server'
command was executed before, even in non-CLI context
2. usermsgs_ctx is thread local, so this is only true if this runs on
the same thread as 'add server' handler.
To fix this, ensure that "CLI" prefix is now resetted after
cli_parse_add_server(). This is done thanks to the addition to
cli_umsg()/cli_umsgerr() functions.
This can be backported up to 2.5 if we prefer to ensure output
consistency at the risk of changing stderr behaviors in stable versions.
In this case, the previous commit should be backported before :
MINOR: cli: define usermsgs print context
Amaury Denoyelle [Thu, 10 Nov 2022 13:24:51 +0000 (14:24 +0100)]
MINOR: cli: define usermsgs print context
CLI 'add server' handler relies on usermsgs_ctx to display errors in
internal function on CLI output. This may be also extended to other
handlers.
However, to not clutter stderr from another contextes, usermsgs_ctx must
be resetted when it is not needed anymore. This operation cannot be
conducted in the CLI parse handler as display is conducted after it.
To achieve this, define new CLI states CLI_ST_PRINT_UMSG /
CLI_ST_PRINT_UMSGERR. Their principles is nearly identical to states for
dynamic messages printing.
Amaury Denoyelle [Thu, 10 Nov 2022 10:47:36 +0000 (11:47 +0100)]
CLEANUP: cli: rename dynamic error printing state
Rename CLI_ST_PRINT_FREE to CLI_ST_PRINT_DYNERR.
Most notably, this highlights that this is reserved to error printing.
This is done to ensure consistency between CLI_ST_PRINT/CLI_ST_PRINT_DYN
and CLI_ST_PRINT_ERR/CLI_ST_PRINT_DYNERR. The name is also consistent
with the function cli_dynerr() which activates it.
MEDIUM: ssl: {ca,crt}-ignore-err can now use error constant name
The ca-ignore-err and crt-ignore-err directives are now able to use the
openssl X509_V_ERR constant names instead of the numerical values.
This allow a configuration to survive an OpenSSL upgrade, because the
numerical ID can change between versions. For example
X509_V_ERR_INVALID_CA was 24 in OpenSSL 1 and is 79 in OpenSSL 3.
The list of errors must be updated when a new major OpenSSL version is
released.
The CRT and CA verify error codes were stored in 6 bits each in the
xprt_st field of the ssl_sock_ctx meaning that only error code up to 63
could be stored. Likewise, the ca-ignore-err and crt-ignore-err options
relied on two unsigned long longs that were used as bitfields for all
the ignored error codes. On the latest OpenSSL1.1.1 and with OpenSSLv3
and newer, verify errors have exceeded this value so these two storages
must be increased. The error codes will now be stored on 7 bits each and
the ignore-err bitfields are replaced by a big enough array and
dedicated bit get and set functions.
It can be backported on all stable branches.
[wla: let it be tested a little while before backport] Signed-off-by: William Lallemand <wlallemand@haproxy.org>
When running HAProxy with OpenSSLv3, the two BIGNUMs used to build our
own DH parameters are not freed. It was not necessary previously because
ownership of those parameters was transferred to OpenSSL through the
DH_set0_pqg call.
BUG/MINOR: httpclient: fixed memory allocation for the SSL ca_file
The memory for the SSL ca_file was allocated only once (in the function
httpclient_create_proxy()) and that pointer was assigned to each created
proxy that the HTTP client uses. This would not be a problem if this
memory was not freed in each individual proxy when it was deinitialized
in the function ssl_sock_free_srv_ctx().
Memory allocation:
src/http_client.c, function httpclient_create_proxy():
1277: if (!httpclient_ssl_ca_file)
1278: httpclient_ssl_ca_file = strdup("@system-ca");
1280: srv_ssl->ssl_ctx.ca_file = httpclient_ssl_ca_file;
Memory deallocation:
src/ssl_sock.c, function ssl_sock_free_srv_ctx():
5613: ha_free(&srv->ssl_ctx.ca_file);
Amaury Denoyelle [Tue, 25 Oct 2022 09:38:21 +0000 (11:38 +0200)]
BUG/MINOR: quic: fix race condition on datagram purging
Each datagram is received by a random thread and dispatch to its
destination thread linked to the connection. Then, the datagram is
handled by the connection thread. Once this is done, datagram buffer
pointer is atomically set to NULL to mark it as consumed.
Consumed datagrams are purged before recvfrom() invocation on random
receiver threads. The check for NULL buffer must thus be done
atomically. This was not the case before this patch, which may have
triggered race conditions.
Amaury Denoyelle [Thu, 27 Oct 2022 15:56:27 +0000 (17:56 +0200)]
MINOR: quic: add counter for interrupted reception
Add a new counter "quic_rxbuf_full". It is incremented each time
quic_sock_fd_iocb() is interrupted on full buffer.
This should help to debug github issue #1903. It is suspected that
QUIC receiver buffers are full which in turn cause quic_sock_fd_iocb()
to be called repeatedly resulting in a high CPU consumption.
BUG/MINOR: log: fixing bug in tcp syslog_io_handler Octet-Counting
syslog_io_handler does specific treatment to handle syslog tcp octet
counting:
Logic was good, but a sneaky mistake prevented
rfc-6587 octet counting from working properly.
trash.area was used as an input buffer.
It does not make sense here since it is uninitialized.
Compilation was unaffected because trash is a thread
local "global" variable.
Subscribing was not properly designed between quic-conn and quic MUX
layers. Align this as with in other haproxy components : <subs> field is
moved from the MUX to the quic-conn structure. All mention of qcc MUX is
cleaned up in quic_conn_subscribe()/quic_conn_unsubscribe().
Thanks to this change, ACK reception notification has been simplified.
It's now unnecessary to check for the MUX existence before waking it.
Instead, if <subs> quic-conn field is set, just wake-up the upper layer
tasklet without mentionning MUX. This should probably be extended to
other part in quic-conn code.
A specialized listener accept was previously used for QUIC. This is now
unneeded and we can revert to the default one session_accept_fd().
One change of importance is that the call order between
conn_xprt_start() and conn_complete_session() is now reverted to the
default one. This means that MUX instance is now NULL during
qc_xprt_start() and its app-ops layer cannot be set here. This operation
has been delayed to qc_init() to prevent a segfault.
BUG/MAJOR: stick-table: don't process store-response rules for applets
The commit bc7c207f74 ("BUG/MAJOR: stick-tables: do not try to index a
server name for applets") tried to catch applets case when we tried to index
the server name. However, there is still an issue. The applets are
unconditionally casted to servers and this bug exists since a while. it's
just luck if it doesn't crash.
Now, when store rules are processed, we skip the rule if the stream's target
is not a server or, of course, if it is a server but the "non-stick" option
is set. However, we still take care to release the sticky session.
This patch must be backported to all stable versions.
The error check on certificate chain was ignoring all decoding error,
silently ignoring some errors.
This patch fixes the issue by being stricter on errors when reading the
chain, this is a change of behavior, it could break existing setup that
has a wrong chain.
MINOR: ssl: add the SSL error string before the chain
Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
descriptive errror are emitted. This one is for the certificate before
trying to load the complete chain.
MINOR: ssl: add the SSL error string when failing to load a certificate
Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
descriptive errror are emitted.
Example:
[ALERT] (1264006) : config : parsing [ssl_default_server.cfg:51] : 'bind /tmp/ssl.sock' in section 'listen' : unable to load certificate chain from file 'reg-tests/ssl//common.pem': ASN no PEM Header Error
BUG/MINOR: sink: Set default connect/server timeout for implicit ring buffers
Ring buffers may be implicitly created from log declarations when "tcp@",
"tcp6@", "tcp4@" or "uxst@" prefixes are used. These ring buffers rely on
unconfigurable proxies. While connect and server timeouts should be defined for
explicit ring buffers, it is no possible for implicit ones. However, a default
value must be set and TICK_ETERNITY is not an acceptable one.
Thus, now "1s" is set for the connect timeout and "5s" is set for server one.
BUG/MINOR: sink: Only use backend capability for the sink proxies
When a ring section is parsed, a proxy is created. For now, it has the
frontend (PR_CAP_FE) and the internal (PR_CAP_INT) capabilities, in addition
to the expected backend capability (PR_CAP_BE).
PR_CAP_INT capability was added to silent warning triggered because of
PR_CAP_FE capability. Indeed, Because the proxy is declared as a frontend,
warnings about missing bind lines and missing client timeout should be
triggered during the configuration parsing. These warnings are inhibited
because PR_CAP_INT capability is set. It is an issue on the 2.4 because
PR_CAP_INT capability does not exist. So warnings are always emitted.
But the true bug is that these proxies should not have PR_CAP_FE and
PR_CAP_INT capabilities. Removing these capabilities is enough to remove any
warnings on the 2.4, with no regression on higher versions. However, it may
be a good idea to eval if a dedicated frontend for sinks should be added or
not. This way, a true frontend would be used to start the sink applets. In
addition, proxies capabilities/modes have to be reviewed to have a less
ambiguous API. For instance a dedicate mode for sinks (PR_MODE_SINK ?) may
be added. Finally, it could be very nice to have all proxies in the same
list, including internal ones.
This patch should fix the issue #1900. It must be backported as far as 2.4.
Emeric Brun [Mon, 24 Oct 2022 08:04:59 +0000 (10:04 +0200)]
MINOR: peers: handle multiple resync requests using shards
We considered the resync process is finished if a full resync request
is ended receiving the "resync-finish" message. But in the case of
"shards" each node declared with a "shard" has only a partial view
of the table. And the resync process is ended whereas the original
peer tables content contains only a "shard" of the full content.
This patch allow to retrieve the entire tables requesting a resync
from all different "shards".
To do so we don't commit the end of a resync process receiving a
"resync-finish" if the node is part of "shard", we only flag this
peer and all peers using the same shard as "notup2date" as if we
received a "resync-partial" message, and we re-schedule a request
of a resync as it is done receiving a "resync-partial" message.
Doing this the peers flagged "notup2date" won't be addressed for the
next resync request round and the next resync request will be send to
a shard not yet requested.
Receving a "resync-finish" message we also check if all peers using
"shards" are flagged "notup2date". It meens that all peers have been
addressed and we can considered the resync process is now finished.
Note also that the "resync request" scheduler already handle a timeout
and if we are not able to retrieve a full resync after a delay. The
resync process is ended.
This patch should be backported in all versions handling "shard"
on peer lines.
Add "shards" new keyword for "peers" section to configure the number
of peer shards attached to such secions. This impact all the stick-tables
attached to the section.
Add "shard" new "server" parameter to configure the peers which participate to
all the stick-tables contents distribution. Each peer receive the stick-tables updates
only for keys with this shard value as distribution hash. The "shard" value
is stored in ->shard new server struct member.
cfg_parse_peers() which is the function which is called to parse all
the lines of a "peers" section is modified to parse the "shards" parameter
stored in ->nb_shards new peers struct member.
Add srv_parse_shard() new callback into server.c to pare the "shard"
parameter.
Implement stksess_getkey_hash() to compute the distribution hash for a
stick-table key as the 64-bits xxhash of the key concatenated to the stick-table
name. This function is called by stksess_setkey_shard(), itself
called by the already implemented function which create a new stick-table
key (stksess_new()).
Add ->idlen new stktable struct member to store the stick-table name length
to not have to compute it each time a stick-table key hash is computed.
BUG/MEDIUM: compression: handle rewrite errors when updating response headers
When an HTTP response is compressed by HAProxy, the headers are updated.
However it is possible to encounter a rewrite error because the buffer is
full. In this case, the compression is aborted. Thus, we must be sure to
leave the response in a valid state.
For now, it is an issue because the "Content-Encoding" header is added
before all other headers manipulations. So if the compression is aborted on
error, the "Content-Encoding" header may remain while the payload is not
compressed.
So now, we take care to leave with a valid response on error by reordering
the headers manipulations. It is too painful to really rollback all changes,
especially for an edge case.
This patch should be backported as far as 2.0. Note that on the 2.0, the
legacy HTTP part is also concerned.
Amaury Denoyelle [Fri, 21 Oct 2022 15:02:18 +0000 (17:02 +0200)]
BUG/MINOR: mux-quic: complete flow-control for uni streams
Max stream data was not enforced and respect for local/remote uni
streams. Previously, qcs instances incorrectly reused the limit defined
from bidirectional ones.
This is now fixed. Two fields are added in qcc structure connection :
* value for local flow control to enforce on remote uni streams
* value for remote flow control to respect on local uni streams
These two values can be reused to properly initialized msd field of a
qcs instance in qcs_new(). The rest of the code is similar.
This macro may be used to append an item to an existing
list, like MT_LIST_APPEND.
But here the item will be forced into locked/busy state
prior to appending, so that it is already referenced
in the list while still preventing concurrent accesses
until we decide to unlock it.
The macro returns a struct mt_list "np", that is needed
at unlock time using regular MT_LIST_UNLOCK_ELT macro.
MT_LIST_LOCK_ELT macro was documented with an ambiguous
usage restriction, implying that concurrent list deletion
was not supported.
But it seems that either the code has evolved, or the comment is
wrong because the locking behavior implemented here is exactly
the same one used in MT_LIST_DELETE, and no such restriction is
described for MT_LIST_DELETE.
I made some tests to make sure concurrent MT_LIST_DELETE (or deletion
from mt_list_for_each_entry_safe) don't cause unexepected results.
At the present time, this macro is not used, this fix only
targets upcoming developments that might rely on this.
MINOR: mworker/cli: does no try to dump the startup-logs w/o USE_SHM_OPEN
When haproxy is compiled without USE_SHM_OPEN, does not try to dump the
startup-logs in the "reload" output, because it won't show anything
interesting.
CI: github: dump the backtrace of coredumps in the alpine container
This patch allows to show the backtrace of a coredump produced in the
alpine/musl jobs.
It activates some option required by the containers to allow the
production of coredump, set a shared directory so the kernel could dump
the coredump within the container. Some debug packages were also added.
REGTESTS: httpclient/lua: test the lua task timeout with the httpclient
Test the httpclient when the lua action timeout. The lua timeout is
reached before the httpclient is ended. This test that the httpclient
are correctly cleaned when destroying the hlua context.
BUG/MEDIUM: httpclient: check if the httpclient was released in the IO handler
Upon a applet_release(), the applet can be scheduled again and a call to
the IO handler is still possible. When the struct httpclient is already
free the IO handler could try to access it.
This patch fixes the issue by setting svcctx to NULL in the
applet_release, and checking its value in the IO handler.
BUG/MEDIUM: httpclient/lua: crash when the lua task timeout before the httpclient
When the lua task finished before the httpclient that are associated to
it, there is a risk that the httpclient try to task_wakeup() the lua
task which does not exist anymore.
To fix this issue the httpclient used in a lua task are stored in a
list, and the httpclient are destroyed at the end of the lua task.
The connect timeout in a ring section was not properly parsed. Thus, it was
never set and the server timeout may be overwritten, depending on the
directives order. The first char of the keyword must be tested, not the
third one.
This patch is related to the issue #1900. But it does not fix the issue. It
must be backported as far as 2.4.
BUG/MINOR: log: Preserve message facility when the log target is a ring buffer
When a ring is used as log target, the original facility, if any, must be
preserved. The default facility must only be used if there no facility was
found in the incoming log message.
This patch should fix the issue #1901. It must be backported as far as 2.4.
Amaury Denoyelle [Mon, 17 Oct 2022 09:13:07 +0000 (11:13 +0200)]
MINOR: quic: extend Retry token check function
On Initial packet reception, token is checked for validity through
quic_retry_token_check() function. However, some related parts were left
in the parent function quic_rx_pkt_retrieve_conn(). Move this code
directly into quic_retry_token_check() to facilitate its call in various
context.
The API of quic_retry_token_check() has also been refactored. Instead of
working on a plain char* buffer, it now uses a quic_rx_packet instance.
This helps to reduce the number of parameters.
This change will allow to check Retry token even if data were received
with a FD-owned quic-conn socket. Indeed, in this case,
quic_rx_pkt_retrieve_conn() call will probably be skipped.
Amaury Denoyelle [Mon, 17 Oct 2022 10:04:49 +0000 (12:04 +0200)]
MINOR: quic: refactor packet drop on reception
Sometimes, a packet is dropped on reception. Several goto statements are
used, mostly to increment a proxy drop counter or drop silently the
packet. However, this labels are interleaved. Re-arrang goto labels to
simplify this process :
* drop label is used to drop a packet with counter incrementation. This
is the default method.
* drop_silent is the next label which does the same thing but skip the
counter incrementation. This is useful when we do not need to report
the packet dropping operation.
Amaury Denoyelle [Wed, 19 Oct 2022 13:37:44 +0000 (15:37 +0200)]
MINOR: quic: split and rename qc_lstnr_pkt_rcv()
This change is the following of qc_lstnr_pkt_rcv() refactoring. This
function has finally been split into several ones.
The first half is renamed quic_rx_pkt_parse(). This function is
responsible to parse a QUIC packet header and calculate the packet
length.
QUIC connection retrieval has been extracted and is now called directly
by quic_lstnr_dghdlr().
The second half of qc_lstnr_pkt_rcv() is renamed to qc_rx_pkt_handle().
This function is responsible to copy a QUIC packet content to a
quic-conn receive buffer.
A third function named qc_rx_check_closing() is responsible to detect if
the connection is already in closing state. As this requires to drop the
whole datagram, it seems justified to be in a separate function.
This change has no functional impact. It is part of a refactoring series
on qc_lstnr_pkt_rcv(). The objective is to facilitate the integration of
FD-owned quic-conn socket patches.
Amaury Denoyelle [Wed, 19 Oct 2022 13:28:44 +0000 (15:28 +0200)]
MINOR: quic: extract connection retrieval
Simplify qc_lstnr_pkt_rcv() by extracting code responsible to retrieve
the quic-conn instance. This code is put in a dedicated function named
quic_rx_pkt_retrieve_conn(). This new function could be skipped if a
FD-owned quic-conn socket is used.
The first traces of qc_lstnr_pkt_rcv() have been clean up as qc instance
is always NULL here : thus qc parameter can be removed without any
change.
This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.
Amaury Denoyelle [Wed, 19 Oct 2022 15:14:28 +0000 (17:14 +0200)]
MINOR: quic: define first packet flag
Received packets treatment has some difference regarding if this is the
first one or not of the encapsulating datagram. Previously, this was set
via a function argument. Simplify this by defining a new Rx packet flag
named QUIC_FL_RX_PACKET_DGRAM_FIRST.
This change does not have functional impact. It will simplify API when
qc_lstnr_pkt_rcv() is broken into several functions : their number of
arguments will be reduced thanks to this patch.
Amaury Denoyelle [Mon, 17 Oct 2022 16:05:26 +0000 (18:05 +0200)]
MINOR: quic: extend pn_offset field from quic_rx_packet
pn_offset field was only set if header protection cannot be removed.
Extend the usage of this field : it is now set everytime on packet
parsing in qc_lstnr_pkt_rcv().
This change helps to clean up API of Rx functions by removing
unnecessary variables and function argument.
This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.
Amaury Denoyelle [Mon, 17 Oct 2022 16:05:18 +0000 (18:05 +0200)]
MINOR: quic: add version field on quic_rx_packet
Add a new field version on quic_rx_packet structure. This is set on
header parsing in qc_lstnr_pkt_rcv() function.
This change has no functional impact. It is a part of a refactoring
series on qc_lstnr_pkt_rcv(). The objective is facilitate integration of
FD-owned socket patches.
Amaury Denoyelle [Tue, 18 Oct 2022 09:05:02 +0000 (11:05 +0200)]
BUG/MINOR: quic: fix buffer overflow on retry token generation
When generating a Retry token, client CID is used as encryption input.
The client must reuse the same CID when emitting the token in a new
Initial packet.
A memory overflow can occur on quic_generate_retry_token() depending on
the size of client CID. This is because space reserved for <aad> only
accounted for QUIC_HAP_CID_LEN (size of haproxy owned generated CID).
However, the client CID size only depends on client parameter and is
instead limited to QUIC_CID_MAXLEN as specified in RFC9000.
This was reproduced with ngtcp2 and haproxy built with ASAN. Here is the error
log :
==14964==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffee228cee at pc 0x7ffff785f427 bp 0x7fffee2289e0 sp 0x7fffee228188
WRITE of size 17 at 0x7fffee228cee thread T5
#0 0x7ffff785f426 in __interceptor_memcpy /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
#1 0x555555906ea7 in quic_generate_retry_token_aad src/quic_conn.c:5452
#2 0x555555907e72 in quic_retry_token_check src/quic_conn.c:5577
#3 0x55555590d01e in qc_lstnr_pkt_rcv src/quic_conn.c:6103
#4 0x5555559190fa in quic_lstnr_dghdlr src/quic_conn.c:7179
#5 0x555555eb0abf in run_tasks_from_lists src/task.c:590
#6 0x555555eb285f in process_runnable_tasks src/task.c:855
#7 0x555555d9118f in run_poll_loop src/haproxy.c:2853
#8 0x555555d91f88 in run_thread_poll_loop src/haproxy.c:3042
#9 0x7ffff709f8fc (/usr/lib/libc.so.6+0x868fc)
#10 0x7ffff7121a5f (/usr/lib/libc.so.6+0x108a5f)
src/qmux_trace.c:80:45: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ {aka ‘const long long unsigned int’} [-Werror=format=]
80 | chunk_appendf(&trace_buf, " qcs=%p .id=%lu .st=%s",
| ~~^
| |
| long unsigned int
| %llu
81 | qcs, qcs->id,
| ~~~~~~~
| |
| uint64_t {aka const long long unsigned int}
compilation terminated due to -Wfatal-errors.
Cast remaining uint64_t variables as ullong with %llu as printf format and size_t
others as ulong with %lu as printf format.
Thank you to Ilya for having reported this issue in GH #1899.
Amaury Denoyelle [Mon, 17 Oct 2022 16:46:49 +0000 (18:46 +0200)]
BUILD: ssl_sock: fix null dereference for QUIC build
A previous commit tries to fix uninitialized GCC warning on ssl code for
QUIC build. See the fix here : 48e46f98ccf97427995eb41c6f28cc38705bdd7e
BUILD: ssl_sock: bind_conf uninitialized in ssl_sock_bind_verifycbk()
However, this is incomplete as it still reports possible NULL
dereference on ctx variable (GCC v12.2.0). Here is the compilation
result :
src/ssl_sock.c: In function ‘ssl_sock_bind_verifycbk’:
src/ssl_sock.c:1739:12: error: potential null pointer dereference [-Werror=null-dereference]
1739 | ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
|
To fix this, remove check on qc which can also never happens and replace
it with a BUG_ON. This seems to satisfy GCC on my machine.
Willy Tarreau [Fri, 14 Oct 2022 18:45:23 +0000 (20:45 +0200)]
[RELEASE] Released version 2.7-dev8
Released version 2.7-dev8 with the following main changes :
- BUG/MINOR: checks: update pgsql regex on auth packet
- DOC: config: Fix pgsql-check documentation to make user param mandatory
- CLEANUP: mux-quic: remove usage of non-standard ull type
- CLEANUP: quic: remove global var definition in quic_tls header
- BUG/MINOR: quic: adjust quic_tls prototypes
- CLEANUP: quic: fix headers
- CLEANUP: quic: remove unused function prototype
- CLEANUP: quic: remove duplicated varint code from xprt_quic.h
- CLEANUP: quic: create a dedicated quic_conn module
- BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream
- BUG/MEDIUM: lua: Don't crash in hlua_lua2arg_check on failure
- BUG/MEDIUM: lua: handle stick table implicit arguments right.
- BUILD: h1: silence an initiialized warning with gcc-4.7 and -Os
- MINOR: fd: add a new function to only raise RLIMIT_NOFILE
- MINOR: init: do not try to shrink existing RLIMIT_NOFIlE
- BUG/MINOR: http-fetch: Update method after a prefetch in smp_fetch_meth()
- BUILD: http_fetch: silence an uninitiialized warning with gcc-4/5/6 at -Os
- BUG/MINOR: hlua: hlua_channel_insert_data() behavior conflicts with documentation
- MINOR: quic: limit usage of ssl_sock_ctx in favor of quic_conn
- MINOR: mux-quic: check quic-conn return code on Tx
- CLEANUP: quic: fix indentation
- MEDIUM: quic: retrieve frontend destination address
- CLEANUP: Reapply ist.cocci (2)
- CLEANUP: Reapply strcmp.cocci
- CLEANUP: quic/receiver: remove the now unused tx_qring list
- BUG/MINOR: quic: set IP_PKTINFO socket option for QUIC receivers only
- MINOR: hlua: some luaL_checktype() calls were not guarded with MAY_LJMP
- DOC: configuration: missing 'if' in tcp-request content example
- MINOR: hlua: removing ambiguous lua_pushvalue with 0 index
- BUG/MAJOR: stick-tables: do not try to index a server name for applets
- MINOR: plock: support disabling exponential back-off
- MINOR: freq_ctr: use the thread's local time whenever possible
- MEDIUM: stick-table: switch the table lock to rwlock
- MINOR: stick-table: do not take an exclusive lock when downing ref_cnt
- MINOR: stick-table: move the write lock inside stktable_touch_with_exp()
- MEDIUM: stick-table: only take the lock when needed in stktable_touch_with_exp()
- MEDIUM: stick-table: make stksess_kill_if_expired() avoid the exclusive lock
- MEDIUM: stick-table: return inserted entry in __stktable_store()
- MEDIUM: stick-table: free newly allocated stkess if it couldn't be inserted
- MEDIUM: stick-table: switch to rdlock in stktable_lookup() and lookup_key()
- MEDIUM: stick-table: make stktable_get_entry() look up under a read lock
- MEDIUM: stick-table: do not take a lock to update t->current anymore.
- MEDIUM: stick-table: make stktable_set_entry() look up under a read lock
- MEDIUM: stick-table: requeue the expiration task out of the exclusive lock
- MINOR: stick-table: split stktable_store() between key and requeue
- MEDIUM: stick-table: always use atomic ops to requeue the table's task
- MEDIUM: stick-table: requeue the wakeup task out of the write lock
- BUG/MINOR: stick-table: fix build with DEBUG_THREAD
- REORG: mux-fcgi: Extract flags and enums into mux_fcgi-t.h
- MINOR: flags/mux-fcgi: Decode FCGI connection and stream flags
- BUG/MEDIUM: mux-h1: Add connection error handling when reading/sending on a pipe
- BUG/MEDIUM: mux-h1: Handle abort with an incomplete message during parsing
- BUG/MINOR: server: make sure "show servers state" hides private bits
- MINOR: checks: use the lighter PRNG for spread checks
- MEDIUM: checks: spread the checks load over random threads
- CI: SSL: use proper version generating when "latest" semantic is used
- CI: SSL: temporarily stick to LibreSSL=3.5.3
- MINOR: quic: New quic_cstream object implementation
- MINOR: quic: Extract CRYPTO frame parsing from qc_parse_pkt_frms()
- MINOR: quic: Use a non-contiguous buffer for RX CRYPTO data
- BUG/MINOR: quic: Stalled 0RTT connections with big ClientHello TLS message
- MINOR: quic: Split the secrets key allocation in two parts
- CLEANUP: quic: remove unused rxbufs member in receiver
- CLEANUP: quic: improve naming for rxbuf/datagrams handling
- MINOR: quic: implement datagram cleanup for quic_receiver_buf
- MINOR: ring: ring_cast_from_area() cast from an allocated area
- MINOR: buffers: split b_force_xfer() into b_cpy() and b_force_xfer()
- MINOR: logs: startup-logs can use a shm for logging the reload
- MINOR: mworker/cli: reload command displays the startup-logs
- MEDIUM: quic: respect the threads assigned to a bind line
- DOC: management: update the "reload" command of the master CLI
- BUILD: ssl_sock: bind_conf uninitialized in ssl_sock_bind_verifycbk()
- BUG/MEDIUM: httpclient: Don't set EOM flag on an empty HTX message
- MINOR: httpclient/lua: Don't set req_payload callback if body is empty
- DOC/CLEANUP: lua-api: some minor corrections
- DOC: lua-api: updating toolbox link
- DOC/CLEANUP: lua-api: removing duplicate core.proxies attribute
- DOC: management: add forgotten "show startup-logs"
- DOC: management: "show startup-logs" for master CLI
- CI: Replace the deprecated `::set-output` command by writing to $GITHUB_OUTPUT in matrix.py
- CI: Replace the deprecated `::set-output` command by writing to $GITHUB_OUTPUT in workflow definition
the `::set-output` command is deprecated, because processes during the workflow
execution might output untrusted information that might include the
`::set-output` command, thus allowing these untrusted information to hijack the
build.
The replacement is writing to the file indicated by the `$GITHUB_OUTPUT`
environment variable.
Link to lua toolbox was dead (project has been deprecated).
Adding a legacy link to get old toolbox source code as well as
a link to luarocks that seems to have superseded it.