Commit Graph

15992 Commits

Author SHA1 Message Date
Christopher Faulet
e8f3596cd0 MINOR: stream: Improve dump of bogus streams
Stream flags and information about the HTTP txn, if defined, are now
emitted. This will help us to identify bugs when such message is reported.
2021-11-02 17:25:48 +01:00
Christopher Faulet
9ed1a0601d BUG/MEDIUM: resolvers: Track api calls with a counter to free resolutions
The kill list introduced in commit f766ec6b5 ("MEDIUM: resolvers: use a kill
list to preserve the list consistency") contains a bug. The deatch_row must
be initialized before calling resolv_process_responses() function. However,
this function is called for the dns code. The death_row is not visible from
the outside. So, it is possible to add a resolution in an uninitialized
death_row, leading to a crash.

But, with the current implementation, it is not possible to handle the
death_row in resolv_process_responses() function because, internally, the
kill list may be freed via a call to resolv_unlink_resolution(). At the end,
we are unable to determine all call chains to guarantee a safe use of the
kill list. It is a shameful observation, but unfortunatly true.

So, to make the fix simple, we track all calls to the public resolvers
api. A counter is incremented when we enter in the resolver code and
decremented when we leave it. This way, we are able to track the recursions
to init and release the kill list only once, at the edge.

Following functions are incrementing/decrementing the recurse counter:

  * resolv_trigger_resolution()
  * resolv_srvrq_expire_task()
  * resolv_link_resolution()
  * resolv_unlink_resolution()
  * resolv_detach_from_resolution_answer_items()
  * resolv_process_responses()
  * process_resolvers()
  * resolvers_finalize_config()
  * resolv_action_do_resolve()

This patch should fix the issue #1404. It must be backported everywhere the
above commit was backported.
2021-11-02 16:55:01 +01:00
Christopher Faulet
69fad00ebf BUG/MEDIUM: stream-int: Block reads if channel cannot receive more data
First of all, we must be careful here because this part was modified and
each time, this introduced a bug. But, in si_update_rx(), we must not
re-enables receives if the channel buffer cannot receive more
data. Otherwise the multiplexer will be wake up for nothing. Because the
stream is woken up when the multiplexer is waiting for more room to move on,
this may lead to a ping-pong loop between the stream and the mux.

Note that for now, it does not fix any known bug. All reported issues in
this area were fixed in another way.

This patch must be backported with a special care. Technically speaking, it
may be backported as far as 2.0.
2021-11-02 16:55:01 +01:00
William Lallemand
0f41c384ea BUG/MINOR: httpclient: use a placeholder value for Host header
A Host header must be present for http_update_host() to success.
htx_add_header(htx, ist("Host"), IST_NULL) was used but this is not a
good idea from a semantic point of view. It also tries to make a memcpy
with a len of 0, which is unrequired.

Use an ist("h") instead as a placeholder value.

This patch fixes bug #1439.
2021-11-02 15:53:09 +01:00
David Carlier
5592c7b455 BUILD/MINOR: cpuset freebsd build fix
Add missing strings.h header. Required for ffsl definition used by
CPU_FFS macro.

This must be backported up to 2.4.
2021-11-02 13:58:28 +01:00
William Lallemand
d1187eb3e1 BUG/MINOR: httpclient/lua: misplaced luaL_buffinit()
Some luaL_buffinit() call was done before the push of the variable name,
where it seems to work correctly with lua < 5.4.3, it brokes
systematically on this version.

This patch inverts the pushstring and the buffinit.
2021-11-02 10:40:06 +01:00
Remi Tricot-Le Breton
7da35bff9f BUG/MINOR: http: http_auth_bearer fetch does not work on custom header name
The http_auth_bearer sample fetch can take a header name as parameter,
in which case it will try to extract a Bearer value out of the given
header name instead of the default "Authorization" one. In this case,
the extraction would not have worked because of a misuse of strncasecmp.
This patch fixes this by replacing the standard string functions by ist
ones.
It also properly manages the multiple spaces that could be found between
the scheme and its value.

No backport needed, that's part of JWT which is only in 2.5.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
2021-10-29 17:40:17 +02:00
Remi Tricot-Le Breton
68c4eae87f BUG/MINOR: http: Authorization value can have multiple spaces after the scheme
As per RFC7235, there can be multiple spaces in the value of an
Authorization header, between the scheme and the actual authentication
parameters.

This can be backported to all stable versions since basic auth has almost
always been there.
2021-10-29 17:40:17 +02:00
Christopher Faulet
b0c87f1c61 BUG/MEDIUM: http-ana: Drain request data waiting the tarpit timeout expiration
When a tarpit action is performed, we must be sure to drain data from the
request channel. Otherwise, the mux on the frontend side may be blocked
because the request channel buffer is full.

This may lead to Two bugs. The first one is a HOL blocking on the H2
multiplexer. A tarpitted stream may block all the others because data are
not drained for the whole tarpit timeout. The second bug is a ping-pong loop
between the multiplexer and the stream. The mux is waiting for more space in
the channel buffer, so it wakes up the stream. And the stream systematically
re-enables receives.

This last part is not pretty clean and it will be addressed with another
fix. But draning request data is a good way to fix both bugs in same time.

This patch must be backported as far as 2.0. The legacy HTTP mode is
probably affected, but I don't know if same bugs may be experienced in this
mode.
2021-10-29 15:06:31 +02:00
Christopher Faulet
bce6db6c3c BUG/MEDIUM: resolvers: Don't recursively perform requester unlink
When a requester is unlink from a resolution, by reading the code, we can
have this call chain:

_resolv_unlink_resolution(srv->resolv_requester)
  resolv_detach_from_resolution_answer_items(resolution, requester)
    resolv_srvrq_cleanup_srv(srv)
      _resolv_unlink_resolution(srv->resolv_requester)

A loop on the resolution answer items is performed inside
resolv_detach_from_resolution_answer_items(). But by reading the code, it
seems possible to recursively unlink the same requester.

To avoid any loop at this stage, the requester clean up must be performed
before the call to resolv_detach_from_resolution_answer_items(). This way,
the second call to _resolv_unlink_resolution() does nothing and returns
immediately because the requester was already detached from the resolution.

This patch is related to the issue #1404. It must be backported as far as
2.2.
2021-10-29 15:06:31 +02:00
Christopher Faulet
e76b4f055d BUG/MEDIUM: mux-h1: Perform a connection shutdown when the h1c is released
When the H1 connection is released, a connection shutdown is now performed.
If it was already performed when the stream was detached, this action has no
effect. But it is mandatory, when an idle H1C is released. Otherwise the
xprt and the socket shutdown is never perfmed. It is especially important
for SSL client connections, because it is the only way to perform a clean
SSL shutdown.

Without this patch, SSL_shutdown is never called, preventing, among other
things, the SSL session caching.

This patch depends on the commit "BUG/MINOR: mux-h1: Save shutdown mode if
the shutdown is delayed". It should be backported as far as 2.0.
2021-10-29 15:06:31 +02:00
Christopher Faulet
a85c522d42 BUG/MINOR: mux-h1: Save shutdown mode if the shutdown is delayed
The connection shutdown may be delayed if there are pending outgoing
data. The action is performed once data are fully sent. In this case the
mode (dirty/clean) was lost and a clean shutdown was always performed. Now,
the mode is saved to be sure to perform the connection shutdown using the
right mode. To do so, H1C_F_ST_SILENT_SHUT flag is introduced.

This patch should be backported as far as 2.0.
2021-10-29 15:06:31 +02:00
Anubhav
e09efaa7cb DOC: Typo fixed "it" should be "is"
This patch was proposed in GitHub PR #1415.

Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
2021-10-29 08:22:15 +02:00
Tim Duesterhus
66255f7bbf MINOR: halog: Add support for extracting captures using -hdr
This patch adds support for extracting captured header fields to halog. A field
can be extracted by passing the `-hdr <block>:<field>` output filter.

Both `<block>` and `<field>` are 1-indexed.

`<block>` refers to the index of the brace-delimited list of headers. If both
request and response headers are captured, then request headers are referenced
by `<block> = 1`, response headers are `2`. If only one direction is captured,
there will only be a single block `1`.

`<field>` refers to a single field within the selected block.

The output will contain one line, possibly empty, per log line processed.
Passing a non-existent `<block>` or `<field>` will result in an empty line.

Example:

    capture request  header a len 50
    capture request  header b len 50
    capture request  header c len 50
    capture response header d len 50
    capture response header e len 50
    capture response header f len 50

`-srv 1:1` will extract request  header `a`
`-srv 1:2` will extract request  header `b`
`-srv 1:3` will extract request  header `c`
`-srv 2:3` will extract response header `f`

This resolves GitHub issue #1146.
2021-10-28 19:44:18 +02:00
Tim Duesterhus
a02c7b882f BUG/MINOR: halog: Add missing newlines in die() messages
This newline is required to correctly print the usage.
2021-10-28 18:50:51 +02:00
Tim Duesterhus
e0992584b9 CLEANUP: halog: Use consistent indentation in help()
Consistently use 1 Tab per line.
2021-10-28 18:50:34 +02:00
Tim Duesterhus
385338b30b MINOR: halog: Rename -qry to -query
With the query flag moved into the correct help section, there is enough space
for two additional characters.
2021-10-28 18:50:07 +02:00
Tim Duesterhus
a0635f8d10 DOC: halog: Move the -qry parameter into the correct section in help text
This is not an output filter, but instead a modifier. Specifically "only one
may be used at a time" is not true.

see 24b8d693b202b01b649f64ed878d8f9dd1b242e4
2021-10-28 18:49:51 +02:00
William Lallemand
207f0cb3be REGTESTS: lua: test httpclient with body streaming
Improve the httpclient reg-tests to test the streaming,

The regtest now sends a big payload to vtest, then receive a payload
from vtest and send it again.
2021-10-28 16:26:47 +02:00
William Lallemand
bd5739e93e MINOR: httpclient/lua: handle the streaming into the lua applet
With this feature the lua implementation of the httpclient is now able
to stream a payload larger than an haproxy buffer.

The hlua_httpclient_send() function is now split into:

hlua_httpclient_send() which initiate the httpclient and parse the lua
parameters

hlua_httpclient_snd_yield() which will send the request and be called
again to stream the request if the body is larger than an haproxy buffer

hlua_httpclient_rcv_yield() which will receive the response and store it
in the lua buffer.
2021-10-28 16:24:14 +02:00
William Lallemand
0da616ee18 MINOR: httpclient: request streaming with a callback
This patch add a way to handle HTTP requests streaming using a
callback.

The end of the data must be specified by using the "end" parameter in
httpclient_req_xfer().
2021-10-28 16:24:14 +02:00
Willy Tarreau
04065b87ce MINOR: atomic: remove the memcpy() call and dependency on string.h
The memcpy() call in the aarch64 version of __ha_cas_dw() is sometimes
inlined and sometimes not, depending on the gcc version. It's only used
to copy two void*, so let's use direct assignment instead of memcpy().
It would also be possible to change the asm code to directly write there,
but it's not worth it.

With this change the code is 8kB smaller with gcc-5.4.
2021-10-28 09:45:48 +02:00
David CARLIER
c050dc6c68 BUILD: atomic: fix build on mac/arm64
The inline assembly is invalid for this platform thus falling back
to the builtin instead.
2021-10-28 09:45:48 +02:00
Willy Tarreau
4957a32f9e BUILD: atomic: prefer __atomic_compare_exchange_n() for __ha_cas_dw()
__atomic_compare_exchange() is incorrectly documented in the gcc builtins
doc, it says the desired value is "type *desired" while in reality it is
"const type *desired" as expected since that value must in no way be
modified by the operation. However it seems that clang has implemented
it as documented, and reports build warnings when fed a const.

This is quite problematic because it means we have to betry the callers,
pretending we won't touch their constants but not knowing what the
compiler would do with them, and possibly hiding future bugs.

Instead of forcing a cast, let's just switch to the better
__atomic_compare_exchange_n() that takes a value instead of a pointer.
At least with this one there is no doubt about how the input will be
used.

It was verified that the output object code is the same both in clang
and gcc with this change.
2021-10-28 09:45:48 +02:00
Tim Duesterhus
8aee3030f8 CLEANUP: hlua: Remove obsolete branch in hlua_alloc()
This branch is no longer required, because the `!nsize` case is handled for any
value of `ptr` now.

see 22586524e32f14c44239063088a38ccea8abc9b7
see a5efdff93c36f75345a2a18f18bffee9b602bc7b
2021-10-28 09:45:48 +02:00
Tim Duesterhus
e6c04507d8 DEV: coccinelle: Add realloc_leak.cocci
This coccinelle patch finds locations where the return value of `realloc()` is
assigned to the pointer passed to `realloc()`. This calls will leak memory if
`realloc()` returns `NULL`.
2021-10-28 09:45:48 +02:00
Tim Duesterhus
e0c1d749a8 CLEANUP: jwt: Remove the use of a trash buffer in jwt_jwsverify_rsa_ecdsa()
`trash` was completely unused within this function.
2021-10-28 09:45:48 +02:00
Tim Duesterhus
c87d3c21bf CLEANUP: jwt: Remove the use of a trash buffer in jwt_jwsverify_hmac()
The OpenSSL documentation (https://www.openssl.org/docs/man1.1.0/man3/HMAC.html)
specifies:

> It places the result in md (which must have space for the output of the hash
> function, which is no more than EVP_MAX_MD_SIZE bytes). If md is NULL, the
> digest is placed in a static array. The size of the output is placed in
> md_len, unless it is NULL. Note: passing a NULL value for md to use the
> static array is not thread safe.

`EVP_MAX_MD_SIZE` appears to be defined as `64`, so let's simply use a stack
buffer to avoid the whole memory management.
2021-10-28 09:45:48 +02:00
Tim Duesterhus
24b8d693b2 MINOR: halog: Add -qry parameter allowing to preserve the query string in -uX
Our use-case for this is a dynamic application that performs routing based on
the query string. Without this option all URLs will just point to the central
entrypoint of this location, making the output completely useless.
2021-10-28 09:45:44 +02:00
Willy Tarreau
14e7f29e86 MINOR: protocols: replace protocol_by_family() with protocol_lookup()
At a few places we were still using protocol_by_family() instead of
the richer protocol_lookup(). The former is limited as it enforces
SOCK_STREAM and a stream protocol at the control layer. At least with
protocol_lookup() we don't have this limitationn. The values were still
set for now but later we can imagine making them configurable on the
fly.
2021-10-27 17:41:07 +02:00
Willy Tarreau
e3b4518414 MINOR: protocols: make use of the protocol type to select the protocol
Instead of using sock_type and ctrl_type to select a protocol, let's
make use of the new protocol type. For now they always match so there
is no change. This is applied to address parsing and to socket retrieval
from older processes.
2021-10-27 17:31:20 +02:00
Willy Tarreau
337edfdbc5 MINOR: protocols: add a new protocol type selector
The protocol selection is currently performed based on the family,
control type and socket type. But this is often not enough, as both
only provide DGRAM or STREAM, leaving few variants. Protocols like
SCTP for example might be indistinguishable from TCP here. Same goes
for TCP extensions like MPTCP.

This commit introduces a new enum proto_type that is placed in each
and every protocol definition, that will usually more or less match
the sock_type, but being an enum, will support additional values.
2021-10-27 17:05:36 +02:00
Willy Tarreau
bdcee7fbc9 DEBUG: protocol: yell loudly during registration of invalid sock_domain
The test on the sock_domain is a bit useless because the protocols are
registered at boot time, and the test silently fails and returns no
error. Use a BUG_ON() instead to make sure to catch such bugs in the
code if any.
2021-10-27 15:50:49 +02:00
Christopher Faulet
52b28d2f30 BUILD: log: Fix compilation without SSL support
When compiled without SSL support, a variable is reported as not used by
GCC.

src/log.c: In function ‘sess_build_logline’:
src/log.c:2056:36: error: unused variable ‘conn’ [-Werror=unused-variable]
 2056 |                 struct connection *conn;
      |                                    ^~~~

This does not need to be backported.
2021-10-27 12:00:15 +02:00
Christopher Faulet
16f16afb31 MINOR: stream: Use backend stream-interface dst address instead of target_addr
target_addr field in the stream structure is removed. The backend
stream-interface destination address is now used.
2021-10-27 11:35:59 +02:00
Christopher Faulet
e1f3b547a1 REGTESTS: Add script to test client src/dst manipulation at different levels
This script tests various set-src and set-dst actions at different levels.
2021-10-27 11:35:59 +02:00
Christopher Faulet
888cd700f4 MINOR: tcp-sample: Add samples to get original info about client connection
Because source and destination address of the client connection are now
updated at the appropriated level (connection, session or stream), original
info about the client connection are preserved.  src/src_port/src_is_local
and dst/dst_port/dst_is_local return current info about the client
connection. It is the info at the highest available level. Most of time, the
stream. Any tcp/http rules may alter this info.

To get original info, "fc_" prefix must be added. For instance
"fc_src". Here, only "tcp-request connection" rules may alter source and
destination address/port.
2021-10-27 11:35:59 +02:00
Christopher Faulet
7bd21921d1 DOC: config: Fix alphabetical order of fc_* samples
fc_* samples were not properly ordered. This patch may be backported as far
as 1.8.
2021-10-27 11:35:59 +02:00
Christopher Faulet
1e83b70409 MINOR: tcp-act: Add set-src/set-src-port for "tcp-request content" rules
This patch was reverted because it was inconsitent to change connection
addresses at stream level. Especially in HTTP because all requests was
affected by this change and not only the current one. In HTTP/2, it was
worse. Several streams was able to change the connection addresses at the
same time.

It is no longer an issue, thanks to recent changes. With multi-level client
source and destination addresses, it is possible to limit the change to the
current request. Thus this patch can be reintroduced.

If it possible to set source IP/Port from "tcp-request connection",
"tcp-request session" and "http-request" rules but not from "tcp-request
content" rules. There is no reason for this limitation and it may be a
problem for anyone wanting to call a lua fetch to dynamically set source
IP/Port from a TCP proxy. Indeed, to call a lua fetch, we must have a
stream. And there is no stream when "tcp-request connection/session" rules
are evaluated.

Thanks to this patch, "set-src" and "set-src-port" action are now supported
by "tcp_request content" rules.

This patch is related to the issue #1303.
2021-10-27 11:35:59 +02:00
Christopher Faulet
d69377eb02 MEDIUM: tcp-act: Set addresses at the apprioriate level in set-(src/dst) actions
When client source or destination addresses are changed via a tcp/http
action, we update addresses at the appropriate level. When "tcp-request
connection" rules are evaluated, we update addresses at the connection
level. When "tcp-request session" rules is evaluated, we update those at the
session level. And finally, when "tcp-request content" or "http-request"
rules are evaluated, we update the addresses at the stream level.

The same is performed when source or destination ports are changed.

Of course, for now, not all level are supported. But thanks to this patch,
it will be possible.
2021-10-27 11:35:59 +02:00
Christopher Faulet
e83e8821bb MEDIUM: connection: Assign session addresses when NetScaler CIP proto is parsed
Just like for the PROXY protocol, when the NetScaler Client IP insertion
header is received, the retrieved client source and destination addresses
are set at the session level. This leaves those at the connection level
intact.
2021-10-27 11:35:59 +02:00
Christopher Faulet
c105c9213f MEDIUM: connection: Assign session addresses when PROXY line is received
When PROXY protocol line is received, the retrieved client source and
destination addresses are set at the session level. This leaves those at the
connection level intact.
2021-10-27 11:35:59 +02:00
Christopher Faulet
a8e95fed43 MEDIUM: backend: Rely on addresses at stream level to init server connection
Client source and destination addresses at stream level are used to initiate
the connections to a server. For now, stream-interface addresses are never
set. So, thanks to the fallback mechanism, no changes are expected with this
patch. But its purpose is to rely on addresses at the appropriate level when
set instead of those at the connection level.
2021-10-27 11:35:59 +02:00
Christopher Faulet
b097aef2ef MEDIUM: connection: Rely on addresses at stream level to make proxy line
If the stream exists, the frontend stream-interface is used to get the
client source and destination addresses when the proxy line is built. For
now, stream-interface or session addresses are never set. So, thanks to the
fallback mechanism, no changes are expected with this patch. But its purpose
is to rely on addresses at the appropriate level when set instead of those
at the connection level.
2021-10-27 11:35:57 +02:00
Christopher Faulet
c03be1a129 MEDIUM: tcp-sample: Rely on addresses at the appropriate level in tcp samples
In src, src-port, dst and dst-port sample fetches, the client source and
destination addresses are retrieved from the appropriate level. It means
that, if the stream exits, we use the frontend stream-interface to get the
client source and destination addresses. Otherwise, the session is used. For
now, stream-interface or session addresses are never set. So, thanks to the
fallback mechanism, no changes are expected with this patch. But its purpose
is to rely on addresses at the appropriate level when set instead of those
at the connection level.
2021-10-27 11:34:21 +02:00
Christopher Faulet
568008d199 MINOR: mux-fcgi: Rely on client addresses at stream level to set default params
Client source and destination addresses at stream level are now used to emit
SERVER_NAME/SERVER_PORT and REMOTE_ADDR/REMOTE_PORT parameters. For now,
stream-interface addresses are never set. So, thanks to the fallback
mechanism, no changes are expected with this patch. But its purpose is to
rely on addresses at the stream level, when set, instead of those at the
connection level.
2021-10-27 11:34:21 +02:00
Christopher Faulet
6fc817a28e MINOR: http-fetch: Rely on addresses at stream level in HTTP sample fetches
Client source and destination addresses at stream level are now used to
compute base32+src and url32+src hashes. For now, stream-interface addresses
are never set. So, thanks to the fallback mechanism, no changes are expected
with this patch. But its purpose is to rely on addresses at the stream
level, when set, instead of those at the connection level.
2021-10-27 11:34:21 +02:00
Christopher Faulet
8a104ba3e0 MINOR: http-ana: Rely on addresses at stream level to set xff and xot headers
Client source and destination addresses at stream level are now used to emit
X-Forwarded-For and X-Original-To headers. For now, stream-interface addresses
are never set. So, thanks to the fallback mechanism, no changes are expected
with this patch. But its purpose is to rely on addresses at the stream level,
when set, instead of those at the connection level.
2021-10-27 11:34:21 +02:00
Christopher Faulet
c269f664bd MINOR: session: Rely on client source address at session level to log error
When an embryonic session is killed, if no log format is defined for this
error, a generic error is emitted. When this happens, we now rely on the
session to get the client source address. For now, session addresses are
never set. So, thanks to the fallback mechanism, no changes are expected
with this patch. But its purpose is to rely on addresses at the session
level when set instead of those at the connection level.
2021-10-27 11:34:21 +02:00
Christopher Faulet
f9c4d8d5be MINOR: log: Rely on client addresses at the appropriate level to log messages
When a log message is emitted, if the stream exits, we use the frontend
stream-interface to retrieve the client source and destination
addresses. Otherwise, the session is used. For now, stream-interface or
session addresses are never set. So, thanks to the fallback mechanism, no
changes are expected with this patch. But its purpose is to rely on
addresses at the appropriate level when set instead of those at the
connection level.
2021-10-27 11:34:21 +02:00