Commit Graph

12598 Commits

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

This was introduced by commit be2774d ("MEDIUM: ssl: Added support for
Multi-Cert OCSP Stapling").

Could be backported as far as 1.7.
2020-07-31 11:51:20 +02:00
Ilya Shipitsin
6b79f38a7a CLEANUP: assorted typo fixes in the code and comments
This is 12th iteration of typo fixes
2020-07-31 11:18:07 +02:00
Willy Tarreau
f5ea3a8c58 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").
2020-07-31 09:30:12 +02:00
Willy Tarreau
2febb846a4 MINOR: mux-h1: do not even try to receive if the connection is not fully set up
If the connection is still waiting for L4/L6, there's no point even trying
to receive as it will fail, so better return zero in h1_recv_allowed().
2020-07-31 09:30:12 +02:00
Willy Tarreau
8dbd1a2e09 MINOR: connection: avoid a useless recvfrom() on outgoing connections
When a connect() doesn't immediately succeed (i.e. most of the times),
fd_cant_send() is called to enable polling. But given that we don't
mark that we cannot receive either, we end up performing a failed
recvfrom() immediately when the connect() is finally confirmed, as
indicated in issue #253.

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

Without this patch, forwarding 100k connections does this:

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

With the patch:

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

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

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

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

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

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

It could make sense to backport this patch to 2.2 and maybe 2.1 after
it has been sufficiently checked for absence of side effects in 2.3-dev,
as some people had reported an extra overhead like in issue #168.
2020-07-31 09:29:36 +02:00
Willy Tarreau
8e979fa74f 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.
2020-07-31 08:49:31 +02:00
Willy Tarreau
a3b17563e1 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.
2020-07-31 08:47:58 +02:00
Ilya Shipitsin
61d85b06dd CI: travis-ci: use proper linking flags for SLZ build
previously SSL_INC and SSL_INC were set for all builds, and SLZ lib
was linked because of those flags. After we switched SLZ build to
stock openssl lib, SSL_INC, SSL_LIB are not set anymore.

Good time to set SLZ_INC, SLZ_LIB for such builds
2020-07-31 04:46:50 +02:00
Ilya Shipitsin
d681702709 CI: travis-ci: use better name for Coverity scan job
Let's add Coverity in the job name.
2020-07-31 04:46:44 +02:00
Willy Tarreau
be789dfc5d SCRIPTS: announce-release: add the link to the wiki in the announce messages
Let's add the link to the wiki to the announce messages, plenty of
users don't even know it exists.
2020-07-30 17:41:42 +02:00
Christopher Faulet
2361fd9487 BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx
This bug was introduced by the commit 8f587ea3 ("MEDIUM: lua: Set the analyse
expiration date with smaller wake_time only"). At the end of hlua_action(), the
lua context may be null if the alloc failed.

No backport needed, this is 2.3-dev.
2020-07-30 10:40:59 +02:00
Christopher Faulet
e96993b1f2 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.
2020-07-30 09:39:20 +02:00
Christopher Faulet
b4de420472 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).
2020-07-30 09:31:09 +02:00
Christopher Faulet
3f5bcd0c96 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
8f587ea347 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
2747fbb7ac 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
54f3e183c8 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
19dbf2d625 BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval abort
When a tcp-request or a tcp-response content ruleset evaluation is aborted, the
corresponding FLT_END analyser must be preserved, if any. But because of a typo
error, on the tcp-response content ruleset evaluation, we try to preserve the
request analyser instead of the response one.

This patch must be backported to 2.2.
2020-07-30 09:31:09 +02:00
Christopher Faulet
99aaca99b5 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
498c483009 BUG/MINOR: lua: Abort execution of actions that yield on a final evaluation
A Lua action may yield. It may happen because the action returns explicitly
act.YIELD or because the script itself yield. In the first case, we must abort
the script execution if it is the final rule evaluation, i.e if the
ACT_OPT_FINAL flag is set. The second case is already covered.

This patch must be backported to 2.2.
2020-07-30 09:31:09 +02:00
Christopher Faulet
385101e538 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.
2020-07-30 09:31:09 +02:00
Christopher Faulet
08ed98fd79 MEDIUM: lua: Add support for the Lua 5.4
On Lua 5.4, some API changes make HAProxy compilation to fail. Among other
things, the lua_resume() function has changed and now takes an extra argument in
Lua 5.4 and the error LUA_ERRGCMM was removed. Thus the LUA_VERSION_NUM macro is
now tested to know the lua version is used and adapt the code accordingly.

Here are listed the incompatibilities with the previous Lua versions :

   http://www.lua.org/manual/5.4/manual.html#8

This patch comes from the HAproxy's fedora RPM, committed by Tom Callaway :

  https://src.fedoraproject.org/rpms/haproxy/blob/db970613/f/haproxy-2.2.0-lua-5.4.patch

This patch should fix the issue #730. It must be backported to 2.2 and probably
as far as 2.0.
2020-07-30 09:31:09 +02:00
Ilya Shipitsin
90ff590962 CI: travis-ci : switch to stock openssl when openssl-1.1.1 is used
instead of building openssl-1.1.1 let us use stock package and save
some electricity
2020-07-30 07:37:15 +02:00
Ilya Shipitsin
73b520b958 CI : travis-ci : prepare for using stock OpenSSL
initially SSL_LIB and SSL_INC were set globally and we assumed
that any OpenSSL variant is supposed to be built using "script/build-ssl.sh".

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

Let us make stock openssl lib first class citizen.

SSL_LIB and SSL_INC are only set when custom openssl variant
is built.
2020-07-30 07:36:52 +02:00
Jerome Magnin
4002f8dc03 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
2020-07-29 12:06:59 +02:00
Jerome Magnin
012261ab34 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.
2020-07-29 12:05:55 +02:00
Christopher Faulet
471425f51d 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.
2020-07-27 09:37:18 +02:00
Jackie Tapia
749f74c622 DOC: Use gender neutral language
This patch updates the documentation files and code comments to avoid
the use of gender specific phrasing in favor of "they" or "it".
2020-07-26 22:35:43 +02:00
Baruch Siach
e1651b2970 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).
2020-07-24 13:55:40 +02:00
Christopher Faulet
0f17a4444e 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.
2020-07-23 15:44:39 +02:00
Willy Tarreau
f1ea47d896 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.
2020-07-23 06:53:27 +02:00
Willy Tarreau
963f701f4f 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.
2020-07-22 17:09:20 +02:00
Christopher Faulet
2151cdd38c 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.
2020-07-22 16:46:37 +02:00
Christopher Faulet
9e3dc8305b 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.
2020-07-22 16:43:32 +02:00
Christopher Faulet
810df06145 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.
2020-07-22 16:43:32 +02:00
Christopher Faulet
010ab35a91 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.
2020-07-22 16:43:07 +02:00
Christopher Faulet
5098a08c2f 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.
2020-07-22 14:59:22 +02:00
Willy Tarreau
f2452b3c70 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.
2020-07-22 14:42:52 +02:00
Willy Tarreau
e5d79bccc0 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.
2020-07-22 14:42:52 +02:00
Willy Tarreau
2447bce554 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.
2020-07-22 14:22:38 +02:00
Willy Tarreau
783afbe93b 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.

This must be backported to 2.2 only.
2020-07-22 14:12:45 +02:00
Emeric Brun
d3db3846c5 BUG/MEDIUM: resolve: fix init resolving for ring and peers section.
Reported github issue #759 shows there is no name resolving
on server lines for ring and peers sections.

This patch introduce the resolving for those lines.

This patch adds  boolean a parameter to parse_server function to specify
if we want the function to perform an initial name resolving using libc.

This boolean is forced to true in case of peers or ring section.

The boolean is kept to false in case of classic servers (from
backend/listen)

This patch should be backported in branches where peers sections
support 'server' lines.
2020-07-21 17:59:20 +02:00
Willy Tarreau
75fd2ff83a BUG/MEDIUM: arg: empty args list must be dropped
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.
2020-07-21 15:53:54 +02:00
Lukas Tribus
a267b5df4a DOC: ssl: req_ssl_sni needs implicit TLS
req_ssl_sni is not compatible with protocols negotiating TLS
explicitly, like SMTP on port 25 or 587 and IMAP on port 143.

Fix an example referring to 587 (SMTPS port with implicit TLS
is 465) and amend the req_ssl_sni documentation.

This doc fix should be backported to supported versions.
2020-07-19 06:36:12 +02:00
Willy Tarreau
e732cbd8a9 [RELEASE] Released version 2.3-dev1
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
2020-07-17 15:13:19 +02:00