Commit Graph

10780 Commits

Author SHA1 Message Date
Willy Tarreau 9b013701f1 MINOR: stats/debug: maintain a counter of debug commands issued
Debug commands will usually mark the fate of the process. We'd rather
have them counted and visible in a core or in stats output than trying
to guess how a flag combination could happen. The counter is only
incremented when the command is about to be issued however, so that
failed attempts are ignored.
2019-10-24 18:38:00 +02:00
Willy Tarreau b24ab22ac0 MINOR: debug: make most debug CLI commands accessible in expert mode
Instead of relying on DEBUG_DEV for most debugging commands, which is
limiting, let's condition them to expert mode. Only one ("debug dev exec")
remains conditionned to DEBUG_DEV because it can have a security implication
on the system. The commands are not listed unless "expert-mode on" was first
entered on the CLI :

 > expert-mode on
 > help
   debug dev close <fd>        : close this file descriptor
   debug dev delay [ms]        : sleep this long
   debug dev exec  [cmd] ...   : show this command's output
   debug dev exit  [code]      : immediately exit the process
   debug dev hex   <addr> [len]: dump a memory area
   debug dev log   [msg] ...   : send this msg to global logs
   debug dev loop  [ms]        : loop this long
   debug dev panic             : immediately trigger a panic
   debug dev stream ...        : show/manipulate stream flags
   debug dev tkill [thr] [sig] : send signal to thread

 > debug dev stream
 Usage: debug dev stream { <obj> <op> <value> | wake }*
      <obj>   = {strm | strm.f | sif.f | sif.s | sif.x | sib.f | sib.s | sib.x |
                 txn.f | req.f | req.r | req.w | res.f | res.r | res.w}
      <op>    = {'' (show) | '=' (assign) | '^' (xor) | '+' (or) | '-' (andnot)}
      <value> = 'now' | 64-bit dec/hex integer (0x prefix supported)
      'wake' wakes the stream asssigned to 'strm' (default: current)
2019-10-24 18:38:00 +02:00
Willy Tarreau abb9f9b057 MINOR: cli: add an expert mode to hide dangerous commands
Some commands like the debug ones are not enabled by default but can be
useful on some production environments. In order to avoid the temptation
of using them incorrectly, let's introduce an "expert" mode for a CLI
connection, which allows some commands to appear and be used. It is
enabled by command "expert-mode on" which is not listed by default.
2019-10-24 18:38:00 +02:00
Willy Tarreau 86bfe146c9 REORG: move CLI access level definitions to cli.h
These ones were still in global.h which is misplaced.
2019-10-24 18:38:00 +02:00
Willy Tarreau 2b5520da47 MINOR: cli/debug: validate addresses using may_access() in "debug dev stream"
This function adds some control by verifying that the target address is
really readable. It will not protect against writing to wrong places,
but will at least protect against a large number of mistakes such as
incorrectly copy-pasted addresses.
2019-10-24 18:38:00 +02:00
Willy Tarreau 68680bb14e MINOR: debug: add a new "debug dev stream" command
This new "debug dev stream" command allows to manipulate flags, timeouts,
states for streams, channels and stream interfaces, as well as waking a
stream up. These may be used to help reproduce certain bugs during
development. The operations are performed to the stream assigned by
"strm" which defaults to the CLI's stream. This stream pointer can be
chosen from one of those reported in "show sess". Example:

  socat - /tmp/sock1 <<< "debug dev stream strm=0x1555b80 req.f=-1 req.r=now wake"
2019-10-24 10:43:04 +02:00
William Dauchy b705b4d7d3 MINOR: tcp: avoid confusion in time parsing init
We never enter val_fc_time_value when an associated fetcher such as `fc_rtt` is
called without argument.  meaning `type == ARGT_STOP` will never be true and so
the default `data.sint = TIME_UNIT_MS` will never be set.  remove this part to
avoid thinking default data.sint is set to ms while reading the code.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>

[Cf: This patch may safely backported as far as 1.7. But no matter if not.]
2019-10-24 10:25:00 +02:00
William Lallemand 705e088f0a BUG/MINOR: ssl: fix build of X509_chain_up_ref() w/ libreSSL
LibreSSL brought X509_chain_up_ref() in 2.7.5, so no need to build our
own version starting from this version.
2019-10-23 23:20:08 +02:00
William Lallemand 89f5807315 BUG/MINOR: ssl: fix build with openssl < 1.1.0
8c1cddef ("MINOR: ssl: new functions duplicate and free a ckch_store")
use some OpenSSL refcount functions that were introduced in OpenSSL
1.0.2 and OpenSSL 1.1.0.

Fix the problem by introducing them in openssl-compat.h.

Fix #336.
2019-10-23 19:44:50 +02:00
William Lallemand f29cdefccd BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
Commit 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP")
introduced a bug in which we iterate outside the array durint a 'set ssl
cert' if we didn't built with the ocsp or sctl.
2019-10-23 15:05:00 +02:00
William Lallemand 541a534c9f BUG/MINOR: ssl/cli: fix build of SCTL and OCSP
Fix the build issue of SCTL and OCSP for boring/libressl introduced by
44b3532 ("MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI")
2019-10-23 14:47:16 +02:00
William Lallemand 8f840d7e55 MEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler
To avoid affecting too much the traffic during a certificate update,
create the SNIs in a IO handler which yield every 10 ckch instances.

This way haproxy continues to respond even if we tries to update a
certificate which have 50 000 instances.
2019-10-23 11:54:51 +02:00
William Lallemand 0c3b7d9e1c MINOR: ssl/cli: assignate a new ckch_store
When updating a certificate from the CLI, it is not possible to revert
some of the changes if part of the certicate update failed. We now
creates a copy of the ckch_store for the changes so we can revert back
if something goes wrong.

Even if the ckch_store was affected before this change, it wasn't
affecting the SSL_CTXs used for the traffic. It was only a problem if we
try to update a certificate after we failed to do it the first time.

The new ckch_store is also linked to the new sni_ctxs so it's easy to
insert the sni_ctxs before removing the old ones.
2019-10-23 11:54:51 +02:00
William Lallemand 8c1cddef6d MINOR: ssl: new functions duplicate and free a ckch_store
ckchs_dup() alloc a new ckch_store and copy the content of its source.

ckchs_free() frees a ckch_store and its content.
2019-10-23 11:54:51 +02:00
William Lallemand 8d0f893222 MINOR: ssl: copy a ckch from src to dst
ssl_sock_copy_cert_key_and_chain() copy the content of a
<src> cert_key_and_chain to a <dst>.

It applies a refcount increasing on every SSL structures (X509, DH,
privte key..) and allocate new buffers for the other fields.
2019-10-23 11:54:51 +02:00
William Lallemand 455af50fac MINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents
The struct cert_key_and_chain now contains the DH, the sctl and the
ocsp_response. Free them.
2019-10-23 11:54:51 +02:00
William Lallemand 44b3532250 MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI
It is now possible to update new parts of a CKCH from the CLI.

Currently you will be able to update a PEM (by default), a OCSP response
in base64, an issuer file, and a SCTL file.

Each update will creates a new CKCH and new sni_ctx structure so we will
need a "commit" command later to apply several changes and create the
sni_ctx only once.
2019-10-23 11:54:51 +02:00
William Lallemand 849eed6b25 BUG/MINOR: ssl/cli: fix looking up for a bundle
If we want a bundle but we didn't find a bundle, we shouldn't try to
apply the changes.
2019-10-23 11:54:51 +02:00
William Lallemand 96a9c97369 MINOR: ssl: split ssl_sock_load_crt_file_into_ckch()
Split the ssl_sock_load_crt_file_into_ckch() in two functions:

- ssl_sock_load_files_into_ckch() which is dedicated to opening every
files related to a filename during the configuration parsing (PEM, sctl,
ocsp, issuer etc)

- ssl_sock_load_pem_into_ckch() which is dedicated to opening a PEM,
either in a file or a buffer
2019-10-23 11:54:51 +02:00
William Lallemand f9568fcd79 MINOR: ssl: load issuer from file or from buffer
ssl_sock_load_issuer_file_into_ckch() is a new function which is able to
load an issuer from a buffer or from a file to a CKCH.

Use this function directly in ssl_sock_load_crt_file_into_ckch()
2019-10-23 11:54:51 +02:00
William Lallemand 0dfae6c315 MINOR: ssl: load sctl from buf OR from a file
The ssl_sock_load_sctl_from_file() function was modified to
fill directly a struct cert_key_and_chain.

The function prototype was normalized in order to be used with the CLI
payload parser.

This function either read  text from a buffer or read a file on the
filesystem.

It fills the ocsp_response buffer of the struct cert_key_and_chain.
2019-10-23 11:54:51 +02:00
William Lallemand 3b5f360744 MINOR: ssl: OCSP functions can load from file or buffer
The ssl_sock_load_ocsp_response_from_file() function was modified to
fill directly a struct cert_key_and_chain.

The function prototype was normalized in order to be used with the CLI
payload parser.

This function either read a base64 from a buffer or read a binary file
on the filesystem.

It fills the ocsp_response buffer of the struct cert_key_and_chain.
2019-10-23 11:54:51 +02:00
William Lallemand 02010478e9 CLEANUP: ssl: fix SNI/CKCH lock labels
The CKCH and the SNI locks originally used the same label, we split them
but we forgot to change some of them.
2019-10-23 11:54:51 +02:00
William Lallemand 34779c34fc CLEANUP: ssl: remove old TODO commentary
Remove an old commentary above ckch_inst_new_load_multi_store().
This function doe not do filesystem syscalls anymore.
2019-10-23 11:54:51 +02:00
Willy Tarreau 9364a5fda3 BUG/MINOR: mux-h2: do not emit logs on backend connections
The logs were added to the H2 mux so that we can report logs in case
of errors that prevent a stream from being created, but as a side effect
these logs are emitted twice for backend connections: once by the H2 mux
itself and another time by the upper layer stream. It can even happen
more with connection retries.

This patch makes sure we do not emit logs for backend connections.

It should be backported to 2.0 and 1.9.
2019-10-23 11:12:22 +02:00
Willy Tarreau 403bfbb130 BUG/MEDIUM: pattern: make the pattern LRU cache thread-local and lockless
As reported in issue #335, a lot of contention happens on the PATLRU lock
when performing expensive regex lookups. This is absurd since the purpose
of the LRU cache was to have a fast cache for expressions, thus the cache
must not be shared between threads and must remain lockless.

This commit makes the LRU cache thread-local and gets rid of the PATLRU
lock. A test with 7 threads on 4 cores climbed from 67kH/s to 369kH/s,
or a scalability factor of 5.5.

Given the huge performance difference and the regression caused to
users migrating from processes to threads, this should be backported at
least to 2.0.

Thanks to Brian Diekelman for his detailed report about this regression.
2019-10-23 07:27:25 +02:00
Willy Tarreau 28c63c15f5 BUG/MINOR: stick-table: fix an incorrect 32 to 64 bit key conversion
As reported in issue #331, the code used to cast a 32-bit to a 64-bit
stick-table key is wrong. It only copies the 32 lower bits in place on
little endian machines or overwrites the 32 higher ones on big endian
machines. It ought to simply remove the wrong cast dereference.

This bug was introduced when changing stick table keys to samples in
1.6-dev4 by commit bc8c404449 ("MAJOR: stick-tables: use sample types
in place of dedicated types") so it the fix must be backported as far
as 1.6.
2019-10-23 06:24:58 +02:00
Emeric Brun eb46965bbb BUG/MINOR: ssl: fix memcpy overlap without consequences.
A trick is used to set SESSION_ID, and SESSION_ID_CONTEXT lengths
to 0 and avoid ASN1 encoding of these values.

There is no specific function to set the length of those parameters
to 0 so we fake this calling these function to a different value
with the same buffer but a length to zero.

But those functions don't seem to check the length of zero before
performing a memcpy of length zero but with src and dst buf on the
same pointer, causing valgrind to bark.

So the code was re-work to pass them different pointers even
if buffer content is un-used.

In a second time, reseting value, a memcpy overlap
happened on the SESSION_ID_CONTEXT. It was re-worked and this is
now reset using the constant global value SHCTX_APPNAME which is a
different pointer with the same content.

This patch should be backported in every version since ssl
support was added to haproxy if we want valgrind to shut up.
This is tracked in github issue #56.
2019-10-22 18:57:45 +02:00
Baptiste Assmann 25e6fc2030 BUG/MINOR: dns: allow srv record weight set to 0
Processing of SRV record weight was inaccurate and when a SRV record's
weight was set to 0, HAProxy enforced it to '1'.
This patch aims at fixing this without breaking compability with
previous behavior.

Backport status: 1.8 to 2.0
2019-10-22 13:44:12 +02:00
Willy Tarreau 04068a1939 REGTESTS: server/cli_set_fqdn requires version 1.8 minimum
This test uses "set server <srv> fqdn" which is not available in 1.7.
All reg-tests now pass on 1.7.
2019-10-22 13:06:59 +02:00
Willy Tarreau 1545a59c7b REGTESTS: make seamless-reload depend on 1.9 and above
Since latest updates, vtest requires the master CLI when running in
master-worker mode, and this one is only available starting with 1.9.
The seamless reload test is the only one depending on this and now
fails on 1.8, so let's adjust it accordingly.
2019-10-22 10:42:10 +02:00
Vedran Furac 5d48627aba BUG/MINOR: server: check return value of fopen() in apply_server_state()
fopen() can return NULL when state file is missing. This patch adds a
check of fopen() return value so we can skip processing in such case.

No backport needed.
2019-10-21 16:00:24 +02:00
Tim Duesterhus 4381d26edc BUG/MINOR: sample: Make the `field` converter compatible with `-m found`
Previously an expression like:

    path,field(2,/) -m found

always returned `true`.

Bug exists since the `field` converter exists. That is:
f399b0debf

The fix should be backported to 1.6+.
2019-10-21 15:49:42 +02:00
William Lallemand d1d1e22945 BUG/MINOR: cache: alloc shctx after check config
When running haproxy -c, the cache parser is trying to allocate the size
of the cache. This can be a problem in an environment where the RAM is
limited.

This patch moves the cache allocation in the post_check callback which
is not executed during a -c.

This patch may be backported at least to 2.0 and 1.9. In 1.9, the callbacks
registration mechanism is not the same. So the patch will have to be adapted. No
need to backport it to 1.8, the code is probably too different.
2019-10-21 15:05:46 +02:00
Christopher Faulet a9fa88a1ea BUG/MINOR: stick-table: Never exceed (MAX_SESS_STKCTR-1) when fetching a stkctr
When a stick counter is fetched, it is important that the requested counter does
not exceed (MAX_SESS_STKCTR -1). Actually, there is no bug with a default build
because, by construction, MAX_SESS_STKCTR is defined to 3 and we know that we
never exceed the max value. scN_* sample fetches are numbered from 0 to 2. For
other sample fetches, the value is tested.

But there is a bug if MAX_SESS_STKCTR is set to a lower value. For instance
1. In this case the counters sc1_* and sc2_* may be undefined.

This patch fixes the issue #330. It must be backported as far as 1.7.
2019-10-21 11:17:04 +02:00
Christopher Faulet e566f3db11 BUG/MINOR: ssl: Fix fd leak on error path when a TLS ticket keys file is parsed
When an error occurred in the function bind_parse_tls_ticket_keys(), during the
configuration parsing, the opened file is not always closed. To fix the bug, all
errors are catched at the same place, where all ressources are released.

This patch fixes the bug #325. It must be backported as far as 1.7.
2019-10-21 10:04:51 +02:00
William Lallemand f7f488d8e9 BUG/MINOR: mworker/cli: reload fail with inherited FD
When using the master CLI with 'fd@', during a reload, the master CLI
proxy is stopped. Unfortunately if this is an inherited FD it is closed
too, and the master CLI won't be able to bind again during the
re-execution. It lead the master to fallback in waitpid mode.

This patch forbids the inherited FDs in the master's listeners to be
closed during a proxy_stop().

This patch is mandatory to use the -W option in VTest versions that contain the
-mcli feature.
(86e65f1024)

Should be backported as far as 1.9.
2019-10-18 21:45:42 +02:00
Emeric Brun a9363eb6a5 BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
If openssl 1.1.1 is used, c2aae74f0 commit mistakenly enables DH automatic
feature from openssl instead of ECDH automatic feature. There is no impact for
the ECDH one because the feature is always enabled for that version. But doing
this, the 'tune.ssl.default-dh-param' was completely ignored for DH parameters.

This patch fix the bug calling 'SSL_CTX_set_ecdh_auto' instead of
'SSL_CTX_set_dh_auto'.

Currently some users may use a 2048 DH bits parameter, thinking they're using a
1024 bits one. Doing this, they may experience performance issue on light hardware.

This patch warns the user if haproxy fails to configure the given DH parameter.
In this case and if openssl version is > 1.1.0, haproxy will let openssl to
automatically choose a default DH parameter. For other openssl versions, the DH
ciphers won't be usable.

A commonly case of failure is due to the security level of openssl.cnf
which could refuse a 1024 bits DH parameter for a 2048 bits key:

 $ cat /etc/ssl/openssl.cnf
 ...

 [system_default_sect]
 MinProtocol = TLSv1
 CipherString = DEFAULT@SECLEVEL=2

This should be backport into any branch containing the commit c2aae74f0.
It requires all or part of the previous CLEANUP series.

This addresses github issue #324.
2019-10-18 15:18:52 +02:00
Emeric Brun 0655c9b222 CLEANUP: bind: handle warning label on bind keywords parsing.
All bind keyword parsing message were show as alerts.

With this patch if the message is flagged only with ERR_WARN and
not ERR_ALERT it will show a label [WARNING] and not [ALERT].
2019-10-18 15:18:52 +02:00
Emeric Brun 7a88336cf8 CLEANUP: ssl: make ssl_sock_load_dh_params handle errcode/warn
ssl_sock_load_dh_params used to return >0 or -1 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. An error message was added in the case of failure and the
comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun a96b582d0e CLEANUP: ssl: make ssl_sock_put_ckch_into_ctx handle errcode/warn
ssl_sock_put_ckch_into_ctx used to return 0 or >0 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. And a comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun 054563de13 CLEANUP: ssl: make ckch_inst_new_load_(multi_)store handle errcode/warn
ckch_inst_new_load_store() and ckch_inst_new_load_multi_store used to
return 0 or >0 to indicate success or failure. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
his is the only code returned for now. And the comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun f69ed1d21c CLEANUP: ssl: make cli_parse_set_cert handle errcode and warnings.
cli_parse_set_cert was re-work to show errors and warnings depending
of ERR_* bitfield value.
2019-10-18 15:18:52 +02:00
Willy Tarreau 8c5414a546 CLEANUP: ssl: make ssl_sock_load_ckchs() return a set of ERR_*
ssl_sock_load_ckchs() used to return 0 or >0 to indicate success or
failure even though this was not documented. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
this is the only code returned for now. And a comment was added.
2019-10-18 15:18:52 +02:00
Willy Tarreau bbc91965bf CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
These functions were returning only 0 or 1 to mention success or error,
and made it impossible to return a warning. Let's make them return error
codes from ERR_* and map all errors to ERR_ALERT|ERR_FATAL for now since
this is the only code that was set on non-zero return value.

In addition some missing comments were added or adjusted around the
functions' return values.
2019-10-18 15:18:52 +02:00
William Lallemand cd48277469 REGTEST: mcli/mcli_show_info: launch a 'show info' on the master CLI
This test launches a HAProxy process in master worker with 'nbproc 4'.
It sends a "show info" to the process 3 and verify that the right
process replied.

This regtest depends on the support of the master CLI for VTest.
2019-10-18 14:47:30 +02:00
Olivier Houchard 2ed389dc6e BUG/MEDIUM: mux_pt: Only call the wake emthod if nobody subscribed to receive.
In mux_pt_io_cb(), instead of always calling the wake method, only do so
if nobody subscribed for receive. If we have a subscription, just wake the
associated tasklet up.

This should be backported to 1.9 and 2.0.
2019-10-18 14:18:29 +02:00
Olivier Houchard ea510fc5e7 BUG/MEDIUM: mux_pt: Don't destroy the connection if we have a stream attached.
There's a small window where the mux_pt tasklet may be woken up, and thus
mux_pt_io_cb() get scheduled, and then the connection is attached to a new
stream. If this happen, don't do anything, and just let the stream know
by calling its wake method. If the connection had an error, the stream should
take care of destroying it by calling the detach method.

This should be backported to 2.0 and 1.9.
2019-10-18 14:07:22 +02:00
Olivier Houchard 9dce2c53a8 Revert e8826ded5f.
This reverts commit "BUG/MEDIUM: mux_pt: Make sure we don't have a
conn_stream before freeing.".
mux_pt_io_cb() is only used if we have no associated stream, so we will
never have a cs, so there's no need to check that, and we of course have to
destroy the mux in mux_pt_detach() if we have no associated session, or if
there's an error on the connection.

This should be backported to 2.0 and 1.9.
2019-10-18 11:24:04 +02:00
Willy Tarreau 8cdc167df8 BUG/MEDIUM: task: make tasklets either local or shared but not both at once
Tasklets may be woken up to run on the calling thread or by a specific thread
(the owner). But since we use a non-thread safe mechanism when the calling
thread is also the for the owner, there may sometimes be collisions when two
threads decide to wake the same tasklet up at the same time and one of them
is the owner.

This is more of a matter of usage than code, in that a tasklet usually is
designed to be woken up and executed on the calling thread only (most cases)
or on a specific thread. Thus it is a property of the tasklet itself as this
solely depends how the code is constructed around it.

This patch performs a small change to address this. By default tasklet_new()
creates a "local" tasklet, which will run on the calling thread, like in 2.0.
This is done by setting tl->tid to a negative value. If the caller wants the
tasklet to run exclusively on a specific thread, it just has to set tl->tid,
which is already what shared tasklet callers do anyway.

No backport is needed.
2019-10-18 09:04:55 +02:00