Remove ABORT_NOW() statement on unhandled sendto error. Instead use a
dedicated counter sendto_err_unknown to report these cases.
If we detect increment of this counter, strace can be used to detect
errno value :
$ strace -p $(pidof haproxy) -f -e trace=sendto -Z
This should be backported up to 2.6.
This should help to debug github issue #1903.
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.
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 must be backported up to 2.6.
adding a new mt macro: MT_LIST_APPEND_LOCKED.
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.
No backport needed.
A minor typo was made in MT_LIST_LOCK_ELT, preventing
haproxy from compiling if MT_LIST_LOCK_ELT is
used in the code.
Today, the macro is unused, and that's the reason why
the typo has remained unnoticed for such a long time.
Fixing it so it can be used in upcoming developments.
No backport required.
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.
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.
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.
Must be backported as far as 2.5.
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.
Must be backported as far as 2.5.
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.
Must be backported in 2.5 and 2.6.
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.
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.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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.
This should be backported up to 2.6.
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)
This must be backported up to 2.6.
Fix several warinings as this one:
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.
Must be backported to 2.6
A previous commit tries to fix uninitialized GCC warning on ssl code for
QUIC build. See the fix here :
48e46f98cc
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.
This must be backported up to 2.6.
If the uri is unexpected ("/" in place of "http://xxx/"), some parsing
function fails. The failure is not handled.
This patch handle these errors. Note: the return code is boolean, maybe
we can return more precise error for Lua reporting ?
Must be backported in 2.6.
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
As announced in
https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
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.
The HTTPclient callback req_payload callback is set when a request payload
must be streamed. In the lua, this callback is set when a body is passed as
argument in one of httpclient functions (head/get/post/put/delete). However,
there is no reason to set it if body string is empty.
This patch is related to the issue #1898. It may be backported as far as
2.5.
In the HTTP client, when the request body is streamed, at the end of the
payload, we must be sure to not set the EOM flag on an empty message.
Otherwise, because there is no data, the buffer is reset to be released and
the flag is lost. Thus, the HTTP client is never notified of the end of
payload for the request and the applet is blocked. If the HTTP client is
instanciated from a Lua script, it is even worse because we fall into a
wakeup loop between the lua script and the HTTP client applet. At the end,
HAProxy is killed because of the watchdog.
This patch should fix the issue #1898. It must be backported to 2.6.
Even if this cannot happen, ensure <bind_conf> is initialized in this
function to please some compilers.
Takes the opportunity of this patch to replace an ABORT_NOW() by
a BUG_ON() because if the variable values they test are not initialized,
this is really because there is a bug.
Must be backported to 2.6.
Right now the QUIC thread mapping derives the thread ID from the CID
by dividing by global.nbthread. This is a problem because this makes
QUIC work on all threads and ignores the "thread" directive on the
bind lines. In addition, only 8 bits are used, which is no more
compatible with the up to 4096 threads we may have in a configuration.
Let's modify it this way:
- the CID now dedicates 12 bits to the thread ID
- on output we continue to place the TID directly there.
- on input, the value is extracted. If it corresponds to a valid
thread number of the bind_conf, it's used as-is.
- otherwise it's used as a rank within the current bind_conf's
thread mask so that in the end we still get a valid thread ID
for this bind_conf.
The extraction function now requires a bind_conf in order to get the
group and thread mask. It was better to use bind_confs now as the goal
is to make them support multiple listeners sooner or later.
Change the output of the "reload" command, it now displays "Success=0"
if the reload failed and "Success=1" if it succeed.
If the startup-logs is available (USE_SHM_OPEN=1), the command will
print a "--\n" line, followed by the content of the startup-logs.
Example:
$ echo "reload" | socat /tmp/master.sock -
Success=1
--
[NOTICE] (482713) : haproxy version is 2.7-dev7-4827fb-69
[NOTICE] (482713) : path to executable is ./haproxy
[WARNING] (482713) : config : 'http-request' rules ignored for proxy 'frt1' as they require HTTP mode.
[NOTICE] (482713) : New worker (482720) forked
[NOTICE] (482713) : Loading success.
$ echo "reload" | socat /tmp/master.sock -
Success=0
--
[NOTICE] (482886) : haproxy version is 2.7-dev7-4827fb-69
[NOTICE] (482886) : path to executable is ./haproxy
[ALERT] (482886) : config : parsing [test3.cfg:1]: unknown keyword 'Aglobal' out of section.
[ALERT] (482886) : config : Fatal errors found in configuration.
[WARNING] (482886) : Loading failure!
$
When compiled with USE_SHM_OPEN=1 the startup-logs are now able to use
an shm which is used to keep the logs when switching to mworker wait
mode. This allows to keep the failed reload logs.
When allocating the startup-logs at first start of the process, haproxy
will do a shm_open with a unique path using the PID of the process, the
file is unlink immediatly so we don't let unwelcomed files be. The fd
resulting from this shm is stored in the HAPROXY_STARTUPLOGS_FD
environment variable so it can be mmap again when switching to wait
mode.
When forking children, the process is copying the mmap to a a mallocated
ring so we never share the same memory section between the master and
the workers. When switching to wait mode, the shm is not used anymore as
it is also copied to a mallocated structure.
This allow to use the "show startup-logs" command over the master CLI,
to get the logs of the latest startup or reload. This way the logs of
the latest failed reload are also kept.
This is only activated on the linux-glibc target for now.
Split the b_force_xfer() into b_ncat() and b_force_xfer().
The previous b_force_xfer() implementation was basically a copy with a
b_del on the src buffer. Keep this implementation to make b_ncat(), and
just call b_ncat() + b_del() into b_force_xfer().
Cast an unified ring + storage area to a ring from area, without
reinitializing the data buffer. Reinitialize the waiters and the lock.
It helps retrieving a previously allocated ring, from an mmap for
example.
Each time data is read on QUIC receiver socket, we try to reuse the
first datagram of the currently used quic_receiver_buf instead of
allocating a new one. This algorithm is suboptimal if there is several
unused datagrams as only the first one is tested and its buffer removed
from quic_receiver_buf.
If QUIC traffic is quite substential, this can lead to an important
number of quic_dgram occurences allocated from pool_head_quic_dgram and
a lack of free space in allocated quic_receiver_buf buffers.
To improve this, each time we want to reuse a datagram, we pop elements
until a non-yet released datagram is found or the list is empty. All
intermediary elements are freed and the last found datagram can be
reused. This operation has been extracted in a dedicated function named
quic_rxbuf_purge_dgrams().
This should improve memory consumption incured by quic_dgram instances under heavy
QUIC traffic. Note that there is still room for improvement as if the
first datagram is still in use, it may block several unused datagram
after him. However this requires to support removal of datagrams out of
order which is currently not possible.
This should be backported up to 2.6.
QUIC datagrams are read from a random thread. They are then redispatch
to the connection thread according to the first packet DCID. These
operations are implemented through a special buffer designed to avoid
locking.
Refactor this code with the following changes :
* <rxbuf> type is renamed <quic_receiver_buf>. Its list element is also
renamed to highligh its attach point to a receiver.
* <quic_dgram> and <quic_receiver_buf> definition are moved to
quic_sock-t.h. This helps to reduce the size of quic_conn-t.h.
* <quic_dgram> list elements are renamed to highlight their attach point
into a <quic_receiver_buf> and a <quic_dghdlr>.
This should be backported up to 2.6.
rxbuf is the structure used to store QUIC datagrams and redispatch them
to the connection thread.
Each receiver manages a list of rxbuf. This was stored both as an array
and a mt_list. Currently, only mt_list is needed so removed <rxbufs>
member from receiver structure.
This should be backported up to 2.6.
Implement quic_tls_secrets_keys_alloc()/quic_tls_secrets_keys_free() to allocate
the memory for only one direction (RX or TX).
Modify ha_quic_set_encryption_secrets() to call these functions for one of this
direction (or both). So, for now on we can rely on the value of the secret keys
to know if it was derived.
Remove QUIC_FL_TLS_SECRETS_SET flag which is no more useful.
Consequently, the secrets are dumped by the traces only if derived.
Must be backported to 2.6.
