When integrating the number of warnings in "show info" in 2.8 with commit
3c4a297d2 ("MINOR: stats: report the total number of warnings issued"),
the update of the trash buffer used by the Tainted flag got displaced
lower. There's no harm for now util someone adds a new metric requiring
a call to chunk_newstr() and gets both values merged. Let's move the
call to its location now.
In process_chk_conn(), some assignments to NULL are useless and are reported
by Coverity as unused value. while it is harmless, these assignments can be
removed.
This patch should fix the coverity report #2158.
When server is transitionning from UP to DOWN, a log message is generated.
e.g.: "Server backend_name/server_name is DOWN")
However since f71e064 ("MEDIUM: server: split srv_update_status() in two
functions"), the allocated buffer tmptrash which is used to prepare the
log message is not freed after it has been used, resulting in a small
memory leak each time a server goes DOWN because of an operational
change.
This is a 2.8 specific bug, no backport needed unless the above commit
gets backported.
Within srv_update_status subfunctions _op() and _adm(), each time tmptrash
is freed, we assign it to NULL to ensure it will not be reused.
However, within those functions it is not very useful given that tmptrash
is never checked against NULL except upon allocation through
alloc_trash_chunk(), which happens everytime a new log message is
generated, sent, and then freed right away, so there are no code paths
that could lead to tmptrash being checked for reuse (tmptrash is
systematically overwritten since all log messages are independant from
each other).
This was raised by coverity, see GH #2162.
Because of the previous fix, log/last_rule.vtc script is failing. The
inspect-delay is no longer shorten when the end of the message is
reached. Thus WAIT_END acl is trully respected. 10s is too high and hit the
Vtext timeout, making the script fails.
A regression was introduced with the commit cb59e0bc3 ("BUG/MINOR:
tcp-rules: Stop content rules eval on read error and end-of-input"). We
should not shorten the inspect-delay when the EOI flag is set on the SC.
Idea of the inspect-delay is to wait a TCP rule is matching. It is only
interrupted if an error occurs, on abort or if the peer shuts down. It is
also interrupted if the buffer is full. This last case is a bit ambiguous
and discutable. It could be good to add ACLS, like "wait_complete" and
"wait_full" to do so. But for now, we only remove the test on SC_FL_EOI
flag.
This patch must be backported to all stable versions.
This makes use of spread-checks also for the startup of the check tasks.
This provides a smoother load on startup for uneven configurations which
tend to enable only *some* servers. Below is the connection distribution
per second of the SSL checks of a config with 5k servers spread over 800
backends, with a check inter of 5 seconds:
- default:
682 08:00:50
826 08:00:51
773 08:00:52
1016 08:00:53
885 08:00:54
889 08:00:55
825 08:00:56
773 08:00:57
1016 08:00:58
884 08:00:59
888 08:01:00
491 08:01:01
- with spread-checks 50:
437 08:01:19
866 08:01:20
777 08:01:21
1023 08:01:22
1118 08:01:23
923 08:01:24
641 08:01:25
859 08:01:26
962 08:01:27
860 08:01:28
929 08:01:29
909 08:01:30
866 08:01:31
849 08:01:32
114 08:01:33
- with spread-checks 50 + this patch:
680 08:01:55
922 08:01:56
962 08:01:57
899 08:01:58
819 08:01:59
843 08:02:00
916 08:02:01
896 08:02:02
886 08:02:03
846 08:02:04
903 08:02:05
894 08:02:06
178 08:02:07
The load is much smoother from the start, this can help initial health
checks succeed when many target the same overloaded server for example.
This could be backported as it should make border-line configs more
reliable across reloads.
When a full message is received for a stream, MUX is responsible to set
EOI flag. This was done through rcv_buf stream callback by checking if
QCS HTX buffer contained the EOM flag.
This is not correct for HTTP without body. In this case, QCS HTX buffer
is never used. Only a local HTX buffer is used to transfer headers just
as stream endpoint is created. As such, EOI is never transmitted to the
upper layer.
If the transfer occur without any issue, this does not seem to cause any
problem. However, in case the transfer is aborted, the stream is never
released which cause a memory leak and prevent the process soft-stop.
To fix this, also check if EOM is put by application layer during
headers conversion. If true, this is transferred through a new argument
to qc_attach_sc() MUX function which is responsible to set the EOI flag.
This issue was reproduced using h2load with hundred of connections.
h2load is interrupted with a SIGINT which causes streams to never be
closed on haproxy side.
This should be backported up to 2.6.
Uninline and move qc_attach_sc() function to implementation source file.
This will be useful for next commit to add traces in it.
This should be backported up to 2.7.
MUX is responsible to put EOS on stream when read channel is closed.
This happens if underlying connection is closed or a RESET_STREAM is
received. FIN STREAM is ignored in this case.
For connection closure, simply check for CO_FL_SOCK_RD_SH.
For RESET_STREAM reception, a new flag QC_CF_RECV_RESET has been
introduced. It is set when RESET_STREAM is received, unless we already
received all data. This is conform to QUIC RFC which allows to ignore a
RESET_STREAM in this case. During RESET_STREAM processing, input buffer
is emptied so EOS can be reported right away on recv_buf operation.
This should be backported up to 2.7.
Add traces to render each stream transition more explicit. Also, move
ERR_PENDING to ERROR transition after other stream flags are set, as
with the MUX H2 implementation. This is purely a cosmetic change and it
should have no functional impact.
This should be backported up to 2.7.
The issue was introduced with commit c108f37c2 ("BUILD: makefile:
rework 51D to split v3/v4"), and is also related to commit b16d9b58
("BUILD: makefile: never force -latomic, set USE_LIBATOMIC instead")
where USE_ATOMIC has been replaced.
The following patch introduced proper error management on buffer
allocation failure :
0abde9dee6
BUG/MINOR: mux-quic: properly handle buf alloc failure
However, when decoding an empty STREAM frame with just FIN bit set, this
was not done correctly. Indeed, there is a missing goto statement in
case of a NULL buffer check.
This was reported thanks to coverity analysis. This should fix github
issue #2163.
This must be backported up to 2.6.
Since the following patch
commit 6c501ed23b
BUG/MINOR: mux-quic: differentiate failure on qc_stream_desc alloc
it is not possible to check if Tx buf allocation failed due to a
configured limit exhaustion or a simple memory failure.
This patch fixes it as the condition was inverted. Indeed, if buf_avail
is null, this means that the limit has been reached. On the contrary
case, this is a real memory alloc failure. This caused the flag
QC_CF_CONN_FULL to not be properly used and may have caused disruption
on transfer with several streams or large data.
This was detected due to an abnormal error QUIC MUX traces. Also change
in consequence trace for limit exhaustion to be more explicit.
This must be backported up to 2.6.
Fix the openssl build with older openssl version by disabling the new
ssl_c_r_dn fetch.
This also disable the ssl_client_samples.vtc file for OpenSSL version
older than 1.1.1
Christopher found as part of the analysis of Tim's issue #1891 that commit
15a4733d5 ("BUG/MEDIUM: mux-h2: make use of http-request and keep-alive
timeouts") introduced in 2.6 incompletely addressed a timeout issue in the
H2 mux. The problem was that the http-keepalive and http-request timeouts
were not applied before it. With that commit they are now considered, but
if a GOAWAY is sent (or even attempted to be sent), then they are not used
anymore again, because the way the code is arranged consists in applying
the client-fin timeout (if set) to the current date, and falling back to
the client timeout, without considering the idle_start period. This means
that a config having a "timeout http-keepalive" would still not close the
connection quickly when facing a client that periodically sends PING,
PRIORITY or whatever other frame types.
In addition, after the GOAWAY was attempted to be sent, there was no check
for pending data in the output buffer, meaning that it would be possible
to truncate some responses in configs involving a very short client-fin
timeout.
Finally the spreading of the closures during the soft-stop brought in 2.6
by commit b5d968d9b ("MEDIUM: global: Add a "close-spread-time" option to
spread soft-stop on time window") didn't consider the particular case of
an idle "pre-connect" connection, which would also live long if a browser
failed to deliver a valid request for a long time.
All of this indicates that the conditions must be reworked so as not to
have that level of exclusion between conditions, but rather stick to the
rules from the doc that are already enforced on other muxes:
- timeout client always applies if there are data pending, and is
relative to each new I/O ;
- timeout http-request applies before the first complete request and
is relative to the entry in idle state ;
- timeout http-keepalive applies between idle and the next complete
request and is relative to the entry in idle state ;
- timeout client-fin applies when in idle after a shut was sent (here
the shut is the GOAWAY). The shut may only be considered as sent if
the buffer is empty and the flags indicate that it was successfully
sent (or failed) but not if it's still waiting for some room in the
output buffer for example. This implies that this timeout may then
lower the http-keepalive/http-request ones.
This is what this patch implements. Of course the client timeout still
applies as a fallback when all the ones above are not set or when their
conditions are not met.
It would seem reasoanble to backport this to 2.7 first, then only after
one or two releases to 2.6.
This patch addresses #1514, adds the ability to fetch DN of the root
ca that was in the chain when client certificate was verified during SSL
handshake.
The HTTPCLIENT and the OCSP-UPDATE proxies are internal proxies, we
don't need to display logs of them stopping during the stopping of the
process.
This patch checks if a proxy has the flag PR_CAP_INT so it doesn't
display annoying messages.
When the SC is detached from the endpoint, the xref between the endpoints is
removed. At this stage, the sedesc cannot be undefined. So we can remove the
test on it.
This issue should fix the issue #2156. No backport needed.
In the proxy CLI analyzer, when pcli_parse_request returns -1, the
client was shut to prevent any problem with the master CLI.
This behavior is a little bit excessive and not handy at all in prompt
mode. For example one could have activated multiples mode, then have an
error which disconnect the CLI, and they would have to reconnect and
enter all the modes again.
This patch introduces the pcli_error() function, which only output an
error and flush the input buffer, instead of closing everything.
When encountering a parsing error, this function is used, and the prompt
is written again, without any disconnection.
SSL hanshake error were unable to dump the OpenSSL error string by
default, to do so it was mandatory to configure a error-log-format with
the ssl_fc_err fetch.
This patch implements the session_build_err_string() function which creates
the error log to send during session_kill_embryonic(), a special case is
made with CO_ER_SSL_HANDSHAKE which is able to dump the error string
with ERR_error_string().
Before:
<134>May 12 17:14:04 haproxy[183151]: 127.0.0.1:49346 [12/May/2023:17:14:04.571] frt2/1: SSL handshake failure
After:
<134>May 12 17:14:04 haproxy[183151]: 127.0.0.1:49346 [12/May/2023:17:14:04.571] frt2/1: SSL handshake failure (error:0A000418:SSL routines::tlsv1 alert unknown ca)
qc_init() is used to initialize a QUIC MUX instance. On failure, each
resources are released via a series of goto statements. There is one
issue if the app_ops.init callback fails. In this case, MUX task is not
freed.
This can cause a crash as the task is already scheduled. When the
handler will run, it will crash when trying to access qcc instance.
To fix this, properly destroy qcc task on fail_install_app_ops label.
The impact of this bug is minor as app_ops.init callback succeeds most
of the time. However, it may fail on allocation failure due to memory
exhaustion.
This may fix github issue #2154.
This must be backported up to 2.7.
qc_stream_buf_alloc() can fail for two reasons :
* limit of Tx buffer per connection reached
* allocation failure
The first case is properly treated. A flag QC_CF_CONN_FULL is set on the
connection to interrupt emission. It is cleared when a buffer became
available after in order ACK reception and the MUX tasklet is woken up.
The allocation failure was handled with the same mechanism which in this
case is not appropriate and could lead to a connection transfer freeze.
Instead, prefer to close the connection with a QUIC internal error code.
To differentiate the two causes, qc_stream_buf_alloc() API was changed
to return the number of available buffers to the caller.
This must be backported up to 2.6.
The total number of buffer per connection for sending is limited by a
configuration value. To ensure this, <stream_buf_count> quic_conn field
is incremented on qc_stream_buf_alloc().
qc_stream_buf_alloc() may fail if the buffer cannot be allocated. In
this case, <stream_buf_count> should not be incremented. To fix this,
simply move increment operation after buffer allocation.
The impact of this bug is low. However, if a connection suffers from
several buffer allocation failure, it may cause the <stream_buf_count>
to be incremented over the limit without being able to go back down.
This must be backported up to 2.6.
The function qc_get_ncbuf() is used to allocate a ncbuf content.
Allocation failure was handled using a plain BUG_ON.
Fix this by a proper error management. This buffer is only used for
STREAM frame reception to support out-of-order offsets. When an
allocation failed, close the connection with a QUIC internal error code.
This should be backported up to 2.6.
A convenience function qc_get_buf() is implemented to centralize buffer
allocation on MUX and H3 layers. However, allocation failure was not
handled properly with a BUG_ON() statement.
Replace this by proper error management. On emission, streams is
temporarily skip over until the next qc_send() invocation. On reception,
H3 uses this function for HTX conversion; on alloc failure the
connection will be closed with QUIC internal error code.
This must be backported up to 2.6.
Remove QUIC MUX function qcs_http_handle_standalone_fin(). The purpose
of this function was only used when receiving an empty STREAM frame with
FIN bit. Besides, it was called by each application protocol which could
have different approach and render the function purpose unclear.
Invocation of qcs_http_handle_standalone_fin() have been replaced by
explicit code in both H3 and HTTP/0.9 module. In the process, use
htx_set_eom() to reliably put EOM on the HTX message.
This should be backported up to 2.7, along with the previous patch which
introduced htx_set_eom().
Implement a new HTX utility function htx_set_eom(). If the HTX message
is empty, it will first add a dummy EOT block. This is a small trick
needed to ensure readers will detect the HTX buffer as not empty and
retrieve the EOM flag.
Replace the H2 code related by a htx_set_eom() invocation. QUIC also has
the same code which will be replaced in the next commit.
This should be backported up to 2.7 before the related QUIC patch.
It is possible to receive datagram from other connection on a dedicated
quic-conn socket. This is due to a race condition between bind() and
connect() system calls.
To handle this, an explicit check is done on each datagram. If the DCID
is not associated to the connection which owns the socket, the datagram
is redispatch as if it arrived on the listener socket.
This redispatch step was not properly done because the source address
specified for the redispatch function was incorrect. Instead of using
the datagram source address, we used the address of the socket
quic-conn which received the datagram due to the above race condition.
Fix this simply by using the address from the recvmsg() system call.
The impact of this bug is minor as redispatch on connection socket
should be really rare. However, when it happens it can lead to several
kinds of problems, like for example a connection initialized with an
incorrect peer address. It can also break the Retry token check as this
relies on the peer address.
In fact, Retry token check failure was the reason this bug was found.
When using h2load with thousands of clients, the counter of Retry token
failure was unusually high. With this patch, no failure is reported
anymore for Retry.
Must be backported to 2.7.
A check was missing in parse_logsrv() to make sure that malloc-dependent
variable is checked for non-NULL before using it.
If malloc fails, the function raises an error and stops, like it's already
done at a few other places within the function.
This partially fixes GH #2130.
It should be backported to every stable versions.
usermsgs_buf.size is set without first checking if previous malloc
attempt succeeded.
This could fool the buffer API into assuming that the buffer is
initialized, resulting in unsafe read/writes.
Guarding usermsgs_buf.size assignment with the malloc attempt result
to make the buffer initialization safe against malloc failures.
This partially fixes GH #2130.
It should be backported up to 2.6.
Christopher reported a rare race condition involving 'healthcheckmail.vtc'
The regtest would randomly FAIL with this kind of error:
** S1 === expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server b...
**** S1 EXPECT MATCH ~ "[^:\[ ]\[581669\]: Health check for server be1/srv1 failed.+check duration: [[:digit:]]+ms.+status: 0/1 DOWN."
** S1 === recv info
**** S1 syslog|<25>May 11 15:38:46 haproxy[581669]: Server be1/srv1 is DOWN. 0 active and 0 backup servers left. 0 sessions active, 0 requeued, 0 remaining in queue.
**** S1 syslog|<24>May 11 15:38:46 haproxy[581669]: backend be1 has no server available!
It turns out that this it due to the recent commit 7963fb5 ("REGTESTS: use
lua mailer script for mailers tests") in which we tell the regtest to use
the new lua mailers instead of the legacy mailers API.
However, in the lua mailers script, due to the event subscriptions being
performed from a lua task, it is possible that the subscription may be
delayed during startup. Indeed lua tasks relie on the scheduler which runs
tasks with no ordering guarantees. Thus early tasks, including server
checks which are used in the regtest are competing during startup.
As such, we may end up with some events that are generated right before
the lua mailers script starts subscribing to events (because the lua task
is scheduled but started yet), resulting in events loss from lua point of
view.
To fix this and to make lua mailers more reliable during startup, we now
perform the events subscription from an init function instead of an
asynchronous task. (The init function is called synchronously during
haproxy post_init, and exclusively runs before the scheduler starts)
This should be enough to prevent healthcheckmail.vtc from randomly failing
Despite the doc not mentionning it, core.{proxies,frontends,backends}
methods are also available from init context.
(through core.register_init() functions)
Updating the documentation to reflect this possibility.
Some malloc resulsts were not checked in standalone ncbuf code.
As this is debug/test code, we don't need to explicitly handle memory
errors, we just add some BUG_ON() to ensure that memory is properly
allocated and prevent unexpected results.
This partially fixes issue GH #2130.
No backport needed.
Commit 986798718 ("DEBUG: cli: add "debug dev task" to show/wake/expire/kill
tasks and tasklets") caused a build failure on 32-bit platforms when parsing
the task's pointer. Let's use strtoul() and not strtoll(). No backport is
needed, unless the commit above gets backported.
httpclient.resolvers.disabled allow to disable completely the resolvers
of the httpclient, prevents the creation of the "default" resolvers
section, and does not insert the http do-resolve rule in the proxies.
Now we can detect the listener associated with a QUIC listener and report
a bit more info (e.g. listening port and frontend name), and provide a bit
more info about connections as well, and filter on both front connections
and listeners using the "l" and "f" flags.
This provides more consistency between the master and the worker. When
"prompt timed" is passed on the master, the timed mode is toggled. When
enabled, for a master it will show the master process' uptime, and for
a worker it will show this worker's uptime. Example:
master> prompt timed
[0:00:00:50] master> show proc
#<PID> <type> <reloads> <uptime> <version>
11940 master 1 [failed: 0] 0d00h02m10s 2.8-dev11-474c14-21
# workers
11955 worker 0 0d00h00m59s 2.8-dev11-474c14-21
# old workers
11942 worker 1 0d00h02m10s 2.8-dev11-474c14-21
# programs
[0:00:00:58] master> @!11955
[0:00:01:03] 11955> @!11942
[0:00:02:17] 11942> @
[0:00:01:10] master>
Entering "prompt timed" toggles reporting of the process' uptime in
the prompt, which will report days, hours, minutes and seconds since
it was started. As discussed with Tim in issue #2145, this can be
convenient to roughly estimate the time between two outputs, as well
as detecting that a process failed to be reloaded for example.
There's something very irritating on the CLI, when just pressing ENTER,
it complains "Unknown command: ''..." and dumps all the help. This
action is often done to add a bit of clearance after a dump to visually
find delimitors later, but this stupid error makes it unusable. This
patch addresses this by just returning on empty command instead of trying
to look up a matching keyword. It will result in an empty line to mark
the end of the empty command and a prompt again.
It's probably not worth backporting this given that nobody seems to have
complained about it yet.
Now that we have free_acl_cond(cond) function that does cond prune then
frees cond, replace all occurences of this pattern:
| prune_acl_cond(cond)
| free(cond)
with:
| free_acl_cond(cond)
http_parse_redirect_rule() doesn't perform enough checks around NULL
returning allocating functions.
Moreover, existing error paths don't perform cleanups. This could lead to
memory leaks.
Adding a few checks and a cleanup path to ensure memory errors are
properly handled and that no memory leaks occurs within the function
(already allocated structures are freed on error path).
It should partially fix GH #2130.
This patch depends on ("MINOR: proxy: add http_free_redirect_rule() function")
This could be backported up to 2.4. The patch is also relevant for
2.2 but "MINOR: proxy: add http_free_redirect_rule() function" would
need to be adapted first.
==
Backport notes:
-> For 2.2 only:
Replace:
(strcmp(args[cur_arg], "drop-query") == 0)
with:
(!strcmp(args[cur_arg],"drop-query"))
-> For 2.2 and 2.4:
Replace:
"expects 'code', 'prefix', 'location', 'scheme', 'set-cookie', 'clear-cookie', 'drop-query', 'ignore-empty' or 'append-slash' (was '%s')",
with:
"expects 'code', 'prefix', 'location', 'scheme', 'set-cookie', 'clear-cookie', 'drop-query' or 'append-slash' (was '%s')",
Adding http_free_redirect_rule() function to free a single redirect rule
since it may be required to free rules outside of free_proxy() function.
This patch is required for an upcoming bugfix.
[for 2.2, free_proxy function did not exist (first seen in 2.4), thus
http_free_redirect_rule() needs to be deducted from haproxy.c deinit()
function if the patch is required]
cookie_str from struct redirect, which may be allocated through
http_parse_redirect_rule() function is not properly freed on proxy
cleanup within free_proxy().
This could be backported to all stable versions.
[for 2.2, free_proxy() did not exist so the fix needs to be performed
directly in deinit() function from haproxy.c]
A xref is added between the endpoint descriptors. It is created when the
server endpoint is attached to the SC and it is destroyed when an endpoint
is detached.
This xref is not used for now. But it will be useful to retrieve info about
an endpoint for the opposite side. It is also the warranty there is still a
endpoint attached on the other side.
