Commit Graph

730 Commits

Author SHA1 Message Date
William Lallemand
9fc6c97fb3 BUG/MINOR: mworker: fix a memleak when execvp() failed
Free next_argv when execvp() failed.

Must be backported as far as 1.8.

Should fix issue #668.
2020-06-08 10:01:13 +02:00
William Lallemand
0041741ef7 BUG/MEDIUM: mworker: fix the reload with an -- option
When HAProxy is started with a '--' option, all following parameters are
considered configuration files. You can't add new options after a '--'.

The current reload system of the master-worker adds extra options at the
end of the arguments list. Which is a problem if HAProxy was started wih
'--'.

This patch fixes the issue by copying the new option at the beginning of
the arguments list instead of the end.

This patch must be backported as far as 1.8.
2020-06-05 14:30:53 +02:00
William Lallemand
a6b3249935 BUG/MINOR: init: -S can have a parameter starting with a dash
There is no reason the -S option can't take an argument which starts with
a -. This limitation must only be used for options that take a
non-finite list of parameters (-sf/-st)

This can be backported only if the previous patch which fixes
copy_argv() is backported too.

Could be backported as far as 1.9.
2020-06-05 14:30:49 +02:00
William Lallemand
4f71d304aa BUG/MINOR: init: -x can have a parameter starting with a dash
There is no reason the -x option can't take an argument which starts with
a -. This limitation must only be used for options that take a
non-finite list of parameters (-sf/-st)

This can be backported only if the previous patch which fixes
copy_argv() is backported too.

Could be backported as far as 1.8.
2020-06-05 14:30:45 +02:00
William Lallemand
df6c5a8ffa BUG/MEDIUM: mworker: fix the copy of options in copy_argv()
The copy_argv() function, which is used to copy and remove some of the
arguments of the command line in order to re-exec() the master process,
is poorly implemented.

The function tries to remove the -x and the -sf/-st options but without
taking into account that some of the options could take a parameter
starting with a dash.

In issue #644, haproxy starts with "-L -xfoo" which is perfectly
correct. However, the re-exec is done without "-xfoo" because the master
tries to remove the "-x" option. Indeed, the copy_argv() function does
not know how much arguments an option can have, and just assume that
everything starting with a dash is an option. So haproxy is exec() with
"-L" but without a parameter, which is wrong and leads to the exit of
the master, with usage().

To fix this issue, copy_argv() must know how much parameters an option
takes, and copy or skip the parameters correctly.

This fix is a first step but it should evolve to a cleaner way of
declaring the options to avoid deduplication of the parsing code, so we
avoid new bugs.

Should be backported with care as far as 1.8, by removing the options
that does not exists in the previous versions.
2020-06-05 14:30:34 +02:00
Willy Tarreau
d645574fd4 MINOR: soft-stop: let the first stopper only signal other threads
When the first thread stops and wakes others up, it's possible some of
them will also start to wake others in parallel. Let's make give this
notification task to the very first one instead since it's enough and
can reduce the amount of needless (though harmless) wakeup calls.
2020-05-13 14:30:25 +02:00
Willy Tarreau
d7a6b2f742 BUG/MINOR: soft-stop: always wake up waiting threads on stopping
Currently the soft-stop can lead to old processes remaining alive for as
long as two seconds after receiving a soft-stop signal. What happens is
that when receiving SIGUSR1, one thread (usually the first one) wakes up,
handles the signal, sets "stopping", goes into runn_poll_loop(), and
discovers that stopping is set, so its also sets itself in the
stopping_thread_mask bit mask. After this it sees that other threads are
not yet willing to stop, so it continues to wait.

From there, other threads which were waiting in poll() expire after one
second on poll timeout and enter run_poll_loop() in turn. That's already
one second of wait time. They discover each in turn that they're stopping
and see that other threads are not yet stopping, so they go back waiting.

After the end of the first second, all threads know they're stopping and
have set their bit in stopping_thread_mask. It's only now that those who
started to wait first wake up again on timeout to discover that all other
ones are stopping, and can now quit. One second later all threads will
have done it and the process will quit.

This is effectively strictly larger than one second and up to two seconds.

What the current patch does is simple, when the first thread stops, it sets
its own bit into stopping_thread_mask then wakes up all other threads to do
also set theirs. This kills the first second which corresponds to the time
to discover the stopping state. Second, when a thread exists, it wakes all
other ones again because some might have gone back sleeping waiting for
"jobs" to go down to zero (i.e. closing the last connection). This kills
the last second of wait time.

Thanks to this, as SIGUSR1 now acts instantly again if there's no active
connection, or it stops immediately after the last connection has left if
one was still present.

This should be backported as far as 2.0.
2020-05-13 14:11:18 +02:00
Christopher Faulet
e5870d872b MAJOR: checks: Implement HTTP check using tcp-check rules
HTTP health-checks are now internally based on tcp-checks. Of course all the
configuration parsing of the "http-check" keyword and the httpchk option has
been rewritten. But the main changes is that now, as for tcp-check ruleset, it
is possible to perform several send/expect sequences into the same
health-checks. Thus the connect rule is now also available from HTTP checks, jst
like set-var, unset-var and comment rules.

Because the request defined by the "option httpchk" line is used for the first
request only, it is now possible to set the method, the uri and the version on a
"http-check send" line.
2020-04-27 09:39:38 +02:00
Christopher Faulet
8892e5d30b BUG/MEDIUM: server/checks: Init server check during config validity check
The options and directives related to the configuration of checks in a backend
may be defined after the servers declarations. So, initialization of the check
of each server must not be performed during configuration parsing, because some
info may be missing. Instead, it must be done during the configuration validity
check.

Thus, callback functions are registered to be called for each server after the
config validity check, one for the server check and another one for the server
agent-check. In addition deinit callback functions are also registered to
release these checks.

This patch should be backported as far as 1.7. But per-server post_check
callback functions are only supported since the 2.1. And the initcall mechanism
does not exist before the 1.9. Finally, in 1.7, the code is totally
different. So the backport will be harder on older versions.
2020-04-27 09:39:37 +02:00
Christopher Faulet
f61f33a1b2 BUG/MINOR: checks: Respect the no-check-ssl option
This options is used to force a non-SSL connection to check a SSL server or to
invert a check-ssl option inherited from the default section. The use_ssl field
in the check structure is used to know if a SSL connection must be used
(use_ssl=1) or not (use_ssl=0). The server configuration is used by default.

The problem is that we cannot distinguish the default case (no specific SSL
check option) and the case of an explicit non-SSL check. In both, use_ssl is set
to 0. So the server configuration is always used. For a SSL server, when
no-check-ssl option is set, the check is still performed using a SSL
configuration.

To fix the bug, instead of a boolean value (0=TCP, 1=SSL), we use a ternary value :

  * 0  = use server config
  * 1  = force SSL
  * -1 = force non-SSL

The same is done for the server parameter. It is not really necessary for
now. But it is a good way to know is the server no-ssl option is set.

In addition, the PR_O_TCPCHK_SSL proxy option is no longer used to set use_ssl
to 1 for a check. Instead the flag is directly tested to prepare or destroy the
server SSL context.

This patch should be backported as far as 1.8.
2020-04-27 09:39:37 +02:00
Christopher Faulet
8acb1284bc MINOR: checks: Add a way to send custom headers and payload during http chekcs
The 'http-check send' directive have been added to add headers and optionnaly a
payload to the request sent during HTTP healthchecks. The request line may be
customized by the "option httpchk" directive but there was not official way to
add extra headers. An old trick consisted to hide these headers at the end of
the version string, on the "option httpchk" line. And it was impossible to add
an extra payload with an "http-check expect" directive because of the
"Connection: close" header appended to the request (See issue #16 for details).

So to make things official and fully support payload additions, the "http-check
send" directive have been added :

    option httpchk POST /status HTTP/1.1

    http-check send hdr Content-Type "application/json;charset=UTF-8" \
        hdr X-test-1 value1 hdr X-test-2 value2 \
        body "{id: 1, field: \"value\"}"

When a payload is defined, the Content-Length header is automatically added. So
chunk-encoded requests are not supported yet. For now, there is no special
validity checks on the extra headers.

This patch is inspired by Kiran Gavali's work. It should fix the issue #16 and
as far as possible, it may be backported, at least as far as 1.8.
2020-04-27 09:39:37 +02:00
Tim Duesterhus
dfad6a41ad MINOR: version: Show uname output in display_version()
This patch adds the sysname, release, version and machine fields from
the uname results to the version output. It intentionally leaves out the
machine name, because it is usually not useful and users might not want to
expose their machine names for privacy reasons.

May be backported if it is considered useful for debugging.
2020-04-18 22:04:29 +02:00
Willy Tarreau
bb1b63c079 MINOR: init: report the compiler version in haproxy -vv
Some portability issues were met a few times in the past depending on
compiler versions, but this one was not reported in haproxy -vv output
while it's trivial to add it. This patch tries to be the most accurate
by explicitly reporting the clang version if detected, otherwise the
gcc version.
2020-04-15 17:00:03 +02:00
Willy Tarreau
3eb10b8e98 MINOR: init: add -dW and "zero-warning" to reject configs with warnings
Since some systems switched to service managers which hide all warnings
by default, some users are not aware of some possibly important warnings
and get caught too late with errors that could have been detected earlier.

This patch adds a new global keyword, "zero-warning" and an equivalent
command-line option "-dW" to refuse to start in case any warning is
detected. It is recommended to use these with configurations that are
managed by humans in order to catch mistakes very early.
2020-04-15 16:42:39 +02:00
Willy Tarreau
bebd212064 MINOR: init: report in "haproxy -c" whether there were warnings or not
This helps quickly checking if the config produces any warning. For
this we reuse the "warned" bit field to add a new WARN_ANY bit that is
set by ha_warning(). The rest of the bit field was also cleaned from
unused bits.
2020-04-15 16:42:00 +02:00
Willy Tarreau
95abd5be9f CLEANUP: haproxy/threads: don't check global_tasks_mask twice
In run_thread_poll_loop() we test both for (global_tasks_mask & tid_bit)
and thread_has_tasks(), but the former is useless since this test is
already part of the latter.
2020-03-23 09:33:32 +01:00
Willy Tarreau
4f46a354e6 BUG/MINOR: haproxy/threads: close a possible race in soft-stop detection
Commit 4b3f27b ("BUG/MINOR: haproxy/threads: try to make all threads
leave together") improved the soft-stop synchronization but it left a
small race open because it looks at tasks_run_queue, which can drop
to zero then back to one while another thread picks the task from the
run queue to insert it into the tasklet_list. The risk is very low but
not null. In addition the condition didn't consider the possible presence
of signals in the queue.

This patch moves the stopping detection just after the "wake" calculation
which already takes care of the various queues' sizes and signals. It
avoids needlessly duplicating these tests.

The bug was discovered during a code review but will probably never be
observed. This fix may be backported to 2.1 and 2.0 along with the commit
above.
2020-03-23 09:27:28 +01:00
Olivier Houchard
dc2f2753e9 MEDIUM: servers: Split the connections into idle, safe, and available.
Revamp the server connection lists. We know have 3 lists :
- idle_conns, which contains idling connections
- safe_conns, which contains idling connections that are safe to use even
for the first request
- available_conns, which contains connections that are not idling, but can
still accept new streams (those are HTTP/2 or fastcgi, and are always
considered safe).
2020-03-19 22:07:33 +01:00
Willy Tarreau
2e8ab6b560 MINOR: use DISGUISE() everywhere we deliberately want to ignore a result
It's more generic and versatile than the previous shut_your_big_mouth_gcc()
that was used to silence annoying warnings as it's not limited to ignoring
syscalls returns only. This allows us to get rid of the aforementioned
function and the shut_your_big_mouth_gcc_int variable, that started to
look ugly in multi-threaded environments.
2020-03-14 11:04:49 +01:00
Willy Tarreau
4b3f27b67f BUG/MINOR: haproxy/threads: try to make all threads leave together
There's a small issue with soft stop combined with the incoming
connection load balancing. A thread may dispatch a connection to
another one at the moment stopping=1 is set, and the second one could
stop by seeing (jobs - unstoppable_jobs) == 0 in run_poll_loop(),
without ever picking these connections from the queue. This is
visible in that it may occasionally cause a connection drop on
reload since no remaining thread will ever pick that connection
anymore.

In order to address this, this patch adds a stopping_thread_mask
variable by which threads acknowledge their willingness to stop
when their runqueue is empty. And all threads will only stop at
this moment, so that if finally some late work arrives in the
thread's queue, it still has a chance to process it.

This should be backported to 2.1 and 2.0.
2020-03-12 19:17:19 +01:00
Willy Tarreau
f8ea00e05e BUG/MINOR: haproxy: always initialize sleeping_thread_mask
Surprizingly the variable was never initialized, though on most
platforms it's zeroed at boot, and it is relatively harmless
anyway since in the worst case the bits are updated around poll().

This was introduced by commit 79321b95a8 and needs to be backported
as far as 1.9.
2020-03-12 19:09:46 +01:00
Olivier Houchard
8676514d4e MINOR: servers: Kill priv_conns.
Remove the list of private connections from server, it has been largely
unused, we only inserted connections in it, but we would never actually
use it.
2020-03-11 19:20:01 +01:00
Willy Tarreau
304e17eb88 MEDIUM: init: always try to push the FD limit when maxconn is set from -m
When a maximum memory setting is passed to haproxy and maxconn is not set
and ulimit-n is not set, it is expected that maxconn will be set to the
highest value permitted by this memory setting, possibly affecting the
FD limit.

When maxconn was changed to be deduced from the current process's FD limit,
the automatic setting above was partially lost because it now remains
limited to the current FD limit in addition to being limited to the
memory usage. For unprivileged processes it does not change anything,
but for privileged processes the difference is important. Indeed, the
previous behavior ensured that the new FD limit could be enforced on
the process as long as the user had the privilege to do so. Now this
does not happen anymore, and some people rely on this for automatic
sizing in VM environments.

This patch implements the ability to verify if the setting will be
enforceable on the process or not. First it computes maxconn based on
the memory limits alone, then checks if the process is willing to accept
them, otherwise tries again by respecting the process' hard limit.

Thanks to this we now have the best of the pre-2.0 behavior and the
current one, in that privileged users will be able to get as high a
maxconn as they need just based on the memory limit, while unprivileged
users will still get as high a setting as permitted by the intersection
of the memory limit and the process' FD limit.

Ideally, after some observation period, this patch along with the
previous one "MINOR: init: move the maxsock calculation code to
compute_ideal_maxsock()" should be backported to 2.1 and 2.0.

Thanks to Baptiste for raising the issue.
2020-03-10 18:08:11 +01:00
Willy Tarreau
a409f30d09 MINOR: init: move the maxsock calculation code to compute_ideal_maxsock()
The maxsock value is currently derived from global.maxconn and a few other
settings, some of which also depend on global.maxconn. This makes it
difficult to check if a limit is already too high or not during the maxconn
automatic sizing.

Let's move this code into a new function, compute_ideal_maxsock() which now
takes a maxconn in argument. It performs the same operations and returns
the maxsock value if global.maxconn were to be set to that value. It now
replaces the previous code to compute maxsock.
2020-03-10 18:08:11 +01:00
Willy Tarreau
52bf839394 BUG/MEDIUM: random: implement a thread-safe and process-safe PRNG
This is the replacement of failed attempt to add thread safety and
per-process sequences of random numbers initally tried with commit
1c306aa84d ("BUG/MEDIUM: random: implement per-thread and per-process
random sequences").

This new version takes a completely different approach and doesn't try
to work around the horrible OS-specific and non-portable random API
anymore. Instead it implements "xoroshiro128**", a reputedly high
quality random number generator, which is one of the many variants of
xorshift, which passes all quality tests and which is described here:

   http://prng.di.unimi.it/

While not cryptographically secure, it is fast and features a 2^128-1
period. It supports fast jumps allowing to cut the period into smaller
non-overlapping sequences, which we use here to support up to 2^32
processes each having their own, non-overlapping sequence of 2^96
numbers (~7*10^28). This is enough to provide 1 billion randoms per
second and per process for 2200 billion years.

The implementation was made thread-safe either by using a double 64-bit
CAS on platforms supporting it (x86_64, aarch64) or by using a local
lock for the time needed to perform the shift operations. This ensures
that all threads pick numbers from the same pool so that it is not
needed to assign per-thread ranges. For processes we use the fast jump
method to advance the sequence by 2^96 for each process.

Before this patch, the following config:
    global
        nbproc 8

    frontend f
        bind :4445
        mode http
        log stdout format raw daemon
        log-format "%[uuid] %pid"
        redirect location /

Would produce this output:
    a4d0ad64-2645-4b74-b894-48acce0669af 12987
    a4d0ad64-2645-4b74-b894-48acce0669af 12992
    a4d0ad64-2645-4b74-b894-48acce0669af 12986
    a4d0ad64-2645-4b74-b894-48acce0669af 12988
    a4d0ad64-2645-4b74-b894-48acce0669af 12991
    a4d0ad64-2645-4b74-b894-48acce0669af 12989
    a4d0ad64-2645-4b74-b894-48acce0669af 12990
    82d5f6cd-f6c1-4f85-a89c-36ae85d26fb9 12987
    82d5f6cd-f6c1-4f85-a89c-36ae85d26fb9 12992
    82d5f6cd-f6c1-4f85-a89c-36ae85d26fb9 12986
    (...)

And now produces:
    f94b29b3-da74-4e03-a0c5-a532c635bad9 13011
    47470c02-4862-4c33-80e7-a952899570e5 13014
    86332123-539a-47bf-853f-8c8ea8b2a2b5 13013
    8f9efa99-3143-47b2-83cf-d618c8dea711 13012
    3cc0f5c7-d790-496b-8d39-bec77647af5b 13015
    3ec64915-8f95-4374-9e66-e777dc8791e0 13009
    0f9bf894-dcde-408c-b094-6e0bb3255452 13011
    49c7bfde-3ffb-40e9-9a8d-8084d650ed8f 13014
    e23f6f2e-35c5-4433-a294-b790ab902653 13012

There are multiple benefits to using this method. First, it doesn't
depend anymore on a non-portable API. Second it's thread safe. Third it
is fast and more proven than any hack we could attempt to try to work
around the deficiencies of the various implementations around.

This commit depends on previous patches "MINOR: tools: add 64-bit rotate
operators" and "BUG/MEDIUM: random: initialize the random pool a bit
better", all of which will need to be backported at least as far as
version 2.0. It doesn't require to backport the build fixes for circular
include files dependecy anymore.
2020-03-08 10:09:02 +01:00
Willy Tarreau
0fbf28a05b Revert "BUG/MEDIUM: random: implement per-thread and per-process random sequences"
This reverts commit 1c306aa84d.

It breaks the build on all non-glibc platforms. I got confused by the
man page (which possibly is the most confusing man page I've ever read
about a standard libc function) and mistakenly understood that random_r
was portable, especially since it appears in latest freebsd source as
well but not in released versions, and with a slightly different API :-/

We need to find a different solution with a fallback. Among the
possibilities, we may reintroduce this one with a fallback relying on
locking around the standard functions, keeping fingers crossed for no
other library function to call them in parallel, or we may also provide
our own PRNG, which is not necessarily more difficult than working
around the totally broken up design of the portable API.
2020-03-07 11:24:39 +01:00
Willy Tarreau
1c306aa84d BUG/MEDIUM: random: implement per-thread and per-process random sequences
As mentioned in previous patch, the random number generator was never
made thread-safe, which used not to be a problem for health checks
spreading, until the uuid sample fetch function appeared. Currently
it is possible for two threads or processes to produce exactly the
same UUID. In fact it's extremely likely that this will happen for
processes, as can be seen with this config:

    global
        nbproc 8

    frontend f
        bind :4445
        mode http
        log stdout daemon format raw
        log-format "%[uuid] %pid"
        redirect location /

It typically produces this log:

  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30645
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30641
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30644
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30639
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30646
  07764439-c24d-4e6f-a5a6-0138be59e7a8 30645
  07764439-c24d-4e6f-a5a6-0138be59e7a8 30639
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30643
  07764439-c24d-4e6f-a5a6-0138be59e7a8 30646
  b6773fdd-678f-4d04-96f2-4fb11ad15d6b 30646
  551ce567-0bfb-4bbd-9b58-cdc7e9365325 30642
  07764439-c24d-4e6f-a5a6-0138be59e7a8 30642

What this patch does is to use a distinct per-thread and per-process
seed to make sure the same sequences will not appear, and will then
extend these seeds by "burning" a number of randoms that depends on
the global random seed, the thread ID and the process ID. This adds
roughly 20 extra bits of randomness, resulting in 52 bits total per
thread and per process.

It only takes a few milliseconds to burn these randoms and given
that threads start with a different seed, we know they will not
catch each other. So these random extra bits are essentially added
to ensure randomness between boots and cluster instances.

This replaces all uses of random() with ha_random() which uses the
thread-local state.

This must be backported as far as 2.0 or any version having the
UUID sample-fetch function since it's the main victim here.

It's important to note that this patch, in addition to depending on
the previous one "BUG/MEDIUM: init: initialize the random pool a bit
better", also depends on the preceeding build fixes to address a
circular dependency issue in the include files that prevented it
from building. Part or all of these patches may need to be backported
or adapted as well.
2020-03-07 06:11:15 +01:00
Willy Tarreau
6c3a681bd6 BUG/MEDIUM: random: initialize the random pool a bit better
Since the UUID sample fetch was created, some people noticed that in
certain virtualized environments they manage to get exact same UUIDs
on different instances started exactly at the same moment. It turns
out that the randoms were only initialized to spread the health checks
originally, not to provide "clean" randoms.

This patch changes this and collects more randomness from various
sources, including existing randoms, /dev/urandom when available,
RAND_bytes() when OpenSSL is available, as well as the timing for such
operations, then applies a SHA1 on all this to keep a 160 bits random
seed available, 32 of which are passed to srandom().

It's worth mentioning that there's no clean way to pass more than 32
bits to srandom() as even initstate() provides an opaque state that
must absolutely not be tampered with since known implementations
contain state information.

At least this allows to have up to 4 billion different sequences
from the boot, which is not that bad.

Note that the thread safety was still not addressed, which is another
issue for another patch.

This must be backported to all versions containing the UUID sample
fetch function, i.e. as far as 2.0.
2020-03-07 06:11:11 +01:00
Willy Tarreau
b1beaa302c BUG/MINOR: init: make the automatic maxconn consider the max of soft/hard limits
James Stroehmann reported something working as documented but that can
be considered as a regression in the way the automatic maxconn is
calculated from the process' limits :

  https://www.mail-archive.com/haproxy@formilux.org/msg36523.html

The purpose of the changes in 2.0 was to have maxconn default to the
highest possible value permitted to the user based on the ulimit -n
setting, however the calculation starts from the soft limit, which
can be lower than what users were allowed to with previous versions
where the default value of 2000 would force a higher ulimit -n as
long as it fitted in the hard limit.

Usually this is not noticeable if the user changes the limits, because
quite commonly setting a new value restricts both the soft and hard
values.

Let's instead always use the max between the hard and soft limits, as
we know these values are permitted. This was tried on the following
setup:

  $ cat ulimit-n.cfg
  global
    stats socket /tmp/sock1 level admin
  $ ulimit -n
  1024

Before the change the limits would show like this:

  $ socat - /tmp/sock1 <<< "show info" | grep -im2 ^Max
  Maxsock: 1023
  Maxconn: 489

After the change the limits are now much better and more in line with
the default settings in earlier versions:

  $ socat - /tmp/sock1 <<< "show info" | grep -im2 ^Max
  Maxsock: 4095
  Maxconn: 2025

The difference becomes even more obvious when running moderately large
configs with hundreds of checked servers and hundreds of listeners:

  $ cat ulimit-n.cfg
  global
    stats socket /tmp/sock1 level admin

  listen l
    bind :10000-10300
    server-template srv- 300 0.0.0.0 check disabled

          Before   After
  Maxsock  1024    4096
  Maxconn  189     1725

This issue is tagged as minor since a trivial config change fixes it,
but it would help new users to have it backported as far as 2.0.
2020-03-06 10:49:55 +01:00
Carl Henrik Lunde
f91ac19299 OPTIM: startup: fast unique_id allocation for acl.
pattern_finalize_config() uses an inefficient algorithm which is a
problem with very large configuration files. This affects startup, and
therefore reload time. When haproxy is deployed as a router in a
Kubernetes cluster the generated configuration file may be large and
reloads are frequently occuring, which makes this a significant issue.

The old algorithm is O(n^2)
* allocate missing uids - O(n^2)
* sort linked list - O(n^2)

The new algorithm is O(n log n):
* find the user allocated uids - O(n)
* store them for efficient lookup - O(n log n)
* allocate missing uids - n times O(log n)
* sort all uids - O(n log n)
* convert back to linked list - O(n)

Performance examples, startup time in seconds:

    pat_refs old     new
    1000      0.02   0.01
    10000     2.1    0.04
    20000    12.3    0.07
    30000    27.9    0.10
    40000    52.5    0.14
    50000    77.5    0.17

Please backport to 1.8, 2.0 and 2.1.
2020-03-06 08:11:58 +01:00
Willy Tarreau
3ebd55ee51 MINOR: haproxy: export run_poll_loop
This will help refine debug traces.
2020-03-03 15:26:10 +01:00
Willy Tarreau
908071171b BUILD: general: always pass unsigned chars to is* functions
The isalnum(), isalpha(), isdigit() etc functions from ctype.h are
supposed to take an int in argument which must either reflect an
unsigned char or EOF. In practice on some platforms they're implemented
as macros referencing an array, and when passed a char, they either cause
a warning "array subscript has type 'char'" when lucky, or cause random
segfaults when unlucky. It's quite unconvenient by the way since none of
them may return true for negative values. The recent introduction of
cygwin to the list of regularly tested build platforms revealed a lot
of breakage there due to the same issues again.

So this patch addresses the problem all over the code at once. It adds
unsigned char casts to every valid use case, and also drops the unneeded
double cast to int that was sometimes added on top of it.

It may be backported by dropping irrelevant changes if that helps better
support uncommon platforms. It's unlikely to fix bugs on platforms which
would already not emit any warning though.
2020-02-25 08:16:33 +01:00
Christopher Faulet
6d0c3dfac6 MEDIUM: http: Add a ruleset evaluated on all responses just before forwarding
This patch introduces the 'http-after-response' rules. These rules are evaluated
at the end of the response analysis, just before the data forwarding, on ALL
HTTP responses, the server ones but also all responses generated by
HAProxy. Thanks to this ruleset, it is now possible for instance to add some
headers to the responses generated by the stats applet. Following actions are
supported :

   * allow
   * add-header
   * del-header
   * replace-header
   * replace-value
   * set-header
   * set-status
   * set-var
   * strict-mode
   * unset-var
2020-02-06 14:55:34 +01:00
Christopher Faulet
546c4696bb MINOR: global: Set default tune.maxrewrite value during global structure init
When the global structure is initialized, instead of setting tune.maxrewrite to
-1, its default value can be immediately set. This way, it is always defined
during the configuration validity check. Otherwise, the only way to have it at
this stage, it is to explicity set it in the global section.
2020-02-06 09:36:36 +01:00
Willy Tarreau
71f95fa20e [RELEASE] Released version 2.2-dev1
Released version 2.2-dev1 with the following main changes :
    - DOC: this is development again
    - MINOR: version: this is development again, update the status
    - SCRIPTS: update create-release to fix the changelog on new branches
    - CLEANUP: ssl: Clean up error handling
    - BUG/MINOR: contrib/prometheus-exporter: decode parameter and value only
    - BUG/MINOR: h1: Don't test the host header during response parsing
    - BUILD/MINOR: trace: fix use of long type in a few printf format strings
    - DOC: Clarify behavior of server maxconn in HTTP mode
    - MINOR: ssl: deduplicate ca-file
    - MINOR: ssl: compute ca-list from deduplicate ca-file
    - MINOR: ssl: deduplicate crl-file
    - CLEANUP: dns: resolution can never be null
    - BUG/MINOR: http-htx: Don't make http_find_header() fail if the value is empty
    - DOC: ssl/cli: set/commit/abort ssl cert
    - BUG/MINOR: ssl: fix SSL_CTX_set1_chain compatibility for openssl < 1.0.2
    - BUG/MINOR: fcgi-app: Make the directive pass-header case insensitive
    - BUG/MINOR: stats: Fix HTML output for the frontends heading
    - BUG/MINOR: ssl: fix X509 compatibility for openssl < 1.1.0
    - DOC: clarify matching strings on binary fetches
    - DOC: Fix ordered list in summary
    - DOC: move the "group" keyword at the right place
    - MEDIUM: init: prevent process and thread creation at runtime
    - BUG/MINOR: ssl/cli: 'ssl cert' cmd only usable w/ admin rights
    - BUG/MEDIUM: stream-int: don't subscribed for recv when we're trying to flush data
    - BUG/MINOR: stream-int: avoid calling rcv_buf() when splicing is still possible
    - BUG/MINOR: ssl/cli: don't overwrite the filters variable
    - BUG/MEDIUM: listener/thread: fix a race when pausing a listener
    - BUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1
    - BUG/MEDIUM: mux-h1: Never reuse H1 connection if a shutw is pending
    - BUG/MINOR: mux-h1: Don't rely on CO_FL_SOCK_RD_SH to set H1C_F_CS_SHUTDOWN
    - BUG/MINOR: mux-h1: Fix conditions to know whether or not we may receive data
    - BUG/MEDIUM: tasks: Make sure we switch wait queues in task_set_affinity().
    - BUG/MEDIUM: checks: Make sure we set the task affinity just before connecting.
    - MINOR: debug: replace popen() with pipe+fork() in "debug dev exec"
    - MEDIUM: init: set NO_NEW_PRIVS by default when supported
    - BUG/MINOR: mux-h1: Be sure to set CS_FL_WANT_ROOM when EOM can't be added
    - BUG/MEDIUM: mux-fcgi: Handle cases where the HTX EOM block cannot be inserted
    - BUG/MINOR: proxy: make soft_stop() also close FDs in LI_PAUSED state
    - BUG/MINOR: listener/threads: always use atomic ops to clear the FD events
    - BUG/MINOR: listener: also clear the error flag on a paused listener
    - BUG/MEDIUM: listener/threads: fix a remaining race in the listener's accept()
    - MINOR: listener: make the wait paths cleaner and more reliable
    - MINOR: listener: split dequeue_all_listener() in two
    - REORG: listener: move the global listener queue code to listener.c
    - DOC: document the listener state transitions
    - BUG/MEDIUM: kqueue: Make sure we report read events even when no data.
    - BUG/MAJOR: dns: add minimalist error processing on the Rx path
    - BUG/MEDIUM: proto_udp/threads: recv() and send() must not be exclusive.
    - DOC: listeners: add a few missing transitions
    - BUG/MINOR: tasks: only requeue a task if it was already in the queue
    - MINOR: tasks: split wake_expired_tasks() in two parts to avoid useless wakeups
    - DOC: proxies: HAProxy only supports 3 connection modes
    - DOC: remove references to the outdated architecture.txt
    - BUG/MINOR: log: fix minor resource leaks on logformat error path
    - BUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers
    - BUG/MINOR: listener: do not immediately resume on transient error
    - BUG/MINOR: server: make "agent-addr" work on default-server line
    - BUG/MINOR: listener: fix off-by-one in state name check
    - BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()
    - MEDIUM: h1-htx: Add HTX EOM block when the message is in H1_MSG_DONE state
    - MINOR: http-htx: Add some htx sample fetches for debugging purpose
    - REGTEST: Add an HTX reg-test to check an edge case
    - DOC: clarify the fact that replace-uri works on a full URI
    - BUG/MINOR: sample: fix the closing bracket and LF in the debug converter
    - BUG/MINOR: sample: always check converters' arguments
    - MINOR: sample: Validate the number of bits for the sha2 converter
    - BUG/MEDIUM: ssl: Don't set the max early data we can receive too early.
    - MINOR: ssl/cli: 'show ssl cert' give information on the certificates
    - BUG/MINOR: ssl/cli: fix build for openssl < 1.0.2
    - MINOR: debug: support logging to various sinks
    - MINOR: http: add a new "replace-path" action
    - REGTEST: ssl: test the "set ssl cert" CLI command
    - REGTEST: run-regtests: implement #REQUIRE_BINARIES
    - MINOR: task: only check TASK_WOKEN_ANY to decide to requeue a task
    - BUG/MAJOR: task: add a new TASK_SHARED_WQ flag to fix foreing requeuing
    - BUG/MEDIUM: ssl: Revamp the way early data are handled.
    - MINOR: fd/threads: make _GET_NEXT()/_GET_PREV() use the volatile attribute
    - BUG/MEDIUM: fd/threads: fix a concurrency issue between add and rm on the same fd
    - REGTEST: make the "set ssl cert" require version 2.1
    - BUG/MINOR: ssl: openssl-compat: Fix getm_ defines
    - BUG/MEDIUM: state-file: do not allocate a full buffer for each server entry
    - BUG/MINOR: state-file: do not store duplicates in the global tree
    - BUG/MINOR: state-file: do not leak memory on parse errors
    - BUG/MAJOR: mux-h1: Don't pretend the input channel's buffer is full if empty
    - BUG/MEDIUM: stream: Be sure to never assign a TCP backend to an HTX stream
    - BUILD: ssl: improve SSL_CTX_set_ecdh_auto compatibility
    - BUILD: travis-ci: link with ssl libraries using rpath instead of LD_LIBRARY_PATH/DYLD_LIBRARY_PATH
    - BUILD: travis-ci: reenable address sanitizer for clang builds
    - BUG/MINOR: checks: refine which errno values are really errors.
    - BUG/MINOR: connection: only wake send/recv callbacks if the FD is active
    - CLEANUP: connection: conn->xprt is never NULL
    - MINOR: pollers: add a new flag to indicate pollers reporting ERR & HUP
    - MEDIUM: tcp: make tcp_connect_probe() consider ERR/HUP
    - REORG: connection: move tcp_connect_probe() to conn_fd_check()
    - MINOR: connection: check for connection validation earlier
    - MINOR: connection: remove the double test on xprt_done_cb()
    - CLEANUP: connection: merge CO_FL_NOTIFY_DATA and CO_FL_NOTIFY_DONE
    - MINOR: poller: do not call the IO handler if the FD is not active
    - OPTIM: epoll: always poll for recv if neither active nor ready
    - OPTIM: polling: do not create update entries for FD removal
    - BUG/MEDIUM: checks: Only attempt to do handshakes if the connection is ready.
    - BUG/MEDIUM: connections: Hold the lock when wanting to kill a connection.
    - BUILD: CI: modernize cirrus-ci
    - MINOR: config: disable busy polling on old processes
    - MINOR: ssl: Remove unused variable "need_out".
    - BUG/MINOR: h1: Report the right error position when a header value is invalid
    - BUG/MINOR: proxy: Fix input data copy when an error is captured
    - BUG/MEDIUM: http-ana: Truncate the response when a redirect rule is applied
    - BUG/MINOR: channel: inject output data at the end of output
    - BUG/MEDIUM: session: do not report a failure when rejecting a session
    - MEDIUM: dns: implement synchronous send
    - MINOR: raw_sock: make sure to disable polling once everything is sent
    - MINOR: http: Add 410 to http-request deny
    - MINOR: http: Add 404 to http-request deny
    - CLEANUP: mux-h2: remove unused goto "out_free_h2s"
    - BUILD: cirrus-ci: choose proper openssl package name
    - BUG/MAJOR: listener: do not schedule a task-less proxy
    - CLEANUP: server: remove unused err section in server_finalize_init
    - REGTEST: set_ssl_cert.vtc: replace "echo" with "printf"
    - BUG/MINOR: stream-int: Don't trigger L7 retry if max retries is already reached
    - BUG/MEDIUM: tasks: Use the MT macros in tasklet_free().
    - BUG/MINOR: mux-h2: use a safe list_for_each_entry in h2_send()
    - BUG/MEDIUM: mux-h2: fix missing test on sending_list in previous patch
    - CLEANUP: ssl: remove opendir call in ssl_sock_load_cert
    - MEDIUM: lua: don't call the GC as often when dealing with outgoing connections
    - BUG/MEDIUM: mux-h2: don't stop sending when crossing a buffer boundary
    - BUG/MINOR: cli/mworker: can't start haproxy with 2 programs
    - REGTEST: mcli/mcli_start_progs: start 2 programs
    - BUG/MEDIUM: mworker: remain in mworker mode during reload
    - DOC: clarify crt-base usage
    - CLEANUP: compression: remove unused deinit_comp_ctx section
    - BUG/MEDIUM: mux_h1: Don't call h1_send if we subscribed().
    - BUG/MEDIUM: raw_sock: Make sur the fd and conn are sync.
    - CLEANUP: proxy: simplify proxy_parse_rate_limit proxy checks
    - BUG/MAJOR: hashes: fix the signedness of the hash inputs
    - REGTEST: add sample_fetches/hashes.vtc to validate hashes
    - BUG/MEDIUM: cli: _getsocks must send the peers sockets
    - CLEANUP: cli: deduplicate the code in _getsocks
    - BUG/MINOR: stream: don't mistake match rules for store-request rules
    - BUG/MEDIUM: connection: add a mux flag to indicate splice usability
    - BUG/MINOR: pattern: handle errors from fgets when trying to load patterns
    - MINOR: connection: move the CO_FL_WAIT_ROOM cleanup to the reader only
    - MINOR: stream-int: remove dependency on CO_FL_WAIT_ROOM for rcv_buf()
    - MEDIUM: connection: get rid of CO_FL_CURR_* flags
    - BUILD: pattern: include errno.h
    - MEDIUM: mux-h2: do not try to stop sending streams on blocked mux
    - MEDIUM: mux-fcgi: do not try to stop sending streams on blocked mux
    - MEDIUM: mux-h2: do not make an h2s subscribe to itself on deferred shut
    - MEDIUM: mux-fcgi: do not make an fstrm subscribe to itself on deferred shut
    - REORG: stream/backend: move backend-specific stuff to backend.c
    - MEDIUM: backend: move the connection finalization step to back_handle_st_con()
    - MEDIUM: connection: merge the send_wait and recv_wait entries
    - MEDIUM: xprt: merge recv_wait and send_wait in xprt_handshake
    - MEDIUM: ssl: merge recv_wait and send_wait in ssl_sock
    - MEDIUM: mux-h1: merge recv_wait and send_wait
    - MEDIUM: mux-h2: merge recv_wait and send_wait event notifications
    - MEDIUM: mux-fcgi: merge recv_wait and send_wait event notifications
    - MINOR: connection: make the last arg of subscribe() a struct wait_event*
    - MINOR: ssl: Add support for returning the dn samples from ssl_(c|f)_(i|s)_dn in LDAP v3 (RFC2253) format.
    - DOC: Fix copy and paste mistake in http-response replace-value doc
    - BUG/MINOR: cache: Fix leak of cache name in error path
    - BUG/MINOR: dns: Make dns_query_id_seed unsigned
    - BUG/MINOR: 51d: Fix bug when HTX is enabled
    - MINOR: http-htx: Move htx sample fetches in the scope "internal"
    - MINOR: http-htx: Rename 'internal.htx_blk.val' to 'internal.htx_blk.data'
    - MINOR: http-htx: Make 'internal.htx_blk_data' return a binary string
    - DOC: Add a section to document the internal sample fetches
    - MINOR: mux-h1: Inherit send flags from the upper layer
    - MINOR: contrib/prometheus-exporter: Add heathcheck status/code in server metrics
    - BUG/MINOR: http-ana/filters: Wait end of the http_end callback for all filters
    - BUG/MINOR: http-rules: Remove buggy deinit functions for HTTP rules
    - BUG/MINOR: stick-table: Use MAX_SESS_STKCTR as the max track ID during parsing
    - MEDIUM: http-rules: Register an action keyword for all http rules
    - MINOR: tcp-rules: Always set from which ruleset a rule comes from
    - MINOR: actions: Use ACT_RET_CONT code to ignore an error from a custom action
    - MINOR: tcp-rules: Kill connections when custom actions return ACT_RET_ERR
    - MINOR: http-rules: Return an error when custom actions return ACT_RET_ERR
    - MINOR: counters: Add a counter to report internal processing errors
    - MEDIUM: http-ana: Properly handle internal processing errors
    - MINOR: http-rules: Add a rule result to report internal error
    - MINOR: http-rules: Handle internal errors during HTTP rules evaluation
    - MINOR: http-rules: Add more return codes to let custom actions act as normal ones
    - MINOR: tcp-rules: Handle denied/aborted/invalid connections from TCP rules
    - MINOR: http-rules: Handle denied/aborted/invalid connections from HTTP rules
    - MINOR: stats: Report internal errors in the proxies/listeners/servers stats
    - MINOR: contrib/prometheus-exporter: Export internal errors per proxy/server
    - MINOR: counters: Remove failed_secu counter and use denied_resp instead
    - MINOR: counters: Review conditions to increment counters from analysers
    - MINOR: http-ana: Add a txn flag to support soft/strict message rewrites
    - MINOR: http-rules: Handle all message rewrites the same way
    - MINOR: http-rules: Add a rule to enable or disable the strict rewriting mode
    - MEDIUM: http-rules: Enable the strict rewriting mode by default
    - REGTEST: Fix format of set-uri HTTP request rule in h1or2_to_h1c.vtc
    - MINOR: actions: Add a function pointer to release args used by actions
    - MINOR: actions: Regroup some info about HTTP rules in the same struct
    - MINOR: http-rules/tcp-rules: Call the defined action function first if defined
    - MINOR: actions: Rename the act_flag enum into act_opt
    - MINOR: actions: Add flags to configure the action behaviour
    - MINOR: actions: Use an integer to set the action type
    - MINOR: http-rules: Use a specific action type for some custom HTTP actions
    - MINOR: http-rules: Make replace-header and replace-value custom actions
    - MINOR: http-rules: Make set-header and add-header custom actions
    - MINOR: http-rules: Make set/del-map and add/del-acl custom actions
    - MINOR: http-rules: Group all processing of early-hint rule in its case clause
    - MEDIUM: http-rules: Make early-hint custom actions
    - MINOR: http-rule/tcp-rules: Make track-sc* custom actions
    - MINOR: tcp-rules: Make tcp-request capture a custom action
    - MINOR: http-rules: Add release functions for existing HTTP actions
    - BUG/MINOR: http-rules: Fix memory releases on error path during action parsing
    - MINOR: tcp-rules: Add release functions for existing TCP actions
    - BUG/MINOR: tcp-rules: Fix memory releases on error path during action parsing
    - MINOR: http-htx: Add functions to read a raw error file and convert it in HTX
    - MINOR: http-htx: Add functions to create HTX redirect message
    - MINOR: config: Use dedicated function to parse proxy's errorfiles
    - MINOR: config: Use dedicated function to parse proxy's errorloc
    - MEDIUM: http-htx/proxy: Use a global and centralized storage for HTTP error messages
    - MINOR: proxy: Register keywords to parse errorfile and errorloc directives
    - MINOR: http-htx: Add a new section to create groups of custom HTTP errors
    - MEDIUM: proxy: Add a directive to reference an http-errors section in a proxy
    - MINOR: http-rules: Update txn flags and status when a deny rule is executed
    - MINOR: http-rules: Support an optional status on deny rules for http reponses
    - MINOR: http-rules: Use same function to parse request and response deny actions
    - MINOR: http-ana: Add an error message in the txn and send it when defined
    - MEDIUM: http-rules: Support an optional error message in http deny rules
    - REGTEST: Add a strict rewriting mode reg test
    - REGEST: Add reg tests about error files
    - MINOR: ssl: accept 'verify' bind option with 'set ssl cert'
    - BUG/MINOR: ssl: ssl_sock_load_ocsp_response_from_file memory leak
    - BUG/MINOR: ssl: ssl_sock_load_issuer_file_into_ckch memory leak
    - BUG/MINOR: ssl: ssl_sock_load_sctl_from_file memory leak
    - BUG/MINOR: http_htx: Fix some leaks on error path when error files are loaded
    - CLEANUP: http-ana: Remove useless test on txn when the error message is retrieved
    - BUILD: CI: introduce ARM64 builds
    - BUILD: ssl: more elegant anti-replay feature presence check
    - MINOR: proxy/http-ana: Add support of extra attributes for the cookie directive
    - MEDIUM: dns: use Additional records from SRV responses
    - CLEANUP: Consistently `unsigned int` for bitfields
    - CLEANUP: pattern: remove the pat_time definition
    - BUG/MINOR: http_act: don't check capture id in backend
    - BUG/MINOR: ssl: fix build on development versions of openssl-1.1.x
2020-01-22 10:34:58 +01:00
Christopher Faulet
2f5339079b MINOR: proxy/http-ana: Add support of extra attributes for the cookie directive
It is now possible to insert any attribute when a cookie is inserted by
HAProxy. Any value may be set, no check is performed except the syntax validity
(CTRL chars and ';' are forbidden). For instance, it may be used to add the
SameSite attribute:

    cookie SRV insert attr "SameSite=Strict"

The attr option may be repeated to add several attributes.

This patch should fix the issue #361.
2020-01-22 07:18:31 +01:00
Christopher Faulet
5885775de1 MEDIUM: http-htx/proxy: Use a global and centralized storage for HTTP error messages
All custom HTTP errors are now stored in a global tree. Proxies use a references
on these messages. The key used for errorfile directives is the file name as
specified in the configuration. For errorloc directives, a key is created using
the redirect code and the url. This means that the same custom error message is
now stored only once. It may be used in several proxies or for several status
code, it is only parsed and stored once.
2020-01-20 15:18:46 +01:00
Christopher Faulet
58b3564fde MINOR: actions: Add a function pointer to release args used by actions
Arguments used by actions are never released during HAProxy deinit. Now, it is
possible to specify a function to do so. ".release_ptr" field in the act_rule
structure may be set during the configuration parsing to a specific deinit
function depending on the action type.
2020-01-20 15:18:45 +01:00
Christopher Faulet
cb5501327c BUG/MINOR: http-rules: Remove buggy deinit functions for HTTP rules
Functions to deinitialize the HTTP rules are buggy. These functions does not
check the action name to release the right part in the arg union. Only few info
are released. For auth rules, the realm is released and there is no problem
here. But the regex <arg.hdr_add.re> is always unconditionally released. So it
is easy to make these functions crash. For instance, with the following rule
HAProxy crashes during the deinit :

      http-request set-map(/path/to/map) %[src] %[req.hdr(X-Value)]

For now, These functions are simply removed and we rely on the deinit function
used for TCP rules (renamed as deinit_act_rules()). This patch fixes the
bug. But arguments used by actions are not released at all, this part will be
addressed later.

This patch must be backported to all stable versions.
2020-01-20 15:18:45 +01:00
William Lallemand
24c928c8bd BUG/MEDIUM: mworker: remain in mworker mode during reload
If you reload an haproxy started in master-worker mode with
"master-worker" in the configuration, and no "-W" argument,
the new process lost the fact that is was in master-worker mode
resulting in weird behaviors.

The bigest problem is that if it is reloaded with an bad configuration,
the master will exits instead of remaining in waitpid mode.

This problem was discovered in bug #443.

Should be backported in every version using the master-worker mode.
(as far as 1.8)
2020-01-14 18:10:29 +01:00
Willy Tarreau
719e07c989 BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()
Apparently gcc developers decided that strncpy() semantics are no longer
valid and now deserve a warning, especially if used exactly as designed.
This results in issue #304. Let's just remove one to the target size to
please her majesty gcc, the God of C Compilers, who tries hard to make
users completely eliminate any use of string.h and reimplement it by
themselves at much higher risks. Pfff....

This can be backported to stable version, the fix is harmless since it
ignores the last zero that is already set on next line.
2019-12-11 16:29:10 +01:00
Willy Tarreau
d26c9f9465 BUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers
If a new process is started with -sf and it fails to bind, it may send
a SIGTTOU to the master process in hope that it will temporarily unbind.
Unfortunately this one doesn't catch it and stops to background instead
of forwarding the signal to the workers. The same is true for SIGTTIN.

This commit simply implements an extra signal handler for the master to
deal with such signals that must be passed down to the workers. It must
be backported as far as 1.8, though there the code differs in that it's
entirely in haproxy.c and doesn't require an extra sig handler.
2019-12-11 14:26:53 +01:00
Willy Tarreau
c49ba52524 MINOR: tasks: split wake_expired_tasks() in two parts to avoid useless wakeups
We used to have wake_expired_tasks() wake up tasks and return the next
expiration delay. The problem this causes is that we have to call it just
before poll() in order to consider latest timers, but this also means that
we don't wake up all newly expired tasks upon return from poll(), which
thus systematically requires a second poll() round.

This is visible when running any scheduled task like a health check, as there
are systematically two poll() calls, one with the interval, nothing is done
after it, and another one with a zero delay, and the task is called:

  listen test
    bind *:8001
    server s1 127.0.0.1:1111 check

  09:37:38.200959 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8696843}) = 0
  09:37:38.200967 epoll_wait(3, [], 200, 1000) = 0
  09:37:39.202459 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8712467}) = 0
>> nothing run here, as the expired task was not woken up yet.
  09:37:39.202497 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8715766}) = 0
  09:37:39.202505 epoll_wait(3, [], 200, 0) = 0
  09:37:39.202513 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8719064}) = 0
>> now the expired task was woken up
  09:37:39.202522 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 7
  09:37:39.202537 fcntl(7, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
  09:37:39.202565 setsockopt(7, SOL_TCP, TCP_NODELAY, [1], 4) = 0
  09:37:39.202577 setsockopt(7, SOL_TCP, TCP_QUICKACK, [0], 4) = 0
  09:37:39.202585 connect(7, {sa_family=AF_INET, sin_port=htons(1111), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:37:39.202659 epoll_ctl(3, EPOLL_CTL_ADD, 7, {EPOLLOUT, {u32=7, u64=7}}) = 0
  09:37:39.202673 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8814713}) = 0
  09:37:39.202683 epoll_wait(3, [{EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=7, u64=7}}], 200, 1000) = 1
  09:37:39.202693 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8818617}) = 0
  09:37:39.202701 getsockopt(7, SOL_SOCKET, SO_ERROR, [111], [4]) = 0
  09:37:39.202715 close(7)                = 0

Let's instead split the function in two parts:
  - the first part, wake_expired_tasks(), called just before
    process_runnable_tasks(), wakes up all expired tasks; it doesn't
    compute any timeout.
  - the second part, next_timer_expiry(), called just before poll(),
    only computes the next timeout for the current thread.

Thanks to this, all expired tasks are properly woken up when leaving
poll, and each poll call's timeout remains up to date:

  09:41:16.270449 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10223556}) = 0
  09:41:16.270457 epoll_wait(3, [], 200, 999) = 0
  09:41:17.270130 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10238572}) = 0
  09:41:17.270157 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 7
  09:41:17.270194 fcntl(7, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
  09:41:17.270204 setsockopt(7, SOL_TCP, TCP_NODELAY, [1], 4) = 0
  09:41:17.270216 setsockopt(7, SOL_TCP, TCP_QUICKACK, [0], 4) = 0
  09:41:17.270224 connect(7, {sa_family=AF_INET, sin_port=htons(1111), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:41:17.270299 epoll_ctl(3, EPOLL_CTL_ADD, 7, {EPOLLOUT, {u32=7, u64=7}}) = 0
  09:41:17.270314 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10337841}) = 0
  09:41:17.270323 epoll_wait(3, [{EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=7, u64=7}}], 200, 1000) = 1
  09:41:17.270332 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10341860}) = 0
  09:41:17.270340 getsockopt(7, SOL_SOCKET, SO_ERROR, [111], [4]) = 0
  09:41:17.270367 close(7)                = 0

This may be backported to 2.1 and 2.0 though it's unlikely to bring any
user-visible improvement except to clarify debugging.
2019-12-11 09:42:58 +01:00
Willy Tarreau
a1d97f88e0 REORG: listener: move the global listener queue code to listener.c
The global listener queue code and declarations were still lying in
haproxy.c while not needed there anymore at all. This complicates
the code for no reason. As a result, the global_listener_queue_task
and the global_listener_queue were made static.
2019-12-10 14:16:03 +01:00
Willy Tarreau
241797a3fc MINOR: listener: split dequeue_all_listener() in two
We use it half times for the global_listener_queue and half times
for a proxy's queue and this requires the callers to take care of
these. Let's split it in two versions, the current one working only
on the global queue and another one dedicated to proxies for the
per-proxy queues. This cleans up quite a bit of code.
2019-12-10 14:14:09 +01:00
Willy Tarreau
a45a8b5171 MEDIUM: init: set NO_NEW_PRIVS by default when supported
HAProxy doesn't need to call executables at run time (except when using
external checks which are strongly recommended against), and is even expected
to isolate itself into an empty chroot. As such, there basically is no valid
reason to allow a setuid executable to be called without the user being fully
aware of the risks. In a situation where haproxy would need to call external
checks and/or disable chroot, exploiting a vulnerability in a library or in
haproxy itself could lead to the execution of an external program. On Linux
it is possible to lock the process so that any setuid bit present on such an
executable is ignored. This significantly reduces the risk of privilege
escalation in such a situation. This is what haproxy does by default. In case
this causes a problem to an external check (for example one which would need
the "ping" command), then it is possible to disable this protection by
explicitly adding this directive in the global section. If enabled, it is
possible to turn it back off by prefixing it with the "no" keyword.

Before the option:

  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  uid=0(root) gid=0(root) groups=0(root

After the option:
  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  sudo: effective uid is not 0, is /usr/bin/sudo on a file system with the
        'nosuid' option set or an NFS file system without root privileges?
2019-12-06 17:20:26 +01:00
Willy Tarreau
d96f1126fe MEDIUM: init: prevent process and thread creation at runtime
Some concerns are regularly raised about the risk to inherit some Lua
files which make use of a fork (e.g. via os.execute()) as well as
whether or not some of bugs we fix might or not be exploitable to run
some code. Given that haproxy is event-driven, any foreground activity
completely stops processing and is easy to detect, but background
activity is a different story. A Lua script could very well discretely
fork a sub-process connecting to a remote location and taking commands,
and some injected code could also try to hide its activity by creating
a process or a thread without blocking the rest of the processing. While
such activities should be extremely limited when run in an empty chroot
without any permission, it would be better to get a higher assurance
they cannot happen.

This patch introduces something very simple: it limits the number of
processes and threads to zero in the workers after the last thread was
created. By doing so, it effectively instructs the system to fail on
any fork() or clone() syscall. Thus any undesired activity has to happen
in the foreground and is way easier to detect.

This will obviously break external checks (whose concept is already
totally insecure), and for this reason a new option
"insecure-fork-wanted" was added to disable this protection, and it
is suggested in the fork() error report from the checks. It is
obviously recommended not to use it and to reconsider the reasons
leading to it being enabled in the first place.

If for any reason we fail to disable forks, we still start because it
could be imaginable that some operating systems refuse to set this
limit to zero, but in this case we emit a warning, that may or may not
be reported since we're after the fork point. Ideally over the long
term it should be conditionned by strict-limits and cause a hard fail.
2019-12-03 11:49:00 +01:00
Willy Tarreau
47479eb0e7 MINOR: version: emit the link to the known bugs in output of "haproxy -v"
The link to the known bugs page for the current version is built and
reported there. When it is a development version (less than 2 dots),
instead a link to github open issues is reported as there's no way to
be sure about the current situation in this case and it's better that
users report their trouble there.
2019-11-21 18:48:20 +01:00
Willy Tarreau
08dd202d73 MINOR: version: report the version status in "haproxy -v"
As discussed on Discourse here:

    https://discourse.haproxy.org/t/haproxy-branch-support-lifetime/4466

it's not always easy for end users to know the lifecycle of the version
they are using. This patch introduces a "Status" line in the output of
"haproxy -vv" indicating whether it's a development, stable, long-term
supported version, possibly with an estimated end of life for the branch
when it can be anticipated (e.g. for stable versions). This field should
be adjusted when creating a major release to reflect the new status.

It may make sense to backport this to other branches to clarify the
situation.
2019-11-21 18:47:54 +01:00
William Lallemand
677e2f2c35 BUG/MEDIUM: mworker: don't fill the -sf argument with -1 during the reexec
Upon a reexec_on_failure, if the process tried to exit after the
initialization of the process structure but before it was filled with a
PID, the PID in the mworker_proc structure is set to -1.

In this particular case the -sf argument is filled with -1 and haproxy
will exit with the usage message because of that argument.

Should be backported in 2.0.
2019-11-19 17:30:34 +01:00