Commit Graph

454 Commits

Author SHA1 Message Date
Willy Tarreau
44c5ff69ac MEDIUM: vars: make the var() sample fetch function really return type ANY
A long-standing issue was reported in issue #1215.

In short, var() was initially internally declared as returning a string
because it was not possible by then to return "any type". As such, users
regularly get trapped thinking that when they're storing an integer there,
then the integer matching method automatically applies. Except that this
is not possible since this is related to the config parser and is decided
at boot time where the variable's type is not known yet.

As such, what is done is that the output being declared as type string,
the string match will automatically apply, and any value will first be
converted to a string. This results in several issues like:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) lt 0 }

not working. This is because the string match on the second line will in
fact compare the string representation of the variable against strings
"lt" and "0", none of which matches.

The doc says that the matching method is mandatory, though that's not
the case in the code due to that default string type being permissive.
There's not even a warning when no explicit match is placed, because
this happens very deep in the expression evaluator and making a special
case just for "var" can reveal very complicated.

The set-var() converter already mandates a matching method, as the
following will be rejected:

    ... if { int(12),set-var(txn.truc) 12 }

  while this one will work:

    ... if { int(12),set-var(txn.truc) -m int 12 }

As such, this patch this modifies var() to match the doc, returning the
type "any", and mandating the matching method, implying that this bogus
config which does not work:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) lt 0 }

  will need to be written like this:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) -m int lt 0 }

This *will* break some configs (and even 3 of our regtests relied on
this), but except those which already match string exclusively, all
other ones are already broken and silently fail (and one of the 3
regtests, the one on FIX, was bogus regarding this).

In order to fix existing configs, one can simply append "-m str"
after a "var()" in an ACL or "if" expression:

    http-request deny unless { var(txn.jwt_alg) "ES" }

  must become:

    http-request deny unless { var(txn.jwt_alg) -m str "ES" }

Most commonly, patterns such as "le", "lt", "ge", "gt", "eq", "ne" in
front of a number indicate that the intent was to match an integer,
and in this case "-m int" would be desired:

    tcp-response content reject if ! { var(res.size) gt 3800 }

  ought to become:

    tcp-response content reject if ! { var(res.size) -m int gt 3800 }

This must not be backported, but if a solution is found to at least
detect this exact condition in the generic expression parser and
emit a warning, this could probably help spot configuration bugs.

Link: https://www.mail-archive.com/haproxy@formilux.org/msg41341.html
Cc: Christopher Faulet <cfaulet@haproxy.com>
Cc: Tim Düsterhus <tim@bastelstu.be>
2021-11-02 17:28:43 +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
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
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
William Lallemand
dc2cc9008b MINOR: httpclient/lua: support more HTTP methods
Add support for HEAD/PUT/POST/DELETE method with the lua httpclient.

This patch use the httpclient_req_gen() function with a different meth
parameter to implement this.

Also change the reg-test to support a POST request with a body.
2021-10-27 10:19:49 +02:00
William Lallemand
dec25c3e14 MINOR: httpclient: support payload within a buffer
httpclient_req_gen() takes a payload argument which can be use to put a
payload in the request. This payload can only fit a request buffer.

This payload can also be specified by the "body" named parameter within
the lua. httpclient.

It is also used within the CLI httpclient when specified as a CLI
payload with "<<".
2021-10-27 10:19:41 +02:00
Remi Tricot-Le Breton
1c891bcc90 MINOR: jwt: jwt_verify returns negative values in case of error
In order for all the error return values to be distributed on the same
side (instead of surrounding the success error code), the return values
for errors other than a simple verification failure are switched to
negative values. This way the result of the jwt_verify converter can be
compared strictly to 1 as well relative to 0 (any <= 0 return value is
an error).
The documentation was also modified to discourage conversion of the
return value into a boolean (which would definitely not work).
2021-10-18 16:02:29 +02:00
Ilya Shipitsin
bd6b4be721 CLEANUP: assorted typo fixes in the code and comments
This is 27th iteration of typo fixes
2021-10-18 07:26:19 +02:00
Christopher Faulet
e41b497978 REGTESTS: Add scripts to test support of TCP/HTTP rules in defaults sections
3 scripts are added:

  * startup/default_rules.vtc to check configuration parsing
  * http-rules/default_rules.vtc to check evaluation of HTTP rules
  * tcp-rules/default_rules.vtc to check evaluation of TCP rules
2021-10-15 14:12:19 +02:00
Christopher Faulet
597909f4e6 BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back
http-after-response rules evaluation must be stopped after a "allow". It
means the frontend ruleset must not be evaluated if a "allow" was performed
in the backend ruleset. Internally, the evaluation must be stopped if on
HTTP_RULE_RES_STOP return value. Only the "allow" action is concerned by
this change.

Thanks to this patch, http-response and http-after-response behave in the
same way.

This patch should be backported as far as 2.2.
2021-10-15 14:12:19 +02:00
Willy Tarreau
468c000db0 BUG/MEDIUM: jwt: fix base64 decoding error detection
Tim reported that a decoding error from the base64 function wouldn't
be matched in case of bad input, and could possibly cause trouble
with -1 being passed in decoded_sig->data. In the case of HMAC+SHA
it is harmless as the comparison is made using memcmp() after checking
for length equality, but in the case of RSA/ECDSA this result is passed
as a size_t to EVP_DigetVerifyFinal() and may depend on the lib's mood.

The fix simply consists in checking the intermediary result before
storing it.

That's precisely what happens with one of the regtests which returned
0 instead of 4 on the intentionally defective token, so the regtest
was fixed as well.

No backport is needed as this is new in this release.
2021-10-15 11:41:16 +02:00
Remi Tricot-Le Breton
36da606324 REGTESTS: jwt: Add tests for the jwt_verify converter
This regtest uses the new jwt_header_query, jwt_payload_query and
jwt_verify converters that can be used to validate a JSON Web Token.
2021-10-14 16:38:14 +02:00
William Lallemand
1d58b01316 MINOR: ssl: add ssl_fc_is_resumed to "option httpslog"
In order to trace which session were TLS resumed, add the
ssl_fc_is_resumed in the httpslog option.
2021-10-14 14:27:48 +02:00
William Lallemand
e5dfd405b3 REGTESTS: ssl: re-enable set_ssl_cert_bundle.vtc
The new "ssllib_name_startswith(OpenSSL)" command allows us to
reactivate set_ssl_cert_bundle.vtc with >= OpenSSL 1.1.1 only.
2021-10-14 11:06:16 +02:00
Remi Tricot-Le Breton
e1b61090a0 REGTESTS: ssl: Use mostly TLSv1.2 in ssl_errors test
In order for the test to run with OpenSSL 1.0.2 the test will now mostly
use TLSv1.2 and use TLS 1.3 only on some specific tests (covered by
preconditions).
2021-10-13 11:28:12 +02:00
Remi Tricot-Le Breton
d12e13a55a REGTESTS: ssl: Reenable ssl_errors test for OpenSSL only
The test is strongly dependent on the way the errors are output by the
SSL library so it is not possible to perform the same checks when using
OpenSSL or LibreSSL. It is then reenabled for OpenSSL (whatever the
version) but still disabled for LibreSSL.
This limitation is added thanks to the new ssllib_name_startswith
precondition check.
2021-10-13 11:28:11 +02:00
Remi Tricot-Le Breton
d266cdad2a REGTESTS: ssl: Fix ssl_errors test for OpenSSL v3
The OpenSSL error codes for the same errors are not consistent between
OpenSSL versions. The ssl_errors test needs to be modified to only take
into account a fixed part of those error codes.
This patch focuses on the reason part of the error code by applying a
mask on the error code (whose size varies depending on the lib version).
2021-10-13 11:28:10 +02:00
Remi Tricot-Le Breton
1ac65f8668 REGTESTS: ssl: Fix references to removed option in test description
The log-error-via-logformat option was removed in commit
3d6350e108 and was replaced by a dedicated
error-log-format option. The references to this option need to be
removed from the test's description.
2021-10-13 11:28:07 +02:00
William Lallemand
746e6f3f8e MINOR: httpclient/lua: supports headers via named arguments
Migrate the httpclient:get() method to named arguments so we can
specify optional arguments.

This allows to pass headers as an optional argument as an array.

The () in the method call must be replaced by {}:

	local res = httpclient:get{url="http://127.0.0.1:9000/?s=99",
	            headers= {["X-foo"]  = { "salt" }, ["X-bar"] = {"pepper" }}}
2021-10-06 15:21:02 +02:00
Christopher Faulet
d28b2b2352 BUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release
When a filter is attached to a stream, the wrong FLT_END analyzer is added
on the request channel. AN_REQ_FLT_END must be added instead of
AN_RES_FLT_END. Because of this bug, the stream may hang on the filter
release stage.

It seems to be ok for HTTP filters (cache & compression) in HTTP mode. But
when enabled on a TCP proxy, the stream is blocked until the client or the
server timeout expire because data forwarding is blocked. The stream is then
prematurely aborted.

This bug was introduced by commit 26eb5ea35 ("BUG/MINOR: filters: Always set
FLT_END analyser when CF_FLT_ANALYZE flag is set"). The patch must be
backported in all stable versions.
2021-10-04 08:28:44 +02:00
William Lallemand
f542941f71 REGTESTS: ssl: wrong feature cmd in show_ssl_ocspresponse.vtc
The "feature cmd" needs to be separated in 2 parts to check the openssl
command.
2021-09-30 18:45:18 +02:00
William Lallemand
2655f2ba33 REGTESTS: ssl: show_ssl_ocspresponse w/ freebsd won't use base64
The reg-test show_ssl_ocspresponse.vtc won't use the "base64" binary on
freebsd, replace it by a "openssl base64" which does the same thing.
2021-09-30 17:58:58 +02:00
William Lallemand
8d264387c3 REGTESTS: ssl: enable ssl_crt-list_filters.vtc again
ssl_crt-list_filters.vtc was deactivated because they were not compatible with
previous version of OpenSSL and it was not possible to
filter by versions.

Activate it again with a openssl_version_atleast(1.1.1)
check.
2021-09-30 15:39:59 +02:00
William Lallemand
2f52fdb52e REGTESTS: ssl: enable show_ssl_ocspresponse.vtc again
Since we disabled boringssl from the CI we can enable this test again.
2021-09-30 15:28:30 +02:00
Remi Tricot-Le Breton
1fe0fad88b MINOR: ssl: Rename ssl_bc_hsk_err to ssl_bc_err
The ssl_bc_hsk_err sample fetch will need to raise more errors than only
handshake related ones hence its renaming to a more generic ssl_bc_err.
This patch is required because some handshake failures that should have
been caught by this fetch (verify error on the server side for instance)
were missed. This is caused by a change in TLS1.3 in which the
'Finished' state on the client is reached before its certificate is sent
(and verified) on the server side (see the "Protocol Overview" part of
RFC 8446).
This means that the SSL_do_handshake call is finished long before the
server can verify and potentially reject the client certificate.

The ssl_bc_hsk_err will then need to be expanded to catch other types of
errors.

This change is also applied to the frontend fetches (ssl_fc_hsk_err
becomes ssl_fc_err) and to their string counterparts.
2021-09-30 11:04:35 +02:00
Christopher Faulet
c7e9492166 REGTESTS: Add script to validate T-E header parsing
Some changes were pushed to improve parsing of the Transfer-Encoding header
parsing annd all related stuff. This new script adds some tests to validate
these changes.
2021-09-28 16:43:07 +02:00
William Lallemand
039cc083ff REGTESTS: lua: test the httpclient:get() feature
This reg-test is heavily inspired by the lua_socket.vtc one.

It replaces the HTTP/1.1 request made manually with a socket object with
an httpclient object.
2021-09-24 19:05:53 +02:00
Christopher Faulet
46e058dda5 BUG/MEDIUM: mux-h1: Adjust conditions to ask more space in the channel buffer
When a message is parsed and copied into the channel buffer, in
h1_process_demux(), more space is requested if some pending data remain
after the parsing while the channel buffer is not empty. To do so,
CS_FL_WANT_ROOM flag is set. It means the H1 parser needs more space in the
channel buffer to continue. In the stream-interface, when this flag is set,
the SI is considered as blocked on the RX path. It is only unblocked when
some data are sent.

However, it is not accurrate because the parsing may be stopped because
there is not enough data to continue. For instance in the middle of a chunk
size. In this case, some data may have been already copied but the parser is
blocked because it must receive more data to continue. If the calling SI is
blocked on RX at this stage when the stream is waiting for the payload
(because http-buffer-request is set for instance), the stream remains stuck
infinitely.

To fix the bug, we must request more space to the app layer only when it is
not possible to copied more data. Actually, this happens when data remain in
the input buffer while the H1 parser is in states MSG_DATA or MSG_TUNNEL, or
when we are unable to copy headers or trailers into a non-empty buffer.

The first condition is quite easy to handle. The second one requires an API
refactoring. h1_parse_msg_hdrs() and h1_parse_msg_tlrs() fnuctions have been
updated. Now it is possible to know when we need more space in the buffer to
copy headers or trailers (-2 is returned). In the H1 mux, a new H1S flag
(H1S_F_RX_CONGESTED) is used to track this state inside h1_process_demux().

This patch is part of a series related to the issue #1362. It should be
backported as far as 2.0, probably with some adaptations. So be careful
during backports.
2021-09-23 16:13:17 +02:00
Christopher Faulet
8a0e5f822b BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check
When the LDAP response is parsed, the message length is not properly
decoded. While it works for LDAP servers encoding it on 1 byte, it does not
work for those using a multi-bytes encoding. Among others, Active Directory
servers seems to encode messages or elements length on 4 bytes.

In this patch, we only handle length of BindResponse messages encoded on 1,
2 or 4 bytes. In theory, it may be encoded on any bytes number less than 127
bytes. But it is useless to make this part too complex. It should be ok this
way.

This patch should fix the issue #1390. It should be backported to all stable
versions. While it should be easy to backport it as far as 2.2, the patch
will have to be totally rewritten for lower versions.
2021-09-16 17:24:50 +02:00
Willy Tarreau
3a4bedccc6 MEDIUM: vars: replace the global name index with a hash
The global table of known variables names can only grow and was designed
for static names that are registered at boot. Nowadays it's possible to
set dynamic variable names from Lua or from the CLI, which causes a real
problem that was partially addressed in 2.2 with commit 4e172c93f
("MEDIUM: lua: Add `ifexist` parameter to `set_var`"). Please see github
issue #624 for more context.

This patch simplifies all this by removing the need for a central
registry of known names, and storing 64-bit hashes instead. This is
highly sufficient given the low number of variables in each context.
The hash is calculated using XXH64() which is bijective over the 64-bit
space thus is guaranteed collision-free for 1..8 chars. Above that the
risk remains around 1/2^64 per extra 8 chars so in practice this is
highly sufficient for our usage. A random seed is used at boot to seed
the hash so that it's not attackable from Lua for example.

There's one particular nit though. The "ifexist" hack mentioned above
is now limited to variables of scope "proc" only, and will only match
variables that were already created or declared, but will now verify
the scope as well. This may affect some bogus Lua scripts and SPOE
agents which used to accidentally work because a similarly named
variable used to exist in a different scope. These ones may need to be
fixed to comply with the doc.

Now we can sum up the situation as this one:
  - ephemeral variables (scopes sess, txn, req, res) will always be
    usable, regardless of any prior declaration. This effectively
    addresses the most problematic change from the commit above that
    in order to work well could have required some script auditing ;

  - process-wide variables (scope proc) that are mentioned in the
    configuration, referenced in a "register-var-names" SPOE directive,
    or created via "set-var" in the global section or the CLI, are
    permanent and will always accept to be set, with or without the
    "ifexist" restriction (SPOE uses this internally as well).

  - process-wide variables (scope proc) that are only created via a
    set-var() tcp/http action, via Lua's set_var() calls, or via an
    SPOE with the "force-set-var" directive), will not be permanent
    but will always accept to be replaced once they are created, even
    if "ifexist" is present

  - process-wide variables (scope proc) that do not exist will only
    support being created via the set-var() tcp/http action, Lua's
    set_var() calls without "ifexist", or an SPOE declared with
    "force-set-var".

This means that non-proc variables do not care about "ifexist" nor
prior declaration, and that using "ifexist" should most often be
reliable in Lua and that SPOE should most often work without any
prior declaration. It may be doable to turn "ifexist" to 1 by default
in Lua to further ease the transition. Note: regtests were adjusted.

Cc: Tim Düsterhus <tim@bastelstu.be>
2021-09-08 15:06:11 +02:00
Willy Tarreau
54496a6a5b MINOR: vars: make the vars() sample fetch function support a default value
It is quite common to see in configurations constructions like the
following one:

    http-request set-var(txn.bodylen) 0
    http-request set-var(txn.bodylen) req.hdr(content-length)
    ...
    http-request set-header orig-len %[var(txn.bodylen)]

The set-var() rules are almost always duplicated when manipulating
integers or any other value that is mandatory along operations. This is
a problem because it makes the configurations complicated to maintain
and slower than needed. And it becomes even more complicated when several
conditions may set the same variable because the risk of forgetting to
initialize it or to accidentally reset it is high.

This patch extends the var() sample fetch function to take an optional
argument which contains a default value to be returned if the variable
was not set. This way it becomes much simpler to use the variable, just
set it where needed, and read it with a fall back to the default value:

    http-request set-var(txn.bodylen) req.hdr(content-length)
    ...
    http-request set-header orig-len %[var(txn.bodylen,0)]

The default value is always passed as a string, thus it will experience
a cast to the output type. It doesn't seem userful to complicate the
configuration to pass an explicit type at this point.

The vars.vtc regtest was updated accordingly.
2021-09-03 12:08:54 +02:00
Willy Tarreau
e93bff4107 MEDIUM: vars: also support format strings in CLI's "set var" command
Most often "set var" on the CLI is used to set a string, and using only
expressions is not always convenient, particularly when trying to
concatenate variables sur as host names and paths.

Now the "set var" command supports an optional keyword before the value
to indicate its type. "expr" takes an expression just like before this
patch, and "fmt" a format string, making it work like the "set-var-fmt"
actions.

The VTC was updated to include a test on the format string.
2021-09-03 11:01:48 +02:00
Willy Tarreau
753d4db5f3 MINOR: vars: add a "set-var-fmt" directive to the global section
Just like the set-var-fmt action for tcp/http rules, the set-var-fmt
directive in global sections allows to pre-set process-wide variables
using a format string instead of a sample expression. This is often
more convenient when it is required to concatenate multiple fields,
or when emitting just one word.
2021-09-03 11:01:48 +02:00
Willy Tarreau
9a621ae76d MEDIUM: vars: add a new "set-var-fmt" action
The set-var() action is convenient because it preserves the input type
but it's a pain to deal with when trying to concatenate values. The
most recurring example is when it's needed to build a variable composed
of the source address and the source port. Usually it ends up like this:

    tcp-request session set-var(sess.port) src_port
    tcp-request session set-var(sess.addr) src,concat(":",sess.port)

This is even worse when trying to aggregate multiple fields from stick-table
data for example. Due to this a lot of users instead abuse headers from HTTP
rules:

    http-request set-header(x-addr) %[src]:%[src_port]

But this requires some careful cleanups to make sure they won't leak, and
it's significantly more expensive to deal with. And generally speaking it's
not clean. Plus it must be performed for each and every request, which is
expensive for this common case of ip+port that doesn't change for the whole
session.

This patch addresses this limitation by implementing a new "set-var-fmt"
action which performs the same work as "set-var" but takes a format string
in argument instead of an expression. This way it becomes pretty simple to
just write:

    tcp-request session set-var-fmt(sess.addr) %[src]:%[src_port]

It is usable in all rulesets that already support the "set-var" action.
It is not yet implemented for the global "set-var" directive (which already
takes a string) and the CLI's "set var" command, which would definitely
benefit from it but currently uses its own parser and engine, thus it
must be reworked.

The doc and regtests were updated.
2021-09-02 21:22:22 +02:00
Willy Tarreau
bc1223be79 MINOR: http-rules: add a new "ignore-empty" option to redirects.
Sometimes it is convenient to remap large sets of URIs to new ones (e.g.
after a site migration for example). This can be achieved using
"http-request redirect" combined with maps, but one difficulty there is
that non-matching entries will return an empty response. In order to
avoid this, duplicating the operation as an ACL condition ending in
"-m found" is possible but it becomes complex and error-prone while it's
known that an empty URL is not valid in a location header.

This patch addresses this by improving the redirect rules to be able to
simply ignore the rule and skip to the next one if the result of the
evaluation of the "location" expression is empty. However in order not
to break existing setups, it requires a new "ignore-empty" keyword.

There used to be an ACT_FLAG_FINAL on redirect rules that's used during
the parsing to emit a warning if followed by another rule, so here we
only set it if the option is not there. The http_apply_redirect_rule()
function now returns a 3rd value to mention that it did nothing and
that this was not an error, so that callers can just ignore the rule.
The regular "redirect" rules were not modified however since this does
not apply there.

The map_redirect VTC was completed with such a test and updated to 2.5
and an example was added into the documentation.
2021-09-02 17:06:18 +02:00
Remi Tricot-Le Breton
b061fb31ab REGTESTS: ssl: Add tests for bc_conn_err and ssl_bc_hsk_err sample fetches
Those fetches are used to identify connection errors and SSL handshake
errors on the backend side of a connection. They can for instance be
used in a log-format line as in the regtest.
2021-09-01 22:55:56 +02:00
Remi Tricot-Le Breton
fe21fe76bd MINOR: log: Add new "error-log-format" option
This option can be used to define a specific log format that will be
used in case of error, timeout, connection failure on a frontend... It
will be used for any log line concerned by the log-separate-errors
option. It will also replace the format of specific error messages
decribed in section 8.2.6.
If no "error-log-format" is defined, the legacy error messages are still
emitted and the other error logs keep using the regular log-format.
2021-08-31 12:13:08 +02:00
Marcin Deranek
310a260e4a MEDIUM: config: Deprecate tune.ssl.capture-cipherlist-size
Deprecate tune.ssl.capture-cipherlist-size in favor of
tune.ssl.capture-buffer-size which better describes the purpose of the
setting.
2021-08-26 19:52:04 +02:00
Marcin Deranek
da0264a968 MINOR: sample: Add be2hex converter
Add be2hex converter to convert big-endian binary data into hex string
with optional string separators.
2021-08-26 19:48:34 +02:00
Marcin Deranek
40ca09c7bb MINOR: sample: Add be2dec converter
Add be2dec converter which allows to build JA3 compatible TLS
fingerprints by converting big-endian binary data into string
separated unsigned integers eg.

http-request set-header X-SSL-JA3 %[ssl_fc_protocol_hello_id],\
    %[ssl_fc_cipherlist_bin(1),be2dec(-,2)],\
    %[ssl_fc_extlist_bin(1),be2dec(-,2)],\
    %[ssl_fc_eclist_bin(1),be2dec(-,2)],\
    %[ssl_fc_ecformats_bin,be2dec(-,1)]
2021-08-26 19:48:34 +02:00
Tim Duesterhus
cbad112a81 REGTESTS: Remove REQUIRE_VERSION=1.5 from all tests
HAProxy 1.5 is EOL, thus this always matches.

1.6 / 1.7 were already removed in:
d8be0018fe (1.6)
1b095cac94 (1.7)
2021-08-25 21:38:38 +02:00
Tim Duesterhus
7ba98480cc REGTESTS: Use feature cmd for 2.5+ tests
Using `REQUIRE_VERSION` is deprecated for tests targeting HAProxy with `-cc`
support.
2021-08-25 21:38:38 +02:00
Amaury Denoyelle
956be9d242 REGTEST: fix haproxy required version for server removal test
The ability to delete all servers is introduced in 2.5 release.
2021-08-25 16:35:25 +02:00
Amaury Denoyelle
104b8e514d REGTEST: add missing lua requirements on server removal test
The test that removes server via CLI is using LUA to check that servers
referenced in a LUA script cannot be removed. This requires LUA support
to be built in haproxy.

Split the test and create a new one containing only the LUA relevant
test. Mark it as LUA dependant.
2021-08-25 16:34:33 +02:00
Amaury Denoyelle
14c3c5c121 MEDIUM: server: allow to remove servers at runtime except non purgeable
Relax the condition on "delete server" CLI handler to be able to remove
all servers, even non dynamic, except if they are flagged as non
purgeable.

This change is necessary to extend the use cases for dynamic servers
with reload. It's expected that each dynamic server created via the CLI
is manually commited in the haproxy configuration by the user. Dynamic
servers will be present on reload only if they are present in the
configuration file. This means that non-dynamic servers must be allowed
to be removable at runtime.

The dynamic servers removal reg-test has been updated and renamed to
reflect its purpose. A new test is present to check that non-purgeable
servers cannot be removed.
2021-08-25 15:53:54 +02:00
Willy Tarreau
3b2533fa1a REGTESTS: server: fix agent-check syntax and expectation
Since commit 8d6c6bd ("Leak-plugging on barriers") VTest has become
stricter in its expectations, making this one fail. The agent-check
test was expecting a close on the server, which normally does not
happen before the server responds. In addition, it was really sending
"hello" (with the quotes) due to the config file syntax, which explains
why test test log reported that '"hell' was received, and complained
that 0x6f ('o') was read instead of a shutdown. This has been fixed
as well by using single-quotes.

There is no need to backport this test as it's only in 2.5.
2021-08-20 11:28:15 +02:00
Willy Tarreau
1713eec36d REGTESTS: abortonclose: after retries, 503 is expected, not close
The abortonclose test was only expecting a close after all server
retries were exhausted, it didn't check for the pending 503, which
fails with new versions of vtest starting with commit 8d6c6bd
("Leak-plugging on barriers").

This may be backported, but carefully in case older versions would
really close without responding.
2021-08-20 11:12:47 +02:00
Willy Tarreau
ab4fa24cd8 REGTESTS: http_upgrade: fix incorrect expectation on TCP->H1->H2
Commit e1b9e1bb1 ("REGTESTS: Add script to tests TCP to HTTP upgrades")
included a mistake in the TCP->H1->H2 test, it expected a close while
it ought to expect a 400 bad req, which is what the mux returns in this
case. It happens that this used to work fine with older versions of
vtest which see the close regardless of the 400, but since Vtest commit
8d6c6bd ("Leak-plugging on barriers"), this doesn't work anymore.

Let's fix this by expecting the proper response. This should be backported
where this regtest is present, but only after verifying that it still
works; indeed at the time of writing it's uncertain whether an earlier
version used to immediately close.
2021-08-20 11:02:28 +02:00
Remi Tricot-Le Breton
74f6ab6e87 MEDIUM: ssl: Keep a reference to the client's certificate for use in logs
Most of the SSL sample fetches related to the client certificate were
based on the SSL_get_peer_certificate function which returns NULL when
the verification process failed. This made it impossible to use those
fetches in a log format since they would always be empty.

The patch adds a reference to the X509 object representing the client
certificate in the SSL structure and makes use of this reference in the
fetches.

The reference can only be obtained in ssl_sock_bind_verifycbk which
means that in case of an SSL error occurring before the verification
process ("no shared cipher" for instance, which happens while processing
the Client Hello), we won't ever start the verification process and it
will be impossible to get information about the client certificate.

This patch also allows most of the ssl_c_XXX fetches to return a usable
value in case of connection failure (because of a verification error for
instance) by making the "conn->flags & CO_FL_WAIT_XPRT" test (which
requires a connection to be established) less strict.

Thanks to this patch, a log-format such as the following should return
usable information in case of an error occurring during the verification
process :
    log-format "DN=%{+Q}[ssl_c_s_dn] serial=%[ssl_c_serial,hex] \
                hash=%[ssl_c_sha1,hex]"

It should answer to GitHub issue #693.
2021-08-19 23:26:05 +02:00
Amaury Denoyelle
7ef244d73b REGTESTS: add a test to prevent h2 desync attacks
This test ensure that h2 pseudo headers are properly checked for invalid
characters and the host header is ignored if :authority is present. This
is necessary to prevent h2 desync attacks as described here
https://portswigger.net/research/http2
2021-08-17 10:22:20 +02:00
Ilya Shipitsin
01881087fc CLEANUP: assorted typo fixes in the code and comments
This is 25th iteration of typo fixes
2021-08-16 12:37:59 +02:00
Amaury Denoyelle
a026783bd7 REGTESTS: server: fix dynamic server with checks test
Add a missing 'rxreq' statement in first server. Without it the test is
unstable. The issue is frequent when running with one thread only.

This should fix github issue #1342.
2021-08-06 15:34:04 +02:00
Amaury Denoyelle
3e7d468e80 REGTESTS: server: add dynamic check server test
Write a regtest to validate check support by dynamic servers. Three
differents servers are added on various configuration :
- server OK
- server DOWN
- agent-check
2021-08-06 11:22:01 +02:00
William Lallemand
56f1f75715 MINOR: log: rename 'dontloglegacyconnerr' to 'log-error-via-logformat'
Rename the 'dontloglegacyconnerr' option to 'log-error-via-logformat'
which is much more self-explanatory and readable.

Note: only legacy keywords don't use hyphens, it is recommended to
separate words with them in new keywords.
2021-08-02 10:42:42 +02:00
William Lallemand
4f59c67c4f REGTESTS: ssl: ssl_errors.vtc does not work with old openssl version
Disable the new ssl_errors.vtc reg-tests because in does not work
correctly on the CI since it requires a version of OpenSSL which is
compatible with TLSv1.3 and the ciphersuites keyword.
2021-07-29 16:00:24 +02:00
Remi Tricot-Le Breton
54f63836d2 REGTESTS: ssl: Add tests for the connection and SSL error fetches
This reg-test checks that the connection and SSL sample fetches related
to errors are functioning properly. It also tests the proper behaviour
of the default HTTPS log format and of the log-legacy-conn-error option
which enables or disables the output of a special error message in case
of connection failure (otherwise a line following the configured
log-format is output).
2021-07-29 15:40:45 +02:00
Willy Tarreau
f2e44d4e87 REGTESTS: add more complex check conditions to check_conditions.vtc
Now that we support logic expressions, variables and parenthesis, let's
add a few more tests to check_conditions.vtc. The tests are conditionned
by the version being at least 2.5-dev2 so that it will not cause failures
during a possible later bisect session or if backported.

The test verifies that exported variables are seen, that operators precedence
works as expected, that parenthesis work at least through two levels, that an
empty condition is false while a negative number is true, and that extraneous
chars in an expression, or unfinished strings are properly caught.
2021-07-17 11:01:47 +02:00
Willy Tarreau
b333db3fd2 REGTEST: make check_condition.vtc fail as soon as possible
The test consists in a sequence of shell commands, but the shell is not
necessarily started with strict errors enabled, so only the last command
provides the verdict. Let's add "set -e" to make it fail on the first
test that fails.
2021-07-17 10:56:32 +02:00
Amaury Denoyelle
79c52ec6b4 REGTESTS: test track support for dynamic servers
Create a regtest for the 'track' keyword support by dynamic servers.

First checks are executed to ensure that tracking cannot be activated on
non-check server or dynamic servers.

Then, 3 scenarii are written to ensure that the deletion of a dynamic
server with track is properly handled and other servers in the track
chain are properly maintained.
2021-07-16 10:22:58 +02:00
Remi Tricot-Le Breton
0498fa4059 BUG/MINOR: ssl: Default-server configuration ignored by server
When a default-server line specified a client certificate to use, the
frontend would not take it into account and create an empty SSL context,
which would raise an error on the backend side ("peer did not return a
certificate").

This bug was introduced by d817dc733e in
which the SSL contexts are created earlier than before (during the
default-server line parsing) without setting it in the corresponding
server structures. It then made the server create an empty SSL context
in ssl_sock_prepare_srv_ctx because it thought it needed one.

It was raised on redmine, in Bug #3906.

It can be backported to 2.4.
2021-07-13 18:35:38 +02:00
Amaury Denoyelle
ff5adf82a5 REGTESTS: add http scheme-based normalization test
This test ensure that http scheme-based normalization is properly
applied on target URL and host header. It uses h2 clients as it is not
possible to specify an absolute url for h1 vtc clients.
2021-07-07 15:34:01 +02:00
Christopher Faulet
0de0becf0b BUG/MINOR: mqtt: Support empty client ID in CONNECT message
As specified by the MQTT specification (MQTT-3.1.3-6), the client ID may be
empty. That means the length of the client ID string may be 0. However, The
MQTT parser does not support empty strings.

So, to fix the bug, the mqtt_read_string() function may now parse empty
string. 2 bytes must be found to decode the string length, but the length
may be 0 now. It is the caller responsibility to test the string emptiness
if necessary. In addition, in mqtt_parse_connect(), the client ID may be
empty now.

This patch should partely fix the issue #1310. It must be backported to 2.4.
2021-06-28 16:29:44 +02:00
Christopher Faulet
ca925c9c28 BUG/MINOR: mqtt: Fix parser for string with more than 127 characters
Parsing of too long strings (> 127 characters) was buggy because of a wrong
cast on the length bytes. To fix the bug, we rely on mqtt_read_2byte_int()
function. This way, the string length is properly decoded.

This patch should partely fix the issue #1310. It must be backported to 2.4.
2021-06-28 16:29:44 +02:00
Amaury Denoyelle
e500e593a7 REGTESTS: fix maxconn update with agent-check
Correct the typo in the parameter used to update the 'maxconn' via
agent-check. The test is also completed to detect the update of maxconn
using CLI 'show stats'.
2021-06-22 16:34:23 +02:00
Amaury Denoyelle
0ffad2d76c REGTESTS: server: test ssl support for dynamic servers
Create a new regtest to test SSL support for dynamic servers.

The first step of the test is to create the ca-file via the CLI. Then a
dynamic server is created with the ssl option using the ca-file. A
client request is made through it to achieve the test.
2021-06-18 16:49:58 +02:00
Tim Duesterhus
3bc6af417d BUG/MINOR: cache: Correctly handle existing-but-empty 'accept-encoding' header
RFC 7231#5.3.4 makes a difference between a completely missing
'accept-encoding' header and an 'accept-encoding' header without any values.

This case was already correctly handled by accident, because an empty accept
encoding does not match any known encoding. However this resulted in the
'other' encoding being added to the bitmap. Usually this also succeeds in
serving cached responses, because the cached response likely has no
'content-encoding', thus matching the identity case instead of not serving the
response, due to the 'other' encoding. But it's technically not 100% correct.

Fix this by special-casing 'accept-encoding' values with a length of zero and
extend the test to check that an empty accept-encoding is correctly handled.
Due to the reasons given above the test also passes without the change in
cache.c.

Vary support was added in HAProxy 2.4. This fix should be backported to 2.4+.
2021-06-18 15:48:20 +02:00
Christopher Faulet
c7b391aed2 BUG/MEDIUM: server/cli: Fix ABBA deadlock when fqdn is set from the CLI
To perform servers resolution, the resolver's lock is first acquired then
the server's lock when necessary. However, when the fqdn is set via the CLI,
the opposite is performed. So, it is possible to experience an ABBA
deadlock.

To fix this bug, the server's lock is acquired and released for each
subcommand of "set server" with an exception when the fqdn is set. The
resolver's lock is first acquired. Of course, this means we must be sure to
have a resolver to lock.

This patch must be backported as far as 1.8.
2021-06-17 16:52:14 +02:00
Tim Duesterhus
4ee192f072 REGTESTS: Replace REQUIRE_BINARIES with 'command -v'
This migrates the tests to the native `feature cmd` functionality of VTest.
2021-06-17 14:59:55 +02:00
Tim Duesterhus
c9570483b0 REGTESTS: Replace REQUIRE_OPTIONS with 'haproxy -cc' for 2.5+ tests
This migrates the tests for HAProxy versions that support '-cc' to the native
VTest functionality.
2021-06-17 14:59:55 +02:00
Tim Duesterhus
5efc48dcf1 REGTESTS: Replace REQUIRE_VERSION=2.5 with 'haproxy -cc'
This is safe, because running `haproxy -cc 'version_atleast(2.5-dev0)'` on
HAProxy 2.4 will also result in an exit code of 1.
2021-06-17 14:59:55 +02:00
Tim Duesterhus
1b095cac94 REGTESTS: Remove REQUIRE_VERSION=1.7 from all tests
HAProxy 1.7 is the lowest supported version, thus this always matches.
2021-06-11 19:21:28 +02:00
Tim Duesterhus
d8be0018fe REGTESTS: Remove REQUIRE_VERSION=1.6 from all tests
HAProxy 1.6 is EOL, thus this always matches.
2021-06-11 19:21:28 +02:00
Willy Tarreau
b63dbb7b2e MAJOR: config: remove parsing of the global "nbproc" directive
This one was deprecated in 2.3 and marked for removal in 2.5. It suffers
too many limitations compared to threads, and prevents some improvements
from being engaged. Instead of a bypassable startup error, there is now
a hard error.

The parsing code was removed, and very few obvious cases were as well.
The code is deeply rooted at certain places (e.g. "for" loops iterating
from 0 to nbproc) so it will not be that trivial to remove everywhere.
The "bind" and "bind-process" parsers will have to be adjusted, though
maybe not completely changed if we later want to support thread groups
for large NUMA machines. Some stats socket restrictions were removed,
and the doc was updated according to what was done. A few places in the
doc still refer to nbproc and will have to be revisited. The master-worker
code also refers to the process number to distinguish between master and
workers and will have to be carefully adjusted. The MAX_PROCS macro was
reset to 1, this will at least reduce the size of some remaining arrays.

Two regtests were dependieng on this directive, one with an explicit
"nbproc 1" and another one testing the master's CLI using nbproc 4.
Both were adapted.
2021-06-11 17:02:13 +02:00
William Lallemand
0061323114 REGTESTS: ssl: show_ssl_ocspresponce.vtc is broken with BoringSSL
The `show ssl ocsp-response` feature is not available with BoringSSL,
but we don't have a way to disable this feature only with boringSSL on
the CI. Disable the reg-test until we do.
2021-06-11 10:03:08 +02:00
Remi Tricot-Le Breton
2a77c62c18 REGTESTS: ssl: Add "show ssl ocsp-response" test
This file adds tests for the new "show ssl ocsp-response" command and
the new "show ssl cert foo.pem.ocsp" and "show ssl cert *foo.pem.ocsp"
special cases. They are all used to display information about an OCSP
response, committed or not.
2021-06-10 16:44:11 +02:00
Maximilian Mader
fc0cceb08a MINOR: haproxy: Add -cc argument
This patch adds the `-cc` (check condition) argument to evaluate conditions on
startup and return the result as the exit code.

As an example this can be used to easily check HAProxy's version in scripts:

    haproxy -cc 'version_atleast(2.4)'

This resolves GitHub issue #1246.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
2021-06-08 11:17:19 +02:00
Tim Duesterhus
a9334df5a9 CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests
The legacy HTTP subsystem has been removed. HTX is always enabled.
2021-06-04 15:41:21 +02:00
Christopher Faulet
3b9cb60059 REGTESTS: Fix http_abortonclose.vtc to support -1 status for some client aborts
Since the commit 5e702fcad ("MINOR: http-ana: Use -1 status for client
aborts during queuing and connect"), -1 status is reported in the log
message when the client aborts during queuing and
connect. http_abortonclose.vtc script must be update accordingly.
2021-06-02 17:23:48 +02:00
Remi Tricot-Le Breton
a3b2e099c2 REGTESTS: ssl: Add "set/commit ssl crl-file" test
This file adds tests for the new "set ssl crl-file" and "commit ssl
crl-file" commands which allow the hot update of CRL file through CLI
commands.
2021-05-17 10:50:24 +02:00
Remi Tricot-Le Breton
f615070bcc REGTESTS: ssl: Add "new/del ssl crl-file" tests
This vtc tests the "new ssl crl-file" which allows to create a new empty
CRL file that can then be set through a "set+commit ssl crl-file"
command pair. It also tests the "del ssl crl-file" command which allows
to delete an unused CRL file.
2021-05-17 10:50:24 +02:00
Remi Tricot-Le Breton
efcc5b28d1 REGTESTS: ssl: Add "new/del ssl ca-file" tests
This vtc tests the "new ssl ca-file" which allows to create a new empty
CA file that can then be set through a "set+commit ssl ca-file" command
pair. It also tests the "del ssl ca-file" command which allows to delete
an unused CA file.
2021-05-17 10:50:24 +02:00
Remi Tricot-Le Breton
2a22e16cb8 MEDIUM: ssl: Add "show ssl ca-file" CLI command
This patch adds the "show ssl ca-file [<cafile>[:index]]" CLI command.
This command can be used to display the list of all the known CA files
when no specific file name is specified, or to display the details of a
specific CA file when a name is given. If an index is given as well, the
command will only display the certificate having the specified index in
the CA file (if it exists).
The details displayed for each certificate are the same as the ones
showed when using the "show ssl cert" command on a single certificate.

This fixes a subpart of GitHub issue #1057.
2021-05-17 10:50:24 +02:00
Remi Tricot-Le Breton
d5fd09d339 MINOR: ssl: Add "abort ssl ca-file" CLI command
The "abort" command aborts an ongoing transaction started by a "set ssl
ca-file" command. Since the updated CA file data is not pushed into the
cafile tree until a "commit ssl ca-file" call is performed, the abort
command simply clears the new cafile_entry that was stored in the
cafile_transaction.

This fixes a subpart of GitHub issue #1057.
2021-05-17 10:50:24 +02:00
Remi Tricot-Le Breton
2db6101ed7 REGTESTS: ssl: Add new ca-file update tests
This vtc tests the "set ssl ca-file" and "commit ssl ca-file" cli
commands. Those commands allow the hot update of CA files through cli
commands.
2021-05-17 10:50:24 +02:00
Amaury Denoyelle
94fd1339e7 REGTESTS: stick-table: add src_conn_rate test
Add a simple test which uses src_conn_rate stick table fetch. Limit the
connection rate to 3. The 4th connection should return a 403.
2021-05-12 15:30:03 +02:00
Tim Duesterhus
dec1c36b3a MINOR: uri_normalizer: Add fragment-encode normalizer
This normalizer encodes '#' as '%23'.

See GitHub Issue #714.
2021-05-11 17:24:32 +02:00
Tim Duesterhus
c9e05ab2de MINOR: uri_normalizer: Add fragment-strip normalizer
This normalizer strips the URI's fragment component which should never be sent
to the server.

See GitHub Issue #714.
2021-05-11 17:23:46 +02:00
Willy Tarreau
e1465c1e46 REGTESTS: disable inter-thread idle connection sharing on sensitive tests
Some regtests involve multiple requests from multiple clients, which can
be dispatched as multiple requests to a server. It turns out that the
idle connection sharing works so well that very quickly few connections
are used, and regularly some of the remaining idle server connections
time out at the moment they were going to be reused, causing those random
"HTTP header incomplete" traces in the logs that make them fail often. In
the end this is only an artefact of the test environment.

And indeed, some tests like normalize-uri which perform a lot of reuse
fail very often, about 20-30% of the times in the CI, and 100% of the
time in local when running 1000 tests in a row. Others like ubase64,
sample_fetches or vary_* fail less often but still a lot in tests.

This patch addresses this by adding "tune.idle-pool.shared off" to all
tests which have at least twice as many requests as clients. It proves
very effective as no single error happens on normalize-uri anymore after
10000 tests. Also 100 full runs of all tests yield no error anymore.

One test is tricky, http_abortonclose, it used to fail ~10 times per
1000 runs and with this workaround still fails once every 1000 runs.
But the test is complex and there's a warning in it mentioning a
possible issue when run in parallel due to a port reuse.
2021-05-09 14:41:41 +02:00
Amaury Denoyelle
a9e639afe2 MINOR: http_act: mark normalize-uri as experimental
normalize-uri http rule is marked as experimental, so it cannot be
activated without the global 'expose-experimental-directives'. The
associated vtc is updated to be able to use it.
2021-05-07 14:35:02 +02:00
Christopher Faulet
16b37510bc REGTESTS: Add script to test abortonclose option
This script test abortonclose option for HTTP/1 client only. It may be
backported as far as 2.0. But on the 2.2 and prior, the syslog part must be
adapted to catch log messages emitted by proxy during HAProxy
startup. Following lines must be added :

    recv
    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy fe1 started."
    recv
    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy fe2 started."
2021-05-06 09:19:20 +02:00
Willy Tarreau
deee369cfa REGTESTS: add minimal CLI "add map" tests
The map_redirect test already tests for "show map", "del map" and
"clear map" but doesn't have any "add map" command. Let's add some
trivial ones involving one regular entry and two other ones added as
payload, checking they are properly returned.
2021-04-29 16:19:03 +02:00
Amaury Denoyelle
996190a70d REGTESTS: server: fix cli_add_server due to previous trace update
Error output for dynamic server creation if invalid lb algo has changed
since previous commit :
MINOR: server: fix doc/trace on lb algo for dynamic server creation

The vtest regex should have been updated has well to match it.
2021-04-29 15:38:02 +02:00
Tim Duesterhus
2e4a18e04a MINOR: uri_normalizer: Add a percent-decode-unreserved normalizer
This normalizer decodes percent encoded characters within the RFC 3986
unreserved set.

See GitHub Issue #714.
2021-04-23 19:43:45 +02:00
Maximilian Mader
ff3bb8b609 MINOR: uri_normalizer: Add a strip-dot normalizer
This normalizer removes "/./" segments from the path component.
Usually the dot refers to the current directory which renders those segments redundant.

See GitHub Issue #714.
2021-04-21 12:15:14 +02:00
Amaury Denoyelle
e558043e13 MINOR: server: implement delete server cli command
Implement a new CLI command 'del server'. It can be used to removed a
dynamically added server. Only servers in maintenance mode can be
removed, and without pending/active/idle connection on it.

Add a new reg-test for this feature. The scenario of the reg-test need
to first add a dynamic server. It is then deleted and a client is used
to ensure that the server is non joinable.

The management doc is updated with the new command 'del server'.
2021-04-21 11:00:31 +02:00
Tim Duesterhus
5be6ab269e MEDIUM: http_act: Rename uri-normalizers
This patch renames all existing uri-normalizers into a more consistent naming
scheme:

1. The part of the URI that is being touched.
2. The modification being performed as an explicit verb.
2021-04-19 09:05:57 +02:00
Tim Duesterhus
a407193376 MINOR: uri_normalizer: Add a percent-upper normalizer
This normalizer uppercases the hexadecimal characters used in percent-encoding.

See GitHub Issue #714.
2021-04-19 09:05:57 +02:00
Tim Duesterhus
d7b89be30a MINOR: uri_normalizer: Add a sort-query normalizer
This normalizer sorts the `&` delimited query parameters by parameter name.

See GitHub Issue #714.
2021-04-19 09:05:57 +02:00
Tim Duesterhus
560e1a6352 MINOR: uri_normalizer: Add support for supressing leading ../ for dotdot normalizer
This adds an option to supress `../` at the start of the resulting path.
2021-04-19 09:05:57 +02:00
Tim Duesterhus
9982fc2bbd MINOR: uri_normalizer: Add a dotdot normalizer to http-request normalize-uri
This normalizer merges `../` path segments with the predecing segment, removing
both the preceding segment and the `../`.

Empty segments do not receive special treatment. The `merge-slashes` normalizer
should be executed first.

See GitHub Issue #714.
2021-04-19 09:05:57 +02:00