haproxy-2.3.git
4 years agoBUG/MINOR: ssl: segv on startup when AKID but no keyid
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>

4 years agoDOC: add missing 3.10 in the summary
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>

4 years agoBUG/MINOR: http-ana: Don't wait for the body of CONNECT requests
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>

4 years agoBUG/MEDIUM: filters: Forward all filtered data at the end of http filtering
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>

4 years agoCLEANUP: cfgparse: remove duplicate registration for transparent build options
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>

4 years agoBUILD: http-htx: fix build warning regarding long type in printf
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>

4 years ago[RELEASE] Released version 2.3.1 v2.3.1
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

4 years agoREGTEST: 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>

4 years agoMINOR: peers: Add traces to peer_treat_updatemsg().
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>

4 years agoREGTEST: ssl: mark reg-tests/ssl/ssl_crt-list_filters.vtc as broken
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>

4 years agoREGTEST: ssl: test wildcard and multi-type + exclusions
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>

4 years agoMINOR: cfgparse: tighten the scope of newnameserver variable, free it on error.
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>

4 years agoMINOR: config/mux-h2: Return ERR_ flags from init_h2() instead of a status
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>

4 years agoMINOR: init: Fix the prototype for per-thread free callbacks
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>

4 years agoBUG/MINOR: tcpcheck: Don't warn on unused rules if check option is after
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>

4 years agoMINOR: spoe: Don't close connection in sync mode on processing timeout
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>

4 years agoBUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
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>

4 years agoBUG/MINOR: http-htx: Handle warnings when parsing http-error and http-errors
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>

4 years agoMINOR: check: report error on incompatible connect proto
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>

4 years agoMINOR: check: report error on incompatible proto
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>

4 years agoBUG/MEDIUM: check: reuse srv proto only if using same mode
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>

4 years agoBUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
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>

4 years agoBUG/MINOR: http-fetch: Extract cookie value even when no cookie name
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>

4 years agoBUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
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>

4 years agoBUG/MINOR: peers: Missing TX cache entries reset.
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>

4 years agoBUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
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>

4 years agoBUG/MINOR: stats: free dynamically stats fields/lines on shutdown
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>

4 years agoBUG/MINOR: lua: set buffer size during map lookups
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>

4 years agoBUG/MINOR: pattern: a sample marked as const could be written
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>

4 years agoBUG/MEDIUM: ssl/crt-list: correctly insert crt-list line if crt already loaded
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>

4 years agoDOC: config: Fix a typo on ssl_c_chain_der
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>

4 years agoMINOR: http-htx: Add understandable errors for the errorfiles parsing
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>

4 years agoBUG/MINOR: ssl: don't report 1024 bits DH param load error when it's higher
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>

4 years ago[RELEASE] Released version 2.3.0 v2.3.0
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

4 years agoMINOR: 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.

4 years agoDOC: mention in INSTALL that haproxy 2.3 is a stable version
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 :-)

4 years agoDOC: update INSTALL to mention that TCC is supported
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.

4 years agoBUILD: makefile: usual reorder of objects for faster builds
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.

4 years agoDOC: Add dns as an available domain to show stat
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.

4 years agoBUILD: ssl: use feature macros for detecting ec curves manipulation support
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.

4 years agoMINOR: ssl: define SSL_CTX_set1_curves_list to itself on BoringSSL
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.

4 years agoMINOR: mworker/cli: the master CLI use its own applet
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.

4 years agoBUG/MEDIUM: server: make it possible to kill last idle connections
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.

4 years agoCI: github actions: limit OpenSSL no-deprecated builds to "default,bug,devel" reg...
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

4 years agoMINOR: sock: add a check against cross worker<->master socket activities
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).

4 years agoCLEANUP: mux-h2: Remove the h1 parser state from the h2 stream
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.

4 years agoREGTESTS: mark abns_socket as working now
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.

4 years agoMEDIUM: listeners: make use of fd_want_recv_safe() to enable early receivers
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.

4 years agoMINOR: fd: add fd_want_recv_safe()
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.

4 years agoBUG/MEDIUM: listener: make the master also keep workers' inherited FDs
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.

4 years agoBUG/MEDIUM: listener: never suspend inherited sockets
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.

4 years agoBUG/MEDIUM: listener: only enable a listening listener if needed
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.

4 years agoBUG/MEDIUM: stick-table: limit the time spent purging old entries
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!

4 years agoMINOR: stats: do not display empty stat module title on html
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.

4 years agoMINOR: mux_h2: add stat for total count of connections/streams
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.

4 years agoMINOR: mux_h2: capitalize frame type in stats
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.

4 years agoBUG/MINOR: filters: Skip disabled proxies during startup only
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.

4 years agoMINOR: debug: don't count free(NULL) in memstats
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.

4 years agoBUILD: ssl: use HAVE_OPENSSL_KEYLOG instead of OpenSSL versions
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

4 years agoBUG/MEDIUM: mux-pt: Release the tasklet during an HTTP upgrade
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.

4 years agoMINOR: server: Copy configuration file and line for server templates
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.

4 years agoBUG/MINOR: server: Set server without addr but with dns in RMAINT on startup
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.

4 years agoBUG/MINOR: checks: Report a socket error before any connection attempt
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.

4 years agoBUG/MINOR: proxy/server: Skip per-proxy/server post-check for disabled proxies
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.

4 years agoBUG/MEDIUM: filters: Don't try to init filters for disabled proxies
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).

4 years agoBUILD: ssl: use SSL_CTRL_GET_RAW_CIPHERLIST instead of OpenSSL versions
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>

4 years agoCLEANUP: pattern: remove unused entry "tree" in pattern.val
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.

4 years ago[RELEASE] Released version 2.3-dev9 v2.3-dev9
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

4 years agoCLEANUP: 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!

4 years agoCLEANUP: pattern: use calloc() rather than malloc for structures
Willy Tarreau [Fri, 30 Oct 2020 14:35:11 +0000 (15:35 +0100)]
CLEANUP: pattern: use calloc() rather than malloc for structures

It's particularly difficult to make sure that the various pattern
structures are properly initialized given that they can be allocated
at multiple places and systematically via malloc() instead of calloc(),
thus not even leaving the possibility of default values. Let's adjust
a few of them.

4 years agoMINOR: pattern: export pat_ref_push()
Willy Tarreau [Wed, 28 Oct 2020 10:18:06 +0000 (11:18 +0100)]
MINOR: pattern: export pat_ref_push()

Strangely this one was marked static inline within the file itself.
Let's export it.

4 years agoMINOR: pattern: make pat_ref_add() rely on pat_ref_append()
Willy Tarreau [Wed, 28 Oct 2020 09:58:05 +0000 (10:58 +0100)]
MINOR: pattern: make pat_ref_add() rely on pat_ref_append()

Let's remove unneeded code duplication, both are exactly the same.

4 years agoMINOR: pattern: make pat_ref_append() return the newly added element
Willy Tarreau [Wed, 28 Oct 2020 09:52:46 +0000 (10:52 +0100)]
MINOR: pattern: make pat_ref_append() return the newly added element

It's more convenient to return the element than to return just 0 or 1,
as the next thing we'll want to do is to act on this element! In addition
it was using variable arguments instead of consts, causing some reuse
constraints which were also addressed. This doesn't change its use as
a boolean, hence why call places were not modified.

4 years agoBUG/MINOR: cache: Inverted variables in http_calc_maxage function
Remi Tricot-Le Breton [Fri, 30 Oct 2020 13:26:13 +0000 (14:26 +0100)]
BUG/MINOR: cache: Inverted variables in http_calc_maxage function

The maxage and smaxage variables were inadvertently assigned the
Cache-Control s-maxage and max-age values respectively when it should
have been the other way around.

This can be backported on all branches after 1.8 (included).

4 years agoBUG/MINOR: cache: Manage multiple values in cache-control header value
Remi Tricot-Le Breton [Wed, 28 Oct 2020 10:35:15 +0000 (11:35 +0100)]
BUG/MINOR: cache: Manage multiple values in cache-control header value

If an HTTP request or response had a "Cache-Control" header that had
multiple comma-separated subparts in its value (like "max-age=1,
no-store" for instance), we did not process the values correctly and
only parsed the first one. That made us store some HTTP responses in the
cache when they were explicitely uncacheable.
This patch replaces the way the values are parsed by an http_find_header
loop that manages every sub part of the value independently.

This patch should be backported to 2.2 and 2.1. The bug also exists on
previous versions but since the sources changed, a new commit will have
to be created.

[wla: This patch requires bb4582c ("MINOR: ist: Add a case insensitive
istmatch function"). Backporting for < 2.1 is not a requirement since it
works well enough for most cases, it was a known limitation of the
implementation of non-htx version too]

4 years agoMINOR: ist: Add a case insensitive istmatch function
Remi Tricot-Le Breton [Wed, 28 Oct 2020 10:35:14 +0000 (11:35 +0100)]
MINOR: ist: Add a case insensitive istmatch function

Add a helper function that checks if a string starts with another string
while ignoring case.

4 years agoMINOR: cache: Add Expires header value parsing
Remi Tricot-Le Breton [Wed, 28 Oct 2020 16:52:53 +0000 (17:52 +0100)]
MINOR: cache: Add Expires header value parsing

When no Cache-Control max-age or s-maxage information is present in a
cached response, we need to parse the Expires header value (RFC 7234#5.3).
An invalid Expires date value or a date earlier than the reception date
will make the cache_entry stale upon creation.
For now, the Cache-Control and Expires headers are parsed after the
insertion of the response in the cache so even if the parsing of the
Expires results in an already stale entry, the entry will exist in the
cache.

4 years agoBUG/MINOR: lua: initialize sample before using it
Amaury Denoyelle [Thu, 29 Oct 2020 16:21:20 +0000 (17:21 +0100)]
BUG/MINOR: lua: initialize sample before using it

Memset the sample before using it through hlua_lua2smp. This function is
ORing the smp.flags, so this field need to be cleared before its use.
This was reported by a coverity warning.

Fixes the github issue #929.
This bug can be backported up to 1.8.

4 years agoBUG/MINOR: server: fix down_time report for stats
Amaury Denoyelle [Thu, 29 Oct 2020 14:59:05 +0000 (15:59 +0100)]
BUG/MINOR: server: fix down_time report for stats

Adjust condition used to report down_time for statistics. There was a
tiny probabilty to have a negative downtime if last_change was superior
to now. If this is the case, return only down_time.

This bug can backported up to 1.8.

4 years agoBUG/MINOR: server: fix srv downtime calcul on starting
Amaury Denoyelle [Thu, 29 Oct 2020 14:59:04 +0000 (15:59 +0100)]
BUG/MINOR: server: fix srv downtime calcul on starting

When a server is up after a failure, its downtime was reset to 0 on the
statistics. This is due to a wrong condition that causes srv.down_time
to never be set. Fix this by updating down_time each time the server is in
STARTING state.

Fixes the github issue #920.
This bug can be backported up to 1.8.

4 years agoMINOR: mux-h2: count open connections/streams on stats
Amaury Denoyelle [Tue, 27 Oct 2020 16:16:04 +0000 (17:16 +0100)]
MINOR: mux-h2: count open connections/streams on stats

Implement as a gauge h2 counters for currently open connections and
streams. The counters are decremented when closing the stream or the
connection.

4 years agoMINOR: mux-h2: report detected error on stats
Amaury Denoyelle [Tue, 27 Oct 2020 16:16:03 +0000 (17:16 +0100)]
MINOR: mux-h2: report detected error on stats

Implement counters for h2 protocol error on connection or stream level.
Also count the total number of rst_stream and goaway frames sent by the
mux in response to a detected error.

4 years agoMINOR: mux-h2: add stats for received frame types
Amaury Denoyelle [Tue, 27 Oct 2020 16:16:02 +0000 (17:16 +0100)]
MINOR: mux-h2: add stats for received frame types

Implement counters for h2 frame received based on their type for
HEADERS, DATA, SETTINGS, RST_STREAM and GOAWAY.

4 years agoMINOR: mux-h2: add counters instance to h2c
Amaury Denoyelle [Tue, 27 Oct 2020 16:16:01 +0000 (17:16 +0100)]
MINOR: mux-h2: add counters instance to h2c

Add pointer to counters as a member for h2c structure. This pointer is
initialized on h2_init function. This is useful to quickly access and
manipulate the counters inside every h2 functions.

4 years agoMINOR: mux-h2: register a stats module
Amaury Denoyelle [Tue, 27 Oct 2020 16:16:00 +0000 (17:16 +0100)]
MINOR: mux-h2: register a stats module

Use statistics API to register a new stats module generating counters
on h2 module. The counters are attached to frontend/backend instances.

4 years agoMINOR: cache: Create res.cache_hit and res.cache_name sample fetches
Remi Tricot-Le Breton [Tue, 27 Oct 2020 10:55:57 +0000 (11:55 +0100)]
MINOR: cache: Create res.cache_hit and res.cache_name sample fetches

Res.cache_hit sample fetch returns a boolean which is true when the HTTP
response was built out of a cache. The cache's name is returned by the
res.cache_name sample_fetch.

This resolves GitHub issue #900.

4 years agoMINOR: cache: Process the If-Modified-Since header in conditional requests
Remi Tricot-Le Breton [Fri, 23 Oct 2020 08:51:28 +0000 (10:51 +0200)]
MINOR: cache: Process the If-Modified-Since header in conditional requests

If a client sends a conditional request containing an If-Modified-Since
header (and no If-None-Match header), we try to compare the date with
the one stored in the cache entry (coming either from a Last-Modified
head, or a Date header, or corresponding to the first response's
reception time). If the request's date is earlier than the stored one,
we send a "304 Not Modified" response back. Otherwise, the stored is sent
(through a 200 OK response).

This resolves GitHub issue #821.

4 years agoMINOR: cache: Store the "Last-Modified" date in the cache_entry
Remi Tricot Le Breton [Fri, 23 Oct 2020 08:51:27 +0000 (10:51 +0200)]
MINOR: cache: Store the "Last-Modified" date in the cache_entry

In order to manage "If-Modified-Since" requests, we need to keep a
reference time for our cache entries (to which the conditional request's
date will be compared).
This reference is either extracted from the "Last-Modified" header, or
the "Date" header, or the reception time of the response (in decreasing
order of priority).
The date values are converted into seconds since epoch in order to ease
comparisons and to limit storage space.

4 years agoBUG/MINOR: cache: Check the return value of http_replace_res_status
Tim Duesterhus [Thu, 22 Oct 2020 19:15:06 +0000 (21:15 +0200)]
BUG/MINOR: cache: Check the return value of http_replace_res_status

Send the full body if the status `304` cannot be applied. This should be the
most graceful failure.

Specific for 2.3, no backport needed.

4 years agoCI: github actions: update h2spec to 2.6.0
Ilya Shipitsin [Sun, 25 Oct 2020 14:36:02 +0000 (19:36 +0500)]
CI: github actions: update h2spec to 2.6.0

4 years agoBUILD: ssl: more elegant OpenSSL early data support check
Ilya Shipitsin [Sat, 24 Oct 2020 18:42:30 +0000 (23:42 +0500)]
BUILD: ssl: more elegant OpenSSL early data support check

BorinSSL pretends to be 1.1.1 version of OpenSSL. It messes some
version based feature presense checks. For example, OpenSSL specific
early data support.

Let us change that feature detction to SSL_READ_EARLY_DATA_SUCCESS
macro check instead of version comparision.

4 years agoBUG/MINOR: log: fix risk of null deref on error path
Willy Tarreau [Tue, 27 Oct 2020 09:35:32 +0000 (10:35 +0100)]
BUG/MINOR: log: fix risk of null deref on error path

Previous commit ae32ac74db ("BUG/MINOR: log: fix memory leak on logsrv
parse error") addressed one issue and introduced another one, the logsrv
pointer may also be null at the end of the function so we must test it
before deciding to dereference it.

This should be backported along with the patch above to 2.2.

4 years agoBUG/MINOR: log: fix memory leak on logsrv parse error
Willy Tarreau [Tue, 27 Oct 2020 08:51:37 +0000 (09:51 +0100)]
BUG/MINOR: log: fix memory leak on logsrv parse error

In case of parsing error on logsrv, we can leave parse_logsrv() without
releasing logsrv->ring_name or smp_rgs. Let's free them on the error path.
This should fix issue #926 detected by Coverity.

The impact is only a tiny leak just before reporting a fatal error, so it
will essentially annoy valgrind.

This can be backported to 2.0 (just drop the ring part).

4 years agoBUG/MEDIUM: ssl: OCSP must work with BoringSSL
Emmanuel Hocdet [Mon, 26 Oct 2020 12:55:30 +0000 (13:55 +0100)]
BUG/MEDIUM: ssl: OCSP must work with BoringSSL

It's a regression from b3201a3e "BUG/MINOR: disable dynamic OCSP load
with BoringSSL". The origin bug is link to 76b4a12 "BUG/MEDIUM: ssl:
memory leak of ocsp data at SSL_CTX_free()": ssl_sock_free_ocsp()
shoud be in #ifndef OPENSSL_IS_BORINGSSL.
To avoid long #ifdef for small code, the BoringSSL part for ocsp load
is isolated in a simple #ifdef.

This must be backported in 2.2 and 2.1

4 years agoCLEANUP: http_ana: remove unused assignation of `att_beg`
William Dauchy [Sun, 25 Oct 2020 13:01:33 +0000 (14:01 +0100)]
CLEANUP: http_ana: remove unused assignation of `att_beg`

`att_beg` is assigned to `next` at the end of the `for` loop, but is
assigned to `prev` at the beginning of the loop, which is itself
assigned to `next` after each loop. So it represents a double
assignation for the same value. Also `att_beg` is not used after the end
of the loop.

this is a partial fix for github issue #923, all the others could
probably be marked as intentional to protect future changes.

no backport needed.

Signed-off-by: William Dauchy <wdauchy@gmail.com>

4 years ago[RELEASE] Released version 2.3-dev8 v2.3-dev8
Willy Tarreau [Sat, 24 Oct 2020 11:14:31 +0000 (13:14 +0200)]
[RELEASE] Released version 2.3-dev8

Released version 2.3-dev8 with the following main changes :
    - MINOR: backend: replace the lbprm lock with an rwlock
    - MINOR: lb/map: use seek lock and read locks where appropriate
    - MINOR: lb/leastconn: only take a read lock in fwlc_get_next_server()
    - MINOR: lb/first: use a read lock in fas_get_next_server()
    - MINOR: lb/chash: use a read lock in chash_get_server_hash()
    - BUG/MINOR: disable dynamic OCSP load with BoringSSL
    - BUILD: ssl: make BoringSSL use its own version numbers
    - CLEANUP: threads: don't register an initcall when not debugging
    - MINOR: threads: change lock_t to an unsigned int
    - CLEANUP: tree-wide: reorder a few structures to plug some holes around locks
    - CLEANUP: task: remove the unused and mishandled global_rqueue_size
    - BUG/MEDIUM: connection: Never cleanup server lists when freeing private conns
    - MEDIUM: config: report that "nbproc" is deprecated
    - BUG/MINOR: listener: close before free in `listener_accept`
    - MINOR: ssl: 'ssl-load-extra-del-ext' removes the certificate extension
    - BUG/MINOR: queue: properly report redistributed connections
    - CONTRIB: tcploop: remove unused local variables in tcp_pause()
    - BUILD: makefile: add entries to build common debugging tools
    - BUG/MEDIUM: server: support changing the slowstart value from state-file
    - MINOR: http: Add `enum etag_type http_get_etag_type(const struct ist)`
    - MINOR: http: Add etag comparison function
    - MEDIUM: cache: Store the ETag information in the cache_entry
    - MEDIUM: cache: Add support for 'If-None-Match' request header
    - REGTEST: cache: Add if-none-match test case
    - CLEANUP: compression: Make use of http_get_etag_type()
    - BUG/MINOR: http-ana: Don't send payload for internal responses to HEAD requests
    - BUG/MAJOR: mux-h2: Don't try to send data if we know it is no longer possible
    - MINOR: threads/debug: only report used lock stats
    - MINOR: threads/debug: only report lock stats for used operations
    - MINOR: proxy; replace the spinlock with an rwlock
    - MINOR: server: read-lock the cookie during srv_set_dyncookie()
    - MINOR: proxy/cli: only take a read lock in "show errors"
    - OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued
    - MINOR: queue: split __pendconn_unlink() in per-srv and per-prx
    - MINOR: queue: reduce the locked area in pendconn_add()
    - OPTIM: queue: make the nbpend counters atomic
    - OPTIM: queue: decrement the nbpend and totpend counters outside of the lock
    - MINOR: leastconn: take the queue length into account when queuing servers
    - MEDIUM: fwlc: re-enable per-server queuing up to maxqueue
    - Revert "OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued"
    - MINOR: stats: support the "up" output modifier for "show stat"
    - MINOR: stats: also support a "no-maint" show stat modifier
    - MINOR: stats: indicate the number of servers in a backend's status
    - MEDIUM: ssl: ssl-load-extra-del-ext work only with .crt
    - REGTEST: ssl: test "set ssl cert" with separate key / crt
    - DOC: management: apply the "show stat" modifiers to "show stat", not "show info"
    - MINOR: stats: report server's user-configured weight next to effective weight
    - CI: travis-ci: switch to Ubuntu 20.04
    - CONTRIB: release-estimator: Add release estimating tool
    - BUG/MEDIUM: queue: fix unsafe proxy pointer when counting nbpend
    - BUG/MINOR: extcheck: add missing checks on extchk_setenv()

4 years agoBUG/MINOR: extcheck: add missing checks on extchk_setenv()
Willy Tarreau [Sat, 24 Oct 2020 11:07:39 +0000 (13:07 +0200)]
BUG/MINOR: extcheck: add missing checks on extchk_setenv()

Issue #910 reports that we fail to check a few extchk_setenv() in the
child process. These are mostly harmless, but instead of counting on
the external check script to fail the dirty way, better fail cleanly
when detecting the failure.

This could probably be backported to all stable branches.

4 years agoBUG/MEDIUM: queue: fix unsafe proxy pointer when counting nbpend
Willy Tarreau [Sat, 24 Oct 2020 10:57:41 +0000 (12:57 +0200)]
BUG/MEDIUM: queue: fix unsafe proxy pointer when counting nbpend

As reported by Coverity in issue #917, commit 96bca33 ("OPTIM: queue:
decrement the nbpend and totpend counters outside of the lock")
introduced a bug when moving the increments outside of the loop,
because we can't always rely on the pendconn "p" here as it may
be null. We can retrieve the proxy pointer directly from s->proxy
instead. The same is true for pendconn_redistribute(), though the
last "p" pointer there was still valid. This patch fixes both.

No backport is needed, this was introduced just before 2.3-dev8.

4 years agoCONTRIB: release-estimator: Add release estimating tool
Daniel Corbett [Thu, 22 Oct 2020 15:19:55 +0000 (11:19 -0400)]
CONTRIB: release-estimator: Add release estimating tool

This tool monitors the HAProxy stable branches and calculates a proposed
release date for the next minor release based on the bug fixes that are in
the queue.

Print only:
    ./release-estimator.py --print

Send email:
    ./release-estimator.py --send-mail --from-email from@domain.local --to-email to@domain.local

See contrib/release-estimator/README.md for details.

4 years agoCI: travis-ci: switch to Ubuntu 20.04
Ilya Shipitsin [Wed, 21 Oct 2020 10:12:29 +0000 (15:12 +0500)]
CI: travis-ci: switch to Ubuntu 20.04

we were blocked by https://github.com/vtest/VTest/issues/20
issue is resolved, let us update to focal