Commit Graph

22058 Commits

Author SHA1 Message Date
Christopher Faulet
75fb0afde4 BUG/MINOR: hlua: Fix log level to the right value when set via TXN:set_loglevel
When the log level is changed in lua, by calling TXN:set_loglevel function,
it must be incremented by one because it is decremented in strm_log()
function.

This patch must be backport to all stable versions.
2024-03-01 15:01:18 +01:00
Christopher Faulet
573ed242e3 BUG/MINOR: config/quic: Alert about PROXY protocol use on a QUIC listener
PROXY procotol is not supported on QUIC for now. Thus return an error during
configuration parsing if 'accept-proxy' option is used for a QUIC listener.

This patch should fix the issue #2186. It should be backport as far as 2.6.
2024-03-01 15:01:18 +01:00
William Lallemand
e2a44d6c94 DOC: configuration: clarify ciphersuites usage
Ciphersuites can be used with any TLS/SSL protocol version and are not
specific to TLSv1.3. However you can only specify the TLSv1.3 ciphers in
ciphersuite format.

Should fix issue #2459.

Backport to every stable branches.
2024-02-29 18:07:09 +01:00
Christopher Faulet
69f15b9a40 CLEANUP: mux-h2: Fix h2s_make_data() comment about the return value
2 return values are specified in the h2s_make_data() function comment. Both
are more or less equivalent but the later is probably more accurate. So,
keep the right one and remove the other one.

This patch should fix the issue #2175.
2024-02-29 13:57:44 +01:00
Amaury Denoyelle
f913d42aaf MINOR: quic: add MUX output for show quic
Extend "show quic" to be able to dump MUX related information. This is
done via the new function qcc_show_quic(). This replaces the old streams
dumping list which was incomplete.

These info are displayed on full output or by specifying "mux" field.
2024-02-29 10:03:36 +01:00
Amaury Denoyelle
dda3a0d8fc MINOR: quic: specify show quic output fields
Add the possibility to customize show quic full output with only a
specific set of printed fields. This is specified as a comma-separated
list. Here are the currently supported values :
* tp: transport parameters
* sock: connection addresses and socket FD
* pktns: packet number space with ack ranges and in flight bytes
* cc: congestion controler and loss information

Note that streams output is not filtered by this mechanism. It's because
it will be replaced soon by an output generated from the MUX which will
use its owned field name.
2024-02-29 10:03:36 +01:00
Amaury Denoyelle
c4f5ff8369 MINOR: quic: filter show quic by address
Add the possibilty to restrict show quic output to only a single
connection. This is done by specifying a quic_conn address pointer.

Default format selection has evolved with it. Indeed, it seems more
fitting to use full format by default when filtering on a connection.
However, it's still possible to revert to the original oneline format
with it by specifying it explicitely.
2024-02-29 10:03:33 +01:00
Christopher Faulet
60fcc27577 MEDIUM: htx/http-ana: No longer close connection on early HAProxy response
When a response was returned by HAProxy, a dedicated HTX flag was
set. Thanks to this flag, it was possible to add a "connection: close"
header to the response if the request was not fully received and to close
the connection. In the same way, when a redirect rule was applied,
keep-alive was forcefully disabled for unfinished requests.

All these mechanisms are now useless because the H1 mux is able to drain the
response. So HTX_FL_PROXY_RESP flag is removed and no special processing is
performed on HAProxy response when the request is unfinished.
2024-02-28 16:02:33 +01:00
Christopher Faulet
077906da14 MAJOR: mux-h1: Drain requests on client side before shut a stream down
unlike for H2 and H3, there is no mechanism in H1 to notify the client it
must stop to upload data when a response is replied before the end of the
request without closing the connection. There is no RST_STREAM frame
equivalent.

Thus, there is only two ways to deal with this situation: closing the
connection or draining the request. Until now, HAProxy didn't support
draining H1 messages. Closing the connection in this case has however a
major drawback. It leads to send a TCP reset, dropping this way all in-fly
data. There is no warranty the client has fully received the response.

Draining H1 messages was never implemented because in old versions it was a
bit tricky to implement. However, it is now far simplier to support this
feature because it is possible to have a H1 stream without any applicative
stream. It is the purpose of this patch. Now, when a shutdown is requested
and the stream is detached from the connection, if the request is unfinished
while the response was fully sent, the request in drained.

To do so, in this case the shutdown and the detach are delayed. From the
upper layer point of view, there is no changes. The endpoint is shut down
and detached as usual. But on H1 mux point of view, the H1 stream is still
alive and is being able to drain data. However the stream-endpoint
descriptor is orphan. Once the request is fully received (and drained), the
connection is shut down if it cannot be reused for a new transaction and the
H1 stream is destroyed.
2024-02-28 16:02:33 +01:00
Christopher Faulet
14db433db9 MINOR: mux-h1: Move all stuff to detach a stream in an internal function
All code from h1_detach() function was moved in a internal function,
h1s_finish_detach(). It will be used to defer the detach and be able to
drain the requests payload.
2024-02-28 15:31:07 +01:00
Christopher Faulet
7ae280d091 MINOR: mux-h1: Move checks performed before a shutdown in a dedicated function
Checks performed in h1_shutw() to determine if the connection must be
shutdown now or not was move in a dedicated function. This will be used to
be able to drain the requests payload.
2024-02-28 15:31:07 +01:00
Christopher Faulet
81f75d32b2 BUG/MINOR: mux-h1: Properly report when mux is blocked during a nego
During a zero-copy forwarding negociation, if the H1 mux is blocked for any
reason, the IOBUF_FL_FF_BLOCKED flag must be set on its iobuf to notfiy the
producer it must wait. However, there were two places where it was not
performed: when the output buffer allocation failed and when the chunk
formatting failed.

This patch fixes the issue. It must be backported to 2.9.
2024-02-28 15:31:07 +01:00
Christopher Faulet
489e583ac5 BUG/MEDIUM: mux-h1: Fix again 0-copy forwarding of chunks with an unknown size
There is still an issue with zero-copy forwarding of chunks with an unknown
size. It is possible for a producer to fill the sapce reserved for the CRLF
at the end of the chunk. The root cause is that this space is not accounted
in the iobuf offset. So, from the producer point of view, the space may be
used. We can also argue the current design for iobuf is not well suited for
this case. Instead of using a pointer on the consumer's buffer, it could be
easier to use a custom buffer built on top of the consumer one, via a call
to b_make(), with the size, head and data field reflecting the avaialble
space the producer can use.

By the way, because of this bug, it is possible to trigger a BUG_ON() when
we try to write the CRLF at the end of the chunk because the buffer is
full. It is unexpected. Only the stats applet may hit this bug.

To fix the issue, instead of writting this CRLF when the current chunk is
consumed, it is written before consuming the next one. This way, all space
reserved to create the chunk formatting is always placed before forwarding
data.

No backport needed.
2024-02-28 15:31:07 +01:00
Aurelien DARRAGON
60edfabc7b LICENSE: http_ext: fix GPL license version
This is a followup of the previous commit: GH user @songliumeng initially
reported an issue with the GPL license version for event_hdl source file
which was fixed by the previous commit. It turns out the same mistake was
made in http_ext source file: due to a mixup between LGPL and GPL, GPL
version '2.1' was referenced instead of '2'.

Again, clarify that this is indeed GPL by making use of the banner
provided in doc/gpl.txt

This should be backported in 2.8 with b2bb925 ("MINOR: proxy/http_ext:
introduce proxy forwarded option")
2024-02-28 15:13:35 +01:00
Aurelien DARRAGON
e9261fff29 LICENSE: event_hdl: fix GPL license version
As spotted by user @songliumeng in GH #2463, there was a mixup between
LGPL and GPL in event_hdl source file: GPL version '2.1' was referenced
instead of '2'. Clarify that this is indeed GPL by making use of the
banner provided in doc/gpl.txt.

This should be backported in 2.8 with 68e692d ("MINOR: event_hdl: add
event handler base api")
2024-02-28 15:13:27 +01:00
William Lallemand
bb7af8b2f1 BUG/MINOR: ssl/cli: duplicate cleaning code in cli_parse_del_crtlist
Since 23cab33 ("BUG/MINOR: ssl: Clear the ckch instance when deleting a
crt-list line"), LIST_DELETE is done twice, one time in
cli_parse_del_crtlist() and another time in ckch_inst_free().

It could trigger a crash with -DDEBUG_LIST.

This isn't a major problem since the ptr is not freed in the meantime so
it will only trigger with the debug.

This patch removes the LIST_DELETE as well as the loop done on link_ref
which is also don in ckch_inst_free()

Could be backported as far as 2.4. 2.4 version does not have a link_ref
loop.
2024-02-27 18:10:43 +01:00
Amaury Denoyelle
8a31783b64 BUG/MEDIUM: server: fix dynamic servers initial settings
Contrary to static servers, dynamic servers does not initialize their
settings from a default server instance. As such, _srv_parse_init() was
responsible to set a set of minimal values to have a correct behavior.

However, some settings were not properly initialized. This caused
dynamic servers to not behave as static ones without explicit
parameters.

Currently, the main issue detected is connection reuse which was
completely impossible. This is due to incorrect pool_purge_delay and
max_reuse settings incompatible with srv_add_to_idle_list().

To fix the connection reuse, but also more generally to ensure dynamic
servers are aligned with other server instances, define a new function
srv_settings_init(). This is used to set initial values for both default
servers and dynamic servers. For static servers, srv_settings_cpy() is
kept instead, using their default server as reference.

This patch could have unexpected effects on dynamic servers behavior as
it restored proper initial settings. Previously, they were set to 0 via
calloc() invocation from new_server().

This should be backported up to 2.6, after a brief period of
observation.
2024-02-27 17:02:20 +01:00
William Lallemand
4895fdac5a BUG/MAJOR: ssl/ocsp: crash with ocsp when old process exit or using ocsp CLI
This patch reverts 2 fixes that were made in an attempt to fix the
ocsp-update feature used with the 'commit ssl cert' command.

The patches crash the worker when doing a soft-stop when the 'set ssl
ocsp-response' command was used, or during runtime if the ocsp-update
was used.

This was reported in issue #2462 and #2442.

The last patch reverted is the associated reg-test.

Revert "BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when an update is ongoing"
This reverts commit 5e66bf26ec.

Revert "BUG/MEDIUM: ocsp: Separate refcount per instance and per store"
This reverts commit 04b77f84d1b52185fc64735d7d81137479d68b00.

Revert "REGTESTS: ssl: Add OCSP related tests"
This reverts commit acd1b85d3442fc58164bd0fb96e72f3d4b501d15.
2024-02-26 18:04:25 +01:00
Christopher Faulet
19559d4447 BUG/MEDIUM: applet: Fix HTX .rcv_buf callback function to release outbuf buffer
In appctx_htx_rcv_buf(), HTX blocks found in the appctx output buffer are
copied into the channel buffer. At the end, the state of the underlying
buffer must be updated. If everything was copied, the buffer is reset. This
way, it will be released later, at the end of the applet process function.

However, here there was a typo. We do it on the input buffer instead of the
output buffer. As side effect, an empty HTX message remained stuck in the
appctx outbut buffer, blocking the applet and leading to blocked session
with no expiration date.

No backport needed.
2024-02-26 16:40:13 +01:00
Willy Tarreau
dec017575d [RELEASE] Released version 3.0-dev4
Released version 3.0-dev4 with the following main changes :
    - BUG/MEDIUM: ssl: Fix crash when calling "update ssl ocsp-response" when an update is ongoing
    - BUG/MEDIUM: quic: Wrong K CUBIC calculation.
    - MINOR: quic: Update K CUBIC calculation (RFC 9438)
    - MINOR: quic: Dynamic packet reordering threshold
    - MINOR: quic: Add a counter for reordered packets
    - BUG/MAJOR: mux-h1: Fix zero-copy forwarding when sending chunks of unknown size
    - MINOR: stats: Use a dedicated function to check if output is almost full
    - BUG/MEDIUM: applet: Add a flag to state an applet is using zero-copy forwarding
    - BUG/MEDIUM: stconn/applet: Block 0-copy forwarding if producer needs more room
    - MINOR: applet: Remove uselelss test on SE_FL_SHR/SHW flags
    - MEDIUM: applet: Add notion of shutdown for write for applets
    - MINOR: cli: No longer check SC for shutdown to interrupt wait command
    - BUG/MEDIUM: stconn: Allow expiration update when READ/WRITE event is pending
    - BUG/MEDIUM: stconn: Don't check pending shutdown to wake an applet up
    - CLEANUP: stconn: Move SE flags set by app layer at the end of the bitfield
    - MINOR: stconn: Rename SE_FL_MAY_FASTFWD and reorder bitfield
    - MINOR: stconn: Add SE flag to announce zero-copy forwarding on consumer side
    - MINOR: muxes: Announce support for zero-copy forwarding on consumer side
    - BUG/MAJOR: stconn: Check support for zero-copy forwarding on both sides
    - MINOR: muxes/applet: Simplify checks on options to disable zero-copy forwarding
    - BUG/MINOR: quic: reject unknown frame type
    - MINOR: quic: handle all frame types on reception
    - BUG/MINOR: quic: reject HANDSHAKE_DONE as server
    - BUG/MINOR: qpack: reject invalid increment count decoding
    - BUG/MINOR: qpack: reject invalid dynamic table capacity
    - DOC/MINOR: userlists: mention solutions to high cpu with hashes
    - DOC: quic: Missing tuning setting in "Global parameters"
    - BUG/MEDIUM: applet: Immediately free appctx on early error
    - BUG/MEDIUM: hlua: Be able to garbage collect uninitialized lua sockets
    - BUG/MEDIUM: hlua: Don't loop if a lua socket does not consume received data
    - BUG/MEDIUM: quic: fix transient send error with listener socket
    - MINOR: log: custom name for logformat node
    - MINOR: sample: add type_to_smp() helper function
    - MINOR: log: explicit typecasting for logformat nodes
    - MINOR: log: simplify last_isspace in sess_build_logline()
    - MINOR: log: simplify quotes handling in sess_build_logline()
    - MINOR: log: print metadata prefixes separately in sess_build_logline()
    - MINOR: log: automate string array construction in sess_build_logline()
    - DOC: quic: fix recommandation for bind on multiple address
    - MINOR: quic: warn on bind on multiple addresses if no IP_PKTINFO support
    - OPTIM: quic: improve slightly qc_snd_buf() internal
    - MINOR: quic: move IP_PKTINFO on send on a dedicated function
    - MINOR: quic: remove sendto() usage variant
    - MINOR: quic: only use sendmsg() syscall variant
    - BUILD: applet: fix build on some 32-bit archs
    - BUG/MINOR: quic: initialize msg_flags before sendmsg
    - BUG/MEDIUM: mux-h1: Don't emit 0-CRLF chunk in h1_done_ff() when iobuf is empty
    - CLEANUP: proxy/log: remove unused proxy flag
    - CLEANUP: log: fix process_send_log() indentation
    - CLEANUP: log: use free_logformat_list() in parse_logformat_string()
    - MINOR: log: add free_logformat_node() helper function
    - BUG/MINOR: log: fix potential lf->name memory leak
    - BUG/MINOR: ist: allocate nul byte on istdup
    - BUG/MINOR: stats: drop srv refcount on early release
    - BUG/MAJOR: promex: fix crash on deleted server
    - BUG/MAJOR: server: fix stream crash due to deleted server
    - BUG/MEDIUM: mux-quic: do not crash on qcs_destroy for connection error
    - MINOR: cli: Remove useless loop on commands to find unescaped semi-colon
    - BUG/MEDIUM: cli: Warn if pipelined commands are delimited by a \n
    - BUG/MAJOR: cli: Restore non-interactive mode behavior with pipelined commands
    - BUG/MINOR: quic: fix output of show quic
    - MINOR: ssl: Call callback function after loading SSL CRL data
    - BUG/MINOR: ist: only store NUL byte on succeeded alloc
2024-02-23 20:01:45 +01:00
Willy Tarreau
a4d44250eb BUG/MINOR: ist: only store NUL byte on succeeded alloc
The trailing NUL added at the end of istdup() by recent commit de0216758
("BUG/MINOR: ist: allocate nul byte on istdup") was placed outside of
the pointer validity test, rightfully showing null deref warnings. This
fix should be backported along with the fix above, to the same versions.
2024-02-23 19:51:54 +01:00
Miroslav Zagorac
3f771f5118 MINOR: ssl: Call callback function after loading SSL CRL data
Due to the possibility of calling a control process after adding CRLs, the
ssl_commit_crlfile_cb variable was added.  It is actually a pointer to the
callback function, which is called if defined after initial loading of CRL
data from disk and after committing CRL data via CLI command
'commit ssl crl-file ..'.

If the callback function returns an error, then the CLI commit operation
is terminated.

Also, one case was added to the CLI context used by "commit cafile" and
"commit crlfile": CACRL_ST_CRLCB in which the callback function is called.

Signed-off-by: William Lallemand <wlallemand@haproxy.com>
2024-02-23 18:12:27 +01:00
Amaury Denoyelle
ba9f905da9 BUG/MINOR: quic: fix output of show quic
Output of 'show quic' is messed up since the introduction of reordered
packets counter in the following commit. The new counter is mixed up
with the first stream line. This is due to the wrong placement of the
newline delimiter.

  167e38e0e0
  MINOR: quic: Add a counter for reordered packets

This should be backported up to 2.6.
2024-02-23 17:32:24 +01:00
Christopher Faulet
3d93ecc132 BUG/MAJOR: cli: Restore non-interactive mode behavior with pipelined commands
The issue was decribed in commit "BUG/MEDIUM: cli: Warn if pipelined commands
are delimited by a \n". In non-interactive mode, it was possible to use a
newline character as delimiter for pipelined commands. As a consequence, it was
possible to stop commands processing on the middle.

With the above commit, a warning is emitted to notify users. With this one,
we restore the expected behavior, as documented in the management guide.
Only the first line of commands is parsed. This commit will not be
backported to avoid breaking changes on stable versions.

This commit has of course some visible effects. All script using a newline
character as delimiter to pipeline commands in non-interactive mode will
stop working. Only the first command will be evaluated, all others will be
ignored. Pipelined commands MUST now be separated by a semi-colon.

But there is a more subtle and probably more annoying change. It is no
longer possible to pipeline commands with a payload ! A command with a
payload will always be the last one evaluated because it must be finished by
a newline (eventually preceeded by a custom pattern).

It is really annoying to introduce such breaking change. But, on the long
term, it is mandatory. The 2.8 will be the last LST version supporting the
old behavior (with some warning however). This will let 4 years to users to
adapt their scripts.

No backport needed.
2024-02-23 15:19:49 +01:00
Christopher Faulet
598c7f164c BUG/MEDIUM: cli: Warn if pipelined commands are delimited by a \n
This was broken since commit 0011c25144 ("BUG/MINOR: cli: avoid O(bufsize)
parsing cost on pipelined commands"). It is not really a bug fix but it is
labelled as is to make it more visible.

Before, a full line was first retrieved from the request buffer before
extracting the first command to eval it. Now, only one command is retrieved.
But we rely on the request buffer state to interrupt processing in
non-interactive mode. After a command processing, if output of the request
buffer is empty, we leave. Before the above commit, this was not a problem.
But since then, it is obviously a bad statement. First because some input
data may still be there. It is not true today, but it might change. Then,
there is no warranty to receive all commands in same time. For small list of
commands, it will be most of time the case, but it is a dangerous
assumption. For long list of commands, it is almost always false.

To be an issue, commands must be chunked exactly between two commands. But
in this case, remaining commands are skipped. A good way to reproduce the
issue is to wait a bit between two commands, for instance:

    (printf "show info;"; sleep 2; printf "show stat\n") | socat ...

In fact, to properly fix the issue, we should exit on the first command
finished by a newline. Indeed, as stated in the documentation, in
non-interactive mode, a single line is processed. To pipeline commands,
commands must be separated by a semi-colon. Unfortunately, the above commit
introduced another change. It is possible to pipeline commands delimited by
a newline. It was pushed 2 years ago and backported to all stable versions.
Several scripts may rely on this behavior.

So, on stable version, the bug will not be fixed. However a warning will be
emitted to notify users their scripts don't respect the documentation and
they must adapt it. Mainly because the cli behavior on this point will be
changed in 3.0 to stick to the doc. This warning will only be emitted once
over the whole worker process life. Idea is to not flood the logs with the
same warning for every offending commands.

This commit should probably be backported to all stable versions. But with
some cautions because the CLI was often modified.
2024-02-23 15:19:49 +01:00
Christopher Faulet
e018e8a419 MINOR: cli: Remove useless loop on commands to find unescaped semi-colon
This loop was added to detect pipelined commands when only co_getline() was
used to get commands. Now, co_getdelim() is used and the semi-colon is also
considered as a command delimiter.

As side effet, the last semi-colon, if any, is no longer replaced by a
newline. Thus, we must take care to adapt the test to detect partial
commands.
2024-02-23 15:19:49 +01:00
Amaury Denoyelle
73806f0675 BUG/MEDIUM: mux-quic: do not crash on qcs_destroy for connection error
On qcs_destroy(), a BUG_ON() statement check that QCS does not have
anymore prepared data. This is to ensure connection flow control is
always coherent and prevent transfer freeze.

However, this BUG_ON() may cause a spurrious crash in case QCC is
considered on error. Indeed, in this case, all transfers are interrupted
and qmux_strm_detach() will proceed to immediate QCS free before
releasing the connection. In this situation, connection flow control is
irrelevant so the BUG_ON() should be ignored.

This crash occurs since the MUX refactoring via the following patch.
Previously, a similar BUG_ON() was used but it was incorrectly
implemented rendering it immune even to targetted cause.

  3fe3251593
  MEDIUM: mux-quic: simplify sending API

This should fix github issue #2456.

This does not need to be backported.
2024-02-23 11:41:33 +01:00
Amaury Denoyelle
1b8c5abeeb BUG/MAJOR: server: fix stream crash due to deleted server
Before a dynamic server can be deleted, a set of preconditions must be
validated to ensure it is not referenced naymore by a stream or a
connection. This is implemented in srv_check_for_deletion().

The various criteria specified were incomplete. This allows a server
instance to be deleted while still be referenced by a stream and a
connection.

This bug was reproduced by using ASAN compilation. A script was used to
add and delete a server every second, while using h2load to generate
traffic with download of 1k objects. Here is the ASAN error.

==140916==ERROR: AddressSanitizer: heap-use-after-free on address 0x520000020080 at pc 0x63cb25679537 bp 0x701529ff5070 sp 0x701529ff5060
READ of size 1 at 0x520000020080 thread T7
    #0 0x63cb25679536 in objt_server include/haproxy/obj_type.h:99
    #1 0x63cb2568f465 in process_stream src/stream.c:1823
    #2 0x63cb25a4a4a2 in run_tasks_from_lists src/task.c:632
    #3 0x63cb25a4bf62 in process_runnable_tasks src/task.c:876
    #4 0x63cb2596a220 in run_poll_loop src/haproxy.c:3050
    #5 0x63cb2596b192 in run_thread_poll_loop src/haproxy.c:3252
    #6 0x701539aa9559  (/usr/lib/libc.so.6+0x8b559) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #7 0x701539b26a3b  (/usr/lib/libc.so.6+0x108a3b) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

To fix this, add <curr_used_conns> to the counters checked in
srv_check_for_deletion().

Outside of this bug, one case which remains sensible is for SF_DIRECT
streams which referenced a server instance early in process_stream()
before connect_server(). This occurs with use-server directive,
force-persist rule or cookie persistence. However, after code
reexamination, the code is considered reliable as process_stream() is
not rescheduled before connect_server() invocation. These observations
have been saved in sess_change_server() documentation to ensure it
remains valid in the future.

This must be backported up to 2.6.
2024-02-22 18:36:54 +01:00
Amaury Denoyelle
f81c00cd44 BUG/MAJOR: promex: fix crash on deleted server
Promex applet is used to dump many metrics. Some of them are related to
a server instance. The applet can be interrupted in the middle of a
dump, for example waiting for output buffer space. In this case, its
context is save to resume dump on the correct instance.

A crash can occur if dump is interrupted during servers loop. If the
server instance is deleted during two scheduling of the promex applet,
its context will still referenced the deleted server on resume.

To fix this, use server refcount to prevent its deletion during parsing.

No backport is needed, despite all stable releases being affected. This
is because promex applet context has been recently rewritten to use
generic pointers. As such, a specific commit will be applied for earlier
releases.
2024-02-22 18:27:42 +01:00
Amaury Denoyelle
4adf2c9f00 BUG/MINOR: stats: drop srv refcount on early release
Server refcount is used to protect from server deletion while dumping a
server instance, for stats dump on both CLI and HTTP applet. However,
dump can be aborted prematurely before reaching the end. In this case,
server refcount is never decremented.

This bug can cause an inconsistency on servers refcount, preventing them
to be deleted even after "del server" success.

To fix this, implement release handler for both stats CLI and HTTP
applet. Drop server reference if dump was interrupted during servers
loop.

This should be backported up to 2.6.
2024-02-22 18:24:35 +01:00
Amaury Denoyelle
de02167584 BUG/MINOR: ist: allocate nul byte on istdup
istdup() is documented as having the same behavior as strdup(). However,
it may cause confusion as it allocates a block of input length, without
an extra byte for \0 delimiter. This behavior is incoherent as in case
of an empty string however a single \0 is allocated.

This API inconsistency could cause a bug anywhere an IST is used as a
C-string after istdup() invocation. Currently, the only found issue is
with 'wait' CLI command using 'srv-unused'. This causes a buffer
overflow due to ist0() invocation after istdup() for be_name and
sv_name.

Backport should be done to all stable releases. Even if no bug has been
found outside of wait CLI implementation, it ensures the code is more
consistent on every releases.
2024-02-22 18:24:35 +01:00
Aurelien DARRAGON
2462e5bcca BUG/MINOR: log: fix potential lf->name memory leak
Recent commit 2ed6068 ("MINOR: log: custom name for logformat node")
introduced a potential memory leak because when custom name is provided,
lf->name value is allocated using strdup(), thus is expected to be freed
alongside the node when the node is released.

However lf->name was only freed in some common places within log.c
cleanups and helpers func, but in reality there are still cases where
lf nodes are manually freed without making use of freeing helpers.

So this is what this patch does, it makes sure all lf freeing places now
leverage the free_logformat_node() helper function that takes care of
freeing all known allocated elements within the node, including custom
name.

This commit depends on:
 - "MINOR: log: add free_logformat_node() helper function"

No backport needed unless 2ed6068 gets backported.
2024-02-22 15:32:42 +01:00
Aurelien DARRAGON
1c2e16ba8a MINOR: log: add free_logformat_node() helper function
Function may be used to free a single logformat node.
2024-02-22 15:32:42 +01:00
Aurelien DARRAGON
62121d5b90 CLEANUP: log: use free_logformat_list() in parse_logformat_string()
This is a follow up for 24a5e42db6 ("CLEANUP: log: deinitialization of
the log buffer in one function") as there was another opportunity to
make use of the new cleanup function.
2024-02-22 15:32:42 +01:00
Aurelien DARRAGON
e7aee6edd5 CLEANUP: log: fix process_send_log() indentation
Fix bad indentation for process_send_log() prototype (tab was used instead
of spaces)
2024-02-22 15:32:42 +01:00
Aurelien DARRAGON
891bac673b CLEANUP: proxy/log: remove unused proxy flag
Since 3d6350e10 ("MINOR: log: Remove log-error-via-logformat option"),
PR_O_ERR_LOGFMT flag is not used anymore, but it was left in the proxy-t.h
header file. Simply removing it and adding a comment to indicate that the
corresponding bit is now unused.
2024-02-22 15:32:42 +01:00
Christopher Faulet
1a2a196fcf BUG/MEDIUM: mux-h1: Don't emit 0-CRLF chunk in h1_done_ff() when iobuf is empty
A chunk message transferred via zero-copy forwarding in H1 may be
corrupted. This only happens when the chunk size is not known during the
nego stage and when there is nothing to forward when h1_donn_ff() is
called. In this case, we always emit a chunk. Because there is nothing to
forward, a 0-CRLF is emitted in the middle of the message.

The issue occurred with the HTTP stats applet only.

A simple fix is to check the size of data in the iobuf before emitting a new
chunk in h1_done_ff(). However, we still try to send outgoing data because
when this happens, it is most of time because the H1 output buffer is almost
full.

This patch should fix the issue #2453. No backport needed.
2024-02-21 11:49:58 +01:00
Amaury Denoyelle
a17eaf7763 BUG/MINOR: quic: initialize msg_flags before sendmsg
Previously, msghdr struct used for sendmsg was memset to 0. This was
updated for performance reason with each members individually defined.
This is done by the following commit :

  commit 107d6d7546
  OPTIM: quic: improve slightly qc_snd_buf() internal

msg_flags is the only member unset, as sendmsg manual page reports that
it is unused. However, this caused a coverity report. In the end, it is
better to explicitely set it to 0 to avoid any future interrogations,
compiler warning or even portability issues.

This should fix coverity report from github issue #2455.

No need to backport unless above patch is.
2024-02-21 10:13:53 +01:00
Willy Tarreau
9d572952a2 BUILD: applet: fix build on some 32-bit archs
The to_forward field was added to debugging output of applets with commit
62a81cb6a ("MINOR: applet: Add callback function to deal with zero-copy
forwarding"), though it's a size_t printed as %lu, which causes complaints
on 32-bit archs. Let's just cast as %lu.

No backport is needed.
2024-02-21 04:18:32 +01:00
Amaury Denoyelle
8b950f40fa MINOR: quic: only use sendmsg() syscall variant
This patch is the direct followup of the previous one :
  MINOR: quic: remove sendto() usage variant

This finalizes qc_snd_buf() simplification by removing send() syscall
usage for quic-conn owned socket. Syscall invocation is merged in a
single code location to the sendmsg() variant.

The only difference for owned socket is that destination address for
sendmsg() is set to NULL. This usage is documented in man 2 sendmsg as
valid for connected sockets. This allows maximum performance by avoiding
unnecessary lookups on kernel socket address tables.

As the previous patch, no functional change should happen here. However,
it will be simpler to extend qc_snd_buf() for GSO usage.
2024-02-20 16:42:05 +01:00
Amaury Denoyelle
8de9f8f193 MINOR: quic: remove sendto() usage variant
qc_snd_buf() is a wrapper around emission syscalls. Given QUIC
configuration, a different variant is used. When using connection
socket, send() is the only used. For listener sockets, sendmsg() and
sendto() are possible. The first one is used only if local address has
been retrieved prior. This allows to fix it on sending to guarantee the
source address selection. Finally, sendto() is used for systems which do
not support local address retrieval.

All of these variants render the code too complex. As such, this patch
simplifies this by removing sendto() alternative. Now, sendmsg() is
always used for listener sockets. Source address is then specified only
if supported by the system.

This patch should not exhibit functional behavior changes. It will be
useful when implementing GSO as the code is now simpler.
2024-02-20 16:42:05 +01:00
Amaury Denoyelle
ea90c39302 MINOR: quic: move IP_PKTINFO on send on a dedicated function
When using listener socket, source address for emission is explicitely
set using ancillary data for sendmsg(). This is useful to guarantee the
correct address is used when binding on a non-explicit address.

This code was implemented directly under qc_snd_buf(). However, it is
quite complex due to portability issue. For IPv4, two parallel
implementations coexist, defined under IP_PKTINFO or IP_RECVDSTADDR. For
IPv6, another option is defined under IPV6_RECVPKTINFO. Each variant
uses its distinct name which increase the code complexity.

Extract ancillary data filling in a dedicated function named
cmsg_set_saddr(). This reduces greatly the body of qc_snd_buf(). Such
functions can be replicated when other ancillary data type will be
implemented. This will notably be useful for GSO implementation.
2024-02-20 16:42:05 +01:00
Amaury Denoyelle
107d6d7546 OPTIM: quic: improve slightly qc_snd_buf() internal
qc_snd_buf() is a wrapper for sendmsg() syscall (or its derivatives)
used for all QUIC emissions. This patch aims at removing several
non-optimal code sections :

* fd_send_ready() for connected sockets is only checked on the function
  preambule instead of inside the emission loop

* zero-ing msghdr structure for unconnected sockets is removed. This is
  unnecessary as all fields are properly initialized then.

* extra memcpy/memset invocations when using IP_PKTINFO/IPV6_RECVPKTINFO
  are removed by setting directly the address value into cmsg buffer
2024-02-20 16:42:05 +01:00
Amaury Denoyelle
9b806550b7 MINOR: quic: warn on bind on multiple addresses if no IP_PKTINFO support
Binding on multiple addresses for QUIC is safe only if IP_PKTINFO or
equivalent is available. Else, the behavior may be undefined as the
system is responsible to choose the network interface and source address
on response.

This commit adds a warning on boot if no or partial support for
IP_PKTINFO or equivalent is detected and configuration contains UDP
binding on multiple addresses.

This should be backported up to 2.6. Special backport recommdations :
* change ha_warning() to ha_diag_warning() to ensure no spurrious
  warnings will be triggered on stable releases
* IP_PKTINFO usage was introduced on 2.7. For 2.6, multiple addresses
  QUIC binding is always unreliable. As such, preprocessor condition
  must simply be removed so that the warning is always active regarding
  of the system. Warning message should also be truncated to suppress
  IP_PKTINFO reference.
2024-02-20 16:40:14 +01:00
Amaury Denoyelle
f01ae9f075 DOC: quic: fix recommandation for bind on multiple address
Documentation falsely mentions that binding on multiple addresses is
forbidden for QUIC listeners. This is not the case. Moreover, this
behavior is reliable when using destination address retrieval on receive
via IP_PKTINFO, which allows to determine the proper source address for
response.

This should be backported up to 2.7. On 2.6 specific source address
definition on sendmsg via IP_PKTINFO is not implemented. As such, bind
on multiple addresses should remain forbidden for this release.
2024-02-20 16:10:21 +01:00
Aurelien DARRAGON
ee88c4418f MINOR: log: automate string array construction in sess_build_logline()
make it so string array construction is performed by dedicated macro
helpers instead of manual char insertion between string members.

The goal is to easily be able to support multiple forms of array
construction depending on the data encoding format (raw, json..).

Only %hrl and %hsl logformats are concerned.
2024-02-20 15:49:55 +01:00
Aurelien DARRAGON
8d2b9e2acd MINOR: log: print metadata prefixes separately in sess_build_logline()
Some log variables may be prefixed with specific chars that represent
extra informations that are relevant with it but are are not directly
part of the "raw" value.

ie: '+' char is prepended before some values when "option logasap" is
used to indicate that the value has not yet reached its final value.

However, as those "metadata" are printed using the general purpose
LOGCHAR() printing helper, it's not easy to tell if they are part of the
base value or not.

In this patch we add the LOGMETACHAR() helper that is a wrapper for
LOGCHAR(). The goal is to prepare for adding some logic to prevent such
additional infos from being generated when not relevant or needed.
2024-02-20 15:49:55 +01:00
Aurelien DARRAGON
a2fc40bc28 MINOR: log: simplify quotes handling in sess_build_logline()
quotes building for some log formats is directly performed under each
switch case statement so it would become painful to add other conditions
to prevent the quotes from being generated when it's not supported by the
the data encoding format for instance (ie: JSON).

Let's centralize and simplify quotes handling by adding LOGQUOTE_START()
and LOGQUOTE_END() helper macros. If a quotation is started and not
explicitly ended, it will be automatically ended at the end of the current
logformat node:

LOGQUOTE_START() sets 'quote' variable to 1, this way LOGQUOTE_END() only
prints the ending quote when needed. LOGQUOTE_END() is systematically
called after each node switch-case (after each value). LOGQUOTE_START()
does nothing if LOG_OPT_QUOTE isn't set, so does LOGQUOTE_END().

Some rare cases such as %hsl (list of captured headers) required special
handling: in this case multiple quoted texts are generated for the same
field value so explicit LOGQUOTE_START() + LOGQUOTE_END() combination was
needed.
2024-02-20 15:49:55 +01:00
Aurelien DARRAGON
c6a7138420 MINOR: log: simplify last_isspace in sess_build_logline()
last_isspace variable is explicitly set to 0 in all cases except
LOG_FMT_SEPARATOR case. So we can actually simplify the code by setting
last_isspace to 0 by default and skipping the assignment for the
LOG_FMT_SEPARATOR case.
2024-02-20 15:49:55 +01:00
Aurelien DARRAGON
1448478d62 MINOR: log: explicit typecasting for logformat nodes
Add the ability to manually specify desired output type after a custom
field name for logformat nodes. Forcing the type can be useful to ensure
value is stored with the proper type representation. (i.e.: forcing
numerical to string to work around the limited resolution of JS number
types)

By default, type is set to SMP_T_SAME, which means the original type will
be preserved.

Currently supported types are: bool, str, sint
2024-02-20 15:49:54 +01:00