Olivier Houchard [Wed, 25 Nov 2020 19:38:00 +0000 (20:38 +0100)]
BUG/MEDIUM: lists: Lock the element while we check if it is in a list.
In MT_LIST_TRY_ADDQ() and MT_LIST_TRY_ADD() we can't just check if the
element is already in a list, because there's a small race condition, it
could be added between the time we checked, and the time we actually set
its next and prev, so we have to lock it first.
This is required to address issue #958.
This should be backported to 2.3, 2.2 and 2.1.
(cherry picked from commit
1f05324cbe92a7dde71f44dc740eb8240539746f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Your Name [Sat, 28 Nov 2020 15:37:14 +0000 (15:37 +0000)]
MINOR: plock: use an ARMv8 instruction barrier for the pause instruction
As suggested by @AGSaidi in issue #958, on ARMv8 its convenient to use
an "isb" instruction in pl_cpu_relax() to improve fairness. Without it
I've met a few watchdog conditions on valid locks with 16 threads,
indicating that some threads couldn't manage to get it in 2 seconds. I
never happened again with it. In addition, the performance increased
by slightly more than 5% thanks to the reduced contention.
This should be backported as far as 2.2, possibly even 2.0.
(cherry picked from commit
1e237d037b3a45ec92d1dfa80dfd2c6bd7fc3af9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Sat, 28 Nov 2020 15:51:33 +0000 (16:51 +0100)]
[RELEASE] Released version 2.3.2
Released version 2.3.2 with the following main changes :
- BUILD: http-htx: fix build warning regarding long type in printf
- CLEANUP: cfgparse: remove duplicate registration for transparent build options
- BUG/MEDIUM: filters: Forward all filtered data at the end of http filtering
- BUG/MINOR: http-ana: Don't wait for the body of CONNECT requests
- DOC: add missing 3.10 in the summary
- BUG/MINOR: ssl: segv on startup when AKID but no keyid
- BUG/MEDIUM: http-ana: Don't eval http-after-response ruleset on empty messages
- BUG/MEDIUM: ssl/crt-list: bundle support broken in crt-list
- BUG/MEDIUM: ssl: error when no certificate are found
- BUG/MINOR: ssl/crt-list: load bundle in crt-list only if activated
- BUG/MEDIUM: ssl/crt-list: fix error when no file found
- BUILD: makefile: enable crypt(3) for OpenBSD
- DOC: clarify how to create a fallback crt
- CLEANUP: connection: do not use conn->owner when the session is known
- BUG/MAJOR: connection: reset conn->owner when detaching from session list
- BUG/MINOR: http_htx: Fix searching headers by substring
- DOC: better describes how to configure a fallback crt
- BUG/MAJOR: filters: Always keep all offsets up to date during data filtering
- MEDIUM: cache: Change caching conditions
- DOC: cache: Add new caching limitation information
- REGTESTS: Add sample_fetches/cook.vtc
- REGTESTS: converter: add url_dec test
- MINOR: http_act: Add -m flag for del-header name matching method
- BUILD: Make DEBUG part of .build_opts
- BUILD: Show the value of DEBUG= in haproxy -vv
- BUG/MEDIUM: http_act: Restore init of log-format list
- BUG/MAJOR: peers: fix partial message decoding
- DOC: better document the config file format and escaping/quoting rules
- DOC: Clarify %HP description in log-format
- BUG/MINOR: tcpcheck: Don't forget to reset tcp-check flags on new kind of check
- MINOR: tcpcheck: Don't handle anymore in-progress send rules in tcpcheck_main
- BUG/MAJOR: tcpcheck: Allocate input and output buffers from the buffer pool
- DOC: config: Move req.hdrs and req.hdrs_bin in L7 samples fetches section
- BUG/MINOR: http-fetch: Fix smp_fetch_body() when called from a health-check
Christopher Faulet [Wed, 25 Nov 2020 07:08:08 +0000 (08:08 +0100)]
BUG/MINOR: http-fetch: Fix smp_fetch_body() when called from a health-check
res.body may be called from a health-check. It is probably never used. But it is
possibe. In such case, there is no channel. Thus we must not use it
unconditionally to set the flag SMP_F_MAY_CHANGE on the smp.
Now the condition test the channel first. In addtion, the flag is not set if the
payload is fully received.
This patch must be backported as far as 2.2.
(cherry picked from commit
a9ffc416377e0df9859526dc3c1d769c6a68636f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Tue, 24 Nov 2020 16:13:24 +0000 (17:13 +0100)]
DOC: config: Move req.hdrs and req.hdrs_bin in L7 samples fetches section
req.hdrs and req.hdrs_bin are L7 sample fetches, not L6. They were in the wrong
section.
This patch may be backported as far as 1.8.
(cherry picked from commit
687a68e2d04900e84317d066de80891a7f848747)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Wed, 25 Nov 2020 12:47:00 +0000 (13:47 +0100)]
BUG/MAJOR: tcpcheck: Allocate input and output buffers from the buffer pool
Historically, the input and output buffers of a check are allocated by hand
during the startup, with a specific size (not necessarily the same than
other buffers). But since the recent refactoring of the checks to rely
exclusively on the tcp-checks and to use the underlying mux layer, this part
is totally buggy. Indeed, because these buffers are now passed to a mux,
they maybe be swapped if a zero-copy is possible. In fact, for now it is
only possible in h2_rcv_buf(). Thus the bug concretely only exists if a h2
health-check is performed. But, it is a latent bug for other muxes.
Another problem is the size of these buffers. because it may differ for the
other buffer size, it might be source of bugs.
Finally, for configurations with hundreds of thousands of servers, having 2
buffers per check always allocated may be an issue.
To fix the bug, we now allocate these buffers when required using the buffer
pool. Thus not-running checks don't waste memory and muxes may swap them if
possible. The only drawback is the check buffers have now always the same
size than buffers used by the streams. This deprecates indirectly the
"tune.chksize" global option.
In addition, the http-check regtest have been update to perform some h2
health-checks.
Many thanks to @VigneshSP94 for its help on this bug.
This patch should solve the issue #936. It relies on the commit "MINOR:
tcpcheck: Don't handle anymore in-progress send rules in tcpcheck_main".
Both must be backport as far as 2.2.
bla
(cherry picked from commit
b381a505c1010bb11abbe7b31e8d2307c4dab541)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Wed, 25 Nov 2020 12:34:51 +0000 (13:34 +0100)]
MINOR: tcpcheck: Don't handle anymore in-progress send rules in tcpcheck_main
The special handling of in-progress send rules at the begining of
tcpcheck_main() function can be removed. Instead, at the begining of the
tcpcheck_eval_send() function, we test is there is some data in the output
buffer. In this case, it means we are evaluating an unfinished send rule and
we can jump to the sending part, skipping the formatting part.
This patch is mandatory for a major fix on the checks and must be backported
as far as 2.2.
(cherry picked from commit
39066c27384653b41b95370fc4dec4469a637a23)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Wed, 25 Nov 2020 15:43:12 +0000 (16:43 +0100)]
BUG/MINOR: tcpcheck: Don't forget to reset tcp-check flags on new kind of check
When a new kind of check is found during the parsing of a proxy section (via
an option directive), we must reset tcpcheck flags for this proxy. It is
mandatory to not inherit some flags from a previously declared check (for
instance in the default section).
This patch must be backported as far as 2.2.
(cherry picked from commit
1faf18ae3976cddfb17afa95ea447205330c821e)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Maciej Zdeb [Thu, 26 Nov 2020 10:45:52 +0000 (10:45 +0000)]
DOC: Clarify %HP description in log-format
%HP is used to report HTTP request URI in logs, which might be relative
or absolute. Description in documentation should not suggest that it
behaves exactly the same as "path" sample fetch.
This is even more important after
30ee1efe676e8264af16bab833c621d60a72a4d7
because right now, when HTTP2 is a standard, %HP usually returns absolute
URI.
This might be backported as far as 2.1
(cherry picked from commit
21acc33266cc04eac5e70c22839d622284a6f46a)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Wed, 25 Nov 2020 18:58:20 +0000 (19:58 +0100)]
DOC: better document the config file format and escaping/quoting rules
It's always a pain to figure how to proceed when special characters need
to be embedded inside arguments of an expression. Let's document the
configuration file format and how unquoting/unescaping works at each
level (top level and argument level) so that everyone hopefully finds
suitable reminders or examples for complex cases.
This is related to github issue #200 and addresses issues #712 and #966.
(cherry picked from commit
6f1129d14dace99687f8681bf825dfda2905502a)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Thu, 26 Nov 2020 16:06:04 +0000 (17:06 +0100)]
BUG/MAJOR: peers: fix partial message decoding
Another bug in the peers message parser was uncovered by last commit
1dfd4f106 ("BUG/MEDIUM: peers: fix decoding of multi-byte length in
stick-table messages"): the function return on incomplete message does
not check if the channel has a pending close before deciding to return
0. It did not hurt previously because the loop calling co_getblk() once
per character would have depleted the buffer and hit the end, causing
<0 to be returned and matching the condition. But now that we process
at once what is available this cannot be relied on anymore and it's
now clearly visible that the final check is missing.
What happens when this strikes is that if a peer connection breaks in
the middle of a message, the function will return 0 (missing data) but
the caller doesn't check for the closed buffer, subscribes to reads,
and the applet handler is immediately called again since some data are
still available. This is detected by the loop prevention and the process
dies complaining that an appctx is spinning.
This patch simply adds the check for closed channel. It must be
backported to the same versions as the fix above.
(cherry picked from commit
345ebcfc010e397cb718400a32b4db845dda7a2f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Maciej Zdeb [Mon, 23 Nov 2020 16:03:09 +0000 (16:03 +0000)]
BUG/MEDIUM: http_act: Restore init of log-format list
Restore init of log-format list in parse_http_del_header which was
accidently deleted by commit
ebdd4c55da4360bde7878604ea528c2031a26541
(implementation of different header matching methods for
http-request/response del-header).
This is related to GitHub issue #909
(cherry picked from commit
6dee9969b9b1ff131b49f09000234a21f194b014)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Tim Duesterhus [Sat, 21 Nov 2020 17:07:59 +0000 (18:07 +0100)]
BUILD: Show the value of DEBUG= in haproxy -vv
Previously this was not visible after building.
(cherry picked from commit
c8d19702f46867900d7b4de240c168f1c1bb594e)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Tim Duesterhus [Sat, 21 Nov 2020 17:07:58 +0000 (18:07 +0100)]
BUILD: Make DEBUG part of .build_opts
This forces a recompilation if the value of DEBUG= changes.
(cherry picked from commit
81e948e05160099272b01e31c72796477a4ab472)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Maciej Zdeb [Fri, 20 Nov 2020 13:58:48 +0000 (13:58 +0000)]
MINOR: http_act: Add -m flag for del-header name matching method
This patch adds -m flag which allows to specify header name
matching method when deleting headers from http request/response.
Currently beg, end, sub, str and reg are supported.
This is related to GitHub issue #909
(cherry picked from commit
ebdd4c55da4360bde7878604ea528c2031a26541)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Dauchy [Sun, 15 Nov 2020 13:04:43 +0000 (14:04 +0100)]
REGTESTS: converter: add url_dec test
while looking at `url_dec` implementation I realised there was not yet a
simple test to avoid future regressions.
This one is testing simple case, including the "+" behaviour depending
on the argument passed to `url_dec`
Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit
a2a46ee5724ca10640caad2fd2cc8648e11c1d9c)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Tim Duesterhus [Fri, 13 Nov 2020 18:36:47 +0000 (19:36 +0100)]
REGTESTS: Add sample_fetches/cook.vtc
Add a reg-test verifying the fix in
dea7c209f8a77b471323dd97bdc1ac4d7a17b812.
Some parts of the configuration used in the were taken from the initial bug
report from Maciej.
Should be backported together with
dea7c209f8a77b471323dd97bdc1ac4d7a17b812
(all stable versions).
Co-authored-by: Maciej Zdeb <maciej@zdeb.pl>
(cherry picked from commit
afe36e457fbc21d403004d664ff0d926f0dea401)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Remi Tricot-Le Breton [Thu, 26 Nov 2020 14:51:29 +0000 (15:51 +0100)]
DOC: cache: Add new caching limitation information
Responses that do not have an explicit expiration time or a validator
will not be cached anymore.
Must be backported if cc9bf2e ("MEDIUM: cache: Change caching
conditions") is backported.
(cherry picked from commit
d493bc863d85182b663d4a8869ec805d74e712e8)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Remi Tricot-Le Breton [Thu, 12 Nov 2020 10:14:41 +0000 (11:14 +0100)]
MEDIUM: cache: Change caching conditions
Do not cache responses that do not have an explicit expiration time
(s-maxage or max-age Cache-Control directives or Expires header) or a
validator (ETag or Last-Modified headers) anymore, as suggested in
RFC 7234#3.
The TX_FLAG_IGNORE flag is used instead of the TX_FLAG_CACHEABLE so as
not to change the behavior of the checkcache option.
(cherry picked from commit
cc9bf2e5fe1fe6f15de9e78b6aaea2cd6be5ca4f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Tue, 24 Nov 2020 08:49:01 +0000 (09:49 +0100)]
BUG/MAJOR: filters: Always keep all offsets up to date during data filtering
When at least one data filter is registered on a channel, the offsets of all
filters must be kept up to date. For data filters but also for others. It is
safer to do it in that way. Indirectly, this patch fixes 2 hidden bugs
revealed by the commit
22fca1f2c ("BUG/MEDIUM: filters: Forward all filtered
data at the end of http filtering").
The first one, the worst of both, happens at the end of http filtering when
at least one data filtered is registered on the channel. We call the
http_end() callback function on the filters, when defined, to finish the
http filtering. But it is performed for all filters. Before the commit
22fca1f2c, the only risk was to call the http_end() callback function
unexpectedly on a filter. Now, we may have an overflow on the offset
variable, used at the end to forward all filtered data. Of course, from the
moment we forward an arbitrary huge amount of data, all kinds of bad things
may happen. So offset computation is performed for all filters and
http_end() callback function is called only for data filters.
The other one happens when a data filter alter the data of a channel, it
must update the offsets of all previous filters. But the offset of non-data
filters must be up to date, otherwise, here too we may have an integer
overflow.
Another way to fix these bugs is to always ignore non-data filters from the
offsets computation. But this patch is safer and probably easier to
maintain.
This patch must be backported in all versions where the above commit is. So
as far as 2.0.
(cherry picked from commit
401e6dbff3ee0b1932f6a16e3f280246752a7edf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Joao Morais [Tue, 24 Nov 2020 11:24:30 +0000 (08:24 -0300)]
DOC: better describes how to configure a fallback crt
A default certificate is always the first one declared in the bind line,
either from `crt` or from `crt-line` option. This commit updates the
description of how to configure a fallback certificate, clarifying that
it needs to be the first one of the bind line.
Should be merged as far as the first SNI filter implementation.
(cherry picked from commit
aa8fcc4692b8c2afda455199a694067fea9e9262)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Maciej Zdeb [Fri, 20 Nov 2020 12:12:24 +0000 (12:12 +0000)]
BUG/MINOR: http_htx: Fix searching headers by substring
Function __http_find_header is used to search headers by name using specified
matching method. Matching by substring returned unexpected results due to wrong
length of substring supplied to strnistr function.
Fixed also the boolean condition by inverting it, as we're interested in
headers that contains the substring.
This patch should be backported as far as 2.2
(cherry picked from commit
302b9f8d7a3805bfd25ecf267a8ddd730c6887b3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 20 Nov 2020 16:22:44 +0000 (17:22 +0100)]
BUG/MAJOR: connection: reset conn->owner when detaching from session list
Baptiste reported a new crash affecting 2.3 which can be triggered
when using H2 on the backend, with http-reuse always and with a tens
of clients doing close only. There are a few combined cases which cause
this to happen, but each time the issue is the same, an already freed
session is dereferenced in session_unown_conn().
Two cases were identified to cause this:
- a connection referencing a session as its owner, which is detached
from the session's list and is destroyed after this session ends.
The test on conn->owner before calling session_unown_conn() is not
sufficent as the pointer is not null but is not valid anymore.
- a connection that never goes idle and that gets killed form the
mux, where session_free() is called first, then conn_free() calls
session_unown_conn() which scans the just freed session for older
connections. This one is only triggered with DEBUG_UAF
The reason for this session to be present here is that it's needed during
the connection setup, to be passed to conn_install_mux_be() to mux->init()
as the owning session, but it's never deleted aftrewards. Furthermore, even
conn_session_free() doesn't delete this pointer after freeing the session
that lies there. Both do definitely result in a use-after-free that's more
easily triggered under DEBUG_UAF.
This patch makes sure that the owner is always deleted after detaching
or killing the session. However it is currently not possible to clear
the owner right after a synchronous init because the proxy protocol
apparently needs it (a reg test checks this), and if we leave it past
the connection setup with the session not attached anywhere, it's hard
to catch the right moment to detach it. This means that the session may
remain in conn->owner as long as the connection has never been added to
nor removed from the session's idle list. Given that this patch needs to
remain simple enough to be backported, instead it adds a workaround in
session_unown_conn() to detect that the element is already not attached
anywhere.
This fix absolutely requires previous patch "CLEANUP: connection: do not
use conn->owner when the session is known" otherwise the situation will
be even worse, as some places used to rely on conn->owner instead of the
session.
The fix could theorically be backported as far as 1.8. However, the code
in this area has significantly changed along versions and there are more
risks of breaking working stuff than fixing real issues there. The issue
was really woken up in two steps during 2.3-dev when slightly reworking
the idle conns with commit
08016ab82 ("MEDIUM: connection: Add private
connections synchronously in session server list") and when adding
support for storing used H2 connections in the session and adding the
necessary call to session_unown_conn() in the muxes. But the same test
managed to crash 2.2 when built in DEBUG_UAF and patched like this,
proving that we used to already leave dangling pointers behind us:
| diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h
| index
f8f235c1a..
dd30b5f80 100644
| --- a/include/haproxy/connection.h
| +++ b/include/haproxy/connection.h
| @@ -458,6 +458,10 @@ static inline void conn_free(struct connection *conn)
| sess->idle_conns--;
| session_unown_conn(sess, conn);
| }
| + else {
| + struct session *sess = conn->owner;
| + BUG_ON(sess && sess->origin != &conn->obj_type);
| + }
|
| sockaddr_free(&conn->src);
| sockaddr_free(&conn->dst);
It's uncertain whether an existing code path there can lead to dereferencing
conn->owner when it's bad, though certain suspicious memory corruption bugs
make one think it's a likely candidate. The patch should not be hard to
adapt there.
Backports to 2.1 and older are left to the appreciation of the person
doing the backport.
A reproducer consists in this:
global
nbthread 1
listen l
bind :9000
mode http
http-reuse always
server s 127.0.0.1:8999 proto h2
frontend f
bind :8999 proto h2
mode http
http-request return status 200
Then this will make it crash within 2-3 seconds:
$ h1load -e -r 1 -c 10 http://0:9000/
If it does not, it might be that DEBUG_UAF was not used (it's harder then)
and it might be useful to restart.
(cherry picked from commit
3aab17bd56614f05cfbec553e618b774ed07cd45)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 20 Nov 2020 16:08:15 +0000 (17:08 +0100)]
CLEANUP: connection: do not use conn->owner when the session is known
At a few places we used to rely on conn->owner to retrieve the session
while the session is already known. This is not correct because at some
of these points the reason the connection's owner was still the session
(instead of NULL) is a mistake. At one place a comparison is even made
between the session and conn->owner assuming it's valid without checking
if it's NULL. Let's clean this up to use the session all the time.
Note that this will be needed for a forthcoming fix and will have to be
backported.
(cherry picked from commit
38b4d2eb22fec0f11af50f8a9977ccae0e7c66c6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Joao Morais [Sat, 21 Nov 2020 10:42:20 +0000 (07:42 -0300)]
DOC: clarify how to create a fallback crt
HAProxy uses CN and SAN of the certificates to match incoming SNI, and
use the matching certificate in the TLS handshake. `crt-list` goes
further and allows to configure SNI filters to explicitly define the
FQDNs that should match a certificate.
The first declared certificate of the `crt-list` option follows the same
rules, and it's also used as a fallback - the certificate that should be
used if SNI isn't provided or the provided one cannot match any
certificate or SNI filter. If a provided SNI matches the CN or SAN of
the first certificate, the first certificate would be used even if a
matching SNI filter is declared later.
This change clarifies this scenario and documents a filter that can be
used to convert the first declared certificate as a proper fallback.
Should be merged as far as the first SNI filter implementation.
(cherry picked from commit
e51fab0a4aba977b111c9d0926d6adbaf62204a1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Matthieu Guegan [Fri, 20 Nov 2020 09:50:39 +0000 (10:50 +0100)]
BUILD: makefile: enable crypt(3) for OpenBSD
Allow OpenBSD to support encrypted passwords in Userlists.
OpenBSD's crypt(3) function is provided directly by libc and does not
require -lcrypt.
Signed-off-by: Matthieu Guegan <matthieu.guegan@deindeal.ch>
(cherry picked from commit
496374e59246349fe0c84af68c5490ba5b6bad33)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 20 Nov 2020 17:26:09 +0000 (18:26 +0100)]
BUG/MEDIUM: ssl/crt-list: fix error when no file found
When a file from a crt-list was not found, this one was ignored silently
letting HAProxy starts without it.
This bug was introduced by 47da821 ("MEDIUM: ssl: emulates the
multi-cert bundles in the crtlist").
This commit adds a found variable which is checked once we tried every
bundle combination so we can exits with an error if none were found.
Must be backported in 2.3.
(cherry picked from commit
77e1c6fb0a5c5704315f05162f9a964bd8689c41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 20 Nov 2020 17:23:40 +0000 (18:23 +0100)]
BUG/MINOR: ssl/crt-list: load bundle in crt-list only if activated
Don't try to load a bundle from a crt-list if the bundle support was
disabled with ssl-load-extra-files.
Must be backported to 2.3.
(cherry picked from commit
7340457158b20fa89d9eba0e231b3a122f5620d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 20 Nov 2020 14:36:13 +0000 (15:36 +0100)]
BUG/MEDIUM: ssl: error when no certificate are found
When a non-existing file was specified in the configuration, haproxy
does not exits with an error which is not normal.
This bug was introduced by dfa93be ("MEDIUM: ssl: emulate multi-cert
bundles loading in standard loading") which does nothing if the stat
failed.
This patch introduce a "found" variable which is checked at the end of
the function so we exit with an error if no find were found.
Must be backported to 2.3.
(cherry picked from commit
06ce84a10079388e95e5959dbcb50fcc563c702b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 20 Nov 2020 13:23:38 +0000 (14:23 +0100)]
BUG/MEDIUM: ssl/crt-list: bundle support broken in crt-list
In issue #970 it was reported that the bundle loading does not work
anymore with crt-list.
This bug was introduced by 47da821 ("MEDIUM: ssl: emulates the
multi-cert bundles in the crtlist") which incorrectly uses "path"
instead of "crt_path" in the name resolution.
Must be backported to 2.3.
(cherry picked from commit
86c2dd60f1a0b8332a6ffc6e95dca27470fe44f7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 18 Nov 2020 15:44:02 +0000 (16:44 +0100)]
BUG/MEDIUM: http-ana: Don't eval http-after-response ruleset on empty messages
It is not possible on response comming from a server, but an errorfile may be
empty. In this case, the http-after-response ruleset must not be evaluated
because it is totally unexpected to manipulate headers on an empty HTX message.
This patch must be backported everywhere the http-after-response rules are
supported, i.e as far as 2.2.
(cherry picked from commit
aab1b67383993b93bd70144825bc4350e3986a10)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 19 Nov 2020 15:24:13 +0000 (16:24 +0100)]
BUG/MINOR: ssl: segv on startup when AKID but no keyid
In bug #959 it was reported that haproxy segfault on startup when trying
to load a certifcate which use the X509v3 AKID extension but without the
keyid field.
This field is not mandatory and could be replaced by the serial or the
DirName.
For example:
X509v3 extensions:
X509v3 Basic Constraints:
CA:FALSE
X509v3 Subject Key Identifier:
42:7D:5F:6C:3E:0D:B7:2C:FD:6A:8A:32:C6:C6:B9:90:05:D1:B2:9B
X509v3 Authority Key Identifier:
DirName:/O=HAProxy Technologies/CN=HAProxy Test Intermediate CA
serial:F2:AB:C1:41:9F:AB:45:8E:86:23:AD:C5:54:ED:DF:FA
This bug was introduced by 70df7b ("MINOR: ssl: add "issuers-chain-path" directive").
This patch must be backported as far as 2.2.
(cherry picked from commit
f69cd6873756510e6e4ef82624c16719da6f2c80)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Wed, 18 Nov 2020 09:41:24 +0000 (10:41 +0100)]
DOC: add missing 3.10 in the summary
3.10. Log forwarding was missing in the summary.
(cherry picked from commit
0217b7b24bb33d746d2bf625f5e894007517d1b0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Mon, 16 Nov 2020 15:03:35 +0000 (16:03 +0100)]
BUG/MINOR: http-ana: Don't wait for the body of CONNECT requests
CONNECT requests are bodyless messages but with no EOM blocks. Thus, conditions
to stop waiting for the message payload are not suited to this kind of
messages. Indeed, the message finishes on an EOH block. But the tunnel mode at
the stream level is only set in HTTP_XFER_BODY analyser. So, the stream is
blocked, waiting for a body that does not exist till a timeout expires.
To fix this bug, we just stop waiting for a body for CONNECT requests. Another
solution is to rely on HTX_SL_F_BODYLESS/HTTP_MSGF_BODYLESS flags. But this one
is less intrusive.
This message must be backported as far as 2.0. For the 2.0, only the HTX part
must be fixed.
(cherry picked from commit
63c69a9b4ef1136c66967463b9e4b3538e35c016)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 16 Nov 2020 09:10:38 +0000 (10:10 +0100)]
BUG/MEDIUM: filters: Forward all filtered data at the end of http filtering
When http filtering ends, if there are some filtered data not forwarded yet, we
forward them, in flt_http_end(). Most of time, this doesn't happen, except when
a tunnel is established using a CONNECT. In this case, there is not EOM on the
request and there is no body. Thus the headers are never forwarded, blocking the
stream.
This patch must be backported as far as 2.0. Prior versions don't suffer of this
bug because there is no HTX support. On the 2.0, the change is only applicable
on HTX streams. A special test must be performed to make sure.
(cherry picked from commit
22fca1f2c84334096e38d78ffc674be19c3e0292)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Jerome Magnin [Wed, 30 Sep 2020 16:05:38 +0000 (18:05 +0200)]
CLEANUP: cfgparse: remove duplicate registration for transparent build options
Since commit
37bafdcbb ("MINOR: sock_inet: move the IPv4/v6 transparent mode code
to sock_inet"), build options for transparent proxying are registered twice.
This patch removes the older one.
(cherry picked from commit
eff2e0a958816ace5ea71c4e958dff647b5bf170)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 6 Nov 2020 13:24:02 +0000 (14:24 +0100)]
BUILD: http-htx: fix build warning regarding long type in printf
Commit
a66adf41e ("MINOR: http-htx: Add understandable errors for the
errorfiles parsing") added a warning when loading malformed error files,
but this warning may trigger another build warning due to the %lu format
used. Let's simply cast it for output since it's just used for end user
output.
This must be backported to 2.0 like the commit above.
(cherry picked from commit
431a12cafeeec7300b7cea7e19b892d4e8c4900d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 13 Nov 2020 20:20:03 +0000 (21:20 +0100)]
[RELEASE] Released version 2.3.1
Released version 2.3.1 with the following main changes :
- BUG/MINOR: ssl: don't report 1024 bits DH param load error when it's higher
- MINOR: http-htx: Add understandable errors for the errorfiles parsing
- DOC: config: Fix a typo on ssl_c_chain_der
- BUG/MEDIUM: ssl/crt-list: correctly insert crt-list line if crt already loaded
- BUG/MINOR: pattern: a sample marked as const could be written
- BUG/MINOR: lua: set buffer size during map lookups
- BUG/MINOR: stats: free dynamically stats fields/lines on shutdown
- BUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
- BUG/MINOR: peers: Missing TX cache entries reset.
- BUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
- BUG/MINOR: http-fetch: Extract cookie value even when no cookie name
- BUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
- BUG/MEDIUM: check: reuse srv proto only if using same mode
- MINOR: check: report error on incompatible proto
- MINOR: check: report error on incompatible connect proto
- BUG/MINOR: http-htx: Handle warnings when parsing http-error and http-errors
- BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
- MINOR: spoe: Don't close connection in sync mode on processing timeout
- BUG/MINOR: tcpcheck: Don't warn on unused rules if check option is after
- MINOR: init: Fix the prototype for per-thread free callbacks
- MINOR: config/mux-h2: Return ERR_ flags from init_h2() instead of a status
- MINOR: cfgparse: tighten the scope of newnameserver variable, free it on error.
- REGTEST: ssl: test wildcard and multi-type + exclusions
- REGTEST: ssl: mark reg-tests/ssl/ssl_crt-list_filters.vtc as broken
- MINOR: peers: Add traces to peer_treat_updatemsg().
- REGTEST: make ssl_client_samples and ssl_server_samples require to 2.2
Christopher Faulet [Fri, 13 Nov 2020 16:10:51 +0000 (17:10 +0100)]
REGTEST: make ssl_client_samples and ssl_server_samples require to 2.2
Some missing sample fetches was backported to 2.2 making these tests compatible
with the 2.2.
(cherry picked from commit
c300747decc4a554a52d07acfd674ab7855ae15f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 10 Nov 2020 15:18:03 +0000 (16:18 +0100)]
MINOR: peers: Add traces to peer_treat_updatemsg().
Add minimalistic traces for peers with only one event to diagnose potential
issues when decode peer update messages.
(cherry picked from commit
d865935f3212f994d8868200d9a84315dbce1518)
[wt: also merge traces from
f9e51beec and
1dfd4f10]
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 10 Nov 2020 21:40:24 +0000 (22:40 +0100)]
REGTEST: ssl: mark reg-tests/ssl/ssl_crt-list_filters.vtc as broken
This regtest requires a version of OpenSSL which supports the
ClientHello callback which is only the case of recents SSL libraries
(openssl 1.1.1).
This was reported in issue #944.
(cherry picked from commit
8f04e1849d40243ba7c103b06bd2c061840ceabc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 6 Nov 2020 13:46:36 +0000 (14:46 +0100)]
REGTEST: ssl: test wildcard and multi-type + exclusions
This test checks that the bug #818 and #810 are fixed.
It test if there is no inconsistency with multiple certificate types and
that the exclusion of the certificate is correctly working with a negative
filter.
(cherry picked from commit
3ff9591ea21848257f9ae7a7a700654649465c85)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Eric Salama [Fri, 13 Nov 2020 14:56:36 +0000 (15:56 +0100)]
MINOR: cfgparse: tighten the scope of newnameserver variable, free it on error.
This should fix issue GH #931.
Also remove a misleading comment.
This commit can be backported as far as 1.9
(cherry picked from commit
9139ec34ed7aae4fe63926ca5c87a5b32b376488)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 6 Nov 2020 14:23:39 +0000 (15:23 +0100)]
MINOR: config/mux-h2: Return ERR_ flags from init_h2() instead of a status
post-check function callbacks must return ERR_* flags. Thus, init_h2() is fixed
to return ERR_NONE on success or (ERR_ALERT|ERR_FATAL) on error.
This patch may be backported as far as 2.2.
(cherry picked from commit
5214099233d99b8288fb392dfe911bc9ea86e9af)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 6 Nov 2020 14:19:19 +0000 (15:19 +0100)]
MINOR: init: Fix the prototype for per-thread free callbacks
Functions registered to release memory per-thread have no return value. But the
registering function and the function pointer in per_thread_free_fct structure
specify it should return an integer. This patch fixes it.
This patch may be backported as far as 2.0.
(cherry picked from commit
83fefbcdff9f562d719fcb5da6b4a11e20e13c7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Nov 2020 07:55:57 +0000 (08:55 +0100)]
BUG/MINOR: tcpcheck: Don't warn on unused rules if check option is after
When tcp-check or http-check rules are used, if the corresponding check option
(option tcp-check and option httpchk) is declared after the ruleset, a warning
is emitted about an unused check ruleset while there is no problem in reality.
This patch must be backported as far as 2.2.
(cherry picked from commit
c751b4508d9245baa54a19f7658aa7307d76e447)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 10 Nov 2020 13:31:39 +0000 (14:31 +0100)]
MINOR: spoe: Don't close connection in sync mode on processing timeout
In sync mode, if an applet receives a ack while the processing delay has already
expired, there is not frame waiting for this ack. But there is no reason to
close the connection in this case. The ack may be ignored and the connection may
be reused to process another frame. The only reason to trigger an error and
close the connection is when the wrong ack is received while there is still a
frame waiting for its ack. In sync mode, this should never happen.
This patch may be backported in all versions supporting the SPOE.
(cherry picked from commit
c7ba91039a7b0703971efd791ca2ca609afedb96)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 10 Nov 2020 17:45:34 +0000 (18:45 +0100)]
BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
When a SPOE applet is used to send a frame, a reference on this applet is saved
in the spoe context of the offladed stream. But, if the applet is released
before receving the corresponding ack, we must be sure to remove this
reference. This was performed for fragmented frames only. But it must also be
performed for a spoe contexts in the applet waiting_queue and in the thread
waiting_queue (used in async mode).
This bug leads to a memory corruption when an offloaded stream try to update the
state of a released applet because it still have a reference on it. There are
many ways to trigger this bug. The easiest is probably during reloads. On the
old process, all applets are woken up to be released ASAP.
Many thanks to Maciej Zdeb to report the bug and to work on it for 2
months. Without his help, it would have been much more difficult to fix the
bug. It is always a huge pleasure to see how some users are enthousiast and
helpful. Thanks again Maciej !
This patch must be backported to all versions where the spoe is supported (>=
1.7).
(cherry picked from commit
cf181c76e341f2d49f6cae0ca8200158058073f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Nov 2020 09:58:01 +0000 (10:58 +0100)]
BUG/MINOR: http-htx: Handle warnings when parsing http-error and http-errors
First of all, this patch is tagged as a bug. But in fact, it only fixes a bug in
the 2.2. On the 2.3 and above, it only add the ability to display warnings, when
an http-error directive is parsed from a proxy section and when an errorfile
directive is parsed from a http-errors section.
But on the 2.2, it make sure to display the warning emitted on a content-length
mismatch when an errorfile is parsed. The following is only applicable to the
2.2.
commit "BUG/MINOR: http-htx: Just warn if payload of an errorfile doesn't match
the C-L" (which is only present in 2.2, 2.1 and 2.0 trees, i.e see commit
7bf3d81d3cf4b9f4587 in 2.2 tree), is changing the behavior of `http_str_to_htx`
function. It may now emit warnings. And, it is the caller responsibility to
display it.
But the warning is missing when an 'http-error' directive is parsed from
a proxy section. It is also missing when an 'errorfile' directive is
parsed from a http-errors section.
This bug only exists on the 2.2. On earlier versions, these directives
are not supported and on later ones, an error is triggered instead of a
warning.
Thanks to William Dauchy that spotted the bug.
This patch must be backported as far as 2.2.
(cherry picked from commit
3005d28eb825f700820a8d38d6d7fa6ab51a1f80)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 13 Nov 2020 11:34:58 +0000 (12:34 +0100)]
MINOR: check: report error on incompatible connect proto
Report an error when using an explicit proto for a connect rule with
non-compatible mode in regards with the selected check type (tcp-check
vs http-check).
(cherry picked from commit
90eb93f79204336e38fdb859f61ab75ce44a4367)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 13 Nov 2020 11:34:57 +0000 (12:34 +0100)]
MINOR: check: report error on incompatible proto
If the check mux has been explicitly defined but is incompatible with
the selected check type (tcp-check vs http-check), report a warning and
prevent haproxy startup.
(cherry picked from commit
7c148901837c5deed0f33bc037480faf10cb9a1f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 13 Nov 2020 11:34:56 +0000 (12:34 +0100)]
BUG/MEDIUM: check: reuse srv proto only if using same mode
Only reuse the mux from server if the check is using the same mode.
For example, this prevents a tcp-check on a h2 server to select the h2
multiplexer instead of passthrough.
This bug was introduced by the following commit :
BUG/MEDIUM: checks: Use the mux protocol specified on the server line
It must be backported up to 2.2.
Fixes github issue #945.
(cherry picked from commit
0519bd4d04cc3455233d9a74c5d682a958fc02b2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Nov 2020 12:41:04 +0000 (13:41 +0100)]
BUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
req.cook, req.cook_val, req.cook_cnt and and their response counterparts may be
called without cookie name. In this case, empty parentheses may be used, or no
parentheses at all. In both, the result must be the same. But only the first one
works. The second one always returns a failure. This patch fixes this bug.
Note that on old versions (< 2.2), both cases fail.
This patch must be backported in all stable versions.
(cherry picked from commit
97fc8da2643531ade4163d6662f13f76fa59d677)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Maciej Zdeb [Fri, 13 Nov 2020 09:38:06 +0000 (09:38 +0000)]
BUG/MINOR: http-fetch: Extract cookie value even when no cookie name
HTTP sample fetches dealing with the cookies (req/res.cook,
req/res.cook_val and req/res.cook_cnt) must be prepared to be called
without cookie name. For the first two, the first cookie value is
returned, regardless its name. For the last one, all cookies are counted.
To do so, http_extract_cookie_value() may now be called with no cookie
name (cookie_name_l set to 0). In this case, the matching on the cookie
name is ignored and the first value found is returned.
Note this patch also fixes matching on cookie values in ACLs.
This should be backported in all stable versions.
(cherry picked from commit
dea7c209f8a77b471323dd97bdc1ac4d7a17b812)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 13 Nov 2020 13:10:20 +0000 (14:10 +0100)]
BUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
There is a bug in peer_recv_msg() due to an incorrect cast when trying
to decode the varint length of a stick-table message, causing lengths
comprised between 128 and 255 to consume one extra byte, ending in
protocol errors. The root cause of this is that peer_recv_msg() tries
hard to reimplement all the parsing and control that is already done in
intdecode() just to measure the length before calling it. And it got it
wrong.
Let's just get rid of this unneeded code duplication and solely rely on
intdecode() instead. The bug was introduced in 2.0 as part of a cleanup
pass on this code with commit
95203f218 ("MINOR: peers: Move high level
receive code to reduce the size of I/O handler."), so this patch must
be backported to 2.0.
Thanks to Yves Lafon for reporting the problem.
(cherry picked from commit
1dfd4f106f15bc4e6e992f8babbc863c12975b5a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 12 Nov 2020 20:01:54 +0000 (21:01 +0100)]
BUG/MINOR: peers: Missing TX cache entries reset.
The TX part of a cache for a dictionary is made of an reserved array of ebtree nodes
which are pointers to dictionary entries. So when we flush the TX part of such a
cache, we must not only remove these nodes to dictionary entries from their ebtree.
We must also reset their values. Furthermore, the LRU key and the last lookup
result must also be reset.
(cherry picked from commit
ea875e62e6b2f69c50533c5cd52eb5284c69723f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 12 Nov 2020 18:53:11 +0000 (19:53 +0100)]
BUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
If we could not decode the ID of a dictionary entry from a peer update message,
we must inform the remote peer about such an error as this is done for
any other decoding error.
(cherry picked from commit
f9e51beec118f1bbd558ed689fdad35046160529)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 10 Nov 2020 13:24:30 +0000 (14:24 +0100)]
BUG/MINOR: stats: free dynamically stats fields/lines on shutdown
Register a new function on POST DEINIT to free stats fields/lines for
each domain.
This patch does not fix a critical bug but may be backported to 2.3.
(cherry picked from commit
a2a6899bee5add180afb02ac2b1ce7cb360a96e3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Thierry Fournier [Tue, 10 Nov 2020 19:38:20 +0000 (20:38 +0100)]
BUG/MINOR: lua: set buffer size during map lookups
This size is used by some pattern matching to determine if there
is sufficient room in the buffer to add final \0 if necessary.
If the size is not set, the conditions use uninitialized value.
Note: it seems this bug can't cause a crash.
Should be backported until 2.2 (at least)
(cherry picked from commit
91dc0c0d8fdc2fb091b49699ebb323d01aa1d9f6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Thierry Fournier [Tue, 10 Nov 2020 19:51:36 +0000 (20:51 +0100)]
BUG/MINOR: pattern: a sample marked as const could be written
The functions add final 0 to string if the final 0 is not set,
but don't check the flag CONST. This patch duplicates the strings
if the final zero is not set and the string is CONST.
Should be backported until 2.2 (at least)
(cherry picked from commit
a68affeaa9377f88f773ef62a9bb2541dfb672d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 6 Nov 2020 15:24:07 +0000 (16:24 +0100)]
BUG/MEDIUM: ssl/crt-list: correctly insert crt-list line if crt already loaded
In issue #940, it was reported that the crt-list does not work correctly
anymore. Indeed when inserting a crt-list line which use a certificate
previously seen in the crt-list, this one won't be inserted in the SNI
list and will be silently ignored.
This bug was introduced by commit 47da821 "MEDIUM: ssl: emulates the
multi-cert bundles in the crtlist".
This patch also includes a reg-test which tests this issue.
This bugfix must be backported in 2.3.
(cherry picked from commit
50c03aac0417f7d70f98f31e513441c0fb743110)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Christopher Faulet [Fri, 6 Nov 2020 11:10:33 +0000 (12:10 +0100)]
DOC: config: Fix a typo on ssl_c_chain_der
There is a typo on the ssl_c_chain_der sample fetch
(s/ssl_c_der_chain/ssl_c_chain_der/). This implies a move of the fetch to keep
it at the right place.
This should be backported as far as 2.2 or anywhere the commit
a598b500b
("MINOR: ssl: add ssl_{c,s}_chain_der fetch methods") is.
(cherry picked from commit
70d10d1fb657eb6f8ac8e86c9b6037b53cc907ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 5 Nov 2020 21:43:41 +0000 (22:43 +0100)]
MINOR: http-htx: Add understandable errors for the errorfiles parsing
No details are provided when an error occurs during the parsing of an errorfile,
Thus it is a bit hard to diagnose where the problem is. Now, when it happens, an
understandable error message is reported.
This patch is not a bug fix in itself. But it will be required to change an
fatal error into a warning in last stable releases. Thus it must be backported
as far as 2.0.
(cherry picked from commit
a66adf41ea28a0fa29437d1675f225b5cc589b59)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 5 Nov 2020 18:38:05 +0000 (19:38 +0100)]
BUG/MINOR: ssl: don't report 1024 bits DH param load error when it's higher
The default dh_param value is 2048 and it's preset to zero unless explicitly
set, so we must not report a warning about DH param not being loadble in 1024
bits when we're going to use 2048. Thanks to Dinko for reporting this.
This should be backported to 2.2.
(cherry picked from commit
6d27a92b83f75bab42bda08ed28b70fb95525fd9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 5 Nov 2020 16:04:53 +0000 (17:04 +0100)]
[RELEASE] Released version 2.3.0
Released version 2.3.0 with the following main changes :
- CLEANUP: pattern: remove unused entry "tree" in pattern.val
- BUILD: ssl: use SSL_CTRL_GET_RAW_CIPHERLIST instead of OpenSSL versions
- BUG/MEDIUM: filters: Don't try to init filters for disabled proxies
- BUG/MINOR: proxy/server: Skip per-proxy/server post-check for disabled proxies
- BUG/MINOR: checks: Report a socket error before any connection attempt
- BUG/MINOR: server: Set server without addr but with dns in RMAINT on startup
- MINOR: server: Copy configuration file and line for server templates
- BUG/MEDIUM: mux-pt: Release the tasklet during an HTTP upgrade
- BUILD: ssl: use HAVE_OPENSSL_KEYLOG instead of OpenSSL versions
- MINOR: debug: don't count free(NULL) in memstats
- BUG/MINOR: filters: Skip disabled proxies during startup only
- MINOR: mux_h2: capitalize frame type in stats
- MINOR: mux_h2: add stat for total count of connections/streams
- MINOR: stats: do not display empty stat module title on html
- BUG/MEDIUM: stick-table: limit the time spent purging old entries
- BUG/MEDIUM: listener: only enable a listening listener if needed
- BUG/MEDIUM: listener: never suspend inherited sockets
- BUG/MEDIUM: listener: make the master also keep workers' inherited FDs
- MINOR: fd: add fd_want_recv_safe()
- MEDIUM: listeners: make use of fd_want_recv_safe() to enable early receivers
- REGTESTS: mark abns_socket as working now
- CLEANUP: mux-h2: Remove the h1 parser state from the h2 stream
- MINOR: sock: add a check against cross worker<->master socket activities
- CI: github actions: limit OpenSSL no-deprecated builds to "default,bug,devel" reg-tests
- BUG/MEDIUM: server: make it possible to kill last idle connections
- MINOR: mworker/cli: the master CLI use its own applet
- MINOR: ssl: define SSL_CTX_set1_curves_list to itself on BoringSSL
- BUILD: ssl: use feature macros for detecting ec curves manipulation support
- DOC: Add dns as an available domain to show stat
- BUILD: makefile: usual reorder of objects for faster builds
- DOC: update INSTALL to mention that TCC is supported
- DOC: mention in INSTALL that haproxy 2.3 is a stable version
- MINOR: version: mention that it's stable now
Willy Tarreau [Thu, 5 Nov 2020 15:50:53 +0000 (16:50 +0100)]
MINOR: version: mention that it's stable now
This version will be maintained up to around Q1 2022.
Willy Tarreau [Thu, 5 Nov 2020 16:00:07 +0000 (17:00 +0100)]
DOC: mention in INSTALL that haproxy 2.3 is a stable version
One day we'll have a script to update the status and support dates :-)
Willy Tarreau [Thu, 5 Nov 2020 15:56:37 +0000 (16:56 +0100)]
DOC: update INSTALL to mention that TCC is supported
TinyCC as found at https://repo.or.cz/tinycc.git does work to some extents
and is very convenient for developers, so let's mention it.
Willy Tarreau [Thu, 5 Nov 2020 15:42:25 +0000 (16:42 +0100)]
BUILD: makefile: usual reorder of objects for faster builds
Reordered the objets by reverse build times made the total build time
go down from 17.7s to 17.2s at -O2 using make -j8 on my PC, and from
~3.2 to ~2.7s on the build farm.
Daniel Corbett [Sun, 1 Nov 2020 15:54:17 +0000 (10:54 -0500)]
DOC: Add dns as an available domain to show stat
Within management.txt, proxy was listed as the only available option. "dns"
is now supported so let's add that. This change also updates the command to list
the available options <dns|proxy> for "domain" as previously it only specified
<domain>, which could be confusing as a user may think this field accepts
dynamic options when it actually requires a specific keyword.
Ilya Shipitsin [Tue, 3 Nov 2020 19:39:07 +0000 (00:39 +0500)]
BUILD: ssl: use feature macros for detecting ec curves manipulation support
Let us use SSL_CTX_set1_curves_list, defined by OpenSSL, as well as in
openssl-compat when SSL_CTRL_SET_CURVES_LIST is present (BoringSSL),
for feature detection instead of versions.
Willy Tarreau [Thu, 5 Nov 2020 14:03:45 +0000 (15:03 +0100)]
MINOR: ssl: define SSL_CTX_set1_curves_list to itself on BoringSSL
OpenSSL 1.0.2 and onwards define SSL_CTX_set1_curves_list which is both a
function and a macro. OpenSSL 1.0.2 to 1.1.0 define SSL_CTRL_SET_CURVES_LIST
as a macro, which disappeared from 1.1.1. BoringSSL only has that one and
not the former macro but it does have the function. Let's keep the test on
the macro matching the function name by defining the macro to itself when
needed.
William Lallemand [Thu, 5 Nov 2020 09:28:53 +0000 (10:28 +0100)]
MINOR: mworker/cli: the master CLI use its own applet
Following the patch b4daee ("MINOR: sock: add a check against cross
worker<->master socket activities"), this patch adds a dedicated applet
for the master CLI. It ensures that the CLI connection can't be
used with the master rights in the case of bugs.
Willy Tarreau [Thu, 5 Nov 2020 08:12:20 +0000 (09:12 +0100)]
BUG/MEDIUM: server: make it possible to kill last idle connections
In issue #933, @jaroslawr provided a report indicating that when using
many threads and many servers, it's very difficult to terminate the last
idle connections on each server. The issue has two causes in fact. The
first one is that during the calculation of the estimate of needed
connections, we round the computation up while in previous round it was
already rounded up, so we end up adding 1 to 1 which once divided by 2
remains 1. The second issue is that servers are not woken up anymore for
purging their connections if they don't have activity. The only reason
that was there to wake them up again was in case insufficient connections
were purged. And even then the purge task itself was not woken up. But
that is not enough for getting rid of the long tail of old connections
nor updating est_need_conns.
This patch makes sure to properly wake up as long as at least one idle
connection remains, and not to round up the needed connections anymore.
Prior to this patch, a test involving many connections which suddenly
stopped would keep many idle connections, now they're effectively halved
every pool-purge-delay.
This needs to be backported to 2.2.
Ilya Shipitsin [Tue, 3 Nov 2020 19:41:39 +0000 (00:41 +0500)]
CI: github actions: limit OpenSSL no-deprecated builds to "default,bug,devel" reg-tests
Willy Tarreau [Wed, 4 Nov 2020 13:58:36 +0000 (14:58 +0100)]
MINOR: sock: add a check against cross worker<->master socket activities
Given that the previous issues caused spurious worker socket wakeups in
the master for inherited FDs that couldn't be closed, let's add a strict
test in the I/O callback to make sure that an accept() event is always
caught by the appropriate type of process (master for master listeners,
worker for worker listeners).
Christopher Faulet [Tue, 3 Nov 2020 17:25:52 +0000 (18:25 +0100)]
CLEANUP: mux-h2: Remove the h1 parser state from the h2 stream
Since the h2 multiplexer no longer relies on the legacy HTTP representation, and
uses exclusively the HTX, the H1 parser state (h1m) is no longer used by the h2
streams. Thus it can be removed.
This patch may be backported as far as 2.1.
Willy Tarreau [Tue, 3 Nov 2020 17:43:48 +0000 (18:43 +0100)]
REGTESTS: mark abns_socket as working now
William noticed that the real issue in the abns test was that it was
failing to rebind some listeners, issues which were addressed in the
previous commits (in some cases, the master would even accept traffic
on the worker's socket which was not properly disabled, causing some
of the strange error messages).
The other issue documented in commit
2ea15a080 was the lack of
reliability when the test was run in parallel. This is caused by the
abns socket which uses a hard-coded address, making all tests in
parallel to step onto each others' toes. Since vtest cannot provide
abns sockets, we're instead concatenating the number of the listening
port that vtest allocated for another frontend to the abns path, which
guarantees to make them unique in the system.
The test works fine in all cases now, even with 100 in parallel.
Willy Tarreau [Wed, 4 Nov 2020 12:59:04 +0000 (13:59 +0100)]
MEDIUM: listeners: make use of fd_want_recv_safe() to enable early receivers
We used to refrain from calling fd_want_recv() if fd_updt was not allocated
but it's not the right solution as this does not allow the FD to be set.
Instead, let's use the new fd_want_recv_safe() which will update the FD and
create an update entry only if possible. In addition, the equivalent test
before calling fd_stop_recv() was removed as totally useless since there's
not fd_updt creation in this case.
Willy Tarreau [Wed, 4 Nov 2020 12:52:22 +0000 (13:52 +0100)]
MINOR: fd: add fd_want_recv_safe()
This does the same as fd_want_recv() except that it does check for
fd_updt[] to be allocated, as this may be called during early listener
initialization. Previously we used to check fd_updt[] before calling
fd_want_recv() but this is not correct since it does not update the
FD flags. This method will be safer.
Willy Tarreau [Tue, 3 Nov 2020 17:38:05 +0000 (18:38 +0100)]
BUG/MEDIUM: listener: make the master also keep workers' inherited FDs
In commit
374e9af35 ("MEDIUM: listener: let do_unbind_listener() decide
whether to close or not") it didn't appear necessary to have the master
process keep open the workers' inherited FDs. But this is actually
necessary to handle the reload on "bind fd@foo" situations, otherwise
the FD may be reassigned and the new socket cannot be set up, sometimes
causing "socket operation on non-socket" or other types of errors.
William found that this was the cause for the consistent failures of the
abns regtest, which already used to fail very often before this and was
as such marked as broken.
Interestingly I didn't have this issue with my test configs because
the FD number I used was higher and within the range of other listening
sockets. But this means that one of these wouldn't work as expected.
No backport is needed, this was introduced as part of the listeners
rework in 2.3.
Willy Tarreau [Wed, 4 Nov 2020 13:14:55 +0000 (14:14 +0100)]
BUG/MEDIUM: listener: never suspend inherited sockets
It is not acceptable to suspend an inherited socket because we'd kill
its listening state, making it possibly unrecoverable for future
processes. The situation which can trigger this is when there is an
abns socket in a config and an inherited FD on another listener. Upon
soft reload, the abns fails to bind, a SIGTTOU is sent to the old
process which suspends everything, including the inherited FD, then
the new process can bind and tell the old one to quit. Except that the
new FD was not set back to the listen state, which is detected by
listener_accept() which can pause it. It's only upon second reload
that the FD works again.
The solution is to refrain from suspending such FDs since we don't own
them. And the next process will get them right anyway from its config.
For now only TCP and UDP face this issue so it's better to address this
on a protocol basis
No backport is needed, this is related to the new listeners in 2.3.
Willy Tarreau [Wed, 4 Nov 2020 12:54:00 +0000 (13:54 +0100)]
BUG/MEDIUM: listener: only enable a listening listener if needed
The test on listener->state == LI_LISTEN is not sufficient to decide
if we need to enable a listener. Indeed, there is a very special case
which is the inherited FD shared, which has to reflect the real socket
state even after the previous test, and as such needs to remain in
LI_LISTEN state. In this case we don't want a worker to start the
master's listener nor conversely. Let's add a specific test for this.
Willy Tarreau [Tue, 3 Nov 2020 16:47:41 +0000 (17:47 +0100)]
BUG/MEDIUM: stick-table: limit the time spent purging old entries
An interesting case was reported with threads and moderately sized
stick-tables. Sometimes the watchdog would trigger during the purge.
It turns out that the stick tables were sized in the 10s of K entries
which is the order of magnitude of the possible number of connections,
and that threads were used over distinct NUMA nodes. While at first
glance nothing looks problematic there, actually there is a risk that
a thread trying to purge the table faces 100% of entries still in use
by a connection with (ts->ref_cnt > 0), and ends up scanning the whole
table, while other threads on the other NUMA node are causing the
cache lines to bounce back and forth and considerably slow down its
progress to the point of possibly spending hundreds of milliseconds
there, multiplied by the number of queued threads all failing on the
same point.
Interestingly, smaller tables would not trigger it because the scan
would be faster, and larger ones would not trigger it because plenty
of entries would be idle!
The most efficient solution is to increase the table size to be large
enough for this never to happen, but this is not reliable. We could
have a parallel list of idle entries but that would significantly
increase the storage and processing cost only to improve a few rare
corner cases.
This patch takes a more pragmatic approach, it considers that it will
not visit more than twice the number of nodes to be deleted, which
means that it accepts to fail up to 50% of the time. Given that very
small batches are programmed each time (1/256 of the table size), this
means the operation will finish quickly (128 times faster than now),
and will reduce the inter-thread contention. If this needs to be
reconsidered, it will probably mean that the batch size needs to be
fixed differently.
This needs to be backported to stable releases which extensively use
threads, typically 2.0.
Kudos to Nenad Merdanovic for figuring the root cause triggering this!
Amaury Denoyelle [Tue, 3 Nov 2020 14:04:46 +0000 (15:04 +0100)]
MINOR: stats: do not display empty stat module title on html
If a stat module is not available on the current proxy scope, do not
display its title on the related html box. This is clearer for the user.
Amaury Denoyelle [Tue, 3 Nov 2020 14:04:45 +0000 (15:04 +0100)]
MINOR: mux_h2: add stat for total count of connections/streams
Add counters for total number of http2 connections/stream since haproxy
startup. Contrary to open_conn/stream, they are never reset to zero.
Amaury Denoyelle [Tue, 3 Nov 2020 14:04:44 +0000 (15:04 +0100)]
MINOR: mux_h2: capitalize frame type in stats
http/2 frame type names are capitalized in the rfc, use the same
notation on the stats labels.
Christopher Faulet [Tue, 3 Nov 2020 15:40:37 +0000 (16:40 +0100)]
BUG/MINOR: filters: Skip disabled proxies during startup only
This partially reverts the patch
400829cd2 ("BUG/MEDIUM: filters: Don't try to
init filters for disabled proxies"). Disabled proxies must not be skipped in
flt_deinit() and flt_deinit_all_per_thread() when HAProxy is stopped because,
obvioulsy, at this step, all proxies appear as disabled (or stopped, it is the
same state). It is safe to do so because, during startup, filters declared on
disabled proxies are removed. Thus they don't exist anymore during shutdown.
This patch must be backported in all versions where the patch above is.
Willy Tarreau [Tue, 3 Nov 2020 14:59:23 +0000 (15:59 +0100)]
MINOR: debug: don't count free(NULL) in memstats
The mem stats are pretty convenient to spot leaks, except that they count
free(NULL) as 1, and the code does actually have quite a number of free(foo)
guards where foo is NULL if the object was already freed. Let's just not
count these ones so that the stats remain consistent. Now it's possible
to compare the strdup()/malloc() and free() and verify they are consistent.
Ilya Shipitsin [Tue, 3 Nov 2020 09:15:38 +0000 (14:15 +0500)]
BUILD: ssl: use HAVE_OPENSSL_KEYLOG instead of OpenSSL versions
let us use HAVE_OPENSSL_KEYLOG for feature detection instead
of versions
Christopher Faulet [Tue, 3 Nov 2020 08:11:43 +0000 (09:11 +0100)]
BUG/MEDIUM: mux-pt: Release the tasklet during an HTTP upgrade
When a TCP connection is upgraded to HTTP, the passthrough multiplexer owning
the client connection is detroyed and replaced by an HTTP multiplexer. When it
happens, the connection context is changed (it is in fact the mux itself). Thus,
when the mux-pt is destroyed, the connection is not released. But, only the
connection must be kept. Everything else concerning the mux must be
released. Especially, the tasklet used for I/O subscriptions. In this part,
there was a bug and the tasklet was never released.
This patch should fix the issue #935. It must be backported as far as 2.0.
Christopher Faulet [Mon, 2 Nov 2020 21:04:55 +0000 (22:04 +0100)]
MINOR: server: Copy configuration file and line for server templates
When servers based on server templates are initialized, the configuration file
and line are now copied. This helps to emit understandable warning and alert
messages.
This patch may be backported if needed, as far as 1.8.
Christopher Faulet [Mon, 26 Oct 2020 09:31:17 +0000 (10:31 +0100)]
BUG/MINOR: server: Set server without addr but with dns in RMAINT on startup
On startup, if a server has no address but the dns resolutions are configured,
"none" method is added to the default init-addr methods, in addition to "last"
and "libc". Thus on startup, this server is set to RMAINT mode if no address is
found. It is only performed if no other init-addr method is configured.
Setting the RMAINT mode on startup is important to inhibit the health checks.
For instance, following servers will now be set to RMAINT mode on startup :
server srv nofound.tld:80 check resolvers mydns
server srv _http._tcp.service.local check resolvers mydns
server-template srv 1-3 _http._tcp.service.local check resolvers mydns
while followings ones will trigger an error :
server srv nofound.tld:80 check
server srv nofound.tld:80 check resolvers mydns init-addr libc
server srv _http._tcp.service.local check
server srv _http._tcp.service.local check resolvers mydns init-addr libc
server-template srv 1-3 _http._tcp.service.local check resolvers mydns init-addr libc
This patch must be backported as far as 1.8.
Christopher Faulet [Mon, 26 Oct 2020 10:10:49 +0000 (11:10 +0100)]
BUG/MINOR: checks: Report a socket error before any connection attempt
When a health-check fails, if no connection attempt was performed, a socket
error must be reported. But this was only done if the connection was not
allocated. It must also be done if there is no control layer. Otherwise, a
L7TOUT will be reported instead.
It is possible to not having a control layer for a connection if the connection
address family is invalid or not defined.
This patch must be backported to 2.2.
Christopher Faulet [Mon, 2 Nov 2020 15:20:13 +0000 (16:20 +0100)]
BUG/MINOR: proxy/server: Skip per-proxy/server post-check for disabled proxies
per-proxy and per-server post-check callback functions must be skipped for
disabled proxies because most of the configuration validity check is skipped for
these proxies.
This patch must be backported as far as 2.1.
Christopher Faulet [Mon, 2 Nov 2020 15:08:09 +0000 (16:08 +0100)]
BUG/MEDIUM: filters: Don't try to init filters for disabled proxies
Configuration is parsed for such proxies but not validated. Concretely, it means
check_config_validity() function does almost nothing for such proxies. Thus, we
must be careful to not initialize filters for disabled proxies because the check
callback function is not called. In fact, to be sure to avoid any trouble,
filters for disabled proxies are released.
This patch fixes a segfault at startup if the SPOE is configured for a disabled
proxy. It must be backported as far as 1.7 (maybe with some adaptations).
Ilya Shipitsin [Fri, 30 Oct 2020 21:10:02 +0000 (02:10 +0500)]
BUILD: ssl: use SSL_CTRL_GET_RAW_CIPHERLIST instead of OpenSSL versions
let us use SSL_CTRL_GET_RAW_CIPHERLIST for feature detection instead
of versions
[wla: SSL_CTRL_GET_RAW_CIPHERLIST was introduced by OpenSSL commit
94a209 along with SSL_CIPHER_find. It was removed in boringSSL.]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Sun, 1 Nov 2020 20:14:39 +0000 (21:14 +0100)]
CLEANUP: pattern: remove unused entry "tree" in pattern.val
This one might have disappeared since patterns were reworked, but the
entry was not removed from the structure, let's do it now.
Willy Tarreau [Sat, 31 Oct 2020 12:17:06 +0000 (13:17 +0100)]
[RELEASE] Released version 2.3-dev9
Released version 2.3-dev9 with the following main changes :
- CLEANUP: http_ana: remove unused assignation of `att_beg`
- BUG/MEDIUM: ssl: OCSP must work with BoringSSL
- BUG/MINOR: log: fix memory leak on logsrv parse error
- BUG/MINOR: log: fix risk of null deref on error path
- BUILD: ssl: more elegant OpenSSL early data support check
- CI: github actions: update h2spec to 2.6.0
- BUG/MINOR: cache: Check the return value of http_replace_res_status
- MINOR: cache: Store the "Last-Modified" date in the cache_entry
- MINOR: cache: Process the If-Modified-Since header in conditional requests
- MINOR: cache: Create res.cache_hit and res.cache_name sample fetches
- MINOR: mux-h2: register a stats module
- MINOR: mux-h2: add counters instance to h2c
- MINOR: mux-h2: add stats for received frame types
- MINOR: mux-h2: report detected error on stats
- MINOR: mux-h2: count open connections/streams on stats
- BUG/MINOR: server: fix srv downtime calcul on starting
- BUG/MINOR: server: fix down_time report for stats
- BUG/MINOR: lua: initialize sample before using it
- MINOR: cache: Add Expires header value parsing
- MINOR: ist: Add a case insensitive istmatch function
- BUG/MINOR: cache: Manage multiple values in cache-control header value
- BUG/MINOR: cache: Inverted variables in http_calc_maxage function
- MINOR: pattern: make pat_ref_append() return the newly added element
- MINOR: pattern: make pat_ref_add() rely on pat_ref_append()
- MINOR: pattern: export pat_ref_push()
- CLEANUP: pattern: use calloc() rather than malloc for structures
- CLEANUP: pattern: fix spelling/grammatical/copy-paste in comments
Willy Tarreau [Fri, 30 Oct 2020 15:03:50 +0000 (16:03 +0100)]
CLEANUP: pattern: fix spelling/grammatical/copy-paste in comments
The code is horrible to work with because most functions are documented
with misleading comments resulting from many spelling and grammatical
mistakes, and plenty of remains of copy-paste mentioning arguments that
do not exist and return values that are never set. Too many hours wasted
writing non-working code because of assumptions resulting from this,
let's fix this once for all now!