]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoBUILD: hpack: include global.h for the trash that is needed in debug mode
Willy Tarreau [Thu, 19 Jan 2023 23:02:37 +0000 (00:02 +0100)] 
BUILD: hpack: include global.h for the trash that is needed in debug mode

When building with -DDEBUG_HPACK, the trash is needed, but it's declared
in global.h.

This may be backported to all supported versions.

2 years agoBUG/MINOR: mux-h2: add missing traces on failed headers decoding
Willy Tarreau [Thu, 19 Jan 2023 22:58:11 +0000 (23:58 +0100)] 
BUG/MINOR: mux-h2: add missing traces on failed headers decoding

In case HPACK cannot be decoded, logs are emitted but there's no info
in the H2 traces, so let's add them.

This may be backported to all supported versions.

2 years agoBUG/MINOR: mux-h2: make sure to produce a log on invalid requests
Willy Tarreau [Thu, 19 Jan 2023 22:22:03 +0000 (23:22 +0100)] 
BUG/MINOR: mux-h2: make sure to produce a log on invalid requests

As reported by Dominik Froehlich in github issue #1968, some H2 request
parsing errors do not result in a log being emitted. This is annoying
for debugging because while an RST_STREAM is correctly emitted to the
client, there's no way without enabling traces to find it on the
haproxy side.

After some testing with various abnormal requests, a few places were
found where logs were missing and could be added. In this case, we
simply use sess_log() so some sample fetch functions might not be
available since the stream is not created. But at least there will
be a BADREQ in the logs. A good eaxmple of this consists in sending
forbidden headers or header syntax (e.g. presence of LF in value).
Some quick tests can be done this way:

- protocol error (LF in value):
  curl -iv --http2-prior-knowledge -H "$(printf 'a:b\na')" http://0:8001/

- too large header block after decoding:
  curl -v --http2-prior-knowledge -H "a:$(perl -e "print('a'x10000)")"  -H "a:$(perl -e "print('a'x10000)")"  http://localhost:8001/

This should be backported where needed, most likely 2.7 and 2.6 at
least for a start, and progressively to other versions.

2 years agoBUG/MEDIUM: debug/thread: make the debug handler not wait for !rdv_requests
Willy Tarreau [Thu, 19 Jan 2023 17:52:31 +0000 (18:52 +0100)] 
BUG/MEDIUM: debug/thread: make the debug handler not wait for !rdv_requests

The debug handler may deadlock with some threads waiting for isolation.
This may happend during a "show threads" command or even during a panic.
The reason is the call to thread_harmless_end() which waits for rdv_requests
to turn to zero before releasing its position in thread_dump_state,
while that one may not progress if another thread was interrupted in
thread_isolate() and is waiting for that thread to drop thread_dump_state.

In order to address this, we now use thread_harmless_end_sig() introduced
by previous commit:

   MINOR: threads: add a thread_harmless_end() version that doesn't wait

However there's a catch: since commit f7afdd910 ("MINOR: debug: mark
oneself harmless while waiting for threads to finish"), there's a second
pair of thread_harmless_now()/thread_harmless_end() that surround the
loop around thread_dump_state. Marking a thread harmless before this
loop and dropping that without checking rdv_requests there could break
the harmless promise made to the other thread if it returns first and
proceeds with its isolated work. Hence we just drop this pair which was
only preventive for other signal handlers, while as indicated in that
patch's commit message, other signals are handled asynchronously and do
not require that extra protection.

This fix must be backported to 2.7.

The problem can be seen by running "show threads" in fast loops (100/s)
while reloading haproxy very quickly (10/s) and sending lots of traffic
to it (100krps, 15 Gbps). In this case the soft stop calls pool_gc()
which isolates a lot and manages to race with the dumps after a few
tens of seconds, leaving the process with all threads at 100%.

2 years agoMINOR: threads: add a thread_harmless_end() version that doesn't wait
Willy Tarreau [Thu, 19 Jan 2023 17:43:48 +0000 (18:43 +0100)] 
MINOR: threads: add a thread_harmless_end() version that doesn't wait

thread_harmless_end() needs to wait for rdv_requests to disappear so
that we're certain to respect a harmless promise that possibly allowed
another thread to proceed under isolation. But this doesn't work in a
signal handler because a thread could be interrupted by the debug
handler while already waiting for isolation and with rdv_request>0.
As such this function could cause a deadlock in such a signal handler.

Let's implement a specific variant for this, thread_harmless_end_sig(),
that just resets the thread's bit and doesn't wait. It must of course
not be used past a check point that would allow the isolation requester
to return and see the thread as temporarily harmless then turning back
on its promise.

This will be needed to fix a race in the debug handler.

2 years agoBUG/MINOR: thread: always reload threads_enabled in loops
Willy Tarreau [Thu, 19 Jan 2023 18:14:18 +0000 (19:14 +0100)] 
BUG/MINOR: thread: always reload threads_enabled in loops

A few loops waiting for threads to synchronize such as thread_isolate()
rightfully filter the thread masks via the threads_enabled field that
contains the list of enabled threads. However, it doesn't use an atomic
load on it. Before 2.7, the equivalent variables were marked as volatile
and were always reloaded. In 2.7 they're fields in ha_tgroup_ctx[], and
the risk that the compiler keeps them in a register inside a loop is not
null at all. In practice when ha_thread_relax() calls sched_yield() or
an x86 PAUSE instruction, it could be verified that the variable is
always reloaded. If these are avoided (e.g. architecture providing
neither solution), it's visible in asm code that the variables are not
reloaded. In this case, if a thread exists just between the moment the
two values are read, the loop could spin forever.

This patch adds the required _HA_ATOMIC_LOAD() on the relevant
threads_enabled fields. It must be backported to 2.7.

2 years agoBUG/MEDIUM: fd/threads: fix again incorrect thread selection in wakeup broadcast
Willy Tarreau [Thu, 19 Jan 2023 16:10:10 +0000 (17:10 +0100)] 
BUG/MEDIUM: fd/threads: fix again incorrect thread selection in wakeup broadcast

Commit c1640f79f ("BUG/MEDIUM: fd/threads: fix incorrect thread selection
in wakeup broadcast") fixed an incorrect range being used to pick a thread
when broadcasting a wakeup for a foreign thread, but the selection was still
wrong as the number of threads and their mask was taken from the current
thread instead of the target thread. In addition, the code dealing with the
wakeup of a thread from the same group was still relying on MAX_THREADS
instead of tg->count.

This could theoretically cause random crashes with more than one thread
group though this was never encountered.

This needs to be backported to 2.7.

2 years agoMINOR: h3: implement TRAILERS decoding
Amaury Denoyelle [Fri, 13 Jan 2023 15:40:31 +0000 (16:40 +0100)] 
MINOR: h3: implement TRAILERS decoding

Implement the conversion of H3 request trailers as HTX blocks. This is
done through a new function h3_trailers_to_htx(). If the request
contains forbidden trailers it is rejected with a stream error.

This should be backported up to 2.7.

2 years agoBUG/MINOR: bwlim: Fix parameters check for set-bandwidth-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:39:54 +0000 (15:39 +0100)] 
BUG/MINOR: bwlim: Fix parameters check for set-bandwidth-limit actions

First, the inspect-delay is now tested if the action is used on a
tcp-response content rule. Then, when an expressions scope is checked, we
now take care to detect the right scope depending on the ruleset used
(tcp-request, tcp-response, http-request or http-response).

This patch could be backported to 2.7.

2 years agoMEDIUM: bwlim: Support constants limit or period on set-bandwidth-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:33:32 +0000 (15:33 +0100)] 
MEDIUM: bwlim: Support constants limit or period on set-bandwidth-limit actions

It is now possible to set a constant for the limit or period parameters on a
set-bandwidth-limit actions. The limit must follow the HAProxy size format
and is expressed in bytes. The period must follow the HAProxy time format
and is expressed in milliseconds. Of course, it is still possible to use
sample expressions instead.

The documentation was updated accordingly.

It is not really a bug. Only exemples were written this way in the
documentation. But it could be good to backport this change in 2.7.

2 years agoBUG/MINOR: bwlim: Check scope for period expr for set-bandwitdh-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:21:53 +0000 (15:21 +0100)] 
BUG/MINOR: bwlim: Check scope for period expr for set-bandwitdh-limit actions

If a period expression is defined for a set-bandwitdh-limit action, its
scope must be tested.

This patch must be backported to 2.7.

2 years agoMINOR: h3: implement TRAILERS encoding
Amaury Denoyelle [Thu, 12 Jan 2023 13:53:43 +0000 (14:53 +0100)] 
MINOR: h3: implement TRAILERS encoding

This patch implement the conversion of an HTX response containing
trailer into a H3 HEADERS frame. This is done through a new function
named h3_resp_trailers_send().

This was tested with a nginx configuration using <add_trailer>
statement.

It may be possible that HTX buffer only contains a EOT block without
preceeding trailer. In this case, the conversion will produce nothing
but fin will be reported. This causes QUIC mux to generate an empty
STREAM frame with FIN bit set.

This should be backported up to 2.7.

2 years agoMINOR: h3: extend function for QUIC varint encoding
Amaury Denoyelle [Tue, 17 Jan 2023 14:21:16 +0000 (15:21 +0100)] 
MINOR: h3: extend function for QUIC varint encoding

Slighty adjust b_quic_enc_int(). This function is used to encode an
integer as a QUIC varint in a struct buffer.

A new parameter is added to the function API to specify the width of the
encoded integer. By default, 0 should be use to ensure that the minimum
space is used. Other valid values are 1, 2, 4 or 8. An error is reported
if the width is not large enough.

This new parameter will be useful when buffer space is reserved prior to
encode an unknown integer value. The maximum size of 8 bytes will be
reserved and some data can be put after. When finally encoding the
integer, the width can be requested to be 8 bytes.

With this new parameter, a small refactoring of the function has been
conducted to remove some useless internal variables.

This should be backported up to 2.7. It will be mostly useful to
implement H3 trailers encoding.

2 years agoBUG/MINOR: h3: properly handle connection headers
Amaury Denoyelle [Tue, 17 Jan 2023 16:47:06 +0000 (17:47 +0100)] 
BUG/MINOR: h3: properly handle connection headers

Connection headers are not used in HTTP/3. As specified by RFC 9114, a
received message containing one of those is considered as malformed and
rejected. When converting an HTX message to HTTP/3, these headers are
silently skipped.

This must be backported up to 2.6. Note that assignment to <h3s.err>
must be removed on 2.6 as stream level error has been introduced in 2.7
so this field does not exist in 2.6 A connection error will be used
instead automatically.

2 years agoBUG/MINOR: listener: close tiny race between resume_listener() and stopping
Willy Tarreau [Thu, 19 Jan 2023 10:34:21 +0000 (11:34 +0100)] 
BUG/MINOR: listener: close tiny race between resume_listener() and stopping

Pierre Cheynier reported a very rare race condition on soft-stop in the
listeners. What happens is that if a previously limited listener is
being resumed by another thread finishing an accept loop, and at the
same time a soft-stop is performed, the soft-stop will turn the
listener's state to LI_INIT, and once the listener's lock is released,
resume_listener() in the second thread will try to resume this listener
which has an fd==-1, yielding a crash in listener_set_state():

  FATAL: bug condition "l->rx.fd == -1" matched at src/listener.c:288

The reason is that resume_listener() only checks for LI_READY, but doesn't
consider being called with a non-initialized or a stopped listener. Let's
also make sure we don't try to ressuscitate such a listener there.

This will have to be backported to all versions.

2 years agoBUG/MINOR: ssl: Fix compilation with OpenSSL 1.0.2 (missing ECDSA_SIG_set0)
Remi Tricot-Le Breton [Wed, 18 Jan 2023 16:29:54 +0000 (17:29 +0100)] 
BUG/MINOR: ssl: Fix compilation with OpenSSL 1.0.2 (missing ECDSA_SIG_set0)

This function was introduced in OpenSSL 1.1.0. Prior to that, the
ECDSA_SIG structure was public.
This function was used in commit 5a8f02ae "BUG/MEDIUM: jwt: Properly
process ecdsa signatures (concatenated R and S params)".

This patch needs to be backported up to branch 2.5 alongside commit
5a8f02ae.

2 years agoRevert "BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7"
William Lallemand [Thu, 19 Jan 2023 10:08:42 +0000 (11:08 +0100)] 
Revert "BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7"

This reverts commit d65791e26c12b57723f2feb7eacdbbd99601371b.

Conflict with the patch which was originally written and lacks the
BN_clear_free() and the NULL check.

2 years agoBUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7
Willy Tarreau [Thu, 19 Jan 2023 09:50:13 +0000 (10:50 +0100)] 
BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7

Commit 5a8f02ae6 ("BUG/MEDIUM: jwt: Properly process ecdsa signatures
(concatenated R and S params)") makes use of ECDSA_SIG_set0() which only
appeared in openssl-1.1.0 and libressl 2.7, and breaks the build before.
Let's just do what it minimally does (only assigns the two fields to the
destination).

This will need to be backported where the commit above is, likely 2.5.

2 years agoBUG/MEDIUM: jwt: Properly process ecdsa signatures (concatenated R and S params)
Remi Tricot-Le Breton [Wed, 18 Jan 2023 14:32:28 +0000 (15:32 +0100)] 
BUG/MEDIUM: jwt: Properly process ecdsa signatures (concatenated R and S params)

When the JWT token signature is using ECDSA algorithm (ES256 for
instance), the signature is a direct concatenation of the R and S
parameters instead of OpenSSL's DER format (see section
3.4 of RFC7518).
The code that verified the signatures wrongly assumed that they came in
OpenSSL's format and it did not actually work.
We now have the extra step of converting the signature into a complete
ECDSA_SIG that can be fed into OpenSSL's digest verification functions.

The ECDSA signatures in the regtest had to be recalculated and it was
made via the PyJWT python library so that we don't end up checking
signatures that we built ourselves anymore.

This patch should fix GitHub issue #2001.
It should be backported up to branch 2.5.

2 years agoDOC: config: fix "Address formats" chapter syntax
Daniel Corbett [Tue, 17 Jan 2023 23:32:31 +0000 (18:32 -0500)] 
DOC: config: fix "Address formats" chapter syntax

The section on "Address formats" doesn't provide the dot (.) after the
chapter numbers, which breaks parsing within the HTML converter.
This commit adds the dot (.) after each chapter within Section 11.

This should be backported to versions 2.4 and above.

2 years agoBUG/MINOR: mux-fcgi: Correctly set pathinfo
Paul Barnetta [Mon, 16 Jan 2023 22:44:11 +0000 (09:44 +1100)] 
BUG/MINOR: mux-fcgi: Correctly set pathinfo

Existing logic for checking whether a regex subexpression for pathinfo
is matched results in valid matches being ignored and non-matches having
a new zero length string stored in params->pathinfo. This patch reverses
the logic so params->pathinfo is set when the subexpression is matched.

Without this patch the example configuration in the documentation:

path-info ^(/.+\.php)(/.*)?$

does not result in PATH_INFO being sent to the FastCGI application, as
expected, when the second subexpression is matched (in which case both
pmatch[2].rm_so and pmatch[2].rm_eo will be non-negative integers).

This patch may be backported as far as 2.2, the first release that made
the capture of this second subexpression optional.

2 years agoMINOR: quic: Replace v2 draft definitions by those of the final 2 version
Frédéric Lécaille [Fri, 13 Jan 2023 15:37:02 +0000 (16:37 +0100)] 
MINOR: quic: Replace v2 draft definitions by those of the final 2 version

This should finalize the support for the QUIC version 2.

Must be backported to 2.7.

2 years agoMINOR: sample: Add "quic_enabled" sample fetch
Frédéric Lécaille [Thu, 12 Jan 2023 16:55:45 +0000 (17:55 +0100)] 
MINOR: sample: Add "quic_enabled" sample fetch

This sample fetch returns a boolean. True if the support for QUIC transport
protocol was built and if this protocol was not disabled by "no-quic"
global option.

Must be backported to 2.7.

2 years agoMINOR: quic: Add "no-quic" global option
Frédéric Lécaille [Thu, 12 Jan 2023 14:23:54 +0000 (15:23 +0100)] 
MINOR: quic: Add "no-quic" global option

Add "no-quic" to "global" section to disable the use of QUIC transport protocol
by all configured QUIC listeners. This is listeners with QUIC addresses on their
"bind" lines. Internally, the socket addresses binding is skipped by
protocol_bind_all() for receivers with <proto_quic4> or <proto_quic6> as
protocol (see protocol struct).
Add information about "no-quic" global option to the documentation.

Must be backported to 2.7.

2 years agoMINOR: quic: Disable the active connection migrations
Frédéric Lécaille [Thu, 12 Jan 2023 07:29:23 +0000 (08:29 +0100)] 
MINOR: quic: Disable the active connection migrations

Set "disable_active_migration" transport parameter to inform the peer
haproxy listeners does not the connection migration feature.
Also drop all received datagrams with a modified source address.

Must be backported to 2.7.

2 years agoMINOR: quic: Useless test about datagram destination addresses
Frédéric Lécaille [Thu, 12 Jan 2023 09:36:26 +0000 (10:36 +0100)] 
MINOR: quic: Useless test about datagram destination addresses

There is no reason to check if the peer has modified the destination
address of its connection.

May be backported to 2.7.

2 years agoBUG/MEDIUM: stconn: also consider SE_FL_EOI to switch to SE_FL_ERROR
Willy Tarreau [Tue, 17 Jan 2023 15:27:35 +0000 (16:27 +0100)] 
BUG/MEDIUM: stconn: also consider SE_FL_EOI to switch to SE_FL_ERROR

In se_fl_set_error() we used to switch to SE_FL_ERROR only when there
is already SE_FL_EOS indicating that the read side is closed. But that
is not sufficient, we need to consider all cases where no more reads
will be performed on the connection, and as such also include SE_FL_EOI.

Without this, some aborted connections during a transfer sometimes only
stop after the timeout, because the ERR_PENDING is never promoted to
ERROR.

This must be backported to 2.7 and requires previous patch "CLEANUP:
stconn: always use se_fl_set_error() to set the pending error".

2 years agoCLEANUP: stconn: always use se_fl_set_error() to set the pending error
Willy Tarreau [Tue, 17 Jan 2023 15:25:29 +0000 (16:25 +0100)] 
CLEANUP: stconn: always use se_fl_set_error() to set the pending error

In mux-h2 and mux-quic we still had two places manually setting
SE_FL_ERR_PENDING or SE_FL_ERROR depending on the EOS state, instead
of using se_fl_set_error() which takes care of the condition. Better
use the specialized function for this, it will allow to centralize
the conditions. Note that this will be needed to fix a bug.

2 years agoMINOR: listener: also support "quic+" as an address prefix
Willy Tarreau [Mon, 16 Jan 2023 12:55:27 +0000 (13:55 +0100)] 
MINOR: listener: also support "quic+" as an address prefix

While we do support quic4@ and quic6@ for listening addresses, it was
not possible to specify that we want to use an FD inherited from the
parent with QUIC. It's just a matter of making it possible to enable
a dgram-type socket and a stream-type transport, so let's add this.

Now it becomes possible to write "quic+fd@12", "quic+ipv4@addr" etc.

2 years agoDOC: config: mention the missing "quic4@" and "quic6@" in protocol prefixes
Willy Tarreau [Mon, 16 Jan 2023 11:14:11 +0000 (12:14 +0100)] 
DOC: config: mention the missing "quic4@" and "quic6@" in protocol prefixes

These two variants were missing from the section on protocol prefixes.

2 years agoDOC: config: fix aliases for protocol prefixes "udp4@" and "udp6@"
Willy Tarreau [Mon, 16 Jan 2023 11:11:38 +0000 (12:11 +0100)] 
DOC: config: fix aliases for protocol prefixes "udp4@" and "udp6@"

It was mentioned that they are equivalent to "stream+ipv*@" while it's
the equivalent of "dgram+ipv*@".

2 years agoDOC: config: fix wrong section number for "protocol prefixes"
Willy Tarreau [Mon, 16 Jan 2023 11:07:12 +0000 (12:07 +0100)] 
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".

2 years agoBUG/MINOR: listeners: fix suspend/resume of inherited FDs
Willy Tarreau [Mon, 16 Jan 2023 10:47:01 +0000 (11:47 +0100)] 
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.

2 years agoBUG/MINOR: http-ana: make set-status also update txn->status
Willy Tarreau [Tue, 10 Jan 2023 13:50:44 +0000 (14:50 +0100)] 
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.

2 years agoMINOR: htx: Add an HTX value for the extra field is payload length is unknown
Christopher Faulet [Fri, 13 Jan 2023 10:40:24 +0000 (11:40 +0100)] 
MINOR: htx: Add an HTX value for the extra field is payload length is unknown

When the payload length cannot be determined, the htx extra field is set to
the magical vlaue ULLONG_MAX. It is not obvious. This a dedicated HTX value
is now used. Now, HTX_UNKOWN_PAYLOAD_LENGTH must be used in this case,
instead of ULLONG_MAX.

2 years agoBUG/MEDIUM: mux-h2: Don't send CANCEL on shutw when response length is unkown
Christopher Faulet [Fri, 13 Jan 2023 10:28:31 +0000 (11:28 +0100)] 
BUG/MEDIUM: mux-h2: Don't send CANCEL on shutw when response length is unkown

Since commit 473e0e54 ("BUG/MINOR: mux-h2: send a CANCEL instead of ES on
truncated writes"), a CANCEL may be reported when the response length is
unkown. It happens for H1 reponses without "Content-lenght" or
"Transfer-encoding" header.

Indeed, in this case, the end of the reponse is detected when the server
connection is closed. On the fontend side, the H2 multiplexer handles this
event as an abort and sensd a RST_STREAM frame with CANCEL error code.

The issue is not with the above commit but with the commit 4877045f1
("MINOR: mux-h2: make streams know if they need to send more data"). The
H2_SF_MORE_HTX_DATA flag must only be set if the payload length can be
determined.

This patch should fix the issue #1992. It must be backported to 2.7.

2 years agoCLEANUP: http-ana: Remove HTTP_MSG_ERROR state
Christopher Faulet [Fri, 13 Jan 2023 10:22:12 +0000 (11:22 +0100)] 
CLEANUP: http-ana: Remove HTTP_MSG_ERROR state

This state is now unused. Thus it can be removed.

2 years agoMAJOR: http-ana: Review error handling during HTTP payload forwarding
Christopher Faulet [Fri, 13 Jan 2023 10:02:28 +0000 (11:02 +0100)] 
MAJOR: http-ana: Review error handling during HTTP payload forwarding

The error handling in the HTTP payload forwarding is far to be ideal because
both sides (request and response) are tested each time. It is espcially ugly
on the request side. To report a server error instead of a client error,
there are some workarounds to delay the error handling. The reason is that
the request analyzer is evaluated before the response one. In addition,
errors are tested before the data analysis. It means it is possible to
truncate data because errors may be handled to early.

So the error handling at this stages was totally reviewed. Aborts are now
handled after the data analysis. We also stop to finish the response on
request error or the opposite. As a side effect, the HTTP_MSG_ERROR state is
now useless. As another side effect, the termination flags are now set by
the HTTP analysers and not process_stream().

2 years agoBUG/MINOR: http-fetch: Don't block HTTP sample fetch eval in HTTP_MSG_ERROR state
Christopher Faulet [Fri, 13 Jan 2023 09:58:20 +0000 (10:58 +0100)] 
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).

2 years agoMINOR: http-ana: Use http_set_term_flags() when waiting the request body
Christopher Faulet [Fri, 13 Jan 2023 09:20:20 +0000 (10:20 +0100)] 
MINOR: http-ana: Use http_set_term_flags() when waiting the request body

When HAProxy is waiting for the request body and an abort or an error is
detected, we can now use http_set_term_flags() function to set the termination
flags of the stream instead of handling it by hand.

2 years agoBUG/MINOR: http-ana: Report SF_FINST_R flag on error waiting the request body
Christopher Faulet [Fri, 13 Jan 2023 09:49:31 +0000 (10:49 +0100)] 
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.

2 years agoMINOR: http-ana: Use http_set_term_flags() in most of HTTP analyzers
Christopher Faulet [Fri, 13 Jan 2023 08:43:21 +0000 (09:43 +0100)] 
MINOR: http-ana: Use http_set_term_flags() in most of HTTP analyzers

We use the new function to set the HTTP termination flags in the most
obvious places. The other places are a bit specific and will be handled one
by one in dedicated patched.

2 years agoMINOR: http-ana: Add a function to set HTTP termination flags
Christopher Faulet [Fri, 13 Jan 2023 08:06:38 +0000 (09:06 +0100)] 
MINOR: http-ana: Add a function to set HTTP termination flags

There is already a function to set termination flags but it is not well
suited for HTTP streams. So a function, dedicated to the HTTP analysis, was
added. This way, this new function will be called for HTTP analysers on
error. And if the error is not caugth at this stage, the generic function
will still be called from process_stream().

Here, by default a PRXCOND error is reported and depending on the stream
state, the reson will be set accordingly:

  * If the backend SC is in INI state, SF_FINST_T is reported on tarpit and
    SF_FINST_R otherwise.

  * SF_FINST_Q is the server connection is queued

  * SF_FINST_C in any connection attempt state (REQ/TAR/ASS/CONN/CER/RDY).
    Except for applets, a SF_FINST_R is reported.

  * Once the server connection is established, SF_FINST_H is reported while
    HTTP_MSG_DATA state on the response side.

  * SF_FINST_L is reported if the response is in HTTP_MSG_DONE state or
    higher and a client error/timeout was reported.

  * Otherwise SF_FINST_D is reported.

2 years agoBUG/MINOR: promex: Don't forget to consume the request on error
Christopher Faulet [Fri, 13 Jan 2023 07:53:23 +0000 (08:53 +0100)] 
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.

2 years agoBUG/MEDIUM: peers: make "show peers" more careful about partial initialization
Willy Tarreau [Thu, 12 Jan 2023 16:09:34 +0000 (17:09 +0100)] 
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.

2 years agoOPTIM: global: move byte counts out of global and per-thread
Willy Tarreau [Thu, 12 Jan 2023 15:08:41 +0000 (16:08 +0100)] 
OPTIM: global: move byte counts out of global and per-thread

During multiple tests we've already noticed that shared stats counters
have become a real bottleneck under large thread counts. With QUIC it's
pretty visible, with qc_snd_buf() taking 2.5% of the CPU on a 48-thread
machine at only 25 Gbps, and this CPU is entirely spent in the atomic
increment of the byte count and byte rate. It's also visible in H1/H2
but slightly less since we're working with larger buffers, hence less
frequent updates. These counters are exclusively used to report the
byte count in "show info" and the byte rate in the stats.

Let's move them to the thread_ctx struct and make the stats reader
just collect each thread's stats when requested. That's way more
efficient than competing on a single cache line.

After this, qc_snd_buf has totally disappeared from the perf profile
and tests made in h1 show roughly 1% performance increase on small
objects.

2 years agoREGTEST: ssl: Add test for 'update ssl ocsp-response' CLI command
Remi Tricot-Le Breton [Thu, 12 Jan 2023 08:49:12 +0000 (09:49 +0100)] 
REGTEST: ssl: Add test for 'update ssl ocsp-response' CLI command

This patch adds tests for the newly added 'update ssl ocsp-response' CLI
command.

2 years agoMINOR: ssl: Reinsert updated ocsp response later in tree in case of http error
Remi Tricot-Le Breton [Thu, 12 Jan 2023 08:49:11 +0000 (09:49 +0100)] 
MINOR: ssl: Reinsert updated ocsp response later in tree in case of http error

When updating an OCSP response, in case of HTTP error (host unreachable
for instance) we do not want to reinsert the entry at the same place in
the update tree otherwise we might retry immediately the update of the
same response. This patch adds an arbitrary 1min time to the next_update
of a response in such a case.
After an HTTP error, instead of waking the update task up after an
arbitrary 10s time, we look for the first entry of the update tree and
sleep for the apropriate time.

2 years agoMINOR: ssl: Do not wake ocsp update task if update tree empty
Remi Tricot-Le Breton [Thu, 12 Jan 2023 08:49:10 +0000 (09:49 +0100)] 
MINOR: ssl: Do not wake ocsp update task if update tree empty

In the unlikely event that the ocsp update task is started but the
update tree is empty, put the update task to sleep indefinitely.
The only way this can happen is if the same certificate is loaded under
two different names while the second one has the 'ocsp-update on'
option. Since the certificate names are distinct we will have two
ckch_stores but a single certificate_ocsp because they are identified by
the OCSP_CERTID which is built out of the issuer certificate and the
certificate id (which are the same regardless of the .pem file name).

2 years agoMINOR: ssl: Treat ocsp-update inconsistencies as fatal errors
Remi Tricot-Le Breton [Thu, 12 Jan 2023 08:49:09 +0000 (09:49 +0100)] 
MINOR: ssl: Treat ocsp-update inconsistencies as fatal errors

If incompatibilities are found in a certificate's ocsp-update mode we
raised a single alert that will be considered fatal from here on. This
is changed because in case of incompatibilities we will end up with an
undefined behaviour. The ocsp response might or might not be updated
depending on the order in which the multiple ocsp-update options are
taken into account.

2 years agoBUG/MINOR: ssl: OCSP minimum update threshold not properly set
Remi Tricot-Le Breton [Thu, 12 Jan 2023 08:49:08 +0000 (09:49 +0100)] 
BUG/MINOR: ssl: OCSP minimum update threshold not properly set

An arbitrary 5 minutes minimum interval between two updates of the same
OCSP response is defined but it was not properly used when inserting
entries in the update tree.

This patch does not need to be backported.

2 years agoBUG/MEDIUM: listener: duplicate inherited FDs if needed
Willy Tarreau [Wed, 11 Jan 2023 09:59:52 +0000 (10:59 +0100)] 
BUG/MEDIUM: listener: duplicate inherited FDs if needed

Since commit 36d9097cf ("MINOR: fd: Add BUG_ON checks on fd_insert()"),
there is currently a test in fd_insert() to detect that we're not trying
to reinsert an FD that had already been inserted. This test catches the
following anomalies:

   frontend fail1
       bind fd@0
       bind fd@0

and:

   frontend fail2
       bind fd@0 shards 2

What happens is that clone_listener() is called on a listener already
having an FD, and when sock_{inet,unix}_bind_receiver() are called, the
same FD will be registered multiple times and rightfully crash in the
sanity check.

It wouldn't be correct to block shards though (e.g. they could be used
in a default-bind line). What looks like a safer and more future-proof
approach simply is to dup() the FD so that each listener has one copy.
This is also the only solution that might allow later to support more
than 64 threads on an inherited FD.

This needs to be backported as far as 2.4. Better wait for at least
one extra -dev version before backporting though, as the bug should
not be triggered often anyway.

2 years agoDEV: tcploop: add minimal support for unix sockets
Willy Tarreau [Wed, 11 Jan 2023 09:54:59 +0000 (10:54 +0100)] 
DEV: tcploop: add minimal support for unix sockets

Since the tool permits to pass an FD bound for listening, it's convenient
to test haproxy's "bind fd@". Let's add support for UNIX sockets the same
way. -U needs to be passed to change the default address family, and the
address must contain a "/".

E.g.
  $ dev/tcploop/tcploop -U /tmp/ux L Xi ./haproxy -f fd1.cfg

2 years agoBUG/MINOR: ssl: Missing ssl_conf pointer check when checking ocsp update inconsistencies
Remi Tricot-Le Breton [Tue, 10 Jan 2023 10:44:15 +0000 (11:44 +0100)] 
BUG/MINOR: ssl: Missing ssl_conf pointer check when checking ocsp update inconsistencies

The ssl_conf might be NULL when processing ocsp_update option in
crt-lists.

This patch fixes GitHub issue #1995.
It does not need to be backported.

2 years agoBUG/MINOR: ssl: Remove unneeded pointer check in ocsp cli release function
Remi Tricot-Le Breton [Tue, 10 Jan 2023 10:21:40 +0000 (11:21 +0100)] 
BUG/MINOR: ssl: Remove unneeded pointer check in ocsp cli release function

The ctx pointer cannot be NULL so we can remove the check.

This patch fixes GitHub issue #1996.
It does not need to be backported.

2 years agoBUG/MINOR: resolvers: Wait the resolution execution for a do_resolv action
Christopher Faulet [Wed, 11 Jan 2023 09:31:10 +0000 (10:31 +0100)] 
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.

2 years agoBUG/MINOR: hlua: Fix Channel.line and Channel.data behavior regarding the doc
Christopher Faulet [Tue, 10 Jan 2023 14:29:54 +0000 (15:29 +0100)] 
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.

2 years agoBUG/MINOR: h1-htx: Remove flags about protocol upgrade on non-101 responses
Christopher Faulet [Tue, 10 Jan 2023 17:51:55 +0000 (18:51 +0100)] 
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.

2 years agoMINOR: mux-quic: use send-list for immediate sending retry
Amaury Denoyelle [Fri, 6 Jan 2023 16:16:47 +0000 (17:16 +0100)] 
MINOR: mux-quic: use send-list for immediate sending retry

Sending is done with several iterations over qcs streams in qc_send().
The first loop is conducted over streams in <qcc.send_list>. After this
first iteration, some streams may still have data in their Tx buffer but
were blocked by a full qc_stream_desc buffer. In this case, they have
release their qc_stream_desc buffer in qcc_streams_sent_done().

New iterations can be done for these streams which can allocate new
qc_stream_desc buffer if available. Before this patch, this was done
through another stream list <qcc.send_retry_list>. Now, we can reuse the
new <qcc.send_list> for this usage.

This is safe to use as after first iteration, we have guarantee that
either one of the following is true if there is still streams in
<qcc.send_list> :
* transport layer has rejected data due to congestion
* stream is left because it is blocked on stream flow control
* stream still has data and has released a fulfilled qc_stream_desc
  buffer. Immediate retry is useful for these streams : they will
  allocate a new qc_stream_desc buffer if possible to continue sending.

This must be backported up to 2.7.

2 years agoMINOR: mux-quic: use send-list for STOP_SENDING/RESET_STREAM emission
Amaury Denoyelle [Fri, 6 Jan 2023 16:43:11 +0000 (17:43 +0100)] 
MINOR: mux-quic: use send-list for STOP_SENDING/RESET_STREAM emission

When a STOP_SENDING or RESET_STREAM must be send, its corresponding qcs
is inserted into <qcc.send_list> via qcc_reset_stream() or
qcc_abort_stream_read().

This allows to remove the iteration on full qcs tree in qc_send().
Instead, STOP_SENDING and RESET_STREAM is done in the loop over
<qcc.send_list> as with STREAM frames. This should improve slightly the
performance, most notably when large number of streams are opened.

This must be backported up to 2.7.

2 years agoMEDIUM: h3: send SETTINGS before STREAM frames
Amaury Denoyelle [Mon, 9 Jan 2023 09:34:25 +0000 (10:34 +0100)] 
MEDIUM: h3: send SETTINGS before STREAM frames

Complete qcc_send_stream() function to allow to specify if the stream
should be handled in priority. Internally this will insert the qcs
instance in front of <qcc.send_list> to be able to treat it before other
streams.

This functionality is useful when some QUIC streams should be sent
before others. Most notably, this is used to guarantee that H3 SETTINGS
is done first via the control stream.

This must be backported up to 2.7.

2 years agoMAJOR: mux-quic: rework stream sending priorization
Amaury Denoyelle [Tue, 3 Jan 2023 13:39:24 +0000 (14:39 +0100)] 
MAJOR: mux-quic: rework stream sending priorization

Implement a mechanism to register streams ready to send data in new
STREAM frames. Internally, this is implemented with a new list
<qcc.send_list> which contains qcs instances.

A qcs can be registered safely using the new function qcc_send_stream().
This is done automatically in qc_send_buf() which covers most cases.
Also, application layer is free to use it for internal usage streams.
This is currently the case for H3 control stream with SETTINGS sending.

The main point of this patch is to handle stream sending fairly. This is
in stark contrast with previous code where streams with lower ID were
always prioritized. This could cause other streams to be indefinitely
blocked behind a stream which has a lot of data to transfer. Now,
streams are handled in an order scheduled by se_desc layer.

This commit is the first one of a serie which will bring other
improvments which also relied on the send_list implementation.

This must be backported up to 2.7 when deemed sufficiently stable.

2 years agoMINOR: mux-quic: add traces for flow-control limit reach
Amaury Denoyelle [Fri, 6 Jan 2023 14:29:59 +0000 (15:29 +0100)] 
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.

2 years agoBUG/MINOR: mux-quic: fix transfer of empty HTTP response
Amaury Denoyelle [Tue, 10 Jan 2023 09:41:41 +0000 (10:41 +0100)] 
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.

2 years agoDOC: management: add details about @system-ca in "show ssl ca-file"
William Lallemand [Tue, 10 Jan 2023 14:07:12 +0000 (15:07 +0100)] 
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.

2 years agoDOC: management: add details on "Used" status
William Lallemand [Tue, 10 Jan 2023 13:44:27 +0000 (14:44 +0100)] 
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.

2 years agoMINOR: channel/applets: Stop to test CF_WRITE_ERROR flag if CF_SHUTW is enough
Christopher Faulet [Wed, 4 Jan 2023 13:11:10 +0000 (14:11 +0100)] 
MINOR: channel/applets: Stop to test CF_WRITE_ERROR flag if CF_SHUTW is enough

In applets, we stop processing when a write error (CF_WRITE_ERROR) or a shutdown
for writes (CF_SHUTW) is detected. However, any write error leads to an
immediate shutdown for writes. Thus, it is enough to only test if CF_SHUTW is
set.

2 years agoMINOR: channel: Stop to test CF_READ_ERROR flag if CF_SHUTR is enough
Christopher Faulet [Wed, 4 Jan 2023 10:55:15 +0000 (11:55 +0100)] 
MINOR: channel: Stop to test CF_READ_ERROR flag if CF_SHUTR is enough

When a read error (CF_READ_ERROR) is reported, a shutdown for reads is
always performed (CF_SHUTR). Thus, there is no reason to check if
CF_READ_ERROR is set if CF_SHUTR is also checked.

2 years agoMEDIUM: channel: Remove CF_READ_ATTACHED and report CF_READ_EVENT instead
Christopher Faulet [Tue, 20 Dec 2022 17:47:39 +0000 (18:47 +0100)] 
MEDIUM: channel: Remove CF_READ_ATTACHED and report CF_READ_EVENT instead

CF_READ_ATTACHED flag is only used in input events for stream analyzers,
CF_MASK_ANALYSER. A read event can be reported instead and this flag can be
removed. We must only take care to report a read event when the client
connection is upgraded from TCP to HTTP.

2 years agoMINOR: channel: Remove CF_ANA_TIMEOUT and report CF_READ_EVENT instead
Christopher Faulet [Tue, 20 Dec 2022 17:28:27 +0000 (18:28 +0100)] 
MINOR: channel: Remove CF_ANA_TIMEOUT and report CF_READ_EVENT instead

It appears CF_ANA_TIMEOUT is flag only used in CF_MASK_ANALYSER. All
analyzer timeout relies on the analysis expiration date (chn->analyse_exp).
Worst, once set, this flag is never removed. Thus this flag can be removed
and replaced by a read event (CF_READ_EVENT).

2 years agoMINOR: channel: Remove CF_WRITE_ACTIVITY
Christopher Faulet [Tue, 20 Dec 2022 17:18:00 +0000 (18:18 +0100)] 
MINOR: channel: Remove CF_WRITE_ACTIVITY

Thanks to previous changes, CF_WRITE_ACTIVITY flags can be removed.
Everywhere it was used, its value is now directly used
(CF_WRITE_EVENT|CF_WRITE_ERROR).

2 years agoMINOR: channel: Remove CF_READ_ACTIVITY
Christopher Faulet [Tue, 20 Dec 2022 17:14:56 +0000 (18:14 +0100)] 
MINOR: channel: Remove CF_READ_ACTIVITY

Thanks to previous changes, CF_READ_ACTIVITY flags can be removed.
Everywhere it was used, its value is now directly used
(CF_READ_EVENT|CF_READ_ERROR).

2 years agoMEDIUM: channel: Use CF_WRITE_EVENT instead of CF_WRITE_PARTIAL
Christopher Faulet [Tue, 20 Dec 2022 17:10:04 +0000 (18:10 +0100)] 
MEDIUM: channel: Use CF_WRITE_EVENT instead of CF_WRITE_PARTIAL

Just like CF_READ_PARTIAL, CF_WRITE_PARTIAL is now merged with
CF_WRITE_EVENT. There a subtlety in sc_notify(). The "connect" event
(formely CF_WRITE_NULL) is now detected with
(CF_WRITE_EVENT + sc->state < SC_ST_EST).

2 years agoMEDIUM: channel: Use CF_READ_EVENT instead of CF_READ_PARTIAL
Christopher Faulet [Mon, 12 Dec 2022 07:28:55 +0000 (08:28 +0100)] 
MEDIUM: channel: Use CF_READ_EVENT instead of CF_READ_PARTIAL

CF_READ_PARTIAL flag is now merged with CF_READ_EVENT. It means
CF_READ_EVENT is set when a read0 is received (formely CF_READ_NULL) or when
data are received (formely CF_READ_ACTIVITY).

There is nothing special here, except conditions to wake the stream up in
sc_notify(). Indeed, the test was a bit changed to reflect recent
change. read0 event is now formalized by (CF_READ_EVENT + CF_SHUTR).

2 years agoREORG: channel: Rename CF_WRITE_NULL to CF_WRITE_EVENT
Christopher Faulet [Mon, 12 Dec 2022 07:11:36 +0000 (08:11 +0100)] 
REORG: channel: Rename CF_WRITE_NULL to CF_WRITE_EVENT

As for CF_READ_NULL, it appears CF_WRITE_NULL and other write events on a
channel are mainly used to wake up the stream and may be replace by on write
event.

In this patch, we introduce CF_WRITE_EVENT flag as a replacement to
CF_WRITE_EVENT_NULL. There is no breaking change for now, it is just a
rename. Gradually, other write events will be merged with this one.

2 years agoREORG: channel: Rename CF_READ_NULL to CF_READ_EVENT
Christopher Faulet [Mon, 12 Dec 2022 07:08:15 +0000 (08:08 +0100)] 
REORG: channel: Rename CF_READ_NULL to CF_READ_EVENT

CF_READ_NULL flag is not really useful and used. It is a transient event
used to wakeup the stream. As we will see, all read events on a channel may
be resumed to only one and are all used to wake up the stream.

In this patch, we introduce CF_READ_EVENT flag as a replacement to
CF_READ_NULL. There is no breaking change for now, it is just a
rename. Gradually, other read events will be merged with this one.

2 years agoMINOR: channel: Don't test CF_READ_NULL while CF_SHUTR is enough
Christopher Faulet [Mon, 12 Dec 2022 06:53:18 +0000 (07:53 +0100)] 
MINOR: channel: Don't test CF_READ_NULL while CF_SHUTR is enough

If CF_READ_NULL flag is set on a channel, it implies a shutdown for reads
was performed and CF_SHUTR is also set on this channel. Thus, there is no
reason to test is any of these flags is present, testing CF_SHUTR is enough.

2 years agoMINOR: ssl: Remove mention of ckch_store in error message of cli command
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:49 +0000 (12:02 +0100)] 
MINOR: ssl: Remove mention of ckch_store in error message of cli command

When calling 'update ssl ocsp-response' with an unknown certificate file
name, the error message would mention a "ckch_store" which is an
internal structure unknown by users.

2 years agoMINOR: ssl: Limit ocsp_uri buffer size to minimum
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:48 +0000 (12:02 +0100)] 
MINOR: ssl: Limit ocsp_uri buffer size to minimum

The ocsp_uri field of the certificate_ocsp structure was a 16k buffer
when it could be hand allocated to just the required size to store the
OCSP uri. This field is now behaving the same way as the sctl and
ocsp_response buffers of the ckch_store structure.

2 years agoBUG/MINOR: ssl: Fix OCSP_CERTID leak when same certificate is used multiple times
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:47 +0000 (12:02 +0100)] 
BUG/MINOR: ssl: Fix OCSP_CERTID leak when same certificate is used multiple times

If a given certificate is used multiple times in a configuration, the
ocsp_cid field would have been overwritten during each
ssl_sock_load_ocsp call even if it was previously filled.

This patch does not need to be backported.

2 years agoMINOR: ssl: Detect more OCSP update inconsistencies
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:46 +0000 (12:02 +0100)] 
MINOR: ssl: Detect more OCSP update inconsistencies

If a configuration such as the following was included in a crt-list
file, it would not have raised a warning about 'ocsp-update'
inconsistencies for the concerned certificate:
    cert.pem [ocsp-update on]
    cert.pem
because the second line as a NULL entry->ssl_conf.

2 years agoMINOR: ssl: Release ssl_ocsp_task_ctx.cur_ocsp when destroying task
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:45 +0000 (12:02 +0100)] 
MINOR: ssl: Release ssl_ocsp_task_ctx.cur_ocsp when destroying task

In the unlikely event that the OCSP udpate task is killed in the middle
of an update process (request sent but no response received yet) the
cur_ocsp member of the update context would keep an unneeded reference
to a certificate_ocsp object. It must then be freed during the task's
cleanup.

2 years agoMINOR: ssl: Only set ocsp->issuer if issuer not in cert chain
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:44 +0000 (12:02 +0100)] 
MINOR: ssl: Only set ocsp->issuer if issuer not in cert chain

If the ocsp issuer certificate was actually taken from the certificate
chain in ssl_sock_load_ocsp, we don't need to keep an extra reference on
it since we already keep a reference to the full certificate chain.

2 years agoMINOR: ssl: Create temp X509_STORE filled with cert chain when checking ocsp response
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:43 +0000 (12:02 +0100)] 
MINOR: ssl: Create temp X509_STORE filled with cert chain when checking ocsp response

When calling OCSP_basic_verify to check the validity of the received
OCSP response, we need to provide an untrusted certificate chain as well
as an X509_STORE holding only trusted certificates. Since the
certificate chain and the issuer certificate are all provided by the
user, we assume that they are valid and we add them all to a temporary
store. This enables to focus only on the response's validity.

2 years agoBUG/MINOR: ssl: Crash during cleanup because of ocsp structure pointer UAF
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:42 +0000 (12:02 +0100)] 
BUG/MINOR: ssl: Crash during cleanup because of ocsp structure pointer UAF

When ocsp-update is enabled for a given certificate, its
certificate_ocsp objects is inserted in two separate trees (the actual
ocsp response one and the ocsp update one). But since the same instance
is used for the two trees, its ownership is kept by the regular ocsp
response one. The ocsp update task should then never have to free the
ocsp entries. The crash actually occurred because of this. The update
task was freeing entries whose reference counter was not increased while
a reference was still held by the SSL_CTXs.
The only time during which the ocsp update task will need to increase
the reference counter is during an actual update, because at this moment
the entry is taken out of the update tree and a 'flying' reference to
the certificate_ocsp is kept in the ocsp update context.

This bug could be reproduced by calling './haproxy -f conf.cfg -c' with
any of the used certificates having the 'ocsp-update on' option. For
some reason asan caught the bug easily but valgrind did not.

This patch does not need to be backported.

2 years agoBUG/MINOR: ssl: Fix crash in 'update ssl ocsp-response' CLI command
Remi Tricot-Le Breton [Mon, 9 Jan 2023 11:02:41 +0000 (12:02 +0100)] 
BUG/MINOR: ssl: Fix crash in 'update ssl ocsp-response' CLI command

This CLI command crashed when called for a certificate which did not
have an OCSP response during startup because it assumed that the
ocsp_issuer pointer of the ckch_data object would be valid. It was only
true for already known OCSP responses though.
The ocsp issuer certificate is now taken either from the ocsp_issuer
pointer or looked for in the certificate chain. This is the same logic
as the one in ssl_sock_load_ocsp.

This patch does not need to be backported.

2 years agoDOC: config: added optional rst-ttl argument to silent-drop in action lists
Mathias Weiersmueller [Mon, 9 Jan 2023 12:52:06 +0000 (13:52 +0100)] 
DOC: config: added optional rst-ttl argument to silent-drop in action lists

This patch adds the optional silent-drop rst-ttl argument to the action lists in
5 places in the configuration manual.

2 years agoCLEANUP: htx: fix a typo in an error message of http_str_to_htx
Manu Nicolas [Mon, 9 Jan 2023 01:31:06 +0000 (01:31 +0000)] 
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.

2 years ago[RELEASE] Released version 2.8-dev1 v2.8-dev1
Willy Tarreau [Sat, 7 Jan 2023 08:45:17 +0000 (09:45 +0100)] 
[RELEASE] Released version 2.8-dev1

Released version 2.8-dev1 with the following main changes :
    - MEDIUM: 51d: add support for 51Degrees V4 with Hash algorithm
    - MINOR: debug: support pool filtering on "debug dev memstats"
    - MINOR: debug: add a balance of alloc - free at the end of the memstats dump
    - LICENSE: wurfl: clarify the dummy library license.
    - MINOR: event_hdl: add event handler base api
    - DOC/MINOR: api: add documentation for event_hdl feature
    - MEDIUM: ssl: rename the struct "cert_key_and_chain" to "ckch_data"
    - MINOR: quic: remove qc from quic_rx_packet
    - MINOR: quic: complete traces in qc_rx_pkt_handle()
    - MINOR: quic: extract datagram parsing code
    - MINOR: tools: add port for ipcmp as optional criteria
    - MINOR: quic: detect connection migration
    - MINOR: quic: ignore address migration during handshake
    - MINOR: quic: startup detect for quic-conn owned socket support
    - MINOR: quic: test IP_PKTINFO support for quic-conn owned socket
    - MINOR: quic: define config option for socket per conn
    - MINOR: quic: allocate a socket per quic-conn
    - MINOR: quic: use connection socket for emission
    - MEDIUM: quic: use quic-conn socket for reception
    - MEDIUM: quic: move receive out of FD handler to quic-conn io-cb
    - MINOR: mux-quic: rename duplicate function names
    - MEDIUM: quic: requeue datagrams received on wrong socket
    - MINOR: quic: reconnect quic-conn socket on address migration
    - MINOR: quic: activate socket per conn by default
    - BUG/MINOR: ssl: initialize SSL error before parsing
    - BUG/MINOR: ssl: initialize WolfSSL before parsing
    - BUG/MINOR: quic: fix fd leak on startup check quic-conn owned socket
    - BUG/MEDIIM: stconn: Flush output data before forwarding close to write side
    - MINOR: server: add srv->rid (revision id) value
    - MINOR: stats: add server revision id support
    - MINOR: server/event_hdl: add support for SERVER_ADD and SERVER_DEL events
    - MINOR: server/event_hdl: add support for SERVER_UP and SERVER_DOWN events
    - BUG/MEDIUM: checks: do not reschedule a possibly running task on state change
    - BUG/MINOR: checks: make sure fastinter is used even on forced transitions
    - CLEANUP: assorted typo fixes in the code and comments
    - MINOR: mworker: display an alert upon a wait-mode exit
    - BUG/MEDIUM: mworker: fix segv in early failure of mworker mode with peers
    - BUG/MEDIUM: mworker: create the mcli_reload socketpairs in case of upgrade
    - BUG/MINOR: checks: restore legacy on-error fastinter behavior
    - MINOR: check: use atomic for s->consecutive_errors
    - MINOR: stats: properly handle ST_F_CHECK_DURATION metric
    - MINOR: mworker: remove unused legacy code in mworker_cleanlisteners
    - MINOR: peers: unused code path in process_peer_sync
    - BUG/MINOR: init/threads: continue to limit default thread count to max per group
    - CLEANUP: init: remove useless assignment of nbthread
    - BUILD: atomic: atomic.h may need compiler.h on ARMv8.2-a
    - BUILD: makefile/da: also clean Os/ in Device Atlas dummy lib dir
    - BUG/MEDIUM: httpclient/lua: double LIST_DELETE on end of lua task
    - CLEANUP: pools: move the write before free to the uaf-only function
    - CLEANUP: pool: only include pool-os from pool.c not pool.h
    - REORG: pool: move all the OS specific code to pool-os.h
    - CLEANUP: pools: get rid of CONFIG_HAP_POOLS
    - DEBUG: pool: show a few examples in -dMhelp
    - MINOR: pools: make DEBUG_UAF a runtime setting
    - BUG/MINOR: promex: create haproxy_backend_agg_server_status
    - MINOR: promex: introduce haproxy_backend_agg_check_status
    - DOC: promex: Add missing backend metrics
    - BUG/MAJOR: fcgi: Fix uninitialized reserved bytes
    - REGTESTS: fix the race conditions in iff.vtc
    - CI: github: reintroduce openssl 1.1.1
    - BUG/MINOR: quic: properly handle alloc failure in qc_new_conn()
    - BUG/MINOR: quic: handle alloc failure on qc_new_conn() for owned socket
    - CLEANUP: mux-quic: remove unused attribute on qcs_is_close_remote()
    - BUG/MINOR: mux-quic: remove qcs from opening-list on free
    - BUG/MINOR: mux-quic: handle properly alloc error in qcs_new()
    - CI: github: split ssl lib selection based on git branch
    - REGTESTS: startup: check maxconn computation
    - BUG/MINOR: startup: don't use internal proxies to compute the maxconn
    - REGTESTS: startup: change the expected maxconn to 11000
    - CI: github: set ulimit -n to a greater value
    - REGTESTS: startup: activate automatic_maxconn.vtc
    - MINOR: sample: add param converter
    - CLEANUP: ssl: remove check on srv->proxy
    - BUG/MEDIUM: freq-ctr: Don't compute overshoot value for empty counters
    - BUG/MEDIUM: resolvers: Use tick_first() to update the resolvers task timeout
    - REGTESTS: startup: add alternatives values in automatic_maxconn.vtc
    - BUG/MEDIUM: h3: reject request with invalid header name
    - BUG/MEDIUM: h3: reject request with invalid pseudo header
    - MINOR: http: extract content-length parsing from H2
    - BUG/MEDIUM: h3: parse content-length and reject invalid messages
    - CI: github: remove redundant ASAN loop
    - CI: github: split matrix for development and stable branches
    - BUG/MEDIUM: mux-h1: Don't release H1 stream upgraded from TCP on error
    - BUG/MINOR: mux-h1: Fix test instead a BUG_ON() in h1_send_error()
    - MINOR: http-htx: add BUG_ON to prevent API error on http_cookie_register
    - BUG/MEDIUM: h3: fix cookie header parsing
    - BUG/MINOR: h3: fix memleak on HEADERS parsing failure
    - MINOR: h3: check return values of htx_add_* on headers parsing
    - MINOR: ssl: Remove unneeded buffer allocation in show ocsp-response
    - MINOR: ssl: Remove unnecessary alloc'ed trash chunk in show ocsp-response
    - BUG/MINOR: ssl: Fix memory leak of find_chain in ssl_sock_load_cert_chain
    - MINOR: stats: provide ctx for dumping functions
    - MINOR: stats: introduce stats field ctx
    - BUG/MINOR: stats: fix show stat json buffer limitation
    - MINOR: stats: make show info json future-proof
    - BUG/MINOR: quic: fix crash on PTO rearm if anti-amplification reset
    - BUILD: 51d: fix build issue with recent compilers
    - REGTESTS: startup: disable automatic_maxconn.vtc
    - BUILD: peers: peers-t.h depends on stick-table-t.h
    - BUG/MEDIUM: tests: use tmpdir to create UNIX socket
    - BUG/MINOR: mux-h1: Report EOS on parsing/internal error for not running stream
    - BUG/MINOR:: mux-h1: Never handle error at mux level for running connection
    - BUG/MEDIUM: stats: Rely on a local trash buffer to dump the stats
    - OPTIM: pool: split the read_mostly from read_write parts in pool_head
    - MINOR: pool: make the thread-local hot cache size configurable
    - MINOR: freq_ctr: add opportunistic versions of swrate_add()
    - MINOR: pool: only use opportunistic versions of the swrate_add() functions
    - REGTESTS: ssl: enable the ssl_reuse.vtc test for WolfSSL
    - BUG/MEDIUM: mux-quic: fix double delete from qcc.opening_list
    - BUG/MEDIUM: quic: properly take shards into account on bind lines
    - BUG/MINOR: quic: do not allocate more rxbufs than necessary
    - MINOR: ssl: Add a lock to the OCSP response tree
    - MINOR: httpclient: Make the CLI flags public for future use
    - MINOR: ssl: Add helper function that extracts an OCSP URI from a certificate
    - MINOR: ssl: Add OCSP request helper function
    - MINOR: ssl: Add helper function that checks the validity of an OCSP response
    - MINOR: ssl: Add "update ssl ocsp-response" cli command
    - MEDIUM: ssl: Add ocsp_certid in ckch structure and discard ocsp buffer early
    - MINOR: ssl: Add ocsp_update_tree and helper functions
    - MINOR: ssl: Add crt-list ocsp-update option
    - MINOR: ssl: Store 'ocsp-update' mode in the ckch_data and check for inconsistencies
    - MEDIUM: ssl: Insert ocsp responses in update tree when needed
    - MEDIUM: ssl: Add ocsp update task main function
    - MEDIUM: ssl: Start update task if at least one ocsp-update option is set to on
    - DOC: ssl: Add documentation for ocsp-update option
    - REGTESTS: ssl: Add tests for ocsp auto update mechanism
    - MINOR: ssl: Move OCSP code to a dedicated source file
    - BUG/MINOR: ssl/ocsp: check chunk_strcpy() in ssl_ocsp_get_uri_from_cert()
    - CLEANUP: ssl/ocsp: add spaces around operators
    - BUG/MEDIUM: mux-h2: Refuse interim responses with end-stream flag set
    - BUG/MINOR: pool/stats: Use ullong to report total pool usage in bytes in stats
    - BUG/MINOR: ssl/ocsp: httpclient blocked when doing a GET
    - MINOR: httpclient: don't add body when istlen is empty
    - MEDIUM: httpclient: change the default log format to skip duplicate proxy data
    - BUG/MINOR: httpclient/log: free of invalid ptr with httpclient_log_format
    - MEDIUM: mux-quic: implement shutw
    - MINOR: mux-quic: do not count stream flow-control if already closed
    - MINOR: mux-quic: handle RESET_STREAM reception
    - MEDIUM: mux-quic: implement STOP_SENDING emission
    - MINOR: h3: use stream error when needed instead of connection
    - CI: github: enable github api authentication for OpenSSL tags read
    - BUG/MINOR: mux-quic: ignore remote unidirectional stream close
    - CI: github: use the GITHUB_TOKEN instead of a manually generated token
    - BUILD: makefile: build the features list dynamically
    - BUILD: makefile: move common options-oriented macros to include/make/options.mk
    - BUILD: makefile: sort the features list
    - BUILD: makefile: initialize all build options' variables at once
    - BUILD: makefile: add a function to collect all options' CFLAGS/LDFLAGS
    - BUILD: makefile: start to automatically collect CFLAGS/LDFLAGS
    - BUILD: makefile: ensure that all USE_* handlers appear before CFLAGS are used
    - BUILD: makefile: clean the wolfssl include and lib generation rules
    - BUILD: makefile: make sure to also ignore SSL_INC when using wolfssl
    - BUILD: makefile: reference libdl only once
    - BUILD: makefile: make sure LUA_INC and LUA_LIB are always initialized
    - BUILD: makefile: do not restrict Lua's prepend path to empty LUA_LIB_NAME
    - BUILD: makefile: never force -latomic, set USE_LIBATOMIC instead
    - BUILD: makefile: add an implicit USE_MATH variable for -lm
    - BUILD: makefile: properly report USE_PCRE/USE_PCRE2 in features
    - CLEANUP: makefile: properly indent ifeq/ifneq conditional blocks
    - BUILD: makefile: rework 51D to split v3/v4
    - BUILD: makefile: support LIBCRYPT_LDFLAGS
    - BUILD: makefile: support RT_LDFLAGS
    - BUILD: makefile: support THREAD_LDFLAGS
    - BUILD: makefile: support BACKTRACE_LDFLAGS
    - BUILD: makefile: support SYSTEMD_LDFLAGS
    - BUILD: makefile: support ZLIB_CFLAGS and ZLIB_LDFLAGS
    - BUILD: makefile: support ENGINE_CFLAGS
    - BUILD: makefile: support OPENSSL_CFLAGS and OPENSSL_LDFLAGS
    - BUILD: makefile: support WOLFSSL_CFLAGS and WOLFSSL_LDFLAGS
    - BUILD: makefile: support LUA_CFLAGS and LUA_LDFLAGS
    - BUILD: makefile: support DEVICEATLAS_CFLAGS and DEVICEATLAS_LDFLAGS
    - BUILD: makefile: support PCRE[2]_CFLAGS and PCRE[2]_LDFLAGS
    - BUILD: makefile: refactor support for 51DEGREES v3/v4
    - BUILD: makefile: support WURFL_CFLAGS and WURFL_LDFLAGS
    - BUILD: makefile: make all OpenSSL variants use the same settings
    - BUILD: makefile: remove the special case of the SSL option
    - BUILD: makefile: only consider settings from enabled options
    - BUILD: makefile: also list per-option settings in 'make opts'
    - BUG/MINOR: debug: don't mask the TH_FL_STUCK flag before dumping threads
    - MINOR: cfgparse-ssl: avoid a possible crash on OOM in ssl_bind_parse_npn()
    - BUG/MINOR: ssl: Missing goto in error path in ocsp update code
    - BUG/MINOR: stick-table: report the correct action name in error message
    - CI: Improve headline in matrix.py
    - CI: Add in-memory cache for the latest OpenSSL/LibreSSL
    - CI: Use proper `if` blocks instead of conditional expressions in matrix.py
    - CI: Unify the `GITHUB_TOKEN` name across matrix.py and vtest.yml
    - CI: Explicitly check environment variable against `None` in matrix.py
    - CI: Reformat `matrix.py` using `black`
    - MINOR: config: add environment variables for default log format
    - REGTESTS: Remove REQUIRE_VERSION=1.9 from all tests
    - REGTESTS: Remove REQUIRE_VERSION=2.0 from all tests
    - REGTESTS: Remove tests with REQUIRE_VERSION_BELOW=1.9
    - BUG/MINOR: http-fetch: Only fill txn status during prefetch if not already set
    - BUG/MAJOR: buf: Fix copy of wrapping output data when a buffer is realigned
    - DOC: config: fix alphabetical ordering of http-after-response rules
    - MINOR: http-rules: Add missing actions in http-after-response ruleset
    - DOC: config: remove duplicated "http-response sc-set-gpt0" directive
    - BUG/MINOR: proxy: free orgto_hdr_name in free_proxy()
    - REGTEST: fix the race conditions in json_query.vtc
    - REGTEST: fix the race conditions in add_item.vtc
    - REGTEST: fix the race conditions in digest.vtc
    - REGTEST: fix the race conditions in hmac.vtc
    - BUG/MINOR: fd: avoid bad tgid assertion in fd_delete() from deinit()
    - BUG/MINOR: http: Memory leak of http redirect rules' format string
    - MEDIUM: stick-table: set the track-sc limit at boottime via tune.stick-counters
    - MINOR: stick-table: implement the sc-add-gpc() action

2 years agoMINOR: stick-table: implement the sc-add-gpc() action
Willy Tarreau [Mon, 2 Jan 2023 17:15:20 +0000 (18:15 +0100)] 
MINOR: stick-table: implement the sc-add-gpc() action

This action increments the General Purpose Counter at the index <idx> of
the array associated to the sticky counter designated by <sc-id> by the
value of either integer <int> or the integer evaluation of expression
<expr>. Integers and expressions are limited to unsigned 32-bit values.
If an error occurs, this action silently fails and the actions evaluation
continues. <idx> is an integer between 0 and 99 and <sc-id> is an integer
between 0 and 2. It also silently fails if the there is no GPC stored at
this index. The entry in the table is refreshed even if the value is zero.
The 'gpc_rate' is automatically adjusted to reflect the average growth
rate of the gpc value.

The main use of this action is to count scores or total volumes (e.g.
estimated danger per source IP reported by the server or a WAF, total
uploaded bytes, etc).

2 years agoMEDIUM: stick-table: set the track-sc limit at boottime via tune.stick-counters
Willy Tarreau [Fri, 6 Jan 2023 15:09:58 +0000 (16:09 +0100)] 
MEDIUM: stick-table: set the track-sc limit at boottime via tune.stick-counters

The number of stick-counter entries usable by track-sc rules is currently
set at build time. There is no good value for this since the vast majority
of users don't need any, most need only a few and rare users need more.
Adding more counters for everyone increases memory and CPU usages for no
reason.

This patch moves the per-session and per-stream arrays to a pool of a size
defined at boot time. This way it becomes possible to set the number of
entries at boot time via a new global setting "tune.stick-counters" that
sets the limit for the whole process. When not set, the MAX_SESS_STR_CTR
value still applies, or 3 if not set, as before.

It is also possible to lower the value to 0 to save a bit of memory if
not used at all.

Note that a few low-level sample-fetch functions had to be protected due
to the ability to use sample-fetches in the global section to set some
variables.

2 years agoBUG/MINOR: http: Memory leak of http redirect rules' format string
Remi Tricot-Le Breton [Fri, 6 Jan 2023 15:31:06 +0000 (16:31 +0100)] 
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.

2 years agoBUG/MINOR: fd: avoid bad tgid assertion in fd_delete() from deinit()
Willy Tarreau [Thu, 5 Jan 2023 17:06:58 +0000 (18:06 +0100)] 
BUG/MINOR: fd: avoid bad tgid assertion in fd_delete() from deinit()

In 2.7, commit 0dc1cc93b ("MAJOR: fd: grab the tgid before manipulating
running") added a check to make sure we never try to delete an FD from
the wrong thread group. It already handles the specific case of an
isolated thread (e.g. stop a listener from the CLI) but forgot to take
into account the deinit() code iterating over all idle server connections
to close them. This results in the crash below during deinit() if thread
groups are enabled and idle connections exist on a thread group higher
than 1.

  [WARNING]  (15711) : Proxy decrypt stopped (cumulated conns: FE: 64, BE: 374511).
  [WARNING]  (15711) : Proxy stats stopped (cumulated conns: FE: 0, BE: 0).
  [WARNING]  (15711) : Proxy GLOBAL stopped (cumulated conns: FE: 0, BE: 0).

  FATAL: bug condition "fd_tgid(fd) != ti->tgid && !thread_isolated()" matched at src/fd.c:369
    call trace(11):
    |       0x4a6060 [c6 04 25 01 00 00 00 00]: main-0x1d60
    |       0x67fcc6 [c7 43 68 fd ad de fd 5b]: sock_conn_ctrl_close+0x16/0x1f
    |       0x59e6f5 [48 89 ef e8 83 65 11 00]: main+0xf6935
    |       0x60ad16 [48 8b 1b 48 81 fb a0 91]: free_proxy+0x716/0xb35
    |       0x62750e [48 85 db 74 35 48 89 dd]: deinit+0xbe/0x87a
    |       0x627ce2 [89 ef e8 97 76 e7 ff 0f]: deinit_and_exit+0x12/0x19
    |       0x4a9694 [bf e6 ff 9d 00 44 89 6c]: main+0x18d4/0x2c1a

There's no harm though since all traffic already ended. This must be
backported to 2.7.

2 years agoREGTEST: fix the race conditions in hmac.vtc
Aurelien DARRAGON [Mon, 2 Jan 2023 14:03:20 +0000 (15:03 +0100)] 
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.

2 years agoREGTEST: fix the race conditions in digest.vtc
Aurelien DARRAGON [Mon, 2 Jan 2023 14:02:10 +0000 (15:02 +0100)] 
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.

2 years agoREGTEST: fix the race conditions in add_item.vtc
Aurelien DARRAGON [Mon, 2 Jan 2023 13:59:50 +0000 (14:59 +0100)] 
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.

2 years agoREGTEST: fix the race conditions in json_query.vtc
Aurelien DARRAGON [Mon, 2 Jan 2023 13:54:31 +0000 (14:54 +0100)] 
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.

2 years agoBUG/MINOR: proxy: free orgto_hdr_name in free_proxy()
Aurelien DARRAGON [Wed, 28 Dec 2022 11:18:15 +0000 (12:18 +0100)] 
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.

2 years agoDOC: config: remove duplicated "http-response sc-set-gpt0" directive
Christopher Faulet [Thu, 5 Jan 2023 10:24:55 +0000 (11:24 +0100)] 
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.

2 years agoMINOR: http-rules: Add missing actions in http-after-response ruleset
Christopher Faulet [Thu, 5 Jan 2023 10:17:38 +0000 (11:17 +0100)] 
MINOR: http-rules: Add missing actions in http-after-response ruleset

This patch adds the support of following actions in the http-after-response
ruleset:

  * set-map, del-map and del-acl
  * set-log-level
  * sc-inc-gpc, sc-inc-gpc0 and set-inc-gpc1
  * sc-inc-gpt and sc-set-gpt0

This patch should solve the issue #1980.