]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
4 years agoMINOR: ssl: remove uneeded check in crtlist_parse_file
William Dauchy [Fri, 2 Oct 2020 13:27:20 +0000 (15:27 +0200)] 
MINOR: ssl: remove uneeded check in crtlist_parse_file

this condition is never true as we either break or goto error, so those
two lines could be removed in the current state of the code.

this is fixing github issue #862

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 years agoMINOR: ssl: Add error if a crt-list might be truncated
Tim Duesterhus [Tue, 29 Sep 2020 16:00:28 +0000 (18:00 +0200)] 
MINOR: ssl: Add error if a crt-list might be truncated

Similar to warning during the parsing of the regular configuration file
that was added in 2fd5bdb439da29f15381aeb57c51327ba57674fc this patch adds
a warning to the parsing of a crt-list if the file does not end in a
newline (and thus might have been truncated).

The logic essentially just was copied over. It might be good to refactor
this in the future, allowing easy re-use within all line-based config
parsers.

see https://github.com/haproxy/haproxy/issues/860#issuecomment-693422936
see 0354b658f061d00d5ab4b728d7deeff2c8f1503a

This should be backported as a warning to 2.2.

4 years agoCLEANUP: ssl: Use structured format for error line report during crt-list parsing
Tim Duesterhus [Tue, 29 Sep 2020 16:00:27 +0000 (18:00 +0200)] 
CLEANUP: ssl: Use structured format for error line report during crt-list parsing

This reuses the known `parsing [%s:%d]:` from regular config file error
reporting.

4 years agoBUILD: makefile: Enable closefrom() support on Solaris
Brad Smith [Wed, 30 Sep 2020 19:46:16 +0000 (15:46 -0400)] 
BUILD: makefile: Enable closefrom() support on Solaris

Solaris 9 (released 2002) added support for closefrom().

I bumped the version in the comment to 10 as the default feature
flags already has event ports enabled which were introduced in
Solaris 10.

4 years agoBUILD: tools: fix minor build issue on isspace()
Willy Tarreau [Thu, 1 Oct 2020 16:04:40 +0000 (18:04 +0200)] 
BUILD: tools: fix minor build issue on isspace()

Previous commit fa41cb679 ("MINOR: tools: support for word expansion
of environment in parse_line") introduced two new isspace() on a char
and broke the build on systems using an array disguised in a macro
instead of a function (like cygwin). Just use the usual cast.

4 years agoMINOR: tools: support for word expansion of environment in parse_line
Amaury Denoyelle [Thu, 1 Oct 2020 12:32:35 +0000 (14:32 +0200)] 
MINOR: tools: support for word expansion of environment in parse_line

Allow the syntax "${...[*]}" to expand an environment variable
containing several values separated by spaces as individual arguments. A
new flag PARSE_OPT_WORD_EXPAND has been added to toggle this feature on
parse_line invocation. In case of an invalid syntax, a new error
PARSE_ERR_WRONG_EXPAND will be triggered.

This feature has been asked on the github issue #165.

4 years agoBUILD: makefile: add an EXTRAVERSION variable to ease local naming
Willy Tarreau [Thu, 1 Oct 2020 02:05:39 +0000 (04:05 +0200)] 
BUILD: makefile: add an EXTRAVERSION variable to ease local naming

Sometimes it's desirable to append local version naming to packages,
and currently it can only be done using SUBVERS which is already set
by default to the git commit ID and patch count since last known tag,
making the addition a bit complicated.

Let's just add a new EXTRAVERSION field that is empty by default, and
systematically appended verbatim to the version string everywhere. This
way it becomes trivial to append some local strings, such as:

   make TARGET=foo EXTRAVERSION=+$(quilt applied|wc -l)
   -> 2.3-dev5-5018aa-15+1

or :

   make TARGET=foo EXTRAVERSION=-$(date +%F)
   -> 2.3-dev5-5018aa-15-20200110

Let's be careful not to add double quotes (used as the string delimiter)
nor spaces (which can confuse version parsers on the output). The extra
version is also used to name a tarball. It's always pre-initialized to an
empty string so that it's not accidently inherited from the environment.
It's not reported in "make version" to avoid fooling tools (it would be
pointless anyway).

As a side effect it also becomes possible to force VERSION and SUBVERS
to an empty string and use EXTRAVERSION alone to force a specific version
(could possibly be useful when bisecting from patch queues outside of Git
for example).

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

4 years agoOPTIM: backend: skip LB when we know the backend is full
Willy Tarreau [Tue, 29 Sep 2020 15:07:21 +0000 (17:07 +0200)] 
OPTIM: backend: skip LB when we know the backend is full

For some algos (roundrobin, static-rr, leastconn, first) we know that
if there is any request queued in the backend, it's because a previous
attempt failed at finding a suitable server after trying all of them.
This alone is sufficient to decide that the next request will skip the
LB algo and directly reach the backend's queue. Doing this alone avoids
an O(N) lookup when load-balancing on a saturated farm of N servers,
which starts to be very expensive for hundreds of servers, especially
under the lbprm lock. This change alone has increased the request rate
from 110k to 148k RPS for 200 saturated servers on 8 threads, and
fwlc_reposition_srv() doesn't show up anymore in perf top. See github
issue #880 for more context.

It could have been the same for random, except that random is performed
using a consistent hash and it only considers a small set of servers (2
by default), so it may result in queueing at the backend despite having
some free slots on unknown servers. It's no big deal though since random()
only performs two attempts by default.

For hashing algorithms this is pointless since we don't queue at the
backend, except when there's no hash key found, which is the least of
our concerns here.

4 years agoOPTIM: backend/random: never queue on the server, always on the backend
Willy Tarreau [Tue, 29 Sep 2020 14:58:30 +0000 (16:58 +0200)] 
OPTIM: backend/random: never queue on the server, always on the backend

If random() returns a server whose maxconn is reached or the queue is
used, instead of adding the request to the server's queue, better add
it to the backend queue so that it can be served by any server (hence
the fastest one).

4 years agoBUILD: makefile: Update feature flags for FreeBSD
Brad Smith [Tue, 15 Sep 2020 07:10:04 +0000 (03:10 -0400)] 
BUILD: makefile: Update feature flags for FreeBSD

This updates the feature flags for FreeBSD.

FreeBSD 10 adds support for accept4().

Enable getaddrinfo().

From the FreeBSD port / package.

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

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

4 years agoREGTEST: make agent-check.vtc require 1.8
Willy Tarreau [Tue, 29 Sep 2020 08:58:04 +0000 (10:58 +0200)] 
REGTEST: make agent-check.vtc require 1.8

This one looks at the output of "show server state", which change slightly
in 1.8 (shows DNS fields).

4 years agoREGTEST: the iif converter test requires 2.3
Willy Tarreau [Tue, 29 Sep 2020 08:51:16 +0000 (10:51 +0200)] 
REGTEST: the iif converter test requires 2.3

This one was recently added in 2.3.

4 years agoREGTEST: make ssl_client_samples and ssl_server_samples requiret to 2.3
Willy Tarreau [Tue, 29 Sep 2020 08:50:23 +0000 (10:50 +0200)] 
REGTEST: make ssl_client_samples and ssl_server_samples requiret to 2.3

These ones added new sample fetches that are only available in 2.3 and
which fail on older versions.

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

4 years agoBUG/MINOR: ssl/crt-list: exit on warning out of crtlist_parse_line()
William Lallemand [Mon, 28 Sep 2020 13:45:16 +0000 (15:45 +0200)] 
BUG/MINOR: ssl/crt-list: exit on warning out of crtlist_parse_line()

We should not exits on error out of the crtlist_parse_line() function.
The cfgerr error must be checked with the ERR_CODE mask.

Must be backported in 2.2.

4 years agoDOC: crt: advise to move away from cert bundle
William Dauchy [Sat, 26 Sep 2020 11:35:52 +0000 (13:35 +0200)] 
DOC: crt: advise to move away from cert bundle

especially when starting to use `new ssl cert` runtime API, it might
become a bit confusing for users to mix bundle and single cert,
especially when it comes to use the commit command:
e.g.:
- start the process with `crt` loading a bundle
- use `set ssl cert my_cert.pem.ecdsa`: API detects it as a replacement
  of a bundle.
- `commit` has to be done on the bundle: `commit ssl cert my_cert.pem`

however:
- add a new cert: `new ssl cert my_cert.pem.rsa`: added as a single
  certificate
- `commit` has to be done on the certificate: `commit ssl cert
  my_cert.pem.rsa`

this should resolve github issue #872

this should probably be backported in >= v2.2 in order to encourage
people to move away from bundle certificates loading.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
4 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>
4 years agoBUILD: makefile: Update feature flags for OpenBSD
Brad Smith [Sun, 27 Sep 2020 03:05:25 +0000 (23:05 -0400)] 
BUILD: makefile: Update feature flags for OpenBSD

Update the OpenBSD target features being enabled.

I updated the list of features after noticing
"BUILD: makefile: disable threads by default on OpenBSD".

The Makefile utilizing gcc(1) by default resulted in utilizing
our legacy and obsolete compiler (GCC 4.2.1) instead of the
proper system compiler (Clang), which does support TLS. With
"BUILD: makefile: change default value of CC from gcc to cc"
that is resolved.

4 years agoREGTESTS: use "command" instead of "which" for better POSIX compatibility
Ilya Shipitsin [Sat, 26 Sep 2020 06:54:27 +0000 (11:54 +0500)] 
REGTESTS: use "command" instead of "which" for better POSIX compatibility

for example, "which" is not installed by default in Fedora docker image.

4 years ago[RELEASE] Released version 2.3-dev5 v2.3-dev5
Christopher Faulet [Fri, 25 Sep 2020 16:40:47 +0000 (18:40 +0200)] 
[RELEASE] Released version 2.3-dev5

Released version 2.3-dev5 with the following main changes :
    - DOC: Fix typo in iif() example
    - CLEANUP: Update .gitignore
    - BUILD: introduce possibility to define ABORT_NOW() conditionally
    - CI: travis-ci: help Coverity to recognize abort()
    - BUG/MINOR: Fix type passed of sizeof() for calloc()
    - CLEANUP: Do not use a fixed type for 'sizeof' in 'calloc'
    - CLEANUP: tree-wide: use VAR_ARRAY instead of [0] in various definitions
    - BUILD: connection: fix build on clang after the VAR_ARRAY cleanup
    - BUG/MINOR: ssl: verifyhost is case sensitive
    - BUILD: makefile: change default value of CC from gcc to cc
    - CI: travis-ci: split asan step out of running tests
    - BUG/MINOR: server: report correct error message for invalid port on "socks4"
    - BUG/MEDIUM: ssl: Don't call ssl_sock_io_cb() directly.
    - BUG/MINOR: ssl/crt-list: crt-list could end without a \n
    - BUG/MINOR: log-forward: fail on unknown keywords
    - MEDIUM: log-forward: use "dgram-bind" instead of "bind" for the listener
    - BUG/MEDIUM: log-forward: always quit on parsing errors
    - MEDIUM: ssl: remove bundle support in crt-list and directories
    - MEDIUM: ssl/cli: remove support for multi certificates bundle
    - MINOR: ssl: crtlist_dup_ssl_conf() duplicates a ssl_bind_conf
    - MINOR: ssl: crtlist_entry_dup() duplicates a crtlist_entry
    - MEDIUM: ssl: emulates the multi-cert bundles in the crtlist
    - MEDIUM: ssl: emulate multi-cert bundles loading in standard loading
    - CLEANUP: ssl: remove test on "multi" variable in ckch functions
    - CLEANUP: ssl/cli: remove test on 'multi' variable in CLI functions
    - CLEANUP: ssl: remove utility functions for bundle
    - DOC: explain bundle emulation in configuration.txt
    - BUILD: fix build with openssl < 1.0.2 since bundle removal
    - BUG/MINOR: log: gracefully handle the "udp@" address format for log servers
    - BUG/MINOR: dns: gracefully handle the "udp@" address format for nameservers
    - MINOR: listener: create a new struct "settings" in bind_conf
    - MINOR: listener: move bind_proc and bind_thread to struct settings
    - MINOR: listener: move the interface to the struct settings
    - MINOR: listener: move the network namespace to the struct settings
    - REORG: listener: create a new struct receiver
    - REORG: listener: move the listening address to a struct receiver
    - REORG: listener: move the receiving FD to struct receiver
    - REORG: listener: move the listener's proto to the receiver
    - MINOR: listener: make sock_find_compatible_fd() check the socket type
    - REORG: listener: move the receiver part to a new file
    - MINOR: receiver: link the receiver to its settings
    - MINOR: receiver: link the receiver to its owner
    - MINOR: listener: prefer to retrieve the socket's settings via the receiver
    - MINOR: receiver: add a receiver-specific flag to indicate the socket is bound
    - MINOR: listener: move the INHERITED flag down to the receiver
    - MINOR: receiver: move the FOREIGN and V6ONLY options from listener to settings
    - MINOR: sock: make sock_find_compatible_fd() only take a receiver
    - MINOR: protocol: rename the ->bind field to ->listen
    - MINOR: protocol: add a new ->bind() entry to bind the receiver
    - MEDIUM: sock_inet: implement sock_inet_bind_receiver()
    - MEDIUM: tcp: make use of sock_inet_bind_receiver()
    - MEDIUM: udp: make use of sock_inet_bind_receiver()
    - MEDIUM: sock_unix: implement sock_unix_bind_receiver()
    - MEDIUM: uxst: make use of sock_unix_bind_receiver()
    - MEDIUM: sockpair: implement sockpair_bind_receiver()
    - MEDIUM: proto_sockpair: make use of sockpair_bind_receiver()
    - MEDIUM: protocol: explicitly start the receiver before the listener
    - MEDIUM: protocol: do not call proto->bind() anymore from bind_listener()
    - MINOR: protocol: add a new proto_fam structure for protocol families
    - MINOR: protocol: retrieve the family-specific fields from the family
    - CLEANUP: protocol: remove family-specific fields from struct protocol
    - MINOR: protocol: add a real family for existing FDs
    - CLEANUP: tools: make str2sa_range() less awful for fd@ and sockpair@
    - MINOR: tools: make str2sa_range() take more options than just resolve
    - MINOR: tools: add several PA_O_PORT_* flags in str2sa_range() callers
    - MEDIUM: tools: make str2sa_range() validate callers' port specifications
    - MEDIUM: config: remove all checks for missing/invalid ports/ranges
    - MINOR: tools: add several PA_O_* flags in str2sa_range() callers
    - MINOR: listener: remove the inherited arg to create_listener()
    - MINOR: tools: make str2sa_range() optionally return the fd
    - MINOR: log: detect LOG_TARGET_FD from the fd and not from the syntax
    - MEDIUM: tools: make str2sa_range() resolve pre-bound listeners
    - MINOR: config: do not test an inherited socket again
    - MEDIUM: tools: make str2sa_range() check for the sockpair's FD usability
    - MINOR: tools: start to distinguish stream and dgram in str2sa_range()
    - MEDIUM: tools: make str2sa_range() only report AF_CUST_UDP on listeners
    - MINOR: tools: remove the central test for "udp" in str2sa_range()
    - MINOR: cfgparse: add str2receiver() to parse dgram receivers
    - MINOR: log-forward: use str2receiver() to parse the dgram-bind address
    - MEDIUM: config: make str2listener() not accept datagram sockets anymore
    - MINOR: listener: pass the chosen protocol to create_listeners()
    - MINOR: tools: make str2sa_range() directly return the protocol
    - MEDIUM: tools: make str2sa_range() check that the protocol has ->connect()
    - MINOR: protocol: add the control layer type in the protocol struct
    - MEDIUM: protocol: store the socket and control type in the protocol array
    - MEDIUM: tools: make str2sa_range() use protocol_lookup()
    - MEDIUM: proto_udp: replace last AF_CUST_UDP* with AF_INET*
    - MINOR: tools: drop listener detection hack from str2sa_range()
    - BUILD: sock_unix: add missing errno.h
    - MINOR: sock_inet: report the errno string in binding errors
    - MINOR: sock_unix: report the errno string in binding errors
    - BUILD: sock_inet: include errno.h
    - MINOR: h2/trace: also display the remaining frame length in traces
    - BUG/MINOR: h2/trace: do not display "stream error" after a frame ACK
    - BUG/MEDIUM: h2: report frame bits only for handled types
    - BUG/MINOR: http-fetch: Don't set the sample type during the htx prefetch
    - BUG/MINOR: Fix memory leaks cfg_parse_peers
    - BUG/MINOR: config: Fix memory leak on config parse listen
    - 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
    - BUG/MEDIUM: listeners: do not pause foreign listeners
    - BUG/MINOR: listeners: properly close listener FDs
    - BUILD: trace: include tools.h

4 years agoBUILD: trace: include tools.h
Miroslav Zagorac [Thu, 24 Sep 2020 07:15:46 +0000 (09:15 +0200)] 
BUILD: trace: include tools.h

If the TRACE option is used when compiling the haproxy source,
the following error occurs on debian 9.13:

src/calltrace.o: In function `make_line':
.../src/calltrace.c:204: undefined reference to `rdtsc'
src/calltrace.o: In function `calltrace':
.../src/calltrace.c:277: undefined reference to `rdtsc'
collect2: error: ld returned 1 exit status
Makefile:866: recipe for target 'haproxy' failed

4 years agoBUG/MINOR: listeners: properly close listener FDs
Willy Tarreau [Wed, 23 Sep 2020 14:33:00 +0000 (16:33 +0200)] 
BUG/MINOR: listeners: properly close listener FDs

The code dealing with zombie proxies in soft_stop() is bogus, it uses
close() instead of fd_delete(), leaving a live entry in the fdtab with
a dangling pointer to a free memory location. The FD might be reassigned
for an outgoing connection for the time it takes the proxy to completely
stop, or could be dumped on the CLI's "show fd" command. In addition,
the listener's FD was not even reset, leaving doubts about whether or
not it will happen again in deinit().

And in deinit(), the loop in charge of closing zombie FDs is particularly
unsafe because it closes the fd then calls unbind_listener() then
delete_listener() hoping none of them will touch it again. Since it
requires some mental efforts to figure what's done there, let's correctly
reset the fd here as well and close it using fd_delete() to eliminate any
remaining doubts.

It's uncertain whether this should be backported. Zombie proxies are rare
and the situations capable of triggering such issues are not trivial to
setup. However it's easy to imagine how things could go wrong if backported
too far. Better wait for any matching report if at all (this code has been
there since 1.8 without anobody noticing).

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

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

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

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

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

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

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

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

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

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

4 years agoBUILD: sock_inet: include errno.h
Willy Tarreau [Thu, 17 Sep 2020 12:02:01 +0000 (14:02 +0200)] 
BUILD: sock_inet: include errno.h

I was careful to have it for sock_unix.c but missed it for sock_inet
which broke with commit 36722d227 ("MINOR: sock_inet: report the errno
string in binding errors") depending on the build options. No backport
is needed.

4 years agoMINOR: sock_unix: report the errno string in binding errors
Willy Tarreau [Thu, 17 Sep 2020 06:35:38 +0000 (08:35 +0200)] 
MINOR: sock_unix: report the errno string in binding errors

Just like with previous patch, let's report UNIX socket binding errors
in plain text. we can now see for example:

  [ALERT] 260/083531 (13365) : Starting frontend f: cannot switch final and temporary UNIX sockets (Operation not permitted) [/tmp/root.sock]
  [ALERT] 260/083640 (13375) : Starting frontend f: cannot change UNIX socket ownership (Operation not permitted) [/tmp/root.sock]

4 years agoMINOR: sock_inet: report the errno string in binding errors
Willy Tarreau [Thu, 17 Sep 2020 06:32:17 +0000 (08:32 +0200)] 
MINOR: sock_inet: report the errno string in binding errors

With the socket binding code cleanup it becomes easy to add more info to
error messages. One missing thing used to be the error string, which is
now added after the generic one, for example:

  [ALERT] 260/082852 (12974) : Starting frontend f: cannot bind socket (Permission denied) [0.0.0.0:4]
  [ALERT] 260/083053 (13292) : Starting frontend f: cannot bind socket (Address already in use) [0.0.0.0:4444]
  [ALERT] 260/083104 (13298) : Starting frontend f: cannot bind socket (Cannot assign requested address) [1.1.1.1:4444]

4 years agoBUILD: sock_unix: add missing errno.h
Willy Tarreau [Wed, 16 Sep 2020 20:15:40 +0000 (22:15 +0200)] 
BUILD: sock_unix: add missing errno.h

It builds fine when openssl is enabled, but fails otherwise. No backport
is needed.

4 years agoMINOR: tools: drop listener detection hack from str2sa_range()
Willy Tarreau [Wed, 16 Sep 2020 20:04:46 +0000 (22:04 +0200)] 
MINOR: tools: drop listener detection hack from str2sa_range()

We used to resort to a trick to detect whether the caller was a listener
or an outgoing socket in order never to present an AF_CUST_UDP* socket
to a log server nor a nameserver. This is no longer necessary, the socket
type alone will be enough.

4 years agoMEDIUM: proto_udp: replace last AF_CUST_UDP* with AF_INET*
Willy Tarreau [Wed, 16 Sep 2020 19:58:52 +0000 (21:58 +0200)] 
MEDIUM: proto_udp: replace last AF_CUST_UDP* with AF_INET*

We don't need to cheat with the sock_domain anymore, we now always have
the SOCK_DGRAM sock_type as a complementary selector. This patch restores
the sock_domain to AF_INET* in the udp* protocols and removes all traces
of the now unused AF_CUST_*.

4 years agoMEDIUM: tools: make str2sa_range() use protocol_lookup()
Willy Tarreau [Wed, 16 Sep 2020 19:37:31 +0000 (21:37 +0200)] 
MEDIUM: tools: make str2sa_range() use protocol_lookup()

By doing so we can remove the hard-coded mapping from AF_INET to AF_CUST_UDP
but we still need to keep the test on the listeners as long as these dummy
families remain present in the code.

4 years agoMEDIUM: protocol: store the socket and control type in the protocol array
Willy Tarreau [Wed, 16 Sep 2020 19:28:15 +0000 (21:28 +0200)] 
MEDIUM: protocol: store the socket and control type in the protocol array

The protocol array used to be only indexed by socket family, which is very
problematic with UDP (requiring an extra family) and with the forthcoming
QUIC (also requiring an extra family), especially since that binds them to
certain families, prevents them from supporting dgram UNIX sockets etc.

In order to address this, we now start to register the protocols with more
info, namely the socket type and the control type (either stream or dgram).
This is sufficient for the protocols we have to deal with, but could also
be extended further if multiple protocol variants were needed. But as is,
it still fits nicely in an array, which is convenient for lookups that are
instant.

4 years agoMINOR: protocol: add the control layer type in the protocol struct
Willy Tarreau [Wed, 16 Sep 2020 15:50:45 +0000 (17:50 +0200)] 
MINOR: protocol: add the control layer type in the protocol struct

This one will be needed to more accurately select a protocol. It may
differ from the socket type for QUIC, which uses dgram at the socket
layer and provides stream at the control layer. The upper level requests
a control layer only so we need this field.

4 years agoMEDIUM: tools: make str2sa_range() check that the protocol has ->connect()
Willy Tarreau [Wed, 16 Sep 2020 17:17:08 +0000 (19:17 +0200)] 
MEDIUM: tools: make str2sa_range() check that the protocol has ->connect()

Most callers of str2sa_range() need the protocol only to check that it
provides a ->connect() method. It used to be used to verify that it's a
stream protocol, but it might be a bit early to get rid of it. Let's keep
the test for now but move it to str2sa_range() when the new flag PA_O_CONNECT
is present. This way almost all call places could be cleaned from this.

There's a strange test in the server address parsing code that rechecks
the family from the socket which seems to be a duplicate of the previously
removed tests. It will have to be rechecked.

4 years agoMINOR: tools: make str2sa_range() directly return the protocol
Willy Tarreau [Wed, 16 Sep 2020 16:25:03 +0000 (18:25 +0200)] 
MINOR: tools: make str2sa_range() directly return the protocol

We'll need this so that it can return pointers to stacked protocol in
the future (for QUIC). In addition this removes a lot of tests for
protocol validity in the callers.

Some of them were checked further apart, or after a call to
str2listener() and they were simplified as well.

There's still a trick, we can fail to return a protocol in case the caller
accepts an fqdn for use later. This is what servers do and in this case it
is valid to return no protocol. A typical example is:

   server foo localhost:1111

4 years agoMINOR: listener: pass the chosen protocol to create_listeners()
Willy Tarreau [Wed, 16 Sep 2020 15:58:55 +0000 (17:58 +0200)] 
MINOR: listener: pass the chosen protocol to create_listeners()

The function will need to use more than just a family, let's pass it
the selected protocol. The caller will then be able to do all the fancy
stuff required to pick the best protocol.

4 years agoMEDIUM: config: make str2listener() not accept datagram sockets anymore
Willy Tarreau [Wed, 16 Sep 2020 14:28:08 +0000 (16:28 +0200)] 
MEDIUM: config: make str2listener() not accept datagram sockets anymore

str2listener() was temporarily hacked to support datagram sockets for
the log-forward listeners. This has has an undesirable side effect that
"bind udp@1.2.3.4:5555" was silently accepted as TCP for a bind line.

We don't need this hack anymore since the only user (log-forward) now
relies on str2receiver(). Now such an address will properly be rejected.

4 years agoMINOR: log-forward: use str2receiver() to parse the dgram-bind address
Willy Tarreau [Wed, 16 Sep 2020 13:22:19 +0000 (15:22 +0200)] 
MINOR: log-forward: use str2receiver() to parse the dgram-bind address

Thanks to this we don't need to specify "udp@" as it's implicitly a
datagram type listener that is expected, so any AF_INET/AF_INET4 address
will work.

4 years agoMINOR: cfgparse: add str2receiver() to parse dgram receivers
Willy Tarreau [Wed, 16 Sep 2020 13:13:04 +0000 (15:13 +0200)] 
MINOR: cfgparse: add str2receiver() to parse dgram receivers

This is at least temporary, as the migration at once is way too difficuly.
For now it still creates listeners but only allows DGRAM sockets. This
aims at easing the split between listeners and receivers.

4 years agoMINOR: tools: remove the central test for "udp" in str2sa_range()
Willy Tarreau [Wed, 16 Sep 2020 13:20:59 +0000 (15:20 +0200)] 
MINOR: tools: remove the central test for "udp" in str2sa_range()

Now we only rely on dgram type associated with AF_INET/AF_INET6 to infer
UDP4/UDP6. We still keep the hint based on PA_O_SOCKET_FD to detect that
the caller is a listener though. It's still far from optimal but UDP
remains rooted into the protocols and needs to be taken out first.

4 years agoMEDIUM: tools: make str2sa_range() only report AF_CUST_UDP on listeners
Willy Tarreau [Wed, 16 Sep 2020 18:35:12 +0000 (20:35 +0200)] 
MEDIUM: tools: make str2sa_range() only report AF_CUST_UDP on listeners

For now only listeners can make use of AF_CUST_UDP and it requires hacks
in the DNS and logsrv code to remap it to AF_INET. Make str2sa_range()
smarter by detecting that it's called for a listener and only set these
protocol families for listeners. This way we can get rid of the hacks.

4 years agoMINOR: tools: start to distinguish stream and dgram in str2sa_range()
Willy Tarreau [Wed, 16 Sep 2020 09:35:47 +0000 (11:35 +0200)] 
MINOR: tools: start to distinguish stream and dgram in str2sa_range()

The parser now supports a socket type for the control layer and a possible
other one for the transport layer. Usually they are the same except for
protocols like QUIC which will provide a stream transport layer based on
a datagram control layer. The default types are preset based on the caller's
expectations, and may be refined using "stream+" and "dgram+" prefixes.

For now they were not added to the docuemntation because other changes
will probably happen around UDP as well. It is conceivable that "tcpv4@"
or "udpv6@" will appear later as aliases for "stream+ipv4" or "dgram+ipv6".

4 years agoMEDIUM: tools: make str2sa_range() check for the sockpair's FD usability
Willy Tarreau [Wed, 16 Sep 2020 08:14:16 +0000 (10:14 +0200)] 
MEDIUM: tools: make str2sa_range() check for the sockpair's FD usability

Just like for inherited sockets, we want to make sure that FDs that are
mentioned in "sockpair@" are actually usable. Right now this test is
performed by the callers, but not everywhere. Typically, the following
config will fail if fd #5 is not bound:

  frontend
      bind sockpair@5

But this one will pass if fd #6 is not bound:

  backend
      server s1 sockpair@6

Now both will return an error in such a case:
   - 'bind' : cannot use file descriptor '5' : Bad file descriptor.
   - 'server s1' : cannot use file descriptor '6' : Bad file descriptor.

As such the test in str2listener() is not needed anymore (and it was
wrong by the way, as it used to test for the socket by overwriting the
local address with a new address that's made of the FD encoded on 16
bits and happens to still be at the same place, but that strictly
depends on whatever the kernel wants to put there).

4 years agoMINOR: config: do not test an inherited socket again
Willy Tarreau [Wed, 16 Sep 2020 08:43:38 +0000 (10:43 +0200)] 
MINOR: config: do not test an inherited socket again

Since previous patch we know that a successfully bound fd@XXX socket
is returned as its own protocol family from str2sa_range() and not as
AF_CUST_EXISTING_FD anymore o we don't need to check for that case
in str2listener().

4 years agoMEDIUM: tools: make str2sa_range() resolve pre-bound listeners
Willy Tarreau [Tue, 15 Sep 2020 15:41:56 +0000 (17:41 +0200)] 
MEDIUM: tools: make str2sa_range() resolve pre-bound listeners

When str2sa_range() is invoked for a bind or log line, and it gets a file
descriptor number, it will immediately resolve the socket's address (when
it's a socket) so that the address family, address and port are correctly
set. This will later allow to resolve some transport protocols that are
attached to existing FDs. For raw FDs (e.g. logs) and for socket pairs,
the FD number is still returned in the address, because we need the
underlying address management to complete the bind/listen/connect/whatever
needed. One immediate benefit is that passing a bad FD will now result in
one of these errors:

  'bind' : cannot use file descriptor '3' : Socket operation on non-socket.
  'bind' : socket on file descriptor '3' is of the wrong type.

Note that as of now, we never return a listening socket with a family of
AF_CUST_EXISTING_FD. The only case where this family is seen is for a raw
FD (e.g. logs).

4 years agoMINOR: log: detect LOG_TARGET_FD from the fd and not from the syntax
Willy Tarreau [Tue, 15 Sep 2020 12:03:26 +0000 (14:03 +0200)] 
MINOR: log: detect LOG_TARGET_FD from the fd and not from the syntax

Now that we have the FD value reported we don't need to cheat and detect
"fd@" in the address, we can safely rely on the FD value.

4 years agoMINOR: tools: make str2sa_range() optionally return the fd
Willy Tarreau [Tue, 15 Sep 2020 12:01:16 +0000 (14:01 +0200)] 
MINOR: tools: make str2sa_range() optionally return the fd

If a file descriptor was passed, we can optionally return it. This will
be useful for listening sockets which are both a pre-bound FD and a ready
socket.

4 years agoMINOR: listener: remove the inherited arg to create_listener()
Willy Tarreau [Tue, 15 Sep 2020 11:50:58 +0000 (13:50 +0200)] 
MINOR: listener: remove the inherited arg to create_listener()

This argument can now safely be determined from fd != -1, let's just
drop it.

4 years agoMINOR: tools: add several PA_O_* flags in str2sa_range() callers
Willy Tarreau [Fri, 4 Sep 2020 13:53:16 +0000 (15:53 +0200)] 
MINOR: tools: add several PA_O_* flags in str2sa_range() callers

These flags indicate whether the call is made to fill a bind or a server
line, or even just send/recv calls (like logs or dns). Some special cases
are made for outgoing FDs (e.g. pipes for logs) or socket FDs (e.g external
listeners), and there's a distinction between stream or dgram usage that's
expected to significantly help str2sa_range() proceed appropriately with
the input information. For now they are not used yet.

4 years agoMEDIUM: config: remove all checks for missing/invalid ports/ranges
Willy Tarreau [Tue, 15 Sep 2020 09:52:23 +0000 (11:52 +0200)] 
MEDIUM: config: remove all checks for missing/invalid ports/ranges

Now that str2sa_range() checks for appropriate port specification, we
don't need to implement adhoc test cases in every call place, if the
result is valid, the conditions are met otherwise the error message is
appropriately filled.

4 years agoMEDIUM: tools: make str2sa_range() validate callers' port specifications
Willy Tarreau [Tue, 15 Sep 2020 09:12:44 +0000 (11:12 +0200)] 
MEDIUM: tools: make str2sa_range() validate callers' port specifications

Now str2sa_range() will enforce the caller's port specification passed
using the PA_O_PORT_* flags, and will return an error on failure. For
optional ports, values 0-65535 will be enforced. For mandatory ports,
values 1-65535 are enforced. In case of ranges, it is also verified that
the upper bound is not lower than the lower bound, as this used to result
in empty listeners.

I couldn't find an easy way to test this using VTC since the purpose is
to trigger parse errors, so instead a test file is provided as
tests/ports.cfg with comments about what errors are expected for each
line.

4 years agoMINOR: tools: add several PA_O_PORT_* flags in str2sa_range() callers
Willy Tarreau [Tue, 15 Sep 2020 08:30:39 +0000 (10:30 +0200)] 
MINOR: tools: add several PA_O_PORT_* flags in str2sa_range() callers

These flags indicate what is expected regarding port specifications. Some
callers accept none, some need fixed ports, some have it mandatory, some
support ranges, and some take an offset. Each possibilty is reflected by
an option. For now they are not exploited, but the goal is to instrument
str2sa_range() to properly parse that.

4 years agoMINOR: tools: make str2sa_range() take more options than just resolve
Willy Tarreau [Fri, 4 Sep 2020 13:30:46 +0000 (15:30 +0200)] 
MINOR: tools: make str2sa_range() take more options than just resolve

We currently have an argument to require that the address is resolved
but we'll soon add more, so let's turn it into a bit field. The old
"resolve" boolean is now PA_O_RESOLVE.

4 years agoCLEANUP: tools: make str2sa_range() less awful for fd@ and sockpair@
Willy Tarreau [Fri, 4 Sep 2020 14:54:05 +0000 (16:54 +0200)] 
CLEANUP: tools: make str2sa_range() less awful for fd@ and sockpair@

The code is built to match prefixes at one place and to parse the address
as a second step, except for fd@ and sockpair@ where the test first passes
via AF_UNSPEC that is changed again. This is ugly and confusing, so let's
proceed like for the other ones.

4 years agoMINOR: protocol: add a real family for existing FDs
Willy Tarreau [Fri, 4 Sep 2020 14:44:20 +0000 (16:44 +0200)] 
MINOR: protocol: add a real family for existing FDs

At some places (log fd@XXX, bind fd@XXX) we support using an explicit
file descriptor number, that is placed into the sockaddr for later use.
The problem is that till now it was done with an AF_UNSPEC family, which
is also used for other situations like missing info or rings (for logs).

Let's create an "official" family AF_CUST_EXISTING_FD for this case so
that we are certain the FD can be found in the address when it is set.

4 years agoCLEANUP: protocol: remove family-specific fields from struct protocol
Willy Tarreau [Fri, 4 Sep 2020 06:23:14 +0000 (08:23 +0200)] 
CLEANUP: protocol: remove family-specific fields from struct protocol

This removes the following fields from struct protocol that are now
retrieved from the protocol family instead: .sock_family, .sock_addrlen,
.l3_addrlen, .addrcmp, .bind, .get_src, .get_dst.

This also removes the UDP-specific udp{,6}_get_{src,dst}() functions
which were referenced but not used yet. Their goal was only to remap
the original AF_INET* addresses to AF_CUST_UDP*.

Note that .sock_domain is still there as it's used as a selector for
the protocol struct to be used.

4 years agoMINOR: protocol: retrieve the family-specific fields from the family
Willy Tarreau [Fri, 4 Sep 2020 06:15:31 +0000 (08:15 +0200)] 
MINOR: protocol: retrieve the family-specific fields from the family

We now take care of retrieving sock_family, l3_addrlen, bind(),
addrcmp(), get_src() and get_dst() from the protocol family and
not just the protocol itself. There are very few places, this was
only seldom used. Interestingly in sock_inet.c used to rely on
->sock_family instead of ->sock_domain, and sock_unix.c used to
hard-code PF_UNIX instead of using ->sock_domain.

Also it appears obvious we have something wrong it the protocol
selection algorithm because sock_domain is the one set to the custom
protocols while it ought to be sock_family instead, which would avoid
having to hard-code some conversions for UDP namely.

4 years agoMINOR: protocol: add a new proto_fam structure for protocol families
Willy Tarreau [Fri, 4 Sep 2020 06:07:11 +0000 (08:07 +0200)] 
MINOR: protocol: add a new proto_fam structure for protocol families

We need to specially handle protocol families which regroup common
functions used for a given address family. These functions include
bind(), addrcmp(), get_src() and get_dst() for now. Some fields are
also added about the address family, socket domain (protocol family
passed to the socket() syscall), and address length.

These protocol families are referenced from the protocols but not yet
used.

4 years agoMEDIUM: protocol: do not call proto->bind() anymore from bind_listener()
Willy Tarreau [Wed, 2 Sep 2020 16:40:02 +0000 (18:40 +0200)] 
MEDIUM: protocol: do not call proto->bind() anymore from bind_listener()

All protocol's listeners now only take care of themselves and not of
the receiver anymore since that's already being done in proto_bind_all().
Now it finally becomes obvious that UDP doesn't need a listener, as the
only thing it does is to set the listener's state to LI_LISTEN!

4 years agoMEDIUM: protocol: explicitly start the receiver before the listener
Willy Tarreau [Wed, 2 Sep 2020 16:22:11 +0000 (18:22 +0200)] 
MEDIUM: protocol: explicitly start the receiver before the listener

Now protocol_bind_all() starts the receivers before their respective
listeners so that ultimately we won't need the listeners for non-
connected protocols.

We still have to resort to an ugly trick to set the I/O handler in
case of syslog over UDP because for now it's still not set in the
receiver, so we hard-code it.

4 years agoMEDIUM: proto_sockpair: make use of sockpair_bind_receiver()
Willy Tarreau [Wed, 2 Sep 2020 16:02:00 +0000 (18:02 +0200)] 
MEDIUM: proto_sockpair: make use of sockpair_bind_receiver()

Now we rely on the address family's receiver instead of binding everything
ourselves.

4 years agoMEDIUM: sockpair: implement sockpair_bind_receiver()
Willy Tarreau [Wed, 2 Sep 2020 15:52:23 +0000 (17:52 +0200)] 
MEDIUM: sockpair: implement sockpair_bind_receiver()

Note that for now we don't have a sockpair.c file to host that unusual
family, so the new function was placed directly into proto_sockpair.c.
It's no big deal given that this family is currently not shared with
multiple protocols.

The function does almost nothing but setting up the receiver. This is
normal as the socket the FDs are passed onto are supposed to have been
already created somewhere else, and the only usable identifier for such
a socket pair is the receiving FD itself.

The function was assigned to sockpair's ->bind() and is not used yet.

4 years agoMEDIUM: uxst: make use of sock_unix_bind_receiver()
Willy Tarreau [Wed, 2 Sep 2020 15:21:02 +0000 (17:21 +0200)] 
MEDIUM: uxst: make use of sock_unix_bind_receiver()

This removes all the AF_UNIX-specific code from uxst_bind_listener()
and now simply relies on sock_unix_bind_listener() to do the same
job. As mentionned in previous commit, the only difference is that
now an unlikely failure on listen() will not result in a roll back
of the temporary socket names since they will have been renamed
during the bind() operation (as expected). But such failures do not
correspond to any normal case and mostly denote operating system
issues so there's no functionality loss here.

4 years agoMEDIUM: sock_unix: implement sock_unix_bind_receiver()
Willy Tarreau [Wed, 2 Sep 2020 15:14:29 +0000 (17:14 +0200)] 
MEDIUM: sock_unix: implement sock_unix_bind_receiver()

This function performs all the bind-related stuff for UNIX sockets that
was previously done in uxst_bind_listener(). There is a very tiny
difference however, which is that previously, in the unlikely event
where listen() would fail, it was still possible to roll back the binding
and rename the backup to the original socket. Now we have to rename it
before calling returning, hence it will be done before calling listen().
However, this doesn't cover any particular use case since listen() has no
reason to fail there (and the rollback is not done for inherited sockets),
that was just done that way as a generic error processing path.

The code is not used yet and is referenced in the uxst proto's ->bind().

4 years agoMEDIUM: udp: make use of sock_inet_bind_receiver()
Willy Tarreau [Tue, 1 Sep 2020 14:23:29 +0000 (16:23 +0200)] 
MEDIUM: udp: make use of sock_inet_bind_receiver()

This removes all the AF_INET-specific code from udp_bind_listener()
and now simply relies on sock_inet_bind_listener() to do the same
job. The function is now basically just a wrapper around
sock_inet_bind_receiver().

4 years agoMEDIUM: tcp: make use of sock_inet_bind_receiver()
Willy Tarreau [Tue, 1 Sep 2020 14:12:50 +0000 (16:12 +0200)] 
MEDIUM: tcp: make use of sock_inet_bind_receiver()

This removes all the AF_INET-specific code from tcp_bind_listener()
and now simply relies on sock_inet_bind_listener() to do the same
job. The function was now roughly cut in half and its error path
significantly simplified.

4 years agoMEDIUM: sock_inet: implement sock_inet_bind_receiver()
Willy Tarreau [Tue, 1 Sep 2020 12:18:04 +0000 (14:18 +0200)] 
MEDIUM: sock_inet: implement sock_inet_bind_receiver()

This function collects all the receiver-specific code from both
tcp_bind_listener() and udp_bind_listener() in order to provide a more
generic AF_INET/AF_INET6 socket binding function. For now the API is
not very elegant because some info are still missing from the receiver
while there's no ideal place to fill them except when calling ->listen()
at the protocol level. It looks like some polishing code is needed in
check_config_validity() or somewhere around this in order to finalize
the receivers' setup. The main issue is that listeners and receivers
are created *before* bind_conf options are parsed and that there's no
finishing step to resolve some of them.

The function currently sets up a receiver and subscribes it to the
poller. In an ideal world we wouldn't subscribe it but let the caller
do it after having finished to configure the L4 stuff. The problem is
that the caller would then need to perform an fd_insert() call and to
possibly set the exported flag on the FD while it's not its job. Maybe
an improvement could be to have a separate sock_start_receiver() call
in sock.c.

For now the function is not used but it will soon be. It's already
referenced as tcp and udp's ->bind().

4 years agoMINOR: protocol: add a new ->bind() entry to bind the receiver
Willy Tarreau [Wed, 2 Sep 2020 11:48:45 +0000 (13:48 +0200)] 
MINOR: protocol: add a new ->bind() entry to bind the receiver

This will be the function that must be used to bind the receiver. It
solely depends on the address family but for now it's simpler to have
it per protocol.

4 years agoMINOR: protocol: rename the ->bind field to ->listen
Willy Tarreau [Tue, 1 Sep 2020 08:26:22 +0000 (10:26 +0200)] 
MINOR: protocol: rename the ->bind field to ->listen

The function currently is doing both the bind() and the listen(), so
let's call it ->listen so that the bind() operation can move to another
place.

4 years agoMINOR: sock: make sock_find_compatible_fd() only take a receiver
Willy Tarreau [Tue, 1 Sep 2020 13:20:52 +0000 (15:20 +0200)] 
MINOR: sock: make sock_find_compatible_fd() only take a receiver

We don't need to have a listener anymore to find an fd, a receiver with
its settings properly set is enough now.

4 years agoMINOR: receiver: move the FOREIGN and V6ONLY options from listener to settings
Willy Tarreau [Tue, 1 Sep 2020 13:12:08 +0000 (15:12 +0200)] 
MINOR: receiver: move the FOREIGN and V6ONLY options from listener to settings

The new RX_O_FOREIGN, RX_O_V6ONLY and RX_O_V4V6 options are now set into
the rx_settings part during the parsing, so that we don't need to adjust
them in each and every listener anymore. We have to keep both v4v6 and
v6only due to the precedence from v6only over v4v6.

4 years agoMINOR: listener: move the INHERITED flag down to the receiver
Willy Tarreau [Tue, 1 Sep 2020 13:41:59 +0000 (15:41 +0200)] 
MINOR: listener: move the INHERITED flag down to the receiver

It's the receiver's FD that's inherited from the parent process, not
the listener's so the flag must move to the receiver so that appropriate
actions can be taken.

4 years agoMINOR: receiver: add a receiver-specific flag to indicate the socket is bound
Willy Tarreau [Tue, 1 Sep 2020 08:47:07 +0000 (10:47 +0200)] 
MINOR: receiver: add a receiver-specific flag to indicate the socket is bound

In order to split the receiver from the listener, we'll need to know that
a socket is already bound and ready to receive. We used to do that via
tha LI_O_ASSIGNED state but that's not sufficient anymore since the
receiver might not belong to a listener anymore. The new RX_F_BOUND flag
is used for this.

4 years agoMINOR: listener: prefer to retrieve the socket's settings via the receiver
Willy Tarreau [Thu, 3 Sep 2020 05:50:19 +0000 (07:50 +0200)] 
MINOR: listener: prefer to retrieve the socket's settings via the receiver

Some socket settings used to be retrieved via the listener and the
bind_conf. Now instead we use the receiver and its settings whenever
appropriate. This will simplify the removal of the dependency on the
listener.

4 years agoMINOR: receiver: link the receiver to its owner
Willy Tarreau [Thu, 3 Sep 2020 08:05:03 +0000 (10:05 +0200)] 
MINOR: receiver: link the receiver to its owner

A receiver will have to pass a context to be installed into the fdtab
for use by the handler. We need to set this into the receiver struct
as the bind will happen longer after the configuration.

4 years agoMINOR: receiver: link the receiver to its settings
Willy Tarreau [Thu, 3 Sep 2020 05:46:06 +0000 (07:46 +0200)] 
MINOR: receiver: link the receiver to its settings

Just like listeners keep a pointer to their bind_conf, receivers now also
have a pointer to their rx_settings. All those belonging to a listener are
automatically initialized with a pointer to the bind_conf's settings.

4 years agoREORG: listener: move the receiver part to a new file
Willy Tarreau [Tue, 1 Sep 2020 08:37:19 +0000 (10:37 +0200)] 
REORG: listener: move the receiver part to a new file

We'll soon add flags for the receivers, better add them to the final
file, so it's time to move the definition to receiver-t.h. The struct
receiver and rx_settings were placed there.

4 years agoMINOR: listener: make sock_find_compatible_fd() check the socket type
Willy Tarreau [Wed, 16 Sep 2020 19:44:42 +0000 (21:44 +0200)] 
MINOR: listener: make sock_find_compatible_fd() check the socket type

sock_find_compatible_fd() can now access the protocol via the receiver
hence it can access its socket type and know whether the receiver has
dgram or stream sockets, so we don't need to hack around AF_CUST_UDP*
anymore there.

4 years agoREORG: listener: move the listener's proto to the receiver
Willy Tarreau [Fri, 28 Aug 2020 17:51:44 +0000 (19:51 +0200)] 
REORG: listener: move the listener's proto to the receiver

The receiver is the one which depends on the protocol while the listener
relies on the receiver. Let's move the protocol there. Since there's also
a list element to get back to the listener from the proto list, this list
element (proto_list) was moved as well. For now when scanning protos, we
still see listeners which are linked by their rx.proto_list part.

4 years agoREORG: listener: move the receiving FD to struct receiver
Willy Tarreau [Thu, 27 Aug 2020 06:16:52 +0000 (08:16 +0200)] 
REORG: listener: move the receiving FD to struct receiver

The listening socket is represented by its file descriptor, which is
generic to all receivers and not just listeners, so it must move to
the rx struct.

It's worth noting that in order to extend receivers and listeners to
other protocols such as QUIC, we'll need other handles than file
descriptors here, and that either a union or a cast to uintptr_t
will have to be used. This was not done yet and the field was
preserved under the name "fd" to avoid adding confusion.

4 years agoREORG: listener: move the listening address to a struct receiver
Willy Tarreau [Thu, 27 Aug 2020 05:48:42 +0000 (07:48 +0200)] 
REORG: listener: move the listening address to a struct receiver

The address will be specific to the receiver so let's move it there.

4 years agoREORG: listener: create a new struct receiver
Willy Tarreau [Thu, 27 Aug 2020 05:48:42 +0000 (07:48 +0200)] 
REORG: listener: create a new struct receiver

In order to start to split the listeners into the listener part and the
event receiver part, we introduce a new field "rx" into struct listener
that will eventually become a separate struct receiver. This patch only
adds the struct with an options field that the receivers will need.

4 years agoMINOR: listener: move the network namespace to the struct settings
Willy Tarreau [Thu, 3 Sep 2020 05:27:34 +0000 (07:27 +0200)] 
MINOR: listener: move the network namespace to the struct settings

The netns is common to all listeners/receivers and is used to bind the
listening socket so it must be in the receiver settings and not in the
listener. This removes some yet another set of unnecessary loops.

4 years agoMINOR: listener: move the interface to the struct settings
Willy Tarreau [Thu, 3 Sep 2020 05:23:34 +0000 (07:23 +0200)] 
MINOR: listener: move the interface to the struct settings

The interface is common to all listeners/receivers and is used to bind
the listening socket so it must be in the receiver settings and not in
the listener. This removes some unnecessary loops.

4 years agoMINOR: listener: move bind_proc and bind_thread to struct settings
Willy Tarreau [Thu, 3 Sep 2020 05:18:55 +0000 (07:18 +0200)] 
MINOR: listener: move bind_proc and bind_thread to struct settings

As mentioned previously, these two fields come under the settings
struct since they'll be used to bind receivers as well.

4 years agoMINOR: listener: create a new struct "settings" in bind_conf
Willy Tarreau [Thu, 3 Sep 2020 05:09:09 +0000 (07:09 +0200)] 
MINOR: listener: create a new struct "settings" in bind_conf

There currently is a large inconsistency in how binding parameters are
split between bind_conf and listeners. It happens that for historical
reasons some parameters are available at the listener level but cannot
be configured per-listener but only for a bind_conf, and thus, need to
be replicated. In addition, some of the bind_conf parameters are in fact
for the listening socket itself while others are for the instanciated
sockets.

A previous attempt at splitting listeners into receivers failed because
the boundary between all these settings is not well defined.

This patch introduces a level of listening socket settings in the
bind_conf, that will be detachable later. Such settings that are solely
for the listening socket are:
  - unix socket permissions (used only during binding)
  - interface (used for binding)
  - network namespace (used for binding)
  - process mask and thread mask (used during startup)

The rest seems to be used only to initialize the resulting sockets, or
to control the accept rate. For now, only the unix params (bind_conf->ux)
were moved there.

4 years agoBUG/MINOR: dns: gracefully handle the "udp@" address format for nameservers
Willy Tarreau [Wed, 16 Sep 2020 18:04:17 +0000 (20:04 +0200)] 
BUG/MINOR: dns: gracefully handle the "udp@" address format for nameservers

Just like with previous commit, DNS nameservers are affected as well with
addresses starting in "udp@", but here it's different, because due to
another bug in the DNS parser, the address is rejected, indicating that
it doesn't have a ->connect() method. Similarly, the DNS code believes
it's working on top of TCP at this point and this used to work because of
this. The same fix is applied to remap the protocol and the ->connect test
was dropped.

No backport is needed, as the ->connect() test will never strike in 2.2
or below.

4 years agoBUG/MINOR: log: gracefully handle the "udp@" address format for log servers
Willy Tarreau [Wed, 16 Sep 2020 17:58:34 +0000 (19:58 +0200)] 
BUG/MINOR: log: gracefully handle the "udp@" address format for log servers

Commit 3835c0dcb ("MEDIUM: udp: adds minimal proto udp support for
message listeners.") introduced a problematic side effect in log server
address parser: if "udp@", "udp4@" or "udp6@" prefixes a log server's
address, the adress is passed as-is to the log server with a non-existing
family and fails like this when trying to send:

  [ALERT] 259/195708 (3474) : socket() failed in logger #1: Address family not supported by protocol (errno=97)

The problem is that till now there was no UDP family, so logs expect an
AF_INET family to be passed for UDP there.

This patch manually remaps AF_CUST_UDP4 and AF_CUST_UDP6 to their "tcp"
equivalent that the log server parser expects. No backport is needed.

4 years agoBUILD: fix build with openssl < 1.0.2 since bundle removal
William Lallemand [Wed, 16 Sep 2020 16:08:14 +0000 (18:08 +0200)] 
BUILD: fix build with openssl < 1.0.2 since bundle removal

Bundle removal broke the build with openssl version < 1.0.2.

Remove the #ifdef around SSL_SOCK_KEYTYPE_NAMES.