Commit Graph

13842 Commits

Author SHA1 Message Date
Christopher Faulet 469676423e CLEANUP: server: Remove useless "filepath" variable in apply_server_state()
This variable is now only used to point on the local server-state file. When
the server-state is global, it is unused. So, we now use "localfilepath"
instead. Thus, the "filepath" variable can safely be removed.
2021-02-12 16:42:00 +01:00
Christopher Faulet 8952ea636b BUG/MINOR: server: Don't call fopen() with server-state filepath set to NULL
When a local server-state file is loaded, if its name is too long, the error
is not properly handled, resulting to a call to fopen() with the "filepath"
variable set to NULL. To fix the bug, when this error occurs, we jump to the
next proxy, via a "continue" statement. And we take case to set "filepath"
variable after the error handling to be sure.

This patch should fix the issue #1111. It must be backported as far as 1.6.
2021-02-12 16:42:00 +01:00
Christopher Faulet b1d19eab1c CLEANUP: tcpcheck: Remove a useless test on port variable
When a connect rule is evaluated a test is performed on the "port" variable
while it is set to 0 just on the line just above. Just remove this useless
test to make ccpcheck happy.

This patch fixes the issue #1113.
2021-02-12 16:42:00 +01:00
Yves Lafon b4d3708cb7 MINOR: http: add baseq sample fetch
Symetrical to path/pathq, baseq returns the concatenation of
the Host header and the path including the query string.
2021-02-12 16:38:50 +01:00
Willy Tarreau 7c0b4d861e MEDIUM: cfgparse: allow a proxy to designate the defaults section to use
Now it becomes possible to specify "from foo" on a frontend/listen/backend
or even on a "defaults" line, to mention that defaults section "foo" needs
to be used to preset the proxy's settings.

When not set, the last section remains used. In case the designated name
is found at multiple places, it is rejected and an error indicates two
occurrences of the same name. Similarly, if the section name is found,
its name must only use valid characters. This allows multiple named
defaults section to continue to coexist without the risk that they will
cause trouble by accident.

When it comes to "defaults" relying on another defaults, what happens is
just that a new defaults section is created from the designated one. This
will make it possible for example to reuse some settings such as log-format
like below:

    defaults tcp-clear
        log stdout local0 info
        log-format "%ci:%cp/%b/%si:%sp %ST %ts %U/%B %{+Q}r"

    defaults tcp-ssl
        log stdout local0 info
        log-format "%ci:%cp/%b/%si:%sp %ST %ts %U/%B %{+Q}r ssl=%sslv"

    defaults http-clear from tcp-clear
        mode http

    defaults http-ssl from tcp-ssl
        mode http

    frontend fe1 from http-clear
        bind :8001

    frontend fe2 from http-ssl
        bind :8002

A small corner case remains in the error detection, if a second defaults
section appears with the same name after the point where it was used, and
nobody references it, the duplicate will not be detected. This could be
addressed by performing the syntactic checks in check_config_validity(),
and by postponing the freeing of the defaults, after tagging a defaults
section as explicitly looked up by another section. This doesn't seem
that important at the moment though.
2021-02-12 16:23:46 +01:00
Willy Tarreau e90904d5a9 MEDIUM: proxy: store the default proxies in a tree by name
Now default proxies are stored into a dedicated tree, sorted by name.
Only unnamed entries are not kept upon new section creation. The very
first call to cfg_parse_listen() will automatically allocate a dummy
defaults section which corresponds to the previous static one, since
the code requires to have one at a few places.

The first immediately visible benefit is that it allows to reuse
alloc_new_proxy() to allocate a defaults section instead of doing it by
hand. And the secret goal is to allow to keep multiple named defaults
section in memory to reuse them from various proxies.
2021-02-12 16:23:46 +01:00
Willy Tarreau 0a0f6a7e4f MINOR: proxy: support storing defaults sections into their own tree
Now we'll have a tree of named defaults sections. The regular insertion
and lookup functions take care of the capability in order to select the
appropriate tree. A new function proxy_destroy_defaults() removes a
proxy from this tree and frees it entirely.
2021-02-12 16:23:46 +01:00
Willy Tarreau c02ab03142 MINOR: proxy: also store the name for a defaults section
There's an optional name, but till now it was not even saved into the
structure, let's keep it.
2021-02-12 16:23:46 +01:00
Willy Tarreau ab3410c65d MINOR: cfgparse: use a pointer to the current default proxy
In order to make the default proxy configurable, we'll need to have a
pointer to it which might differ from &defproxy. cfg_parse_listen()
now gets curr_defproxy for this.
2021-02-12 16:23:46 +01:00
Willy Tarreau 5d095c2fac MINOR: cfgparse: check PR_CAP_DEF instead of comparing poiner against defproxy
We want to get rid of this defproxy, let's now simply check the proxy's
capabilities instead of comparing its pointer to the known default one.
2021-02-12 16:23:46 +01:00
Willy Tarreau 80dc6fea59 MINOR: proxy: add a new capability PR_CAP_DEF
In order to more easily distinguish a default proxy from a standard one,
let's introduce a new capability PR_CAP_DEF.
2021-02-12 16:23:46 +01:00
Willy Tarreau 7d0c143185 MINOR: cfgparse: move defproxy to cfgparse-listen as a static
We don't want to expose this one anymore as we'll soon keep multiple
default proxies. Let's move it inside the parser which is the only
place which still uses it, and initialize it on the fly once needed
instead of doing it at boot time.
2021-02-12 16:23:46 +01:00
Willy Tarreau bb8669ae28 BUG/MINOR: server: parse_server() must take a const for the defproxy
The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.
2021-02-12 16:23:46 +01:00
Willy Tarreau 54fa7e332a BUG/MINOR: tcpcheck: proxy_parse_*check*() must take a const for the defproxy
The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.
2021-02-12 16:23:46 +01:00
Willy Tarreau 220fd70694 BUG/MINOR: extcheck: proxy_parse_extcheck() must take a const for the defproxy
The default proxy was passed as a variable, which in addition to being
a PITA to deal with in the config parser, doesn't feel safe to use when
it ought to be const.

This will only affect new code so no backport is needed.
2021-02-12 16:23:46 +01:00
Willy Tarreau 818ec78af8 MINOR: proxy: always properly reset the just freed default instance pointers
In proxy_free_defaults(); none of the free() calls was followed by a
pointer reset. Not only it's hard to figure if one of them is duplicated,
but this code started to call other functions which might or might not
rely on such just freed pointers. Let's reset them as they should be to
make sure there will never be any case of use-after-free. The 3 functions
called there were inspected and are all unaffected by this so this remains
safe to do right now.
2021-02-12 16:23:46 +01:00
Willy Tarreau a3320a0509 MINOR: proxy: move the defproxy freeing code to proxy.c
This used to be open-coded in cfgparse-listen.c when facing a "defaults"
keyword. Let's move this into proxy_free_defaults(). This code is ugly and
doesn't even reset the just freed pointers. Let's not change this yet.

This code should probably be merged with a generic proxy deinit function
called from deinit(). However there's a catch on uri_auth which cannot be
freed because it might be used by one or several proxies. We definitely
need refcounts there!
2021-02-12 16:23:46 +01:00
Willy Tarreau 3b06eaec86 MEDIUM: proxy: only take defaults when a default proxy is passed.
The proxy initialization code relies on three phases, allocation,
pre-initialization, and assignments from defaults. This last part is
entirely taken from the defaults proxy when arguments are set. This
sensibly complexifies the initialization code as it requires to always
have a default proxy.

This patch instead first applies the original default settings on a
proxy, and then uses those from a default proxy only if one such is
used. This will allow to initialize a proxy out of any default proxy
while still using valid defaults. A careful inspection of the function
showed that only 4 fields used to be set regardless of the default
proxy, and those were moved to init_new_proxy() where they ought to
have been in the first place.
2021-02-12 16:23:46 +01:00
Willy Tarreau 7683893c70 REORG: proxy: centralize the proxy allocation code into alloc_new_proxy()
This new function takes over the old open-coding that used to be done
for too long in cfg_parse_listen() and it now does everything at once
in a proxy-centric function. The function does all the job of allocating
the structure, initializing it, presetting its defaults from the default
proxy and checking for errors. The code was almost unchanged except for
defproxy being passed as a pointer, and the error message being passed
using memprintf().

This change will be needed to ease reuse of multiple default proxies,
or to create dynamic backends in a distant future.
2021-02-12 16:23:46 +01:00
Willy Tarreau 144289b459 REORG: move init_default_instance() to proxy.c and pass it the defproxy pointer
init_default_instance() was still left in cfgparse.c which is not the
best place to pre-initialize a proxy. Let's place it in proxy.c just
after init_new_proxy(), take this opportunity for renaming it to
proxy_preset_defaults() and taking out init_new_proxy() from it, and
let's pass it the pointer to the default proxy to be initialized instead
of implicitly assuming defproxy. We'll soon be able to exploit this.
Only two call places had to be updated.
2021-02-12 16:23:46 +01:00
Willy Tarreau 168a414037 BUILD: proxy: add missing compression-t.h to proxy-t.h
struct comp is used in struct proxy but never declared prior to this
so depending on where proxy.h is included, touching the <comp> field
can break the build.
2021-02-12 16:23:46 +01:00
Willy Tarreau 09f2e77eb1 BUG/MINOR: tcpheck: the source list must be a const in dup_tcpcheck_var()
This is just an API bug but it's annoying when trying to tidy the code.
The source list passed in argument must be a const and not a variable,
as it's typically the list head from a default proxy and must obviously
not be modified by the function. No backport is needed as it only impacts
new code.
2021-02-12 16:23:46 +01:00
Willy Tarreau 016255a483 BUG/MINOR: http-htx: defpx must be a const in proxy_dup_default_conf_errors()
This is just an API bug but it's annoying when trying to tidy the code.
The default proxy passed in argument must be a const and not a variable.
No backport is needed as it only impacts new code.
2021-02-12 16:23:46 +01:00
Willy Tarreau b2ec994523 BUG/MINOR: cfgparse: do not mention "addr:port" as supported on proxy lines
The very old error message indicating that a proxy name is mandatory
still had a reference to the optional addr:port argument while this one
is explicitly rejected a few lines later since at least 1.9.

This is harmless but confusing. This can be backported to 2.0.
2021-02-12 16:23:45 +01:00
Willy Tarreau 5bbc676608 BUG/MINOR: stats: revert the change on ST_CONVDONE
In 2.1, commit ee4f5f83d ("MINOR: stats: get rid of the ST_CONVDONE flag")
introduced a subtle bug. By testing curproxy against defproxy in
check_config_validity(), it tried to eliminate the need for a flag
to indicate that stats authentication rules were already compiled,
but by doing so it left the issue opened for the case where a new
defaults section appears after the two proxies sharing the first
one:

      defaults
          mode http
          stats auth foo:bar

      listen l1
          bind :8080

      listen l2
          bind :8181

      defaults
          # just to break above

This config results in:
  [ALERT] 042/113725 (3121) : proxy 'f2': stats 'auth'/'realm' and 'http-request' can't be used at the same time.
  [ALERT] 042/113725 (3121) : Fatal errors found in configuration.

Removing the last defaults remains OK. It turns out that the cleanups
that followed that patch render it useless, so the best fix is to revert
the change (with the up-to-date flags instead). The flag was marked as
belonging to the config. It's not exact but it's the closest to the
reality, as it's not there to configure the behavior but ti mention
that the config parser did its job.

This could be backported as far as 2.1, but in practice it looks like
nobody ever hit it.
2021-02-12 16:23:45 +01:00
Willy Tarreau 937c3ead34 BUG/MEDIUM: config: don't pick unset values from last defaults section
Since commit 1.3.14 with commit 1fa3126ec ("[MEDIUM] introduce separation
between contimeout, and tarpit + queue"), check_config_validity() looks
at the last defaults section to update all proxies' queue and tarpit
timeouts if they were not set!

This was apparently an attempt to properly set them on the fallback values,
except that the fallback values were taken from the default proxy before
looking at the current proxy itself. The worst part of it is that it might
have randomly worked by accident for some configurations when there was a
single defaults section, but has certainly caused too short queue
expirations once another defaults section was added later in the file with
these explicitly defined.

Let's remove the defproxy part and keep only the curproxy ones. This could
be backported everywhere, the bug has been there for 13 years.
2021-02-12 16:23:45 +01:00
Christopher Faulet f5ea269723 CLEANUP: deinit: release global and per-proxy server-state variables on deinit
The global server-state base directory and file name are now released on
deinit, as well as per-proxy server-state file name.
2021-02-12 16:04:52 +01:00
Christopher Faulet 583b6de68a BUG/MINOR: server: Fix server-state-file-name directive
Since the beginning, this directive is documented to accept an optional file
name. But it should also be possible to use it without any argument to use
the backend name as file name. However, when no argument is provided, an
error is reported during the configuration parsing requesting an argument, a
file name or "use-backend-name". And This last special argument is not
documented.

So, to respect the documentation and to avoid configuration breakages, all
modes are now supported. If this directive is called with no argument or
with "use-backend-name", the backend name is use as file name for the
server-state file. Otherwise, the provided string is used.

In addition, we take care to release any previously allocated file name in
case this directive is defines multiple times in the same backend. And an
error is reported if more than one argument are defined. Finally, the
documentation is updated accordingly. Sections supporting this directive are
also mentioned.

This patch should be backported as far as 1.6.
2021-02-12 16:04:52 +01:00
William Dauchy ddc7ce9645 MINOR: server: enhance error precision when applying server state
server health checks and agent parameters are written the same way as
others to be able to enahcne code reuse: basically we make use of
parsing and assignment at the same place. It makes it difficult for
error handling to know whether srv object was modified partially or not.
The problem was already present with SRV resolution though.

I was a bit puzzled about the approach to take to be honest, and I did
not wanted to go into a full refactor, so I assumed it was ok to simply
notify whether the line was failed or partially applied.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
2021-02-12 16:04:52 +01:00
William Dauchy d1a7b85a40 MEDIUM: server: support {check,agent}_addr, agent_port in server state
logical followup from cli commands addition, so that the state server
file stays compatible with the changes made at runtime; use previously
added helper to load server attributes.

also alloc a specific chunk to avoid mixing with other called functions
using it

Signed-off-by: William Dauchy <wdauchy@gmail.com>
2021-02-12 16:04:52 +01:00
William Dauchy 63e6cba12a MEDIUM: server: add server-states version 2
Even if it is possibly too much work for the current usage, it makes
sure we don't break states file from v2.3 to v2.4; indeed, since v2.3,
we introduced two new fields, so we put them aside to guarantee we can
easily reload from a version 1.
The diff seems huge but there is no specific change apart from:
- introduce v2 where it is needed (parsing, update)
- move away from switch/case in update to be able to reuse code
- move srv lock to the whole function to make it easier

this patch confirm how painful it is to maintain this functionality.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
2021-02-12 16:04:52 +01:00
William Dauchy 7cabc06da6 MEDIUM: cli: add agent-port command
this patch allows to set agent port at runtime. In order to align with
both `addr` and `check-addr` commands, also add the possibility to
optionnaly set port on `agent-addr` command. This led to a small
refactor in order to use the same function for both `agent-addr` and
`agent-port` commands.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
2021-02-12 16:04:52 +01:00
William Dauchy b456e1f389 MEDIUM: cli: add check-addr command
this patch allows to set server health check address at runtime. In
order to align with `addr` command, also allow to set port optionnaly.
This led to a small refactor in order to use the same function for both
`check-addr` and `check-port` commands.
for `check-port`, we however don't permit the change anymore if checks
are not enabled on the server.

This command becomes more and more useful for people having a consul
like architecture:
- the backend server is located on a container with its own IP
- the health checks are done the consul instance located on the host
  with the host IP

Signed-off-by: William Dauchy <wdauchy@gmail.com>
2021-02-12 16:04:52 +01:00
Ilya Shipitsin 4d838c4c3c CI: cirrus: update FreeBSD image to 12.2
we already tried to run FreeBSD-stable. it is pain,
so we use FreeBSD releases, we need to keep packages and release in sync.

let us update to released FreeBSD-12.2
2021-02-12 16:04:52 +01:00
Amaury Denoyelle 7f583be822 REGTESTS: fix sni used in http_reuse_conn_hash for libressl 3.3.0
libressl 3.3.0 is stricter on the sni field and fails if it contains
illegal characters such as the underscore. Replace sni field with proper
name to pass the test on the CI environment.
2021-02-12 15:27:33 +01:00
Amaury Denoyelle 49aa5e9eca REGTESTS: deactivate a failed test on CI in http_reuse_conn_hash
The last request with proxy protocol fails on the CI, reporting
first_http_request as 1. Currently this is not reproducible on my
local machine.
2021-02-12 14:18:39 +01:00
Amaury Denoyelle 8abbd2dcc3 REGTESTS: adjust http_reuse_conn_hash requirements
This test is only valid for 2.4+ as the tested feature was introduced in
this version. Besides, it can only run with ssl support to test sni
reuse.
2021-02-12 14:18:16 +01:00
Amaury Denoyelle edadf192fe BUG/MINOR: backend: fix compilation without ssl
sni_smp/sni_hash are reported as unused on compilation without
USE_OPENSL and may cause compilation failure

This does not need to be backported.
2021-02-12 13:49:42 +01:00
Amaury Denoyelle d773a4ee23 MINOR: doc: update http reuse for new eligilible connections
Update the doc to remove entries on http-reuse marking private
connection for specific source address or sni.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle 7ef06c8253 MINOR: reg-tests: test http-reuse with proxy protocol
Complete the http-reuse test with the proxy protocol usage. A server
connection can be reused if the proxy protocol fields are the same.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle 1921d20fff MINOR: connection: use proxy protocol as parameter for srv conn hash
Use the proxy protocol frame if proxy protocol is activated on the
server line. Do not add anymore these connections in the private list.
If some requests are made with the same proxy fields, they can reuse
the idle connection.

The reg-tests proxy_protocol_send_unique_id must be adapted has it
relied on the side effect behavior that every requests from a same
connection reused a private server connection. Now, a new connection is
created as expected if the proxy protocol fields differ.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle d10a200f62 MINOR: connection: use src addr as parameter for srv conn hash
The source address is used as an input to the the server connection hash. The
address and port are used as separate hash inputs. Do not add anymore these
connections in the private list.

This parameter is set only if used in the transparent-proxy mode.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle f7bdf00071 MINOR: backend: rewrite alloc of connection src address
This commit is similar to
"MINOR: backend: rewrite alloc of stream target address" but with source
address.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle f679d9bbf3 MINOR: reg-test: test http-reuse with specific dst addr
Add a reg-test for connection reuse with a specific destination address.
It is based on the set-dst-port http rule.
2021-02-12 12:54:04 +01:00
Amaury Denoyelle 01a287f1e5 MINOR: connection: use dst addr as parameter for srv conn hash
The destination address is used as an input to the server connection hash. The
address and port are used as separated hash inputs. Note that they are not used
when statically specified on the server line. This is only useful for dynamic
destination address.

This is typically used when the server address is dynamically set via the
set-dst action. The address and port are separated hash parameters.

Most notably, it should fixed set-dst use case (cf github issue #947).
2021-02-12 12:53:56 +01:00
Amaury Denoyelle 68cf3959b3 MINOR: backend: rewrite alloc of stream target address
Change the API of the function used to allocate the stream target
address. This is done in order to be able to allocate the destination
address and use it to reuse a connection sharing with the same address.
In particular, the flag stream SF_ADDR_SET is now set outside of the
function.
2021-02-12 12:53:56 +01:00
Amaury Denoyelle e676239922 MINOR: reg-tests: test http-reuse with sni
Add a reg-test for connection reuse with the sni parameter. Ensure that
only connections that use the same sni are reused.
2021-02-12 12:48:11 +01:00
Amaury Denoyelle 9b626e3c19 MINOR: connection: use sni as parameter for srv conn hash
The sni parameter is an input to the server connection hash. Do not add
anymore connections with dynamic sni in the private list. Thus, it is
now possible to reuse a server connection if they use the same sni.
2021-02-12 12:48:11 +01:00
Amaury Denoyelle 293dcc400e MINOR: backend: compare conn hash for session conn reuse
Compare the connection hash when reusing a connection from the session.
This ensures that a private connection is reused only if it shares the
same set of parameters.
2021-02-12 12:33:05 +01:00
Amaury Denoyelle 1a58aca84e MINOR: connection: use the srv pointer for the srv conn hash
The pointer of the target server is used as a first parameter for the
server connection hash calcul. This prevents the hash to be null when no
specific parameters are present, and can serve as a simple defense
against an attacker trying to reuse a non-conform connection.
2021-02-12 12:33:05 +01:00