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