]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
4 years agoMINOR: sock: introduce sock_inet and sock_unix
Willy Tarreau [Fri, 28 Aug 2020 13:10:11 +0000 (15:10 +0200)] 
MINOR: sock: introduce sock_inet and sock_unix

These files will regroup everything specific to AF_INET, AF_INET6 and
AF_UNIX socket definitions and address management. Some code there might
be agnostic to the socket type and could later move to af_xxxx.c but for
now we only support regular sockets so no need to go too far.

The files are quite poor at this step, they only contain the address
comparison function for each address family.

4 years agoREORG: sock: start to move some generic socket code to sock.c
Willy Tarreau [Fri, 28 Aug 2020 10:07:22 +0000 (12:07 +0200)] 
REORG: sock: start to move some generic socket code to sock.c

The new file sock.c will contain generic code for standard sockets
relying on file descriptors. We currently have way too much duplication
between proto_uxst, proto_tcp, proto_sockpair and proto_udp.

For now only get_src, get_dst and sock_create_server_socket were moved,
and are used where appropriate.

4 years agoREORG: unix: move UNIX bind/server keywords from proto_uxst.c to cfgparse-unix.c
Willy Tarreau [Fri, 28 Aug 2020 09:54:59 +0000 (11:54 +0200)] 
REORG: unix: move UNIX bind/server keywords from proto_uxst.c to cfgparse-unix.c

Let's finish the cleanup and get rid of all bind and server keywords
parsers from proto_uxst.c. They're now moved to cfgparse-unix.c. Now
proto_uxst.c is clean and only contains code related to binding and
connecting.

4 years agoREORG: tcp: move TCP bind/server keywords from proto_tcp.c to cfgparse-tcp.c
Willy Tarreau [Fri, 28 Aug 2020 09:49:31 +0000 (11:49 +0200)] 
REORG: tcp: move TCP bind/server keywords from proto_tcp.c to cfgparse-tcp.c

Let's continue the cleanup and get rid of all bind and server keywords
parsers from proto_tcp.c. They're now moved to cfgparse-tcp.c, just as
was done for ssl before 2.2 release. Nothing has changed beyond this.
Now proto_tcp.c is clean and only contains code related to binding and
connecting.

4 years agoREORG: tcp: move TCP sample fetches from proto_tcp.c to tcp_sample.c
Willy Tarreau [Fri, 28 Aug 2020 09:37:21 +0000 (11:37 +0200)] 
REORG: tcp: move TCP sample fetches from proto_tcp.c to tcp_sample.c

Let's continue the cleanup and get rid of all sample fetch functions
from proto_tcp.c. They're now moved to tcp_sample.c, just as was done
for ssl before 2.2 release. Nothing has changed beyond this.

4 years agoCLEANUP: tcp: stop exporting smp_fetch_src()
Willy Tarreau [Fri, 28 Aug 2020 09:31:31 +0000 (11:31 +0200)] 
CLEANUP: tcp: stop exporting smp_fetch_src()

This is totally ugly, smp_fetch_src() is exported only so that stick_table.c
can (ab)use it in the {sc,src}_* sample fetch functions. It could be argued
that the sample could have been reconstructed there in place, but we don't
even need to duplicate the code. We'd rather simply retrieve the "src"
fetch's function from where it's used at init time and be done with it.

4 years agoREORG: tcp: move TCP actions from proto_tcp.c to tcp_act.c
Willy Tarreau [Fri, 28 Aug 2020 09:03:28 +0000 (11:03 +0200)] 
REORG: tcp: move TCP actions from proto_tcp.c to tcp_act.c

The file proto_tcp.c has become a real mess because it still contains
tons of definitions that have nothing to do with the TCP protocol setup.
This commit moves the ruleset actions "set-src-port", "set-dst-port",
"set-src", "set-dst", and "silent-drop" to a new file "tcp_act.c".
Nothing has changed beyond this.

4 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.

4 years agoDOC: add description of pidfile in master-worker mode
MIZUTA Takeshi [Wed, 26 Aug 2020 04:46:19 +0000 (13:46 +0900)] 
DOC: add description of pidfile in master-worker mode

Previously, pidfile was only described for daemon mode. In the case of
master-worker mode, the handling of pidfile is different from daemon mode,
so the description has been added.

4 years agoMEDIUM: reload: pass all exportable FDs, not just listeners
Willy Tarreau [Wed, 19 Aug 2020 15:03:55 +0000 (17:03 +0200)] 
MEDIUM: reload: pass all exportable FDs, not just listeners

Now we don't limit ourselves to listeners found in proxies nor peers
anymore, we're instead scanning all known FDs for those marked with
".exported=1". Just doing so has significantly simplified the code,
and will later allow to yield while sending FDs if desired.

When it comes to retrieving a possible namespace name or interface
name, for now this is only performed on listeners since these are the
only ones carrying such info. Once this moves somewhere else, we'll
be able to also pass these info for UDP receivers for example, with
only tiny changes.

4 years agoMINOR: fd: add a new "exported" flag and use it for all regular listeners
Willy Tarreau [Wed, 19 Aug 2020 08:00:57 +0000 (10:00 +0200)] 
MINOR: fd: add a new "exported" flag and use it for all regular listeners

This new flag will be used to mark FDs that must be passed to any future
process across the CLI's "_getsocks" command.

The scheme here is quite complex and full of special cases:
  - FDs inherited from parent processes are *not* exported this way, as
    they are supposed to instead be passed by the master process itself
    across reloads. However such FDs ought never to be paused otherwise
    this would disrupt the socket in the parent process as well;

  - FDs resulting from a "bind" performed over a socket pair, which are
    in fact one side of a socket pair passed inside another control socket
    pair must not be passed either. Since all of them are used the same
    way, for now it's enough never to put this "exported" flag to FDs
    bound by the socketpair code.

  - FDs belonging to temporary listeners (e.g. a passive FTP data port)
    must not be passed either. Fortunately we don't have such FDs yet.

  - the rest of the listeners for now are made of TCP, UNIX stream, ABNS
    sockets and are exportable, so they get the flag.

  - UDP listeners were wrongly created as listeners and are not suitable
    here. Their FDs should be passed but for now they are not since the
    client doesn't even distinguish the SO_TYPE of the retrieved sockets.

In addition, it's important to keep in mind that:
  - inherited FDs may never be closed in master process but may be closed
    in worker processes if the service is shut down (useless since still
    bound, but technically possible) ;

  - inherited FDs may not be disabled ;

  - exported FDs may be disabled because the caller will perform the
    subsequent listen() on them. However that might not work for all OSes

  - exported FDs may be closed, it just means the service was shut down
    from the worker, and will be rebound in the new process. This implies
    that we have to disable exported on close().

=> as such, contrary to an apparently obvious equivalence, the "exported"
   status doesn't imply anything regarding the ability to close a
   listener's FD or not.

4 years agoCLEANUP: fd: remove fd_remove() and rename fd_dodelete() to fd_delete()
Willy Tarreau [Wed, 26 Aug 2020 09:54:06 +0000 (11:54 +0200)] 
CLEANUP: fd: remove fd_remove() and rename fd_dodelete() to fd_delete()

This essentially undoes what we did in fd.c in 1.8 to support seamless
reload. Since we don't need to remove an fd anymore we can turn
fd_delete() to the simple function it used to be.

4 years agoMEDIUM: fd: replace usages of fd_remove() with fd_stop_both()
Willy Tarreau [Wed, 26 Aug 2020 09:44:17 +0000 (11:44 +0200)] 
MEDIUM: fd: replace usages of fd_remove() with fd_stop_both()

We used to require fd_remove() to remove an FD from a poller when we
still had the FD cache and it was not possible to directly act on the
pollers. Nowadays we don't need this anymore as the pollers will
automatically unregister disabled FDs. The fd_remove() hack is
particularly problematic because it additionally hides the FD from
the known FD list and could make one think it's closed.

It's used at two places:
  - with the async SSL engine
  - with the listeners (when unbinding from an fd for another process)

Let's just use fd_stop_both() instead, which will propagate down the
stack to do the right thing, without removing the FD from the array
of known ones.

Now when dumping FDs using "show fd" on a process which still knows some
of the other workers' FDs, the FD will properly be listed with a listener
state equal to "ZOM" for "zombie". This guarantees that the FD is still
known and will properly be passed using _getsocks().

4 years agoBUG/MEDIUM: ssl: fix ssl_bind_conf double free w/ wildcards
William Lallemand [Wed, 26 Aug 2020 15:34:44 +0000 (17:34 +0200)] 
BUG/MEDIUM: ssl: fix ssl_bind_conf double free w/ wildcards

The fix 7df5c2d ("BUG/MEDIUM: ssl: fix ssl_bind_conf double free") was
not complete. The problem still occurs when using wildcards in
certificate, during the deinit.

This patch removes the free of the ssl_conf structure in
ssl_sock_free_all_ctx() since it's already done in the crtlist deinit.

It must be backported in 2.2.

4 years agoMEDIUM: reload: stop passing listener options along with FDs
Willy Tarreau [Wed, 26 Aug 2020 08:30:09 +0000 (10:30 +0200)] 
MEDIUM: reload: stop passing listener options along with FDs

During a reload operation, we used to send listener options associated
with each passed file descriptor. These were passed as binary contents
for the size of the "options" field in the struct listener. This means
that any flag value change or field size change would be problematic,
the former failing to properly grab certain options, the latter possibly
causing permanent failures during this operation.

Since these two previous commits:
  MINOR: reload: determine the foreing binding status from the socket
  BUG/MINOR: reload: detect the OS's v6only status before choosing an old socket

we don't need this anymore as the values are determined from the file
descriptor itself.

Let's just turn the previous 32 bits to vestigal space, send them as
zeroes and ignore them on receipt. The only possible side effect is if
someone would want to roll back from a 2.3 to 2.2 or earlier, such options
might be ignored during this reload. But other forthcoming changes might
make this fail as well anyway so that's not a reason for keeping this
behavior.

4 years agoMINOR: reload: determine the foreing binding status from the socket
Willy Tarreau [Wed, 26 Aug 2020 08:23:40 +0000 (10:23 +0200)] 
MINOR: reload: determine the foreing binding status from the socket

Let's not look at the listener options passed by the original process
and determine from the socket itself whether it is configured for
transparent mode or not. This is cleaner and safer, and doesn't rely
on flag values that could possibly change between versions.

4 years agoBUG/MINOR: reload: detect the OS's v6only status before choosing an old socket
Willy Tarreau [Wed, 26 Aug 2020 07:29:21 +0000 (09:29 +0200)] 
BUG/MINOR: reload: detect the OS's v6only status before choosing an old socket

The v4v6 and v6only options are passed as data during the socket transfer
between processes so that the new process can decide whether it wants to
reuse a socket or not. But this actually misses one point: if no such option
is set and the OS defaults are changed between the reloads, then the socket
will still be inherited and will never be rebound using the new options.

This can be seen by starting the following config:

  global
    stats socket /tmp/haproxy.sock level admin expose-fd listeners

  frontend testme
    bind :::1234
    timeout client          2000ms

Having a look at the OS settins, v6only is disabled:

  $ cat /proc/sys/net/ipv6/bindv6only
  0

A first check shows it's indeed bound to v4 and v6:

  $ ss -an -6|grep 1234
  tcp   LISTEN 0      2035                                   *:1234             *:*

Reloading the process doesn't change anything (which is expected). Now let's set
bindv6only:

  $ echo 1 | sudo tee /proc/sys/net/ipv6/bindv6only
  1
  $ cat /proc/sys/net/ipv6/bindv6only
  1

Reloading gives the same state:

  $ ss -an -6|grep 1234
  tcp   LISTEN 0      2035                                   *:1234             *:*

However a restart properly shows a correct bind:

  $ ss -an -6|grep 1234
  tcp   LISTEN 0      2035                                [::]:1234          [::]:*

This one doesn't change once bindv6only is reset, for the same reason.

This patch attacks this problem differently. Instead of passing the two
options at once for each listening fd, it ignores the options and reads
the socket's current state for the IPV6_V6ONLY flag and sets it only.
Then before looking for a compatible FD, it checks the OS's defaults
before deciding which of the v4v6 and v6only needs to be kept on the
listener. And the selection is only made on this.

First, it addresses this issue. Second, it also ensures that if such
options are changed between reloads to identical states, the socket
can still be inherited. For example adding v4v6 when bindv6only is not
set will allow the socket to still be usable. Third, it avoids an
undesired dependency on the LI_O_* bit values between processes across
a reload (for these ones at least).

It might make sense to backport this to some recent stable versions, but
quite frankly the likelyhood that anyone will ever notice it is extremely
faint.

4 years agoMINOR: tcp: don't try to set/clear v6only on inherited sockets
Willy Tarreau [Wed, 26 Aug 2020 08:21:06 +0000 (10:21 +0200)] 
MINOR: tcp: don't try to set/clear v6only on inherited sockets

If a socket was already bound (inherited from a parent or retrieved from
a previous process), there's no point trying to change its IPV6_V6ONLY
state since it will fail. This is visible in strace as an EINVAL during
a reload when passing FDs.

4 years agoMINOR: ssl: Support SAN extension for certificate generation
Shimi Gersner [Sun, 23 Aug 2020 10:58:13 +0000 (13:58 +0300)] 
MINOR: ssl: Support SAN extension for certificate generation

The use of Common Name is fading out in favor of the RFC recommended
way of using SAN extensions. For example, Chrome from version 58
will only match server name against SAN.

The following patch adds SAN extension by default to all generated certificates.
The SAN extension will be of type DNS and based on the server name.

4 years agoMEDIUM: ssl: Support certificate chaining for certificate generation
Shimi Gersner [Sun, 23 Aug 2020 10:58:12 +0000 (13:58 +0300)] 
MEDIUM: ssl: Support certificate chaining for certificate generation

haproxy supports generating SSL certificates based on SNI using a provided
CA signing certificate. Because CA certificates may be signed by multiple
CAs, in some scenarios, it is neccesary for the server to attach the trust chain
in addition to the generated certificate.

The following patch adds the ability to serve the entire trust chain with
the generated certificate. The chain is loaded from the provided
`ca-sign-file` PEM file.

4 years agoBUILD: task: work around a bogus warning in gcc 4.7/4.8 at -O1
Willy Tarreau [Fri, 21 Aug 2020 03:48:34 +0000 (05:48 +0200)] 
BUILD: task: work around a bogus warning in gcc 4.7/4.8 at -O1

As reported in issue #816, when building task.o at -O1 with gcc 4.7 or
4.8, we get the following warning:

    CC      src/task.o
  In file included from include/haproxy/proxy.h:31:0,
                   from include/haproxy/cfgparse.h:27,
                   from src/task.c:19:
  src/task.c: In function 'next_timer_expiry':
  include/haproxy/ticks.h:121:10: warning: 'key' may be used uninitialized in this function [-Wmaybe-uninitialized]
  src/task.c:349:2: note: 'key' was declared here

It is wrong since the condition to use 'key' is exactly the same as
the one used to set it. This warning disappears at -O2 and disappeared
from gcc 5 and above. Let's just initialize 'key' there, it only adds
16 bytes of code and remains cheap enough for this function.

This should be backported to 2.2.

4 years agoBUILD: tools: include auxv a bit later
Willy Tarreau [Thu, 20 Aug 2020 14:39:14 +0000 (16:39 +0200)] 
BUILD: tools: include auxv a bit later

As reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=24745,
haproxy fails to build with TARGET=generic and without extra options due
to auxv.h not being included, since the __GLIBC__ macro is not yet defined.
Let's include it after other libc headers so that the __GLIBC__ definition
is known. Thanks to David and Tim for the diag.

This should be backported to 2.2.

4 years agoMINOR: stats: prevent favicon.ico requests for stats page
zurikus [Tue, 18 Aug 2020 08:16:05 +0000 (11:16 +0300)] 
MINOR: stats: prevent favicon.ico requests for stats page

Haproxy stats page don't have a favicon.ico, but browsers always makes a request for it.
This lead to errors during stats page requests:

Aug 18 08:46:41 somehost.example.net haproxy[1521534]: X.X.X.X:61403 [18/Aug/2020:08:46:41.437] stats stats/ -1/-1/-1/-1/0 503 222 - - SC-- 2/2/0/0/0 0/0 "GET /favicon.ico HTTP/1.1"
Aug 18 08:46:42 somehost.example.net haproxy[1521534]: X.X.X.X:61403 [18/Aug/2020:08:46:42.650] stats stats/ -1/-1/-1/-1/0 503 222 - - SC-- 2/2/0/0/0 0/0 "GET /favicon.ico HTTP/1.1"

Patch provided disables favicon.ico requests for haproxy stats page.

4 years agoREGTEST: remove stray leading spaces in converteers_ref_cnt_never_dec.vtc
Willy Tarreau [Wed, 19 Aug 2020 09:20:28 +0000 (11:20 +0200)] 
REGTEST: remove stray leading spaces in converteers_ref_cnt_never_dec.vtc

Since commit f92afb732 ("MEDIUM: cfgparse: Emit hard error on truncated lines")
we now produce parsing errors on truncated lines, in an effort to clean
up dangerous or broken config files. And it turns out that one of our own
regression tests was suffering from this, as diagnosed by William and Tim.
The cause is the leading spaces in front of "} -start" that vtest makes
part of the output file, so the file finishes with a partial line made
of spaces.

4 years agoMINOR: cache: Reject duplicate cache names
Tim Duesterhus [Tue, 18 Aug 2020 20:20:27 +0000 (22:20 +0200)] 
MINOR: cache: Reject duplicate cache names

Using a duplicate cache name most likely is the result of a misgenerated
configuration. There is no good reason to allow this, as the duplicate
caches can't be referred to.

This commit resolves GitHub issue #820.

It can be argued whether this is a fix for a bug or not. I'm erring on the
side of caution and marking this as a "new feature". It can be considered for
backporting to 2.2, but for other branches the risk of accidentally breaking
some working (but non-ideal) configuration might be too large.

4 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+.

4 years agoMEDIUM: cfgparse: Emit hard error on truncated lines
Tim Duesterhus [Tue, 18 Aug 2020 20:00:04 +0000 (22:00 +0200)] 
MEDIUM: cfgparse: Emit hard error on truncated lines

As announced within the emitted log message this is going to become a hard
error in 2.3. It's 2.3 time now, let's do this.

see 2fd5bdb439da29f15381aeb57c51327ba57674fc

4 years agoDOC: overhauling github issue templates
Lukas Tribus [Mon, 17 Aug 2020 18:17:11 +0000 (20:17 +0200)] 
DOC: overhauling github issue templates

as per the suggestions from Cyril and Willy on the mailing list:

Message-ID: <a0010c72-70b8-0647-c269-55c6614627c3@free.fr>

and with direct contributions from Tim, this changes large parts
of the bug issue template.

The Feature template is also updated as well as a new template for
Code Reports is introduced.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
4 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.

4 years agoMINOR: hlua: Add error message relative to the Channel manipulation and HTTP mode
Thierry Fournier [Sat, 15 Aug 2020 12:35:51 +0000 (14:35 +0200)] 
MINOR: hlua: Add error message relative to the Channel manipulation and HTTP mode

When the developper try to manipulate HAProxy channels in HTTP mode,
an error throws without explanation. This patch adds an explanation.

4 years ago[RELEASE] Released version 2.3-dev3 v2.3-dev3
Willy Tarreau [Fri, 14 Aug 2020 16:54:05 +0000 (18:54 +0200)] 
[RELEASE] Released version 2.3-dev3

Released version 2.3-dev3 with the following main changes :
    - SCRIPTS: git-show-backports: make -m most only show the left branch
    - SCRIPTS: git-show-backports: emit the shell command to backport a commit
    - BUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set
    - CI: travis-ci: specify SLZ_LIB, SLZ_INC for travis builds
    - BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
    - CLEANUP: dns: typo in reported error message
    - BUG/MAJOR: dns: disabled servers through SRV records never recover
    - BUG/MINOR: spoa-server: fix size_t format printing
    - DOC: spoa-server: fix false friends `actually`
    - 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
    - MINOR: lua: Add support for userlist as fetches and converters arguments
    - MINOR: lua: Add support for regex as fetches and converters arguments
    - MINOR: arg: Use chunk_destroy() to release string arguments
    - BUG/MINOR: snapshots: leak of snapshots on deinit()
    - CLEANUP: ssl: ssl_sock_crt2der semicolon and spaces
    - MINOR: ssl: add ssl_{c,s}_chain_der fetch methods
    - CLEANUP: fix all duplicated semicolons
    - BUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option
    - BUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2
    - BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
    - BUILD: makefile: don't disable -Wstringop-overflow anymore
    - BUG/MINOR: ssl: double free w/ smp_fetch_ssl_x_chain_der()
    - BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
    - BUG/MEDIUM: ssl: never generates the chain from the verify store
    - OPTIM: regex: PCRE2 use JIT match when JIT optimisation occured.
    - BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
    - CLEANUP: ssl: remove poorly readable nested ternary

4 years agoCLEANUP: ssl: remove poorly readable nested ternary
William Lallemand [Fri, 14 Aug 2020 13:30:13 +0000 (15:30 +0200)] 
CLEANUP: ssl: remove poorly readable nested ternary

Replace a four level nested ternary expression by an if/else expression
in ssl_sock_switchctx_cbk()

4 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.

4 years agoOPTIM: regex: PCRE2 use JIT match when JIT optimisation occured.
David Carlier [Thu, 13 Aug 2020 13:53:41 +0000 (14:53 +0100)] 
OPTIM: regex: PCRE2 use JIT match when JIT optimisation occured.

When a regex had been succesfully compiled by the JIT pass, it is better
 to use the related match, thanksfully having same signature, for better
 performance.

Signed-off-by: David Carlier <devnexen@gmail.com>
4 years agoBUG/MEDIUM: ssl: never generates the chain from the verify store
William Lallemand [Wed, 12 Aug 2020 18:02:10 +0000 (20:02 +0200)] 
BUG/MEDIUM: ssl: never generates the chain from the verify store

In bug #781 it was reported that HAProxy completes the certificate chain
using the verify store in the case there is no chain.

Indeed, according to OpenSSL documentation, when generating the chain,
OpenSSL use the chain store OR the verify store in the case there is no
chain store.

As a workaround, this patch always put a NULL chain in the SSL_CTX so
OpenSSL does not tries to complete it.

This must be backported in all branches, the code could be different,
the important part is to ALWAYS set a chain, and uses sk_X509_new_null()
if the chain is NULL.

4 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).

4 years agoBUG/MINOR: ssl: double free w/ smp_fetch_ssl_x_chain_der()
William Lallemand [Tue, 11 Aug 2020 09:18:46 +0000 (11:18 +0200)] 
BUG/MINOR: ssl: double free w/ smp_fetch_ssl_x_chain_der()

smp_fetch_ssl_x_chain_der() uses the SSL_get_peer_cert_chain() which
does not increment the refcount of the chain, so it should not be free'd.

The bug was introduced by a598b50 ("MINOR: ssl: add ssl_{c,s}_chain_der
fetch methods"). No backport needed.

4 years agoBUILD: makefile: don't disable -Wstringop-overflow anymore
Willy Tarreau [Tue, 11 Aug 2020 08:31:18 +0000 (10:31 +0200)] 
BUILD: makefile: don't disable -Wstringop-overflow anymore

This basically reverts commit c4e6460f6 ("MINOR: build: Disable
-Wstringop-overflow.") which is no more needed after previous one.

4 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.

4 years agoBUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2
William Lallemand [Mon, 10 Aug 2020 15:28:23 +0000 (17:28 +0200)] 
BUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2

The previous fix for ssl-skip-self-issued-ca requires the use of
SSL_CTX_build_cert_chain() which is only available starting from OpenSSL
1.0.2

4 years agoBUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option
William Lallemand [Mon, 10 Aug 2020 14:18:45 +0000 (16:18 +0200)] 
BUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option

In commit f187ce6, the ssl-skip-self-issued-ca option was accidentally
made useless by reverting the SSL_CTX reworking.

The previous attempt of making this feature was putting each certificate
of the chain in the SSL_CTX with SSL_CTX_add_extra_chain_cert() and was
skipping the Root CA.
The problem here is that doing it this way instead of doing a
SSL_CTX_set1_chain() break the support of the multi-certificate bundles.

The SSL_CTX_build_cert_chain() function allows one to remove the Root CA
with the SSL_BUILD_CHAIN_FLAG_NO_ROOT flag. Use it instead of doing
tricks with the CA.

Should fix issue #804.

Must be backported in 2.2.

4 years agoCLEANUP: fix all duplicated semicolons
William Dauchy [Fri, 7 Aug 2020 20:19:23 +0000 (22:19 +0200)] 
CLEANUP: fix all duplicated semicolons

trivial commit, does not change the code behaviour

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 years agoMINOR: ssl: add ssl_{c,s}_chain_der fetch methods
William Dauchy [Thu, 6 Aug 2020 16:11:38 +0000 (18:11 +0200)] 
MINOR: ssl: add ssl_{c,s}_chain_der fetch methods

Following work from Arjen and Mathilde, it adds ssl_{c,s}_chain_der
methods; it returns DER encoded certs from SSL_get_peer_cert_chain

Also update existing vtc tests to add random intermediate certificates

When getting the result through this header:
  http-response add-header x-ssl-chain-der %[ssl_c_chain_der,hex]
One can parse it with any lib accepting ASN.1 DER data, such as in go:
  bin, err := encoding/hex.DecodeString(cert)
  certs_parsed, err := x509.ParseCertificates(bin)

Cc: Arjen Nienhuis <arjen@zorgdoc.nl>
Signed-off-by: Mathilde Gilles <m.gilles@criteo.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 years agoCLEANUP: ssl: ssl_sock_crt2der semicolon and spaces
William Dauchy [Thu, 6 Aug 2020 16:11:37 +0000 (18:11 +0200)] 
CLEANUP: ssl: ssl_sock_crt2der semicolon and spaces

trivial commit, does not change the code behaviour

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 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.

4 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.

4 years agoMINOR: lua: Add support for regex as fetches and converters arguments
Christopher Faulet [Thu, 6 Aug 2020 09:10:57 +0000 (11:10 +0200)] 
MINOR: lua: Add support for regex as fetches and converters arguments

It means now regsub() converter is now exported to the lua. Map converters based
on regex are not available because the map arguments are not supported.

4 years agoMINOR: lua: Add support for userlist as fetches and converters arguments
Christopher Faulet [Thu, 6 Aug 2020 09:04:46 +0000 (11:04 +0200)] 
MINOR: lua: Add support for userlist as fetches and converters arguments

It means now http_auth() and http_auth_group() sample fetches are now exported
to the lua.

4 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)".

4 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.

4 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.

4 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.

4 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.

4 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.

4 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".

4 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.

4 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.

4 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.

4 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.

4 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.

4 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>
4 years agoBUG/MINOR: spoa-server: fix size_t format printing
William Dauchy [Sat, 1 Aug 2020 14:28:51 +0000 (16:28 +0200)] 
BUG/MINOR: spoa-server: fix size_t format printing

From https://www.python.org/dev/peps/pep-0353/
"A new type Py_ssize_t is introduced, which has the same size as the
compiler's size_t type, but is signed. It will be a typedef for ssize_t
where available."

For integer types, causes printf to expect a size_t-sized integer
argument.

This should fix github issue #702

This should be backported to >= v2.2

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 years agoBUG/MAJOR: dns: disabled servers through SRV records never recover
Baptiste Assmann [Tue, 4 Aug 2020 08:57:21 +0000 (10:57 +0200)] 
BUG/MAJOR: dns: disabled servers through SRV records never recover

A regression was introduced by 13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b
when I added support for Additional section of the SRV responses..

Basically, when a server is managed through SRV records additional
section and it's disabled (because its associated Additional record has
disappeared), it never leaves its MAINT state and so never comes back to
production.
This patch updates the "snr_update_srv_status()" function to clear the
MAINT status when the server now has an IP address and also ensure this
function is called when parsing Additional records (and associating them
to new servers).

This can cause severe outage for people using HAProxy + consul (or any
other service registry) through DNS service discovery).

This should fix issue #793.
This should be backported to 2.2.

4 years agoCLEANUP: dns: typo in reported error message
Baptiste Assmann [Tue, 4 Aug 2020 08:54:14 +0000 (10:54 +0200)] 
CLEANUP: dns: typo in reported error message

"record" instead of "recrd".

This should be backported to 2.2.

4 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.

4 years agoCI: travis-ci: specify SLZ_LIB, SLZ_INC for travis builds
Ilya Shipitsin [Tue, 4 Aug 2020 10:39:41 +0000 (15:39 +0500)] 
CI: travis-ci: specify SLZ_LIB, SLZ_INC for travis builds

These variables must be explicitly set as they are not inherited from
the environment. Till now they were ignored.

4 years agoBUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set
Ilya Shipitsin [Tue, 4 Aug 2020 10:36:24 +0000 (15:36 +0500)] 
BUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set

The SSL_INC and SSL_LIB variables were not initialized in the Makefile,
so they could be accidently inherited from the environment. We require
that any makefile variable is explicitly set on the command line so they
must be initialized.

Note that the Travis scripts used to rely only on these variables to be
exported, so it was adjusted as well.

4 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.

4 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.

4 years ago[RELEASE] Released version 2.3-dev2 v2.3-dev2
Willy Tarreau [Fri, 31 Jul 2020 12:48:32 +0000 (14:48 +0200)] 
[RELEASE] Released version 2.3-dev2

Released version 2.3-dev2 with the following main changes :
    - DOC: ssl: req_ssl_sni needs implicit TLS
    - BUG/MEDIUM: arg: empty args list must be dropped
    - BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
    - BUG/MAJOR: tasks: don't requeue global tasks into the local queue
    - MINOR: tasks/debug: make the thread affinity BUG_ON check a bit stricter
    - MINOR: tasks/debug: add a few BUG_ON() to detect use of wrong timer queue
    - MINOR: tasks/debug: add a BUG_ON() check to detect requeued task on free
    - BUG/MAJOR: dns: Make the do-resolve action thread-safe
    - BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
    - MEDIUM: htx: Add a flag on a HTX message when no more data are expected
    - BUG/MEDIUM: stream-int: Don't set MSG_MORE flag if no more data are expected
    - BUG/MEDIUM: http-ana: Only set CF_EXPECT_MORE flag on data filtering
    - CLEANUP: dns: remove 45 "return" statements from dns_validate_dns_response()
    - BUG/MINOR: htx: add two missing HTX_FL_EOI and remove an unexpected one
    - BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
    - BUILD: tools: fix build with static only toolchains
    - DOC: Use gender neutral language
    - BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
    - BUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status
    - BUG/MAJOR: dns: don't treat Authority records as an error
    - CI : travis-ci : prepare for using stock OpenSSL
    - CI: travis-ci : switch to stock openssl when openssl-1.1.1 is used
    - 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: lua: Abort execution of actions that yield on a final evaluation
    - MINOR: tcp-rules: Return an internal error if an action yields on a final eval
    - BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort
    - BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
    - MEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset
    - MEDIUM: lua: Set the analyse expiration date with smaller wake_time only
    - BUG/MEDIUM: connection: Be sure to always install a mux for sync connect
    - MINOR: connection: Preinstall the mux for non-ssl connect
    - MINOR: stream-int: Be sure to have a mux to do sends and receives
    - BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx
    - SCRIPTS: announce-release: add the link to the wiki in the announce messages
    - CI: travis-ci: use better name for Coverity scan job
    - CI: travis-ci: use proper linking flags for SLZ build
    - BUG/MEDIUM: backend: always attach the transport before installing the mux
    - BUG/MEDIUM: tcp-checks: always attach the transport before installing the mux
    - MINOR: connection: avoid a useless recvfrom() on outgoing connections
    - MINOR: mux-h1: do not even try to receive if the connection is not fully set up
    - MINOR: mux-h1: do not try to receive on backend before sending a request
    - CLEANUP: assorted typo fixes in the code and comments
    - BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()

4 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.

4 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Wed, 22 Jul 2020 19:32:55 +0000 (00:32 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 12th iteration of typo fixes

4 years agoMINOR: mux-h1: do not try to receive on backend before sending a request
Willy Tarreau [Fri, 31 Jul 2020 07:16:23 +0000 (09:16 +0200)] 
MINOR: mux-h1: do not try to receive on backend before sending a request

There's no point trying to perform an recv() on a back connection if we
have a stream before having sent a request, as it's expected to fail.
It's likely that this may avoid some spurious subscribe() calls in some
keep-alive cases (the close case was already addressed at the connection
level by "MINOR: connection: avoid a useless recvfrom() on outgoing
connections").

4 years agoMINOR: mux-h1: do not even try to receive if the connection is not fully set up
Willy Tarreau [Fri, 31 Jul 2020 07:15:43 +0000 (09:15 +0200)] 
MINOR: mux-h1: do not even try to receive if the connection is not fully set up

If the connection is still waiting for L4/L6, there's no point even trying
to receive as it will fail, so better return zero in h1_recv_allowed().

4 years agoMINOR: connection: avoid a useless recvfrom() on outgoing connections
Willy Tarreau [Fri, 31 Jul 2020 06:59:09 +0000 (08:59 +0200)] 
MINOR: connection: avoid a useless recvfrom() on outgoing connections

When a connect() doesn't immediately succeed (i.e. most of the times),
fd_cant_send() is called to enable polling. But given that we don't
mark that we cannot receive either, we end up performing a failed
recvfrom() immediately when the connect() is finally confirmed, as
indicated in issue #253.

This patch simply adds fd_cant_recv() as well so that we're only
notified once the recv path is ready. The reason it was not there
is purely historic, as in the past when there was the fd cache,
doing it would have caused a pending recv request to be placed into
the fd cache, hence a useless recvfrom() upon success (i.e. what
happens now).

Without this patch, forwarding 100k connections does this:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 17.51    0.704229           7    100000    100000 connect
 16.75    0.673875           3    200000           sendto
 16.24    0.653222           3    200036           close
 10.82    0.435082           1    300000    100000 recvfrom
 10.37    0.417266           1    300012           setsockopt
  7.12    0.286511           1    199954           epoll_ctl
  6.80    0.273447           2    100000           shutdown
  5.34    0.214942           2    100005           socket
  4.65    0.187137           1    105002      5002 accept4
  3.35    0.134757           1    100004           fcntl
  0.61    0.024585           4      5858           epoll_wait

With the patch:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 18.04    0.697365           6    100000    100000 connect
 17.40    0.672471           3    200000           sendto
 17.03    0.658134           3    200036           close
 10.57    0.408459           1    300012           setsockopt
  7.69    0.297270           1    200000           recvfrom
  7.32    0.282934           1    199922           epoll_ctl
  7.09    0.274027           2    100000           shutdown
  5.59    0.216041           2    100005           socket
  4.87    0.188352           1    104697      4697 accept4
  3.35    0.129641           1    100004           fcntl
  0.65    0.024959           4      5337         1 epoll_wait

Note the total disappearance of 1/3 of failed recvfrom() *without*
adding any extra syscall anywhere else.

The trace of an HTTP health check is now totally clean, with no useless
syscall at all anymore:

  09:14:21.959255 connect(9, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:14:21.959292 epoll_ctl(4, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLOUT|EPOLLRDHUP, {u32=9, u64=9}}) = 0
  09:14:21.959315 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
  09:14:21.959376 sendto(9, "OPTIONS / HTTP/1.0\r\ncontent-leng"..., 41, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 41
  09:14:21.959436 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
  09:14:21.959456 epoll_ctl(4, EPOLL_CTL_MOD, 9, {EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}) = 0
  09:14:21.959512 epoll_wait(4, [{EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}], 200, 1000) = 1
  09:14:21.959548 recvfrom(9, "HTTP/1.0 200\r\nContent-length: 0\r"..., 16320, 0, NULL, NULL) = 126
  09:14:21.959570 close(9)                = 0

With the edge-triggered poller, it gets even better:

  09:29:15.776201 connect(9, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:29:15.776256 epoll_ctl(4, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLET, {u32=9, u64=9}}) = 0
  09:29:15.776287 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
  09:29:15.776320 sendto(9, "OPTIONS / HTTP/1.0\r\ncontent-leng"..., 41, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 41
  09:29:15.776374 epoll_wait(4, [{EPOLLIN|EPOLLOUT|EPOLLRDHUP, {u32=9, u64=9}}], 200, 1000) = 1
  09:29:15.776406 recvfrom(9, "HTTP/1.0 200\r\nContent-length: 0\r"..., 16320, 0, NULL, NULL) = 126
  09:29:15.776434 close(9)                = 0

It could make sense to backport this patch to 2.2 and maybe 2.1 after
it has been sufficiently checked for absence of side effects in 2.3-dev,
as some people had reported an extra overhead like in issue #168.

4 years agoBUG/MEDIUM: tcp-checks: always attach the transport before installing the mux
Willy Tarreau [Fri, 31 Jul 2020 06:49:31 +0000 (08:49 +0200)] 
BUG/MEDIUM: tcp-checks: always attach the transport before installing the mux

Similarly to the issue described in commit "BUG/MEDIUM: backend: always
attach the transport before installing the mux", in tcpcheck_eval_connect()
we can install a handshake transport layer underneath the mux and replace
its subscriptions, causing a crash if the mux had already subscribed for
whatever reason.

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
    option httpchk
    server srv1 127.0.0.1:8888 send-proxy-v2 check inter 1000

The mux may only be installed *after* xprt_handshake is set up, so that
it registers against it and not against raw_sock or ssl_sock. This needs
to be backported to 2.2 which is the first version using muxes for checks.

4 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.

4 years agoCI: travis-ci: use proper linking flags for SLZ build
Ilya Shipitsin [Thu, 30 Jul 2020 21:08:13 +0000 (02:08 +0500)] 
CI: travis-ci: use proper linking flags for SLZ build

previously SSL_INC and SSL_INC were set for all builds, and SLZ lib
was linked because of those flags. After we switched SLZ build to
stock openssl lib, SSL_INC, SSL_LIB are not set anymore.

Good time to set SLZ_INC, SLZ_LIB for such builds

4 years agoCI: travis-ci: use better name for Coverity scan job
Ilya Shipitsin [Thu, 30 Jul 2020 21:05:55 +0000 (02:05 +0500)] 
CI: travis-ci: use better name for Coverity scan job

Let's add Coverity in the job name.

4 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.

4 years agoBUG/MINOR: lua: Fix a possible null pointer deref on lua ctx
Christopher Faulet [Thu, 30 Jul 2020 08:40:58 +0000 (10:40 +0200)] 
BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx

This bug was introduced by the commit 8f587ea3 ("MEDIUM: lua: Set the analyse
expiration date with smaller wake_time only"). At the end of hlua_action(), the
lua context may be null if the alloc failed.

No backport needed, this is 2.3-dev.

4 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.

4 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).

4 years agoBUG/MEDIUM: connection: Be sure to always install a mux for sync connect
Christopher Faulet [Wed, 29 Jul 2020 20:42:27 +0000 (22:42 +0200)] 
BUG/MEDIUM: connection: Be sure to always install a mux for sync connect

Sometime, a server connection may be performed synchronously. Most of time on
TCP socket, it does not happen. It is easier to have sync connect with unix
socket. When it happens, if we are not waiting for any hanshake completion, we
must be sure to have a mux installed before leaving the connect_server()
function because an attempt to send may be done before the I/O connection
handler have a chance to be executed to install the mux, if not already done.

For now, It is not expected to perform a send with no mux installed, leading to
a crash if it happens.

This patch should fix the issue #762 and probably #779 too. It must be
backported as far as 1.9.

4 years agoMEDIUM: lua: Set the analyse expiration date with smaller wake_time only
Christopher Faulet [Tue, 28 Jul 2020 10:01:55 +0000 (12:01 +0200)] 
MEDIUM: lua: Set the analyse expiration date with smaller wake_time only

If a lua action yields for any reason and if the wake timeout is set, it only
override the analyse expiration date if it is smaller. This way, a lower
inspect-delay will be respected, if any.

4 years agoMEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset
Christopher Faulet [Tue, 28 Jul 2020 09:56:13 +0000 (11:56 +0200)] 
MEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset

A dedicated expiration date is now used to apply the inspect-delay of the
tcp-request or tcp-response rulesets. Before, the analyse expiratation date was
used but it may also be updated by the lua (at least). So a lua script may
extend or reduce the inspect-delay by side effect. This is not expected. If it
becomes necessary, a specific function will be added to do this. Because, for
now, it is a bit confusing.

4 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.

4 years agoBUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort
Christopher Faulet [Tue, 28 Jul 2020 09:40:07 +0000 (11:40 +0200)] 
BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort

When a tcp-request or a tcp-response content ruleset evaluation is aborted, the
corresponding FLT_END analyser must be preserved, if any. But because of a typo
error, on the tcp-response content ruleset evaluation, we try to preserve the
request analyser instead of the response one.

This patch must be backported to 2.2.

4 years agoMINOR: tcp-rules: Return an internal error if an action yields on a final eval
Christopher Faulet [Tue, 28 Jul 2020 09:30:19 +0000 (11:30 +0200)] 
MINOR: tcp-rules: Return an internal error if an action yields on a final eval

On a final evaluation of a tcp-request or tcp-response content ruleset, it is
forbidden for an action to yield. To quickly identify bugs an internal error is
now returned if it happens and a warning log message is emitted.

4 years agoBUG/MINOR: lua: Abort execution of actions that yield on a final evaluation
Christopher Faulet [Tue, 28 Jul 2020 09:59:58 +0000 (11:59 +0200)] 
BUG/MINOR: lua: Abort execution of actions that yield on a final evaluation

A Lua action may yield. It may happen because the action returns explicitly
act.YIELD or because the script itself yield. In the first case, we must abort
the script execution if it is the final rule evaluation, i.e if the
ACT_OPT_FINAL flag is set. The second case is already covered.

This patch must be backported to 2.2.

4 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.

4 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.

4 years agoCI: travis-ci : switch to stock openssl when openssl-1.1.1 is used
Ilya Shipitsin [Wed, 29 Jul 2020 20:47:55 +0000 (01:47 +0500)] 
CI: travis-ci : switch to stock openssl when openssl-1.1.1 is used

instead of building openssl-1.1.1 let us use stock package and save
some electricity

4 years agoCI : travis-ci : prepare for using stock OpenSSL
Ilya Shipitsin [Wed, 29 Jul 2020 20:39:24 +0000 (01:39 +0500)] 
CI : travis-ci : prepare for using stock OpenSSL

initially SSL_LIB and SSL_INC were set globally and we assumed
that any OpenSSL variant is supposed to be built using "script/build-ssl.sh".

starting with ARM64 build we use stock openssl, also it makes sense
to use stock openssl for 1.1.1 builds for velocity sake.

Let us make stock openssl lib first class citizen.

SSL_LIB and SSL_INC are only set when custom openssl variant
is built.

4 years agoBUG/MAJOR: dns: don't treat Authority records as an error
Jerome Magnin [Sun, 26 Jul 2020 10:13:12 +0000 (12:13 +0200)] 
BUG/MAJOR: dns: don't treat Authority records as an error

Support for DNS Service Discovery by means of SRV records was enhanced with
commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
to use the content of the answers Additional records when present.

If there are Authority records before the Additional records we mistakenly
treat that as an invalid response. To fix this, just ignore the Authority
section if it exist and skip to the Additional records.

As 13a9232eb was introduced during 2.2-dev, it must be backported to 2.2.
This is a fix for issue #778

4 years agoBUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status
Jerome Magnin [Tue, 28 Jul 2020 11:38:22 +0000 (13:38 +0200)] 
BUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status

Since commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV
responses"), a struct server can have a NULL dns_requester->resolution,
when SRV records are used and DNS answers contain an Additional section.

This is a problem when we call snr_update_srv_status() because it does
not check that resolution is NULL, and dereferences it. This patch
simply adds a test for resolution being NULL. When that happens, it means
we are using SRV records with Additional records, and an entry was removed.

This should fix issue #775.
This should be backported to 2.2.

4 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.

4 years agoDOC: Use gender neutral language
Jackie Tapia [Wed, 22 Jul 2020 23:59:40 +0000 (18:59 -0500)] 
DOC: Use gender neutral language

This patch updates the documentation files and code comments to avoid
the use of gender specific phrasing in favor of "they" or "it".

4 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).

4 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.