MEDIUM: http-rules: Rely on http reply for http deny/tarpit rules
"http-request deny", "http-request tarpit" and "http-response deny" rules now
use the same syntax than http return rules and internally rely on the http
replies. The behaviour is not the same when no argument is specified (or only
the status code). For http replies, a dummy response is produced, with no
payload. For old deny/tarpit rules, the proxy's error messages are used. Thus,
to be compatible with existing configuration, the "default-errorfiles" parameter
is implied. For instance :
MINOR: http-ana: Use a dedicated function to send a response from an http reply
The http_reply_message() function may be used to send an http reply to a
client. This function is responsile to convert the reply in HTX, to push it in
the response buffer and to forward it to the client. It is also responsible to
terminate the transaction.
This function is used during evaluation of http return rules.
MINOR: http-htx: Use a dedicated function to check http reply validity
A dedicated function is added to check the validity of an http reply object,
after parsing. It is used to check the validity of http return rules.
For now, this function is only used to find the right error message in an
http-errors section for http replies of type HTTP_REPLY_ERRFILES (using
"errorfiles" argument). On success, such replies are updated to point on the
corresponding error message and their type is set to HTTP_REPLY_ERRMSG. If an
unknown http-errors section is referenced, anx error is returned. If a unknown
error message is referenced inside an existing http-errors section, a warning is
emitted and the proxy's error messages are used instead.
MINOR: http-htx: Use a dedicated function to parse http reply arguments
A dedicated function to parse arguments and create an http_reply object is
added. It is used to parse http return rule. Thus, following arguments are
parsed by this function :
MINOR: http-rules: Use http_reply structure for http return rules
No real change here. Instead of using an internal structure to the action rule,
the http return rules are now stored as an http reply. The main change is about
the action type. It is now always set to ACT_CUSTOM. The http reply type is used
to know how to evaluate the rule.
CLEANUP: http-htx: Rename http_error structure into http_error_msg
The structure owns an error message, most of time loaded from a file, and
converted to HTX. It is created when an errorfile or errorloc directive is
parsed. It is renamed to avoid ambiguities with http_reply structure.
MINOR: http-htx: Add http_reply type based on what is used for http return rules
The http_reply structure is added. It represents a generic HTTP message used as
internal response by HAProxy. It is based on the structure used to store http
return rules. The aim is to store all error messages using this structure, as
well as http return and http deny rules.
BUG/MINOR: http-rules: Mark http return rules as final
For HTTP rules, this flag is only used to trigger a warning during HAProxy
startup when a final rule without ACL is not the last one. So this patch is
marked as a bug, but its impact is really limited.
No backport needed because http return rules were introduced in the 2.2.
TX_CLDENY, TX_CLALLOW, TX_SVDENY and TX_SVALLOW flags are unused. Only
TX_CLTARPIT is used to make the difference between an http deny rule and an http
tarpit rule. So these unused flags are removed.
MINOR: ssl: split config and runtime variable for ssl-{min,max}-ver
In the CLI command 'show ssl crt-list', the ssl-min-ver and the
ssl-min-max arguments were always displayed because the dumped versions
were the actual version computed and used by haproxy, instead of the
version found in the configuration.
To fix the problem, this patch separates the variables to have one with
the configured version, and one with the actual version used. The dump
only shows the configured version.
Ilya Shipitsin [Mon, 18 May 2020 20:47:07 +0000 (01:47 +0500)]
CI: travis-ci: fix libslz download URL
let us switch to github mirror, travis cannot download original libslz
error is:
fatal: unable to access 'http://git.1wt.eu/git/libslz.git/': Failed
to connect to git.1wt.eu port 80: Connection timed out The command
"git clone http://git.1wt.eu/git/libslz.git/" failed and exited with
128...
Willy Tarreau [Tue, 19 May 2020 17:14:42 +0000 (19:14 +0200)]
MINOR: ring: make the applet code not depend on the CLI
The ring to applet communication was only made to deal with CLI functions
but it's generic. Let's have generic appctx functions and have the CLI
rely on these instead. This patch introduces ring_attach_appctx() and
ring_detach_appctx().
Willy Tarreau [Tue, 19 May 2020 15:07:30 +0000 (17:07 +0200)]
MINOR: applet: adopt the wait list entry from the CLI
A few fields, including a generic list entry, were added to the CLI context
by commit 300decc8d9 ("MINOR: cli: extend the CLI context with a list and
two offsets"). It turns out that the list entry (l0) is solely used to
consult rings and that the generic ring_write() code is restricted to a
consumer on the CLI due to this, which was not the initial intent. Let's
make it a general purpose wait_entry field that is properly initialized
during appctx_init(). This will allow any applet to wait on a ring, not
just the CLI.
Willy Tarreau [Tue, 19 May 2020 17:21:45 +0000 (19:21 +0200)]
BUG/MEDIUM: ring: write-lock the ring while attaching/detaching
The LIST_ADDQ() and LIST_DEL_INIT() calls made to attach/detach a waiter
to the ring were made under a read lock which was sufficient in front of
the writer's write lock. But it's not sufficient against other readers!
Thus theorically multiple "show events" on the same ring buffer on the
CLI could result in a crash, even though for now I couldn't manage to
reproduce it.
This fixes commit 1d181e489c ("MEDIUM: ring: implement a wait mode for
watchers") so it must be backported to 2.1, possibly further if the ring
code gets backported.
BUG/MAJOR: mux-fcgi: Stop sending loop if FCGI stream is blocked for any reason
Because of a typo error in conditions to exit the sending loop, it is possible
to loop infinitely in fcgi_snd_buf() function. Instead of checking the FCGI
stream is not blocked to continue sending data, the FCGI connection is used. So
it is possible to have a stream blocked because there is not enough space in the
mux buffers to copy more data but continue to loop to send more data.
This patch should fix the issue #637. It must be backported to 2.1.
Willy Tarreau [Tue, 19 May 2020 09:31:11 +0000 (11:31 +0200)]
MEDIUM: hpack: use a pool for the hpack table
Instead of using malloc/free to allocate an HPACK table, let's declare
a pool. However the HPACK size is configured by the H2 mux, so it's
also this one which allocates it after post_check.
Emeric Brun [Mon, 11 May 2020 12:41:31 +0000 (14:41 +0200)]
MEDIUM: sink: add global statement to create a new ring (sink buffer)
This patch adds the new global statement:
ring <name> [desc <desc>] [format <format>] [size <size>] [maxlen <length>]
Creates a named ring buffer which could be used on log line for instance.
<desc> is an optionnal description string of the ring. It will appear on
CLI. By default, <name> is reused to fill this field.
<format> is the log format used when generating syslog messages. It may be
one of the following :
iso A message containing only the ISO date, followed by the text.
The PID, process name and system name are omitted. This is
designed to be used with a local log server.
raw A message containing only the text. The level, PID, date, time,
process name and system name are omitted. This is designed to be
used in containers or during development, where the severity only
depends on the file descriptor used (stdout/stderr). This is
the default.
rfc3164 The RFC3164 syslog message format. This is the default.
(https://tools.ietf.org/html/rfc3164)
rfc5424 The RFC5424 syslog message format.
(https://tools.ietf.org/html/rfc5424)
short A message containing only a level between angle brackets such as
'<3>', followed by the text. The PID, date, time, process name
and system name are omitted. This is designed to be used with a
local log server. This format is compatible with what the systemd
logger consumes.
timed A message containing only a level between angle brackets such as
'<3>', followed by ISO date and by the text. The PID, process
name and system name are omitted. This is designed to be
used with a local log server.
<length> is the maximum length of event message stored into the ring,
including formatted header. If the event message is longer
than <length>, it would be truncated to this length.
<name> is the ring identifier, which follows the same naming convention as
proxies and servers.
<size> is the optionnal size in bytes. Default value is set to BUFSIZE.
Note: Historically sink's name and desc were refs on const strings. But with new
configurable rings a dynamic allocation is needed.
Emeric Brun [Wed, 6 May 2020 15:23:59 +0000 (17:23 +0200)]
MEDIUM: logs: buffer targets now rely on new sink_write
Before this path, they rely directly on ring_write bypassing
a part of the sink API.
Now the maxlen parameter of the log will apply only on the text
message part (and not the header, for this you woud prefer
to use the maxlen parameter on the sink/ring).
sink_write prototype was also reviewed to return the number of Bytes
written to be compliant with the other write functions.
Emeric Brun [Wed, 6 May 2020 12:33:46 +0000 (14:33 +0200)]
MEDIUM: sink: build header in sink_write for log formats
This patch extends the sink_write prototype and code to
handle the rfc5424 and rfc3164 header.
It uses header building tools from log.c. Doing this some
functions/vars have been externalized.
facility and minlevel have been removed from the struct sink
and passed to args at sink_write because they depends of the log
and not of the sink (they remained unused by rest of the code
until now).
Emeric Brun [Tue, 12 May 2020 17:33:15 +0000 (19:33 +0200)]
BUG/MINOR: logs: prevent double line returns in some events.
Historically some messages used to already contain the trailing LF but
not all, and __do_send_log adds a new one in needed cases. It also does
trim a trailing LF in certain cases while computing the max message
length, as a result of subtracting 1 to the available room in the
destination buffer. But the way it's done is wrong since some messages
still contain it.
So the code was fixed to always trim the trailing LF from messages if
present, and then only subtract 1 from the destination buffer room
instead of the size..
Note: new sink API is not designed to receive a trailing LF on
event messages
This could be backported to relevant stable versions with particular
care since the logic of the code changed a bit since 1.6 and there
may be other locations that need to be adjusted.
MEDIUM: checks: Make post-41 the default mode for mysql checks
MySQL 4.1 is old enough to be the default mode for mysql checks. So now, once a
username is defined, post-41 mode is automatically used. To do mysql checks on
previous MySQL version, the argument "pre-41" must be used.
Note, it is a compatibility breakage for everyone using an antique and
unsupported MySQL version.
MINOR: config: Don't dump keywords if argument is NULL
Helper functions are used to dump bind, server or filter keywords. These
functions are used to report errors during the configuration parsing. To have a
coherent API, these functions are now prepared to handle a null pointer as
argument. If so, no action is performed and functions immediately return.
This patch should fix the issue #631. It is not a bug. There is no reason to
backport it.
BUG/MINOR: cache: Don't needlessly test "cache" keyword in parse_cache_flt()
parse_cache_flt() is the registered callback for the "cache" filter keyword. It
is only called when the "cache" keyword is found on a filter line. So, it is
useless to test the filter name in the callback function.
This patch should fix the issue #634. It may be backported as far as 1.9.
Ilya Shipitsin [Sat, 16 May 2020 17:42:12 +0000 (22:42 +0500)]
CLEANUP: acl: remove unused assignment
Coverity found unused variable assignment
CID 1299671 (#1 of 1): Unused value (UNUSED_VALUE)assigned_pointer:
Assigning value from args[arg + 1] to word here, but that stored
value is overwritten before it can be used.
958 word = args[arg + 1];
959 arg = arg_end;
William Dauchy [Sun, 17 May 2020 11:41:53 +0000 (13:41 +0200)]
BUILD: ssl: include buffer common headers for ssl_sock_ctx
since commit c0cdaffaa338 ("REORG: ssl: move ssl_sock_ctx and fix
cross-dependencies issues"), `struct ssl_sock_ctx` was moved in
ssl_sock.h. As it contains a `struct buffer`, including
`common/buffer.h` is now mandatory. I encountered an issue while
including ssl_sock.h on another patch:
include/types/ssl_sock.h:240:16: error: field ‘early_buf’ has incomplete type
240 | struct buffer early_buf; /* buffer to store the early data received */
no backport needed.
Fixes: c0cdaffaa338 ("REORG: ssl: move ssl_sock_ctx and fix
cross-dependencies issues") Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Marcin Deranek [Fri, 15 May 2020 16:32:51 +0000 (18:32 +0200)]
MINOR: stats: Expose native cum_req metric for a server
Expose native cum_req metric for a server: so far it was calculated as a
sum or all responses. Rename it from Cum. HTTP Responses to Cum. HTTP
Requests to be consistent with Frontend and Backend.
Marcin Deranek [Fri, 15 May 2020 16:26:18 +0000 (18:26 +0200)]
MINOR: stats: Prepare for more accurate moving averages
Add swrate_add_dynamic function which is similar to swrate_add, but more
accurate when calculating moving averages when not enough samples have
been processed yet.
CI: travis-ci: switch arm64 builds to use openssl from distro
there are ongoing arm64 failures in travis-ci. we suspect that
build stucks on scripts/build-ssl.sh which is called during build.
let us try to use openssl from ubuntu distro and not build it
ourselves. to achive that we add "install" section without
build-ssl.sh
REORG: ssl: move ssl_sock_ctx and fix cross-dependencies issues
In order to move all SSL sample fetches in another file, moving the
ssl_sock_ctx definition in a .h file is required.
Unfortunately it became a cross dependencies hell to solve, because of
the struct wait_event field, so <types/connection.h> is needed which
created other problems.
CLEANUP: ssl: avoid circular dependencies in ssl_crtlist.h
Add forward declarations in types/ssl_crtlist.h in order to avoid
circular dependencies. Also remove the listener.h include which is not
needed anymore.
REORG: ssl: move macros and structure definitions to ssl_sock.h
The ssl_sock.c file contains a lot of macros and structure definitions
that should be in a .h. Move them to the more appropriate
types/ssl_sock.h file.
Dragan Dosen [Mon, 11 May 2020 13:51:45 +0000 (15:51 +0200)]
MEDIUM: ssl: split ssl_sock_msgcbk() and use a new callback mechanism
Make use of ssl_sock_register_msg_callback(). Function ssl_sock_msgcbk()
is now split into two dedicated functions for heartbeat and clienthello.
They are both registered by using a new callback mechanism for SSL/TLS
protocol messages.
Jerome Magnin [Wed, 13 May 2020 18:09:57 +0000 (20:09 +0200)]
DOC: retry-on can only be used with mode http
The documentation for retry-on hints at it being meant to be used
in conjuction with mode http, but since we've a had bug report
involving mode tcp and retry-on, lets make it explicit in the
documentation that it only works with mode http and will be
ignored otherwise.
Olivier Houchard [Wed, 13 May 2020 17:07:20 +0000 (19:07 +0200)]
BUG/MEDIUM: stream: Only allow L7 retries when using HTTP.
Only allow L7 retries when using HTTP, it only really makes sense for HTTP,
anyway, and as the L7 retries code assume the message will be HTX, it will
crash when used with mode TCP.
This should fix github issue #627.
Olivier Houchard [Tue, 12 May 2020 20:18:14 +0000 (22:18 +0200)]
BUG/MEDIUM: streams: Remove SF_ADDR_SET if we're retrying due to L7 retry.
In do_l7_retry(), remove the SF_ADDR_SET flag. Otherwise,
assign_server_address() won't be called again, which means for 2.1 or 2.2,
we will always retry to connect to the server that just failed, and for 2.0,
that we will try to use to whatever the address is for the connection,
probably the last server used by that connection before it was pool_free()
and reallocated.
Jerome Magnin [Wed, 13 May 2020 13:11:02 +0000 (15:11 +0200)]
BUILD: select: only declare existing local labels to appease clang
Commit 42a50bd19 ("BUG/MINOR: pollers: remove uneeded free in global
init") removed the 'fail_revt' label from the _do_init() function
in src/ev_select.c but left the local label declaration, which makes
clang unhappy and unable to build.
These labels are only historic and unneeded anyway so let's remove them.
This should be backported where 42a50bd19 is backported.
Willy Tarreau [Wed, 13 May 2020 12:30:25 +0000 (14:30 +0200)]
MINOR: soft-stop: let the first stopper only signal other threads
When the first thread stops and wakes others up, it's possible some of
them will also start to wake others in parallel. Let's make give this
notification task to the very first one instead since it's enough and
can reduce the amount of needless (though harmless) wakeup calls.
Willy Tarreau [Wed, 13 May 2020 11:51:01 +0000 (13:51 +0200)]
BUG/MINOR: soft-stop: always wake up waiting threads on stopping
Currently the soft-stop can lead to old processes remaining alive for as
long as two seconds after receiving a soft-stop signal. What happens is
that when receiving SIGUSR1, one thread (usually the first one) wakes up,
handles the signal, sets "stopping", goes into runn_poll_loop(), and
discovers that stopping is set, so its also sets itself in the
stopping_thread_mask bit mask. After this it sees that other threads are
not yet willing to stop, so it continues to wait.
From there, other threads which were waiting in poll() expire after one
second on poll timeout and enter run_poll_loop() in turn. That's already
one second of wait time. They discover each in turn that they're stopping
and see that other threads are not yet stopping, so they go back waiting.
After the end of the first second, all threads know they're stopping and
have set their bit in stopping_thread_mask. It's only now that those who
started to wait first wake up again on timeout to discover that all other
ones are stopping, and can now quit. One second later all threads will
have done it and the process will quit.
This is effectively strictly larger than one second and up to two seconds.
What the current patch does is simple, when the first thread stops, it sets
its own bit into stopping_thread_mask then wakes up all other threads to do
also set theirs. This kills the first second which corresponds to the time
to discover the stopping state. Second, when a thread exists, it wakes all
other ones again because some might have gone back sleeping waiting for
"jobs" to go down to zero (i.e. closing the last connection). This kills
the last second of wait time.
Thanks to this, as SIGUSR1 now acts instantly again if there's no active
connection, or it stops immediately after the last connection has left if
one was still present.
William Dauchy [Mon, 11 May 2020 13:20:03 +0000 (15:20 +0200)]
BUG/MINOR: pollers: remove uneeded free in global init
Since commit d4604adeaa8c ("MAJOR: threads/fd: Make fd stuffs
thread-safe"), we init pollers per thread using a helper. It was still
correct for mono-thread mode until commit cd7879adc2c4 ("BUG/MEDIUM:
threads: Run the poll loop on the main thread too"). We now use a deinit
helper for all threads, making those free uneeded.
Only poll and select are affected by this very minor issue.
it could be backported from v1.8 to v2.1.
Fixes: cd7879adc2c4 ("BUG/MEDIUM: threads: Run the poll loop on the main
thread too") Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Willy Tarreau [Wed, 6 May 2020 15:10:37 +0000 (17:10 +0200)]
BUG/MINOR: pools: use %u not %d to report pool stats in "show pools"
In dump_pools_to_trash() we happen to use %d to display unsigned ints!
This has probably been there since "show pools" was introduced so this
fix must be backported to all versions. The impact is negligible since
no pool uses 2 billion entries. It could possibly affect the report of
failed allocation counts but in this case there's a bigger problem to
solved!
BUG/MINOR: sample/ssl: Fix digest converter for openssl < 1.1.0
The EVP_MD_CTX_create() and EVP_MD_CTX_destroy() functions were renamed to
EVP_MD_CTX_new() and EVP_MD_CTX_free() in OpenSSL 1.1.0, respectively. These
functions are used by the digest converter, introduced by the commit 8e36651ed
("MINOR: sample: Add digest and hmac converters"). So for prior versions of
openssl, macros are used to fallback on old functions.
This patch must only be backported if the commit 8e36651ed is backported too.
BUG/MEDIUM: checks: Subscribe to I/O events only if a mux was installed
In the commit 2fabd9d53 ("BUG/MEDIUM: checks: Subscribe to I/O events on an
unfinished connect"), we force the subscribtion to I/O events when a new
connection is opened if it is not fully established. But it must only be done if
a mux was immediately installed. If there is no mux, no subscription must be
performed.
BUG/MINOR: checks: Rely on next I/O oriented rule when waiting for a connection
In tcpcheck_main(), when we are waiting for a connection, we must rely on the
next connect/send/expect rule to subscribe to I/O events, not on the immediate
next rule. Because, if it is a set-var or an unset-var rule, we will not
subscribe to I/O events while it is in fact mandatory because a send or an
expect rule is coming. It is required to wake-up the health check as soon as I/O
are possible, instead of hitting a timeout.
BUG/MEDIUM: checks: Subscribe to I/O events on an unfinished connect
In tcp-check based health check, when a new connection is opened, we must wait
it is really established before moving to the next rule. But at this stage, we
must also be sure to subscribe to I/O events. Otherwise, depending on the
timing, the health check may remains sleepy till the timeout.
No backport needed. This patch should fix the issue #622.
MAJOR: contrib: porting spoa_server to support python3
Background:
Python 2 is no longer supported since January, 1st 2020 as per
https://www.python.org/doc/sunset-python-2/
The purpose of this change is to make the spoa_server contrib library
compatible with Python 3 to allow transition to Python 3.
Test Settings:
ps_python.py:
...
spoa.set_var_null("null", spoa.scope_txn)
spoa.set_var_boolean("boolean", spoa.scope_txn, True)
spoa.set_var_int32("int32", spoa.scope_txn, 1234)
spoa.set_var_uint32("uint32", spoa.scope_txn, 1234)
spoa.set_var_int64("int64", spoa.scope_txn, 1234)
spoa.set_var_uint64("uint64", spoa.scope_txn, 1234)
spoa.set_var_ipv4("ipv4", spoa.scope_txn, ipaddress.IPv4Address(u"127.0.0.1"))
spoa.set_var_ipv6("ipv6", spoa.scope_txn, ipaddress.IPv6Address(u"1::f"))
spoa.set_var_str("str", spoa.scope_txn, "1::f")
spoa.set_var_bin("bin", spoa.scope_txn, "1:\x01:\x42f\x63\x63")
spoa.set_var_str("python_version", spoa.scope_sess, str(sys.version_info))
...
haproxy.cfg:
...
http-request capture var(txn.verb.null),debug len 1
http-request capture var(txn.verb.boolean),debug len 1
http-request capture var(txn.verb.int32),debug len 4
http-request capture var(txn.verb.uint32),debug len 4
http-request capture var(txn.verb.int64),debug len 4
http-request capture var(txn.verb.uint64),debug len 4
http-request capture var(txn.verb.ipv4),debug len 16
http-request capture var(txn.verb.ipv6),debug len 45
http-request capture var(txn.verb.str),debug len 32
http-request capture var(txn.verb.bin),debug len 32
http-request capture var(sess.verb.python_version),debug len 100
...
Willy Tarreau [Sat, 9 May 2020 07:08:09 +0000 (09:08 +0200)]
CLEANUP: remove THREAD_LOCAL from config.h
This one really ought to be defined in hathreads.h like all other thread
definitions, which is what this patch does. As expected, all files but
one (regex.h) were already including hathreads.h when using THREAD_LOCAL;
regex.h was fixed for this.
This was the last entry in config.h which is now useless.
Willy Tarreau [Sat, 9 May 2020 07:02:35 +0000 (09:02 +0200)]
CLENAUP: config: move CONFIG_HAP_LOCKLESS_POOLS out of config.h
The setting of CONFIG_HAP_LOCKLESS_POOLS depending on threads and
compat was done in config.h for use only in memory.h and memory.c
where other settings are dealt with. Further, the default pool cache
size was set there from a fixed value instead of being set from
defaults.h
Let's move the decision to enable lockless pools via
CONFIG_HAP_LOCKLESS_POOLS to memory.h, and set the default pool
cache size in defaults.h like other default settings.
Willy Tarreau [Sat, 9 May 2020 06:53:31 +0000 (08:53 +0200)]
CLEANUP: config: drop unused setting CONFIG_HAP_MEM_OPTIM
CONFIG_HAP_MEM_OPTIM was introduced with memory pools in 1.3 and dropped
in 1.6 when pools became the only way to allocate memory. Still the
option remained present in config.h. Let's kill it.
Willy Tarreau [Fri, 8 May 2020 12:18:50 +0000 (14:18 +0200)]
CI: travis-ci: extend the build time for SSL to 60 minutes
As discussed in the thread below we're still facing build timeouts
on Travis on aarch64 and the truncated logs doesn't help. It's not
PCRE2 so let's try to extend the permitted time to build the SSL
lib.
Willy Tarreau [Fri, 8 May 2020 07:31:18 +0000 (09:31 +0200)]
MEDIUM: ssl: increase default-dh-param to 2048
For 6 years now we've been seeing a warning suggesting to set dh-param
beyond 1024 if possible when it was not set. It's about time to do it
and get rid of this warning since most users seem to already use 2048.
It will remain possible to set a lower value of course, so only those
who were experiencing the warning and were relying on the default value
may notice a change (higher CPU usage). For more context, please refer
to this thread :
Tim Duesterhus [Thu, 7 May 2020 17:21:31 +0000 (19:21 +0200)]
BUG/MINOR: cfgparse: Abort parsing the current line if an invalid \x sequence is encountered
This fixes OSS Fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=21931.
OSS Fuzz detected a hang on configuration parsing for a 200kB line with a large number of
invalid escape sequences. Most likely due to the amounts of error output generated.
This issue is very minor, because usually generated configurations are to be trusted.
The bug exists since at the very least HAProxy 1.4. The patch may be backported if desired.
Willy Tarreau [Thu, 7 May 2020 17:27:02 +0000 (19:27 +0200)]
BUG/MEDIUM: http_ana: make the detection of NTLM variants safer
In issue #511 a problem was reported regarding NTLM and undesired session
sharing. This was caused by an attempt to limit the protection against
NTLM breakage to just NTLM and not properly working schemes in commit fd9b68c48 ("BUG/MINOR: only mark connections private if NTLM is detected").
Unfortunately as reported in the issue above, the extent of possible
challenges for NTLM is a bit more complex than just the "NTLM" or
"Negotiate" words. There's also "Nego2" and these words can be followed
by a base64 value, which is not validated here. The list of possible
entries doesn't seem to be officially documented but can be reconstructed
from different public documents:
This patch tries to fix all this on top of previous attempts by making
as private any connection that returns a www-authenticate header starting
with "Nego" or "NTLM". We don't need to be too strict, we really just
want to leave the connection shared if really sure it can be.
This must be backported to 1.8 but will require some adaptations. In
1.9 and 2.0 the check appears both for legacy and HTX. The simplest
thing to do is to look for "Negotiate" and fix all relevant places.
Willy Tarreau [Thu, 7 May 2020 17:10:15 +0000 (19:10 +0200)]
BUG/MINOR: http-ana: fix NTLM response parsing again
Commit 9df188695f ("BUG/MEDIUM: http-ana: Handle NTLM messages correctly.")
tried to address an HTTP-reuse issue reported in github issue #511 by making
sure we properly detect extended NTLM responses, but made the match case-
sensitive while it's a token so it's case insensitive.
This should be backported to the same versions as the commit above.
BUG/MINOR: config: Make use_backend and use-server post-parsing less obscur
During use_backend and use-server post-parsing, if the log-format string used to
specify the backend or the server is just a single string, the log-format string
(a list, internally) is replaced by the string itself. Because the field is an
union, the list is not emptied according to the rules of art. The element, when
released, is not removed from the list. There is no bug, but it is clearly not
obvious and error prone.
This patch should fix #544. The fix for the use_backend post-parsing may be
backported to all stable releases. use-server is static in 2.1 and prior.
MINOR: checks: Make matching on HTTP headers for expect rules less obscure
A default statement in the switch testing the header name has been added to be
sure it is impossible to eval the value pattern on an uninitialized header
value. It should never happen of course. But this way, it is explicit.
And a comment has been added to make clear that ctx.value is always defined when
it is evaluated.
DOC: Be more explicit about configurable check ok/error/timeout status
It is possible to configure the check status on success, on error and on
timeout, for http-check and tcp-check expect rules. But the documentation relies
on internal names. These names are reported on the stats and are describe in the
management guide. But it is probably a good idea to be more explicit too in the
doc describing these options.
MINOR: checks: Support log-format string to set the body for HTTP send rules
For http-check send rules, it is now possible to use a log-format string to set
the request's body. the keyword "body-lf" should be used instead of "body". If the
string eval fails, no body is added.
MINOR: checks: Support log-format string to set the URI for HTTP send rules
For http-check send rules, it is now possible to use a log-format string to set
the request URI. the keyword "uri-lf" should be used instead of "uri". If the
string eval fails, we fall back on the default uri "/".
MINOR: mworker: replace ha_alert by ha_warning when exiting successfuly
Since commit bb86986 ("MINOR: init: report the haproxy version and
executable path once on errors") the master-worker displays its version
and path upon a successful exits of a current worker. Which is kind of
confusing upon a clean exits.
This is due to the fact that that commit displays this upon a ha_alert()
which was used during the exit of the process.
Replace the ha_alert() by an ha_warning() if the process exit correctly
and was supposed to.
It still displays the message upon a SIGINT since the workers are
catching the signal.
REGTEST: checks: Adapt SSL error message reported when connection is rejected
Depending on the SSL library version, the reported error may differ when the
connection is rejected during the handshake. An empty handshke may be detected
or just an generic handshake error. So tcp-check-ssl.vtc has been adapted to
support both error messages.
MINOR: checks/sample: Remove unnecessary tests on the sample session
A sample must always have a session defined. Otherwise, it is a bug. So it is
unnecessary to test if it is defined when called from a health checks context.