Commit Graph

13230 Commits

Author SHA1 Message Date
Christopher Faulet
f8c869bac4 MINOR: config: Add a warning if tune.chksize is used
This option is now deprecated. It is recent, but it is now marked as
deprecated as far as 2.2. Thus, there is now a warning in the 2.4 if this
option is still used. It will be removed in 2.5.

Becaue the 2.3 is quite new, this patch may be backported to 2.3.
2020-11-27 10:30:23 +01:00
Christopher Faulet
bb9fb8b7f8 MINOR: config: Deprecate and ignore tune.chksize global option
This option is now ignored because I/O check buffers are now allocated using the
buffer pool. Thus, it is marked as deprecated in the documentation and ignored
during the configuration parsing. The field is also removed from the global
structure.

Because this option is ignored since a recent fix, backported as fare as 2.2,
this patch should be backported too. Especially because it updates the
documentation.
2020-11-27 10:30:23 +01:00
Christopher Faulet
b1bb069c15 MINOR: tcpcheck: Don't handle anymore in-progress connect rules in tcpcheck_main
The special handling of in-progress connect rules at the begining of
tcpcheck_main() function can be removed. Instead, at the begining of the
tcpcheck_eval_connect() function, we test is there is already an existing
connection. In this case, it means we are waiting for a connection
establishment. In addition, before evaluating a new connect rule, we take
care to release any previous connection.
2020-11-27 10:29:41 +01:00
Christopher Faulet
b381a505c1 BUG/MAJOR: tcpcheck: Allocate input and output buffers from the buffer pool
Historically, the input and output buffers of a check are allocated by hand
during the startup, with a specific size (not necessarily the same than
other buffers). But since the recent refactoring of the checks to rely
exclusively on the tcp-checks and to use the underlying mux layer, this part
is totally buggy. Indeed, because these buffers are now passed to a mux,
they maybe be swapped if a zero-copy is possible. In fact, for now it is
only possible in h2_rcv_buf(). Thus the bug concretely only exists if a h2
health-check is performed. But, it is a latent bug for other muxes.

Another problem is the size of these buffers. because it may differ for the
other buffer size, it might be source of bugs.

Finally, for configurations with hundreds of thousands of servers, having 2
buffers per check always allocated may be an issue.

To fix the bug, we now allocate these buffers when required using the buffer
pool. Thus not-running checks don't waste memory and muxes may swap them if
possible. The only drawback is the check buffers have now always the same
size than buffers used by the streams. This deprecates indirectly the
"tune.chksize" global option.

In addition, the http-check regtest have been update to perform some h2
health-checks.

Many thanks to @VigneshSP94 for its help on this bug.

This patch should solve the issue #936. It relies on the commit "MINOR:
tcpcheck: Don't handle anymore in-progress send rules in tcpcheck_main".
Both must be backport as far as 2.2.

bla
2020-11-27 10:29:41 +01:00
Christopher Faulet
39066c2738 MINOR: tcpcheck: Don't handle anymore in-progress send rules in tcpcheck_main
The special handling of in-progress send rules at the begining of
tcpcheck_main() function can be removed. Instead, at the begining of the
tcpcheck_eval_send() function, we test is there is some data in the output
buffer. In this case, it means we are evaluating an unfinished send rule and
we can jump to the sending part, skipping the formatting part.

This patch is mandatory for a major fix on the checks and must be backported
as far as 2.2.
2020-11-27 10:08:21 +01:00
Christopher Faulet
1faf18ae39 BUG/MINOR: tcpcheck: Don't forget to reset tcp-check flags on new kind of check
When a new kind of check is found during the parsing of a proxy section (via
an option directive), we must reset tcpcheck flags for this proxy. It is
mandatory to not inherit some flags from a previously declared check (for
instance in the default section).

This patch must be backported as far as 2.2.
2020-11-27 10:08:18 +01:00
Willy Tarreau
5a7d6ebf2c MINOR: fd/threads: silence a build warning with threads disabled
Building with gcc-9.3.0 without threads may result in this warning:

In file included from include/haproxy/api-t.h:36,
                 from include/haproxy/api.h:33,
                 from src/fd.c:90:
src/fd.c: In function 'updt_fd_polling':
include/haproxy/fd.h:507:11: warning: array subscript 63 is above array bounds of 'int[1]' [-Warray-bounds]
  507 |  DISGUISE(write(poller_wr_pipe[tid], &c, 1));
include/haproxy/compiler.h:92:41: note: in definition of macro 'DISGUISE'
   92 | #define DISGUISE(v) ({ typeof(v) __v = (v); ALREADY_CHECKED(__v); __v; })
      |                                         ^
src/fd.c:113:5: note: while referencing 'poller_wr_pipe'
  113 | int poller_wr_pipe[MAX_THREADS]; // Pipe to wake the threads
      |     ^~~~~~~~~~~~~~

gcc is wrong but this time it cannot be blamed because it doesn't know
that the FD's thread_mask always has at least one bit set. Let's add
the test for all_threads_mask there. It will also remove that test and
drop the else block.
2020-11-26 22:28:41 +01:00
Ilya Shipitsin
b34aee8294 CI: github actions: enable 51degrees feature 2020-11-26 19:08:15 +01:00
Ilya Shipitsin
f500359708 CI: github actions: update LibreSSL to 3.3.0 2020-11-26 19:08:02 +01:00
Maciej Zdeb
21acc33266 DOC: Clarify %HP description in log-format
%HP is used to report HTTP request URI in logs, which might be relative
or absolute. Description in documentation should not suggest that it
behaves exactly the same as "path" sample fetch.

This is even more important after 30ee1efe67
because right now, when HTTP2 is a standard, %HP usually returns absolute
URI.

This might be backported as far as 2.1
2020-11-26 19:07:21 +01:00
Willy Tarreau
6f1129d14d DOC: better document the config file format and escaping/quoting rules
It's always a pain to figure how to proceed when special characters need
to be embedded inside arguments of an expression. Let's document the
configuration file format and how unquoting/unescaping works at each
level (top level and argument level) so that everyone hopefully finds
suitable reminders or examples for complex cases.

This is related to github issue #200 and addresses issues #712 and #966.
2020-11-26 18:50:12 +01:00
Remi Tricot-Le Breton
4f7308335e DOC: cache: Add information about Vary support
We do not skip all responses containing a Vary in the cachign mechanism
anymore. Under certain conditions such responses might be cached.
2020-11-26 18:01:43 +01:00
Remi Tricot-Le Breton
d493bc863d DOC: cache: Add new caching limitation information
Responses that do not have an explicit expiration time or a validator
will not be cached anymore.

Must be backported if cc9bf2e ("MEDIUM: cache: Change caching
conditions") is backported.
2020-11-26 17:58:01 +01:00
Willy Tarreau
345ebcfc01 BUG/MAJOR: peers: fix partial message decoding
Another bug in the peers message parser was uncovered by last commit
1dfd4f106 ("BUG/MEDIUM: peers: fix decoding of multi-byte length in
stick-table messages"): the function return on incomplete message does
not check if the channel has a pending close before deciding to return
0. It did not hurt previously because the loop calling co_getblk() once
per character would have depleted the buffer and hit the end, causing
<0 to be returned and matching the condition. But now that we process
at once what is available this cannot be relied on anymore and it's
now clearly visible that the final check is missing.

What happens when this strikes is that if a peer connection breaks in
the middle of a message, the function will return 0 (missing data) but
the caller doesn't check for the closed buffer, subscribes to reads,
and the applet handler is immediately called again since some data are
still available. This is detected by the loop prevention and the process
dies complaining that an appctx is spinning.

This patch simply adds the check for closed channel. It must be
backported to the same versions as the fix above.
2020-11-26 17:12:47 +01:00
Tim Duesterhus
23b2945c1c BUG/CRITICAL: cache: Fix trivial crash by sending accept-encoding header
Since commit 3d08236cb3 HAProxy can be trivially
crashed remotely by sending an `accept-encoding` HTTP request header that
contains 16 commas.

This is because the `values` array in `accept_encoding_normalizer` accepts only
16 entries and it is not verified whether the end is reached during looping.

Fix this issue by checking the length. This patch also simplifies the ist
processing in the loop, because it manually calculated offsets and lengths,
when the ist API exposes perfectly safe functions to advance and truncate ists.

I wonder whether the accept_encoding_normalizer function is able to re-use some
existing function for parsing headers that may contain lists of values. I'll
leave this evaluation up to someone else, only patching the obvious crash.

This commit is 2.4-dev specific and was merged just a few hours ago. No
backport needed.
2020-11-25 10:23:00 +01:00
Remi Tricot-Le Breton
754b2428d3 MINOR: cache: Add a process-vary option that can enable/disable Vary processing
The cache section's process-vary option takes a 0 or 1 value to disable
or enable the vary processing.
When disabled, a response containing such a header will never be cached.
When enabled, we will calculate a preliminary hash for a subset of request
headers on all the incoming requests (which might come with a cpu cost) which
will be used to build a secondary key for a given request (see RFC 7234#4.1).
The default value is 0 (disabled).
2020-11-24 16:52:57 +01:00
Remi Tricot-Le Breton
1785f3dd96 MEDIUM: cache: Add the Vary header support
Calculate a preliminary secondary key for every request we see so that
we can have a real secondary key if the response is cacheable and
contains a manageable Vary header.
The cache's ebtree is now allowed to have multiple entries with the same
primary key. Two of those entries will be distinguished thanks to
secondary keys stored in the cache_entry (based on hashes of a subset of
their headers).
When looking for an entry in the cache (cache_use), we still use the
primary key (built the same way as before), but in case of match, we
also need to check if the entry has a vary signature. If it has one, we
need to perform an extra check based on the newly built secondary key.
We will only be able to forge a response out of the cache if both the
primary and secondary keys match with one of our entries. Otherwise the
request will be forwarder to the server.
2020-11-24 16:52:57 +01:00
Remi Tricot-Le Breton
3d08236cb3 MINOR: cache: Prepare helper functions for Vary support
The Vary functionality is based on a secondary key that needs to be
calculated for every request to which a server answers with a Vary
header. The Vary header, which can only be found in server responses,
determines which headers of the request need to be taken into account in
the secondary key. Since we do not want to have to store all the headers
of the request until we have the response, we will pre-calculate as many
sub-hashes as there are headers that we want to manage in a Vary
context. We will only focus on a subset of headers which are likely to
be mentioned in a Vary response (accept-encoding and referer for now).
Every managed header will have its own normalization function which is
in charge of transforming the header value into a core representation,
more robust to insignificant changes that could exist between multiple
clients. For instance, two accept-encoding values mentioning the same
encodings but in different orders should give the same hash.
This patch adds a function that parses a Vary header value and checks if
all the values belong to our supported subset. It also adds the
normalization functions for our two headers, as well as utility
functions that can prebuild a secondary key for a given request and
transform it into an actual secondary key after the vary signature is
determined from the response.
2020-11-24 16:52:57 +01:00
Christopher Faulet
401e6dbff3 BUG/MAJOR: filters: Always keep all offsets up to date during data filtering
When at least one data filter is registered on a channel, the offsets of all
filters must be kept up to date. For data filters but also for others. It is
safer to do it in that way. Indirectly, this patch fixes 2 hidden bugs
revealed by the commit 22fca1f2c ("BUG/MEDIUM: filters: Forward all filtered
data at the end of http filtering").

The first one, the worst of both, happens at the end of http filtering when
at least one data filtered is registered on the channel. We call the
http_end() callback function on the filters, when defined, to finish the
http filtering. But it is performed for all filters. Before the commit
22fca1f2c, the only risk was to call the http_end() callback function
unexpectedly on a filter. Now, we may have an overflow on the offset
variable, used at the end to forward all filtered data. Of course, from the
moment we forward an arbitrary huge amount of data, all kinds of bad things
may happen. So offset computation is performed for all filters and
http_end() callback function is called only for data filters.

The other one happens when a data filter alter the data of a channel, it
must update the offsets of all previous filters. But the offset of non-data
filters must be up to date, otherwise, here too we may have an integer
overflow.

Another way to fix these bugs is to always ignore non-data filters from the
offsets computation. But this patch is safer and probably easier to
maintain.

This patch must be backported in all versions where the above commit is. So
as far as 2.0.
2020-11-24 14:17:32 +01:00
Joao Morais
aa8fcc4692 DOC: better describes how to configure a fallback crt
A default certificate is always the first one declared in the bind line,
either from `crt` or from `crt-line` option. This commit updates the
description of how to configure a fallback certificate, clarifying that
it needs to be the first one of the bind line.

Should be merged as far as the first SNI filter implementation.
2020-11-24 13:23:06 +01:00
Maciej Zdeb
6dee9969b9 BUG/MEDIUM: http_act: Restore init of log-format list
Restore init of log-format list in parse_http_del_header which was
accidently deleted by commit ebdd4c55da
(implementation of different header matching methods for
http-request/response del-header).

This is related to GitHub issue #909
2020-11-24 10:33:46 +01:00
Ilya Shipitsin
5bfe66366c BUILD: SSL: do not "update" BoringSSL version equivalent anymore
we have added all required fine guarding, no need to reduce
BoringSSL version back to 1.1.0 anymore, we do not depend on it
2020-11-24 09:54:44 +01:00
Ilya Shipitsin
d9a16dc0f2 BUILD: SSL: add BoringSSL guarding to "RAND_keep_random_devices_open"
"RAND_keep_random_devices_open" is OpenSSL specific, does not present
in other OpenSSL variants like LibreSSL or BoringSSL. BoringSSL recently
"updated" its internal openssl version to 1.1.1, we temporarily set it
back to 1.1.0, as we are going to remove that hack, let us add proper
guarding.
2020-11-24 09:54:44 +01:00
Ilya Shipitsin
f04a89c549 CLEANUP: remove unused function "ssl_sock_is_ckch_valid"
"ssl_sock_is_ckch_valid" is not used anymore, let us remove it
2020-11-24 09:54:44 +01:00
Julien Pivotto
2de240a676 MINOR: stream: Add level 7 retries on http error 401, 403
Level-7 retries are only possible with a restricted number of HTTP
return codes. While it is usually not safe to retry on 401 and 403, I
came up with an authentication backend which was not synchronizing
authentication of users. While not perfect, being allowed to also retry
on those return codes is really helpful and acts as a hotfix until we
can fix the backend.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
2020-11-23 09:33:14 +01:00
Tim Duesterhus
9fee7e02d1 CI: Set DEBUG=-DDEBUG_STRICT=1 in GitHub Actions
This was missing when migrating from Travis.
2020-11-21 18:27:33 +01:00
Tim Duesterhus
c8d19702f4 BUILD: Show the value of DEBUG= in haproxy -vv
Previously this was not visible after building.
2020-11-21 18:27:33 +01:00
Tim Duesterhus
81e948e051 BUILD: Make DEBUG part of .build_opts
This forces a recompilation if the value of DEBUG= changes.
2020-11-21 18:27:33 +01:00
Willy Tarreau
1a38ffcb0f [RELEASE] Released version 2.4-dev1
Released version 2.4-dev1 with the following main changes :
    - MINOR: ist: Add istend() function to return a pointer to the end of the string
    - MINOR: sample: Add converters to parse FIX messages
    - REGTEST: converter: Add a regtest for fix converters
    - MINOR: sample: Add converts to parses MQTT messages
    - REGTEST: converter: Add a regtest for MQTT converters
    - MINOR: compat: automatically include malloc.h on glibc
    - MEDIUM: pools: call malloc_trim() from pool_gc()
    - MEDIUM: pattern: call malloc_trim() on pat_ref_reload()
    - MINOR: pattern: move the update revision to the pat_ref, not the expression
    - CLEANUP: pattern: delete the back refs at once during pat_ref_reload()
    - MINOR: pattern: new sflag PAT_SF_REGFREE indicates regex_free() is needed
    - MINOR: pattern: make the delete and prune functions more generic
    - MEDIUM: pattern: link all final elements from the reference
    - MEDIUM: pattern: change the pat_del_* functions to delete from the references
    - MINOR: pattern: remerge the list and tree deletion functions
    - MINOR: pattern: perform a single call to pat_delete_gen() under the expression
    - CLEANUP: acl: don't reference the generic pattern deletion function anymore
    - CLEANUP: pattern: remove pat_delete_fcts[] and pattern_head->delete()
    - MINOR: pattern: introduce pat_ref_delete_by_ptr() to delete a valid reference
    - MINOR: pattern: store a generation number in the reference patterns
    - MEDIUM: pattern: only match patterns that match the current generation
    - MINOR: pattern: add pat_ref_commit() to commit a previously inserted element
    - MINOR: pattern: implement pat_ref_load() to load a pattern at a given generation
    - MINOR: pattern: add pat_ref_purge_older() to purge old entries
    - MEDIUM: pattern: make pat_ref_prune() rely on pat_ref_purge_older()
    - MINOR: pattern: during reload, delete elements frem the ref, not the expression
    - MINOR: pattern: prepare removal of a pattern from the list head
    - MEDIUM: pattern: turn the pattern chaining to single-linked list
    - CLEANUP: cfgparse: remove duplicate registration for transparent build options
    - BUG/MINOR: ssl: don't report 1024 bits DH param load error when it's higher
    - MINOR: http-htx: Add understandable errors for the errorfiles parsing
    - MINOR: ssl: instantiate stats module
    - MINOR: ssl: count client hello for stats
    - MINOR: ssl: add counters for ssl sessions
    - DOC: config: Fix a typo on ssl_c_chain_der
    - MINOR: server: remove idle lock in srv_cleanup_connections
    - BUILD: ssl: silence build warning on uninitialised counters
    - BUILD: http-htx: fix build warning regarding long type in printf
    - REGTEST: ssl: test wildcard and multi-type + exclusions
    - BUG/MEDIUM: ssl/crt-list: correctly insert crt-list line if crt already loaded
    - CI: Expand use of GitHub Actions for CI
    - REGTEST: ssl: mark reg-tests/ssl/ssl_crt-list_filters.vtc as broken
    - BUG/MINOR: pattern: a sample marked as const could be written
    - BUG/MINOR: lua: set buffer size during map lookups
    - MEDIUM: cache: Change caching conditions
    - BUG/MINOR: stats: free dynamically stats fields/lines on shutdown
    - BUG/MEDIUM: stats: prevent crash if counters not alloc with dummy one
    - MINOR: peers: Add traces to peer_treat_updatemsg().
    - BUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
    - BUG/MINOR: peers: Missing TX cache entries reset.
    - BUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
    - BUG/MINOR: http-fetch: Extract cookie value even when no cookie name
    - BUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
    - BUG/MEDIUM: check: reuse srv proto only if using same mode
    - MINOR: check: report error on incompatible proto
    - MINOR: check: report error on incompatible connect proto
    - BUG/MINOR: http-htx: Handle warnings when parsing http-error and http-errors
    - BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
    - MINOR: spoe: Don't close connection in sync mode on processing timeout
    - BUG/MINOR: tcpcheck: Don't warn on unused rules if check option is after
    - MINOR: init: Fix the prototype for per-thread free callbacks
    - MINOR: config/mux-h2: Return ERR_ flags from init_h2() instead of a status
    - CLEANUP: config: Return ERR_NONE from config callbacks instead of 0
    - MINOR: cfgparse: tighten the scope of newnameserver variable, free it on error.
    - REGTEST: make ssl_client_samples and ssl_server_samples require to 2.2
    - REGTESTS: Add sample_fetches/cook.vtc
    - BUG/MEDIUM: filters: Forward all filtered data at the end of http filtering
    - BUG/MINOR: http-ana: Don't wait for the body of CONNECT requests
    - CLEANUP: flt-trace: Remove unused random-parsing option
    - MINOR: flt-trace: Add an option to inhibits trace messages
    - MINOR: flt-trace: Use a bitfield for the trace options
    - REGTESTS: Add a script to test the random forwarding with several filters
    - REGTESTS: mark the abns test as broken again
    - REGTESTS: converter: add url_dec test
    - CI: Stop hijacking the hosts file
    - CI: Make the h2spec workflow more consistent with the VTest workflow
    - CI: travis-ci: remove amd64, osx builds
    - CI: travis-ci: arm64 are not allowed to fail anymore
    - DOC: add missing 3.10 in the summary
    - MINOR: ssl: remove client hello counters
    - MEDIUM: stats: add counters for failed handshake
    - MINOR: ssl: create common ssl_ctx init
    - MEDIUM: cli/ssl: configure ssl on server at runtime
    - REGTEST: server/cli_set_ssl.vtc requires OpenSSL
    - DOC: coding-style: update a few rules about pointers
    - BUG/MINOR: ssl: segv on startup when AKID but no keyid
    - BUILD: ssl: use SSL_MODE_ASYNC macro instead of OPENSSL_VERSION
    - BUG/MEDIUM: http-ana: Don't eval http-after-response ruleset on empty messages
    - BUG/MEDIUM: ssl/crt-list: bundle support broken in crt-list
    - BUG/MEDIUM: ssl: error when no certificate are found
    - BUG/MINOR: ssl/crt-list: load bundle in crt-list only if activated
    - BUG/MEDIUM: ssl/crt-list: fix error when no file found
    - CI: Github Actions: enable prometheus exporter
    - CI: Github Actions: remove LibreSSL-3.0.2 builds
    - CI: Github Actions: enable BoringSSL builds
    - CI: travis-ci: remove builds migrated to GH actions
    - BUILD: makefile: enable crypt(3) for OpenBSD
    - CI: Github Action: run "apt-get update" before packages restore
    - BUILD: SSL: guard TLS13 ciphersuites with HAVE_SSL_CTX_SET_CIPHERSUITES
    - CI: Pass the github.event_name to matrix.py
    - CI: Clean up Windows CI
    - DOC: clarify how to create a fallback crt
    - CLEANUP: connection: do not use conn->owner when the session is known
    - BUG/MAJOR: connection: reset conn->owner when detaching from session list
    - REGTESTS: mark proxy_protocol_random_fail as broken
    - BUG/MINOR: http_htx: Fix searching headers by substring
    - MINOR: http_act: Add -m flag for del-header name matching method
2020-11-21 16:00:40 +01:00
Maciej Zdeb
ebdd4c55da MINOR: http_act: Add -m flag for del-header name matching method
This patch adds -m flag which allows to specify header name
matching method when deleting headers from http request/response.
Currently beg, end, sub, str and reg are supported.

This is related to GitHub issue #909
2020-11-21 15:54:30 +01:00
Maciej Zdeb
302b9f8d7a BUG/MINOR: http_htx: Fix searching headers by substring
Function __http_find_header is used to search headers by name using specified
matching method. Matching by substring returned unexpected results due to wrong
length of substring supplied to strnistr function.

Fixed also the boolean condition by inverting it, as we're interested in
headers that contains the substring.

This patch should be backported as far as 2.2
2020-11-21 15:54:26 +01:00
Willy Tarreau
4137889911 REGTESTS: mark proxy_protocol_random_fail as broken
As indicated in issue #907 it very frequently fails on FreeBSD, and
looking at it, it's broken in multiple ways. It relies on log ordering
between two layers, the first one being allowed to support h2. Given
that on FreeBSD it usually ends up in timeouts, it's very likely that
for some reason one frontend logs before the other one or that curl
uses h2 instead of h1 there, and that the log instance waits forever
for its lines.

Usually it works fine when run locally though, so let's not remove it
and mark it as broken instead so that it can still be used when relevant.
2020-11-21 15:33:03 +01:00
Willy Tarreau
3aab17bd56 BUG/MAJOR: connection: reset conn->owner when detaching from session list
Baptiste reported a new crash affecting 2.3 which can be triggered
when using H2 on the backend, with http-reuse always and with a tens
of clients doing close only. There are a few combined cases which cause
this to happen, but each time the issue is the same, an already freed
session is dereferenced in session_unown_conn().

Two cases were identified to cause this:
  - a connection referencing a session as its owner, which is detached
    from the session's list and is destroyed after this session ends.
    The test on conn->owner before calling session_unown_conn() is not
    sufficent as the pointer is not null but is not valid anymore.

  - a connection that never goes idle and that gets killed form the
    mux, where session_free() is called first, then conn_free() calls
    session_unown_conn() which scans the just freed session for older
    connections. This one is only triggered with DEBUG_UAF

The reason for this session to be present here is that it's needed during
the connection setup, to be passed to conn_install_mux_be() to mux->init()
as the owning session, but it's never deleted aftrewards. Furthermore, even
conn_session_free() doesn't delete this pointer after freeing the session
that lies there. Both do definitely result in a use-after-free that's more
easily triggered under DEBUG_UAF.

This patch makes sure that the owner is always deleted after detaching
or killing the session. However it is currently not possible to clear
the owner right after a synchronous init because the proxy protocol
apparently needs it (a reg test checks this), and if we leave it past
the connection setup with the session not attached anywhere, it's hard
to catch the right moment to detach it. This means that the session may
remain in conn->owner as long as the connection has never been added to
nor removed from the session's idle list. Given that this patch needs to
remain simple enough to be backported, instead it adds a workaround in
session_unown_conn() to detect that the element is already not attached
anywhere.

This fix absolutely requires previous patch "CLEANUP: connection: do not
use conn->owner when the session is known" otherwise the situation will
be even worse, as some places used to rely on conn->owner instead of the
session.

The fix could theorically be backported as far as 1.8. However, the code
in this area has significantly changed along versions and there are more
risks of breaking working stuff than fixing real issues there. The issue
was really woken up in two steps during 2.3-dev when slightly reworking
the idle conns with commit 08016ab82 ("MEDIUM: connection: Add private
connections synchronously in session server list") and when adding
support for storing used H2 connections in the session and adding the
necessary call to session_unown_conn() in the muxes. But the same test
managed to crash 2.2 when built in DEBUG_UAF and patched like this,
proving that we used to already leave dangling pointers behind us:

|  diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h
|  index f8f235c1a..dd30b5f80 100644
|  --- a/include/haproxy/connection.h
|  +++ b/include/haproxy/connection.h
|  @@ -458,6 +458,10 @@ static inline void conn_free(struct connection *conn)
|                          sess->idle_conns--;
|                  session_unown_conn(sess, conn);
|          }
|  +       else {
|  +               struct session *sess = conn->owner;
|  +               BUG_ON(sess && sess->origin != &conn->obj_type);
|  +       }
|
|          sockaddr_free(&conn->src);
|          sockaddr_free(&conn->dst);

It's uncertain whether an existing code path there can lead to dereferencing
conn->owner when it's bad, though certain suspicious memory corruption bugs
make one think it's a likely candidate. The patch should not be hard to
adapt there.

Backports to 2.1 and older are left to the appreciation of the person
doing the backport.

A reproducer consists in this:

  global
    nbthread 1

  listen l
    bind :9000
    mode http
    http-reuse always
    server s 127.0.0.1:8999 proto h2

  frontend f
    bind :8999 proto h2
    mode http
    http-request return status 200

Then this will make it crash within 2-3 seconds:

  $ h1load -e -r 1 -c 10 http://0:9000/

If it does not, it might be that DEBUG_UAF was not used (it's harder then)
and it might be useful to restart.
2020-11-21 15:29:22 +01:00
Willy Tarreau
38b4d2eb22 CLEANUP: connection: do not use conn->owner when the session is known
At a few places we used to rely on conn->owner to retrieve the session
while the session is already known. This is not correct because at some
of these points the reason the connection's owner was still the session
(instead of NULL) is a mistake. At one place a comparison is even made
between the session and conn->owner assuming it's valid without checking
if it's NULL. Let's clean this up to use the session all the time.

Note that this will be needed for a forthcoming fix and will have to be
backported.
2020-11-21 15:29:22 +01:00
Joao Morais
e51fab0a4a DOC: clarify how to create a fallback crt
HAProxy uses CN and SAN of the certificates to match incoming SNI, and
use the matching certificate in the TLS handshake. `crt-list` goes
further and allows to configure SNI filters to explicitly define the
FQDNs that should match a certificate.

The first declared certificate of the `crt-list` option follows the same
rules, and it's also used as a fallback - the certificate that should be
used if SNI isn't provided or the provided one cannot match any
certificate or SNI filter. If a provided SNI matches the CN or SAN of
the first certificate, the first certificate would be used even if a
matching SNI filter is declared later.

This change clarifies this scenario and documents a filter that can be
used to convert the first declared certificate as a proper fallback.

Should be merged as far as the first SNI filter implementation.
2020-11-21 15:29:22 +01:00
Tim Duesterhus
ed54c3baa5 CI: Clean up Windows CI
This patch cleans up the Windows CI to look more similar to the refactored
Linux CI on GitHub Actions.

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

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

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

The only drawback is that Lua does not appear to be readily available. I expect
this to be acceptable and that the benefits far outweight this small drawback.
2020-11-21 11:05:16 +01:00
Tim Duesterhus
8d173e17c0 CI: Pass the github.event_name to matrix.py
This is a preparation to later run some matrix entries on schedule only.

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

    if build_type == "schedule":
        matrix.append(...)
2020-11-21 11:05:16 +01:00
Ilya Shipitsin
f34ed0b74c BUILD: SSL: guard TLS13 ciphersuites with HAVE_SSL_CTX_SET_CIPHERSUITES
HAVE_SSL_CTX_SET_CIPHERSUITES is newly defined macro set in openssl-compat.h,
which helps to identify ssl libs (currently OpenSSL-1.1.1 only) that supports
TLS13 cipersuites manipulation on TLS13 context
2020-11-21 11:04:36 +01:00
Ilya Shipitsin
e48853aaf4 CI: Github Action: run "apt-get update" before packages restore
ubuntu somehow needs it, no idea why does not apt-get do it itself.
2020-11-21 10:44:16 +01:00
Matthieu Guegan
496374e592 BUILD: makefile: enable crypt(3) for OpenBSD
Allow OpenBSD to support encrypted passwords in Userlists.

OpenBSD's crypt(3) function is provided directly by libc and does not
require -lcrypt.
Signed-off-by: Matthieu Guegan <matthieu.guegan@deindeal.ch>
2020-11-21 05:45:05 +01:00
Ilya Shipitsin
d394fd35e7 CI: travis-ci: remove builds migrated to GH actions 2020-11-21 05:40:27 +01:00
Ilya Shipitsin
f644c5a95f CI: Github Actions: enable BoringSSL builds 2020-11-21 05:40:27 +01:00
Ilya Shipitsin
cf67a12598 CI: Github Actions: remove LibreSSL-3.0.2 builds 2020-11-21 05:40:27 +01:00
Ilya Shipitsin
69c10f1d50 CI: Github Actions: enable prometheus exporter 2020-11-21 05:40:27 +01:00
William Lallemand
77e1c6fb0a BUG/MEDIUM: ssl/crt-list: fix error when no file found
When a file from a crt-list was not found, this one was ignored silently
letting HAProxy starts without it.

This bug was introduced by 47da821 ("MEDIUM: ssl: emulates the
multi-cert bundles in the crtlist").

This commit adds a found variable which is checked once we tried every
bundle combination so we can exits with an error if none were found.

Must be backported in 2.3.
2020-11-20 18:38:56 +01:00
William Lallemand
7340457158 BUG/MINOR: ssl/crt-list: load bundle in crt-list only if activated
Don't try to load a bundle from a crt-list if the bundle support was
disabled with ssl-load-extra-files.

Must be backported to 2.3.
2020-11-20 18:38:56 +01:00
William Lallemand
06ce84a100 BUG/MEDIUM: ssl: error when no certificate are found
When a non-existing file was specified in the configuration, haproxy
does not exits with an error which is not normal.

This bug was introduced by dfa93be ("MEDIUM: ssl: emulate multi-cert
bundles loading in standard loading") which does nothing if the stat
failed.

This patch introduce a "found" variable which is checked at the end of
the function so we exit with an error if no find were found.

Must be backported to 2.3.
2020-11-20 18:38:56 +01:00
William Lallemand
86c2dd60f1 BUG/MEDIUM: ssl/crt-list: bundle support broken in crt-list
In issue #970 it was reported that the bundle loading does not work
anymore with crt-list.

This bug was introduced by 47da821 ("MEDIUM: ssl: emulates the
multi-cert bundles in the crtlist") which incorrectly uses "path"
instead of "crt_path" in the name resolution.

Must be backported to 2.3.
2020-11-20 18:38:51 +01:00
Christopher Faulet
aab1b67383 BUG/MEDIUM: http-ana: Don't eval http-after-response ruleset on empty messages
It is not possible on response comming from a server, but an errorfile may be
empty. In this case, the http-after-response ruleset must not be evaluated
because it is totally unexpected to manipulate headers on an empty HTX message.

This patch must be backported everywhere the http-after-response rules are
supported, i.e as far as 2.2.
2020-11-20 09:43:31 +01:00
Ilya Shipitsin
bdec3ba796 BUILD: ssl: use SSL_MODE_ASYNC macro instead of OPENSSL_VERSION 2020-11-19 19:59:32 +01:00