CLEANUP: ssl: Remove useless loop in tlskeys_list_get_next()

This loop was always exited in the first iteration by `return`.

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

MEDIUM: task: extend the state field to 32 bits

It's been too short for quite a while now and is now full. It's still
time to extend it to 32-bits since we have room for this without
wasting any space, so we now gained 16 new bits for future flags.

The values were not reassigned just in case there would be a few
hidden u16 or short somewhere in which these flags are placed (as
it used to be the case with stream->pending_events).

The patch is tagged MEDIUM because this required to update the task's
process() prototype to use an int instead of a short, that's quite a
bunch of places.

(cherry picked from commit 144f84a09d48e758396ba17f1ddef41b63f30a72)
Add every other prototypes which use a "unsigned short state".
This patch was cherry-picked in 2.3 in order to shut clang warning:
src/mux_h2.c:4085:49: error: implicit conversion from 'int' to 'unsigned short' changes value from -32769 to 32767 [-Werror,-Wconstant-conversion]
        HA_ATOMIC_AND(&h2c->wait_event.tasklet->state, ~TASK_F_USR1);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
include/haproxy/atomic.h:270:62: note: expanded from macro 'HA_ATOMIC_AND'
                                     ~~~~~~~~~~~~~~~~~~      ^~~~~
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: Github Actions: temporarily disable BoringSSL builds

A recent update to BoringSSL broke the build again, and given that
it's not used except for QUIC development, let's temporarily disable
it until the issue is analysed and fixed.

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

CI: Github Actions: switch to LibreSSL-3.3.3

stable LibreSSL-3.3.3 released, let us switch to it

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

CI: github actions: update LibreSSL to 3.2.5

LibreSSL-3.2.5 is released, let us update to it

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

Revert "CI: Pin VTest to a known good commit"

The issue with VTest now is fixed in VTest master since commit
vtest/VTest@040bb6737a6bd6c1f8941bc65ab8f3fc52d71130.

This reverts commit 24105300b87b5cfc1f21fd0788a0865723b043c8.

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

CI: github actions: switch to stable LibreSSL release

LibreSSL-3.3.0 appeared to have its own bugs, it is development release,
let us switch to stable LibreSSL-3.2.4 instead

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

CI: Fix the coverity builds

In an attempt to fix the use of DEBUG_STRICT commit
7f0f4786d1927f1450392e871480e3122796024e unfortunately broke the Coverity
builds completely.

It turns out that Coverity does not properly handle quoting within
`COVERITY_SCAN_BUILD_COMMAND`, instead breaking up single arguments at
whitespace, thus passing `-DDEBUG_USE_ABORT=1` to `make` as-is.

Fix this issue by hijacking the Makefile within the Coverity workflow. We
simply replace the default value of the `DEBUG` option with whatever values we
need. The build command now only includes the TARGET and USE_* flags, each of
which works without any spaces.

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

CI: Fix DEBUG_STRICT definition for Coverity

The DEBUG_STRICT define needs to be passed as part of `DEBUG`, not as a bare
parameter.

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

CI: Pin VTest to a known good commit

As of January, 11th the macOS builds fail due to regression introduced in
VTest. This patch pins VTest to the newest good commit.

This patch should be reverted once VTest's 'master' is stable again.

see vtest/VTest#26

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

CI: github actions: build several popular "contrib" tools

this adds "halog", "flags" and "poll" builds. builds are done in
separate steps for better failure identification

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

CI: GitHub Actions: enable daily Coverity scan

That scan was previously implemented on Travis. Let us migrate
it to GitHub Actions.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
(cherry picked from commit 64b6f367882acf8b82c634e19fb5e3d0e64d11ef)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

CI: github actions: enable 51degrees feature

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

CI: github actions: update LibreSSL to 3.3.0

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

CI: Set DEBUG=-DDEBUG_STRICT=1 in GitHub Actions

This was missing when migrating from Travis.

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

CI: Clean up Windows CI

This patch cleans up the Windows CI to look more similar to the refactored
Linux CI on GitHub Actions.

It switches the environment set-up from some manual cygwin setup via choco to
the msys2/setup-msys2@v2 action which just works and allows later steps to look
like any others without need to manually specify the shell.

This new setup is much faster than before where a single Windows build required
more than 10 minutes with more than 5 minutes just spent setting up the
environment and more than 6 minutes compiling HAProxy.

With this patch the setting of of the environment is done in less than a minute
and HAProxy is compiled in less than 2 minutes.

The only drawback is that Lua does not appear to be readily available. I expect
this to be acceptable and that the benefits far outweight this small drawback.

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

CI: Pass the github.event_name to matrix.py

This is a preparation to later run some matrix entries on schedule only.

Within the matrix.py script it can now be detected whether the workflow is
running on schedule by using:

    if build_type == "schedule":
        matrix.append(...)

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

CI: Github Action: run "apt-get update" before packages restore

ubuntu somehow needs it, no idea why does not apt-get do it itself.

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

CI: Github Actions: enable BoringSSL builds

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

CI: Github Actions: remove LibreSSL-3.0.2 builds

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

CI: Github Actions: enable prometheus exporter

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

CI: Make the h2spec workflow more consistent with the VTest workflow

This PR aims to make the workflow more consistent, by reusing the same wording
for the step names and the same commands to make it look like the vtest
workflow as much as possible.

It was renamed to compliance.yml to match the human readable name better. This
also allows to extend the workflow with other compliance tools later on, nicely
grouping those jobs together in a single file.

No functional changes have been made.

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

CI: Stop hijacking the hosts file

vtest/VTest#24 is merged now. This step is no longer required.

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

CI: Expand use of GitHub Actions for CI

Travis is becoming overall increasingly unreliable lately. We've already
seen that the timing sensitive tests regularly fail and thus they were
disabled.

Additionally they recently announced a new pricing model that caps the number
of minutes for Open Source projects:
https://blog.travis-ci.com/2020-11-02-travis-ci-new-billing

GitHub Actions VMs are working well, possibly allowing to use custom runners
for special tasks in the future.

In addition to this better performance its workflow configuration language
is more expressive compared to the Travis CI one. Specifically the build
matrix does not need to be specified in YAML. Instead it can be generated
ad-hoc using a script. This allows us to cleanly define the various build
configurations without having an unreadable 80 line mess where the flags
are inconsistently activated. As an example in the current Travis CI
configuration the prometheus exporter is tested together with LibreSSL 2.9.2
for whatever reason.

In addition to all the previous points the UI of Travis is not that nice.
On GitHub you are just seeing that "Travis failed" without any details which
exact job failed. This requires you to visit the slow Travis page and look
up the details there. GitHub Actions creates a single entry for each
configuration that is tested, allowing you to see the details without needing
to leave GitHub.

This new GitHub Actions workflow aims to reproduce the configurations tested
in Travis. It comes close, but is not completely there, yet. Consider this
patch a proof of concept that will evolve in the future, ideally with Ilya's
expertise.

The current configurations are as follows. Each one is tested with both gcc
and clang.
- All features disabled (no USE flags)
- All features enabled (all USE flags)
- Standalone test of each of the supported compression libraries:
  - USE_ZLIB=1
  - USE_SLZ=1
- Standalone test of various SSL libraries:
  - stock (the SSL installed by default on the VM)
  - OpenSSL 1.0.2u
  - LibreSSL 2.9.2, 3.0.2, 3.1.1
- All features enabled with ASAN (clang only)

Future additions of new tests should take care to not test unrelated stuff.
Instead a distinct configuration should be added.

Additionally there is a Mac OS test with clang and all features disabled.

Known issues:
- Apparently the git commit is not properly detected during build. The HEAD
  currently shows as 2.4-dev0.

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

DOC: configuration: issuers-chain-path only applies to bind lines

issuers-chain-path was introduced in 2.2 into the ckch codepath, however
as far as 2.3, the ckch only applies to the bind lines, and not the
server ones.

This must be backported to 2.2.

BUG/MAJOR: segfault using multiple log forward sections.

For each new log forward section, the proxy was added to the log forward
proxy list but the ref on the previous log forward section's proxy was
scratched using "init_new_proxy" which performs a memset. After configuration
parsing this list contains only the last section's proxy.

The post processing walk through this list to resolve "ring" names.
Since some section's proxies are missing in this list, the resolving
is not done for those ones and the pointer on the ring is kept to null
causing a segfault at runtime trying to write a log message
into the ring.

This patch shift the "init_new_proxy" before adding the ref on the
previous log forward section's proxy on currently parsed one.

This patch shoud fix github issue #1464

This patch should be backported to 2.3

(cherry picked from commit 2ad2b1c94cc7bb88b879fb0895a3bee566fdf053)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e58cefb519056c2299b1b14c9e598c312c65848f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9ecb3bc2a642e676b6239f07bd837cf14b202f82)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Detach query item on response error

When a new response is parsed, it is unexpected to have an old query item
still attached to the resolution. And indeed, when the response is parsed
and validated, the query item is detached and used for a last check on its
dname. However, this is only true for a valid response. If an error is
detected, the query is not detached. This leads to undefined behavior (most
probably a crash) on the next response because the first element in the
query list is referencing an old response.

This patch must be backported as far as 2.0.

(cherry picked from commit 80b2e34b18b805d28a9569d8157c7bcf088eef8f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3d8fa909ca54a5997abdcc46cc7ec50d0048e720)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 67f6af5b75226d122809e1cc369b8116a8127ce8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: cli: Properly set stream analyzers to process one command at a time

The proxy used by the master CLI is an internal proxy and no filter are
registered on it. Thus, there is no reason to take care to set or unset
filter analyzers in the master CLI analyzers. AN_REQ_FLT_END was set on the
request channel to prevent the infinite forward and be sure to be able to
process one commande at a time. However, the only work because
CF_FLT_ANALYZE flag was used by error as a channel analyzer instead of a
channel flag. This erroneously set AN_RES_FLT_END on the request channel,
that really prevent the infinite forward, be side effet.

In fact, We must avoid this kind of trick because this only work by chance
and may be source of bugs in future. Instead, we must always keep the CLI
request analyzer and add an early return if the response is not fully
processed. It happens when the CLI response analyzer is set.

This patch must be backported as far as 2.0.

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

[RELEASE] Released version 2.3.16

Released version 2.3.16 with the following main changes :
    - BUG/MEDIUM: mux-h1: Fix H1C_F_ST_SILENT_SHUT value
    - BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3
    - DOC: config: Fix typo in ssl_fc_unique_id description
    - BUG/MINOR: http-ana: Apply stop to the current section for http-response rules
    - Revert "BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back"
    - DOC: lua: Be explicit with the Reply object limits
    - BUG/MEDIUM: conn-stream: Don't reset CS flags on close
    - BUG/MINOR: mworker: doesn't launch the program postparser
    - BUG/MINOR: mux-h2: Fix H2_CF_DEM_SHORT_READ value
    - BUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent
    - BUG/MINOR: stick-table/cli: Check for invalid ipv6 key
    - MINOR: connection: add a new CO_FL_WANT_DRAIN flag to force drain on close
    - MINOR: mux-h2: perform a full cycle shutdown+drain on close
    - BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found
    - BUG/MEDIUM: mux-h2: always process a pending shut read
    - BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found
    - BUG/MINOR: shctx: do not look for available blocks when the first one is enough
    - MINOR: shctx: add a few BUG_ON() for consistency checks

MINOR: shctx: add a few BUG_ON() for consistency checks

The shctx code relies on sensitive conditions that are hard to infer
from the code itself, let's add some BUG_ON() to verify them. They
helped spot the previous bugs.

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

BUG/MINOR: shctx: do not look for available blocks when the first one is enough

In shctx_row_reserve_hot() we only leave if we've found the exact
requested size instead of at least as large, as is documented. This
results in extra lookups and free calls in the avail loop while it is
not needed, and participates to seeing a negative data_len early as
spotted in previous bugs.

It doesn't seem to have any other impact however, but it's better to
backport it to stable branches.

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

BUG/MEDIUM: shctx: leave the block allocator when enough blocks are found

In shctx_row_reserve_hot(), a missing break allows the avail loop to
loop for a while after having allocated the required blocks, possibly
leading to the point where it could trigger the watchdog after checking
up to 2 million blocks. In addition, the extra iteration may leave one
block assigned with size zero at the head of the avail list, and mark
it as being an isolated chain of 1 block. It's unclear whether this
could have had other consequences.

There is a non-negligible chance that it addreses bugs #1451 and #1284,
as the pattern observed in the loop looks exactly the same as the one
reported there in the crashes.

It's only marked medium because it is extremely hard to trigger. Here
the conditions were reproduced when starting 4k connections at once
requesting objects of random sizes between 0 and 20k to store them into
a small 1MB cache. However the watchdog will never trigger in such a case
so one needs to instrument the functions.

Thanks to Sohaib Ahmad and @g0uZ for providing useful traces.

This will need to be backported to all stable branches.

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

BUG/MEDIUM: mux-h2: always process a pending shut read

During 2.4-dev, an issue with partial frames was fixed with commit
3d4631fec ("BUG/MEDIUM: mux-h2: fix read0 handling on partial frames").
However this patch is not completely correct. It makes h2_recv() return
0 if the connection was shut for reads, but this not make h2_io_cb()
call h2_process(), so if there are any pending data left in the demux
buffer, they will never be processed, and the I/O callback will be
called in loops forever from the poller.

The correct return value there is 1, as is done at the end of the
function to report a pending read0.

This should definitely fix issue #1328. However even after a lot of
tests I couldn't manage to reproduce it, the conditions to enter that
situation are quite racy.

This must be backported to 2.0 since the fix above was merged into
2.0.21 and 2.2.9.

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

BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found

Since commit c2aae74 ("MEDIUM: ssl: Handle early data with OpenSSL
1.1.1"), the codepath of the clientHello callback changed, letting an
unknown SNI escape with a 'return 1' instead of passing through the
abort label.

An error was still emitted because the frontend continued the handshake
with the initial_ctx, which can't be used to achieve an handshake.
However, it had the ugly side effect of letting the request pass in the
case of a TLS resume. Which could be surprising when combining strict-sni
with the removing of a crt-list entry over the CLI for example. (like
its done in the ssl/new_del_ssl_crlfile.vtc reg-test).

This patch switches the code path of the allow_early and abort label, so
the default code path is the abort one, letting the clientHello returns
the correct SSL_AD_UNRECOGNIZED_NAME in case of errors.

Which means the client will now receive:

OpenSSL error[0x14094458] ssl3_read_bytes: tlsv1 unrecognized name

Instead of:

OpenSSL error[0x14094410] ssl3_read_bytes: sslv3 alert handshake failure

Which was the error emitted before HAProxy 1.8.

This patch must be carrefuly backported as far as 1.8 once we validated
its impact.

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

MINOR: mux-h2: perform a full cycle shutdown+drain on close

While in H1 we can usually close quickly, in H2 a client might be sending
window updates or anything while we're sending a GOAWAY and the pending
data in the socket buffers at the moment the close() is performed on the
socket results in the output data being lost and an RST being emitted.

One example where this happens easily is with h2spec, which randomly
reports connection resets when waiting for a GOAWAY while haproxy sends
it, as seen in issue #1422. With h2spec it's not window updates that are
causing this but the fact that h2spec has to upload the payload that
comes with invalid frames to accommodate various implementations, and
does that in two different segments. When haproxy aborts on the invalid
frame header, the payload was not yet received and causes an RST to
be sent.

Here we're dealing with this two ways:
  - we perform a shutdown(WR) on the connection to forcefully push pending
    data on a front connection after the xprt is shut and closed ;
  - we drain pending data
  - then we close

This totally solves the issue with h2spec, and the extra cost is very
low, especially if we consider that H2 connections are not set up and
torn down often. This issue was never observed with regular clients,
most likely because this pattern does not happen in regular traffic.

After more testing it could make sense to backport this, at least to
avoid reporting errors on h2spec tests.

(cherry picked from commit 0b222476062d76a1660892a81e9a97962dce1582)
[cf: depends on "MINOR: connection: add a new CO_FL_WANT_DRAIN flag to force
     drain on close". This patch is in fact a bug fix, related to the issue
     #1297.]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit daa0e5f7d152619962a81983d2e7391246a6e7b4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: connection: add a new CO_FL_WANT_DRAIN flag to force drain on close

Sometimes we'd like to do our best to drain pending data before closing
in order to save the peer from risking to receive an RST on close.

This adds a new connection flag CO_FL_WANT_DRAIN that is used to
trigger a call to conn_ctrl_drain() from conn_ctrl_close(), and the
sock_drain() function ignores fd_recv_ready() if this flag is set,
in order to catch latest data. It's not used for now.

(cherry picked from commit 20b622e04b27e76f7b4be0c09b435e8799143d75)
[Cf: On previous versions, conn_sock_drain() must be used instead of
     conn_ctrl_drain()]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit b17952cec409252817d09b95a1a97ffaff895fe9)
[Cf: As expected, conn_sock_drain() was used instead of
     conn_ctrl_drain(). Because sock_drain() does not exist on the 2.3,
     conn_sock_drain() was updated to do nothing when CO_FL_WANT_DRAIN flag
     is not set]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: stick-table/cli: Check for invalid ipv6 key

When an ipv6 key is used to filter a CLI command on a stick table
(clear/set/show table ...), the return value of inet_pton() call must be
checked to be sure the key is valid.

This patch should fix the issue #1163. It should be backported to all
supported versions.

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

BUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent

In 1.8 when muxes and conn_streams were introduced, the call to
conn_full_close() was replaced with a call to cs_close() which only
relied on shutr/shutw (commits 6978db35e ("MINOR: connection:
add cs_close() to close a conn_stream") and a553ae96f ("MEDIUM:
connection: replace conn_full_close() with cs_close()")). By then
this was fine, and the rare risk of non-idempotent calls was addressed
by the muxes implementing the functions (e.g. mux_pt).

Later with commit 325607397 ("MEDIUM: stream: do not forcefully close
the client connection anymore"), stream_free() started to call cs_close()
instead of forcibly closing the connection via conn_full_close(). At this
point this started to break idempotence because it was possible to emit
a shutw() (e.g. when option httpclose was set), then to have it called
agian upon stream_free() via cs_close(). By then it was not a problem
since only mux_pt would implement this and did check for idempotence.

When HTX was implemented and mux-h1/h2 offered support for shutw/shutr,
the idempotence changed a little bit because the last shutdown mode
(normal/silent) was recorded and used at the moment of closing. The
call to cs_close() uses the silent mode and will replace the current
one. This has an effect on data pending in the buffer if the FIN could
not be sent before cs_close(), because lingering may be disabled and
final data lost in the network stack.

Interestingly, during 2.4-dev3, this was addressed as the side effect
of an improvement by commit 3c82d8b32 ("MINOR: mux-h1: Rework how
shutdowns are handled"), where the H1 mux's shutdown function becomes
explicitly idempotent. However older versions (2.3 to 2.0) do not have
it.

This patch addresses the issue globally by making sure that cs_shutr()
and cs_shutw() are idempotent and cannot have their data truncated by
a late cs_close(). It fixes the truncation that is observed in 2.3 and
2.2 as described in issue #1450.

This must be backported as far as 2.0, along with commit f14d750bf
("BUG/MEDIUM: conn-stream: Don't reset CS flags on close") which it
depends on.

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

BUG/MINOR: mux-h2: Fix H2_CF_DEM_SHORT_READ value

The value for H2_CF_DEM_SHORT_READ flag is wrong. 2 bits are erroneously
set, 0x200 and 0x80000.  It is not an issue because both bits are not used
anywhere else.

The typo was introduced in the commit b5f7b5296 ("BUG/MEDIUM: mux-h2: Handle
remaining read0 cases on partial frames"). Thus this patch must also be
backported as far a 2.0.

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

BUG/MINOR: mworker: doesn't launch the program postparser

When in wait mode, the mworker-prog postparser is launched, but
unfortunately the child structure doesn't contain all required
information to be able to launch the test.

This test is only required when doing a configuration parsing.

Must be backported as far as 2.0.

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

BUG/MEDIUM: conn-stream: Don't reset CS flags on close

cs_close() and cs_drain_and_close() are called to close a conn-stream.
cs_shutr() and cs_shutw() are called with the appropriate modes. But the
conn-stream is not released at this stage. However the flags are
reset. Thus, after a cs_close(), we loose shutdown flags. If cs_close() is
performed several times, it is a problem. And indeed, it is possible. On one
hand, the stream-interface may close the conn-stream. On the other end, the
stream always closes it when it is released.

It is a problem for the H1 multiplexer. Because the conn-stream flags are
reset, the shutr and shutw are performed twice. For a delayed shutdown, the
dirty mode is used instead of the normal one because the last call to
h1_shutw() overwrite H1C flags set by the first call. This leads to dirty
shutdowns while normal ones are required. At the end, it is possible to
truncate the messages.

This bug was revealed by the commit a85c522d4 ("BUG/MINOR: mux-h1: Save
shutdown mode if the shutdown is delayed").

This patch is related to the issue #1450. It must be backported as far as
2.0.

(cherry picked from commit f14d750bf7ef6ce95fcf0de14a423c456ebf82c5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 68a5fa373ead9552c470e14615001653f94dc7ee)
[cf: cs_drain_and_close does not exist on the 2.3]]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: lua: Be explicit with the Reply object limits

In HTTP, when a lua action is evaluated, a reply object can be used to send
a response to the client and interrupt the transaction. This reply object is
converted into HTX and is limited to the response channel buffer. Its size,
once converted, cannot exceed the buffer size. There is no streaming at this
stage. However, this limitation was not documented.

Note that, for now, there is no easy way to know if the reply will fit or
not int the response channel buffer. Thus the reply must be reasonably
small. Otherwise a 500-Internal-Error message is returned.

This patch is related to the issue #1447. It may be backported as far as
2.2.

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

Revert "BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back"

This reverts commit 597909f4e67866c4f3ecf77f95f2cd4556c0c638

http-after-response rules evaluation was changed to do the same that was
done for http-response, in the code. However, the opposite must be performed
instead. Only the rules of the current section must be stopped. Thus the
above commit is reverted and the http-response rules evaluation will be
fixed instead.

Note that only "allow" action is concerned. It is most probably an uncommon
action for an http-after-request rule.

This patch must be backported as far as 2.2 if the above commit was
backported.

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

BUG/MINOR: http-ana: Apply stop to the current section for http-response rules

A TCP/HTTP action can stop the rules evaluation. However, it should be
applied on the current section only. For instance, for http-requests rules,
an "allow" on a frontend must stop evaluation of rules defined in this
frontend. But the backend rules, if any, must still be evaluated.

For http-response rulesets, according the configuration manual, the same
must be true. Only "allow" action is concerned. However, since the
beginning, this action stops evaluation of all remaining rules, not only
those of the current section.

This patch may be backported to all supported versions. But it is not so
critical because the bug exists since a while. I doubt it will break any
existing configuration because the current behavior is
counterintuitive.

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

DOC: config: Fix typo in ssl_fc_unique_id description

In ssl_fc_unique_id decription, threre is a reference to the wrong sample
fetch. ssl_bc_unique_id is used instead of ssl_fc_unique_id.

This patch should fix the issue #1449. It may be backported to all
supportted versions.

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

BUG/MEDIUM: ssl: backend TLS resumption with sni and TLSv1.3

When establishing an outboud connection, haproxy checks if the cached
TLS session has the same SNI as the connection we are trying to
resume.

This test was done by calling SSL_get_servername() which in TLSv1.2
returned the SNI. With TLSv1.3 this is not the case anymore and this
function returns NULL, which invalidates any outboud connection we are
trying to resume if it uses the sni keyword on its server line.

This patch fixes the problem by storing the SNI in the "reused_sess"
structure beside the session itself.

The ssl_sock_set_servername() now has a RWLOCK because this session
cache entry could be accessed by the CLI when trying to update a
certificate on the backend.

This fix must be backported in every maintained version, however the
RWLOCK only exists since version 2.4.

(cherry picked from commit e18d4e82862eb5c9b54b89f85d4c3b7c82d7395e)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit c78ac5ade6bdaf46634edf08fecd735ab7d0a5f1)
[wla: ha_free was replaced by free+NULL, some context changed, there is
no server side certificate change on the CLI in 2.3, so the backend
cache is not flushed]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

BUG/MEDIUM: mux-h1: Fix H1C_F_ST_SILENT_SHUT value

The commit 7700441a25 ("BUG/MINOR: mux-h1: Save shutdown mode if the
shutdown is delayed") introduced a regression during the backport process.
The H1C_F_ST_SILENT_SHUT value was not changed. However, on the 2.3 and
lowers, another H1C flag alreay have the value.

Because of this bug, H1C_F_ST_SILENT_SHUT and H1C_F_CS_SHUTDOWN shared the
same value. Mixed with some other bugs, this may lead to truncated messages.

This patch help to fix issue #1450. There is no upstream ID because the bug
only affects the 2.3 and lowers. The patch must be backported as far as 2.0.

[RELEASE] Released version 2.3.15

Released version 2.3.15 with the following main changes :
    - MINOR: lua: Add a flag on lua context to know the yield capability at run time
    - BUG/MINOR: lua: Yield in channel functions only if lua context can yield
    - BUG/MINOR: lua: Don't yield in channel.append() and channel.set()
    - BUG/MINOR: stream: Don't release a stream if FLT_END is still registered
    - BUG/MEDIUM: http-ana: Reset channels analysers when returning an error
    - BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set
    - BUG/MINOR: filters: Set right FLT_END analyser depending on channel
    - BUG/MINOR: systemd: ExecStartPre must use -Ws
    - BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is reached
    - Revert "REGTESTS: mark http_abortonclose as broken"
    - BUG/MINOR: compat: make sure __WORDSIZE is always defined
    - CLEANUP: pools: factor all malloc_trim() calls into trim_all_pools()
    - MINOR: pools: automatically disable malloc_trim() with external allocators
    - MINOR: pools: use mallinfo2() when available instead of mallinfo()
    - BUG/MINOR: tcpcheck: Improve LDAP response parsing to fix LDAP check
    - DOC: management: certificate files must be sanitized before injection
    - BUILD: ist: prevent gcc11 maybe-uninitialized warning on istalloc
    - BUG/MINOR: cli/payload: do not search for args inside payload
    - BUG/MINOR: server: allow 'enable health' only if check configured
    - BUG/MINOR: h1-htx: Fix a typo when request parser is reset
    - BUG/MINOR: http-ana: increment internal_errors counter on response error
    - BUG/MEDIUM: mux-h1: Adjust conditions to ask more space in the channel buffer
    - BUG/MEDIUM: stream-int: Notify stream that the mux wants more room to xfer data
    - BUG/MEDIUM: stream: Stop waiting for more data if SI is blocked on RXBLK_ROOM
    - BUG/MINOR: mux-h1/mux-fcgi: Sanitize TE header to only send "trailers"
    - DOC: peers: fix doc "enable" statement on "peers" sections
    - BUG/MEDIUM: lua: fix wakeup condition from sleep()
    - BUG/MAJOR: lua: use task_wakeup() to properly run a task once
    - MINOR: arg: Be able to forbid unresolved args when building an argument list
    - BUG/MINOR: tcpcheck: Don't use arg list for default proxies during parsing
    - BUG/MINOR: tcp-rules: Stop content rules eval on read error and end-of-input
    - MINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()
    - BUG/MEDIUM: filters: Fix a typo when a filter is attached blocking the release
    - BUG/MEDIUM: http-ana: Clear request analyzers when applying redirect rule
    - BUG/MEDIUM: leastconn: fix rare possibility of divide by zero
    - MINOR: htx: Add an HTX flag to know when a message is fragmented
    - MINOR: htx: Add a function to know if the free space wraps
    - BUG/MEDIUM: stream-int: Defrag HTX message in si_cs_recv() if necessary
    - BUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames
    - MINOR: initcall: Rename __GLOBL and __GLOBL1.
    - BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back
    - BUG/MEDIUM: stream: Keep FLT_END analyzers if a stream detects a channel error
    - CLEANUP: sample: rename sample_conv_var2smp() to *_sint
    - CLEANUP: sample: uninline sample_conv_var2smp_str()
    - MINOR: sample: provide a generic var-to-sample conversion function
    - BUG/MEDIUM: sample: properly verify that variables cast to sample
    - BUG/MEDIUM: tcpcheck: Properly catch early HTTP parsing errors
    - MINOR: resolvers: fix the resolv_str_to_dn_label() API about trailing zero
    - BUG/MEDIUM: resolver: make sure to always use the correct hostname length
    - BUG/MINOR: resolvers: do not reject host names of length 255 in SRV records
    - MINOR: resolvers: fix the resolv_dn_label_to_str() API about trailing zero
    - BUG/MEDIUM: resolvers: fix truncated TLD consecutive to the API fix
    - BUG/MEDIUM: resolvers: use correct storage for the target address
    - MINOR: resolvers: merge address and target into a union "data"
    - BUG/MAJOR: resolvers: add other missing references during resolution removal
    - BUILD: resolvers: avoid a possible warning on null-deref
    - BUG/MEDIUM: resolvers: always check a valid item in query_list
    - BUG/MAJOR: buf: fix varint API post- vs pre- increment
    - BUG/MINOR: mux-h2: do not prevent from sending a final GOAWAY frame
    - BUILD: fix compilation on NetBSD
    - BUG/MINOR: mux-h1: Save shutdown mode if the shutdown is delayed
    - BUG/MEDIUM: mux-h1: Perform a connection shutdown when the h1c is released
    - CLEANUP: resolvers: do not export resolv_purge_resolution_answer_records()
    - CLEANUP: always initialize the answer_list
    - CLEANUP: resolvers: simplify resolv_link_resolution() regarding requesters
    - CLEANUP: resolvers: replace all LIST_DELETE with LIST_DEL_INIT
    - MEDIUM: resolvers: use a kill list to preserve the list consistency
    - MEDIUM: resolvers: remove the last occurrences of the "safe" argument
    - BUG/MEDIUM: resolvers: Don't recursively perform requester unlink
    - BUG/MEDIUM: resolvers: Track api calls with a counter to free resolutions
    - BUG/MEDIUM: http-ana: Drain request data waiting the tarpit timeout expiration
    - BUG/MINOR: http: Authorization value can have multiple spaces after the scheme
    - BUG/MEDIUM: stream-int: Block reads if channel cannot receive more data
    - BUG/MINOR: vars: improve accuracy of the rules used to check expression validity
    - MINOR: sample: add missing ARGC_ entries
    - BUG/MINOR: vars: properly set the argument parsing context in the expression
    - BUG/MEDIUM: sample: Cumulate frontend and backend sample validity flags
    - BUG/MINOR: sample: fix backend direction flags consecutive to last fix
    - DOC: config: Fix alphabetical order of fc_* samples
    - MINOR: halog: Add -qry parameter allowing to preserve the query string in -uX
    - DOC: halog: Move the `-qry` parameter into the correct section in help text
    - MINOR: halog: Rename -qry to -query
    - CLEANUP: halog: Use consistent indentation in help()
    - BUG/MINOR: halog: Add missing newlines in die() messages
    - MINOR: halog: Add support for extracting captures using -hdr
    - MINOR: stream: Improve dump of bogus streams
    - DOC/peers: some grammar fixes for peers 2.1 spec
    - SCRIPTS: git-show-backports: re-enable file-based filtering

SCRIPTS: git-show-backports: re-enable file-based filtering

The early version of the script used to support passing non-branch
arguments but as it evolved we lost that option. Let's use "--" as a
delimiter after the branch(es) to pass optional file names to filter
on. This is convenient to list missing patches on a specific set of
files.

(cherry picked from commit 0d026edaefcee30d5532a9fde7cbfbeed411379e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 98115e7e95500c6ee50868b44a527fd853d9ce62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC/peers: some grammar fixes for peers 2.1 spec

These are a few minor grammar fixes for the peers protocol 2.1 documentation.

(cherry picked from commit 7f31fec8961e2512b0eaca5855a3674a5ee245de)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 1b3956f4d145c93fe8e956ec4bf7bc8b94b91662)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: stream: Improve dump of bogus streams

Stream flags and information about the HTTP txn, if defined, are now
emitted. This will help us to identify bugs when such message is reported.

(cherry picked from commit e8f3596cd0be8044b0c2e71deb7d100cfe030c79)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c86efe14acbfa2379255984c5b8294f75a1b725d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: halog: Add support for extracting captures using -hdr

This patch adds support for extracting captured header fields to halog. A field
can be extracted by passing the `-hdr <block>:<field>` output filter.

Both `<block>` and `<field>` are 1-indexed.

`<block>` refers to the index of the brace-delimited list of headers. If both
request and response headers are captured, then request headers are referenced
by `<block> = 1`, response headers are `2`. If only one direction is captured,
there will only be a single block `1`.

`<field>` refers to a single field within the selected block.

The output will contain one line, possibly empty, per log line processed.
Passing a non-existent `<block>` or `<field>` will result in an empty line.

Example:

    capture request  header a len 50
    capture request  header b len 50
    capture request  header c len 50
    capture response header d len 50
    capture response header e len 50
    capture response header f len 50

`-srv 1:1` will extract request  header `a`
`-srv 1:2` will extract request  header `b`
`-srv 1:3` will extract request  header `c`
`-srv 2:3` will extract response header `f`

This resolves GitHub issue #1146.

(cherry picked from commit 66255f7bbf9dfa18545d96f87d7a0f6fb8684d1c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a7e8485838e30f921b9d34045f0ebf8672bb0259)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: halog: Add missing newlines in die() messages

This newline is required to correctly print the usage.

(cherry picked from commit a02c7b882f34fbb29cc71bf5ce744b0f42bacbe3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 75a0a1f88df53ae48874a8173e685d4b77c2fe6f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: halog: Use consistent indentation in help()

Consistently use 1 Tab per line.

(cherry picked from commit e0992584b9c21018dffad1fef211be6493bf7200)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit aefb6301ee051717442315af46be53f8c8f39e75)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: halog: Rename -qry to -query

With the query flag moved into the correct help section, there is enough space
for two additional characters.

(cherry picked from commit 385338b30ba7bcd18688b82ad6467014d8b46749)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a88266698bcf0781cfb96daa74bfe89822c0649e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: halog: Move the `-qry` parameter into the correct section in help text

This is not an output filter, but instead a modifier. Specifically "only one
may be used at a time" is not true.

see 24b8d693b202b01b649f64ed878d8f9dd1b242e4

(cherry picked from commit a0635f8d1030d52b19bbe89cefabb74151af34f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit eb6bf96472ecea058fa4f76430b2886ecbcaccc0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: halog: Add -qry parameter allowing to preserve the query string in -uX

Our use-case for this is a dynamic application that performs routing based on
the query string. Without this option all URLs will just point to the central
entrypoint of this location, making the output completely useless.

(cherry picked from commit 24b8d693b202b01b649f64ed878d8f9dd1b242e4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b09bdeecdf202f91ade21a92cd6cb62baf914f12)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

DOC: config: Fix alphabetical order of fc_* samples

fc_* samples were not properly ordered. This patch may be backported as far
as 1.8.

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

BUG/MINOR: sample: fix backend direction flags consecutive to last fix

Commit 7a06ffb85 ("BUG/MEDIUM: sample: Cumulate frontend and backend
sample validity flags") introduced a typo confusing the request and
the response direction when checking for validity of a rule applied
to a backend. This was reported by Coverity in issue #1417.

This needs to be backported where the patch above is backported.

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

BUG/MEDIUM: sample: Cumulate frontend and backend sample validity flags

When the sample validity flags are computed to check if a sample is used in
a valid scope, the flags depending on the proxy capabilities must be
cumulated. Historically, for a sample on the request, only the frontend
capability was used to set the sample validity flags while for a sample on
the response only the backend was used. But it is a problem for listen or
defaults proxies. For those proxies, all frontend and backend samples should
be valid. However, at many place, only frontend ones are possible.

For instance, it is impossible to set the backend name (be_name) into a
variable from a listen proxy.

This bug exists on all stable versions. Thus this patch should probably be
backported. But with some caution because the code has probably changed
serveral times. Note that nobody has ever noticed this issue. So the need to
backport this patch must be evaluated for each branch.

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

BUG/MINOR: vars: properly set the argument parsing context in the expression

When the expression called in "set-var" uses argments that require late
resolution, the context must be set. At the moment, any unknown argument
is misleadingly reported as "ACL":

    frontend f
        bind :8080
        mode http
        http-request set-var(proc.a) be_conn(foo)

   parsing [b1.cfg:4]: unable to find backend 'foo' referenced in arg 1 \
   of ACL keyword 'be_conn' in proxy 'f'.

Once the context is properly set, it now says the truth:

   parsing [b1.cfg:8]: unable to find backend 'foo' referenced in arg 1 \
   of sample fetch keyword 'be_conn' in http-request expression in proxy 'f'.

This may be backported but is not really important. If so, the preceeding
patches "BUG/MINOR: vars: improve accuracy of the rules used to check
expression validity" and "MINOR: sample: add missing ARGC_ entries" must
be backported as well.

(cherry picked from commit 54b96d99556a27e15ee77b8b498d011cda2771e9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit a59858fe2410c07670b7a2d00f4c0e1aa9a67bf1)
[cf: backported to ease other backports]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: sample: add missing ARGC_ entries

For a long time we couldn't have arguments in expressions used in
tcp-request, tcp-response etc rules. But now due to the variables
it's possible, and their context in case of failure to resolve an
argument (e.g. backend name not found) is not properly reported
because there is no arg context values in ARGC_* to report them.

Let's add a number of missing ones for tcp-request {connection,
session,content}, tcp-response content, tcp-check, the config
parser (for "set-var" in the global section) and the CLI parser
(for "set-var" on the CLI).

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

BUG/MINOR: vars: improve accuracy of the rules used to check expression validity

The set-var() expression naturally checks whether expressions are valid
in the context of the rule, but it fails to differentiate frontends from
backends. As such for tcp-content and http-request rules, it will only
accept frontend-compatible sample-fetches, excluding those declared with
SMP_UES_BKEND (a few such as be_id, be_name). For the response it accepts
the backend-compatible expressions only, though it seems that there are
no sample-fetch function that are valid only in the frontend's content,
so that should not cause any problem.

Note that while allowing valid configs to be used, the fix might also
uncover some incorrect configurations where some expressions currently
return nothing (e.g. something depending on frontend declared in a
backend), and which could be rejected, but there does not seem to be
any such keyword. Thus while it should be backported, better not backport
it too far (2.4 and possibly 2.3 only).

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

BUG/MEDIUM: stream-int: Block reads if channel cannot receive more data

First of all, we must be careful here because this part was modified and
each time, this introduced a bug. But, in si_update_rx(), we must not
re-enables receives if the channel buffer cannot receive more
data. Otherwise the multiplexer will be wake up for nothing. Because the
stream is woken up when the multiplexer is waiting for more room to move on,
this may lead to a ping-pong loop between the stream and the mux.

Note that for now, it does not fix any known bug. All reported issues in
this area were fixed in another way.

This patch must be backported with a special care. Technically speaking, it
may be backported as far as 2.0.

(cherry picked from commit 69fad00ebf65ad0dedabaf10db43fb3783f7ecae)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit dce066777a3072945c3b52210f503104139c2b0d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: http: Authorization value can have multiple spaces after the scheme

As per RFC7235, there can be multiple spaces in the value of an
Authorization header, between the scheme and the actual authentication
parameters.

This can be backported to all stable versions since basic auth has almost
always been there.

(cherry picked from commit 68c4eae87f2366a9485f5d09250d7ec82d9a1b94)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2ad2ed46262a19b6012fe0ceb0243fe04aff9b71)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: http-ana: Drain request data waiting the tarpit timeout expiration

When a tarpit action is performed, we must be sure to drain data from the
request channel. Otherwise, the mux on the frontend side may be blocked
because the request channel buffer is full.

This may lead to Two bugs. The first one is a HOL blocking on the H2
multiplexer. A tarpitted stream may block all the others because data are
not drained for the whole tarpit timeout. The second bug is a ping-pong loop
between the multiplexer and the stream. The mux is waiting for more space in
the channel buffer, so it wakes up the stream. And the stream systematically
re-enables receives.

This last part is not pretty clean and it will be addressed with another
fix. But draning request data is a good way to fix both bugs in same time.

This patch must be backported as far as 2.0. The legacy HTTP mode is
probably affected, but I don't know if same bugs may be experienced in this
mode.

(cherry picked from commit b0c87f1c61edbc4e4affd81d2fd02e113528cf03)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b02d5f024ee379102320e6fe514aa902d1b26a47)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Track api calls with a counter to free resolutions

The kill list introduced in commit f766ec6b5 ("MEDIUM: resolvers: use a kill
list to preserve the list consistency") contains a bug. The deatch_row must
be initialized before calling resolv_process_responses() function. However,
this function is called for the dns code. The death_row is not visible from
the outside. So, it is possible to add a resolution in an uninitialized
death_row, leading to a crash.

But, with the current implementation, it is not possible to handle the
death_row in resolv_process_responses() function because, internally, the
kill list may be freed via a call to resolv_unlink_resolution(). At the end,
we are unable to determine all call chains to guarantee a safe use of the
kill list. It is a shameful observation, but unfortunatly true.

So, to make the fix simple, we track all calls to the public resolvers
api. A counter is incremented when we enter in the resolver code and
decremented when we leave it. This way, we are able to track the recursions
to init and release the kill list only once, at the edge.

Following functions are incrementing/decrementing the recurse counter:

  * resolv_trigger_resolution()
  * resolv_srvrq_expire_task()
  * resolv_link_resolution()
  * resolv_unlink_resolution()
  * resolv_detach_from_resolution_answer_items()
  * resolv_process_responses()
  * process_resolvers()
  * resolvers_finalize_config()
  * resolv_action_do_resolve()

This patch should fix the issue #1404. It must be backported everywhere the
above commit was backported.

(cherry picked from commit 9ed1a0601d6b5ccaf6d410fee4d5d4b49d36652a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7a3a5361324d0b40a723f560e977e7370f9471da)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: Don't recursively perform requester unlink

When a requester is unlink from a resolution, by reading the code, we can
have this call chain:

_resolv_unlink_resolution(srv->resolv_requester)
  resolv_detach_from_resolution_answer_items(resolution, requester)
    resolv_srvrq_cleanup_srv(srv)
      _resolv_unlink_resolution(srv->resolv_requester)

A loop on the resolution answer items is performed inside
resolv_detach_from_resolution_answer_items(). But by reading the code, it
seems possible to recursively unlink the same requester.

To avoid any loop at this stage, the requester clean up must be performed
before the call to resolv_detach_from_resolution_answer_items(). This way,
the second call to _resolv_unlink_resolution() does nothing and returns
immediately because the requester was already detached from the resolution.

This patch is related to the issue #1404. It must be backported as far as
2.2.

(cherry picked from commit bce6db6c3c208191e8c1962e3ded0dbfc015d052)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e7e0e4be2cbe7e03b78753b2a09ae903aa9bffa7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MEDIUM: resolvers: remove the last occurrences of the "safe" argument

This one was used to indicate whether the callee had to follow particularly
safe code path when removing resolutions. Since the code now uses a kill
list, this is not needed anymore.

(cherry picked from commit 6878f80427aa8befc6662d85caddb8affe799b9c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 33360872d1c56a1cc283ba79c736a71bcf7ce464)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MEDIUM: resolvers: use a kill list to preserve the list consistency

When scanning resolution.curr it's possible to try to free some
resolutions which will themselves result in freeing other ones. If
one of these other ones is exactly the next one in the list, the list
walk visits deleted nodes and causes memory corruption, double-frees
and so on. The approach taken using the "safe" argument to some
functions seems to work but it's extremely brittle as it is required
to carefully check all call paths from process_ressolvers() and pass
the argument to 1 there to refrain from deleting entries, so the bug
is very likely to come back after some tiny changes to this code.

A variant was tried, checking at various places that the current task
corresponds to process_resolvers() but this is also quite brittle even
though a bit less.

This patch uses another approach which consists in carefully unlinking
elements from the list and deferring their removal by placing it in a
kill list instead of deleting them synchronously. The real benefit here
is that the complexity only has to be placed where the complications
are.

A thread-local list is fed with elements to be deleted before scanning
the resolutions, and it's flushed at the end by picking the first one
until the list is empty. This way we never dereference the next element
and do not care about its presence or not in the list. One function,
resolv_unlink_resolution(), is exported and used outside, so it had to
be modified to use this list as well. Internal code has to use
_resolv_unlink_resolution() instead.

(cherry picked from commit f766ec6b536725870db2ef48300c371c70c7dda0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2075165e20e5aa08382bf290eaa9bf08ad28f579)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: replace all LIST_DELETE with LIST_DEL_INIT

The code as it is uses crossed lists between many elements, and at
many places the code relies on list iterators or emptiness checks,
which does not work with only LIST_DELETE. Further, it is quite
difficult to place debugging code and checks in the current situation,
and gdb is helpless.

This code replaces all LIST_DELETE calls with LIST_DEL_INIT so that
it becomes possible to trust the lists.

(cherry picked from commit aae7320b0dd9a379792d7cbe3304b23bdb045740)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8c3c523097988005ee69128ace10bcc16c81398b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: simplify resolv_link_resolution() regarding requesters

This function allocates requesters by hand for each and every type. This
is complex and error-prone, and it doesn't even initialize the list part,
leaving dangling pointers that complicate debugging.

This patch introduces a new function resolv_get_requester() that either
returns the current pointer if valid or tries to allocate a new one and
links it to its destination. Then it makes use of it in the function
above to clean it up quite a bit. This allows to remove complicated but
unneeded tests.

(cherry picked from commit 239675e4a955b219e915fa11a1a03c7aacc13ccd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 4d7ca9149914b1188e117608a96b5608232f0bc5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: always initialize the answer_list

Similar to the previous patch, the answer's list was only initialized the
first time it was added to a list, leading to bogus outdated pointer to
appear when debugging code is added around it to watch it. Let's make
sure it's always initialized upon allocation.

(cherry picked from commit 48664c048d17486b80e58459d2d4fa41a36e579e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a2373e82f574fdba5fb52ff0ef917b16dd44484a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: resolvers: do not export resolv_purge_resolution_answer_records()

This code is dangerous enough that we certainly don't want external code
to ever approach it, let's not export unnecessary functions like this one.
It was made static and a comment was added about its purpose.

(cherry picked from commit 2acc160c05c0c8dc28ecce5941451c916d29bc63)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit aab7593db39e1083a77ac792723623bd9c402273)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: mux-h1: Perform a connection shutdown when the h1c is released

When the H1 connection is released, a connection shutdown is now performed.
If it was already performed when the stream was detached, this action has no
effect. But it is mandatory, when an idle H1C is released. Otherwise the
xprt and the socket shutdown is never perfmed. It is especially important
for SSL client connections, because it is the only way to perform a clean
SSL shutdown.

Without this patch, SSL_shutdown is never called, preventing, among other
things, the SSL session caching.

This patch depends on the commit "BUG/MINOR: mux-h1: Save shutdown mode if
the shutdown is delayed". It should be backported as far as 2.0.

(cherry picked from commit e76b4f055d8d6a5514f5c67e37ad7a8858b9eed2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 37c42549234d4b3caab795af96fa8a9866629e7b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h1: Save shutdown mode if the shutdown is delayed

The connection shutdown may be delayed if there are pending outgoing
data. The action is performed once data are fully sent. In this case the
mode (dirty/clean) was lost and a clean shutdown was always performed. Now,
the mode is saved to be sure to perform the connection shutdown using the
right mode. To do so, H1C_F_ST_SILENT_SHUT flag is introduced.

This patch should be backported as far as 2.0.

(cherry picked from commit a85c522d42f866d57cd79335a945402b81cb3948)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit db451fb7d790d2675bfeda435b47a560e515b437)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: fix compilation on NetBSD

Use include file <sys/time.h> to fix compilation error with timeval in
some files. This is as reported as 'man 7 system_data_types'. The build
error is reported on NetBSD 9.2.

This should be backported up to 2.2.

(cherry picked from commit 28c5b3c0bc40f96579845f13a9fafa6f63eb10c0)
[wt: only applied to check-t.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 08fc157c389572ae49a1c80d85df242074e4d2bb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: mux-h2: do not prevent from sending a final GOAWAY frame

Some checks were added by commit 9a3d3fcb5 ("BUG/MAJOR: mux-h2: Don't try
to send data if we know it is no longer possible") to make sure we don't
loop forever trying to send data that cannot leave. But one of the
conditions there is not correct, the one relying on H2_CS_ERROR2. Indeed,
this state indicates that the error code was serialized into the mux
buffer, and since the test is placed before trying to send the data to
the socket, if the connection states only contains a GOAWAY frame, it
may refrain from sending and may close without sending anything. It's
not dramatic, as GOAWAY reports connection errors in situations where
delivery is not even certain, but it's cleaner to make sure the error
is properly sent, and it avoids upsetting h2spec, as seen in github
issue #1422.

Given that the patch above was backported as far as 1.8, this patch will
also have to be backported that far.

Thanks to Ilya for reporting this one.

(cherry picked from commit e6dc7a0129b8b34e7e1ce1f4bda5d4bfee4ab749)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e90653b0612f53bdada4f59baebf1e2c8796fa41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: buf: fix varint API post- vs pre- increment

A bogus test in b_get_varint(), b_put_varint(), b_peek_varint() shifts
the end of the buffer by one byte. Since the bug is the same in the read
and write functions, the buffer contents remain compatible, which explains
why this bug was not detected earlier. But if the buffer ends on an
aligned address or page, it can result in a one-byte overflow which will
typically cause a crash or an inconsistent behavior.

This API is only used by rings (e.g. for traces and boot messages) and
by DNS responses, so the probability to hit it is extremely low, but a
crash on boot was observed.

This must be backported to 2.2.

(cherry picked from commit dd362b7b24ea24d8f3c882dab1a69cf78af7cc90)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit db5bf3d89cc4290aacb15ec348bb2bcdcaa1c201)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: always check a valid item in query_list

The query_list is physically stored in the struct resolution itself,
so we have a list that contains a list to items stored in itself (and
there is a single item). But the list is first initialized in
resolv_validate_dns_response(), while it's scanned in
resolv_process_responses() later after calling the former. First,
this results in crashes as soon as the code is instrumented a little
bit for debugging, as elements from a previous incarnation can appear.

But in addition to this, the presence of an element is checked by
verifying that the return of LIST_NEXT() is not NULL, while it may
never be NULL even for an empty list, resulting in bugs or crashes
if the number of responses does not match the list's contents. This
is easily triggered by testing for the list non-emptiness outside of
the function.

Let's make sure the list is always correct, i.e. it's initialized to
an empty list when the structure is allocated, elements are checked by
first verifying the list is not empty, they are deleted once checked,
and in any case at end so that there are no dangling pointers.

This should be backported, but only as long as the patch fits without
modifications, as adaptations can be risky there given that bugs tend
to hide each other.

(cherry picked from commit 25e010906a016c9d6e8677726e6e55de80010616)
[wt: use resolv_hostname_cmp() in 2.4]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 51128f86129a1257b59005356253f78411d77e95)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUILD: resolvers: avoid a possible warning on null-deref

Depending on the code that precedes the loop, gcc may emit this warning:

  src/resolvers.c: In function 'resolv_process_responses':
  src/resolvers.c:1009:11: warning: potential null pointer dereference [-Wnull-dereference]
   1009 |  if (query->type != DNS_RTYPE_SRV && flags & DNS_FLAG_TRUNCATED) {
        |      ~~~~~^~~~~~

However after carefully checking, r_res->header.qdcount it exclusively 1
when reaching this place, which forces the for() loop to enter for at
least one iteration, and <query> to be set. Thus there's no code path
leading to a null deref. It's possibly just because the assignment is
too far and the compiler cannot figure that the condition is always OK.
Let's just mark it to please the compiler.

(cherry picked from commit 10c1a8c3bd6a24f51616172a37f1e3ef58f8fa84)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3a8897f94773849c6dc15218f0f4ca7831a8df85)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MAJOR: resolvers: add other missing references during resolution removal

There is a fundamental design bug in the resolvers code which is that
a list of active resolutions is being walked to try to delete outdated
entries, and that the code responsible for removing them also removes
other elements, including the next one which will be visited by the
list iterator. This randomly causes a use-after-free condition leading
to crashes, infinite loops and various other issues such as random memory
corruption.

A first fix for the memory fix for this was brought by commit 0efc0993e
("BUG/MEDIUM: resolvers: Don't release resolution from a requester
callbacks"). While preparing for more fixes, some code was factored by
commit 11c6c3965 ("MINOR: resolvers: Clean server in a dedicated function
when removing a SRV item"), which inadvertently passed "0" as the "safe"
argument all the time, missing one case of removal protection, instead
of always using "safe". This patch reintroduces the correct argument.

This must be backported with all fixes above.

Cc: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 2a67aa0a51acc2d3469061b8732faf7e597f9c69)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3223d8574e37b1baaf1e9000244c5b1c07178e0a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: merge address and target into a union "data"

These two fields are exclusive as they depend on the data type.
Let's move them into a union to save some precious bytes. This
reduces the struct resolv_answer_item size from 600 to 576 bytes.

(cherry picked from commit cc8fd4c0404523101f4d1473bcd8589086da022d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 20a023765963c22d7b57ebe41493be848b63669c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: use correct storage for the target address

The struct resolv_answer_item contains an address field of type
"sockaddr" which is only 16 bytes long, but which is used to store
either IPv4 or IPv6. Fortunately, the contents only overlap with
the "target" field that follows it and that is large enough to
absorb the extra bytes needed to store AAAA records. But this is
dangerous as just moving fields around could result in memory
corruption.

The fix uses a union and removes the casts that were used to hide
the problem.

Older versions need to be checked and possibly fixed. This needs
to be backported anyway.

(cherry picked from commit b4ca0195a9591614bfcebe9b7fe80c1ce9f94d8d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c54aabaf7c73e563a4cfdef760e2ae767612c601)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolvers: fix truncated TLD consecutive to the API fix

A bug was introduced by commit previous bf9498a31 ("MINOR: resolvers:
fix the resolv_str_to_dn_label() API about trailing zero") as the code
is particularly contrived and hard to test. The output writes the last
char at [i+1] so the trailing zero and return value must be at i+1.

This will have to be backported where the patch above is backported
since it was needed for a fix.

(cherry picked from commit 7b232f132dc9a0a6ee6fa3fabfa6e50196af9240)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 68c7b36f637c5d9a05b32b2868e44ea838f83f78)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: fix the resolv_dn_label_to_str() API about trailing zero

This function suffers from the same API issue as its sibling that does the
opposite direction, it demands that the input string is zero-terminated
*and* that its length *including* the trailing zero is passed on input,
forcing callers to pass length + 1, and itself to use that length - 1
everywhere internally.

This patch addressess this. There is a single caller, which is the
location of the previous bug, so it should probably be backported at
least to keep the code consistent across versions. Note that the
function is called dns_dn_label_to_str() in 2.3 and earlier.

(cherry picked from commit 875ee704dd76b12e5db9ad5c9ae5f2a23b4a4a86)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ca0170bc83c9c5cd067eaf00283df3a2ae36eb5b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MINOR: resolvers: do not reject host names of length 255 in SRV records

An off-by-one issue in buffer size calculation used to limit the output
of resolv_dn_label_to_str() to 254 instead of 255.

This must be backported to 2.0.

(cherry picked from commit 85c15e6bff92c2dae609d76f72c1d231d048dd74)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2859c162c2e8499e5be77972000aca2cb2e53a67)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: resolver: make sure to always use the correct hostname length

In issue #1411, @jjiang-stripe reports that do-resolve() sometimes seems
to be trying to resolve crap from random memory contents.

The issue is that action_prepare_for_resolution() tries to measure the
input string by itself using strlen(), while resolv_action_do_resolve()
directly passes it a pointer to the sample, omitting the known length.
Thus of course any other header present after the host in memory are
appended to the host value. It could theoretically crash if really
unlucky, with a buffer that does not contain any zero including in the
index at the end, and if the HTX buffer ends on an allocation boundary.
In practice it should be too low a probability to have ever been observed.

This patch modifies the action_prepare_for_resolution() function to take
the string length on with the host name on input and pass that down the
chain. This should be backported to 2.0 along with commit "MINOR:
resolvers: fix the resolv_str_to_dn_label() API about trailing zero".

(cherry picked from commit 947ae125cc3a00c05a5356de177b732fdc55aab5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a13a610841988fa9713eae50e3fd91e0fd9038f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

MINOR: resolvers: fix the resolv_str_to_dn_label() API about trailing zero

This function is bogus at the API level: it demands that the input string
is zero-terminated *and* that its length *including* the trailing zero is
passed on input. While that already looks smelly, the trailing zero is
copied as-is, and is then explicitly replaced with a zero... Not only
all callers have to pass hostname_len+1 everywhere to work around this
absurdity, but this requirement causes a bug in the do-resolve() action
that passes random string lengths on input, and that will be fixed on a
subsequent patch.

Let's fix this API issue for now.

This patch will have to be backported, and in versions 2.3 and older,
the function is in dns.c and is called dns_str_to_dn_label().

(cherry picked from commit bf9498a31beb41078ebf257c54b7acb879bcb98b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c1c765f27d2a31f90ffe233d218282aa16d52e7c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: tcpcheck: Properly catch early HTTP parsing errors

When an HTTP response is parsed, early parsing errors are not properly
handled. When this error is reported by the multiplexer, nothing is copied
into the input buffer. The HTX message remains empty but the
HTX_FL_PARSING_ERROR flag is set. In addition CS_FL_EOI is set on the
conn-stream. This last flag must be handled to prevent subscription for
receive events. Otherwise, in the best case, a L7 timeout error is
reported. But a transient loop is also possible if a shutdown is received
because the multiplexer notifies the check of the event while the check
never handles it and waits for more data.

Now, if CS_FL_EOI flag is set on the conn-stream, expect rules are
evaluated. Any error must be handled there.

Thanks to @kazeburo for his valuable report.

This patch should fix the issue #1420. It must be backported at least to
2.4. On 2.3 and 2.2, there is no loop but the wrong error is reported (empty
response instead of invalid one). Thus it may also be backported as far as
2.2.

(cherry picked from commit d16e7dd0e4cf07c9731a1a1c288d003a9f84d1b5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit bd0174758fed578d685c52271b030b10c475fb2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: sample: properly verify that variables cast to sample

The various variable-to-sample converters allow to turn a variable to
a sample of type string, sint or binary, but both the string one used
by strcmp() and the binary one used by secure_memcmp() are missing a
pointer check on the ability to the cast, making them crash if a
variable of type addr is used with strcmp(), or if an addr or bool is
used with secure_memcmp().

Let's rely on the new sample_conv_var2smp() function to run the proper
checks.

This will need to be backported to all supported version. It relies on
previous commits:

  CLEANUP: server: always include the storage for SSL settings
  CLEANUP: sample: rename sample_conv_var2smp() to *_sint
  CLEANUP: sample: uninline sample_conv_var2smp_str()
  MINOR: sample: provide a generic var-to-sample conversion function

For backports it's probably easier to check the sample_casts[] pointer
before calling it in sample_conv_strcmp() and sample_conv_secure_memcmp().

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

MINOR: sample: provide a generic var-to-sample conversion function

We're using variable-to-sample conversion at least 4 times in the code,
two of which are bogus. Let's introduce a generic conversion function
that performs the required checks.

(cherry picked from commit 168e8de1d06adc7aa3e7e2cc2a36935a77c79b9c)
[cf: call to vars_get_by_desc() was updated to remove the third param]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ea5735057fcf62c74b28fe3f13a0ab96b8014852)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

CLEANUP: sample: uninline sample_conv_var2smp_str()

There's no reason to limit this one to this file, it could be used in
other contexts.

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

CLEANUP: sample: rename sample_conv_var2smp() to *_sint

This one only handles integers, contrary to its sibling with the suffix
_str that only handles strings. Let's rename it and uninline it since it
may well be used from outside.

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

BUG/MEDIUM: stream: Keep FLT_END analyzers if a stream detects a channel error

If a channel error (READ_ERRO|READ_TIMEOUT|WRITE_ERROR|WRITE_TIMEOUT) is
detected by the stream, in process_stream(), FLT_END analyers must be
preserved. It is important to be sure to ends filter analysis and be able to
release the stream.

First, filters may release some ressources when FLT_END analyzers are
called. Then, the CF_FL_ANALYZE flag is used to sync end of analysis for the
request and the response. If FLT_END analyzer is ignored on a channel, this
may block the other side and freeze the stream.

This patch must be backported to all stable versions

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

BUG/MINOR: http-ana: Don't eval front after-response rules if stopped on back

http-after-response rules evaluation must be stopped after a "allow". It
means the frontend ruleset must not be evaluated if a "allow" was performed
in the backend ruleset. Internally, the evaluation must be stopped if on
HTTP_RULE_RES_STOP return value. Only the "allow" action is concerned by
this change.

Thanks to this patch, http-response and http-after-response behave in the
same way.

This patch should be backported as far as 2.2.

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

MINOR: initcall: Rename __GLOBL and __GLOBL1.

Rename __GLOBL and __GLOBL1 to __HA_GLOBL and __HA_GLOBL1, as the former are
already defined on FreeBSD.

This should be backported to 2.4, 2.3 and 2.2.

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

BUG/MEDIUM: mux_h2: Handle others remaining read0 cases on partial frames

We've found others places where the read0 is ignored because of an
incomplete frame parsing. This time, it happens during parsing of
CONTINUATION frames.

When frames are parsed, incomplete frames are properly handled and
H2_CF_DEM_SHORT_READ flag is set. It is also true for HEADERS
frames. However, for CONTINUATION frames, there is an exception. Besides
parsing the current frame, we try to peek header of the next one to merge
payload of both frames, the current one and the next one. Idea is to create
a sole HEADERS frame before parsing the payload. However, in this case, it
is possible to have an incomplete frame too, not the current one but the
next one. From the demux point of view, the current frame is complete. We
must go to the internal function h2c_decode_headers() to detect an
incomplete frame. And this case was not identified and fixed when
H2_CF_DEM_SHORT_READ flag was introduced in the commit b5f7b5296
("BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames")

This bug was reported in a comment of the issue #1362. The patch must be
backported as far as 2.0.

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

BUG/MEDIUM: stream-int: Defrag HTX message in si_cs_recv() if necessary

The stream interface is now responsible for defragmenting the HTX message of
the input channel if necessary, before calling the mux's .rcv_buf()
function. The defrag is performed if the underlying buffer contains only
input data while the HTX message free space is not contiguous.

The defrag is important here to be sure the mux and the app layer have the
same criteria to decide if a buffer is full or not. Otherwise, the app layer
may wait for more data because the buffer is not full while the mux is
blocked because it needs more space to proceed.

This patch depends on following commits:

  * MINOR: htx: Add an HTX flag to know when a message is fragmented
  * MINOR: htx: Add a function to know if the free space wraps

This patch is related to the issue #1362. It may be backported as far as 2.0
after some observation period (not sure it is required or not).

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

MINOR: htx: Add a function to know if the free space wraps

the htx_space_wraps() function may now be used to know if the free space of
an HTX message wraps. It does the same as b_space_wraps().

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