haproxy-2.3.git
4 years agoBUG/MEDIUM: resolvers: Reset server address and port for obselete SRV records
Christopher Faulet [Tue, 23 Feb 2021 11:24:09 +0000 (12:24 +0100)]
BUG/MEDIUM: resolvers: Reset server address and port for obselete SRV records

When a SRV record expires, the ip/port assigned to the associated server are
now removed. Otherwise, the server is stopped but keeps its ip/port while
the server hostname is removed. It is confusing when the servers state are
retrieve on the CLI and may be a problem if saved in a server-state
file. Because the reload may fail because of this inconsistency.

Here is an example:

 * Declare a server template in a backend, using the resolver <dns>

server-template test 2 _http._tcp.example.com resolvers dns check

 * 2 SRV records are announced with the corresponding additional
   records. Thus, 2 servers are filled. Here is the "show servers state"
   output :

2 frt 1 test1 192.168.1.1 2 64 0 1 2 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 2 64 0 1 1 15 3 4 6 0 0 0 http2.example.com 8002 _http._tcp.example.com 0 0 - - 0

 * Then, one additional record is removed (or a SRV record is removed, the
   result is the same). Here is the new "show servers state" output :

2 frt 1 test1 192.168.1.1 2 64 0 1 38 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 0 96 0 1 19 15 3 0 14 0 0 0 - 8002 _http._tcp.example.com 0 0 - - 0

On reload, if a server-state file is used, this leads to undefined behaviors
depending on the configuration.

This patch should be backported as far as 2.0.

(cherry picked from commit 52d4d3010991e851b8e9e4d9f923ad1f74d30d69)
[cf: Changes applied in src/dns.c]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: resolvers: new callback to properly handle SRV record errors
Baptiste Assmann [Thu, 19 Nov 2020 21:38:33 +0000 (22:38 +0100)]
BUG/MINOR: resolvers: new callback to properly handle SRV record errors

When a SRV record was created, it used to register the regular server name
resolution callbacks. That said, SRV records and regular server name
resolution don't work the same way, furthermore on error management.

This patch introduces a new call back to manage DNS errors related to
the SRV queries.

this fixes github issue #50.

Backport status: 2.3, 2.2, 2.1, 2.0

(cherry picked from commit b4badf720ce484001f606011aee7cd216e5ce4e3)
[cf: Changes applied in src/dns.c and structures renamed]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record
Christopher Faulet [Tue, 23 Feb 2021 11:22:29 +0000 (12:22 +0100)]
BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record

If no additional record is associated to a SRV record, its TTL must not be
renewed. Otherwise the entry never expires. Thus once announced a first
time, the entry remains blocked on the same IP/port except if a new announce
replaces the old one.

Now, the TTL is updated if a SRV record is received while a matching
existing one is found with an additional record or when an new additional
record is assigned to an existing SRV record.

This patch should be backported as far as 2.2.

(cherry picked from commit a331a1e8eb2ad4750711a477ca3e22d940495faf)
[cf: Changes applied in src/dns.c]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: resolvers: Fix condition to release received ARs if not assigned
Christopher Faulet [Tue, 23 Feb 2021 10:59:19 +0000 (11:59 +0100)]
BUG/MINOR: resolvers: Fix condition to release received ARs if not assigned

At the end of resolv_validate_dns_response(), if a received additionnal
record is not assigned to an existing server record, it is released. But the
condition to do so is buggy. If "answer_record" (the received AR) is not
assigned, "tmp_record" is not a valid record object. It is just a dummy
record "representing" the head of the record list.

Now, the condition is far cleaner. This patch must be backported as far as
2.2.

(cherry picked from commit 9c246a4b6ce3fa0e70399e0158866d41b8662a7f)
[cf: Changes applied in src/dns.c]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: fd: properly wait for !running_mask in fd_set_running_excl()
Willy Tarreau [Wed, 24 Feb 2021 18:40:49 +0000 (19:40 +0100)]
BUG/MINOR: fd: properly wait for !running_mask in fd_set_running_excl()

In fd_set_running_excl() we don't reset the old mask in the CAS loop,
so if we fail on the first round, we'll forcefully take the FD on the
next one.

In practice it's used bu fd_insert() and fd_delete() only, none of which
is supposed to be passed an FD which is still in use since in practice,
given that for now only listeners may be enabled on multiple threads at
once.

This can be backported to 2.2 but shouldn't result in fixing any user
visible bug for now.

(cherry picked from commit 5926e384e62d6e47272062f1fbec235bd11cc517)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: proxy: wake up all threads when sending the hard-stop signal
Willy Tarreau [Wed, 24 Feb 2021 10:13:59 +0000 (11:13 +0100)]
BUG/MINOR: proxy: wake up all threads when sending the hard-stop signal

The hard-stop event didn't wake threads up. In the past it wasn't an issue
as the poll timeout was limited to 1 second, but since commit 4f59d3861
("MINOR: time: increase the minimum wakeup interval to 60s") it has become
a problem because old processes can remain live for up to one minute after
the hard-stop-after delay. Let's just wake them up.

This may be backported to older releases, though before 2.4 the extra
delay was only one second.

(cherry picked from commit 0d03825b93cc59a289e838105f9d83d53ccdfc8b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: cli/shutdown sessions: make it thread-safe
Willy Tarreau [Wed, 24 Feb 2021 10:11:06 +0000 (11:11 +0100)]
BUG/MEDIUM: cli/shutdown sessions: make it thread-safe

There's no locking around the lookup of a stream nor its shutdown
when issuing "shutdown sessions" over the CLI so the risk of crashing
the process is particularly high.

Let's use a thread_isolate() there which is suitable for this task, and
there are not that many alternatives.

This must be backported to 1.8.

(cherry picked from commit 3f5dd2945ccbed6b4baf23a453e9d1d071a2d835)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: proxy: use thread-safe stream killing on hard-stop
Willy Tarreau [Wed, 24 Feb 2021 10:08:56 +0000 (11:08 +0100)]
BUG/MEDIUM: proxy: use thread-safe stream killing on hard-stop

When setting hard-stop-after, hard_stop() is called at the end to kill
last pending streams. Unfortunately there's no locking there while
walking over the streams list nor when shutting them down, so it's
very likely that some old processes have been crashing or gone wild
due to this. Let's use a thread_isolate() call for this as we don't
have much other choice (and it happens once in the process' life,
that's OK).

This must be backported to 1.8.

(cherry picked from commit 92b887e20a995323d3fbd42d2be035733cefd6ba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: vars: make functions vars_get_by_{name,desc} thread-safe
Dragan Dosen [Mon, 22 Feb 2021 16:20:01 +0000 (17:20 +0100)]
BUG/MEDIUM: vars: make functions vars_get_by_{name,desc} thread-safe

This patch adds a lock to functions vars_get_by_name() and
vars_get_by_desc() to protect accesses to the list of variables.

After the variable is fetched, a sample data is duplicated by using
smp_dup() because the variable may be modified by another thread.

This should be backported to all versions supporting vars along with
"BUG/MINOR: sample: secure convs that accept base64 string and var name
as args" which this patch depends on.

(cherry picked from commit 14518f2305027dfd537c1be0f88350337b5fba23)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: sample: secure convs that accept base64 string and var name as args
Dragan Dosen [Mon, 22 Feb 2021 09:03:53 +0000 (10:03 +0100)]
BUG/MINOR: sample: secure convs that accept base64 string and var name as args

This patch adds a few improvements in order to secure the use of
converters that accept base64 string and variable name as arguments.

The first change is within related function sample_conv_var2smp_str()
which now flags the sample as SMP_F_CONST if the argument is of type
ARGT_STR. This makes the sample more safe for later use.

A new function sample_check_arg_base64() is added. It checks an argument
and fills it with a variable type if the argument string contains a
valid variable name. If failed, it tries to perform a base64 decode
operation on a non-empty string, and fills the argument with the decoded
content which can be used later, without any additional base64dec()
function calls during runtime. This means that haproxy configuration
check may fail if variable lookup fails and an invalid base64 encoded
string is specified as an argument for such converters.

Both converters, "aes_gcm_dec" and "hmac", now use alloc_trash_chunk()
in order to allocate additional buffers for various conversions, and
avoid the use of a pre-allocated trash chunks directly (usually returned
by get_trash_chunk()). The function sample_check_arg_base64() is used
for both converters in order to check their arguments specified within
the haproxy configuration.

This patch should be backported as far as 2.0. However, it is important
to keep in mind a few things. The "hmac" converter is only available
starting with 2.2. In versions prior to 2.2, the "aes_gcm_dec" converter
and sample_conv_var2smp_str() are implemented in src/ssl_sock.c. Thus
the patch will have to be adapted on these versions.

Note that this patch is required for a subsequent, more important fix.

(cherry picked from commit 9e8db138c9e50262f2aae898bbc9b9b0b9a93449)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: Configure the `cpp` userdiff driver for *.[ch] in .gitattributes
Tim Duesterhus [Sat, 20 Feb 2021 18:21:35 +0000 (19:21 +0100)]
MINOR: Configure the `cpp` userdiff driver for *.[ch] in .gitattributes

This might improve the output of `git diff` in certain cases. Especially
`git diff --word-diff` will be much more useful.

Does not affect the generated code, may be backported for consistency if
desired.

(cherry picked from commit 6bcdc6530a437bb4eca24fb41439e34690376972)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: ssl/cli: potential null pointer dereference in "set ssl cert"
William Lallemand [Tue, 23 Feb 2021 13:45:45 +0000 (14:45 +0100)]
BUG/MINOR: ssl/cli: potential null pointer dereference in "set ssl cert"

A potential null pointer dereference was reported with an old gcc
version (6.5)

    src/ssl_ckch.c: In function 'cli_parse_set_cert':
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
      if (!ssl_sock_copy_cert_key_and_chain(src->ckch, dst->ckch))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
    src/ssl_ckch.c: In function 'ckchs_dup':
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
      if (!ssl_sock_copy_cert_key_and_chain(src->ckch, dst->ckch))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    src/ssl_ckch.c:838:7: error: potential null pointer dereference [-Werror=null-dereference]
    cc1: all warnings being treated as errors

This case does not actually happen but it's better to fix the ckch API
with a NULL check.

Could be backported as far as 2.1.

(cherry picked from commit 6c0961442c5e19a1bfc706374f96cfbd42feaeb2)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

4 years agoBUG/MEDIUM: mux-h1: Fix handling of responses to CONNECT other than 200-ok
Christopher Faulet [Mon, 22 Feb 2021 07:11:59 +0000 (08:11 +0100)]
BUG/MEDIUM: mux-h1: Fix handling of responses to CONNECT other than 200-ok

For a CONNECT request, if the tunnel establishment is refused by the server,
the connection is always closed on the client side. This happen because we
fail to detect the end of the tunnel. Now, when a reponse other than 200-ok
is received, the request is switch back to MSG_DONE state and the end of the
transaction is handled as a classical request/response exchange.

This patch should fix the issue #1140. It must be backported as far as
2.0. There is no upstream commit ID because tunnel management was already
fixed in a non-backportable way in 2.4.

4 years agoBUG/MINOR: server: Be sure to cut the last parsed field of a server-state line
Christopher Faulet [Fri, 19 Feb 2021 15:57:20 +0000 (16:57 +0100)]
BUG/MINOR: server: Be sure to cut the last parsed field of a server-state line

If a line of a server-state file has too many fields, the last one is not
cut on the first following space, as all other fileds. It contains all the
end of the line. It is not the expected behavior. So, now, we cut it on the
next following space, if any. The parsing loop was slighly rewritten.

Note that for now there is no error reported if the line is too long.

This patch may be backported at least as far as 2.1. On 2.0 and prior the
code is not the same. The line parsing is inlined in apply_server_state()
function.

(cherry picked from commit 868a5757e584431fafe713546c8ef8e799865476)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: server: Init params before parsing a new server-state line
Christopher Faulet [Fri, 19 Feb 2021 15:47:11 +0000 (16:47 +0100)]
BUG/MINOR: server: Init params before parsing a new server-state line

Same static arrays of parameters are used to parse all server-state
lines. Thus it is important to reinit them to be sure to not get params from
the previous line, eventually from the previous loaded file.

This patch should be backported to all stable branches. However, in 2.0 and
prior, the parsing of server-state lines are inlined in apply_server_state()
function. Thus the patch will have to be adapted on these versions.

(cherry picked from commit 06cd2569786d31ebb15832cb1053be9aaaadc3f7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: http-rules: Always replace the response status on a return action
Christopher Faulet [Fri, 19 Feb 2021 10:41:01 +0000 (11:41 +0100)]
BUG/MINOR: http-rules: Always replace the response status on a return action

When a HTTP return action is triggered, HAProxy is responsible to return the
response, based on the configured status code. On the request side, there is
no problem because there is no server response to replace. But on the
response side, we must take care to override the server response status
code, if any, to be sure to use the rigth status code to get the http reply
message.

In short, we must always set the configured status code of the HTTP return
action before returning the http reply to be sure to get the right reply,
the one base on the http return action status code and not a reply based on
the server response status code..

This patch should fix the issue #1139. It must be backported as far as 2.2.

(cherry picked from commit 2d36df275b1323cfae8105b5a39f6cfb96f50811)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: spoe: Resolve the sink if a SPOE logs in a ring buffer
Christopher Faulet [Fri, 19 Feb 2021 09:56:41 +0000 (10:56 +0100)]
BUG/MEDIUM: spoe: Resolve the sink if a SPOE logs in a ring buffer

If a SPOE filter is configured to send its logs to a ring buffer, the
corresponding sink must be resolved during the configuration post
parsing. Otherwise, the sink is undefined when a log message is emitted,
crashing HAProxy.

This patch must be backported as far as 2.2.

(cherry picked from commit 1d7d0f86b8a7f0d74b47e2afe08f87d9b963d9fb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: lists: Avoid an infinite loop in MT_LIST_TRY_ADDQ().
Olivier Houchard [Thu, 18 Feb 2021 22:55:30 +0000 (23:55 +0100)]
BUG/MEDIUM: lists: Avoid an infinite loop in MT_LIST_TRY_ADDQ().

In MT_LIST_TRY_ADDQ(), deal with the "prev" field of the element before the
"next". If the element is the first in the list, then its next will
already have been locked when we locked list->prev->next, so locking it
again will fail, and we'll start over and over.

This should be backported to 2.3.

(cherry picked from commit 5567f41d0ab61dd6843535edc8081407d599024d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: explain the relation between pool-low-conn and tune.idle-pool.shared
Willy Tarreau [Fri, 19 Feb 2021 10:45:22 +0000 (11:45 +0100)]
DOC: explain the relation between pool-low-conn and tune.idle-pool.shared

Disabling idle-pool sharing can result in awful performance in presence
of a not so high number of threads, because the number of available idle
connections will be shared among threads, resulting in most of them
abandonning their connections after a request is done if there are already
enough total available. This is a case where pool-low-conn ought to be
used to preserve a number of connections for each thread, but this relation
isn't obvious as is. Let's add mentions about this with both keywords.

(cherry picked from commit 0784db8566b5c4eea5b18bf533ef978a1909b97a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions
Ilya Shipitsin [Sat, 13 Feb 2021 06:45:33 +0000 (11:45 +0500)]
BUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions

SCTL (signed certificate timestamp list) specified in RFC6962
was implemented in c74ce24cd22e8c683ba0e5353c0762f8616e597d, let
us introduce macro HAVE_SSL_SCTL for the HAVE_SSL_SCTL sake,
which in turn is based on SN_ct_cert_scts, which comes in the same commit

(cherry picked from commit c47d676bd75e339f92faae430486ee337c45f134)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: sample: Always consider zero size string samples as unsafe
Christopher Faulet [Thu, 18 Feb 2021 09:22:48 +0000 (10:22 +0100)]
BUG/MINOR: sample: Always consider zero size string samples as unsafe

smp_is_safe() function is used to be sure a sample may be safely
modified. For string samples, a test is performed to verify if there is a
null-terminated byte. If not, one is added, if possible. It means if the
sample is not const and if there is some free space in the buffer, after
data. However, we must not try to read the null-terminated byte if the
string sample is too long (data >= size) or if the size is equal to
zero. This last test was not performed. Thus it was possible to consider a
string sample as safe by testing a byte outside the buffer.

Now, a zero size string sample is always considered as unsafe and is
duplicated when smp_make_safe() is called.

This patch must be backported in all stable versions.

(cherry picked from commit 8dd40fbde9d51cf7bf0ee622a5bc5c1f56048d84)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: checks: don't needlessly take the server lock in health_adjust()
Willy Tarreau [Wed, 17 Feb 2021 14:20:19 +0000 (15:20 +0100)]
BUG/MEDIUM: checks: don't needlessly take the server lock in health_adjust()

The server lock was taken preventively for anything in health_adjust(),
including the static config checks needed to detect that the lock was not
needed, while the function is always called on the response path to update
a server's status. This was responsible for huge contention causing a
performance drop of about 17% on 16 threads. Let's move the lock only
where it should be, i.e. inside the function around the critical sections
only. By doing this, a 16-thread process jumped back from 575 to 675 krps.

This should be backported to 2.3 as the situation degraded there, and
maybe later to 2.2.

(cherry picked from commit 4e9df2737dde3f6d1d171fa17e2594c4c765c00c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: checks: properly handle wrapping time in __health_adjust()
Willy Tarreau [Wed, 17 Feb 2021 14:15:15 +0000 (15:15 +0100)]
BUG/MINOR: checks: properly handle wrapping time in __health_adjust()

There's an issue when a server state changes, we use an integer comparison
to decide whether or not to reschedule a test instead of using a wrapping
timer comparison. This will cause some health-checks not to be immediately
triggered half of the time, and some unneeded calls to task_queue() to be
performed in other cases.

This bug has always been there as it was introduced with the commit that
added the feature, 97f07b832 ("[MEDIUM] Decrease server health based on
http responses / events, version 3"). This may be backported everywhere.

(cherry picked from commit 64ba5ebadcd5d98e00989d08dfaa3c94c15196c9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: session: atomically increment the tracked sessions counter
Willy Tarreau [Tue, 16 Feb 2021 17:08:12 +0000 (18:08 +0100)]
BUG/MINOR: session: atomically increment the tracked sessions counter

In session_count_new() the tracked counter was still incremented with
a "++" outside of any lock, resulting in occasional slightly off values
such as the following:

    # table: foo, type: string, size:1000, used:1
    0xb2a398: key=127.1.2.3 use=0 exp=86398318 sess_cnt=999959 http_req_cnt=1000004

Now with the correct atomic increment:

    # table: foo, type: string, size:1000, used:1
    0x7f82a4026d38: key=127.1.2.3 use=0 exp=86399294 sess_cnt=1000004 http_req_cnt=1000004

This can be backported to 1.8.

(cherry picked from commit 9805859f245f4f59fc3baa098cb349786e21aaba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: server: Remove RMAINT from admin state when loading server state
Christopher Faulet [Fri, 12 Feb 2021 16:36:08 +0000 (17:36 +0100)]
BUG/MINOR: server: Remove RMAINT from admin state when loading server state

The RMAINT admin state is dynamic and should be remove from the
srv_admin_state parameter when a server state is loaded from a server-state
file. Otherwise an erorr is reported, the server-state line is ignored and
the server state is not updated.

This patch should fix the issue #576. It must be backported as far as 1.8.

(cherry picked from commit eaab7325a797e61d16a80b2969ab5f9cbd9679c5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCLEANUP: channel: fix comment in ci_putblk.
Emeric Brun [Mon, 11 Jan 2021 09:30:42 +0000 (10:30 +0100)]
CLEANUP: channel: fix comment in ci_putblk.

The comment is outdated and refer to an old code.

Should be backported until branch 1.5

(cherry picked from commit 147b3f05b5d5c3ec33f9d3ef2a6f79bbda1b3617)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: tune: explain the origin of block size for ssl.cachesize
William Dauchy [Fri, 12 Feb 2021 14:58:46 +0000 (15:58 +0100)]
DOC: tune: explain the origin of block size for ssl.cachesize

A user could eventually ask himself where those 200 bytes block size are
coming from. This patch tries to better explain the origin in case
people are curious or want to double check the reality.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 9a4bbfe151b8db72ef4f353b5a1c5e1d60b20646)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: server: Don't call fopen() with server-state filepath set to NULL
Christopher Faulet [Fri, 12 Feb 2021 15:31:03 +0000 (16:31 +0100)]
BUG/MINOR: server: Don't call fopen() with server-state filepath set to NULL

When a local server-state file is loaded, if its name is too long, the error
is not properly handled, resulting to a call to fopen() with the "filepath"
variable set to NULL. To fix the bug, when this error occurs, we jump to the
next proxy, via a "continue" statement. And we take case to set "filepath"
variable after the error handling to be sure.

This patch should fix the issue #1111. It must be backported as far as 1.6.

(cherry picked from commit 8952ea636b03fceb44c8172c4d9725a393e9146d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: cfgparse: do not mention "addr:port" as supported on proxy lines
Willy Tarreau [Fri, 12 Feb 2021 12:28:22 +0000 (13:28 +0100)]
BUG/MINOR: cfgparse: do not mention "addr:port" as supported on proxy lines

The very old error message indicating that a proxy name is mandatory
still had a reference to the optional addr:port argument while this one
is explicitly rejected a few lines later since at least 1.9.

This is harmless but confusing. This can be backported to 2.0.

(cherry picked from commit b2ec994523067bc03c0e631e4f5d4ba8dae02cb9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: stats: revert the change on ST_CONVDONE
Willy Tarreau [Fri, 12 Feb 2021 10:49:25 +0000 (11:49 +0100)]
BUG/MINOR: stats: revert the change on ST_CONVDONE

In 2.1, commit ee4f5f83d ("MINOR: stats: get rid of the ST_CONVDONE flag")
introduced a subtle bug. By testing curproxy against defproxy in
check_config_validity(), it tried to eliminate the need for a flag
to indicate that stats authentication rules were already compiled,
but by doing so it left the issue opened for the case where a new
defaults section appears after the two proxies sharing the first
one:

      defaults
          mode http
          stats auth foo:bar

      listen l1
          bind :8080

      listen l2
          bind :8181

      defaults
          # just to break above

This config results in:
  [ALERT] 042/113725 (3121) : proxy 'f2': stats 'auth'/'realm' and 'http-request' can't be used at the same time.
  [ALERT] 042/113725 (3121) : Fatal errors found in configuration.

Removing the last defaults remains OK. It turns out that the cleanups
that followed that patch render it useless, so the best fix is to revert
the change (with the up-to-date flags instead). The flag was marked as
belonging to the config. It's not exact but it's the closest to the
reality, as it's not there to configure the behavior but ti mention
that the config parser did its job.

This could be backported as far as 2.1, but in practice it looks like
nobody ever hit it.

(cherry picked from commit 5bbc676608f654ae76c7a4cc5852a443bfe8bd41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: config: don't pick unset values from last defaults section
Willy Tarreau [Fri, 12 Feb 2021 10:14:35 +0000 (11:14 +0100)]
BUG/MEDIUM: config: don't pick unset values from last defaults section

Since commit 1.3.14 with commit 1fa3126ec ("[MEDIUM] introduce separation
between contimeout, and tarpit + queue"), check_config_validity() looks
at the last defaults section to update all proxies' queue and tarpit
timeouts if they were not set!

This was apparently an attempt to properly set them on the fallback values,
except that the fallback values were taken from the default proxy before
looking at the current proxy itself. The worst part of it is that it might
have randomly worked by accident for some configurations when there was a
single defaults section, but has certainly caused too short queue
expirations once another defaults section was added later in the file with
these explicitly defined.

Let's remove the defproxy part and keep only the curproxy ones. This could
be backported everywhere, the bug has been there for 13 years.

(cherry picked from commit 937c3ead34becd6851572a8280831d760f612a09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCLEANUP: deinit: release global and per-proxy server-state variables on deinit
Christopher Faulet [Fri, 12 Feb 2021 08:28:13 +0000 (09:28 +0100)]
CLEANUP: deinit: release global and per-proxy server-state variables on deinit

The global server-state base directory and file name are now released on
deinit, as well as per-proxy server-state file name.

(cherry picked from commit f5ea269723a205c22d1ac9fd40b8d7fab5cb47ed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: server: Fix server-state-file-name directive
Christopher Faulet [Fri, 12 Feb 2021 08:27:10 +0000 (09:27 +0100)]
BUG/MINOR: server: Fix server-state-file-name directive

Since the beginning, this directive is documented to accept an optional file
name. But it should also be possible to use it without any argument to use
the backend name as file name. However, when no argument is provided, an
error is reported during the configuration parsing requesting an argument, a
file name or "use-backend-name". And This last special argument is not
documented.

So, to respect the documentation and to avoid configuration breakages, all
modes are now supported. If this directive is called with no argument or
with "use-backend-name", the backend name is use as file name for the
server-state file. Otherwise, the provided string is used.

In addition, we take care to release any previously allocated file name in
case this directive is defines multiple times in the same backend. And an
error is reported if more than one argument are defined. Finally, the
documentation is updated accordingly. Sections supporting this directive are
also mentioned.

This patch should be backported as far as 1.6.

(cherry picked from commit 583b6de68aa1a1070ac3b9c5e21605916aed2de0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: backend: hold correctly lock when killing idle conn
Amaury Denoyelle [Thu, 28 Jan 2021 09:16:29 +0000 (10:16 +0100)]
BUG/MINOR: backend: hold correctly lock when killing idle conn

The wrong lock seems to be held when trying to remove another thread
connection if max fd limit has been reached (locking the current thread
instead of the target thread lock).

This could be backported up to 2.0.

(cherry picked from commit a3bf62ec541479531ebe93bde46b436cb95c9a87)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: tools: Fix a memory leak on error path in parse_dotted_uints()
Christopher Faulet [Thu, 11 Feb 2021 09:42:41 +0000 (10:42 +0100)]
BUG/MINOR: tools: Fix a memory leak on error path in parse_dotted_uints()

When an invalid character is found during parsing in parse_dotted_uints()
function, the allocated array of uint must be released. This patch fixes a
memory leak on error path during the configuration parsing.

This patch should fix the issue #1106. It should be backported as far as
2.0. Note that, for 2.1 and 2.0, the function is in src/standard.c

(cherry picked from commit 4b524124db9dc6e64b4e0f0882b5fc71d24970e0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: server: re-align state file fields number
William Dauchy [Mon, 8 Feb 2021 22:53:29 +0000 (23:53 +0100)]
BUG/MINOR: server: re-align state file fields number

Since commit 3169471964fdc49963e63f68c1fd88686821a0c4 ("MINOR: Add
server port field to server state file.") max_fields was not increased
on version number 1. So this patch aims to fix it. This should be
backported as far as v1.8, but the numbering should be adpated depending
on the version: simply increase the field by 1.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 38cd986c54975add4e14ef0f693dff494e36336d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: mux-h1: Always set CS_FL_EOI for response in MSG_DONE state
Christopher Faulet [Mon, 8 Feb 2021 16:18:01 +0000 (17:18 +0100)]
BUG/MEDIUM: mux-h1: Always set CS_FL_EOI for response in MSG_DONE state

During the message parsing, if in MSG_DONE state, the CS_FL_EOI flag must
always be set on the conn-stream if following conditions are met :

  * It is a response or
  * It is a request but not a protocol upgrade nor a CONNECT.

For now, there is no test on the message type (request or response). Thus
the CS_FL_EOI flag is not set for a response with a "Connection: upgrade"
header but not a 101 response.

This bug was introduced by the commit 3e1748bbf ("BUG/MINOR: mux-h1: Don't
set CS_FL_EOI too early for protocol upgrade requests"). It was backported
as far as 2.0. Thus, this patch must also be backported as far as 2.0.

(cherry picked from commit a22782b597ee9a3bfecb18a66e29633c8e814216)
[cf: context adjustments]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: http-ana: Don't increment HTTP error counter on internal errors
Christopher Faulet [Wed, 10 Feb 2021 13:58:01 +0000 (14:58 +0100)]
BUG/MINOR: http-ana: Don't increment HTTP error counter on internal errors

If internal error is reported by the mux during HTTP request parsing, the
HTTP error counter should not be incremented. It should only be incremented
on parsing error to reflect errors caused by clients.

This patch must be backported as far as 2.0. During the backport, the same
must be performed for 408-request-time-out errors.

(cherry picked from commit bf7175f9b6480fa25e859e226a1f460744e163cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: intops: fix mul32hi()'s off-by-one
Willy Tarreau [Tue, 9 Feb 2021 16:10:54 +0000 (17:10 +0100)]
BUG/MINOR: intops: fix mul32hi()'s off-by-one

mul32hi() multiples a constant a with a variable b from 0 to 0xffffffff
and shifts the result by 32 bits. It's visible that it's always impossible
to reach the constant a this way because the product always misses exactly
one unit of a to be preserved. And this cannot be corrected by the caller
either as adding one to the output will only shift the output range, and
it's not possible to pass 2^32 on the ratio <b>. The right approach is to
add "a" after the multiplication so that the input range is always
preserved for all ratio values from 0 to 0xffffffff:

     (a=0x00000000 * b=0x00000000 + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000000 * b=0x00000001 + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000000 * b=0xffffffff + a=0x00000000) >> 32 = 0x00000000
     (a=0x00000001 * b=0x00000000 + a=0x00000001) >> 32 = 0x00000000
     (a=0x00000001 * b=0x00000001 + a=0x00000001) >> 32 = 0x00000000
     (a=0x00000001 * b=0xffffffff + a=0x00000001) >> 32 = 0x00000001
     (a=0xffffffff * b=0x00000000 + a=0xffffffff) >> 32 = 0x00000000
     (a=0xffffffff * b=0x00000001 + a=0xffffffff) >> 32 = 0x00000001
     (a=0xffffffff * b=0xffffffff + a=0xffffffff) >> 32 = 0xffffffff

This is only used in freq_ctr calculations and the slightly lower value
is unlikely to have ever been noticed by anyone. This may be backported
though it is not important.

(cherry picked from commit e66ee1a65133bfa64370d841d2b6de3e50ca376e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: ssl: guard SSL_CTX_set_msg_callback with SSL_CTRL_SET_MSG_CALLBACK macro
Ilya Shipitsin [Mon, 8 Feb 2021 11:55:06 +0000 (16:55 +0500)]
BUILD: ssl: guard SSL_CTX_set_msg_callback with SSL_CTRL_SET_MSG_CALLBACK macro

both SSL_CTX_set_msg_callback and SSL_CTRL_SET_MSG_CALLBACK defined since
ea262260469e49149cb10b25a87dfd6ad3fbb4ba, we can safely switch to that guard
instead of OpenSSL version

(cherry picked from commit 7ff7747a1750cc416c3731cf53858011e02dd546)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: ssl: guard SSL_CTX_add_server_custom_ext with special macro
Ilya Shipitsin [Sat, 6 Feb 2021 13:59:22 +0000 (18:59 +0500)]
BUILD: ssl: guard SSL_CTX_add_server_custom_ext with special macro

special guard macros HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT was defined earlier
exactly for guarding SSL_CTX_add_server_custom_ext, let us use it wherever
appropriate

(cherry picked from commit f00cdb18563e6f57a4ad6c0b40e0116ca9c8fd69)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: ssl: fix typo in HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT macro
Ilya Shipitsin [Sat, 6 Feb 2021 13:55:27 +0000 (18:55 +0500)]
BUILD: ssl: fix typo in HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT macro

HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT was introduced in ec609098718b9c1cd803ca57442b2b98c9ba4a16
however it was defined as HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT (missing "S")
let us fix typo

(cherry picked from commit 7bbf5866e011d22ee8f1d00471ce330cc72dcd0e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: check: do not ignore a connection header for http-check send
Amaury Denoyelle [Tue, 22 Dec 2020 13:08:52 +0000 (14:08 +0100)]
MINOR: check: do not ignore a connection header for http-check send

Allow the user to specify a custom Connection header for http-check
send. This is useful for example to implement a websocket upgrade check.

If no connection header has been set, a 'Connection: close' header is
automatically appended to allow the server to close the connection
immediately after the request/response.

Update the documentation related to http-check send.

This fixes the github issue #1009.

(cherry picked from commit 6d975f0af650e51c5c8e584d9b6beb413deb6868)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years ago[RELEASE] Released version 2.3.5 v2.3.5
Willy Tarreau [Sat, 6 Feb 2021 09:49:57 +0000 (10:49 +0100)]
[RELEASE] Released version 2.3.5

Released version 2.3.5 with the following main changes :
    - BUG/MINOR: init: Use a dynamic buffer to set HAPROXY_CFGFILES env variable
    - MINOR: config: Add failifnotcap() to emit an alert on proxy capabilities
    - MINOR: server: Forbid server definitions in frontend sections
    - BUG/MINOR: threads: Fixes the number of possible cpus report for Mac.
    - MINOR: peers: Add traces for peer control messages.
    - BUG/MINOR: dns: SRV records ignores duplicated AR records (v2)
    - BUILD: peers: fix build warning about unused variable
    - BUG/MEDIUM: stats: add missing INF_BUILD_INFO definition
    - BUG/MINOR: peers: Possible appctx pointer dereference.
    - MINOR: build: discard echoing in help target
    - BUG/MINOR: peers: Wrong "new_conn" value for "show peers" CLI command.
    - BUG/MINOR: mux_h2: missing space between "st" and ".flg" in the "show fd" helper
    - BUG/MINOR: mworker: define _GNU_SOURCE for strsignal()
    - BUG/MEDIUM: tcpcheck: Don't destroy connection in the wake callback context
    - BUG/MEDIUM: mux-h2: fix read0 handling on partial frames
    - BUILD/MINOR: lua: define _GNU_SOURCE for LLONG_MAX
    - DOC: Improve documentation of the various hdr() fetches
    - BUG/MEDIUM: filters/htx: Fix data forwarding when payload length is unknown
    - BUG/MINOR: config: fix leak on proxy.conn_src.bind_hdr_name
    - BUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()
    - BUG/MEDIUM: session: only retrieve ready idle conn from session
    - REORG: backend: simplify conn_backend_get
    - BUG/MEDIUM: backend: never reuse a connection for tcp mode
    - BUG/MINOR: backend: check available list allocation for reuse
    - MINOR: contrib: Make the wireshark peers dissector compile for more distribs.
    - CLEANUP: tools: make resolve_sym_name() take a const pointer
    - CLEANUP: cli: make "show fd" use a const connection to access other fields
    - MINOR: cli: make "show fd" also report the xprt and xprt_ctx
    - MINOR: xprt: add a new show_fd() helper to complete some "show fd" dumps.
    - MINOR: ssl: provide a "show fd" helper to report important SSL information
    - MINOR: xprt/mux: export all *_io_cb functions so that "show fd" resolves them
    - MINOR: mux-h2: make the "show fd" helper also decode the h2s subscriber when known
    - MINOR: mux-h1: make the "show fd" helper also decode the h1s subscriber when known
    - MINOR: mux-fcgi: make the "show fd" helper also decode the fstrm subscriber when known
    - MINOR: cli: give the show_fd helpers the ability to report a suspicious entry
    - MINOR: cli/show_fd: report some easily detectable suspicious states
    - MINOR: ssl/show_fd: report some FDs as suspicious when possible
    - MINOR: mux-h2/show_fd: report as suspicious an entry with too many calls
    - MINOR: mux-h1/show_fd: report as suspicious an entry with too many calls
    - MINOR: h1: Raise the chunk size limit up to (2^52 - 1)
    - DOC: management: fix "show resolvers" alphabetical ordering
    - BUG/MINOR: stick-table: Always call smp_fetch_src() with a valid arg list
    - BUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store
    - BUG/MEDIUM: ssl: check a connection's status before computing a handshake
    - BUG/MINOR: mux_h2: fix incorrect stat titles
    - BUG/MINOR: xxhash: make sure armv6 uses memcpy()
    - BUG/MINOR: ssl: do not try to use early data if not configured
    - BUILD: ssl: fix build breakage with last commit
    - MINOR: cli/show_fd: report local and report ports when known
    - BUILD: Makefile: move REGTESTST_TYPE default setting
    - BUG/MEDIUM: mux-h2: handle remaining read0 cases
    - BUG/MEDIUM: mux-h2: do not quit the demux loop before setting END_REACHED
    - BUG/MINOR: sock: Unclosed fd in case of connection allocation failure
    - MINOR: config: Deprecate and ignore tune.chksize global option

4 years agoMINOR: config: Deprecate and ignore tune.chksize global option
Christopher Faulet [Wed, 25 Nov 2020 16:20:57 +0000 (17:20 +0100)]
MINOR: config: Deprecate and ignore tune.chksize global option

This option is now ignored because I/O check buffers are now allocated using the
buffer pool. Thus, it is marked as deprecated in the documentation and ignored
during the configuration parsing. The field is also removed from the global
structure.

Because this option is ignored since a recent fix, backported as fare as 2.2,
this patch should be backported too. Especially because it updates the
documentation.

(cherry picked from commit bb9fb8b7f832d3a2cb62f5b4046390ae7943fc16)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: sock: Unclosed fd in case of connection allocation failure
Remi Tricot-Le Breton [Thu, 14 Jan 2021 14:26:24 +0000 (15:26 +0100)]
BUG/MINOR: sock: Unclosed fd in case of connection allocation failure

If allocating a connection object failed right after a successful accept
on a listener, the new file descriptor was not properly closed.

This fixes GitHub issue #905.
It can be backported to 2.3.

(cherry picked from commit 25dd0ad1239627aa87d9ce25d268546037a97197)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: mux-h2: do not quit the demux loop before setting END_REACHED
Willy Tarreau [Fri, 5 Feb 2021 11:16:01 +0000 (12:16 +0100)]
BUG/MEDIUM: mux-h2: do not quit the demux loop before setting END_REACHED

The demux loop could quit on missing data but the H2_CF_END_REACHED flag
would not be set in this case. This fixes a remaining situation where
previous commit f09612289 ("BUG/MEDIUM: mux-h2: handle remaining read0
cases") could not be sufficient and still leave CLOSE_WAIT. It's harder
to reproduce but was still observed in prod.

Now we quit via the end of the loop which already takes care of shutr.

This should be backported along with the patch above as far as 2.0.

(cherry picked from commit 133aaa9f110f5b78e57a23f9db0553e2978eca0e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: mux-h2: handle remaining read0 cases
Willy Tarreau [Fri, 5 Feb 2021 10:41:46 +0000 (11:41 +0100)]
BUG/MEDIUM: mux-h2: handle remaining read0 cases

Commit 3d4631fec ("BUG/MEDIUM: mux-h2: fix read0 handling on partial
frames") tried to address an issue introduced in commit aade4edc1 where
read0 wasn't properly handled in the middle of a frame. But the fix was
incomplete for two reasons:

  - first, it would set H2_CF_RCVD_SHUT in h2_recv() after detecting
    a read0 but the condition was guarded by h2_recv_allowed() which
    explicitly excludes read0 ;

  - second, h2_process would only call h2_process_demux() when there
    were still data in the buffer, but closing after a short pause to
    leave a buffer empty wouldn't be caught in this case.

This patch fixes this by properly taking care of the received shutdown
and by also waking up h2_process_demux() on an empty buffer if the demux
is not blocked.

Given the patches above were tagged for backporting to 2.0, this one
should be as well.

(cherry picked from commit f09612289f4a6e358524df385473323ea4254883)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUILD: Makefile: move REGTESTST_TYPE default setting
William Lallemand [Fri, 5 Feb 2021 10:27:54 +0000 (11:27 +0100)]
BUILD: Makefile: move REGTESTST_TYPE default setting

In patch 3bad3d5 ("BUILD: Makefile: exclude broken tests by default"),
the default setting of the REGTESTST_TYPE variable was set in the
Makefile instead of the run-regtests.sh script.

Doing it in the Makefile was breaking the use of this environment
varible with make ( REGTESTS_TYPES=slow,default make reg-tests )

This patch move the default setting from the Makefile to
run-regtests.sh. It also change the documentation in `make
reg-tests-help` about the default value.

This patch should be backported where 3bad3d5 is backported.

(cherry picked from commit c1ddcafdf9b425ff4ac2f1e3f1662a9133fc4cf6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli/show_fd: report local and report ports when known
Willy Tarreau [Fri, 5 Feb 2021 09:54:52 +0000 (10:54 +0100)]
MINOR: cli/show_fd: report local and report ports when known

FD dumps are not always easy to match against netstat dumps, and often
require an lsof as a third dump. Let's emit the socket family, and the
local and remore ports when the FD is an IPv4/IPv6 socket, this will
significantly ease the matching.

(cherry picked from commit ed9892018c327c61342bf95b2d83f44f27bfc7c4)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUILD: ssl: fix build breakage with last commit
Willy Tarreau [Fri, 5 Feb 2021 09:24:54 +0000 (10:24 +0100)]
BUILD: ssl: fix build breakage with last commit

Bah I messed up the test of commit 5930fb23e ("BUG/MINOR: ssl: do not
try to use early data if not configured") for 2.3, I ran regtest without
rebuilding and naturally it wouldn't have built.

Now fixed and tested again. If the commit above is backported further,
this one will be needed as well.

4 years agoBUG/MINOR: ssl: do not try to use early data if not configured
Willy Tarreau [Wed, 3 Feb 2021 10:21:38 +0000 (11:21 +0100)]
BUG/MINOR: ssl: do not try to use early data if not configured

The CO_FL_EARLY_SSL_HS flag was inconditionally set on the connection,
resulting in SSL_read_early_data() always being used first in handshake
calculations. While this seems to work well (probably that there are
fallback paths inside openssl), it's particularly confusing and makes
the debugging quite complicated. It possibly is not optimal by the way.

This flag ought to be set only when early_data is configured on the bind
line. Apparently there used to be a good reason for doing it this way in
1.8 times, but it really does not make sense anymore. It may be OK to
backport this to 2.3 if this helps with troubleshooting, but better not
go too far as it's unlikely to fix any real issue while it could introduce
some in old versions.

(cherry picked from commit a84986ae4f27f5335aedd5216ee3f4db06db74d0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: xxhash: make sure armv6 uses memcpy()
Willy Tarreau [Thu, 4 Feb 2021 16:02:39 +0000 (17:02 +0100)]
BUG/MINOR: xxhash: make sure armv6 uses memcpy()

There was a special case made to allow ARMv6 to use unaligned accesses
via a cast in xxHash when __ARM_FEATURE_UNALIGNED is defined. But while
ARMv6 (and v7) does support unaligned accesses, it's only for 32-bit
pointers, not 64-bit ones, leading to bus errors when the compiler emits
an ldrd instruction and the input (e.g. a pattern) is not aligned, as in
issue #1035.

Note that v7 was properly using the packed approach here and was safe,
however haproxy versions 2.3 and older use the old r39 xxhash code which
has the same issue for armv7. A slightly different fix is required there,
by using a different definition of packed for 32 and 64 bits.

The problem is really visible when running v7 code on a v8 kernel because
such kernels do not implement alignment trap emulation, and the process
dies when this happens. This is why in the issue above it was only detected
under lxc. The emulation could have been disabled on v7 as well by writing
zero to /proc/cpu/alignment though.

This commit is a backport of xxhash commit a470f2ef ("update default memory
access for armv6").

Thanks to @srkunze for the report and tests, @stgraber for his help on
setting up an easy reproducer outside of lxc, and @Cyan4973 for the
discussion around the best way to fix this. Details and alternate patches
available on https://github.com/Cyan4973/xxHash/issues/490.

(cherry picked from commit 4acb99f8672232753adb36e57b45e80e5bd87783)
[wt: used the different version suitable for backpotring, using the
 distinct packed settings]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: mux_h2: fix incorrect stat titles
Amaury Denoyelle [Wed, 3 Feb 2021 15:27:22 +0000 (16:27 +0100)]
BUG/MINOR: mux_h2: fix incorrect stat titles

Duplicate titles for the stats H2_ST_{OPEN,TOTAL}_{CONN,STREAM}. These
entries are used on csv for the heading.

This must be backported up to 2.3.

This fixes the github issue #1102.

(cherry picked from commit 377d8786a79dad763ccdd40afbd52dccff9c9980)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: ssl: check a connection's status before computing a handshake
Willy Tarreau [Tue, 2 Feb 2021 14:42:25 +0000 (15:42 +0100)]
BUG/MEDIUM: ssl: check a connection's status before computing a handshake

As spotted in issue #822, we're having a problem with error detection in
the SSL layer. The problem is that on an overwhelmed machine, accepted
connections can start to pile up, each of them requiring a slow handshake,
and during all this time if the client aborts, the handshake will still be
calculated.

The error controls are properly placed, it's just that the SSL layer
reads records exactly of the advertised size, without having the ability
to encounter a pending connection error. As such if injecting many TLS
connections to a listener with a huge backlog, it's fairly possible to
meet this situation:

  12:50:48.236056 accept4(8, {sa_family=AF_INET, sin_port=htons(62794), sin_addr=inet_addr("127.0.0.1")}, [128->16], SOCK_NONBLOCK) = 1109
  12:50:48.236071 setsockopt(1109, SOL_TCP, TCP_NODELAY, [1], 4) = 0
  (process other connections' handshakes)

  12:50:48.257270 getsockopt(1109, SOL_SOCKET, SO_ERROR, [ECONNRESET], [4]) = 0
  (proof that error was detectable there but this code was added for the PoC)

  12:50:48.257297 recvfrom(1109, "\26\3\1\2\0", 5, 0, NULL, NULL) = 5
  12:50:48.257310 recvfrom(1109, "\1\0\1\3"..., 512, 0, NULL, NULL) = 512

  (handshake calculation taking 700us)

  12:50:48.258004 sendto(1109, "\26\3\3\0z"..., 1421, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = -1 EPIPE (Broken pipe)
  12:50:48.258036 close(1109)             = 0

The situation was amplified by the multi-queue accept code, as it resulted
in many incoming connections to be accepted long before they could be
handled. Prior to this they would have been accepted and the handshake
immediately started, which would have resulted in most of the connections
waiting in the the system's accept queue, and dying there when the client
aborted, thus the error would have been detected before even trying to
pass them to the handshake code.

As a result, with a listener running on a very large backlog, it's possible
to quickly accept tens of thousands of connections and waste time slowly
running their handshakes while they get replaced by other ones.

This patch adds an SO_ERROR check on the connection's FD before starting
the handshake. This is not pretty as it requires to access the FD, but it
does the job.

Some improvements should be made over the long term so that the transport
layers can report extra information with their ->rcv_buf() call, or at the
very least, implement a ->get_conn_status() function to report various
flags such as shutr, shutw, error at various stages, allowing an upper
layer to inquire for the relevance of engaging into a long operation if
it's known the connection is not usable anymore. An even simpler step
could probably consist in implementing this in the control layer.

This patch is simple enough to be backported as far as 2.0.

Many thanks to @ngaugler for his numerous tests with detailed feedback.

(cherry picked from commit 0630038e771d4d08ae726080e2ef240d5ddaba68)
[wt: context adjustments]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store
William Lallemand [Mon, 1 Feb 2021 14:31:00 +0000 (15:31 +0100)]
BUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store

The "abort ssl cert" command is buggy and removes the current ckch store,
and instances, leading to SNI removal. It must only removes the new one.

This patch also adds a check in set_ssl_cert.vtc and
set_ssl_server_cert.vtc.

Must be backported as far as 2.2.

(cherry picked from commit 8695ce0bae21238eba660438c819797a245be71e)
[wt: dropped reg-tests/ssl/set_ssl_server_cert.vtc]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: stick-table: Always call smp_fetch_src() with a valid arg list
Christopher Faulet [Fri, 29 Jan 2021 09:27:47 +0000 (10:27 +0100)]
BUG/MINOR: stick-table: Always call smp_fetch_src() with a valid arg list

The sample fetch functions must always be called with a valid argument
list. When called by hand, if there is no argument to pass, empty_arg_list must
be used.

In the stick-table code, there are some calls to smp_fetch_src() with NULL as
argument list. It is changed to use empty_arg_list instead. It is not really a
bug because smp_fetch_src() does not use the argument list. But it is an API
bug.

This patch may be backported to all stable branches as a cleanup.

(cherry picked from commit bdbd5db2a50461fceb7fb89329e73d08f90e44fd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoDOC: management: fix "show resolvers" alphabetical ordering
Willy Tarreau [Fri, 29 Jan 2021 11:01:46 +0000 (12:01 +0100)]
DOC: management: fix "show resolvers" alphabetical ordering

Not sure why it was located between "show ssl" and "show table"...
This should be backported.

(cherry picked from commit 87ef32397101c7bc4783982af952da12c1750e88)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: h1: Raise the chunk size limit up to (2^52 - 1)
Christopher Faulet [Wed, 27 Jan 2021 14:17:13 +0000 (15:17 +0100)]
MINOR: h1: Raise the chunk size limit up to (2^52 - 1)

The allowed chunk size was historically limited to 2GB to avoid risk of
overflow. This restriction is no longer necessary because the chunk size is
immediately stored into a 64bits integer after the parsing. Thus, it is now
possible to raise this limit. However to never fed possibly bogus values
from languages that use floats for their integers, we don't get more than 13
hexa-digit (2^52 - 1). 4 petabytes is probably enough !

This patch should fix the issue #1065. It may be backported as far as
2.1. For the 2.0, the legacy HTTP part must be reviewed. But there is
honestely no reason to do so.

(cherry picked from commit 405f05465252498429fb9dc38db40f0803f7cb69)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h1/show_fd: report as suspicious an entry with too many calls
Willy Tarreau [Thu, 21 Jan 2021 08:13:35 +0000 (09:13 +0100)]
MINOR: mux-h1/show_fd: report as suspicious an entry with too many calls

An FD entry that maps to an H1 connection whose stream was woken
up more than 1M times is now flagged as suspicious.

(cherry picked from commit 0c0c0a287834283eaaf5f25437a39a8f059e9878)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h2/show_fd: report as suspicious an entry with too many calls
Willy Tarreau [Thu, 21 Jan 2021 08:13:35 +0000 (09:13 +0100)]
MINOR: mux-h2/show_fd: report as suspicious an entry with too many calls

An FD entry that maps to an H2C connection whose last stream was woken
up more than 1M times is now flagged as suspicious.

(cherry picked from commit 06bf83e0ae101567796e05630ae227fb18d4e3e8)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: ssl/show_fd: report some FDs as suspicious when possible
Willy Tarreau [Thu, 21 Jan 2021 07:53:50 +0000 (08:53 +0100)]
MINOR: ssl/show_fd: report some FDs as suspicious when possible

If a subscriber's tasklet was called more than one million times, if
the ssl_ctx's connection doesn't match the current one, or if the
connection appears closed in one direction while the SSL stack is
still subscribed, the FD is reported as suspicious. The close cases
may occasionally trigger a false positive during very short and rare
windows. Similarly the 1M calls will trigger after 16GB are transferred
over a given connection. These are rare enough events to be reported as
suspicious.

(cherry picked from commit 4bd5d630ace94ee6b4cbb7375ec75f50a927a826)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli/show_fd: report some easily detectable suspicious states
Willy Tarreau [Thu, 21 Jan 2021 08:07:29 +0000 (09:07 +0100)]
MINOR: cli/show_fd: report some easily detectable suspicious states

A file descriptor which maps to a connection but has more than one
thread in its mask, or an FD handle that doesn't correspond to the FD,
or wiht no mux context, or an FD with no thread in its mask, or with
more than 1 million events is flagged as suspicious.

(cherry picked from commit dacfde4ba42e3a13dd5b13b7df7a37db06d702a6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli: give the show_fd helpers the ability to report a suspicious entry
Willy Tarreau [Thu, 21 Jan 2021 07:26:06 +0000 (08:26 +0100)]
MINOR: cli: give the show_fd helpers the ability to report a suspicious entry

Now the show_fd helpers at the transport and mux levels return an integer
which indicates whether or not the inspected entry looks suspicious. When
an entry is reported as suspicious, "show fd" will suffix it with an
exclamation mark ('!') in the dump, that is supposed to help detecting
them.

For now, helpers were adjusted to adapt to the new API but none of them
reports any suspicious entry yet.

(cherry picked from commit 8050efeacb554f798a49db5ef213c8f673e5596e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-fcgi: make the "show fd" helper also decode the fstrm subscriber when...
Willy Tarreau [Wed, 20 Jan 2021 16:10:46 +0000 (17:10 +0100)]
MINOR: mux-fcgi: make the "show fd" helper also decode the fstrm subscriber when known

When dumping a live fcgi stream, also take the opportunity for reporting
the subscriber including the event, tasklet, handler and context.

(cherry picked from commit 1776ffb975017f91a12a9c6404bd3cb73789c7c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h1: make the "show fd" helper also decode the h1s subscriber when known
Willy Tarreau [Wed, 20 Jan 2021 16:05:58 +0000 (17:05 +0100)]
MINOR: mux-h1: make the "show fd" helper also decode the h1s subscriber when known

When dumping a live h1 stream, also take the opportunity for reporting
the subscriber including the event, tasklet, handler and context. Example:

   3030 : st=0x21(R:rA W:Ra) ev=0x04(heOpi) [Lc] tmask=0x4 umask=0x0 owner=0x7f97805c1f70 iocb=0x65b847(sock_conn_iocb) back=1 cflg=0x00002300 sv=s1/recv mux=H1 ctx=0x7f97805c21b0 h1c.flg=0x80000200 .sub=1 .ibuf=0@(nil)+0/0 .obuf=0@(nil)+0/0 h1s=0x7f97805c2380 h1s.flg=0x4010 .req.state=MSG_DATA .res.state=MSG_RPBEFORE .meth=POST status=0 .cs.flg=0x00000000 .cs.data=0x7f97805c1720 .subs=0x7f97805c1748(ev=1 tl=0x7f97805c1990 tl.calls=2 tl.ctx=0x7f97805c1720 tl.fct=si_cs_io_cb) xprt=RAW

(cherry picked from commit 150c4f8b729749a074ccdde75457fbcf8c8728f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h2: make the "show fd" helper also decode the h2s subscriber when known
Willy Tarreau [Wed, 20 Jan 2021 15:27:01 +0000 (16:27 +0100)]
MINOR: mux-h2: make the "show fd" helper also decode the h2s subscriber when known

When dumping a valid h2 stream, also dump the subscriber, its events,
tasklet context and handler. Example:

    128 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x1 umask=0x0 owner=0x7f40380d7370 iocb=0x65b71b(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x1ad23e0 h2c.st0=FRP .err=0 .maxid=3 .lastid=-1 .flg=0x10000 .nbst=2 .nbcs=2 .fctl_cnt=0 .send_cnt=0 .tree_cnt=2 .orph_cnt=0 .sub=1 .dsi=3 .dbuf=16366@0x1ea9380+16441/16448 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] last_h2s=0x20a8340 .id=3 .st=OPN .flg=0x4100 .rxbuf=0@(nil)+0/0 .cs=0x20a8440(.flg=0x00100000 .data=0x20a8738) .subs=0x20a8760(ev=1 tl=0x20a89b0 tl.calls=22 tl.ctx=0x20a8738 tl.fct=si_cs_io_cb) xprt=SSL xprt_ctx=0x1aaf4c0 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x1ad28e0(ev=1 tl=0x1ab3c70 tl.calls=176 tl.ctx=0x1ad23e0 tl.fct=h2_io_cb) .sent_early=0 .early_in=0

(cherry picked from commit 98e40b98180632ac2cd479b8e1900e6b698121f5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: xprt/mux: export all *_io_cb functions so that "show fd" resolves them
Willy Tarreau [Wed, 20 Jan 2021 13:55:01 +0000 (14:55 +0100)]
MINOR: xprt/mux: export all *_io_cb functions so that "show fd" resolves them

In FD dumps it's often very important to figure what upper layer function
is going to be called. Let's export the few I/O callbacks that appear as
tasklet functions so that "show fd" can resolve them instead of printing
a pointer relative to main. For example:

   1028 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x2 umask=0x2 owner=0x7f00b889b200 iocb=0x65b638(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x7f00c8824de0 h2c.st0=FRH .err=0 .maxid=795 .lastid=-1 .flg=0x0000 .nbst=0 .nbcs=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=0 .orph_cnt=0 .sub=1 .dsi=795 .dbuf=0@(nil)+0/0 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] xprt=SSL xprt_ctx=0x7f00c86d0750 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x7f00c88252e0(ev=1 tl=0x7f00a07d1aa0 tl.calls=1047 tl.ctx=0x7f00c8824de0 tl.fct=h2_io_cb) .sent_early=0 .early_in=0

(cherry picked from commit 691d503896f2dc4944782cfe989fc8b44d66a6c0)
[wt: context adjustments; dropped quic]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: ssl: provide a "show fd" helper to report important SSL information
Willy Tarreau [Wed, 20 Jan 2021 13:41:29 +0000 (14:41 +0100)]
MINOR: ssl: provide a "show fd" helper to report important SSL information

The SSL context contains a lot of important details that are currently
missing from debug outputs. Now that we detect ssl_sock, we can perform
some sanity checks, print the next xprt, the subscriber callback's context,
handler and number of calls. The process function is also resolved. This
now gives for example on an H2 connection:

   1029 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x2 umask=0x2 owner=0x7fc714881700 iocb=0x65b528(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x7fc734545e50 h2c.st0=FRH .err=0 .maxid=217 .lastid=-1 .flg=0x0000 .nbst=0 .nbcs=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=0 .orph_cnt=0 .sub=1 .dsi=217 .dbuf=0@(nil)+0/0 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] xprt=SSL xprt_ctx=0x7fc73478f230 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x7fc734546350(ev=1 tl=0x7fc7346702e0 tl.calls=278 tl.ctx=0x7fc734545e50 tl.fct=main-0x144efa) .sent_early=0 .early_in=0

(cherry picked from commit de5675a38c50a5c5d79c261f303c52ff9df241b3)
[wt: context adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: xprt: add a new show_fd() helper to complete some "show fd" dumps.
Willy Tarreau [Wed, 20 Jan 2021 14:30:56 +0000 (15:30 +0100)]
MINOR: xprt: add a new show_fd() helper to complete some "show fd" dumps.

Just like we did for the muxes, now the transport layers will have the
ability to provide helpers to report more detailed information about their
internal context. When the helper is not known, the pointer continues to
be dumped as-is if it's not NULL. This way a transport with no context nor
dump function will not add a useless "xprt_ctx=(nil)" but the pointer will
be emitted if valid or if a helper is defined.

(cherry picked from commit 108a27104960da8196eb6ce3686a2b65be3c6067)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli: make "show fd" also report the xprt and xprt_ctx
Willy Tarreau [Wed, 20 Jan 2021 13:40:04 +0000 (14:40 +0100)]
MINOR: cli: make "show fd" also report the xprt and xprt_ctx

These ones are definitely missing from some dumps, let's report them! We
print the xprt's name instead of its useless pointer, as well as its ctx
when xprt is not NULL.

(cherry picked from commit 37be9534245152130fad75c74133c3111a2f382b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoCLEANUP: cli: make "show fd" use a const connection to access other fields
Willy Tarreau [Wed, 20 Jan 2021 13:13:46 +0000 (14:13 +0100)]
CLEANUP: cli: make "show fd" use a const connection to access other fields

Over time the code has uglified, casting fdt.owner as a struct connection
for about everything. Let's have a const struct connection* there and take
this opportunity for passing all fields as const as well.

Additionally a misplaced closing parenthesis on the output was fixed.

(cherry picked from commit eb0595d039b5e5c56bf3f574ec7e364d926c406b)
[wt: s/sock_conn_iocb/conn_fd_handler]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoCLEANUP: tools: make resolve_sym_name() take a const pointer
Willy Tarreau [Wed, 20 Jan 2021 13:37:59 +0000 (14:37 +0100)]
CLEANUP: tools: make resolve_sym_name() take a const pointer

When 0c439d895 ("BUILD: tools: make resolve_sym_name() return a const")
was written, the pointer argument ought to have been turned to const for
more flexibility. Let's do it now.

(cherry picked from commit 45fd1030d5eebb318fbcf70adadfcc1ff6b261b3)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: contrib: Make the wireshark peers dissector compile for more distribs.
Frédéric Lécaille [Tue, 19 Jan 2021 13:33:24 +0000 (14:33 +0100)]
MINOR: contrib: Make the wireshark peers dissector compile for more distribs.

With a 2.6.8 wireshark, this module could not compile because of ws_version.h
missing header. This patch offers the possibility to compile this plugin without
having to include this header. Furthermore with my wireshark version a
"plugin_release" object is required to make it be loaded by wireshark. This is
a string which seems to have to match a dotted string made of you wireshark
major and minor versions.

(cherry picked from commit aab6f7c3e642aa5e298a9d8765b5063abf518bfb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: backend: check available list allocation for reuse
Amaury Denoyelle [Thu, 28 Jan 2021 16:33:26 +0000 (17:33 +0100)]
BUG/MINOR: backend: check available list allocation for reuse

Do not consider reuse connection if available list is not allocated for
the target server. This will prevent a crash when using a standalone
server for an external purpose like socket_tcp/socket_ssl on hlua code.
For the idle/safe lists, they are considered allocated if
srv.max_idle_conns is not null.

Note that the hlua code is currently safe thanks to the additional
checks on proxy http mode and stream reuse policy not never. However,
this might not be sufficient for future code.

This patch should be backported in every branches containing the
following patch :
  7f68d815af356fbe1b2e1080a88b9935581c91d2 (2.4 tree)
  REORG: backend: simplify conn_backend_get

(cherry picked from commit a81bb7197eb538628274bffe3b09e87e564aeb89)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: backend: never reuse a connection for tcp mode
Amaury Denoyelle [Tue, 26 Jan 2021 16:35:46 +0000 (17:35 +0100)]
BUG/MEDIUM: backend: never reuse a connection for tcp mode

The reuse of idle connections should only happen for a proxy with the
http mode. In case of a backend with the tcp mode, the reuse selection
and insertion in session list are skipped.

This behavior is present since commit :
MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.

(cherry picked from commit 08d87b3f49867440f66aee09173c84bf58cbc859)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoREORG: backend: simplify conn_backend_get
Amaury Denoyelle [Tue, 26 Jan 2021 13:35:26 +0000 (14:35 +0100)]
REORG: backend: simplify conn_backend_get

Reorganize the conditions for the reuse of idle/safe connections :
- reduce code by using variable to store reuse mode and idle/safe conns
  counts
- consider that idle/safe/avail lists are properly allocated if
  max_idle_conns not null. An allocation failure prevents haproxy
  startup.

(cherry picked from commit 7f68d815af356fbe1b2e1080a88b9935581c91d2)
[wt: harmless, backported since needed for the next one after careful
     review]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: session: only retrieve ready idle conn from session
Amaury Denoyelle [Tue, 26 Jan 2021 13:14:37 +0000 (14:14 +0100)]
BUG/MEDIUM: session: only retrieve ready idle conn from session

A bug was introduced by the early insertion of idle connections at the
end of connect_server. It is possible to reuse a connection not yet
ready waiting for an handshake (for example with proxy protocol or ssl).
A wrong duplicate xprt_handshake_io_cb tasklet is thus registered as a
side-effect.

This triggers the BUG_ON statement of xprt_handshake_subscribe :
    BUG_ON(ctx->subs && ctx->subs != es);

To counter this, a check is now present in session_get_conn to only
return a connection without the flag CO_FL_WAIT_XPRT. This might cause
sometimes the creation of dedicated server connections when in theory
reuse could have been used, but probably only occurs rarely in real
condition.

This behavior is present since commit :
    MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
    MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.

NOTE : This bug seems to be only reproducible with mode tcp, for an
unknown reason. However, reuse should never happen when not in http
mode. This improper behavior will be the subject of a dedicated patch.

This bug can easily be reproducible with the following config (a
webserver is required to accept proxy protocol on port 31080) :

    global

    defaults
      mode tcp
      timeout connect 1s
      timeout server 1s
      timeout client 1s

    listen li
      bind 0.0.0.0:4444
      server bla1 127.0.0.1:31080 check send-proxy-v2

with the inject client :
    $ inject -u 10000 -d 10 -G 127.0.0.1:4444

This should fix the github issue #1058.

(cherry picked from commit 3f07c20fab8bc9d98c4f9c0c6e356fb3023c5632)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()
William Lallemand [Wed, 27 Jan 2021 13:58:51 +0000 (14:58 +0100)]
BUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()

Use chunk_inistr() for a chunk initialisation in
ssl_sock_load_sctl_from_file() instead of a manual initialisation which
was not initialising head.

Fix issue #1073.

Must be backported as far as 2.2

(cherry picked from commit 8d67394f6915c6d2db40bc1e9593fd392827da8d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: config: fix leak on proxy.conn_src.bind_hdr_name
Amaury Denoyelle [Tue, 26 Jan 2021 13:35:22 +0000 (14:35 +0100)]
BUG/MINOR: config: fix leak on proxy.conn_src.bind_hdr_name

Leak for parsing of option usesrc of the source keyword.

This can be backported to 1.8.

(cherry picked from commit 69c5c3ab330584f9c53e2cf7c86af371a84f104d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: filters/htx: Fix data forwarding when payload length is unknown
Christopher Faulet [Mon, 25 Jan 2021 11:02:00 +0000 (12:02 +0100)]
BUG/MEDIUM: filters/htx: Fix data forwarding when payload length is unknown

It is only a problem on the response path because the request payload length
it always known. But when a filter is registered to analyze the response
payload, the filtering may hang if the server closes just after the headers.

The root cause of the bug comes from an attempt to allow the filters to not
immediately forward the headers if necessary. A filter may choose to hold
the headers by not forwarding any bytes of the payload. For a message with
no payload but a known payload length, there is always a EOM block to
forward. Thus holding the EOM block for bodyless messages is a good way to
also hold the headers. However, messages with an unknown payload length,
there is no EOM block finishing the message, but only a SHUTR flag on the
channel to mark the end of the stream. If there is no payload when it
happens, there is no payload at all to forward. In the filters API, it is
wrongly detected as a condition to not forward the headers.

Because it is not the most used feature and not the obvious one, this patch
introduces another way to hold the message headers at the begining of the
forwarding. A filter flag is added to explicitly says the headers should be
hold. A filter may choose to set the STRM_FLT_FL_HOLD_HTTP_HDRS flag and not
forwad anything to hold the headers. This flag is removed at each call, thus
it must always be explicitly set by filters. This flag is only evaluated if
no byte has ever been forwarded because the headers are forwarded with the
first byte of the payload.

reg-tests/filters/random-forwarding.vtc reg-test is updated to also test
responses with unknown payload length (with and without payload).

This patch must be backported as far as 2.0.

(cherry picked from commit 6071c2d12dd2ff1f5876a2ace313a81259f211cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: Improve documentation of the various hdr() fetches
Tim Duesterhus [Sat, 23 Jan 2021 16:50:21 +0000 (17:50 +0100)]
DOC: Improve documentation of the various hdr() fetches

GitHub issue #796 notes that many administrators miss the fact that the `hdr()`
fetch (without the `f`) splits the header value at commas. This is only
mentioned at the end of a long paragraph.

This patch attempts to improve the documentation by:
- Explaning the "comma issue" as early as possible.
- Adding newlines to split the explanation into distinct sections.
- Reducing duplication by making the `res` siblings refer to their `req`
  counterparts.

This patch may be backported as long as it applies cleanly. During the
refactoring I needed to adjust several explanations for consistency and not all
of them might be available in older branches.

(cherry picked from commit 27c70ae23c1f5b39c9757387559072a096c8aa8c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD/MINOR: lua: define _GNU_SOURCE for LLONG_MAX
Bertrand Jacquin [Thu, 21 Jan 2021 21:14:07 +0000 (21:14 +0000)]
BUILD/MINOR: lua: define _GNU_SOURCE for LLONG_MAX

Lua requires LLONG_MAX defined with __USE_ISOC99 which is set by
_GNU_SOURCE, not necessarely defined by default on old compiler/glibc.

  $ make V=1 TARGET=linux-glibc-legacy USE_THREAD= USE_ACCEPT4= USE_PCRE=1 USE_OPENSSL=1 USE_ZLIB=1  USE_LUA=1
  ..
cc -Iinclude -O2 -g -Wall -Wextra -Wdeclaration-after-statement -fwrapv -Wno-strict-aliasing -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers -DUSE_EPOLL -DUSE_NETFILTER -DUSE_PCRE -DUSE_POLL -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_FUTEX -DUSE_ZLIB -DUSE_CPU_AFFINITY -DUSE_DL -DUSE_RT -DUSE_PRCTL -DUSE_THREAD_DUMP -I/usr/include/openssl101e/ -DUSE_PCRE -I/usr/include -DCONFIG_HAPROXY_VERSION=\"2.4-dev5-73246d-83\" -DCONFIG_HAPROXY_DATE=\"2021/01/21\" -c -o src/hlua.o src/hlua.c
  In file included from /usr/local/include/lua.h:15,
                   from /usr/local/include/lauxlib.h:15,
                   from src/hlua.c:16:
  /usr/local/include/luaconf.h:581:2: error: #error "Compiler does not support 'long long'. Use option '-DLUA_32BITS'   or '-DLUA_C89_NUMBERS' (see file 'luaconf.h' for details)"
  ..
cc -Iinclude -O2 -g -Wall -Wextra -Wdeclaration-after-statement -fwrapv -Wno-strict-aliasing -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers -DUSE_EPOLL -DUSE_NETFILTER -DUSE_PCRE -DUSE_POLL -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_FUTEX -DUSE_ZLIB -DUSE_CPU_AFFINITY -DUSE_DL -DUSE_RT -DUSE_PRCTL -DUSE_THREAD_DUMP -I/usr/include/openssl101e/ -DUSE_PCRE -I/usr/include -DCONFIG_HAPROXY_VERSION=\"2.4-dev5-73246d-83\" -DCONFIG_HAPROXY_DATE=\"2021/01/21\" -c -o src/hlua_fcn.o src/hlua_fcn.c
  In file included from /usr/local/include/lua.h:15,
                   from /usr/local/include/lauxlib.h:15,
                   from src/hlua_fcn.c:17:
  /usr/local/include/luaconf.h:581:2: error: #error "Compiler does not support 'long long'. Use option '-DLUA_32BITS'   or '-DLUA_C89_NUMBERS' (see file 'luaconf.h' for details)"
  ..

Cc: Thierry Fournier <tfournier@arpalert.org>
(cherry picked from commit f4c12d4da25cd9da14215b0c4ee740d2b2ef762e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: mux-h2: fix read0 handling on partial frames
Willy Tarreau [Wed, 20 Jan 2021 09:53:13 +0000 (10:53 +0100)]
BUG/MEDIUM: mux-h2: fix read0 handling on partial frames

Since commit aade4edc1 ("BUG/MEDIUM: mux-h2: Don't handle pending read0
too early on streams"), we've met a few cases where an early connection
close wouldn't be properly handled if some data were pending in a frame
header, because the test now considers the buffer's contents before
accepting to report the close, but given that frame headers or preface
are consumed at once, the buffer cannot make progress when it's stuck
at intermediary lengths.

In order to address this, this patch introduces two flags in the h2c
connection to store any reported shutdown and failed parsing. The idea
is that we cannot rely on conn_xprt_read0_pending() in the parser since
it wouldn't consider data pending in the buffer nor intermediary layers,
but we know for certain that after a read0 is reported by the transport
layer in presence of an RD_SH on the connection, no more progress will
be made there. This alone is not sufficient to decide to end processing,
we can only do this once these final data have been submitted to a parser.
Therefore, now when a parser fails on missing data, we check if a read0
has already been reported on this connection, and if so we set a new
END_REACHED flag on the connection to indicate a failure to process the
final data. The h2c_read0_pending() function now simply reports this
flag's status. This way we're certain that the input shutdown is only
considered after the demux attempted to parse the last frame.

Maybe over the long term the subscribe() API should be improved to
synchronously fail when trying to subscribe for an even that will not
happen. This may be an elegant solution that could possibly work across
multiple layers and even muxes, and be usable at a few specific places
where that's needed.

Given the patch above was backported as far as 2.0, this one should be
backported there as well. It is possible that the fcgi mux has the same
issue, but this was not analysed yet.

Thanks to Pierre Cheynier for providing detailed traces allowing to
quickly narrow the problem down, and to Olivier for his analysis.

(cherry picked from commit 3d4631fec626c5aa8f12582ee3713563b5e38519)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: tcpcheck: Don't destroy connection in the wake callback context
Christopher Faulet [Mon, 18 Jan 2021 14:47:03 +0000 (15:47 +0100)]
BUG/MEDIUM: tcpcheck: Don't destroy connection in the wake callback context

When a tcpcheck ruleset uses multiple connections, the existing one must be
closed and destroyed before openning the new one. This part is handled in
the tcpcheck_main() function, when called from the wake callback function
(wake_srv_chk). But it is indeed a problem, because this function may be
called from the mux layer. This means a mux may call the wake callback
function of the data layer, which may release the connection and the mux. It
is easy to see how it is hazardous. And actually, depending on the
scheduling, it leads to crashes.

Thus, we must avoid to release the connection in the wake callback context,
and move this part in the check's process function instead. To do so, we
rely on the CHK_ST_CLOSE_CONN flags. When a connection must be replaced by a
new one, this flag is set on the check, in tcpcheck_main() function, and the
check's task is woken up. Then, the connection is really closed in
process_chk_conn() function.

This patch must be backported as far as 2.2, with some adaptations however
because the code is not exactly the same.

(cherry picked from commit 8f100427c4bf56acff967e313cd9cb8d42da035d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: mworker: define _GNU_SOURCE for strsignal()
Bertrand Jacquin [Thu, 21 Jan 2021 01:31:46 +0000 (01:31 +0000)]
BUG/MINOR: mworker: define _GNU_SOURCE for strsignal()

glibc < 2.10 requires _GNU_SOURCE in order to make use of strsignal(),
otherwise leading to SEGV at runtime.

  $ make V=1 TARGET=linux-glibc-legacy USE_THREAD= USE_ACCEPT4=
  ..
  src/mworker.c: In function 'mworker_catch_sigchld':
  src/mworker.c:285: warning: implicit declaration of function 'strsignal'
  src/mworker.c:285: warning: pointer/integer type mismatch in conditional expression
  ..

  $ make V=1 reg-tests REGTESTS_TYPES=slow,default
  ..
  ###### Test case: reg-tests/mcli/mcli_start_progs.vtc ######
  ## test results in: "/tmp/haregtests-2021-01-19_15-18-07.n24989/vtc.29077.28f6153d"
  ---- h1    Bad exit status: 0x008b exit 0x0 signal 11 core 128
  ---- h1    Assert error in haproxy_wait(), src/vtc_haproxy.c line 792:  Condition(*(&h->fds[1]) >= 0) not true.  Errno=0 Success
  ..

  $ gdb ./haproxy /tmp/core.0.haproxy.30270
  ..
  Core was generated by `/root/haproxy/haproxy -d -W -S fd@8 -dM -f /tmp/haregtests-2021-01-19_15-18-07.'.
  Program terminated with signal 11, Segmentation fault.
  #0  0x00002aaaab387a10 in strlen () from /lib64/libc.so.6
  (gdb) bt
  #0  0x00002aaaab387a10 in strlen () from /lib64/libc.so.6
  #1  0x00002aaaab354b69 in vfprintf () from /lib64/libc.so.6
  #2  0x00002aaaab37788a in vsnprintf () from /lib64/libc.so.6
  #3  0x00000000004a76a3 in memvprintf (out=0x7fffedc680a0, format=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n", orig_args=0x7fffedc680d0)
      at src/tools.c:3868
  #4  0x00000000004bbd40 in print_message (label=0x58abed "ALERT", fmt=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n", argp=0x7fffedc680d0)
      at src/log.c:1066
  #5  0x00000000004bc07f in ha_alert (fmt=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n") at src/log.c:1109
  #6  0x0000000000534b7b in mworker_catch_sigchld (sh=<value optimized out>) at src/mworker.c:293
  #7  0x0000000000556af3 in __signal_process_queue () at src/signal.c:88
  #8  0x00000000004f6216 in signal_process_queue () at include/haproxy/signal.h:39
  #9  run_poll_loop () at src/haproxy.c:2859
  #10 0x00000000004f63b7 in run_thread_poll_loop (data=<value optimized out>) at src/haproxy.c:3028
  #11 0x00000000004faaac in main (argc=<value optimized out>, argv=0x7fffedc68498) at src/haproxy.c:904

See: https://man7.org/linux/man-pages/man3/strsignal.3.html

Must be backported as far as 2.0.

(cherry picked from commit 25439de1811aeda7bf472f70f40c3fac9725907e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: mux_h2: missing space between "st" and ".flg" in the "show fd" helper
Willy Tarreau [Wed, 20 Jan 2021 14:50:03 +0000 (15:50 +0100)]
BUG/MINOR: mux_h2: missing space between "st" and ".flg" in the "show fd" helper

That was causing confusing outputs like this one whenan H2S is known:

   1030 : ... last_h2s=0x2ed8390 .id=775 .st=HCR.flg=0x4001 .rxbuf=...
                                                ^^^^

This was introduced by commit ab2ec4540 in 2.1-dev2 so the fix can be
backported as far as 2.1.

(cherry picked from commit ed4464e6c698bfb410610b7f3850cd0fa3331eb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: peers: Wrong "new_conn" value for "show peers" CLI command.
Frédéric Lécaille [Mon, 18 Jan 2021 14:14:39 +0000 (15:14 +0100)]
BUG/MINOR: peers: Wrong "new_conn" value for "show peers" CLI command.

This counter could be hugely incremented by the peer task responsible of managing
peer synchronizations and reconnections, for instance when a peer is not reachable
there is a period where the appctx is not created. If we receive  stick-table
updates before the peer session (appctx) is instantiated, we reach the code
responsible of incrementing the "new_conn" counter.
With this patch we increment this counter only when we really instantiate a new
peer session thanks to peer_session_create().

May be backported as far as 2.0.

(cherry picked from commit 2b0ba54ddb7d555bd0116eb8278b37ac409e7779)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: build: discard echoing in help target
Bertrand Jacquin [Sun, 17 Jan 2021 18:47:47 +0000 (18:47 +0000)]
MINOR: build: discard echoing in help target

When V=1 is used in conjuction with help, the output becomes pretty
difficult to read properly.

  $ make TARGET=linux-glibc V=1 help
  ..
    DEBUG_USE_ABORT: use abort() for program termination, see include/haproxy/bug.h for details
  echo; \
     if [ -n "" ]; then \
       if [ -n "" ]; then \
          echo "Current TARGET: "; \
       else \
          echo "Current TARGET:  (custom target)"; \
       fi; \
     else \
       echo "TARGET not set, you may pass 'TARGET=xxx' to set one among :";\
       echo "  linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, netbsd,"; \
       echo "  osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, generic,"; \
       echo "  custom"; \
     fi

  TARGET not set, you may pass 'TARGET=xxx' to set one among :
    linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, netbsd,
    osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, generic,
    custom
  echo;echo "Enabled features for TARGET '' (disable with 'USE_xxx=') :"

  Enabled features for TARGET '' (disable with 'USE_xxx=') :
  set --        POLL                                  ; echo "  $*" | (fmt || cat) 2>/dev/null
    POLL
  echo;echo "Disabled features for TARGET '' (enable with 'USE_xxx=1') :"

  Disabled features for TARGET '' (enable with 'USE_xxx=1') :
  set -- EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT  PRIVATE_CACHE THREAD PTHREAD_PSHARED BACKTRACE STATIC_PCRE STATIC_PCRE2 TPROXY LINUX_TPROXY LINUX_SPLICE LIBCRYPT CRYPT_H GETADDRINFO OPENSSL LUA FUTEX ACCEPT4 CLOSEFROM ZLIB SLZ CPU_AFFINITY TFO NS DL RT DEVICEATLAS 51DEGREES WURFL SYSTEMD OBSOLETE_LINKER PRCTL THREAD_DUMP EVPORTS OT QUIC; echo "  $*" | (fmt || cat) 2>/dev/null
    EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT PRIVATE_CACHE

This commit ensure the help target always discard line echoing
regardless of V variable as done for reg-tests-help target.

(cherry picked from commit b399a992f34f248466f4740432b506455d89236e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: peers: Possible appctx pointer dereference.
Frédéric Lécaille [Sun, 17 Jan 2021 12:08:39 +0000 (13:08 +0100)]
BUG/MINOR: peers: Possible appctx pointer dereference.

This bug may occur when enabling peers traces. It is possible that
peer->appctx is NULL when entering peer_session_release().

(cherry picked from commit 4b1a05fcf81b4343d29aeb86394d44f29d2116a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: stats: add missing INF_BUILD_INFO definition
Adis Nezirovic [Fri, 15 Jan 2021 12:12:33 +0000 (13:12 +0100)]
BUG/MEDIUM: stats: add missing INF_BUILD_INFO definition

commit 5a982a71656ce885be4b1d4b90b8db31204788a1 ("MINOR:
contrib/prometheus-exporter: export build_info") is breaking lua
`core.get_info()`.

This patch makes sure build_info is correctly initialised in all cases.

Reviewed-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit b62b78be131de1848d71350d369deac07daf448a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: peers: fix build warning about unused variable
Willy Tarreau [Fri, 15 Jan 2021 16:08:38 +0000 (17:08 +0100)]
BUILD: peers: fix build warning about unused variable

Previous commit da2b0844f ("MINOR: peers: Add traces for peer control
messages.") introduced a build warning on some compiler versions after
the removal of variable "peers" in peer_send_msgs() because variable
"s" was used only to assign this one, and variable "si" to assign "s".
Let's remove both to fix the warning. No backport is needed.

(cherry picked from commit 81d7092dbd25db04cc72d2dca77647d548342256)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: dns: SRV records ignores duplicated AR records (v2)
Baptiste Assmann [Fri, 15 Jan 2021 16:01:24 +0000 (17:01 +0100)]
BUG/MINOR: dns: SRV records ignores duplicated AR records (v2)

V2 of this fix which includes a missing pointer initialization which was
causing a segfault in v1 (949a7f64591458eb06c998acf409093ea991dc3a)

This bug happens when a service has multiple records on the same host
and the server provides the A/AAAA resolution in the response as AR
(Additional Records).

In such condition, the first occurence of the host will be taken from
the Additional section, while the second (and next ones) will be process
by an independent resolution task (like we used to do before 2.2).
This can lead to a situation where the "synchronisation" of the
resolution may diverge, like described in github issue #971.

Because of this behavior, HAProxy mixes various type of requests to
resolve the full list of servers: SRV+AR for all "first" occurences and
A/AAAA for all other occurences of an existing hostname.
IE: with the following type of response:

   ;; ANSWER SECTION:
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A2.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 86 A3.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A1.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 85 A3.tld.

   ;; ADDITIONAL SECTION:
   A2.tld.                 3600    IN      A       192.168.0.2
   A3.tld.                 3600    IN      A       192.168.0.3
   A1.tld.                 3600    IN      A       192.168.0.1
   A3.tld.                 3600    IN      A       192.168.0.3

the first A3 host is resolved using the Additional Section and the
second one through a dedicated A request.

When linking the SRV records to their respective Additional one, a
condition was missing (chek if said SRV record is already attached to an
Additional one), leading to stop processing SRV only when the target
SRV field matches the Additional record name. Hence only the first
occurence of a target was managed by an additional record.
This patch adds a condition in this loop to ensure the record being
parsed is not already linked to an Additional Record. If so, we can
carry on the parsing to find a possible next one with the same target
field value.

backport status: 2.2 and above

(cherry picked from commit 6554742b15880bc19e41f89761905baed33bc398)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: peers: Add traces for peer control messages.
Frédéric Lécaille [Fri, 15 Jan 2021 15:21:28 +0000 (16:21 +0100)]
MINOR: peers: Add traces for peer control messages.

Display traces when sending/receiving peer control messages (synchronisation, heartbeat).
Add remaining traces when parsing malformed messages (acks, stick-table definitions)
or ignoring them.
Also add traces when releasing session or when reaching the PEER_SESS_ST_ERRPROTO
peer protocol state.

(cherry picked from commit da2b0844fc4eafd9c63e3eb20a1a7020d9f92088)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: threads: Fixes the number of possible cpus report for Mac.
David CARLIER [Fri, 15 Jan 2021 08:09:56 +0000 (08:09 +0000)]
BUG/MINOR: threads: Fixes the number of possible cpus report for Mac.

There is no low level api to achieve same as Linux/FreeBSD, we rely
on CPUs available. Without this, the number of threads is just 1 for
Mac while having 8 cores in my M1.

Backporting to 2.1 should be enough if that's possible.

Signed-off-by: David CARLIER <devnexen@gmail.com>
(cherry picked from commit 6a9060189d66ca931984706d5e2a970ed913f457)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: server: Forbid server definitions in frontend sections
Christopher Faulet [Wed, 13 Jan 2021 12:14:13 +0000 (13:14 +0100)]
MINOR: server: Forbid server definitions in frontend sections

An fatal error is now reported if a server is defined in a frontend
section. til now, a warning was just emitted and the server was ignored. The
warning was added in the 1.3.4 when the frontend/backend keywords were
introduced to allow a smooth transition and to not break existing
configs. It is old enough now to emit an fatal error in this case.

This patch is related to the issue #1043. It may be backported at least as
far as 2.2, and possibly to older versions. It relies on the previous commit
("MINOR: config: Add failifnotcap() to emit an alert on proxy capabilities").

(cherry picked from commit e3bdc81f8aeb02b5c92b5d10fc9352daaea1cf20)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: config: Add failifnotcap() to emit an alert on proxy capabilities
Christopher Faulet [Wed, 13 Jan 2021 11:10:00 +0000 (12:10 +0100)]
MINOR: config: Add failifnotcap() to emit an alert on proxy capabilities

This function must be used to emit an alert if a proxy does not have at
least one of the requested capabilities. An additional message may be
appended to the alert.

(cherry picked from commit d4a83dd6b30a68605f13a447b398a15ab38f28d4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: init: Use a dynamic buffer to set HAPROXY_CFGFILES env variable
Christopher Faulet [Tue, 12 Jan 2021 17:57:38 +0000 (18:57 +0100)]
BUG/MINOR: init: Use a dynamic buffer to set HAPROXY_CFGFILES env variable

The HAPROXY_CFGFILES env variable is built using a static trash chunk, via a
call to get_trash_chunk() function. This chunk is reserved during the whole
configuration parsing. It is far too large to guarantee it will not be
reused during the configuration parsing. And in fact, it happens in the lua
code since the commit f67442efd ("BUG/MINOR: lua: warn when registering
action, conv, sf, cli or applet multiple times"), when a lua script is
loaded.

To fix the bug, we now use a dynamic buffer instead. And we call memprintf()
function to handle both the allocation and the formatting. Allocation errors
at this stage are fatal.

This patch should fix the issue #1041. It must be backported as far as 2.0.

(cherry picked from commit 4e36682d51f4e206c63d792a8cde3e669fb8a0d4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years ago[RELEASE] Released version 2.3.4 v2.3.4
Christopher Faulet [Wed, 13 Jan 2021 15:10:29 +0000 (16:10 +0100)]
[RELEASE] Released version 2.3.4

Released version 2.3.4 with the following main changes :
    - MINOR: reg-tests: add a way to add service dependency
    - BUG/MINOR: sample: check alloc_trash_chunk return value in concat()
    - BUG/MINOR: reg-tests: fix service dependency script
    - MINOR: reg-tests: add base prometheus test
    - Revert "BUG/MINOR: dns: SRV records ignores duplicated AR records"
    - BUG/MINOR: sample: Memory leak of sample_expr structure in case of error
    - BUG/MINOR: check: Don't perform any check on servers defined in a frontend
    - BUG/MINOR: init: enforce strict-limits when using master-worker
    - MINOR: contrib/prometheus-exporter: avoid connection close header
    - MINOR: contrib/prometheus-exporter: use fill_info for process dump

4 years agoMINOR: contrib/prometheus-exporter: use fill_info for process dump
William Dauchy [Mon, 11 Jan 2021 19:07:49 +0000 (20:07 +0100)]
MINOR: contrib/prometheus-exporter: use fill_info for process dump

use `stats_fill_info` when possible to avoid duplicating code.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 5d9b8f3c9347a1a10b86f81d70b22c3cab0e6925)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>