Christopher Faulet [Mon, 18 Oct 2021 13:06:20 +0000 (15:06 +0200)]
BUG/MEDIUM: stream: Keep FLT_END analyzers if a stream detects a channel error
If a channel error (READ_ERRO|READ_TIMEOUT|WRITE_ERROR|WRITE_TIMEOUT) is
detected by the stream, in process_stream(), FLT_END analyers must be
preserved. It is important to be sure to ends filter analysis and be able to
release the stream.
First, filters may release some ressources when FLT_END analyzers are
called. Then, the CF_FL_ANALYZE flag is used to sync end of analysis for the
request and the response. If FLT_END analyzer is ignored on a channel, this
may block the other side and freeze the stream.
This patch must be backported to all stable versions
(cherry picked from commit
813f913444726eafbc06050ce1d1623f5d84bc38)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c94f727ea1a38656d93097bee551ab1375c8fe6d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 15 Oct 2021 11:51:34 +0000 (13:51 +0200)]
BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back
http-after-response rules evaluation must be stopped after a "allow". It
means the frontend ruleset must not be evaluated if a "allow" was performed
in the backend ruleset. Internally, the evaluation must be stopped if on
HTTP_RULE_RES_STOP return value. Only the "allow" action is concerned by
this change.
Thanks to this patch, http-response and http-after-response behave in the
same way.
This patch should be backported as far as 2.2.
(cherry picked from commit
597909f4e67866c4f3ecf77f95f2cd4556c0c638)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
085ccd4fe42706230c2a31c162d8664ca6fb16a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Olivier Houchard [Fri, 8 Oct 2021 23:53:03 +0000 (01:53 +0200)]
MINOR: initcall: Rename __GLOBL and __GLOBL1.
Rename __GLOBL and __GLOBL1 to __HA_GLOBL and __HA_GLOBL1, as the former are
already defined on FreeBSD.
This should be backported to 2.4, 2.3 and 2.2.
(cherry picked from commit
e972c0acde990de30a00f76f7f61470d3325c757)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
1e74052267a3a09a9dba8b2ba6131d4bd1d80923)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 8 Oct 2021 06:56:00 +0000 (08:56 +0200)]
BUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames
We've found others places where the read0 is ignored because of an
incomplete frame parsing. This time, it happens during parsing of
CONTINUATION frames.
When frames are parsed, incomplete frames are properly handled and
H2_CF_DEM_SHORT_READ flag is set. It is also true for HEADERS
frames. However, for CONTINUATION frames, there is an exception. Besides
parsing the current frame, we try to peek header of the next one to merge
payload of both frames, the current one and the next one. Idea is to create
a sole HEADERS frame before parsing the payload. However, in this case, it
is possible to have an incomplete frame too, not the current one but the
next one. From the demux point of view, the current frame is complete. We
must go to the internal function h2c_decode_headers() to detect an
incomplete frame. And this case was not identified and fixed when
H2_CF_DEM_SHORT_READ flag was introduced in the commit
b5f7b5296
("BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames")
This bug was reported in a comment of the issue #1362. The patch must be
backported as far as 2.0.
(cherry picked from commit
485da0b0535cc575ed8d4f3bf407e9101dbb80aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0de3f552a4e19fa28b2beb9dc90f8aa6b9e1fd62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 21 Sep 2021 13:22:12 +0000 (15:22 +0200)]
BUG/MEDIUM: stream-int: Defrag HTX message in si_cs_recv() if necessary
The stream interface is now responsible for defragmenting the HTX message of
the input channel if necessary, before calling the mux's .rcv_buf()
function. The defrag is performed if the underlying buffer contains only
input data while the HTX message free space is not contiguous.
The defrag is important here to be sure the mux and the app layer have the
same criteria to decide if a buffer is full or not. Otherwise, the app layer
may wait for more data because the buffer is not full while the mux is
blocked because it needs more space to proceed.
This patch depends on following commits:
* MINOR: htx: Add an HTX flag to know when a message is fragmented
* MINOR: htx: Add a function to know if the free space wraps
This patch is related to the issue #1362. It may be backported as far as 2.0
after some observation period (not sure it is required or not).
(cherry picked from commit
2bc364c19105c12dc1dd6b043e81c48b184d4174)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
8d521427742f12b4377c0b01672a278ee974b8ac)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 21 Sep 2021 13:45:05 +0000 (15:45 +0200)]
MINOR: htx: Add a function to know if the free space wraps
the htx_space_wraps() function may now be used to know if the free space of
an HTX message wraps. It does the same as b_space_wraps().
(cherry picked from commit
361fbcc14acc4a3bed9161934f2524db8ac2069e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
4a7f6358b0339b2c21f793ae926d274934abbdeb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 21 Sep 2021 13:39:30 +0000 (15:39 +0200)]
MINOR: htx: Add an HTX flag to know when a message is fragmented
HTX_FL_FRAGMENTED flag is now set on an HTX message when it is
fragmented. It happens when an HTX block is removed in the middle of the
message and flagged as unused. HTX_FL_FRAGMENTED flag is removed when all
data are removed from the message or when the message is defragmented.
Note that some optimisations are still possible because the flag can be
avoided in other situations. For instance when the last header of a bodyless
message is removed.
(cherry picked from commit
4697c92c9d7f875b30413a20bfeb56f318c182d1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
95bf88d719fa04bfa89808522971cabcbc6ffd4f)
[cf: Value of HTX_FL_FRAGMENTED flag was changed]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 22 Sep 2021 05:15:57 +0000 (07:15 +0200)]
BUG/MEDIUM: leastconn: fix rare possibility of divide by zero
An optimization was brought in commit
5064ab6a9 ("OPTIM: lb-leastconn:
do not unlink the server if it did not change") to avoid locking the
server just to discover it did not move. However a mistake was made
because the operation involves a divide with a value that is read
outside of its usual lock, which makes it possible to be zero at the
exact moment we watch it if another thread takes the server down under
the lbprm lock, resulting in a divide by zero.
Therefore we must check that the value is not null there.
This must be backported to 2.4.
(cherry picked from commit
6f97b4ef3383f59ff3218f73fcd21f94335b2c1b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
8031464835df8166666f315515604fe6bc3c5598)
[cf: Above commit was backported to 2.3. Thus this patch should be backported
too.]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 4 Oct 2021 12:16:46 +0000 (14:16 +0200)]
BUG/MEDIUM: http-ana: Clear request analyzers when applying redirect rule
A bug was introduced by the commit
2d5650082 ("BUG/MEDIUM: http-ana: Reset
channels analysers when returning an error").
The request analyzers must be cleared when a redirect rule is applied. It is
not a problem if the redirect rule is inside an http-request ruleset because
the analyzer takes care to clear it. However, when it comes from a redirect
ruleset (via the "redirect ..." directive), because of the above commit,
the request analyzers are no longer cleared. It means some HTTP request
analyzers may be called while the request channel was already flushed. It is
totally unexpected and may lead to crash.
Thanks to Yves Lafon for reporting the problem.
This patch must be backported everywhere the above commit was backported.
(cherry picked from commit
d34758849e1f7e070a25022fcfe38e4fc82e22c6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9132227728115431dbf379b9006347c2430d0dda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 4 Oct 2021 05:50:13 +0000 (07:50 +0200)]
BUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release
When a filter is attached to a stream, the wrong FLT_END analyzer is added
on the request channel. AN_REQ_FLT_END must be added instead of
AN_RES_FLT_END. Because of this bug, the stream may hang on the filter
release stage.
It seems to be ok for HTTP filters (cache & compression) in HTTP mode. But
when enabled on a TCP proxy, the stream is blocked until the client or the
server timeout expire because data forwarding is blocked. The stream is then
prematurely aborted.
This bug was introduced by commit
26eb5ea35 ("BUG/MINOR: filters: Always set
FLT_END analyser when CF_FLT_ANALYZE flag is set"). The patch must be
backported in all stable versions.
(cherry picked from commit
d28b2b2352291991190fd0693fdfd3d7430e6ddb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
4a9db77783773955bb9d76ac7b2ef1c8c06eed28)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 30 Sep 2021 14:38:09 +0000 (16:38 +0200)]
MINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()
__task_queue() must absolutely not be called with TICK_ETERNITY or it
will place a never-expiring node upfront in the timers queue, preventing
any timer from expiring until the process is restarted. Code was found
to cause this using "task_schedule(task, now_ms)" which does this one
millisecond every 49.7 days, so let's add a condition against this. It
must never trigger since any process susceptible to trigger it would
already accumulate tasks until it dies.
An extra test was added in wake_expired_tasks() to detect tasks whose
timeout would have been changed after being queued.
An improvement over this could be in the future to use a non-scalar
type (union/struct) for expiration dates so as to avoid the risk of
using them directly like this. But now_ms is already such a valid
time and this specific construct would still not be caught.
This could even be backported to stable versions to help detect other
occurrences if any.
(cherry picked from commit
7a9699916a92a98ca5daaff77e2a25f9bec8817d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
aaed609f9189915ef610cc47d8d1930ebb257b9d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 30 Sep 2021 12:56:30 +0000 (14:56 +0200)]
BUG/MINOR: tcp-rules: Stop content rules eval on read error and end-of-input
For now, tcp-request and tcp-response content rules evaluation is
interrupted before the inspect-delay when the channel's buffer is full, the
RX path is blocked or when a shutdown for reads was received. To sum up, the
evaluation is interrupted when no more input data are expected. However, it
is not exhaustive. It also happens when end of input is reached (CF_EOI flag
set) or when a read error occurred (CF_READ_ERROR flag set).
Note that, AFAIK, it is only a problem on HAProy 2.3 and prior when a H1 to
H2 upgrade is performed. On newer versions, it works as expected because the
stream is not created at this stage.
This patch must be backported as far as 2.0.
(cherry picked from commit
cb59e0bc3cbe48eedd88e91b999a471724a6b709)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
fc553c9f0b9cacd4e0ec10bc9ff56879248a9b1c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 30 Sep 2021 14:22:51 +0000 (16:22 +0200)]
BUG/MINOR: tcpcheck: Don't use arg list for default proxies during parsing
During tcp/http check rules parsing, when a sample fetch or a log-format
string is parsed, the proxy's argument list used to track unresolved
argument is no longer passed for default proxies. It means it is no longer
possible to rely on sample fetches depending on the execution context (for
instance 'nbsrv').
It is important to avoid HAProxy crashes because these arguments are
resolved during the configuration validity check. But, default proxies are
not evaluated during this stage. Thus, these arguments remain unresolved.
It will probably be possible to relax this rule. But to ease backports, it
is forbidden for now.
This patch must be backported as far as 2.2. It depends on the commit
"MINOR: arg: Be able to forbid unresolved args when building an argument
list". It must be adapted for the 2.3 because PR_CAP_DEF capability was
introduced in the 2.4. A solution may be to test The proxy's id agains NULL.
(cherry picked from commit
eaba25dd97e95a45ba1fa1e9ce41c951410621c0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
8551c3430adc063d614cb0bd0cd716fdf5b6bcfb)
[cf: As expected, the test on the proxy was changed. Instead of relying on
PR_CAP_DEF, we test the proxy's ID against NULL. Only default servers have
no id]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 30 Sep 2021 06:48:56 +0000 (08:48 +0200)]
MINOR: arg: Be able to forbid unresolved args when building an argument list
In make_arg_list() function, unresolved dependencies are pushed in an
argument list to be resolved later, during the configuration validity
check. It is now possible to forbid such unresolved dependencies by omitting
<al> parameter (setting it to NULL). It is usefull when the parsing context
is not the same than the running context or when the parsing context is lost
after the startup stage. For instance, an argument may be defined in
defaults section during parsing and executed in a frontend/backend section.
(cherry picked from commit
35926a16ac087ac737026206a8420a65a9fce0ca)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
45bb15f6403ece51d1bedb94110d427c1184ea34)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 30 Sep 2021 14:17:37 +0000 (16:17 +0200)]
BUG/MAJOR: lua: use task_wakeup() to properly run a task once
The Lua tasks registered vi core.register_task() use a dangerous
task_schedule(task, now_ms) to start them, that will most of the
time work by accident, except when the time wraps every 49.7 days,
if now_ms is 0, because it's not valid to queue a task with an
expiration date set to TICK_ETERNITY, as it will fail all wakeup
checks and prevent all subsequent timers from being seen as expired.
The only solution in this case is to restart the process.
Fortunately for the vast majority of users it is extremely unlikely
to ever be met (only one millisecond every 49.7 days is at risk), but
this can be systematic for a process dealing with 1000 req/s, hence
the major tag.
The bug was introduced in 1.6-dev with commit
24f335340 ("MEDIUM: lua:
add coroutine as tasks."), so the fix must be backported to all stable
branches.
(cherry picked from commit
e3957f83e070fab55853d4803fcb3aba890091ce)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
790810fe9652cbf26b11a41f43c5592537a7131f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 30 Sep 2021 14:12:31 +0000 (16:12 +0200)]
BUG/MEDIUM: lua: fix wakeup condition from sleep()
A time comparison was wrong in hlua_sleep_yield(), making the sleep()
code do nothing for periods of 24 days every 49 days. An arithmetic
comparison was performed on now_ms instead of using tick_is_expired().
This bug was added in 1.6-dev by commit
5b8608f1e ("MINOR: lua: core:
add sleep functions") so the fix should be backported to all stable
versions.
(cherry picked from commit
12c02701d304f29ca74f521cfcc5fe1bc6f3e03d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
9cc2e4c9962bf83a0d08629444848f3ddae64473)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emeric Brun [Wed, 29 Sep 2021 08:29:52 +0000 (10:29 +0200)]
DOC: peers: fix doc "enable" statement on "peers" sections
Checking in code the right keyword is "enabled" and not "enable".
In addition the comment was also completed:
This could appear useless because the "defaults" sections not
yet apply on "peers" sections, but it could be the case in the future.
This statement can currently cancel a previous "disabled" keyword in
the same section.
This patch should be backported in all supported branches (keyword
is present since 1.5)
(cherry picked from commit
620761f934fe24f3943ec2ff346d8ecd0e6a6576)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
7214dcfef69909f516f04f509421c4d915624c51)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 28 Sep 2021 08:56:36 +0000 (10:56 +0200)]
BUG/MINOR: mux-h1/mux-fcgi: Sanitize TE header to only send "trailers"
Only chunk-encoded response payloads are supported by HAProxy. All other
transfer encodings are not supported and will be an issue if the HTTP
compression is enabled. So be sure only "trailers" is send in TE request
headers.
The patch is related to the issue #1301. It must be backported to all stable
versions. Be carefull for 2.0 and lower because the HTTP legacy must also be
fixed.
(cherry picked from commit
f56e8465f067c84b820dbedd89e6f44f1e02c179)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
f9bb8d0fbcc945e15e8d1dc8ed792b27f1ca693b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 23 Sep 2021 12:46:32 +0000 (14:46 +0200)]
BUG/MEDIUM: stream: Stop waiting for more data if SI is blocked on RXBLK_ROOM
If the stream-interface is waiting for more buffer room to store incoming
data, it is important at the stream level to stop to wait for more data to
continue. Thanks to the previous patch ("BUG/MEDIUM: stream-int: Notify
stream that the mux wants more room to xfer data"), the stream is woken up
when this happens. In this patch, we take care to interrupt the
corresponding tcp-content ruleset or to stop waiting for the HTTP message
payload.
To ease detection of the state, si_rx_blocked_room() helper function has
been added. It returns non-zero if the stream interface's Rx path is blocked
because of lack of room in the input buffer.
This patch is part of a series related to the issue #1362. It should be
backported as ar as 2.0, probably with some adaptations. So be careful
during backports.
(cherry picked from commit
7833596ff42d5f5ba2e9f639b9698cec16613058)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
59df812bf1eb75b96f1da1697ce861cafa8b59ff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 23 Sep 2021 12:17:20 +0000 (14:17 +0200)]
BUG/MEDIUM: stream-int: Notify stream that the mux wants more room to xfer data
When the mux failed to transfer data to the upper layer because of a lack of
room, it is important to wake the stream up to let it handle this
event. Otherwise, if the stream is waiting for more data, both the stream
and the mux reamin blocked waiting for each other.
When this happens, the mux set the CS_FL_WANT_ROOM flag on the
conn-stream. Thus, in si_cs_recv() we are able to detect this event. Today,
the stream-interface is blocked. But, it is not enough to wake the stream
up. To fix the bug, CF_READ_PARTIAL flag is extended to also handle cases
where a read exception occurred. This flag should idealy be renamed. But for
now, it is good enough. By setting this flag, we are sure the stream will be
woken up.
This patch is part of a series related to the issue #1362. It should be
backported as far as 2.0, probably with some adaptations. So be careful
during backports.
(cherry picked from commit
df99408e0d4e59cf54666772a4224a8665743fbd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
47545caccaf4b429f4fd002062c66d2c2592abcd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 20 Sep 2021 05:47:27 +0000 (07:47 +0200)]
BUG/MEDIUM: mux-h1: Adjust conditions to ask more space in the channel buffer
When a message is parsed and copied into the channel buffer, in
h1_process_demux(), more space is requested if some pending data remain
after the parsing while the channel buffer is not empty. To do so,
CS_FL_WANT_ROOM flag is set. It means the H1 parser needs more space in the
channel buffer to continue. In the stream-interface, when this flag is set,
the SI is considered as blocked on the RX path. It is only unblocked when
some data are sent.
However, it is not accurrate because the parsing may be stopped because
there is not enough data to continue. For instance in the middle of a chunk
size. In this case, some data may have been already copied but the parser is
blocked because it must receive more data to continue. If the calling SI is
blocked on RX at this stage when the stream is waiting for the payload
(because http-buffer-request is set for instance), the stream remains stuck
infinitely.
To fix the bug, we must request more space to the app layer only when it is
not possible to copied more data. Actually, this happens when data remain in
the input buffer while the H1 parser is in states MSG_DATA or MSG_TUNNEL, or
when we are unable to copy headers or trailers into a non-empty buffer.
The first condition is quite easy to handle. The second one requires an API
refactoring. h1_parse_msg_hdrs() and h1_parse_msg_tlrs() fnuctions have been
updated. Now it is possible to know when we need more space in the buffer to
copy headers or trailers (-2 is returned). In the H1 mux, a new H1S flag
(H1S_F_RX_CONGESTED) is used to track this state inside h1_process_demux().
This patch is part of a series related to the issue #1362. It should be
backported as far as 2.0, probably with some adaptations. So be careful
during backports.
(cherry picked from commit
46e058dda51cf09ae0a734ce0931be53dcc179a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
58f21dae3d82a928ae76a15634c45c8db9bf0aca)
[cf: value of H1S_F_RX_CONGESTED flag was changed]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Dragan Dosen [Tue, 21 Sep 2021 11:02:09 +0000 (13:02 +0200)]
BUG/MINOR: http-ana: increment internal_errors counter on response error
A bug was introduced in the commit
cff0f739e51 ("MINOR: counters: Review
conditions to increment counters from analysers"). The internal_errors
counter for the target server was incremented twice. The counter for the
session listener needs to be incremented instead.
This must be backported everywhere the commit
cff0f739e51 is.
(cherry picked from commit
9a006f9641ceb353c007f8f7e8ae949ace786099)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
84426cd01cd805dc26570312d6c8485f67939b37)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 23 Sep 2021 13:38:26 +0000 (15:38 +0200)]
BUG/MINOR: h1-htx: Fix a typo when request parser is reset
In h1_postparse_req_hdrs(), if we need more space to copy headers, the request
parser is reset. However, because of a typo, it was reset as a response parser
instead of a request one. h1m_init_req() must be called.
This patch must be backported as far as 2.2.
(cherry picked from commit
216d3352b1aac1bff19383b6a6e5476e11e613e9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c7c20a7692130c42796b1a63e9d72c3e933fe737)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 21 Sep 2021 08:29:09 +0000 (10:29 +0200)]
BUG/MINOR: server: allow 'enable health' only if check configured
Test that checks have been configured on the server before enabling via
the 'enable health' CLI. This mirrors the 'enable agent' command.
Without this, a user can use the command on the server without checks.
This leaves the server in an undefined state. Notably, the stat page
reports the server in check transition.
This condition was left on the following reorg commit.
2c04eda8b58636ad2ae44e42b1f50f3b5a24a642
REORG: cli: move "{enable|disable} health" to server.c
This should be backported up to 1.8.
(cherry picked from commit
0f456d502963f3555552d4b3d5f97ed504353d85)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
e636a1106961ea0a0986657fc9943b03eb6f531a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 17 Sep 2021 09:07:45 +0000 (11:07 +0200)]
BUG/MINOR: cli/payload: do not search for args inside payload
The CLI's payload parser is over-complicated and as such contains more
bugs than needed. One of them is that it uses strstr() to find the
ending tag, ignoring spaces before it, while the argument locator
creates a new arg on each space, without checking if the end of the
word appears past the previously found end. This results in "<<" being
considered as the start of a new argument if preceeded by more than
one space, and the payload being damaged with a \0 inserted at the
first space or tab.
Let's make an easily backportable fix for now. This fix makes sure that
the trailing zero from the first line is properly kept after '<<' and
that the end tag is looked for only as an isolated argument and nothing
else. This also gets rid of the unsuitable strstr() call and now makes
sure that strcspn() will not return elements that are found in the
payload.
For the long term the loop must be rewritten to get rid of those
unsuitable strcspn() and strstr() calls which work past each other, and
the cli_parse_request() function should be split into a tokenizer and
an executor that are used from the caller instead of letting the caller
play games with what it finds there.
This should be backported wherever CLI payload is supported, i.e. 2.0+.
(cherry picked from commit
f2dda52e784f3eebc693a70b9491a428cbbf188e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c5c170a0e2b59f1580bfa7578bdc0410a7ccbf0e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 18 May 2021 09:33:57 +0000 (11:33 +0200)]
BUILD: ist: prevent gcc11 maybe-uninitialized warning on istalloc
A new warning is reported by gcc11 when using a pointer to uninitialized
memory block for a function with a const pointer argument. The warning
is triggered for istalloc, used by http_client.c / proxy.c / tcpcheck.c.
This warning is reported because the uninitialized memory block
allocated by malloc should not be passed to a const argument as in ist2.
See https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/Warning-Options.html#index-Wmaybe-uninitialized
This should be backported up to 2.2.
(cherry picked from commit
7a8aff26881c58061d6ca98d3c3e5fc70060c8f4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
3471051fe113619fd3a6165cc301894a085056aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 16 Sep 2021 15:30:51 +0000 (17:30 +0200)]
DOC: management: certificate files must be sanitized before injection
A lot of people encounter problems when trying to inject a certificate
file which contains extra informations or empty lines.
This patch adds a paragraph and a sanitizing example.
Must be backported as far as 2.1.
(cherry picked from commit
ed8bfadd8d17fc59b8a1f57bb2476cd7df1ce190)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
827347512a198e07f5acf9cb6767a06ef4898036)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 16 Sep 2021 14:01:09 +0000 (16:01 +0200)]
BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check
When the LDAP response is parsed, the message length is not properly
decoded. While it works for LDAP servers encoding it on 1 byte, it does not
work for those using a multi-bytes encoding. Among others, Active Directory
servers seems to encode messages or elements length on 4 bytes.
In this patch, we only handle length of BindResponse messages encoded on 1,
2 or 4 bytes. In theory, it may be encoded on any bytes number less than 127
bytes. But it is useless to make this part too complex. It should be ok this
way.
This patch should fix the issue #1390. It should be backported to all stable
versions. While it should be easy to backport it as far as 2.2, the patch
will have to be totally rewritten for lower versions.
(cherry picked from commit
8a0e5f822b84c984681547eb0e9ee00d8a19ce56)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
81011211b71b4263ac096d538ba3823e1b693b08)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 16 Sep 2021 07:18:21 +0000 (09:18 +0200)]
MINOR: pools: use mallinfo2() when available instead of mallinfo()
Ilya reported in issue #1391 a build warning on Fedora about mallinfo()
being deprecated in favor of mallinfo2() since glibc-2.33. Let's add
support for it. This should be backported where the following commit is
also backported:
157e39303 ("MINOR: pools: automatically disable
malloc_trim() with external allocators").
(cherry picked from commit
c2afb860f297eb82039d821bd25bdd3cf66abe28)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
3bf75694c2ba89c1b705ce94b5543aa30fccf4a7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 15 Sep 2021 08:05:48 +0000 (10:05 +0200)]
MINOR: pools: automatically disable malloc_trim() with external allocators
Pierre Cheynier reported some occasional crashes in malloc_trim() on a
recent glibc when running with jemalloc(). While in theory there should
not be any link between the two, it remains plausible that something
allocated early with one is tentatively freed with the other and that
attempts to trim end up badly. There's no point calling the glibc specific
malloc_trim() with external allocators anyway. However these ones are often
enabled at link time or even at run time with LD_PRELOAD, so we cannot rely
on build options for this.
This patch implements runtime detection for the allocator in use by checking
with mallinfo() that a malloc() call is properly accounted for in glibc's
malloc. It only enables malloc_trim() in this case, and ignores it for
other cases. It's fine to proceed like this because mallinfo() is provided
by a wider range of glibcs than malloc_trim().
This could be backported to 2.4 and 2.3. If so, it will also need previous
patch "CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()".
(cherry picked from commit
157e393039966fa9a1b4f9a5ca964ec56f28204b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
562bf20a8501f01f56009c959d0e58e590d2ef66)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 15 Sep 2021 08:38:21 +0000 (10:38 +0200)]
CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()
The code was slightly cleaned up by removing repeated occurrences of ifdefs
and moving that into a single trim_all_pools() function.
(cherry picked from commit
ea3323f62c093f2ee5e09b131ab6e3e23d113838)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
33298de642027a51451e40eae7a122f29c18dc09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 15 Sep 2021 08:15:03 +0000 (10:15 +0200)]
BUG/MINOR: compat: make sure __WORDSIZE is always defined
-Wundef triggered on a MIPS-based musl build on __WORDSIZE that's used
in ultoa_o() and some Lua initialization. The former will fail to convert
integers larger to 1 billion to proper string in this case. Let's make
sure this macro is defined and fall back to values determined from
__SIZEOF_LONG__ otherwise. A cleaner long-term approach would consist
in removing all remaining occurrences of this macro.
This can be backported to all versions.
(cherry picked from commit
4f5485bfad8355b7cf2d62a36897558dfc44bcb6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
3f41b89598ed8978ee878d35b98a5c0e997c3b05)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Mon, 20 Sep 2021 14:46:30 +0000 (16:46 +0200)]
Revert "REGTESTS: mark http_abortonclose as broken"
This reverts commit
e3abd92c6c3496bf14bc51b4143fe7c6e72a6acf.
The abortonclose option is fixed again, thanks to the commit
3dae79e59d
("BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is
reached"). The corresponding regtest should work again.
Christopher Faulet [Thu, 9 Sep 2021 08:17:59 +0000 (10:17 +0200)]
BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is reached
If the end of input is reported by the mux on the conn-stream during a
receive, we leave without evaluating the channel policies. It is especially
important to be able to catch client aborts during server connection
establishment. Indeed, in this case, without this patch, the
stream-interface remains blocked and read events are not forwarded to the
stream. It means it is not possible to detect client aborts.
Thanks to this fix, the abortonclose option should fixed for HAProxy 2.3 and
lower. On 2.4 and 2.5, it seems to work because the stream is created after
the request parsing.
Note that a previous fix of abortonclose option was reverted. This one
should be the right way to fix it. It must carefully be backported as far as
2.0. A observation period on the 2.3 is probably a good idea.
(cherry picked from commit
883d83e83c54c77cd16735716a029670b6317926)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
82219b562b512bda3b71489577a23f0c92ea875f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 20 Aug 2021 21:29:53 +0000 (23:29 +0200)]
BUG/MINOR: systemd: ExecStartPre must use -Ws
This line should disappear in a future version but we should still fix
ExecStartPre with -Ws like we've done in 9def142.
It's a complementary fix that must be backported with 9def142
("BUG/MINOR: systemd: must check the configuration using -Ws").
(cherry picked from commit
6f58c13797dc02a5b0c5be943f6d5396e486bd71)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
0857f96996f669fc17557dfc045c873b4d09b2d3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 10 Sep 2021 08:29:54 +0000 (10:29 +0200)]
BUG/MINOR: filters: Set right FLT_END analyser depending on channel
A bug was introduced by the commit
26eb5ea35 ("BUG/MINOR: filters: Always
set FLT_END analyser when CF_FLT_ANALYZE flag is set"). Depending on the
channel evaluated, the rigth FLT_END analyser must be set. AN_REQ_FLT_END
for the request channel and AN_RES_FLT_END for the response one.
Ths patch must be backported everywhere the above commit was backported.
(cherry picked from commit
949b6ca961904c7414fc22204461f9882afff4ea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
2f3ead14a522e1ffb587e8315b0b0a24acab5d56)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 13 Aug 2021 12:00:46 +0000 (14:00 +0200)]
BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set
CF_FLT_ANALYZE flags may be set before the FLT_END analyser. Thus if an error is
triggered in the mean time, this may block the stream and prevent it to be
released. It is indeed a problem only for the response channel because the
response analysers may be skipped on early errors.
So, to prevent any issue, depending on the code path, the FLT_END analyser is
systematically set when the CF_FLT_ANALYZE flag is set.
This patch must be backported in all stable branches.
(cherry picked from commit
26eb5ea352c2ba8b25f014222773fb6946004bef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
feca2a4534092158319d43c65dd23ffd61889a5a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 10 Sep 2021 07:17:50 +0000 (09:17 +0200)]
BUG/MEDIUM: http-ana: Reset channels analysers when returning an error
When an error is returned to the client, via a call to
http_reply_and_close(), the request channel is flushed and shut down and
HTTP analysis on both direction is finished. So it is safer to centralize
reset of channels analysers at this place. It is especially important when a
filter is attached to the stream when a client abort is detected. Because,
otherwise, the stream remains blocked because request analysers are not
reset.
This bug was hidden for a while. But since the fix
6fcd2d328 ("BUG/MINOR:
stream: Don't release a stream if FLT_END is still registered"), it is
possible to trigger it.
This patch must be backported everywhere the above commit was backported.
(cherry picked from commit
2d56500826d54fca445de2c0170384763b1cfdb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
db8ba1069af969f0ad344c8c9d1be0a650ca3005)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 13 Nov 2019 10:12:32 +0000 (11:12 +0100)]
BUG/MINOR: stream: Don't release a stream if FLT_END is still registered
When at least one filter is registered on a stream, the FLT_END analyzer is
called on both direction when all other analyzers have finished their
processing. During this step, filters may release any allocated elements if
necessary. So it is important to not skip it.
Unfortunately, if both stream interfaces are closed, it is possible to not
wait the end of this analyzer. It is possible to be in this situation if a
filter must wait and prevents the analyzer completion. To fix the bug, we
now wait FLT_END analyzer is no longer registered on both direction before
releasing the stream.
This patch may be backported as far as 1.7, but AFAIK, no filter is affected
by this bug. So the backport seems to be optional for now. In any case, it
should remain under observation for some weeks first.
(cherry picked from commit
6fcd2d32805d949672babae8ca70806f8b308870)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
223f48e6fa8a3ddb75f41dafa398c5f147637e3e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Fri, 6 Aug 2021 07:59:49 +0000 (09:59 +0200)]
BUG/MINOR: lua: Don't yield in channel.append() and channel.set()
Lua functions to set or append data to the input part of a channel must not
yield because new data may be received while the lua script is suspended. So
adding data to the input part in several passes is highly unpredicatble and
may be interleaved with received data.
Note that if necessary, it is still possible to suspend a lua action by
returning act.YIELD. This way the whole action will be reexecuted later
because of I/O events or a timer. Another solution is to call core.yield().
This bug affects all stable versions. So, it may be backported. But it is
probably not necessary because nobody notice it till now.
(cherry picked from commit
3b7bc3e38b7c3e889ca77415cb6b5d91e92cc0c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 5 Aug 2021 09:58:37 +0000 (11:58 +0200)]
BUG/MINOR: lua: Yield in channel functions only if lua context can yield
When a script is executed, it is not always allowed to yield. Lua sample
fetches and converters cannot yield. For lua actions, it depends on the
context. When called from tcp content ruleset, an action may yield until the
expiration of the inspect-delay timeout. From http rulesets, yield is not
possible.
Thus, when channel functions (dup, get, append, send...) are called, instead
of yielding when it is not allowed and triggering an error, we just give
up. In this case, some functions do nothing (dup, append...), some others
just interrupt the in-progress job (send, forward...). But, because these
functions don't yield anymore when it is not allowed, the script regains the
control and can continue its execution.
This patch depends on "MINOR: lua: Add a flag on lua context to know the
yield capability at run time". Both may be backported in all stable
versions. However, because nobody notice this bug till now, it is probably
not necessary, excepted if someone ask for it.
(cherry picked from commit
055310ff49749ab1f4b3e4bb2ea675fbc597f3f3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 4 Aug 2021 15:58:21 +0000 (17:58 +0200)]
MINOR: lua: Add a flag on lua context to know the yield capability at run time
When a script is executed, a flag is used to allow it to yield. An error is
returned if a lua function yield, explicitly or not. But there is no way to
get this capability in C functions. So there is no way to choose to yield or
not depending on this capability.
To fill this gap, the flag HLUA_NOYIELD is introduced and added on the lua
context if the current script execution is not authorized to yield. Macros
to set, clear and test this flags are also added.
This feature will be usefull to fix some bugs in lua actions execution.
(cherry picked from commit
a6e792ff52f895234bb1499e9df5417bf498a8f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 7 Sep 2021 14:25:26 +0000 (16:25 +0200)]
[RELEASE] Released version 2.3.14
Released version 2.3.14 with the following main changes :
- BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
- BUG/MEDIUM: sock: really fix detection of early connection failures in for 2.3-
- REGTESTS: abortonclose: after retries, 503 is expected, not close
- BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
- BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}
- MINOR: compiler: implement an ONLY_ONCE() macro
- BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
- BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
- BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
- BUG/MINOR: tools: Fix loop condition in dump_text()
- CLEANUP: Add missing include guard to signal.h
- BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
- DOC: configuration: remove wrong tcp-request examples in tcp-response
- BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
- CLEANUP: htx: remove comments about "must be < 256 MB"
- BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer
- Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive"
- MINOR: action: Use a generic function to check validity of an action rule list
- REGTESTS: mark http_abortonclose as broken
Willy Tarreau [Tue, 7 Sep 2021 14:24:11 +0000 (16:24 +0200)]
REGTESTS: mark http_abortonclose as broken
This is a consequence of the revert of the commit below, until a better
fix is found for this issue:
BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
Christopher Faulet [Thu, 25 Mar 2021 16:19:04 +0000 (17:19 +0100)]
MINOR: action: Use a generic function to check validity of an action rule list
The check_action_rules() function is now used to check the validity of an
action rule list. It is used from check_config_validity() function to check
L5/6/7 rulesets.
(cherry picked from commit
42c6cf950111736327863de5e82036a1d51deb04)
[cf: This patch is in fact a fix because the "tcp-resonse content" ruleset
was not fully configured. It was ignored during Post-parsing
stage. This patch should fix a bug reported in #1263 by @ HiggTh. It
must be backported in all stable versions.]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 7 Sep 2021 12:31:02 +0000 (14:31 +0200)]
Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive"
This reverts commit
e0dec4b7b258101f6d5faa15234103a45c16f0f8.
At first glance, channel_is_empty() was used on purpose in si_update_rx(),
because of the HTX ("
b3e0de46c" MEDIUM: stream-int: Rely only on
SI_FL_WAIT_ROOM to stop data receipt). It is not pretty clear for now why
channel_may_recv() sould not be used here but this change introduce a
possible infinite loop with the stats applet. So, it is safer to revert the
patch, waiting for a better understanding of the probelm.
This means the abortonclose option will be broken again on the 2.3 and lower
versions.
This patch should fix the issue #1360. It must be backported as far as 2.0.
(cherry picked from commit
b7308f00cb03e5accd2f6092bf09fc2ac1360ff9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
62fc11cb2da250f6c70ea3a1b3af767d8d0a9b2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 26 Aug 2021 14:23:37 +0000 (16:23 +0200)]
BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer
Ori Hollander of JFrog Security reported that htx_add_header() and
htx_add_trailer() were missing a length check on the header name. While
this does not allow to overwrite any memory area, it results in bits of
the header name length to slip into the header value length and may
result in forging certain header names on the input. The sad thing here
is that a FIXME comment was present suggesting to add the required length
checks :-(
The injected headers are visible to the HTTP internals and to the config
rules, so haproxy will generally stay synchronized with the server. But
there is one exception which is the content-length header field, because
it is already deduplicated on the input, but before being indexed. As
such, injecting a content-length header after the deduplication stage
may be abused to present a different, shorter one on the other side and
help build a request smuggling attack, or even maybe a response splitting
attack. CVE-2021-40346 was assigned to this problem.
As a mitigation measure, it is sufficient to verify that no more than
one such header is present in any message, which is normally the case
thanks to the duplicate checks:
http-request deny if { req.hdr_cnt(content-length) gt 1 }
http-response deny if { res.hdr_cnt(content-length) gt 1 }
This must be backported to all HTX-enabled versions, hence as far as 2.0.
In 2.3 and earlier, the functions are in src/htx.c instead.
Many thanks to Ori for his work and his responsible report!
(cherry picked from commit
3b69886f7dcc3cfb3d166309018e6cfec9ce2c95)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
1fd2566683f6fb66b180ce9c3d9062ddaa81d6d7)
[wt: code is in src/htx.c in 2.3 and older]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 26 Aug 2021 14:07:22 +0000 (16:07 +0200)]
CLEANUP: htx: remove comments about "must be < 256 MB"
Since commit "BUG/MINOR: config: reject configs using HTTP with bufsize
>= 256 MB" we are now sure that it's not possible anymore to have an HTX
block of a size 256 MB or more, even after concatenation thanks to the
tests for len >= htx_free_data_space(). Let's remove these now obsolete
comments.
A BUG_ON() was added in htx_add_blk() to track any such exception if
the conditions would change later, to complete the one that is performed
on the start address that must remain within the buffer.
(cherry picked from commit
3d5f19e04d88e7c8f71cba4ea12e383c91de89f6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
86cb2cd3c68a0f3072a326def89449e10760423d)
[wt: no change in htx.h, ctx updates in htx.c]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 26 Aug 2021 13:59:44 +0000 (15:59 +0200)]
BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
As seen in commit
5ef965606 ("BUG/MINOR: lua: use strlcpy2() not
strncpy() to copy sample keywords"), configs with large values of
tune.bufsize were not practically usable since Lua was introduced,
regardless of the machine's available memory.
In addition, HTX encoding already limits block sizes to 256 MB, thus
it is not technically possible to use that large a buffer size when
HTTP is in use. This is absurdly high anyway, and for example Lua
initialization would take around one minute on a 4 GHz CPU. Better
prevent such a config from starting than having to deal with bug
reports that make no sense.
The check is only enforced if at least one HTX proxy was found, as
there is no techincal reason to block it for configs that are solely
based on raw TCP, and it could still be imagined that some such might
exist with single connections (e.g. a log forwarder that buffers to
cover for the storage I/O latencies).
This should be backported to all HTX-enabled versions (2.0 and above).
(cherry picked from commit
32b51cdf303cb30425000f1db6ecdae5de84ff8d)
[wt: minor ctx adj]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
95d0810ddec6d778bf8a08f4369bacbc0f19ad6e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 2 Sep 2021 18:51:21 +0000 (20:51 +0200)]
DOC: configuration: remove wrong tcp-request examples in tcp-response
There is a massive abuse of copy-paste in the doc that is visible in
the examples and arguments declaration. Let's at least remove irrelevant
examples for now.
(cherry picked from commit
e7267120d5622acaf6fcc5d82f520655cf86e11f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9de54ba791d2453709920131738366d796bf5e7e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 2 Sep 2021 16:46:22 +0000 (18:46 +0200)]
BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
The parser checks first for "set-var" then "unset-var" from the updated
offset instead of testing it only when the other one fails, so it
validates this rule as "unset-var":
http-request set-varunset-var(proc.a)
This should be backported everywhere relevant, though it's mostly harmless
as it's unlikely that some users are purposely writing this in their conf!
(cherry picked from commit
2819210a8328ff60505e499acf9f8bc323bec840)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f6bda6191520885fc3af06c3dec8a74e56d46cbd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Wed, 1 Sep 2021 19:23:25 +0000 (21:23 +0200)]
CLEANUP: Add missing include guard to signal.h
Found using GitHub's CodeQL scan.
(cherry picked from commit
abc6b31ab863be589758b0760b92b5d3a4f903d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
af75f82be0c7a822bf23d01fb7bf1b862b8f51e9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sat, 28 Aug 2021 22:58:22 +0000 (00:58 +0200)]
BUG/MINOR: tools: Fix loop condition in dump_text()
The condition should first check whether `bsize` is reached, before
dereferencing the offset. Even if this always works fine, due to the
string being null-terminated, this certainly looks odd.
Found using GitHub's CodeQL scan.
This bug traces back to at least
97c2ae13bc0d7961a348102d6719fbcaf34d46d5
(1.7.0+) and this patch should be backported accordingly.
(cherry picked from commit
18795d48a9bb09aedc57e547029828a56322e49d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cd5521e7ca7472bf69874603c0c5785d4ff1d1e2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Tim Duesterhus [Sat, 28 Aug 2021 21:57:01 +0000 (23:57 +0200)]
BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
Using localtime / gmtime is not thread-safe, whereas the `get_*` wrappers are.
Found using GitHub's CodeQL scan.
The use in sample_conv_ltime() can be traced back to at least
fac9ccfb705702f211f99e67d5f5d5129002086a (first appearing in 1.6-dev3), so all
supported branches with thread support are affected.
(cherry picked from commit
1f269c12dc31bb63db31559cb44c187ab91abb64)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9da4b1fc83fe083c8f194c61402ba4ffeb5b330c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Sat, 28 Aug 2021 09:55:53 +0000 (11:55 +0200)]
BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
The code used to rely on BITS_PER_LONG to decide on the most efficient
way to perform a 64-bit shift, but this macro is not defined (at best
it's __BITS_PER_LONG) and it's likely that it's been like this since
the early implementation of ebtrees designed on i386. Let's remove the
test on this macro and rely on sizeof(long) instead, it also has the
benefit of letting the compiler validate the two branches.
This can be backported to all versions. Thanks to Ezequiel Garcia for
reporting this one in issue #1369.
(cherry picked from commit
cbdc74b4b32af93b4724adcb4b2354bb811693ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
d822d9ae2cbc8e713c40c0f7b9e0f2f2225a606d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 26 Aug 2021 14:48:53 +0000 (16:48 +0200)]
BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
The lua initialization code which creates the Lua mapping of all converters
and sample fetch keywords makes use of strncpy(), and as such can take ages
to start with large values of tune.bufsize because it spends its time zeroing
gigabytes of memory for nothing. A test performed with an extreme value of
16 MB takes roughly 4 seconds, so it's possible that some users with huge
1 MB buffers (e.g. for payload analysis) notice a small startup latency.
However this does not affect config checks since the Lua stack is not yet
started. Let's replace this with strlcpy2().
This should be backported to all supported versions.
(cherry picked from commit
5ef965606b5bacb12769c97f85b2cfd1c4e4ffe7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
fa92f36beb65b4d0515892c783d40e94d33b131c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Thu, 26 Aug 2021 13:46:51 +0000 (15:46 +0200)]
MINOR: compiler: implement an ONLY_ONCE() macro
There are regularly places, especially in config analysis, where we
need to report certain things (warnings or errors) only once, but
where implementing a counter is sufficiently deterrent so that it's
not done.
Let's add a simple ONLY_ONCE() macro that implements a static variable
(char) which is atomically turned on, and returns true if it's set for
the first time. This uses fairly compact code, a single byte of BSS
and is thread-safe. There are probably a number of places in the config
parser where this could be used. It may also be used to implement a
WARN_ON() similar to BUG_ON() but which would only warn once.
(cherry picked from commit
906f7daed16a3b1c4058b77fed44b3c4265e645f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6902527b83ed426793b3e89936f27b78c5c21f37)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Dragan Dosen [Tue, 24 Aug 2021 07:48:04 +0000 (09:48 +0200)]
BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}
Ensure that no more than olen bytes is written to the output buffer,
otherwise we might experience an unexpected behavior.
While the original code used to validate that the output size was
always large enough before starting to write, this validation was
later broken by the commit below, allowing to 3-byte blocks to
areas whose size is not multiple of 3:
commit
ed697e4856e5ac0b9931fd50fd8ff1b7739e5d88
Author: Emeric Brun <ebrun@haproxy.com>
Date: Mon Jan 14 14:38:39 2019 +0100
BUG/MINOR: base64: dec func ignores padding for output size checking
Decode function returns an error even if the ouptut buffer is
large enought because the padding was not considered. This
case was never met with current code base.
For base64urldec(), it's basically the same problem except that since
the input format supports arbitrary lengths, the problem has always
been there since its introduction in 2.4.
This should be backported to all stable branches having a backport of
the patch above (i.e. 2.0), with some adjustments depending on the
availability of the base64dec() and base64urldec().
(cherry picked from commit
f3899ddbcbf4c445d4a519fb04cc253e60d30e6e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
aec6e0244e8b1693c5f78bb5a0483986ef2ac688)
[wt: dropped base64urldec]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 24 Aug 2021 12:57:28 +0000 (14:57 +0200)]
BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
The sc-set-gpt0() parser was extended in 2.1 by commit
0d7712dff ("MINOR:
stick-table: allow sc-set-gpt0 to set value from an expression") to support
sample expressions in addition to plain integers. However there is a
subtlety there, which is that while the arg position must be incremented
when parsing an integer, it must not be touched when calling an expression
since the expression parser already does it.
The effect is that rules making use of sc-set-gpt0() followed by an
expression always ignore one word after that expression, and will typically
fail to parse if followed by an "if" as the parser will restart after the
"if". With no condition it's different because an empty condition doesn't
result in trying to parse anything.
This patch moves the increment at the right place and adds a few
explanations for a code part that was far from being obvious.
This should be backported to branches having the commit above (2.1+).
(cherry picked from commit
ece4c4a35265cbc28d25826205ee66dbf9ac6f7d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f1e1c91953c28f48a5b0bfde8e616b16c33ebb26)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 20 Aug 2021 09:12:47 +0000 (11:12 +0200)]
REGTESTS: abortonclose: after retries, 503 is expected, not close
The abortonclose test was only expecting a close after all server
retries were exhausted, it didn't check for the pending 503, which
fails with new versions of vtest starting with commit 8d6c6bd
("Leak-plugging on barriers").
This may be backported, but carefully in case older versions would
really close without responding.
(cherry picked from commit
1713eec36d347e7e7d6208980f0abd2db8d1cd81)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2b0ba5fe349f80013fb028f523c3ad945a8c5ffa)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 27 Aug 2021 13:08:47 +0000 (15:08 +0200)]
BUG/MEDIUM: sock: really fix detection of early connection failures in for 2.3-
Commit
165560f1a in 2.3 ("BUG/MEDIUM: sock: make sure to never miss early
connection failures") was a backport of the same 2.4 commit that aims at
detecting early connection failures. But it doesn't work in 2.3 and older
because fdtab[].state was split between fdtab[].ev and fdtab[].state before
2.4. So the extra condition that checks for the presence of FD_POLL_HUP or
FD_POLL_ERR in fdtab[].state results in checking for a flag that is never
there or for send activity, both not matching there, resulting in the fix
not working, explaining issues #1367, which in fact, is the same as #1251.
The proper test consists in testing fdtab[].ev.
This needs to be backported as far as the commit above. There is no mainline
equivalent since the issue is caused by the incorrect backport to 2.3.
Thanks to Christian Albrecht (@albix) for providing lots of extremely useful
captures!
Willy Tarreau [Thu, 19 Aug 2021 21:06:58 +0000 (23:06 +0200)]
BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
RFC7540 states that :path follows RFC3986's path-absolute. However
that was a bug introduced in the spec between draft 04 and draft 05
of the spec, which implicitly causes paths starting with "//" to be
forbidden. HTTP/1 (and now HTTP core semantics) made it explicit
that the request-target in origin-form follows a purposely defined
absolute-path defined as 1*(/ segment) to explicitly allow "//".
http2bis now fixes this by relying on absolute-path so that "//"
becomes valid and matches other versions. Full discussion here:
https://lists.w3.org/Archives/Public/ietf-http-wg/2021JulSep/0245.html
This issue appeared in haproxy with commit
4b8852c70 ("BUG/MAJOR: h2:
verify that :path starts with a '/' before concatenating it") when
making the checks on :path fully comply with the spec, and was backported
as far as 2.0, so this fix must be backported there as well to allow
"//" in H2 again.
(cherry picked from commit
46b7dff8f08cb6c5c3004d8874d6c5bc689a4c51)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
512cee88df5c40f1d3901a82cf6643fe9f74229e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 17 Aug 2021 12:12:05 +0000 (14:12 +0200)]
[RELEASE] Released version 2.3.13
Released version 2.3.13 with the following main changes :
- BUILD: add detection of missing important CFLAGS
- BUILD: lua: silence a build warning with TCC
- BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
- BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
- BUG/MEDIUM: ssl_sample: fix segfault for srv samples on invalid request
- BUG/MINOR: check: fix the condition to validate a port-less server
- BUG/MINOR: resolvers: Use a null-terminated string to lookup in servers tree
- BUG/MINOR: systemd: must check the configuration using -Ws
- BUG/MINOR: mux-h2: Obey dontlognull option during the preface
- BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
- BUG/MINOR: connection: Add missing error labels to conn_err_code_str
- BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
- BUG/MINOR: select: fix excess number of dead/skip reported
- BUG/MINOR: poll: fix abnormally high skip_fd counter
- BUG/MINOR: pollers: always program an update for migrated FDs
- BUG/MINOR: server: update last_change on maint->ready transitions too
- MINOR: spoe: Add a pointer on the filter config in the spoe_agent structure
- BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is released
- BUG/MINOR: buffer: fix buffer_dump() formatting
- BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
- DOC: Improve the lua documentation
- DOC: config: Fix 'http-response send-spoe-group' documentation
- BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are queued
- BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config check
- DOC/MINOR: fix typo in management document
- MINOR: http: add a new function http_validate_scheme() to validate a scheme
- BUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax
- BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
- BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
- BUG/MEDIUM: h2: give :authority precedence over Host
- REGTESTS: add a test to prevent h2 desync attacks
Amaury Denoyelle [Fri, 13 Aug 2021 07:43:24 +0000 (09:43 +0200)]
REGTESTS: add a test to prevent h2 desync attacks
This test ensure that h2 pseudo headers are properly checked for invalid
characters and the host header is ignored if :authority is present. This
is necessary to prevent h2 desync attacks as described here
https://portswigger.net/research/http2
(cherry picked from commit
7ef244d73b073edf3d493ed826ca1b0233c330e0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
39faba79f254dac92668f4852db4ef67a8421658)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 11 Aug 2021 13:39:13 +0000 (15:39 +0200)]
BUG/MEDIUM: h2: give :authority precedence over Host
The wording regarding Host vs :authority in RFC7540 is ambiguous as it
says that an intermediary must produce a host header from :authority if
Host is missing, but, contrary to HTTP/1.1, doesn't say anything regarding
the possibility that Host and :authority differ, which leaves Host with
higher precedence there. In addition it mentions that clients should use
:authority *instead* of Host, and that H1->H2 should use :authority only
if the original request was in authority form. This leaves some gray
area in the middle of the chain for fully valid H2 requests arboring a
Host header that are forwarded to the other side where it's possible to
drop the Host header and use the authority only after forwarding to a
second H2 layer, thus possibly seeing two different values of Host at
a different stage. There's no such issue when forwarding from H2 to H1
as the authority is dropped only only the Host is kept.
Note that the following request is sufficient to re-normalize such a
request:
http-request set-header host %[req.hdr(host)]
The new spec in progress (draft-ietf-httpbis-http2bis-03) addresses
this trouble by being a bit is stricter on these rules. It clarifies
that :authority must always be used instead of Host and that Host ought
to be ignored. This is much saner as it avoids to convey two distinct
values along the chain. This becomes the protocol-level equivalent of:
http-request set-uri %[url]
So this patch does exactly this, which we were initially a bit reluctant
to do initially by lack of visibility about other implementations'
expectations. In addition it slightly simplifies the Host header field
creation by always placing it first in the list of headers instead of
last; this could also speed up the look up a little bit.
This needs to be backported to 2.0. Non-HTX versions are safe regarding
this because they drop the URI during the conversion to HTTP/1.1 so
only Host is used and transmitted.
Thanks to Tim Düsterhus for reporting that one.
(cherry picked from commit
b5d2b9e154d78e4075db163826c5e0f6d31b2ab1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
b4934f044706e35a61de2e29b9d2a3ae7ae09f77)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 11 Aug 2021 09:12:46 +0000 (11:12 +0200)]
BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
Before HTX was introduced, all the HTTP request elements passed in
pseudo-headers fields were used to build an HTTP/1 request whose syntax
was then scrutinized by the HTTP/1 parser, leaving no room to inject
invalid characters.
While NUL, CR and LF are properly blocked, it is possible to inject
spaces in the method so that once translated to HTTP/1, fields are
shifted by one spcae, and a lenient HTTP/1 server could possibly be
fooled into using a part of the method as the URI. For example, the
following request:
H2 request
:method: "GET /admin? HTTP/1.1"
:path: "/static/images"
would become:
GET /admin? HTTP/1.1 /static/images HTTP/1.1
It's important to note that the resulting request is *not* valid, and
that in order for this to be a problem, it requires that this request
is delivered to an already vulnerable HTTP/1 server.
A workaround here is to reject malformed methods by placing this rule
in the frontend or backend, at least before leaving haproxy in H1:
http-request reject if { method -m reg [^A-Z0-9] }
Alternately H2 may be globally disabled by commenting out the "alpn"
directive on "bind" lines, and by rejecting H2 streams creation by
adding the following statement to the global section:
tune.h2.max-concurrent-streams 0
This patch adds a check for each character of the method to make sure
they belong to the ones permitted in a token, as mentioned in RFC7231#4.1.
This should be backported to versions 2.0 and above. For older versions
not having HTX_FL_PARSING_ERROR, a "goto fail" works as well as it
results in a protocol error at the stream level. Non-HTX versions are
safe because the resulting invalid request will be rejected by the
internal HTTP/1 parser.
Thanks to Tim Düsterhus for reporting that one.
(cherry picked from commit
89265224d314a056d77d974284802c1b8a0dc97f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e9f1f1e7726084580b8eab5dfa1e07e202c376f9)
[wt: adapted since no meth_sl in 2.3]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 10 Aug 2021 14:30:55 +0000 (16:30 +0200)]
BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
Tim Düsterhus found that while the H2 path is checked for non-emptiness,
invalid chars and '*', a test is missing to verify that except for '*',
it always starts with exactly one '/'. During the reconstruction of the
full URI when passing to HTX, this missing test allows to affect the
apparent authority by appending a port number or a suffix name.
This only affects H2-to-H2 communications, as H2-to-H1 do not use the
full URI. Like for previous fix, the following rule inserted before
other ones in the frontend is sufficient to renormalize the internal
URI and let haproxy see the same authority as the target server:
http-request set-uri %[url]
This needs to be backported to 2.2. Earlier versions do not rebuild a
full URI using the authority and will fail on the malformed path at the
HTTP layer, so they are safe.
(cherry picked from commit
4b8852c70d8c4b7e225e24eb58258a15eb54c26e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
6c6b9a5f6de8f13a2242fedfb8c9eb1d5fe51db9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 10 Aug 2021 13:37:34 +0000 (15:37 +0200)]
BUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax
While we do explicitly check for strict character sets in the scheme,
this is only done when extracting URL components from an assembled one,
and we have special handling for "http" and "https" schemes directly in
the H2-to-HTX conversion. Sadly, this lets all other ones pass through
if they start exactly with "http://" or "https://", allowing the
reconstructed URI to start with a different looking authority if it was
part of the scheme.
It's interesting to note that in this case the valid authority is in
the Host header and that the request will only be wrong if emitted over
H2 on the backend side, since H1 will not emit an absolute URI by
default and will drop the scheme. So in essence, this is a variant of
the scheme-based attack described below in that it only affects H2-H2
and not H2-H1 forwarding:
https://portswigger.net/research/http2
As such, a simple workaround consists in just inserting the following
rule before other ones in the frontend, which will have for effect to
renormalize the authority in the request line according to the
concatenated version (making haproxy see the same authority and host
as what the target server will see):
http-request set-uri %[url]
This patch simply adds the missing syntax checks for non-http/https
schemes before the concatenation in the H2 code. An improvement may
consist in the future in splitting these ones apart in the start
line so that only the "url" sample fetch function requires to access
them together and that all other places continue to access them
separately. This will then allow the core code to perform such checks
itself.
The patch needs to be backported as far as 2.2. Before 2.2 the full
URI was not being reconstructed so the scheme and authority part were
always dropped from H2 requests to leave only origin requests. Note
for backporters: this depends on this previous patch:
MINOR: http: add a new function http_validate_scheme() to validate a scheme
Many thanks to Tim Düsterhus for figuring that one and providing a
reproducer.
(cherry picked from commit
a495e0d94876c9d39763db319f609351907a31e8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
9e0c2b5a418666d0de4cfc61b41c96d98add40f3)
[wt: no rfc8441 in 2.3]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Tue, 10 Aug 2021 13:35:36 +0000 (15:35 +0200)]
MINOR: http: add a new function http_validate_scheme() to validate a scheme
While http_parse_scheme() extracts a scheme from a URI by extracting
exactly the valid characters and stopping on delimiters, this new
function performs the same on a fixed-size string.
(cherry picked from commit
d3d8d03d98c6624c8b15b81f7b80dd22fc3f9fd2)
[wt: context adj]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a12054991079cfb6c0f8b7484a8b8209be785cd2)
[wt: context adj]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Jonathon Lacher [Wed, 4 Aug 2021 05:29:05 +0000 (00:29 -0500)]
DOC/MINOR: fix typo in management document
s/Not/Note.
(cherry picked from commit
c5b5e7b475a080efe38400013ea34f2e2439816c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
b6ed0cbefd4236b36c7b2604b1930fc780f13bcd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Emeric Brun [Fri, 13 Aug 2021 07:32:50 +0000 (09:32 +0200)]
BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config check
User reported that the config check returns an error with the message:
"Configuration file has no error but will not start (no listener) => exit(2)."
if the configuration present only a log-forward section with bind or dgram-bind
listeners but no listen/backend nor peer sections.
The process checked if there was 'peers' section avalaible with
an internal frontend (and so a listener) or a 'listen/backend'
section not disabled with at least one configured listener (into the
global proxies_list). Since the log-forward proxies appear in a
different list, they were not checked.
This patch adds a lookup on the 'log-forward' proxies list to check
if one of them presents a listener and is not disabled. And
this is done only if there was no available listener found into
'listen/backend' sections.
I have also studied how to re-work this check considering the 'listeners'
counter used after startup/init to keep the same algo and avoid further
mistakes but currently this counter seems increased during config parsing
and if a proxy is disabled, decreased during startup/init which is done
after the current config check. So the fix still not rely on this
counter.
This patch should fix the github issue #1346
This patch should be backported as far as 2.3 (so on branches
including the "log-forward" feature)
(cherry picked from commit
bc5c821cc259d343b6f1c8e457ead9518fd8198d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
3a9c000738e90f73e5479658dc3987483d8a0f1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Aug 2021 16:13:27 +0000 (18:13 +0200)]
BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are queued
It is the second part of the fix that should solve fairness issues with the
connections management inside the SPOE filter. Indeed, in multithreaded
mode, when the SPOE detects there are some connections in queue on a server,
it closes existing connections by releasing SPOE applets. It is mandatory
when a maxconn is set because few connections on a thread may prenvent new
connections establishment.
The first attempt to fix this bug (
9e647e5af "BUG/MEDIUM: spoe: Kill applets
if there are pending connections and nbthread > 1") introduced a bug. In
pipelining mode, SPOE applets might be closed while some frames are pending
for the ACK reply. To fix the bug, in the processing stage, if there are
some connections in queue, only truly idle applets may process pending
requests. In this case, only one request at a time is processed. And at the
end of the processing stage, only truly idle applets may be released. It is
an empirical workaround, but it should be good enough to solve contention
issues when a low maxconn is set.
This patch should partely fix the issue #1340. It must be backported as far
as 2.0.
(cherry picked from commit
d7da3dd928b4bc71c1c207908c455b81d2514676)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f028bb62471ffb3ea2976398c8086ca8ac4c0cb6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Thu, 12 Aug 2021 07:32:07 +0000 (09:32 +0200)]
DOC: config: Fix 'http-response send-spoe-group' documentation
Arguments were missing in the rule heading. This patch may be backported as
far as 2.0.
(cherry picked from commit
24e7f354e9ccf292eb4404e7e50b87b5b0a61ccd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
68fc3a1ad8ecfeb78938889c416deb8d6b2d4ff4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 11 Aug 2021 08:14:30 +0000 (10:14 +0200)]
DOC: Improve the lua documentation
The ReST syntax is used for note and warning blocks.
(cherry picked from commit
1e9b1b6d093a6b26c103541eb659cdc96260db35)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
fce629426819955a93402b2a43874b73fca1bbe6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 11 Aug 2021 13:46:29 +0000 (15:46 +0200)]
BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
In tcpcheck_eval_send(), the condition to detect there are still pending
data in the output buffer is buggy. Presence of raw data must be tested for
TCP connection only. But a condition on the connection was missing to be
sure it is not an HTX connection.
This patch must be backported as far as 2.2.
(cherry picked from commit
47bfd7b9b78c71ffa08d65f0cef475f5d2ae7b80)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
18280ca240afe28f79a1bbfa5c8f6949230bfe1c)
[wt: ctx adj: no trace in 2.3]
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Mon, 9 Aug 2021 17:37:16 +0000 (19:37 +0200)]
BUG/MINOR: buffer: fix buffer_dump() formatting
The formatting of the buffer_dump() output must be calculated using the
relative counter, not the absolute one, or everything will be broken if
the <from> variable is not a multiple of 16.
Could be backported in all maintained versions.
(cherry picked from commit
7e7765a4515bc23d536e2840c5bbdf18ffca6d45)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4d593e01961e6c28ea7cdd068f74807c28fd39e2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Aug 2021 15:53:56 +0000 (17:53 +0200)]
BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is released
On a thread, when the last SPOE applet is released, if there are still
pending streams, a new one is created. Of course, HAproxy must not be
stopping. It is important to start a new applet in this case to not abort
in-progress jobs, especially when a maxconn is set. Because applets may be
closed to be fair with connections waiting for a free slot.
This patch should partely fix the issue #1340. It depends on the commit
"MINOR: spoe: Create a SPOE applet if necessary when the last one on a
thread is closed". Both must be backported as far as 2.0.
(cherry picked from commit
6f1296b5c70d2f513836b61cfb9a06aed2db10eb)
[wt: previous patch to be picked is in fact "MINOR: spoe: Add a pointer
on the filter config in the spoe_agent structure"]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
4d350af9da9a417717468d49c45d5c59ff740c80)
[wt: context updates (LIST_DEL vs LISTçDELETE): reindent displaced block
instead]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 2 Aug 2021 15:51:01 +0000 (17:51 +0200)]
MINOR: spoe: Add a pointer on the filter config in the spoe_agent structure
There was no way to access the SPOE filter configuration from the agent
object. However it could be handy to have it. And in fact, this will be
required to fix a bug.
(cherry picked from commit
434b8525ee333f511b5461eae41b23842d3636d4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
541e261ce3c1af30e172ab97d8ce8e865a941e77)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Wed, 4 Aug 2021 17:35:13 +0000 (19:35 +0200)]
BUG/MINOR: server: update last_change on maint->ready transitions too
Nenad noticed that when leaving maintenance, the servers' last_change
field was not updated. This is visible in the Status column of the stats
page in front of the state, as the cumuled time spent in the current state
is wrong, it starts from the last transition (typically ready->maint). In
addition, the backend's state was not updated either, because the down
transition is performed by set_backend_down() which also emits a log, and
it is this function which was extended to update the backend's last_change,
but it's not called for down->up transitions so that was not done.
The most visible (and unpleasant) effect of this bug is that it affects
slowstart so such a server could immediately restart with a significant
load ratio.
This should likely be backported to all stable releases.
(cherry picked from commit
d332f1396b101f3f03e9fef405d42f03fc3647b6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
f79c4cf84967ffcc96e375c48afcb472ad246876)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 30 Jul 2021 12:18:49 +0000 (14:18 +0200)]
BUG/MINOR: pollers: always program an update for migrated FDs
If an MT-aware poller reports that a file descriptor was migrated, it
must stop reporting it. The simplest way to do this is to program an
update if not done yet. This will automatically mark the FD for update
on next round. Otherwise there's a risk that some events are reported
a bit too often and cause extra CPU usage with these pollers. Note
that epoll is currently OK regarding this. Select does not need this
because it uses a single shared events table, so in case of migration
no FD change is expected.
This should be backported as far as 2.2.
(cherry picked from commit
79e90b96158911535c22d1676f7ab8a0e3a9f7af)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
ea2036d40f103b6443d22224abbd6460e0e5401f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 30 Jul 2021 12:04:28 +0000 (14:04 +0200)]
BUG/MINOR: poll: fix abnormally high skip_fd counter
The skip_fd counter that is incremented when a migrated FD is reported
was abnormally high in with poll. The reason is that it was accounted
for before preparing the polled events instead of being measured from
the reported events.
This mistake was done when the counters were introduced in 1.9 with
commit
d80cb4ee1 ("MINOR: global: add some global activity counters to
help debugging"). It may be backported as far as 2.0.
(cherry picked from commit
177119bb110123b8b7510b93a77f07cc210af26c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
2565905728d65bc31b4be20e0629cb4c5299d9b8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 30 Jul 2021 11:55:36 +0000 (13:55 +0200)]
BUG/MINOR: select: fix excess number of dead/skip reported
In 1.8, commit
ab62f5195 ("MINOR: polling: Use fd_update_events to update
events seen for a fd") updated the pollers to rely on fd_update_events(),
but the modification delayed the test of presence of the FD in the report,
resulting in owner/thread_mask and possibly event updates being performed
for each FD appearing in a block of 32 FDs around an active one. This
caused the request rate to be ~3 times lower with select() than poll()
under 6 threads.
This can be backported as far as 1.8.
(cherry picked from commit
fcc5281513eabe0d7790d98e50ee8cd9be216c1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
cf3092fc2d863491f4ae8b115f679a234ead965f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Willy Tarreau [Fri, 30 Jul 2021 08:57:09 +0000 (10:57 +0200)]
BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
A bug was introduced in 2.1-dev2 by commit
305d5ab46 ("MAJOR: fd: Get
rid of the fd cache."). Pollers "poll" and "evport" had the sleeping
bit accidentally removed before the syscall instead of after. This
results in them not being woken up by inter-thread wakeups, which is
particularly visible with the multi-queue accept() and with queues.
As a work-around, when these pollers are used, "nbthread 1" should
be used.
The fact that it has remained broken for 2 years is a great indication
that threads are definitely not enabled outside of epoll and kqueue,
hence why this patch is only tagged medium.
This must be backported as far as 2.2.
(cherry picked from commit
c37ccd70b4f622a4148cb63d3b584357b02cda47)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
e148cb042ef64410c80b4d9e62165172c935bba3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Remi Tricot-Le Breton [Thu, 29 Jul 2021 07:45:48 +0000 (09:45 +0200)]
BUG/MINOR: connection: Add missing error labels to conn_err_code_str
The CO_ER_SSL_EARLY_FAILED and CO_ER_CIP_TIMEOUT connection error codes
were missing in the conn_err_code_str switch which converts the error
codes into string.
This patch can be backported on all stable branches.
(cherry picked from commit
0aa4130d6545ced3ad396f9a904fd6f579df291d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
abc2d3c4268d9a4fcf2393b2502f207f400ac29b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 26 Jul 2021 10:06:53 +0000 (12:06 +0200)]
BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
This part was fixed several times since commit
aade4edc1 ("BUG/MEDIUM:
mux-h2: Don't handle pending read0 too early on streams") and there are
still some cases where a read0 event may be ignored because a partial frame
inhibits the event.
Here, we must take care to set H2_CF_END_REACHED flag if a read0 was
received while a partial frame header is received or if the padding length
is missing.
To ease partial frame detection, H2_CF_DEM_SHORT_READ flag is introduced. It
is systematically removed when some data are received and is set when a
partial frame is found or when dbuf buffer is empty. At the end of the
demux, if the connection must be closed ASAP or if data are missing to move
forward, we may acknowledge the pending read0 event, if any. For now,
H2_CF_DEM_SHORT_READ is not part of H2_CF_DEM_BLOCK_ANY mask.
This patch should fix the issue #1328. It must be backported as far as 2.0.
(cherry picked from commit
b5f7b52968b617ce527f307089ec1c42ffeeab03)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
a9cc1e8b51400642450d7e47acd90dd01c01f903)
[wt: small context adjustments]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 26 Jul 2021 08:18:35 +0000 (10:18 +0200)]
BUG/MINOR: mux-h2: Obey dontlognull option during the preface
If a connection is closed during the preface while no data are received, if
the dontlognull option is set, no log message must be emitted. However, this
will still be handled as a protocol error. Only the log is omitted.
This patch should fix the issue #1336 for H2 sessions. It must be backported
to 2.4 and 2.3 at least, and probably as far as 2.0.
(cherry picked from commit
3f35da296ee30bba3783f343a586091c66f21c65)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
79b347d50f13d3a918871f82035ed7de8f08dd50)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Mon, 26 Jul 2021 09:03:54 +0000 (11:03 +0200)]
BUG/MINOR: systemd: must check the configuration using -Ws
When doing a reload with a configuration which requires the
master-worker mode, the configuration check will fail because the check
is not done with -W/-Ws.
Example:
wla@kikyo:~/haproxy$ ./haproxy -Ws -c -f haproxy.cfg
Configuration file is valid
wla@kikyo:~/haproxy$ ./haproxy -c -f haproxy.cfg
[NOTICE] (13153) : haproxy version is 2.5-dev2-4567b3-16
[NOTICE] (13153) : path to executable is ./haproxy
[ALERT] (13153) : config : Can't use a 'program' section without master worker mode.
[ALERT] (13153) : config : Fatal errors found in configuration.
This patch fixes the issue by adding -Ws on the check command line.
Must be backported in all stable branches. (The file was previously in
contrib/systemd/haproxy.service.in).
(cherry picked from commit
9def1425ce5c8c1db6589795cb171986d2a028a3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
dd8bec37c66dd5394bc12ee221ce61ff37f6a1e7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 22 Jul 2021 12:29:26 +0000 (14:29 +0200)]
BUG/MINOR: resolvers: Use a null-terminated string to lookup in servers tree
When we evaluate a DNS response item, it may be necessary to look for a
server with a hostname matching the item target into the named servers
tree. To do so, the item target is transformed to a lowercase string. It
must be a null-terminated string. Thus we must explicitly set the trailing
'\0' character.
For a specific resolution, the named servers tree contains all servers using
this resolution with a hostname loaded from a state file. Because of this
bug, same entry may be duplicated because we are unable to find the right
server, assigning this way the item to a free server slot.
This patch should fix the issue #1333. It must be backported as far as 2.2.
(cherry picked from commit
1f923391d1841435de7891682f585c040faef2bc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
c20d64256cc3c02a012709ce3bb398356fff97ad)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 22 Jul 2021 09:06:41 +0000 (11:06 +0200)]
BUG/MINOR: check: fix the condition to validate a port-less server
A config like the below fails to validate because of a bogus test:
backend b1
tcp-check connect port 1234
option tcp-check
server s1 1.2.3.4 check
[ALERT] (18887) : config : config: proxy 'b1': server 's1' has neither
service port nor check port, and a tcp_check rule
'connect' with no port information.
A || instead of a && only validates the connect rule when both the
address *and* the port are set. A work around is to set the rule like
this:
tcp-check connect addr 0:1234 port 1234
This needs to be backported as far as 2.2 (2.0 is OK).
(cherry picked from commit
acff30975341a907784e489e3a7e384f3550211e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
565f333088e59a3a45509237cdcdc3e5ae3602ea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Wed, 21 Jul 2021 09:50:12 +0000 (11:50 +0200)]
BUG/MEDIUM: ssl_sample: fix segfault for srv samples on invalid request
Some ssl samples cause a segfault when the stream is not instantiated,
for example during an invalid HTTP request. A new check is added to
prevent the stream dereferencing if NULL.
This is the list of the affected samples :
- ssl_s_chain_der
- ssl_s_der
- ssl_s_i_dn
- ssl_s_key_alg
- ssl_s_notafter
- ssl_s_notbefore
- ssl_s_s_dn
- ssl_s_serial
- ssl_s_sha1
- ssl_s_sig_alg
- ssl_s_version
This bug can be reproduced easily by using one of these samples in a
log-format string. Emit an invalid HTTP request with an HTTP client to
trigger the crash.
This bug has been reported in redmine issue 3913.
This must be backported up to 2.2.
(cherry picked from commit
5fcd428c35c45f7222b9aade0c0484fcdb558de9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
73617f937e001514305a3ee5a259bf45a3f0961b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 21 Jul 2021 08:17:02 +0000 (10:17 +0200)]
BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
This undocumented variable is only for internal use, and its sole
presence affects the process' behavior, as shown in bug #1324. It must
not be exported to workers, external checks, nor programs. Let's unset
it before forking programs and workers.
This should be backported as far as 1.8. The worker code might differ
a bit before 2.5 due to the recent removal of multi-process support.
(cherry picked from commit
3c032f2d4d3ba508d508ae70603ccac6d5cf9d4a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
a9274a17f6466b82359a5b6954bb3a35bce814f3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 21 Jul 2021 08:01:36 +0000 (10:01 +0200)]
BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
The master-worker code registers an exit handler to deal with configuration
issues during reload, leading to a restart of the master process in wait
mode. But it shouldn't do that when it's expected that the program stops
during config parsing or condition checks, as the reload operation is
unexpectedly called and results in abnormal behavior and even crashes:
$ HAPROXY_MWORKER_REEXEC=1 ./haproxy -W -c -f /dev/null
Configuration file is valid
[NOTICE] (18418) : haproxy version is 2.5-dev2-ee2420-6
[NOTICE] (18418) : path to executable is ./haproxy
[WARNING] (18418) : config : Reexecuting Master process in waitpid mode
Segmentation fault
$ HAPROXY_MWORKER_REEXEC=1 ./haproxy -W -cc 1
[NOTICE] (18412) : haproxy version is 2.5-dev2-ee2420-6
[NOTICE] (18412) : path to executable is ./haproxy
[WARNING] (18412) : config : Reexecuting Master process in waitpid mode
[WARNING] (18412) : config : Reexecuting Master process
Note that the presence of this variable happens by accident when haproxy
is called from within its own programs (see issue #1324), but this should
be the object of a separate fix.
This patch fixes this by preventing the atexit registration in such
situations. This should be backported as far as 1.8. MODE_CHECK_CONDITION
has to be dropped for versions prior to 2.5.
(cherry picked from commit
26146194d3c2945ad3d692fbcddaf8d5525117a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
7a534ebcef5531152691c4efbd9e434ba196e924)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 14 Jul 2021 17:41:25 +0000 (19:41 +0200)]
BUILD: lua: silence a build warning with TCC
TCC doesn't have the equivalent of __builtin_unreachable() and complains
that hlua_panic_ljmp() may return no value. Let's add a return 0 there.
All compilers that know that longjmp() doesn't return will see no change
and tcc will be happy.
(cherry picked from commit
6a510907807b7fb901654b4f5e5100aa91868fb7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
69c66e3584ac81b70dd445ff56f8c9815de8f28c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Wed, 14 Jul 2021 15:54:01 +0000 (17:54 +0200)]
BUILD: add detection of missing important CFLAGS
Modern compilers love to break existing code, and some options detected
at build time (such as -fwrapv) are absolutely critical otherwise some
bad code can be generated.
Given that some users rely on packages that force CFLAGS without being
aware of this and can be hit by runtime bugs, we have to help packagers
figure that they need to be careful about their build options.
The test here consists in detecting correct wrapping of signed integers.
Some of the old code relies on it, and modern compilers recently decided
to break it. It's normally addressed using -fwrapv which users will
rarely enforce in their own flags. Thus it is a good indicator of missing
critical CFLAGS, and it happens to be very easy to detect at run time.
Note that the test uses argc in order to have a variable. While gcc
ignores wrapping even for constants, clang only ignores it for variables.
The way the code is constructed doesn't result in code being emitted for
optimized builds thanks to value range propagation.
This should address GitHub issue #1315, and should be backported to all
stable versions. It may result in instantly breaking binaries that seemed
to work fine (typically the ones suddenly showing a busy loop after a few
weeks of uptime), and require packagers to fix their flags. The vast
majority of distro packages are fine and will not be affected though.
(cherry picked from commit
1335da38f4e1d73df4e7d5fb1e98846e34c9367d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
08293ed8f964cd2dc3faafbe81562b1eb59187d1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Thu, 8 Jul 2021 15:30:35 +0000 (17:30 +0200)]
[RELEASE] Released version 2.3.12
Released version 2.3.12 with the following main changes :
- BUG/MAJOR: pools: fix incomplete backport of lockless pool fix
- BUG/MAJOR: pools: second fix for incomplete backport of lockless pool fix
Willy Tarreau [Thu, 8 Jul 2021 15:05:10 +0000 (17:05 +0200)]
BUG/MAJOR: pools: second fix for incomplete backport of lockless pool fix
Commit
bc76411e0 ("BUG/MAJOR: pools: fix possible race with free() in
the lockless variant") was missing another unprotected access to the
pool's free_list in __pool_refill_alloc() because this one was completely
dropped from 2.4 and above. There we need to loop over POOL_BUSY just like
in the __pool_free() code, otherwise we risk to insert such a POOL_BUSY
into the list.
This fix is only for 2.3 and 2.2 since 2.2 now also contains the faulty
backport of the patch above.
Willy Tarreau [Thu, 8 Jul 2021 13:23:46 +0000 (15:23 +0200)]
BUG/MAJOR: pools: fix incomplete backport of lockless pool fix
Commit
bc76411e0 ("BUG/MAJOR: pools: fix possible race with free() in
the lockless variant") wasn't complete. The __pool_free() function also
needed a part of the lockless variant of the code that was in
pool_put_to_shared_cache() in 2.4. Without it, a __pool_free() during
a contented pool_alloc() may catch free_list == POOL_BUSY and copy it
into the updated free list, so that the next call to __pool_get_first()
loops forever and gets killed by the watchdog.
Sadly it requires load under contention on a non-glibc system to notice
it :-/
This fix is only for 2.3 and 2.2 since 2.2 now also contains the faulty
backport of the patch above.
Christopher Faulet [Wed, 7 Jul 2021 16:26:59 +0000 (18:26 +0200)]
[RELEASE] Released version 2.3.11
Released version 2.3.11 with the following main changes :
- BUG/MINOR: mux-fcgi: Don't send normalized uri to FCGI application
- BUG/MINOR: htx: Preserve HTX flags when draining data from an HTX message
- BUG/MINOR: applet: Notify the other side if data were consumed by an applet
- BUG/MEDIUM: peers: initialize resync timer to get an initial full resync
- BUG/MEDIUM: peers: register last acked value as origin receiving a resync req
- BUG/MEDIUM: peers: stop considering ack messages teaching a full resync
- BUG/MEDIUM: peers: reset starting point if peers appears longly disconnected
- BUG/MEDIUM: peers: reset commitupdate value in new conns
- BUG/MEDIUM: peers: re-work updates lookup during the sync on the fly
- BUG/MEDIUM: peers: reset tables stage flags stages on new conns
- MINOR: peers: add informative flags about resync process for debugging
- REGTESTS: add minimal CLI "add map" tests
- BUG/MINOR: hlua: Don't rely on top of the stack when using Lua buffers
- BUG/MEDIUM: cli: prevent memory leak on write errors
- BUG/MINOR: ssl/cli: fix a lock leak when no memory available
- MINOR: compat: automatically include malloc.h on glibc
- MEDIUM: pools: call malloc_trim() from pool_gc()
- MINOR: pools/debug: slightly relax DEBUG_DONT_SHARE_POOLS
- MINOR: debug: add a new "debug dev sym" command in expert mode
- BUG/MEDIUM: dns: reset file descriptor if send returns an error
- BUG/MINOR: stream: Decrement server current session counter on L7 retry
- BUG/MINOR: stream: properly clear the previous error mask on L7 retries
- BUG/MINOR: stream: Reset stream final state and si error type on L7 retry
- BUG/MINOR: checks: Handle synchronous connect when a tcpcheck is started
- BUG/MINOR: checks: Reschedule check on observe mode only if fastinter is set
- BUG/MINOR: http_fetch: fix possible uninit sockaddr in fetch_url_ip/port
- MINOR: channel: Rely on HTX version if appropriate in channel_may_recv()
- BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn may receive
- MINOR: conn-stream: Force mux to wait for read events if abortonclose is set
- MEDIUM: mux-h1: Don't block reads when waiting for the other side
- BUG/MEDIUM: mux-h1: Properly report client close if abortonclose option is set
- REGTESTS: Add script to test abortonclose option
- BUG/MEDIUM: ebtree: Invalid read when looking for dup entry
- BUG/MAJOR: server: prevent deadlock when using 'set maxconn server'
- BUG/MEDIUM: filters: Exec pre/post analysers only one time per filter
- BUG/MINOR: http-comp: Preserve HTTP_MSGF_COMPRESSIONG flag on the response
- BUG/MINOR: http-ana: Handle L7 retries on refused early data before K/A aborts
- BUG/MINOR: server: Missing calloc return value check in srv_parse_source
- BUG/MINOR: peers: Missing calloc return value check in peers_register_table
- BUG/MINOR: ssl: Missing calloc return value check in ssl_init_single_engine
- BUG/MINOR: http: Missing calloc return value check in parse_http_req_capture
- BUG/MINOR: proxy: Missing calloc return value check in proxy_parse_declare
- BUG/MINOR: proxy: Missing calloc return value check in proxy_defproxy_cpy
- BUG/MINOR: http: Missing calloc return value check while parsing tcp-request/tcp-response
- BUG/MINOR: http: Missing calloc return value check while parsing tcp-request rule
- BUG/MINOR: compression: Missing calloc return value check in comp_append_type/algo
- BUG/MINOR: worker: Missing calloc return value check in mworker_env_to_proc_list
- BUG/MINOR: http: Missing calloc return value check while parsing redirect rule
- BUG/MINOR: http: Missing calloc return value check in make_arg_list
- BUG/MINOR: proxy: Missing calloc return value check in chash_init_server_tree
- BUG/MINOR: lua/vars: prevent get_var() from allocating a new name
- DOC/MINOR: move uuid in the configuration to the right alphabetical order
- BUG/MAJOR: stream-int: Release SI endpoint on server side ASAP on retry
- DOC: use the req.ssl_sni in examples
- BUILD: make tune.ssl.keylog available again
- BUG/MINOR: ssl: OCSP stapling does not work if expire too far in the future
- BUG/MEDIUM: compression: Add a flag to know the filter is still processing data
- BUG/MINOR: pools: fix a possible memory leak in the lockless pool_flush()
- MINOR: pools: do not maintain the lock during pool_flush()
- MINOR: pools: call malloc_trim() under thread isolation
- MEDIUM: pools: use a single pool_gc() function for locked and lockless
- BUG/MAJOR: pools: fix possible race with free() in the lockless variant
- CLEANUP: pools: remove now unused seq and pool_free_list
- BUG/MAJOR: htx: Fix htx_defrag() when an HTX block is expanded
- BUG/MINOR: mux-fcgi: Expose SERVER_SOFTWARE parameter by default
- DOC: lua: Add a warning about buffers modification in HTTP
- BUG/MINOR: stick-table: insert srv in used_name tree even with fixed id
- BUG/MEDIUM: shctx: use at least thread-based locking on USE_PRIVATE_CACHE
- BUG/MINOR: ssl: use atomic ops to update global shctx stats
- BUG/MINOR: mworker: fix typo in chroot error message
- BUG/MAJOR: queue: set SF_ASSIGNED when setting strm->target on dequeue
- MINOR: backend: only skip LB when there are actual connections
- BUG/MINOR: stats: make "show stat typed desc" work again
- MINOR: mux-h2: obey http-ignore-probes during the preface
- BUG/MEDIUM: dns: send messages on closed/reused fd if fd was detected broken
- BUILD: cfgparse-ssl: Remove const from defpx param in keylog parsing function
- BUG/MINOR: resolvers: answser item list was randomly purged or errors
- MEDIUM: resolvers: add a ref on server to the used A/AAAA answer item
- MEDIUM: resolvers: add a ref between servers and srv request or used SRV record
- BUG/MAJOR: resolvers: segfault using server template without SRV RECORDs
- BUG/MEDIUM: server/cli: Fix ABBA deadlock when fqdn is set from the CLI
- BUG/MINOR: server/cli: Fix locking in function processing "set server" command
- BUG/MAJOR: server: fix deadlock when changing maxconn via agent-check
- REGTESTS: fix maxconn update with agent-check
- MINOR: tcp-act: Add set-src/set-src-port for "tcp-request content" rules
- DOC: config: Add missing actions in "tcp-request session" documentation
- BUG/MINOR: checks: return correct error code for srv_parse_agent_check
- BUG/MINOR: tcpcheck: Fix numbering of implicit HTTP send/expect rules
- BUG/MEDIUM: sock: make sure to never miss early connection failures
- BUG/MINOR: cli: fix server name output in "show fd"
- BUG/MINOR: stick-table: fix several printf sign errors dumping tables
- DOC: stick-table: add missing documentation about gpt0 stored type
- DOC: peers: fix the protocol tag name in the doc
- DOC: config: use CREATE USER for mysql-check
- BUG/MINOR: server-state: load SRV resolution only if params match the config
- BUG/MINOR: server: Forbid to set fqdn on the CLI if SRV resolution is enabled
- MINOR: resolvers: Clean server in a dedicated function when removing a SRV item
- MINOR: resolvers: Remove server from named_servers tree when removing a SRV item
- BUG/MEDIUM: resolvers: Add a task on servers to check SRV resolution status
- BUG/MINOR: resolvers: Use resolver's lock in resolv_srvrq_expire_task()
- BUG/MINOR: resolvers: Always attach server on matching record on resolution
- BUG/MINOR: resolvers: Reset server IP when no ip is found in the response
- MINOR: resolvers: Reset server IP on error in resolv_get_ip_from_response()
- BUG/MEDIUM: resolvers: Make 1st server of a template take part to SRV resolution
- BUG/MINOR: peers: fix data_type bit computation more than 32 data_types
- Revert "MINOR: tcp-act: Add set-src/set-src-port for "tcp-request content" rules"
Christopher Faulet [Tue, 6 Jul 2021 09:25:36 +0000 (11:25 +0200)]
Revert "MINOR: tcp-act: Add set-src/set-src-port for "tcp-request content" rules"
This reverts commit
19bbbe05629ea947dd60d5b96d96f0066b047b97.
For now, set-src/set-src-port actions are directly performed on the client
connection. Using these actions at the stream level is really a problem with
HTTP connection (See #90) because all requests are affected by this change
and not only the current request. And it is worse with the H2, because
several requests can set their source address into the same connection at
the same time.
It is already an issue when these actions are called from "http-request"
rules. It is safer to wait a bit before adding the support to "tcp-request
content" rules. The solution is to be able to set src/dst address on the
stream and not on the connection when the action if performed from the L7
level..
Reverting the above commit means the issue #1303 is no longer fixed.
This patch must be backported in all branches containing the above commit
(as far as 2.0 for now).
(cherry picked from commit
23048875a4eacf5d7d4450d677cb077e67778b95)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit
390f49477159de53d0506cd52bd6ed323febde0a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Emeric Brun [Thu, 1 Jul 2021 16:54:05 +0000 (18:54 +0200)]
BUG/MINOR: peers: fix data_type bit computation more than 32 data_types
This patch fixes the computation of the bit of the current data_type
in some part of code of peer protocol where the computation is limited
to 32bits whereas the bitfield of data_types can support 64bits.
Without this patch it could result in bugs when we will define more
than 32 data_types.
Backport is useless because there is currently less than 32 data_types
(cherry picked from commit
08b0f6780c45099b8d03bfd9e398d3f51519e667)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit
45f12b06b491b433fdb951ba851284e46901a917)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>