Christopher Faulet [Tue, 4 Jan 2022 09:56:03 +0000 (10:56 +0100)]
BUG/MEDIUM: http-ana: Preserve response's FLT_END analyser on L7 retry
When a filter is attached on a stream, the FLT_END analyser must not be
removed from the response channel on L7 retry. It is especially important
because CF_FLT_ANALYZE flag is still set. This means the synchronization
between the two sides when the filter ends can be blocked. Depending on the
timing, this can freeze the stream infinitely or lead to a spinning loop.
Note that the synchronization between the two sides at the end of the
analysis was introduced because the stream was reused in HTTP between two
transactions. But, since the HTX was introduced, a new stream is created for
each transaction. So it is probably possible to remove this step for 2.2 and
higher.
This patch must be backported as far as 2.0.
(cherry picked from commit
7bf46bb9a972c1e9de50b31ce20811f2f59a6849)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Mon, 3 Jan 2022 18:43:41 +0000 (19:43 +0100)]
BUG/MINOR: cli: fix _getsocks with musl libc
In ticket #1413, the transfer of FDs couldn't correctly work on alpine
linux. After a few tests with musl on another distribution it seems to
be a limitation of this libc.
The number of FD that could be sent per sendmsg was set to 253, which
does not seem to work with musl, decreasing it 252 seems to work
better, so lets set this value everywhere since it does not have that
much impact.
This must be backported in every maintained version.
(cherry picked from commit
148d7a030156ca927e744013dd6fc32b473c1ef6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
David Carlier [Fri, 31 Dec 2021 08:15:29 +0000 (08:15 +0000)]
BUILD/MINOR: tools: solaris build fix on dladdr.
dladdr takes a mutable address on this platform.
(cherry picked from commit
ae5c42f4d0ce74af7554b9a94c8a3c43286efc35)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Sat, 25 Dec 2021 09:01:52 +0000 (14:01 +0500)]
CI: github actions: update OpenSSL to 3.0.1
OpenSSL-3.0.1 was released on 14 Dec 2021, let's switch to it
(cherry picked from commit
874c907a2e5ffae50599b1f368e0488d8eb2fe4f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
David CARLIER [Fri, 31 Dec 2021 05:00:12 +0000 (05:00 +0000)]
BUILD/MINOR: cpuset FreeBSD 14 build fix.
The 14th release started to introduce api compatibility layer with Linux
for the cpuset part and doing so irrevocably change the CPU* macros as well.
(cherry picked from commit
f64504716843f69a9b10582b2d9cce6a7919795a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Thu, 30 Dec 2021 13:57:32 +0000 (14:57 +0100)]
REGTESTS: ssl: update of a crt with server deletion
This test verifies that a certificate is in a "Unused" state once every
server which uses it are dynamically removed.
(cherry picked from commit
acd546b07cd68537d173dde38342d86fadd706d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Thu, 30 Dec 2021 13:45:19 +0000 (14:45 +0100)]
BUG/MEDIUM: ssl: free the ckch instance linked to a server
This patch unlinks and frees the ckch instance linked to a server during
the free of this server.
This could have locked certificates in a "Used" state when removing
servers dynamically from the CLI. And could provoke a segfault once we
try to dynamically update the certificate after that.
This must be backported as far as 2.4.
(cherry picked from commit
e69563fd8ed1a492ae4547451935908c0802e2c4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Thu, 30 Dec 2021 10:25:43 +0000 (11:25 +0100)]
BUG/MINOR: ssl: free the fields in srv->ssl_ctx
A lot of free are missing in ssl_sock_free_srv_ctx(), this could result
in memory leaking when removing dynamically a server via the CLI.
This must be backported in every branches, by removing the fields that
does not exist in the previous branches.
(cherry picked from commit
231610ad9ccc2470930f7a728ba710a548677a65)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Sat, 25 Dec 2021 08:53:04 +0000 (13:53 +0500)]
CI: Github Actions: do not show VTest failures if build failed
this is mostly cleanup, issue is minor. If build failed, VTest execution
tried to be performed as well as VTest result show. This change ignores
those steps if build failed.
(cherry picked from commit
2ef4c7c84363f5a9b80a2093df1370514319db28)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 7 Jan 2022 13:51:56 +0000 (14:51 +0100)]
BUILD: makefile: add -Wno-atomic-alignment to work around clang abusive warning
As reported in github issue #1502, clang, when building for i386, will
try to use CMPXCHG8B-based loops for 64-bit atomic operations, and emits
warnings for all 64-bit operands that are not 64-bit aligned, an alignment
that is *not* required by the ABI, that the compiler itself does not
enforce, and that the intel SDM clearly says is not required on this
32-bit platform for this operation. But this is likely an excessive
outcome of the same code being used in 64-bit for CMPXCHG16B which does
require proper alignment. Firefox already gave up on this one 3 years
ago, let's not waste our time arguing and just shut up the warning
instead. It might hide some real bugs in the future but till now
experience showed that overall it's unlikely.
This should be backported to all maintained branches that use 64-bit
atomic ops (e.g. for counters).
Thanks to Brad Smith for reporting it and confirming that shutting the
warning addresses it.
(cherry picked from commit
790169fe69c139e4fb6fc3046985696aa78c0569)
Signed-off-by: Willy Tarreau <w@1wt.eu>
David CARLIER [Thu, 6 Jan 2022 18:53:50 +0000 (18:53 +0000)]
MINOR: cpuset: switch to sched_setaffinity for FreeBSD 14 and above.
Following up previous update on cpuset-t.h. Ultimately, at some point
the cpuset_setaffinity code path could be removed.
(cherry picked from commit
df91cbd5845eb7ccdd2586712cb62585fd55d7c0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Wed, 5 Jan 2022 21:53:24 +0000 (22:53 +0100)]
MINOR: proxy: add option idle-close-on-response
Avoid closing idle connections if a soft stop is in progress.
By default, idle connections will be closed during a soft stop. In some
environments, a client talking to the proxy may have prepared some idle
connections in order to send requests later. If there is no proper retry
on write errors, this can result in errors while haproxy is reloading.
Even though a proper implementation should retry on connection/write
errors, this option was introduced to support back compat with haproxy <
v2.4. Indeed before v2.4, we were waiting for a last request to be able
to add a "connection: close" header and advice the client to close the
connection.
In a real life example, this behavior was seen in AWS using the ALB in
front of a haproxy. The end result was ALB sending 502 during haproxy
reloads.
This patch was tested on haproxy v2.4, with a regular reload on the
process, and a constant trend of requests coming in. Before the patch,
we see regular 502 returned to the client; when activating the option,
the 502 disappear.
This patch should help fixing github issue #1506.
In order to unblock some v2.3 to v2.4 migraton, this patch should be
backported up to v2.4 branch.
Signed-off-by: William Dauchy <wdauchy@gmail.com>
[wt: minor edits to the doc to mention other options to care about]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a9dd901143385fef3a5113d0bf1681cd0536357a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 28 Dec 2021 14:43:11 +0000 (15:43 +0100)]
MINOR: debug: add support for -dL to dump library names at boot
This is a second help to dump loaded library names late at boot, once
external code has already been initialized. The purpose is to provide
a format that makes it easy to pass to "tar" to produce an archive
containing the executable and the list of dependencies. For example
if haproxy is started as "haproxy -f foo.cfg", a config check only
will suffice to quit before starting, "-q" will be used to disable
undesired output messages, and -dL will be use to dump libraries.
This will result in such a command to trivially produce a tarball
of loaded libraries:
./haproxy -q -c -dL -f foo.cfg | tar -T - -hzcf archive.tgz
(cherry picked from commit
654726db5ab1160ad5dc8d356e2965e69c163dcf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 28 Dec 2021 08:57:10 +0000 (09:57 +0100)]
MINOR: debug: add ability to dump loaded shared libraries
Many times core dumps reported by users who experience trouble are
difficult to exploit due to missing system libraries. Sometimes,
having just a list of loaded libraries and their respective addresses
can already provide some hints about some problems.
This patch makes a step in that direction by adding a new "show libs"
command that will try to enumerate the list of object files that are
loaded in memory, relying on the dynamic linker for this. It may also
be used to detect that some foreign code embarks other undesired libs
(e.g. some external Lua modules).
At the moment it's only supported on glibc when USE_DL is set, but it's
implemented in a way that ought to make it reasonably easy to be extended
to other platforms.
(cherry picked from commit
6ab7b21a1108c0f187d09196dfc20d40f654e6c9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 28 Dec 2021 14:13:12 +0000 (15:13 +0100)]
MINOR: compat: detect support for dl_iterate_phdr()
We'll use this glibc function to dump loaded libs. It's been
available since glibc-2.2.4, and as it requires dlpi headers defined
in link.h, it implicitly relies on dlfcn, thus we condition it to
USE_DL. Other operating systems or libc might have different
dependencies so let's stick to the bare minimum for now.
(cherry picked from commit
3f3a56c9b07175f342f30c6271df24f783c892c6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Wed, 29 Dec 2021 17:16:27 +0000 (18:16 +0100)]
REGTESTS: ssl: fix ssl_default_server.vtc
Patch 2c776f1 ("BUG/MEDIUM: ssl: initialize correctly ssl w/
default-server") added tests that are not relevant anymore and broke the
reg-test. revert them.
(cherry picked from commit
0387632ac0db520402550cf19a96d41f8c1661de)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 28 Dec 2021 17:47:17 +0000 (18:47 +0100)]
BUG/MEDIUM: ssl: initialize correctly ssl w/ default-server
This bug was introduced by
d817dc73 ("MEDIUM: ssl: Load client
certificates in a ckch for backend servers") in which the creation of
the SSL_CTX for a server was moved to the configuration parser when
using a "crt" keyword instead of being done in ssl_sock_prepare_srv_ctx().
The patch
0498fa40 ("BUG/MINOR: ssl: Default-server configuration ignored by
server") made it worse by setting the same SSL_CTX for every servers
using a default-server. Resulting in any SSL option on a server applied
to every server in its backend.
This patch fixes the issue by reintroducing a string which store the
path of certificate inside the server structure, and loading the
certificate in ssl_sock_prepare_srv_ctx() again.
This is a quick fix to backport, a cleaner way can be achieve by always
creating the SSL_CTX in ssl_sock_prepare_srv_ctx() and splitting
properly the ssl_sock_load_srv_cert() function.
This patch fixes issue #1488.
Must be backported as far as 2.4.
(cherry picked from commit
2c776f1c30c85be11c9ba8ca8d9a7d62690d1a32)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Miroslav Zagorac [Mon, 27 Dec 2021 11:44:07 +0000 (12:44 +0100)]
BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time
Please do not set the OT_USE_VARS configuration variable, as the source
will probably not be able to compile! For now, this variable can only
be used for experimental purposes, and is not intended for wider use.
For further clarification, please see commit
4cb2c83f4.
Must be backported to 2.5.
(cherry picked from commit
6c9f7faa62a00a4d028704d7b11e290c83f8a49d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 23 Dec 2021 10:12:13 +0000 (11:12 +0100)]
DEBUG: ssl: make sure we never change a servername on established connections
Since this case was already met previously with commit
655dec81b
("BUG/MINOR: backend: do not set sni on connection reuse"), let's make
sure that we don't change reused connection settings. This could be
generalized to most settings that are only in effect before the handshake
in fact (like set_alpn and a few other ones).
(cherry picked from commit
77bfa66124e8532a4ecbe5025657574bb43b7160)
[wt: it's not enabled in default builds thus is safe to backport; it may
help sort out certain future bug reports in field]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Thierry Fournier [Wed, 15 Dec 2021 18:03:52 +0000 (19:03 +0100)]
DOC: fix misspelled keyword "resolve_retries" in resolvers
"resolve_retries" was spelled "resolve_retires".
(cherry picked from commit
55c40ea17778b1afa3cf8f0f0a2cc42717c9364a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Daniel Jakots [Wed, 8 Dec 2021 01:34:39 +0000 (20:34 -0500)]
BUILD: ssl: unbreak the build with newer libressl
In LibreSSL 3.5.0, BIO is going to become opaque, so haproxy's
compat macros will no longer work. The functions they substitute
have been available since LibreSSL 2.7.0.
(cherry picked from commit
d1a2e2b0d1da0dff726738343fbaed044fb93470)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 26 Nov 2021 16:26:19 +0000 (17:26 +0100)]
BUG/MINOR: mux-h1: Fix splicing for messages with unknown length
Splicing was disabled fo Messages with an unknown length (no C-L or T-E
header) with no valid reason. So now, it is possible to use the kernel
splicing for such messages.
This patch should be backported as far as 2.4.
(cherry picked from commit
f5ce320156c1a1233755440dad766e7b2b401c0a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 26 Nov 2021 15:37:55 +0000 (16:37 +0100)]
BUG/MEDIUM: mux-h1: Fix splicing by properly detecting end of message
Since the 2.4.4, the splicing support in the H1 multiplexer is buggy because
end of the message is not properly detected.
On the 2.4, when the requests is spliced, there is no issue. But when the
response is spliced, the client connection is always closed at the end of the
message. Note the response is still fully sent.
On the 2.5 and higher, when the last requests on a connection is spliced, a
client abort is reported. For other requests there is no issue. In all cases,
the requests are fully sent. When the response is spliced, the server connection
hangs till the server timeout and a server abort is reported. The response is
fully sent with no delay.
The root cause is the EOM block suppression. There is no longer extra block to
be sure to call a last time rcv_buf()/snd_buf() callback functions. At the end,
to fix the issue, we must now detect end of the message in rcv_pipe() and
snd_pipe() callback functions. To do so, we rely on the announced message length
to know when the payload is finished. This works because the chunk-encoded
messages are not spliced.
This patch must be backported as far as 2.4 after an observation period.
(cherry picked from commit
140f1a585248a7da1fefbb5f20f260dfafb32a9a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Dec 2021 12:38:49 +0000 (13:38 +0100)]
BUG/MEDIUM: peers: properly skip conn_cur from incoming messages
The approach used for skipping conn_cur in commit
db2ab8218 ("MEDIUM:
stick-table: never learn the "conn_cur" value from peers") was wrong,
it only works with simple tables but as soon as frequency counters or
arrays are exchanged after conn_cur, the stream is desynchronized and
incorrect values are read. This is because the fields have a variable
length depending on their types and cannot simply be skipped by a
"continue" statement.
Let's change the approach to make sure we continue to completely parse
these local-only fields, and only drop the value at the moment we're
about to store them, since this is exactly the intent.
A simpler approach could consist in having two sets of stktable_data_ptr()
functions, one for retrieval and one for storage, and to make the store
function return a NULL pointer for local types. For now this doesn't
seem worth the trouble.
This fixes github issue #1497. Thanks to @brenc for the reproducer.
This must be backported to 2.5.
(cherry picked from commit
b4ff6f4ae9267620827f7da9b519f4e1b28b10e9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Dec 2021 10:27:53 +0000 (11:27 +0100)]
BUG/MEDIUM: backend: fix possible sockaddr leak on redispatch
A subtle change of target address allocation was introduced with commit
68cf3959b ("MINOR: backend: rewrite alloc of stream target address") in
2.4. Prior to this patch, a target address was allocated by function
assign_server_address() only if none was previously allocated. After
the change, the allocation became unconditional. Most of the time it
makes no difference, except when we pass multiple times through
connect_server() with SF_ADDR_SET cleared.
The most obvious fix would be to avoid allocating that address there
when already set, but the root cause is that since introduction of
dynamically allocated addresses, the SF_ADDR_SET flag lies. It can
be cleared during redispatch or during a queue redistribution without
the address being released.
This patch instead gives back all its correct meaning to SF_ADDR_SET
and guarantees that when not set no address is allocated, by freeing
that address at the few places the flag is cleared. The flag could
even be removed so that only the address is checked but that would
require to touch many areas for no benefit.
The easiest way to test it is to send requests to a proxy with l7
retries enabled, which forwards to a server returning 500:
defaults
mode http
timeout client 1s
timeout server 1s
timeout connect 1s
retry-on all-retryable-errors
retries 1
option redispatch
listen proxy
bind *:5000
server app 0.0.0.0:5001
frontend dummy-app
bind :5001
http-request return status 500
Issuing "show pools" on the CLI will show that pool "sockaddr" grows
as requests are redispatched, and remains stable with the fix. Even
"ps" will show that the process' RSS grows by ~160B per request.
This fix will need to be backported to 2.4. Note that before 2.5,
there's no strm->si[1].dst, strm->target_addr must be used instead.
This addresses github issue #1499. Special thanks to Daniil Leontiev
for providing a well-documented reproducer.
(cherry picked from commit
266d5405490050adeaf414158f7f4b9bad5298bc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 23 Dec 2021 08:26:30 +0000 (09:26 +0100)]
MINOR: pools: work around possibly slow malloc_trim() during gc
During 2.4-dev, support for malloc_trim() was implemented to ease
release of memory in a stopping process. This was found to be quite
effective and later backported to 2.3.7.
Then it was found that sometimes malloc_trim() could take a huge time
to complete it if was competing with other threads still allocating and
releasing memory, reason why it was decided in 2.5-dev to move
malloc_trim() under the thread isolation that was already in place in
the shared pool version of pool_gc() (this was commit
26ed1835).
However, other instances of pool_gc() that used to call malloc_trim()
were not updated since they were not using thread isolation. Currently
we have two other such instances, one for when there is absolutely no
pool and one for when there are only thread-local pools.
Christian Ruppert reported in GH issue #1490 that he's sometimes seeing
and old process die upon reload when upgrading from 2.3 to 2.4, and
that this happens inside malloc_trim(). The problem is that since
2.4-dev11 with commit
0bae07592 we detect modern libc that provide a
faster thread-aware allocator and do not maintain shared pools anymore.
As such we're using again the simpler pool_gc() implementations that do
not use thread isolation around the malloc_trim() call.
All this code was cleaned up recently and the call moved to a new
function trim_all_pools(). This patch implements explicit thread isolation
inside that function so that callers do not have to care about this
anymore. The thread isolation is conditional so that this doesn't affect
the one already in place in the larger version of pool_gc(). This way it
will solve the problem for all callers.
This patch must be backported as far as 2.3. It may possibly require
some adaptations. If trim_all_pools() is not present, copy-pasting the
tests in each version of pool_gc() will have the same effect.
Thanks to Christian for his detailed report and his testing.
(cherry picked from commit
0d93a8186331972d9c60624ed822e7c2c008d1d4)
[wt: dropped the 2.6-specific mallctl() context]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Remi Tricot-Le Breton [Fri, 17 Dec 2021 17:53:23 +0000 (18:53 +0100)]
MINOR: ssl: Remove empty lines from "show ssl ocsp-response" output
There were empty lines in the output of the CLI's "show ssl
ocsp-response" command (after the certificate ID and between two
certificates). This patch removes them since an empty line should mark
the end of the output.
Must be backported in 2.5.
(cherry picked from commit
cc750efbc5c2180ed63b222a51029609ea96d0f7)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Fri, 10 Dec 2021 13:14:53 +0000 (14:14 +0100)]
BUG/MEDIUM: mworker/cli: crash when trying to access an old PID in prompt mode
The master process encounter a crash when trying to access an old
process which left from the master CLI.
To reproduce the problem, you need a prompt to a previous worker, then
wait for this worker to leave, once it left launch a command from this
prompt. The s->target is then filled with a NULL which is dereferenced
when trying to connect().
This patch fixes the problem by checking if s->target is NULL.
Must be backported as far as 2.0.
(cherry picked from commit
dcbe7b91d69f6857961d1545ae71205d9afb905f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Lukas Tribus [Thu, 9 Dec 2021 00:27:14 +0000 (01:27 +0100)]
DOC: config: fix error-log-format example
In commit
6f7497616 ("MEDIUM: connection: rename fc_conn_err and
bc_conn_err to fc_err and bc_err"), fc_conn_err became fc_err, so
update this example.
(cherry picked from commit
2b94973e0f1102568d9393998e23f4402ac459a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Lukas Tribus [Wed, 8 Dec 2021 10:33:01 +0000 (11:33 +0100)]
DOC: config: retry-on list is space-delimited
We are using comma-delimited list for init-addr for example, let's
document that this is space-delimited to avoid the guessing game.
(cherry picked from commit
de16008c298d87b212807b9bb42d27f675be732b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Dec 2021 09:48:36 +0000 (10:48 +0100)]
DOC: config: Specify %Ta is only available in HTTP mode
%Ta format can only be used in HTTP mode but it was not specify in the
configuration manual.
This patch should fix the issue #1317.
(cherry picked from commit
3010e00e1cc8388c2c01a43c597aa3ca07bff487)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Dec 2021 09:18:09 +0000 (10:18 +0100)]
DOC: spoe: Clarify use of the event directive in spoe-message section
Only one event is possible for a spoe-message section. If defined several
time, only the last one is considered. The documentation is now explicit on
this point.
This patch is related to the the issue #1351.
(cherry picked from commit
d322e948c979cff5bd419aa1b716186615651ba1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 7 Dec 2021 17:49:44 +0000 (18:49 +0100)]
BUG/MINOR: cli/server: Don't crash when a server is added with a custom id
When a server is dynamically added via the CLI with a custom id, the key
used to insert it in the backend's tree of used names is not initialized.
The server id must be used but it is only used when no custom id is
provided. Thus, with a custom id, HAProxy crashes.
Now, the server id is always used to init this key, to be able to insert the
server in the corresponding tree.
This patch should fix the issue #1481. It must be backported as far as 2.4.
(cherry picked from commit
70f8948364524b83f4763f02ceea884019451e06)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 6 Dec 2021 07:43:22 +0000 (08:43 +0100)]
MINOR: http-rules: Add capture action to http-after-response ruleset
It is now possible to perform captures on the response when
http-after-response rules are evaluated. It may be handy to capture headers
from responses generated by HAProxy.
This patch is trivial, it may be backported if necessary.
(cherry picked from commit
ba8f06304e538d763a506526a63285c33d26af47)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 3 Dec 2021 16:38:42 +0000 (17:38 +0100)]
IMPORT: slz: use the correct CRC32 instruction when running in 32-bit mode
Many ARMv8 processors also support Aarch32 and can run armv7 and even
thumb2 code. While armv8 compilers will not emit these instructions,
armv7 compilers that are aware of these processors will do. For
example, using gcc built for an armv7 target and passing it
"-mcpu=cortex-a72" or "-march=armv8-a+crc" will result in the CRC32
instruction to be used.
In this case the current assembly code fails because with the ARM and
Thumb2 instruction sets there is no such "%wX" half-registers. We need
to use "%X" instead as the native 32-bit register when running with a
32-bit instruction set, and use "%wX" when using the 64-bit instruction
set (A64).
This is slz upstream commit
fab83248612a1e8ee942963fe916a9cdbf085097
(cherry picked from commit
b154422db10bee52050668d7679c140ed27800cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Mon, 6 Dec 2021 07:01:02 +0000 (07:01 +0000)]
BUILD: tree-wide: avoid warnings caused by redundant checks of obj_types
At many places we use construct such as:
if (objt_server(blah))
do_something(objt_server(blah));
At -O2 the compiler manages to simplify the operation and see that the
second one returns the same result as the first one. But at -O1 that's
not always the case, and the compiler is able to emit a second
expression and sees the potential null that results from it, and may
warn about a potential null deref (e.g. with gcc-6.5). There are two
solutions to this:
- either the result of the first test has to be passed to a local
variable
- or the second reference ought to be unchecked using the __objt_*
variant.
This patch fixes all occurrences at once by taking the second approach
(the least intrusive). For constructs like:
objt_server(blah) ? objt_server(blah)->name : "no name"
a macro could be useful. It would for example take the object type
(server), the field name (name) and the default value. But there
are probably not enough occurrences across the whole code for this
to really matter.
This should be backported wherever it applies.
(cherry picked from commit
88bc800eae8d0a2118a273afc52ecdc529c9f523)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Tue, 14 Dec 2021 14:22:29 +0000 (15:22 +0100)]
MINOR: cli: "show version" displays the current process version
This patch implements a simple "show version" command which returns
the version of the current process.
It's available from the master and the worker processes, so it is easy
to check if the master and the workers have the same version.
This is a minor patch that really improve compatibility checks
for scripts.
Could be backported in haproxy version as far as 2.0.
(cherry picked from commit
740629e2966641871b183d1c728fa7aeda45a1cc)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Tim Duesterhus [Wed, 1 Dec 2021 22:04:15 +0000 (23:04 +0100)]
BUG/MEDIUM: sample: Fix memory leak in sample_conv_jwt_member_query
The function leaked one full buffer per invocation. Fix this by simply removing
the call to alloc_trash_chunk(), the static chunk from get_trash_chunk() is
sufficient.
This bug was introduced in
0a72f5ee7c2a61bdb379436461269315c776b50a, which is
2.5-dev10. This fix needs to be backported to 2.5+.
(cherry picked from commit
caf5f5d3026e5cb00c1428242209624731eeee19)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 3 Dec 2021 07:58:22 +0000 (08:58 +0100)]
BUILD: bug: Fix error when compiling with -DDEBUG_STRICT_NOCRASH
ha_backtrace_to_stderr() must be declared in CRASH_NOW() macro whe HAProxy
is compiled with DEBUG_STRICT_NOCRASH. Otherwise an error is reported during
compilation:
include/haproxy/bug.h:58:26: error: implicit declaration of function ‘ha_backtrace_to_stderr’ [-Werror=implicit-function-declaration]
58 | #define CRASH_NOW() do { ha_backtrace_to_stderr(); } while (0)
This patch must be backported as far as 2.4.
(cherry picked from commit
d6ae912b04a38a544b2d37f549c7f86ae51e929c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 26 Nov 2021 16:31:35 +0000 (17:31 +0100)]
MINOR: mux-h1: Improve H1 traces by adding info about http parsers
Info about the request and the response parsers are now displayed in H1
traces for advanced and complete verbosity only. This should help debugging.
This patch may be backported as far as 2.4.
(cherry picked from commit
6580f2868e8828a8ea1c9e4a6481c9e54abfdd25)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 26 Nov 2021 13:43:57 +0000 (14:43 +0100)]
BUG/MINOR: mworker: deinit of thread poller was called when not initialized
Commit 67e371e ("BUG/MEDIUM: mworker: FD leak of the eventpoll in wait
mode") introduced a regression. Upon a reload it tries to deinit the
poller per thread, but no poll loop was initialized after loading the
configuration.
This patch fixes the issue by moving this part of the code in
mworker_reload(), since this function will be called only when the
poller is fully initialized.
This patch must be backported in 2.5.
(cherry picked from commit
efd954793e60131932c61883d0869c7912a5d12c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 25 Nov 2021 09:03:44 +0000 (10:03 +0100)]
BUG/MEDIUM: mworker: FD leak of the eventpoll in wait mode
Since 2.5, before re-executing in wait mode, the master can have a
working configuration loaded, with a eventpoll fd. This case was not
handled correctly and a new eventpoll FD is leaking in the master at
each reload, which is inherited by the new worker.
Must be backported in 2.5.
(cherry picked from commit
67e371ea1440c67ed84577d37566b577f7ecd336)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 1 Dec 2021 17:01:48 +0000 (18:01 +0100)]
BUG/MEDIUM: h1: Properly reset h1m flags when headers parsing is restarted
If H1 headers are not fully received at once, the parsing is restarted a
last time when all headers are finally received. When this happens, the h1m
flags are sanitized to remove all value set during parsing.
But some flags where erroneously preserved. Among others, H1_MF_TE_CHUNKED
flag was not removed, what could lead to parsing error.
To fix the bug and make things easy, a mask has been added with all flags
that must be preserved. It will be more stable. This mask is used to
sanitize h1m flags.
This patch should fix the issue #1469. It must be backported to 2.5.
(cherry picked from commit
02c893332bb22b49dbc96e804ca6018446c40f7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emeric Brun [Wed, 1 Dec 2021 11:08:42 +0000 (12:08 +0100)]
BUG/MAJOR: segfault using multiple log forward sections.
For each new log forward section, the proxy was added to the log forward
proxy list but the ref on the previous log forward section's proxy was
scratched using "init_new_proxy" which performs a memset. After configuration
parsing this list contains only the last section's proxy.
The post processing walk through this list to resolve "ring" names.
Since some section's proxies are missing in this list, the resolving
is not done for those ones and the pointer on the ring is kept to null
causing a segfault at runtime trying to write a log message
into the ring.
This patch shift the "init_new_proxy" before adding the ref on the
previous log forward section's proxy on currently parsed one.
This patch shoud fix github issue #1464
This patch should be backported to 2.3
(cherry picked from commit
2ad2b1c94cc7bb88b879fb0895a3bee566fdf053)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 1 Dec 2021 09:18:08 +0000 (10:18 +0100)]
BUG/MEDIUM: resolvers: Detach query item on response error
When a new response is parsed, it is unexpected to have an old query item
still attached to the resolution. And indeed, when the response is parsed
and validated, the query item is detached and used for a last check on its
dname. However, this is only true for a valid response. If an error is
detected, the query is not detached. This leads to undefined behavior (most
probably a crash) on the next response because the first element in the
query list is referencing an old response.
This patch must be backported as far as 2.0.
(cherry picked from commit
80b2e34b18b805d28a9569d8157c7bcf088eef8f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 1 Dec 2021 08:50:41 +0000 (09:50 +0100)]
BUG/MINOR: server: Don't rely on last default-server to init server SSL context
During post-parsing stage, the SSL context of a server is initialized if SSL
is configured on the server or its default-server. It is required to be able
to enable SSL at runtime. However a regression was introduced, because the
last parsed default-server is used. But it is not necessarily the
default-server line used to configure the server. This may lead to
erroneously initialize the SSL context for a server without SSL parameter or
the skip it while it should be done.
The problem is the default-server used to configure a server is not saved
during configuration parsing. So, the information is lost during the
post-parsing. To fix the bug, the SRV_F_DEFSRV_USE_SSL flag is
introduced. It is used to know when a server was initialized with a
default-server using SSL.
For the record, the commit
f63704488e ("MEDIUM: cli/ssl: configure ssl on
server at runtime") has introduced the bug.
This patch must be backported as far as 2.4.
(cherry picked from commit
4ab2679689c02882d9ea743ab0c458cd0c3b5388)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Remi Tricot-Le Breton [Fri, 26 Nov 2021 17:08:39 +0000 (18:08 +0100)]
BUG/MINOR: vars: Fix the set-var and unset-var converters
In commit
3a4bedccc6 the variable logic was changed. Instead of
accessing variables by their name during runtime, the variable tables
are now indexed by a hash of the name. But the set-var and unset-var
converters try to access the correct variable by calculating a hash on
the sample instead of the already calculated variable hash.
It should be backported to 2.5.
(cherry picked from commit
bb3e80e1814f63b76f100b342c217d08ca719688)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 30 Nov 2021 08:32:21 +0000 (09:32 +0100)]
BUILD: evports: remove a leftover from the dead_fd cleanup
Commit
b1f29bc62 ("MINOR: activity/fd: remove the dead_fd counter") got
rid of FD_UPDT_DEAD, but evports managed to slip through the cracks and
wasn't cleaned up, thus it doesn't build anymore, as reported in github
issue #1467. We just need to remove the related lines since the situation
is already handled by the remaining conditions.
Thanks to Dominik Hassler for reporting the issue and confirming the fix.
This must be backported to 2.5 only.
(cherry picked from commit
3cc1e3d5ca9b0429f4f1e6618c4fbabac0cfb38b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 18 Oct 2021 12:52:49 +0000 (14:52 +0200)]
BUG/MEDIUM: cli: Properly set stream analyzers to process one command at a time
The proxy used by the master CLI is an internal proxy and no filter are
registered on it. Thus, there is no reason to take care to set or unset
filter analyzers in the master CLI analyzers. AN_REQ_FLT_END was set on the
request channel to prevent the infinite forward and be sure to be able to
process one commande at a time. However, the only work because
CF_FLT_ANALYZE flag was used by error as a channel analyzer instead of a
channel flag. This erroneously set AN_RES_FLT_END on the request channel,
that really prevent the infinite forward, be side effet.
In fact, We must avoid this kind of trick because this only work by chance
and may be source of bugs in future. Instead, we must always keep the CLI
request analyzer and add an early return if the response is not fully
processed. It happens when the CLI response analyzer is set.
This patch must be backported as far as 2.0.
(cherry picked from commit
d98da3bc902fcde45909eee13bdf94c99b19647d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Bertrand Jacquin [Wed, 24 Nov 2021 21:16:06 +0000 (21:16 +0000)]
BUG/MINOR: lua: remove loop initial declarations
HAProxy is documented to support gcc >= 3.4 as per INSTALL file, however
hlua.c makes use of c11 only loop initial declarations leading to build
failure when using gcc-4.9.4:
x86_64-unknown-linux-gnu-gcc -Iinclude -Wchar-subscripts -Wcomment -Wformat -Winit-self -Wmain -Wmissing-braces -Wno-pragmas -Wparentheses -Wreturn-type -Wsequence-point -Wstrict-aliasing -Wswitch -Wtrigraphs -Wuninitialized -Wunknown-pragmas -Wunused-label -Wunused-variable -Wunused-value -Wpointer-sign -Wimplicit -pthread -fdiagnostics-color=auto -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -O3 -msse -mfpmath=sse -march=core2 -g -fPIC -g -Wall -Wextra -Wundef -Wdeclaration-after-statement -fwrapv -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wno-missing-field-initializers -Wtype-limits -DUSE_EPOLL -DUSE_NETFILTER -DUSE_PCRE2 -DUSE_PCRE2_JIT -DUSE_POLL -DUSE_THREAD -DUSE_BACKTRACE -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_ACCEPT4 -DUSE_SLZ -DUSE_CPU_AFFINITY -DUSE_TFO -DUSE_NS -DUSE_DL -DUSE_RT -DUSE_PRCTL -DUSE_THREAD_DUMP -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8 -I/usr/local/include -DCONFIG_HAPROXY_VERSION=\"2.5.0\" -DCONFIG_HAPROXY_DATE=\"2021/11/23\" -c -o src/connection.o src/connection.c
src/hlua.c: In function 'hlua_config_prepend_path':
src/hlua.c:11292:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode
for (size_t i = 0; i < 2; i++) {
^
src/hlua.c:11292:2: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
This commit moves loop iterator to an explicit declaration.
Must be backported to 2.5 because this issue was introduced in
v2.5-dev10~69 with commit
9e5e586e35c5 ("BUG/MINOR: lua: Fix lua error
handling in `hlua_config_prepend_path()`")
(cherry picked from commit
7fbc7708d49ffad50b8e14ef97b82e49affe4eed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Wed, 24 Nov 2021 15:14:24 +0000 (16:14 +0100)]
BUG/MINOR: lua: don't expose internal proxies
Since internal proxies are now in the global proxy list, they are now
reachable from core.proxies, core.backends, core.frontends.
This patch fixes the issue by checking the PR_CAP_INT flag before
exposing them in lua, so the user can't have access to them.
This patch must be backported in 2.5.
(cherry picked from commit
82d5f013f96d38867985469bf47bd36ff199a256)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Wed, 24 Nov 2021 14:38:17 +0000 (15:38 +0100)]
BUG/MINOR: httpclient: allow to replace the host header
This patch allows to replace the host header generated by the
httpclient instead of adding a new one, resulting in the server replying
an error 400.
The host header is now generated from the uri only if it wasn't found in
the list of headers.
Also add a new request in the VTC file to test this.
This patch must be backported in 2.5.
(cherry picked from commit
f03b53c81d52344c29d7a5434ac32f3127cebf75)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Tue, 23 Nov 2021 15:03:05 +0000 (16:03 +0100)]
BUG/MINOR: cache: Fix loop on cache entries in "show cache"
A regression was introduced in the commit
da91842b6 ("BUG/MEDIUM: cache/cli:
make "show cache" thread-safe"). When cli_io_handler_show_cache() is called,
only one node is retrieved and is used to fill the output buffer in loop.
Once set, the "node" variable is never renewed. At the end, all nodes are
dumped but each one is duplicated several time into the output buffer.
This patch must be backported everywhere the above commit is. It means only
to 2.5 and 2.4.
(cherry picked from commit
27f88a9059d0e343d82a0319b622e0f4248b0bb8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 23 Nov 2021 14:40:21 +0000 (15:40 +0100)]
[RELEASE] Released version 2.5.0
Released version 2.5.0 with the following main changes :
- BUILD: SSL: add quictls build to scripts/build-ssl.sh
- BUILD: SSL: add QUICTLS to build matrix
- CLEANUP: sock: Wrap `accept4_broken = 1` into additional parenthesis
- BUILD: cli: clear a maybe-unused warning on some older compilers
- BUG/MEDIUM: cli: make sure we can report a warning from a bind keyword
- BUG/MINOR: ssl: make SSL counters atomic
- CLEANUP: assorted typo fixes in the code and comments
- BUG/MINOR: ssl: free correctly the sni in the backend SSL cache
- MINOR: version: mention that it's stable now
Willy Tarreau [Tue, 23 Nov 2021 14:38:10 +0000 (15:38 +0100)]
MINOR: version: mention that it's stable now
This version will be maintained up to around Q1 2023. The INSTALL file
also mentions it.
William Lallemand [Tue, 23 Nov 2021 14:15:09 +0000 (15:15 +0100)]
BUG/MINOR: ssl: free correctly the sni in the backend SSL cache
__ssl_sock_load_new_ckch_instance() does not free correctly the SNI in
the session cache, it only frees the one in the current tid.
This bug was introduced with e18d4e8 ("BUG/MEDIUM: ssl: backend TLS
resumption with sni and TLSv1.3").
This fix must be backported where the mentionned commit was backported.
(all maintained versions).
Ilya Shipitsin [Sat, 20 Nov 2021 18:11:12 +0000 (23:11 +0500)]
CLEANUP: assorted typo fixes in the code and comments
This is 28th iteration of typo fixes
Willy Tarreau [Mon, 22 Nov 2021 16:46:13 +0000 (17:46 +0100)]
BUG/MINOR: ssl: make SSL counters atomic
SSL counters were added with commit
d0447a7c3 ("MINOR: ssl: add counters
for ssl sessions") in 2.4, but their updates were not atomic, so it's
likely that under significant loads they are not correct.
This needs to be backported to 2.4.
Willy Tarreau [Sat, 20 Nov 2021 19:10:41 +0000 (20:10 +0100)]
BUG/MEDIUM: cli: make sure we can report a warning from a bind keyword
Since recent 2.5 commit
c8cac04bd ("MEDIUM: listener: deprecate "process"
in favor of "thread" on bind lines"), the "process" bind keyword may
report a warning. However some parts like the "stats socket" parser
will call such bind keywords and do not expect to face warnings, so
this will instantly cause a fatal error to be reported. A concrete
effect is that "stats socket ... process 1" will hard-fail indicating
the keyword is deprecated and will be removed in 2.7.
We must relax this test, but the code isn't designed to report warnings,
it uses a single string and only supports reporting an error code (-1).
This patch makes a special case of the ERR_WARN code and uses ha_warning()
to report it, and keeps the rest of the existing error code for other
non-warning codes. Now "process" on the "stats socket" is properly
reported as a warning.
No backport is needed.
Willy Tarreau [Sat, 20 Nov 2021 18:17:38 +0000 (19:17 +0100)]
BUILD: cli: clear a maybe-unused warning on some older compilers
The SHOW_TOT() and SHOW_AVG() macros used in cli_io_handler_show_activity()
produce a warning on gcc 4.7 on MIPS with threads disabled because the
compiler doesn't know that global.nbthread is necessarily non-null, hence
that at least one iteration is performed. Let's just change the loop for
a do {} while () that lets the compiler know it's always initialized. It
also has the tiny benefit of making the code shorter.
Tim Duesterhus [Sat, 20 Nov 2021 13:39:47 +0000 (14:39 +0100)]
CLEANUP: sock: Wrap `accept4_broken = 1` into additional parenthesis
This makes it clear to static analysis tools that this assignment is
intentional and not a mistyped comparison.
Ilya Shipitsin [Thu, 18 Nov 2021 13:27:57 +0000 (18:27 +0500)]
BUILD: SSL: add QUICTLS to build matrix
It also enables QUIC when QUICTLS is used.
Ilya Shipitsin [Thu, 18 Nov 2021 13:27:56 +0000 (18:27 +0500)]
BUILD: SSL: add quictls build to scripts/build-ssl.sh
script/build-ssl.sh is used mostly in CI, let us introduce QUIC
OpenSSL fork support
Willy Tarreau [Fri, 19 Nov 2021 18:30:04 +0000 (19:30 +0100)]
[RELEASE] Released version 2.5-dev15
Released version 2.5-dev15 with the following main changes :
- BUG/MINOR: stick-table/cli: Check for invalid ipv6 key
- CLEANUP: peers: Remove useless test on peer variable in peer_trace()
- DOC: log: Add comments to specify when session's listener is defined or not
- BUG/MEDIUM: mux-h1: Handle delayed silent shut in h1_process() to release H1C
- REGTESTS: ssl_crt-list_filters: feature cmd incorrectly set
- DOC: internals: document the list API
- BUG/MINOR: h3: ignore unknown frame types
- MINOR: quic: redirect app_ops snd_buf through mux
- MEDIUM: quic: inspect ALPN to install app_ops
- MINOR: quic: support hq-interop
- MEDIUM: quic: send version negotiation packet on unknown version
- BUG/MEDIUM: mworker: cleanup the listeners when reexecuting
- DOC: internals: document the scheduler API
- BUG/MINOR: quic: fix version negotiation packet generation
- CLEANUP: ssl: fix wrong #else commentary
- MINOR: config: support default values for environment variables
- SCRIPTS: run-regtests: reduce the number of processes needed to check options
- SCRIPT: run-regtests: avoid several calls to grep to test for features
- SCRIPT: run-regtests: avoid calling awk to compute the version
- REGTEST: set retries count to zero for all tests that expect at 503
- REGTESTS: make tcp-check_min-recv fail fast
- REGTESTS: extend the default I/O timeouts and make them overridable
- BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3
- BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found
- REGTESTS: ssl: test the TLS resumption
- BUILD: makefile: stop opening sub-shells for each and every command
- BUILD: makefile: reorder objects by build time
- BUG/MEDIUM: mux-h2: always process a pending shut read
- MINOR: quic_sock: missing CO_FL_ADDR_TO_SET flag
- MINOR: quic: Possible wrong connection identification
- MINOR: quic: Correctly pad UDP datagrams
- MINOR: quic: Support transport parameters draft TLS extension
- MINOR: quic: Anti-amplification implementation
- MINOR: quic: Wrong Initial packet connection initialization
- MINOR: quic: Wrong ACK range building
- MINOR: quic: Update some QUIC protocol errors
- MINOR: quic: Send CONNECTION_CLOSE frame upon TLS alert
- MINOR: quic: Wrong largest acked packet number parsing
- MINOR: quic: Add minimalistic support for stream flow control frames
- MINOR: quic: Wrong value for version negotiation packet 'Unused' field
- MINOR: quic: Support draft-29 QUIC version
- BUG/MINOR: quic: fix segfault on trace for version negotiation
- BUG/MINOR: hq-interop: fix potential NULL dereference
- BUILD: quic: fix potential NULL dereference on xprt_quic
- DOC: lua: documentation about the httpclient API
- BUG/MEDIUM: cache/cli: make "show cache" thread-safe
- BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found
- BUG/MINOR: shctx: do not look for available blocks when the first one is enough
- MINOR: shctx: add a few BUG_ON() for consistency checks
Willy Tarreau [Fri, 19 Nov 2021 16:47:18 +0000 (17:47 +0100)]
MINOR: shctx: add a few BUG_ON() for consistency checks
The shctx code relies on sensitive conditions that are hard to infer
from the code itself, let's add some BUG_ON() to verify them. They
helped spot the previous bugs.
Willy Tarreau [Fri, 19 Nov 2021 16:42:49 +0000 (17:42 +0100)]
BUG/MINOR: shctx: do not look for available blocks when the first one is enough
In shctx_row_reserve_hot() we only leave if we've found the exact
requested size instead of at least as large, as is documented. This
results in extra lookups and free calls in the avail loop while it is
not needed, and participates to seeing a negative data_len early as
spotted in previous bugs.
It doesn't seem to have any other impact however, but it's better to
backport it to stable branches.
Willy Tarreau [Fri, 19 Nov 2021 16:29:23 +0000 (17:29 +0100)]
BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found
In shctx_row_reserve_hot(), a missing break allows the avail loop to
loop for a while after having allocated the required blocks, possibly
leading to the point where it could trigger the watchdog after checking
up to 2 million blocks. In addition, the extra iteration may leave one
block assigned with size zero at the head of the avail list, and mark
it as being an isolated chain of 1 block. It's unclear whether this
could have had other consequences.
There is a non-negligible chance that it addreses bugs #1451 and #1284,
as the pattern observed in the loop looks exactly the same as the one
reported there in the crashes.
It's only marked medium because it is extremely hard to trigger. Here
the conditions were reproduced when starting 4k connections at once
requesting objects of random sizes between 0 and 20k to store them into
a small 1MB cache. However the watchdog will never trigger in such a case
so one needs to instrument the functions.
Thanks to Sohaib Ahmad and @g0uZ for providing useful traces.
This will need to be backported to all stable branches.
Willy Tarreau [Fri, 19 Nov 2021 16:25:41 +0000 (17:25 +0100)]
BUG/MEDIUM: cache/cli: make "show cache" thread-safe
The "show cache" command restarts from the previous node to look for a
duplicate key, but does this after having released the lock, so under
high write load, the node has many chances of having been reassigned
and the dereference of the node crashes after a few iterations. Since
the keys are unique anyway, there's no point looking for a dup, so
let's just continue from the next value.
This is only marked as medium as it seems to have been there for a
while, and discovering it that late simply means that nobody uses that
command, thus in practice it has a very limited impact on real users.
This should be backported to all stable versions.
William Lallemand [Fri, 19 Nov 2021 15:02:44 +0000 (16:02 +0100)]
DOC: lua: documentation about the httpclient API
The patch adds the documentation about the httpclient lua
implementation.
Amaury Denoyelle [Fri, 19 Nov 2021 14:49:29 +0000 (15:49 +0100)]
BUILD: quic: fix potential NULL dereference on xprt_quic
A warning is triggered by gcc9 on this code path, which is the compiler
version used by ubuntu20.04 on the github CI.
This is linked to github issue #1445.
Amaury Denoyelle [Thu, 18 Nov 2021 13:40:26 +0000 (14:40 +0100)]
BUG/MINOR: hq-interop: fix potential NULL dereference
Test return from htx_add_stline() and returns an error if NULL.
Amaury Denoyelle [Thu, 18 Nov 2021 13:38:00 +0000 (14:38 +0100)]
BUG/MINOR: quic: fix segfault on trace for version negotiation
When receiving Initial packets for Version Negotiation, no quic_conn is
instantiated. Thus, on the final trace, the quic_conn dereferencement
must be tested before using it.
Frédéric Lécaille [Fri, 19 Nov 2021 13:32:52 +0000 (14:32 +0100)]
MINOR: quic: Support draft-29 QUIC version
This is only to support quic-tracker test suite.
Frédéric Lécaille [Thu, 18 Nov 2021 12:54:43 +0000 (13:54 +0100)]
MINOR: quic: Wrong value for version negotiation packet 'Unused' field
The seven less significant bits of the first byte must be arbitrary.
Without this fix, QUIC tracker "version_negotiation" test could not pass.
Frédéric Lécaille [Thu, 18 Nov 2021 09:57:18 +0000 (10:57 +0100)]
MINOR: quic: Add minimalistic support for stream flow control frames
This simple patch add the parsing support for theses frames. But nothing is
done at this time about the streams or flow control concerned. This is only to
prevent some QUIC tracker or interop runner tests from failing for a reason
independant of their tested features.
Frédéric Lécaille [Wed, 17 Nov 2021 15:16:04 +0000 (16:16 +0100)]
MINOR: quic: Wrong largest acked packet number parsing
When we have already received ACK frames with the same largest packet
number, this is not an error at all. In this case, we must continue
to parse the ACK current frame.
Frédéric Lécaille [Wed, 17 Nov 2021 10:56:21 +0000 (11:56 +0100)]
MINOR: quic: Send CONNECTION_CLOSE frame upon TLS alert
Add ->err member to quic_conn struct to store the connection errors.
This is the responsability of ->send_alert callback of SSL_QUIC_METHOD
struct to handle the TLS alert and consequently update ->err value.
At this time, when entering qc_build_pkt() we build a CONNECTION_CLOSE
frame close the connection when ->err value is not null.
Frédéric Lécaille [Tue, 16 Nov 2021 13:34:24 +0000 (14:34 +0100)]
MINOR: quic: Update some QUIC protocol errors
Add QC_ERR_ prefix to the macro names for the protocol errors.
Add new ones which came with the last RFC drafts.
Frédéric Lécaille [Tue, 16 Nov 2021 09:54:19 +0000 (10:54 +0100)]
MINOR: quic: Wrong ACK range building
When adding a range, if no "lower" range was present in the ack range root for
the packet number space concerned, we did not check if the new added range could
overlap the next one. This leaded haproxy to crash when encoding negative integer
when building ACK frames.
This bug was revealed thanks to "multi_packet_client_hello" QUIC tracker
test which makes a client send two first Initial packets out of order.
Frédéric Lécaille [Mon, 15 Nov 2021 15:21:40 +0000 (16:21 +0100)]
MINOR: quic: Wrong Initial packet connection initialization
->qc (QUIC connection) member of packet structure were badly initialized
when received as second Initial packet (from picoquic -Q for instance).
This leaded to corrupt the quic_conn structure with random behaviors
as size effects. This bug came with this commit:
"MINOR: quic: Possible wrong connection identification"
Frédéric Lécaille [Wed, 10 Nov 2021 16:30:15 +0000 (17:30 +0100)]
MINOR: quic: Anti-amplification implementation
A QUIC server MUST not send more than three times as many bytes as received
by clients before its address validation.
Frédéric Lécaille [Wed, 10 Nov 2021 08:24:22 +0000 (09:24 +0100)]
MINOR: quic: Support transport parameters draft TLS extension
If we want to run quic-tracker against haproxy, we must at least
support the draft version of the TLS extension for the QUIC transport
parameters (0xffa5). quic-tracker QUIC version is draft-29 at this time.
We select this depending on the QUIC version. If draft, we select the
draft TLS extension.
Frédéric Lécaille [Tue, 9 Nov 2021 13:12:12 +0000 (14:12 +0100)]
MINOR: quic: Correctly pad UDP datagrams
UDP datagrams with Initial packet were padded only for the clients (haproxy
servers). But such packets MUST also be padded for the servers (haproxy
listeners). Furthere, for servers, only UDP datagrams containing ack-eliciting
Initial packet must be padded.
Frédéric Lécaille [Mon, 8 Nov 2021 16:01:46 +0000 (17:01 +0100)]
MINOR: quic: Possible wrong connection identification
A client may send several Initial packets. This is the case for picoquic
with -Q option. In this case we must identify the connection of incoming
Initial packets thanks to the original destination connection ID.
Frédéric Lécaille [Fri, 5 Nov 2021 10:40:50 +0000 (11:40 +0100)]
MINOR: quic_sock: missing CO_FL_ADDR_TO_SET flag
When allocating destination addresses for QUIC connections we did not set
this flag which denotes these addresses have been set. This had as side
effect to prevent the H3 request results from being returned to the QUIC clients.
Note that this bug was revealed by this commit:
"MEDIUM: backend: Rely on addresses at stream level to init server connection"
Thanks to Christopher for having found the real cause of this issue.
Willy Tarreau [Fri, 19 Nov 2021 10:41:10 +0000 (11:41 +0100)]
BUG/MEDIUM: mux-h2: always process a pending shut read
During 2.4-dev, an issue with partial frames was fixed with commit
3d4631fec ("BUG/MEDIUM: mux-h2: fix read0 handling on partial frames").
However this patch is not completely correct. It makes h2_recv() return
0 if the connection was shut for reads, but this not make h2_io_cb()
call h2_process(), so if there are any pending data left in the demux
buffer, they will never be processed, and the I/O callback will be
called in loops forever from the poller.
The correct return value there is 1, as is done at the end of the
function to report a pending read0.
This should definitely fix issue #1328. However even after a lot of
tests I couldn't manage to reproduce it, the conditions to enter that
situation are quite racy.
This must be backported to 2.0 since the fix above was merged into
2.0.21 and 2.2.9.
Willy Tarreau [Fri, 19 Nov 2021 10:22:25 +0000 (11:22 +0100)]
BUILD: makefile: reorder objects by build time
This is the usual pre-release reordering. It saves roughly one
second of build time at -O2 on my machine, which is always nice to
have.
Willy Tarreau [Fri, 19 Nov 2021 09:23:36 +0000 (10:23 +0100)]
BUILD: makefile: stop opening sub-shells for each and every command
We're spending ~8% of the total build time calling a shell to display
"CC" using the "echo" command! We don't really need this, as make also
knows a "$(info ...)" command to print a message. However there's a catch,
this command trims leading spaces, so we need to use an invisible space
using "$ ". Furthermore, in GNU make 3.80 and older, $(info) doesn't show
anything, so we only do that for 3.81 and above, older versions continue
to use echo.
This measurably speeds up build time especially at -O0 that developers
use most of the time for quick checks.
William Lallemand [Wed, 17 Nov 2021 01:52:51 +0000 (02:52 +0100)]
REGTESTS: ssl: test the TLS resumption
This test is able to check if the TLS resumption is working correctly
with TLSv1.2, TLSv1.3, with tickets and session cache.
William Lallemand [Thu, 18 Nov 2021 16:46:26 +0000 (17:46 +0100)]
BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found
Since commit c2aae74 ("MEDIUM: ssl: Handle early data with OpenSSL
1.1.1"), the codepath of the clientHello callback changed, letting an
unknown SNI escape with a 'return 1' instead of passing through the
abort label.
An error was still emitted because the frontend continued the handshake
with the initial_ctx, which can't be used to achieve an handshake.
However, it had the ugly side effect of letting the request pass in the
case of a TLS resume. Which could be surprising when combining strict-sni
with the removing of a crt-list entry over the CLI for example. (like
its done in the ssl/new_del_ssl_crlfile.vtc reg-test).
This patch switches the code path of the allow_early and abort label, so
the default code path is the abort one, letting the clientHello returns
the correct SSL_AD_UNRECOGNIZED_NAME in case of errors.
Which means the client will now receive:
OpenSSL error[0x14094458] ssl3_read_bytes: tlsv1 unrecognized name
Instead of:
OpenSSL error[0x14094410] ssl3_read_bytes: sslv3 alert handshake failure
Which was the error emitted before HAProxy 1.8.
This patch must be carrefuly backported as far as 1.8 once we validated
its impact.
William Lallemand [Wed, 17 Nov 2021 01:59:21 +0000 (02:59 +0100)]
BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3
When establishing an outboud connection, haproxy checks if the cached
TLS session has the same SNI as the connection we are trying to
resume.
This test was done by calling SSL_get_servername() which in TLSv1.2
returned the SNI. With TLSv1.3 this is not the case anymore and this
function returns NULL, which invalidates any outboud connection we are
trying to resume if it uses the sni keyword on its server line.
This patch fixes the problem by storing the SNI in the "reused_sess"
structure beside the session itself.
The ssl_sock_set_servername() now has a RWLOCK because this session
cache entry could be accessed by the CLI when trying to update a
certificate on the backend.
This fix must be backported in every maintained version, however the
RWLOCK only exists since version 2.4.
Willy Tarreau [Thu, 18 Nov 2021 16:46:22 +0000 (17:46 +0100)]
REGTESTS: extend the default I/O timeouts and make them overridable
With the CI occasionally slowing down, we're starting to see again some
spurious failures despite the long 1-second timeouts. This reports false
positives that are disturbing and doesn't provide as much value as this
could. However at this delay it already becomes a pain for developers
to wait for the tests to complete.
This commit adds support for the new environment variable
HAPROXY_TEST_TIMEOUT that will allow anyone to modify the connect,
client and server timeouts. It was set to 5 seconds by default, which
should be plenty for quite some time in the CI. All relevant values
that were 200ms or above were replaced by this one. A few larger
values were left as they are special. One test for the set-timeout
action that used to rely on a fixed 1-sec value was extended to a
fixed 5-sec, as the timeout is normally not reached, but it needs
to be known to compare the old and new values.
Willy Tarreau [Thu, 18 Nov 2021 16:26:25 +0000 (17:26 +0100)]
REGTESTS: make tcp-check_min-recv fail fast
This test was dependent on the check/server timeout to detect a failure,
which is not logical since we also use that one as an upper bound for
success in the second test, and that needlessly extends the test duration.
Let's make sur the timeout strikes immediately with only 1 ms timeout. Now
the total tests time is around 5.3-5.4s down from 8.7s in dev14. There is
still quite some room for improvement in the peers tests which all wait 2s
before starting but this will be more complicated.
Willy Tarreau [Thu, 18 Nov 2021 16:15:59 +0000 (17:15 +0100)]
REGTEST: set retries count to zero for all tests that expect at 503
Some tests expect a 503, typically those that check that wrong CA/CRL
will not be accepted between a server and a frontend. But such tests
tend to last very long simply because of the 1-second turn-around on
connection retries that happens during the failure. Let's properly set
the retries count to zero for these ones. One test purposely wants to
exhaust the retries so the retries was set to 1 instead.
Willy Tarreau [Thu, 18 Nov 2021 14:32:16 +0000 (15:32 +0100)]
SCRIPT: run-regtests: avoid calling awk to compute the version
For each test, the version number is evaluated using a call to awk,
which can be slow to start depending on the versions and OS. This is
only needed for a printf() call to keep only leading digits of each
component, multiply them by 1000 and pad them to 3 digits, something
that's clearly doable in plain shell in a portable way. This is what
this patch does, and it saves yet another 400 ms here on the full
test sequence.
Willy Tarreau [Thu, 18 Nov 2021 14:05:45 +0000 (15:05 +0100)]
SCRIPT: run-regtests: avoid several calls to grep to test for features
grep is used in the arguments loops to check for features such as OPENSSL
or LUA or services like prometheus-exporter. Let's just look for the words
inside the list, which requires to prepend a delimitor at the beginning of
the list and add one at the end.
Willy Tarreau [Thu, 18 Nov 2021 12:49:01 +0000 (13:49 +0100)]
SCRIPTS: run-regtests: reduce the number of processes needed to check options
run-tegtests is starting to take a lot of time to spot which tests are
eligible, because for each test file a lot of "sed" sub-processes are
launched. This commit eliminates calls to sed by using the shell's
internal processing and parsing the VTC file only once. Instead of
extracting each option one by one from the file, all entries that look
like a valid option are passed to a single case/esac statement and their
value is extracted. Splitting into lists is simply done by adjusting the
IFS depending on the list's delimiter, which, contrary to the // pattern
modifier, is supported on every shell.
This was tested on both bash and dash, and the tests' execution time
dropped by 31% from 8.7 seconds to 6.0 seconds.
Willy Tarreau [Thu, 18 Nov 2021 16:42:50 +0000 (17:42 +0100)]
MINOR: config: support default values for environment variables
Sometimes it is really useful to be able to specify a default value for
an optional environment variable, like the ${name-value} construct in
shell. In fact we're really missing this for a number of settings in
reg tests, starting with timeouts.
This commit simply adds support for the common syntax above. Other
common forms like '+' to replace existing variables, or ':-' and ':+'
to act on empty variables, were not implemented at this stage, as they
are less commonly needed.
William Lallemand [Thu, 18 Nov 2021 14:25:16 +0000 (15:25 +0100)]
CLEANUP: ssl: fix wrong #else commentary
The else is not for boringSSL but for the lack of Client Hello callback.
Should have been changed in 1fc44d4 ("BUILD: ssl: guard Client Hello
callbacks with HAVE_SSL_CLIENT_HELLO_CB macro instead of openssl
version").
Could be backported in 2.4.
Amaury Denoyelle [Thu, 18 Nov 2021 12:48:57 +0000 (13:48 +0100)]
BUG/MINOR: quic: fix version negotiation packet generation
Fix wrong memcpy usage for source and connection ID in generated Version
Negotiation packet.