William Dauchy [Thu, 6 Aug 2020 16:11:38 +0000 (18:11 +0200)]
MINOR: ssl: add ssl_{c,s}_chain_der fetch methods
Following work from Arjen and Mathilde, it adds ssl_{c,s}_chain_der
methods; it returns DER encoded certs from SSL_get_peer_cert_chain
Also update existing vtc tests to add random intermediate certificates
When getting the result through this header:
http-response add-header x-ssl-chain-der %[ssl_c_chain_der,hex]
One can parse it with any lib accepting ASN.1 DER data, such as in go:
bin, err := encoding/hex.DecodeString(cert)
certs_parsed, err := x509.ParseCertificates(bin)
MINOR: arg: Use chunk_destroy() to release string arguments
This way, all fields of the buffer structure are reset when a string argument
(ARGT_STR) is released. It is also a good way to explicitly specify this kind
of argument is a chunk. So .data and .size fields must be set.
MINOR: lua: Add support for regex as fetches and converters arguments
It means now regsub() converter is now exported to the lua. Map converters based
on regex are not available because the map arguments are not supported.
MEDIUM: lua: Don't filter exported fetches and converters
Thanks to previous commits, it is now safe to use from lua the sample fetches
and sample converters that convert arguments, especially the strings
(ARGT_STR). So now, there are all exported to the lua. They was filtered on the
validation functions. Only fetches without validation functions or with val_hdr
or val_payload_lv functions were exported, and converters without validation
functions.
This patch depends on following commits :
* aec27ef44 "BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array"
* fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It must be backported as far as 2.1 because the date() and http_date()
converters are no longer exported because of the filter on the validation
function, since the commit ae6f125c7 ("MINOR: sample: add us/ms support to
date/http_date)".
BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
Strings in the argument array used by sample fetches and converters must be
duplicated. This is mandatory because, during the arguments validations, these
strings may be converted and released. It works this way during the
configuration parsing and there is no reason to adapt this behavior during the
runtime when a sample fetch or a sample converter is called from the lua. In
fact, there is a reason to not change the behavior. It must reamain simple for
everyone to add new fetches or converters.
Thus, lua strings are duplicated. It is only performed at the end of the
hlua_lua2arg_check() function, if the argument is still a ARGT_STR. Of course,
it requires a cleanup loop after the call or when an error is triggered.
This patch depends on following commits:
* 959171376 "BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters"
* fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It may be backported to all supported versions, most probably as far as 2.1
only.
MINOR: hlua: Don't needlessly copy lua strings in trash during args validation
Lua strings are NULL terminated. So in the hlua_lua2arg_check() function, used
to check arguments against the sample fetches specification, there is no reason
to copy these strings in a trash to add a terminating null byte.
In addition, when the array of arguments is built from lua values, we must take
care to count this terminating null bytes in the size of the buffer where a
string is stored. The same must be done when a sample is built from a lua value.
BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
In hlua_lua2arg_check() function, before converting an argument to an IPv4 or
IPv6 mask, we must be sure to have an integer or a string argument (ARGT_SINT or
ARGT_STR).
This patch must be backported to all supported versions.
BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
Some sample fetches or sample converters uses a validation functions for their
arguments. In these function, string arguments (ARGT_STR) may be converted to
another type (for instance a regex, a variable or a integer). Because these
strings are allocated when the argument list is built, they must be freed after
a conversion. Most of time, it is done. But not always. This patch fixes these
minor memory leaks (only on few strings, during the configuration parsing).
This patch may be backported to all supported versions, most probably as far as
2.1 only. If this commit is backported, the previous one 73292e9e6 ("BUG/MINOR:
lua: Duplicate map name to load it when a new Map object is created") must also
be backported. Note that some validation functions does not exists on old
version. It should be easy to resolve conflicts.
BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
When a new map is created, the sample_load_map() function is called. To do so,
an argument array is created with the name as first argument. Because it is a
lua string, owned by the lua, it must be duplicated. The sample_load_map()
function will convert this argument to a map. In theory, after the conversion,
it must release the original string. It is not performed for now and it is a bug
that will be fixed in the next commit.
This patch may be backported to all supported versions, most probably as far as
2.1 only. But it must be backported with the next commit "BUG/MINOR: arg: Fix
leaks during arguments validation for fetches/converters".
BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
The debug() converter uses a string to reference the sink where to send debug
events. During the configuration parsing, this string is converted to a sink
object but it is still store as a string argument. It is a problem on deinit
because string arguments are released. So the sink pointer will be released
twice.
To fix the bug, we keep a reference on the sink using an ARGT_PTR argument. This
way, it will not be freed on the deinit.
This patch depends on the commit e02fc4d0d ("MINOR: arg: Add an argument type to
keep a reference on opaque data"). Both must be backported as far as 2.1.
MINOR: arg: Add an argument type to keep a reference on opaque data
The ARGT_PTR argument type may now be used to keep a reference to opaque data in
the argument array used by sample fetches and converters. It is a generic way to
point on data. I guess it could be used for some other arguments, like proxy,
server, map or stick-table.
BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
In sample_load_map() function, the global mode is now tested to be sure to be in
the starting mode. If not, an error is returned.
At first glance, this patch may seem useless because maps are loaded during the
configuration parsing. But in fact, it is possible to load a map from the lua,
using Map:new() method. And, there is nothing to forbid to call this method at
runtime, during a script execution. It must never be done because it may perform
an filesystem access for unknown maps or allocation for known ones. So at
runtime, it means a blocking call or a memroy leak. Note it is still possible to
load a map from the lua, but in the global part of a script only. This part is
executed during the configuration parsing.
This patch must be backported in all stable versions.
BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
This bug affects all version of HAProxy since the OCSP data is not free
in the deinit(), but leaking on exit() is not really an issue. However,
when doing dynamic update of certificates over the CLI, those data are
not free'd upon the free of the SSL_CTX.
3 leaks are happening, the first leak is the one of the ocsp_arg
structure which serves the purpose of containing the pointers in the
case of a multi-certificate bundle. The second leak is the one ocsp
struct. And the third leak is the one of the struct buffer in the
ocsp_struct.
The problem lies with SSL_CTX_set_tlsext_status_arg() which does not
provide a way to free the argument upon an SSL_CTX_free().
This fix uses ex index functions instead of registering a
tlsext_status_arg(). This is really convenient because it allows to
register a free callback which will free the ex index content upon a
SSL_CTX_free().
A refcount was also added to the ocsp_response structure since it is
stored in a tree and can be reused in another SSL_CTX.
William Dauchy [Sat, 1 Aug 2020 14:28:51 +0000 (16:28 +0200)]
BUG/MINOR: spoa-server: fix size_t format printing
From https://www.python.org/dev/peps/pep-0353/
"A new type Py_ssize_t is introduced, which has the same size as the
compiler's size_t type, but is signed. It will be a typedef for ssize_t
where available."
For integer types, causes printf to expect a size_t-sized integer
argument.
This should fix github issue #702
This should be backported to >= v2.2
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Basically, when a server is managed through SRV records additional
section and it's disabled (because its associated Additional record has
disappeared), it never leaves its MAINT state and so never comes back to
production.
This patch updates the "snr_update_srv_status()" function to clear the
MAINT status when the server now has an IP address and also ensure this
function is called when parsing Additional records (and associating them
to new servers).
This can cause severe outage for people using HAProxy + consul (or any
other service registry) through DNS service discovery).
This should fix issue #793.
This should be backported to 2.2.
BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
The H1 multiplexer is able to perform synchronous send. When a large body is
transfer, if nothing is received and if no error or shutdown occurs, it is
possible to not go down at the H1 connection level to do I/O for a long
time. When this happens, we must still take care to refresh the H1 connection
timeout. Otherwise it is possible to hit the connection timeout during the
transfer while it should not expire.
This bug exists because only h1_process() refresh the H1 connection timeout. To
fix the bug, h1_snd_buf() must also refresh this timeout. To make things more
readable, a dedicated function has been introduced and called to refresh the
timeout.
This bug exists on all HTX versions. But it is harder to hit it on 2.1 and below
because when a H1 mux is initialized, we actively try to read data instead of
subscribing for receiving. So there is at least one call to h1_process().
This patch should fix the issue #790. It must be backported as far as 2.0.
Ilya Shipitsin [Tue, 4 Aug 2020 10:36:24 +0000 (15:36 +0500)]
BUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set
The SSL_INC and SSL_LIB variables were not initialized in the Makefile,
so they could be accidently inherited from the environment. We require
that any makefile variable is explicitly set on the command line so they
must be initialized.
Note that the Travis scripts used to rely only on these variables to be
exported, so it was adjusted as well.
SCRIPTS: git-show-backports: emit the shell command to backport a commit
It's cumbersome to copy-paste a commit ID into another window after having
typed "git cherry-pick -sx", so let's have the suggested output format of
git-show prepare this line just before the subject line, it remains at a
stable position on the terminal when searching for "/^commit". One just
has to copy-paste the line into another terminal will result in the commit
being properly picked.
SCRIPTS: git-show-backports: make -m most only show the left branch
We've never used the output of the rightmost branch with this tool,
and it systematically causes two identical outputs making the job
harder during backport sessions. Let's simply remove the right part
when it's identical to the left one. This also adds a few line feeds
to make the output more readable.
Released version 2.3-dev2 with the following main changes :
- DOC: ssl: req_ssl_sni needs implicit TLS
- BUG/MEDIUM: arg: empty args list must be dropped
- BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
- BUG/MAJOR: tasks: don't requeue global tasks into the local queue
- MINOR: tasks/debug: make the thread affinity BUG_ON check a bit stricter
- MINOR: tasks/debug: add a few BUG_ON() to detect use of wrong timer queue
- MINOR: tasks/debug: add a BUG_ON() check to detect requeued task on free
- BUG/MAJOR: dns: Make the do-resolve action thread-safe
- BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
- MEDIUM: htx: Add a flag on a HTX message when no more data are expected
- BUG/MEDIUM: stream-int: Don't set MSG_MORE flag if no more data are expected
- BUG/MEDIUM: http-ana: Only set CF_EXPECT_MORE flag on data filtering
- CLEANUP: dns: remove 45 "return" statements from dns_validate_dns_response()
- BUG/MINOR: htx: add two missing HTX_FL_EOI and remove an unexpected one
- BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
- BUILD: tools: fix build with static only toolchains
- DOC: Use gender neutral language
- BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
- BUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status
- BUG/MAJOR: dns: don't treat Authority records as an error
- CI : travis-ci : prepare for using stock OpenSSL
- CI: travis-ci : switch to stock openssl when openssl-1.1.1 is used
- MEDIUM: lua: Add support for the Lua 5.4
- BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
- BUG/MINOR: lua: Abort execution of actions that yield on a final evaluation
- MINOR: tcp-rules: Return an internal error if an action yields on a final eval
- BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort
- BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
- MEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset
- MEDIUM: lua: Set the analyse expiration date with smaller wake_time only
- BUG/MEDIUM: connection: Be sure to always install a mux for sync connect
- MINOR: connection: Preinstall the mux for non-ssl connect
- MINOR: stream-int: Be sure to have a mux to do sends and receives
- BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx
- SCRIPTS: announce-release: add the link to the wiki in the announce messages
- CI: travis-ci: use better name for Coverity scan job
- CI: travis-ci: use proper linking flags for SLZ build
- BUG/MEDIUM: backend: always attach the transport before installing the mux
- BUG/MEDIUM: tcp-checks: always attach the transport before installing the mux
- MINOR: connection: avoid a useless recvfrom() on outgoing connections
- MINOR: mux-h1: do not even try to receive if the connection is not fully set up
- MINOR: mux-h1: do not try to receive on backend before sending a request
- CLEANUP: assorted typo fixes in the code and comments
- BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
MINOR: mux-h1: do not try to receive on backend before sending a request
There's no point trying to perform an recv() on a back connection if we
have a stream before having sent a request, as it's expected to fail.
It's likely that this may avoid some spurious subscribe() calls in some
keep-alive cases (the close case was already addressed at the connection
level by "MINOR: connection: avoid a useless recvfrom() on outgoing
connections").
MINOR: connection: avoid a useless recvfrom() on outgoing connections
When a connect() doesn't immediately succeed (i.e. most of the times),
fd_cant_send() is called to enable polling. But given that we don't
mark that we cannot receive either, we end up performing a failed
recvfrom() immediately when the connect() is finally confirmed, as
indicated in issue #253.
This patch simply adds fd_cant_recv() as well so that we're only
notified once the recv path is ready. The reason it was not there
is purely historic, as in the past when there was the fd cache,
doing it would have caused a pending recv request to be placed into
the fd cache, hence a useless recvfrom() upon success (i.e. what
happens now).
Without this patch, forwarding 100k connections does this:
It could make sense to backport this patch to 2.2 and maybe 2.1 after
it has been sufficiently checked for absence of side effects in 2.3-dev,
as some people had reported an extra overhead like in issue #168.
BUG/MEDIUM: tcp-checks: always attach the transport before installing the mux
Similarly to the issue described in commit "BUG/MEDIUM: backend: always
attach the transport before installing the mux", in tcpcheck_eval_connect()
we can install a handshake transport layer underneath the mux and replace
its subscriptions, causing a crash if the mux had already subscribed for
whatever reason.
A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue
listen foo
bind :8181
mode http
option httpchk
server srv1 127.0.0.1:8888 send-proxy-v2 check inter 1000
The mux may only be installed *after* xprt_handshake is set up, so that
it registers against it and not against raw_sock or ssl_sock. This needs
to be backported to 2.2 which is the first version using muxes for checks.
BUG/MEDIUM: backend: always attach the transport before installing the mux
In connect_server(), we can enter in a stupid situation:
- conn_install_mux_be() is called to install the mux. This one
subscribes for receiving and quits ;
- then we discover that a handshake is required on the connection
(e.g. send-proxy), so xprt_add_hs() is called and subscribes as
well.
- we crash in conn_subscribe() which gets a different subscriber.
And if BUG_ON is disabled, we'd likely lose one event.
Note that it doesn't seem to happen by default, but definitely does
if connect() rightfully performs fd_cant_recv(), so it's a matter of
who does what and in what order.
A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue
listen foo
bind :8181
mode http
server srv1 127.0.0.1:8888 send-proxy-v2
The root cause is that xprt_add_hs() installs an xprt layer underneath
the mux without taking over its subscriptions. Ideally if we want to
support this, we'd need to steal the connection's wait_events and
replace them by new ones. But there doesn't seem to be any case where
we're interested in doing this so better simply always install the
transport layer before installing the mux, that's safer and simpler.
This needs to be backported to 2.1 which is constructed the same way
and thus suffers from the same issue, though the code is slightly
different there.
CI: travis-ci: use proper linking flags for SLZ build
previously SSL_INC and SSL_INC were set for all builds, and SLZ lib
was linked because of those flags. After we switched SLZ build to
stock openssl lib, SSL_INC, SSL_LIB are not set anymore.
BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx
This bug was introduced by the commit 8f587ea3 ("MEDIUM: lua: Set the analyse
expiration date with smaller wake_time only"). At the end of hlua_action(), the
lua context may be null if the alloc failed.
MINOR: stream-int: Be sure to have a mux to do sends and receives
In si_cs_send() and si_cs_recv(), we explicitly test the connection's mux is
defined to proceed. For si_cs_recv(), it is probably a bit overkill. But
opportunistic sends are possible from the moment the server connection is
created. So it is safer to do this test.
This patch may be backported as far as 1.9 if necessary.
MINOR: connection: Preinstall the mux for non-ssl connect
In the connect_server() function, there is an optim to install the mux as soon
as possible. It is possible if we can determine the mux to use from the
configuration only. For instance if the mux is explicitly specified or if no ALPN
is set. This patch adds a new condition to preinstall the mux for non-ssl
connection. In this case, by default, we always use the mux_pt for raw
connections and the mux-h1 for HTTP ones.
This patch is related to the issue #762. It may be backported to 2.2 (and
possibly as far as 1.9 if necessary).
BUG/MEDIUM: connection: Be sure to always install a mux for sync connect
Sometime, a server connection may be performed synchronously. Most of time on
TCP socket, it does not happen. It is easier to have sync connect with unix
socket. When it happens, if we are not waiting for any hanshake completion, we
must be sure to have a mux installed before leaving the connect_server()
function because an attempt to send may be done before the I/O connection
handler have a chance to be executed to install the mux, if not already done.
For now, It is not expected to perform a send with no mux installed, leading to
a crash if it happens.
This patch should fix the issue #762 and probably #779 too. It must be
backported as far as 1.9.
MEDIUM: lua: Set the analyse expiration date with smaller wake_time only
If a lua action yields for any reason and if the wake timeout is set, it only
override the analyse expiration date if it is smaller. This way, a lower
inspect-delay will be respected, if any.
MEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset
A dedicated expiration date is now used to apply the inspect-delay of the
tcp-request or tcp-response rulesets. Before, the analyse expiratation date was
used but it may also be updated by the lua (at least). So a lua script may
extend or reduce the inspect-delay by side effect. This is not expected. If it
becomes necessary, a specific function will be added to do this. Because, for
now, it is a bit confusing.
BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action yields
On a tcp-response content ruleset evaluation, the inspect-delay is engaged when
rule's conditions are not validated but not when the rule's action yields.
This patch must be backported to all supported versions.
BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort
When a tcp-request or a tcp-response content ruleset evaluation is aborted, the
corresponding FLT_END analyser must be preserved, if any. But because of a typo
error, on the tcp-response content ruleset evaluation, we try to preserve the
request analyser instead of the response one.
MINOR: tcp-rules: Return an internal error if an action yields on a final eval
On a final evaluation of a tcp-request or tcp-response content ruleset, it is
forbidden for an action to yield. To quickly identify bugs an internal error is
now returned if it happens and a warning log message is emitted.
BUG/MINOR: lua: Abort execution of actions that yield on a final evaluation
A Lua action may yield. It may happen because the action returns explicitly
act.YIELD or because the script itself yield. In the first case, we must abort
the script execution if it is the final rule evaluation, i.e if the
ACT_OPT_FINAL flag is set. The second case is already covered.
BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
When an action is evaluated, flags are passed to know if it is the first call
(ACT_OPT_FIRST) and if it must be the last one (ACT_OPT_FINAL). For the
do-resolve DNS action, the ACT_OPT_FINAL flag must be handled because the
action may yield. It must never yield when this flag is set. Otherwise, it may
lead to a wakeup loop of the stream because the inspected-delay of a tcp-request
content ruleset was reached without stopping the rules evaluation.
This patch is related to the issue #222. It must be backported as far as 2.0.
On Lua 5.4, some API changes make HAProxy compilation to fail. Among other
things, the lua_resume() function has changed and now takes an extra argument in
Lua 5.4 and the error LUA_ERRGCMM was removed. Thus the LUA_VERSION_NUM macro is
now tested to know the lua version is used and adapt the code accordingly.
Here are listed the incompatibilities with the previous Lua versions :
http://www.lua.org/manual/5.4/manual.html#8
This patch comes from the HAproxy's fedora RPM, committed by Tom Callaway :
BUG/MAJOR: dns: don't treat Authority records as an error
Support for DNS Service Discovery by means of SRV records was enhanced with
commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV responses")
to use the content of the answers Additional records when present.
If there are Authority records before the Additional records we mistakenly
treat that as an invalid response. To fix this, just ignore the Authority
section if it exist and skip to the Additional records.
As 13a9232eb was introduced during 2.2-dev, it must be backported to 2.2.
This is a fix for issue #778
BUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status
Since commit 13a9232eb ("MEDIUM: dns: use Additional records from SRV
responses"), a struct server can have a NULL dns_requester->resolution,
when SRV records are used and DNS answers contain an Additional section.
This is a problem when we call snr_update_srv_status() because it does
not check that resolution is NULL, and dereferences it. This patch
simply adds a test for resolution being NULL. When that happens, it means
we are using SRV records with Additional records, and an entry was removed.
This should fix issue #775.
This should be backported to 2.2.
BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
When the watchdog is fired because of the lua, the stack of the corresponding
lua context is dumped. But we must be sure the lua context is fully initialized
to do so. If we are blocked on the global lua lock, during the lua context
initialization, the lua stask may be NULL.
This patch should fix the issue #776. It must be backported as far as 2.0.
BUILD: tools: fix build with static only toolchains
uClibc toolchains built with no dynamic library support don't provide
the dlfcn.h header. That leads to build failure:
CC src/tools.o
src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory
#include <dlfcn.h>
^~~~~~~~~
Enable dladdr on Linux platforms only when USE_DL is defined.
This should be backported wherever 109201fc5 ("BUILD: tools: rely on
__ELF__ not USE_DL to enable use of dladdr()") is backported (currently
only 2.2 and 2.1).
BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
In the CGI/1.1 specification, it is specified the QUERY_STRING must not be
url-decoded. However, this parameter is sent decoded because it is extracted
after the URI's path decoding. Now, the query-string is first extracted, then
the script part of the path is url-decoded. This way, the QUERY_STRING parameter
is no longer decoded.
This patch should fix the issue #769. It must be backported as far as 2.1.
BUG/MINOR: htx: add two missing HTX_FL_EOI and remove an unexpected one
A workaround for some difficulties encountered to anticipate end of
messages was addressed by commit 810df0614 ("MEDIUM: htx: Add a flag on
a HTX message when no more data are expected"), but there were 3 issues
in it (with minor impact):
- the flag was mistakenly set before an EOH in Lua, which would only
cause incomplete packets to be emitted for now but could cause
truncated responses in the future. It's not needed to add it on
the next EOM block as http_forward_proxy_resp() already does it.
- one was still missing in hlua_applet_http_fct(), possibly causing
delays on Lua services
- one was missing in the Prometheus exporter.
All this simply shows that this mechanism is still quite fragile and
not trivial to use, especially in order to deal with the impossibility
to append the EOM, so we'll need to improve the solution in the future
and future backports should not be completely ruled out.
This fix must be backported where the patch above is backported,
typically 2.1 and later as it was required for a set of fixes.
CLEANUP: dns: remove 45 "return" statements from dns_validate_dns_response()
The previous leak on do-resolve was particularly tricky to check due
to the important code repetition in dns_validate_dns_response() which
required careful examination of all return statements to check whether
they needed a pool_free() or not. Let's clean all this up using a common
leave point which releases the element itself. This also encourages
to properly set the current response to null right after freeing or
adding it so that it doesn't get added. 45 return and 22 pool_free()
were replaced by one of each.
BUG/MEDIUM: http-ana: Only set CF_EXPECT_MORE flag on data filtering
This flag is set by HTTP analyzers to notify that more data are epxected. It is
used to know if the CO_SFL_MSG_MORE flag must be set on the connection when data
are sent. Historically, it was set on chuncked messages and on compressed
responses. But in HTX, the chunked messages are parsed by the H1 multipexer. So
for this case, the infinite forwarding is enabled and the flag must no longer be
set. For the compression, the test must be extended and be applied on all data
filters. Thus it is also true for the request channel.
So, now, CF_EXPECT_MORE flag is set on a request or a response channel if there
is at least one data filter attached to the stream. In addition, the flag is
removed when the HTTP message analysis is finished.
This patch should partially fix the issue #756. It must be backported to 2.1.
BUG/MEDIUM: stream-int: Don't set MSG_MORE flag if no more data are expected
In HTX, if the HTX_FL_EOI message is set on the message, we don't set the
CO_SFL_MSG_MORE flag on the connection. This way, the send is not delayed if
only the EOM is missing in the HTX message.
This patch depends on the commit "MEDIUM: htx: Add a flag on a HTX message when
no more data are expected".
This patch should partially fix the issue #756. It must be backported to
2.1. For earlier versions, CO_SFL_MSG_MORE is ignored by HTX muxes.
MEDIUM: htx: Add a flag on a HTX message when no more data are expected
The HTX_FL_EOI flag must now be set on a HTX message when no more data are
expected. Most of time, it must be set before adding the EOM block. Thus, if
there is no space for the EOM, there is still an information to know all data
were received and pushed in the HTX message. There is only an exception for the
HTTP replies (deny, return...). For these messages, the flag is set after all
blocks are pushed in the message, including the EOM block, because, on error,
we remove all inserted data.
BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
When a DNS resolution is freed, the remaining items in .ar_list and .answer_list
are also released. It must be done to avoid a memory leak. And it is the last
chance to release these objects. I've honestly no idea if there is a better
place to release them earlier. But at least, there is no more leak.
This patch should solve the issue #222. It must be backported, at least, as far
as 2.0, and probably, with caution, as far as 1.8 or 1.7.
BUG/MAJOR: dns: Make the do-resolve action thread-safe
The do-resolve HTTP action, performing a DNS resolution of a sample expression
output, is not thread-safe at all. The resolver object used to do the resolution
must be locked when the action is executed or when the stream is released
because its curr or wait resolution lists and the requester list inside a
resolution are updated. It is also important to not wake up a released stream
(with a destroyed task).
Of course, because of this bug, various kind of crashes may be observed.
This patch should fix the issue #236. It must be backported as far as 2.0.
MINOR: tasks/debug: add a BUG_ON() check to detect requeued task on free
__task_free() cannot be called with a task still in the queue. This
test adds a check which confirms there is no concurrency issue on such
a case where a thread could requeue nor wakeup a task being freed.
MINOR: tasks/debug: add a few BUG_ON() to detect use of wrong timer queue
This aims at catching calls to task_unlink_wq() performed by the wrong
thread based on the shared status for the task, as well as calls to
__task_queue() with the wrong timer queue being used based on the task's
capabilities. This will at least help eliminate some hypothesis during
debugging sessions when suspecting that a wrong thread has attempted to
queue a task at the wrong place.
MINOR: tasks/debug: make the thread affinity BUG_ON check a bit stricter
The BUG_ON() test in task_queue() only tests for the case where
we're queuing a task that doesn't run on the current thread. Let's
refine it a bit further to catch all cases where the task does not
run *exactly* on the current thread alone.
BUG/MAJOR: tasks: don't requeue global tasks into the local queue
A bug was introduced by commit 77015abe0 ("MEDIUM: tasks: clean up the
front side of the wait queue in wake_expired_tasks()"): front tasks
that are not yet expired were incorrectly requeued into the local
wait queue instead of the global one. Because of this, the same task
could be found by the same thread on next invocation and be unlinked
without locking, allowing another thread to requeue it in parallel,
and conversely another thread could unlink it while the task was being
walked over, causing all sorts of crashes and endless loops in
wake_expired_tasks() and affiliates.
This bug can easily be triggered by stressing the do_resolve action
in multi-thread (after applying the fixes required to get do_resolve
to work with threads). It certainly is the cause of issue #758.
Before commit 80b53ffb1 ("MEDIUM: arg: make make_arg_list() stop after
its own arguments"), consumers of arguments would measure the length of
the string between the first opening and closing parenthesis before
calling make_arg_list(), and this latter one would detect an empty string
early by len==0 and would not allocate an argument list.
Since that commit, this has a changed a bit because the argument parser
is now the one in charge for delimiting the argument string, so the early
test cannot be used anymore. But the argument list is still allocated,
and despite the number of arguments being returned, consumers do not
necessarily rely on it but instead they rely on the non-null arg_p
pointer that used to be allocated only if at least one argument was
present. But as it's now always allocated, the first argument always
carries the first argument's type with an empty value, which confuses
all functions that take a unique optional argument (such as uuid()).
The proper long term solution would be to always use the returned argument
count, but at least we can make sure the function always returns an empty
argument list when fed with an empty set of parenthesis, as it always used
to do. This is what this patch does.
This fix must be backported to 2.2 and fixes github issue #763. Thanks to
Luke Seelenbinder for reporting the problem.
Released version 2.3-dev1 with the following main changes :
- MINOR: config: make strict limits enabled by default
- BUG/MINOR: acl: Fix freeing of expr->smp in prune_acl_expr
- BUG/MINOR: sample: Fix freeing of conv_exprs in release_sample_expr
- BUG/MINOR: haproxy: Free proxy->format_unique_id during deinit
- BUG/MINOR: haproxy: Add missing free of server->(hostname|resolvers_id)
- BUG/MINOR: haproxy: Free proxy->unique_id_header during deinit
- BUG/MINOR: haproxy: Free srule->file during deinit
- BUG/MINOR: haproxy: Free srule->expr during deinit
- BUG/MINOR: sample: Free str.area in smp_check_const_bool
- BUG/MINOR: sample: Free str.area in smp_check_const_meth
- CLEANUP: haproxy: Free proxy_deinit_list in deinit()
- CLEANUP: haproxy: Free post_deinit_list in deinit()
- CLEANUP: haproxy: Free server_deinit_list in deinit()
- CLEANUP: haproxy: Free post_server_check_list in deinit()
- CLEANUP: Add static void vars_deinit()
- CLEANUP: Add static void hlua_deinit()
- CLEANUP: contrib/prometheus-exporter: typo fixes for ssl reuse metric
- BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
- BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
- MINOR: tcp: Support TCP keepalive parameters customization
- BUILD: tcp: condition TCP keepalive settings to platforms providing them
- MINOR: lists: rename some MT_LIST operations to clarify them
- MINOR: buffer: use MT_LIST_ADDQ() for buffer_wait lists additions
- MINOR: connection: use MT_LIST_ADDQ() to add connections to idle lists
- MINOR: tasks: use MT_LIST_ADDQ() when killing tasks.
- CONTRIB: da: fix memory leak in dummy function da_atlas_open()
- CI: travis-ci: speed up osx build by running brew scripted, switch to latest osx image
- BUG/MEDIUM: mux-h2: Don't add private connections in available connection list
- BUG/MEDIUM: mux-fcgi: Don't add private connections in available connection list
- MINOR: connection: Set the SNI on server connections before installing the mux
- MINOR: connection: Set new connection as private on reuse never
- MINOR: connection: Add a wrapper to mark a connection as private
- MEDIUM: connection: Add private connections synchronously in session server list
- MINOR: connection: Use a dedicated function to look for a session's connection
- MINOR: connection: Set the conncetion target during its initialisation
- MINOR: session: Take care to decrement idle_conns counter in session_unown_conn
- MINOR: server: Factorize code to deal with reuse of server idle connections
- MINOR: server: Factorize code to deal with connections removed from an idle list
- CLEANUP: connection: remove unused field idle_time from the connection struct
- BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
- MINOR: raw_sock: Report the number of bytes emitted using the splicing
- MINOR: contrib/prometheus-exporter: Add missing global and per-server metrics
- MINOR: backend: Add sample fetches to get the server's weight
- BUG/MINOR: mux-fcgi: Handle empty STDERR record
- BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
- BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
- BUG/MINOR: backend: fix potential null deref on srv_conn
- BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
- MEDIUM: udp: adds minimal proto udp support for message listeners.
- MEDIUM: log/sink: re-work and merge of build message API.
- MINOR: log: adds syslog udp message handler and parsing.
- MEDIUM: log: adds log forwarding section.
- MINOR: log: adds counters on received syslog messages.
- BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
- BUG/MEDIUM: server: resolve state file handle leak on reload
- BUG/MEDIUM: server: fix possibly uninitialized state file on close
- BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
- BUILD: config: address build warning on raspbian+rpi4
- BUG/MAJOR: tasks: make sure to always lock the shared wait queue if needed
- BUILD: config: fix again bugs gcc warnings on calloc
BUILD: config: fix again bugs gcc warnings on calloc
Since commit ad37c7ab ("BUILD: config: address build warning on
raspbian+rpi4") gcc 7.3.0 complains again on x86_64 (while 8.2.0
does not) :
src/cfgparse.c: In function 'check_config_validity':
src/cfgparse.c:3593:26: warning: argument 1 range [18446744071562067968, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
newsrv->idle_conns = calloc(global.nbthread, sizeof(*newsrv->idle_conns));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This thing is completely bogus (actually the RPi one was the most wrong).
Let's try to shut them both by using an unsigned short for the cast which
is expected to satisfy everyone. It's worth noting that the exact same call
a few lines above and below do not trigger this stupid warning.
This should be backported to 2.2 since the fix above was put there already.
BUG/MAJOR: tasks: make sure to always lock the shared wait queue if needed
In run_tasks_from_task_list() we may free some tasks that have been
killed. Before doing so we unlink them from the wait queue. But if such
a task is in the global wait queue, the queue isn't locked so this can
result in corrupting the global task list and causing loops or crashes.
It's very likely one cause of issue #758.
This must be backported to 2.2. For 2.1 there doesn't seem to be any
case where a task could be freed this way while in the global queue,
but it doesn't cost much to apply the same change (the code is in
process_runnable_task there).
BUILD: config: address build warning on raspbian+rpi4
Issue #747 reports that building on raspbian for rpi4 triggers this
warning:
src/cfgparse.c: In function 'check_config_validity':
src/cfgparse.c:3584:26: warning: argument 1 range [2147483648, 4294967295] exceeds maximum object size 2147483647 [-Walloc-size-larger-than=]
newsrv->idle_conns = calloc((unsigned)global.nbthread, sizeof(*newsrv->idle_conns));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It's surprising because the declared type is size_t and the argument is
unsigned (i.e. the same type on 32-bit) precisely to avoid cast issues,
but gcc seems to be too smart at this one and to issue a warning over
the valid range, implying that passing the originally required type would
also warn. Given that these are the only casts in calloc and other ones
don't complain, let's drop them.
All 3 were added by commit dc2f2753e ("MEDIUM: servers: Split the
connections into idle, safe, and available.") that went into 2.2, so
this should be backported.
BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are peeked
The CF_SHUTW_NOW flag must be handled the same way than the CF_SHUTW flag in
co_getblk_nc() and co_getline_nc() functions. It is especally important when we
try to peek a line from outgoing data. In this case, an unfinished line is
blocked an nothing is peeked if the CF_SHUTW_NOW flag is set. But the blocked
data pevent the transition to CF_SHUTW.
The above functions are only used by LUA cosockets. Because of this bug, we may
experienced wakeups in loop of the cosocket's io handler if we try to read a
line on a closed socket with a pending unfinished line (no LF found at the end).
This patch should fix issue #744. It must be backported to all supported
versions.
BUG/MEDIUM: server: fix possibly uninitialized state file on close
Previous fix dc6e8a9a7 ("BUG/MEDIUM: server: resolve state file handle
leak on reload") traded a bug for another one, now we get this warning
when building server.c, which is valid since f is not necessarily
initialized (e.g. if no global state file is passed):
src/server.c: In function 'apply_server_state':
src/server.c:3272:3: warning: 'f' may be used uninitialized in this function [-Wmaybe-uninitialized]
fclose(f);
^~~~~~~~~
Let's initialize it first. This whole code block should really be
splitted, cleaned up and reorganized as it's possible that other
similar bugs are hidden in it.
This must be backported to the same branches the commit above is
backported to (likely 2.2 and 2.1).
BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
When the loop is continued early, the memory for param_rule is not freed. This
can leak memory per request, which will eventually consume all available memory
on the server.
This patch should fix the issue #750. It must be backported as far as 2.1.
MINOR: log: adds counters on received syslog messages.
This patch adds a global counter of received syslog messages
and this one is exported on CLI "show info" as "CumRecvLogs".
This patch also updates internal conn counter and freq
of the listener and the proxy for each received log message to
prepare a further export on the "show stats".
It is possible to declare one or multiple log forwarding section,
haproxy will forward all received log messages to a log servers list.
log-forward <name>
Creates a new log forwarder proxy identified as <name>.
bind <addr> [param*]
Used to configure a log udp listener to receive messages to forward.
Only udp listeners are allowed, address must be prefixed using
'udp@', 'udp4@' or 'udp6@'. This supports for all "bind" parameters
found in 5.1 paragraph but most of them are irrelevant for udp/syslog case.
log global
log <address> [len <length>] [format <format>] [sample <ranges>:<smp_size>]
<facility> [<level> [<minlevel>]]
Used to configure target log servers. See more details on proxies
documentation.
If no format specified, haproxy tries to keep the incoming log format.
Configured facility is ignored, except if incoming message does not
present a facility but one is mandatory on the outgoing format.
If there is no timestamp available in the input format, but the field
exists in output format, haproxy will use the local date.
Example:
global
log stderr format iso local7
ring myring
description "My local buffer"
format rfc5424
maxlen 1200
size 32764
timeout connect 5s
timeout server 10s
# syslog tcp server
server mysyslogsrv 127.0.0.1:514 log-proto octet-count
log-forward sylog-loadb
bind udp4@127.0.0.1:1514
# all messages on stderr
log global
# all messages on local tcp syslog server
log ring@myring local0
# load balance messages on 4 udp syslog servers
log 127.0.0.1:10001 sample 1:4 local0
log 127.0.0.1:10002 sample 2:4 local0
log 127.0.0.1:10003 sample 3:4 local0
log 127.0.0.1:10004 sample 4:4 local0
MEDIUM: log/sink: re-work and merge of build message API.
This patch merges build message code between sink and log
and introduce a new API based on struct ist array to
prepare message header with zero copy, targeting the
log forwarding feature.
Log format 'iso' and 'timed' are now avalaible on logs line.
A new log format 'priority' is also added.
BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
A boolean was mistakenly declared 'static THREAD_LOCAL' causing
the probe of a log to a 'not sampled' log server conditionned by
the last evaluated 'sampled log' server test on the same thread.
This results to unpredictable drops of logs on 'not sampled'
log servers as soon a 'sampled' log server is declared.
This patch removes the static THREAD_LOCAL attribute from this
boolean, fixing the issue and allowing to mix 'sampled' and
'not sampled' servers.
This fix should be backported in any branches which includes
the log sampling feature.
BUG/MINOR: backend: fix potential null deref on srv_conn
Commit 08016ab82 ("MEDIUM: connection: Add private connections
synchronously in session server list") introduced a build warning about
a potential null dereference which is actually true: in case a reuse
fails an we fail to allocate a new connection, we could crash. The
issue was already present earlier but the compiler couldn't detect
it since it was guarded by an independent condition.
This should be carefully backported to older versions (at least 2.2
and maybe 2.1), the change consists in only adding a test on srv_conn.
The whole sequence of "if" blocks is ugly there and would deserve being
cleaned up so that the !srv_conn condition is matched ASAP and the
assignment is done later. This would remove complicated conditions.
BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
In fcgi_strm_handle_empty_stdout(), the FCGI_SF_ES_RCVD flag is set on "->state"
stream field instead of "->flags". It is obviously wrong. This bug is not
noticeable because the right state is set in the fcgi_process_demux() function a
bit later.
BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
When the padding of a "stream" record (STDOUT or STDERR) is skipped, we must set
the connection state to RECORD_P. It is especially important if the padding is
not fully received.
As mentionned in the FastCGI specification, FCGI "streams" are series of
non-empty stream records (length != 0), followed by an empty one. It is properly
handled for FCGI_STDOUT records, but not for FCGI_STDERR ones. If an empty
FCGI_STDERR record is received, the connection is blocked waiting for data which
will never come.
To fix the bug, when an empty FCGI_STDERR record is received, we drop it, eating
the padding if any.
This patch should fix the issue #743. It must be backported as far as 2.1.
MINOR: raw_sock: Report the number of bytes emitted using the splicing
In the continuity of the commit 7cf0e4517 ("MINOR: raw_sock: report global
traffic statistics"), we are now able to report the global number of bytes
emitted using the splicing. It can be retrieved in "show info" output on the
CLI.
Note this counter is always declared, regardless the splicing support. This
eases the integration with monitoring tools plugged on the CLI.
BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
When input data are processed, if the request is switched in tunnel mode on a
protocol upgrade, we must continue the processing. Otherwise, pending input data
will only be processed on the next wakeup. So when new input data are received,
on a timeout expiration or shutdown. Worst, if the input buffer is full when it
happens, only a timeout or a shutdown will unblock the situation.
This patch should fix the issue #737. It must be backported as far as 1.9. The
bug does not seem to affect the 2.0 and 1.9 because, on a protocol upgrade, the
request is switched in tunnel mode when the response is sent to the client. But
the bug is present, so the backport remains necessary.
MINOR: server: Factorize code to deal with connections removed from an idle list
The srv_del_conn_from_list() function is now responsible to update the server
counters and the connection flags when a connection is removed from an idle list
(safe, idle or available). It is called when a connection is released or when a
connection is set as private. This function also removes the connection from the
idle list if necessary.
MINOR: server: Factorize code to deal with reuse of server idle connections
The srv_use_idle_conn() function is now responsible to update the server
counters and the connection flags when an idle connection is reused. The same
function is called when a new connection is created. This simplifies a bit the
connect_server() function.
MINOR: connection: Set the conncetion target during its initialisation
When a new connection is created, its target is always set just after. So the
connection target may set when it is created instead, during its initialisation
to be precise. It is the purpose of this patch. Now, conn_new() function is
called with the connection target as parameter. The target is then passed to
conn_init(). It means the target must be passed when cs_new() is called. In this
case, the target is only used when the conn-stream is created with no
connection. This only happens for tcpchecks for now.
MINOR: connection: Use a dedicated function to look for a session's connection
The session_get_conn() must now be used to look for an available connection
matching a specific target for a given session. This simplifies a bit the
connect_server() function.
projects / thirdparty / haproxy.git / log
