Commit Graph

13159 Commits

Author SHA1 Message Date
Christopher Faulet
c300747dec REGTEST: make ssl_client_samples and ssl_server_samples require to 2.2
Some missing sample fetches was backported to 2.2 making these tests compatible
with the 2.2.
2020-11-13 17:12:30 +01:00
Eric Salama
9139ec34ed MINOR: cfgparse: tighten the scope of newnameserver variable, free it on error.
This should fix issue GH #931.

Also remove a misleading comment.

This commit can be backported as far as 1.9
2020-11-13 16:26:10 +01:00
Christopher Faulet
fc633b6eff CLEANUP: config: Return ERR_NONE from config callbacks instead of 0
Return ERR_NONE instead of 0 on success for all config callbacks that should
return ERR_* codes. There is no change because ERR_NONE is a macro equals to
0. But this makes the return value more explicit.
2020-11-13 16:26:10 +01:00
Christopher Faulet
5214099233 MINOR: config/mux-h2: Return ERR_ flags from init_h2() instead of a status
post-check function callbacks must return ERR_* flags. Thus, init_h2() is fixed
to return ERR_NONE on success or (ERR_ALERT|ERR_FATAL) on error.

This patch may be backported as far as 2.2.
2020-11-13 16:26:10 +01:00
Christopher Faulet
83fefbcdff MINOR: init: Fix the prototype for per-thread free callbacks
Functions registered to release memory per-thread have no return value. But the
registering function and the function pointer in per_thread_free_fct structure
specify it should return an integer. This patch fixes it.

This patch may be backported as far as 2.0.
2020-11-13 16:26:10 +01:00
Christopher Faulet
c751b4508d BUG/MINOR: tcpcheck: Don't warn on unused rules if check option is after
When tcp-check or http-check rules are used, if the corresponding check option
(option tcp-check and option httpchk) is declared after the ruleset, a warning
is emitted about an unused check ruleset while there is no problem in reality.

This patch must be backported as far as 2.2.
2020-11-13 16:26:10 +01:00
Christopher Faulet
c7ba91039a MINOR: spoe: Don't close connection in sync mode on processing timeout
In sync mode, if an applet receives a ack while the processing delay has already
expired, there is not frame waiting for this ack. But there is no reason to
close the connection in this case. The ack may be ignored and the connection may
be reused to process another frame. The only reason to trigger an error and
close the connection is when the wrong ack is received while there is still a
frame waiting for its ack. In sync mode, this should never happen.

This patch may be backported in all versions supporting the SPOE.
2020-11-13 16:26:10 +01:00
Christopher Faulet
cf181c76e3 BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
When a SPOE applet is used to send a frame, a reference on this applet is saved
in the spoe context of the offladed stream. But, if the applet is released
before receving the corresponding ack, we must be sure to remove this
reference. This was performed for fragmented frames only. But it must also be
performed for a spoe contexts in the applet waiting_queue and in the thread
waiting_queue (used in async mode).

This bug leads to a memory corruption when an offloaded stream try to update the
state of a released applet because it still have a reference on it. There are
many ways to trigger this bug. The easiest is probably during reloads. On the
old process, all applets are woken up to be released ASAP.

Many thanks to Maciej Zdeb to report the bug and to work on it for 2
months. Without his help, it would have been much more difficult to fix the
bug. It is always a huge pleasure to see how some users are enthousiast and
helpful. Thanks again Maciej !

This patch must be backported to all versions where the spoe is supported (>=
1.7).
2020-11-13 16:26:10 +01:00
Christopher Faulet
3005d28eb8 BUG/MINOR: http-htx: Handle warnings when parsing http-error and http-errors
First of all, this patch is tagged as a bug. But in fact, it only fixes a bug in
the 2.2. On the 2.3 and above, it only add the ability to display warnings, when
an http-error directive is parsed from a proxy section and when an errorfile
directive is parsed from a http-errors section.

But on the 2.2, it make sure to display the warning emitted on a content-length
mismatch when an errorfile is parsed. The following is only applicable to the
2.2.

commit "BUG/MINOR: http-htx: Just warn if payload of an errorfile doesn't match
the C-L" (which is only present in 2.2, 2.1 and 2.0 trees, i.e see commit
7bf3d81d3cf4b9f4587 in 2.2 tree), is changing the behavior of `http_str_to_htx`
function. It may now emit warnings. And, it is the caller responsibility to
display it.

But the warning is missing when an 'http-error' directive is parsed from
a proxy section. It is also missing when an 'errorfile' directive is
parsed from a http-errors section.

This bug only exists on the 2.2. On earlier versions, these directives
are not supported and on later ones, an error is triggered instead of a
warning.

Thanks to William Dauchy that spotted the bug.

This patch must be backported as far as 2.2.
2020-11-13 16:26:10 +01:00
Amaury Denoyelle
90eb93f792 MINOR: check: report error on incompatible connect proto
Report an error when using an explicit proto for a connect rule with
non-compatible mode in regards with the selected check type (tcp-check
vs http-check).
2020-11-13 16:26:10 +01:00
Amaury Denoyelle
7c14890183 MINOR: check: report error on incompatible proto
If the check mux has been explicitly defined but is incompatible with
the selected check type (tcp-check vs http-check), report a warning and
prevent haproxy startup.
2020-11-13 16:26:10 +01:00
Amaury Denoyelle
0519bd4d04 BUG/MEDIUM: check: reuse srv proto only if using same mode
Only reuse the mux from server if the check is using the same mode.
For example, this prevents a tcp-check on a h2 server to select the h2
multiplexer instead of passthrough.

This bug was introduced by the following commit :
BUG/MEDIUM: checks: Use the mux protocol specified on the server line
It must be backported up to 2.2.

Fixes github issue #945.
2020-11-13 16:26:10 +01:00
Christopher Faulet
97fc8da264 BUG/MINOR: http-fetch: Fix calls w/o parentheses of the cookie sample fetches
req.cook, req.cook_val, req.cook_cnt and and their response counterparts may be
called without cookie name. In this case, empty parentheses may be used, or no
parentheses at all. In both, the result must be the same. But only the first one
works. The second one always returns a failure. This patch fixes this bug.

Note that on old versions (< 2.2), both cases fail.

This patch must be backported in all stable versions.
2020-11-13 16:26:10 +01:00
Maciej Zdeb
dea7c209f8 BUG/MINOR: http-fetch: Extract cookie value even when no cookie name
HTTP sample fetches dealing with the cookies (req/res.cook,
req/res.cook_val and req/res.cook_cnt) must be prepared to be called
without cookie name. For the first two, the first cookie value is
returned, regardless its name. For the last one, all cookies are counted.

To do so, http_extract_cookie_value() may now be called with no cookie
name (cookie_name_l set to 0). In this case, the matching on the cookie
name is ignored and the first value found is returned.

Note this patch also fixes matching on cookie values in ACLs.

This should be backported in all stable versions.
2020-11-13 16:26:10 +01:00
Willy Tarreau
1dfd4f106f BUG/MEDIUM: peers: fix decoding of multi-byte length in stick-table messages
There is a bug in peer_recv_msg() due to an incorrect cast when trying
to decode the varint length of a stick-table message, causing lengths
comprised between 128 and 255 to consume one extra byte, ending in
protocol errors.  The root cause of this is that peer_recv_msg() tries
hard to reimplement all the parsing and control that is already done in
intdecode() just to measure the length before calling it. And it got it
wrong.

Let's just get rid of this unneeded code duplication and solely rely on
intdecode() instead. The bug was introduced in 2.0 as part of a cleanup
pass on this code with commit 95203f218 ("MINOR: peers: Move high level
receive code to reduce the size of I/O handler."), so this patch must
be backported to 2.0.

Thanks to Yves Lafon for reporting the problem.
2020-11-13 15:21:50 +01:00
Frdric Lcaille
ea875e62e6 BUG/MINOR: peers: Missing TX cache entries reset.
The TX part of a cache for a dictionary is made of an reserved array of ebtree nodes
which are pointers to dictionary entries. So when we flush the TX part of such a
cache, we must not only remove these nodes to dictionary entries from their ebtree.
We must also reset their values. Furthermore, the LRU key and the last lookup
result must also be reset.
2020-11-13 06:04:18 +01:00
Frdric Lcaille
f9e51beec1 BUG/MINOR: peers: Do not ignore a protocol error for dictionary entries.
If we could not decode the ID of a dictionary entry from a peer update message,
we must inform the remote peer about such an error as this is done for
any other decoding error.
2020-11-13 06:04:08 +01:00
Frdric Lcaille
d865935f32 MINOR: peers: Add traces to peer_treat_updatemsg().
Add minimalistic traces for peers with only one event to diagnose potential
issues when decode peer update messages.
2020-11-12 17:38:49 +01:00
Amaury Denoyelle
7f8f6cb926 BUG/MEDIUM: stats: prevent crash if counters not alloc with dummy one
Define a per-thread counters allocated with the greatest size of any
stat module counters. This variable is named trash_counters.

When using a proxy without allocated counters, return the trash counters
from EXTRA_COUNTERS_GET instead of a dangling pointer to prevent
segfault.

This is useful for all the proxies used internally and not
belonging to the global proxy list. As these objects does not appears on
the stat report, it does not matter to use the dummy counters.

For this fix to be functional, the extra counters are explicitly
initialized to NULL on proxy/server/listener init functions.

Most notably, the crash has already been detected with the following
vtc:
- reg-tests/lua/txn_get_priv.vtc
- reg-tests/peers/tls_basic_sync.vtc
- reg-tests/peers/tls_basic_sync_wo_stkt_backend.vtc
There is probably other parts that may be impacted (SPOE for example).

This bug was introduced in the current release and do not need to be
backported. The faulty commits are
"MINOR: ssl: count client hello for stats" and
"MINOR: ssl: add counters for ssl sessions".
2020-11-12 15:16:05 +01:00
Amaury Denoyelle
a2a6899bee BUG/MINOR: stats: free dynamically stats fields/lines on shutdown
Register a new function on POST DEINIT to free stats fields/lines for
each domain.

This patch does not fix a critical bug but may be backported to 2.3.
2020-11-12 15:16:05 +01:00
Remi Tricot-Le Breton
cc9bf2e5fe MEDIUM: cache: Change caching conditions
Do not cache responses that do not have an explicit expiration time
(s-maxage or max-age Cache-Control directives or Expires header) or a
validator (ETag or Last-Modified headers) anymore, as suggested in
RFC 7234#3.
The TX_FLAG_IGNORE flag is used instead of the TX_FLAG_CACHEABLE so as
not to change the behavior of the checkcache option.
2020-11-12 11:22:05 +01:00
Thierry Fournier
91dc0c0d8f BUG/MINOR: lua: set buffer size during map lookups
This size is used by some pattern matching to determine if there
is sufficient room in the buffer to add final \0 if necessary.
If the size is not set, the conditions use uninitialized value.

Note: it seems this bug can't cause a crash.

Should be backported until 2.2 (at least)
2020-11-11 10:43:21 +01:00
Thierry Fournier
a68affeaa9 BUG/MINOR: pattern: a sample marked as const could be written
The functions add final 0 to string if the final 0 is not set,
but don't check the flag CONST. This patch duplicates the strings
if the final zero is not set and the string is CONST.

Should be backported until 2.2 (at least)
2020-11-11 10:43:15 +01:00
William Lallemand
8f04e1849d REGTEST: ssl: mark reg-tests/ssl/ssl_crt-list_filters.vtc as broken
This regtest requires a version of OpenSSL which supports the
ClientHello callback which is only the case of recents SSL libraries
(openssl 1.1.1).

This was reported in issue #944.
2020-11-10 22:40:24 +01:00
Tim Duesterhus
288c0772e6 CI: Expand use of GitHub Actions for CI
Travis is becoming overall increasingly unreliable lately. We've already
seen that the timing sensitive tests regularly fail and thus they were
disabled.

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

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

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

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

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

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

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

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

Known issues:
- Apparently the git commit is not properly detected during build. The HEAD
  currently shows as 2.4-dev0.
2020-11-10 10:15:17 +01:00
William Lallemand
50c03aac04 BUG/MEDIUM: ssl/crt-list: correctly insert crt-list line if crt already loaded
In issue #940, it was reported that the crt-list does not work correctly
anymore. Indeed when inserting a crt-list line which use a certificate
previously seen in the crt-list, this one won't be inserted in the SNI
list and will be silently ignored.

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

This patch also includes a reg-test which tests this issue.

This bugfix must be backported in 2.3.
2020-11-06 16:39:39 +01:00
William Lallemand
3ff9591ea2 REGTEST: ssl: test wildcard and multi-type + exclusions
This test checks that the bug #818 and #810 are fixed.

It test if there is no inconsistency with multiple certificate types and
that the exclusion of the certificate is correctly working with a negative
filter.
2020-11-06 14:59:36 +01:00
Willy Tarreau
431a12cafe BUILD: http-htx: fix build warning regarding long type in printf
Commit a66adf41e ("MINOR: http-htx: Add understandable errors for the
errorfiles parsing") added a warning when loading malformed error files,
but this warning may trigger another build warning due to the %lu format
used. Let's simply cast it for output since it's just used for end user
output.

This must be backported to 2.0 like the commit above.
2020-11-06 14:24:02 +01:00
Willy Tarreau
4299528390 BUILD: ssl: silence build warning on uninitialised counters
Since commit d0447a7c3 ("MINOR: ssl: add counters for ssl sessions"),
gcc 9+ complains about this:

  CC      src/ssl_sock.o
src/ssl_sock.c: In function 'ssl_sock_io_cb':
src/ssl_sock.c:5416:3: warning: 'counters_px' may be used uninitialized in this function [-Wmaybe-uninitialized]
 5416 |   ++counters_px->reused_sess;
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/ssl_sock.c:5133:23: note: 'counters_px' was declared here
 5133 |  struct ssl_counters *counters, *counters_px;
      |                                  ^~~~~~~~~~~

Either a listener or a server are expected there, so ther counters are
always initialized and the compiler cannot know this. Let's preset
them and test before updating the counter, we're not in a hot path
here.

No backport is needed.
2020-11-06 13:22:44 +01:00
Willy Tarreau
f5fe70620c MINOR: server: remove idle lock in srv_cleanup_connections
This function used to grab the idle lock when scanning the threads for
idle connections, but it doesn't need it since the lock only protects
the tree. Let's remove it.
2020-11-06 13:22:44 +01:00
Christopher Faulet
70d10d1fb6 DOC: config: Fix a typo on ssl_c_chain_der
There is a typo on the ssl_c_chain_der sample fetch
(s/ssl_c_der_chain/ssl_c_chain_der/). This implies a move of the fetch to keep
it at the right place.

This should be backported as far as 2.2 or anywhere the commit a598b500b
("MINOR: ssl: add ssl_{c,s}_chain_der fetch methods") is.
2020-11-06 12:20:04 +01:00
Amaury Denoyelle
d0447a7c3e MINOR: ssl: add counters for ssl sessions
Add counters for newly established and resumed sessions.
2020-11-06 12:05:17 +01:00
Amaury Denoyelle
fbc3377cd4 MINOR: ssl: count client hello for stats
Add a counter for ssl client_hello received on frontends.
2020-11-06 12:05:17 +01:00
Amaury Denoyelle
9963fa74d2 MINOR: ssl: instantiate stats module
This module is responsible for providing statistics for ssl. It allocates
counters for frontend/backend/listener/server objects.
2020-11-06 12:05:17 +01:00
Christopher Faulet
a66adf41ea MINOR: http-htx: Add understandable errors for the errorfiles parsing
No details are provided when an error occurs during the parsing of an errorfile,
Thus it is a bit hard to diagnose where the problem is. Now, when it happens, an
understandable error message is reported.

This patch is not a bug fix in itself. But it will be required to change an
fatal error into a warning in last stable releases. Thus it must be backported
as far as 2.0.
2020-11-06 09:13:58 +01:00
Willy Tarreau
6d27a92b83 BUG/MINOR: ssl: don't report 1024 bits DH param load error when it's higher
The default dh_param value is 2048 and it's preset to zero unless explicitly
set, so we must not report a warning about DH param not being loadble in 1024
bits when we're going to use 2048. Thanks to Dinko for reporting this.

This should be backported to 2.2.
2020-11-05 19:40:14 +01:00
Jerome Magnin
eff2e0a958 CLEANUP: cfgparse: remove duplicate registration for transparent build options
Since commit 37bafdcbb ("MINOR: sock_inet: move the IPv4/v6 transparent mode code
to sock_inet"), build options for transparent proxying are registered twice.
This patch removes the older one.
2020-11-05 19:27:16 +01:00
Willy Tarreau
38d41996c1 MEDIUM: pattern: turn the pattern chaining to single-linked list
It does not require heavy deletion from the expr anymore, so we can now
turn this to a single-linked list since most of the time we want to delete
all instances of a given pattern from the head. By doing so we save 32 bytes
of memory per pattern. The pat_unlink_from_head() function was adjusted
accordingly.
2020-11-05 19:27:09 +01:00
Willy Tarreau
867a8a5a10 MINOR: pattern: prepare removal of a pattern from the list head
Instead of using LIST_DEL() on the pattern itself inside an expression,
we look it up from its head. The goal is to get rid of the double-linked
list while this usage remains exclusively for freeing on startup error!
2020-11-05 19:27:09 +01:00
Willy Tarreau
2817472bb0 MINOR: pattern: during reload, delete elements frem the ref, not the expression
Instead of scanning all elements from the expression and using the
slow delete path there, let's use the faster way which involves
pat_delete_gen() while the elements are detached from ther reference.
2020-11-05 19:27:09 +01:00
Willy Tarreau
ae83e63b48 MEDIUM: pattern: make pat_ref_prune() rely on pat_ref_purge_older()
When purging all of a reference, it's much more efficient to scan the
reference patterns from the reference head and delete all derivative
patterns than to scan the expressions. The only thing is that we need
to proceed both for the current and next generations, in case there is
a huge gap between the two. With this, purging 20M IP addresses in small
batches of 100 takes roughly 3 seconds.
2020-11-05 19:27:09 +01:00
Willy Tarreau
94b9abe200 MINOR: pattern: add pat_ref_purge_older() to purge old entries
This function will be usable to purge at most a specified number of old
entries from a reference. Entries are declared old if their generation
number is in the past compared to the one passed in argument. This will
ease removal of early entries when new ones have been appended.

We also call malloc_trim() when available, at the end of the series,
because this is one place where there is a lot of memory to save. Reloads
of 1M IP addresses used in an ACL made the process grow up to 1.7 GB RSS
after 10 reloads and roughly stabilize there without this call, versus
only 260 MB when the call is present. Sadly there is no direct equivalent
for jemalloc, which stabilizes around 800MB-1GB.
2020-11-05 19:27:09 +01:00
Willy Tarreau
1a6857b9c1 MINOR: pattern: implement pat_ref_load() to load a pattern at a given generation
pat_ref_load() basically combines pat_ref_append() and pat_ref_commit().
It's very similar to pat_ref_add() except that it also allows to set the
generation ID and the line number. pat_ref_add() was modified to directly
rely on it to avoid code duplication. Note that a previous declaration
of pat_ref_load() was removed as it was just a leftover of an earlier
incarnation of something possibly similar, so no existing functionality
was changed here.
2020-11-05 19:27:09 +01:00
Willy Tarreau
0439e5eeb4 MINOR: pattern: add pat_ref_commit() to commit a previously inserted element
This function will be used after a successful pat_ref_append() to propagate
the pattern to all use places (including parsing and indexing). On failure,
it will entirely roll back all insertions and free the pattern itself. It
also preserves the generation number so that it is convenient for use in
association with pat_ref_append(). pat_ref_add() was modified to rely on
it instead of open-coding the insertion and roll-back.
2020-11-05 19:27:09 +01:00
Willy Tarreau
c93da6950e MEDIUM: pattern: only match patterns that match the current generation
Instead of matching any pattern found in the tree, only match those
matching the current generation of entries. This will make sure that
reloads are atomic, regardless of the time they take to complete, and
that newly added data are not matched until the whole reference is
committed. For consistency we proceed the same way on "show map" and
"show acl".

This will have no impact for now since generations are not used.
2020-11-05 19:27:09 +01:00
Willy Tarreau
29947745b5 MINOR: pattern: store a generation number in the reference patterns
Right now it's not possible to perform a safe reload because we don't
know what patterns were recently added or were already present. This
patch adds a generation counter to the reference patterns so that it
is possible to know what generation of the reference they were loaded
with. A reference now has two generations, the current one, used for
all additions, and the next one, allocated to those wishing to update
the contents. The generation wraps at 2^32 so comparisons must be made
relative to the current position.

The idea will be that upon full reload, the caller will first get a new
generation ID, will insert all new patterns using it, will then switch
the current ID to the new one, and will delete all entries older than
the current ID. This has the benefit of supporting chunked updates that
remain consistent and that won't block the whole process for ages like
pat_ref_reload() currently does.
2020-11-05 19:27:09 +01:00
Willy Tarreau
1fd52f70e5 MINOR: pattern: introduce pat_ref_delete_by_ptr() to delete a valid reference
Till now the only way to remove a known reference was via
pat_ref_delete_by_id() which scans the whole list to find a matching pointer.
Let's add pat_ref_delete_by_ptr() which takes a valid pointer. It can be
called by the function above after the pointer is found, and can also be
used to roll back a failed insertion much more efficiently.
2020-11-05 19:27:09 +01:00
Willy Tarreau
a98b2882ac CLEANUP: pattern: remove pat_delete_fcts[] and pattern_head->delete()
These ones are not used anymore, so let's remove them to remove a bit
of the complexity. The ACL keyword's delete() function could be removed
as well, though most keyword declarations are positional and we have a
high risk of introducing a mistake here, so let's not touch the ACL part.
2020-11-05 19:27:09 +01:00
Willy Tarreau
b35aa9b256 CLEANUP: acl: don't reference the generic pattern deletion function anymore
A few ACL keyword used to reference pat_delete_gen() as the deletion
function but this is not needed since it's the default one now. Let's
just remove this reference.
2020-11-05 19:27:09 +01:00
Willy Tarreau
e828d8f0e8 MINOR: pattern: perform a single call to pat_delete_gen() under the expression
When we're removing an element under the expression lock, we don't need
anymore to run over all ->delete() functions via the expressions, since
we know that the single function does it fine now. Note that at this
point, pattern->delete() is not used at all through out the code anymore.
2020-11-05 19:27:09 +01:00