Commit Graph

15267 Commits

Author SHA1 Message Date
Willy Tarreau
8441deb1e2 [RELEASE] Released version 2.5-dev3
Released version 2.5-dev3 with the following main changes :
    - BUG/MINOR: arg: free all args on make_arg_list()'s error path
    - BUG/MINOR: cfgcond: revisit the condition freeing mechanism to avoid a leak
    - MEDIUM: proxy: remove long-broken 'option http_proxy'
    - CLEANUP: http_ana: Remove now unused label from http_process_request()
    - MINOR: deinit: always deinit the init_mutex on failed initialization
    - BUG/MEDIUM: cfgcond: limit recursion level in the condition expression parser
    - BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
    - BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
    - BUILD/MINOR: memprof fix macOs build.
    - BUG/MEDIUM: ssl_sample: fix segfault for srv samples on invalid request
    - BUG/MINOR: stats: Add missing agent stats on servers
    - BUG/MINOR: check: fix the condition to validate a port-less server
    - BUILD: threads: fix pthread_mutex_unlock when !USE_THREAD
    - BUG/MINOR: resolvers: Use a null-terminated string to lookup in servers tree
    - MINOR: ssl: use __objt_* variant when retrieving counters
    - BUG/MINOR: systemd: must check the configuration using -Ws
    - BUG/MINOR: mux-h1: Obey dontlognull option for empty requests
    - BUG/MINOR: mux-h2: Obey dontlognull option during the preface
    - BUG/MINOR: mux-h1: Be sure to swap H1C to splice mode when rcv_pipe() is called
    - BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
    - MINOR: proxy: rename PR_CAP_LUA to PR_CAP_INT
    - MINOR: mworker: the mworker CLI proxy is internal
    - MINOR: stats: don't output internal proxies (PR_CAP_INT)
    - CLEANUP: mworker: use the proxy helper functions in mworker_cli_proxy_create()
    - CLEANUP: mworker: PR_CAP already initialized with alloc_new_proxy()
    - BUG/MINOR: connection: Add missing error labels to conn_err_code_str
    - MINOR: connection: Add a connection error code sample fetch
    - MINOR: ssl: Enable error fetches in case of handshake error
    - MINOR: ssl: Add new ssl_fc_hsk_err sample fetch
    - MINOR: ssl: Define a default https log format
    - MEDIUM: connection: Add option to disable legacy error log
    - REGTESTS: ssl: Add tests for the connection and SSL error fetches
    - REGTESTS: ssl: ssl_errors.vtc does not work with old openssl version
    - BUG/MEDIUM: connection: close a rare race between idle conn close and takeover
    - BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
    - BUG/MINOR: select: fix excess number of dead/skip reported
    - BUG/MINOR: poll: fix abnormally high skip_fd counter
    - BUG/MINOR: pollers: always program an update for migrated FDs
    - BUG/MINOR: fd: protect fd state harder against a concurrent takeover
    - DOC: internals: document the FD takeover process
    - MINOR: fd: update flags only once in fd_update_events()
    - MINOR: poll/epoll: move detection of RDHUP support earlier
    - REORG: fd: uninline fd_update_events()
    - MEDIUM: fd: rely more on fd_update_events() to detect changes
    - BUG/MINOR: freq_ctr: use stricter barriers between updates and readings
    - MEDIUM: atomic: simplify the atomic load/store/exchange operations
    - MEDIUM: atomic: relax the load/store barriers on x86_64
    - BUILD: opentracing: fixed build when using pkg-config utility
2021-08-01 18:19:51 +02:00
Miroslav Zagorac
3571111de3 BUILD: opentracing: fixed build when using pkg-config utility
In case the OpenTracing C Wrapper library was installed as part of the
system (ie in a directory that pkg-config considers part of the system),
HAProxy building was not possible because in that case pkg-config did
not set the value of the OT_CFLAGS variable in the addon Makefile.

This resolves GitHub issue #1323.
2021-08-01 18:18:29 +02:00
Willy Tarreau
99198546f6 MEDIUM: atomic: relax the load/store barriers on x86_64
The x86-tso model makes the load and store barriers unneeded for our
usage as long as they perform at least a compiler barrier: the CPU
will respect store ordering and store vs load ordering. It's thus
safe to remove the lfence and sfence which are normally needed only
to communicate with external devices. Let's keep the mfence though,
to make sure that reads of same memory location after writes report
the value from memory and not the one snooped from the write buffer
for too long.

An in-depth review of all use cases tends to indicate that this is
okay in the rest of the code. Some parts could be cleaned up to
use atomic stores and atomic loads instead of explicit barriers
though.

Doing this reliably increases the overall performance by about 2-2.5%
on a 8c-16t Xeon thanks to less frequent flushes (it's likely that the
biggest gain is in the MT lists which use them a lot, and that this
results in less cache line flushes).
2021-08-01 17:34:06 +02:00
Willy Tarreau
cb0451146f MEDIUM: atomic: simplify the atomic load/store/exchange operations
The atomic_load/atomic_store/atomic_xchg operations were all forced to
__ATOMIC_SEQ_CST, which results in explicit store or even full barriers
even on x86-tso while we do not need them: we're not communicating with
external devices for example and are only interested in respecting the
proper ordering of loads and stores between each other.

These ones being rarely used, the emitted code on x86 remains almost the
same (barring a handful of locations). However they will allow to place
correct barriers at other places where atomics are accessed a bit lightly.

The patch is marked medium because we can never rule out the risk of some
bugs on more relaxed platforms due to the rest of the code.
2021-08-01 17:34:06 +02:00
Willy Tarreau
55a0975b1e BUG/MINOR: freq_ctr: use stricter barriers between updates and readings
update_freq_ctr_period() was using relaxed atomics without using barriers,
which usually works fine on x86 but not everywhere else. In addition, some
values were read without being enclosed by barriers, allowing the compiler
to possibly prefetch them a bit earlier. Finally, freq_ctr_total() was also
reading these without enough barriers. Let's make explicit use of atomic
loads and atomic stores to get rid of this situation. This required to
slightly rearrange the freq_ctr_total() loop, which could possibly slightly
improve performance under extreme contention by avoiding to reread all
fields.

A backport may be done to 2.4 if a problem is encountered, but last tests
on arm64 with LSE didn't show any issue so this can possibly stay as-is.
2021-08-01 17:34:06 +02:00
Willy Tarreau
200bd50b73 MEDIUM: fd: rely more on fd_update_events() to detect changes
This function already performs a number of checks prior to calling the
IOCB, and detects the change of thread (FD migration). Half of the
controls are still in each poller, and these pollers also maintain
activity counters for various cases.

Note that the unreliable test on thread_mask was removed so that only
the one performed by fd_set_running() is now used, since this one is
reliable.

Let's centralize all that fd-specific logic into the function and make
it return a status among:

  FD_UPDT_DONE,        // update done, nothing else to be done
  FD_UPDT_DEAD,        // FD was already dead, ignore it
  FD_UPDT_CLOSED,      // FD was closed
  FD_UPDT_MIGRATED,    // FD was migrated, ignore it now

Some pollers already used to call it last and have nothing to do after
it, regardless of the result. epoll has to delete the FD in case a
migration is detected. Overall this removes more code than it adds.
2021-07-30 17:45:18 +02:00
Willy Tarreau
84c7922c52 REORG: fd: uninline fd_update_events()
This function has become a monster (80 lines and 2/3 of a kB), it doesn't
benefit from being static nor inline anymore, let's move it to fd.c.
2021-07-30 17:41:55 +02:00
Willy Tarreau
53a16187fd MINOR: poll/epoll: move detection of RDHUP support earlier
Let's move the detection of support for RDHUP earlier and out of the
FD update chain, as it complicates its simplification.
2021-07-30 17:41:55 +02:00
Willy Tarreau
a199a17d72 MINOR: fd: update flags only once in fd_update_events()
Since 2.4 with commit f50906519 ("MEDIUM: fd: merge fdtab[].ev and state
for FD_EV_* and FD_POLL_* into state") we can merge all flag updates at
once in fd_update_events(). Previously this was performed in 1 to 3 steps,
setting the polling state, then setting READY_R if in/err/hup, and setting
READY_W if out/err. But since the commit above, all flags are stored
together in the same structure field that is being updated with the new
flags, thus we can simply update the flags altogether and avoid multiple
atomic operations. This even removes the need for atomic ops for FDs that
are not shared.
2021-07-30 17:41:55 +02:00
Willy Tarreau
88babd9944 DOC: internals: document the FD takeover process
This explains the traps to avoid and the sequence that leads to
consistent use of an FD known by multiple threads at once. This
was co-authored with Olivier.
2021-07-30 17:41:55 +02:00
Willy Tarreau
d5402b8df8 BUG/MINOR: fd: protect fd state harder against a concurrent takeover
There's a theoretical race (that we failed to trigger) in function
fd_update_events(), which could strike on idle connections. The "locked"
variable will most often be 0 as the FD is bound to the current thread
only. Another thread could take it over once "locked" is set, change
the thread and running masks. Then the first thread updates the FD's
state non-atomically and possibly overwrites what the other thread was
preparing. It still looks like the FD's state will ultimately converge
though.

The solution against this is to set the running flag earlier so that a
takeover() attempt cannot succeed, or that the fd_set_running() attempt
fails, indicating that nothing needs to be done on this FD.

While this is sufficient for a simple fix to be backported, it leaves
the FD actively polled in the calling thread, this will trigger a second
wakeup which will notice the absence of tid_bit in the thread_mask,
getting rid of it.

A more elaborate solution would consist in calling fd_set_running()
directly from the pollers before calling fd_update_events(), getting
rid of the thread_mask test and letting the caller eliminate that FD
from its list if needed.

Interestingly, this code also proves to be suboptimal in that it sets
the FD state twice instead of calculating the new state at once and
always using a CAS to set it. This is a leftover of a simplification
that went into 2.4 and which should be explored in a future patch.

This may be backported as far as 2.2.
2021-07-30 14:54:19 +02:00
Willy Tarreau
79e90b9615 BUG/MINOR: pollers: always program an update for migrated FDs
If an MT-aware poller reports that a file descriptor was migrated, it
must stop reporting it. The simplest way to do this is to program an
update if not done yet. This will automatically mark the FD for update
on next round. Otherwise there's a risk that some events are reported
a bit too often and cause extra CPU usage with these pollers. Note
that epoll is currently OK regarding this. Select does not need this
because it uses a single shared events table, so in case of migration
no FD change is expected.

This should be backported as far as 2.2.
2021-07-30 14:21:43 +02:00
Willy Tarreau
177119bb11 BUG/MINOR: poll: fix abnormally high skip_fd counter
The skip_fd counter that is incremented when a migrated FD is reported
was abnormally high in with poll. The reason is that it was accounted
for before preparing the polled events instead of being measured from
the reported events.

This mistake was done when the counters were introduced in 1.9 with
commit d80cb4ee1 ("MINOR: global: add some global activity counters to
help debugging"). It may be backported as far as 2.0.
2021-07-30 14:04:28 +02:00
Willy Tarreau
fcc5281513 BUG/MINOR: select: fix excess number of dead/skip reported
In 1.8, commit ab62f5195 ("MINOR: polling: Use fd_update_events to update
events seen for a fd") updated the pollers to rely on fd_update_events(),
but the modification delayed the test of presence of the FD in the report,
resulting in owner/thread_mask and possibly event updates being performed
for each FD appearing in a block of 32 FDs around an active one. This
caused the request rate to be ~3 times lower with select() than poll()
under 6 threads.

This can be backported as far as 1.8.
2021-07-30 13:55:36 +02:00
Willy Tarreau
c37ccd70b4 BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
A bug was introduced in 2.1-dev2 by commit 305d5ab46 ("MAJOR: fd: Get
rid of the fd cache."). Pollers "poll" and "evport" had the sleeping
bit accidentally removed before the syscall instead of after. This
results in them not being woken up by inter-thread wakeups, which is
particularly visible with the multi-queue accept() and with queues.

As a work-around, when these pollers are used, "nbthread 1" should
be used.

The fact that it has remained broken for 2 years is a great indication
that threads are definitely not enabled outside of epoll and kqueue,
hence why this patch is only tagged medium.

This must be backported as far as 2.2.
2021-07-30 10:57:09 +02:00
Willy Tarreau
6ed242ece6 BUG/MEDIUM: connection: close a rare race between idle conn close and takeover
The takeover of idle conns between threads is particularly tricky, for
two reasons:
  - there's no way to atomically synchronize kernel-side polling with
    userspace activity, so late events will always be reported for some
    FDs just migrated ;

  - upon error, an FD may be immediately reassigned to whatever other
    thread since it's process-wide.

The current model uses the FD's thread_mask to figure if an FD still
ought to be reported or not, and a per-thread idle connection queue
from which eligible connections are atomically added/picked. I/Os
coming from the bottom for such a connection must remove it from the
list so that it's not elected. Same for timeout tasks and iocbs. And
these last ones check their context under the idle_conn lock to judge
if they're still allowed to run.

One rare case was omitted: the wake() callback. This one is rare, it
may serve to notify about finalized connect() calls that are not being
polled, as well as unhandled shutdowns and errors. This callback was
not protected till now because it wasn't seen as sensitive, but there
exists a particular case where it may be called without protectoin in
parallel to a takeover. This happens in the following sequence:

  - thread T1 wants to establish an outgoing connection
  - the connect() call returns EINPROGRESS
  - the poller adds it using epoll_ctl()
  - epoll_wait() reports it, connect() is done. The connection is
    not being marked as actively polled anymore but is still known
    from the poller.
  - the request is sent over that connection using send(), which
    queues to system buffers while data are being delivered
  - the scheduler switches to other tasks
  - the request is physically sent
  - the server responds
  - the stream is notified that send() succeeded, and makes progress,
    trying to recv() from that connection
  - the recv() succeeds, the response is delivered
  - the poller doesn't need to be touched (still no active polling)
  - the scheduler switches to other tasks
  - the server closes the connection
  - the poller on T1 is notified of the SHUTR and starts to call mux->wake()
  - another thread T2 takes over the connection
  - T2 continues to run inside wake() and releases the connection
  - T2 is just dereferencing it.
  - BAM.

The most logical solution here is to surround the call to wake() with an
atomic removal/insert of the connection from/into the idle conns lists.
This way, wake() is guaranteed to run alone. Any other poller reporting
the FD will not have its tid_bit in the thread_mask si will not bother
it. Another thread trying a takeover will not find this connection. A
task or tasklet being woken up late will either be on the same thread,
or be called on another one with a NULL context since it will be the
consequence of previous successful takeover, and will be ignored. Note
that the extra cost of a lock and tree access here have a low overhead
which is totally amortized given that these ones roughly happen 1-2
times per connection at best.

While it was possible to crash the process after 10-100k req using H2
and a hand-refined configuration achieving perfect synchronism between
a long (20+) chain of proxies and a short timeout (1ms), now with that
fix this never happens even after 10M requests.

Many thanks to Olivier for proposing this solution and explaining why
it works.

This should be backported as far as 2.2 (when inter-thread takeover was
introduced). The code in older versions will be found in conn_fd_handler().
A workaround consists in disabling inter-thread pool sharing using:

    tune.idle-pool.shared off
2021-07-30 08:34:38 +02:00
William Lallemand
4f59c67c4f REGTESTS: ssl: ssl_errors.vtc does not work with old openssl version
Disable the new ssl_errors.vtc reg-tests because in does not work
correctly on the CI since it requires a version of OpenSSL which is
compatible with TLSv1.3 and the ciphersuites keyword.
2021-07-29 16:00:24 +02:00
Remi Tricot-Le Breton
54f63836d2 REGTESTS: ssl: Add tests for the connection and SSL error fetches
This reg-test checks that the connection and SSL sample fetches related
to errors are functioning properly. It also tests the proper behaviour
of the default HTTPS log format and of the log-legacy-conn-error option
which enables or disables the output of a special error message in case
of connection failure (otherwise a line following the configured
log-format is output).
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
4a6328f066 MEDIUM: connection: Add option to disable legacy error log
In case of connection failure, a dedicated error message is output,
following the format described in section "Error log format" of the
documentation. These messages cannot be configured through a log-format
option.
This patch adds a new option, "dontloglegacyconnerr", that disables
those error logs when set, and "replaces" them by a regular log line
that follows the configured log-format (thanks to a call to sess_log in
session_kill_embryonic).
The new fc_conn_err sample fetch allows to add the legacy error log
information into a regular log format.
This new option is unset by default so the logging logic will remain the
same until this new option is used.
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
98b930d043 MINOR: ssl: Define a default https log format
This patch adds a new httpslog option and a new HTTP over SSL log-format
that expands the default HTTP format and adds SSL specific information.
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
7c6898ee49 MINOR: ssl: Add new ssl_fc_hsk_err sample fetch
This new sample fetch along the ssl_fc_hsk_err_str fetch contain the
last SSL error of the error stack that occurred during the SSL
handshake (from the frontend's perspective). The errors happening during
the client's certificate verification will still be given by the
ssl_c_err and ssl_c_ca_err fetches. This new fetch will only hold errors
retrieved by the OpenSSL ERR_get_error function.
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
89b65cfd52 MINOR: ssl: Enable error fetches in case of handshake error
The ssl_c_err, ssl_c_ca_err and ssl_c_ca_err_depth sample fetches values
were not recoverable when the connection failed because of the test
"conn->flags & CO_FL_WAIT_XPRT" (which required the connection to be
established). They could then not be used in a log-format since whenever
they would have sent a non-null value, the value fetching was disabled.
This patch ensures that all these values can be fetched in case of
connection failure.
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
3d2093af9b MINOR: connection: Add a connection error code sample fetch
The fc_conn_err and fc_conn_err_str sample fetches give information
about the problem that made the connection fail. This information would
previously only have been given by the error log messages meaning that
thanks to these fetches, the error log can now be included in a custom
log format. The log strings were all found in the conn_err_code_str
function.
2021-07-29 15:40:45 +02:00
Remi Tricot-Le Breton
0aa4130d65 BUG/MINOR: connection: Add missing error labels to conn_err_code_str
The CO_ER_SSL_EARLY_FAILED and CO_ER_CIP_TIMEOUT connection error codes
were missing in the conn_err_code_str switch which converts the error
codes into string.

This patch can be backported on all stable branches.
2021-07-29 15:40:45 +02:00
William Lallemand
df9caeb9ae CLEANUP: mworker: PR_CAP already initialized with alloc_new_proxy()
Remove the PR_CAP initialization in mworker_cli_proxy_create() which is
already done in alloc_new_proxy().
2021-07-29 15:35:48 +02:00
William Lallemand
ae787bad80 CLEANUP: mworker: use the proxy helper functions in mworker_cli_proxy_create()
Cleanup the mworker_cli_proxy_create() function by removing the
allocation and init of the proxy which is done manually, and replace it
by alloc_new_proxy(). Do the same with the free_proxy() function.

This patch also move the insertion at the end of the function.
2021-07-29 15:13:22 +02:00
William Lallemand
e7f74623e4 MINOR: stats: don't output internal proxies (PR_CAP_INT)
Disable the output of the statistics of internal proxies (PR_CAP_INT),
wo we don't rely only on the px->uuid > 0. This will allow to hide more
cleanly the internal proxies in the stats.
2021-07-28 17:45:18 +02:00
William Lallemand
d11c5728b4 MINOR: mworker: the mworker CLI proxy is internal
Sets the mworker CLI proxy as a internal one (PR_CAP_INT) so we could
exlude it from stats and other tests.
2021-07-28 17:40:56 +02:00
William Lallemand
6bb77b9c64 MINOR: proxy: rename PR_CAP_LUA to PR_CAP_INT
This patch renames the proxy capability "LUA" to "INT" so it could be
used for any internal proxy.

Every proxy that are not user defined should use this flag.
2021-07-28 15:51:42 +02:00
Christopher Faulet
b5f7b52968 BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
This part was fixed several times since commit aade4edc1 ("BUG/MEDIUM:
mux-h2: Don't handle pending read0 too early on streams") and there are
still some cases where a read0 event may be ignored because a partial frame
inhibits the event.

Here, we must take care to set H2_CF_END_REACHED flag if a read0 was
received while a partial frame header is received or if the padding length
is missing.

To ease partial frame detection, H2_CF_DEM_SHORT_READ flag is introduced. It
is systematically removed when some data are received and is set when a
partial frame is found or when dbuf buffer is empty. At the end of the
demux, if the connection must be closed ASAP or if data are missing to move
forward, we may acknowledge the pending read0 event, if any. For now,
H2_CF_DEM_SHORT_READ is not part of H2_CF_DEM_BLOCK_ANY mask.

This patch should fix the issue #1328. It must be backported as far as 2.0.
2021-07-27 09:26:02 +02:00
Christopher Faulet
cf30756f0c BUG/MINOR: mux-h1: Be sure to swap H1C to splice mode when rcv_pipe() is called
The splicing does not work anymore because the H1 connection is not swap to
splice mode when rcv_pipe() callback function is called. It is important to
set H1C_F_WANT_SPLICE flag to inhibit data receipt via the buffer
API. Otherwise, because there are always data in the buffer, it is not
possible to use the kernel splicing.

This bug was introduced by the commit 2b861bf72 ("MINOR: mux-h1: clean up
conditions to enabled and disabled splicing").

The patch must be backported to 2.4.
2021-07-26 15:14:35 +02:00
Christopher Faulet
3f35da296e BUG/MINOR: mux-h2: Obey dontlognull option during the preface
If a connection is closed during the preface while no data are received, if
the dontlognull option is set, no log message must be emitted. However, this
will still be handled as a protocol error. Only the log is omitted.

This patch should fix the issue #1336 for H2 sessions. It must be backported
to 2.4 and 2.3 at least, and probably as far as 2.0.
2021-07-26 15:14:35 +02:00
Christopher Faulet
07e10deb36 BUG/MINOR: mux-h1: Obey dontlognull option for empty requests
If a H1 connection is closed while no data are received, if the dontlognull
option is set, no log message must be emitted. Because the H1 multiplexer
handles early errors, it must take care to obey this option. It is true for
400-Bad-Request, 408-Request-Time-out and 501-Not-Implemented
responses. 500-Internal-Server-Error responses are still logged.

This patch should fix the issue #1336 for H1 sessions. It must be backported
to 2.4.
2021-07-26 15:14:35 +02:00
William Lallemand
9def1425ce BUG/MINOR: systemd: must check the configuration using -Ws
When doing a reload with a configuration which requires the
master-worker mode, the configuration check will fail because the check
is not done with -W/-Ws.

Example:
	wla@kikyo:~/haproxy$ ./haproxy -Ws -c -f haproxy.cfg
	Configuration file is valid
	wla@kikyo:~/haproxy$ ./haproxy -c -f haproxy.cfg
	[NOTICE]   (13153) : haproxy version is 2.5-dev2-4567b3-16
	[NOTICE]   (13153) : path to executable is ./haproxy
	[ALERT]    (13153) : config : Can't use a 'program' section without master worker mode.
	[ALERT]    (13153) : config : Fatal errors found in configuration.

This patch fixes the issue by adding -Ws on the check command line.

Must be backported in all stable branches. (The file was previously in
contrib/systemd/haproxy.service.in).
2021-07-26 11:03:54 +02:00
Amaury Denoyelle
2bf5d41ada MINOR: ssl: use __objt_* variant when retrieving counters
Use non-checked function to retrieve listener/server via obj_type. This
is done as a previous obj_type function ensure that the type is well
known and the instance is not NULL.

Incidentally, this should prevent the coverity report from the #1335
github issue which warns about a possible NULL dereference.
2021-07-26 09:59:06 +02:00
Christopher Faulet
1f923391d1 BUG/MINOR: resolvers: Use a null-terminated string to lookup in servers tree
When we evaluate a DNS response item, it may be necessary to look for a
server with a hostname matching the item target into the named servers
tree. To do so, the item target is transformed to a lowercase string. It
must be a null-terminated string. Thus we must explicitly set the trailing
'\0' character.

For a specific resolution, the named servers tree contains all servers using
this resolution with a hostname loaded from a state file. Because of this
bug, same entry may be duplicated because we are unable to find the right
server, assigning this way the item to a free server slot.

This patch should fix the issue #1333. It must be backported as far as 2.2.
2021-07-22 15:03:25 +02:00
Willy Tarreau
b3c4a8f59d BUILD: threads: fix pthread_mutex_unlock when !USE_THREAD
Commit 048368ef6 ("MINOR: deinit: always deinit the init_mutex on
failed initialization") added the missing unlock but forgot to
condition it on USE_THREAD, resulting in a build failure. No
backport is needed.

This addresses oss-fuzz issue 36426.
2021-07-22 14:43:21 +02:00
Willy Tarreau
acff309753 BUG/MINOR: check: fix the condition to validate a port-less server
A config like the below fails to validate because of a bogus test:

  backend b1
      tcp-check connect port 1234
      option tcp-check
      server s1 1.2.3.4 check

[ALERT] (18887) : config : config: proxy 'b1': server 's1' has neither
                  service port nor check port, and a tcp_check rule
                  'connect' with no port information.

A || instead of a && only validates the connect rule when both the
address *and* the port are set. A work around is to set the rule like
this:

      tcp-check connect addr 0:1234 port 1234

This needs to be backported as far as 2.2 (2.0 is OK).
2021-07-22 11:21:33 +02:00
Christopher Faulet
59bab61649 BUG/MINOR: stats: Add missing agent stats on servers
Agent stats were lost during the stats refactoring performed in the 2.4 to
simplify the Prometheus exporter. stats_fill_sv_stats() function must fill
ST_F_AGENT_* and ST_F_LAST_AGT stats.

This patch should fix the issue #1331. It must be backported to 2.4.
2021-07-22 08:47:55 +02:00
Amaury Denoyelle
5fcd428c35 BUG/MEDIUM: ssl_sample: fix segfault for srv samples on invalid request
Some ssl samples cause a segfault when the stream is not instantiated,
for example during an invalid HTTP request. A new check is added to
prevent the stream dereferencing if NULL.

This is the list of the affected samples :
- ssl_s_chain_der
- ssl_s_der
- ssl_s_i_dn
- ssl_s_key_alg
- ssl_s_notafter
- ssl_s_notbefore
- ssl_s_s_dn
- ssl_s_serial
- ssl_s_sha1
- ssl_s_sig_alg
- ssl_s_version

This bug can be reproduced easily by using one of these samples in a
log-format string. Emit an invalid HTTP request with an HTTP client to
trigger the crash.

This bug has been reported in redmine issue 3913.

This must be backported up to 2.2.
2021-07-21 14:23:06 +02:00
David CARLIER
534197c721 BUILD/MINOR: memprof fix macOs build.
this platform has a similar malloc_usable_size too.
2021-07-21 10:22:48 +02:00
Willy Tarreau
3c032f2d4d BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
This undocumented variable is only for internal use, and its sole
presence affects the process' behavior, as shown in bug #1324. It must
not be exported to workers, external checks, nor programs. Let's unset
it before forking programs and workers.

This should be backported as far as 1.8. The worker code might differ
a bit before 2.5 due to the recent removal of multi-process support.
2021-07-21 10:17:02 +02:00
Willy Tarreau
26146194d3 BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
The master-worker code registers an exit handler to deal with configuration
issues during reload, leading to a restart of the master process in wait
mode. But it shouldn't do that when it's expected that the program stops
during config parsing or condition checks, as the reload operation is
unexpectedly called and results in abnormal behavior and even crashes:

  $ HAPROXY_MWORKER_REEXEC=1  ./haproxy -W -c -f /dev/null
  Configuration file is valid
  [NOTICE]   (18418) : haproxy version is 2.5-dev2-ee2420-6
  [NOTICE]   (18418) : path to executable is ./haproxy
  [WARNING]  (18418) : config : Reexecuting Master process in waitpid mode
  Segmentation fault

  $ HAPROXY_MWORKER_REEXEC=1 ./haproxy -W -cc 1
  [NOTICE]   (18412) : haproxy version is 2.5-dev2-ee2420-6
  [NOTICE]   (18412) : path to executable is ./haproxy
  [WARNING]  (18412) : config : Reexecuting Master process in waitpid mode
  [WARNING]  (18412) : config : Reexecuting Master process

Note that the presence of this variable happens by accident when haproxy
is called from within its own programs (see issue #1324), but this should
be the object of a separate fix.

This patch fixes this by preventing the atexit registration in such
situations. This should be backported as far as 1.8. MODE_CHECK_CONDITION
has to be dropped for versions prior to 2.5.
2021-07-21 10:01:36 +02:00
Willy Tarreau
dc70c18ddc BUG/MEDIUM: cfgcond: limit recursion level in the condition expression parser
Oss-fuzz reports in issue 36328 that we can recurse too far by passing
extremely deep expressions to the ".if" parser. I thought we were still
limited to the 1024 chars per line, that would be highly sufficient, but
we don't have any limit now :-/

Let's just pass a maximum recursion counter to the recursive parsers.
It's decremented for each call and the expression fails if it reaches
zero. On the most complex paths it can add 3 levels per parenthesis,
so with a limit of 1024, that's roughly 343 nested sub-expressions that
are supported in the worst case. That's more than sufficient, for just
a few kB of RAM.

No backport is needed.
2021-07-20 18:03:08 +02:00
jenny-cheung
048368ef6f MINOR: deinit: always deinit the init_mutex on failed initialization
The init_mutex was not unlocked in case an error is encountered during
a thread initialization, and the polling loop was aborted during startup.
In practise it does not have any observable effect since an explicit
exit() is placed there, but it could confuse some debugging tools or
some static analysers, so let's release it as expected.

This addresses issue #1326.
2021-07-20 16:38:23 +02:00
Christopher Faulet
b73f653d00 CLEANUP: http_ana: Remove now unused label from http_process_request()
Since last change on HTTP analysers (252412316 "MEDIUM: proxy: remove
long-broken 'option http_proxy'"), http_process_request() may only return
internal errors on failures. Thus the label used to handle bad requests may
be removed.

This patch should fix the issue #1330.
2021-07-19 10:32:17 +02:00
Willy Tarreau
252412316e MEDIUM: proxy: remove long-broken 'option http_proxy'
This option had always been broken in HTX, which means that the first
breakage appeared in 1.9, that it was broken by default in 2.0 and that
no workaround existed starting with 2.1. The way this option works is
praticularly unfit to the rest of the configuration and to the internal
architecture. It had some uses when it was introduced 14 years ago but
nowadays it's possible to do much better and more reliable using a
set of "http-request set-dst" and "http-request set-uri" rules, which
additionally are compatible with DNS resolution (via do-resolve) and
are not exclusive to normal load balancing. The "option-http_proxy"
example config file was updated to reflect this.

The option is still parsed so that an error message gives hints about
what to look for.
2021-07-18 19:35:32 +02:00
Willy Tarreau
f1db20c473 BUG/MINOR: cfgcond: revisit the condition freeing mechanism to avoid a leak
The cfg_free_cond_{term,and,expr}() functions used to take a pointer to
the pointer to be freed in order to replace it with a NULL once done.
But this doesn't cope well with freeing lists as it would require
recursion which the current code tried to avoid.

Let's just change the API to free the area and let the caller set the NULL.

This leak was reported by oss-fuzz (issue 36265).
2021-07-17 18:46:30 +02:00
Willy Tarreau
69a23ae091 BUG/MINOR: arg: free all args on make_arg_list()'s error path
While we do free the array containing the arguments, we do not free
allocated ones. Most of them are unresolved, but strings are allocated
and have to be freed as well. Note that for the sake of not breaking
the args resolution list that might have been set, we still refrain
from doing this if a resolution was already programmed, but for most
common cases (including the ones that can be found in config conditions
and at run time) we're safe.

This may be backported to stable branches, but it relies on the new
free_args() function that was introduced by commit ab213a5b6 ("MINOR:
arg: add a free_args() function to free an args array"), and which is
likely safe to backport as well.

This leak was reported by oss-fuzz (issue 36265).
2021-07-17 18:36:43 +02:00
Willy Tarreau
bccc91d33e [RELEASE] Released version 2.5-dev2
Released version 2.5-dev2 with the following main changes :
    - BUILD/MEDIUM: tcp: set-mark support for OpenBSD
    - DOC: config: use CREATE USER for mysql-check
    - BUG/MINOR: stick-table: fix several printf sign errors dumping tables
    - BUG/MINOR: peers: fix data_type bit computation more than 32 data_types
    - MINOR: stick-table: make skttable_data_cast to use only std types
    - MEDIUM: stick-table: handle arrays of standard types into stick-tables
    - MEDIUM: peers: handle arrays of std types in peers protocol
    - DOC: stick-table: add missing documentation about gpt0 stored type
    - MEDIUM: stick-table: add the new array of gpt data_type
    - MEDIUM: stick-table: make the use of 'gpt' excluding the use of 'gpt0'
    - MEDIUM: stick-table: add the new arrays of gpc and gpc_rate
    - MEDIUM: stick-table: make the use of 'gpc' excluding the use of 'gpc0/1''
    - BUG/MEDIUM: sock: make sure to never miss early connection failures
    - BUG/MINOR: cli: fix server name output in "show fd"
    - Revert "MINOR: tcp-act: Add set-src/set-src-port for "tcp-request content" rules"
    - MEDIUM: stats: include disabled proxies that hold active sessions to stats
    - BUILD: stick-table: shut up invalid "uninitialized" warning in gcc 8.3
    - MINOR: http: implement http_get_scheme
    - MEDIUM: http: implement scheme-based normalization
    - MEDIUM: h1-htx: apply scheme-based normalization on h1 requests
    - MEDIUM: h2: apply scheme-based normalization on h2 requests
    - REGTESTS: add http scheme-based normalization test
    - BUILD: http_htx: fix ci compilation error with isdigit for Windows
    - MINOR: http: implement http uri parser
    - MINOR: http: use http uri parser for scheme
    - MINOR: http: use http uri parser for authority
    - REORG: http_ana: split conditions for monitor-uri in wait for request
    - MINOR: http: use http uri parser for path
    - BUG/MEDIUM: http_ana: fix crash for http_proxy mode during uri rewrite
    - MINOR: mux_h2: define config to disable h2 websocket support
    - CLEANUP: applet: remove unused thread_mask
    - BUG/MINOR: ssl: Default-server configuration ignored by server
    - BUILD: add detection of missing important CFLAGS
    - BUILD: lua: silence a build warning with TCC
    - MINOR: srv: extract tracking server config function
    - MINOR: srv: do not allow to track a dynamic server
    - MEDIUM: server: support track keyword for dynamic servers
    - REGTESTS: test track support for dynamic servers
    - MINOR: init: verify that there is a single word on "-cc"
    - MINOR: init: make -cc support environment variables expansion
    - MINOR: arg: add a free_args() function to free an args array
    - CLEANUP: config: use free_args() to release args array in cfg_eval_condition()
    - CLEANUP: hlua: use free_args() to release args arrays
    - REORG: config: move the condition preprocessing code to its own file
    - MINOR: cfgcond: start to split the condition parser to introduce terms
    - MEDIUM: cfgcond: report invalid trailing chars after expressions
    - MINOR: cfgcond: remerge all arguments into a single line
    - MINOR: cfgcond: support negating conditional expressions
    - MINOR: cfgcond: make the conditional term parser automatically allocate nodes
    - MINOR: cfgcond: insert an expression between the condition and the term
    - MINOR: cfgcond: support terms made of parenthesis around expressions
    - REGTEST: make check_condition.vtc fail as soon as possible
    - REGTESTS: add more complex check conditions to check_conditions.vtc
    - BUG/MEDIUM: init: restore behavior of command-line "-m" for memory limitation
2021-07-17 12:35:11 +02:00