haproxy-2.1.git
5 years agoDOC: tcp-rules: Refresh details about L7 matching for tcp-request content rules
Christopher Faulet [Fri, 2 Oct 2020 09:38:46 +0000 (11:38 +0200)]
DOC: tcp-rules: Refresh details about L7 matching for tcp-request content rules

Because the parsing of HTTP message is now performed in the HTTP multiplexers,
the content is immediatly available when "tcp-request content" rules are
evaluated for an HTTP frontend. So, it is a good idea to make the documentation
explicit on this point. In addition, because in all cases, the parsing is
already performed, there is no reason to still use "tcp-request content" rules
based on L7 matching, although it is still valid. The recommended way is to use
"http-request" rules instead. Again, it is a good idea to update the
documentation on this point.

(cherry picked from commit 7ea509e15f6fe3390a969f6eec29d76b8732d3aa)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e821fbde4a7b75ad52fb98a1ea3d77b3ff483054)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: Fix several leaks of 'log_tag' in init().
Eric Salama [Fri, 2 Oct 2020 09:58:19 +0000 (11:58 +0200)]
BUG/MINOR: Fix several leaks of 'log_tag' in init().

We use chunk_initstr() to store the program name as the default log-tag.

If we use the log-tag directive in the config file, this chunk will be
destroyed and replaced. chunk_initstr() sets the chunk size to 0 so we
will free the chunk itself, but not its content.

This happens for a global section and also for a proxy.

We fix this by using chunk_initlen() instead of chunk_initstr().
We also check that the memory allocation was successfull, otherwise we quit.

This fixes github issue #850.
It can be backported as far as 1.9, with minor adjustments to includes.

(cherry picked from commit 7cea6065aca04c91fc5109e581f124a46b2b5242)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7566bcfa5a1d28e8c9d5f9ca1dfe0ebe791052eb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUILD: makefile: Fix building with closefrom() support enabled
Brad Smith [Wed, 30 Sep 2020 05:04:48 +0000 (01:04 -0400)]
BUILD: makefile: Fix building with closefrom() support enabled

I noticed the USE_CLOSEFROM define was not being passed along like the rest
during the build.

Looking around I see this was broken with the following two commits and related
series..

BUILD: Makefile: also report disabled options in the BUILD_OPTIONS variable
http://git.haproxy.org/?p=haproxy.git;a=commit;h=05fd82da76d1bbc8d65d63ab246bda7cbcf8481a

BUILD: pass all "USE_*" variables as -DUSE_* to the compiler
http://git.haproxy.org/?p=haproxy.git;a=commit;h=824cd00d3bda8f7f6d4c30baf77ba6c19ab47811

Looks like this should be back ported to 2.0, 2.1 and 2.2.

(cherry picked from commit 5018aacae5f2fb65365bce5a3a390c5126203952)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4aafadb4d9a47e0cb3cc524d9306c35bd64ccd75)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoMINOR: ssl: reach a ckch_store from a sni_ctx
William Lallemand [Thu, 5 Mar 2020 09:17:47 +0000 (10:17 +0100)]
MINOR: ssl: reach a ckch_store from a sni_ctx

It was only possible to go down from the ckch_store to the sni_ctx but
not to go up from the sni_ctx to the ckch_store.

To allow that, 2 pointers were added:

- a ckch_inst pointer in the struct sni_ctx
- a ckckh_store pointer in the struct ckch_inst

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

5 years agoBUG/MEDIUM: ssl: crt-list negative filters don't work
William Lallemand [Mon, 17 Aug 2020 12:31:19 +0000 (14:31 +0200)]
BUG/MEDIUM: ssl: crt-list negative filters don't work

The negative filters which are supposed to exclude a SNI from a
wildcard, never worked. Indeed the negative filters were skipped in the
code.

To fix the issue, this patch looks for negative filters that are on the
same line as a the wildcard that just matched.

This patch should fix issue #818. It must be backported in 2.2.  The
problem also exists in versions > 1.8 but the infrastructure required to
fix this was only introduced in 2.1.  In older versions we should
probably change the documentation to state that negative filters are
useless.

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

5 years ago[RELEASE] Released version 2.1.9 v2.1.9
Willy Tarreau [Wed, 30 Sep 2020 05:20:26 +0000 (07:20 +0200)]
[RELEASE] Released version 2.1.9

Released version 2.1.9 with the following main changes :
    - BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
    - SCRIPTS: git-show-backports: make -m most only show the left branch
    - SCRIPTS: git-show-backports: emit the shell command to backport a commit
    - BUG/MINOR: ssl: fix memory leak at OCSP loading
    - BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
    - BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
    - MINOR: arg: Add an argument type to keep a reference on opaque data
    - BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
    - BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
    - BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
    - BUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation
    - BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
    - MINOR: hlua: Don't needlessly copy lua strings in trash during args validation
    - BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
    - MEDIUM: lua: Don't filter exported fetches and converters
    - BUG/MINOR: snapshots: leak of snapshots on deinit()
    - BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
    - BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
    - BUG/MINOR: reload: do not fail when no socket is sent
    - MINOR: http-htx: Add an option to eval query-string when the path is replaced
    - BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action
    - DOC: cache: Use '<name>' instead of '<id>' in error message
    - BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to memory leak
    - BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
    - BUG/MINOR: contrib/spoa-server: Do not free reference to NULL
    - BUG/MINOR: contrib/spoa-server: Updating references to free in case of failure
    - BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of ipv6_address
    - BUG/MINOR: startup: haproxy -s cause 100% cpu
    - BUG/MEDIUM: doc: Fix replace-path action description
    - Revert "BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action"
    - BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
    - BUG/MINOR: threads: work around a libgcc_s issue with chrooting
    - BUILD: thread: limit the libgcc_s workaround to glibc only
    - MINOR: Commit .gitattributes
    - CLEANUP: Update .gitignore
    - BUG/MINOR: auth: report valid crypto(3) support depending on build options
    - BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections
    - BUILD: threads: better workaround for late loading of libgcc_s
    - BUG/MEDIUM: pattern: Renew the pattern expression revision when it is pruned
    - MINOR: arg: Use chunk_destroy() to release string arguments
    - BUG/MEDIUM: http-ana: Don't wait to send 1xx responses received from servers
    - BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
    - BUG/MINOR: ssl: verifyhost is case sensitive
    - BUG/MINOR: server: report correct error message for invalid port on "socks4"
    - BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
    - BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
    - BUG/MEDIUM: h2: report frame bits only for handled types
    - MINOR: h2/trace: also display the remaining frame length in traces
    - BUG/MINOR: Fix memory leaks cfg_parse_peers
    - MINOR: backend: make the "whole" option of balance uri take only one bit
    - MINOR: backend: add a new "path-only" option to "balance uri"
    - REGTESTS: add a few load balancing tests
    - DOC: spoa-server: fix false friends `actually`
    - BUG/MINOR: config: Fix memory leak on config parse listen
    - BUG/MEDIUM: listeners: do not pause foreign listeners
    - DOC: agent-check: fix typo in "fail" word expected reply
    - REGTEST: fix host part in balance-uri-path-only.vtc
    - REGTEST: make abns_socket.vtc require 1.8
    - REGTEST: make map_regm_with_backref require 1.7

5 years agoREGTEST: make map_regm_with_backref require 1.7
Willy Tarreau [Tue, 29 Sep 2020 09:00:51 +0000 (11:00 +0200)]
REGTEST: make map_regm_with_backref require 1.7

map_regm was only introduced in 1.7, I don't know why the require field
was set to 1.6, probably tha the test evolved and didn't start with
map_regm.

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

5 years agoREGTEST: make abns_socket.vtc require 1.8
Willy Tarreau [Tue, 29 Sep 2020 08:58:44 +0000 (10:58 +0200)]
REGTEST: make abns_socket.vtc require 1.8

It's not as much an abns test as it is a seamless reload test, which
appeared in 1.8 due to "expose-fd listeners".

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

5 years agoREGTEST: fix host part in balance-uri-path-only.vtc
Willy Tarreau [Tue, 29 Sep 2020 07:58:23 +0000 (09:58 +0200)]
REGTEST: fix host part in balance-uri-path-only.vtc

We used to set it to ${h1_px_addr} but it randomly fails on certain
hosts (FreeBSD and OSX) where the address is surprisingly set to "::1"
while the Host field contains 127.0.0.1 (hence two different address
families). While this is likely a minor issue in vtest, we don't need
to depend on this and can easily hard-code 127.0.0.1 which is already
used in other tests.

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

5 years agoDOC: agent-check: fix typo in "fail" word expected reply
William Dauchy [Sat, 26 Sep 2020 11:35:51 +0000 (13:35 +0200)]
DOC: agent-check: fix typo in "fail" word expected reply

`tcpcheck_agent_expect_reply` expects "fail" not "failed"

This should fix github issue #876

This can be backported to all maintained versions (i.e >= 1.6) as of
today.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit f8e795ca04a2e89b801c40843b1b79fd97251606)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit df223005a3f61d19c140e8bffbb17ec8507cc486)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MEDIUM: listeners: do not pause foreign listeners
Willy Tarreau [Wed, 23 Sep 2020 15:17:22 +0000 (17:17 +0200)]
BUG/MEDIUM: listeners: do not pause foreign listeners

There's a nasty case with listeners that belong to foreign processes.
If a proxy is defined this way:

     global
         nbproc 2

     frontend f
         bind :1111 process 1
         bind :2222 process 2

and if stats expose-fd listeners is set, the listeners' FDs will not
be closed on the processes that don't use them. At this point it's not
a big deal, except that they're shared between processes and that a
"disable frontend f" issued on one process will pause all of them and
cause the other process to see accept() fail, turning its own listener
to state LI_LIMITED to try to leave it some time to recover. But it
will never recover, even after an enable.

The root cause of the issue is that the ZOMBIE state doesn't cover
this situation since it's only for a proxy being entirely bound to a
process.

What we do here to address this is that we refrain from pausing a
file descriptor that belongs to a foreign process in pause_listener().
This definitely solves the problem. A similar test is present in
resume_listener() and is the reason why the FD doesn't recover upon the
"enable" action by the way.

This ought to be backported to 1.8 where seamless reload was integrated.

The config above should be sufficient to validate that the fix works;
after a pair of "disable/enable frontend" no process will handle the
traffic to one of the ports anymore.

(cherry picked from commit 02e1975c29eb19da98205164f23154f903802644)
[wt: context adjustment; bind_proc is in l->bind_conf in 2.2; test ok]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c7239d5da5801f2238b56601bfae29441153d4b8)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: config: Fix memory leak on config parse listen
Amaury Denoyelle [Fri, 18 Sep 2020 13:59:39 +0000 (15:59 +0200)]
BUG/MINOR: config: Fix memory leak on config parse listen

This memory leak happens if there is two or more defaults section. When
the default proxy is reinitialized, the structure member containing the
config filename must be freed.

Fix github issue #851.
Should be backported as far as 1.6.

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

5 years agoDOC: spoa-server: fix false friends `actually`
William Dauchy [Sat, 1 Aug 2020 14:28:52 +0000 (16:28 +0200)]
DOC: spoa-server: fix false friends `actually`

it seems like `actually` was wrongly used instead of `currently`

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit 4896d279aa159ba1fc9f7728a30120993c7ac577)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 96bcd2110b6d1219236ace2bdb0fa32ee0c092f4)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoREGTESTS: add a few load balancing tests
Willy Tarreau [Wed, 23 Sep 2020 07:02:13 +0000 (09:02 +0200)]
REGTESTS: add a few load balancing tests

This adds "balance-rr" to test round robin, "balance-uri" to test the
default balance-uri method, and "balance-uri-path-only" which mixes H1
and H2 through "balance uri path-only" and verifies that they reach
the same server.

Note that for the latter, "proto h2" explicitly had to be placed on
the listening socket otherwise it would timeout. This may indicate an
issue in the H1->H2 upgrade depending how the H2 preface is sent maybe.

(cherry picked from commit 19490907b3836369215a9860f1bd6d2bc3297937)
[wt: marked the balance-uri-path-only for 2.2 since already backported]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6a551a0ea58218ab02f45b0fb1eb26248b7a5263)
[wt: marked the balance-uri-path-only for 2.1 since already backported]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoMINOR: backend: add a new "path-only" option to "balance uri"
Willy Tarreau [Wed, 23 Sep 2020 06:56:29 +0000 (08:56 +0200)]
MINOR: backend: add a new "path-only" option to "balance uri"

Since we've fixed the way URIs are handled in 2.1, some users have started
to experience inconsistencies in "balance uri" between requests received
over H1 and the same ones received over H2. This is caused by the fact
that H1 rarely uses absolute URIs while H2 always uses them. Similar
issues were reported already around replace-uri etc, leading to "pathq"
recently being introduced, so this isn't new.

Here what this patch does is add a new option to "balance uri" to indicate
that the hashing should only start at the path and not cover the authority.
This makes H1 relative URIs and H2 absolute URI hashes equally again.

Some extra options could be added to normalize URIs by always hashing the
authority (or host) in front of them, which would make sure that both
absolute and relative requests provide the same hash. This is left for
later if needed.

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

5 years agoMINOR: backend: make the "whole" option of balance uri take only one bit
Willy Tarreau [Wed, 23 Sep 2020 06:05:47 +0000 (08:05 +0200)]
MINOR: backend: make the "whole" option of balance uri take only one bit

We'll want to add other boolean options on "balance uri", so let's make
some room aside "whole" and make it take only one bit and not one int.

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

5 years agoBUG/MINOR: Fix memory leaks cfg_parse_peers
Eric Salama [Fri, 18 Sep 2020 09:55:17 +0000 (11:55 +0200)]
BUG/MINOR: Fix memory leaks cfg_parse_peers

When memory allocation fails in cfg_parse_peers or when an error occurs
while parsing a stick-table, the temporary table and its id must be freed.

This fixes github issue #854. It should be backported as far as 2.0.

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

5 years agoMINOR: h2/trace: also display the remaining frame length in traces
Willy Tarreau [Fri, 18 Sep 2020 05:39:29 +0000 (07:39 +0200)]
MINOR: h2/trace: also display the remaining frame length in traces

It's often missing when debugging, even though it's often zero for
control frames or after data are consumed.

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

5 years agoBUG/MEDIUM: h2: report frame bits only for handled types
Willy Tarreau [Fri, 18 Sep 2020 05:43:38 +0000 (07:43 +0200)]
BUG/MEDIUM: h2: report frame bits only for handled types

As part of his GREASE experiments on Chromium, Bence Béky reported in
https://lists.w3.org/Archives/Public/ietf-http-wg/2020JulSep/0202.html
and https://bugs.chromium.org/p/chromium/issues/detail?id=1127060 that
a certain combination of frame type and frame flags was causing an error
on app.slack.com. It turns out that it's haproxy that is causing this
issue because the frame type is wrongly assumed to support padding, the
frame flags indicate padding is present, and the frame is too short for
this, resulting in an error.

The reason why only some frame types are affected is due to the frame
type being used in a bit shift to match against a mask, and where the
5 lower bits of the frame type only are used to compute the frame bit.
If the resulting frame bit matches a DATA, HEADERS or PUSH_PROMISE frame
bit, then padding support is assumed and the test is enforced, resulting
in a PROTOCOL_ERROR or FRAME_SIZE_ERROR depending on the payload size.

We must never match any such bit for unsupported frame types so let's
add a check for this. This must be backported as far as 1.8.

Thanks to Cooper Bethea for providing enough context to help narrow the
issue down and to Bence Béky for creating a simple reproducer.

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

5 years agoBUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
Christopher Faulet [Fri, 18 Sep 2020 08:19:02 +0000 (10:19 +0200)]
BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch

A subtle bug was introduced by the commit a6d9879e6 ("BUG/MEDIUM: htx:
smp_prefetch_htx() must always validate the direction"), for the "method"
sample fetch only. The sample data type and the method id are always
overwritten because smp_prefetch_htx() function is called later in the
sample fetch evaluation. The bug is in the smp_prefetch_htx() function but
it is only visible for the "method" sample fetch, for an unknown method.

In fact, when smp_prefetch_htx() is called, the sample object is
altered. The data type is set to SMP_T_BOOL and, on success, the data value
is set to 1.  Thus, if the caller has already set some infos into the sample
object, they may be lost. AFAIK, there is no reason to do so. It is
inherited from the legacy HTTP code and I honestely don't known why it was
done this way. So, instead of fixing the "method" sample fetch to set useful
info after the call to smp_prefetch_htx() function, I prefer to not alter
the sample object in smp_prefetch_htx().

This patch must be backported as far as 2.0. On the 2.0, only the HTX part
must be fixed.

(cherry picked from commit d2414a23c4c20b14c54ee9279e99fb96c81a29b1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b8aa5f28cea612eadfbd6d775d8402e45a6547c6)
[wt: adjusted ctx]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
Willy Tarreau [Fri, 18 Sep 2020 05:41:28 +0000 (07:41 +0200)]
BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK

When sending a frame ACK, the parser state is not equal to H2_CS_FRAME_H
and we used to report it as an error, which is not true. In fact we should
only indicate when we skip remaining data.

This may be backported as far as 2.1.

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

5 years agoBUG/MINOR: server: report correct error message for invalid port on "socks4"
Willy Tarreau [Tue, 15 Sep 2020 09:25:54 +0000 (11:25 +0200)]
BUG/MINOR: server: report correct error message for invalid port on "socks4"

The socks4 keyword parser was a bit too much copy-pasted, it only checks
for a null port and reports "invalid range". Let's properly check for the
1-65535 range and report the correct error.

It may be backported everywhere "socks4" is present (2.0).

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

5 years agoBUG/MINOR: ssl: verifyhost is case sensitive
William Lallemand [Mon, 14 Sep 2020 13:20:10 +0000 (15:20 +0200)]
BUG/MINOR: ssl: verifyhost is case sensitive

In bug #835, @arjenzorgdoc reported that the verifyhost option on the
server line is case-sensitive, that shouldn't be the case.

This patch fixes the issue by replacing memcmp by strncasecmp and strcmp
by strcasecmp. The patch was suggested by @arjenzorgdoc.

This must be backported in all versions supporting the verifyhost
option.

(cherry picked from commit 2d6fd0a90df8d3ab77f13c59e5f1efa3d271c42c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 656c331cbd02b4bef43e6ceac347b6e9bd440296)
[wt: adjusted context]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
William Lallemand [Fri, 14 Aug 2020 12:43:35 +0000 (14:43 +0200)]
BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate

In bug #810, the SNI are not matched correctly, indeed when trying to
match a certificate type in ssl_sock_switchctx_cbk() all SNIs were not
looked up correctly.

In the case you have in a crt-list:

wildcard.subdomain.domain.tld.pem.rsa *.subdomain.domain.tld record.subdomain.domain.tld
record.subdomain.domain.tld.pem.ecdsa record.subdomain.domain.tld another-record.subdomain.domain.tld

If the client only supports RSA and requests
"another-record.subdomain.domain.tld", HAProxy will find the single
ECDSA certificate and won't try to look up for a wildcard RSA
certificate.

This patch fixes the code so we look for all single and
wildcard before chosing the certificate type.

This bug was introduced by commit 3777e3a ("BUG/MINOR: ssl: certificate
choice can be unexpected with openssl >= 1.1.1").

It must be backported as far as 1.8 once it is heavily tested.

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

5 years agoBUG/MEDIUM: http-ana: Don't wait to send 1xx responses received from servers
Christopher Faulet [Mon, 31 Aug 2020 09:07:07 +0000 (11:07 +0200)]
BUG/MEDIUM: http-ana: Don't wait to send 1xx responses received from servers

When an informational response (1xx) is received, we must be sure to send it
ASAP. To do so, CF_SEND_DONTWAIT flag must be set on the response channel to
instruct the stream-interface to not set the CO_SFL_MSG_MORE flag on the
transport layer. Otherwise the response delivery may be delayed, because of the
commit 8945bb6c0 ("BUG/MEDIUM: stream-int: fix loss of CO_SFL_MSG_MORE flag in
forwarding").

Note that a previous patch (cf6898cd ["BUG/MINOR: http-ana: Don't wait to send
1xx responses generated by HAProxy"]) add this flag on 1xx responses generated
by HAProxy but not on responses coming from servers.

This patch must be backported to 2.2 and may be backported as far as 1.9, for
HTX part only. But this part has changed in the 2.2, so it may be a bit
tricky. Note it does not fix any known bug on 2.1 and below because the
CO_SFL_MSG_MORE flag is ignored by the h1 mux.

(cherry picked from commit 7d518454bb501659e8b13f38c22d216f5485be34)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 4777db423493d10cb97607b601c8ba9821b9a149)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoMINOR: arg: Use chunk_destroy() to release string arguments
Christopher Faulet [Fri, 7 Aug 2020 09:45:18 +0000 (11:45 +0200)]
MINOR: arg: Use chunk_destroy() to release string arguments

This way, all fields of the buffer structure are reset when a string argument
(ARGT_STR) is released.  It is also a good way to explicitly specify this kind
of argument is a chunk. So .data and .size fields must be set.

This patch may be backported to ease backports.

(cherry picked from commit 6ad7df423b71470babef545506a1af569074fc59)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 1c67ab418b2adfd99e0c9d2796af382a86ce4a0f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: pattern: Renew the pattern expression revision when it is pruned
Christopher Faulet [Wed, 9 Sep 2020 14:09:44 +0000 (16:09 +0200)]
BUG/MEDIUM: pattern: Renew the pattern expression revision when it is pruned

It must be done to expire patterns cached in the LRU cache. Otherwise it is
possible to retrieve an already freed pattern, attached to a released pattern
expression.

When a specific pattern is deleted (->delete() callback), the pattern expression
revision is already renewed. Thus it is not affected by this bug. Only prune
action on the pattern expression is concerned.

In addition, for a pattern expression, in ->prune() callbacks when the pattern
list is released, a missing LIST_DEL() has been added. It is not a real issue
because the list is reinitialized at the end and all elements are released and
should never be reused. But it is less confusing this way.

This bug may be triggered when a map is cleared from the cli socket. A
workaround is to set the pattern cache size (tune.pattern.cache-size) to 0 to
disable it.

This patch should fix the issue #844. It must be backported to all supported
versions.

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

5 years agoBUILD: threads: better workaround for late loading of libgcc_s
Willy Tarreau [Wed, 9 Sep 2020 15:07:54 +0000 (17:07 +0200)]
BUILD: threads: better workaround for late loading of libgcc_s

Commit 77b98220e ("BUG/MINOR: threads: work around a libgcc_s issue with
chrooting") tried to address an issue with libgcc_s being loaded too late.
But it turns out that the symbol used there isn't present on armhf, thus
it breaks the build.

Given that the issue manifests itself during pthread_exit(), the safest
and most portable way to test this is to call pthread_exit(). For this
we create a dummy thread which exits, during the early boot. This results
in the relevant library to be loaded if needed, making sure that a later
call to pthread_exit() will still work. It was tested to work fine under
linux on the following platforms:

 glibc:
   - armhf
   - aarch64
   - x86_64
   - sparc64
   - ppc64le

 musl:
   - mipsel

Just running the code under strace easily shows the call in the dummy
thread, for example here on armhf:

  $ strace -fe trace=file ./haproxy -v 2>&1 | grep gcc_s
  [pid 23055] open("/lib/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3

The code was isolated so that it's easy to #ifdef it out if needed.
This should be backported where the patch above is backported (likely
2.0).

(cherry picked from commit f734ebfac4c406f245347527bd0e5831a251cc61)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 10c627ab2ebeb0a38ae8df477a5f2d870ad77f7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections
Willy Tarreau [Tue, 8 Sep 2020 13:40:57 +0000 (15:40 +0200)]
BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections

The condition in h1_refresh_timeout() seems insufficient to properly
take care of the half-closed timeout, because depending on the ordering
of operations when performing the last send() to a client, the stream
may or may not still be there and we may fail to shrink the client
timeout on our last opportunity to do so.

Here we want to make sure that the timeout is always reduced when the
last chunk was sent and the shutdown completed, regardless of the
presence of a stream or not. This is what this patch does.

This should be backported as far as 2.0, and should fix the issue
reported in #541.

(cherry picked from commit 4313d5ae98b90613318da7e1181a6c4a1db29799)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ee6ed47bba75896b6bbdd15f6a622e88a8168390)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: auth: report valid crypto(3) support depending on build options
Victor Kislov [Thu, 6 Aug 2020 16:21:39 +0000 (19:21 +0300)]
BUG/MINOR: auth: report valid crypto(3) support depending on build options

Since 1.8 with commit e8692b41e ("CLEANUP: auth: use the build options list
to report its support"), crypt(3) is always reported as being supported in
"haproxy -vv" because no test on USE_LIBCRYPT is made anymore when
producing the output.

This reintroduces the distinction between with and without USE_LIBCRYPT
in the output by indicating "yes" or "no". It may be backported as far
as 1.8, though the code differs due to a number of include files cleanups.

(cherry picked from commit ec00251c88a91615cc0446c8501dccd290484395)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 0688433d7fce12c885afd96d40b66aee0cbe67ab)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoCLEANUP: Update .gitignore
Tim Duesterhus [Sun, 6 Sep 2020 16:34:12 +0000 (18:34 +0200)]
CLEANUP: Update .gitignore

This excludes the currently tracked files from being ignored.

(cherry picked from commit c14d20eda38e48ff3db588e9dad34f56967f72e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 397a7076ba2aaa470866d4a7c3d7da0a32f6f048)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoMINOR: Commit .gitattributes
Tim Duesterhus [Sat, 5 Sep 2020 10:46:11 +0000 (12:46 +0200)]
MINOR: Commit .gitattributes

Commit .gitattributes to the repository to allow GitHub downloads to
properly fill in the SUBVERS and VERDATE files. It also prevents the
engineer in charge of the release to forget creating it when a new branch
is released.

see https://www.mail-archive.com/haproxy@formilux.org/msg38107.html

No functional change, may be backported everywhere.

(cherry picked from commit 78b9c54c666b88abd0c2a001ba23f420be29ea8c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 43bf9ecea6864f90f227190682a1dc4355ef1fb0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUILD: thread: limit the libgcc_s workaround to glibc only
Willy Tarreau [Wed, 2 Sep 2020 07:53:47 +0000 (09:53 +0200)]
BUILD: thread: limit the libgcc_s workaround to glibc only

Previous commit 77b98220e ("BUG/MINOR: threads: work around a libgcc_s
issue with chrooting") broke the build on cygwin. I didn't even know we
supported threads on cygwin. But the point is that it's actually the
glibc-based libpthread which requires libgcc_s, so in absence of other
reports we should not apply the workaround on other libraries.

This should be backported along with the aforementioned patch.

(cherry picked from commit 06a1806083dd9f0d21e045baad4c61559211f956)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e6a4e318db089df3dbadcbed286ed12a3742a7e5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: threads: work around a libgcc_s issue with chrooting
Willy Tarreau [Wed, 2 Sep 2020 06:04:35 +0000 (08:04 +0200)]
BUG/MINOR: threads: work around a libgcc_s issue with chrooting

Sander Hoentjen reported another issue related to libgcc_s in issue #671.
What happens is that when the old process quits, pthread_exit() calls
something from libgcc_s.so after the process was chrooted, and this is
the first call to that library, causing an attempt to load it. In a
chroot, this fails, thus libthread aborts. The behavior widely differs
between operating systems because some decided to use a static build for
this library.

In 2.2 this was resolved as a side effect of a workaround for the same issue
with the backtrace() call, which is also in libgcc_s. This was in commit
0214b45 ("MINOR: debug: call backtrace() once upon startup"). But backtraces
are not necessarily enabled, and we need something for older versions.

By inspecting a significant number of ligcc_s on various gcc versions and
platforms, it appears that a few functions have been present since gcc 3.0,
one of which, _Unwind_Find_FDE() has no side effect (it only returns a
pointer). What this patch does is that in the thread initialization code,
if built with gcc >= 3.0, a call to this function is made in order to make
sure that libgcc_s is loaded at start up time and that there will be no
need to load it upon exit.

An easy way to check which libs are loaded under Linux is :

  $ strace -e trace=openat ./haproxy -v

With this patch applied, libgcc_s now appears during init.

Sander confirmed that this patch was enough to put an end to the core
dumps on exit in 2.0, so this patch should be backported there, and maybe
as far as 1.8.

(cherry picked from commit 77b98220e81207aad854e480e6207d501d476666)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c2678c60e02f0bc9ba22ee3e2b8fe78e714e869a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
William Lallemand [Fri, 31 Jul 2020 09:43:20 +0000 (11:43 +0200)]
BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()

Check the return of the calloc in ssl_sock_load_ocsp() which could lead
to a NULL dereference.

This was introduced by commit be2774d ("MEDIUM: ssl: Added support for
Multi-Cert OCSP Stapling").

Could be backported as far as 1.7.

(cherry picked from commit a560c06af7d9ba68cb0e82d1d25652b9a6a5a336)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ea22337f040fd563b5bd5457976d90568a0b4697)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoRevert "BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action"
Christopher Faulet [Wed, 2 Sep 2020 09:10:38 +0000 (11:10 +0200)]
Revert "BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action"

This reverts commit 4b9c0d1fc08388bf44c6ebbd88f786032dd010fc.

Actually, the "replace-path" action is ambiguous. "set-path" action preserves
the query-string. The "path" sample fetch does not contain the query-string. But
"replace-path" action is documented to handle the query-string. It is probably
not the expected behavior. So instead of fixing the code, we will fix the
documentation to make "replace-path" action consistent with other parts of the
code. In addition actions and sample fetches to handle the path with the
query-string will be added.

If the commit above is ever backported, this one must be as well.

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

5 years agoBUG/MEDIUM: doc: Fix replace-path action description
Christopher Faulet [Wed, 2 Sep 2020 12:16:59 +0000 (14:16 +0200)]
BUG/MEDIUM: doc: Fix replace-path action description

The description of the replace-path action does not reflect what the code
do. When the request path is replaced, the query-string is preserved. But the
documentation stated the query-string is part of the replacement, if any is
present. Most of time, when the doc and the code differ, the code is fixed. But
here, the replace-path action is pretty confusing because the set-path action is
only applied on the path. The query-string is left intact. And the path sample
fetch also ignores the query-string. In addition, the replace-path action is
quite recent. It was added in the 2.2. Thus, exceptionally, the documentation is
fixed instead.

Note that set-pathq and replace-pathq actions and pathq sample fetch will be
added to manipulate the path with the query-string.

This patch must be backported as far as 2.0.

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

5 years agoBUG/MINOR: startup: haproxy -s cause 100% cpu
William Lallemand [Wed, 2 Sep 2020 14:12:23 +0000 (16:12 +0200)]
BUG/MINOR: startup: haproxy -s cause 100% cpu

It was reported in bug #837 that haproxy -s causes a 100% CPU.

However this option does not exist and haproxy must exit with the
usage message.

The parser was not handling the case where -s is not followed by 't' or
'f' which are the only two valid cases.

This bug was introduced by df6c5a ("BUG/MEDIUM: mworker: fix the copy of
options in copy_argv()") which was backported as far as 1.8.

This fix must be backported as far as 1.8.

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

5 years agoBUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of ipv6_address
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:35 +0000 (19:21 +0000)]
BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of ipv6_address

Typo leads to checking the wrong object for memory issues

This patch must be backported as far as 2.0.

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

5 years agoBUG/MINOR: contrib/spoa-server: Updating references to free in case of failure
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:34 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Updating references to free in case of failure

When we encounter a failure, all previously borrowed references should
be freed. Especially if the program is not failing immediately

This patch must be backported as far as 2.0.

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

5 years agoBUG/MINOR: contrib/spoa-server: Do not free reference to NULL
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:33 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Do not free reference to NULL

As per https://docs.python.org/3/c-api/refcounting.html, Py_DECREF
should not be called on NULL objects.

This patch must be backported as far as 2.0.

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

5 years agoBUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:32 +0000 (19:21 +0000)]
BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed

IP addresses references passed in argument for ps_python are not freed after
they have been used. Leading to a small chance of mem leak if a lot of ip
addresses are passed around

This patch must be backported as far as 2.0.

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

5 years agoBUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to memory leak
Gilchrist Dadaglo [Mon, 24 Aug 2020 19:21:31 +0000 (19:21 +0000)]
BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to memory leak

The result from spoa evaluation of the user provided python code is
never passed back to the main spoa process nor freed.
Same for the keyword list passed.
This results into the elements never freed by Python as reference count
never goes down.
https://docs.python.org/3/extending/extending.html#reference-counting-in-python

This patch must be backported as far as 2.0.

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

5 years agoDOC: cache: Use '<name>' instead of '<id>' in error message
Tim Duesterhus [Tue, 18 Aug 2020 20:06:51 +0000 (22:06 +0200)]
DOC: cache: Use '<name>' instead of '<id>' in error message

When the cache name is left out in 'filter cache' the error message refers
to a missing '<id>'. The name of the cache is called 'name' within the docs.

Adjust the error message for consistency.

The error message was introduced in 99a17a2d91f9044ea20bba6617048488aed80555.
This commit first appeared in 1.9, thus the patch must be backported to 1.9+.

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

5 years agoBUG/MINOR: http-rules: Replace path and query-string in "replace-path" action
Christopher Faulet [Mon, 31 Aug 2020 14:27:42 +0000 (16:27 +0200)]
BUG/MINOR: http-rules: Replace path and query-string in "replace-path" action

The documentation stated the "replace-path" action replaces the path, including
the query-string if any is present. But in the code, only the path is
replaced. The query-string is preserved. So, now, instead of relying on the same
action code than "set-uri" action (1), a new action code (4) is used for
"replace-path" action. In http_req_replace_stline() function, when the action
code is 4, we call http_replace_req_path() setting the last argument (with_qs)
to 1. This way, the query-string is not skipped but included to the path to be
replaced.

This patch relies on the commit b8ce505c6 ("MINOR: http-htx: Add an option to
eval query-string when the path is replaced"). Both must be backported as far as
2.0. It should fix the issue #829.

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

5 years agoMINOR: http-htx: Add an option to eval query-string when the path is replaced
Christopher Faulet [Mon, 31 Aug 2020 14:11:57 +0000 (16:11 +0200)]
MINOR: http-htx: Add an option to eval query-string when the path is replaced

The http_replace_req_path() function now takes a third argument to evaluate the
query-string as part of the path or to preserve it. If <with_qs> is set, the
query-string is replaced with the path. Otherwise, only the path is replaced.

This patch is mandatory to fix issue #829. The next commit depends on it. So be
carefull during backports.

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

5 years agoBUG/MINOR: reload: do not fail when no socket is sent
Willy Tarreau [Fri, 28 Aug 2020 16:45:01 +0000 (18:45 +0200)]
BUG/MINOR: reload: do not fail when no socket is sent

get_old_sockets() mistakenly sets ret=0 instead of ret2=0 before leaving
when the old process announces zero FD. So it will return an error
instead of success. This must be particularly rare not to have a
single socket to offer though!

A few comments were added to make it more obvious what to expect in
return.

This must be backported to 1.8 since the bug has always been there.

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

5 years agoBUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
Willy Tarreau [Wed, 12 Aug 2020 12:04:52 +0000 (14:04 +0200)]
BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction

It is possible to process a channel based on desynchronized info if a
request fetch is called from a response and conversely. However, the
code in smp_prefetch_htx() already makes sure the analysis has already
started before trying to fetch from a buffer, so the problem effectively
lies in response rules making use of request expressions only.

Usually it's not a problem as extracted data are checked against the
current HTTP state, except when it comes to the start line, which is
usually accessed directly from sample fetch functions such as status,
path, url, url32, query and so on. In this case, trying to access the
request buffer from the response path will lead to unpredictable
results. When building with DEBUG_STRICT, a process violating these
rules will simply die after emitting:

  FATAL: bug condition "htx->first == -1" matched at src/http_htx.c:67

But when this is not enabled, it may or may not crash depending on what
the pending request buffer data look like when trying to spot a start
line there. This is typically what happens in issue #806.

This patch adds a test in smp_prefetch_htx() so that it does not try
to parse an HTX buffer in a channel belonging to the wrong direction.

There's one special case on the "method" sample fetch since it can
retrieve info even without a buffer, from the other direction, as
long as the method is one of the well known ones. Three, we call
smp_prefetch_htx() only if needed.

This was reported in 2.0 and must be backported there (oldest stable
version with HTX).

(cherry picked from commit a6d9879e69d65e8821347860394829c4dc72f937)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 05f3910f58a8d5e0710a4a8eea82f8bc24861e32)
[wt: adjusted context; chn already checked; 3 args to smp_prefetch_htx()]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: stats: use strncmp() instead of memcmp() on health states
Willy Tarreau [Tue, 11 Aug 2020 08:26:36 +0000 (10:26 +0200)]
BUG/MINOR: stats: use strncmp() instead of memcmp() on health states

The reports for health states are checked using memcmp() in order to
only focus on the first word and possibly ignore trailing %d/%d etc.
This makes gcc unhappy about a potential use of "" as the string, which
never happens since the string is always set. This resulted in commit
c4e6460f6 ("MINOR: build: Disable -Wstringop-overflow.") to silence
these messages. However some lengths are incorrect (though cannot cause
trouble), and in the end strncmp() is just safer and cleaner.

This can be backported to all stable branches as it will shut a warning
with gcc 8 and above.

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

5 years agoBUG/MINOR: snapshots: leak of snapshots on deinit()
William Lallemand [Fri, 7 Aug 2020 12:48:37 +0000 (14:48 +0200)]
BUG/MINOR: snapshots: leak of snapshots on deinit()

Free the snapshots on deinit() when they were initialized in a proxy
upon an error.

This was introduced by c55015e ("MEDIUM: snapshots: dynamically allocate
the snapshots").

Should be backported as far as 1.9.

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

5 years agoMEDIUM: lua: Don't filter exported fetches and converters
Christopher Faulet [Thu, 6 Aug 2020 08:32:28 +0000 (10:32 +0200)]
MEDIUM: lua: Don't filter exported fetches and converters

Thanks to previous commits, it is now safe to use from lua the sample fetches
and sample converters that convert arguments, especially the strings
(ARGT_STR). So now, there are all exported to the lua. They was filtered on the
validation functions. Only fetches without validation functions or with val_hdr
or val_payload_lv functions were exported, and converters without validation
functions.

This patch depends on following commits :

  * aec27ef44 "BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array"
  * fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"

It must be backported as far as 2.1 because the date() and http_date()
converters are no longer exported because of the filter on the validation
function, since the commit ae6f125c7 ("MINOR: sample: add us/ms support to
date/http_date)".

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

5 years agoBUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
Christopher Faulet [Thu, 6 Aug 2020 06:54:25 +0000 (08:54 +0200)]
BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array

Strings in the argument array used by sample fetches and converters must be
duplicated. This is mandatory because, during the arguments validations, these
strings may be converted and released. It works this way during the
configuration parsing and there is no reason to adapt this behavior during the
runtime when a sample fetch or a sample converter is called from the lua. In
fact, there is a reason to not change the behavior. It must reamain simple for
everyone to add new fetches or converters.

Thus, lua strings are duplicated. It is only performed at the end of the
hlua_lua2arg_check() function, if the argument is still a ARGT_STR. Of course,
it requires a cleanup loop after the call or when an error is triggered.

This patch depends on following commits:

  * 959171376 "BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters"
  * fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"

It may be backported to all supported versions, most probably as far as 2.1
only.

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

5 years agoMINOR: hlua: Don't needlessly copy lua strings in trash during args validation
Christopher Faulet [Thu, 6 Aug 2020 06:29:18 +0000 (08:29 +0200)]
MINOR: hlua: Don't needlessly copy lua strings in trash during args validation

Lua strings are NULL terminated. So in the hlua_lua2arg_check() function, used
to check arguments against the sample fetches specification, there is no reason
to copy these strings in a trash to add a terminating null byte.

In addition, when the array of arguments is built from lua values, we must take
care to count this terminating null bytes in the size of the buffer where a
string is stored. The same must be done when a sample is built from a lua value.

This patch may be backported to easy backports.

(cherry picked from commit fdea1b6319bc014320a18ba7b7043ba0694dd0dd)
[wt: backported for next commit]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f657d400b442b83d1565579c2b78eba9d4e12abc)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
Christopher Faulet [Fri, 7 Aug 2020 07:11:22 +0000 (09:11 +0200)]
BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation

In hlua_lua2arg_check() function, before converting an argument to an IPv4 or
IPv6 mask, we must be sure to have an integer or a string argument (ARGT_SINT or
ARGT_STR).

This patch must be backported to all supported versions.

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

5 years agoBUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation
Christopher Faulet [Fri, 7 Aug 2020 07:07:26 +0000 (09:07 +0200)]
BUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation

In hlua_lua2arg_check() function, before converting a string to an IP address,
we must be to sure to have a string argument (ARGT_STR).

This patch must be backported to all supported versions.

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

5 years agoBUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
Christopher Faulet [Wed, 5 Aug 2020 21:07:07 +0000 (23:07 +0200)]
BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters

Some sample fetches or sample converters uses a validation functions for their
arguments. In these function, string arguments (ARGT_STR) may be converted to
another type (for instance a regex, a variable or a integer). Because these
strings are allocated when the argument list is built, they must be freed after
a conversion. Most of time, it is done. But not always. This patch fixes these
minor memory leaks (only on few strings, during the configuration parsing).

This patch may be backported to all supported versions, most probably as far as
2.1 only. If this commit is backported, the previous one 73292e9e6 ("BUG/MINOR:
lua: Duplicate map name to load it when a new Map object is created") must also
be backported. Note that some validation functions does not exists on old
version. It should be easy to resolve conflicts.

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

5 years agoBUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
Christopher Faulet [Thu, 6 Aug 2020 06:40:09 +0000 (08:40 +0200)]
BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created

When a new map is created, the sample_load_map() function is called. To do so,
an argument array is created with the name as first argument. Because it is a
lua string, owned by the lua, it must be duplicated. The sample_load_map()
function will convert this argument to a map. In theory, after the conversion,
it must release the original string. It is not performed for now and it is a bug
that will be fixed in the next commit.

This patch may be backported to all supported versions, most probably as far as
2.1 only. But it must be backported with the next commit "BUG/MINOR: arg: Fix
leaks during arguments validation for fetches/converters".

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

5 years agoBUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
Christopher Faulet [Fri, 7 Aug 2020 12:00:23 +0000 (14:00 +0200)]
BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter

The debug() converter uses a string to reference the sink where to send debug
events. During the configuration parsing, this string is converted to a sink
object but it is still store as a string argument. It is a problem on deinit
because string arguments are released. So the sink pointer will be released
twice.

To fix the bug, we keep a reference on the sink using an ARGT_PTR argument. This
way, it will not be freed on the deinit.

This patch depends on the commit e02fc4d0d ("MINOR: arg: Add an argument type to
keep a reference on opaque data"). Both must be backported as far as 2.1.

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

5 years agoMINOR: arg: Add an argument type to keep a reference on opaque data
Christopher Faulet [Fri, 7 Aug 2020 11:56:00 +0000 (13:56 +0200)]
MINOR: arg: Add an argument type to keep a reference on opaque data

The ARGT_PTR argument type may now be used to keep a reference to opaque data in
the argument array used by sample fetches and converters. It is a generic way to
point on data. I guess it could be used for some other arguments, like proxy,
server, map or stick-table.

(cherry picked from commit e02fc4d0dd57c92bbe96f3ff2ae0b890405458f2)
[wt: needed by next commit]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit bf1fcab0670c3a94270d19c50a23a88b3d0edfba)
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
Christopher Faulet [Wed, 5 Aug 2020 21:23:37 +0000 (23:23 +0200)]
BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime

In sample_load_map() function, the global mode is now tested to be sure to be in
the starting mode. If not, an error is returned.

At first glance, this patch may seem useless because maps are loaded during the
configuration parsing. But in fact, it is possible to load a map from the lua,
using Map:new() method. And, there is nothing to forbid to call this method at
runtime, during a script execution. It must never be done because it may perform
an filesystem access for unknown maps or allocation for known ones. So at
runtime, it means a blocking call or a memroy leak. Note it is still possible to
load a map from the lua, but in the global part of a script only. This part is
executed during the configuration parsing.

This patch must be backported in all stable versions.

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

5 years agoBUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
William Lallemand [Tue, 4 Aug 2020 15:41:39 +0000 (17:41 +0200)]
BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()

This bug affects all version of HAProxy since the OCSP data is not free
in the deinit(), but leaking on exit() is not really an issue. However,
when doing dynamic update of certificates over the CLI, those data are
not free'd upon the free of the SSL_CTX.

3 leaks are happening, the first leak is the one of the ocsp_arg
structure which serves the purpose of containing the pointers in the
case of a multi-certificate bundle. The second leak is the one ocsp
struct. And the third leak is the one of the struct buffer in the
ocsp_struct.

The problem lies with SSL_CTX_set_tlsext_status_arg() which does not
provide a way to free the argument upon an SSL_CTX_free().

This fix uses ex index functions instead of registering a
tlsext_status_arg(). This is really convenient because it allows to
register a free callback which will free the ex index content upon a
SSL_CTX_free().

A refcount was also added to the ocsp_response structure since it is
stored in a tree and can be reused in another SSL_CTX.

Should fix part of the issue #746.

This must be backported in 2.2 and 2.1.

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

5 years agoBUG/MINOR: ssl: fix memory leak at OCSP loading
William Lallemand [Thu, 6 Aug 2020 22:44:32 +0000 (00:44 +0200)]
BUG/MINOR: ssl: fix memory leak at OCSP loading

Fix a memory leak when loading an OCSP file when the file was already
loaded elsewhere in the configuration.

Indeed, if the OCSP file already exists, a useless chunk_dup() will be
done during the load.

To fix it we reverts "ocsp" to "iocsp" like it was done previously.

This was introduced by commit 246c024 ("MINOR: ssl: load the ocsp
in/from the ckch").

Should fix part of the issue #746.

It must be backported in 2.1 and 2.2.

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

5 years agoSCRIPTS: git-show-backports: emit the shell command to backport a commit
Willy Tarreau [Fri, 31 Jul 2020 14:47:44 +0000 (16:47 +0200)]
SCRIPTS: git-show-backports: emit the shell command to backport a commit

It's cumbersome to copy-paste a commit ID into another window after having
typed "git cherry-pick -sx", so let's have the suggested output format of
git-show prepare this line just before the subject line, it remains at a
stable position on the terminal when searching for "/^commit". One just
has to copy-paste the line into another terminal will result in the commit
being properly picked.

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

5 years agoSCRIPTS: git-show-backports: make -m most only show the left branch
Willy Tarreau [Fri, 31 Jul 2020 14:38:57 +0000 (16:38 +0200)]
SCRIPTS: git-show-backports: make -m most only show the left branch

We've never used the output of the rightmost branch with this tool,
and it systematically causes two identical outputs making the job
harder during backport sessions. Let's simply remove the right part
when it's identical to the left one. This also adds a few line feeds
to make the output more readable.

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

5 years agoBUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
Christopher Faulet [Wed, 5 Aug 2020 09:31:16 +0000 (11:31 +0200)]
BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send

The H1 multiplexer is able to perform synchronous send. When a large body is
transfer, if nothing is received and if no error or shutdown occurs, it is
possible to not go down at the H1 connection level to do I/O for a long
time. When this happens, we must still take care to refresh the H1 connection
timeout. Otherwise it is possible to hit the connection timeout during the
transfer while it should not expire.

This bug exists because only h1_process() refresh the H1 connection timeout. To
fix the bug, h1_snd_buf() must also refresh this timeout. To make things more
readable, a dedicated function has been introduced and called to refresh the
timeout.

This bug exists on all HTX versions. But it is harder to hit it on 2.1 and below
because when a H1 mux is initialized, we actively try to read data instead of
subscribing for receiving. So there is at least one call to h1_process().

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

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

5 years ago[RELEASE] Released version 2.1.8 v2.1.8
Willy Tarreau [Fri, 31 Jul 2020 11:34:55 +0000 (13:34 +0200)]
[RELEASE] Released version 2.1.8

Released version 2.1.8 with the following main changes :
    - BUG/MEDIUM: log: don't hold the log lock during writev() on a file descriptor
    - BUG/MEDIUM: pattern: fix thread safety of pattern matching
    - BUILD: make dladdr1 depend on glibc version and not __USE_GNU
    - REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv
    - REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for compression/lua_validation
    - BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
    - BUG/MEDIUM: ssl: crt-list must continue parsing on ERR_WARN
    - MINOR: http: Add 410 to http-request deny
    - MINOR: http: Add 404 to http-request deny
    - BUG/MINOR: http: make smp_fetch_body() report that the contents may change
    - BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
    - BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
    - BUG/MINOR: spoe: add missing key length check before checking key names
    - BUG/MINOR: cli: allow space escaping on the CLI
    - BUG/MINOR: mworker/cli: fix the escaping in the master CLI
    - BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
    - REGTEST: http-rules: test spaces in ACLs
    - REGTEST: http-rules: test spaces in ACLs with master CLI
    - MEDIUM: map: make the "clear map" operation yield
    - BUG/MINOR: systemd: Wait for network to be online
    - REGTEST: Add a simple script to tests errorfile directives in proxy sections
    - BUG/MINOR: spoe: correction of setting bits for analyzer
    - BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
    - MINOR: spoe: Don't systematically create new applets if processing rate is low
    - REGTEST: ssl: tests the ssl_f_* sample fetches
    - REGTEST: ssl: add some ssl_c_* sample fetches test
    - BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL
    - MINOR: cli: make "show sess" stop at the last known session
    - DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
    - BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
    - BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
    - BUG/MINOR: proxy: always initialize the trash in show servers state
    - DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
    - DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
    - BUILD: haproxy: fix build error when RLIMIT_AS is not set
    - BUG/MINOR: http_act: don't check capture id in backend (2)
    - BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
    - BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
    - BUG/MINOR: mux-h1: Disable splicing only if input data was processed
    - BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
    - MINOR: mux-h1: Improve traces about the splicing
    - BUG/MEDIUM: mux-h1: Subscribe rather than waking up in h1_rcv_buf()
    - MINOR: connection: move the CO_FL_WAIT_ROOM cleanup to the reader only
    - BUG/MEDIUM: connection: Continue to recv data to a pipe when the FD is not ready
    - BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
    - MINOR: http: Add support for http 413 status
    - DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
    - BUG/MINOR: sample: Free str.area in smp_check_const_bool
    - BUG/MINOR: sample: Free str.area in smp_check_const_meth
    - BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
    - BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
    - CONTRIB: da: fix memory leak in dummy function da_atlas_open()
    - BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
    - BUG/MINOR: mux-fcgi: Handle empty STDERR record
    - BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
    - BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
    - BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
    - BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
    - BUG/MEDIUM: server: resolve state file handle leak on reload
    - BUG/MEDIUM: server: fix possibly uninitialized state file on close
    - BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
    - BUILD: ebtree: fix build on libmusl after recent introduction of eb_memcmp()
    - REGEST: Add reg tests about error files
    - BUG/MINOR: threads: Don't forget to init each thread toremove_lock.
    - MINOR: pools: increase MAX_BASE_POOLS to 64
    - BUILD: thread: add parenthesis around values of locking macros
    - BUG/MINOR: cfgparse: don't increment linenum on incomplete lines
    - BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
    - BUG/MAJOR: dns: Make the do-resolve action thread-safe
    - BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
    - BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
    - BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are expected
    - BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
    - BUILD: tools: fix build with static only toolchains
    - BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
    - MEDIUM: lua: Add support for the Lua 5.4
    - BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
    - BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
    - MINOR: connection: Preinstall the mux for non-ssl connect
    - MINOR: stream-int: Be sure to have a mux to do sends and receives
    - SCRIPTS: announce-release: add the link to the wiki in the announce messages
    - BUG/MEDIUM: backend: always attach the transport before installing the mux

5 years agoBUG/MEDIUM: backend: always attach the transport before installing the mux
Willy Tarreau [Fri, 31 Jul 2020 06:39:31 +0000 (08:39 +0200)]
BUG/MEDIUM: backend: always attach the transport before installing the mux

In connect_server(), we can enter in a stupid situation:

  - conn_install_mux_be() is called to install the mux. This one
    subscribes for receiving and quits ;

  - then we discover that a handshake is required on the connection
    (e.g. send-proxy), so xprt_add_hs() is called and subscribes as
    well.

  - we crash in conn_subscribe() which gets a different subscriber.
    And if BUG_ON is disabled, we'd likely lose one event.

Note that it doesn't seem to happen by default, but definitely does
if connect() rightfully performs fd_cant_recv(), so it's a matter of
who does what and in what order.

A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue

  listen foo
    bind :8181
    mode http
    server srv1 127.0.0.1:8888 send-proxy-v2

The root cause is that xprt_add_hs() installs an xprt layer underneath
the mux without taking over its subscriptions. Ideally if we want to
support this, we'd need to steal the connection's wait_events and
replace them by new ones. But there doesn't seem to be any case where
we're interested in doing this so better simply always install the
transport layer before installing the mux, that's safer and simpler.

This needs to be backported to 2.1 which is constructed the same way
and thus suffers from the same issue, though the code is slightly
different there.

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

5 years agoSCRIPTS: announce-release: add the link to the wiki in the announce messages
Willy Tarreau [Thu, 30 Jul 2020 15:41:42 +0000 (17:41 +0200)]
SCRIPTS: announce-release: add the link to the wiki in the announce messages

Let's add the link to the wiki to the announce messages, plenty of
users don't even know it exists.

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

5 years agoMINOR: stream-int: Be sure to have a mux to do sends and receives
Christopher Faulet [Thu, 30 Jul 2020 07:26:46 +0000 (09:26 +0200)]
MINOR: stream-int: Be sure to have a mux to do sends and receives

In si_cs_send() and si_cs_recv(), we explicitly test the connection's mux is
defined to proceed. For si_cs_recv(), it is probably a bit overkill. But
opportunistic sends are possible from the moment the server connection is
created. So it is safer to do this test.

This patch may be backported as far as 1.9 if necessary.

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

5 years agoMINOR: connection: Preinstall the mux for non-ssl connect
Christopher Faulet [Thu, 30 Jul 2020 07:10:36 +0000 (09:10 +0200)]
MINOR: connection: Preinstall the mux for non-ssl connect

In the connect_server() function, there is an optim to install the mux as soon
as possible. It is possible if we can determine the mux to use from the
configuration only. For instance if the mux is explicitly specified or if no ALPN
is set. This patch adds a new condition to preinstall the mux for non-ssl
connection. In this case, by default, we always use the mux_pt for raw
connections and the mux-h1 for HTTP ones.

This patch is related to the issue #762. It may be backported to 2.2 (and
possibly as far as 1.9 if necessary).

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

5 years agoBUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
Christopher Faulet [Wed, 29 Jul 2020 10:00:23 +0000 (12:00 +0200)]
BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields

On a tcp-response content ruleset evaluation, the inspect-delay is engaged when
rule's conditions are not validated but not when the rule's action yields.

This patch must be backported to all supported versions.

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

5 years agoBUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
Christopher Faulet [Tue, 28 Jul 2020 08:21:54 +0000 (10:21 +0200)]
BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation

When an action is evaluated, flags are passed to know if it is the first call
(ACT_OPT_FIRST) and if it must be the last one (ACT_OPT_FINAL). For the
do-resolve DNS action, the ACT_OPT_FINAL flag must be handled because the
action may yield. It must never yield when this flag is set. Otherwise, it may
lead to a wakeup loop of the stream because the inspected-delay of a tcp-request
content ruleset was reached without stopping the rules evaluation.

This patch is related to the issue #222. It must be backported as far as 2.0.

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

5 years agoMEDIUM: lua: Add support for the Lua 5.4
Christopher Faulet [Tue, 28 Jul 2020 08:33:25 +0000 (10:33 +0200)]
MEDIUM: lua: Add support for the Lua 5.4

On Lua 5.4, some API changes make HAProxy compilation to fail. Among other
things, the lua_resume() function has changed and now takes an extra argument in
Lua 5.4 and the error LUA_ERRGCMM was removed. Thus the LUA_VERSION_NUM macro is
now tested to know the lua version is used and adapt the code accordingly.

Here are listed the incompatibilities with the previous Lua versions :

   http://www.lua.org/manual/5.4/manual.html#8

This patch comes from the HAproxy's fedora RPM, committed by Tom Callaway :

  https://src.fedoraproject.org/rpms/haproxy/blob/db970613/f/haproxy-2.2.0-lua-5.4.patch

This patch should fix the issue #730. It must be backported to 2.2 and probably
as far as 2.0.

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

5 years agoBUG/MINOR: debug: Don't dump the lua stack if it is not initialized
Christopher Faulet [Fri, 24 Jul 2020 17:08:05 +0000 (19:08 +0200)]
BUG/MINOR: debug: Don't dump the lua stack if it is not initialized

When the watchdog is fired because of the lua, the stack of the corresponding
lua context is dumped. But we must be sure the lua context is fully initialized
to do so. If we are blocked on the global lua lock, during the lua context
initialization, the lua stask may be NULL.

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

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

5 years agoBUILD: tools: fix build with static only toolchains
Baruch Siach [Fri, 24 Jul 2020 04:52:20 +0000 (07:52 +0300)]
BUILD: tools: fix build with static only toolchains

uClibc toolchains built with no dynamic library support don't provide
the dlfcn.h header. That leads to build failure:

CC src/tools.o
src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory
 #include <dlfcn.h>
          ^~~~~~~~~
Enable dladdr on Linux platforms only when USE_DL is defined.

This should be backported wherever 109201fc5 ("BUILD: tools: rely on
__ELF__ not USE_DL to enable use of dladdr()") is backported (currently
only 2.2 and 2.1).

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

5 years agoBUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
Christopher Faulet [Fri, 24 Jul 2020 14:13:12 +0000 (16:13 +0200)]
BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received

When nothing is received when xprt->rcv_pipe() is called, and if the pipe is
empty, the mux rcv_buf() callback is called. Because the splicing is enabled,
nothing is performed and the H1 connection is woken up. Unfortunately, the H1
connection will do the same with the H1 stream. Thus, while nothing is received,
a ping-pong loop between the H1 connection and a H1 stream will consume all the
CPU. To avoid this loop, the splicing is disabled in this case. This way, the H1
connection will try to read data and it will wake up the H1 stream only if it
reads something.

This bug was previously fixed by the commit a1dde23 ("BUG/MEDIUM: mux-h1:
Subscribe rather than waking up in h1_rcv_buf()"). But it was reverted because
it relies on not-backported changes on the connection layer.

This patch must be backported to 2.0.

5 years agoBUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are expected
Christopher Faulet [Fri, 24 Jul 2020 14:31:14 +0000 (16:31 +0200)]
BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are expected

After the input processing, in h1_rcv_buf(), if more data are expected and the
H1C is not subscribed on receives, we now wake up the H1C to try to read more
data ASAP. This reverts commit a1dde23e. Depending on how data are received,
subscribing on receives at this stage may freeze the H1 connection because a
read event is missing.

This bug seems to no exist in the 2.2 because of changes on the connection
subscriptions. But we must be careful because I don't really know why the read
event is missed.

Moreover, the bug fixed by the commit a1dde23e will be fixed another way in
the next commit.

This patch should fix the issue #774. It must be backported to 2.0.

5 years agoBUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
Christopher Faulet [Thu, 23 Jul 2020 13:44:37 +0000 (15:44 +0200)]
BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore

In the CGI/1.1 specification, it is specified the QUERY_STRING must not be
url-decoded. However, this parameter is sent decoded because it is extracted
after the URI's path decoding. Now, the query-string is first extracted, then
the script part of the path is url-decoded. This way, the QUERY_STRING parameter
is no longer decoded.

This patch should fix the issue #769. It must be backported as far as 2.1.

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

5 years agoBUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
Christopher Faulet [Wed, 22 Jul 2020 13:55:49 +0000 (15:55 +0200)]
BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed

When a DNS resolution is freed, the remaining items in .ar_list and .answer_list
are also released. It must be done to avoid a memory leak. And it is the last
chance to release these objects. I've honestly no idea if there is a better
place to release them earlier. But at least, there is no more leak.

This patch should solve the issue #222. It must be backported, at least, as far
as 2.0, and probably, with caution, as far as 1.8 or 1.7.

(cherry picked from commit 010ab35a9118daf17a670fb2b42e40447f967f7c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c58ac80d00284886b108b209a5bf993de5ab38ed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MAJOR: dns: Make the do-resolve action thread-safe
Christopher Faulet [Wed, 22 Jul 2020 09:46:32 +0000 (11:46 +0200)]
BUG/MAJOR: dns: Make the do-resolve action thread-safe

The do-resolve HTTP action, performing a DNS resolution of a sample expression
output, is not thread-safe at all. The resolver object used to do the resolution
must be locked when the action is executed or when the stream is released
because its curr or wait resolution lists and the requester list inside a
resolution are updated. It is also important to not wake up a released stream
(with a destroyed task).

Of course, because of this bug, various kind of crashes may be observed.

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

(cherry picked from commit 5098a08c2fafb0d9513996729d2a30c9785378f3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 99f4623952cbbad2bcae451abdd0f3133bcbe75c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: resolve: fix init resolving for ring and peers section.
Emeric Brun [Tue, 21 Jul 2020 14:54:36 +0000 (16:54 +0200)]
BUG/MEDIUM: resolve: fix init resolving for ring and peers section.

Reported github issue #759 shows there is no name resolving
on server lines for ring and peers sections.

This patch introduce the resolving for those lines.

This patch adds  boolean a parameter to parse_server function to specify
if we want the function to perform an initial name resolving using libc.

This boolean is forced to true in case of peers or ring section.

The boolean is kept to false in case of classic servers (from
backend/listen)

This patch should be backported in branches where peers sections
support 'server' lines.

(cherry picked from commit d3db3846c5d2f3a2a27a6055d28d146838b7189d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit de103017907a09d257fb0d3d75cc1dcef8588ada)
[wt: dropped ring changes for 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: cfgparse: don't increment linenum on incomplete lines
Willy Tarreau [Thu, 25 Jun 2020 07:37:54 +0000 (09:37 +0200)]
BUG/MINOR: cfgparse: don't increment linenum on incomplete lines

When fgets() returns an incomplete line we must not increment linenum
otherwise line numbers become incorrect. This may happen when parsing
files with extremely long lines which require a realloc().

The bug has been present since unbounded line length was supported, so
the fix should be backported to older branches.

(cherry picked from commit 40cb26f6ec8cf9d8ba54706d95df4cfed3a7b332)
[wt: adjusted context wrt missing fatal++]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUILD: thread: add parenthesis around values of locking macros
Willy Tarreau [Fri, 12 Jun 2020 09:42:25 +0000 (11:42 +0200)]
BUILD: thread: add parenthesis around values of locking macros

clang just failed on fd.c with this error:

  src/fd.c:491:9: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses]
          while (HA_SPIN_TRYLOCK(OTHER_LOCK, &log_lock) != 0) {
                 ^                                      ~~
That's because this expands to this:

          while (!pl_try_s(&log_lock) != 0) {

Let's just add parenthesis in the TRYLOCK macros to avoid this.
This may need to be backported if commit df187875d ("BUG/MEDIUM: log:
don't hold the log lock during writev() on a file descriptor") is
backported as well as it seems to be the first one to trigger it.

(cherry picked from commit db57a142c31016ff3e0dd533cb2b4de14445781e)
[wt: applied by hand to common/hathread.h since context significantly differed]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoMINOR: pools: increase MAX_BASE_POOLS to 64
Willy Tarreau [Tue, 30 Jun 2020 12:29:02 +0000 (14:29 +0200)]
MINOR: pools: increase MAX_BASE_POOLS to 64

When not sharing pools (i.e. when building with -DDEBUG_DONT_SHARE_POOLS)
we have about 47 pools right now, while MAX_BASE_POOLS is only 32, meaning
that only the first 32 ones will benefit from a per-thread cache entry.
This totally kills performance when pools are not shared (roughly -20%).

Let's double the limit to gain some margin, and make it possible to set
it as a build option.

It might be useful to backport this to stable versions as they're likely
to be affected as well.

(cherry picked from commit daf8aa62a8d0210f81db5c93235da19d5ed22ab3)
[wt: applied to include/common/memory.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUG/MINOR: threads: Don't forget to init each thread toremove_lock.
Olivier Houchard [Mon, 29 Jun 2020 15:48:27 +0000 (17:48 +0200)]
BUG/MINOR: threads: Don't forget to init each thread toremove_lock.

Don't forget to use HA_SPIN_INIT() on each toremove_lock, or DEBUG_THREAD may
not work reliably with it.

This should be backported to 2.1 and 2.0.

(cherry picked from commit f21695bd8b59d582efc99c85f9a1afac200eda81)
[wt: the lock is called toremove_lock[i] in 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoREGEST: Add reg tests about error files
Christopher Faulet [Mon, 20 Jan 2020 12:49:48 +0000 (13:49 +0100)]
REGEST: Add reg tests about error files

2 reg tests are added. The first one ensures the declaration of errors in a
proxy is fonctionnal. It declares http-errors sections and declare error files
using the errorfile and the errorfiles directives, both in the default section
and the frontend sections. The second one ensures it is possible to use a custom
error file for an HTTP deny rule.

(cherry picked from commit a5afb0bf3668851d130d75ccd7dbf4fcadd3a2b7)
[wt: needed since the backport of 60837d340 ("REGTEST: Add a simple
 script to tests errorfile directives in proxy sections")]
Signed-off-by: Willy Tarreau <w@1wt.eu>

5 years agoBUILD: ebtree: fix build on libmusl after recent introduction of eb_memcmp()
Willy Tarreau [Mon, 20 Jul 2020 19:40:14 +0000 (21:40 +0200)]
BUILD: ebtree: fix build on libmusl after recent introduction of eb_memcmp()

In order to address a corner case with memory compares, commit 853926a
("BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory
blocks") introduced a new eb_memcmp() function, which uses ssize_t for
an offset. However this one is not known by default in ebtree for versions
prior to 2.2, and it depends on the libs. Musl requires unistd to be included
to have it.

This patch just adds this harmless include, which was verified to address
issue #760. There is no mainline commit ID as 2.2 and above already include
unistd.h as part of haproxy/api.h. It must be backported to older versions
(2.0 already has the commit above).

5 years agoBUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
Christopher Faulet [Thu, 16 Jul 2020 09:43:46 +0000 (11:43 +0200)]
BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked

The CF_SHUTW_NOW flag must be handled the same way than the CF_SHUTW flag in
co_getblk_nc() and co_getline_nc() functions. It is especally important when we
try to peek a line from outgoing data. In this case, an unfinished line is
blocked an nothing is peeked if the CF_SHUTW_NOW flag is set. But the blocked
data pevent the transition to CF_SHUTW.

The above functions are only used by LUA cosockets. Because of this bug, we may
experienced wakeups in loop of the cosocket's io handler if we try to read a
line on a closed socket with a pending unfinished line (no LF found at the end).

This patch should fix issue #744. It must be backported to all supported
versions.

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

5 years agoBUG/MEDIUM: server: fix possibly uninitialized state file on close
Willy Tarreau [Thu, 16 Jul 2020 04:40:27 +0000 (06:40 +0200)]
BUG/MEDIUM: server: fix possibly uninitialized state file on close

Previous fix dc6e8a9a7 ("BUG/MEDIUM: server: resolve state file handle
leak on reload") traded a bug for another one, now we get this warning
when building server.c, which is valid since f is not necessarily
initialized (e.g. if no global state file is passed):

  src/server.c: In function 'apply_server_state':
  src/server.c:3272:3: warning: 'f' may be used uninitialized in this function [-Wmaybe-uninitialized]
     fclose(f);
   ^~~~~~~~~

Let's initialize it first. This whole code block should really be
splitted, cleaned up and reorganized as it's possible that other
similar bugs are hidden in it.

This must be backported to the same branches the commit above is
backported to (likely 2.2 and 2.1).

(cherry picked from commit 2d067f93fbbecc2b25bb0374a1cd2552299f19f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4881a40915f618a8b7455f5229f46724e6f9a70b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: server: resolve state file handle leak on reload
Ilya Shipitsin [Wed, 15 Jul 2020 21:02:40 +0000 (02:02 +0500)]
BUG/MEDIUM: server: resolve state file handle leak on reload

During reload, server state file is read and file handle is not released
this was indepently reported in #738 and #660.

partially resolves #660. This should be backported to 2.2 and 2.1.

(cherry picked from commit dc6e8a9a7b19052b8fd8ab7f9a84a4a95b4a6d5b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit d74774bca53a2f4c30995d01eeb028dde313fc29)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
Harris Kaufmann [Wed, 15 Jul 2020 14:26:13 +0000 (16:26 +0200)]
BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers

When the loop is continued early, the memory for param_rule is not freed. This
can leak memory per request, which will eventually consume all available memory
on the server.

This patch should fix the issue #750. It must be backported as far as 2.1.

(cherry picked from commit b605a736b0766030746b50c322879d929fcebacf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4f472bedc8c696f9c18da0eb16904a98a37f076c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
Emeric Brun [Fri, 10 Jul 2020 13:47:11 +0000 (15:47 +0200)]
BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.

A boolean was mistakenly declared 'static THREAD_LOCAL' causing
the probe of a log to a 'not sampled' log server conditionned by
the last evaluated 'sampled log' server test on the same thread.

This results to unpredictable drops of logs on 'not sampled'
log servers as soon a 'sampled' log server is declared.

This patch removes the static THREAD_LOCAL attribute from this
boolean, fixing the issue and allowing to mix 'sampled' and
'not sampled' servers.

This fix should be backported in any branches which includes
the log sampling feature.

(cherry picked from commit 2f4cc28e0f1c8c3deca8b1b7fa446c6523dcc93c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 9ce61f6d578876f1eda258d4cfad997e09c140a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
Christopher Faulet [Wed, 15 Jul 2020 14:04:49 +0000 (16:04 +0200)]
BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT

In fcgi_strm_handle_empty_stdout(), the FCGI_SF_ES_RCVD flag is set on "->state"
stream field instead of "->flags". It is obviously wrong. This bug is not
noticeable because the right state is set in the fcgi_process_demux() function a
bit later.

This patch must be backported as far as 2.1.

(cherry picked from commit 3b3096ede1b52007fa49a563436df8ee9323d78c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ca5e8c058e157ee7d549014705e23e3be6fc505c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
Christopher Faulet [Wed, 15 Jul 2020 13:55:52 +0000 (15:55 +0200)]
BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding

When the padding of a "stream" record (STDOUT or STDERR) is skipped, we must set
the connection state to RECORD_P. It is especially important if the padding is
not fully received.

This patch must be backported as far as 2.1.

(cherry picked from commit 6c99d3baeabef9aa3a798b2251215f1026b48751)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 82cffcd95ae7dc81387344e69432c1776d02102c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: mux-fcgi: Handle empty STDERR record
Christopher Faulet [Wed, 15 Jul 2020 13:46:30 +0000 (15:46 +0200)]
BUG/MINOR: mux-fcgi: Handle empty STDERR record

As mentionned in the FastCGI specification, FCGI "streams" are series of
non-empty stream records (length != 0), followed by an empty one. It is properly
handled for FCGI_STDOUT records, but not for FCGI_STDERR ones. If an empty
FCGI_STDERR record is received, the connection is blocked waiting for data which
will never come.

To fix the bug, when an empty FCGI_STDERR record is received, we drop it, eating
the padding if any.

This patch should fix the issue #743. It must be backported as far as 2.1.

(cherry picked from commit 7f85433a912529c5cda1629001b34fd2b2e54758)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit bd6dc07038f4c6b552559ccde41d31ff9a7dbf6d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
Christopher Faulet [Fri, 10 Jul 2020 08:01:26 +0000 (10:01 +0200)]
BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode

When input data are processed, if the request is switched in tunnel mode on a
protocol upgrade, we must continue the processing. Otherwise, pending input data
will only be processed on the next wakeup. So when new input data are received,
on a timeout expiration or shutdown. Worst, if the input buffer is full when it
happens, only a timeout or a shutdown will unblock the situation.

This patch should fix the issue #737. It must be backported as far as 1.9. The
bug does not seem to affect the 2.0 and 1.9 because, on a protocol upgrade, the
request is switched in tunnel mode when the response is sent to the client. But
the bug is present, so the backport remains necessary.

(cherry picked from commit 23021ad7f1d9d0e924aa5e5c6c4103b51a805af0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4814c74842d13dcf8d948910902f248263f80eed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoCONTRIB: da: fix memory leak in dummy function da_atlas_open()
Willy Tarreau [Sun, 12 Jul 2020 07:12:07 +0000 (09:12 +0200)]
CONTRIB: da: fix memory leak in dummy function da_atlas_open()

The dummy function takes care of doing a bit of work using a malloc()
to avoid returning a constant but it doesn't free the tested pointer,
which coverity noticed in issue #741. Let's free it before testing it
for the return value.

This may be backported but is not important since this code is only
present to allow to build the device detection code and not to actually
run it.

(cherry picked from commit 62fd12149f73feb4e9c93a961670d527436a6e33)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7bd62cad8b711ed0b39ca00bf74ca326909ff73d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
Willy Tarreau [Thu, 9 Jul 2020 03:01:27 +0000 (05:01 +0200)]
BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ

The torture test run for previous commit 787dc20 ("BUG/MEDIUM: lists: add
missing store barrier on MT_LIST_BEHEAD()") finally broke again after 34M
connections. It appeared that MT_LIST_ADD and MT_LIST_ADDQ were suffering
from the same missing barrier when restoring the original pointers before
giving up, when checking if the element was already added. This is indeed
something which seldom happens with the shared scheduler, in case two
threads simultaneously try to wake up the same tasklet.

With a store barrier there after reverting the pointers, the torture test
survived 750M connections on the NanoPI-Fire3, so it looks good this time.
Probably that MT_LIST_BEHEAD should be added to test-list.c since it seems
to be more sensitive to concurrent accesses with MT_LIST_ADDQ.

It's worth noting that there is no barrier between the last two pointers
update, while there is one in MT_LIST_POP and MT_LIST_BEHEAD, the latter
having shown to be needed, but I cannot demonstrate why we would need
one here. Given that the code seems solid here, let's stick to what is
shown to work.

This fix should be backported to 2.1, just for the sake of safety since
the issue couldn't be triggered there, but it could change with the
compiler or when backporting a fix for example.

(cherry picked from commit 3b8f9b7b880397db03bfa54b26c8b6dbf6b5dc5e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ae8b8361ce2d50df0fb8c2d4b4d23e13ff497029)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
Willy Tarreau [Wed, 8 Jul 2020 17:45:50 +0000 (19:45 +0200)]
BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()

When running multi-threaded tests on my NanoPI-Fire3 (8 A53 cores), I
managed to occasionally get either a bus error or a segfault in the
scheduler, but only when running at a high connection rate (injecting
on a tcp-request connection reject rule). The bug is rare and happens
around once per million connections. I could never reproduce it with
less than 4 threads nor on A72 cores.

Haproxy 2.1.0 would also fail there but not 2.1.7.

Every time the crash happened with the TL_URGENT task list corrupted,
though it was not immediately after the LIST_SPLICE() call, indicating
background activity survived the MT_LIST_BEHEAD() operation. This queue
is where the shared runqueue is transferred, and the shared runqueue
gets fast inter-thread tasklet wakeups from idle conn takeover and new
connections.

Comparing the MT_LIST_BEHEAD() and MT_LIST_DEL() implementations, it's
quite obvious that a few barriers are missing from the former, and these
will simply fail on weakly ordered caches.

Two store barriers were added before the break() on failure, to match
what is done on the normal path. Missing them almost always results in
a segfault which is quite rare but consistent (after ~3M connections).

The 3rd one before updating n->prev seems intuitively needed though I
coudln't make the code fail without it. It's present in MT_LIST_DEL so
better not be needlessly creative. The last one is the most important
one, and seems to be the one that helps a concurrent MT_LIST_ADDQ()
detect a late failure and try again. With this, the code survives at
least 30M connections.

Interestingly the exact same issue was addressed in 2.0-dev2 for MT_LIST_DEL
with commit 690d2ad4d ("BUG/MEDIUM: list: add missing store barriers when
updating elements and head").

This fix must be backported to 2.1 as MT_LIST_BEHEAD() is also used there.

It's only tagged as medium because it will only affect entry-level CPUs
like Cortex A53 (x86 are not affected), and requires load levels that are
very hard to achieve on such machines to trigger it. In practice it's
unlikely anyone will ever hit it.

(cherry picked from commit 787dc20952aecdfb9322fb511f07d1d21b5733c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 826c7b73820b44b210f97b0c24320c2aa6990bf5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

5 years agoBUG/MINOR: sample: Free str.area in smp_check_const_meth
Tim Duesterhus [Sat, 4 Jul 2020 09:49:46 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_meth

Given the following example configuration:

    listen foo
     mode http
     bind *:8080
     http-request set-var(txn.leak) meth(GET)
     server x example.com:80

Running a configuration check with valgrind reports:

    ==25992== 4 bytes in 1 blocks are definitely lost in loss record 1 of 344
    ==25992==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==25992==    by 0x4E239D: my_strndup (tools.c:2261)
    ==25992==    by 0x581E20: make_arg_list (arg.c:253)
    ==25992==    by 0x4DE91D: sample_parse_expr (sample.c:890)
    ==25992==    by 0x58E304: parse_store (vars.c:772)
    ==25992==    by 0x566A3F: parse_http_req_cond (http_rules.c:95)
    ==25992==    by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
    ==25992==    by 0x494C59: readcfgfile (cfgparse.c:2049)
    ==25992==    by 0x545145: init (haproxy.c:2029)
    ==25992==    by 0x421E42: main (haproxy.c:3175)

After this patch is applied the leak is gone as expected.

This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.

(cherry picked from commit 041a626a8acfd53aa3710cd3d620f7f9f01fe893)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 761200b597cb36378527ba8cca1ad10a5d09a922)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>