Willy Tarreau [Wed, 4 Mar 2020 09:31:58 +0000 (10:31 +0100)]
BUILD: tools: rely on __ELF__ not USE_DL to enable use of dladdr()
The approach was wrong. USE_DL is for the makefile to know if it's required
to append -ldl at link time. Some platforms do not need it (and in fact do
not have it) yet they have a working dladdr(). The real condition is related
to ELF. Given that due to Lua, all platforms that require -ldl already have
USE_DL set, let's replace USE_DL with __ELF__ here and consider that dladdr
is always needed on ELF, which basically is already the case.
(cherry picked from commit
109201fc5c64ea1ae98e8f75f6890efa88d981bf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 09:19:36 +0000 (10:19 +0100)]
BUILD: tools: unbreak resolve_sym_name() on non-GNU platforms
resolve_sym_name() doesn't build when USE_DL is set on non-GNU platforms
because "Elf(W)" isn't defined. Since it's only used for dladdr1(), let's
refactor all this so that we can completely ifdef out that part on other
platforms. Now we have a separate function to perform the call depending
on the platform and it only returns the size when available.
(cherry picked from commit
9133e48f2a6fa9deccfe02798dcb2f2abd26ed9e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 06:39:32 +0000 (07:39 +0100)]
MINOR: debug: dump the whole trace if we can't spot the starting point
Instead of special-casing the use of the symbol resolving to decide
whether to dump a partial or complete trace, let's simply start over
and dump everything when we reach the end after having found nothing.
It will be more robust against dirty traces as well.
(cherry picked from commit
a91b7946bdd69fd837085f67f085565317efb1b1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 10:54:16 +0000 (11:54 +0100)]
MINOR: debug: use our own backtrace function on clang+x86_64
A test on FreeBSD with clang 4 to 8 produces this on a call to a
spinning loop on the CLI:
call trace(5):
| 0x53e2bc [eb 16 48 63 c3 48 c1 e0]: wdt_handler+0x10c
| 0x800e02cfe [e8 5d 83 00 00 8b 18 8b]: libthr:pthread_sigmask+0x53e
with our own function it correctly produces this:
call trace(20):
| 0x53e2dc [eb 16 48 63 c3 48 c1 e0]: wdt_handler+0x10c
| 0x800e02cfe [e8 5d 83 00 00 8b 18 8b]: libthr:pthread_sigmask+0x53e
| 0x800e022bf [48 83 c4 38 5b 41 5c 41]: libthr:pthread_getspecific+0xdef
| 0x7ffffffff003 [48 8d 7c 24 10 6a 00 48]: main+0x7fffffb416f3
| 0x801373809 [85 c0 0f 84 6f ff ff ff]: libc:__sys_gettimeofday+0x199
| 0x801373709 [89 c3 85 c0 75 a6 48 8b]: libc:__sys_gettimeofday+0x99
| 0x801371c62 [83 f8 4e 75 0f 48 89 df]: libc:gettimeofday+0x12
| 0x51fa0a [48 89 df 4c 89 f6 e8 6b]: ha_thread_dump_all_to_trash+0x49a
| 0x4b723b [85 c0 75 09 49 8b 04 24]: mworker_cli_sockpair_new+0xd9b
| 0x4b6c68 [85 c0 75 08 4c 89 ef e8]: mworker_cli_sockpair_new+0x7c8
| 0x532f81 [4c 89 e7 48 83 ef 80 41]: task_run_applet+0xe1
So let's add clang+x86_64 to the list of platforms that will use our
simplified version. As a bonus it will not require to link with
-lexecinfo on FreeBSD and will work out of the box when passing
USE_BACKTRACE=1.
(cherry picked from commit
899e5f69a1085983f52a46cd842921874fd2de36)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 06:44:06 +0000 (07:44 +0100)]
MINOR: debug: improve backtrace() on aarch64 and possibly other systems
It happens that on aarch64 backtrace() only returns one entry (tested
with gcc 4.7.4, 5.5.0 and 7.4.1). Probably that it refrains from unwinding
the stack due to the risk of hitting a bad pointer. Here we can use
may_access() to know when it's safe, so we can actually unwind the stack
without taking risks. It happens that the faulting function (the one
just after the signal handler) is not listed here, very likely because
the signal handler uses a special stack and did not create a new frame.
So this patch creates a new my_backtrace() function in standard.h that
either calls backtrace() or does its own unrolling. The choice depends
on HA_HAVE_WORKING_BACKTRACE which is set in compat.h based on the build
target.
(cherry picked from commit
13faf16e1ebfc93e49dc5a8948519fad600e1136)
[wt: adjusted context in debug.c]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 06:38:23 +0000 (07:38 +0100)]
MINOR: debug: report the number of entries in the backtrace
It's useful to get an indication of unresolved stuff or memory
corruption to have the apparent depth of the stack trace in the
output, especially if we dump nothing.
(cherry picked from commit
cdd8074433d8ada867ba9b2bc11db098fd363035)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 09:53:07 +0000 (10:53 +0100)]
MINOR: wdt: do not depend on USE_THREAD
There is no reason for restricting the use of the watchdog to threads
anymore, as it works perfectly without threads as well.
(cherry picked from commit
e58114e0e5e1ef1f67194bde0b0b8d159bd3ac48)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 07:31:47 +0000 (08:31 +0100)]
BUILD: Makefile: include librt before libpthread
Statically building on for i386/x86_64 on linux+glibc 2.18 fails in rt with
undefined references to pthread_attr_init and a few others. Let's just swap
the two libs in order to fix this.
(cherry picked from commit
c0bbdc196ded7b6d28221ca9b96f0cf8b41203ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 05:01:40 +0000 (06:01 +0100)]
MINOR: debug: call backtrace() once upon startup
Calling backtrace() will access libgcc at runtime. We don't want to do
it after the chroot, so let's perform a first call to have it ready in
memory for later use.
(cherry picked from commit
0214b45a61cd7cfd59d729b23f497a687192cde6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 14:40:23 +0000 (15:40 +0100)]
MEDIUM: debug: add support for dumping backtraces of stuck threads
When a panic() occurs due to a stuck thread, we'll try to dump a
backtrace of this thread if the config directive USE_BACKTRACE is
set (which is the case on linux+glibc). For this we use the
backtrace() call provided by glibc and iterate the pointers through
resolve_sym_name(). In order to minimize the output (which is limited
to one buffer), we only do this for stuck threads, and we start the
dump above ha_panic()/ha_thread_dump_all_to_trash(), and stop when
meeting known points such as main/run_tasks_from_list/run_poll_loop.
If enabled without USE_DL, the dump will be complete with no details
except that pointers will all be given relative to main, which is
still better than nothing.
The new USE_BACKTRACE config option is enabled by default on glibc since
it has been present for ages. When it is set, the export-dynamic linker
option is enabled so that all non-static symbols are properly resolved.
(cherry picked from commit
f5b4e064dcb1f7c97c87b68dbbbf7a4371e05bc7)
[wt: adjusted context in makefile and debug.c ;
s/run_tasks_from_list/process_runnable_tasks for 2.1 and older]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 16:29:58 +0000 (17:29 +0100)]
MINOR: cli: make "show fd" rely on resolve_sym_name()
This way we can drop all hard-coded iocb matching.
(cherry picked from commit
cf12f2ee6665281342487a0a3517ba3286245550)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 16:13:02 +0000 (17:13 +0100)]
MINOR: debug: use resolve_sym_name() to dump task handlers
Now in "show threads", the task/tasklet handler will be resolved
using this function, which will provide more detailed results and
will still support offsets to main for unresolved symbols.
(cherry picked from commit
2e89b0930b32b6f694df1c7b35271686acc7cb93)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 16:09:08 +0000 (17:09 +0100)]
MINOR: tools: add resolve_sym_name() to resolve function pointers
We use various hacks at a few places to try to identify known function
pointers in debugging outputs (show threads & show fd). Let's centralize
this into a new function dedicated to this. It already knows about the
functions matched by "show threads" and "show fd", and when built with
USE_DL, it can rely on dladdr1() to resolve other functions. There are
some limitations, as static functions are not resolved, linking with
-rdynamic is mandatory, and even then some functions will not necessarily
appear. It's possible to do a better job by rebuilding the whole symbol
table from the ELF headers in memory but it's less portable and the gains
are still limited, so this solution remains a reasonable tradeoff.
(cherry picked from commit
eb8b1ca3eb4c8d4688e1a4a1d9c1b91f68324e09)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 14:57:10 +0000 (15:57 +0100)]
MINOR: tools: add new function dump_addr_and_bytes()
This function dumps <n> bytes from <addr> in hex form into buffer <buf>
enclosed in brackets after the address itself, formatted on 14 chars
including the "0x" prefix. This is meant to be used as a prefix for code
areas. For example: "0x7f10b6557690 [48 c7 c0 0f 00 00 00 0f]: "
It relies on may_access() to know if the bytes are dumpable, otherwise "--"
is emitted. An optional prefix is supported.
(cherry picked from commit
762fb3ec8e05ca7a61b28e8a876c194c9db591ef)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 13:59:56 +0000 (14:59 +0100)]
MINOR: haproxy: export run_poll_loop
This will help refine debug traces.
(cherry picked from commit
3ebd55ee5154910241cc7541772e039f510b6750)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 14:57:02 +0000 (16:57 +0200)]
MINOR: stream: report the list of active filters on stream crashes
Now we very rarely catch spinning streams, and whenever we catch one it
seems a filter is involved, but we currently report no info about them.
Let's print the list of enabled filters on the stream with such a crash
to help with the reports. A typical output will now look like this:
[ALERT] 121/165908 (1110) : A bogus STREAM [0x7fcaf4016a60] is spinning at 2 calls per second and refuses to die, aborting now! Please report this error to developers [strm=0x7fcaf4016a60 src=127.0.0.1 fe=l1 be=l1 dst=<CACHE> rqf=
6dc42000 rqa=48000 rpf=
a0040223 rpa=
24000000 sif=EST,10008 sib=DIS,80110 af=(nil),0 csf=0x7fcaf4023c00,10c000 ab=0x7fcaf40235f0,4 csb=(nil),0 cof=0x7fcaf4016610,1300:H1(0x7fcaf4016840)/RAW((nil))/tcpv4(29) cob=(nil),0:NONE((nil))/NONE((nil))/NONE(0) filters={0x7fcaf4016fb0="cache store filter", 0x7fcaf4017080="compression filter"}]
This may be backported to 2.0.
(cherry picked from commit
9753d61288936db804628c1e0bad20b7593ad384)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 11:15:32 +0000 (13:15 +0200)]
BUG/MEDIUM: shctx: bound the number of loops that can happen around the lock
Given that a "count" value of 32M was seen in _shctx_wait4lock(), it
is very important to prevent this from happening again. It's absolutely
essential to prevent the value from growing unbounded because with an
increase of the number of threads, the number of successive failed
attempts will necessarily grow.
Instead now we're scanning all 2^p-1 values from 3 to 255 and are
bounding to count to 255 so that in the worst case each thread tries an
xchg every 255 failed read attempts. That's one every 4 on average per
thread when there are 64 threads, which corresponds to the initial count
of 4 for the first attempt so it seems like a reasonable value to keep a
low latency.
The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.
(cherry picked from commit
86c6a9221aa5bd7516300f399e87aba09d61de06)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 11:05:29 +0000 (13:05 +0200)]
BUG/MEDIUM: shctx: really check the lock's value while waiting
Jérôme reported an amazing crash in the spinlock version of
_shctx_wait4lock() with an extremely high <count> value of 32M! The
root cause is that the function cannot deal with contention on the lock
at all because it forgets to check if the lock's value has changed! As
such, every time it's called due to a contention, it waits twice as
long before trying again and lets the caller check for the contention
by itself.
The correct thing to do is to compare the value again at each loop.
This way it makes sure to mostly perform read accesses on the shared
cache line without writing too often, and to be ready fast enough to
try to grab the lock. And we must not increase the count on success
either!
Unfortunately I'd have expected to see a performance boost on the cache
with this but there was absolutely no change, so it's very likely that
these issues only happen once in a while and are sufficient to derail
the process when they strike, but not to have a permanent performance
impact.
The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.
(cherry picked from commit
3801bdc3fc1c2c6d596b4c4ee3449a76f2da8654)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 10:26:03 +0000 (12:26 +0200)]
BUG/MINOR: debug: properly use long long instead of long for the thread ID
I changed my mind twice on this one and pushed after the last test with
threads disabled, without re-enabling long long, causing this rightful
build warning.
This needs to be backported if the previous commit
ff64d3b027 ("MINOR:
threads: export the POSIX thread ID in panic dumps") is backported as
well.
(cherry picked from commit
f0e5da20e1a6e9b532f7de66079da5fa53e9d01c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 09:28:49 +0000 (11:28 +0200)]
MINOR: threads: export the POSIX thread ID in panic dumps
It is very difficult to map a panic dump against a gdb thread dump
because the thread numbers do not match. However gdb provides the
pthread ID but this one is supposed to be opaque and not to be cast
to a scalar.
This patch provides a fnuction, ha_get_pthread_id() which retrieves
the pthread ID of the indicated thread and casts it to an unsigned
long long so as to lose the least possible amount of information from
it. This is done cleanly using a union to maintain alignment so as
long as these IDs are stored on 1..8 bytes they will be properly
reported. This ID is now presented in the panic dumps so it now
becomes possible to map these threads. When threads are disabled,
zero is returned. For example, this is a panic dump:
Thread 1 is about to kill the process.
*>Thread 1 : id=0x7fe92b825180 act=0 glob=0 wq=1 rq=0 tl=0 tlsz=0 rqsz=0
stuck=1 prof=0 harmless=0 wantrdv=0
cpu_ns: poll=5119122 now=
2009446995 diff=
2004327873
curr_task=0xc99bf0 (task) calls=4 last=0
fct=0x592440(task_run_applet) ctx=0xca9c50(<CLI>)
strm=0xc996a0 src=unix fe=GLOBAL be=GLOBAL dst=<CLI>
rqf=848202 rqa=0 rpf=
80048202 rpa=0 sif=EST,200008 sib=EST,204018
af=(nil),0 csf=0xc9ba40,8200
ab=0xca9c50,4 csb=(nil),0
cof=0xbf0e50,1300:PASS(0xc9cee0)/RAW((nil))/unix_stream(20)
cob=(nil),0:NONE((nil))/NONE((nil))/NONE(0)
call trace(20):
| 0x59e4cf [48 83 c4 10 5b 5d 41 5c]: wdt_handler+0xff/0x10c
| 0x7fe92c170690 [48 c7 c0 0f 00 00 00 0f]: libpthread:+0x13690
| 0x7ffce29519d9 [48 c1 e2 20 48 09 d0 48]: linux-vdso:+0x9d9
| 0x7ffce2951d54 [eb d9 f3 90 e9 1c ff ff]: linux-vdso:__vdso_gettimeofday+0x104/0x133
| 0x57b484 [48 89 e6 48 8d 7c 24 10]: main+0x157114
| 0x50ee6a [85 c0 75 76 48 8b 55 38]: main+0xeaafa
| 0x50f69c [48 63 54 24 20 85 c0 0f]: main+0xeb32c
| 0x59252c [48 c7 c6 d8 ff ff ff 44]: task_run_applet+0xec/0x88c
Thread 2 : id=0x7fe92b6e6700 act=0 glob=0 wq=0 rq=0 tl=0 tlsz=0 rqsz=0
stuck=0 prof=0 harmless=1 wantrdv=0
cpu_ns: poll=786738 now=1086955 diff=300217
curr_task=0
Thread 3 : id=0x7fe92aee5700 act=0 glob=0 wq=0 rq=0 tl=0 tlsz=0 rqsz=0
stuck=0 prof=0 harmless=1 wantrdv=0
cpu_ns: poll=828056 now=1129738 diff=301682
curr_task=0
Thread 4 : id=0x7fe92a6e4700 act=0 glob=0 wq=0 rq=0 tl=0 tlsz=0 rqsz=0
stuck=0 prof=0 harmless=1 wantrdv=0
cpu_ns: poll=818900 now=1153551 diff=334651
curr_task=0
And this is the gdb output:
(gdb) info thr
Id Target Id Frame
* 1 Thread 0x7fe92b825180 (LWP 15234) 0x00007fe92ba81d6b in raise () from /lib64/libc.so.6
2 Thread 0x7fe92b6e6700 (LWP 15235) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
3 Thread 0x7fe92a6e4700 (LWP 15237) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
4 Thread 0x7fe92aee5700 (LWP 15236) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
We can clearly see that while threads 1 and 2 are the same, gdb's
threads 3 and 4 respectively are haproxy's threads 4 and 3.
This may be backported to 2.0 as it removes some confusion in github issues.
(cherry picked from commit
ff64d3b027d968a29b5bac55121bbce591fe54b2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 1 May 2020 07:51:11 +0000 (09:51 +0200)]
BUG/MEDIUM: listener: mark the thread as not stuck inside the loop
We tried hard to make sure we report threads as not stuck at various
crucial places, but one of them is special, it's the listener_accept()
function. The reason it is special is because it will loop a certain
number of times (default: 64) accepting incoming connections, allocating
resources, dispatching them to other threads or running L4 rules on them,
and while all of this is supposed to be extremely fast, when the machine
slows down or runs low on memory, the expectedly small delays in malloc()
caused by contention with other threads can quickly accumulate and suddenly
become critical to the point of triggering the watchdog. Furthermore, it
is technically possible to trigger this by pure configuration by setting
a huge tune.maxaccept value, which should not be possible.
Given that each operation isn't related to the same task but to a different
one each time, it is appropriate to mark the thread as not stuck each time
it accepts new work that possibly gets dispatched to other threads which
execute it.
This looks like this could be a good reason for the issue reported in
issue #388.
This fix must be backported to 2.0.
(cherry picked from commit
8d2c98b76c505e199ba3abac632fa98aab9f7b20)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 29 Apr 2020 09:59:02 +0000 (11:59 +0200)]
BUG/MEDIUM: sample: make the CPU and latency sample fetches check for a stream
cpu_calls, cpu_ns_avg, cpu_ns_tot, lat_ns_avg and lat_ns_tot depend on the
stream to find the current task and must check for it or they may cause a
crash if misused or used in a log-format string after commit
5f940703b3
("MINOR: log: Don't depends on a stream to process samples in log-format
string").
This must be backported as far as 1.9.
(cherry picked from commit
e0dd210cea36972a6f566b9b49d8f0c33340df99)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 29 Apr 2020 09:50:38 +0000 (11:50 +0200)]
BUG/MEDIUM: http: the "unique-id" sample fetch could crash without a steeam
Since commit
5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The unique-id sample fetch function, if called without a stream, will result
in a crash.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.7.
(cherry picked from commit
a1062a4de8ba300979cc380b69b284f86dc13853)
[wt: adjusted ctx: unique_id was a ptr not an ist in 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 29 Apr 2020 09:52:13 +0000 (11:52 +0200)]
BUG/MEDIUM: http: the "http_first_req" sample fetch could crash without a steeam
Since commit
5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The http_first_req sample fetch function, if called without a stream, will
result in a crash.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
(cherry picked from commit
79512b6d8f2f049a86b2332d4f4dc972cc33d631)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 29 Apr 2020 09:44:54 +0000 (11:44 +0200)]
BUG/MEDIUM: capture: capture.{req,res}.* crash without a stream
Since commit
5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The capture.req.hdr, capture.res.hdr, capture.req.method, capture.req.uri,
capture.req.ver and capture.res.ver sample fetches used to assume the
presence of a stream, which is not necessarily the case (especially after
the commit above) and would crash haproxy if incorrectly used. Let's make
sure they check for this stream.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
(cherry picked from commit
0898c2d2f2c5ae043a443550992efd90ec18b2ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 29 Apr 2020 09:22:08 +0000 (11:22 +0200)]
BUG/MEDIUM: capture: capture-req/capture-res converters crash without a stream
Since commit
5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The capture-req and capture-res converters were in this case and could
crash the process if misused.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
(cherry picked from commit
5575896ba1120f136e680fd669afb5b084faa530)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 24 Apr 2020 05:19:04 +0000 (07:19 +0200)]
BUG/MINOR: mux-fcgi: Be sure to have a connection as session's origin to use it
When default parameters are set in a request message, we get the client
connection using the session's origin. But it is not necessarily a
conncection. For instance, for health checks, thanks to recent changes, it may
be a check object. At this step, the client connection may be NULL. Thus, we
must be sure to have a client connection before using it.
This patch must be backported to 2.1.
(cherry picked from commit
bb86a0f7be6ba8a248321a59c4b865e01e33f106)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 21 Apr 2020 09:48:53 +0000 (11:48 +0200)]
BUG/MINOR: obj_type: Handle stream object in obj_base_ptr() function
The stream object (OBJ_TYPE_STREAM) was missing in the switch statement of the
obj_base_ptr() function.
This patch must be backported as far as 2.0.
(cherry picked from commit
a142c1deb401308055f29b9d52a47e5a0ad962e7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Gaetan Rivet [Thu, 13 Feb 2020 09:30:01 +0000 (10:30 +0100)]
BUG/MINOR: checks: chained expect will not properly wait for enough data
TCP check expect matching strings or binary patterns are able to know
prior to applying their match function whether the available data is
already sufficient to attempt the match or not.
As such, on insufficient data the expect is postponed. This behavior
avoids unnecessary matches when the data could not possibly match.
When chaining expect, upon passing the previous and going onto the next
however, this length check is not done again. Then the match is done and
will necessarily fail, triggering a new wait for more data. The end
result is the same for a slightly higher cost.
Check received data length for all expects in a chain.
This bug exists since the introduction of the feature:
Fixes:
5ecb77f4c76f ("MEDIUM: checks: add send/expect tcp based check")
Version 1.5+ impacted.
(cherry picked from commit
738ee76aa7a9968cb9c9610725c26622c50f0b33)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 26 Mar 2020 18:48:20 +0000 (19:48 +0100)]
BUG/MEDIUM: server/checks: Init server check during config validity check
The options and directives related to the configuration of checks in a backend
may be defined after the servers declarations. So, initialization of the check
of each server must not be performed during configuration parsing, because some
info may be missing. Instead, it must be done during the configuration validity
check.
Thus, callback functions are registered to be called for each server after the
config validity check, one for the server check and another one for the server
agent-check. In addition deinit callback functions are also registered to
release these checks.
This patch should be backported as far as 1.7. But per-server post_check
callback functions are only supported since the 2.1. And the initcall mechanism
does not exist before the 1.9. Finally, in 1.7, the code is totally
different. So the backport will be harder on older versions.
(cherry picked from commit
8892e5d30b2c20fce04f2f44202bc1f127076c15)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 27 Mar 2020 17:55:49 +0000 (18:55 +0100)]
BUG/MINOR: checks: Respect the no-check-ssl option
This options is used to force a non-SSL connection to check a SSL server or to
invert a check-ssl option inherited from the default section. The use_ssl field
in the check structure is used to know if a SSL connection must be used
(use_ssl=1) or not (use_ssl=0). The server configuration is used by default.
The problem is that we cannot distinguish the default case (no specific SSL
check option) and the case of an explicit non-SSL check. In both, use_ssl is set
to 0. So the server configuration is always used. For a SSL server, when
no-check-ssl option is set, the check is still performed using a SSL
configuration.
To fix the bug, instead of a boolean value (0=TCP, 1=SSL), we use a ternary value :
* 0 = use server config
* 1 = force SSL
* -1 = force non-SSL
The same is done for the server parameter. It is not really necessary for
now. But it is a good way to know is the server no-ssl option is set.
In addition, the PR_O_TCPCHK_SSL proxy option is no longer used to set use_ssl
to 1 for a check. Instead the flag is directly tested to prepare or destroy the
server SSL context.
This patch should be backported as far as 1.8.
(cherry picked from commit
f61f33a1b274c2a42afd96aab19ee8e1d8b121cc)
[wt: minor context adjustments]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 9 Apr 2020 06:44:06 +0000 (08:44 +0200)]
MINOR: checks: Add a way to send custom headers and payload during http chekcs
The 'http-check send' directive have been added to add headers and optionnaly a
payload to the request sent during HTTP healthchecks. The request line may be
customized by the "option httpchk" directive but there was not official way to
add extra headers. An old trick consisted to hide these headers at the end of
the version string, on the "option httpchk" line. And it was impossible to add
an extra payload with an "http-check expect" directive because of the
"Connection: close" header appended to the request (See issue #16 for details).
So to make things official and fully support payload additions, the "http-check
send" directive have been added :
option httpchk POST /status HTTP/1.1
http-check send hdr Content-Type "application/json;charset=UTF-8" \
hdr X-test-1 value1 hdr X-test-2 value2 \
body "{id: 1, field: \"value\"}"
When a payload is defined, the Content-Length header is automatically added. So
chunk-encoded requests are not supported yet. For now, there is no special
validity checks on the extra headers.
This patch is inspired by Kiran Gavali's work. It should fix the issue #16 and
as far as possible, it may be backported, at least as far as 1.8.
(cherry picked from commit
8acb1284bcd47301fce0aa45a7083dbcef94cb5f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Sun, 26 Apr 2020 07:50:31 +0000 (09:50 +0200)]
BUG/MINOR: check: Update server address and port to execute an external check
Server address and port may change at runtime. So the address and port passed as
arguments and as environment variables when an external check is executed must
be updated. The current number of connections on the server was already updated
before executing the command. So the same mechanism is used for the server
address and port. But in addition, command arguments are also updated.
This patch must be backported to all stable versions. It should fix the
issue #577.
(cherry picked from commit
aaae9a0e9952bef30bb0da0eecd714f2db64d545)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Sat, 25 Apr 2020 20:03:29 +0000 (22:03 +0200)]
MINOR: contrib: make the peers wireshark dissector a plugin
The wireshark dissector could only be build within wireshark, which
means maintaining a wireshark binary just for this dissector. It was not
really convenient to update wireshark because of this.
This patch converts the dissector into a .so plugin which is built with
the .h found in distributions instead of the whole wireshark sources.
(cherry picked from commit
2be58f758402010f35d67279bec7125c9665e936)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Apr 2020 04:15:24 +0000 (06:15 +0200)]
MEDIUM: memory: make pool_gc() run under thread isolation
pool_gc() causes quite some stress on the memory allocator because
it calls a lot of free() calls while other threads are using malloc().
In addition, pool_gc() needs to take care of possible locking because
it may be called from pool allocators. One way to avoid all this is to
use thread_isolate() to make sure the gc runs alone. By putting less
pressure on the pools and getting rid of the locks, it may even take
less time to complete.
(cherry picked from commit
c0e2ff202bda535aa60f3dc272ccf19a7b4f5094)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Jerome Magnin [Thu, 23 Apr 2020 17:01:17 +0000 (19:01 +0200)]
DOC: option logasap does not depend on mode
The documentation for option logasap misleads into thinking it is
only valid for mode http. It is actually valid for mode tcp too,
so this patch tries to disambiguate the current wording.
(cherry picked from commit
95fb57b92331c211f69af333cf5e043284ffbf25)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 23 Apr 2020 15:54:47 +0000 (17:54 +0200)]
BUG/MINOR: http: make url_decode() optionally convert '+' to SP
The url_decode() function used by the url_dec converter and a few other
call points is ambiguous on its processing of the '+' character which
itself isn't stable in the spec. This one belongs to the reserved
characters for the query string but not for the path nor the scheme,
in which it must be left as-is. It's only in argument strings that
follow the application/x-www-form-urlencoded encoding that it must be
turned into a space, that is, in query strings and POST arguments.
The problem is that the function is used to process full URLs and
paths in various configs, and to process query strings from the stats
page for example.
This patch updates the function to differentiate the situation where
it's parsing a path and a query string. A new argument indicates if a
query string should be assumed, otherwise it's only assumed after seeing
a question mark.
The various locations in the code making use of this function were
updated to take care of this (most call places were using it to decode
POST arguments).
The url_dec converter is usually called on path or url samples, so it
needs to remain compatible with this and will default to parsing a path
and turning the '+' to a space only after a question mark. However in
situations where it would explicitly be extracted from a POST or a
query string, it now becomes possible to enforce the decoding by passing
a non-null value in argument.
It seems to be what was reported in issue #585. This fix may be
backported to older stable releases.
(cherry picked from commit
62ba9ba6ca259f45f7bd8de436b743b3ad9ac04a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 23 Apr 2020 15:08:02 +0000 (17:08 +0200)]
BUG/MINOR: tools: fix the i386 version of the div64_32 function
As reported in issue #596, the edx register isn't marked as clobbered
in div64_32(), which could technically allow gcc to try to reuse it
if it needed a copy of the 32 highest bits of the o1 register after
the operation.
Two attempts were tried, one using a dummy 32-bit local variable to
store the intermediary edx and another one switching to "=A" and making
result a long long. It turns out the former makes the resulting object
code significantly dirtier while the latter makes it better and was
kept. This is due to gcc's difficulties at working with register pairs
mixing 32- and 64- bit values on i386. It was verified that no code
change happened at all on x86_64, armv7, aarch64 nor mips32.
In practice it's only used by the frequency counters so this bug
cannot even be triggered but better fix it.
This may be backported to stable branches though it will not fix any
issue.
(cherry picked from commit
09568fd54d2f091860cafa5173893445cd55c44c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Wed, 22 Apr 2020 19:51:14 +0000 (21:51 +0200)]
BUG/MEDIUM: http-ana: Handle NTLM messages correctly.
When checking www-authenticate headers, we don't want to just accept
"NTLM" as value, because the server may send "HTLM <base64 value>". Instead,
just check that it starts with NTLM.
This should be backported to 2.1, 2.0, 1.9 and 1.8.
(cherry picked from commit
9df188695fbf1ff17de3861ec5b281365800c7f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Jerome Magnin [Wed, 22 Apr 2020 09:40:18 +0000 (11:40 +0200)]
BUG/MINOR: ssl: default settings for ssl server options are not used
Documentation states that default settings for ssl server options can be set
using either ssl-default-server-options or default-server directives. In practice,
not all ssl server options can have default values, such as ssl-min-ver, ssl-max-ver,
etc..
This patch adds the missing ssl options in srv_ssl_settings_cpy() and srv_parse_ssl(),
making it possible to write configurations like the following examples, and have them
behave as expected.
global
ssl-default-server-options ssl-max-ver TLSv1.2
defaults
mode http
listen l1
bind 1.2.3.4:80
default-server ssl verify none
server s1 1.2.3.5:443
listen l2
bind 2.2.3.4:80
default-server ssl verify none ssl-max-ver TLSv1.3 ssl-min-ver TLSv1.2
server s1 1.2.3.6:443
This should be backported as far as 1.8.
This fixes issue #595.
(cherry picked from commit
2e8d52f869ed7673a8274ec7b045161e09350251)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Doucet [Tue, 21 Apr 2020 07:32:56 +0000 (09:32 +0200)]
DOC: Improve documentation on http-request set-src
This patch adds more explanation on how to use "http-request set-src"
and a link to "option forwardfor".
This patch can be applied to all previous version starting at 1.6
Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
(cherry picked from commit
56e3120f9ee0db8de166f5e6c9cf2ce2fc4c2364)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sat, 18 Apr 2020 14:02:47 +0000 (16:02 +0200)]
MINOR: version: Show uname output in display_version()
This patch adds the sysname, release, version and machine fields from
the uname results to the version output. It intentionally leaves out the
machine name, because it is usually not useful and users might not want to
expose their machine names for privacy reasons.
May be backported if it is considered useful for debugging.
(cherry picked from commit
dfad6a41ad9f012671b703788dd679cf24eb8c5a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Adam Mills [Tue, 14 Apr 2020 23:45:15 +0000 (16:45 -0700)]
DOC: hashing: update link to hashing functions
Bret Mulvey, the author of the article cited in this pulication
has migrated his work to papa.bretmulvey.com. I was able to
view an archival version of Bret M.'s original post
(http://home.comcast.net/~bretm/hash/3.html) and have validated
that this is the same paper that is originally cited.
(cherry picked from commit
bbf697e7520df4031ac4650f43b95e926d29e509)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frédéric Lécaille [Fri, 3 Apr 2020 07:43:47 +0000 (09:43 +0200)]
BUG/MINOR: peers: Incomplete peers sections should be validated.
Before supporting "server" line in "peers" section, such sections without
any local peer were removed from the configuration to get it validated.
This patch fixes the issue where a "server" line without address and port which
is a remote peer without address and port makes the configuration parsing fail.
When encoutering such cases we now ignore such lines remove them from the
configuration.
Thank you to Jérôme Magnin for having reported this bug.
Must be backported to 2.1 and 2.0.
(cherry picked from commit
8ba10fea69aff458ee0be9c32a432dcc319f8f0b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 14 Apr 2020 10:54:10 +0000 (12:54 +0200)]
BUG/MINOR: connection: always send address-less LOCAL PROXY connections
Commit
7f26391bc5 ("BUG/MINOR: connection: make sure to correctly tag
local PROXY connections") revealed that some implementations do not
properly ignore addresses in LOCAL connections (at least Dovecot was
spotted). More context information in the thread below:
https://www.mail-archive.com/haproxy@formilux.org/msg36890.html
The patch above was using LOCAL on top of local addresses in order to
minimize the risk of breakage but revealed worse than a clean fix. So
let's partially revert it and send pure LOCAL connections instead now.
After a bit of observation, this patch should be progressively backported
to stable branches. However if it reveals new breakage, the backport of
the patch above will have to be reverted from stable branches while other
products work on fixing their code based on the master branch.
(cherry picked from commit
02c88036a61e09d0676a2b6b4086af677b023b94)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Wed, 8 Apr 2020 15:38:27 +0000 (17:38 +0200)]
BUG/MINOR: ssl: memleak of the struct cert_key_and_chain
Free the struct cert_key_and_chain when calling ckchs_free(),
a memory leak can occur when using 'commit ssl cert'.
Must be backported to 2.1.
(cherry picked from commit
8621ac5570a7cd225005f35808616d28a9774e88)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Wed, 8 Apr 2020 13:16:51 +0000 (15:16 +0200)]
BUG/MINOR: ssl/cli: memory leak in 'set ssl cert'
When deleting the previous SNI entries with 'set ssl cert', the old
SSL_CTX' were not free'd, which probably prevent the completion of the
free of the X509 in the old ckch_store, because of the refcounts in the
SSL library.
This bug was introduced by 150bfa8 ("MEDIUM: cli/ssl: handle the
creation of SSL_CTX in an IO handler").
Must be backported to 2.1.
(cherry picked from commit
24be710609fe24781b489339273beec29114a3b8)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
William Lallemand [Tue, 7 Apr 2020 12:16:32 +0000 (14:16 +0200)]
MINOR: ssl: improve the errors when a crt can't be open
Issue #574 reported an unclear error when trying to open a file with not
enough permission.
[ALERT] 096/032117 (835) : parsing [/etc/haproxy/haproxy.cfg:54] : 'bind :443' : error encountered while processing 'crt'.
[ALERT] 096/032117 (835) : Error(s) found in configuration file : /etc/haproxy/haproxy.cfg
[ALERT] 096/032117 (835) : Fatal errors found in configuration.
Improve the error to give us more information:
[ALERT] 097/142030 (240089) : parsing [test.cfg:22] : 'bind :443' : cannot open the file 'kikyo.pem.rsa'.
[ALERT] 097/142030 (240089) : Error(s) found in configuration file : test.cfg
[ALERT] 097/142030 (240089) : Fatal errors found in configuration.
This patch could be backported in 2.1.
(cherry picked from commit
7fd01b36257ffa9fe76930ccfe8a34ad40911fef)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Frédéric Lécaille [Thu, 2 Apr 2020 12:24:31 +0000 (14:24 +0200)]
BUG/MINOR: protocol_buffer: Wrong maximum shifting.
This patch fixes a bad stop condition when decoding a protocol buffer variable integer
whose maximum lenghts are 10, shifting a uint64_t value by more than 63.
Thank you to Ilya for having reported this issue.
Must be backported to 2.1 and 2.0.
(cherry picked from commit
876ed55d9b8d0c298b6cac1003ec365a19bf7aad)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 2 Apr 2020 07:01:53 +0000 (09:01 +0200)]
[RELEASE] Released version 2.1.4
Released version 2.1.4 with the following main changes :
- SCRIPTS: make announce-release executable again
- BUG/MINOR: namespace: avoid closing fd when socket failed in my_socketat
- BUG/MEDIUM: muxes: Use the right argument when calling the destroy method.
- BUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param
- MINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex
- SCRIPTS: announce-release: use mutt -H instead of -i to include the draft
- MINOR: http-htx: Add a function to retrieve the headers size of an HTX message
- MINOR: filters: Forward data only if the last filter forwards something
- BUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them
- BUG/MINOR: http-htx: Don't return error if authority is updated without changes
- BUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive
- MINOR: http-ana: Match on the path if the monitor-uri starts by a /
- BUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered
- MINOR: ist: add an iststop() function
- BUG/MINOR: http: http-request replace-path duplicates the query string
- BUG/MEDIUM: shctx: make sure to keep all blocks aligned
- MINOR: compiler: move CPU capabilities definition from config.h and complete them
- BUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support
- BUILD: fix recent build failure on unaligned archs
- CLEANUP: cfgparse: Fix type of second calloc() parameter
- BUG/MINOR: sample: fix the json converter's endian-sensitivity
- BUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions
- BUG/MINOR: connection: make sure to correctly tag local PROXY connections
- MINOR: compiler: add new alignment macros
- BUILD: ebtree: improve architecture-specific alignment
- BUG/MINOR: h2: reject again empty :path pseudo-headers
- BUG/MINOR: sample: Make sure to return stable IDs in the unique-id fetch
- BUG/MINOR: dns: ignore trailing dot
- BUG/MINOR: http-htx: Do case-insensive comparisons on Host header name
- MINOR: contrib/prometheus-exporter: Add heathcheck status/code in server metrics
- MINOR: contrib/prometheus-exporter: Add the last heathcheck duration metric
- BUG/MEDIUM: random: initialize the random pool a bit better
- MINOR: tools: add 64-bit rotate operators
- BUG/MEDIUM: random: implement a thread-safe and process-safe PRNG
- MINOR: backend: use a single call to ha_random32() for the random LB algo
- BUG/MINOR: checks/threads: use ha_random() and not rand()
- BUG/MAJOR: list: fix invalid element address calculation
- MINOR: debug: report the task handler's pointer relative to main
- BUG/MEDIUM: debug: make the debug_handler check for the thread in threads_to_dump
- MINOR: haproxy: export main to ease access from debugger
- BUILD: tools: remove obsolete and conflicting trace() from standard.c
- BUG/MINOR: wdt: do not return an error when the watchdog couldn't be enabled
- DOC: fix incorrect indentation of http_auth_*
- OPTIM: startup: fast unique_id allocation for acl.
- BUG/MINOR: pattern: Do not pass len = 0 to calloc()
- DOC: configuration.txt: fix various typos
- DOC: assorted typo fixes in the documentation and Makefile
- BUG/MINOR: init: make the automatic maxconn consider the max of soft/hard limits
- BUG/MAJOR: proxy_protocol: Properly validate TLV lengths
- REGTEST: make the PROXY TLV validation depend on version 2.2
- BUG/MINOR: filters: Use filter offset to decude the amount of forwarded data
- BUG/MINOR: filters: Forward everything if no data filters are called
- MINOR: htx: Add a function to return a block at a specific offset
- BUG/MEDIUM: cache/filters: Fix loop on HTX blocks caching the response payload
- BUG/MEDIUM: compression/filters: Fix loop on HTX blocks compressing the payload
- BUG/MINOR: http-ana: Reset request analysers on a response side error
- BUG/MINOR: lua: Ignore the reserve to know if a channel is full or not
- BUG/MINOR: http-rules: Preserve FLT_END analyzers on reject action
- BUG/MINOR: http-rules: Fix a typo in the reject action function
- BUG/MINOR: rules: Preserve FLT_END analyzers on silent-drop action
- BUG/MINOR: rules: Increment be_counters if backend is assigned for a silent-drop
- DOC: fix typo about no-tls-tickets
- DOC: improve description of no-tls-tickets
- DOC: assorted typo fixes in the documentation
- DOC: ssl: clarify security implications of TLS tickets
- BUILD: wdt: only test for SI_TKILL when compiled with thread support
- BUG/MEDIUM: mt_lists: Make sure we set the deleted element to NULL;
- MINOR: mt_lists: Appease gcc.
- BUG/MEDIUM: random: align the state on 2*64 bits for ARM64
- BUG/MEDIUM: pools: Always update free_list in pool_gc().
- BUG/MINOR: haproxy: always initialize sleeping_thread_mask
- BUG/MINOR: listener/mq: do not dispatch connections to remote threads when stopping
- BUG/MINOR: haproxy/threads: try to make all threads leave together
- DOC: proxy_protocol: Reserve TLV type 0x05 as PP2_TYPE_UNIQUE_ID
- DOC: correct typo in alert message about rspirep
- BUILD: on ARM, must be linked to libatomic.
- BUILD: makefile: fix regex syntax in ARM platform detection
- BUILD: makefile: fix expression again to detect ARM platform
- BUG/MEDIUM: peers: resync ended with RESYNC_PARTIAL in wrong cases.
- DOC: assorted typo fixes in the documentation
- MINOR: wdt: Move the definitions of WDTSIG and DEBUGSIG into types/signal.h.
- BUG/MEDIUM: wdt: Don't ignore WDTSIG and DEBUGSIG in __signal_process_queue().
- MINOR: memory: Change the flush_lock to a spinlock, and don't get it in alloc.
- BUG/MINOR: connections: Make sure we free the connection on failure.
- REGTESTS: use "command -v" instead of "which"
- REGTEST: increase timeouts on the seamless-reload test
- BUG/MINOR: haproxy/threads: close a possible race in soft-stop detection
- BUG/MINOR: peers: init bind_proc to 1 if it wasn't initialized
- BUG/MINOR: peers: avoid an infinite loop with peers_fe is NULL
- BUG/MINOR: peers: Use after free of "peers" section.
- MINOR: listener: add so_name sample fetch
- BUILD: ssl: only pass unsigned chars to isspace()
- BUG/MINOR: stats: Fix color of draining servers on stats page
- DOC: internals: Fix spelling errors in filters.txt
- MINOR: http-rules: Add a flag on redirect rules to know the rule direction
- BUG/MINOR: http_ana: make sure redirect flags don't have overlapping bits
- MINOR: http-rules: Handle the rule direction when a redirect is evaluated
- BUG/MINOR: http-ana: Reset request analysers on error when waiting for response
- BUG/CRITICAL: hpack: never index a header into the headroom after wrapping
Willy Tarreau [Sun, 29 Mar 2020 06:53:31 +0000 (08:53 +0200)]
BUG/CRITICAL: hpack: never index a header into the headroom after wrapping
The HPACK header table is implemented as a wrapping list inside a contigous
area. Headers names and values are stored from right to left while indexes
are stored from left to right. When there's no more room to store a new one,
we wrap to the right again, or possibly defragment it if needed. The condition
do use the right part (called tailroom) or the left part (called headroom)
depends on the location of the last inserted header. After wrapping happens,
the code forces to stick to tailroom by pretending there's no more headroom,
so that the size fit test always fails. The problem is that nothing prevents
from storing a header with an empty name and empty value, resulting in a
total size of zero bytes, which satisfies the condition to use the headroom.
Doing this in a wrapped buffer results in changing the "front" header index
and causing miscalculations on the available size and the addresses of the
next headers. This may even allow to overwrite some parts of the index,
opening the possibility to perform arbitrary writes into a 32-bit relative
address space.
This patch fixes the issue by making sure the headroom is considered only
when the buffer does not wrap, instead of relying on the zero size. This
must be backported to all versions supporting H2, which is as far as 1.8.
Many thanks to Felix Wilhelm of Google Project Zero for responsibly
reporting this problem with a reproducer and a detailed analysis.
CVE-2020-11100 was assigned to this issue.
(cherry picked from commit
5dfc5d5cd0d2128d77253ead3acf03a421ab5b88)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 1 Apr 2020 06:14:16 +0000 (08:14 +0200)]
BUG/MINOR: http-ana: Reset request analysers on error when waiting for response
This bug was supposed to be fixed by the commit
f6df2b4a ("BUG/MINOR: http-ana:
Reset request analysers on a response side error"). It is a backported patch
from the 2.2. But, while it is enough on the 2.2, it is not on 2.1 and
lower. For these versions, the error handling is not grouped at the end of
analysers. At many places, when we are waiting for a response, several errors
are immediately returned. For all of these, the same fix must be applied.
The patch was directly introduced on 2.1, there is no upstream commit ID for
this patch. It must be backported everywhere the commit
f6df2b4a is.
Christopher Faulet [Tue, 28 Jan 2020 08:18:10 +0000 (09:18 +0100)]
MINOR: http-rules: Handle the rule direction when a redirect is evaluated
The rule direction must be tested to do specific processing on the request
path. intercepted_req counter shoud be updated if the rule is evaluated on the
frontend and remaining request's analyzers must be removed. But only on the
request path. The rule direction must also be tested to set the right final
stream state flag.
This patch depends on the commit "MINOR: http-rules: Add a flag on redirect
rules to know the rule direction". Both must be backported to all stable
versions.
(cherry picked from commit
60b33a5a625cdc8e956d4f43aca5ab795240be33)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
[Cf: Thes BUG label is missing for this patch. It is the bug mentionned by the
commit "MINOR: http-rules: Add a flag on redirect rules to know the rule
direction".]
Jerome Magnin [Thu, 27 Feb 2020 22:36:56 +0000 (23:36 +0100)]
BUG/MINOR: http_ana: make sure redirect flags don't have overlapping bits
commit
c87e46881 ("MINOR: http-rules: Add a flag on redirect rules to know the
rule direction") introduced a new flag for redirect rules, but its value has
bits in common with REDIRECT_FLAG_DROP_QS, which makes us enter this code path
in http_apply_redirect_rule(), which will then drop the query string.
To fix this, just give REDIRECT_FLAG_FROM_REQ its own unique value.
This must be backported where
c87e46881687b8ddb9b3f459e60edb1e8d7c5d7c is backported.
This should fix issue 521.
(cherry picked from commit
967d3cc1057f1c2e83b7969db1406104fbf843df)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 28 Jan 2020 08:13:41 +0000 (09:13 +0100)]
MINOR: http-rules: Add a flag on redirect rules to know the rule direction
HTTP redirect rules can be evaluated on the request or the response path. So
when a redirect rule is evaluated, it is important to have this information
because some specific processing may be performed depending on the direction. So
the REDIRECT_FLAG_FROM_REQ flag has been added. It is set when applicable on the
redirect rule during the parsing.
This patch is mandatory to fix a bug on redirect rule. It must be backported to
all stable versions.
(cherry picked from commit
c87e46881687b8ddb9b3f459e60edb1e8d7c5d7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Miroslav Zagorac [Thu, 26 Mar 2020 19:45:04 +0000 (20:45 +0100)]
DOC: internals: Fix spelling errors in filters.txt
(cherry picked from commit
d80f5c0d0cd9133fada06bff71f98a025366c7c8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Daniel Corbett [Sat, 28 Mar 2020 16:35:50 +0000 (12:35 -0400)]
BUG/MINOR: stats: Fix color of draining servers on stats page
This patch fixes #53 where it was noticed that when an active
server is set to DRAIN it no longer has the color blue reflected
within the stats page. This patch addresses that and adds the
color back to drain. It's to be noted that backup servers are
configured to have an orange color when they are draining.
Should be backported as far as 1.7.
(cherry picked from commit
b428517fee01b92546ee82cf420644d548f15a90)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 25 Feb 2020 06:51:59 +0000 (07:51 +0100)]
BUILD: ssl: only pass unsigned chars to isspace()
A build failure on cygwin was reported on github actions here:
https://github.com/haproxy/haproxy/runs/
466507874
It's caused by a signed char being passed to isspace(), and this one
being implemented as a macro instead of a function as the man page
suggests. It's the same issue that regularly pops up on Solaris. This
comes from commit
98263291cc3 which was merged in 1.8-dev1. A backport
is possible though not incredibly useful.
(cherry picked from commit
ded15b75640d8f96929beefa3357cafbea25171e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Jerome Magnin [Fri, 27 Mar 2020 21:08:40 +0000 (22:08 +0100)]
MINOR: listener: add so_name sample fetch
Add a sample fetch for the name of a bind. This can be useful to
take decisions when PROXY protocol is used and we can't rely on dst,
such as the sample config below.
defaults
mode http
listen bar
bind 127.0.0.1:1111
server s1 127.0.1.1:1234 send-proxy
listen foo
bind 127.0.1.1:1234 name foo accept-proxy
http-request return status 200 hdr dst %[dst] if { dst 127.0.1.1 }
(cherry picked from commit
eb421b2fe08ff2cfda1a38138591b8cca303f771)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frédéric Lécaille [Tue, 24 Mar 2020 19:08:30 +0000 (20:08 +0100)]
BUG/MINOR: peers: Use after free of "peers" section.
When a "peers" section has not any local peer, it is removed of the list
of "peers" sections by check_config_validity(). But a stick-table which
refers to a "peers" section stores a pointer to this peers section.
These pointer must be reset to NULL value for each stick-table refering to
such a "peers" section to prevent stktable_init() to start the peers frontend
attached to the peers section dereferencing the invalid pointer.
Furthemore this patch stops the peers frontend as this is done for other
configurations invalidated by check_config_validity().
Thank you to Olivier D for having reported this issue with such a
simple configuration file which made haproxy crash when started with
-c option for configuration file validation.
defaults
mode http
peers mypeers
peer toto 127.0.0.1:1024
backend test
stick-table type ip size 10k expire 1h store http_req_rate(1h) peers mypeers
Must be backported to 2.1 and 2.0.
(cherry picked from commit
87eacbb12fb819adb3acd68a221ead27c70efb43)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 24 Mar 2020 15:42:15 +0000 (16:42 +0100)]
BUG/MINOR: peers: avoid an infinite loop with peers_fe is NULL
Fix an infinite loop which was added in an attempt to fix #558.
If the peers_fe is NULL, it will loop forever.
Must be backported with a2cfd7e as far as 1.8.
(cherry picked from commit
3ef2d565303af6dd7c24b62fdb22e43f1d1638bf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Tue, 24 Mar 2020 15:02:48 +0000 (16:02 +0100)]
BUG/MINOR: peers: init bind_proc to 1 if it wasn't initialized
Tim reported that in master-worker mode, if a stick-table is declared
but not used in the configuration, its associated peers listener won't
bind.
This problem is due the fact that the master-worker and the daemon mode,
depend on the bind_proc field of the peers proxy to disable the listener.
Unfortunately the bind_proc is left to 0 if no stick-table were used in
the configuration, stopping the listener on all processes.
This fixes sets the bind_proc to the first process if it wasn't
initialized.
Should fix bug #558. Should be backported as far as 1.8.
(cherry picked from commit
a2cfd7e356f4d744294b510b05d88bf58304db25)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 23 Mar 2020 08:27:28 +0000 (09:27 +0100)]
BUG/MINOR: haproxy/threads: close a possible race in soft-stop detection
Commit 4b3f27b ("BUG/MINOR: haproxy/threads: try to make all threads
leave together") improved the soft-stop synchronization but it left a
small race open because it looks at tasks_run_queue, which can drop
to zero then back to one while another thread picks the task from the
run queue to insert it into the tasklet_list. The risk is very low but
not null. In addition the condition didn't consider the possible presence
of signals in the queue.
This patch moves the stopping detection just after the "wake" calculation
which already takes care of the various queues' sizes and signals. It
avoids needlessly duplicating these tests.
The bug was discovered during a code review but will probably never be
observed. This fix may be backported to 2.1 and 2.0 along with the commit
above.
(cherry picked from commit
4f46a354e67f4a7781570f6f4e17738eeca9d5ac)
[wt: context adjustment around call to wake_expired_tasks()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 23 Mar 2020 08:11:51 +0000 (09:11 +0100)]
REGTEST: increase timeouts on the seamless-reload test
The abns_socket in seamless-reload regtest regularly fails in Travis-CI
on smaller machines only (typically the ppc64le and sometimes s390x).
The error always reports an incomplete HTTP header as seen from the
client. And this can occasionally be reproduced on the minicloud ppc64le
image when setting a huge file descriptors limit (1 million).
What happens in fact is the following: depending on the binding order,
some connections from the client might reach the TCP listener on the
old instance and be forwarded to the ABNS listener of the second
instance just being prepared to start up. But due to the huge number
of FDs, setting them up takes slightly more time and the 20ms server
timeout may expire before the new instance finishes its startup. This
can result in an occasional 504, except that since the client timeout
is the same as the server timeout, both sides are closed at the same
time and the client doesn't receive the 504.
In addition a second problem plugs onto this: by default http-reuse is
enabled. Some requests being forwarded to the older instance will be
sent over an already established connection. But the CPU used by the
starting process using many FDs will be taken away from the older
process, whose abns listener will not see a request for more than 20ms,
and will decide to kill the idle client connection. At the same moment
the TCP proxy forwards a request over this closing connection, it
detects the close and silently closes the other side to let the
client retry, which is detected by the vtest client as another case
of empty header. This is easier to reproduce in VMs with few CPUs
(2 or less) and some noisy neighbors such as a few spinning loops in
background.
Let's just increase this tests' timeout to avoid this. While a few
ms are close to the scheduler's granularity, this test is never
supposed to trigger the timeouts so it's safe to go higher without
impacts on the test execution time. At one second the problem seems
impossible to reproduce on the minicloud VMs.
(cherry picked from commit
ce6fc25b175a77da8bc4e6cebb1e024af34c0724)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 18 Feb 2020 13:42:33 +0000 (14:42 +0100)]
REGTESTS: use "command -v" instead of "which"
Ilya reported that the "which" utility is not that much portable and is
absent from Fedora. "type -p" is not portable either, and the correct
solution appears to be "command -v", so let's use this for now, we can
change it again in the future in case of problems.
Link: https://www.mail-archive.com/haproxy@formilux.org/msg36332.html
(cherry picked from commit
b5e62679aaf1369b9a8f5604d8ab438e6bc70e2d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Fri, 20 Mar 2020 13:26:32 +0000 (14:26 +0100)]
BUG/MINOR: connections: Make sure we free the connection on failure.
In connect_server(), make sure we properly free a newly created connection
if we somehow fail, and it has not yet been attached to a conn_stream, or
it would lead to a memory leak.
This should appease coverity for backend.c, as reported in inssue #556.
This should be backported to 2.1, 2.0 and 1.9
(cherry picked from commit
c0caac2cc8f51c5802f0128a4ceb0c73ff601ead)
[wt: minor ctx adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Wed, 18 Mar 2020 14:48:29 +0000 (15:48 +0100)]
MINOR: memory: Change the flush_lock to a spinlock, and don't get it in alloc.
The flush_lock was introduced, mostly to be sure that pool_gc() will never
dereference a pointer that has been free'd. __pool_get_first() was acquiring
the lock to, the fear was that otherwise that pointer could get free'd later,
and then pool_gc() would attempt to dereference it. However, that can not
happen, because the only functions that can free a pointer, when using
lockless pools, are pool_gc() and pool_flush(), and as long as those two
are mutually exclusive, nobody will be able to free the pointer while
pool_gc() attempts to access it.
So change the flush_lock to a spinlock, and don't bother acquire/release
it in __pool_get_first(), that way callers of __pool_get_first() won't have
to wait while the pool is flushed. The worst that can happen is we call
__pool_refill_alloc() while the pool is getting flushed, and memory can
get allocated just to be free'd.
This may help with github issue #552
This may be backported to 2.1, 2.0 and 1.9.
(cherry picked from commit
899fb8abdcda90e3b3c69f6945f83c4c1107c459)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Wed, 18 Mar 2020 12:10:05 +0000 (13:10 +0100)]
BUG/MEDIUM: wdt: Don't ignore WDTSIG and DEBUGSIG in __signal_process_queue().
When running __signal_process_queue(), we ignore most signals. We can't,
however, ignore WDTSIG and DEBUGSIG, otherwise that thread may end up
waiting for another one that could hold a glibc lock, while the other thread
wait for this one to enter debug_handler().
So make sure WDTSIG and DEBUGSIG aren't ignored, if they are defined.
This probably explains the watchdog deadlock described in github issue
This should be backported to 2.1, 2.0 and 1.9.
(cherry picked from commit
b0198cc4132381910cdeb9b5a867632b8b83262c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Wed, 18 Mar 2020 12:07:19 +0000 (13:07 +0100)]
MINOR: wdt: Move the definitions of WDTSIG and DEBUGSIG into types/signal.h.
Move the definition of WDTSIG and DEBUGSIG from wdt.c and debug.c into
types/signal.h, so that we can access them in another file.
We need those definition to avoid blocking those signals when running
__signal_process_queue().
This should be backported to 2.1, 2.0 and 1.9.
(cherry picked from commit
de01ea9878870dad5df1471743097851f53e7793)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Sat, 14 Mar 2020 12:47:28 +0000 (17:47 +0500)]
DOC: assorted typo fixes in the documentation
This is the fourth round of cleanups in various docs
(cherry picked from commit
1fae8db7b7a9db2a7442674232f1382205947abb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Emeric Brun [Mon, 16 Mar 2020 09:51:01 +0000 (10:51 +0100)]
BUG/MEDIUM: peers: resync ended with RESYNC_PARTIAL in wrong cases.
This bug was introduced with peers.c code re-work (
7d0ceeec80):
"struct peer" flags are mistakenly checked instead of
"struct peers" flags to check the resync status of the local peer.
The issue was reported here:
https://github.com/haproxy/haproxy/issues/545
This bug affects all branches >= 2.0 and should be backported.
(cherry picked from commit
70de43b77b10c6868b1ed42ac5439731ae6fa911)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 18 Mar 2020 07:20:45 +0000 (08:20 +0100)]
BUILD: makefile: fix expression again to detect ARM platform
I messed up the fix in 67b095e ("BUILD: makefile: fix regex syntax in
ARM platform detection"), I tried it by hand in the shell without "-v"
but left it in the expression. It works on ARM because it only finds
lines starting with '#' but on other platforms it insists for -latomic.
(cherry picked from commit
48e8603a9c3b5b2509511665cb58d3bde8dfdb65)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Mon, 16 Mar 2020 08:38:00 +0000 (09:38 +0100)]
BUILD: makefile: fix regex syntax in ARM platform detection
Commit d93e6ec ("BUILD: on ARM, must be linked to libatomic.") broke the
build due to a missing backslash in front of the '#':
Makefile:331: *** invalid syntax in conditional. Stop.
Let's address this and make sure we only pick relevant lines (and not
possibly empty lines).
(cherry picked from commit
67b095e797a156ae27b7b52f6ccf57171717dd16)
Signed-off-by: Willy Tarreau <w@1wt.eu>
David Carlier [Sun, 15 Mar 2020 10:30:51 +0000 (10:30 +0000)]
BUILD: on ARM, must be linked to libatomic.
For load/store operations, needs to be linked to.
tested on raspberry.
(cherry picked from commit
d93e6ec4cc44b48bc5370c724d3661b52ba9976c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Balvinder Singh Rawat [Sat, 14 Mar 2020 06:41:50 +0000 (12:11 +0530)]
DOC: correct typo in alert message about rspirep
This message comes when we run:
haproxy -c -V -f /etc/haproxy/haproxy.cfg
[ALERT] 072/233727 (30865) : parsing [/etc/haproxy/haproxy.cfg:34] : The 'rspirep' directive is not supported anymore sionce HAProxy 2.1. Use 'http-response replace-header' instead.
[ALERT] 072/233727 (30865) : Error(s) found in configuration file : /etc/haproxy/haproxy.cfg
[ALERT] 072/233727 (30865) : Fatal errors found in configuration.
(cherry picked from commit
def595e2df82c1d2932fd7275f6e51ba18a0961a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Fri, 13 Mar 2020 11:34:22 +0000 (12:34 +0100)]
DOC: proxy_protocol: Reserve TLV type 0x05 as PP2_TYPE_UNIQUE_ID
This reserves and defines TLV type 0x05.
(cherry picked from commit
b435f7762026dd7d278863900b8ab8603529c3f4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 12 Mar 2020 16:28:01 +0000 (17:28 +0100)]
BUG/MINOR: haproxy/threads: try to make all threads leave together
There's a small issue with soft stop combined with the incoming
connection load balancing. A thread may dispatch a connection to
another one at the moment stopping=1 is set, and the second one could
stop by seeing (jobs - unstoppable_jobs) == 0 in run_poll_loop(),
without ever picking these connections from the queue. This is
visible in that it may occasionally cause a connection drop on
reload since no remaining thread will ever pick that connection
anymore.
In order to address this, this patch adds a stopping_thread_mask
variable by which threads acknowledge their willingness to stop
when their runqueue is empty. And all threads will only stop at
this moment, so that if finally some late work arrives in the
thread's queue, it still has a chance to process it.
This should be backported to 2.1 and 2.0.
(cherry picked from commit
4b3f27b67f2dec16bc06084df2dfe9f20072584e)
[wt: minor ctx adj: wake_expired_tasks() is earlier in 2.2]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 12 Mar 2020 16:33:29 +0000 (17:33 +0100)]
BUG/MINOR: listener/mq: do not dispatch connections to remote threads when stopping
When stopping there is a risk that other threads are already in the
process of stopping, so let's not add new work in their queue and
instead keep the incoming connection local.
This should be backported to 2.1 and 2.0.
(cherry picked from commit
a7da5e8dd058ee1e283a06342e9701a8eca78ac6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 12 Mar 2020 16:24:53 +0000 (17:24 +0100)]
BUG/MINOR: haproxy: always initialize sleeping_thread_mask
Surprizingly the variable was never initialized, though on most
platforms it's zeroed at boot, and it is relatively harmless
anyway since in the worst case the bits are updated around poll().
This was introduced by commit
79321b95a85 and needs to be backported
as far as 1.9.
(cherry picked from commit
f8ea00e05e1e5358311086e2a4b9a47fb3fb306c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Thu, 12 Mar 2020 18:05:39 +0000 (19:05 +0100)]
BUG/MEDIUM: pools: Always update free_list in pool_gc().
In pool_gc(), when we're not using lockless pool, always update free_list,
and read from it the next element to free. As we now unlock the pool while
we're freeing the item, another thread could have updated free_list in our
back. Not doing so could lead to segfaults when pool_gc() is called.
This should be backported to 2.1.
(cherry picked from commit
51d9339d04fb53c63a3f55c9cea2f4a63e6facbe)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 11 Mar 2020 23:31:18 +0000 (00:31 +0100)]
BUG/MEDIUM: random: align the state on 2*64 bits for ARM64
x86_64 and ARM64 do support the double-word atomic CAS. However on
ARM it must be done only on aligned data. The random generator makes
use of such double-word atomic CAS when available but didn't enforce
alignment, which causes ARM64 to crash early in the startup since
commit 52bf839 ("BUG/MEDIUM: random: implement a thread-safe and
process-safe PRNG").
This commit just unconditionally aligns the arrays. It must be
backported to all branches where the commit above is backported
(likely till 2.0).
(cherry picked from commit
1544c14c57f4e77df2a8dcc27bc7eaaba8bbb833)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Wed, 11 Mar 2020 14:09:16 +0000 (15:09 +0100)]
MINOR: mt_lists: Appease gcc.
gcc is confused, and think p may end up being NULL in _MT_LIST_RELINK_DELETED.
It should never happen, so let gcc know that.
(cherry picked from commit
49983a9fe11840e0fc14a96e4097ad56cc2d06c8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Tue, 10 Mar 2020 16:41:53 +0000 (17:41 +0100)]
BUG/MEDIUM: mt_lists: Make sure we set the deleted element to NULL;
In MT_LIST_DEL_SAFE(), when the code was changed to use a temporary variable
instead of using the provided pointer directly, we shouldn't have changed
the code that set the pointer to NULL, as we really want the pointer
provided to be nullified, otherwise other parts of the code won't know
we just deleted an element, and bad things will happen.
This should be backported to 2.1.
(cherry picked from commit
1d117e3dcdae6b59b19e0d875530b5d1f6b24c03)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 10 Mar 2020 08:26:17 +0000 (09:26 +0100)]
BUILD: wdt: only test for SI_TKILL when compiled with thread support
SI_TKILL is not necessarily defined on older systems and is used only
with the pthread_kill() call a few lines below, so it should also be
subject to the USE_THREAD condition.
(cherry picked from commit
0627815f701ee6549bcf4c2611df8ccdd3bd3fc9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Lukas Tribus [Mon, 9 Mar 2020 23:56:09 +0000 (00:56 +0100)]
DOC: ssl: clarify security implications of TLS tickets
Clarifies security implications of TLS ticket usage when not
rotating TLS ticket keys, after commit
7b5e136458 ("DOC:
improve description of no-tls-tickets").
(cherry picked from commit
bdb386d3d9d81b863470086ece1b0709d3cd8ec8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Fri, 6 Mar 2020 18:22:22 +0000 (23:22 +0500)]
DOC: assorted typo fixes in the documentation
This is the third round of cleanups in various docs
(cherry picked from commit
2075ca8a93d45f2319b892ed142c3ccb5aaf6a5c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Björn Jacke [Thu, 13 Feb 2020 13:43:44 +0000 (14:43 +0100)]
DOC: improve description of no-tls-tickets
It was not obvious, that this setting only affects TLS versions <= 1.2 and it
we should also mention the security implication of session tickets here.
Signed-off-by: Bjoern Jacke <bjacke@samba.org>
(cherry picked from commit
7b5e1364587beae59a39da5a86ec095fa8bedef8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Bjoern Jacke [Thu, 13 Feb 2020 13:16:16 +0000 (14:16 +0100)]
DOC: fix typo about no-tls-tickets
It's "no-tls-tickets", not "no-tlsv-tickets"
Signed-off-by: Bjoern Jacke <bjacke@samba.org>
(cherry picked from commit
5ab7eb686078cf22951ed4489df68c328aa9c1cf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 6 Mar 2020 14:23:18 +0000 (15:23 +0100)]
BUG/MINOR: rules: Increment be_counters if backend is assigned for a silent-drop
Backend counters must be incremented only if a backend was already assigned to
the stream (when the stream exists). Otherwise, it means we are still on the
frontend side.
This patch may be backported as far as 1.6.
(cherry picked from commit
ddc005ae5783b6479292156b97db7b6fbdabf6a3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 6 Mar 2020 14:10:46 +0000 (15:10 +0100)]
BUG/MINOR: rules: Preserve FLT_END analyzers on silent-drop action
When at least a filter is attached to a stream, FLT_END analyzers must be
preserved on request and response channels.
This patch should be backported as far as 1.7.
(cherry picked from commit
177f480f2c5b6a73b148a7069a71ddbff3e9f6a6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 6 Mar 2020 14:07:09 +0000 (15:07 +0100)]
BUG/MINOR: http-rules: Fix a typo in the reject action function
A typo was introduced by the commit
c5bb5a0f2 ("BUG/MINOR: http-rules: Preserve
FLT_END analyzers on reject action").
This patch must be backported with the commit
c5bb5a0f2.
(cherry picked from commit
d4a824e533ef940e9726dbf3d3f94993d97d6391)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 6 Mar 2020 13:02:57 +0000 (14:02 +0100)]
BUG/MINOR: http-rules: Preserve FLT_END analyzers on reject action
When at least a filter is attached to a stream, FLT_END analyzers must be
preserved on request and response channels.
This patch should be backported as far as 1.8.
(cherry picked from commit
c5bb5a0f2b36bffcea807bd7e529abdc35c23067)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 26 Feb 2020 10:59:19 +0000 (11:59 +0100)]
BUG/MINOR: lua: Ignore the reserve to know if a channel is full or not
The Lua function Channel.is_full() should not take care of the reserve because
it is not called from a producer (an applet for instance). From an action, it is
allowed to overwrite the buffer reserve.
This patch should be backported as far as 1.7. But it must be adapted for 1.8
and lower because there is no HTX on these versions.
(cherry picked from commit
0ec740eaee5b919b7d568daf251c8ce218143639)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Mar 2020 15:21:01 +0000 (16:21 +0100)]
BUG/MINOR: http-ana: Reset request analysers on a response side error
When an error occurred on the response side, request analysers must be reset. At
this stage, only AN_REQ_HTTP_XFER_BODY analyser remains, and possibly
AN_REQ_FLT_END, if at least one filter is attached to the stream. So it is safe
to remove the AN_REQ_HTTP_XFER_BODY analyser. An error was already handled and a
response was already returned to the client (or it was at least scheduled to be
sent). So there is no reason to continue to process the request payload. It may
cause some troubles for the filters because when an error occurred, data from
the request buffer are truncated.
This patch must be backported as far as 1.9, for the HTX part only. I don't know
if the legacy HTTP code is affected.
(cherry picked from commit
e58c0002ff7dc1f88990171810f268ae26d0cf99)
[wt: context adjustments since this place was already touched by
b8a5371a]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Mar 2020 15:20:05 +0000 (16:20 +0100)]
BUG/MEDIUM: compression/filters: Fix loop on HTX blocks compressing the payload
During the payload filtering, the offset is relative to the head of the HTX
message and not its first index. This index is the position of the first block
to (re)start the HTTP analysis. It must be used during HTTP analysis but not
during the payload forwarding.
So, from the compression filter point of view, when we loop on the HTX blocks to
compress the response payload, we must start from the head of the HTX
message. To ease the loop, we use the function htx_find_offset().
This patch must be backported as far as 2.0. It depends on the commit "MINOR:
htx: Add a function to return a block at a specific an offset". So this one must
be backported first.
(cherry picked from commit
e6a62bf79655623da0ecd7e412175bf38c99ab9d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Mar 2020 15:19:50 +0000 (16:19 +0100)]
BUG/MEDIUM: cache/filters: Fix loop on HTX blocks caching the response payload
During the payload filtering, the offset is relative to the head of the HTX
message and not its first index. This index is the position of the first block
to (re)start the HTTP analysis. It must be used during HTTP analysis but not
during the payload forwarding.
So, from the cache point of view, when we loop on the HTX blocks to cache the
response payload, we must start from the head of the HTX message. To ease the
loop, we use the function htx_find_offset().
This patch must be backported as far as 2.0. It depends on the commit "MINOR:
htx: Add a function to return a block at a specific an offset". So this one must
be backported first.
(cherry picked from commit
497c7595588371e8ac85c322825455d7c7d4e407)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 24 Feb 2020 10:41:59 +0000 (11:41 +0100)]
MINOR: htx: Add a function to return a block at a specific offset
The htx_find_offset() function may be used to look for a block at a specific
offset in an HTX message, starting from the message head. A compound result is
returned, an htx_ret structure, with the found block and the position of the
offset in the block. If the offset is ouside of the HTX message, the returned
block is NULL.
(cherry picked from commit
1cdceb936521ca4dc06d4f57af909310ec374a5e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 26 Feb 2020 14:47:22 +0000 (15:47 +0100)]
BUG/MINOR: filters: Forward everything if no data filters are called
If a filter enable the data filtering, in TCP or in HTTP, but it does not
defined the corresponding callback function (so http_payload() or
tcp_payload()), it will be ignored. If all configured data filter do the same,
we must be sure to forward everything. Otherwise nothing will be forwarded at
all.
This patch must be forwarded as far as 1.9.
(cherry picked from commit
81340d7b533ac24cb791fbebede4f7bb36a9f007)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 24 Feb 2020 15:20:09 +0000 (16:20 +0100)]
BUG/MINOR: filters: Use filter offset to decude the amount of forwarded data
When the tcp or http payload is filtered, it is important to use the filter
offset to decude the amount of forwarded data because this offset may change
during the call to the callback function. So we should not rely on a local
variable defined before this call.
For now, existing HAproxy filters don't change this offset, so this bug may only
affect external filters.
This patch must be forwarded as far as 1.9.
(cherry picked from commit
c50ee0b3b4001300f47f77bbdc38b2991b76d663)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 31 Mar 2020 14:36:20 +0000 (16:36 +0200)]
REGTEST: make the PROXY TLV validation depend on version 2.2
Regtest proxy_protocol_tlv_validation was added by commit
488ee7fb6e
("BUG/MAJOR: proxy_protocol: Properly validate TLV lengths") but it
relies on a trick involving http-after-response to append a header
after a 400-badreq response, which is not possible in earlier versions,
so make it depend on 2.2.
(cherry picked from commit
1d52c7b52bb5e11dee64aff148332b830c93e95a)
Signed-off-by: Willy Tarreau <w@1wt.eu>