Commit Graph

18475 Commits

Author SHA1 Message Date
Willy Tarreau
398eea103e DOC: config: fix wrong section number for "protocol prefixes"
The socket type prefixes used to reference section "11.5.3" instead of
"11.3" for "protocol prefixes".

(cherry picked from commit d4c6fbe87e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 41664236b7b8d1cc8ca146df0b4b8f41a362406f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:36:59 +01:00
Willy Tarreau
dad6068c35 BUG/MINOR: listeners: fix suspend/resume of inherited FDs
FDs inherited from a parent process do not deal well with suspend/resume
since commit 59b5da487 ("BUG/MEDIUM: listener: never suspend inherited
sockets") introduced in 2.3. The problem is that we now report that they
cannot be suspended at all, and they return a failure. As such, if a new
process fails to bind and sends SIGTTOU to the previous process, that
one will notice the failure and instantly switch to soft-stop, leaving
no chance to the new process to give up later and signal its failure.

What we need to do, however, is to stop receiving new connections from
such inherited FDs, which just means that the FD must be unsubscribed
from the poller (and resubscribed later if finally it has to stay).
With this a new process can start on the already bound FD without
problem thanks to the absence of polling, and when the old process
stops the new process will be alone on it.

This may be backported as far as 2.4.

(cherry picked from commit 64763342aa)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 0f6663b9acce9a9fbeafbac3a56f17fbab0b741c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:36:45 +01:00
Willy Tarreau
b2817a3bdc BUG/MINOR: http-ana: make set-status also update txn->status
Patrick Hemmer reported an interesting case where the status present in
the logs doesn't reflect what was reported to the user. During analysis
we could figure that it was in fact solely caused by the code dealing
with the set-status action. Indeed, set-status does update the status
in the HTX message itself but not in the HTTP transaction. However, at
most places where the status is needed to take a decision, it is
retrieved from the transaction, and the logs proceed like this as well,
though the "status" sample fetch function does retrieve it from the HTX
data. This particularly means that once a set-status has been used to
modify the status returned to the user, logs do not match that status,
and the response code distribution doesn't match either. However a
subsequent rule using the status as a condition will still match because
the "status" sample fetch function does also extract the status from the
HTX stream. Here's an example that fails:

  frontend f
    bind :8001
    mode http
    option httplog
    log stdout daemon
    http-after-response set-status 400

This will return a 400 to the client but log a 503 and increment
http_rsp_5xx.

In the end the root cause is that we need to make txn->status the only
authoritative place to get the status, and as such it must be updated
by the set-status rule. Ideally "status" should just use txn->status
but with the two synchronized this way it's not needed.

This should be backported since it addresses some consistency issues
between logs and what's observed. The set-status action appeared in
1.9 so all stable versions are eligible.

(cherry picked from commit 640e253698)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 87c1d795d7e9299d374a7ef9c6beda5e3bbea354)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:36:39 +01:00
Christopher Faulet
365cd65b68 BUG/MINOR: http-fetch: Don't block HTTP sample fetch eval in HTTP_MSG_ERROR state
It was inherited from the legacy HTTP mode, but the message parsing is
handled by the underlying mux now. Thus, if a message is in HTTP_MSG_ERROR
state, it is just an analysis error and not a parsing error. So there is no
reason to block the HTTP sample fetch evaluation in this case.

This patch could be backported in all stable versions (For the 2.0, only the
htx part must be updated).

(cherry picked from commit 5aab0a30c5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6efffd09cff6da32986eef24711ab7e23204c59c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:36:14 +01:00
Christopher Faulet
751e1ec219 BUG/MINOR: http-ana: Report SF_FINST_R flag on error waiting the request body
When we wait for the request body, we are still in the request analysis. So
a SF_FINST_R flag must be reported in logs. Even if some data are already
received, at this staged, nothing is sent to the server.

This patch could be backported in all stable versions.

(cherry picked from commit f4569bbcc1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ec4df6f9668b601e2775b28d6b1250ac37a5f26c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:36:00 +01:00
Christopher Faulet
1101a0504f BUG/MINOR: promex: Don't forget to consume the request on error
When the promex applet triggers an error, for instance because the URI is
invalid, we must still take care to consume the request. Otherwise, the
error will be handled by HTTP analyzers as a server abort.

This patch must be backported as far as 2.0.

(cherry picked from commit c1b013bc61)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f40463aea88b1af63f2967ae311546af70fd72e8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:55 +01:00
Willy Tarreau
8910e078f1 BUG/MEDIUM: peers: make "show peers" more careful about partial initialization
Since 2.6 with commit 34e4085f8 ("MEDIUM: peers: Balance applets across
threads") the initialization of a peers appctx may be postponed with a
wakeup, causing some partially initialized appctx to be visible. The
"show peers" command used to only care about peers without appctx, but
now it must also take care of those with no stconn, otherwise it can
occasionally crash while dumping them.

This fix must be backported to 2.6. Thanks to Patrick Hemmer for
reporting the problem.

(cherry picked from commit 03926129b0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 546bda683e042924c10f6d870fd42aa6580cd6aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:51 +01:00
Christopher Faulet
ba582b85a5 BUG/MINOR: resolvers: Wait the resolution execution for a do_resolv action
The do_resolv action triggers a resolution and must wait for the
result. Concretely, if no cache entry is available, it creates a resolution
and wakes up the resolvers task. Then it yields. When the action is
recalled, if the resolution is still running, it yields again.

However, if the resolution is not running, it does not check it was
running. Thus, it is possible to ignore the resolution because the action
was recalled before the resolvers task had a chance to be executed. If there
is result, the action must yield.

This patch should fix the issue #1993. It must be backported as far as 2.0.

(cherry picked from commit 51dbb4cb79)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4283282a61370c3ae406aa7a9e300c0ed1116f4c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:40 +01:00
Christopher Faulet
c79c7f4912 BUG/MINOR: hlua: Fix Channel.line and Channel.data behavior regarding the doc
These both functions are buggy and don't respect the documentation. They
must wait for more data, if possible.

For Channel.data(), it must happen if not enough data was received orf if no
length was specified and no data was received. The first case is properly
handled but not the second one. An empty string is return instead. In
addition, if there is no data and the channel can't receive more data, 'nil'
value must be returned.

In the same spirit, for Channel.line(), we must try to wait for more data
when no line is found if not enough data was received or if no length was
specified. Here again, only the first case is properly handled. And for this
function too, 'nil' value must be returned if there is no data and the
channel can't receive more data.

This patch is related to the issue #1993. It must be backported as far as
2.5.

(cherry picked from commit 0ae2e63d85)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 08554a4669b92f4d6c988a7dd4fd318c9e8ec903)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:36 +01:00
Christopher Faulet
48bb2f41aa BUG/MINOR: h1-htx: Remove flags about protocol upgrade on non-101 responses
It is possible to have an "upgrade:" header and the corresponding value in
the "connection:" header for a non-101 response. It happens for
426-Upgrade-Required messages. However, on HAProxy side, a parsing error is
reported for this kind of message because no websocket key header
("sec-websocket-accept:") is found in the response.

So a possible fix could be to not perform this test for non-101
responses. However, having flags about protocol upgrade on this kind of
response could lead to other bugs. Instead, corresponding flags are
removed. Thus, during the H1 response post-parsing, H1_MF_CONN_UPG and
H1_MF_UPG_WEBSOCKET flags are removed from any non-101 response.

This patch should fix the issue #1997. It must be backported as far as 2.4.

(cherry picked from commit 5f36bfe42e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b4784965cbcc2b914477a8a47be52cbf6803b62d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:29 +01:00
Amaury Denoyelle
8260e7902b MINOR: mux-quic: add traces for flow-control limit reach
Add new traces when QUIC flow-control limits are reached at stream or
connection level. This may help to explain an interrupted transfer.

This should be backported up to 2.6.

(cherry picked from commit 31d2057c59)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7e32f314bcd3e101e59fd3c32d77d6b31e9dd578)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:14 +01:00
Amaury Denoyelle
61aea88729 BUG/MINOR: mux-quic: fix transfer of empty HTTP response
QUIC stream did not transferred its response if it was an empty HTTP
response without headers nor entity body. This is caused by an
incomplete condition on qc_send() which skips streams with empty
<tx.buf>.

Fix this by extending the condition. Sending will be conducted on a
stream if <tx.buf> is not empty or FIN notification must be provided.
This allows to send the last STREAM frame for this stream.

Such HTTP responses should be extremely rare so this bug is labelled as
MINOR. It was encountered with a HTTP/0.9 request on an empty payload.
The bug was triggered as HTTP/0.9 does not support header in response
message.

Also, note that condition to wakeup MUX tasklet has been changed
similarly in qc_send_buf(). It is not mandatory to work properly
however, most probably because another tasklet_wakeup() is done
before/after.

This should be backported up to 2.6.

(cherry picked from commit ab6cdecd71)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f85eae7f4c30d896537d46c2691c790f0f0c0efc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:11 +01:00
William Lallemand
83d0d0f425 DOC: management: add details about @system-ca in "show ssl ca-file"
Explain why @system-ca is seen in "show ssl ca-file".

Should fix issue #1979.

Can be backported till 2.6.

(cherry picked from commit f29c4155a8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f51c1076f811f3968d2b05a4751d5dafd4d6d61b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:06 +01:00
William Lallemand
54727f77aa DOC: management: add details on "Used" status
Add details on the "Used" status of the "show crl/ca-file/cert" CLI
command.

Could be backported in every branch till 2.5.

Should fix issue #1979.

(cherry picked from commit 0c39526dab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 741bc5cac9f36c6ac74a8b6248ba1217a26cbf55)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:35:00 +01:00
Manu Nicolas
8012c2c812 CLEANUP: htx: fix a typo in an error message of http_str_to_htx
This fixes a typo in an error message about headers in the
http_str_to_htx function.

(cherry picked from commit 45b6b23335)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit bfddb2aae3841fd27cbe999d247e05ddf6465684)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:44 +01:00
Remi Tricot-Le Breton
23db8c664b BUG/MINOR: http: Memory leak of http redirect rules' format string
When the configuration contains such a line:
    http-request redirect location /
a "struct logformat_node" object is created and it contains an "arg"
member which gets alloc'ed as well in which we copy the new location
(see add_to_logformat_list). This internal arg pointer was not freed in
the dedicated release_http_redir release function.
Likewise, the expression pointer was not released as well.

This patch can be backported to all stable branches. It should apply
as-is all the way to 2.2 but it won't on 2.0 because release_http_redir
did not exist yet.

(cherry picked from commit 3120284c29)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit fe5f84ae3eddbfbb34b1fd9b08b50c6246358279)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:34 +01:00
Aurelien DARRAGON
0c34554950 REGTEST: fix the race conditions in hmac.vtc
A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid any "HTTP header incomplete" errors.

(cherry picked from commit 7956aa14d3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 1b767f6b82388e90c9cfe2098a87461a1206c73c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:22 +01:00
Aurelien DARRAGON
387aa39763 REGTEST: fix the race conditions in digest.vtc
A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid any "HTTP header incomplete" errors.

(cherry picked from commit 1858c244c4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit d40737e2cf0ddeac4711bf27d798714b573f2605)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:18 +01:00
Aurelien DARRAGON
7866b5ca33 REGTEST: fix the race conditions in add_item.vtc
A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid any "HTTP header incomplete" errors.

(cherry picked from commit 63762b05b0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 924f2406a9663ec8dd3de377abc19e7844b19c5c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:15 +01:00
Aurelien DARRAGON
77bc9f1f24 REGTEST: fix the race conditions in json_query.vtc
A "Connection: close" header is added to responses to avoid any connection
reuse. This should avoid any "HTTP header incomplete" errors.

(cherry picked from commit d4140a79c5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 35195396dad3af26f8e42126a0c5a789669c498e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:12 +01:00
Aurelien DARRAGON
3fcf827712 BUG/MINOR: proxy: free orgto_hdr_name in free_proxy()
Unlike fwdfor_hdr_name, orgto_hdr_name was not properly freed in
free_proxy().

This did not cause observable memory leaks because originalto proxy option is
only used for user configurable proxies, which are solely freed right before
process termination.
No backport needed unless some architectural changes causing regular proxies
to be freed and reused multiple times within single process lifetime are made.

(cherry picked from commit b2d797a53f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 545f8a12c46174f50ddf6dfe0c8b1b3f379b12c2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:34:05 +01:00
Christopher Faulet
ff7ada7d7a DOC: config: remove duplicated "http-response sc-set-gpt0" directive
This directive was erroneously duplicated.

This patch could be backported as far as 2.5.

(cherry picked from commit 39055d159f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit aac8110c4465a4d1aa5cb8c992fd6ce8f8bd3aa2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:33:42 +01:00
Christopher Faulet
32616807a9 DOC: config: fix alphabetical ordering of http-after-response rules
The 'capture' action must be placed after the 'allow' action.

This patch could be backported as far as 2.5.

(cherry picked from commit d9d36b8b6b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 47192fe67fb71fd27ef4f9c9d3427aa706462051)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:33:38 +01:00
Christopher Faulet
d02e1170f8 BUG/MAJOR: buf: Fix copy of wrapping output data when a buffer is realigned
There is a bug in b_slow_realign() function when wrapping output data are
copied in the swap buffer. block1 and block2 sizes are inverted. Thus blocks
with a wrong size are copied. It leads to data mixin if the first block is
in reality larger than the second one or to a copy of data outside the
buffer is the first block is smaller than the second one.

The bug was introduced when the buffer API was refactored in 1.9. It was
found by a code review and seems never to have been triggered in almost 5
years. However, we cannot exclude it is responsible of some unresolved bugs.

This patch should fix issue #1978. It must be backported as far as 2.0.

(cherry picked from commit 61aded057d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4a048c13f5ec3bcd060c8af955fe51694400b69d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:33:34 +01:00
Christopher Faulet
5df4953f8d BUG/MINOR: http-fetch: Only fill txn status during prefetch if not already set
When an HTTP sample fetch is evaluated, a prefetch is performed to check the
channel contains a valid HTTP message. If the HTTP analysis was not already
started, some info are filled.

It may be an issue when an error is returned before the response analysis
and when http-after-response rules are used because the original HTTP txn
status may be crushed. For instance, with the following configuration:

  listen l1
    log global
    mode http
    bind :8000

    log-format ST=%ST
    http-after-response set-status 400
    #http-after-response set-var(res.foo) status

A "ST=503" is reported in the log messages, independantly on the first
http-after-response rule. The same must happen if the second rule is
uncommented. However, for now, a "ST=400" is logged.

To fix the bug, during the prefetch, the HTTP txn status is only set if it
is undefined (-1). This way, we are sure the original one is never lost.

This patch should be backported as far as 2.2.

(cherry picked from commit 31850b470a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a9f36628395b012cef7f9efddaf90954b68ff167)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:33:28 +01:00
Willy Tarreau
d5036411eb BUG/MINOR: stick-table: report the correct action name in error message
sc-inc-gpc() learned to use arrays in 2.5 with commit 4d7ada8f9 ("MEDIUM:
stick-table: add the new arrays of gpc and gpc_rate"), but the error
message says "sc-set-gpc" instead of "sc-inc-gpc". Let's fix this to
avoid confusion.

This can be backported to 2.5.

(cherry picked from commit 20391519c3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 59bf319279b1457c6f01b160764e79c27a5808c9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:31:40 +01:00
Willy Tarreau
039e90dcc6 BUILD: makefile: sort the features list
The features list that appears in -vv appears in a random order, which
always makes it a pain to look for certain features. Let's sort it.

(cherry picked from commit 848362f2d2)
[wt: applied to Makefile]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a99f189732d6f2c1c2e99cf39d4b3a17b4e040c8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:31:00 +01:00
Willy Tarreau
161ba95c0f BUILD: makefile: build the features list dynamically
The BUILD_FEATURES string was created too early to inherit implicit
additions. This could make the features list report that some features
were disabled while they had later been enabled. Better make it a macro
that is interpreted where needed based on the current state of each
option.

(cherry picked from commit 39d6c34837)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 650959acbecc2a607629fd905a39e0689a02ec92)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:30:55 +01:00
Amaury Denoyelle
ba2817bba8 BUG/MINOR: mux-quic: ignore remote unidirectional stream close
Remove ABORT_NOW() on remote unidirectional stream closure. This is
required to ensure our implementation is evolutive enough to not fail on
unknown stream type.

Note that for the moment MAX_STREAMS_UNI flow-control frame is never
emitted. This should be unnecessary for HTTP/3 which have a limited
usage of unidirectional streams but may be required if other application
protocols are supported in the future.

ABORT_NOW() was triggered by s2n-quic which opens an unknown
unidirectional stream with greasing. This was detected by QUIC interop
runner for http3 testcase.

This must be backported up to 2.6.

(cherry picked from commit 9107731358)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 1ac095486711084895763fe026bd9186f3415bd6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:30:22 +01:00
Christopher Faulet
a6ff9f5361 BUG/MINOR: pool/stats: Use ullong to report total pool usage in bytes in stats
The same change was already performed for the cli. The stats applet and the
prometheus exporter are also concerned. Both use the stats API and rely on
pool functions to get total pool usage in bytes. pool_total_allocated() and
pool_total_used() must return 64 bits unsigned integer to avoid any wrapping
around 4G.

This may be backported to all versions.

(cherry picked from commit c960a3b60f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b174d82dff11d7fb67e9a7f53c20a658f23dd9e7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:29:07 +01:00
Christopher Faulet
84f5cba24f BUG/MEDIUM: mux-h2: Refuse interim responses with end-stream flag set
As state in RFC9113#8.1, HEADERS frame with the ES flag set that carries an
informational status code is malformed. However, there is no test on this
condition.

On 2.4 and higher, it is hard to predict consequences of this bug because
end of the message is only reported with a flag. But on 2.2 and lower, it
leads to a crash because there is an unexpected extra EOM block at the end
of an interim response.

Now, when a ES flag is detected on a HEADERS frame for an interim message, a
stream error is sent (RST_STREAM/PROTOCOL_ERROR).

This patch should solve the issue #1972. It should be backported as far as
2.0.

(cherry picked from commit 827a6299e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ebfae006c6b5de1d1fe0cdd51847ec1e39d5cf59)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:29:00 +01:00
Amaury Denoyelle
b118355b00 BUG/MEDIUM: mux-quic: fix double delete from qcc.opening_list
qcs instances for bidirectional streams are inserted in
<qcc.opening_list>. It is removed from the list once a full HTTP request
has been parsed. This is required to implement http-request timeout.

In case a stream is deleted before receiving full HTTP request, it also
must be removed from <qcc.opening_list>. This was not the case on first
implementation but has been fixed by the following patch :
  641a65ff3c
  BUG/MINOR: mux-quic: remove qcs from opening-list on free

This means that now a stream can be deleted from the list in two
different functions. Sadly, as LIST_DELETE was used in both cases,
nothing prevented a double-deletion from the list, even though
LIST_INLIST was used. Both calls are replaced with LIST_DEL_INIT which
is idempotent.

This bug causes memory corruption which results in most cases in a
segfault, most of times outside of mux-quic code itself. It has been
found first by gabrieltz who reported it on the github issue #1903. Big
thanks to him for his testing.

This bug also causes failures on several 'M' transfer testcase of QUIC
interop-runner. The s2n-quic client is particularly useful in this case
as segfaults triggers were most of the times on the LIST_DELETE
operation itself. This is probably due to its encapsulating of HEADERS
frame with fin bit delayed in a following empty STREAM frame.

This must be backported wherever the above patch is, up to 2.6.

(cherry picked from commit 15337fd808)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 151737fa818ffb37c8eb1706ef16722b6dd68f8b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:28:16 +01:00
Willy Tarreau
8b227febbf OPTIM: pool: split the read_mostly from read_write parts in pool_head
Performance profiling on a 48-thread machine showed a lot of time spent
in pool_free(), precisely at the point where pool->limit was retrieved.
And the reason is simple. Some parts of the pool_head are heavily updated
only when facing a cache miss ("allocated", "used", "needed_avg"), while
others are always accessed (limit, flags, size). The fact that both
entries were stored into the same cache line makes it very difficult for
each thread to access these precious info even when working with its own
cache.

By just splitting the fields apart, a test on QUIC (which stresses pools
a lot) more than doubled performance from 42 Gbps to 96 Gbps!

Given that the patch only reorders fields and addresses such a significant
contention, it should be backported to 2.7 and 2.6.

(cherry picked from commit 4dd33d9c32)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7d1b6977199fb663f39c928f3f159fd078d1b30d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:27:52 +01:00
Christopher Faulet
c1e939f10a BUG/MEDIUM: stats: Rely on a local trash buffer to dump the stats
It is possible to block the stats applet if a line exceeds the free space in
the responsse buffer while the buffer is empty. It is only an issue in HTTP
becaues of the HTX overhead and, AFAIK, only with json output.

In this case, the applet is unable to write anything in the response buffer
and waits for some free space to proceed further. On the other hand, because
the response channel is empty, nothing is sent and thus no space can be
freed. At this stage, the stream and the applet are blocked waiting for the
other side.

To avoid this situation, we must take care to not dump a line exceeding the
free space in the HTX message. It means we cannot rely anymore on the global
trash buffer. At least, not directly. The trick is to use a local trash
buffer, mapped on the global one but with a different size. We use b_make()
to do so. The local trash buffer is thread local to avoid any concurrency
issue.

It is a valid fix. However it could be good to review the internal API of
the stats applet to not rely on a global variable.

This patch should solve the #1873. It must be backported at least as far as
2.6. Older versions must be evaluated first but it is probably possible to
hit this bug with long proxy/server names.

(cherry picked from commit a8b7684319)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 86e057ec4228ec7188db46969c8e91a4c51481e3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:27:39 +01:00
Bertrand Jacquin
fb72716b76 BUG/MEDIUM: tests: use tmpdir to create UNIX socket
testdir can be a very long directory since it depends on source
directory path, this can lead to failure during tests when UNIX socket
path exceeds maximum allowed length of 97 characters as defined in
str2sa_range().

  16:48:14 [ALERT] ***  h1    debug|    (10082) : config : parsing [/tmp/haregtests-2022-12-17_16-47-39.4RNzIN/vtc.4850.5d0d728a/h1/cfg:19] : 'bind' : socket path 'unix@/local/p4clients/pkgbuild-bB20r/workspace/build/HAProxy/HAProxy-2.7.x.68.0/AL2_x86_64/DEV.STD.PTHREAD/build/private/HAProxy-2.7.x/src/reg-tests/lua/srv3' too long (max 97)

Also, it is not advisable to create UNIX socket in actual source
directory, but instead use dedicated temporary directory create for test
purpose.

This should be backported to 2.6

(cherry picked from commit 103966930a)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit ec0b6777d6ec17e7b208e29ad5dbf4cd988c2a39)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:27:17 +01:00
William Lallemand
0d41bc4b0d REGTESTS: startup: disable automatic_maxconn.vtc
The test still need to have more start condition, like ulimit checks
and less strict value checks.

To be backported where it was activated (as far as 2.5)

(cherry picked from commit 7332a123c1)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit a384f2862b3ad162d0b5ba92448c9c2b76836709)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:27:12 +01:00
Amaury Denoyelle
2cadfe2340 BUG/MINOR: quic: fix crash on PTO rearm if anti-amplification reset
There is a possible segfault when accessing qc->timer_task in
quic_conn_io_cb() without testing it. It seems however very rare as it
requires several condition to be encounter.
* quic_conn must be in CLOSING state after having sent a
  CONNECTION_CLOSE which free the qc.timer_task
* quic_conn handshake must still be in progress : in fact, qc.timer_task
  is accessed on this path because of the anti-amplification limit
  lifted.

I was unable thus far to trigger it but benchmarking tests seems to have
fire it with the following backtrace as a result :

  #0  _task_wakeup (f=4096, caller=0x5620ed004a40 <_.46868>, t=0x0) at include/haproxy/task.h:195
  195             state = _HA_ATOMIC_OR_FETCH(&t->state, f);
  [Current thread is 1 (Thread 0x7fc714ff1700 (LWP 14305))]
  (gdb) bt
  #0  _task_wakeup (f=4096, caller=0x5620ed004a40 <_.46868>, t=0x0) at include/haproxy/task.h:195
  #1  quic_conn_io_cb (t=0x7fc5d0e07060, context=0x7fc5d0df49c0, state=<optimized out>) at src/quic_conn.c:4393
  #2  0x00005620ecedab6e in run_tasks_from_lists (budgets=<optimized out>) at src/task.c:596
  #3  0x00005620ecedb63c in process_runnable_tasks () at src/task.c:861
  #4  0x00005620ecea971a in run_poll_loop () at src/haproxy.c:2913
  #5  0x00005620ecea9cf9 in run_thread_poll_loop (data=<optimized out>) at src/haproxy.c:3102
  #6  0x00007fc773c3f609 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
  #7  0x00007fc77372d133 in clone () from /lib/x86_64-linux-gnu/libc.so.6
  (gdb) up
  #1  quic_conn_io_cb (t=0x7fc5d0e07060, context=0x7fc5d0df49c0, state=<optimized out>) at src/quic_conn.c:4393
  4393                            task_wakeup(qc->timer_task, TASK_WOKEN_MSG);
  (gdb) p qc
  $1 = (struct quic_conn *) 0x7fc5d0df49c0
  (gdb) p qc->timer_task
  $2 = (struct task *) 0x0

This fix should be backported up to 2.6.

(cherry picked from commit 5ac6b3b125)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit e8d7fdf498e37ced00683159ca2797018e93b37c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:27:03 +01:00
Aurelien DARRAGON
9a4aaa43f8 BUG/MINOR: stats: fix show stat json buffer limitation
json output type is a lot more verbose than other output types.
Because of this and the increasing number of metrics implemented within
haproxy, we are starting to reach max bufsize limit (defaults to 16k)
when dumping stats to json since 2.6-dev1.
This results in stats output being truncated with
    "[{"errorStr":"output buffer too short"}]"

This was reported by Gabriel in #1964.

Thanks to "MINOR: stats: introduce stats field ctx", we can now make
multipart (using multiple buffers) dumping, in case a single buffer is not big
enough to hold the complete stat line.
For now, only stats_dump_fields_json() makes use of it as it is by
far the most verbose stats output type.
(csv, typed and html outputs should be good for a while and may use this
capability if the need arises in some distant future)

--

It could be backported to 2.6 and 2.7.
This commit depends on:
  - MINOR: stats: provide ctx for dumping functions
  - MINOR: stats: introduce stats field ctx

(cherry picked from commit 42b18fb645)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit e3f1715b82319eafada3dd51d4d468546986ec1d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:26:57 +01:00
Aurelien DARRAGON
dabfdf987f MINOR: stats: introduce stats field ctx
Add a new value in stats ctx: field.
Implement field support in line dumping parent functions
stats_print_proxy_field_json() and stats_dump_proxy_to_buffer().

This will allow child dumping functions to support partial line dumping
when needed. ie: when dumping buffer is exhausted: do a partial send and
wait for a new buffer to finish the dump. Thanks to field ctx, the function
can start dumping where it left off on previous (unterminated) invokation.

(cherry picked from commit 5594184190)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 84f6ea521b4f92779b15d5cd4de6539462dba54a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:26:52 +01:00
Aurelien DARRAGON
ed47170104 MINOR: stats: provide ctx for dumping functions
This is a minor refactor to allow stats_dump_info_* and stats_dump_fields_*
functions to directly access stat ctx pointer instead of explicitly
passing stat ctx struct members to them.

This will allow dumping functions to benefit from upcoming ctx updates.

(cherry picked from commit e76a027b0b)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 298735804e2412d6876b720066e20f767b5f301d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:26:48 +01:00
Remi Tricot-Le Breton
efde502b97 BUG/MINOR: ssl: Fix memory leak of find_chain in ssl_sock_load_cert_chain
The certificate chain that gets passed in the SSL_CTX through
SSL_CTX_set1_chain has its reference counter increased by OpenSSL
itself. But since the ssl_sock_load_cert_chain function might create a
brand new certificate chain if none exists in the ckch_data
(sk_X509_new_null), then we ended up returning a new certificate chain
to the caller that was never destroyed.

This patch can be backported to all stable branches but it might need to
be reworked for branches older than 2.4 because of commit ec805a32b9
that refactorized the modified code.

(cherry picked from commit 4cf0d3f1e8)
[wla: struct member data was called ckch before]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 3fc061bf30b45bbcab66b8bd8b38ce7578bc9ae6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:26:16 +01:00
Amaury Denoyelle
434f74c812 BUG/MINOR: h3: fix memleak on HEADERS parsing failure
If an error is triggered on H3 HEADERS parsing, the allocated buffer for
HTX data is not freed.

To prevent this memleak, all return path have been centralized using
goto statements.

Also, as a small bonus, offer_buffers() is not called anymore if buffer
is not freed because sedesc has taken it. However this change has
probably no noticeable effect as dynamic buffers management is not
functional currently.

This should be backported up to 2.6.

(cherry picked from commit 788fc05401)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit e1c23e5a1662ea3c871215dc579d2f3e20d90c12)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:52 +01:00
Amaury Denoyelle
b4e250237b BUG/MEDIUM: h3: fix cookie header parsing
Cookie header are treated specifically to merge multiple occurences in a
single HTX header. This is treated in a if-block condition inside the
'while (1)' loop for headers parsing. The length value of ist
representing cookie header is set to -1 by http_cookie_register(). The
problem is that then a continue statement is used but without
incrementing 'hdr_idx' to pass on the next header.

This issue was revealed by the introduction of commit :
  commit d6fb7a0e0f
  BUG/MEDIUM: h3: reject request with invalid header name

Before the aformentionned patch, the bug was hidden : on the next while
iteration, all isteq() invocations won't match with cookie header length
now set to -1. htx_add_header() fails silently because length is
invalid. hdr_idx is finally incremented which allows parsing to proceed
normally with the next header.

Now, a cookie header with length -1 do not pass the test on header name
conformance introduced by the above patch. Thus, a spurrious
RESET_STREAM is emitted. This behavior has been reported on the mailing
list by Shawn Heisey who found out that browsers disabled H3 usage due
to the RESET_STREAM received. Big thanks to him for his testing on the
master branch.

This issue is simply resolved by incrementing hdr_idx before continue
statement. It could have been detected earlier if htx_add_header()
return value was checked. This will be the subject of a dedicated commit
outside of the backport scope.

This must be backported up to 2.6.

(cherry picked from commit 19942e3859)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit fda9a5e4351d9b11bc2c1562d86a2da292443298)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:48 +01:00
scientiamobile
db5c4b3d93 LICENSE: wurfl: clarify the dummy library license.
This clarifies that LGPL is also permitted for the wurfl.h dummy file.
Should be backported where relevant.

Signed-off-by: Luca Passani <luca.passani@scientiamobile.com>
(cherry picked from commit 6d6787ba7c)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit d3120774e122280589a1453b640ea7bcc7d60108)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:26 +01:00
Amaury Denoyelle
da684b9d2c BUG/MINOR: mux-quic: handle properly alloc error in qcs_new()
Use qcs_free() on allocation failure in qcs_new() This ensures that all
qcs content is properly deallocated and prevent memleaks. Most notably,
qcs instance is now removed from qcc tree.

This bug is labelled as MINOR as it occurs only on qcs allocation
failure due to memory exhaustion.

This must be backported up to 2.6.

(cherry picked from commit 4b167006fd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 164acf2d8a03ad068e7ca1de0964f5f0f07375df)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:21 +01:00
Amaury Denoyelle
9ac19a6b3b BUG/MINOR: mux-quic: remove qcs from opening-list on free
qcs instances for bidirectional streams are inserted in
<qcc.opening_list>. It is removed from the list once a full HTTP request
has been parsed. This is required to implement http-request timeout.

If a qcs instance is freed before receiving a full HTTP request, it must
be removed from the <qcc.opening_list>. Else a segfault will occur in
qcc_refresh_timeout() when accessing a dangling pointer.

For the moment this bug was not reproduced in production. This is
because there exists only few rare cases where a qcs is freed before
HTTP request parsing. However, as error detection will be improved on
H3, this will occur more frequently in the near future.

This must be backported up to 2.6.

(cherry picked from commit 641a65ff3c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 252b67c4722ff2d4131e7875879364087f27a2fa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:17 +01:00
Amaury Denoyelle
e78bb0f6b1 BUG/MINOR: quic: properly handle alloc failure in qc_new_conn()
qc_new_conn() is used to allocate a quic_conn instance and its various
internal members. If one allocation fails, quic_conn_release() is used
to cleanup things.

For the moment, pool_zalloc() is used which ensures that all content is
null. However, some members must be initialized to a special values
to be able to use quic_conn_release() safely. This is the case for
quic_conn lists and its tasklet.

Also, some quic_conn internal allocation functions were doing their own
cleanup on failure without reset to NULL. This caused an issue with
quic_conn_release() which also frees this members. To fix this, these
functions now only return an error without cleanup. It is the caller
responsibility to free the allocated content, which is done via
quic_conn_release().

Without this patch, allocation failure in qc_new_conn() would often
result in segfault. This was reproduced easily using fail-alloc at 10%.

This should be backported up to 2.6.

(cherry picked from commit dbf6ad470b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit d35d46916d8ff53b13c08862297f49b5d881d738)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:25:07 +01:00
Ilya Shipitsin
ac1504b040 CI: github: split matrix for development and stable branches
ML ref: https://www.mail-archive.com/haproxy@formilux.org/msg42934.html

we agreed to use "latest" images for development branches and fixed
images for stable branches

Can be backported to 2.6.

(cherry picked from commit f5994fc692)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit e557ae9bac049e1a239510cc77c1812404c4d2ea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:24:34 +01:00
Ilya Shipitsin
5f493cbe57 CI: github: remove redundant ASAN loop
it was there because we only ran ASAN for clang, now no need to separate loop

Can be backported to 2.6.

(cherry picked from commit 6dedeb70da)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit a468a38c3cc49b8d8876b05da534654134c38fda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:24:29 +01:00
Amaury Denoyelle
dec6435195 BUG/MEDIUM: h3: parse content-length and reject invalid messages
Ensure that if a request contains a content-length header it matches
with the total size of following DATA frames. This is conformance with
HTTP/3 RFC 9114.

For the moment, this kind of errors triggers a connection close. In the
future, it should be handled only with a stream reset. To reduce
backport surface, this will be implemented in another commit.

This must be backported up to 2.6. It relies on the previous commit :
  MINOR: http: extract content-length parsing from H2

(cherry picked from commit d2c5ee665e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 43bb85f88d4a0273f90fa9d41ed52dbcb8c52abb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2023-01-20 09:24:11 +01:00