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>
Tim Duesterhus [Thu, 5 Mar 2020 21:55:20 +0000 (22:55 +0100)]
BUG/MAJOR: proxy_protocol: Properly validate TLV lengths
This patch fixes PROXYv2 parsing when the payload of the TCP connection is
fused with the PROXYv2 header within a single recv() call.
Previously HAProxy ignored the PROXYv2 header length when attempting to
parse the TLV, possibly interpreting the first byte of the payload as a
TLV type.
This patch adds proper validation. It ensures that:
1. TLV parsing stops when the end of the PROXYv2 header is reached.
2. TLV lengths cannot exceed the PROXYv2 header length.
3. The PROXYv2 header ends together with the last TLV, not allowing for
"stray bytes" to be ignored.
A reg-test was added to ensure proper behavior.
This patch tries to find the sweat spot between a small and easily
backportable one, and a cleaner one that's more easily adaptable to
older versions, hence why it merges the "if" and "while" blocks which
causes a reindent of the whole block. It should be used as-is for
versions 1.9 to 2.1, the block about PP2_TYPE_AUTHORITY should be
dropped for 2.0 and the block about CRC32C should be dropped for 1.8.
This bug was introduced when TLV parsing was added. This happened in commit
b3e54fe387c7c1ea750f39d3029672d640c499f9. This commit was first released
with HAProxy 1.6-dev1.
A similar issue was fixed in commit
7209c204bd6f3c49132264c7a58f689cdc741c12.
This patch must be backported to HAProxy 1.6+.
(cherry picked from commit
488ee7fb6e4a388bb68153341826a6391da794e9)
[wt: the regtest will fail, it requires 2.2]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 6 Mar 2020 09:25:31 +0000 (10:25 +0100)]
BUG/MINOR: init: make the automatic maxconn consider the max of soft/hard limits
James Stroehmann reported something working as documented but that can
be considered as a regression in the way the automatic maxconn is
calculated from the process' limits :
https://www.mail-archive.com/haproxy@formilux.org/msg36523.html
The purpose of the changes in 2.0 was to have maxconn default to the
highest possible value permitted to the user based on the ulimit -n
setting, however the calculation starts from the soft limit, which
can be lower than what users were allowed to with previous versions
where the default value of 2000 would force a higher ulimit -n as
long as it fitted in the hard limit.
Usually this is not noticeable if the user changes the limits, because
quite commonly setting a new value restricts both the soft and hard
values.
Let's instead always use the max between the hard and soft limits, as
we know these values are permitted. This was tried on the following
setup:
$ cat ulimit-n.cfg
global
stats socket /tmp/sock1 level admin
$ ulimit -n
1024
Before the change the limits would show like this:
$ socat - /tmp/sock1 <<< "show info" | grep -im2 ^Max
Maxsock: 1023
Maxconn: 489
After the change the limits are now much better and more in line with
the default settings in earlier versions:
$ socat - /tmp/sock1 <<< "show info" | grep -im2 ^Max
Maxsock: 4095
Maxconn: 2025
The difference becomes even more obvious when running moderately large
configs with hundreds of checked servers and hundreds of listeners:
$ cat ulimit-n.cfg
global
stats socket /tmp/sock1 level admin
listen l
bind :10000-10300
server-template srv- 300 0.0.0.0 check disabled
Before After
Maxsock 1024 4096
Maxconn 189 1725
This issue is tagged as minor since a trivial config change fixes it,
but it would help new users to have it backported as far as 2.0.
(cherry picked from commit
b1beaa302c4c35080f217ce5fb63fed4530e1879)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Fri, 6 Mar 2020 08:07:38 +0000 (13:07 +0500)]
DOC: assorted typo fixes in the documentation and Makefile
This is another round of cleanups in various docs and comments in the
Makefile.
(cherry picked from commit
2a950d02a951715447d4068ec6689a842d452aaa)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Ilya Shipitsin [Sat, 29 Feb 2020 07:34:59 +0000 (12:34 +0500)]
DOC: configuration.txt: fix various typos
This was done using automatic spellcheck.
(cherry picked from commit
8525fd95b2467609a70a62375009374e58d22829)
[wt: removed parts irrelevant to 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Tue, 17 Mar 2020 20:08:24 +0000 (21:08 +0100)]
BUG/MINOR: pattern: Do not pass len = 0 to calloc()
The behavior of calloc() when being passed `0` as `nelem` is implementation
defined. It may return a NULL pointer.
Avoid this issue by checking before allocating. While doing so adjust the local
integer variables that are used to refer to memory offsets to `size_t`.
This issue was introced in commit
f91ac19299fe216a793ba6550dca06b688b31549. This
patch should be backported together with that commit.
(cherry picked from commit
b584b4475be09a20dea07f35c605487a1b73425e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Carl Henrik Lunde [Thu, 27 Feb 2020 15:45:50 +0000 (16:45 +0100)]
OPTIM: startup: fast unique_id allocation for acl.
pattern_finalize_config() uses an inefficient algorithm which is a
problem with very large configuration files. This affects startup, and
therefore reload time. When haproxy is deployed as a router in a
Kubernetes cluster the generated configuration file may be large and
reloads are frequently occuring, which makes this a significant issue.
The old algorithm is O(n^2)
* allocate missing uids - O(n^2)
* sort linked list - O(n^2)
The new algorithm is O(n log n):
* find the user allocated uids - O(n)
* store them for efficient lookup - O(n log n)
* allocate missing uids - n times O(log n)
* sort all uids - O(n log n)
* convert back to linked list - O(n)
Performance examples, startup time in seconds:
pat_refs old new
1000 0.02 0.01
10000 2.1 0.04
20000 12.3 0.07
30000 27.9 0.10
40000 52.5 0.14
50000 77.5 0.17
Please backport to 1.8, 2.0 and 2.1.
(cherry picked from commit
f91ac19299fe216a793ba6550dca06b688b31549)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 5 Mar 2020 15:03:58 +0000 (16:03 +0100)]
DOC: fix incorrect indentation of http_auth_*
These ones were incorrectly indented and thus not displayed optimally
in the HTML version. This addresses issue #533.
(cherry picked from commit
c9c6cdbf9c0e61ee88d68960a3220a4fcbf912cd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Mar 2020 09:46:13 +0000 (10:46 +0100)]
BUG/MINOR: wdt: do not return an error when the watchdog couldn't be enabled
On operating systems not supporting to create a timer on
POSIX_THREAD_CPUTIME we emit a warning but we return an error so the
process fails to start, which is absurd. Let's return a success once
the warning is emitted instead.
This may be backported to 2.1 and 2.0.
(cherry picked from commit
7259fa2b89d20462f0222ac49d51575d20025e6b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 16:06:52 +0000 (17:06 +0100)]
BUILD: tools: remove obsolete and conflicting trace() from standard.c
Since commit
4c2ae48375 ("MINOR: trace: implement a very basic trace()
function") merged in 2.1, trace() is an inline function. It must not
appear in standard.c anymore and may break build depending on includes.
This can be backported to 2.1.
(cherry picked from commit
55a6c4f34de59cc3d5d5079968e382c6fc303ef9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 14:25:10 +0000 (15:25 +0100)]
MINOR: haproxy: export main to ease access from debugger
Better just export main instead of declaring it as extern, it's cleaner
and may be usable elsewhere.
(cherry picked from commit
1827845a3d9f15e2186e1e65dd925e8e7912fab4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 07:31:34 +0000 (08:31 +0100)]
BUG/MEDIUM: debug: make the debug_handler check for the thread in threads_to_dump
It happens that just sending the debug signal to the process makes on
thread wait for its turn while nobody wants to dump. We need to at
least verify that a dump was really requested for this thread.
This can be backported to 2.1 and 2.0.
(cherry picked from commit
82aafc4a0fd5cf5a5203b37f622105a73fe5ba94)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 3 Mar 2020 06:04:42 +0000 (07:04 +0100)]
MINOR: debug: report the task handler's pointer relative to main
Often in crash dumps we see unknown function pointers. Let's display
them relative to main, that helps quite a lot figure the function
from an executable, for example:
(gdb) x/a main+645360
0x4c56a0 <h1_timeout_task>: 0x2e6666666666feeb
This could be backported to 2.0.
(cherry picked from commit
516853f1cc144bfe5d0010fc12d5f385341e67ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 11 Mar 2020 10:54:04 +0000 (11:54 +0100)]
BUG/MAJOR: list: fix invalid element address calculation
Ryan O'Hara reported that haproxy breaks on fedora-32 using gcc-10
(pre-release). It turns out that constructs such as:
while (item != head) {
item = LIST_ELEM(item.n);
}
loop forever, never matching <item> to <head> despite a printf there
showing them equal. In practice the problem is that the LIST_ELEM()
macro is wrong, it assigns the subtract of two pointers (an integer)
to another pointer through a cast to its pointer type. And GCC 10 now
considers that this cannot match a pointer and silently optimizes the
comparison away. A tested workaround for this is to build with
-fno-tree-pta. Note that older gcc versions even with -ftree-pta do
not exhibit this rather surprizing behavior.
This patch changes the test to instead cast the null-based address to
an int to get the offset and subtract it from the pointer, and this
time it works. There were just a few places to adjust. Ideally
offsetof() should be used but the LIST_ELEM() API doesn't make this
trivial as it's commonly called with a typeof(ptr) and not typeof(ptr*)
thus it would require to completely change the whole API, which is not
something workable in the short term, especially for a backport.
With this change, the emitted code is subtly different even on older
versions. A code size reduction of ~600 bytes and a total executable
size reduction of ~1kB are expected to be observed and should not be
taken as an anomaly. Typically this loop in dequeue_proxy_listeners() :
while ((listener = MT_LIST_POP(...)))
used to produce this code where the comparison is performed on RAX
while the new offset is assigned to RDI even though both are always
identical:
53ded8: 48 8d 78 c0 lea -0x40(%rax),%rdi
53dedc: 48 83 f8 40 cmp $0x40,%rax
53dee0: 74 39 je 53df1b <dequeue_proxy_listeners+0xab>
and now produces this one which is slightly more efficient as the
same register is used for both purposes:
53dd08: 48 83 ef 40 sub $0x40,%rdi
53dd0c: 74 2d je 53dd3b <dequeue_proxy_listeners+0x9b>
Similarly, retrieving the channel from a stream_interface using si_ic()
and si_oc() used to cause this (stream-int in rdi):
1cb7: c7 47 1c 00 02 00 00 movl $0x200,0x1c(%rdi)
1cbe: f6 47 04 10 testb $0x10,0x4(%rdi)
1cc2: 74 1c je 1ce0 <si_report_error+0x30>
1cc4: 48 81 ef 00 03 00 00 sub $0x300,%rdi
1ccb: 81 4f 10 00 08 00 00 orl $0x800,0x10(%rdi)
and now causes this:
1cb7: c7 47 1c 00 02 00 00 movl $0x200,0x1c(%rdi)
1cbe: f6 47 04 10 testb $0x10,0x4(%rdi)
1cc2: 74 1c je 1ce0 <si_report_error+0x30>
1cc4: 81 8f 10 fd ff ff 00 orl $0x800,-0x2f0(%rdi)
There is extremely little chance that this fix wakes up a dormant bug as
the emitted code effectively does what the source code intends.
This must be backported to all supported branches (dropping MT_LIST_ELEM
and the spoa_example parts as needed), since the bug is subtle and may
not always be visible even when compiling with gcc-10.
(cherry picked from commit
855796bdc8b1bc2ebcc697ca99fb304bc6e2abe9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Sun, 8 Mar 2020 16:53:53 +0000 (17:53 +0100)]
BUG/MINOR: checks/threads: use ha_random() and not rand()
In order to honor spread_checks we currently call rand() which is not
thread safe and which must never turn its internal state to zero. This
is not thread safe, let's use ha_random() instead. This is a complement
to commimt
52bf839394 ("BUG/MEDIUM: random: implement a thread-safe and
process-safe PRNG") and may be backported with it.
(cherry picked from commit
5a6d3e797edc7ac52560cc1a5bd90a984b6b350b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Sun, 8 Mar 2020 16:31:39 +0000 (17:31 +0100)]
MINOR: backend: use a single call to ha_random32() for the random LB algo
For the random LB algorithm we need a random 32-bit hashing key that used
to be made of two calls to random(). Now we can simply perform a single
call to ha_random32() and get rid of the useless operations.
(cherry picked from commit
b9f54c55929eddb6446f8d5c983fb8f1008eb0c6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Sat, 7 Mar 2020 23:42:37 +0000 (00:42 +0100)]
BUG/MEDIUM: random: implement a thread-safe and process-safe PRNG
This is the replacement of failed attempt to add thread safety and
per-process sequences of random numbers initally tried with commit
1c306aa84d ("BUG/MEDIUM: random: implement per-thread and per-process
random sequences").
This new version takes a completely different approach and doesn't try
to work around the horrible OS-specific and non-portable random API
anymore. Instead it implements "xoroshiro128**", a reputedly high
quality random number generator, which is one of the many variants of
xorshift, which passes all quality tests and which is described here:
http://prng.di.unimi.it/
While not cryptographically secure, it is fast and features a 2^128-1
period. It supports fast jumps allowing to cut the period into smaller
non-overlapping sequences, which we use here to support up to 2^32
processes each having their own, non-overlapping sequence of 2^96
numbers (~7*10^28). This is enough to provide 1 billion randoms per
second and per process for 2200 billion years.
The implementation was made thread-safe either by using a double 64-bit
CAS on platforms supporting it (x86_64, aarch64) or by using a local
lock for the time needed to perform the shift operations. This ensures
that all threads pick numbers from the same pool so that it is not
needed to assign per-thread ranges. For processes we use the fast jump
method to advance the sequence by 2^96 for each process.
Before this patch, the following config:
global
nbproc 8
frontend f
bind :4445
mode http
log stdout format raw daemon
log-format "%[uuid] %pid"
redirect location /
Would produce this output:
a4d0ad64-2645-4b74-b894-
48acce0669af 12987
a4d0ad64-2645-4b74-b894-
48acce0669af 12992
a4d0ad64-2645-4b74-b894-
48acce0669af 12986
a4d0ad64-2645-4b74-b894-
48acce0669af 12988
a4d0ad64-2645-4b74-b894-
48acce0669af 12991
a4d0ad64-2645-4b74-b894-
48acce0669af 12989
a4d0ad64-2645-4b74-b894-
48acce0669af 12990
82d5f6cd-f6c1-4f85-a89c-
36ae85d26fb9 12987
82d5f6cd-f6c1-4f85-a89c-
36ae85d26fb9 12992
82d5f6cd-f6c1-4f85-a89c-
36ae85d26fb9 12986
(...)
And now produces:
f94b29b3-da74-4e03-a0c5-
a532c635bad9 13011
47470c02-4862-4c33-80e7-
a952899570e5 13014
86332123-539a-47bf-853f-
8c8ea8b2a2b5 13013
8f9efa99-3143-47b2-83cf-
d618c8dea711 13012
3cc0f5c7-d790-496b-8d39-
bec77647af5b 13015
3ec64915-8f95-4374-9e66-
e777dc8791e0 13009
0f9bf894-dcde-408c-b094-
6e0bb3255452 13011
49c7bfde-3ffb-40e9-9a8d-
8084d650ed8f 13014
e23f6f2e-35c5-4433-a294-
b790ab902653 13012
There are multiple benefits to using this method. First, it doesn't
depend anymore on a non-portable API. Second it's thread safe. Third it
is fast and more proven than any hack we could attempt to try to work
around the deficiencies of the various implementations around.
This commit depends on previous patches "MINOR: tools: add 64-bit rotate
operators" and "BUG/MEDIUM: random: initialize the random pool a bit
better", all of which will need to be backported at least as far as
version 2.0. It doesn't require to backport the build fixes for circular
include files dependecy anymore.
(cherry picked from commit
52bf839394e683eec2fa8aafff5a0dd51d2dd365)
[wt: minor context adjustments]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Sat, 7 Mar 2020 23:41:00 +0000 (00:41 +0100)]
MINOR: tools: add 64-bit rotate operators
This adds rotl64/rotr64 to rotate a 64-bit word by an arbitrary number
of bits. It's mainly aimed at being used with constants.
(cherry picked from commit
7a40909c00c13d2733bc1d41ddc7dc1d19e05da9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 6 Mar 2020 17:57:15 +0000 (18:57 +0100)]
BUG/MEDIUM: random: initialize the random pool a bit better
Since the UUID sample fetch was created, some people noticed that in
certain virtualized environments they manage to get exact same UUIDs
on different instances started exactly at the same moment. It turns
out that the randoms were only initialized to spread the health checks
originally, not to provide "clean" randoms.
This patch changes this and collects more randomness from various
sources, including existing randoms, /dev/urandom when available,
RAND_bytes() when OpenSSL is available, as well as the timing for such
operations, then applies a SHA1 on all this to keep a 160 bits random
seed available, 32 of which are passed to srandom().
It's worth mentioning that there's no clean way to pass more than 32
bits to srandom() as even initstate() provides an opaque state that
must absolutely not be tampered with since known implementations
contain state information.
At least this allows to have up to 4 billion different sequences
from the boot, which is not that bad.
Note that the thread safety was still not addressed, which is another
issue for another patch.
This must be backported to all versions containing the UUID sample
fetch function, i.e. as far as 2.0.
(cherry picked from commit
6c3a681bd61a3fd54de8fe644db17b9676939396)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 27 Feb 2020 15:12:07 +0000 (16:12 +0100)]
MINOR: contrib/prometheus-exporter: Add the last heathcheck duration metric
ST_F_CHECK_DURATION is now part of exported server metrics, named
haproxy_server_check_duration_seconds and expressed in seconds. For a given
server, this value is exported only if the healthcheck is finished (the status
is greater or equal to HCHK_STATUS_CHECKED).
This patch fixes the issue #519. It may be backported as fat as 2.0.
(cherry picked from commit
2711e51016cfc408deb03e66f46bea0fa5bf954c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 21 Nov 2019 13:35:46 +0000 (14:35 +0100)]
MINOR: contrib/prometheus-exporter: Add heathcheck status/code in server metrics
ST_F_CHECK_STATUS and ST_F_CHECK_CODE are now part of exported server metrics:
* haproxy_server_check_status
* haproxy_server_check_code
The heathcheck status is an integer corresponding to HCHK_STATUS value.
(cherry picked from commit
cf403f32e4bd5217064f8831930b45c5fb37131b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 28 Feb 2020 08:47:07 +0000 (09:47 +0100)]
BUG/MINOR: http-htx: Do case-insensive comparisons on Host header name
When a header is added or modified, in http_add_header() or
http_replace_header(), a comparison is performed on its name to know if it is
the Host header and if the authority part of the uri must be updated or
not. This comparision must be case-insensive.
This patch should fix the issue #522. It must be backported to 2.1.
(cherry picked from commit
3e1f7f4a39456051873aec75eb7d9e8f4ace6813)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Lukas Tribus [Thu, 27 Feb 2020 14:47:24 +0000 (15:47 +0100)]
BUG/MINOR: dns: ignore trailing dot
As per issue #435 a hostname with a trailing dot confuses our DNS code,
as for a zero length DNS label we emit a null-byte. This change makes us
ignore the zero length label instead.
Must be backported to 1.8.
(cherry picked from commit
81725b867c20a688c3877421383bfe1ba9348a09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Wed, 26 Feb 2020 15:20:49 +0000 (16:20 +0100)]
BUG/MINOR: sample: Make sure to return stable IDs in the unique-id fetch
Previously when the `unique-id-format` contained non-deterministic parts,
such as the `uuid` fetch each use of the `unique-id` fetch would generate
a new unique ID, replacing the old one. The following configuration shows
the error:
global
log stdout format short daemon
listen test
log global
log-format "%ID"
unique-id-format %{+X}o\ TEST-%[uuid]
mode http
bind *:8080
http-response set-header A %[unique-id]
http-response set-header B %[unique-id]
server example example.com:80
Without the patch the contents of the `A` and `B` response header would
differ.
This bug was introduced in commit
f4011ddcf5b41284d2b137e84c25f2d1264ce458,
which was first released with HAProxy 1.7-dev3.
This fix should be backported to HAProxy 1.7+.
(cherry picked from commit
530408f976e5fe2f2f2b4b733b39da36770b566f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 26 Feb 2020 12:51:38 +0000 (13:51 +0100)]
BUG/MINOR: h2: reject again empty :path pseudo-headers
Since commit
92919f7fd5 ("MEDIUM: h2: make the request parser rebuild
a complete URI") we make sure to rebuild a complete URI. Unfortunately
the test for an empty :path pseudo-header that is mandated by #8.1.2.3
appened to be performed on the URI before this patch, which is never
empty anymore after being rebuilt, causing h2spec to complain :
8. HTTP Message Exchanges
8.1. HTTP Request/Response Exchange
8.1.2. HTTP Header Fields
8.1.2.3. Request Pseudo-Header Fields
- 1: Sends a HEADERS frame with empty ":path" pseudo-header field
-> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
RST_STREAM Frame (Error Code: PROTOCOL_ERROR)
Connection closed
Actual: DATA Frame (length:0, flags:0x01, stream_id:1)
It's worth noting that this error doesn't trigger when calling h2spec
with a timeout as some scripts do, which explains why it wasn't detected
after the patch above.
This fixes one half of issue #471 and should be backported to 2.1.
(cherry picked from commit
fd2658c0c6a275b497c92de2fc8513e458d0f169)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sat, 22 Feb 2020 14:55:33 +0000 (15:55 +0100)]
BUILD: ebtree: improve architecture-specific alignment
Commit
2c315ee75e ("BUG/MEDIUM: ebtree: don't set attribute packed
without unaligned access support") addressed alignment issues in
ebtrees in a way that is not really optimal since it will leave holes
in eb32trees for example.
This fix is better in that it restores the packed attribute on ebnode
but enforces proper alignment on the carrying nodes where necessary.
This also has the benefit of closing holes wherever possible and to
align data to the minimally required size.
The only thing it cannot close is the 32-bit hole at the end of ebmbnode
due to the required 64-bit on certain archs but at least it guarantees
that the key correctly points to the end of the node and that there is
never a hole after it.
This is a better fix than the one above and should be backported to
branches where the one above will be backported.
(cherry picked from commit
41136de58ef2a8f8fc1fa0f202a161eb7fe2c92d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sat, 22 Feb 2020 14:51:39 +0000 (15:51 +0100)]
MINOR: compiler: add new alignment macros
This commit adds ALWAYS_ALIGN(), MAYBE_ALIGN() and ATOMIC_ALIGN() to
be placed as delimitors inside structures to force alignment to a
given size. These depend on the architecture's capabilities so that
it is possible to always align, align only on archs not supporting
unaligned accesses at all, or only on those not supporting them for
atomic accesses (e.g. before a lock).
(cherry picked from commit
226ef26056d9e6151c9838e70a3e2f4db5068cd8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 19 Feb 2020 14:10:00 +0000 (15:10 +0100)]
BUG/MINOR: connection: make sure to correctly tag local PROXY connections
As reported in issue #511, when sending an outgoing local connection
(e.g. health check) we must set the "local" tag and not a "proxy" tag.
The issue comes from historic support on v1 which required to steal the
address on the outgoing connection for such ones, creating confusion in
the v2 code which believes it sees the incoming connection.
In order not to risk to break existing setups which might rely on seeing
the LB's address in the connection's source field, let's just change the
connection type from proxy to local and keep the addresses. The protocol
spec states that for local, the addresses must be ignored anyway.
This problem has always existed, this can be backported as far as 1.5,
though it's probably not a good idea to change such setups, thus maybe
2.0 would be more reasonable.
(cherry picked from commit
7f26391bc51ad56c31480d03f56e1db604f1c617)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 25 Feb 2020 07:59:23 +0000 (08:59 +0100)]
BUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions
Sample fetch functions ssl_x_sha1(), ssl_fc_npn(), ssl_fc_alpn(),
ssl_fc_session_id(), as well as the CLI's "show cert details" handler
used to dereference the output buffer's <data> field by casting it to
"unsigned *". But while doing this could work before 1.9, it broke
starting with commit
843b7cbe9d ("MEDIUM: chunks: make the chunk struct's
fields match the buffer struct") which merged chunks and buffers, causing
the <data> field to become a size_t. The impact is only on 64-bit platform
and depends on the endianness: on little endian, there should never be any
non-zero bits in the field as it is supposed to have been zeroed before the
call, so it shouldbe harmless; on big endian, the high part of the value
only is written instead of the lower one, often making the result appear
4 billion times larger, and making such values dropped everywhere due to
being larger than a buffer.
It seems that it would be wise to try to re-enable strict-aliasing to
catch such errors.
This must be backported till 1.9.
(cherry picked from commit
105599c1bab84f67e013a266fd846799c7075b69)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 25 Feb 2020 07:37:37 +0000 (08:37 +0100)]
BUG/MINOR: sample: fix the json converter's endian-sensitivity
About every time there's a pointer cast in the code, there's a hidden
bug, and this one was no exception, as it passes the first octet of the
native representation of an integer as a single-character string, which
obviously only works on little endian machines. On big-endian machines,
something as simple as "str(foo),json" only returns zeroes.
This bug was introduced with the JSON converter in 1.6-dev1 by commit
317e1c4f1e ("MINOR: sample: add "json" converter"), the fix may be
backported to all stable branches.
(cherry picked from commit
5715da269d6ec1e178b04d1c7aa25982e10873d7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Tim Duesterhus [Sat, 22 Feb 2020 15:39:05 +0000 (16:39 +0100)]
CLEANUP: cfgparse: Fix type of second calloc() parameter
`curr_idle_thr` is of type `unsigned int`, not `int`. Fix this issue by
taking the size of the dereferenced `curr_idle_thr` array.
This issue was introduced when adding the `curr_idle_thr` struct member
in commit
f131481a0af79037bc6616edf450ae81d80084d7. This commit is first
tagged in 2.0-dev1 and marked for backport to 1.9.
(cherry picked from commit
017484c80f2fd265281853fdf0bc816b19a751da)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2020 16:40:25 +0000 (17:40 +0100)]
BUILD: fix recent build failure on unaligned archs
Last commit 2c315ee ("BUG/MEDIUM: ebtree: don't set attribute packed
without unaligned access support") accidently enclosed the semicolon
in the ifdef so it will fail when HA_UNALIGNED is not set.
(cherry picked from commit
d43183d05f26e29fb0e75cf2b5f8a4ef47f8891d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2020 14:47:36 +0000 (15:47 +0100)]
BUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support
An alignment issue on Sparc64 with ebtrees was reported in early 2017
here https://www.mail-archive.com/haproxy@formilux.org/msg25937.html and
a similar one was finally reported in issue #512.
The problem has its roots in the fact that 64-bit keys will end up being
unaligned on such archs which do not support unaligned accesses. But on
most platforms supporting unaligned accesses, dealing with smaller nodes
results in better performance.
One of the possible problems caused by attribute packed there is that it
promotes a structure both to be unaligned and unpadded, which may come
with fun if some fields of the struct itself are accessed and such a node
is placed at an unaligned location. It's not a problem for regular unaligned
accesses but may become one for atomic operations such as the CAS on leaf_p
that's used in struct task. In practice we know that this struct is properly
aligned and is a very edge case so this patch adds comments there to remind
to be careful about it.
This patch depends on previous patch "MINOR: compiler: move CPU capabilities
definition from config.h and complete them" and could be backported to all
stable branches. It fixes issues #512 and #9.
(cherry picked from commit
2c315ee75e1e6a0a58c30637d4c8499c45528c18)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2020 14:40:58 +0000 (15:40 +0100)]
MINOR: compiler: move CPU capabilities definition from config.h and complete them
These ones are irrelevant to the config but rather to the platform, and
as such are better placed in compiler.h.
Here we take the opportunity for declaring a few extra capabilities:
- HA_UNALIGNED : CPU supports unaligned accesses
- HA_UNALIGNED_LE : CPU supports unaligned accesses in little endian
- HA_UNALIGNED_FAST : CPU supports fast unaligned accesses
- HA_UNALIGNED_ATOMIC : CPU supports unaligned accesses in atomics
This will help remove a number of #ifdefs with arch-specific statements.
(cherry picked from commit
0e2686762fa6b6fe51690cdca4273b799c6f2193)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 21 Feb 2020 12:45:58 +0000 (13:45 +0100)]
BUG/MEDIUM: shctx: make sure to keep all blocks aligned
The blocksize and the extra field are not necessarily aligned on a
machine word. This can result in crashing an align-sensitive machine
when initializing the shctx area. Let's round both sizes up to a pointer
size to make this safe everywhere.
This fixes issue #512. This should be backported as far as 1.8.
(cherry picked from commit
a7ddab0c250a0618e14a0c86aa66bbcf2052b2dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Jerome Magnin [Fri, 21 Feb 2020 09:37:48 +0000 (10:37 +0100)]
BUG/MINOR: http: http-request replace-path duplicates the query string
In http_action_replace_uri() we call http_get_path() in the case of
a replace-path rule. http_get_path() will return an ist pointing to
the start of the path, but uri.ptr + uri.len points to the end of the
uri. As as result, we are matching against a string containing the
query, which we append to the "path" later, effectively duplicating
the query string.
This patch uses the iststop() function introduced in "MINOR: ist: add
an iststop() function" to find the '?' character and update the ist
length when needed.
This fixes issue #510.
The bug was introduced by commit
262c3f1a ("MINOR: http: add a new
"replace-path" action"), which was backported to 2.1 and 2.0.
(cherry picked from commit
4bbc9494b75b6f987c813546c1fa79a5e883a188)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Jerome Magnin [Fri, 21 Feb 2020 09:33:12 +0000 (10:33 +0100)]
MINOR: ist: add an iststop() function
Add a function that finds a character in an ist and returns an
updated ist with the length of the portion of the original string
that doesn't contain the char.
Might be backported to 2.1
(cherry picked from commit
9dde0b2d31dd0e588a6fcd34ca8b6dd6cee73e3d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 21 Feb 2020 09:20:46 +0000 (10:20 +0100)]
BUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered
If an client error is reported on the request channel (CF_READ_ERROR) while a
session is tarpitted, no error is returned to the client. Concretly,
http_reply_and_close() function is not called. This function is reponsible to
forward the error to the client. But not only. It is also responsible to abort
the request. Because this function is not called when a read error is reported
on the request channel, and because the tarpit analyzer is the last one, there
is nothing preventing a connection attempt on a server while it is totally
unexpected.
So, a useless connexion on a backend server may be performed because of this
bug. If an HTTP load-balancing algorithm is used on the backend side, it leads
to a crash of HAProxy because the request was already erased.
If you have tarpit rules and if you use an HTTP load-balancing algorithm on your
backends, you must apply this patch. Otherwise a simple TCP reset on a tarpitted
connexion will most likely crash your HAProxy. A safe workaround is to use a
silent-drop rule or a deny rule instead of a tarpit.
This bug also affect the legacy code. It is in fact an very old hidden bug. But
the refactoring of process_stream() in the 1.9 makes it visible. And,
unfortunately, with the HTX, it is easier to hit it because many processing has
been moved in lower layers, in the muxes.
It must be backported as far as 1.9. For the 2.0 and the 1.9, the legacy HTTP
code must also be patched the same way. For older versions, it may be backported
but the bug seems to not impact them.
Thanks to Olivier D <webmaster@ajeux.com> to have reported the bug and provided
all the infos to analyze it.
(cherry picked from commit
9d9d645409e65069c5267422ac9d8d25ca96258d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 18 Feb 2020 14:34:58 +0000 (15:34 +0100)]
MINOR: http-ana: Match on the path if the monitor-uri starts by a /
if the monitor-uri starts by a slash ('/'), the matching is performed against
the request's path instead of the request's uri. It is a workaround to let the
HTTP/2 requests match the monitor-uri. Indeed, in HTTP/2, clients are encouraged
to send absolute URIs only.
This patch is not tagged as a bug, because the previous behavior matched exactly
what the doc describes. But it may surprise that HTTP/2 requests don't match the
monitor-uri.
This patch may be backported to 2.1 because URIs of HTTP/2 are stored using the
absolute-form starting this version. For previous versions, this patch will only
helps explicitely absolute HTTP/1 requests (and only the HTX part because on the
legacy HTTP, all the URI is matched).
It should fix the issue #509.
(cherry picked from commit
6072beb214d04ee2344872e100f4fdc480dd2496)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 18 Feb 2020 13:51:51 +0000 (14:51 +0100)]
BUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive
The monitor-uri should be case-sensitive. In reality, the scheme and the host
part are case-insensitives and only the path is case-sensive. But concretely,
since the start, the matching on the monitor-uri is case-sensitive. And it is
probably the expected behavior of almost all users.
This patch must be backported as far as 1.9. For HAProxy 2.0 and 1.9, it must be
applied on src/proto_htx.c.
(cherry picked from commit
d27689e952b09e1222c04b4cb238dbb5d797a5f2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 18 Feb 2020 10:02:21 +0000 (11:02 +0100)]
BUG/MINOR: http-htx: Don't return error if authority is updated without changes
When an Host header is updated, the autority part, if any, is also updated to
keep the both syncrhonized. But, when the update is performed while there is no
change, a failure is reported while, in reality, no update is necessary. This
bug was introduced by the commit
d7b7a1ce5 ("MEDIUM: http-htx: Keep the Host
header and the request start-line synchronized").
This commit was pushed in the 2.1. But on this version, the bug is hidden
because rewrite errors are silently ignored. And because it happens when there
is no change, if the rewrite fails, noone notices it. But since the 2.2, rewrite
errors are now fatals by default. So when the bug is hit, a 500 error is
returned to the client. Without this fix, a workaround is to disable the strict
rewriting mode (see the "strict-mode" HTTP rule).
The following HTTP rule is a good way to reproduce the bug if a request with an
authority is received. In HTT2, it is pretty common.
acl host_header_exists req.hdr(host) -m found
http-request set-header host %[req.hdr(host)] if host_header_exists
This patch must be backported to 2.1 and everywhere the commit
d7b7a1ce5 is
backported. It should fix the issue #494.
(cherry picked from commit
34b18e439129ca9df17ff46e60d009bb308d064a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 12 Feb 2020 14:31:20 +0000 (15:31 +0100)]
BUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them
In flt_analyze_http_headers() HTTP analyzer, we must not forward systematically
the headers. We must only count them as filtered data (ie. increment the offset
of the right size). It is the http_payload callback responsibility to decide to
forward headers or not by forwarding at least 1 byte of payload. And there is
always at least 1 byte of payload to forward, the EOM block.
This patch depends on following commits:
* MINOR: filters: Forward data only if the last filter forwards something
* MINOR: http-htx: Add a function to retrieve the headers size of an HTX message
This patch must be backported with commits above as far as 1.9. In HAProxy 2.0
and 1.9, the patch must be adapted because of the legacy HTTP code.
(cherry picked from commit
9c44e4813cbd3c0f7ed6e2840342d45dea9ed4ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 7 Feb 2020 15:40:33 +0000 (16:40 +0100)]
MINOR: filters: Forward data only if the last filter forwards something
In flt_tcp_payload() and flt_http_payload(), if the last filter does not
forwarding anything, nothing is forwarded, not even the already filtered
data. For now, this patch is useless because the last filter is always sync with
the stream's offset. But it will be mandatory for a bugfix.
(cherry picked from commit
71179a3ea97ea46b28788dfab557c675ec3f3a1e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 7 Feb 2020 15:39:41 +0000 (16:39 +0100)]
MINOR: http-htx: Add a function to retrieve the headers size of an HTX message
http_get_hdrs_size() function may now be used to get the bytes held by headers
in an HTX message. It only works if the headers were not already
forwarded. Metadata are not counted here.
(cherry picked from commit
727a3f1ca3b32248d84499fe6bc554e3acaf5af7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Sat, 15 Feb 2020 14:24:28 +0000 (15:24 +0100)]
SCRIPTS: announce-release: use mutt -H instead of -i to include the draft
Commit
0f5ce6014a ("SCRIPTS: announce-release: place the send command
in the mail's header") broke the announce-release script: by not having
to edit the message at all anymore, mutt does nothing when sending, but
it still does if the message is edited (which was the case before). With
some testing, it appears that mutt -H does work when there's no change,
so let's use this instead. This should be backported till 1.7.
(cherry picked from commit
1392d029e6ddd3c800f3f56c654e49800b4f306b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 14 Feb 2020 15:55:52 +0000 (16:55 +0100)]
MINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex
Now, only one capture is mandatory in the path-info regex, the one matching the
script-name. The path-info capture is optional. Of couse, it must be defined to
fill the PATH_INFO parameter. But it is not mandatory. This way, it is possible
to get the script-name part from the path, excluding the path-info.
This patch is small enough to be backported to 2.1.
(cherry picked from commit
6c57f2da4326768c08603480f0ee2fa0a7f73231)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 14 Feb 2020 13:47:37 +0000 (14:47 +0100)]
BUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param
If a regex to match the PATH_INFO parameter is configured, it systematically
fails if a newline or a null character is present in the URL-decoded path. So,
from the moment there is at least a "%0a" or a "%00" in the request path, we
always fail to get the PATH_INFO parameter and all the decoded path is used for
the SCRIPT_NAME parameter.
It is probably not the expected behavior. Because, most of time, these
characters are not expected at all in a path, an error is now triggered when one
of these characters is found in the URL-decoded path before trying to execute
the path_info regex. However, this test is not performed if there is no regex
configured.
Note that in reality, the newline character is only a problem when HAProxy is
complied with pcre or pcre2 library and conversely, the null character is only a
problem for the libc's regex library. But both are always excluded to avoid any
inconsistency depending on compile options.
An alternative, not implemented yet, is to replace these characters by another
one. If someone complains about this behavior, it will be re-evaluated.
This patch must be backported to all versions supporting the FastCGI
applications, so to 2.1 for now.
(cherry picked from commit
28cb36613b29875daf164e9e267a870122aaac76)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Olivier Houchard [Fri, 14 Feb 2020 12:23:45 +0000 (13:23 +0100)]
BUG/MEDIUM: muxes: Use the right argument when calling the destroy method.
When calling the mux "destroy" method, the argument should be the mux
context, not the connection. In a few instances in the mux code, the
connection was used (mainly when the session wouldn't handle the idle
connection, and the server pool was fool), and that could lead to random
segfaults.
This should be backported to 2.1, 2.0, and 1.9
(cherry picked from commit
12ffab03b6b911f4a60871b098656a29253e0e9b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Dauchy [Wed, 12 Feb 2020 20:23:20 +0000 (21:23 +0100)]
BUG/MINOR: namespace: avoid closing fd when socket failed in my_socketat
we cannot return right after socket opening as we need to move back to
the default namespace first
this should fix github issue #500
this might be backported to all version >= 1.6
Fixes:
b3e54fe387c7c1 ("MAJOR: namespace: add Linux network namespace
support")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit
f7dcdc8a6f63b172360019beb6e1e22e7c98f17d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 12 Feb 2020 17:21:11 +0000 (18:21 +0100)]
SCRIPTS: make announce-release executable again
I managed to mess up with the file's permission while using a temporary
one during last release, and to backport the non-exec version everywhere.
This can be backported as far as 1.7 now.
(cherry picked from commit
332ded5f3a74997c0ec2c0df8f19d643bb92b024)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Feb 2020 15:18:52 +0000 (16:18 +0100)]
[RELEASE] Released version 2.1.3
Released version 2.1.3 with the following main changes :
- BUG/MINOR: checks: refine which errno values are really errors.
- BUG/MEDIUM: checks: Only attempt to do handshakes if the connection is ready.
- BUG/MEDIUM: connections: Hold the lock when wanting to kill a connection.
- MINOR: config: disable busy polling on old processes
- MINOR: ssl: Remove unused variable "need_out".
- BUG/MINOR: h1: Report the right error position when a header value is invalid
- BUG/MINOR: proxy: Fix input data copy when an error is captured
- BUG/MEDIUM: http-ana: Truncate the response when a redirect rule is applied
- BUG/MINOR: channel: inject output data at the end of output
- BUG/MEDIUM: session: do not report a failure when rejecting a session
- BUG/MINOR: stream-int: Don't trigger L7 retry if max retries is already reached
- BUG/MEDIUM: tasks: Use the MT macros in tasklet_free().
- BUG/MINOR: mux-h2: use a safe list_for_each_entry in h2_send()
- BUG/MEDIUM: mux-h2: fix missing test on sending_list in previous patch
- BUG/MEDIUM: mux-h2: don't stop sending when crossing a buffer boundary
- BUG/MINOR: cli/mworker: can't start haproxy with 2 programs
- REGTEST: mcli/mcli_start_progs: start 2 programs
- BUG/MEDIUM: mworker: remain in mworker mode during reload
- BUG/MEDIUM: mux_h1: Don't call h1_send if we subscribed().
- BUG/MAJOR: hashes: fix the signedness of the hash inputs
- REGTEST: add sample_fetches/hashes.vtc to validate hashes
- BUG/MEDIUM: cli: _getsocks must send the peers sockets
- BUG/MINOR: stream: don't mistake match rules for store-request rules
- BUG/MEDIUM: connection: add a mux flag to indicate splice usability
- BUG/MINOR: pattern: handle errors from fgets when trying to load patterns
- BUG/MINOR: cache: Fix leak of cache name in error path
- BUG/MINOR: dns: Make dns_query_id_seed unsigned
- BUG/MINOR: 51d: Fix bug when HTX is enabled
- BUILD: pattern: include errno.h
- BUG/MINOR: http-ana/filters: Wait end of the http_end callback for all filters
- BUG/MINOR: http-rules: Remove buggy deinit functions for HTTP rules
- BUG/MINOR: stick-table: Use MAX_SESS_STKCTR as the max track ID during parsing
- BUG/MINOR: tcp-rules: Fix memory releases on error path during action parsing
- BUG/MINOR: ssl: ssl_sock_load_ocsp_response_from_file memory leak
- BUG/MINOR: ssl: ssl_sock_load_issuer_file_into_ckch memory leak
- BUG/MINOR: ssl: ssl_sock_load_sctl_from_file memory leak
- MINOR: proxy/http-ana: Add support of extra attributes for the cookie directive
- BUG/MINOR: http_act: don't check capture id in backend
- BUG/MEDIUM: netscaler: Don't forget to allocate storage for conn->src/dst.
- BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent
- BUG/MINOR: ssl/cli: free the previous ckch content once a PEM is loaded
- CLEANUP: stats: shut up a wrong null-deref warning from gcc 9.2
- BUG/MINOR: ssl: increment issuer refcount if in chain
- BUG/MINOR: ssl: memory leak w/ the ocsp_issuer
- BUG/MINOR: ssl: typo in previous patch
- BUG/MINOR: ssl/cli: ocsp_issuer must be set w/ "set ssl cert"
- BUG/MEDIUM: 0rtt: Only consider the SSL handshake.
- BUG/MINOR: stktable: report the current proxy name in error messages
- BUG/MEDIUM: mux-h2: make sure we don't emit TE headers with anything but "trailers"
- BUILD: cfgparse: silence a bogus gcc warning on 32-bit machines
- MINOR: lua: Add hlua_prepend_path function
- MINOR: lua: Add lua-prepend-path configuration option
- MINOR: lua: Add HLUA_PREPEND_C?PATH build option
- BUG/MEDIUM: ssl: Don't forget to free ctx->ssl on failure.
- BUG/MINOR: tcpchecks: fix the connect() flags regarding delayed ack
- BUG/MEDIUM: pipe: fix a use-after-free in case of pipe creation error
- BUG/MINOR: ssl: Possible memleak when allowing the 0RTT data buffer.
- BUG/MINOR: connection: fix ip6 dst_port copy in make_proxy_line_v2
- BUG/MEDIUM: connections: Don't forget to unlock when killing a connection.
- BUG/MEDIUM: memory_pool: Update the seq number in pool_flush().
- MINOR: memory: Only init the pool spinlock once.
- BUG/MEDIUM: memory: Add a rwlock before freeing memory.
- BUG/MAJOR: memory: Don't forget to unlock the rwlock if the pool is empty.
- BUG/MINOR: ssl: we may only ignore the first 64 errors
- BUG/MINOR: ssl: clear the SSL errors on DH loading failure
- CONTRIB: debug: add missing flags SF_HTX and SF_MUX
- CONTRIB: debug: add the possibility to decode the value as certain types only
- CONTRIB: debug: support reporting multiple values at once
- MINOR: acl: Warn when an ACL is named 'or'
- CONTRIB: debug: also support reading values from stdin
- SCRIPTS: announce-release: place the send command in the mail's header
- SCRIPTS: announce-release: allow the user to force to overwrite old files
- BUG/MEDIUM: ssl/cli: 'commit ssl cert' wrong SSL_CTX init
- DOC: schematic of the SSL certificates architecture
- BUG/MINOR: unix: better catch situations where the unix socket path length is close to the limit
- BUG/MINOR: dns: allow 63 char in hostname
- BUG/MEDIUM: listener: only consider running threads when resuming listeners
- BUG/MINOR: listener: enforce all_threads_mask on bind_thread on init
- BUG/MINOR: tcp: avoid closing fd when socket failed in tcp_bind_listener
- MINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs
- DOC: word converter ignores delimiters at the start or end of input string
- MINOR: htx: Add a function to append an HTX message to another one
- MINOR: htx/channel: Add a function to copy an HTX message in a channel's buffer
- BUG/MINOR: http-ana: Don't overwrite outgoing data when an error is reported
- BUG/MINOR: http-ana: Set HTX_FL_PROXY_RESP flag if a server perform a redirect
- BUG/MINOR: tcp: don't try to set defaultmss when value is negative
William Dauchy [Wed, 12 Feb 2020 14:53:04 +0000 (15:53 +0100)]
BUG/MINOR: tcp: don't try to set defaultmss when value is negative
when `getsockopt` previously failed, we were trying to set defaultmss
with -2 value.
this is a followup of github issue #499
this should be backported to all versions >= v1.8
Fixes:
153659f1ae69a1 ("MINOR: tcp: When binding socket, attempt to
reuse one from the old proc.")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit
97a7bdac3e9e64e9f128943ee3d7b48de88a7daf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 24 Jan 2020 18:16:26 +0000 (19:16 +0100)]
BUG/MINOR: http-ana: Set HTX_FL_PROXY_RESP flag if a server perform a redirect
It is important to not forget to specify the HTX resposne was internally
generated when a server perform a redirect. This information is used by the H1
multiplexer to choose the right connexion mode when the response is sent to the
client.
This patch must be backported to 2.1.
(cherry picked from commit
c20afb810f9368b026f2fea9640d04a004087f19)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 23 Jan 2020 10:57:31 +0000 (11:57 +0100)]
BUG/MINOR: http-ana: Don't overwrite outgoing data when an error is reported
When an error is returned to a client, the right message is injected into the
response buffer. It is performed by http_server_error() or
http_replay_and_close(). Both ignore any data already present into the channel's
buffer. While it is legitimate to remove all input data, it is important to not
remove any outgoing data.
So now, we try to append the error message to the response buffer, only removing
input data. We rely on the channel_htx_copy_msg() function to do so. So this
patch depends on the following two commits:
* MINOR: htx: Add a function to append an HTX message to another one
* MINOR: htx/channel: Add a function to copy an HTX message in a channel's buffer
This patch must be backported as far as 1.9. However, above patches must be
backported first.
(cherry picked from commit
637259e044284882d343e757b1be25a975dcbbf2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 23 Jan 2020 10:53:18 +0000 (11:53 +0100)]
MINOR: htx/channel: Add a function to copy an HTX message in a channel's buffer
The channel_htx_copy_msg() function can now be used to copy an HTX message in a
channel's buffer. This function takes care to not overwrite existing data.
This patch depends on the commit "MINOR: htx: Add a function to append an HTX
message to another one". Both are mandatory to fix a bug in
http_reply_and_close() function. Be careful to backport both first.
(cherry picked from commit
7651362e52af556e900e461c0c5c0d87a8c3c51a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 23 Jan 2020 10:47:53 +0000 (11:47 +0100)]
MINOR: htx: Add a function to append an HTX message to another one
the htx_append_msg() function can now be used to append an HTX message to
another one. All the message is copied or nothing. If an error occurs during the
copy, all changes are rolled back.
This patch is mandatory to fix a bug in http_reply_and_close() function. Be
careful to backport it first.
(cherry picked from commit
0ea0c86753ea55123da51cfce3276792e124f2e4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Jerome Magnin [Tue, 28 Jan 2020 12:33:44 +0000 (13:33 +0100)]
DOC: word converter ignores delimiters at the start or end of input string
The comments for match_word() in pattern.c mention that delimiters
at the start or end of the input string will be ignored, but this
is not mentionned in the documentation.
Backport to all supported versions.
(cherry picked from commit
882093249a5e2261040f1107a74b6d2581a25932)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christian Lachner [Mon, 10 Feb 2020 09:29:13 +0000 (10:29 +0100)]
MINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs
As haproxy wont build on AIX 7.2 using the old "aix52" TARGET a new
TARGET was introduced which adds two special CFLAGS to prevent the
loading of AIXs xmem.h and var.h. This is done by defining the
corresponding include-guards _H_XMEM and _H_VAR. Without excluding
those headers-files the build fails because of redefinition errors:
1)
CC src/mux_fcgi.o
In file included from /usr/include/sys/uio.h:90,
from /opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/include-fixed/sys/socket.h:104,
from include/common/compat.h:32,
from include/common/cfgparse.h:25,
from src/mux_fcgi.c:13:
src/mux_fcgi.c:204:13: error: expected ':', ',', ';', '}' or '__attribute__' before '.' token
struct ist rem_addr;
^~~~~~~~
2)
CC src/cfgparse-listen.o
In file included from include/types/arg.h:31,
from include/types/acl.h:29,
from include/types/proxy.h:41,
from include/proto/log.h:34,
from include/common/cfgparse.h:30,
from src/mux_h2.c:13:
include/types/vars.h:30:8: error: redefinition of 'struct var'
struct var {
^~~
Futhermore, to enable multithreading via USE_THREAD, the atomic
library was added to the LDFLAGS. Finally, two new CPUs were added
to simplify the usage of power8 and power9 optimizations.
This TARGET was only tested on GCC 8.3 and may or may not work on
IBM's native C-compiler (XLC).
Should be backported to 2.1.
(cherry picked from commit
c13223022c125bb43c3a53a26a594dac8a3e90f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Wed, 12 Feb 2020 09:09:14 +0000 (10:09 +0100)]
BUG/MINOR: tcp: avoid closing fd when socket failed in tcp_bind_listener
we were trying to close file descriptor even when `socket` call was
failing.
this should fix github issue #499
this should be backported to all versions >= v1.8
Fixes:
153659f1ae69a1 ("MINOR: tcp: When binding socket, attempt to
reuse one from the old proc.")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit
c0e23aef051dd3916ef3eedf934837942cb7db71)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Feb 2020 09:15:34 +0000 (10:15 +0100)]
BUG/MINOR: listener: enforce all_threads_mask on bind_thread on init
When intializing a listener, let's make sure the bind_thread mask is
always limited to all_threads_mask when inserting the FD. This will
avoid seeing listening FDs with bits corresponding to threads that are
not active (e.g. when using "bind ... process 1/even"). The side effect
is very limited, all that was identified is that atomic operations are
used in fd_update_events() when not necessary. It's more a matter of
long-term correctness in practice.
This fix might be backported as far as 1.8 (then proto_sockpair must
be dropped).
(cherry picked from commit
0948a781fce19a032b71e65239d6bc5d10c97522)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 12 Feb 2020 09:01:29 +0000 (10:01 +0100)]
BUG/MEDIUM: listener: only consider running threads when resuming listeners
In bug #495 we found that it is possible to resume a listener on an
inexistent thread. This happens when a bind's thread_mask contains bits
out of the active threads mask, such as when using "1/odd" or "1/even".
The thread_mask was used as-is to pick a thread number to re-enable the
listener, and given that the highest number is used, 1/odd or 1/even can
produce quite high thread numbers and crash the process by queuing some
entries into non-existent lists.
This bug is an incomplete fix of commit
413e926ba ("BUG/MAJOR: listener:
fix thread safety in resume_listener()") though it will only trigger if
some bind lines are explicitly bound to thread numbers higher than the
thread count. The fix must be backported to all branches having the fix
above (as far as 1.8, though the code is different there, see the commit
message in 1.8 for changes).
There are a few other places where bind_thread is used without
enforcing all_thread_mask, namely when doing fd_insert() while creating
listeners. It seems harmless but would probably deserve another fix.
(cherry picked from commit
50b659476ceb8e5ee3a060df1da0d32d336a58a8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Sun, 26 Jan 2020 18:52:34 +0000 (19:52 +0100)]
BUG/MINOR: dns: allow 63 char in hostname
hostname were limited to 62 char, which is not RFC1035 compliant;
- the parsing loop should stop when above max label char
- fix len label test where d[i] was wrongly used
- simplify the whole function to avoid using two extra char* variable
this should fix github issue #387
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
Acked-by: Baptiste <bedis9@gmail.com>
(cherry picked from commit
aecd5dcac2cd4cd87639596980c449f887b2ca26)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 11 Feb 2020 05:43:37 +0000 (06:43 +0100)]
BUG/MINOR: unix: better catch situations where the unix socket path length is close to the limit
We do have some checks for the UNIX socket path length to validate the
full pathname of a unix socket but the pathname extension is only taken
into account when using a bind_prefix. The second check only matches
against MAXPATHLEN. So this means that path names between 98 and 108
might successfully parse but fail to bind. Let's adjust the check in
the address parser and refine the error checking at the bind() step.
This addresses bug #493.
(cherry picked from commit
327ea5aec83092404bca09df2fb9aa86118c8a73)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Mon, 10 Feb 2020 10:43:43 +0000 (11:43 +0100)]
DOC: schematic of the SSL certificates architecture
This patch provides a schematic of the new architecture based on the
struct cert_key_and_chain which appeared with haproxy 2.1.
Could be backported in 2.1
(cherry picked from commit
90de53dc79a316a79567972cede3673c10fba13d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Fri, 7 Feb 2020 19:45:24 +0000 (20:45 +0100)]
BUG/MEDIUM: ssl/cli: 'commit ssl cert' wrong SSL_CTX init
The code which is supposed to apply the bind_conf configuration on the
SSL_CTX was not called correctly. Indeed it was called with the previous
SSL_CTX so the new ones were left with default settings. For example the
ciphers were not changed.
This patch fixes #429.
Must be backported in 2.1.
(cherry picked from commit
696f317f13151e4427e3f9a8b560730ed6a7bb40)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Fri, 7 Feb 2020 07:11:45 +0000 (08:11 +0100)]
SCRIPTS: announce-release: allow the user to force to overwrite old files
When starting the script multiple times, one had to remove the previous
files by hand. Now with -f it's not needed anymore, they get removed.
(cherry picked from commit
3823408b6063c8955d22532903f96b40601f57ed)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 7 Feb 2020 07:10:06 +0000 (08:10 +0100)]
SCRIPTS: announce-release: place the send command in the mail's header
I'm fed up with having to scroll my terminals trying to look for the
mail send command printed 30 minutes before the release, let's have
it copied into the e-mail template itself, and replace the old headers
that used to be duplicated there and that are not needed anymore.
(cherry picked from commit
0f5ce6014acc7e6cbdcde110f950d367bb19b75b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 6 Feb 2020 17:17:50 +0000 (18:17 +0100)]
CONTRIB: debug: also support reading values from stdin
This is convenient when processing large dumps, it allows to copy-paste
values to inspect from one window to another, or to directly transfer
a "show fd"/"show stream" output through sed. In order to do this, simply
pass "-" alone instead of the value and they will all be read one line at
a time from stdin. For example, in order to quickly print the different
set of connection flags from "show fd", this is sufficient:
sed -ne 's/^.* cflg=\([^ ]*\).*/\1/p' | contrib/debug/flags conn -
(cherry picked from commit
e4f80a076c2d52a8b73f2f7350e047f6e39acd69)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Wed, 5 Feb 2020 20:00:50 +0000 (21:00 +0100)]
MINOR: acl: Warn when an ACL is named 'or'
Consider a configuration like this:
> acl t always_true
> acl or always_false
>
> http-response set-header Foo Bar if t or t
The 'or' within the condition will be treated as a logical disjunction
and the header will be set, despite the ACL 'or' being falsy.
This patch makes it an error to declare such an ACL that will never
work. This patch may be backported to stable releases, turning the
error into a warning only (the code was written in a way to make this
trivial). It should not break anything and might improve the users'
lifes.
(cherry picked from commit
0cf811a5f941261176b67046dbc542d0479ff4a7)
[wt: turned the error into a warning only]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 6 Feb 2020 07:48:16 +0000 (08:48 +0100)]
CONTRIB: debug: support reporting multiple values at once
It's often convenient, for example to dump two channels or two stream-int
at once. Now all input values are decoded and the value is recalled before
the dump when there is more than one to display.
(cherry picked from commit
bde76f0de60c77330f49284b206cbca40832f0bf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 6 Feb 2020 07:33:08 +0000 (08:33 +0100)]
CONTRIB: debug: add the possibility to decode the value as certain types only
It's often confusing to have a whole dump on the screen while only
checking for a set of task or stream flags, and appending "|grep ^chn"
isn't very convenient to repeat the opeation. Instead let's add the
ability to filter the output as certain types only by prepending their
name(s) before the value.
(cherry picked from commit
354b6f5e288f466aed48d46dd8467b023a0b3660)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 6 Feb 2020 06:57:36 +0000 (07:57 +0100)]
CONTRIB: debug: add missing flags SF_HTX and SF_MUX
These two were forgotten when HTX was added. They can be backported
as they're missing for debugging traces in 2.0.
(cherry picked from commit
8a0eabd536ca1354e2f24fcb7810042f31751010)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Wed, 5 Feb 2020 10:46:33 +0000 (11:46 +0100)]
BUG/MINOR: ssl: clear the SSL errors on DH loading failure
In ssl_sock_load_dh_params(), if haproxy failed to apply the dhparam
with SSL_CTX_set_tmp_dh(), it will apply the DH with
SSL_CTX_set_dh_auto().
The problem is that we don't clean the OpenSSL errors when leaving this
function so it could fail to load the certificate, even if it's only a
warning.
Fixes bug #483.
Must be backported in 2.1.
(cherry picked from commit
4dd145a888c7679812664bf2f246fa8199e94ab0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
Willy Tarreau [Tue, 4 Feb 2020 13:02:02 +0000 (14:02 +0100)]
BUG/MINOR: ssl: we may only ignore the first 64 errors
We have the ability per bind option to ignore certain errors (CA, crt, ...),
and for this we use a 64-bit field. In issue #479 coverity reports a risk of
too large a left shift. For now as of OpenSSL 1.1.1 the highest error value
that may be reported by X509_STORE_CTX_get_error() seems to be around 50 so
there should be no risk yet, but it's enough of a warning to add a check so
that we don't accidently hide random errors in the future.
This may be backported to relevant stable branches.
(cherry picked from commit
731248f0dbba03688e433789790f65580a472151)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Mon, 3 Feb 2020 12:03:30 +0000 (13:03 +0100)]
BUG/MAJOR: memory: Don't forget to unlock the rwlock if the pool is empty.
In __pool_get_first(), don't forget to unlock the pool lock if the pool is
empty, otherwise no writer will be able to take the lock, and as it is done
when reloading, it leads to an infinite loop on reload.
This should be backported with commit
04f5fe87d3d3a222b89420f8c1231461f55ebdeb
(cherry picked from commit
1c7c0d6b97513e79c304aaf834f83843f32a674d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Sat, 1 Feb 2020 16:49:31 +0000 (17:49 +0100)]
BUG/MEDIUM: memory: Add a rwlock before freeing memory.
When using lockless pools, add a new rwlock, flush_pool. read-lock it when
getting memory from the pool, so that concurrenct access are still
authorized, but write-lock it when we're about to free memory, in
pool_flush() and pool_gc().
The problem is, when removing an item from the pool, we unreference it
to get the next one, however, that pointer may have been free'd in the
meanwhile, and that could provoke a crash if the pointer has been unmapped.
It should be OK to use a rwlock, as normal operations will still be able
to access the pool concurrently, and calls to pool_flush() and pool_gc()
should be pretty rare.
This should be backported to 2.1, 2.0 and 1.9.
(cherry picked from commit
04f5fe87d3d3a222b89420f8c1231461f55ebdeb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Sat, 1 Feb 2020 16:45:32 +0000 (17:45 +0100)]
MINOR: memory: Only init the pool spinlock once.
In pool_create(), only initialize the pool spinlock if we just created the
pool, in the event we're reusing it, there's no need to initialize it again.
(cherry picked from commit
8af97eb4a19c1d3c0849b5e5e5d350d63f819032)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Sat, 1 Feb 2020 16:37:22 +0000 (17:37 +0100)]
BUG/MEDIUM: memory_pool: Update the seq number in pool_flush().
In pool_flush(), we can't just set the free_list to NULL, or we may suffer
the ABA problem. Instead, use a double-width CAS and update the sequence
number.
This should be backported to 2.1, 2.0 and 1.9.
This may, or may not, be related to github issue #476.
(cherry picked from commit
b6fa08bc7bca48e0098b555e3c433e0969f46d4c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Fri, 31 Jan 2020 16:22:08 +0000 (17:22 +0100)]
BUG/MEDIUM: connections: Don't forget to unlock when killing a connection.
Commit
140237471e408736bb7162e68c572c710a66a526 made sure we hold the
toremove_lock for the corresponding thread before removing a connection
from its idle_orphan_conns list, however it failed to unlock it if we
found a connection, leading to a deadlock, so add the missing deadlock.
This should be backported to 2.1 and 2.0.
(cherry picked from commit
849d4f047f768184f30b989d06502c601f355b9f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Dauchy [Sun, 26 Jan 2020 18:06:39 +0000 (19:06 +0100)]
BUG/MINOR: connection: fix ip6 dst_port copy in make_proxy_line_v2
triggered by coverity; src_port is set earlier.
this should fix github issue #467
Fixes:
7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
protocols are mixed")
This should be backported to 1.8.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
(cherry picked from commit
bd8bf67102d061f777c3e41db206e86bbae60be7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frédéric Lécaille [Fri, 24 Jan 2020 13:56:18 +0000 (14:56 +0100)]
BUG/MINOR: ssl: Possible memleak when allowing the 0RTT data buffer.
â
\80\8b
As the server early data buffer is allocated in the middle of the loop
used to allocate the SSL session without being freed before retrying,
this leads to a memory leak.
â
\80\8b
To fix this we move the section of code responsible of this early data buffer
alloction after the one reponsible of allocating the SSL session.
â
\80\8b
Must be backported to 2.1 and 2.0.
(cherry picked from commit
3139c1b198bbcc14c6940f214234afd004110387)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 4 Feb 2020 09:23:54 +0000 (10:23 +0100)]
BUG/MEDIUM: pipe: fix a use-after-free in case of pipe creation error
In get_pipe(), if pipe() fails, we free the pipe struct but return it to
the caller, resulting in a use-after-free then a double-free provoking a
crash. It's quite hard to trigger the problem as it usually involves a
tweaked configuration with heavy use of splicing and too many allocated
pipes of large size, but it does happen in benchmarks.
The bug was introduced with the very first patch adding pipes support
in 1.3.16-rc1, commit
982b6e37e ("[MEDIUM] introduce pipe pools"). As
such, this fix must be backported to all supported versions.
This patch doesn't exist in mainline because it was fixed as a side
effect of commit
876b411f2b ("BUG/MEDIUM: pipe/thread: fix atomicity of
pipe counters") that only affects mainline.
The bug manifests itself as a crash in pool_free() or __pool_get_first()
and is easily worked around by reducing tune.maxpipes, or since 2.0, by
not forcing any value and letting it configure itself based on maxconn
and ulimit-n.
Willy Tarreau [Fri, 24 Jan 2020 16:52:37 +0000 (17:52 +0100)]
BUG/MINOR: tcpchecks: fix the connect() flags regarding delayed ack
In issue #465, we see that Coverity detected dead code in checks.c
which is in fact a missing parenthesis to build the connect() flags
consecutive to the API change in commit
fdcb007ad8 ("MEDIUM: proto:
Change the prototype of the connect() method.").
The impact should be imperceptible as in the best case it may have
resulted in a missed optimization trying to save a syscall or to merge
outgoing packets.
It may be backported as far as 2.0 though it's not critical.
(cherry picked from commit
74ab7d2b80cf3930e2b3957c9234953a632c5226)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Fri, 24 Jan 2020 14:17:38 +0000 (15:17 +0100)]
BUG/MEDIUM: ssl: Don't forget to free ctx->ssl on failure.
In ssl_sock_init(), if we fail to allocate the BIO, don't forget to free
the SSL *, or we'd end up with a memory leak.
This should be backported to 2.1 and 2.0.
(cherry picked from commit
efe5e8e99890b24dcfb8c925d98bf82e2fdf0b9f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sun, 12 Jan 2020 12:55:41 +0000 (13:55 +0100)]
MINOR: lua: Add HLUA_PREPEND_C?PATH build option
This complements the lua-prepend-path configuration option to allow
distro maintainers to add a default path for HAProxy specific Lua
libraries.
(cherry picked from commit
541fe1ec52a0f9e1912dea5b3a784406dbdfad22)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sun, 12 Jan 2020 12:55:40 +0000 (13:55 +0100)]
MINOR: lua: Add lua-prepend-path configuration option
lua-prepend-path allows the administrator to specify a custom Lua library
path to load custom Lua modules that are useful within the context of HAProxy
without polluting the global Lua library folder.
(cherry picked from commit
dd74b5f2372f610cfa60e8cb2e151e2de377357e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sun, 12 Jan 2020 12:55:39 +0000 (13:55 +0100)]
MINOR: lua: Add hlua_prepend_path function
This function is added in preparation for following patches.
(cherry picked from commit
c9fc9f2836f1e56eef3eaf690421eeff34dd8a2b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Jan 2020 10:19:13 +0000 (11:19 +0100)]
BUILD: cfgparse: silence a bogus gcc warning on 32-bit machines
A first patch was made during 2.0-dev to silence a bogus warning emitted
by gcc :
dd1c8f1f72 ("MINOR: cfgparse: Add a cast to make gcc happier."),
but it happens it was not sufficient as the warning re-appeared on 32-bit
machines under gcc-8 and gcc-9 :
src/cfgparse.c: In function 'check_config_validity':
src/cfgparse.c:3642:33: warning: argument 1 range [
2147483648,
4294967295] exceeds maximum object size
2147483647 [-Walloc-size-larger-than=]
newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, sizeof(*newsrv->idle_orphan_conns));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This warning doesn't trigger in other locations, and it immediately
vanishes if the previous or subsequent loops do not depend on
global.nbthread anymore, or if the field ordering of the struct server
changes! As discussed in the thread at:
https://www.mail-archive.com/haproxy@formilux.org/msg36107.html
playing with -Walloc-size-larger-than has no effect. And a minimal
reproducer could be isolated, indicating it's pointless to circle around
this one. Let's just cast nbthread to ushort so that gcc cannot make
this wrong detection. It's unlikely we'll use more than 65535 threads in
the near future anyway.
This may be backported to older releases if they are also affected, at
least to ease the job of distro maintainers.
Thanks to Ilya for testing.
(cherry picked from commit
645c588e7138526ccb71f3c47f00045cdf1d8510)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Jan 2020 08:07:53 +0000 (09:07 +0100)]
BUG/MEDIUM: mux-h2: make sure we don't emit TE headers with anything but "trailers"
While the H2 parser properly checks for the absence of anything but
"trailers" in the TE header field, we forget to check this when sending
the request to an H2 server. The problem is that an H2->H2 conversion
may keep "gzip" and fail on the next stage.
This patch makes sure that we only send "TE: trailers" if the TE header
contains the "trailers" token, otherwise it's dropped.
This fixes issue #464 and should be backported till 1.9.
(cherry picked from commit
bb2c4ae06566b8a8789caca4c48524aeb88cbc1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 24 Jan 2020 06:19:34 +0000 (07:19 +0100)]
BUG/MINOR: stktable: report the current proxy name in error messages
Since commit
1b8e68e89a ("MEDIUM: stick-table: Stop handling stick-tables
as proxies."), a rule referencing the current proxy with no table leads
to the following error :
[ALERT] 023/071924 (16479) : Proxy 'px': unable to find stick-table '(null)'.
[ALERT] 023/071914 (16479) : Fatal errors found in configuration.
for a config like this one:
backend px
stick on src
This patch fixes it and should be backported as far as 2.0.
(cherry picked from commit
508d232a06cf082ff2cc694d3f1c03b10a07e719)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Olivier Houchard [Thu, 23 Jan 2020 13:57:36 +0000 (14:57 +0100)]
BUG/MEDIUM: 0rtt: Only consider the SSL handshake.
We only add the Early-data header, or get ssl_fc_has_early to return 1, if
we didn't already did the SSL handshake, as otherwise, we know the early
data were fine, and there's no risk of replay attack. But to do so, we
wrongly checked CO_FL_HANDSHAKE, we have to check CO_FL_SSL_WAIT_HS instead,
as we don't care about the status of any other handshake.
This should be backported to 2.1, 2.0, and 1.9.
When deciding if we should add the Early-Data header, or if the sample fetch
should return
(cherry picked from commit
220a26c31647b8cfd76f3922d08cb2e847e3009e)
Signed-off-by: Willy Tarreau <w@1wt.eu>