]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
6 years agoMINOR: mux-h2: stop relying on CS_FL_REOS
Willy Tarreau [Tue, 14 May 2019 09:46:28 +0000 (11:46 +0200)] 
MINOR: mux-h2: stop relying on CS_FL_REOS

This flag was introduced early in 1.9 development (a3f7efe00) to report
the fact that the rxbuf that was present on the conn_stream was followed
by a shutr. Since then the rxbuf moved from the conn_stream to the h2s
(638b799b0) but the flag remained on the conn_stream. It is problematic
because some state transitions inside the mux depend on it, thus depend
on the CS, and as such have to test for its existence before proceeding.

This patch replaces the test on CS_FL_REOS with a test on the only
states that set this flag (H2_SS_CLOSED, H2_SS_HREM, H2_SS_ERROR).
The few places where the flag was set were removed (the flag is not
used by the data layer).

6 years agoMINOR: mux-h2: add macros to check multiple stream states at once
Willy Tarreau [Tue, 14 May 2019 09:44:03 +0000 (11:44 +0200)] 
MINOR: mux-h2: add macros to check multiple stream states at once

At many places we need to test for several stream states at once, let's
have macros to make a bit mask from a state to ease this.

6 years agoCLEANUP: mux-h2: don't test for impossible CS_FL_REOS conditions
Willy Tarreau [Tue, 14 May 2019 09:31:00 +0000 (11:31 +0200)] 
CLEANUP: mux-h2: don't test for impossible CS_FL_REOS conditions

This flag is currently set when an incoming close was received, which
results in the stream being in either H2_SS_HREM, H2_SS_CLOSED, or
H2_SS_ERROR states, so let's remove the test for the OPEN and HLOC
cases.

6 years agoBUG/MINOR: mux-h2: make sure to honor KILL_CONN in do_shut{r,w}
Willy Tarreau [Tue, 14 May 2019 08:44:40 +0000 (10:44 +0200)] 
BUG/MINOR: mux-h2: make sure to honor KILL_CONN in do_shut{r,w}

If the stream closes and quits while there's no room in the mux buffer
to send an RST frame, next time it is attempted it will not lead to
the connection being closed because the conn_stream will have been
released and the KILL_CONN flag with it as well.

This patch reserves a new H2_SF_KILL_CONN flag that is copied from
the CS when calling shut{r,w} so that the stream remains autonomous
on this even when the conn_stream leaves.

This should ideally be backported to 1.9 though it depends on several
previous patches that may or may not be suitable for backporting. The
severity is very low so there's no need to insist in case of trouble.

6 years agoMINOR: mux-h2: make h2s_wake_one_stream() not depend on temporary CS flags
Willy Tarreau [Tue, 7 May 2019 15:48:59 +0000 (17:48 +0200)] 
MINOR: mux-h2: make h2s_wake_one_stream() not depend on temporary CS flags

In h2s_wake_one_stream() we used to rely on the temporary flags used to
adjust the CS to determine the new h2s state. This really is not convenient
and creates far too many dependencies. This commit just moves the same
condition to the places where the temporary flags were set so that we
don't have to rely on these anymore. Whether these are relevant or not
was not the subject of the operation, what matters was to make sure the
conditions to adjust the stream's state and the CS's flags remain the
same. Later it could be studied if these conditions are correct or not.

6 years agoMINOR: mux-h2: make h2s_wake_one_stream() the only function to deal with CS
Willy Tarreau [Tue, 7 May 2019 15:26:05 +0000 (17:26 +0200)] 
MINOR: mux-h2: make h2s_wake_one_stream() the only function to deal with CS

h2s_wake_one_stream() has access to all the required elements to update
the connstream's flags and figure the necessary state transitions, so
let's move the conditions there from h2_wake_some_streams().

6 years agoMINOR: mux-h2: make h2_wake_some_streams() not depend on the CS flags
Willy Tarreau [Tue, 7 May 2019 13:23:14 +0000 (15:23 +0200)] 
MINOR: mux-h2: make h2_wake_some_streams() not depend on the CS flags

It's problematic to have to pass some CS flags to this function because
that forces some h2s state transistions to update them just in time
while some of them are supposed to only be updated during I/O operations.

As a first step this patch transfers the decision to pass CS_FL_ERR_PENDING
from the caller to the leaf function h2s_wake_one_stream(). It is easy
since this is the only flag passed there and it depends on the position of
the stream relative to the last_sid if it was set.

6 years agoMINOR: mux-h2: remove useless test on stream ID vs last in wake function
Willy Tarreau [Tue, 7 May 2019 12:44:41 +0000 (14:44 +0200)] 
MINOR: mux-h2: remove useless test on stream ID vs last in wake function

h2_wake_some_streams() first looks up streams whose IDs are greater than
or equal to last+1, then checks if the id is lower than or equal to last,
which by definition will never match. Let's remove this confusing leftover
from ancient code.

6 years agoBUG/MINOR: mworker: use after free when the PID not assigned
William Lallemand [Tue, 14 May 2019 09:15:18 +0000 (11:15 +0200)] 
BUG/MINOR: mworker: use after free when the PID not assigned

Commit 4528611 ("MEDIUM: mworker: store the leaving state of a process")
introduced a bug in the mworker_env_to_proc_list() function.

This is very unlikely to occur since the PID should always be assigned.
It can probably happen if the environment variable is corrupted.

No backport needed.

6 years agoBUG/MINOR: mux-h2: make the do_shut{r,w} functions more robust against retries
Willy Tarreau [Tue, 14 May 2019 08:40:21 +0000 (10:40 +0200)] 
BUG/MINOR: mux-h2: make the do_shut{r,w} functions more robust against retries

These functions may fail to emit an RST or an empty DATA frame because
the mux is full or busy. Then they subscribe the h2s and try again.
However when doing so, they will already have marked the error state on
the stream and will not pass anymore through the sequence resulting in
the failed frame to be attempted to be sent again nor to the close to
be done, instead they will return a success.

It is important to only leave when the stream is already closed, but
to go through the whole sequence otherwise.

This patch should ideally be backported to 1.9 though it's possible that
the lack of the WANT_SHUT* flags makes this difficult or dangerous. The
severity is low enough to avoid this in case of trouble.

6 years agoBUG/MINOR: log: Wrong log format initialization.
Frédéric Lécaille [Tue, 14 May 2019 08:57:58 +0000 (10:57 +0200)] 
BUG/MINOR: log: Wrong log format initialization.

This patch fixes an issue introduced by 0bad840b commit
"MINOR: log: Extract some code to send syslog messages" which leaded
to wrong log format variable initializations at least for "short" and "raw" format.
This commit skipped the cases where even if passed to __do_send_log(), the
syslog tag and syslog pid string must not be used to format the log message
with "short" and "raw". This is done iniatilizing "tag_max" and "pid_max"
variables (the lengths of the tag and pid strings) to 0, then updating to them to
the length of the tag and pid strings passed as variables to __do_send_log()
depending on the log format and in every cases using this length for the iovec
variable used to send() the log.

This bug is specific to 2.0.

6 years agoCLEANUP: connection: remove the handle field from the wait_event struct
Willy Tarreau [Mon, 13 May 2019 16:27:59 +0000 (18:27 +0200)] 
CLEANUP: connection: remove the handle field from the wait_event struct

It was only set and not consumed after the previous change. The reason
is that the task's context always contains the relevant information,
so there is no need for a second pointer.

6 years agoCLEANUP: mux-h2: simply use h2s->flags instead of ret in h2_deferred_shut()
Willy Tarreau [Mon, 13 May 2019 16:17:53 +0000 (18:17 +0200)] 
CLEANUP: mux-h2: simply use h2s->flags instead of ret in h2_deferred_shut()

This one used to rely on the combined return statuses of the shutr/w
functions but now that we have the H2_SF_WANT_SHUT{R,W} flags we don't
need this anymore if we properly remove these flags after their operations
succeed. This is what this patch does.

6 years agoMINOR: mux-h2: add two H2S flags to report the need for shutr/shutw
Willy Tarreau [Mon, 13 May 2019 16:06:17 +0000 (18:06 +0200)] 
MINOR: mux-h2: add two H2S flags to report the need for shutr/shutw

Currently when a shutr/shutw fails due to lack of buffer space, we abuse
the wait_event's handle pointer to place up to two bits there in addition
to the original pointer. This pointer is not used for anything but this
and overall the intent becomes clearer with h2s flags than with these
two alien bits in the pointer, so let's use clean flags now.

6 years agoCLEANUP: mux-h2: use LIST_ADDED() instead of LIST_ISEMPTY() where relevant
Willy Tarreau [Mon, 13 May 2019 15:56:11 +0000 (17:56 +0200)] 
CLEANUP: mux-h2: use LIST_ADDED() instead of LIST_ISEMPTY() where relevant

Lots of places were using LIST_ISEMPTY() to detect if a stream belongs
to one of the send lists or to detect if a connection was already
waiting for a buffer or attached to an idle list. Since these ones are
not list heads but list elements, let's use LIST_ADDED() instead.

6 years agoMINOR: lists: add LIST_ADDED() to check if an element belongs to a list
Willy Tarreau [Mon, 13 May 2019 15:48:46 +0000 (17:48 +0200)] 
MINOR: lists: add LIST_ADDED() to check if an element belongs to a list

Some code parts use LIST_ISEMPTY() a lot on list elements to detect
if they were reset consecutive to their removal from a list, but this
test is always confusing as this was initially designed for list heads.

Instead let's have a new macro, LIST_ADDED(), which returns true when
the element is in a list (i.e. it's not "empty").

6 years agoBUG/MEDIUM: connections: Don't forget to set xprt_ctx to NULL on close.
Olivier Houchard [Mon, 13 May 2019 17:10:46 +0000 (19:10 +0200)] 
BUG/MEDIUM: connections: Don't forget to set xprt_ctx to NULL on close.

In conn_xprt_close(), after calling xprt->close(), don't forget to set
conn->xprt_ctx to NULL, or we may attempt to reuse the now-free'd
conn->xprt_ctx if the connection failed and we're retrying it.

6 years agoMINOR/DOC: spoe-server: Add documentation
Thierry FOURNIER [Sun, 25 Feb 2018 20:28:05 +0000 (21:28 +0100)] 
MINOR/DOC: spoe-server: Add documentation

This is the documentation and examples.

6 years agoMINOR: spoa-server: Add python
Thierry FOURNIER [Sun, 25 Feb 2018 19:59:57 +0000 (20:59 +0100)] 
MINOR: spoa-server: Add python

This commit adds the Python support for the server.

6 years agoMINOR: spoa-server: Add Lua processing
Thierry FOURNIER [Fri, 23 Feb 2018 14:20:55 +0000 (15:20 +0100)] 
MINOR: spoa-server: Add Lua processing

Use the defined binding for registering Lua engine.

6 years agoMINOR: spoa-server: Execute registered callbacks
Thierry FOURNIER [Fri, 23 Feb 2018 13:42:46 +0000 (14:42 +0100)] 
MINOR: spoa-server: Execute registered callbacks

Call the right function with the right engine for each received message.

6 years agoMINOR: spoa-server: Prepare responses
Thierry FOURNIER [Fri, 23 Feb 2018 17:24:10 +0000 (18:24 +0100)] 
MINOR: spoa-server: Prepare responses

This patch adds SPOP responses managament. It provides SPOP
encoding primitives. It also move the example function
ip_reputation to this new behavior.

6 years agoMINOR: spoa-server: Load files
Thierry FOURNIER [Fri, 23 Feb 2018 14:12:55 +0000 (15:12 +0100)] 
MINOR: spoa-server: Load files

Declare files to be executed at the begining and execute it. The binding
between the engine and the file is done throught the extension.

6 years agoMINOR: spoa-server: Allow registering message processors
Thierry FOURNIER [Fri, 23 Feb 2018 13:27:05 +0000 (14:27 +0100)] 
MINOR: spoa-server: Allow registering message processors

This function register processor executed by any language for processing
an SPOP message.

6 years agoMINOR: spoa-server: Allow registering external processes
Thierry FOURNIER [Fri, 23 Feb 2018 13:58:40 +0000 (14:58 +0100)] 
MINOR: spoa-server: Allow registering external processes

Add struct for declaring an reistrering external processing resource.

6 years agoMINOR: spoa-server: With debug mode, start only one process
Thierry FOURNIER [Fri, 23 Feb 2018 18:11:47 +0000 (19:11 +0100)] 
MINOR: spoa-server: With debug mode, start only one process

Because debug with processes is simpler if only one process is started.

6 years agoMINOR: spoa-server: Replace the thread init system by processes
Thierry FOURNIER [Fri, 23 Feb 2018 12:50:26 +0000 (13:50 +0100)] 
MINOR: spoa-server: Replace the thread init system by processes

I will replace thread by processes. Note that, I keep the pthread_key
system for identifiying process in the same way that threads. Note
also that I keep commented out the original thread code because I hope
to reactivate it.

6 years agoMINOR: spoe-server: rename "worker" functions
Thierry FOURNIER [Fri, 23 Feb 2018 10:59:15 +0000 (11:59 +0100)] 
MINOR: spoe-server: rename "worker" functions

"worker" name is a little bit generic and it is used in many
places, so it is hard to find the expected symbol.

6 years agoMINOR: spoa-server: Externalise debug functions
Thierry FOURNIER [Sun, 25 Feb 2018 09:54:56 +0000 (10:54 +0100)] 
MINOR: spoa-server: Externalise debug functions

Make external LOG and DEBUG function. Other process can use this ones
and later these functions will be replaced by another log system

6 years agoMINOR: spoa-server: move some definition from spoa_server.c to spoa_server.h
Thierry FOURNIER [Fri, 23 Feb 2018 10:42:57 +0000 (11:42 +0100)] 
MINOR: spoa-server: move some definition from spoa_server.c to spoa_server.h

This will allow to add some other files to the project

6 years agoMINOR: spoa-server: Clone the v1.7 spoa-example project
Thierry FOURNIER [Fri, 23 Feb 2018 10:40:03 +0000 (11:40 +0100)] 
MINOR: spoa-server: Clone the v1.7 spoa-example project

This is a working base.

6 years agoBUG/MAJOR: ssl: segfault upon an heartbeat request
William Lallemand [Mon, 13 May 2019 12:31:34 +0000 (14:31 +0200)] 
BUG/MAJOR: ssl: segfault upon an heartbeat request

7b5fd1e ("MEDIUM: connections: Move some fields from struct connection
to ssl_sock_ctx.") introduced a bug in the heartbleed mitigation code.

Indeed the code used conn->ctx instead of conn->xprt_ctx for the ssl
context, resulting in a null dereference.

6 years agoBUG/MINOR: vars: Fix memory leak in vars_check_arg
Tim Duesterhus [Mon, 13 May 2019 08:53:29 +0000 (10:53 +0200)] 
BUG/MINOR: vars: Fix memory leak in vars_check_arg

vars_check_arg previously leaked the string containing the variable
name:

Consider this config:

    frontend fe1
        mode http
        bind :8080
        http-request set-header X %[var(txn.host)]

Starting HAProxy and immediately stopping it by sending a SIGINT makes
Valgrind report this leak:

    ==7795== 9 bytes in 1 blocks are definitely lost in loss record 15 of 71
    ==7795==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==7795==    by 0x4AA2AD: my_strndup (standard.c:2227)
    ==7795==    by 0x51FCC5: make_arg_list (arg.c:146)
    ==7795==    by 0x4CF095: sample_parse_expr (sample.c:897)
    ==7795==    by 0x4BA7D7: add_sample_to_logformat_list (log.c:495)
    ==7795==    by 0x4BBB62: parse_logformat_string (log.c:688)
    ==7795==    by 0x4E70A9: parse_http_req_cond (http_rules.c:239)
    ==7795==    by 0x41CD7B: cfg_parse_listen (cfgparse-listen.c:1466)
    ==7795==    by 0x480383: readcfgfile (cfgparse.c:2089)
    ==7795==    by 0x47A081: init (haproxy.c:1581)
    ==7795==    by 0x4049F2: main (haproxy.c:2591)

This leak can be detected even in HAProxy 1.6, this patch thus should
be backported to all supported branches

[Cf: This fix was reverted because the chunk's area was inconditionnaly
     released, making haproxy to crash when spoe was enabled. Now the chunk is
     released by calling chunk_destroy(). This function takes care of the
     chunk's size to release it or not. It is the responsibility of callers to
     set or not the chunk's size.]

6 years agoMINOR: spoe: Set the argument chunk size to 0 when SPOE variables are checked
Christopher Faulet [Mon, 13 May 2019 08:39:36 +0000 (10:39 +0200)] 
MINOR: spoe: Set the argument chunk size to 0 when SPOE variables are checked

When SPOE variables are registered during HAProxy startup, the argument used to
call the function vars_check_arg() uses the trash area. To be sure it is never
released by the callee function, the size of the internal chunk (arg.data.str)
is set to 0. It is important to do so because, to fix a memory leak, this buffer
must be released by the function vars_check_arg().

This patch must be backported to 1.9.

6 years agoREGTEST: fix tls_health_checks random failures on MacOS in Travis-CI
Willy Tarreau [Mon, 13 May 2019 08:47:41 +0000 (10:47 +0200)] 
REGTEST: fix tls_health_checks random failures on MacOS in Travis-CI

Since commit 2eb1c79df ("REGTEST: make the tls_health_checks test much
faster") the build tests randomly fail on MacOS on Travis-CI. Each time
this test is reponsible for the failure, showing huge response times
possibly indicating that the VMs running the tests are sometimes
overloaded. Since this delay directly impacts the whole regtest execution
time everywhere, it's important not to inflate it too much. It was bumped
to 100ms instead of 40, that doesn't add significantly to the perceived
execution time and should be enough for Travis since test reports have
shown around 60-70 ms.

6 years agoBUG/MINOR: htx: make sure to always initialize the HTTP method when parsing a buffer
Willy Tarreau [Mon, 13 May 2019 06:32:31 +0000 (08:32 +0200)] 
BUG/MINOR: htx: make sure to always initialize the HTTP method when parsing a buffer

smp_prefetch_htx() is used when trying to access the contents of an HTTP
buffer from the TCP rulesets. The method was not properly set in this
case, which will cause the sample fetch methods relying on the method
to randomly fail in this case.

Thanks to Tim Düsterhus for reporting this issue (#97).

This fix must be backported to 1.9.

6 years agoBUG/MINOR: peers: Fix memory leak in cfg_parse_peers
Tim Duesterhus [Sun, 12 May 2019 20:54:50 +0000 (22:54 +0200)] 
BUG/MINOR: peers: Fix memory leak in cfg_parse_peers

cfg_parse_peers previously leaked the contents of the `kws` string,
as it was unconditionally filled using bind_dump_kws, but only used
(and freed) within the error case.

Move the dumping into the error case to:
1. Ensure that the registered keywords are actually printed as least once.
2. The contents of kws are not leaked.

This move allows to narrow the scope of `kws`, so this is done as well.

This bug was found using valgrind:

    ==28217== 590 bytes in 1 blocks are definitely lost in loss record 51 of 71
    ==28217==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==28217==    by 0x4AD4C7: indent_msg (standard.c:3676)
    ==28217==    by 0x47E962: cfg_parse_peers (cfgparse.c:700)
    ==28217==    by 0x480273: readcfgfile (cfgparse.c:2147)
    ==28217==    by 0x479D51: init (haproxy.c:1585)
    ==28217==    by 0x404A02: main (haproxy.c:2585)

with this super simple configuration:

    peers peers
     bind :8081
     server A

This bug exists since the introduction of cfg_parse_peers in commit
355b2033ec0c89660db179b23d6f77b678d8c26d (which was introduced for HAProxy
2.0, but marked as backportable). It should be backported to all branches
containing that commit.

6 years agoRevert "BUG/MINOR: vars: Fix memory leak in vars_check_arg"
Willy Tarreau [Mon, 13 May 2019 08:02:42 +0000 (10:02 +0200)] 
Revert "BUG/MINOR: vars: Fix memory leak in vars_check_arg"

This reverts commit 6ea00195c479d96c5aa651adcca3bc3637e3eceb.

As found by Christopher, this fix is not correct due to the way args
are built at various places. For example some config or runtime parsers
will place a substring pointer there, and calling free() on it will
immediately crash the program. A quick audit of the code shows that
there are not that many users, but the way it's done requires to
properly set the string as a regular chunk (size=0 if free not desired,
then call chunk_destroy() at release time), and given that the size is
currently set to len+1 in all parsers, a deeper audit needs to be done
to figure the impacts of not setting it anymore.

Thus for now better leave this harmless leak which impacts only the
config parsing time.

This fix must be backported to all branches containing the fix above.

6 years agoBUG/MAJOR: mux-h2: do not add a stream twice to the send list
Willy Tarreau [Mon, 13 May 2019 06:05:28 +0000 (08:05 +0200)] 
BUG/MAJOR: mux-h2: do not add a stream twice to the send list

In this long thread, Maciej Zdeb reported that the H2 mux was still
going through endless loops from time to time :

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

What happens is the following :
- in h2s_frt_make_resp_data() we can set H2_SF_BLK_SFCTL and remove the
  stream from the send_list
- then in h2_shutr() and h2_shutw(), we check if the list is empty before
  subscribing the element, which is true after the case above
- then in h2c_update_all_ws() we still have H2_SF_BLK_SFCTL with the item
  in the send_list, thus LIST_ADDQ() adds it a second time.

This patch adds a check of list emptiness before performing the LIST_ADDQ()
when the flow control window opens. Maciej reported that it reliably fixed
the problem for him.

As later discussed with Olivier, this fixes the consequence of the issue
rather than its cause. The root cause is that a stream should never be in
the send_list with a blocking flag set and the various places that can lead
to this situation must be revisited. Thus another fix is expected soon for
this issue, which will require some observation. In the mean time this one
is easy enough to validate and to backport.

Many thanks to Maciej for testing several versions of the patch, each
time providing detailed traces which allowed to nail the problem down.

This patch must be backported to 1.9.

6 years agoBUILD: threads: fix again the __ha_cas_dw() definition
Willy Tarreau [Sat, 11 May 2019 16:04:24 +0000 (18:04 +0200)] 
BUILD: threads: fix again the __ha_cas_dw() definition

This low-level asm implementation of a double CAS was implemented only
for certain architectures (x86_64, armv7, armv8). When threads are not
used, they were not defined, but since they were called directly from
a few locations, they were causing build issues on certain platforms
with threads disabled. This was addressed in commit f4436e1 ("BUILD:
threads: Add __ha_cas_dw fallback for single threaded builds") by
making it fall back to HA_ATOMIC_CAS() when threads are not defined,
but this actually made the situation worse by breaking other cases.

This patch fixes this by creating a high-level macro HA_ATOMIC_DWCAS()
which is similar to HA_ATOMIC_CAS() except that it's intended to work
on a double word, and which rely on the asm implementations when threads
are in use, and uses its own open-coded implementation when threads are
not used. The 3 call places relying on __ha_cas_dw() were updated to
use HA_ATOMIC_DWCAS() instead.

This change was tested on i586, x86_64, armv7, armv8 with and without
threads with gcc 4.7, armv8 with gcc 5.4 with and without threads, as
well as i586 with gcc-3.4 without threads. It will need to be backported
to 1.9 along with the fix above to fix build on armv7 with threads
disabled.

6 years agoCLEANUP: ssl: move all BIO_* definitions to openssl-compat
Willy Tarreau [Sat, 11 May 2019 15:34:03 +0000 (17:34 +0200)] 
CLEANUP: ssl: move all BIO_* definitions to openssl-compat

The following macros are now defined for openssl < 1.1 so that we
can remove the code performing direct access to the structures :

  BIO_get_data(), BIO_set_data(), BIO_set_init(), BIO_meth_free(),
  BIO_meth_new(), BIO_meth_set_gets(), BIO_meth_set_puts(),
  BIO_meth_set_read(), BIO_meth_set_write(), BIO_meth_set_create(),
  BIO_meth_set_ctrl(), BIO_meth_set_destroy()

6 years agoCLEANUP: ssl: remove ifdef around SSL_CTX_get_extra_chain_certs()
Willy Tarreau [Sat, 11 May 2019 15:02:04 +0000 (17:02 +0200)] 
CLEANUP: ssl: remove ifdef around SSL_CTX_get_extra_chain_certs()

Instead define this one in openssl-compat.h when
SSL_CTRL_GET_EXTRA_CHAIN_CERTS is not defined (which was the current
condition used in the ifdef).

6 years agoCLEANUP: ssl: move the SSL_OP_* and SSL_MODE_* definitions to openssl-compat
Willy Tarreau [Sat, 11 May 2019 15:09:44 +0000 (17:09 +0200)] 
CLEANUP: ssl: move the SSL_OP_* and SSL_MODE_* definitions to openssl-compat

These ones were defined in the middle of ssl_sock.c, better move them
to the include file to find them.

6 years agoBUILD: travis-ci: make TMPDIR global variable in travis-ci
Ilya Shipitsin [Fri, 10 May 2019 10:38:52 +0000 (15:38 +0500)] 
BUILD: travis-ci: make TMPDIR global variable in travis-ci

This patch will reveal osx reg-tests errors (after osx build is repaired).

6 years agoBUG/MINOR: vars: Fix memory leak in vars_check_arg
Tim Duesterhus [Fri, 10 May 2019 15:50:50 +0000 (17:50 +0200)] 
BUG/MINOR: vars: Fix memory leak in vars_check_arg

vars_check_arg previously leaked the string containing the variable
name:

Consider this config:

    frontend fe1
        mode http
        bind :8080
        http-request set-header X %[var(txn.host)]

Starting HAProxy and immediately stopping it by sending a SIGINT makes
Valgrind report this leak:

    ==7795== 9 bytes in 1 blocks are definitely lost in loss record 15 of 71
    ==7795==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==7795==    by 0x4AA2AD: my_strndup (standard.c:2227)
    ==7795==    by 0x51FCC5: make_arg_list (arg.c:146)
    ==7795==    by 0x4CF095: sample_parse_expr (sample.c:897)
    ==7795==    by 0x4BA7D7: add_sample_to_logformat_list (log.c:495)
    ==7795==    by 0x4BBB62: parse_logformat_string (log.c:688)
    ==7795==    by 0x4E70A9: parse_http_req_cond (http_rules.c:239)
    ==7795==    by 0x41CD7B: cfg_parse_listen (cfgparse-listen.c:1466)
    ==7795==    by 0x480383: readcfgfile (cfgparse.c:2089)
    ==7795==    by 0x47A081: init (haproxy.c:1581)
    ==7795==    by 0x4049F2: main (haproxy.c:2591)

This leak can be detected even in HAProxy 1.6, this patch thus should
be backported to all supported branches.

6 years agoMINOR: streams: Introduce a new retry-on keyword, all-retryable-errors.
Olivier Houchard [Fri, 10 May 2019 16:05:40 +0000 (18:05 +0200)] 
MINOR: streams: Introduce a new retry-on keyword, all-retryable-errors.

Add a new retry-on keyword, "all-retryable-errors", that activates retry
for all errors that are considered retryable.
This currently activates retry for "conn-failure", "empty-response",
"junk-respones", "response-timeout", "0rtt-rejected", "500", "502", "503" and
"504".

6 years agoMEDIUM: streams: Add a new http action, disable-l7-retry.
Olivier Houchard [Fri, 10 May 2019 11:59:15 +0000 (13:59 +0200)] 
MEDIUM: streams: Add a new http action, disable-l7-retry.

Add a new action for http-request, disable-l7-retry, that can be used to
disable any attempt at retry requests (see retry-on) if it fails for any
reason other than a connection failure.
This is useful for example to make sure POST requests aren't retried.

6 years agoBUG/MEDIUM: streams: Make sur SI_FL_L7_RETRY is set before attempting a retry.
Olivier Houchard [Fri, 10 May 2019 15:48:28 +0000 (17:48 +0200)] 
BUG/MEDIUM: streams: Make sur SI_FL_L7_RETRY is set before attempting a retry.

In a few cases, we'd just check if the backend is configured to do retries,
and not if it's still allowed on the stream_interface.
The SI_FL_L7_RETRY flag could have been removed because we failed to allocate
a buffer, or because the request was too big to fit in a single buffer,
so make sure it's there before attempting a retry.

6 years agoBUG/MEDIUM: h2: Don't check send_wait to know if we're in the send_list.
Olivier Houchard [Fri, 10 May 2019 12:02:21 +0000 (14:02 +0200)] 
BUG/MEDIUM: h2: Don't check send_wait to know if we're in the send_list.

When we have to stop sending due to the stream flow control, don't check
if send_wait is NULL to know if we're in the send_list, because at this
point it'll always be NULL, while we're probably in the list.
Use LIST_ISEMPTY(&h2s->list) instead.
Failing to do so mean we might be added in the send_list when flow control
allows us to emit again, while we're already in it.
While I'm here, replace LIST_DEL + LIST_INIT by LIST_DEL_INIT.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: http: Use pointer to the begining of input to parse message headers
Christopher Faulet [Fri, 10 May 2019 09:36:51 +0000 (11:36 +0200)] 
BUG/MEDIUM: http: Use pointer to the begining of input to parse message headers

In the legacy HTTP, when the message headers are parsed, in http_msg_analyzer(),
we must use the begining of input and not the head of the buffer. Most of time,
it will be the same pointers because there is no outgoing data when a new
message is received. But when a 1xx informational response is parsed, it is
forwarded and the parsing restarts immediatly. In this case, we have outgoing
data when the next response is parsed.

This patch must be backported to 1.9.

6 years agoBUG/MINOR: stream: Attach the read side on the response as soon as possible
Christopher Faulet [Thu, 9 May 2019 09:46:52 +0000 (11:46 +0200)] 
BUG/MINOR: stream: Attach the read side on the response as soon as possible

A backend stream-interface attached to a reused connection remains in the state
SI_ST_CONN until some data are sent to validate the connection. But when the
url_param algorithm is used to balance connections, no data are sent while the
connection is not established. So it is a chicken and egg situation.

To solve the problem, if no error is detected and when the request channel is
waiting for the connect(), we mark the read side as attached on the response
channel as soon as possible and we wake the request channel up once. This
happens in 2 places. The first one is right after the connect(), when the
stream-interface is still in state SI_ST_CON, in the function
sess_update_st_con_tcp(). The second one is when an applet is used instead of a
real connection to a server, in the function sess_prepare_conn_req(). In fact,
it is done when the backend stream-interface is set to the state SI_ST_EST.

This patch must be backported to 1.9.

6 years agoBUILD: threads: Add __ha_cas_dw fallback for single threaded builds
Chris Packham [Thu, 9 May 2019 05:07:40 +0000 (17:07 +1200)] 
BUILD: threads: Add __ha_cas_dw fallback for single threaded builds

__ha_cas_dw() is used in fd_rm_from_fd_list() and when built without
USE_THREADS=1 the linker fails to find __ha_cas_dw(). Add a definition
of __ha_cas_dw() for the #ifndef USE_THREADS case.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
6 years agoBUILD: add BoringSSL to travis-ci build matrix
Ilya Shipitsin [Wed, 8 May 2019 20:15:59 +0000 (01:15 +0500)] 
BUILD: add BoringSSL to travis-ci build matrix

6 years agoCLEANUP: ssl: make inclusion of openssl headers safe
Willy Tarreau [Fri, 10 May 2019 07:58:43 +0000 (09:58 +0200)] 
CLEANUP: ssl: make inclusion of openssl headers safe

It's always a pain to have to stuff lots of #ifdef USE_OPENSSL around
ssl headers, it even results in some of them appearing in a random order
and multiple times just to benefit form an existing ifdef block. Let's
make these headers safe for inclusion when USE_OPENSSL is not defined,
they now perform the test themselves and do nothing if USE_OPENSSL is
not defined. This allows to remove no less than 8 such ifdef blocks
and make include blocks more readable.

6 years agoCLEANUP: ssl: never include openssl/*.h outside of openssl-compat.h anymore
Willy Tarreau [Fri, 10 May 2019 07:35:00 +0000 (09:35 +0200)] 
CLEANUP: ssl: never include openssl/*.h outside of openssl-compat.h anymore

Since we're providing a compatibility layer for multiple OpenSSL
implementations and their derivatives, it is important that no C file
directly includes openssl headers but only passes via openssl-compat
instead. As a bonus this also gets rid of redundant complex rules for
inclusion of certain files (engines etc).

6 years agoREORG: ssl: move some OpenSSL defines from ssl_sock to openssl-compat
Willy Tarreau [Fri, 10 May 2019 07:22:53 +0000 (09:22 +0200)] 
REORG: ssl: move some OpenSSL defines from ssl_sock to openssl-compat

Some defines like OPENSSL_VERSION or X509_getm_notBefore() have nothing
to do in ssl_sock and must move to openssl-compat.h so that they are
consistently shared by the whole code. A warning in the code was added
against wild additions of macros there.

6 years agoREORG: ssl: move openssl-compat from proto to common
Willy Tarreau [Thu, 9 May 2019 12:52:44 +0000 (14:52 +0200)] 
REORG: ssl: move openssl-compat from proto to common

This way we can include it much earlier to cover types/ as well.

6 years agoBUILD: ssl: fix libressl build again after aes-gcm-enc
Willy Tarreau [Fri, 10 May 2019 07:16:53 +0000 (09:16 +0200)] 
BUILD: ssl: fix libressl build again after aes-gcm-enc

Enabling aes-gcm-enc in last commit (MINOR: ssl: enable aes_gcm_dec
on LibreSSL) uncovered a wrong condition on the define of the
EVP_CTRL_AEAD_SET_IVLEN macro which I forgot to add when making the
commit, resulting in breaking libressl build again. In case libressl
later defines this macro, the test will have to change for a version
range instead.

6 years agoMINOR: ssl: enable aes_gcm_dec on LibreSSL
Willy Tarreau [Thu, 9 May 2019 12:15:32 +0000 (14:15 +0200)] 
MINOR: ssl: enable aes_gcm_dec on LibreSSL

This one requires OpenSSL 1.0.1 and above, and libressl was forked from
1.0.1g and is compatible (build-tested). No need to exclude it anymore
from using this converter.

6 years agoCLEANUP: ssl: remove 57 occurrences of useless tests on LIBRESSL_VERSION_NUMBER
Willy Tarreau [Thu, 9 May 2019 12:13:35 +0000 (14:13 +0200)] 
CLEANUP: ssl: remove 57 occurrences of useless tests on LIBRESSL_VERSION_NUMBER

They were all check to comply with the advertised openssl version. Now
that libressl doesn't pretend to be a more recent openssl anymore, we
can simply rely on the regular openssl version tests without having to
deal with exceptions for libressl.

6 years agoBUILD: ssl: make libressl use its own version numbers
Willy Tarreau [Thu, 9 May 2019 11:41:45 +0000 (13:41 +0200)] 
BUILD: ssl: make libressl use its own version numbers

LibreSSL causes lots of build issues by pretending to be OpenSSL 2.0.0,
and it requires lots of care for each #if added to cover any specific
OpenSSL features.

This commit addresses the problem by making LibreSSL only advertise the
version it forked from (1.0.1g) and by starting to use tests based on
its real version to enable features instead of working by exclusion.

6 years agoCLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER instead of OPENSSL_VERSION_NUMBER
Willy Tarreau [Thu, 9 May 2019 11:26:41 +0000 (13:26 +0200)] 
CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER instead of OPENSSL_VERSION_NUMBER

Most tests on OPENSSL_VERSION_NUMBER have become complex and break all
the time because this number is fake for some derivatives like LibreSSL.
This patch creates a new macro, HA_OPENSSL_VERSION_NUMBER, which will
carry the real openssl version defining the compatibility level, and
this version will be adjusted depending on the variants.

6 years agoBUILD: ssl: fix again a libressl build failure after the openssl FD leak fix
Willy Tarreau [Thu, 9 May 2019 11:53:28 +0000 (13:53 +0200)] 
BUILD: ssl: fix again a libressl build failure after the openssl FD leak fix

As with every single OpenSSL fix, LibreSSL build broke again, this time
after commit 56996dabe ("BUG/MINOR: mworker/ssl: close OpenSSL FDs on
reload"). A definitive solution will have to be found quickly. For now,
let's exclude libressl from the version test.

This patch must be backported to 1.9 since the fix above was already
backported there.

6 years agoBUG/MEDIUM: h2: Make sure we set send_list to NULL in h2_detach().
Olivier Houchard [Thu, 9 May 2019 11:24:08 +0000 (13:24 +0200)] 
BUG/MEDIUM: h2: Make sure we set send_list to NULL in h2_detach().

In h2_detach(), if we still have a send_wait pointer, because we woke the
tasklet up, but it hasn't ran yet, explicitely set send_wait to NULL after
we removed the tasklet from the task list.
Failure to do so may lead to crashes if the h2s isn't immediately destroyed,
because we considered there were still something to send.

This should be backported to 1.9.

6 years agoBUG/MEDIUM: servers: Don't use the same srv flag for cookie-set and TFO.
Olivier Houchard [Wed, 8 May 2019 17:48:32 +0000 (19:48 +0200)] 
BUG/MEDIUM: servers: Don't use the same srv flag for cookie-set and TFO.

The tfo code was based on an old patch, and the value of the SRV_F_FASTOPEN
flag it used was since reused for SRV_F_COOKIESET. So give SRV_F_FASTOPEN
its own value.

6 years agoBUILD: travis-ci bugfixes and improvements
Ilya Shipitsin [Mon, 6 May 2019 20:42:43 +0000 (01:42 +0500)] 
BUILD: travis-ci bugfixes and improvements

Call missing scripts/build-ssl.sh (which actually builds SSL variants)
Enable OpenSSL, LibreSSL builds caching, it saves a bunch of time
LibreSSL builds are not allowed to fail anymore
Add openssl to osx builds

6 years agoMINOR: htx: Remove support for unused OOB HTX blocks
Christopher Faulet [Tue, 7 May 2019 19:48:12 +0000 (21:48 +0200)] 
MINOR: htx: Remove support for unused OOB HTX blocks

This type of block was introduced in the early design of the HTX and it is not
used anymore. So, just remove it.

This patch may be backported to 1.9.

6 years agoMINOR: htx: Don't try to append a trailer block with the previous one
Christopher Faulet [Tue, 7 May 2019 19:42:27 +0000 (21:42 +0200)] 
MINOR: htx: Don't try to append a trailer block with the previous one

In H1 and H2, one and only one trailer block is emitted during the HTTP
parsing. So it is useless to try to append this block with the previous one,
like for data block.

This patch may be backported to 1.9.

6 years agoMINOR: htx: Split on DATA blocks only when blocks are moved to an HTX message
Christopher Faulet [Tue, 7 May 2019 08:52:54 +0000 (10:52 +0200)] 
MINOR: htx: Split on DATA blocks only when blocks are moved to an HTX message

When htx_xfer_blks() is called to move blocks from an HTX message to another
one, most of blocks must be transferred atomically. But some may be splitted if
there is not enough space to move all the block. This was true for DATA and TLR
blocks. But it is a bad idea to split trailers. During HTTP parsing, only one
TLR block is emitted. It simplifies the processing of trailers to keep the block
untouched.

This patch must be backported to 1.9 because some fixes may depend on it.

6 years agoBUG/MINOR: htx: Never transfer more than expected in htx_xfer_blks()
Christopher Faulet [Tue, 7 May 2019 08:52:25 +0000 (10:52 +0200)] 
BUG/MINOR: htx: Never transfer more than expected in htx_xfer_blks()

When the maximum free space available for data in the HTX message is compared to
the number of bytes to transfer, we must take into account the amount of data
already transferred. Otherwise we may move more data than expected.

This patch must be backported to 1.9.

6 years agoBUG/MINOR: mux-h1: Fix the parsing of trailers
Christopher Faulet [Tue, 7 May 2019 08:53:32 +0000 (10:53 +0200)] 
BUG/MINOR: mux-h1: Fix the parsing of trailers

Unlike other H1 parsing functions, the 3rd parameter of the function
h1_measure_trailers() is the maximum number of bytes to read. For others
functions, it is the relative offset where to stop the parsing.

This patch must be backported to 1.9.

6 years agoBUG/MEDIUM: spoe: Be sure the sample is found before setting its context
Christopher Faulet [Mon, 6 May 2019 07:53:10 +0000 (09:53 +0200)] 
BUG/MEDIUM: spoe: Be sure the sample is found before setting its context

When a sample fetch is encoded, we use its context to set info about the
fragmentation. But if the sample is not found, the function sample_process()
returns NULL. So we me be sure the sample exists before setting its context.

This patch must be backported to 1.9 and 1.8.

6 years agoBUG/MINOR: mux-h2: fix the condition to close a cs-less h2s on the backend
Willy Tarreau [Tue, 7 May 2019 16:10:10 +0000 (18:10 +0200)] 
BUG/MINOR: mux-h2: fix the condition to close a cs-less h2s on the backend

A typo was introduced in the following commit : 927b88ba0 ("BUG/MAJOR:
mux-h2: fix race condition between close on both ends") making the test
on h2s->cs never being done and h2c->cs being dereferenced without being
tested. This also confirms that this condition does not happen on this
side but better fix it right now to be safe.

This must be backported to 1.9.

6 years agoMINOR: mworker: support a configurable maximum number of reloads
William Lallemand [Tue, 7 May 2019 15:49:33 +0000 (17:49 +0200)] 
MINOR: mworker: support a configurable maximum number of reloads

This patch implements a new global parameter for the master-worker mode.
When setting the mworker-max-reloads value, a worker receive a SIGTERM
if its number of reloads is greater than this value.

6 years agoCLEANUP: task: remove unneeded tests before task_destroy()
Willy Tarreau [Tue, 7 May 2019 17:05:35 +0000 (19:05 +0200)] 
CLEANUP: task: remove unneeded tests before task_destroy()

Since previous commit it's not needed anymore to test a task pointer
before calling task_destory() so let's just remove these tests from
the various callers before they become confusing. The function's
arguments were also documented. The same should probably be done
with tasklet_free() which involves a test in roughly half of the
call places.

6 years agoBUG/MEDIUM: tasks: fix possible segfault on task_destroy()
Dragan Dosen [Tue, 7 May 2019 13:25:25 +0000 (15:25 +0200)] 
BUG/MEDIUM: tasks: fix possible segfault on task_destroy()

Commit 3f795f7 ("MEDIUM: tasks: Merge task_delete() and task_free() into
task_destroy().") replaced task_delete() and task_free() with a single
function named task_destroy().

This patch adds a check for struct task* argument in function
task_destroy() to prevent a possible segfault on NULL and also to make
the function safer for use in other cases.

6 years agoBUG/MEDIUM: stick-table: fix regression caused by a change in proxy struct
Dragan Dosen [Tue, 7 May 2019 12:16:18 +0000 (14:16 +0200)] 
BUG/MEDIUM: stick-table: fix regression caused by a change in proxy struct

In commit 1b8e68e ("MEDIUM: stick-table: Stop handling stick-tables as
proxies."), the ->table member of proxy struct was replaced by a pointer
that is not always checked and in some situations can cause a segfault,
eg. during reload or while using "show table" on CLI socket.

No backport is needed.

6 years agoMINOR: systemd: support /etc/sysconfig/ for redhat based distrib
William Lallemand [Tue, 7 May 2019 12:00:33 +0000 (14:00 +0200)] 
MINOR: systemd: support /etc/sysconfig/ for redhat based distrib

The patch "MINOR: systemd: Make use of master socket in systemd unit"
introduces an environment file in /etc/default.

Unfortunatly this is not supported on redhat-based system, so we add
/etc/sysconfig/haproxy for that.

6 years agoMINOR: systemd: Make use of master socket in systemd unit
Tim Duesterhus [Mon, 6 May 2019 11:00:53 +0000 (13:00 +0200)] 
MINOR: systemd: Make use of master socket in systemd unit

Unless the EXTRAOPTS variable is overriden in /etc/default/haproxy
the unit file will use the master socket by default.

This patch may be backported to 1.9 and depends on
MINOR: systemd: Use the variables from /etc/default/haproxy.

6 years agoMINOR: systemd: Use the variables from /etc/default/haproxy
Apollon Oikonomopoulos [Mon, 6 May 2019 11:00:52 +0000 (13:00 +0200)] 
MINOR: systemd: Use the variables from /etc/default/haproxy

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
6 years agoBUG/MINOR: mworker/ssl: close OpenSSL FDs on reload
Rob Allen [Fri, 3 May 2019 08:11:32 +0000 (09:11 +0100)] 
BUG/MINOR: mworker/ssl: close OpenSSL FDs on reload

From OpenSSL 1.1.1, the default behaviour is to maintain open FDs to any
random devices that get used by the random number library. As a result,
those FDs leak when the master re-execs on reload; since those FDs are
not marked FD_CLOEXEC or O_CLOEXEC, they also get inherited by children.
Eventually both master and children run out of FDs.

OpenSSL 1.1.1 introduces a new function to control whether the random
devices are kept open. When clearing the keep-open flag, it also closes
any currently open FDs, so it can be used to clean-up open FDs too.
Therefore, a call to this function is made in mworker_reload prior to
re-exec.

The call is guarded by whether SSL is in use, because it will cause
initialisation of the OpenSSL random number library if that has not
already been done.

This should be backported to 1.9 and 1.8.

6 years agoREGTEST: Wrong assumption in IP:port logging test.
Frédéric Lécaille [Tue, 7 May 2019 10:00:36 +0000 (12:00 +0200)] 
REGTEST: Wrong assumption in IP:port logging test.

In this reg test, as the client connection is not supposed to receive any
server response, we should try to "rxresp" but we should expect the client
connection to be closed by haproxy. This is done replacing "rxresp" by
"expect_close". Furthermore since dbb75ee3 vtest commit, calling "rxresp"
expects at least to receive a HTTP header as shown by Travis build
here: https://travis-ci.com/haproxy/haproxy/jobs/198126488.

6 years agoREGTEST: Wrong renaming for one reg test.
Frédéric Lécaille [Tue, 7 May 2019 09:18:56 +0000 (11:18 +0200)] 
REGTEST: Wrong renaming for one reg test.

Fix a wrong reg test file renaming which came with d7a8f14 commit
(REGTEST: rename the reg test files). This prevented
reg-tests/log/wrong_ip_port_logging.vtc with "bug" as reg test type
from being run.

6 years agoREGTEST: Reg tests file renaming.
Frédéric Lécaille [Tue, 7 May 2019 09:14:29 +0000 (11:14 +0200)] 
REGTEST: Reg tests file renaming.

Remove old s_ prefixes for some reg tests after they have been flagged
as slow by 91704bfa commit (REGTEST: Flag some slow reg tests).

6 years agoREGTEST: Flag some slow reg tests.
Frédéric Lécaille [Tue, 7 May 2019 09:11:00 +0000 (11:11 +0200)] 
REGTEST: Flag some slow reg tests.

All reg-tests/peers/*basic_sync*.vtc tests are slow reg reg tests.

6 years agoBUG/MEDIUM: h2/htx: never leave a trailers block alone with no EOM block
Willy Tarreau [Tue, 7 May 2019 09:17:32 +0000 (11:17 +0200)] 
BUG/MEDIUM: h2/htx: never leave a trailers block alone with no EOM block

If when receiving an H2 response we fail to add an EOM block after too
large a trailers block, we must not leave the trailers block alone as it
violates the internal assumptions by not being followed by an EOM, even
when an error is reported. We must then make sure the error will safely
be reported to upper layers and that no attempt will be made to forward
partial blocks.

This must be backported to 1.9.

6 years agoBUG/MEDIUM: mux-h2/htx: never wait for EOM when processing trailers
Willy Tarreau [Mon, 6 May 2019 09:23:29 +0000 (11:23 +0200)] 
BUG/MEDIUM: mux-h2/htx: never wait for EOM when processing trailers

In message https://www.mail-archive.com/haproxy@formilux.org/msg33541.html
Patrick Hemmer reported an interesting bug affecting H2 and trailers.

The problem is that in order to close the stream we have to see the EOM
block, but nothing guarantees it will atomically be delivered with the
trailers block(s). So the code currently waits for it by returning zero
when it was not found, resulting in the caller (h2_snd_buf()) to loop
forever calling it again.

The current internal connection/connstream API doesn't allow a send
actor to notify its caller that it cannot process the data until it
gets more, so even returning zero will only lead to calls in loops
without any guarantee that any progress will be made.

Some late amendments to HTX already guaranteed the atomicity of the
trailers block during snd_buf(), which is currently ensured by the
fact that producers create exactly one such trailers block for all
trailers. So in practice we can only loop between trailers and EOM.

This patch changes the behaviour by making h2s_htx_make_trailers()
become atomic by not consuming the EOM block. This way either it finds
the end of trailers marker (empty line) or it fails. Once it sends the
trailers block, ES is set so the stream turns HLOC or CLOSED. Thanks
to previous patch "MEDIUM: mux-h2: discard contents that are to be sent
after a shutdown" is is now safe to interrupt outgoing data processing,
and the late EOM block will silently be discarded when the caller
finally sends it.

This is a bit tricky but should remain solid by design, and seems like
the only option we have that is compatible with 1.9, where it must be
backported along with the aforementioned patch.

6 years agoMEDIUM: mux-h2: discard contents that are to be sent after a shutdown
Willy Tarreau [Mon, 6 May 2019 13:00:22 +0000 (15:00 +0200)] 
MEDIUM: mux-h2: discard contents that are to be sent after a shutdown

In h2_snd_buf() we discard any possible buffer contents requested to be
sent after a close or an error. But in practice we can extend this to
any case where the stream is locally half-closed since it means we will
never be able to send these data anymore.

For now it must not change anything, but it will be used by subsequent
patches to discard lone a HTX EOM block arriving after the trailers
block.

6 years agoBUG/MEDIUM: h2/htx: always fail on too large trailers
Willy Tarreau [Mon, 6 May 2019 09:12:18 +0000 (11:12 +0200)] 
BUG/MEDIUM: h2/htx: always fail on too large trailers

In case a header frame carrying trailers just fits into the HTX buffer
but leaves no room for the EOM block, we used to return the same code
as the one indicating we're missing data. This could would result in
such frames causing timeouts instead of immediate clean aborts. Now
they are properly reported as stream errors (since the frame was
decoded and the compression context is still synchronized).

This must be backported to 1.9.

6 years agoBUG/MINOR: mux-h2: rely on trailers output not input to turn them to empty data
Willy Tarreau [Mon, 6 May 2019 13:13:41 +0000 (15:13 +0200)] 
BUG/MINOR: mux-h2: rely on trailers output not input to turn them to empty data

When sending trailers, we may face an empty HTX trailers block or even
have to discard some of the headers there and be left with nothing to
send. RFC7540 forbids sending of empty HEADERS frames, so in this case
we turn to DATA frames (which is possible since after other DATA).

The code used to only check the input frame's contents to decide whether
or not to switch to a DATA frame, it didn't consider the possibility that
the frame only used to contain headers discarded later, thus it could still
emit an empty HEADERS frame in such a case. This patch makes sure that the
output frame size is checked instead to take the decision.

This patch must be backported to 1.9. In practice this situation is never
encountered since the discarded headers have really nothing to do in a
trailers block.

6 years agoREGTEST: make the "table in peers" test require v2.0
Willy Tarreau [Tue, 7 May 2019 05:53:54 +0000 (07:53 +0200)] 
REGTEST: make the "table in peers" test require v2.0

And the second test also requires openssl otherwise it fails as seen
here : https://travis-ci.com/haproxy/haproxy/jobs/198126488

6 years agoREGTEST: make the tls_health_checks test much faster
Willy Tarreau [Tue, 7 May 2019 05:26:08 +0000 (07:26 +0200)] 
REGTEST: make the tls_health_checks test much faster

This test relies on a server timeout and was using the default 2s check
interval with a full 1s server timeout, thus adding a whole second to the
test series by itself. Let's shrink the server timeout to 20ms which is
way enough to properly trigger a timeout, and set the check interval to
the double of this, or 40ms.

6 years agoMEDIUM: regex: modify regex_comp() to atomically allocate/free the my_regex struct
Dragan Dosen [Tue, 30 Apr 2019 13:54:36 +0000 (15:54 +0200)] 
MEDIUM: regex: modify regex_comp() to atomically allocate/free the my_regex struct

Now we atomically allocate the my_regex struct within function
regex_comp() and compile the regex or free both in case of failure. The
pointer to the allocated my_regex struct is returned directly. The
my_regex* argument to regex_comp() is removed.

Function regex_free() was modified so that it systematically frees the
my_regex entry. The function does nothing when called with a NULL as
argument (like free()). It will avoid existing risk of not properly
freeing the initialized area.

Other structures are also updated in order to be compatible (the ones
related to Lua and action rules).

6 years agoREGTEST: Add reg tests for "table" lines in "peers" sections.
Frédéric Lécaille [Mon, 18 Mar 2019 13:08:46 +0000 (14:08 +0100)] 
REGTEST: Add reg tests for "table" lines in "peers" sections.

These reg tests are there to test the support for stick-table declarations
in "peers" sections ("table" keyword).

6 years agoDOC: Update for "table" lines in "peers" section.
Frédéric Lécaille [Mon, 18 Mar 2019 13:05:58 +0000 (14:05 +0100)] 
DOC: Update for "table" lines in "peers" section.

6 years agoMINOR: peers: Do not emit global stick-table names.
Frédéric Lécaille [Wed, 20 Mar 2019 14:09:45 +0000 (15:09 +0100)] 
MINOR: peers: Do not emit global stick-table names.

This commit "MINOR: stick-table: Add prefixes to stick-table names"
prepended the "peers" section name to stick-table names declared in such "peers"
sections followed by a '/' character.  This is not this name which must be sent
over the network to avoid collisions with stick-table name declared as backends.
As the '/' character is forbidden as first character of a backend name, we prefix
the stick-table names declared in peers sections only with a '/' character.
With such declarations:

    peers mypeers
       table t1

backend t1
   stick-table ... peers mypeers

at peer protocol level, "t1" declared as stick-table in "mypeers" section is different
of "t1" stick-table declared as backend.

In src/peers.c, only two modifications were required: use ->nid stktable struct
member in place of ->id in peer_prepare_switchmsg() to prepare the stick-table
definition messages. Same thing in peer_treat_definemsg() to treat a stick-table
definition messages.

6 years agoMINOR: stick-table: Add prefixes to stick-table names.
Frédéric Lécaille [Wed, 20 Mar 2019 14:06:55 +0000 (15:06 +0100)] 
MINOR: stick-table: Add prefixes to stick-table names.

With this patch we add a prefix to stick-table names declared in "peers" sections
concatenating the "peers" section name followed by a '/' character with
the stick-table name. Consequently, "peers" sections have their own
namespace for their stick-tables. Obviously, these stick-table names are not the
ones which should be sent over the network. So these configurations must be
compatible and should make A and B peers communicate with peers protocol:

    # haproxy A config, old way stick-table declerations
    peers mypeers
        peer A ...
        peer B ...

    backend t1
        stick-table type string size 10m store gpc0 peers mypeers

    # haproxy B config, new way stick-table declerations
    peers mypeers
        peer A ...
        peer B ...
        table t1 type string size store gpc0 10m

This "network" name is stored in ->nid new field of stktable struct. The "local"
stktable-name is still stored in ->id.

6 years agoMINOR: stick-tables: Add peers process binding computing.
Frédéric Lécaille [Tue, 19 Mar 2019 13:55:01 +0000 (14:55 +0100)] 
MINOR: stick-tables: Add peers process binding computing.

Add a list of proxies for all the stick-tables (->proxies_list struct stktable
member) so that to be able to compute the process bindings of the peers after having
parsed the configuration file.
The proxies are added to the stick-tables they reference when parsing
stick-tables lines in proxy sections, when checking the actions in
check_trk_action() and when resolving samples args for stick-tables
without checking is they are duplicates. We check only there is no loop.
Then, after having parsed everything, we add the proxy bindings to the
peers frontend bindings with stick-tables they reference.

6 years agoMEDIUM: stick-table: Stop handling stick-tables as proxies.
Frédéric Lécaille [Thu, 14 Mar 2019 06:07:41 +0000 (07:07 +0100)] 
MEDIUM: stick-table: Stop handling stick-tables as proxies.

This patch adds the support for the "table" line parsing in "peers" sections
to declare stick-table in such sections. This also prevents the user from having
to declare dummy backends sections with a unique stick-table inside.
Even if still supported, this usage will become deprecated.

To do so, the ->table member of proxy struct which is a stktable struct is replaced
by a pointer to a stktable struct allocated at parsing time in src/cfgparse-listen.c
for the dummy stick-table backends and in src/cfgparse.c for "peers" sections.
This has an impact on the code for stick-table sample converters and on the stickiness
rules parsers which first store the name of the dummy before resolving the rules.
This patch replaces proxy_tbl_by_name() calls by stktable_find_by_name() calls
to lookup for stick-tables stored in "stktable_by_name" ebtree at parsing time.
There is only one remaining place where proxy_tbl_by_name() is used: src/hlua.c.

At several places in the code we relied on the fact that ->size member of stick-table
was equal to zero to consider the stick-table was present by not configured,
this do not make sense anymore as ->table member of struct proxyis fow now on a pointer.
These tests are replaced by a test on ->table value itself.

In "peers" section we do not have to temporary store the name of the section the
stick-table are attached to because this name is obviously already known just after
having entered this "peers" section.

About the CLI stick-table I/O handler, the pointer to proxy struct is replaced by
a pointer to a stktable struct.

6 years agoBUILD/MINOR: stick-table: Compilation fix.
Frédéric Lécaille [Fri, 15 Mar 2019 10:24:53 +0000 (11:24 +0100)] 
BUILD/MINOR: stick-table: Compilation fix.

Missing header to dereference struct peers pointer from struct table.