BUG/MEDIUM: stream: Use the front analyzers for new listener-less streams

For now, for a stream, request analyzers are set at 2 stages. The first one
is when the stream is created. The session's listener analyzers, if any, are
set on the request channel. In addition, some HTTP analyzers are set for HTX
streams (AN_REQ_WAIT_HTTP and AN_REQ_HTTP_PROCESS_FE). The second one is
when the backend is set on the stream. At the stage, request analyzers are
updated using the backend settings.

It is an issue for client applets because there is no listener attached to
the stream. In addtion, it may have no specific/dedicated backend. Thus,
several request analyzers are missing. Among others, the HTTP analyzers for
HTTP applets. The HTTP client is the only one affected for now.

To fix the bug, when a stream is created without a listener, we use the
frontend to set the request analyzers. Note that there is no issue with the
response channel because its analyzers are set when the server connection is
established.

This patch may be backported to all stable versions. Because only the HTTP
client is affected, it must at least be backported to 2.5. It is related to
the issue #1593.

(cherry picked from commit e9382e0afe263c06fe4e7b1839e2c482b89ed42c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: promex: Set conn-stream/channel EOI flags at the end of request

This bug is the same than for the HTTP client. See "BUG/MINOR: httpclient:
Set conn-stream/channel EOI flags at the end of request" for details.

This patch must be backported as far as 2.0. But only CF_EOI must be set
because applets are not attached to a conn-stream on older versions.

(cherry picked from commit bef64b23b7e7b4cdcfa201f17053ee58f43c6802)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: cache: Set conn-stream/channel EOI flags at the end of request

This bug is the same than for the HTTP client. See "BUG/MINOR: httpclient:
Set conn-stream/channel EOI flags at the end of request" for details.

Note that because a filter is always attached to the stream when the cache
is used, there is no issue because there is no direct forwarding in this
case. Thus the stream analyzers are able to see the HTX_FL_EOM flag on the
HTX messge.

This patch must be backported as far as 2.0. But only CF_EOI must be set
because applets are not attached to a conn-stream on older versions.

(cherry picked from commit dbf1e88e87748aef2db25fcdfeb22a4005908d41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: stats: Set conn-stream/channel EOI flags at the end of request

This bug is the same than for the HTTP client. See "BUG/MINOR: httpclient:
Set conn-stream/channel EOI flags at the end of request" for details.

This patch must be backported as far as 2.0. But only CF_EOI must be set
because applets are not attached to a conn-stream on older versions.

(cherry picked from commit 3fa5d19d14e70986d366e7c21554a434c1daa64b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: hlua: Set conn-stream/channel EOI flags at the end of request

This bug is the same than for the HTTP client. See "BUG/MINOR: httpclient:
Set conn-stream/channel EOI flags at the end of request" for details.

This patch must be backported as far as 2.0. But only CF_EOI must be set
because applets are not attached to a conn-stream on older versions.

(cherry picked from commit d8d2708cfef417e3c4e2f627089b49e030ebbe80)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: httpclient: Set conn-stream/channel EOI flags at the end of request

In HTX, HTX_FL_EOM flag is added on the message to notifiy the end of the
message was received. In addition, the producer must set CS_FL_EOI flag on
the conn-stream. If it is a mux, the stream-interface is responsible to set
CF_EOI flag on the input channel. But, for now, if the producer is an
applet, in addition to the conn-stream flag, it must also set the channel
one.

These flags are used to notify the stream that the message is finished and
no more data are expected. It is especially important when the message
itself it directly forwarded from one side to the other. Because in this
case, the stream has no way to see the HTX_FL_EOM flag on the
message. Otherwise, the stream will detect a client or a server abort,
depending on the side.

For the HTTP client, it is not really easy to diagnose this error because
there is also another bug hiding this one. All HTTP request analyzers are
not set on the input channel. This will be fixed by another patch.

This patch must be backported to 2.5. It is related to the issue #1593.

(cherry picked from commit 3d4332419c7de882559506742c58050ecee15345)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: cli: shows correct mode in "show sess"

The "show sess" cli command only handles "http" or "tcp" as a fallback
mode, replace this by a call to proxy_mode_str() to show all the modes.

Could be backported in every maintained versions.

(cherry picked from commit b0dfd099c56b8f8fd46686120a6cf1598bd5e470)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: add missing modes in proxy_mode_str()

Add the missing PR_MODE_SYSLOG and PR_MODE_PEERS in proxy_mode_str().

Could be backported in every maintained versions.

(cherry picked from commit 06715af9e565c82f39aa157c09d5b9e823490d14)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: fix recent build breakage of freebsd caused by kFreeBSD build fix

Supporting kFreebsd previously led to FreeBSD (< 14) build breakage:

 In file included from src/cpuset.c:5:
 In file included from include/haproxy/cpuset.h:4:
 include/haproxy/cpuset-t.h:46:2: error: unknown type name 'cpu_set_t'; did you mean 'cpuset_t'?
         CPUSET_REPR cpuset;
         ^~~~~~~~~~~
         cpuset_t
 include/haproxy/cpuset-t.h:21:22: note: expanded from macro 'CPUSET_REPR'
 # define CPUSET_REPR cpu_set_t
                      ^

(cherry picked from commit 6709538068752ead403648ac5836f610d2da79e8)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: pools: fix backport of no-memory-trimming on non-linux OS

Commit 4bcff9ba4 in 2.5 ("MINOR: pools: add a new global option
"no-memory-trimming"") was missing some extra ifdefs for the cases
where the allocator doesn't have malloc_trim() and fails to build
on windows.

This must be backported to 2.4.

MINOR: stats: Add dark mode support for socket rows

In commit e9ed63e548 dark mode support was added to the stats page. The
initial commit does not include  dark mode color overwrites for the
.socket CSS class. This commit colors socket rows the same way as
backends that acre active but do not have a health check defined.

This fixes an issue where reading information from socket lines became
really hard in dark mode due to suboptimal coloring of the cell
background and the font in it.

(cherry picked from commit a690b73fba4d09ff32bc9c1e19c321ae0649a7cd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: pools: add a new global option "no-memory-trimming"

Some users with very large numbers of connections have been facing
extremely long malloc_trim() calls on reload that managed to trigger
the watchdog! That's a bit counter-productive. It's even possible
that some implementations are not perfectly reliable or that their
trimming time grows quadratically with the memory used. Instead of
constantly trying to work around these issues, let's offer an option
to disable this mechanism, since nobody had been complaining in the
past, and this was only meant to be an improvement.

This should be backported to 2.4 where trimming on reload started to
appear.

(cherry picked from commit c4e56dc58c9ada7c4a8d585cb117a5b825916002)
[wt: minor context adj]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: fix kFreeBSD build.

kFreeBSD needs to be treated as a distinct target from FreeBSD
since the underlying system libc is the GNU one. Thus, relying
only on __GLIBC__ no longer suffice.

- freebsd-glibc new target, key difference is including crypt.h
  and linking to libdl like linux.
- cpu affinity available but the api is still the FreeBSD's.
- enabling auxiliary data access only for Linux.

Patch based on preliminary work done by @bigon.

closes #1555

(cherry picked from commit 43a568575f9c14ce57ab437602dbdc089574cc33)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: pools: fix ha_free() on area in the process of being freed

Commit e81248c0c ("BUG/MINOR: pool: always align pool_heads to 64 bytes")
added a free of the allocated pool in pool_destroy() using ha_free(), but
it added a subtle bug by which once the pool is released, setting its
address to NULL inside the structure itself cannot work because the area
has just been freed.

This will need to be backported wherever the patch above is backported.

(cherry picked from commit f9eba78fb82a9ccca2503e318a79be4d7db8d94d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: pool: always align pool_heads to 64 bytes

This is the pool equivalent of commit 97ea9c49f ("BUG/MEDIUM: fd: always
align fdtab[] to 64 bytes"). After a careful code review, it happens that
the pool heads are the other structures allocated with malloc/calloc that
claim to be aligned to a size larger than what the allocator can offer.
While no issue was reported on them, no memset() is performed and no type
is large, this is a problem waiting to happen, so better fix it. In
addition, it's relatively easy to do by storing the allocation address
inside the pool_head itself and use it at free() time. Finally, threads
might benefit from the fact that the caches will really be aligned and
that there will be no false sharing.

This should be backported to all versions where it applies easily.

(cherry picked from commit e81248c0c8c543ab5065b41310e7c9685f61cc87)
[wt: context adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: httpclient/lua: infinite appctx loop with POST

When POSTing a request with a payload, and reusing the same httpclient
lua instance, one could encounter a spinning of the httpclient appctx.

Indeed the sent counter is not reset between 2 POSTs and the condition
for sending the EOM flag is never met.

Must fixed issue #1593.

To be backported in 2.5.

(cherry picked from commit 10a37360c8c5dfc4a65aba81f67ae63fe5fa2161)
[wla: context changes]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

REGTESTS: fix the race conditions in secure_memcmp.vtc

In the same way than for normalize_uri.vtc, a "Connection: close" header is
added to all responses to avoid any connection reuse. This should avoid any
"HTTP header incomplete" errors.

(cherry picked from commit 0dc70ab799f55f044f976b325f64f8ac246585e2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

REGTESTS: fix the race conditions in normalize_uri.vtc

There is no connection reuse to avoid race conditions in HTTP reg-tests. But
time to time, normalize_uri.vtc still report "HTTP header incomplete"
error. It seems to be because HTTP keep-alive is still used at the session
level. Thus when the same server section is used to handle multiple requests
for the same client, via a "-repeat" statement, a new request for this client
may be handled by HAProxy before the server is restarted.

To avoid any trouble, HTTP keep-alive is disabled on the server side by
adding "Connection: close" header in responses. It seems to be ok now. We
let the CI decide.

(cherry picked from commit e07f8b555267b49fe6c6afbf067430f97d5ecbf7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: htx: Fix a possible null derefs in htx_xfer_blks()

In htx_xfer_blks() function, when headers or trailers are partially
transferred, we rollback the copy by removing copied blocks. Internally, all
blocks between <dstref> and <dstblk> are removed. But if the transfer was
stopped because we failed to reserve a block, the variable <dstblk> is
NULL. Thus, we must not try to remove it. It is unexpected to call
htx_remove_blk() in this case.

htx_remove_blk() was updated to test <blk> variable inside the existing
BUG_ON(). The block must be defined.

For now, this bug may only be encountered when H2 trailers are copied. On H2
headers, the destination buffer is empty. Thus a swap is performed.

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

(cherry picked from commit 234a10aa9bce34781b5b0b4c4ffb595e7563eb41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-fcgi: Don't rely on SI src/dst addresses for FCGI health-checks

When an HTTP health-check is performed in FCGI, we must not rely on the SI
source and destination addresses to set default parameters
(REMOTE_ADDR/REMOTE_PORT and SERVER_NAME/SERVER_PORT) because the backend
conn-stream is not attached to a stream but to a healt-check. Thus, there is
no stream-interface. In addition, there is no client connection because it
is an "internal" session.

Thus, for now, in this case, there is only the server connection that can be
used. So src/dst addresses are retrieved from the server connection when the
CS application is a health-check.

This patch should solve issue #1572. It must be backported to 2.5. Note than
the CS api has changed. Thus, on HAProxy 2.5, we should test the session's
origin instead:

    const struct sockaddr_storage *src = (cs_check(fstrm->cs) ? ...);
    const struct sockaddr_storage *dst = (cs_check(fstrm->cs) ? ...);

(cherry picked from commit 4ab8438362e9c37338350984afb76b393769f998)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: tree-wide: mark a few numeric constants as explicitly long long

At a few places in the code the switch/case ond flags are tested against
64-bit constants without explicitly being marked as long long. Some
32-bit compilers complain that the constant is too large for a long, and
other likely always use long long there. Better fix that as it's uncertain
what others which do not complain do. It may be backported to avoid doubts
on uncommon platforms if needed, as it touches very few areas.

(cherry picked from commit 8f0b4e97e78f62eac43e240155192131e7c6d072)
[wt: 2.5 was reporting warnings on a 32-bit platform with gcc-4.4]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: atomic: make the old HA_ATOMIC_LOAD() support const pointers

We have an implementation of atomic ops for older versions of gcc that
do not provide the __builtin_* API (< 4.4). Recent changes to the pools
broke that in pool_releasable() by having a load from a const pointer,
which doesn't work there due to a temporary local variable that is
declared then assigned. Let's make use of a compount statement to assign
it a value when declaring it.

There's no need to backport this.

(cherry picked from commit 8da23393a1ec3f24dcd408f4b35fdd3d9a141a5f)
[wt: 2.5 on gcc-4.4 is reportedly affected by this as well, e.g. in
 freq_ctr_total()]
Signed-off-by: Willy Tarreau <w@1wt.eu>

CI: Consistently use actions/checkout@v2

v2 is the current version of the checkout action and faster than v1.

(cherry picked from commit f42ddf73fc8073070bb7a5d769c73d52722d2b17)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: github actions: use cache for SSL libs

we have two kinds of SSL libs built - git based and version based.
this commit introduces caching for version based SSL libs.

(cherry picked from commit 27df87cc6318a96625568163280476b4077a6e61)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: refactor OpenTracing build script

re-use scripts/build-ot.sh in CI again. Bump opentracing-cpp to 1.6.0

(cherry picked from commit e9efc3a5be4ada50cc004f8acc864efa4ecccf01)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: github actions: use cache for OpenTracing

this caches OpenTracing libs between builds, should save couple of minutes
for each build.

(cherry picked from commit b9e3fb7315e32cee678d3635f715de65b6c53fae)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: github actions: add the output of $CC -dM -E-

Sometimes figuring what differs between platforms is useful to fix
build issues, to decide what ifdef to add for example. Let's always
call $CC -dM -E- before starting make.

(cherry picked from commit 4673c5e2c81e8e2b04018f3ba0b80d41d85f2220)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

[RELEASE] Released version 2.5.4

Released version 2.5.4 with the following main changes :
    - BUG/MEDIUM: htx: Be sure to have a buffer to perform a raw copy of a message
    - BUG/MEDIUM: mux-h1: Don't wake h1s if mux is blocked on lack of output buffer
    - BUG/MAJOR: mux-h2: Be sure to always report HTX parsing error to the app layer
    - DOC: Fix usage/examples of deprecated ACLs
    - BUG/MINOR: proxy: preset the error message pointer to NULL in parse_new_proxy()
    - REGTESTS: fix the race conditions in 40be_2srv_odd_health_checks
    - CI: github: enable pool debugging by default
    - BUG/MEDIUM: stream: Abort processing if response buffer allocation fails

BUG/MEDIUM: stream: Abort processing if response buffer allocation fails

In process_stream(), we force the response buffer allocation before any
processing to be able to return an error message. It is important because,
when an error is triggered, the stream is immediately closed. Thus we cannot
wait for the response buffer allocation.

When the allocation fails, the stream analysis is stopped and the expiration
date of the stream's task is updated before exiting process_stream(). But if
the stream was woken up because of a connection or an analysis timeout, the
expiration date remains blocked in the past. This means the stream is woken
up in loop as long as the response buffer is not properly allocated.

Alone, this behavior is already a bug. But because the mechanism to handle
buffer allocation failures is totally broken since a while, this bug becomes
more problematic. Because, most of time, the watchdog will kill HAProxy in
this case because it will detect a spinning loop.

To fix it, at least temporarily, an allocation failure at this stage is now
reported as an error and the processing is aborted. It's not satisfying but
it is better than nothing. If the buffers allocation mechanism is
refactored, this part will be reviewed.

This patch must be backported, probably as far as 2.0. It may be perceived
as a regression, but the actual behavior is probably even worse. And
because it was not reported, it is probably not a common situation.

(cherry picked from commit 686501cb1c4eabd0ab1e91da70adf0bd322d3ccd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

CI: github: enable pool debugging by default

This enables DEBUG_MEMORY_POOLS and DEBUG_POOL_INTEGRITY so that by
default the tests run under stricter checks, which are likely to
catch more bugs. Note that these ones are permanently used in prod
on haproxy.org.

(cherry picked from commit a0a6911bdef2dfd908cd1ad1bd074d27f95a252c)
Signed-off-by: Willy Tarreau <w@1wt.eu>

REGTESTS: fix the race conditions in 40be_2srv_odd_health_checks

This one started to randomly fail on me again and I could figure the
problem. It mixes one checked server with one unchecked on in each
backend, and tries to make sure that each checked server receives
exactly one request. But that doesn't work and is entirely time-
dependent because if the check starts before the client, a pure
TCP check is sent to the server, which sees an aborted connection
and makes the whole check fail.

Here what is done is that we make sure that only the second server
and not the first one is checked. The traffic is delivered to all
first servers, and each HTTP server must always receive a valid HTTP
request. In parallel, checks must not fail as they're delivered to
dummy servers. The check doesn't fail anymore, even when started on
a single thread at nice +5 while 8 processes are fighting on the same
core to inject HTTP traffic at 25 Gbps, which used to systematically
make it fail previously.

Since it took more than one hour to fix the "expect" line for the stats
output, I did it using a small script that I pasted into the vtc file
in case it's needed later. The relevance of this test is questionable
once its complexity is factored in. Let's keep it as long as it works
without too much effort.

(cherry picked from commit 3d9266f414abef21f978a4723bf60519be6fa1f4)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: proxy: preset the error message pointer to NULL in parse_new_proxy()

As reported by Coverity in issue #1568, a missing initialization of the
error message pointer in parse_new_proxy() may result in displaying garbage
or crashing in case of memory allocation error when trying to create a new
proxy on startup.

This should be backported to 2.4.

(cherry picked from commit 282b6a7539add7517f2b16cc82b6a3509492fd76)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: Fix usage/examples of deprecated ACLs

Some examples or references were still using deprecated ACL variants.

Signed-off-by: Christian Ruppert <idl0r@qasl.de>
(cherry picked from commit 59e66e30c2aa82947c1f00ec64eec117efa8846d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: mux-h2: Be sure to always report HTX parsing error to the app layer

If a parsing error is detected and the corresponding HTX flag is set
(HTX_FL_PARSING_ERROR), we must be sure to always report it to the app
layer. It is especially important when the error occurs during the response
parsing, on the server side. In this case, the RX buffer contains an empty
HTX message to carry the flag. And it remains in this state till the info is
reported to the app layer. This must be done otherwise, on the conn-stream,
the CS_FL_ERR_PENDING flag cannot be switched to CS_FL_ERROR and the
CS_FL_WANT_ROOM flag is always set when h2_rcv_buf() is called. The result
is a ping-pong loop between the mux and the stream.

Note that this patch fixes a bug. But it also reveals a design issue. The
error must not be reported at the HTX level. The error is already carried by
the conn-stream. There is no reason to duplicate it. In addition, it is
errorprone to have an empty HTX message only to report the error to the app
layer.

This patch should fix the issue #1561. It must be backported as far as 2.0
but the bug only affects HAProxy >= 2.4.

(cherry picked from commit ec361bbd843781fb15ebbfca6aea57455d3ac3f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h1: Don't wake h1s if mux is blocked on lack of output buffer

After sending some data, we try to wake the H1 stream to resume data
processing at the stream level, except if the output buffer is still
full. However we must also be sure the mux is not blocked because of an
allocation failure on this buffer. Otherwise, it may lead to a ping-pong
loop between the stream and the mux to send more data with an unallocated
output buffer.

Note there is a mechanism to queue buffers allocations when a failure
happens. However this mechanism is totally broken since the filters were
introducted in HAProxy 1.7. And it is worse now with the multiplexers. So
this patch fixes a possible loop needlessly consuming all the CPU. But
buffer allocation failures must remain pretty rare.

This patch must be backported as far as 2.0.

(cherry picked from commit c17c31c822dc4a5f3270f6ae4f96afa50fb7390f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: htx: Be sure to have a buffer to perform a raw copy of a message

In htx_copy_msg(), if the destination buffer is empty, we perform a raw copy
of the message instead of a copy block per block. But we must be sure the
destianation buffer was really allocated. In other word, to perform a raw
copy, the HTX message must be empty _AND_ it must have some free space
available.

This function is only used to copy an HTTP reply (for instance, an error or
a redirect) in the buffer of the response channel. For now, we are sure the
buffer was allocated because it is a pre-requisite to call stream
analyzers. However, it may be a source of bug in future.

This patch may be backported as far as 2.3.

(cherry picked from commit dc523e3b89d8ff3b73eb7d3218cbf2907c94f6ae)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[RELEASE] Released version 2.5.3

Released version 2.5.3 with the following main changes :
    - MINOR: sock: move the unused socket cleaning code into its own function
    - BUG/MEDIUM: mworker: close unused transferred FDs on load failure
    - BUG/MINOR: mworker: fix a FD leak of a sockpair upon a failed reload
    - BUG/MINOR: sink: Use the right field in appctx context in release callback
    - BUG/MEDIUM: resolvers: Really ignore trailing dot in domain names
    - BUG/MEDIUM: fd: always align fdtab[] to 64 bytes
    - BUG/MAJOR: compiler: relax alignment constraints on certain structures
    - MINOR: httpclient: Don't limit data transfer to 1024 bytes
    - BUG/MINOR: httpclient: reinit flags in httpclient_start()
    - BUG/MINOR: mailers: negotiate SMTP, not ESMTP
    - BUG/MINOR: ssl: Add missing return value check in ssl_ocsp_response_print
    - BUG/MINOR: ssl: Fix leak in "show ssl ocsp-response" CLI command
    - BUG/MINOR: ssl: Missing return value check in ssl_ocsp_response_print
    - CLEANUP: httpclient/cli: fix indentation alignment of the help message
    - BUG/MINOR: tools: url2sa reads ipv4 too far
    - BUG/MEDIUM: httpclient: limit transfers to the maximum available room
    - DEBUG: buffer: check in __b_put_blk() whether the buffer room is respected

DEBUG: buffer: check in __b_put_blk() whether the buffer room is respected

This adds a BUG_ON() to make sure we don't face other situations like the
one fixed by previous commit.

(cherry picked from commit d439a496550ba44ceae09a59c72c278163ff8b99)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: httpclient: limit transfers to the maximum available room

A bug was uncovered by commit fc5912914 ("MINOR: httpclient: Don't limit
data transfer to 1024 bytes"), it happens that callers of b_xfer() and
b_force_xfer() are expected to check for available room in the target
buffer. Previously it was unlikely to be full but now with full buffer-
sized transfers, it happens more often and in practice it is possible
to crash the process with the debug command "httpclient" on the CLI by
going beyond a the max buffer size. Other call places ought to be
rechecked by now and it might be time to rethink this API if it tends
to generalize.

This must be backported to 2.5.

(cherry picked from commit 11adb1d8fcab29ef8b12c93e3b036bb3dcf1607b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: tools: url2sa reads ipv4 too far

The url2sa implementation is inconsitent when parsing an IPv4, indeed
url2sa() takes a <ulen> as a parameter where the call to url2ipv4() takes
a null terminated string. Which means url2ipv4 could try to read more
that it is supposed to.

This function is only used from a buffer so it never reach a unallocated
space. It can only cause an issue when used from the httpclient which
uses it with an ist.

This patch fixes the issue by copying everything in the trash and
null-terminated it.

Must be backported in all supported version.

(cherry picked from commit 8a91374487e13cf139ab36e483163a21ebdc6f4e)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CLEANUP: httpclient/cli: fix indentation alignment of the help message

The output was not aligned with other commands, let's fix it.

(cherry picked from commit 2c8f984441e4b8875e2af306d00e1d00755f8ef7)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: ssl: Missing return value check in ssl_ocsp_response_print

When calling ssl_ocsp_response_print which is used to display an OCSP
response's details when calling the "show ssl ocsp-response" on the CLI,
we use the BIO_read function that copies an OpenSSL BIO into a trash.
The return value was not checked though, which could lead to some
crashes since BIO_read can return a negative value in case of error.

This patch should be backported to 2.5.

(cherry picked from commit 1b01b7f2eff33ca9bd1da9fa628fd07a48c5a7cc)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: ssl: Fix leak in "show ssl ocsp-response" CLI command

When calling the "show ssl ocsp-response" CLI command some OpenSSL
objects need to be created in order to get some information related to
the OCSP response and some of them were not freed.

It should be backported to 2.5.

(cherry picked from commit 8081b6769902899346f4c717007841190118d349)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: ssl: Add missing return value check in ssl_ocsp_response_print

The b_istput function called to append the last data block to the end of
an OCSP response's detailed output was not checked in
ssl_ocsp_response_print. The ssl_ocsp_response_print return value checks
were added as well since some of them were missing.
This error was raised by Coverity (CID 1469513).

This patch fixes GitHub issue #1541.
It can be backported to 2.5.

(cherry picked from commit a9a591ab3dcf316e30506ec79eb9c255d2b2106c)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: mailers: negotiate SMTP, not ESMTP

As per issue #1552 the mailer code currently breaks on ESMTP multiline
responses. Let's negotiate SMTP instead.

Should be backported to 2.0.

(cherry picked from commit 1a16e4ebcb6d5848dd867a4ef3edda3760c60124)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: httpclient: reinit flags in httpclient_start()

When starting for the 2nd time a request from the same httpclient *hc
context, the flags are not reinitialized and the httpclient will stop
after the first call to the IO handler, because the END flag is always
present.

This patch also add a test before httpclient_start() to ensure we don't
start a client already started.

Must be backported in 2.5.

(cherry picked from commit 5085bc3103e5a52f0d8909d13a6e36d24d2a6562)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

MINOR: httpclient: Don't limit data transfer to 1024 bytes

For debug purpose, no more 1024 bytes were copied at a time. But there is no
reason to keep this limitation. Thus, it is removed.

This patch may be backported to 2.5.

(cherry picked from commit fc5912914b320debad432c668b810170f6fd559a)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MAJOR: compiler: relax alignment constraints on certain structures

In github bug #1517, Mike Lothian reported instant crashes on startup
on RHEL8 + gcc-11 that appeared with 2.4 when allocating a proxy.

The analysis brought us down to the THREAD_ALIGN() entries that were
placed inside the "server" struct to avoid false sharing of cache lines.
It turns out that some modern gcc make use of aligned vector operations
to manipulate some fields (e.g. memset() etc) and that these structures
allocated using malloc() are not necessarily aligned, hence the crash.

The compiler is allowed to do that because the structure claims to be
aligned. The problem is in fact that the alignment propagates to other
structures that embed it. While most of these structures are used as
statically allocated variables, some are dynamic and cannot use that.
A deeper analysis showed that struct server does this, propagates to
struct proxy, which propagates to struct spoe_config, all of which
are allocated using malloc/calloc.

A better approach would consist in usins posix_memalign(), but this one
is not available everywhere and will either need to be reimplemented
less efficiently (by always wasting 64 bytes before the area), or a
few functions will have to be specifically written to deal with the
few structures that are dynamically allocated.

But the deeper problem that remains is that it is difficult to track
structure alignment, as there's no available warning to check this.
For the long term we'll probably have to create a macro such as
"struct_malloc()" etc which takes a type and enforces an alignment
based on the one of this type. This also means propagating that to
pools as well, and it's not a tiny task.

For now, let's get rid of the forced alignment in struct server, and
replace it with extra padding. By punching 63-byte holes, we can keep
areas on separate cache lines. Doing so moderately increases the size
of the "server" structure (~+6%) but that's the best short-term option
and it's easily backportable.

This will have to be backported as far as 2.4.

Thanks to Mike for the detailed report.

(cherry picked from commit ecc473b5299ccd0007a73ec4b4af099ac65e9c43)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MEDIUM: fd: always align fdtab[] to 64 bytes

There's a risk that fdtab is not 64-byte aligned. The first effect is that
it may cause false sharing between cache lines resulting in contention
when adjacent FDs are used by different threads. The second is related
to what is explained in commit "BUG/MAJOR: compiler: relax alignment
constraints on certain structures", i.e. that modern compilers might
make use of aligned vector operations to zero some entries, and would
crash. We do not use any memset() or so on fdtab, so the risk is almost
inexistent, but that's not a reason for violating some valid assumptions.

This patch addresses this by allocating 64 extra bytes and aligning the
structure manually (this is an extremely cheap solution for this specific
case). The original address is stored in a new variable "fdtab_addr" and
is the one that gets freed. This remains extremely simple and should be
easily backportable. A dedicated aligned allocator later would help, of
course.

This needs to be backported as far as 2.2. No issue related to this was
reported yet, but it could very well happen as compilers evolve. In
addition this should preserve high performance across restarts (i.e.
no more dependency on allocator's alignment).

(cherry picked from commit 97ea9c49f1d95c7e91e544e8ad9ba09bffbcc023)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MEDIUM: resolvers: Really ignore trailing dot in domain names

When a string is converted to a domain name label, the trailing dot must be
ignored. In resolv_str_to_dn_label(), there is a test to do so. However, the
trailing dot is not really ignored. The character itself is not copied but
the string index is still moved to the next char. Thus, this trailing dot is
counted in the length of the last encoded part of the domain name. Worst,
because the copy is skipped, a garbage character is included in the domain
name.

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

(cherry picked from commit 0a82cf4c1662b8ab00036f65b5e3543a9c1a6ff5)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: sink: Use the right field in appctx context in release callback

In the release callback, ctx.peers was used instead of ctx.sft. Concretly,
it is not an issue because the appctx context is an union and these both
fields are structures with a unique pointer. But it will be a problem if
that changes.

This patch must be backported as far as 2.2.

(cherry picked from commit dd0b144c3ae83c2ea14f969cb58fa4b927c2c455)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MINOR: mworker: fix a FD leak of a sockpair upon a failed reload

When starting HAProxy in master-worker, the master pre-allocate a struct
mworker_proc and do a socketpair() before the configuration parsing. If
the configuration loading failed, the FD are never closed because they
aren't part of listener, they are not even in the fdtab.

This patch fixes the issue by cleaning the mworker_proc structure that
were not asssigned a process, and closing its FDs.

Must be backported as far as 2.0, the srv_drop() only frees the memory
and could be dropped since it's done before an exec().

(cherry picked from commit 55a921c9147c2d45621964428c86c5ceb9fd31d8)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MEDIUM: mworker: close unused transferred FDs on load failure

When the master process is reloaded on a new config, it will try to
connect to the previous process' socket to retrieve all known
listening FDs to be reused by the new listeners. If listeners were
removed, their unused FDs are simply closed.

However there's a catch. In case a socket fails to bind, the master
will cancel its startup and swithc to wait mode for a new operation
to happen. In this case it didn't close the possibly remaining FDs
that were left unused.

It is very hard to hit this case, but it can happen during a
troubleshooting session with fat fingers. For example, let's say
a config runs like this:

   frontend ftp
        bind 1.2.3.4:20000-29999

The admin wants to extend the port range down to 10000-29999 and
by mistake ends up with:

   frontend ftp
        bind 1.2.3.41:20000-29999

Upon restart the bind will fail if the address is not present, and the
master will then switch to wait mode without releasing the previous FDs
for 1.2.3.4:20000-29999 since they're now apparently unused. Then once
the admin fixes the config and does:

   frontend ftp
        bind 1.2.3.4:10000-29999

The service will start, but will bind new sockets, half of them
overlapping with the previous ones that were not properly closed. This
may result in a startup error (if SO_REUSEPORT is not enabled or not
available), in a FD number exhaustion (if the error is repeated many
times), or in connections being randomly accepted by the process if
they sometimes land on the old FD that nobody listens on.

This patch will need to be backported as far as 1.8, and depends on
previous patch:

   MINOR: sock: move the unused socket cleaning code into its own function

Note that before 2.3 most of the code was located inside haproxy.c, so
the patch above should probably relocate the function there instead of
sock.c.

(cherry picked from commit e08acaed19c6d6a86ebaf2b5f3089ebef78bc69d)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

MINOR: sock: move the unused socket cleaning code into its own function

The startup code used to scan the list of unused sockets retrieved from
an older process, and to close them one by one. This also required that
the knowledge of the internal storage of these temporary sockets was
known from outside sock.c and that the code was copy-pasted at every
call place.

This patch moves this into sock.c under the name
sock_drop_unused_old_sockets(), and removes the xfer_sock_list
definition from sock.h since the rest of the code doesn't need to know
this.

This cleanup is minimal and preliminary to a future fix that will need
to be backported to all versions featuring FD transfers over the CLI.

(cherry picked from commit b510116fd2b9a676297a1d2bec7d0107bf3c7383)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

[RELEASE] Released version 2.5.2

Released version 2.5.2 with the following main changes :
    - BUG/MEDIUM: connection: properly leave stopping list on error
    - BUG/MEDIUM: htx: Adjust length to add DATA block in an empty HTX buffer
    - BUG/MINOR: httpclient: don't send an empty body
    - BUG/MINOR: httpclient: set default Accept and User-Agent headers
    - BUG/MINOR: httpclient/lua: don't pop the lua stack when getting headers
    - BUILD/MINOR: fix solaris build with clang.
    - BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl
    - DOC: management: mark "set server ssl" as deprecated
    - MEDIUM: cli: yield between each pipelined command
    - MINOR: channel: add new function co_getdelim() to support multiple delimiters
    - BUG/MINOR: cli: avoid O(bufsize) parsing cost on pipelined commands
    - MEDIUM: h2/hpack: emit a Dynamic Table Size Update after settings change
    - BUG/MEDIUM: cli: Never wait for more data on client shutdown
    - BUG/MEDIUM: mcli: do not try to parse empty buffers
    - BUG/MEDIUM: mcli: always realign wrapping buffers before parsing them
    - BUG/MINOR: stream: make the call_rate only count the no-progress calls
    - DEBUG: cli: add a new "debug dev fd" expert command
    - BUILD: debug/cli: condition test of O_ASYNC to its existence
    - DEBUG: pools: add new build option DEBUG_POOL_INTEGRITY
    - REGTESTS: ssl: Fix ssl_errors regtest with OpenSSL 1.0.2
    - BUG/MEDIUM: mworker: don't lose the stats socket on failed reload
    - BUG/MINOR: mworker: does not add the -sf in wait mode
    - BUG/MINOR: pools: always flush pools about to be destroyed
    - DEBUG: pools: add extra sanity checks when picking objects from a local cache
    - DEBUG: pools: let's add reverse mapping from cache heads to thread and pool
    - DEBUG: pools: replace the link pointer with the caller's address on pool_free()
    - BUG/MAJOR: sched: prevent rare concurrent wakeup of multi-threaded tasks
    - BUG/MINOR: mworker: does not erase the pidfile upon reload
    - DEBUG: fd: make sure we never try to insert/delete an impossible FD number
    - MINOR: listener: replace the listener's spinlock with an rwlock
    - BUG/MEDIUM: listener: read-lock the listener during accept()
    - BUG/MINOR: httpclient: Revisit HC request and response buffers allocation
    - BUG/MEDIUM: httpclient: Xfer the request when the stream is created
    - BUG/MINOR: ssl: Remove empty lines from "show ssl ocsp-response <id>" output
    - BUG/MINOR: jwt: Double free in deinit function
    - BUG/MINOR: jwt: Missing pkey free during cleanup
    - BUG/MINOR: jwt: Memory leak if same key is used in multiple jwt_verify calls
    - BUG/MINOR: httpclient/cli: display junk characters in vsn
    - BUG/MAJOR: http/htx: prevent unbounded loop in http_manage_server_side_cookies
    - BUG/MAJOR: spoe: properly detach all agents when releasing the applet
    - REGTESTS: server: close an occasional race on dynamic_server_ssl.vtc
    - REGTESTS: peers: leave a bit more time to peers to synchronize
    - BUG/MEDIUM: h2/hpack: fix emission of HPACK DTSU after settings change
    - BUG/MINOR: mux-h2: update the session's idle delay before creating the stream

BUG/MINOR: mux-h2: update the session's idle delay before creating the stream

The idle connection delay calculation before a request is a bit tricky,
especially for multiplexed protocols. It changed between 2.3 and 2.4 by
the integration of the idle delay inside the session itself with these
commits:

  dd78921c6 ("MINOR: logs: Use session idle duration when no stream is provided")
  7a6c51324 ("MINOR: stream: Always get idle duration from the session")

and by then it was only set by the H1 mux. But over multiple changes, what
used to be a zero idle delay + a request delay for H2 became a bit odd, with
the idle time slipping into the request time measurement. The effect is that,
as reported in GH issue #1395, some H2 request times look huge.

This patch introduces the calculation of the session's idle time on the
H2 mux before creating the stream. This is made possible because the
stream_new() code immediately copies this value into the stream for use
at log time. Thus we don't care about changing something that will be
touched by every single request. The idle time is calculated as documented,
i.e. the delay from the previous request to the current one. This also
means that when a single stream is present on a connection, a part of
the server's response time may appear in the %Ti measurement, but this
reflects the reality since nothing would prevent the client from using
the connection to fetch more objects. In addition this shows how long
it takes a client to find references to objects in an HTML page and
start to fetch them.

A different approach could have consisted in counting from the last time
the connection was left without any request (i.e. really idle), but this
would at least require a documentation change and it's not certain this
would provide a more useful information.

Thanks to Bart Butler and Luke Seelenbinder for reporting enough elements
to diagnose this issue.

This should be backported to 2.4.

(cherry picked from commit d0de6776826ee18da74e6949752e2f44cba8fdf2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: h2/hpack: fix emission of HPACK DTSU after settings change

Sadly, despite particular care, commit 39a0a1e12 ("MEDIUM: h2/hpack: emit
a Dynamic Table Size Update after settings change") broke H2 when sending
DTSU. A missing negation on the flag caused the DTSU_EMITTED flag to be
lost and the DTSU to be sent again on the next stream, and possibly to
break flow control or a few other internal states.

This will have to be backported wherever the patch above was backported.

Thanks to Yves Lafon for notifying us with elements to reproduce the
issue!

(cherry picked from commit c7d85485a00bd9862ecb726ad1242c2ba724a8ca)
Signed-off-by: Willy Tarreau <w@1wt.eu>

REGTESTS: peers: leave a bit more time to peers to synchronize

tls_basic_sync_wo_stkt_backend fails once every 200 runs for me. This
seems to be because the startup delay doesn't always allow peers to
perform a simultaneous connect, close and new attempt. With 3s I can't
see it fail anymore. In addition the long "delay 0.2" are still way too
much since we do not really care about the startup order in practice.

(cherry picked from commit c38200563645e314bd0af59958c75171cca59d4b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

REGTESTS: server: close an occasional race on dynamic_server_ssl.vtc

Sometimes when sending commands to shut down a server, haproxy complains
that some connections remain, this is because the server-side connection
might not always be completely released at the moment the client leaves
and the operation is emitted. While shutting down server sessions work,
it seems cleaner to just use "option httpclose" which releases the server
earlier and avoids the race.

This can be backported to 2.5.

(cherry picked from commit 42f2a511d32363121c667d51548d15204625301f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MAJOR: spoe: properly detach all agents when releasing the applet

There's a bug in spoe_release_appctx() which checks the presence of items
in the wrong list rt[tid].agents to run over rt[tid].waiting_queue and
zero their spoe_appctx. The effect is that these contexts are not zeroed
and if spoe_stop_processing() is called, "sa->cur_fpa--" will be applied
to one of these recently freed contexts and will corrupt random memory
locations, as found at least in bugs #1494 and #1525.

This must be backported to all stable versions.

Many thanks to Christian Ruppert from Babiel for exchanging so many
useful traces over the last two months, testing debugging code and
helping set up a similar environment to reproduce it!

(cherry picked from commit b042e4f6f7dca655a337fc9ffe1a5e4f25440868)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MAJOR: http/htx: prevent unbounded loop in http_manage_server_side_cookies

Ensure calls to http_find_header() terminate. If a "Set-Cookie2"
header is found then the while(1) loop in
http_manage_server_side_cookies() will never terminate, resulting in
the watchdog firing and the process terminating via SIGABRT.

The while(1) loop becomes unbounded because an unmatched call to
http_find_header("Set-Cookie") will leave ctx->blk=NULL. Subsequent
calls to check for "Set-Cookie2" will now enumerate from the beginning
of all the blocks and will once again match on subsequent
passes (assuming a match first time around), hence the loop becoming
unbounded.

This issue was introduced with HTX and this fix should be backported
to all versions supporting HTX.

Many thanks to Grant Spence (gspence@redhat.com) for working through
this issue with me.

(cherry picked from commit bfb15ab34ead85f64cd6da0e9fb418c9cd14cee8)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: httpclient/cli: display junk characters in vsn

ist are not ended by '\0', leading to junk characters being displayed
when using %s for printing the HTTP start line.

Fix the issue by replacing %s by %.*s + istlen.

Must be backported in 2.5.

(cherry picked from commit de6ecc3aceaf93612eb37ff5a913fcd8e741ea67)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: jwt: Memory leak if same key is used in multiple jwt_verify calls

If the same filename was specified in multiple calls of the jwt_verify
converter, we would have parsed the contents of the file every time it
was used instead of checking if the entry already existed in the tree.
This lead to memory leaks because we would not insert the duplicated
entry and we would not free it (as well as the EVP_PKEY it referenced).
We now check the return value of ebst_insert and free the current entry
if it is a duplicate of an existing entry.
The order in which the tree insert and the pkey parsing happen was also
switched in order to avoid parsing key files in case of duplicates.

Should be backported to 2.5.

(cherry picked from commit d544d33e10868963407212660466aa552d5888e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: jwt: Missing pkey free during cleanup

When emptying the jwt_cert_tree during deinit, the entries are freed but
not the EVP_PKEY reference they kept, leading in a memory leak.

Should be backported in 2.5.

(cherry picked from commit 2b5a6559460b41dd6db2740cc961b461cef12edc)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: jwt: Double free in deinit function

The node pointer was not moving properly along the jwt_cert_tree during
the deinit which ended in a double free during cleanup (or when checking
a configuration that used the jwt_verify converter with an explicit
certificate specified).

This patch fixes GitHub issue #1533.
It should be backported to 2.5.

(cherry picked from commit 4930c6c869fb875554d60ce9a2d6362cf16cf295)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: ssl: Remove empty lines from "show ssl ocsp-response <id>" output

There were empty lines in the output of the CLI's "show ssl
ocsp-response <id>" command. The plain "show ssl ocsp-response" command
(without parameter) was already managed in commit
cc750efbc5c2180ed63b222a51029609ea96d0f7. This patch adds an extra space
to those lines so that the only existing empty lines actually mark the
end of the output. This requires to post-process the buffer filled by
OpenSSL's OCSP_RESPONSE_print function (which produces the output of the
"openssl ocsp -respin <ocsp.pem>" command). This way the output of our
command still looks the same as openssl's one.

Must be backported in 2.5.

(cherry picked from commit 2e7d1eb2a7dad61ee5661086005d3f85ee6ad6ba)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: httpclient: Xfer the request when the stream is created

Since the HTTP legacy mode was removed, it is unexpected to create an HTTP
stream without a valid request. Thanks to this change, the wait_for_request
analyzer was significatly simplified. And it is possible because HTTP
multiplexers already take care to have a valid request to create a stream.

But it means that any HTTP applet on the client side must do the same. The
httpclient client is one of them. And it is not a problem because the
request is generated before starting the applet. We must just take care to
set the right state.

For now it works "by chance", because the applet seems to be scheduled
before the stream itself. But if this change, this will lead to crash
because the stream expects to have a request when wait_for_request analyzer.

This patch should be backported to 2.5.

(cherry picked from commit 6ced61dd0a2056c4ed852163d6855808c07ffe8f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: httpclient: Revisit HC request and response buffers allocation

For now, these buffers are allocated when the httpclient is created and
freed when it is released. Usually, we try to avoid to keep buffer allocated
if it is not required. Empty buffers should be released ASAP. Apart for
that, there is no issue with the response side because a copy is always
performed. However, for the request side, a swap with the channel's buffer
is always performed. And there is no guarantee the channel's buffer is
allocated. Thus, after the swap, the httpclient can retrieve a null
buffer. In practice, this never happens. But this may change. And it will be
required for a futur fix.

So, now, we systematically take care to have an allocated buffer when we
want to write in it. And it is released as soon as it becomes empty.

This patch should be backported to 2.5.

(cherry picked from commit 600985df41c9777b6322f00e26dbf539f786d66f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: listener: read-lock the listener during accept()

Listeners might be disabled by other threads while running in
listener_accept() due to a stopping condition or possibly a rebinding
error after a failed stop/start. When this happens, the listener's FD
is -1 and accesses made by the lower layers to fdtab[-1] do not end up
well. This can occasionally be noticed if running at high connection
rates in master-worker mode when compiled with ASAN and hammered with
10 reloads per second. From time to time an out-of-bounds error will
be reported.

One approach could consist in keeping a copy of critical information
such as the FD before proceeding but that's not correct since in case of
close() the FD might be reassigned to another connection for example.

In fact what is needed is to read-lock the listener during this operation
so that it cannot change while we're touching it.

Tests have shown that using a spinlock only does generally work well but
it doesn't scale much with threads and we can see listener_accept() eat
10-15% CPU on a 24 thread machine at 300k conn/s. For this reason the
lock was turned to an rwlock by previous commit and this patch only takes
the read lock to make sure other operations do not change the listener's
state while threads are accepting connections. With this approach, no
performance loss was noticed at all and listener_accept() doesn't appear
in perf top.

This ought to be backported to about all branches that make use of the
unlocked listeners, but in practice it seems to mostly concern 2.3 and
above, since 2.2 and older will take the FD in the argument (and the
race exists there, this FD could end up being reassigned in parallel
but there's not much that can be done there to prevent that race; at
least a permanent error will be reported).

For backports, the current approach is preferred, with a preliminary
backport of previous commit "MINOR: listener: replace the listener's
spinlock with an rwlock". However if for any reason this commit cannot
be backported, the current patch can be modified to simply take a
spinlock (tested and works), it will just impact high performance
workloads (like DDoS protection).

(cherry picked from commit fed93d367c13c8c79e452837ec64389bc8061b5b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: listener: replace the listener's spinlock with an rwlock

We'll need to lock the listener a little bit more during accept() and
tests show that a spinlock is a massive performance killer, so let's
first switch to an rwlock for this lock.

This patch might have to be backported for the next patch to work, and
if so, the change is almost mechanical (look for LISTENER_LOCK), but do
not forget about the few HA_SPIN_INIT() in the file. There's no reference
to this lock outside of listener.c nor listener-t.h.

(cherry picked from commit 08b6f9645248405bb1cdec59c6aa4f35a8ba3add)
[wt: minor ctx adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: fd: make sure we never try to insert/delete an impossible FD number

It's among the cases that would provoke memory corruption, let's add
some tests against negative FDs and those larger than the table. This
must never ever happen and would currently result in silent corruption
or a crash. Better have a noticeable one exhibiting the call chain if
that were to happen.

(cherry picked from commit 9aa324de2d9f69d74f5b30c33a78d3a38501342f)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: mworker: does not erase the pidfile upon reload

When started in master-worker mode combined with daemon mode, HAProxy
will open() with O_TRUNC the pidfile when switching to wait mode.

In 2.5, it happens  everytime after trying to load the configuration,
since we switch to wait mode.

In previous version this happens upon a failure of the configuration
loading.

Fixes bug #1545.

Must be backported in every supported branches.

(cherry picked from commit 7b820a6191736cdf4167d624b7bcab9957dd0697)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MAJOR: sched: prevent rare concurrent wakeup of multi-threaded tasks

Since the relaxation of the run-queue locks in 2.0 there has been a
very small but existing race between expired tasks and running tasks:
a task might be expiring and being woken up at the same time, on
different threads. This is protected against via the TASK_QUEUED and
TASK_RUNNING flags, but just after the task finishes executing, it
releases it TASK_RUNNING bit an only then it may go to task_queue().
This one will do nothing if the task's ->expire field is zero, but
if the field turns to zero between this test and the call to
__task_queue() then three things may happen:
  - the task may remain in the WQ until the 24 next days if it's in
    the future;
  - the task may prevent any other task after it from expiring during
    the 24 next days once it's queued
  - if DEBUG_STRICT is set on 2.4 and above, an abort may happen
  - since 2.2, if the task got killed in between, then we may
    even requeue a freed task, causing random behaviour next time
    it's found there, or possibly corrupting the tree if it gets
    reinserted later.

The peers code is one call path that easily reproduces the case with
the ->expire field being reset, because it starts by setting it to
TICK_ETERNITY as the first thing when entering the task handler. But
other code parts also use multi-threaded tasks and rightfully expect
to be able to touch their expire field without causing trouble. No
trivial code path was found that would destroy such a shared task at
runtime, which already limits the risks.

This must be backported to 2.0.

(cherry picked from commit 6c8babf6c4d8cb33ff0f5220177207610c22ffcb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: pools: replace the link pointer with the caller's address on pool_free()

Along recent evolutions of the pools, we've lost the ability to reliably
detect double-frees because while in the past the same pointer was being
used to chain the objects in the cache and to store the pool's address,
since 2.0 they're different so the pool's address is never overwritten on
free() and a double-free will rarely be detected.

This patch sets the caller's return address there. It can never be equal
to a pool's address and will help guess what was the previous call path.
It will not work on exotic architectures nor with very old compilers but
these are not the environments where we're trying to get detailed bug
reports, and this is not done by default anyway so we don't care about
this limitation. Note that depending on the inlining status of the
function, the result may differ but that's no big deal either.

A test by placing a double free of an appctx inside the release handler
itself successfully reported the trouble during appctx_free() and showed
that the return address was in stream_int_shutw_applet() (this one calls
the release handler).

(cherry picked from commit 27c8da1fd5e4f906fd3a9f96a4284c886b559305)
[wt: simplified for 2.5 and older, just use POOL_LINK()]
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: pools: let's add reverse mapping from cache heads to thread and pool

During global eviction we're visiting nodes from the LRU tail and we
determine their pool cache head and their pool. In order to make sure
we never mess up, let's add some backwards pointer to the thread number
and pool from the pool_cache_head. It's 64-byte aligned anyway so we're
not wasting space and it helps for debugging and will prevent memory
corruption the earliest possible.

(cherry picked from commit 49bb5d4268adaea64490ee20f4d73d94afe6a903)
[wt: context adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: pools: add extra sanity checks when picking objects from a local cache

These few checks are added to make sure we never try to pick an object from
an empty list, which would have a devastating effect.

(cherry picked from commit e2830addda9baf42c682d7c1856a53e2875deea4)
[wt: adjusted context]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: pools: always flush pools about to be destroyed

When destroying a pool (e.g. at exit or when resizing buffers), it's
important to try to free all their local objects otherwise we can leave
some in the cache. This is particularly visible when changing "bufsize",
because "show pools" will then show two "trash" pools, one of which
contains a single object in cache (which is fortunately not reachable).
In all cases this happens while single-threaded so that's easy to do,
we just have to do it on the current thread.

The easiest way to do this is to pass an extra argument to function
pool_evict_from_local_cache() to force a full flush instead of a
partial one.

This can probably be backported to about all branches where this
applies, but at least 2.4 needs it.

(cherry picked from commit c895c441c7579db652e4ed976c14c2f5b2de0c0e)
[wt: context adjustments as 2.5 doesn't use pool_evict_last_items()]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: mworker: does not add the -sf in wait mode

Since the wait mode is automatically executed after charging the
configuration, -sf was shown in argv[] with the previous PID, which is
normal, but also the current one. This is only a visual problem when
listing the processes, because -sf does not do anything in wait mode.

Fix the issue by removing the whole "-sf" part in wait mode, but the
executed command can be seen in the argv[] of the latest worker forked.

Must be backported in 2.5.

(cherry picked from commit befab9ee4aa2dfe0405d7f1416991175be2fbe67)
[wla: in 2.5 we still need the -x in wait mode, because we don't know
anymore the stats socket in the configuration once we reexec in waitmode]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MEDIUM: mworker: don't lose the stats socket on failed reload

In master mode, when the master reexecs itself, it passes the stats socket
address via "-x" on the command line. However, the address is first
retrieved from the configuration, and only later in the boot sequence
the one passed on the command line is considered. As such, some early
errors on reload like a missing config file will cause the process to
rexec itself with no stats socket address on the command line, preventing
the new master from retrieving previous listeners.

The right thing to do is to preset the value to the one found on the
command line so that it can be preserved across failed reloads.

There is no mainline equivalent for this commit because this code was
removed from 2.6, which now only uses the socket pair. The patch must
be backported to older versions where it applies.

REGTESTS: ssl: Fix ssl_errors regtest with OpenSSL 1.0.2

This test was broken with OpenSSL 1.0.2 after commit a996763619d
(BUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello
error) because it expected the default TLS version to be 1.3 in some
cases (when it can't be the case with OpenSSL 1.0.2).

(cherry picked from commit aab8d255bc0fcbcc50884a4be4f69598ee08fe73)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: pools: add new build option DEBUG_POOL_INTEGRITY

When enabled, objects picked from the cache are checked for corruption
by comparing their contents against a pattern that was placed when they
were inserted into the cache. Objects are also allocated in the reverse
order, from the oldest one to the most recent, so as to maximize the
ability to detect such a corruption. The goal is to detect writes after
free (or possibly hardware memory corruptions). Contrary to DEBUG_UAF
this cannot detect reads after free, but may possibly detect later
corruptions and will not consume extra memory. The CPU usage will
increase a bit due to the cost of filling/checking the area and for the
preference for cold cache instead of hot cache, though not as much as
with DEBUG_UAF. This option is meant to be usable in production.

(cherry picked from commit 0575d8fd760c6cd1de3d6ed66599d685a03c1873)
[wt: adjusted slightly since there is no batch refilling in 2.5; dropped
 the API doc parts; tested with/without option and works fine]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD: debug/cli: condition test of O_ASYNC to its existence

David Carlier reported a build breakage on Haiku since commit
5be7c198e ("DEBUG: cli: add a new "debug dev fd" expert command")
due to O_ASYNC not being defined. Ilya also reported it broke the
build on Cygwin. It's not that portable and sometimes defined as
O_NONBLOCK for portability. But here we don't even need that, as
we already condition other flags, let's just ignore it if it does
not exist.

(cherry picked from commit 410942b92a2687f690c886ae1fdc7ad69a3640e0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DEBUG: cli: add a new "debug dev fd" expert command

This command will scan the whole file descriptors space to look for
existing FDs that are unknown to haproxy's fdtab, and will try to dump
a maximum number of information about them (including type, mode, device,
size, uid/gid, cloexec, O_* flags, socket types and addresses when
relevant). The goal is to help detecting inherited FDs from parent
processes as well as potential leaks.

Some of those listed are actually known but handled so deep into some
systems that they're not in the fdtab (such as epoll FDs or inter-
thread pipes). This might be refined in the future so that these ones
become known and do not appear.

Example of output:

 $ socat - /tmp/sock1 <<< "expert-mode on;debug dev fd"

    0 type=tty. mod=0620 dev=0x8803 siz=0 uid=1000 gid=5 fs=0x16 ino=0x6 getfd=+0 getfl=O_RDONLY,O_APPEND
    1 type=tty. mod=0620 dev=0x8803 siz=0 uid=1000 gid=5 fs=0x16 ino=0x6 getfd=+0 getfl=O_RDONLY,O_APPEND
    2 type=tty. mod=0620 dev=0x8803 siz=0 uid=1000 gid=5 fs=0x16 ino=0x6 getfd=+0 getfl=O_RDONLY,O_APPEND
    3 type=pipe mod=0600 dev=0 siz=0 uid=1000 gid=100 fs=0xc ino=0x18112348 getfd=+0
    4 type=epol mod=0600 dev=0 siz=0 uid=0 gid=0 fs=0xd ino=0x3674 getfd=+0 getfl=O_RDONLY
   33 type=pipe mod=0600 dev=0 siz=0 uid=1000 gid=100 fs=0xc ino=0x24af8251 getfd=+0 getfl=O_RDONLY
   34 type=epol mod=0600 dev=0 siz=0 uid=0 gid=0 fs=0xd ino=0x3674 getfd=+0 getfl=O_RDONLY
   36 type=pipe mod=0600 dev=0 siz=0 uid=1000 gid=100 fs=0xc ino=0x24af8d1b getfd=+0 getfl=O_RDONLY
   37 type=epol mod=0600 dev=0 siz=0 uid=0 gid=0 fs=0xd ino=0x3674 getfd=+0 getfl=O_RDONLY
   39 type=pipe mod=0600 dev=0 siz=0 uid=1000 gid=100 fs=0xc ino=0x24afa04f getfd=+0 getfl=O_RDONLY
   41 type=pipe mod=0600 dev=0 siz=0 uid=1000 gid=100 fs=0xc ino=0x24af8252 getfd=+0 getfl=O_RDONLY
   42 type=epol mod=0600 dev=0 siz=0 uid=0 gid=0 fs=0xd ino=0x3674 getfd=+0 getfl=O_RDONLY

(cherry picked from commit 5be7c198e5991d5b601af3e648bccfd6f345c673)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: stream: make the call_rate only count the no-progress calls

We have an anti-looping protection in process_stream() that detects bugs
that used to affect a few filters like compression in the past which
sometimes forgot to handle a read0 or a particular error, leaving a
thread looping at 100% CPU forever. When such a condition is detected,
an alert it emitted and the process is killed so that it can be replaced
by a sane one:

  [ALERT]    (19061) : A bogus STREAM [0x274abe0] is spinning at 2057156
             calls per second and refuses to die, aborting now! Please
             report this error to developers [strm=0x274abe0,3 src=unix
             fe=MASTER be=MASTER dst=<MCLI> txn=(nil),0 txn.req=-,0
             txn.rsp=-,0 rqf=c02000 rqa=10000 rpf=88000021 rpa=8000000
             sif=EST,40008 sib=DIS,84018 af=(nil),0 csf=0x274ab90,8600
             ab=0x272fd40,1 csb=(nil),0
             cof=0x25d5d80,1300:PASS(0x274aaf0)/RAW((nil))/unix_stream(9)
             cob=(nil),0:NONE((nil))/NONE((nil))/NONE(0) filters={}]
    call trace(11):
    |       0x4dbaab [c7 04 25 01 00 00 00 00]: stream_dump_and_crash+0x17b/0x1b4
    |       0x4df31f [e9 bd c8 ff ff 49 83 7c]: process_stream+0x382f/0x53a3
    (...)

One problem with this detection is that it used to only count the call
rate because we weren't sure how to make it more accurate, but the
threshold was high enough to prevent accidental false positives.

There is actually one case that manages to trigger it, which is when
sending huge amounts of requests pipelined on the master CLI. Some
short requests such as "show version" are sufficient to be handled
extremely fast and to cause a wake up of an analyser to parse the
next request, then an applet to handle it, back and forth. But this
condition is not an error, since some data are being forwarded by
the stream, and it's easy to detect it.

This patch modifies the detection so that update_freq_ctr() only
applies to calls made without CF_READ_PARTIAL nor CF_WRITE_PARTIAL
set on any of the channels, which really indicates that nothing is
happening at all.

This is greatly sufficient and extremely effective, as the call above
is still caught (shutr being ignored by an analyser) while a loop on
the master CLI now has no effect. The "call_rate" field in the detailed
"show sess" output will now be much lower, except for bogus streams,
which may help spot them. This field is only there for developers
anyway so it's pretty fine to slightly adjust its meaning.

This patch could be backported to stable versions in case of reports
of such an issue, but as that's unlikely, it's not really needed.

(cherry picked from commit 6c539c4b8c24718177e7ff38af0d186ec84608ea)
[wt: bringing this to 2.5 only for now so that power users have at
 least a stable branch to upgrade to if they face this issue]
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: mcli: always realign wrapping buffers before parsing them

Pipelined commands easily result in request buffers to wrap, and the
master-cli parser only deals with linear buffers since it needs contiguous
keywords to look for in a list. As soon as a buffer wraps, some commands
are ignored and the parser is called in loops because the wrapped data
do not leave the buffer.

Let's take the easiest path that's already used at the HTTP layer, we
simply realign the buffer if its input wraps. This rarely happens anyway
(typically once per buffer), remains reasonably cheap and guarantees this
cannot happen anymore.

This needs to be backported as far as 2.0.

(cherry picked from commit a4e4d66f701b58da87cad0a2a90dea7e7553bc71)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: mcli: do not try to parse empty buffers

When pcli_parse_request() is called with an empty buffer, it still tries
to parse it and can go on believing it finds an empty request if the last
char before the beginning of the buffer is a '\n'. In this case it overwrites
it with a zero and processes it as an empty command, doing nothing but not
making the buffer progress. This results in an infinite loop that is stopped
by the watchdog. For a reason related to another issue (yet to be fixed),
this can easily be reproduced by pipelining lots of commands such as
"show version".

Let's add a length check after the search for a '\n'.

This needs to be backported as far as 2.0.

(cherry picked from commit 6cd93f52e974aca4dac7aa2756c4e17659ae5354)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: cli: Never wait for more data on client shutdown

When a shutdown is detected on the cli, we try to execute all pending
commands first before closing the connection. It is required because
commands execution is serialized. However, when the last part is a partial
command, the cli connection is not closed, waiting for more data. Because
there is no timeout for now on the cli socket, the connection remains
infinitely in this state. And because the maxconn is set to 10, if it
happens several times, the cli socket quickly becomes unresponsive because
all its slots are waiting for more data on a closed connections.

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

(cherry picked from commit 0f727dabf51d4ffead40fd43feb7c07193ebde99)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MEDIUM: h2/hpack: emit a Dynamic Table Size Update after settings change

As reported by @jinsubsim in github issue #1498, there is an
interoperability issue between nghttp2 as a client and a few servers
among which haproxy (in fact likely all those which do not make use
of the dynamic headers table in responses or which do not intend to
use a larger table), when reducing the header table size below 4096.
These are easily testable this way:

  nghttp -v -H":method: HEAD" --header-table-size=0 https://$SITE

It will result in a compression error for those which do not start
with an HPACK dynamic table size update opcode.

There is a possible interpretation of the H2 and HPACK specs that
says that an HPACK encoder must send an HPACK headers table update
confirming the new size it will be using after having acknowledged
it, because since it's possible for a decoder to advertise a late
SETTINGS and change it after transfers have begun, the initially
advertised value might very well be seen as a first change from the
initial setting, and the HPACK spec doesn't specify the side which
causes the change that triggers a DTSU update, which was essentially
summed up in this question from nghttp2's author when this issue
was already raised 6 years ago, but which didn't really find a solid
response by then:

  https://lists.w3.org/Archives/Public/ietf-http-wg/2015OctDec/0107.html

The ongoing consensus based on what some servers are doing and that aims
at limiting interoperability issues seems to be that a DTSU is expected
for each reduction from the current size, which should be reflected in
the next revision of the H2 spec:

  https://github.com/httpwg/http2-spec/pull/1005

Given that we do not make use of this table we can emit a DTSU of zero
before encoding any HPACK frame. However, some clients do not support
receiving DTSU with such values (e.g. VTest) so we cannot do it
inconditionnally!

The current patch aims at sticking as close to the spec as possible by
proceeding this way:
  - when a SETTINGS_HEADER_TABLE_SIZE is received, a flag is set
    indicating that the value changed
  - before sending any HPACK frame, this flag is checked to see if
    an update is wanted and if none was sent
  - in this case a DTSU of size zero is emitted and a flag is set
    to mention it was emitted so that it never has to be sent again

This addresses the problem with nghttp2 without affecting VTest.

More context is available here:
  https://github.com/nghttp2/nghttp2/issues/1660
  https://lists.w3.org/Archives/Public/ietf-http-wg/2021OctDec/0235.html

Many thanks to @jinsubsim for this report and participating to the issue
that led to an improvement of the H2 spec.

This should be backported to stable releases in a timely manner, ideally
as far as 2.4 once the h2spec update is merged, then to other versions
after a few months of observation or in case an issue around this is
reported.

(cherry picked from commit 39a0a1e12097a80d6707363792aee5caaa254f28)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: cli: avoid O(bufsize) parsing cost on pipelined commands

Sending pipelined commands on the CLI using a semi-colon as a delimiter
has a cost that grows linearly with the buffer size, because co_getline()
is called for each word and looks up a '\n' in the whole buffer while
copying its contents into a temporary buffer.

This causes huge parsing delays, for example 3s for 100k "show version"
versus 110ms if parsed only once for a default 16k buffer.

This patch makes use of the new co_getdelim() function to support both
an LF and a semi-colon as delimiters so that it's no more needed to parse
the whole buffer, and that commands are instantly retrieved. We still
need to rely on co_getline() in payload mode as escapes and semi-colons
are not used there.

It should likely be backported where CLI processing speed matters, but
will require to also backport previous patch "MINOR: channel: add new
function co_getdelim() to support multiple delimiters". It's worth noting
that backporting it without "MEDIUM: cli: yield between each pipelined
command" would significantly increase the ratio of disconnections caused
by empty request buffers, for the sole reason that the currently slow
parsing grants more time to request data to come in. As such it would
be better to backport the patch above before taking this one.

(cherry picked from commit 0011c251448929180216761c760a552c81162226)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MINOR: channel: add new function co_getdelim() to support multiple delimiters

For now we have co_getline() which reads a buffer and stops on LF, and
co_getword() which reads a buffer and stops on one arbitrary delimiter.
But sometimes we'd need to stop on a set of delimiters (CR and LF, etc).

This patch adds a new function co_getdelim() which takes a set of delimiters
as a string, and constructs a small map (32 bytes) that's looked up during
parsing to stop after the first delimiter found within the set. It also
supports an optional escape character that skips a delimiter (typically a
backslash). For the rest it works exactly like the two other variants.

(cherry picked from commit c5143653177368a40f3153dcf79d145284a5495e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

MEDIUM: cli: yield between each pipelined command

Pipelining commands on the CLI is sometimes needed for batched operations
such as map deletion etc, but it causes two problems:
  - some possibly long-running commands will be run in series without
    yielding, possibly causing extremely long latencies that will affect
    quality of service and even trigger the watchdog, as seen in github
    issue #1515.

  - short commands that end on a buffer size boundary, when not run in
    interactive mode, will often cause the socket to be closed when
    the last command is parsed, because the buffer is empty.

This patch proposes a small change to this: by yielding in the CLI applet
after processing a command when there are data left, we significantly
reduce the latency, since only one command is executed per call, and
we leave an opportunity for the I/O layers to refill the request buffer
with more commands, hence to execute all of them much more often.

With this change there's no more watchdog triggered on long series of
"del map" on large map files, and the operations are much less disturbed.
It would be desirable to backport this patch to stable versions after some
period of observation in recent versions.

(cherry picked from commit fa7b4f6691646dc6bdebdb0d11c9cdf0382aeccb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

DOC: management: mark "set server ssl" as deprecated

This command was integrated in 2.4 when it was not possible to handle
SSL with dynamic servers, this is now possible so we should prefer this
way.

Must be backported in 2.5.

(cherry picked from commit 9998a33d3a027ff6863eab71bcc2f2d7158319b4)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

While giving a fresh try to `set server ssl` (which I wrote), I realised
the behavior is a bit inconsistent. Indeed when using this command over
a server with ssl enabled for the data path but also for the health
check path we have:

- data and health check done using tls
- emit `set server be_foo/srv0 ssl off`
- data path and health check path becomes plain text
- emit `set server be_foo/srv0 ssl on`
- data path becomes tls and health check path remains plain text

while I thought the end result would be:
- data path and health check path comes back in tls

In the current code we indeed erase all connections while deactivating,
but restore only the data path while activating.  I made this mistake in
the past because I was testing with a case where the health check plain
text by default.

There are several ways to solve this issue. The cleanest one would
probably be to avoid changing the health check connection when we use
`set server ssl` command, and create a new command `set server
ssl-check` to change this. For now I assumed this would be ok to simply
avoid changing the health check path and be more consistent.

This patch tries to address that and also update the documentation. It
should not break the existing usage with health check on plain text, as
in this case they should have `no-check-ssl` in defaults.  Without this
patch, it makes the command unusable in an env where you have a list of
server to add along the way with initial `server-template`, and all
using tls for data and healthcheck path.

For 2.6 we should probably reconsider and add `set server ssl-check`
command for better granularity of cases.

If this solution is accepted, this patch should be backported up to >=
2.4.

The alternative solution was to restore the previous state, but I
believe this will create even more confusion in the future.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit a087f878753d73042ca9a69649d3f2668489e81d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUILD/MINOR: fix solaris build with clang.

clang 9 sets the level to POSIX 2004.

(cherry picked from commit 4831db2f32077469472ebda03e176e6c77dc7472)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: httpclient/lua: don't pop the lua stack when getting headers

hlua_httpclient_table_to_hdrs() does a lua_pop(L, 1) at the end of the
function, this is supposed to be done in the caller and it is already be
done in hlua_httpclient_send().

This call has the consequence of poping the next parameter of the
httpclient, ignoring it.

This patch fixes the issue by removing the lua_pop(L, 1).

Must be backported in 2.5.

(cherry picked from commit 01e2be84d746c5203f7c5723bb6ca9e469026c3d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: httpclient: set default Accept and User-Agent headers

Some servers require at least an Accept and a User-Agent header in the
request. This patch sets some default value.

Must be backported in 2.5.

(cherry picked from commit bad9c8cac411f11144236abe8f9342f1058c9344)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: httpclient: don't send an empty body

Forbid the httpclient to send an empty chunked client when there is no
data to send. It does happen when doing a simple GET too.

Must be backported in 2.5.

(cherry picked from commit e1e045f4d71f85462414d048449c9307886f7c36)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: htx: Adjust length to add DATA block in an empty HTX buffer

htx_add_data() is able to partially consume data. However there is a bug
when the HTX buffer is empty.  The data length is not properly
adjusted. Thus, if it exceeds the HTX buffer size, no block is added. To fix
the issue, the length is now adjusted first.

This patch must be backported as far as 2.0.

(cherry picked from commit 28e7ba86885d60a29036bc6fa3a89777148a3762)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: connection: properly leave stopping list on error

The stopping-list management introduced by commit d3a88c1c3 ("MEDIUM:
connection: close front idling connection on soft-stop") missed two
error paths in the H1 and H2 muxes. The effect is that if a stream
or HPACK table couldn't be allocated for these incoming connections,
we would leave with the connection freed still attached to the
stopping_list and it would never leave it, resulting in use-after-free
hence either a crash or a data corruption.

This is marked as medium as it only happens under extreme memory pressure
or when playing with tune.fail-alloc. Other stability issues remain in
such a case so that abnormal behaviors cannot be explained by this bug
alone.

This must be backported to 2.4.

(cherry picked from commit 3b990fe0bee3072cb70c965c202707cdad0f29cd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

[RELEASE] Released version 2.5.1

Released version 2.5.1 with the following main changes :
    - BUG/MINOR: cache: Fix loop on cache entries in "show cache"
    - BUG/MINOR: httpclient: allow to replace the host header
    - BUG/MINOR: lua: don't expose internal proxies
    - BUG/MINOR: lua: remove loop initial declarations
    - BUG/MEDIUM: cli: Properly set stream analyzers to process one command at a time
    - BUILD: evports: remove a leftover from the dead_fd cleanup
    - BUG/MINOR: vars: Fix the set-var and unset-var converters
    - BUG/MINOR: server: Don't rely on last default-server to init server SSL context
    - BUG/MEDIUM: resolvers: Detach query item on response error
    - BUG/MAJOR: segfault using multiple log forward sections.
    - BUG/MEDIUM: h1: Properly reset h1m flags when headers parsing is restarted
    - BUG/MEDIUM: mworker: FD leak of the eventpoll in wait mode
    - BUG/MINOR: mworker: deinit of thread poller was called when not initialized
    - MINOR: mux-h1: Improve H1 traces by adding info about http parsers
    - BUILD: bug: Fix error when compiling with -DDEBUG_STRICT_NOCRASH
    - BUG/MEDIUM: sample: Fix memory leak in sample_conv_jwt_member_query
    - MINOR: cli: "show version" displays the current process version
    - BUILD: tree-wide: avoid warnings caused by redundant checks of obj_types
    - IMPORT: slz: use the correct CRC32 instruction when running in 32-bit mode
    - MINOR: http-rules: Add capture action to http-after-response ruleset
    - BUG/MINOR: cli/server: Don't crash when a server is added with a custom id
    - DOC: spoe: Clarify use of the event directive in spoe-message section
    - DOC: config: Specify %Ta is only available in HTTP mode
    - DOC: config: retry-on list is space-delimited
    - DOC: config: fix error-log-format example
    - BUG/MEDIUM: mworker/cli: crash when trying to access an old PID in prompt mode
    - MINOR: ssl: Remove empty lines from "show ssl ocsp-response" output
    - MINOR: pools: work around possibly slow malloc_trim() during gc
    - BUG/MEDIUM: backend: fix possible sockaddr leak on redispatch
    - BUG/MEDIUM: peers: properly skip conn_cur from incoming messages
    - BUG/MEDIUM: mux-h1: Fix splicing by properly detecting end of message
    - BUG/MINOR: mux-h1: Fix splicing for messages with unknown length
    - BUILD: ssl: unbreak the build with newer libressl
    - DOC: fix misspelled keyword "resolve_retries" in resolvers
    - DEBUG: ssl: make sure we never change a servername on established connections
    - BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time
    - BUG/MEDIUM: ssl: initialize correctly ssl w/ default-server
    - REGTESTS: ssl: fix ssl_default_server.vtc
    - MINOR: compat: detect support for dl_iterate_phdr()
    - MINOR: debug: add ability to dump loaded shared libraries
    - MINOR: debug: add support for -dL to dump library names at boot
    - MINOR: proxy: add option idle-close-on-response
    - MINOR: cpuset: switch to sched_setaffinity for FreeBSD 14 and above.
    - BUILD: makefile: add -Wno-atomic-alignment to work around clang abusive warning
    - CI: Github Actions: do not show VTest failures if build failed
    - BUG/MINOR: ssl: free the fields in srv->ssl_ctx
    - BUG/MEDIUM: ssl: free the ckch instance linked to a server
    - REGTESTS: ssl: update of a crt with server deletion
    - BUILD/MINOR: cpuset FreeBSD 14 build fix.
    - CI: github actions: update OpenSSL to 3.0.1
    - BUILD/MINOR: tools: solaris build fix on dladdr.
    - BUG/MINOR: cli: fix _getsocks with musl libc
    - BUG/MEDIUM: http-ana: Preserve response's FLT_END analyser on L7 retry
    - BUG/MEDIUM: mworker: don't use _getsocks in wait mode
    - BUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello error
    - BUG/MAJOR: mux-h1: Don't decrement .curr_len for unsent data
    - BUILD: cpuset: fix build issue on macos introduced by previous change
    - CI: github actions: clean default step conditions