BUG/MEDIUM: listener: only consider running threads when resuming listeners

In bug #495 we found that it is possible to resume a listener on an
inexistent thread. This happens when a bind's thread_mask contains bits
out of the active threads mask, such as when using "1/odd" or "1/even".
The thread_mask was used as-is to pick a thread number to re-enable the
listener, and given that the highest number is used, 1/odd or 1/even can
produce quite high thread numbers and crash the process by queuing some
entries into non-existent lists.

This bug is an incomplete fix of commit 413e926ba ("BUG/MAJOR: listener:
fix thread safety in resume_listener()") though it will only trigger if
some bind lines are explicitly bound to thread numbers higher than the
thread count. The fix must be backported to all branches having the fix
above (as far as 1.8, though the code is different there, see the commit
message in 1.8 for changes).

There are a few other places where bind_thread is used without
enforcing all_thread_mask, namely when doing fd_insert() while creating
listeners. It seems harmless but would probably deserve another fix.

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

BUG/MINOR: dns: allow 63 char in hostname

hostname were limited to 62 char, which is not RFC1035 compliant;
- the parsing loop should stop when above max label char
- fix len label test where d[i] was wrongly used
- simplify the whole function to avoid using two extra char* variable

this should fix github issue #387

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
Acked-by: Baptiste <bedis9@gmail.com>
(cherry picked from commit aecd5dcac2cd4cd87639596980c449f887b2ca26)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: unix: better catch situations where the unix socket path length is close to the limit

We do have some checks for the UNIX socket path length to validate the
full pathname of a unix socket but the pathname extension is only taken
into account when using a bind_prefix. The second check only matches
against MAXPATHLEN. So this means that path names between 98 and 108
might successfully parse but fail to bind. Let's adjust the check in
the address parser and refine the error checking at the bind() step.

This addresses bug #493.

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

DOC: schematic of the SSL certificates architecture

This patch provides a schematic of the new architecture based on the
struct cert_key_and_chain which appeared with haproxy 2.1.

Could be backported in 2.1

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

BUG/MEDIUM: ssl/cli: 'commit ssl cert' wrong SSL_CTX init

The code which is supposed to apply the bind_conf configuration on the
SSL_CTX was not called correctly. Indeed it was called with the previous
SSL_CTX so the new ones were left with default settings. For example the
ciphers were not changed.

This patch fixes #429.

Must be backported in 2.1.

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

SCRIPTS: announce-release: allow the user to force to overwrite old files

When starting the script multiple times, one had to remove the previous
files by hand. Now with -f it's not needed anymore, they get removed.

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

SCRIPTS: announce-release: place the send command in the mail's header

I'm fed up with having to scroll my terminals trying to look for the
mail send command printed 30 minutes before the release, let's have
it copied into the e-mail template itself, and replace the old headers
that used to be duplicated there and that are not needed anymore.

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

CONTRIB: debug: also support reading values from stdin

This is convenient when processing large dumps, it allows to copy-paste
values to inspect from one window to another, or to directly transfer
a "show fd"/"show stream" output through sed. In order to do this, simply
pass "-" alone instead of the value and they will all be read one line at
a time from stdin. For example, in order to quickly print the different
set of connection flags from "show fd", this is sufficient:

     sed -ne 's/^.* cflg=\([^ ]*\).*/\1/p' | contrib/debug/flags conn -

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

MINOR: acl: Warn when an ACL is named 'or'

Consider a configuration like this:

> acl t always_true
> acl or always_false
>
> http-response set-header Foo Bar if t or t

The 'or' within the condition will be treated as a logical disjunction
and the header will be set, despite the ACL 'or' being falsy.

This patch makes it an error to declare such an ACL that will never
work. This patch may be backported to stable releases, turning the
error into a warning only (the code was written in a way to make this
trivial). It should not break anything and might improve the users'
lifes.

(cherry picked from commit 0cf811a5f941261176b67046dbc542d0479ff4a7)
[wt: turned the error into a warning only]
Signed-off-by: Willy Tarreau <w@1wt.eu>

CONTRIB: debug: support reporting multiple values at once

It's often convenient, for example to dump two channels or two stream-int
at once. Now all input values are decoded and the value is recalled before
the dump when there is more than one to display.

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

CONTRIB: debug: add the possibility to decode the value as certain types only

It's often confusing to have a whole dump on the screen while only
checking for a set of task or stream flags, and appending "|grep ^chn"
isn't very convenient to repeat the opeation. Instead let's add the
ability to filter the output as certain types only by prepending their
name(s) before the value.

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

CONTRIB: debug: add missing flags SF_HTX and SF_MUX

These two were forgotten when HTX was added. They can be backported
as they're missing for debugging traces in 2.0.

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

BUG/MINOR: ssl: clear the SSL errors on DH loading failure

In ssl_sock_load_dh_params(), if haproxy failed to apply the dhparam
with SSL_CTX_set_tmp_dh(), it will apply the DH with
SSL_CTX_set_dh_auto().

The problem is that we don't clean the OpenSSL errors when leaving this
function so it could fail to load the certificate, even if it's only a
warning.

Fixes bug #483.

Must be backported in 2.1.

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

BUG/MINOR: ssl: we may only ignore the first 64 errors

We have the ability per bind option to ignore certain errors (CA, crt, ...),
and for this we use a 64-bit field. In issue #479 coverity reports a risk of
too large a left shift. For now as of OpenSSL 1.1.1 the highest error value
that may be reported by X509_STORE_CTX_get_error() seems to be around 50 so
there should be no risk yet, but it's enough of a warning to add a check so
that we don't accidently hide random errors in the future.

This may be backported to relevant stable branches.

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

BUG/MAJOR: memory: Don't forget to unlock the rwlock if the pool is empty.

In __pool_get_first(), don't forget to unlock the pool lock if the pool is
empty, otherwise no writer will be able to take the lock, and as it is done
when reloading, it leads to an infinite loop on reload.

This should be backported with commit 04f5fe87d3d3a222b89420f8c1231461f55ebdeb

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

BUG/MEDIUM: memory: Add a rwlock before freeing memory.

When using lockless pools, add a new rwlock, flush_pool. read-lock it when
getting memory from the pool, so that concurrenct access are still
authorized, but write-lock it when we're about to free memory, in
pool_flush() and pool_gc().
The problem is, when removing an item from the pool, we unreference it
to get the next one, however, that pointer may have been free'd in the
meanwhile, and that could provoke a crash if the pointer has been unmapped.
It should be OK to use a rwlock, as normal operations will still be able
to access the pool concurrently, and calls to pool_flush() and pool_gc()
should be pretty rare.

This should be backported to 2.1, 2.0 and 1.9.

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

MINOR: memory: Only init the pool spinlock once.

In pool_create(), only initialize the pool spinlock if we just created the
pool, in the event we're reusing it, there's no need to initialize it again.

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

BUG/MEDIUM: memory_pool: Update the seq number in pool_flush().

In pool_flush(), we can't just set the free_list to NULL, or we may suffer
the ABA problem. Instead, use a double-width CAS and update the sequence
number.

This should be backported to 2.1, 2.0 and 1.9.
This may, or may not, be related to github issue #476.

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

BUG/MEDIUM: connections: Don't forget to unlock when killing a connection.

Commit 140237471e408736bb7162e68c572c710a66a526 made sure we hold the
toremove_lock for the corresponding thread before removing a connection
from its idle_orphan_conns list, however it failed to unlock it if we
found a connection, leading to a deadlock, so add the missing deadlock.

This should be backported to 2.1 and 2.0.

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

BUG/MINOR: connection: fix ip6 dst_port copy in make_proxy_line_v2

triggered by coverity; src_port is set earlier.

this should fix github issue #467

Fixes: 7fec02153712 ("MEDIUM: proxy_protocol: Convert IPs to v6 when
protocols are mixed")
This should be backported to 1.8.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
(cherry picked from commit bd8bf67102d061f777c3e41db206e86bbae60be7)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MINOR: ssl: Possible memleak when allowing the 0RTT data buffer.

â\80\8b
As the server early data buffer is allocated in the middle of the loop
used to allocate the SSL session without being freed before retrying,
this leads to a memory leak.
â\80\8b
To fix this we move the section of code responsible of this early data buffer
alloction after the one reponsible of allocating the SSL session.
â\80\8b
Must be backported to 2.1 and 2.0.

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

BUG/MEDIUM: pipe: fix a use-after-free in case of pipe creation error

In get_pipe(), if pipe() fails, we free the pipe struct but return it to
the caller, resulting in a use-after-free then a double-free provoking a
crash. It's quite hard to trigger the problem as it usually involves a
tweaked configuration with heavy use of splicing and too many allocated
pipes of large size, but it does happen in benchmarks.

The bug was introduced with the very first patch adding pipes support
in 1.3.16-rc1, commit 982b6e37e ("[MEDIUM] introduce pipe pools"). As
such, this fix must be backported to all supported versions.

This patch doesn't exist in mainline because it was fixed as a side
effect of commit 876b411f2b ("BUG/MEDIUM: pipe/thread: fix atomicity of
pipe counters") that only affects mainline.

The bug manifests itself as a crash in pool_free() or __pool_get_first()
and is easily worked around by reducing tune.maxpipes, or since 2.0, by
not forcing any value and letting it configure itself based on maxconn
and ulimit-n.

BUG/MINOR: tcpchecks: fix the connect() flags regarding delayed ack

In issue #465, we see that Coverity detected dead code in checks.c
which is in fact a missing parenthesis to build the connect() flags
consecutive to the API change in commit fdcb007ad8 ("MEDIUM: proto:
Change the prototype of the connect() method.").

The impact should be imperceptible as in the best case it may have
resulted in a missed optimization trying to save a syscall or to merge
outgoing packets.

It may be backported as far as 2.0 though it's not critical.

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

BUG/MEDIUM: ssl: Don't forget to free ctx->ssl on failure.

In ssl_sock_init(), if we fail to allocate the BIO, don't forget to free
the SSL *, or we'd end up with a memory leak.

This should be backported to 2.1 and 2.0.

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

MINOR: lua: Add HLUA_PREPEND_C?PATH build option

This complements the lua-prepend-path configuration option to allow
distro maintainers to add a default path for HAProxy specific Lua
libraries.

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

MINOR: lua: Add lua-prepend-path configuration option

lua-prepend-path allows the administrator to specify a custom Lua library
path to load custom Lua modules that are useful within the context of HAProxy
without polluting the global Lua library folder.

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

MINOR: lua: Add hlua_prepend_path function

This function is added in preparation for following patches.

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

BUILD: cfgparse: silence a bogus gcc warning on 32-bit machines

A first patch was made during 2.0-dev to silence a bogus warning emitted
by gcc : dd1c8f1f72 ("MINOR: cfgparse: Add a cast to make gcc happier."),
but it happens it was not sufficient as the warning re-appeared on 32-bit
machines under gcc-8 and gcc-9 :

  src/cfgparse.c: In function 'check_config_validity':
  src/cfgparse.c:3642:33: warning: argument 1 range [2147483648, 4294967295] exceeds maximum object size 2147483647 [-Walloc-size-larger-than=]
       newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, sizeof(*newsrv->idle_orphan_conns));
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This warning doesn't trigger in other locations, and it immediately
vanishes if the previous or subsequent loops do not depend on
global.nbthread anymore, or if the field ordering of the struct server
changes! As discussed in the thread at:

   https://www.mail-archive.com/haproxy@formilux.org/msg36107.html

playing with -Walloc-size-larger-than has no effect. And a minimal
reproducer could be isolated, indicating it's pointless to circle around
this one. Let's just cast nbthread to ushort so that gcc cannot make
this wrong detection. It's unlikely we'll use more than 65535 threads in
the near future anyway.

This may be backported to older releases if they are also affected, at
least to ease the job of distro maintainers.

Thanks to Ilya for testing.

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

BUG/MEDIUM: mux-h2: make sure we don't emit TE headers with anything but "trailers"

While the H2 parser properly checks for the absence of anything but
"trailers" in the TE header field, we forget to check this when sending
the request to an H2 server. The problem is that an H2->H2 conversion
may keep "gzip" and fail on the next stage.

This patch makes sure that we only send "TE: trailers" if the TE header
contains the "trailers" token, otherwise it's dropped.

This fixes issue #464 and should be backported till 1.9.

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

BUG/MINOR: stktable: report the current proxy name in error messages

Since commit 1b8e68e89a ("MEDIUM: stick-table: Stop handling stick-tables
as proxies."), a rule referencing the current proxy with no table leads
to the following error :

  [ALERT] 023/071924 (16479) : Proxy 'px': unable to find stick-table '(null)'.
  [ALERT] 023/071914 (16479) : Fatal errors found in configuration.

for a config like this one:

  backend px
        stick on src

This patch fixes it and should be backported as far as 2.0.

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

BUG/MEDIUM: 0rtt: Only consider the SSL handshake.

We only add the Early-data header, or get ssl_fc_has_early to return 1, if
we didn't already did the SSL handshake, as otherwise, we know the early
data were fine, and there's no risk of replay attack. But to do so, we
wrongly checked CO_FL_HANDSHAKE, we have to check CO_FL_SSL_WAIT_HS instead,
as we don't care about the status of any other handshake.

This should be backported to 2.1, 2.0, and 1.9.

When deciding if we should add the Early-Data header, or if the sample fetch
should return

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

BUG/MINOR: ssl/cli: ocsp_issuer must be set w/ "set ssl cert"

ocsp_issuer is primary set from ckch->chain when PEM is loaded from file,
but not set when PEM is loaded via CLI payload. Set ckch->ocsp_issuer in
ssl_sock_load_pem_into_ckch to fix that.

Should be backported in 2.1.

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

BUG/MINOR: ssl: typo in previous patch

The previous patch 5c3c96f ("BUG/MINOR: ssl: memory leak w/ the
ocsp_issuer") contains a typo that prevent it to build.

Should be backported in 2.1.

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

BUG/MINOR: ssl: memory leak w/ the ocsp_issuer

This patch frees the ocsp_issuer in
ssl_sock_free_cert_key_and_chain_contents().

Shoudl be backported in 2.1.

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

BUG/MINOR: ssl: increment issuer refcount if in chain

When using the OCSP response, if the issuer of the response is in
the certificate chain, its address will be stored in ckch->ocsp_issuer.
However, since the ocsp_issuer could be filled by a separate file, this
pointer is free'd. The refcount of the X509 need to be incremented to
avoid a double free if we free the ocsp_issuer AND the chain.

(cherry picked from commit b829dda57b4c8a44eff53682ed56492ad46ce3ad)
[wt: checked with William, needed for 2.1]
Signed-off-by: Willy Tarreau <w@1wt.eu>

CLEANUP: stats: shut up a wrong null-deref warning from gcc 9.2

As reported in bug #447, gcc 9.2 invents impossible code paths and then
complains that we don't check for our pointers to be NULL... This code
path is not critical, better add the test to shut it up than try to
help it being less creative.

This code hasn't changed for a while, so it could help distros to
backport this to older releases.

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

BUG/MINOR: ssl/cli: free the previous ckch content once a PEM is loaded

When using "set ssl cert" on the CLI, if we load a new PEM, the previous
sctl, issuer and OCSP response are still loaded. This doesn't make any
sense since they won't be usable with a new private key.

This patch free the previous data.

Should be backported in 2.1.

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

BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent

"set ssl cert <filename> <payload>" CLI command should have the same
result as reload HAproxy with the updated pem file (<filename>).
Is not the case, DHparams/cert-chain is kept from the previous
context if no DHparams/cert-chain is set in the context (<payload>).

This patch should be backport to 2.1

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

BUG/MEDIUM: netscaler: Don't forget to allocate storage for conn->src/dst.

In conn_recv_netscaler_cip(), don't forget to allocate conn->src and
conn->dst, as those are now dynamically allocated. Not doing so results in
getting a crash when using netscaler.
This should fix github issue #460.

This should be backported to 2.1.

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

BUG/MINOR: http_act: don't check capture id in backend

A wrong behavior was introduced by
e9544935e86278dfa3d49fb4b97b860774730625, leading to preventing loading
any configuration where a capture slot id is used in a backend.
IE, the configuration below does not parse:

  frontend f
   bind *:80
   declare capture request len 32
   default_backend webserver

  backend webserver
   http-request capture req.hdr(Host) id 1

The point is that such type of configuration is valid and should run.

This patch enforces the check of capture slot id only if the action rule
is configured in a frontend.
The point is that at configuration parsing time, it is impossible to
check which frontend could point to this backend (furthermore if we use
dynamic backend name resolution at runtime).

The documentation has been updated to warn the user to ensure that
relevant frontends have required declaration when such rule has to be
used in a backend.
If no capture slot can be found, then the action will just not be
executed and HAProxy will process the next one in the list, as expected.

This should be backported to all supported branches (bug created as part
of a bug fix introduced into 1.7 and backported to 1.6).

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

MINOR: proxy/http-ana: Add support of extra attributes for the cookie directive

It is now possible to insert any attribute when a cookie is inserted by
HAProxy. Any value may be set, no check is performed except the syntax validity
(CTRL chars and ';' are forbidden). For instance, it may be used to add the
SameSite attribute:

    cookie SRV insert attr "SameSite=Strict"

The attr option may be repeated to add several attributes.

This patch should fix the issue #361.

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

BUG/MINOR: ssl: ssl_sock_load_sctl_from_file memory leak

"set ssl cert <filename.sctl> <payload>" CLI command must free
previous context.

This patch should be backport to 2.1

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

BUG/MINOR: ssl: ssl_sock_load_issuer_file_into_ckch memory leak

"set ssl cert <filename.issuer> <payload>" CLI command must free
previous context.

This patch should be backport to 2.1

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

BUG/MINOR: ssl: ssl_sock_load_ocsp_response_from_file memory leak

"set ssl cert <filename.ocsp> <payload>" CLI command must free
previous context.

This patch should be backport to 2.1

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

BUG/MINOR: tcp-rules: Fix memory releases on error path during action parsing

When an error occurred during the parsing of a TCP action, if some memory was
allocated, it should be released before exiting.  Here, the fix consists for
replace a call to free() on a sample expression by a call to
release_sample_expr().

This patch may be backported to all supported versions.

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

BUG/MINOR: stick-table: Use MAX_SESS_STKCTR as the max track ID during parsing

During the parsing of the sc-inc-gpc0, sc-inc-gpc1 and sc-inc-gpt1 actions, the
maximum stick table track ID allowed is tested against ACT_ACTION_TRK_SCMAX. It
is the action number and not the maximum number of stick counters. Instead,
MAX_SESS_STKCTR must be used.

This patch must be backported to all stable versions.

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

BUG/MINOR: http-rules: Remove buggy deinit functions for HTTP rules

Functions to deinitialize the HTTP rules are buggy. These functions does not
check the action name to release the right part in the arg union. Only few info
are released. For auth rules, the realm is released and there is no problem
here. But the regex <arg.hdr_add.re> is always unconditionally released. So it
is easy to make these functions crash. For instance, with the following rule
HAProxy crashes during the deinit :

      http-request set-map(/path/to/map) %[src] %[req.hdr(X-Value)]

For now, These functions are simply removed and we rely on the deinit function
used for TCP rules (renamed as deinit_act_rules()). This patch fixes the
bug. But arguments used by actions are not released at all, this part will be
addressed later.

This patch must be backported to all stable versions.

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

BUG/MINOR: http-ana/filters: Wait end of the http_end callback for all filters

Filters may define the "http_end" callback, called at the end of the analysis of
any HTTP messages. It is called at the end of the payload forwarding and it can
interrupt the stream processing. So we must be sure to not remove the XFER_BODY
analyzers while there is still at least filter in progress on this callback.

Unfortunatly, once the request and the response are borh in the DONE or the
TUNNEL mode, we consider the XFER_BODY analyzer has finished its processing on
both sides. So it is possible to prematurely interrupt the execution of the
filters "http_end" callback.

To fix this bug, we switch a message in the ENDING state. It is then switched in
DONE/TUNNEL mode only after the execution of the filters "http_end" callback.

This patch must be backported (and adapted) to 2.1, 2.0 and 1.9. The legacy HTTP
mode shoud probaly be fixed too.

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

BUILD: pattern: include errno.h

Commit 3c79d4bdc introduced the use of errno in pattern.c without
including errno.h.
If we build haproxy without any option errno is not defined and the
build fails.

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

[Cf: I miissed this one during my last backports]

BUG/MINOR: 51d: Fix bug when HTX is enabled

When HTX is enabled, the sample flags were set too early. When matching for
multiple HTTP headers, the sample is fetched more than once, meaning that the
flags would need to be set again. Instead, the flags are now set last (just
before the outermost function returns). This could be further improved by
passing around the message without calling prefetch again.

This patch must be backported as far as 1.9. it should fix bug #450.

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

BUG/MINOR: dns: Make dns_query_id_seed unsigned

Left shifting of large signed values and negative values is undefined.

In a test script clang's ubsan rightfully complains:

> runtime error: left shift of 1934242336581872173 by 13 places cannot be represented in type 'int64_t' (aka 'long')

This bug was introduced in the initial version of the DNS resolver
in 325137d603aa81bd24cbd8c99d816dd42291daa7. The fix must be backported
to HAProxy 1.6+.

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

BUG/MINOR: cache: Fix leak of cache name in error path

This issue was introduced in commit 99a17a2d91f9044ea20bba6617048488aed80555
which first appeared in tag v1.9-dev11. This bugfix should be backported
to HAProxy 1.9+.

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

BUG/MINOR: pattern: handle errors from fgets when trying to load patterns

We need to do some error handling after we call fgets to make sure everything
went fine. If we don't users can be fooled into thinking they can load pattens
from directory because cfgparse doesn't flinch. This applies to acl patterns
map files.

This should be backported to all supported versions.

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

BUG/MEDIUM: connection: add a mux flag to indicate splice usability

Commit c640ef1a7d ("BUG/MINOR: stream-int: avoid calling rcv_buf() when
splicing is still possible") fixed splicing in TCP and legacy mode but
broke it badly in HTX mode.

What happens in HTX mode is that the channel's to_forward value remains
set to CHN_INFINITE_FORWARD during the whole transfer, and as such it is
not a reliable signal anymore to indicate whether more data are expected
or not. Thus, when data are spliced out of the mux using rcv_pipe(), even
when the end is reached (that only the mux knows about), the call to
rcv_buf() to get the final HTX blocks completing the message were skipped
and there was often no new event to wake this up, resulting in transfer
timeouts at the end of large objects.

All this goes down to the fact that the channel has no more information
about whether it can splice or not despite being the one having to take
the decision to call rcv_pipe() or not. And we cannot afford to call
rcv_buf() inconditionally because, as the commit above showed, this
reduces the forwarding performance by 2 to 3 in TCP and legacy modes
due to data lying in the buffer preventing splicing from being used
later.

The approach taken by this patch consists in offering the muxes the ability
to report a bit more information to the upper layers via the conn_stream.
This information could simply be to indicate that more data are awaited
but the real need being to distinguish splicing and receiving, here
instead we clearly report the mux's willingness to be called for splicing
or not. Hence the flag's name, CS_FL_MAY_SPLICE.

The mux sets this flag when it knows that its buffer is empty and that
data waiting past what is currently known may be spliced, and clears it
when it knows there's no more data or that the caller must fall back to
rcv_buf() instead.

The stream-int code now uses this to determine if splicing may be used
or not instead of looking at the rcv_pipe() callbacks through the whole
chain. And after the rcv_pipe() call, it checks the flag again to decide
whether it may safely skip rcv_buf() or not.

All this bitfield dance remains a bit complex and it starts to appear
obvious that splicing vs reading should be a decision of the mux based
on permission granted by the data layer. This would however increase
the API's complexity but definitely need to be thought about, and should
even significantly simplify the data processing layer.

The way it was integrated in mux-h1 will also result in no more calls
to rcv_pipe() on chunked encoded data, since these ones are currently
disabled at the mux level. However once the issue with chunks+splice
is fixed, it will be important to explicitly check for curr_len|CHNK
to set MAY_SPLICE, so that we don't call rcv_buf() after each chunk.

This fix must be backported to 2.1 and 2.0.

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

BUG/MINOR: stream: don't mistake match rules for store-request rules

In process_sticking_rules() we only want to apply the first store-request
rule for a given table, but when doing so we need to make sure we only
count actual store-request rules when we list the sticking rules.

Failure to do so leads to not being able to write store-request and match
sticking rules in any order as a match rule after a store-request rule
will be ignored.

The following configuration reproduces the issue:

  global
    stats socket /tmp/foobar

  defaults
    mode http

  frontend in
    bind *:8080
    default_backend bar

  backend bar
    server s1 127.0.0.1:21212
    server s2 127.0.0.1:21211
    stick store-request req.hdr(foo)
    stick match req.hdr(foo)
    stick-table type string size 10

  listen foo
    bind *:21212
    bind *:21211
    http-request deny deny_status 200 if { dst_port 21212 }
    http-request deny

This patch fixes issue #448 and should be backported as far as 1.6.

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

BUG/MEDIUM: cli: _getsocks must send the peers sockets

This bug prevents to reload HAProxy when you have both the seamless
reload (-x / expose-fd listeners) and the peers.

Indeed the _getsocks command does not send the FDs of the peers
listeners, so if no reuseport is possible during the bind, the new
process will fail to bind and exits.

With this feature, it is not possible to fallback on the SIGTTOU method
if we didn't receive all the sockets, because you can't close() the
sockets of the new process without closing those of the previous
process, they are the same.

Should fix bug #443.

Must be backported as far as 1.8.

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

REGTEST: add sample_fetches/hashes.vtc to validate hashes

This regtest validates all hashes that we support, on all input bytes from
0x00 to 0xFF. Those supporting avalanche are tested as well. It also tests
len(), hex() and base64(). It purposely does not enable sha2() because this
one relies on OpenSSL and there's no point in validating that OpenSSL knows
how to hash, what matters is that we can test our hashing functions in all
cases. However since the tests were written, they're still present and
commented out in case that helps.

It may be backported to supported versions, possibly dropping a few algos
that were not supported (e.g. crc32c requires 1.9 minimum).

Note that this test will fail on crc32/djb2/sdbm/wt6 unless patches
"BUG/MINOR: stream: init variables when the list is empty" and
"BUG/MAJOR: hashes: fix the signedness of the hash inputs" are included.

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

BUG/MAJOR: hashes: fix the signedness of the hash inputs

Wietse Venema reported in the thread below that we have a signedness
issue with our hashes implementations: due to the use of const char*
for the input key that's often text, the crc32, sdbm, djb2, and wt6
algorithms return a platform-dependent value for binary input keys
containing bytes with bit 7 set. This means that an ARM or PPC
platform will hash binary inputs differently from an x86 typically.
Worse, some algorithms are well defined in the industry (like CRC32)
and do not provide the expected result on x86, possibly causing
interoperability issues (e.g. a user-agent would fail to compare the
CRC32 of a message body against the one computed by haproxy).

Fortunately, and contrary to the first impression, the CRC32c variant
used in the PROXY protocol processing is not affected. Thus the impact
remains very limited (the vast majority of input keys are text-based,
such as user-agent headers for exmaple).

This patch addresses the issue by fixing all hash functions' prototypes
(even those not affected, for API consistency). A reg test will follow
in another patch.

The vast majority of users do not use these hashes. And among those
using them, very few will pass them on binary inputs. However, for the
rare ones doing it, this fix MAY have an impact during the upgrade. For
example if the package is upgraded on one LB then on another one, and
the CRC32 of a binary input is used as a stick table key (why?) then
these CRCs will not match between both nodes. Similarly, if
"hash-type ... crc32" is used, LB inconsistency may appear during the
transition. For this reason it is preferable to apply the patch on all
nodes using such hashes at the same time. Systems upgraded via their
distros will likely observe the least impact since they're expected to
be upgraded within a short time frame.

And it is important for distros NOT to skip this fix, in order to avoid
distributing an incompatible implementation of a hash. This is the
reason why this patch is tagged as MAJOR, eventhough it's extremely
unlikely that anyone will ever notice a change at all.

This patch must be backported to all supported branches since the
hashes were introduced in 1.5-dev20 (commit 98634f0c). Some parts
may be dropped since implemented later.

Link to Wietse's report:
  https://marc.info/?l=postfix-users&m=157879464518535&w=2

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

BUG/MEDIUM: mux_h1: Don't call h1_send if we subscribed().

In h1_snd_buf(), only attempt to call h1_send() if we haven't
already subscribed.
It makes no sense to do it if we subscribed, as we know we failed
to send before, and will create a useless call to sendto(), and
in 2.2, the call to raw_sock_from_buf() will disable polling if
it is enabled.

This should be backported to 2.2, 2.1, 2.0 and 1.9.

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

BUG/MEDIUM: mworker: remain in mworker mode during reload

If you reload an haproxy started in master-worker mode with
"master-worker" in the configuration, and no "-W" argument,
the new process lost the fact that is was in master-worker mode
resulting in weird behaviors.

The bigest problem is that if it is reloaded with an bad configuration,
the master will exits instead of remaining in waitpid mode.

This problem was discovered in bug #443.

Should be backported in every version using the master-worker mode.
(as far as 1.8)

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

REGTEST: mcli/mcli_start_progs: start 2 programs

This regtest tests the issue #446 by starting 2 programs and checking if
they exist in the "show proc" of the master CLI.

Should be backported as far as 2.0.

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

BUG/MINOR: cli/mworker: can't start haproxy with 2 programs

When trying to start HAProxy with the master CLI and more than one
program in the configuration, it refuses to start with:

[ALERT] 013/132926 (1378) : parsing [cur--1:0] : proxy 'MASTER', another server named 'cur--1' was already defined at line 0, please use distinct names.
[ALERT] 013/132926 (1378) : Fatal errors found in configuration.

The problem is that haproxy tries to create a server for the MASTER
proxy but only the worker are supposed to be in the server list.

Fix issue #446.

Must be backported as far as 2.0.

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

BUG/MEDIUM: mux-h2: don't stop sending when crossing a buffer boundary

In version 2.0, after commit 9c218e7521 ("MAJOR: mux-h2: switch to next
mux buffer on buffer full condition."), the H2 mux started to use a ring
buffer for the output data in order to reduce competition between streams.
However, one corner case was suboptimally covered: when crossing a buffer
boundary, we have to shrink the outgoing frame size to the one left in
the output buffer, but this shorter size is later used as a signal of
incomplete send due to a buffer full condition (which used to be true when
using a single buffer). As a result, function h2s_frt_make_resp_data()
used to return less than requested, which in turn would cause h2_snd_buf()
to stop sending and leave some unsent data in the buffer, and si_cs_send()
to subscribe for sending more later.

But it goes a bit further than this, because subscribing to send again
causes the mux's send_list not to be empty anymore, hence extra streams
can be denied the access to the mux till the first stream is woken again.
This causes a nasty wakeup-sleep dance between streams that makes it
totally impractical to try to remove the sending list. A test showed
that it was possible to observe 3 million h2_snd_buf() giveups for only
100k requests when using 100 concurrent streams on 20kB objects.

It doesn't seem likely that a stream could get blocked and time out due
to this bug, though it's not possible either to demonstrate the opposite.
One risk is that incompletely sent streams do not have any blocking flags
so they may not be identified as blocked. However on first scan of the
send_list they meet all conditions for a wakeup.

This patch simply allows to continue on a new frame after a partial
frame. with only this change, the number of failed h2_snd_buf() was
divided by 800 (4% of calls). And by slightly increasing the H2C_MBUF_CNT
size, it can go down to zero.

This fix must be backported to 2.1 and 2.0.

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

BUG/MEDIUM: mux-h2: fix missing test on sending_list in previous patch

Previous commit 989539b048 ("BUG/MINOR: mux-h2: use a safe
list_for_each_entry in h2_send()") accidently lost its sending_list test,
resulting in some elements to be woken up again while already in the
sending_list and h2_unsubscribe() crashing on integrity tests (only
when built with DEBUG_DEV).

If the fix above is backported this one must be as well.

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

BUG/MINOR: mux-h2: use a safe list_for_each_entry in h2_send()

h2_send() uses list_for_each_entry() to scan paused streams and resume
them, but happily deletes any leftover from a previous failed unsubscribe,
which is obviously not safe and would corrupt the list. In practice this
is a proof that this doesn't happen, but it's not the best way to prove it.
In order to fix this and reduce the maintenance burden caused by code
duplication (this list walk exists at 3 places), let's introduce a new
function h2_resume_each_sending_h2s() doing exactly this and use it at
all 3 places.

This bug was introduced as a side effect of fix 998410a41b ("BUG/MEDIUM:
h2: Revamp the way send subscriptions works.") so it should be backported
as far as 1.9.

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

BUG/MEDIUM: tasks: Use the MT macros in tasklet_free().

In tasklet_free(), to attempt to remove ourself, use MT_LIST_DEL, we can't
just use LIST_DEL(), as we theorically could be in the shared tasklet list.

This should be backported to 2.1.

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

BUG/MINOR: stream-int: Don't trigger L7 retry if max retries is already reached

When an HTTP response is received, at the stream-interface level, if a L7 retry
must be triggered because of the status code, the response is trashed and a read
error is reported on the response channel. Then the stream handles this error
and perform the retry. Except if the maximum connection retries is reached. In
this case, an error is reported. Because the server response was already trashed
by the stream-interface, a generic 502 error is returned to the client instead
of the server's one.

Now, the stream-interface triggers a L7 retry only if the maximum connection
retries is not already reached. Thus, at the end, the last server's response is
returned.

This patch must be backported to 2.1 and 2.0. It should fix the issue #439.

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

BUG/MEDIUM: session: do not report a failure when rejecting a session

In session_accept_fd() we can perform a synchronous call to
conn_complete_session() and if it succeeds the connection is accepted
and turned into a session. If it fails we take it as an error while it
is not, in this case, it's just that a tcp-request rule has decided to
reject the incoming connection. The problem with reporting such an event
as an error is that the failed status is passed down to the listener code
which decides to disable accept() for 100ms in order to leave some time
for transient issues to vanish, and that's not what we want to do here.

This fix must be backported as far as 1.7. In 1.7 the code is a bit
different as tcp_exec_l5_rules() is called directly from within
session_new_fd() and ret=0 must be assigned there.

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

BUG/MINOR: channel: inject output data at the end of output

In co_inject(), data must be inserted at the end of output, not the end of
input. For the record, this function does not take care of input data which are
supposed to not exist. But the caller may reset input data after or before the
call. It is its own choice.

This bug, among other effects, is visible when a redirect is performed on
the response path, on legacy HTTP mode (so for HAProxy < 2.1). The redirect
response is appended after the server response when it should overwrite it.

Thanks to Kevin Zhu <ip0tcp@gmail.com> to report the bug. It must be backported
as far as 1.9.

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

BUG/MEDIUM: http-ana: Truncate the response when a redirect rule is applied

When a redirect rule is executed on the response path, we must truncate the
received response. Otherwise, the redirect is appended after the response, which
is sent to the client. So it is obviously a bug because the redirect is not
performed. With bodyless responses, it is the "only" bug. But if the response
has a body, the result may be invalid. If the payload is not fully received yet
when the redirect is performed, an internal error is reported.

It must be backported as far as 1.9.

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

BUG/MINOR: proxy: Fix input data copy when an error is captured

In proxy_capture_error(), input data are copied in the error snapshot. The copy
must take care of the data wrapping. But the length of the first block is
wrong. It should be the amount of contiguous input data that can be copied
starting from the input's beginning. But the mininum between the input length
and the buffer size minus the input length is used instead. So it is a problem
if input data are wrapping or if more than the half of the buffer is used by
input data.

This patch must be backported as far as 1.9.

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

BUG/MINOR: h1: Report the right error position when a header value is invalid

During H1 messages parsing, when the parser has finished to parse a full header
line, some tests are performed on its value, depending on its name, to be sure
it is valid. The content-length is checked and converted in integer and the host
header is also checked. If an error occurred during this step, the error
position must point on the header value. But from the parser point of view, we
are already on the start of the next header. Thus the effective reported
position in the error capture is the beginning of the unparsed header line. It
is a bit confusing when we try to figure out why a message is rejected.

Now, the parser state is updated to point on the invalid value. This way, the
error position really points on the right position.

This patch must be backported as far as 1.9.

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

MINOR: ssl: Remove unused variable "need_out".

The "need_out" variable was used to let the ssl code know we're done
reading early data, and we should start the handshake.
Now that the handshake function is responsible for taking care of reading
early data, all that logic has been removed from ssl_sock_to_buf(), but
need_out was forgotten, and left. Remove it know.
This patch was submitted by William Dauchy <w.dauchy@criteo.com>, and should
fix github issue #434.
This should be backported to 2.0 and 2.1.

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

MINOR: config: disable busy polling on old processes

in the context of seamless reload and busy polling, older processes will
create unecessary cpu conflicts; we can assume there is no need for busy
polling for old processes which are waiting to be terminated.

This patch is not a bug fix itself but might be a good stability
improvment when you are un the context of frequent seamless reloads with
a high "hard-stop-after" value; for that reasons I think this patch
should be backported in all 2.x versions.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
(cherry picked from commit 3894d97fb8b66e247c5a326c6b3aa75816c597dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

BUG/MEDIUM: connections: Hold the lock when wanting to kill a connection.

In connect_server(), when we decide we want to kill the connection of
another thread because there are too many idle connections, hold the
toremove_lock of the corresponding thread, othervise, there's a small race
condition where we could try to add the connection to the toremove_connections
list while it has already been free'd.

This should be backported to 2.0 and 2.1.

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

BUG/MEDIUM: checks: Only attempt to do handshakes if the connection is ready.

When creating a new check connection, only attempt to add an handshake
connection if the connection has fully been initialized. It can not be the
case if a DNS resolution is still pending, and thus we don't yet have the
address for the server, as the handshake code assumes the connection is fully
initialized and would otherwise crash.
This is not ideal, the check shouldn't probably run until we have an address,
as it leads to check failures with "Socket error".
While I'm there, also add an xprt handshake if we're using socks4, otherwise
checks wouldn't be able to use socks4 properly.
This should fix github issue #430

This should be backported to 2.0 and 2.1.

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

BUG/MINOR: checks: refine which errno values are really errors.

Two regtest regularly fail in a random fashion depending on the machine's
load (one could really wonder if it's really worth keeping such
unreproducible tests) :
  - tcp-check_multiple_ports.vtc
  - 4be_1srv_smtpchk_httpchk_layer47errors.vtc

It happens that one of the reason is the time it takes to connect to
the local socket (hence the load-dependent aspect): if connect() on the
loopback returns EINPROGRESS then this status is reported instead of a
real error. Normally such a test is expected to see the error cleaned
by tcp_connect_probe() but it really depends on the timing and instead
we may very well send() first and see this error. The problem is that
everything is collected based on errno, hoping it won't get molested
in the way from the last unsuccesful syscall to wake_srv_chk(), which
obviously is hard to guarantee.

This patch at least makes sure that a few non-errors are reported as
zero just like EAGAIN. It doesn't fix the root cause but makes it less
likely to report incorrect failures.

This fix could be backported as far as 1.9.

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

[RELEASE] Released version 2.1.2

Released version 2.1.2 with the following main changes :
    - DOC: clarify the fact that replace-uri works on a full URI
    - BUG/MINOR: sample: fix the closing bracket and LF in the debug converter
    - BUG/MINOR: sample: always check converters' arguments
    - MINOR: sample: Validate the number of bits for the sha2 converter
    - BUG/MEDIUM: ssl: Don't set the max early data we can receive too early.
    - MINOR: debug: support logging to various sinks
    - MINOR: http: add a new "replace-path" action
    - MINOR: task: only check TASK_WOKEN_ANY to decide to requeue a task
    - BUG/MAJOR: task: add a new TASK_SHARED_WQ flag to fix foreing requeuing
    - BUG/MEDIUM: ssl: Revamp the way early data are handled.
    - MINOR: fd/threads: make _GET_NEXT()/_GET_PREV() use the volatile attribute
    - BUG/MEDIUM: fd/threads: fix a concurrency issue between add and rm on the same fd
    - BUG/MINOR: ssl: openssl-compat: Fix getm_ defines
    - BUG/MEDIUM: state-file: do not allocate a full buffer for each server entry
    - BUG/MINOR: state-file: do not store duplicates in the global tree
    - BUG/MINOR: state-file: do not leak memory on parse errors
    - BUG/MEDIUM: stream: Be sure to never assign a TCP backend to an HTX stream
    - BUILD: ssl: improve SSL_CTX_set_ecdh_auto compatibility

BUILD: ssl: improve SSL_CTX_set_ecdh_auto compatibility

SSL_CTX_set_ecdh_auto() is not defined when OpenSSL 1.1.1 is compiled
with the no-deprecated option. Remove existing, incomplete guards and
add a compatibility macro in openssl-compat.h, just as OpenSSL does:

https://github.com/openssl/openssl/blob/bf4006a6f9be691ba6eef0e8629e63369a033ccf/include/openssl/ssl.h#L1486

This should be backported as far as 2.0 and probably even 1.9.

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

BUG/MEDIUM: stream: Be sure to never assign a TCP backend to an HTX stream

With a TCP frontend, it is possible to upgrade a connection to HTTP when the
backend is in HTTP mode. Concretly the upgrade install a new mux. So, once it is
done, the downgrade to TCP is no longer possible. So we must take care to never
assign a TCP backend to a stream on this connection. Otherwise, HAProxy crashes
because raw data from the server are handled as structured data on the client
side.

This patch fixes the issue #420. It must be backported to all versions
supporting the HTX.

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

BUG/MINOR: state-file: do not leak memory on parse errors

Issue #417 reports a possible memory leak in the state-file loading code.
There's one such place in the loop which corresponds to parsing errors
where the curreently allocated line is not freed when dropped. In any
case this is very minor in that no more than the file's length may be
lost in the worst case, considering that the whole file is kept anyway
in case of success. This fix addresses this.

It should be backported to 2.1.

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

BUG/MINOR: state-file: do not store duplicates in the global tree

The global state file tree isn't configured for unique keys, so if an
entry appears multiple times, e.g. due to a bogus script that concatenates
entries multiple times, this will needlessly eat memory. Let's just drop
duplicates.

This should be backported to 2.1.

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

BUG/MEDIUM: state-file: do not allocate a full buffer for each server entry

Starting haproxy with a state file of 700k servers eats 11.2 GB of RAM
due to a mistake in the function that loads the strings into a tree: it
allocates a full buffer for each backend+server name instead of allocating
just the required string. By just fixing this we're down to 80 MB.

This should be backported to 2.1.

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

BUG/MINOR: ssl: openssl-compat: Fix getm_ defines

LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
always true. Check for the define before checking it.

Signed-off-by: Rosen Penev <rosenp@gmail.com>

[wt: to be backported as far as 1.9]

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

BUG/MEDIUM: fd/threads: fix a concurrency issue between add and rm on the same fd

There's a very hard-to-trigger bug in the FD list code where the
fd_add_to_fd_list() function assumes that if the FD it's trying to add
is already locked, it's in the process of being added. Unfortunately, it
can also be in the process of being removed. It is very hard to trigger
because it requires that one thread is removing the FD while another one
is adding it. First very few FDs run on multiple threads (listeners and
DNS), and second, it does not make sense to add and remove the FD at the
same time.

In practice the DNS code built on the older callback-only model does
perform bursts of fd_want_send() for all resolvers at once when it wants
to send a new query (dns_send_query()). And this is more likely to happen
when here are lots of resolutions in parallel and many resolvers, because
the dns_response_recv() callback can also trigger a series of queries on
all resolvers for each invalid response it receives. This means that it
really is perfectly possible to both stop and start in parallel during
short periods of time there.

This issue was not reported before 2.1, but 2.1 had the FD cache, built
on the exact same code base. It's very possible that the issue caused
exactly the opposite situation, where an event was occasionally lost,
causing a DNS retry that worked, and nobody noticing the problem in the
end. In 2.1 the lost entries are the updates asking for not polling for
writes anymore, and the effect is that the poller contiuously reports
writability on the socket when the issue happens.

This patch fixes bug #416 and must be backported as far as 1.8, and
absolutely requires that previous commit "MINOR: fd/threads: make
_GET_NEXT()/_GET_PREV() use the volatile attribute" is backported as
well otherwise it will make the issue worse.

Special thanks to Julien Pivotto for setting up a reliable reproducer
for this difficult issue.

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

MINOR: fd/threads: make _GET_NEXT()/_GET_PREV() use the volatile attribute

These macros are either used between atomic ops which cause the volatile
to be implicit, or with an explicit volatile cast. However not having it
in the macro causes some traps in the code because certain loop paths
cannot safely be used without risking infinite loops if one isn't careful
enough.

Let's place the volatile attribute inside the macros and remove them from
the explicit places to avoid this. It was verified that the output executable
remains exactly the same byte-wise.

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

BUG/MEDIUM: ssl: Revamp the way early data are handled.

Instead of attempting to read the early data only when the upper layer asks
for data, allocate a temporary buffer, stored in the ssl_sock_ctx, and put
all the early data in there. Requiring that the upper layer takes care of it
means that if for some reason the upper layer wants to emit data before it
has totally read the early data, we will be stuck forever.

This should be backported to 2.1 and 2.0.
This may fix github issue #411.

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

BUG/MAJOR: task: add a new TASK_SHARED_WQ flag to fix foreing requeuing

Since 1.9 with commit b20aa9eef3 ("MAJOR: tasks: create per-thread wait
queues") a task bound to a single thread will not use locks when being
queued or dequeued because the wait queue is assumed to be the owner
thread's.

But there exists a rare situation where this is not true: the health
check tasks may be running on one thread waiting for a response, and
may in parallel be requeued by another thread calling health_adjust()
after a detecting a response error in traffic when "observe l7" is set,
and "fastinter" is lower than "inter", requiring to shorten the running
check's timeout. In this case, the task being requeued was present in
another thread's wait queue, thus opening a race during task_unlink_wq(),
and gets requeued into the calling thread's wait queue instead of the
running one's, opening a second race here.

This patch aims at protecting against the risk of calling task_unlink_wq()
from one thread while the task is queued on another thread, hence unlocked,
by introducing a new TASK_SHARED_WQ flag.

This new flag indicates that a task's position in the wait queue may be
adjusted by other threads than then one currently executing it. This means
that such WQ manipulations must be performed under a lock. There are two
types of such tasks:
  - the global ones, using the global wait queue (technically speaking,
    those whose thread_mask has at least 2 bits set).
  - some local ones, which for now will be placed into the global wait
    queue as well in order to benefit from its lock.

The flag is automatically set on initialization if the task's thread mask
indicates more than one thread. The caller must also set it if it intends
to let other threads update the task's expiration delay (e.g. delegated
I/Os), or if it intends to change the task's affinity over time as this
could lead to the same situation.

Right now only the situation described above seems to be affected by this
issue, and it is very difficult to trigger, and even then, will often have
no visible effect beyond stopping the checks for example once the race is
met. On my laptop it is feasible with the following config, chained to
httpterm:

    global
        maxconn 400 # provoke FD errors, calling health_adjust()

    defaults
        mode http
        timeout client 10s
        timeout server 10s
        timeout connect 10s

    listen px
        bind :8001
        option httpchk /?t=50
        server sback 127.0.0.1:8000 backup
        server-template s 0-999 127.0.0.1:8000 check port 8001 inter 100 fastinter 10 observe layer7

This patch will automatically address the case for the checks because
check tasks are created with multiple threads bound and will get the
TASK_SHARED_WQ flag set.

If in the future more tasks need to rely on this (multi-threaded muxes
for example) and the use of the global wait queue becomes a bottleneck
again, then it should not be too difficult to place locks on the local
wait queues and queue the task on its bound thread.

This patch needs to be backported to 2.1, 2.0 and 1.9. It depends on
previous patch "MINOR: task: only check TASK_WOKEN_ANY to decide to
requeue a task".

Many thanks to William Dauchy for providing detailed traces allowing to
spot the problem.

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

MINOR: task: only check TASK_WOKEN_ANY to decide to requeue a task

After processing a task, its RUNNING bit is cleared and at the same time
we check for other bits to decide whether to requeue the task or not. It
happens that we only want to check the TASK_WOKEN_* bits, because :
  - TASK_RUNNING was just cleared
  - TASK_GLOBAL and TASK_QUEUE cannot be set yet as the task was running,
    preventing it from being requeued

It's important not to catch yet undefined flags there because it would
prevent addition of new task flags. This also shows more clearly that
waking a task up with flags 0 is not something safe to do as the task
will not be woken up if it's already running.

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

MINOR: http: add a new "replace-path" action

This action is very similar to "replace-uri" except that it only acts on the
path component. This is assumed to better match users' expectations when they
used to rely on "replace-uri" in HTTP/1 because mostly origin forms were used
in H1 while mostly absolute URI form is used in H2, and their rules very often
start with a '/', and as such do not match.

It could help users to get this backported to 2.0 and 2.1.

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

MINOR: debug: support logging to various sinks

As discussed in the thread below [1], the debug converter is currently
not of much use given that it's only built when DEBUG_EXPR is set, and
it is limited to stderr only.

This patch changes this to make it take an optional prefix and an optional
target sink so that it can log to stdout, stderr or a ring buffer. The
default output is the "buf0" ring buffer, that can be consulted from the
CLI.

[1] https://www.mail-archive.com/haproxy@formilux.org/msg35671.html

Note: if this patch is backported, it also requires the following commit to
work: 46dfd78cbf ("BUG/MINOR: sample: always check converters' arguments").
(cherry picked from commit 0851fd5eef89c790f18b996093111fe00c840bd6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

BUG/MEDIUM: ssl: Don't set the max early data we can receive too early.

When accepting the max early data, don't set it on the SSL_CTX while parsing
the configuration, as at this point global.tune.maxrewrite may still be -1,
either because it was not set, or because it hasn't been set yet. Instead,
set it for each connection, just after we created the new SSL.
Not doing so meant that we could pretend to accept early data bigger than one
of our buffer.

This should be backported to 2.1, 2.0, 1.9 and 1.8.

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

MINOR: sample: Validate the number of bits for the sha2 converter

Instead of failing the conversion when an invalid number of bits is
given the sha2 converter now fails with an appropriate error message
during startup.

The sha2 converter was introduced in d4376302377e4f51f43a183c2c91d929b27e1ae3,
which is in 2.1 and higher.

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

BUG/MINOR: sample: always check converters' arguments

In 1.5-dev20, sample-fetch arguments parsing was addresse by commit
689a1df0a1 ("BUG/MEDIUM: sample: simplify and fix the argument parsing").
The issue was that argument checks were not run for sample-fetches if
parenthesis were not present. Surprisingly, the fix was mde only for
sample-fetches and not for converters which suffer from the exact same
problem. There are even a few comments in the code mentioning that some
argument validation functions are not called when arguments are missing.

This fix applies the exact same method as the one above. The impact of
this bug is limited because over the years the code has learned to work
around this issue instead of fixing it.

This may be backported to all maintained versions.

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

BUG/MINOR: sample: fix the closing bracket and LF in the debug converter

The closing bracket was emitted for the "debug" converter even when the
opening one was not sent, and the new line was not always emitted. Let's
fix this. This is harmless since this converter is not built by default.

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

DOC: clarify the fact that replace-uri works on a full URI

With H2 deployments becoming more common, replace-uri starts to hit
users by not always matching absolute URIs due to rules expecting the
URI to start with a '/'.

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

[RELEASE] Released version 2.1.1

Released version 2.1.1 with the following main changes :
    - BUG/MINOR: contrib/prometheus-exporter: decode parameter and value only
    - BUG/MINOR: h1: Don't test the host header during response parsing
    - BUILD/MINOR: trace: fix use of long type in a few printf format strings
    - BUG/MINOR: http-htx: Don't make http_find_header() fail if the value is empty
    - DOC: ssl/cli: set/commit/abort ssl cert
    - BUG/MINOR: ssl: fix SSL_CTX_set1_chain compatibility for openssl < 1.0.2
    - BUG/MINOR: fcgi-app: Make the directive pass-header case insensitive
    - BUG/MINOR: stats: Fix HTML output for the frontends heading
    - BUG/MINOR: ssl/cli: 'ssl cert' cmd only usable w/ admin rights
    - DOC: Clarify behavior of server maxconn in HTTP mode
    - DOC: clarify matching strings on binary fetches
    - DOC: Fix ordered list in summary
    - DOC: move the "group" keyword at the right place
    - BUG/MEDIUM: stream-int: don't subscribed for recv when we're trying to flush data
    - BUG/MINOR: stream-int: avoid calling rcv_buf() when splicing is still possible
    - BUG/MINOR: ssl/cli: don't overwrite the filters variable
    - BUG/MEDIUM: listener/thread: fix a race when pausing a listener
    - BUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1
    - BUG/MEDIUM: mux-h1: Never reuse H1 connection if a shutw is pending
    - BUG/MINOR: mux-h1: Don't rely on CO_FL_SOCK_RD_SH to set H1C_F_CS_SHUTDOWN
    - BUG/MINOR: mux-h1: Fix conditions to know whether or not we may receive data
    - BUG/MEDIUM: tasks: Make sure we switch wait queues in task_set_affinity().
    - BUG/MEDIUM: checks: Make sure we set the task affinity just before connecting.
    - BUG/MINOR: mux-h1: Be sure to set CS_FL_WANT_ROOM when EOM can't be added
    - BUG/MEDIUM: mux-fcgi: Handle cases where the HTX EOM block cannot be inserted
    - BUG/MINOR: proxy: make soft_stop() also close FDs in LI_PAUSED state
    - BUG/MINOR: listener/threads: always use atomic ops to clear the FD events
    - BUG/MINOR: listener: also clear the error flag on a paused listener
    - BUG/MEDIUM: listener/threads: fix a remaining race in the listener's accept()
    - DOC: document the listener state transitions
    - BUG/MEDIUM: kqueue: Make sure we report read events even when no data.
    - BUG/MAJOR: dns: add minimalist error processing on the Rx path
    - BUG/MEDIUM: proto_udp/threads: recv() and send() must not be exclusive.
    - DOC: listeners: add a few missing transitions
    - BUG/MINOR: tasks: only requeue a task if it was already in the queue
    - DOC: proxies: HAProxy only supports 3 connection modes
    - DOC: remove references to the outdated architecture.txt
    - BUG/MINOR: log: fix minor resource leaks on logformat error path
    - BUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers
    - BUG/MINOR: listener: do not immediately resume on transient error
    - BUG/MINOR: server: make "agent-addr" work on default-server line
    - BUG/MINOR: listener: fix off-by-one in state name check
    - BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()

BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()

Apparently gcc developers decided that strncpy() semantics are no longer
valid and now deserve a warning, especially if used exactly as designed.
This results in issue #304. Let's just remove one to the target size to
please her majesty gcc, the God of C Compilers, who tries hard to make
users completely eliminate any use of string.h and reimplement it by
themselves at much higher risks. Pfff....

This can be backported to stable version, the fix is harmless since it
ignores the last zero that is already set on next line.

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

BUG/MINOR: listener: fix off-by-one in state name check

As reported in issue #380, the state check in listener_state_str() is
invalid as it allows state value 9 to report crap. We don't use such
a state value so the issue should never happen unless the memory is
already corrupted, but better clean this now while it's harmless.

This should be backported to all maintained branches.

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

BUG/MINOR: server: make "agent-addr" work on default-server line

As reported in issue #408, "agent-addr" doesn't work on default-server
lines. This is due to the transcription of the old "addr" option in commit
6e5e0d8f9e ("MINOR: server: Make 'default-server' support 'addr' keyword.")
which correctly assigns it to the check.addr and agent.addr fields, but
which also copies the default check.addr into both the check's and the
agent's addr fields. Thus the default agent's address is never used.

This fix makes sure to copy the check from the check and the agent from
the agent. However it's worth noting that if "addr" is specified on the
server line, it will still overwrite both the check and the agent's
addresses.

This must be backported as far as 1.8.

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