Emit a warning when the ca-file couldn't be loaded for the httpclient,
and disable the SSL of the httpclient.
We must never be in a case where the verify is disabled without any
configuration, so better disable the SSL completely.
Move the check on the scheme above the initialization of the applet so
we could abort before initializing the appctx.
There were plenty of leftovers from old code that were never removed
and that are not needed at all since these files do not use any
definition depending on fcntl.h, let's drop them.
This gets rid of most open-coded fcntl() calls, some of which were passed
through DISGUISE() to avoid a useless test. The FD_CLOEXEC was most often
set without preserving previous flags, which could become a problem once
new flags are created. Now this will not happen anymore.
Instead of seeing each location manipulate the fcntl() themselves and
often forget to check previous flags, let's centralize the functions to
do this. It also allows to drop fcntl.h from most call places and will
ease the adoption of different OS-specific mechanisms if needed. Note
that the fd_set_nonblock() function purposely doesn't check the previous
flags as it's meant to be used on new FDs only.
Despite what the 'close-spread-time' option should do, the
'connection:close' header was always added to HTTP responses during
soft-stops even with a soft-stop window defined.
This patch adds the proper random based closing to HTTP connections
during a soft-stop (based on the time left in the soft close window).
It should be backported to 2.5 once 'MEDIUM: global: Add a
"close-spread-time" option to spread soft-stop on time window' is
backported as well.
Some older systems may routinely return EWOULDBLOCK for some syscalls
while we tend to check only for EAGAIN nowadays. Modern systems define
EWOULDBLOCK as EAGAIN so that solves it, but on a few older ones (AIX,
VMS etc) both are different, and for portability we'd need to test for
both or we never know if we risk to confuse some status codes with
plain errors.
There were few entries, the most annoying ones are the switch/case
because they require to only add the entry when it differs, but the
other ones are really trivial.
__comp_fetch_init() only presets the maxzlibmem, and only when both
USE_ZLIB and DEFAULT_MAXZLIBMEM are set. The intent is to preset a
default value to protect the system against excessive memory usage
when no setting is set by the user.
Nowadays the entry in the global struct is always there so there's no
point anymore in passing via a constructor to possibly set this value.
Let's go the cleaner way by always presetting DEFAULT_MAXZLIBMEM to 0
in defaults.h unless these conditions are met, and always assigning it
instead of pre-setting the entry to zero. This is more straightforward
and removes some ifdefs and the last constructor. In addition, now the
setting has a chance of being found.
The constructor present there could be replaced with an initcall.
This one is set at level STG_PREPARE because it also zeroes the
lock_stats, and it's a bit odd that it could possibly have been
scheduled to run after other constructors that might already
preset some of these locks by accident.
Transport layers (raw_sock, ssl_sock, xprt_handshake and xprt_quic)
were using 4 constructors and 2 destructors. The 4 constructors were
replaced with INITCALL and the destructors with REGISTER_POST_DEINIT()
so that we do not depend on this anymore.
Pollers are among the few remaining blocks still using constructors
to register themselves. That's not needed anymore since the initcalls
so better turn to initcalls.
On some systems, the hard limit for ulimit -n may be huge, in the order
of 1 billion, and using this to automatically compute maxconn doesn't
work as it requires way too much memory. Users tend to hard-code maxconn
but that's not convenient to manage deployments on heterogenous systems,
nor when porting configs to developers' machines. The ulimit-n parameter
doesn't work either because it forces the limit. What most users seem to
want (and it makes sense) is to respect the system imposed limits up to
a certain value and cap this value. This is exactly what fd-hard-limit
does.
This addresses github issue #1622.
Almost all of our hash-based LB algorithms are implemented as special
cases of something that can now be achieved using sample expressions,
and some of them have adopted some options to adapt their behavior in
ways that could also be achieved using converters.
There are users who want to hash other parameters that are combined
into variables, and who set headers from these values and use
"balance hdr(name)" for this.
Instead of constantly implementing specific options and having users
hack around when they want a real hash, let's implement a native hash
mode that applies to a standard sample expression. This way, any
fetchable element (including variables) may be used to construct the
hash, even modified by any converter if desired.
Any type except bool could cast to bin, while it can cast to string.
That's a bit inconsistent, and prevents a boolean from being used as
the entry of a hash function while any other type can. This is a
problem when passing via variable where someone could use:
... set-var(txn.bar) always_false
to temporarily disable something, but this would result in an empty
hash output when later doing:
... var(txn.bar),sdbm
Instead of using c_int2bin() as is done for the string output, better
enfore an set of inputs or exactly 0 or 1 so that a poorly written sample
fetch function does not result in a difficult to debug hash output.
Surprisingly, while about all calls to a sample cast function carefully
avoid calling c_none(), sample_fetch_as_type() makes no effort regarding
this, while by nature, the function is most often called with an expected
output type similar to the one of the expresison. Let's add it to shorten
the most common call path.
The use_backend and use-server contexts were not enumerated in
smp_resolve_args, and while use-server doesn't currently take an
expression, at least use_backend supports that, and both entries ought
to be listed for completeness. Now an error in a use_backend rule
becomes more precise, from:
[ALERT] (12373) : config : parsing [use-srv.cfg:33]: unable to find
backend 'foo' referenced in arg 1 of sample fetch
keyword 'nbsrv' in proxy 'echo'.
to:
[ALERT] (12307) : config : parsing [use-srv.cfg:33]: unable to find
backend 'foo' referenced in arg 1 of sample fetch
keyword 'nbsrv' in use_backend expression in proxy
'echo'.
This may be backported though this is totally harmless.
Since commit dd7e6c6dc ("BUG/MINOR: http-rules: completely free incorrect
TCP rules on error") free_act_rule() is called on some error paths, and one
of them involves incomplete redirect rules that may cause a crash if the
rule wasn't yet initialized, as shown in this config snippet:
frontend ft
mode http
bind *:8001
http-request redirect location /%[always_false,sdbm]
Let's simply make release_http_redir() more robust against null redirect
rules.
No backport needed since it seems that the only way to trigger this was
the extra check above that was merged during 2.6-dev.
The function checking captures defined in tcp-request content ruleset didn't
use the right rule arguments. "arg.trk_ctr" was used instead of "arg.cap".
This patch must be backported as far as 2.2.
Since the 2.5, it is possible to define TCP/HTTP ruleset in defaults
sections. However, rules defining a capture in defaults sections was not
properly handled because they was not shared with the proxies inheriting
from the defaults section. This led to crash when haproxy tried to store a
new capture.
So now, to fix the issue, when a new proxy is created, the list of captures
points to the list of its defaults section. It may be NULL or not. All new
caputres are prepended to this list. It is not a problem to share the same
defaults section between several proxies, because it is not altered and we
take care to not release it when corresponding proxies are freed but only
when defaults proxies are freed. To do so, defaults proxies are now
unreferenced at the end of free_proxy() function instead of the beginning.
This patch should fix the issue #1674. It must be backported to 2.5.
Captures must only be defined in proxies with the frontend capabilities or
in defaults sections used by proxies with the frontend capabilities. Thus,
an extra check is added to be sure a defaults section defining a capture
will never be references by a backend.
Note that in this case, only named captures in "tcp-request content" or
"http-request" rules are possible. It is not possible in a defaults section
to decalre a capture slot. Not yet at least.
This patch must be backported to 2.5. It is releated to issue #1674.
When using qc_stream_desc_ack(), the stream instance may be freed if
there is no more data in its buffers. This also means that all frames
still stored waiting for ACK for this stream are freed via
qc_stream_desc_free().
This is particularly important in quic_stream_try_to_consume() where we
loop over the frames tree of the stream. A use-after-free is present in
cas the stream has been freed in the trace "stream consumed" which
dereference the frame. Fix this by first checking if the stream has been
freed or not.
This bug was detected by using ASAN + quic traces enabled.
Released version 2.6-dev7 with the following main changes :
- BUILD: calltrace: fix wrong include when building with TRACE=1
- MINOR: ssl: Use DH parameters defined in RFC7919 instead of hard coded ones
- MEDIUM: ssl: Disable DHE ciphers by default
- BUILD: ssl: Fix compilation with OpenSSL 1.0.2
- MINOR: mux-quic: split xfer and STREAM frames build
- REORG: quic: use a dedicated module for qc_stream_desc
- MINOR: quic-stream: use distinct tree nodes for quic stream and qcs
- MINOR: quic-stream: add qc field
- MEDIUM: quic: implement multi-buffered Tx streams
- MINOR: quic-stream: refactor ack management
- MINOR: quic: limit total stream buffers per connection
- MINOR: mux-quic: implement immediate send retry
- MINOR: cfg-quic: define tune.quic.conn-buf-limit
- MINOR: ssl: Add 'show ssl providers' cli command and providers list in -vv option
- REGTESTS: ssl: Update error messages that changed with OpenSSLv3.1.0-dev
- BUG/MEDIUM: quic: Possible crash with released mux
- BUG/MINOR: mux-quic: unsubscribe on release
- BUG/MINOR: mux-quic: handle null timeout
- BUG/MEDIUM: logs: fix http-client's log srv initialization
- BUG/MINOR: mux-quic: remove dead code in qcs_xfer_data()
- DEV: stream: Fix conn-streams dump in full stream message
- CLEANUP: conn-stream: Rename cs_conn_close() and cs_conn_drain_and_close()
- CLEANUP: conn-stream: Rename cs_applet_release()
- MINOR: conn-stream: Rely on endpoint shutdown flags to shutdown an applet
- BUG/MINOR: cache: Disable cache if applet creation fails
- BUG/MINOR: backend: Don't allow to change backend applet
- BUG/MEDIUM: conn-stream: Set back CS to RDY state when the appctx is created
- MINOR: stream: Don't needlessly detach server endpoint on early client abort
- MINOR: conn-stream: Make cs_detach_* private and use cs_destroy() from outside
- MINOR: init: add the pre-check callback
- MEDIUM: httpclient: change the init sequence
- MEDIUM: httpclient/ssl: verify required
- MINOR: httpclient/mworker: disable in the master process
- MEDIUM: httpclient/ssl: verify is configurable and disabled by default
- BUG/MAJOR: connection: Never remove connection from idle lists outside the lock
- BUG/MEDIUM: mux-quic: fix stalled POST requets
- BUG/MINOR: mux-quic: fix POST with abortonclose
- MINOR: task: add a new task_instant_wakeup() function
- MEDIUM: queue: use tasklet_instant_wakeup() to wake tasks
- DOC: remove my name from the config doc
I was surprised to notice that my name was still present as the author
at the top of the config manual. It turns out that this line and a few
other ones in this file remained unchanged since commit 6a06a40501 that
added this doc 15 years ago! It's long been time to get rid of this!
It's long been known that queues didn't scale with threads for various
reasons ranging from the cost of the queue lock to the cost of the
massive amount of inter-thread wakeups.
But some recent reports showing deplorable perfs with threads used at
100% CPU helped us notice that the two elements above add on top of
each other:
- with plenty of inter-thread wakeups, the scheduler takes a lot of
time to dequeue pending tasks from the shared queue ;
- the lock held by the scheduler to do this slows down subsequent
task_wakeup() calls from the the queue that are made under the
queue's lock
- the queue's lock slows down addition of new requests to the queue
and adds up to the number of needed queue entries for a steady
traffic.
But the cost of the share queue has no reason for being paid because
it had already been paid when process_stream() added the request to
the queue. As such an instant wakeup is perfectly fit for this.
This is exactly what this patch does, it uses tasklet_instant_wakeup()
to dequeue pending requests, which has the effect of not bloating the
shared queue, hence not requiring the global queue lock, which in turn
results in the wakeup to be much faster, and the queue lock to be much
shorter. In the end, a test with 4k concurrent connections that was
being limited to 40-80k requests/s before with 16 threads, some of
which were stuck at 100% CPU now reaches 570k req/s with 4% idle.
Given that it's been found that it was possible to trigger the watchdog
on the queue lock under extreme conditions, and that such conditions
could happen when users want to protect their servers during a DoS, it
would definitely make sense to backport it to the most recent releases
(2.5 and 2.4 seem like good candidates especially because their scheduler
is modern enough to receive the change above). If a backport is performed,
the following patch is needed:
MINOR: task: add a new task_instant_wakeup() function
This function's purpose is to wake up either a local or remote task,
bypassing the tree-based run queue. It is meant for fast wakeups that
are supposed to be equivalent to those used with tasklets, i.e. a task
had to pause some processing and can complete (typically a resource
becomes available again). In all cases, it's important to keep in mind
that the task must have gone through the regular scheduling path before
being blocked, otherwise the task priorities would be ignored.
The reason for this is that some wakeups are massively inter-thread
(e.g. server queues), that these inter-thread wakeups cause a huge
contention on the shared runqueue lock. A user reported 47% CPU spent
in process_runnable_tasks with only 32 threads and 80k requests in
queues. With this mechanism, purely one-to-one wakeups can avoid
taking the lock thanks to the mt_list used for the shared tasklet
queue.
Right now the shared tasklet queue moves everything to the TL_URGENT
queue. It's not dramatic but it would seem better to have a new shared
list dedicated to tasks, and that would deliver into TL_NORMAL, for an
even better fairness. This could be improved in the future.
Remove CS_EP_EOS set erroneously on qc_rcv_buf().
This fixes POST with abortonclose. Previously, request was preemptively
aborted by haproxy due to the incorrect EOS flag.
For the moment, EOS flag is not set anymore. It should be set to warn
about a premature close from the client.
Since the idle connections management changed to use eb-trees instead of MT
lists, a lock must be acquired to manipulate servers idle/safe/available
connection lists. However, it remains an unprotected use in
connect_server(), when a connection is removed from an idle list if the mux
has no more streams available. Thus it is possible to remove a connection
from an idle list on a thread, while another one is looking for a idle
connection. Of couse, this may lead to a crash.
To fix the bug, we must take care to acquire the idle connections lock
first. The bug was introduced by the commit f232cb3e9 ("MEDIUM: connection:
replace idle conn lists by eb trees").
The patch must be backported as far as 2.4.
Disable temporary the SSL verify by default in the httpclient. The
initialization of the @system-ca during the init of the httpclient is a
problem in some cases.
The verify can be reactivated with "httpclient-ssl-verify required" in
the global section.
The httpclient HTTPS requests now enable the "verify required" option.
To achieve this, the "@system-ca" ca-file is configured in the
httpclient ssl server. Which means all the system CAs will be loaded at
haproxy startup.
Change the init order of the httpclient, a different init sequence is
required to allow a more complicated init.
The init is splitted in two parts:
- the first part is executed before config_check_validity(), which
allows to create proxy and more advanced stuff than STG_INIT, because
we might want to use stuff already initialized in haproxy (trash
buffers for example)
- the second part is executed after the config_check_validity(),
currently it is used for the log configuration.
This adds a call to function <fct> to the list of functions to be called at
the step just before the configuration validity checks. This is useful when you
need to create things like it would have been done during the configuration
parsing and where the initialization should continue in the configuration
check.
It could be used for example to generate a proxy with multiple servers using
the configuration parser itself. At this step the trash buffers are allocated.
Threads are not yet started so no protection is required. The function is
expected to return non-zero on success, or zero on failure. A failure will make
the process emit a succinct error message and immediately exit.
A conn-stream is never detached from an endpoint or an application alone,
except on a reset. Thus, to avoid any error, these functions are now
private. And cs_destroy() function is added to destroy a conn-stream. This
function is called when a stream is released, on the front and back
conn-streams, and when a health-check is finished.
When a client abort is detected with the server conn-stream in CS_ST_INI
state, there is no reason to detach the endpoing because we know there is no
endpoint attached to this conn-stream. This patch depends on the commit
"BUG/MEDIUM: conn-stream: Set back CS to RDY state when the appctx is
created".
When an appctx is created on the server side, we now set the corresponding
conn-stream to ready state (CS_ST_RDY). When it happens, the backend
conn-stream is in CS_ST_INI state. It is not consistant to let the
conn-stream in this state because it means it is possible to have a target
installed in CS_ST_INI state, while with a connection, the conn-stream is
switch to CS_ST_RDY or CS_ST_EST state.
It is especially anbiguous because we may be tempted to think there is no
endpoint attached to the conn-stream before the CS_ST_CON state. And it is
indeed the reason for a bug leading to a crash because a cs_detach_endp() is
performed if an abort is detected on the backend conn-stream in CS_ST_INI
state. With a mux or a appctx attached to the conn-stream, "->endp" field is
set to NULL. It is unexpected. The API will be changed to be sure it is not
possible. But it exposes a consistency issue with applets.
So, the conn-stream must not stay in CS_ST_INI state when an appctx is
attached. But there is no reason to set it in CS_ST_REQ. The conn-stream
must be set to CS_ST_RDY to handle applets and connections in the same
way. Note that if only the target is set but no appctx is created, the
backend conn-stream is switched from CS_ST_INI to CS_ST_REQ state to be able
to create the corresponding appctx. This part is unchanged.
This patch depends on the commit "MINOR: backend: Don't allow to change
backend applet".
The ambiguity exists on previous versions. But the issue is
2.6-specific. Thus, no backport is needed.
This part was inherited from haproxy-1.5. But since a while (at least 1.8),
the backend applet, once created, is no longer changed. Thus there is no
reason to still check if the target has changed. And in fact, if it was
still possible, there would be a memory leak because the old applet would be
lost and never released.
There is no reason to backport this fix because the leak only exists on a
dead code path.
When we want to serve a resource from the cache, if the applet creation
fails, the "cache-use" action must not yield. Otherwise, the stream will
hang. Instead, we now disable the cache. Thus the request may be served by
the server.
This patch must be backported as far as 1.8.
cs_applet_shut() now relies on CS_EP_SH* flags to performed the applet
shutdown. It means the applet release callback is called if there is no
CS_EP_SHR or CS_EP_SHW flags set. And it set these flags, CS_EP_SHRR and
CS_EP_SHWN more specifically, before exiting.
This way, cs_applet_shut() is the really equivalent to cs_conn_shut().
This function does not release the applet but only call the applet release
callback. It is equivalent to cs_conn_shut() but for applets. Thus the
function is renamed cs_applet_shut().
These functions don't close the connection but only perform shutdown for
reads and writes at the mux level. It is a bit ambiguous. Thus,
cs_conn_close() is renamed cs_conn_shut() and cs_conn_drain_and_close() is
renamed cs_conn_drain_and_shut(). These both functions rely on
cs_conn_shutw() and cs_conn_shutr().
Since the recent changes about the conn-streams, the stream dump in "show
sess all" command is a bit mangled. front and back conn-stream are now
properly displayed (csf and csb). In addition, when there is no backend
endpoint, "APPCTX" was always reported. Now, "NONE" is reported in this
case.
It is 2.6-specific. No backport needed.
Since previous patch
MINOR: mux-quic: split xfer and STREAM frames build
there is no way to report an error in qcs_xfer_data().
This should fix github issue #1669.
As anticipated in commit 211ea252d ("BUG/MINOR: logs: fix logsrv leaks
on clean exit"), there were indeed other corner cases that were not
properly covered. Setting the http client's ring_name to NULL make the
sink lookup crash on startup in sink_find () with a config as simple as:
global
log ring@buf0 local0
The fields must be properly initialized (both config file name and
the ring_name). This only needs to be backported if/when the commit
above is backported.
Do not initialize mux task timeout if timeout client is set to 0 in the
configuration. Check for the task before queuing it in qc_io_cb() or
qc_detach().
This fix a crash when timeout client is 0 or undefined.
Unsubscribe from lower layer on qc_release. This ensures that the lower
layer won't wake up a null tasklet after the MUX has been released and
may prevent a crash.
It is possible the xprt layer have to process retransmitted STREAM frames after
the mux was released. In this case, there is no need to try to wake it up.
