Commit graph

8156 commits

Author SHA1 Message Date
Willy Tarreau
83b5890b47 MINOR: http/htx: use conn_get_dst() to retrieve the destination address
When adding the X-Original-To header, let's use conn_get_dst() and make
sure it succeeds, since  previous call to conn_get_to_addr() was unchecked.
2019-07-19 13:50:09 +02:00
Willy Tarreau
8fa9984a17 MINOR: log: use conn_get_{dst,src}() to retrieve the cli/frt/bck/srv/ addresses
This also allows us to check that the operation succeeded without
logging whatever remained in the memory area in case of failure.
2019-07-19 13:50:09 +02:00
Willy Tarreau
8dfffdb060 MINOR: stream/cli: use conn_get_{src,dst} in "show sess" and "show peers" output
The stream outputs requires to retrieve connections sources and
destinations. The previous call involving conn_get_{to,from}_addr()
was missing a status check which has now been integrated with the
new call since these places already handle connection errors there.

The same code parts were reused for "show peers" and were modified
similarly.
2019-07-19 13:50:09 +02:00
Willy Tarreau
7bb447c3dd MINOR: stream-int: use conn_get_{src,dst} in conn_si_send_proxy()
These ones replace the previous conn_get_{from,to}_addr() used to wait
for the connection establishment before sending a LOCAL line. The
error handling was preserved.
2019-07-19 13:50:09 +02:00
Willy Tarreau
dddd2b422f MINOR: tcp: replace various calls to conn_get_{from,to}_addr with conn_get_{src,dst}
These calls include the operation's status. When the check was already
present, it was merged with the call. when it was not present, it was
added.
2019-07-19 13:50:09 +02:00
Willy Tarreau
f5bdb64d35 MINOR: ssl: switch to conn_get_dst() to retrieve the destination address
This replaces conn_get_to_addr() and the subsequent check.
2019-07-19 13:50:09 +02:00
Willy Tarreau
3cc01d84b3 MINOR: backend: switch to conn_get_{src,dst}() for port and address mapping
The backend connect code uses conn_get_{from,to}_addr to forward addresses
in transparent mode and to map server ports, without really checking if the
operation succeeds. In preparation of future changes, let's switch to
conn_get_{src,dst}() and integrate status check for possible failures.
2019-07-19 13:50:09 +02:00
Willy Tarreau
a0a4b09d08 MINOR: frontend: switch to conn_get_{src,dst}() for logging and debugging
The frontend accept code uses conn_get_{from,to}_addr for logging and
debugging, without really checking if the operation succeeds. In
preparation of future changes, let's switch to conn_get_{src,dst}() and
integrate status check for possible failures.
2019-07-19 13:50:09 +02:00
Christopher Faulet
03627245c6 BUG/MEDIUM: mux-h1: Trim excess server data at the end of a transaction
At the end of a transaction, when the conn_stream is detach from the H1
connection, on the server side, we must release the input buffer to trim any
excess data received from the server to be sure to block invalid responses.  A
typical example of such data would be from a buggy server responding to a HEAD
with some data, or sending more than the advertised content-length.

This issue was reported on Gitbub. See issue #176.

This patch must be backported to 2.0 and 1.9.
2019-07-19 11:39:19 +02:00
Christopher Faulet
f89f0991f6 MINOR: config: Warn only if the option http-use-htx is used with "no" prefix
No warning message is emitted anymore if the option is used to enable the
HTX. But it is still diplayed when the "no" prefix is used to disable the HTX
explicitly. So, for existing configs, we display a warning only if there is a
change in the behavior of HAProxy between the 2.1 and the previous versions.
2019-07-19 11:39:19 +02:00
Willy Tarreau
2ab5c38359 BUG/MINOR: checks: do not exit tcp-checks from the middle of the loop
There's a comment above tcpcheck_main() clearly stating that no return
statement should be placed in the middle, still we did have one after
installing the mux. It looks mostly harmless though as it will only
fail to mark the server as being in error in case of allocation failure
or config issue.

This fix should be backported to 2.0 and probably 1.9 as well.
2019-07-19 11:03:54 +02:00
Christopher Faulet
4da05478e3 CLEANUP: mux-h2: Remove unused flags H2_SF_CHNK_*
Since the legacy HTTP code was removed, these flags are unused anymore.
2019-07-19 09:46:23 +02:00
Christopher Faulet
39566d1892 BUG/MINOR: session: Send a default HTTP error if accept fails for a H1 socket
If session_accept_fd() fails for a raw HTTP socket, we try to send an HTTP error
500. But we must not rely on error messages of the proxy or on the array
http_err_chunks because these are HTX messages. And it should be too expensive
to convert an HTX message to a raw message at this place. So instead, we send a
default HTTP error message from the array http_err_msgs.

This patch must be backported to 2.0 and 1.9.
2019-07-19 09:46:23 +02:00
Christopher Faulet
76f4c370f1 BUG/MINOR: session: Emit an HTTP error if accept fails only for H1 connection
If session_accept_fd() fails for a raw HTTP socket, we try to send an HTTP error
500. But, we must also take care it is an HTTP/1 connection. We cannot rely on
the mux at this stage, because the error, if any, happens before or during its
creation. So, instead, we check if the mux_proto is specified or not. Indeed,
the mux h1 cannot be forced on the bind line and there is no ALPN to choose
another mux on a raw socket. So if there is no mux_proto defined for a raw HTTP
socket, we are sure to have an HTTP/1 connection.

This patch must be backported to 2.0 and 1.9.
2019-07-19 09:46:23 +02:00
Christopher Faulet
f734638976 MINOR: http: Don't store raw HTTP errors in chunks anymore
Default HTTP error messages are stored in an array of chunks. And since the HTX
was added, these messages are also converted in HTX and stored in another
array. But now, the first array is not used anymore because the legacy HTTP mode
was removed.

So now, only the array with the HTX messages are kept. The other one was
removed.
2019-07-19 09:46:23 +02:00
Christopher Faulet
41ba36f8b2 MINOR: global: Preset tune.max_http_hdr to its default value
By default, this tune parameter is set to MAX_HTTP_HDR. This assignment is done
after the configuration parsing, when we check the configuration validity. So
during the configuration parsing, its value is 0. Now, it is set to MAX_HTTP_HDR
from the start. So, it is possible to rely on it during the configuration
parsing.
2019-07-19 09:46:23 +02:00
Christopher Faulet
1b6adb4a51 MINOR: proxy/http_ana: Remove unused req_exp/rsp_exp and req_add/rsp_add lists
The keywords req* and rsp* are now unsupported. So the corresponding lists are
now unused. It is safe to remove them from the structure proxy.

As a result, the code dealing with these rules in HTTP analyzers was also
removed.
2019-07-19 09:24:12 +02:00
Christopher Faulet
8c3b63ae1d MINOR: proxy: Remove the unused list of block rules
The keyword "block" is now unsupported. So the list of block rules is now
unused. It can be safely removed from the structure proxy.
2019-07-19 09:24:12 +02:00
Christopher Faulet
a6a56e6483 MEDIUM: config: Remove parsing of req* and rsp* directives
It was announced for the 2.1. Following keywords are now unsupported:

  * reqadd, reqallow, reqiallow, reqdel, reqidel, reqdeny, reqideny, reqpass,
    reqipass, reqrep, reqirep reqtarpit, reqitarpit

  * rspadd, rspdel, rspidel, rspdeny, rspideny, rsprep, rspirep

a fatal error is emitted if one of these keyword is found during the
configuraion parsing.
2019-07-19 09:24:12 +02:00
Christopher Faulet
73e8ede156 MINOR: proxy: Remove support of the option 'http-tunnel'
The option 'http-tunnel' is deprecated and it was only used in the legacy HTTP
mode. So this option is now totally ignored and a warning is emitted during
HAProxy startup if it is found in a configuration file.
2019-07-19 09:24:12 +02:00
Christopher Faulet
fc9cfe4006 REORG: proto_htx: Move HTX analyzers & co to http_ana.{c,h} files
The old module proto_http does not exist anymore. All code dedicated to the HTTP
analysis is now grouped in the file proto_htx.c. So, to finish the polishing
after removing the legacy HTTP code, proto_htx.{c,h} files have been moved in
http_ana.{c,h} files.

In addition, all HTX analyzers and related functions prefixed with "htx_" have
been renamed to start with "http_" instead.
2019-07-19 09:24:12 +02:00
Christopher Faulet
a8a46e2041 CLEANUP: proto_http: Move remaining code from proto_http.c to proto_htx.c 2019-07-19 09:24:12 +02:00
Christopher Faulet
eb2754bef8 CLEANUP: proto_http: Remove unecessary includes and comments 2019-07-19 09:24:12 +02:00
Christopher Faulet
22dc248c2a CLEANUP: channel: Remove the unused flag CF_WAKE_CONNECT
This flag is tested or cleared but never set anymore.
2019-07-19 09:24:12 +02:00
Christopher Faulet
cc76d5b9a1 MINOR: proto_http: Remove the unused flag HTTP_MSGF_WAIT_CONN
This flag is set but never used. So remove it.
2019-07-19 09:24:12 +02:00
Christopher Faulet
c41547b66e MINOR: proto_http: Remove unused http txn flags
Many flags of the HTTP transction (TX_*) are now unused and useless. So the
flags TX_WAIT_CLEANUP, TX_HDR_CONN_*, TX_CON_CLO_SET and TX_CON_KAL_SET were
removed. Most of TX_CON_WANT_* were also removed. Only TX_CON_WANT_TUN has been
kept.
2019-07-19 09:24:12 +02:00
Christopher Faulet
67bb3bb0c2 MINOR: hlua: Remove useless test on TX_CON_WANT_* flags
When an HTTP applet is initialized, it is useless to force server-close mode on
the HTTP transaction because the connection mode is now handled by muxes. In
HTX, during analysis, the flag TX_CON_WANT_CLO is set by default in
htx_wait_for_request(), and TX_CON_WANT_SCL is never tested anywere.
2019-07-19 09:24:12 +02:00
Christopher Faulet
711ed6ae4a MAJOR: http: Remove the HTTP legacy code
First of all, all legacy HTTP analyzers and all functions exclusively used by
them were removed. So the most of the functions in proto_http.{c,h} were
removed. Only functions to deal with the HTTP transaction have been kept. Then,
http_msg and hdr_idx modules were entirely removed. And finally the structure
http_msg was lightened of all its useless information about the legacy HTTP. The
structure hdr_ctx was also removed because unused now, just like unused states
in the enum h1_state. Note that the memory pool "hdr_idx" was removed and
"http_txn" is now smaller.
2019-07-19 09:24:12 +02:00
Christopher Faulet
bcac786b36 MINOR: stream: Remove code relying on the legacy HTTP mode
Dump of streams information was updated to remove useless info. And it is not
necessary anymore to update msg->sov..
2019-07-19 09:18:27 +02:00
Christopher Faulet
3d11969a91 MAJOR: filters: Remove code relying on the legacy HTTP mode
This commit breaks the compatibility with filters still relying on the legacy
HTTP code. The legacy callbacks were removed (http_data, http_chunk_trailers and
http_forward_data).

For now, the filters must still set the flag FLT_CFG_FL_HTX to be used on HTX
streams.
2019-07-19 09:18:27 +02:00
Christopher Faulet
b7f8890b19 MINOR: stats: Remove code relying on the legacy HTTP mode
The part of the applet dealing with raw buffer was removed, for the HTTP part
only. So the old functions stats_send_http_headers() and
stats_send_http_redirect() were removed and replaced by the htx ones. The legacy
applet I/O handler was replaced by the htx one. And the parsing of POST data was
purged of the legacy HTTP code.
2019-07-19 09:18:27 +02:00
Christopher Faulet
386a0cda23 MINOR: flt_trace: Remove code relying on the legacy HTTP mode
The legacy HTTP callbacks were removed (trace_http_data,
trace_http_chunk_trailers and trace_http_forward_data). And the loop on the HTTP
headers was updated to only handle HTX messages.
2019-07-19 09:18:27 +02:00
Christopher Faulet
89f2b16530 MEDIUM: compression: Remove code relying on the legacy HTTP mode
The legacy HTTP callbacks were removed (comp_http_data, comp_http_chunk_trailers
and comp_http_forward_data). Functions emitting compressed chunks of data for
the legacy HTTP mode were also removed. The state for the compression filter was
updated accordingly. The compression context and the algorigttm used to compress
data are the only useful information remaining.
2019-07-19 09:18:27 +02:00
Christopher Faulet
95e7ea3c62 MEDIUM: cache: Remove code relying on the legacy HTTP mode
The applet delivering cached objects based on the legacy HTTP code was removed
as the filter callback cache_store_http_forward_data(). And the action analyzing
the response coming from the server to store it in the cache or not was purged
of the legacy HTTP code.
2019-07-19 09:18:27 +02:00
Christopher Faulet
12c28b6579 MINOR: http_act: Remove code relying on the legacy HTTP mode
Actions updating the request or the response start-line are concerned.
2019-07-19 09:18:27 +02:00
Christopher Faulet
a209796c80 MEDIUM: hlua: Remove code relying on the legacy HTTP mode
HTTP applets are concerned and functions of the HTTP class too.
2019-07-19 09:18:27 +02:00
Christopher Faulet
7d37fbb753 MEDIUM: backend: Remove code relying on the HTTP legacy mode
The L7 loadbalancing algorithms are concerned (uri, url_param and hdr), the
"sni" parameter on the server line and the "source" parameter on the server line
when used with "use_src hdr_ip()".
2019-07-19 09:18:27 +02:00
Christopher Faulet
4cb2828e96 MINOR: proxy: Don't adjust connection mode of HTTP proxies anymore
This was only used for the legacy HTTP mode where the connection mode was
handled by the HTTP analyzers. In HTX, the function http_adjust_conn_mode() does
nothing. The connection mode is handled by the muxes.
2019-07-19 09:18:27 +02:00
Christopher Faulet
28b18c5e21 CLEANUP: proxy: Remove the flag PR_O2_USE_HTX
This flag is now unused. So we can safely remove it.
2019-07-19 09:18:27 +02:00
Christopher Faulet
8f7fe1c9d7 MINOR: cache: Remove tests on the option 'http-use-htx'
All cache filters now store HTX messages. So it is useless to test if a cache is
used at the same time by a legacy HTTP proxy and an HTX one.
2019-07-19 09:18:27 +02:00
Christopher Faulet
280f85b153 MINOR: hlua: Remove tests on the option 'http-use-htx' to reject TCP applets
TCP applets are now forbidden for all HTTP proxies because all of them use the
HTX mode. So we don't rely anymore on the flag PR_O2_USE_HTX to do so.
2019-07-19 09:18:27 +02:00
Christopher Faulet
60d29b37b2 MINOR: proxy: Remove tests on the option 'http-use-htx' during H1 upgrade
To know if an upgrade from TCP to H1 must be performed, we now only need to know
if a non HTX stream is assigned to an HTTP backend. So we don't rely anymore on
the flag PR_O2_USE_HTX to handle such upgrades.
2019-07-19 09:18:27 +02:00
Christopher Faulet
3494c63770 MINOR: stream: Remove tests on the option 'http-use-htx' in stream_new()
All streams created for an HTTP proxy must now use the HTX internal
resprentation. So, it is no more necessary to test the flag PR_O2_USE_HTX. It
means a stream is an HTX stream if the frontend is an HTTP proxy or if the
frontend multiplexer, if any, set the flag MX_FL_HTX.
2019-07-19 09:18:27 +02:00
Christopher Faulet
0d79c67103 MINOR: config: Remove tests on the option 'http-use-htx'
All proxies have now the option PR_O2_USE_HTX set. So it is useless to still
test it when the validity of the configuratio is checked.
2019-07-19 09:18:27 +02:00
Christopher Faulet
6d1dd46917 MEDIUM: http_fetch: Remove code relying on HTTP legacy mode
Since the legacy HTTP mode is disbabled, all HTTP sample fetches work on HTX
streams. So it is safe to remove all code relying on HTTP legacy mode. Among
other things, the function smp_prefetch_http() was removed with the associated
macros CHECK_HTTP_MESSAGE_FIRST() and CHECK_HTTP_MESSAGE_FIRST_PERM().
2019-07-19 09:18:27 +02:00
Christopher Faulet
9a7e8ce4eb MINOR: stream: Rely on HTX analyzers instead of legacy HTTP ones
Since the legacy HTTP mode is disabled, old HTTP analyzers do nothing but call
those of the HTX. So, it is safe to directly call HTX analyzers from
process_stream().
2019-07-19 09:18:27 +02:00
Christopher Faulet
c985f6c5d8 MINOR: connection: Remove the multiplexer protocol PROTO_MODE_HTX
Since the legacy HTTP mode is disabled and no multiplexer relies on it anymore,
there is no reason to have 2 multiplexer protocols for the HTTP. So the protocol
PROTO_MODE_HTX was removed and all HTTP multiplexers use now PROTO_MODE_HTTP.
2019-07-19 09:18:27 +02:00
Christopher Faulet
5ed8353dcf CLEANUP: h2: Remove functions converting h2 requests to raw HTTP/1.1 ones
Because the h2 multiplexer only uses the HTX mode, following H2 functions were
removed :

  * h2_prepare_h1_reqline
  * h2_make_h1_request()
  * h2_make_h1_trailers()
2019-07-19 09:18:27 +02:00
Christopher Faulet
9b79a1025d MEDIUM: mux-h2: Remove support of the legacy HTTP mode
Now the H2 multiplexer only works in HTX. Code relying on the legacy HTTP mode
was removed.
2019-07-19 09:18:27 +02:00
Christopher Faulet
319303739a MAJOR: http: Deprecate and ignore the option "http-use-htx"
From this commit, the legacy HTTP mode is now definitely disabled. It is the
first commit of a long series to remove the legacy HTTP code. Now, all HTTP
processing is done using the HTX internal representation. Since the version 2.0,
It is the default mode. So now, it is no more possible to disable the HTX to
fallback on the legacy HTTP mode. If you still use "[no] option http-use-htx", a
warning will be emitted during HAProxy startup. Note the passthough multiplexer
is now only usable for TCP proxies.
2019-07-19 09:18:27 +02:00
Christopher Faulet
2bf43f0746 MINOR: htx: Use an array of char to store HTX blocks
Instead of using a array of (struct block), it is more natural and intuitive to
use an array of char. Indeed, not only (struct block) are stored in this array,
but also their payload.
2019-07-19 09:18:27 +02:00
Christopher Faulet
192c6a23d4 MINOR: htx: Deduce the number of used blocks from tail and head values
<head> and <tail> fields are now signed 32-bits integers. For an empty HTX
message, these fields are set to -1. So the field <used> is now useless and can
safely be removed. To know if an HTX message is empty or not, we just compare
<head> against -1 (it also works with <tail>). The function htx_nbblks() has
been added to get the number of used blocks.
2019-07-19 09:18:27 +02:00
Christopher Faulet
5a916f7326 CLEANUP: htx: Remove the unsued function htx_add_blk_type_size() 2019-07-19 09:18:27 +02:00
Christopher Faulet
3b21972061 DOC: htx: Update comments in HTX files
This patch may be backported to 2.0 to have accurate comments.
2019-07-19 09:18:27 +02:00
Christopher Faulet
c63231df55 MINOR: proto_htx: Don't stop forwarding when there is a post-connect processing
The TXN flag HTTP_MSGF_WAIT_CONN is now ignored on HTX streams. There is no
reason to not start to forward data in HTX. This is required for the legacy mode
and this was copied from it during the HTX development. But it is simply
useless.
2019-07-19 09:18:27 +02:00
Christopher Faulet
b5f86f116b MINOR: backend/htx: Don't rewind output data to set the sni on a srv connection
Rewind on output data is useless for HTX streams.
2019-07-19 09:18:27 +02:00
Christopher Faulet
304cc40536 MINOR: proto_htx: Add the function htx_return_srv_error()
Instead of using a function from the legacy HTTP, the HTX code now uses its own
one.
2019-07-19 09:18:27 +02:00
Christopher Faulet
00618aadf9 MINOR: proto_htx: Rely on the HTX function to apply a redirect rules
There is no reason to use the legacy HTTP version here, which falls back on the
HTX version in this case.
2019-07-19 09:18:27 +02:00
Christopher Faulet
75b4cd967d MINOR: proto_htx: Directly call htx_check_response_for_cacheability()
Instead of using the HTTP legacy version.
2019-07-19 09:18:27 +02:00
Christopher Faulet
4d0e263079 BUG/MINOR: hlua: Make the function txn:done() HTX aware
The function hlua_txn_done() still relying, for the HTTP, on the legacy HTTP
mode. Now, for HTX streams, it calls the function htx_reply_and_close().

This patch must be backported to 2.0 and 1.9.
2019-07-19 09:18:27 +02:00
Christopher Faulet
5f2c49f5ee BUG/MINOR: cache/htx: Make maxage calculation HTX aware
The function http_calc_maxage() was not updated to be HTX aware. So the header
"Cache-Control" on the response was never parsed to find "max-age" or "s-maxage"
values.

This patch must be backported to 2.0 and 1.9.
2019-07-19 09:18:27 +02:00
Christopher Faulet
7b889cb387 BUG/MINOR: http_htx: Initialize HTX error messages for TCP proxies
Since the HTX is the default mode for all proxies, HTTP and TCP, we must
initialize all HTX error messages for all HTX-aware proxies and not only for
HTTP ones. It is required to support HTTP upgrade for TCP proxies.

This patch must be backported to 2.0.
2019-07-19 09:18:27 +02:00
Christopher Faulet
cd76195061 BUG/MINOR: http_fetch: Fix http_auth/http_auth_group when called from TCP rules
These sample fetches rely on the static fnuction get_http_auth(). For HTX
streams and TCP proxies, this last one gets its HTX message from the request's
channel. When called from an HTTP rule, There is no problem. Bu when called from
TCP rules for a TCP proxy, this buffer is a raw buffer not an HTX message. For
instance, using the following TCP rule leads to a crash :

  tcp-request content accept if { http_auth(Users) }

To fix the bug, we must rely on the HTX message returned by the function
smp_prefetch_htx(). So now, the HTX message is passed as argument to the
function get_http_auth().

This patch must be backported to 2.0 and 1.9.
2019-07-19 09:18:27 +02:00
Christopher Faulet
6d36e1c282 MINOR: mux-h2: Don't adjust anymore the amount of data sent in h2_snd_buf()
Because the infinite forward is HTX aware, it is useless to tinker with the
number of bytes really sent. This was fixed long ago for the H1 and forgotten to
do so for the H2.
2019-07-19 09:18:27 +02:00
Willy Tarreau
09e0203ef4 BUG/MINOR: backend: do not try to install a mux when the connection failed
If si_connect() failed, do not try to install the mux nor to complete
the operations or add the connection to an idle list, and abort quickly
instead. No obvious side effects were identified, but continuing to
allocate some resources after something has already failed seems risky.

This was a result of a prior fix which already wanted to push this code
further : aa089d80b ("BUG/MEDIUM: server: Defer the mux init until after
xprt has been initialized.") but it ought to have pushed it even further
to maintain the error check just after si_connect().

To be backported to 2.0 and 1.9.
2019-07-18 16:49:11 +02:00
Willy Tarreau
69564b1c49 BUG/MEDIUM: http/htx: unbreak option http_proxy
The temporary connection used to hold the target connection's address
was missing a valid target, resulting in a 500 server error being
reported when trying to connect to a remote host. Strangely this
issue was introduced as a side effect of commit 2c52a2b9e ("MEDIUM:
connection: make mux->detach() release the connection") which at
first glance looks unrelated but solidly stops the bisection (note
that by default this part even crashes). It's suspected that the
error only happens when closing and destroys pending data in fact.

Given that this feature was broken very early during 1.8-rc1 development
it doesn't seem to be used often. This must be backported as far as 1.8.
2019-07-18 16:49:11 +02:00
Olivier Houchard
0ba6c85a0b BUG/MEDIUM: checks: Don't attempt to receive data if we already subscribed.
tcpcheck_main() might be called while we already attempted to subscribe, and
failed. There's no point in trying to call rcv_buf() again, and failing
would lead to us trying to subscribe again, which is not allowed.

This should be backported to 2.0 and 1.9.
2019-07-18 16:42:45 +02:00
Willy Tarreau
8280ea97a0 MINOR: applet: make appctx use their own pool
A long time ago, applets were seen as an alternative to connections,
and since their respective sizes were roughly equal it appeared wise
to share the same pool. Nowadays, connections got significantly larger
but applets are not that often used, except for the cache. However
applets are mostly complementary and not alternatives anymore, as
it's very possible not to have a back connection or to share one with
other streams.

The connections will soon lose their addresses and their size will
shrink so much that appctx won't fit anymore. Given that the old
benefits of sharing these pools have long disappeared, let's stop
doing this and have a dedicated pool for appctx.
2019-07-18 10:45:08 +02:00
Willy Tarreau
45726fd458 BUG/MINOR: dns: remove irrelevant dependency on a client connection
The do-resolve action tests for a client connection to the stream and
tries to get the client's address, otherwise it refrains from performing
the resolution. This really makes no sense at all and looks like an
earlier attempt at resolving the client's address to test that the
code was working. Further, it prevents the action from being used
from other places such as an autonomous applet for example, even if
at the moment this use case does not exist.

This patch simply removes the irrelevant test.

This can be backported to 2.0.
2019-07-17 14:11:57 +02:00
Willy Tarreau
7764a57d32 BUG/MEDIUM: threads: cpu-map designating a single thread/process are ignored
Since commit 81492c989 ("MINOR: threads: flatten the per-thread cpu-map"),
we don't keep the proc*thread matrix anymore to represent the full binding
possibilities, but only the proc and thread ones. The problem is that the
per-process binding is not the same for each thread and for the process,
and the proc[] array was assumed to store the per-proc first thread value
when doing this change. Worse, the logic present there tries to deal with
thread ranges and process ranges in a way which automatically exclused the
other possibility (since ranges cannot be used on both) but as such fails
to apply changes if neither the process nor the thread is expressed as a
range.

The real problem comes from the fact that specifying cpu-map 1/1 doesn't
yet reveal if the per-process mask or the per-thread mask needs to be
updated. In practice it's the thread one but then the current storage
doesn't allow to store the binding of the first thread of each other
process in nbproc>1 configurations.

When removing the proc*thread matrix, what ought to have been kept was
both the thread column for process 1 and the process line for threads 1,
but instead only the thread column was kept. This patch reintroduces the
storage of the configuration for the first thread of each process so that
it is again possible to store either the per-thread or per-process
configuration.

As a partial workaround for existing configurations, it is possible to
systematically indicate at least two processes or two threads at once
and map them by pairs or more so that at least two values are present
in the range. E.g :

  # set processes 1-4 to cpus 0-3 :

     cpu-map auto:1-4/1 0 1 2 3
  # or:
     cpu-map 1-2/1 0 1
     cpu-map 2-3/1 2 3

  # set threads 1-4 to cpus 0-3 :

     cpu-map auto:1/1-4 0 1 2 3
  # or :
     cpu-map 1/1-2 0 1
     cpu-map 3/3-4 2 3

This fix must be backported to 2.0.
2019-07-16 15:23:09 +02:00
Andrew Heberle
9723696759 MEDIUM: mworker-prog: Add user/group options to program section
This patch adds "user" and "group" config options to the "program"
section so the configured command can be run as a different user.
2019-07-15 16:43:16 +02:00
Willy Tarreau
7df8ca6296 BUG/MEDIUM: tcp-check: unbreak multiple connect rules again
The last connect rule used to be ignored and that was fixed by commit
248f1173f ("BUG/MEDIUM: tcp-check: single connect rule can't detect
DOWN servers") during 1.9 development. However this patch went a bit
too far by not breaking out of the loop after a pending connect(),
resulting in a series of failed connect() to be quickly skipped and
only the last one to be taken into account.

Technically speaking the series is not exactly skipped, it's just that
TCP checks suffer from a design issue which is that there is no
distinction between a new rule and this rule's completion in the
"connect" rule handling code. As such, when evaluating TCPCHK_ACT_CONNECT
a new connection is created regardless of any previous connection in
progress, and the previous result is ignored. It seems that this issue
is mostly specific to the connect action if we refer to the comments at
the top of the function, so it might be possible to durably address it
by reworking the connect state.

For now this patch does something simpler, it restores the behaviour
before the commit above consisting in breaking out of the loop when
the connection is in progress and after skipping comment rules. This
way we fall back to the default code waiting for completion.

This patch must be backported as far as 1.8 since the commit above
was backported there. Thanks to Jrme Magnin for reporting and
bisecting this issue.
2019-07-15 11:10:36 +02:00
Willy Tarreau
9cca8dfc0b BUG/MINOR: mux-pt: do not pretend there's more data after a read0
Commit 8706c8131 ("BUG/MEDIUM: mux_pt: Always set CS_FL_RCV_MORE.")
was a bit excessive in setting this flag, it refrained from removing
it after read0 unless it was on an empty call. The problem it causes
is that read0 is thus ignored on the first call :

  $ strace -tts200 -e trace=recvfrom,epoll_wait,sendto  ./haproxy -db -f tcp.cfg
  06:34:23.956897 recvfrom(9, "blah\n", 15360, 0, NULL, NULL) = 5
  06:34:23.956938 recvfrom(9, "", 15355, 0, NULL, NULL) = 0
  06:34:23.956958 recvfrom(9, "", 15355, 0, NULL, NULL) = 0
  06:34:23.957033 sendto(8, "blah\n", 5, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 5
  06:34:23.957229 epoll_wait(3, [{EPOLLIN|EPOLLHUP|EPOLLRDHUP, {u32=8, u64=8}}], 200, 0) = 1
  06:34:23.957297 recvfrom(8, "", 15360, 0, NULL, NULL) = 0

If CO_FL_SOCK_RD_SH is reported by the transport layer, it indicates the
read0 was already seen thus we must not try again and we must immedaitely
report it. The simple fix consists in removing the test on ret==0 :

  $ strace -tts200 -e trace=recvfrom,epoll_wait,sendto  ./haproxy -db -f tcp.cfg
  06:44:21.634835 recvfrom(9, "blah\n", 15360, 0, NULL, NULL) = 5
  06:44:21.635020 recvfrom(9, "", 15355, 0, NULL, NULL) = 0
  06:44:21.635056 sendto(8, "blah\n", 5, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 5
  06:44:21.635269 epoll_wait(3, [{EPOLLIN|EPOLLHUP|EPOLLRDHUP, {u32=8, u64=8}}], 200, 0) = 1
  06:44:21.635330 recvfrom(8, "", 15360, 0, NULL, NULL) = 0

The issue is minor, it only results in extra syscalls and CPU usage.
This fix should be backported to 2.0 and 1.9.
2019-07-15 06:47:54 +02:00
Olivier Houchard
4bd5867627 BUG/MEDIUM: streams: Don't redispatch with L7 retries if redispatch isn't set.
Move the logic to decide if we redispatch to a new server from
sess_update_st_cer() to a new inline function, stream_choose_redispatch(), and
use it in do_l7_retry() instead of just setting the state to SI_ST_REQ.
That way, when using L7 retries, we won't redispatch the request to another
server except if "option redispatch" is used.

This should be backported to 2.0.
2019-07-12 16:17:50 +02:00
Olivier Houchard
29cac3c5f7 BUG/MEDIUM: streams: Don't give up if we couldn't send the request.
In htx_request_forward_body(), don't give up if we failed to send the request,
and we have L7 retries activated. If we do, we will not retry when we should.

This should be backported to 2.0.
2019-07-12 16:17:50 +02:00
Dave Pirotte
234740f65d BUG/MINOR: mux-h1: Correctly report Ti timer when HTX and keepalives are used
When HTTP keepalives are used in conjunction with HTX, the Ti timer
reports the elapsed time since the beginning of the connection instead
of the end of the previous request as stated in the documentation. Th,
Tq and Tt also report incorrectly as a result.

When creating a new h1s, check if it is the first request on the
connection. If not, set the session create times to the current
timestamp rather than the initial session accept timestamp. This makes
the logged timers behave as stated in the documentation.

This fix should be backported to 1.9 and 2.0.
2019-07-12 16:14:12 +02:00
Christopher Faulet
37243bc61f BUG/MEDIUM: mux-h1: Don't release h1 connection if there is still data to send
When the h1 stream (h1s) is detached, If the connection is not really shutdown
yet and if there is still some data to send, the h1 connection (h1c) must not be
released. Otherwise, the remaining data are lost. This bug was introduced by the
commit 3ac0f430 ("BUG/MEDIUM: mux-h1: Always release H1C if a shutdown for
writes was reported").

Here is the conditions to release an h1 connection when the h1 stream is
detached :

  * An error or a shutdown write occurred on the connection
    (CO_FL_ERROR|CO_FL_SOCK_WR_SH)

  * an error, an h2 upgrade or full shutdown occurred on the h1 connection
    (H1C_F_CS_ERROR||H1C_F_UPG_H2C|H1C_F_CS_SHUTDOWN)

  * A shutdown write is pending on the h1 connection and there is no more data
    in the output buffer
    ((h1c->flags & H1C_F_CS_SHUTW_NOW) && !b_data(&h1c->obuf))

If one of these conditions is fulfilled, the h1 connection is
released. Otherwise, the release is delayed. If we are waiting to send remaining
data, a timeout is set.

This patch must be backported to 2.0 and 1.9. It fixes the issue #164.
2019-07-12 10:06:41 +02:00
Willy Tarreau
f2cb169487 BUG/MAJOR: listener: fix thread safety in resume_listener()
resume_listener() can be called from a thread not part of the listener's
mask after a curr_conn has gone lower than a proxy's or the process' limit.
This results in fd_may_recv() being called unlocked if the listener is
bound to only one thread, and quickly locks up.

This patch solves this by creating a per-thread work_list dedicated to
listeners, and modifying resume_listener() so that it bounces the listener
to one of its owning thread's work_list and waking it up. This thread will
then call resume_listener() again and will perform the operation on the
file descriptor itself. It is important to do it this way so that the
listener's state cannot be modified while the listener is being moved,
otherwise multiple threads can take conflicting decisions and the listener
could be put back into the global queue if the listener was used at the
same time.

It seems like a slightly simpler approach would be possible if the locked
list API would provide the ability to return a locked element. In this
case the listener would be immediately requeued in dequeue_all_listeners()
without having to go through resume_listener() with its associated lock.

This fix must be backported to all versions having the lock-less accept
loop, which is as far as 1.8 since deadlock fixes involving this feature
had to be backported there. It is expected that the code should not differ
too much there. However, previous commit "MINOR: task: introduce work lists"
will be needed as well and should not present difficulties either. For 1.8,
the commits introducing thread_mask() and LIST_ADDED() will be needed as
well, either backporting my_flsl() or switching to my_ffsl() will be OK,
and some changes will have to be performed so that the init function is
properly called (and maybe the deinit one can be dropped).

In order to test for the fix, simply set up a multi-threaded frontend with
multiple bind lines each attached to a single thread (reproduced with 16
threads here), set up a very low maxconn value on the frontend, and inject
heavy traffic on all listeners in parallel with slightly more connections
than the configured limit ( typically +20%) so that it flips very
frequently. If the bug is still there, at some point (5-20 seconds) the
traffic will go much lower or even stop, either with spinning threads or
not.
2019-07-12 09:07:48 +02:00
Willy Tarreau
64e6012eb9 MINOR: task: introduce work lists
Sometimes we need to delegate some list processing to a function running
on another thread. In this case the list element will simply be queued
into a dedicated self-locked list and the task responsible for this list
will be woken up, calling the associated function which will run over the
list.

This is what work_list does. Such lists will be dedicated to a limited
type of work but will significantly ease such remote handling. A function
is provided to create these per-thread lists, their tasks and to properly
bind each task to a distinct thread, so that the caller only has to store
the resulting pointer to the start of the structure.

These structures should not be abused though as each head will consume
4 pointers per thread, hence 32 bytes per thread or 2 kB for 64 threads.
2019-07-12 09:07:48 +02:00
Olivier Houchard
4be7190c10 BUG/MEDIUM: servers: Fix a race condition with idle connections.
When we're purging idle connections, there's a race condition, when we're
removing the connection from the idle list, to add it to the list of
connections to free, if the thread owning the connection tries to free it
at the same time.
To fix this, simply add a per-thread lock, that has to be hold before
removing the connection from the idle list, and when, in conn_free(), we're
about to remove the connection from every list. That way, we know for sure
the connection will stay valid while we remove it from the idle list, to add
it to the list of connections to free.
This should happen rarely enough that it shouldn't have any impact on
performances.
This has not been reported yet, but could provoke random segfaults.

This should be backported to 2.0.
2019-07-11 16:16:38 +02:00
Frdric Lcaille
51596c166b CLEANUP: proto_tcp: Remove useless header inclusions.
I guess "sys/un.h" and "sys/stat.h" were included for debugging purposes when
"proto_tcp.c" was initially created. There are no more useful.
2019-07-11 10:40:20 +02:00
David Carlier
7df4185f3c BUG/MEDIUM: da: cast the chunk to string.
in fetch mode, the output was incorrect, setting the type to string
explicitally.

This should be backported to all stable versions.
2019-07-11 10:20:09 +02:00
Olivier Houchard
bc89ad8d94 BUG/MEDIUM: checks: Don't attempt to read if we destroyed the connection.
In event_srv_chk_io(), only call __event_srv_chk_r() if we did not subscribe
for reading, and if wake_srv_chk() didn't return -1, as it would mean it
just destroyed the connection and the conn_stream, and attempting to use
those to recv data would lead to a crash.

This should be backported to 1.9 and 2.0.
2019-07-10 16:29:12 +02:00
Willy Tarreau
828675421e MINOR: pools: always pre-initialize allocated memory outside of the lock
When calling mmap(), in general the system gives us a page but does not
really allocate it until we first dereference it. And it turns out that
this time is much longer than the time to perform the mmap() syscall.
Unfortunately, when running with memory debugging enabled, we mmap/munmap()
each object resulting in lots of such calls and a high contention on the
allocator. And the first accesses to the page being done under the pool
lock is extremely damaging to other threads.

The simple fact of writing a 0 at the beginning of the page after
allocating it and placing the POOL_LINK pointer outside of the lock is
enough to boost the performance by 8x in debug mode and to save the
watchdog from triggering on lock contention. This is what this patch
does.
2019-07-09 10:40:33 +02:00
Willy Tarreau
3e853ea74d MINOR: pools: release the pool's lock during the malloc/free calls
The malloc and free calls and especially the underlying mmap/munmap()
can occasionally take a huge amount of time and even cause the thread
to sleep. This is visible when haproxy is compiled with DEBUG_UAF which
causes every single pool allocation/free to allocate and release pages.
In this case, when using the locked pools, the watchdog can occasionally
fire under high contention (typically requesting 40000 1M objects in
parallel over 8 threads). Then, "perf top" shows that 50% of the CPU
time is spent in mmap() and munmap(). The reason the watchdog fires is
because some threads spin on the pool lock which is held by other threads
waiting on mmap() or munmap().

This patch modifies this so that the pool lock is released during these
syscalls. Not only this allows other threads to request try to allocate
their data in parallel, but it also considerably reduces the lock
contention.

Note that the locked pools are only used on small architectures where
high thread counts would not make sense, so this will not provide any
benefit in the general case. However it makes the debugging versions
way more stable, which is always appreciated.
2019-07-09 10:40:33 +02:00
Lukas Tribus
4979916134 BUG/MINOR: ssl: revert empty handshake detection in OpenSSL <= 1.0.2
Commit 54832b97 ("BUILD: enable several LibreSSL hacks, including")
changed empty handshake detection in OpenSSL <= 1.0.2 and LibreSSL,
from accessing packet_length directly (not available in LibreSSL) to
calling SSL_state() instead.

However, SSL_state() appears to be fully broken in both OpenSSL and
LibreSSL.

Since there is no possibility in LibreSSL to detect an empty handshake,
let's not try (like BoringSSL) and restore this functionality for
OpenSSL 1.0.2 and older, by reverting to the previous behavior.

Should be backported to 2.0.
2019-07-09 04:47:18 +02:00
Olivier Houchard
a1ab97316f BUG/MEDIUM: servers: Don't forget to set srv_cs to NULL if we can't reuse it.
In connect_server(), if there were already a CS assosciated with the stream,
but we can't reuse it, because the target is different (because we tried a
previous connection, it failed, and we use redispatch so we switched servers),
don't forget to set srv_cs to NULL. Otherwise, if we end up reusing another
connection, we would consider we already have a conn_stream, and we won't
create a new one, so we'd have a new connection but we would not be able to
use it.
This can explain frozen streams and connections stuck in CLOSE_WAIT when
using redispatch.

This should be backported to 1.9 and 2.0.
2019-07-08 16:32:58 +02:00
Christopher Faulet
037b3ebd35 BUG/MEDIUM: stream-int: Don't rely on CF_WRITE_PARTIAL to unblock opposite si
In the function stream_int_notify(), when the opposite stream-interface is
blocked because there is no more room into the input buffer, if the flag
CF_WRITE_PARTIAL is set on this buffer, it is unblocked. It is a way to unblock
the reads on the other side because some data was sent.

But it is a problem during the fast-forwarding because only the stream is able
to remove the flag CF_WRITE_PARTIAL. So it is possible to have this flag because
of a previous send while the input buffer of the opposite stream-interface is
now full. In such case, the opposite stream-interface will be woken up for
nothing because its input buffer is full. If the same happens on the opposite
side, we will have a loop consumming all the CPU.

To fix the bug, the opposite side is now only notify if there is some available
room in its input buffer in the function si_cs_send(), so only if some data was
sent.

This patch must be backported to 2.0 and 1.9.
2019-07-05 14:26:15 +02:00
Christopher Faulet
86162db15c MINOR: stream-int: Factorize processing done after sending data in si_cs_send()
In the function si_cs_send(), what is done when an error occurred on the
connection or the conn_stream or when some successfully data was send via a pipe
or the channel's buffer may be factorized at the function. It slightly simplify
the function.

This patch must be backported to 2.0 and 1.9 because a bugfix depends on it.
2019-07-05 14:26:15 +02:00
Christopher Faulet
0e54d547f1 BUG/MINOR: mux-h1: Don't process input or ouput if an error occurred
It is useless to proceed if an error already occurred. Instead, it is better to
wait it will be catched by the stream or the connection, depending on which is
the first one to detect it.

This patch must be backported to 2.0.
2019-07-05 14:26:15 +02:00
Christopher Faulet
f8db73efbe BUG/MEDIUM: mux-h1: Handle TUNNEL state when outgoing messages are formatted
Since the commit 94b2c7 ("MEDIUM: mux-h1: refactor output processing"), the
formatting of outgoing messages is performed on the message state and no more on
the HTX blocks read. But the TUNNEL state was left out. So, the HTTP tunneling
using the CONNECT method or switching the protocol (for instance,
the WebSocket) does not work.

This issue was reported on Github. See #131. This patch must be backported to
2.0.
2019-07-05 14:26:15 +02:00
Christopher Faulet
16b2be93ad BUG/MEDIUM: lb_fas: Don't test the server's lb_tree from outside the lock
In the function fas_srv_reposition(), the server's lb_tree is tested from
outside the lock. So it is possible to remove it after the test and then call
eb32_insert() in fas_queue_srv() with a NULL root pointer, which is
invalid. Moving the test in the scope of the lock fixes the bug.

This issue was reported on Github, issue #126.

This patch must be backported to 2.0, 1.9 and 1.8.
2019-07-05 14:26:15 +02:00
Christopher Faulet
8f1aa77b42 BUG/MEDIUM: http/applet: Finish request processing when a service is registered
In the analyzers AN_REQ_HTTP_PROCESS_FE/BE, when a service is registered, it is
important to not interrupt remaining processing but just the http-request rules
processing. Otherwise, the part that handles the applets installation is
skipped.

Among the several effects, if the service is registered on a frontend (not a
listen), the forwarding of the request is skipped because all analyzers are not
set on the request channel. If the service does not depends on it, the response
is still produced and forwarded to the client. But the stream is infinitly
blocked because the request is not fully consumed. This issue was reported on
Github, see #151.

So this bug is fixed thanks to the new action return ACT_RET_DONE. Once a
service is registered, the action process_use_service() still returns
ACT_RET_STOP. But now, only rules processing is stopped. As a side effet, the
action http_action_reject() must now return ACT_RET_DONE to really stop all
processing.

This patch must be backported to 2.0. It depends on the commit introducing the
return code ACT_RET_DONE.
2019-07-05 14:26:14 +02:00
Christopher Faulet
2e4843d1d2 MINOR: action: Add the return code ACT_RET_DONE for actions
This code should be now used by action to stop at the same time the rules
processing and the possible following processings. And from its side, the return
code ACT_RET_STOP should be used to only stop rules processing.

So concretely, for TCP rules, there is no changes. ACT_RET_STOP and ACT_RET_DONE
are handled the same way. However, for HTTP rules, ACT_RET_STOP should now be
mapped on HTTP_RULE_RES_STOP and ACT_RET_DONE on HTTP_RULE_RES_DONE. So this
way, a action will have the possibilty to stop all processing or only rules
processing.

Note that changes about the TCP is done in this commit but changes about the
HTTP will be done in another one because it will fix a bug in the same time.

This patch must be backported to 2.0 because a bugfix depends on it.
2019-07-05 14:26:14 +02:00
Frédéric Lécaille
1b9423d214 MINOR: server: Add "no-tfo" option.
Simple patch to add "no-tfo" option to "default-server" and "server" lines
to disable any usage of TCP fast open.

Must be backported to 2.0.
2019-07-04 14:45:52 +02:00
Olivier Houchard
8d82db70a5 BUG/MEDIUM: servers: Authorize tfo in default-server.
There's no reason to forbid using tfo with default-server, so allow it.

This should be backported to 2.0.
2019-07-04 13:34:25 +02:00
Olivier Houchard
2ab3dada01 BUG/MEDIUM: connections: Make sure we're unsubscribe before upgrading the mux.
Just calling conn_force_unsubscribe() from conn_upgrade_mux_fe() is not
enough, as there may be multiple XPRT involved. Instead, require that
any user of conn_upgrade_mux_fe() unsubscribe itself before calling it.
This should fix upgrading a TCP connection to HTX when using SSL.

This should be backported to 2.0.
2019-07-03 13:57:30 +02:00
Christopher Faulet
9060fc02b5 BUG/MINOR: hlua/htx: Respect the reserve when HTX data are sent
The previous commit 7e145b3e2 ("BUG/MINOR: hlua: Don't use
channel_htx_recv_max()") is buggy. The buffer's reserve must be respected.

This patch must be backported to 2.0 and 1.9.
2019-07-03 11:47:20 +02:00
Christopher Faulet
7e145b3e24 BUG/MINOR: hlua: Don't use channel_htx_recv_max()
The function htx_free_data_space() must be used intead. Otherwise, if there are
some output data not already forwarded, the maximum amount of data that may be
inserted into the buffer may be greater than what we can really insert.

This patch must be backported to 2.0 and 1.9.
2019-07-02 21:32:45 +02:00
Olivier Houchard
f494957980 BUG/MEDIUM: checks: Make sure the tasklet won't run if the connection is closed.
wake_srv_chk() can be called from conn_fd_handler(), and may decide to
destroy the conn_stream and the connection, by calling cs_close(). If that
happens, we have to make sure the tasklet isn't scheduled to run, or it will
probably crash trying to access the connection or the conn_stream.
This fixes a crash that can be seen when using tcp checks.

This should be backported to 1.9 and 2.0.
For 1.9, the call should be instead :
task_remove_from_tasklet_list((struct task *)check->wait_list.task);
That function was renamed in 2.0.
2019-07-02 17:45:35 +02:00