]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
3 years agoMINOR: proc: setting the process to produce a core dump on FreeBSD.
devnexen@gmail.com [Sat, 21 Aug 2021 08:13:10 +0000 (09:13 +0100)] 
MINOR: proc: setting the process to produce a core dump on FreeBSD.

using the procctl api to set the current process as traceable, thus being able to produce a core dump as well.

making it as compile option if not wished or using freebsd prior to 11.x (last no EOL release).

3 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sun, 22 Aug 2021 17:18:07 +0000 (22:18 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 26th iteration of typo fixes

3 years agoCI: github-actions: remove obsolete options
Willy Tarreau [Wed, 25 Aug 2021 03:10:29 +0000 (05:10 +0200)] 
CI: github-actions: remove obsolete options

2.5-dev1 removed http-use-htx but the h2spec config was not updated
accordingly, causing failures. In addition, let's also remove the
unneeded "nbthread 4" which is either too much or not enough (it's
automatic nowadays), and remove "option httplog" which causes a
warning since there's no defined log destination.

3 years agoBUILD: httpclient: fix build without OpenSSL
William Lallemand [Tue, 24 Aug 2021 16:33:28 +0000 (18:33 +0200)] 
BUILD: httpclient: fix build without OpenSSL

Add some defines around the ssl server so we can build without OpenSSL.

3 years agoBUG/MINOR: httpclient: fix Host header
William Lallemand [Tue, 24 Aug 2021 15:53:03 +0000 (17:53 +0200)] 
BUG/MINOR: httpclient: fix Host header

THe http_update_update_host function takes an URL and extract the domain
to use as a host header. However it only update an existing host header
and does not create one.

This patch add an empty host header so the function can update it.

3 years agoMINOR: httpclient: add the server to the proxy
William Lallemand [Tue, 24 Aug 2021 15:18:13 +0000 (17:18 +0200)] 
MINOR: httpclient: add the server to the proxy

Add the raw and ssl server to the proxy list so they can be freed during
the deinit() of HAProxy. As a side effect the 2 servers need to have a
different ID so the SSL one was renamed "<HTTPSCLIENT>".

3 years agoMINOR: httpclient: set verify none on the https server
William Lallemand [Tue, 24 Aug 2021 15:15:58 +0000 (17:15 +0200)] 
MINOR: httpclient: set verify none on the https server

There is currently no way to specify the CA to verify from the
httpclient API. Sets the verify to none so we can still do https
request.

3 years agoBUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}
Dragan Dosen [Tue, 24 Aug 2021 07:48:04 +0000 (09:48 +0200)] 
BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}

Ensure that no more than olen bytes is written to the output buffer,
otherwise we might experience an unexpected behavior.

While the original code used to validate that the output size was
always large enough before starting to write, this validation was
later broken by the commit below, allowing to 3-byte blocks to
areas whose size is not multiple of 3:

  commit ed697e4856e5ac0b9931fd50fd8ff1b7739e5d88
  Author: Emeric Brun <ebrun@haproxy.com>
  Date:   Mon Jan 14 14:38:39 2019 +0100

    BUG/MINOR: base64: dec func ignores padding for output size checking

    Decode function returns an error even if the ouptut buffer is
    large enought because the padding was not considered. This
    case was never met with current code base.

For base64urldec(), it's basically the same problem except that since
the input format supports arbitrary lengths, the problem has always
been there since its introduction in 2.4.

This should be backported to all stable branches having a backport of
the patch above (i.e. 2.0), with some adjustments depending on the
availability of the base64dec() and base64urldec().

3 years agoBUG/MINOR: httpclient: remove deinit of the httpclient
William Lallemand [Tue, 24 Aug 2021 13:09:15 +0000 (15:09 +0200)] 
BUG/MINOR: httpclient: remove deinit of the httpclient

The httpclient does a free of the servers and proxies it uses, however
since we are including them in the global proxy list, haproxy already
free them during the deinit. We can safely remove these free.

3 years agoBUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
Willy Tarreau [Tue, 24 Aug 2021 12:57:28 +0000 (14:57 +0200)] 
BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions

The sc-set-gpt0() parser was extended in 2.1 by commit 0d7712dff ("MINOR:
stick-table: allow sc-set-gpt0 to set value from an expression") to support
sample expressions in addition to plain integers. However there is a
subtlety there, which is that while the arg position must be incremented
when parsing an integer, it must not be touched when calling an expression
since the expression parser already does it.

The effect is that rules making use of sc-set-gpt0() followed by an
expression always ignore one word after that expression, and will typically
fail to parse if followed by an "if" as the parser will restart after the
"if". With no condition it's different because an empty condition doesn't
result in trying to parse anything.

This patch moves the increment at the right place and adds a few
explanations for a code part that was far from being obvious.

This should be backported to branches having the commit above (2.1+).

3 years agoCLEANUP: ssl: remove useless check on p in openssl_version_parser()
William Lallemand [Sun, 22 Aug 2021 11:36:11 +0000 (13:36 +0200)] 
CLEANUP: ssl: remove useless check on p in openssl_version_parser()

Remove a  useless check on a pointer which reports a NULL dereference on
coverity.

Fixes issue #1358.

3 years agoMINOR: cfgcond: implements openssl_version_atleast and openssl_version_before
William Lallemand [Sat, 21 Aug 2021 21:59:56 +0000 (23:59 +0200)] 
MINOR: cfgcond: implements openssl_version_atleast and openssl_version_before

Implements a way of checking the running openssl version:

If the OpenSSL support was not compiled within HAProxy it will returns a
error, so it's recommanded to do a SSL feature check before:

$ ./haproxy -cc 'feature(OPENSSL) && openssl_version_atleast(0.9.8zh) && openssl_version_before(3.0.0)'

This will allow to select the SSL reg-tests more carefully.

3 years agoMINOR: ssl: add an openssl version string parser
William Lallemand [Sat, 21 Aug 2021 21:16:06 +0000 (23:16 +0200)] 
MINOR: ssl: add an openssl version string parser

openssl_version_parser() parse a string in the OpenSSL version format
which is documented here:

https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_VERSION_NUMBER.html

The function returns an unsigned int that could be used for comparing
openssl versions.

3 years agoMINOR: systemd: remove the ExecStartPre line in the unit file
William Lallemand [Fri, 20 Aug 2021 21:36:45 +0000 (23:36 +0200)] 
MINOR: systemd: remove the ExecStartPre line in the unit file

The ExecStartPre line was introduced a long time ago in the systemd unit
file, at the time of systemd wrapper. With the haproxy master worker
mode, this line is now useless, since starting haproxy itself will check
the configuration.

However this does not concern the check in the ExecReload which is still
needed to return a reload status to HAProxy.

It probably shouldn't be backported.

3 years agoBUG/MINOR: systemd: ExecStartPre must use -Ws
William Lallemand [Fri, 20 Aug 2021 21:29:53 +0000 (23:29 +0200)] 
BUG/MINOR: systemd: ExecStartPre must use -Ws

This line should disappear in a future version but we should still fix
ExecStartPre with -Ws like we've done in 9def142.

It's a complementary fix that must be backported with 9def142
("BUG/MINOR: systemd: must check the configuration using -Ws").

3 years agoMINOR: tools: add FreeBSD support to get_exec_path()
devnexen@gmail.com [Tue, 17 Aug 2021 11:55:49 +0000 (12:55 +0100)] 
MINOR: tools: add FreeBSD support to get_exec_path()

FreeBSD stores the absolute path into the auxiliary vector as well.
The auxiliary vector is found in __elf_aux_vector there.

3 years agoMINOR: hlua: take the global Lua lock inside a global function
Willy Tarreau [Fri, 20 Aug 2021 13:47:25 +0000 (15:47 +0200)] 
MINOR: hlua: take the global Lua lock inside a global function

Some users are facing huge CPU usage or even watchdog panics due to
the Lua global lock when many threads compete on it, but they have
no way to see that in the usual dumps. We take the lock at 2 or 3
places only, thus it's trivial to move it to a global function so
that stack dumps will now explicitly show it, increasing the change
that it rings a bell and someone suggests switch to lua-load-per-thread:

  Current executing Lua from a stream analyser -- stack traceback:
      loop.lua:1: in function line 1
  call trace(27):
  |       0x5ff157 [48 83 c4 10 5b 5d 41 5c]: wdt_handler+0xf7/0x104
  | 0x7fe37fe82690 [48 c7 c0 0f 00 00 00 0f]: libpthread:+0x13690
  |       0x614340 [66 48 0f 7e c9 48 01 c2]: main+0x1e8a40
  |       0x607b85 [48 83 c4 08 48 89 df 31]: main+0x1dc285
  |       0x6070bc [48 8b 44 24 20 48 8b 14]: main+0x1db7bc
  |       0x607d37 [41 89 c4 89 44 24 1c 83]: lua_resume+0xc7/0x214
  |       0x464ad6 [83 f8 06 0f 87 f1 01 00]: main+0x391d6
  |       0x4691a7 [83 f8 06 0f 87 03 20 fc]: main+0x3d8a7
  |       0x51dacb [85 c0 74 61 48 8b 5d 20]: sample_process+0x4b/0xf7
  |       0x51e55c [48 85 c0 74 3f 64 48 63]: sample_fetch_as_type+0x3c/0x9b
  |       0x525613 [48 89 c6 48 85 c0 0f 84]: sess_build_logline+0x2443/0x3cae
  |       0x4af0be [4c 63 e8 4c 03 6d 10 4c]: http_apply_redirect_rule+0xbfe/0xdf8
  |       0x4af523 [83 f8 01 19 c0 83 e0 03]: main+0x83c23
  |       0x4b2326 [83 f8 07 0f 87 99 00 00]: http_process_req_common+0xf6/0x15f6
  |       0x4d5b30 [85 c0 0f 85 9f f5 ff ff]: process_stream+0x2010/0x4e18

It also allows "perf top" to directly show the time spent on this lock.

This may be backported to some stable versions as it improves the
overall debuggability.

3 years agoMINOR: httpclient: cleanup the include files
William Lallemand [Fri, 20 Aug 2021 12:25:15 +0000 (14:25 +0200)] 
MINOR: httpclient: cleanup the include files

Include the correct .h files in http_client.c and http_client.h.

The api.h is needed in http_client.c and http_client-t.h is now include
directly from http_client.h

3 years agoBUG/MINOR: httpclient: check if hdr_num is not 0
William Lallemand [Fri, 20 Aug 2021 09:59:49 +0000 (11:59 +0200)] 
BUG/MINOR: httpclient: check if hdr_num is not 0

Check if hdr_num is not 0 before allocating or copying the headers to
the hc->hdrs space.

3 years agoBUG/MINOR: httpclient/cli: change the appctx test in the callbacks
William Lallemand [Fri, 20 Aug 2021 09:35:29 +0000 (11:35 +0200)] 
BUG/MINOR: httpclient/cli: change the appctx test in the callbacks

The callbacks of the CLI httpclient are testing the appctx pointer
before doing the appctx_wakeup but was dereferencing the appctx pointer
before.

3 years agoBUG/MINOR: httpclient: fix uninitialized sl variable
William Lallemand [Fri, 20 Aug 2021 09:24:13 +0000 (11:24 +0200)] 
BUG/MINOR: httpclient: fix uninitialized sl variable

Reported by coverity in ticket #1355

  CID 1461505:  Memory - illegal accesses  (UNINIT)
  Using uninitialized value "sl".

Fix the problem by initializing sl to NULL.

3 years agoREGTESTS: server: fix agent-check syntax and expectation
Willy Tarreau [Fri, 20 Aug 2021 09:28:15 +0000 (11:28 +0200)] 
REGTESTS: server: fix agent-check syntax and expectation

Since commit 8d6c6bd ("Leak-plugging on barriers") VTest has become
stricter in its expectations, making this one fail. The agent-check
test was expecting a close on the server, which normally does not
happen before the server responds. In addition, it was really sending
"hello" (with the quotes) due to the config file syntax, which explains
why test test log reported that '"hell' was received, and complained
that 0x6f ('o') was read instead of a shutdown. This has been fixed
as well by using single-quotes.

There is no need to backport this test as it's only in 2.5.

3 years agoREGTESTS: abortonclose: after retries, 503 is expected, not close
Willy Tarreau [Fri, 20 Aug 2021 09:12:47 +0000 (11:12 +0200)] 
REGTESTS: abortonclose: after retries, 503 is expected, not close

The abortonclose test was only expecting a close after all server
retries were exhausted, it didn't check for the pending 503, which
fails with new versions of vtest starting with commit 8d6c6bd
("Leak-plugging on barriers").

This may be backported, but carefully in case older versions would
really close without responding.

3 years agoREGTESTS: http_upgrade: fix incorrect expectation on TCP->H1->H2
Willy Tarreau [Fri, 20 Aug 2021 09:02:28 +0000 (11:02 +0200)] 
REGTESTS: http_upgrade: fix incorrect expectation on TCP->H1->H2

Commit e1b9e1bb1 ("REGTESTS: Add script to tests TCP to HTTP upgrades")
included a mistake in the TCP->H1->H2 test, it expected a close while
it ought to expect a 400 bad req, which is what the mux returns in this
case. It happens that this used to work fine with older versions of
vtest which see the close regardless of the 400, but since Vtest commit
8d6c6bd ("Leak-plugging on barriers"), this doesn't work anymore.

Let's fix this by expecting the proper response. This should be backported
where this regtest is present, but only after verifying that it still
works; indeed at the time of writing it's uncertain whether an earlier
version used to immediately close.

3 years agoBUG/MINOR: http_client: make sure to preset the proxy's default settings
Willy Tarreau [Fri, 20 Aug 2021 08:23:12 +0000 (10:23 +0200)] 
BUG/MINOR: http_client: make sure to preset the proxy's default settings

Proxies must call proxy_preset_defaults() to initialize their settings
that are usually learned from defaults sections (e.g. connection retries,
pool purge delay etc). At the moment there was likely no impact, but not
doing so could cause trouble soon when using the client more extensively
or when new defaults are introduced and failed to be initialized.

No backport is needed.

3 years agoBUG/MEDIUM: cfgparse: do not allocate IDs to automatic internal proxies
Willy Tarreau [Fri, 20 Aug 2021 08:15:40 +0000 (10:15 +0200)] 
BUG/MEDIUM: cfgparse: do not allocate IDs to automatic internal proxies

Recent commit 83614a9fb ("MINOR: httpclient: initialize the proxy") broke
reg tests that match the output of "show stats" or "show servers state"
because it changed the proxies' numeric ID.

In fact it did nothing wrong, it just registers a proxy and adds it at
the head of the list. But the automatic numbering scheme, which was made
to make sure that temporarily disabled proxies in the config keep their
ID instead of shifting all others, sees one more proxy and increments
next_pxid for all subsequent proxies.

This patch avoids this by not assigning automatic IDs to such internal
proxies, leaving them with their ID of -1, and by not shifting next_pxid
for them. This is important because the user might experience them
appearing or disappearing depending on apparently unrelated config
options or build options, and this must not cause visible proxy IDs
to change (e.g. stats or minitoring may break).

Though the issue has always been there, it only became a problem with
the recent proxy additions so there is no need to backport this.

3 years agoMINOR: proxy: check if p is NULL in free_proxy()
William Lallemand [Fri, 20 Aug 2021 08:16:37 +0000 (10:16 +0200)] 
MINOR: proxy: check if p is NULL in free_proxy()

Check if p is NULL before trying to do anything  in free_proxy(),
like most free()-like function do.

3 years agoMINOR: server: check if srv is NULL in free_server()
William Lallemand [Fri, 20 Aug 2021 08:10:15 +0000 (10:10 +0200)] 
MINOR: server: check if srv is NULL in free_server()

Check if srv is NULL before trying to do anything  in free_server(),
like most free()-like function do.

3 years agoBUILD/MINOR: ssl: Fix compilation with OpenSSL 1.0.2
Remi Tricot-Le Breton [Fri, 20 Aug 2021 07:51:23 +0000 (09:51 +0200)] 
BUILD/MINOR: ssl: Fix compilation with OpenSSL 1.0.2

The X509_STORE_CTX_get0_cert did not exist yet on OpenSSL 1.0.2 and
neither did X509_STORE_CTX_get0_chain, which was not actually needed
since its get1 equivalent already existed.

3 years agoBUG/MEDIUM: h2: match absolute-path not path-absolute for :path
Willy Tarreau [Thu, 19 Aug 2021 21:06:58 +0000 (23:06 +0200)] 
BUG/MEDIUM: h2: match absolute-path not path-absolute for :path

RFC7540 states that :path follows RFC3986's path-absolute. However
that was a bug introduced in the spec between draft 04 and draft 05
of the spec, which implicitly causes paths starting with "//" to be
forbidden. HTTP/1 (and now HTTP core semantics) made it explicit
that the request-target in origin-form follows a purposely defined
absolute-path defined as 1*(/ segment) to explicitly allow "//".

http2bis now fixes this by relying on absolute-path so that "//"
becomes valid and matches other versions. Full discussion here:

  https://lists.w3.org/Archives/Public/ietf-http-wg/2021JulSep/0245.html

This issue appeared in haproxy with commit 4b8852c70 ("BUG/MAJOR: h2:
verify that :path starts with a '/' before concatenating it") when
making the checks on :path fully comply with the spec, and was backported
as far as 2.0, so this fix must be backported there as well to allow
"//" in H2 again.

3 years agoMEDIUM: ssl: Keep a reference to the client's certificate for use in logs
Remi Tricot-Le Breton [Thu, 19 Aug 2021 16:06:30 +0000 (18:06 +0200)] 
MEDIUM: ssl: Keep a reference to the client's certificate for use in logs

Most of the SSL sample fetches related to the client certificate were
based on the SSL_get_peer_certificate function which returns NULL when
the verification process failed. This made it impossible to use those
fetches in a log format since they would always be empty.

The patch adds a reference to the X509 object representing the client
certificate in the SSL structure and makes use of this reference in the
fetches.

The reference can only be obtained in ssl_sock_bind_verifycbk which
means that in case of an SSL error occurring before the verification
process ("no shared cipher" for instance, which happens while processing
the Client Hello), we won't ever start the verification process and it
will be impossible to get information about the client certificate.

This patch also allows most of the ssl_c_XXX fetches to return a usable
value in case of connection failure (because of a verification error for
instance) by making the "conn->flags & CO_FL_WAIT_XPRT" test (which
requires a connection to be established) less strict.

Thanks to this patch, a log-format such as the following should return
usable information in case of an error occurring during the verification
process :
    log-format "DN=%{+Q}[ssl_c_s_dn] serial=%[ssl_c_serial,hex] \
                hash=%[ssl_c_sha1,hex]"

It should answer to GitHub issue #693.

3 years agoMINOR: httpclient/cli: change the User-Agent to "HAProxy"
William Lallemand [Thu, 19 Aug 2021 13:55:19 +0000 (15:55 +0200)] 
MINOR: httpclient/cli: change the User-Agent to "HAProxy"

Change the User-Agent from "HAProxy HTTP client" to "HAProxy" as the
previous name is not valid according to RFC 7231#5.5.3.

This patch fixes issue #1354.

3 years agoMINOR: httpclient/cli: implement a simple client over the CLI
William Lallemand [Wed, 18 Aug 2021 14:46:21 +0000 (16:46 +0200)] 
MINOR: httpclient/cli: implement a simple client over the CLI

This commit implements an HTTP Client over the CLI, this was made as
working example for the HTTP Client API.

It usable over the CLI by specifying a method and an URL:

    echo "httpclient GET http://127.0.0.1:8000/demo.file" | socat /tmp/haproxy.sock -

Only IP addresses are accessibles since the API does not allow to
resolve addresses yet.

3 years agoMINOR: httpclient: implement a simple HTTP Client API
William Lallemand [Fri, 13 Aug 2021 14:05:53 +0000 (16:05 +0200)] 
MINOR: httpclient: implement a simple HTTP Client API

This commit implements a very simple HTTP Client API.

A client can be operated by several functions:

    - httpclient_new(), httpclient_destroy(): create
      and destroy the struct httpclient instance.

    - httpclient_req_gen(): generate a complete HTX request using the
      the absolute URL, the method and a list of headers. This request
      is complete and sets the HTX End of Message flag. This is limited
      to small request we don't need a body.

    - httpclient_start() fill a sockaddr storage with a IP extracted
      from the URL (it cannot resolve an fqdm for now), start the
      applet. It also stores the ptr of the caller which could be an
      appctx or something else.

   - hc->ops contains a list of callbacks used by the
     HTTPClient, they should be filled manually after an
     httpclient_new():

        * res_stline(): the client received a start line, its content
          will be stored in hc->res.vsn, hc->res.status, hc->res.reason

        * res_headers(): the client received headers, they are stored in
          hc->res.hdrs.

        * res_payload(): the client received some payload data, they are
        stored in the hc->res.buf buffer and could be extracted with the
        httpclient_res_xfer() function, which takes a destination buffer
        as a parameter

        * res_end(): this callback is called once we finished to receive
        the response.

3 years agoMINOR: httpclient: initialize the proxy
William Lallemand [Fri, 13 Aug 2021 12:47:57 +0000 (14:47 +0200)] 
MINOR: httpclient: initialize the proxy

Initialize a proxy which contain a server for the raw HTTP, and another
one for the HTTPS. This proxy will use the global server log definition
and the 'option httplog' directive.

This proxy is internal and will only be used for the HTTP Client API.

3 years ago[RELEASE] Released version 2.5-dev4 v2.5-dev4
Willy Tarreau [Tue, 17 Aug 2021 12:08:55 +0000 (14:08 +0200)] 
[RELEASE] Released version 2.5-dev4

Released version 2.5-dev4 with the following main changes :
    - MINOR: log: rename 'dontloglegacyconnerr' to 'log-error-via-logformat'
    - MINOR: doc: rename conn_status in `option httsplog`
    - MINOR: proxy: disabled takes a stopping and a disabled state
    - MINOR: stats: shows proxy in a stopped state
    - BUG/MINOR: server: fix race on error path of 'add server' CLI if track
    - CLEANUP: thread: fix fantaisist indentation of thread_harmless_till_end()
    - MINOR: threads: make thread_release() not wait for other ones to complete
    - MEDIUM: threads: add a stronger thread_isolate_full() call
    - MEDIUM: servers: make the server deletion code run under full thread isolation
    - BUG/MINOR: server: remove srv from px list on CLI 'add server' error
    - MINOR: activity/fd: remove the dead_fd counter
    - MAJOR: fd: get rid of the DWCAS when setting the running_mask
    - CLEANUP: fd: remove the now unused fd_set_running()
    - CLEANUP: fd: remove the now unneeded fd_mig_lock
    - BUG/MINOR: server: update last_change on maint->ready transitions too
    - MINOR: spoe: Add a pointer on the filter config in the spoe_agent structure
    - BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is released
    - BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are queued
    - MINOR: server: unmark deprecated on enable health/agent cli
    - MEDIUM: task: implement tasklet kill
    - MINOR: server: initialize fields for dynamic server check
    - MINOR: check: allocate default check ruleset for every backends
    - MINOR: check: export check init functions
    - MINOR: check: do not increment global maxsock at runtime
    - MINOR: server: implement a refcount for dynamic servers
    - MEDIUM: check: implement check deletion for dynamic servers
    - MINOR: check: enable safe keywords for dynamic servers
    - MEDIUM: server: implement check for dynamic servers
    - MEDIUM: server: implement agent check for dynamic servers
    - REGTESTS: server: add dynamic check server test
    - MINOR: doc: specify ulimit-n usage for dynamic servers
    - REGTESTS: server: fix dynamic server with checks test
    - CI: travis-ci: temporarily disable arm64 builds
    - BUG/MINOR: check: test if server is not null in purge
    - MINOR: global: define MODE_STOPPING
    - BUG/MINOR: server: do not use refcount in free_server in stopping mode
    - ADMIN: dyncookie: implement a simple dynamic cookie calculator
    - BUG/MINOR: check: do not reset check flags on purge
    - BUG/MINOR: check: fix leak on add dynamic server with agent-check error
    - BUG/MEDIUM: check: fix leak on agent-check purge
    - BUG/MEDIUM: server: support both check/agent-check on a dynamic instance
    - BUG/MINOR: buffer: fix buffer_dump() formatting
    - MINOR: channel: remove an htx block from a channel
    - BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
    - BUG/MINOR: stream: Don't release a stream if FLT_END is still registered
    - 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()
    - MINOR: filters/lua: Release filters before the lua context
    - MINOR: lua: Add a function to get a reference on a table in the stack
    - MEDIUM: lua: Process buffer data using an offset and a length
    - MEDIUM: lua: Improve/revisit the lua api to manipulate channels
    - DOC: Improve the lua documentation
    - MEDIUM: filters/lua: Add support for dummy filters written in lua
    - MINOR: lua: Add a function to get a filter attached to a channel class
    - MINOR: lua: Add flags on the lua TXN to know the execution context
    - MEDIUM: filters/lua: Be prepared to filter TCP payloads
    - MEDIUM: filters/lua: Support declaration of some filter callback functions in lua
    - MEDIUM: filters/lua: Add HTTPMessage class to help HTTP filtering
    - MINOR: filters/lua: Add request and response HTTP messages in the lua TXN
    - MINOR: filters/lua: Support the HTTP filtering from filters written in lua
    - DOC: config: Fix 'http-response send-spoe-group' documentation
    - BUG/MINOR: lua: Properly check negative offset in Channel/HttpMessage functions
    - BUG/MINOR: lua: Properly catch alloc errors when parsing lua filter directives
    - BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config check
    - MINOR: cli: delare the CLI frontend as an internal proxy
    - MINOR: proxy: disable warnings for internal proxies
    - BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set
    - BUG/MINOR: lua/filters: Return right code when txn:done() is called
    - DOC: lua-api: Add documentation about lua filters
    - CI: Remove obsolete USE_SLZ=1 CI job
    - CLEANUP: assorted typo fixes in the code and comments
    - CI: github actions: relax OpenSSL-3.0.0 version comparision
    - BUILD: tools: get the absolute path of the current binary on NetBSD.
    - DOC: Minor typo fix - 'question mark' -> 'exclamation mark'
    - DOC/MINOR: fix typo in management document
    - MINOR: http: add a new function http_validate_scheme() to validate a scheme
    - BUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax
    - BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
    - BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
    - BUG/MEDIUM: h2: give :authority precedence over Host
    - REGTESTS: add a test to prevent h2 desync attacks

3 years agoREGTESTS: add a test to prevent h2 desync attacks
Amaury Denoyelle [Fri, 13 Aug 2021 07:43:24 +0000 (09:43 +0200)] 
REGTESTS: add a test to prevent h2 desync attacks

This test ensure that h2 pseudo headers are properly checked for invalid
characters and the host header is ignored if :authority is present. This
is necessary to prevent h2 desync attacks as described here
https://portswigger.net/research/http2

3 years agoBUG/MEDIUM: h2: give :authority precedence over Host
Willy Tarreau [Wed, 11 Aug 2021 13:39:13 +0000 (15:39 +0200)] 
BUG/MEDIUM: h2: give :authority precedence over Host

The wording regarding Host vs :authority in RFC7540 is ambiguous as it
says that an intermediary must produce a host header from :authority if
Host is missing, but, contrary to HTTP/1.1, doesn't say anything regarding
the possibility that Host and :authority differ, which leaves Host with
higher precedence there. In addition it mentions that clients should use
:authority *instead* of Host, and that H1->H2 should use :authority only
if the original request was in authority form. This leaves some gray
area in the middle of the chain for fully valid H2 requests arboring a
Host header that are forwarded to the other side where it's possible to
drop the Host header and use the authority only after forwarding to a
second H2 layer, thus possibly seeing two different values of Host at
a different stage. There's no such issue when forwarding from H2 to H1
as the authority is dropped only only the Host is kept.

Note that the following request is sufficient to re-normalize such a
request:

   http-request set-header host %[req.hdr(host)]

The new spec in progress (draft-ietf-httpbis-http2bis-03) addresses
this trouble by being a bit is stricter on these rules. It clarifies
that :authority must always be used instead of Host and that Host ought
to be ignored. This is much saner as it avoids to convey two distinct
values along the chain. This becomes the protocol-level equivalent of:

   http-request set-uri %[url]

So this patch does exactly this, which we were initially a bit reluctant
to do initially by lack of visibility about other implementations'
expectations. In addition it slightly simplifies the Host header field
creation by always placing it first in the list of headers instead of
last; this could also speed up the look up a little bit.

This needs to be backported to 2.0. Non-HTX versions are safe regarding
this because they drop the URI during the conversion to HTTP/1.1 so
only Host is used and transmitted.

Thanks to Tim Düsterhus for reporting that one.

3 years agoBUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
Willy Tarreau [Wed, 11 Aug 2021 09:12:46 +0000 (11:12 +0200)] 
BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header

Before HTX was introduced, all the HTTP request elements passed in
pseudo-headers fields were used to build an HTTP/1 request whose syntax
was then scrutinized by the HTTP/1 parser, leaving no room to inject
invalid characters.

While NUL, CR and LF are properly blocked, it is possible to inject
spaces in the method so that once translated to HTTP/1, fields are
shifted by one spcae, and a lenient HTTP/1 server could possibly be
fooled into using a part of the method as the URI. For example, the
following request:

   H2 request
     :method: "GET /admin? HTTP/1.1"
     :path:   "/static/images"

would become:

   GET /admin? HTTP/1.1 /static/images HTTP/1.1

It's important to note that the resulting request is *not* valid, and
that in order for this to be a problem, it requires that this request
is delivered to an already vulnerable HTTP/1 server.

A workaround here is to reject malformed methods by placing this rule
in the frontend or backend, at least before leaving haproxy in H1:

   http-request reject if { method -m reg [^A-Z0-9] }

Alternately H2 may be globally disabled by commenting out the "alpn"
directive on "bind" lines, and by rejecting H2 streams creation by
adding the following statement to the global section:

   tune.h2.max-concurrent-streams 0

This patch adds a check for each character of the method to make sure
they belong to the ones permitted in a token, as mentioned in RFC7231#4.1.

This should be backported to versions 2.0 and above. For older versions
not having HTX_FL_PARSING_ERROR, a "goto fail" works as well as it
results in a protocol error at the stream level. Non-HTX versions are
safe because the resulting invalid request will be rejected by the
internal HTTP/1 parser.

Thanks to Tim Düsterhus for reporting that one.

3 years agoBUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
Willy Tarreau [Tue, 10 Aug 2021 14:30:55 +0000 (16:30 +0200)] 
BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it

Tim Düsterhus found that while the H2 path is checked for non-emptiness,
invalid chars and '*', a test is missing to verify that except for '*',
it always starts with exactly one '/'. During the reconstruction of the
full URI when passing to HTX, this missing test allows to affect the
apparent authority by appending a port number or a suffix name.

This only affects H2-to-H2 communications, as H2-to-H1 do not use the
full URI. Like for previous fix, the following rule inserted before
other ones in the frontend is sufficient to renormalize the internal
URI and let haproxy see the same authority as the target server:

    http-request set-uri %[url]

This needs to be backported to 2.2. Earlier versions do not rebuild a
full URI using the authority and will fail on the malformed path at the
HTTP layer, so they are safe.

3 years agoBUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax
Willy Tarreau [Tue, 10 Aug 2021 13:37:34 +0000 (15:37 +0200)] 
BUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax

While we do explicitly check for strict character sets in the scheme,
this is only done when extracting URL components from an assembled one,
and we have special handling for "http" and "https" schemes directly in
the H2-to-HTX conversion. Sadly, this lets all other ones pass through
if they start exactly with "http://" or "https://", allowing the
reconstructed URI to start with a different looking authority if it was
part of the scheme.

It's interesting to note that in this case the valid authority is in
the Host header and that the request will only be wrong if emitted over
H2 on the backend side, since H1 will not emit an absolute URI by
default and will drop the scheme. So in essence, this is a variant of
the scheme-based attack described below in that it only affects H2-H2
and not H2-H1 forwarding:

   https://portswigger.net/research/http2

As such, a simple workaround consists in just inserting the following
rule before other ones in the frontend, which will have for effect to
renormalize the authority in the request line according to the
concatenated version (making haproxy see the same authority and host
as what the target server will see):

   http-request set-uri %[url]

This patch simply adds the missing syntax checks for non-http/https
schemes before the concatenation in the H2 code. An improvement may
consist in the future in splitting these ones apart in the start
line so that only the "url" sample fetch function requires to access
them together and that all other places continue to access them
separately. This will then allow the core code to perform such checks
itself.

The patch needs to be backported as far as 2.2. Before 2.2 the full
URI was not being reconstructed so the scheme and authority part were
always dropped from H2 requests to leave only origin requests. Note
for backporters: this depends on this previous patch:

  MINOR: http: add a new function http_validate_scheme() to validate a scheme

Many thanks to Tim Düsterhus for figuring that one and providing a
reproducer.

3 years agoMINOR: http: add a new function http_validate_scheme() to validate a scheme
Willy Tarreau [Tue, 10 Aug 2021 13:35:36 +0000 (15:35 +0200)] 
MINOR: http: add a new function http_validate_scheme() to validate a scheme

While http_parse_scheme() extracts a scheme from a URI by extracting
exactly the valid characters and stopping on delimiters, this new
function performs the same on a fixed-size string.

3 years agoDOC/MINOR: fix typo in management document
Jonathon Lacher [Wed, 4 Aug 2021 05:29:05 +0000 (00:29 -0500)] 
DOC/MINOR: fix typo in management document

s/Not/Note.

3 years agoDOC: Minor typo fix - 'question mark' -> 'exclamation mark'
Kunal Gangakhedkar [Tue, 17 Aug 2021 06:25:45 +0000 (11:55 +0530)] 
DOC: Minor typo fix - 'question mark' -> 'exclamation mark'

Signed-off-by: Kunal Gangakhedkar <kunal.gangakhedkar@gmail.com>
3 years agoBUILD: tools: get the absolute path of the current binary on NetBSD.
David Carlier [Tue, 17 Aug 2021 07:44:25 +0000 (08:44 +0100)] 
BUILD: tools: get the absolute path of the current binary on NetBSD.

NetBSD stores the absolute path into the auxiliary vector as well.

3 years agoCI: github actions: relax OpenSSL-3.0.0 version comparision
Ilya Shipitsin [Sun, 15 Aug 2021 07:55:08 +0000 (12:55 +0500)] 
CI: github actions: relax OpenSSL-3.0.0 version comparision

we better to check for 3.0.0 presense, than exact version

3 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Sat, 7 Aug 2021 09:41:56 +0000 (14:41 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 25th iteration of typo fixes

3 years agoCI: Remove obsolete USE_SLZ=1 CI job
Tim Duesterhus [Sat, 14 Aug 2021 12:36:55 +0000 (14:36 +0200)] 
CI: Remove obsolete USE_SLZ=1 CI job

Using SLZ is a default, thus this build is equivalent to the "no features"
build.

3 years agoDOC: lua-api: Add documentation about lua filters
Christopher Faulet [Sun, 15 Aug 2021 18:35:25 +0000 (20:35 +0200)] 
DOC: lua-api: Add documentation about lua filters

Lua filter support is highly experimental. The documentation was added to
allow first lua filter implementations. The API is not stabilized and must
be improved to be fully usable. This docuementation is quite light for
now. But more will be added.

3 years agoBUG/MINOR: lua/filters: Return right code when txn:done() is called
Christopher Faulet [Fri, 13 Aug 2021 12:11:17 +0000 (14:11 +0200)] 
BUG/MINOR: lua/filters: Return right code when txn:done() is called

txn functions can now be called from an action or a filter context. Thus the
return code must be adapted depending on this context. From an action, act.ABORT
is returned. From a filter, -1 is returned. It is the filter error code.

This bug only affects 2.5-dev. No backport needed.

3 years agoBUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set
Christopher Faulet [Fri, 13 Aug 2021 12:00:46 +0000 (14:00 +0200)] 
BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag is set

CF_FLT_ANALYZE flags may be set before the FLT_END analyser. Thus if an error is
triggered in the mean time, this may block the stream and prevent it to be
released. It is indeed a problem only for the response channel because the
response analysers may be skipped on early errors.

So, to prevent any issue, depending on the code path, the FLT_END analyser is
systematically set when the CF_FLT_ANALYZE flag is set.

This patch must be backported in all stable branches.

3 years agoMINOR: proxy: disable warnings for internal proxies
William Lallemand [Fri, 13 Aug 2021 13:21:12 +0000 (15:21 +0200)] 
MINOR: proxy: disable warnings for internal proxies

The internal proxies should be part of the proxies list, because of
this, the check_config_validity() fonction could emit warnings about
these proxies.

This patch disables 3 startup warnings for internal proxies:

- "has no 'bind' directive" (this one was already ignored for the CLI
frontend, but we made it generic instead)
- "missing timeouts"
- "log format ignored"

3 years agoMINOR: cli: delare the CLI frontend as an internal proxy
William Lallemand [Fri, 13 Aug 2021 13:31:33 +0000 (15:31 +0200)] 
MINOR: cli: delare the CLI frontend as an internal proxy

Declare the CLI frontend as an internal proxy so we can check the
PR_CAP_INT flag instead of the global.fe_cli pointer for generic use
cases.

3 years agoBUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config check
Emeric Brun [Fri, 13 Aug 2021 07:32:50 +0000 (09:32 +0200)] 
BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config check

User reported that the config check returns an error with the message:
"Configuration file has no error but will not start (no listener) => exit(2)."
if the configuration present only a log-forward section with bind or dgram-bind
listeners but no listen/backend nor peer sections.

The process checked if there was 'peers' section avalaible with
an internal frontend (and so a listener) or a 'listen/backend'
section not disabled with at least one configured listener (into the
global proxies_list). Since the log-forward proxies appear in a
different list, they were not checked.

This patch adds a lookup on the 'log-forward' proxies list to check
if one of them presents a listener and is not disabled. And
this is done only if there was no available listener found into
'listen/backend' sections.

I have also studied how to re-work this check considering the 'listeners'
counter used after startup/init to keep the same algo and avoid further
mistakes but currently this counter seems increased during config parsing
and if a proxy is disabled, decreased during startup/init which is done
after the current config check. So the fix still not rely on this
counter.

This patch should fix the github issue #1346

This patch should be backported as far as 2.3 (so on branches
including the "log-forward" feature)

3 years agoBUG/MINOR: lua: Properly catch alloc errors when parsing lua filter directives
Christopher Faulet [Fri, 13 Aug 2021 06:33:57 +0000 (08:33 +0200)] 
BUG/MINOR: lua: Properly catch alloc errors when parsing lua filter directives

When a lua filter declaration is parsed, some allocation errors were not
properly handled. In addition, we must be sure the filter identifier is defined
in lua to duplicate it when the filter configuration is filled.

This patch fix a defect reported in the issue #1347. It only concerns
2.5-dev. No backport needed.

3 years agoBUG/MINOR: lua: Properly check negative offset in Channel/HttpMessage functions
Christopher Faulet [Fri, 13 Aug 2021 06:11:00 +0000 (08:11 +0200)] 
BUG/MINOR: lua: Properly check negative offset in Channel/HttpMessage functions

In Channel and HTTPMessage classes, several functions uses an offset that
may be negative to start from the end of incoming data. But, after
calculation, the offset must never be negative. However, there is a bug
because of a bad cast to unsigned when "input + offset" is performed. The
result must be a signed integer.

This patch should fix most of defects reported in the issue #1347. It only
affects 2.5-dev. No backport needed.

3 years agoDOC: config: Fix 'http-response send-spoe-group' documentation
Christopher Faulet [Thu, 12 Aug 2021 07:32:07 +0000 (09:32 +0200)] 
DOC: config: Fix 'http-response send-spoe-group' documentation

Arguments were missing in the rule heading. This patch may be backported as
far as 2.0.

3 years agoMINOR: filters/lua: Support the HTTP filtering from filters written in lua
Christopher Faulet [Wed, 26 Feb 2020 16:15:48 +0000 (17:15 +0100)] 
MINOR: filters/lua: Support the HTTP filtering from filters written in lua

Now an HTTPMessage class is available to manipulate HTTP message from a filter
it is possible to bind HTTP filters callback function on lua functions. Thus,
following methods may now be defined by a lua filter:

  * Filter:http_headers(txn, http_msg)
  * Filter:http_payload(txn, http_msg, offset, len)
  * Filter:http_end(txn, http_msg)

http_headers() and http_end() may return one of the constant filter.CONTINUE,
filter.WAIT or filter.ERROR. If nothing is returned, filter.CONTINUE is used as
the default value. On its side, http_payload() may return the amount of data to
forward. If nothing is returned, all incoming data are forwarded.

For now, these functions are not allowed to yield because this interferes with
the filter workflow.

3 years agoMINOR: filters/lua: Add request and response HTTP messages in the lua TXN
Christopher Faulet [Wed, 26 Feb 2020 16:14:08 +0000 (17:14 +0100)] 
MINOR: filters/lua: Add request and response HTTP messages in the lua TXN

When a lua TXN is created from a filter context, the request and the response
HTTP message objects are accessible from ".http_req" and ".http_res" fields. For
an HTTP proxy, these objects are always defined. Otherwise, for a TCP proxy, no
object is created and nil is used instead. From any other context (action or
sample fetch), these fields don't exist.

3 years agoMEDIUM: filters/lua: Add HTTPMessage class to help HTTP filtering
Christopher Faulet [Wed, 26 Feb 2020 15:57:19 +0000 (16:57 +0100)] 
MEDIUM: filters/lua: Add HTTPMessage class to help HTTP filtering

This new class exposes methods to manipulate HTTP messages from a filter
written in lua. Like for the HTTP class, there is a bunch of methods to
manipulate the message headers. But there are also methods to manipulate the
message payload. This part is similar to what is available in the Channel
class. Thus the payload can be duplicated, erased, modified or
forwarded. For now, only DATA blocks can be retrieved and modified because
the current API is limited. No HTTPMessage method is able to yield. Those
manipulating the headers are always called on messages containing all the
headers, so there is no reason to yield. Those manipulating the payload are
called from the http_payload filters callback function where yielding is
forbidden.

When an HTTPMessage object is instantiated, the underlying Channel object
can be retrieved via the ".channel" field.

For now this class is not used because the HTTP filtering is not supported
yet. It will be the purpose of another commit.

There is no documentation for now.

3 years agoMEDIUM: filters/lua: Support declaration of some filter callback functions in lua
Christopher Faulet [Wed, 26 Feb 2020 14:03:09 +0000 (15:03 +0100)] 
MEDIUM: filters/lua: Support declaration of some filter callback functions in lua

It is now possible to write some filter callback functions in lua. All
filter callbacks are not supported yet but the mechanism to call them is now
in place. Following method may be defined in the Lua filter class to be
bound on filter callbacks:

  * Filter:start_analyse(txn, chn)
  * Filter:end_analyse(txn, chn)
  * Filter:tcp_payload(txn, chn, offset, length)

hlua_filter_callback() function is responsible to call the good lua function
depending on the filter callback function. Using some flags it is possible
to allow a lua call to yield or not, to retrieve a return value or not, and
to specify if a channel or an http message must be passed as second
argument. For now, the HTTP part has not been added yet. It is also possible
to add extra argument adding them on the stack before the call.

3 new functions are exposed by the global object "filter". The first one,
filter.wake_time(ms_delay), to set the wake_time when a Lua callback
function yields (if allowed). The two others,
filter.register_data_filter(filter, chn) and
filter.unregister_data_filter(filter, chn), to enable or disable the data
filtering on a channel for a specific lua filter instance.

start_analyse() and end_analyse() may return one of the constant
filter.CONTINUE, filter.WAIT or filter.ERROR. If nothing is returned,
filter.CONTINUE is used as the default value. On its side, tcp_payload() may
return the amount of data to forward. If nothing is returned, all incoming
data are forwarded.

For now, these functions are not allowed to yield because this interferes
with the filter workflow.

Here is a simple example :

    MyFilter = {}
    MyFilter.id = "My Lua filter"
    MyFilter.flags = filter.FLT_CFG_FL_HTX
    MyFilter.__index = MyFilter

    function MyFilter:new()
        flt = {}
        setmetatable(flt, MyFilter)
        flt.req_len = 0
        flt.res_len = 0
        return flt
     end

    function MyFilter:start_analyze(txn, chn)
        filter.register_data_filter(self, chn)
    end

    function MyFilter:end_analyze(txn, chn)
        print("<Total> request: "..self.req_len.." - response: "..self.res_len)
    end

    function MyFilter:tcp_payload(txn, chn)
        offset = chn:ouput()
len    = chn:input()
        if chn:is_resp() then
            self.res_len = self.res_len + len
            print("<TCP:Response> offset: "..offset.." - length: "..len)
        else
            self.req_len = self.req_len + len
            print("<TCP:Request> offset: "..offset.." - length: "..len)
        end
    end

3 years agoMEDIUM: filters/lua: Be prepared to filter TCP payloads
Christopher Faulet [Mon, 9 Aug 2021 08:22:46 +0000 (10:22 +0200)] 
MEDIUM: filters/lua: Be prepared to filter TCP payloads

For filters written in lua, the tcp payloads will be filtered using methods
exposed by the Channel class. So the corrsponding C binding functions must
be prepared to process payload in a filter context and not only in an action
context.

The main change is the offset where to start to process data in the channel
buffer, and the length of these data. For an action, all input data are
considered. But for a filter, it depends on what the filter is allow to
forward when the tcp_payload callback function is called. It depends on
previous calls but also on other filters.

In addition, when the payload is modified by a lua filter, its context must
be updated. Note also that channel functions cannot yield when called from a
filter context.

For now, it is not possible to define callbacks to filter data and the
documentation has not been updated.

3 years agoMINOR: lua: Add flags on the lua TXN to know the execution context
Christopher Faulet [Fri, 6 Aug 2021 14:29:41 +0000 (16:29 +0200)] 
MINOR: lua: Add flags on the lua TXN to know the execution context

A lua TXN can be created when a sample fetch, an action or a filter callback
function is executed. A flag is now used to track the execute context.
Respectively, HLUA_TXN_SMP_CTX, HLUA_TXN_ACT_CTX and HLUA_TXN_FLT_CTX. The
filter flag is not used for now.

3 years agoMINOR: lua: Add a function to get a filter attached to a channel class
Christopher Faulet [Tue, 25 Feb 2020 14:21:02 +0000 (15:21 +0100)] 
MINOR: lua: Add a function to get a filter attached to a channel class

For now, there is no support for filters written in lua. So this function,
if called, will always return NULL. But when it will be called in a filter
context, it will return the filter structure attached to a channel
class. This function is also responsible to set the offset of data that may
be processed and the length of these data. When called outside a filter
context (so from an action), the offset is the input data position and the
length is the input data length. From a filter, the offset and the length of
data that may be filtered are retrieved the filter context.

3 years agoMEDIUM: filters/lua: Add support for dummy filters written in lua
Christopher Faulet [Mon, 31 May 2021 06:54:04 +0000 (08:54 +0200)] 
MEDIUM: filters/lua: Add support for dummy filters written in lua

It is now possible to write dummy filters in lua. Only the basis to declare
such filters has been added for now. There is no way to declare callbacks to
filter anything. Lua filters are for now empty nutshells.

To do so, core.register_filter() must be called, with 3 arguments, the
filter's name (as it appears in HAProxy config), the lua class that will be
used to instantiate filters and a function to parse arguments passed on the
filter line in HAProxy configuration file. The lua filter class must at
least define the method new(), without any extra args, to create new
instances when streams are created. If this method is not found, the filter
will be ignored.

Here is a template to declare a new Lua filter:

    // haproxy.conf
    global
        lua-load /path/to/my-filter.lua
        ...

    frontend fe
        ...
        filter lua.my-lua-filter arg1 arg2 arg3
        filter lua.my-lua-filter arg4 arg5

    // my-filter.lua
    MyFilter = {}
    MyFilter.id = "My Lua filter"          -- the filter ID (optional)
    MyFilter.flags = filter.FLT_CFG_FL_HTX -- process HTX streams (optional)
    MyFilter.__index = MyFilter

    function MyFilter:new()
        flt = {}
        setmetatable(flt, MyFilter)
        -- Set any flt fields. self.args can be used
flt.args = self.args

        return flt -- The new instance of Myfilter
    end

    core.register_filter("my-lua-filter", MyFilter, function(filter, args)
        -- process <args>, an array of strings. For instance:
        filter.args = args
        return filter
    end)

In this example, 2 filters are declared using the same lua class. The
parsing function is called for both, with its own copy of the lua class. So
each filter will be unique.

The global object "filter" exposes some constants and flags, and later some
functions, to help writting filters in lua.

Internally, when a lua filter is instantiated (so when new() method is
called), 2 lua contexts are created, one for the request channel and another
for the response channel. It is a prerequisite to let some callbacks yield
on one side independently on the other one.

There is no documentation for now.

3 years agoDOC: Improve the lua documentation
Christopher Faulet [Wed, 11 Aug 2021 08:14:30 +0000 (10:14 +0200)] 
DOC: Improve the lua documentation

The ReST syntax is used for note and warning blocks.

3 years agoMEDIUM: lua: Improve/revisit the lua api to manipulate channels
Christopher Faulet [Fri, 6 Aug 2021 14:02:36 +0000 (16:02 +0200)] 
MEDIUM: lua: Improve/revisit the lua api to manipulate channels

First of all, following functions are now considered deprecated:

  * Channel:dup()
  * Channel:get()
  * Channel:getline()
  * Channel:get_in_len()
  * Cahnnel:get_out_len()

It is just informative, there is no warning and functions may still be
used. Howver it is recommended to use new functions. New functions are more
flexible and use a better naming pattern. In addition, the same names will
be used in the http_msg class to manipulate http messages from lua filters.

The new API is:

  * Channel:data()
  * Channel:line()
  * Channel:append()
  * Channel:prepend()
  * Channel:insert()
  * Channel:remove()
  * Channel:set()
  * Channel:input()
  * Channel:output()
  * Channel:send()
  * Channel:forward()
  * Channel:is_resp()
  * Channel:is_full()
  * Channel:may_recv()

The lua documentation was updated accordingly.

3 years agoMEDIUM: lua: Process buffer data using an offset and a length
Christopher Faulet [Fri, 6 Aug 2021 11:49:54 +0000 (13:49 +0200)] 
MEDIUM: lua: Process buffer data using an offset and a length

The main change is that following functions will now process channel's data
using an offset and a length:

  * hlua_channel_dup_yield()
  * hlua_channel_get_yield()
  * hlua_channel_getline_yield()
  * hlua_channel_append_yield()
  * hlua_channel_set()
  * hlua_channel_send_yield()
  * hlua_channel_forward_yield()

So for now, the offset is always the input data position and the length is
the input data length. But with the support for filters, from a filter
context, these values will be relative to the filter.

To make all processing clearer, the function _hlua_channel_dup() has been
updated and _hlua_channel_dupline(), _hlua_channel_insert() and
_hlua_channel_delete() have been added.

This patch is mandatory to allow the support of the filters written in lua.

3 years agoMINOR: lua: Add a function to get a reference on a table in the stack
Christopher Faulet [Tue, 25 Feb 2020 09:20:04 +0000 (10:20 +0100)] 
MINOR: lua: Add a function to get a reference on a table in the stack

The hlua_checktable() function may now be used to create and return a
reference on a table in stack, given its position. This function ensures it
is really a table and throws an exception if not.

This patch is mandatory to allow the support of the filters written in lua.

3 years agoMINOR: filters/lua: Release filters before the lua context
Christopher Faulet [Mon, 24 Feb 2020 15:26:55 +0000 (16:26 +0100)] 
MINOR: filters/lua: Release filters before the lua context

This patch is mandatory to allow the support of the filters written in lua.

3 years agoBUG/MINOR: lua: Don't yield in channel.append() and channel.set()
Christopher Faulet [Fri, 6 Aug 2021 07:59:49 +0000 (09:59 +0200)] 
BUG/MINOR: lua: Don't yield in channel.append() and channel.set()

Lua functions to set or append data to the input part of a channel must not
yield because new data may be received while the lua script is suspended. So
adding data to the input part in several passes is highly unpredicatble and
may be interleaved with received data.

Note that if necessary, it is still possible to suspend a lua action by
returning act.YIELD. This way the whole action will be reexecuted later
because of I/O events or a timer. Another solution is to call core.yield().

This bug affects all stable versions. So, it may be backported. But it is
probably not necessary because nobody notice it till now.

3 years agoBUG/MINOR: lua: Yield in channel functions only if lua context can yield
Christopher Faulet [Thu, 5 Aug 2021 09:58:37 +0000 (11:58 +0200)] 
BUG/MINOR: lua: Yield in channel functions only if lua context can yield

When a script is executed, it is not always allowed to yield. Lua sample
fetches and converters cannot yield. For lua actions, it depends on the
context. When called from tcp content ruleset, an action may yield until the
expiration of the inspect-delay timeout. From http rulesets, yield is not
possible.

Thus, when channel functions (dup, get, append, send...) are called, instead
of yielding when it is not allowed and triggering an error, we just give
up. In this case, some functions do nothing (dup, append...), some others
just interrupt the in-progress job (send, forward...). But, because these
functions don't yield anymore when it is not allowed, the script regains the
control and can continue its execution.

This patch depends on "MINOR: lua: Add a flag on lua context to know the
yield capability at run time". Both may be backported in all stable
versions. However, because nobody notice this bug till now, it is probably
not necessary, excepted if someone ask for it.

3 years agoMINOR: lua: Add a flag on lua context to know the yield capability at run time
Christopher Faulet [Wed, 4 Aug 2021 15:58:21 +0000 (17:58 +0200)] 
MINOR: lua: Add a flag on lua context to know the yield capability at run time

When a script is executed, a flag is used to allow it to yield. An error is
returned if a lua function yield, explicitly or not. But there is no way to
get this capability in C functions. So there is no way to choose to yield or
not depending on this capability.

To fill this gap, the flag HLUA_NOYIELD is introduced and added on the lua
context if the current script execution is not authorized to yield. Macros
to set, clear and test this flags are also added.

This feature will be usefull to fix some bugs in lua actions execution.

3 years agoBUG/MINOR: stream: Don't release a stream if FLT_END is still registered
Christopher Faulet [Wed, 13 Nov 2019 10:12:32 +0000 (11:12 +0100)] 
BUG/MINOR: stream: Don't release a stream if FLT_END is still registered

When at least one filter is registered on a stream, the FLT_END analyzer is
called on both direction when all other analyzers have finished their
processing. During this step, filters may release any allocated elements if
necessary. So it is important to not skip it.

Unfortunately, if both stream interfaces are closed, it is possible to not
wait the end of this analyzer. It is possible to be in this situation if a
filter must wait and prevents the analyzer completion. To fix the bug, we
now wait FLT_END analyzer is no longer registered on both direction before
releasing the stream.

This patch may be backported as far as 1.7, but AFAIK, no filter is affected
by this bug. So the backport seems to be optional for now. In any case, it
should remain under observation for some weeks first.

3 years agoBUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
Christopher Faulet [Wed, 11 Aug 2021 13:46:29 +0000 (15:46 +0200)] 
BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer

In tcpcheck_eval_send(), the condition to detect there are still pending
data in the output buffer is buggy. Presence of raw data must be tested for
TCP connection only. But a condition on the connection was missing to be
sure it is not an HTX connection.

This patch must be backported as far as 2.2.

3 years agoMINOR: channel: remove an htx block from a channel
William Lallemand [Wed, 11 Aug 2021 11:40:14 +0000 (13:40 +0200)] 
MINOR: channel: remove an htx block from a channel

co_htx_remove_blk() implements a way to remove an htx block from a
channel buffer and update the channel output.

3 years agoBUG/MINOR: buffer: fix buffer_dump() formatting
William Lallemand [Mon, 9 Aug 2021 17:37:16 +0000 (19:37 +0200)] 
BUG/MINOR: buffer: fix buffer_dump() formatting

The formatting of the buffer_dump() output must be calculated using the
relative counter, not the absolute one, or everything will be broken if
the <from> variable is not a multiple of 16.

Could be backported in all maintained versions.

3 years agoBUG/MEDIUM: server: support both check/agent-check on a dynamic instance
Amaury Denoyelle [Tue, 10 Aug 2021 14:24:36 +0000 (16:24 +0200)] 
BUG/MEDIUM: server: support both check/agent-check on a dynamic instance

A static server is able to support simultaneously both health chech and
agent-check. Adjust the dynamic server CLI handlers to also support this
configuration.

This should not be backported, unless dynamic server checks are
backported.

3 years agoBUG/MEDIUM: check: fix leak on agent-check purge
Amaury Denoyelle [Tue, 10 Aug 2021 14:23:49 +0000 (16:23 +0200)] 
BUG/MEDIUM: check: fix leak on agent-check purge

There is currently a leak on agent-check for dynamic servers. When
deleted, the check rules and vars are not liberated. This leak grows
each time a dynamic server with agent-check is deleted.

Replace the manual purge code by a free_check invocation which
centralizes all the details on check cleaning.

There is no leak for health check because in this case the proxy is the
owner of the check vars and rules.

This should not be backported, unless dynamic server checks are
backported.

3 years agoBUG/MINOR: check: fix leak on add dynamic server with agent-check error
Amaury Denoyelle [Tue, 10 Aug 2021 14:22:51 +0000 (16:22 +0200)] 
BUG/MINOR: check: fix leak on add dynamic server with agent-check error

If an error occured during a dynamic server creation, free_check is used
to liberate a possible agent-check. However, this does not free
associated vars and rules associated as this is done on another function
named deinit_srv_agent_check.

To simplify the check free and avoid a leak, move free vars/rules in
free_check. This is valid because deinit_srv_agent_check also uses
free_check.

This operation is done only for an agent-check because for a health
check, the proxy instance is the owner of check vars/rules.

This should not be backported, unless dynamic server checks are
backported.

3 years agoBUG/MINOR: check: do not reset check flags on purge
Amaury Denoyelle [Tue, 10 Aug 2021 14:21:55 +0000 (16:21 +0200)] 
BUG/MINOR: check: do not reset check flags on purge

Do not reset check flags when setting CHK_ST_PURGE.

Currently, this change has no impact. However, it is semantically wrong
to clear important flags such as CHK_ST_AGENT on purge.

Furthermore, this change will become mandatoy for a future fix to
properly free agent checks on dynamic servers removal. For this, it will
be needed to differentiate health/agent-check on purge via CHK_ST_AGENT
to properly free agent checks.

This must not be backported unless dynamic servers checks are
backported.

3 years agoADMIN: dyncookie: implement a simple dynamic cookie calculator
Willy Tarreau [Wed, 11 Aug 2021 11:54:52 +0000 (13:54 +0200)] 
ADMIN: dyncookie: implement a simple dynamic cookie calculator

This utility can be useful to figure what cookie value a server will
have based on the secret, its IP and its port.

3 years agoBUG/MINOR: server: do not use refcount in free_server in stopping mode
Amaury Denoyelle [Mon, 9 Aug 2021 13:08:54 +0000 (15:08 +0200)] 
BUG/MINOR: server: do not use refcount in free_server in stopping mode

Currently there is a leak at process shutdown with dynamic servers with
check/agent-check activated. Check purges are not executed on process
stopping, so the server is not liberated due to its refcount.

The solution is simply to ignore the refcount on process stopping mode
and free the server on the first free_server invocation.

This should not be backported, unless dynamic server checks are
backported. In this case, the following commit must be backported first.
  7afa5c1843521ec3be7549592d2b38ccc9d68b73
  MINOR: global: define MODE_STOPPING

3 years agoMINOR: global: define MODE_STOPPING
Amaury Denoyelle [Mon, 9 Aug 2021 13:02:56 +0000 (15:02 +0200)] 
MINOR: global: define MODE_STOPPING

Define a new mode MODE_STOPPING. It is used to indicate that the process
is in the stopping stage and no event loop runs anymore.

3 years agoBUG/MINOR: check: test if server is not null in purge
Amaury Denoyelle [Mon, 9 Aug 2021 13:09:17 +0000 (15:09 +0200)] 
BUG/MINOR: check: test if server is not null in purge

Test if server is not null before using free_server in the check purge
operation. Currently, the null server scenario should not occured as
purge is used with refcounted dynamic servers. However, this might not
be always the case if purge is use in the future in other cases; thus
the test is useful for extensibility.

No need to backport, unless dynamic server checks are backported.

This has been reported through a coverity report in github issue #1343.

3 years agoCI: travis-ci: temporarily disable arm64 builds
Ilya Shipitsin [Tue, 3 Aug 2021 09:54:09 +0000 (14:54 +0500)] 
CI: travis-ci: temporarily disable arm64 builds

few recent builds failed with "gcc: fatal error: Killed signal
terminated program cc1", let us disable arm64 builds until investigation

3 years agoREGTESTS: server: fix dynamic server with checks test
Amaury Denoyelle [Fri, 6 Aug 2021 13:34:04 +0000 (15:34 +0200)] 
REGTESTS: server: fix dynamic server with checks test

Add a missing 'rxreq' statement in first server. Without it the test is
unstable. The issue is frequent when running with one thread only.

This should fix github issue #1342.

3 years agoMINOR: doc: specify ulimit-n usage for dynamic servers
Amaury Denoyelle [Fri, 6 Aug 2021 08:25:32 +0000 (10:25 +0200)] 
MINOR: doc: specify ulimit-n usage for dynamic servers

Complete the documentation of the dynamic servers to warn about a
possible fd resource exhaustion when using a large number of them.

3 years agoREGTESTS: server: add dynamic check server test
Amaury Denoyelle [Thu, 29 Jul 2021 15:34:16 +0000 (17:34 +0200)] 
REGTESTS: server: add dynamic check server test

Write a regtest to validate check support by dynamic servers. Three
differents servers are added on various configuration :
- server OK
- server DOWN
- agent-check

3 years agoMEDIUM: server: implement agent check for dynamic servers
Amaury Denoyelle [Wed, 4 Aug 2021 09:33:14 +0000 (11:33 +0200)] 
MEDIUM: server: implement agent check for dynamic servers

This commit is the counterpart for agent check of
"MEDIUM: server: implement check for dynamic servers".

The "agent-check" keyword is enabled for dynamic servers. The agent
check must manually be activated via "enable agent" CLI. This can
enable the dynamic server if the agent response is "ready" without an
explicit "enable server" CLI.

3 years agoMEDIUM: server: implement check for dynamic servers
Amaury Denoyelle [Thu, 22 Jul 2021 14:04:59 +0000 (16:04 +0200)] 
MEDIUM: server: implement check for dynamic servers

Implement check support for dynamic servers. The "check" keyword is now
enabled for dynamic servers. If used, the server check is initialized
and the check task started in the "add server" CLI handler. The check is
explicitely disabled and must be manually activated via "enable health"
CLI handler.

The dynamic server refcount is incremented if a check is configured. On
"delete server" handler, the check is purged, which decrements the
refcount.

3 years agoMINOR: check: enable safe keywords for dynamic servers
Amaury Denoyelle [Fri, 23 Jul 2021 14:34:58 +0000 (16:34 +0200)] 
MINOR: check: enable safe keywords for dynamic servers

Implement a collection of keywords deemed safe and useful to dynamic
servers. The list of the supported keywords is :
- addr
- check-proto
- check-send-proxy
- check-via-socks4
- rise
- fall
- fastinter
- downinter
- port
- agent-addr
- agent-inter
- agent-port
- agent-send

3 years agoMEDIUM: check: implement check deletion for dynamic servers
Amaury Denoyelle [Thu, 29 Jul 2021 13:51:45 +0000 (15:51 +0200)] 
MEDIUM: check: implement check deletion for dynamic servers

Implement a mechanism to free a started check on runtime for dynamic
servers. A new function check_purge is created for this. The check task
will be marked for deletion and scheduled to properly close connection
elements and free the task/tasklet/buf_wait elements.

This function will be useful to delete a dynamic server wich checks.

3 years agoMINOR: server: implement a refcount for dynamic servers
Amaury Denoyelle [Mon, 2 Aug 2021 13:50:00 +0000 (15:50 +0200)] 
MINOR: server: implement a refcount for dynamic servers

It is necessary to have a refcount mechanism on dynamic servers to be
able to enable check support. Indeed, when deleting a dynamic server
with check activated, the check will be asynchronously removed. This is
mandatory to properly free the check resources in a thread-safe manner.
The server instance must be kept alive for this.

3 years agoMINOR: check: do not increment global maxsock at runtime
Amaury Denoyelle [Thu, 29 Jul 2021 13:39:43 +0000 (15:39 +0200)] 
MINOR: check: do not increment global maxsock at runtime

global maxsock is used to estimate a number of fd to reserve for
internal use, such as checks. It is incremented at startup with the info
from the config file.

Disable this incrementation in checks functions at runtime. First, it
currently serves no purpose to increment it after startup. Worse, it may
lead to out-of-bound accesse on the fdtab.

This will be useful to initiate checks for dynamic servers.

3 years agoMINOR: check: export check init functions
Amaury Denoyelle [Thu, 22 Jul 2021 14:04:40 +0000 (16:04 +0200)] 
MINOR: check: export check init functions

Remove static qualifier on init_srv_check, init_srv_agent_check and
start_check_task. These functions will be called in server.c for dynamic
servers with checks.

3 years agoMINOR: check: allocate default check ruleset for every backends
Amaury Denoyelle [Fri, 23 Jul 2021 13:46:46 +0000 (15:46 +0200)] 
MINOR: check: allocate default check ruleset for every backends

Allocate default tcp ruleset for every backend without explicit rules
defined, even if no server in the backend use check. This change is
required to implement checks for dynamic servers.

This allocation is done on check_config_validity. It must absolutely be
called before check_proxy_tcpcheck (called via post proxy check) which
allocate the implicit tcp connect rule.

3 years agoMINOR: server: initialize fields for dynamic server check
Amaury Denoyelle [Thu, 22 Jul 2021 14:03:36 +0000 (16:03 +0200)] 
MINOR: server: initialize fields for dynamic server check

Set default inter/rise/fall values for dynamic servers check/agent. This
is required because dynamic servers do not inherit from a
default-server.

3 years agoMEDIUM: task: implement tasklet kill
Amaury Denoyelle [Wed, 28 Jul 2021 14:12:57 +0000 (16:12 +0200)] 
MEDIUM: task: implement tasklet kill

Implement an equivalent of task_kill for tasklets. This function can be
used to request a tasklet deletion in a thread-safe way.

Currently this function is unused.

3 years agoMINOR: server: unmark deprecated on enable health/agent cli
Amaury Denoyelle [Tue, 3 Aug 2021 16:05:37 +0000 (18:05 +0200)] 
MINOR: server: unmark deprecated on enable health/agent cli

Remove the "DEPRECATED" marker on "enable/disable health/agent"
commands. Their purpose is to toggle the check/agent on a server.

These commands are still useful because their purpose is not covered by
the "set server" command. Most there was confusion with the commands
'set server health/agent', which in fact serves another goal.

Note that the indication "use 'set server' instead" has been added since
2016 on the commit
  2c04eda8b58636ad2ae44e42b1f50f3b5a24a642
  REORG: cli: move "{enable|disable} health" to server.c

and
  58d9cb7d22c1b0d8239543443131e3e3658375d0
  REORG: cli: move "{enable|disable} agent" to server.c

Besides, these commands will become required to enable check/agent on
dynamic servers which will be created with check disabled.

This should be backported up to 2.4.