A dynamic server may be deleted at runtime at the same moment when the
stats applet is pointing to it. Use the server refcount to prevent
deletion in this case.
This should be backported up to 2.4, with an observability period of 2
weeks. Note that it requires the dynamic server refcounting feature
which has been implemented on 2.5; the following commits are required :
- MINOR: server: implement a refcount for dynamic servers
- BUG/MINOR: server: do not use refcount in free_server in stopping mode
- MINOR: server: return the next srv instance on free_server
As a convenience, return the next server instance from servers list on
free_server.
This is particularily useful when using this function on the servers
list without having to save of the next pointer before calling it.
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).
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.
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.
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>".
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 ed697e4856
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().
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.
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+).
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.
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.
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").
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.
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
Reported by coverity in ticket #1355
CID 1461505: Memory - illegal accesses (UNINIT)
Using uninitialized value "sl".
Fix the problem by initializing sl to NULL.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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
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.
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.
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.
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.
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.
