haproxy-2.1.git
5 years agoMINOR: ssl: improve the errors when a crt can't be open
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>

5 years agoBUG/MINOR: protocol_buffer: Wrong maximum shifting.
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>

5 years ago[RELEASE] Released version 2.1.4 v2.1.4
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

5 years agoBUG/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>

5 years agoBUG/MINOR: http-ana: Reset request analysers on error when waiting for response
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.

5 years agoMINOR: http-rules: Handle the rule direction when a redirect is evaluated
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".]

5 years agoBUG/MINOR: http_ana: make sure redirect flags don't have overlapping bits
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>

5 years agoMINOR: http-rules: Add a flag on redirect rules to know the rule direction
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>

5 years agoDOC: internals: Fix spelling errors in filters.txt
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>

5 years agoBUG/MINOR: stats: Fix color of draining servers on stats page
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>

5 years agoBUILD: ssl: only pass unsigned chars to isspace()
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>

5 years agoMINOR: listener: add so_name sample fetch
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>

5 years agoBUG/MINOR: peers: Use after free of "peers" section.
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>

5 years agoBUG/MINOR: peers: avoid an infinite loop with peers_fe is NULL
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>

5 years agoBUG/MINOR: peers: init bind_proc to 1 if it wasn't initialized
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>

5 years agoBUG/MINOR: haproxy/threads: close a possible race in soft-stop detection
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>

5 years agoREGTEST: increase timeouts on the seamless-reload test
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>

5 years agoREGTESTS: use "command -v" instead of "which"
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>

5 years agoBUG/MINOR: connections: Make sure we free the connection on failure.
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>

5 years agoMINOR: memory: Change the flush_lock to a spinlock, and don't get it in alloc.
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>

5 years agoBUG/MEDIUM: wdt: Don't ignore WDTSIG and DEBUGSIG in __signal_process_queue().
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>

5 years agoMINOR: wdt: Move the definitions of WDTSIG and DEBUGSIG into types/signal.h.
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>

5 years agoDOC: assorted typo fixes in the documentation
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>

5 years agoBUG/MEDIUM: peers: resync ended with RESYNC_PARTIAL in wrong cases.
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>

5 years agoBUILD: makefile: fix expression again to detect ARM platform
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>

5 years agoBUILD: makefile: fix regex syntax in ARM platform detection
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>

5 years agoBUILD: on ARM, must be linked to libatomic.
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>

5 years agoDOC: correct typo in alert message about rspirep
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>

5 years agoDOC: proxy_protocol: Reserve TLV type 0x05 as PP2_TYPE_UNIQUE_ID
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>

5 years agoBUG/MINOR: haproxy/threads: try to make all threads leave together
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>

5 years agoBUG/MINOR: listener/mq: do not dispatch connections to remote threads when stopping
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>

5 years agoBUG/MINOR: haproxy: always initialize sleeping_thread_mask
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>

5 years agoBUG/MEDIUM: pools: Always update free_list in pool_gc().
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>

5 years agoBUG/MEDIUM: random: align the state on 2*64 bits for ARM64
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>

5 years agoMINOR: mt_lists: Appease gcc.
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>

5 years agoBUG/MEDIUM: mt_lists: Make sure we set the deleted element to NULL;
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>

5 years agoBUILD: wdt: only test for SI_TKILL when compiled with thread support
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>

5 years agoDOC: ssl: clarify security implications of TLS tickets
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>

5 years agoDOC: assorted typo fixes in the documentation
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>

5 years agoDOC: improve description of no-tls-tickets
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>

5 years agoDOC: fix typo about no-tls-tickets
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>

5 years agoBUG/MINOR: rules: Increment be_counters if backend is assigned for a silent-drop
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>

5 years agoBUG/MINOR: rules: Preserve FLT_END analyzers on silent-drop action
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>

5 years agoBUG/MINOR: http-rules: Fix a typo in the reject action function
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>

5 years agoBUG/MINOR: http-rules: Preserve FLT_END analyzers on reject action
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>

5 years agoBUG/MINOR: lua: Ignore the reserve to know if a channel is full or not
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>

5 years agoBUG/MINOR: http-ana: Reset request analysers on a response side error
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>

5 years agoBUG/MEDIUM: compression/filters: Fix loop on HTX blocks compressing the payload
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>

5 years agoBUG/MEDIUM: cache/filters: Fix loop on HTX blocks caching the response payload
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>

5 years agoMINOR: htx: Add a function to return a block at a specific offset
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>

5 years agoBUG/MINOR: filters: Forward everything if no data filters are called
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>

5 years agoBUG/MINOR: filters: Use filter offset to decude the amount of forwarded data
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>

5 years agoREGTEST: make the PROXY TLV validation depend on version 2.2
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>

5 years agoBUG/MAJOR: proxy_protocol: Properly validate TLV lengths
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>

5 years agoBUG/MINOR: init: make the automatic maxconn consider the max of soft/hard limits
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>

5 years agoDOC: assorted typo fixes in the documentation and Makefile
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>

5 years agoDOC: configuration.txt: fix various typos
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>

5 years agoBUG/MINOR: pattern: Do not pass len = 0 to calloc()
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>

5 years agoOPTIM: startup: fast unique_id allocation for acl.
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>

5 years agoDOC: fix incorrect indentation of http_auth_*
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>

5 years agoBUG/MINOR: wdt: do not return an error when the watchdog couldn't be enabled
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>

5 years agoBUILD: tools: remove obsolete and conflicting trace() from standard.c
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>

5 years agoMINOR: haproxy: export main to ease access from debugger
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>

5 years agoBUG/MEDIUM: debug: make the debug_handler check for the thread in threads_to_dump
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>

5 years agoMINOR: debug: report the task handler's pointer relative to main
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>

5 years agoBUG/MAJOR: list: fix invalid element address calculation
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>

5 years agoBUG/MINOR: checks/threads: use ha_random() and not rand()
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>

5 years agoMINOR: backend: use a single call to ha_random32() for the random LB algo
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>

5 years agoBUG/MEDIUM: random: implement a thread-safe and process-safe PRNG
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>

5 years agoMINOR: tools: add 64-bit rotate operators
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>

5 years agoBUG/MEDIUM: random: initialize the random pool a bit better
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>

5 years agoMINOR: contrib/prometheus-exporter: Add the last heathcheck duration metric
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>

5 years agoMINOR: contrib/prometheus-exporter: Add heathcheck status/code in server metrics
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>

5 years agoBUG/MINOR: http-htx: Do case-insensive comparisons on Host header name
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>

5 years agoBUG/MINOR: dns: ignore trailing dot
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>

5 years agoBUG/MINOR: sample: Make sure to return stable IDs in the unique-id fetch
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>

5 years agoBUG/MINOR: h2: reject again empty :path pseudo-headers
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>

5 years agoBUILD: ebtree: improve architecture-specific alignment
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>

5 years agoMINOR: compiler: add new alignment macros
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>

5 years agoBUG/MINOR: connection: make sure to correctly tag local PROXY connections
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>

5 years agoBUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions
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>

5 years agoBUG/MINOR: sample: fix the json converter's endian-sensitivity
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>

5 years agoCLEANUP: cfgparse: Fix type of second calloc() parameter
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>

5 years agoBUILD: fix recent build failure on unaligned archs
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>

5 years agoBUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support
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>

5 years agoMINOR: compiler: move CPU capabilities definition from config.h and complete them
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>

5 years agoBUG/MEDIUM: shctx: make sure to keep all blocks aligned
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>

5 years agoBUG/MINOR: http: http-request replace-path duplicates the query string
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>

5 years agoMINOR: ist: add an iststop() function
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>

5 years agoBUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered
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>

5 years agoMINOR: http-ana: Match on the path if the monitor-uri starts by a /
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>

5 years agoBUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive
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>

5 years agoBUG/MINOR: http-htx: Don't return error if authority is updated without changes
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>

5 years agoBUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them
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>

5 years agoMINOR: filters: Forward data only if the last filter forwards something
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>

5 years agoMINOR: http-htx: Add a function to retrieve the headers size of an HTX message
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>

5 years agoSCRIPTS: announce-release: use mutt -H instead of -i to include the draft
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>

5 years agoMINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex
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>

5 years agoBUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param
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>

5 years agoBUG/MEDIUM: muxes: Use the right argument when calling the destroy method.
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>