When a session is allocated for a FE connection, session_free() is
responsible to call listener_release() to decrement listener connection
counters and resume listening.
Until now, <listener> member of session was tested inside session_free()
before invocating listener_release(). To highlight more explicitely the
relation between sessions and listeners, introduce a new flag
SESS_FL_RELEASE_LI. Only session with such flag set will invoke
listener_release() on their cleanup. Flag is set inside
session_accept_fd() on success.
This patch has no functional change. However, it will be useful to
implement session creation for rHTTP preconnect.
TASK_WOKEN_ANY was incorrectly used as argument to task_wakeup() for
rhttp preconnect task. This value is used as a flag. Replace it by
proper individual values. This is labelled as a bug but it has no known
impact.
This should be backported up to 2.9.
Ensure "disable frontend" on a reverse HTTP listener is forbidden by
returing -1 on suspend callback. Suspending such a listener has unknown
effect and so is not properly implemented for now.
This should be backported up to 2.9.
On initialization of a rhttp bind, the first thread available on the
listener is selected to execute the first occurence of the preconnect
task.
This thread selection was incorrect as it used my_ffsl() which returns
value indexed from 1, contrary to tid which are indexed from 0. This
cause the first listener thread to be skipped in favor of the second
one. Worst, if haproxy runs in single-thread mode, calculated thread ID
will be invalid and the task will never run, which prevent any
preconnect execution.
Fix this by substracting the result of my_ffsl() by 1 to have a value
indexed from 0.
This must be backported up to 2.9.
Dynamically allocated servers PROXY TLVs were not freed on server
release. This patch fixes this leak by extending srv_free_params().
Every server line with set-proxy-v2-tlv-fmt keyword is impacted.
For static servers, issue is minimal as it will only cause leak on
deinit(). However, this could be aggravated when performing multiple
removal of dynamic servers.
This should be backported up to 2.9.
conn_recv_proxy() is responsible to parse PROXY protocol header. For v2
of the protocol, TLVs parsing is implemented. However, this step was
only done inside 'PROXY' command label. TLVs were never extracted for
'LOCAL' command mode.
Fix this by extracting TLV parsing loop outside of the switch case. Of
notable importance, tlv_offset is updated on LOCAL label to point to
first TLV location.
This bug should be backported up to 2.9 at least. It should even
probably be backported to every stable versions. Note however that this
code has changed much over time. It may be useful to use option
'--ignore-all-space' to have a clearer overview of the git diff.
This reverts commits 885e40494c and
dff9807188.
We decided to spend some time to refactor and rationnalize the SPOE for the
3.1. Thus there is no reason to still consider it as deprecated for the
3.0. Compatibility between the both versions will be maintained.
See #2502 for more info.
When internal error is reported from an HTTP analyzer, we must take care to
not set the stream termination condition if it was already set. For
instance, it happens when a message rewrite fails. In this case
SF_ERR_PXCOND is set by the rule. The HTTP analyzer must not crush it with
SF_ERR_INTERNAL.
The regression was introduced with the commit 0fd25514d6 ("MEDIUM: http-ana:
Set termination state before returning haproxy response").
The bug was discovered working in the issue #2568. It must be backported to
2.9.
To improve the readability of sock_handle_system_err(), let's
set explicitly conn->err_code as CO_ER_SOCK_ERR in case of EPERM
(could be returned by setns syscall).
This fixes the fd leak, introduced in the commit d3fc982cd7
("MEDIUM: proto: make common fd checks in sock_create_server_socket").
Initially sock_create_server_socket() was designed to return only created
socket FD or -1. Its callers from upper protocol layers were required to test
the returned errno and were required then to apply different configuration
related checks to obtained positive sock_fd. A lot of this code was duplicated
among protocols implementations.
The new refactored version of sock_create_server_socket() gathers in one place
all duplicated checks, but in order to be complient with upper protocol
layers, it needs the 3rd parameter: 'stream_err', in which it sets the
Stream Error Flag for upper levels, if the obtained sock_fd has passed all
additional checks.
No backport needed since this was introduced in 3.0-dev10.
In commit 55e9e9591 ("MEDIUM: ssl: temporarily load files by detecting
their presence in crt-store"), ssl_sock_load_pem_into_ckch() was
replaced by ssl_sock_load_files_into_ckch() in the crt-store loading.
But the side effect was that we always try to autodetect, and this is
not what we want. This patch reverse this, and add specific code in the
crt-list loading, so we could autodetect in crt-list like it was done
before, but still try to load files when a crt-store filename keyword is
specified.
Example:
These crt-list lines won't autodetect files:
foobar.crt [key foobar.key issuer foobar.issuer ocsp-update on] *.foo.bar
foobar.crt [key foobar.key] *.foo.bar
These crt-list lines will autodect files:
foobar.pem [ocsp-update on] *.foo.bar
foobar.pem
Thanks to ("MINOR: tools: add vma_set_name() helper"), set a name hint
for large arrays created by fd api (fdtab arrays and so on) so that
that they can be easily identified in /proc/<pid>/maps.
Depending on malloc() implementation, such memory areas will normally be
merged on the heap under MMAP_THRESHOLD (128 kB by default) and will
have a dedicated memory area once the threshold is exceeded. As such, when
large enough, they will appear like this in /proc/<pid>/maps:
7b8e83200000-7b8e84201000 rw-p 00000000 00:00 0 [anon:fd:fdinfo]
7b8e84400000-7b8e85401000 rw-p 00000000 00:00 0 [anon:fd:polled_mask]
7b8e85600000-7b8e89601000 rw-p 00000000 00:00 0 [anon:fd:fdtab_addr]
7b8e90a00000-7b8e90e01000 rw-p 00000000 00:00 0 [anon:fd:fd_updt]
Thanks to ("MINOR: tools: add vma_set_name() helper"), set a name hint
for startup-logs ring's memory area created using mmap() so it can be
easily indentified in /proc/<pid>/maps.
7b8e91cce000-7b8e91cde000 rw-s 00000000 00:19 46 [anon_shmem:errors:startup_logs]
Thanks to ("MINOR: tools: add vma_set_name() helper"), set a name hint
for large memory areas allocated by pollers upon init so that they can
be easily indentified in /proc/<pid>/maps.
For now, only linux-compatible pollers are considered since vma_set_name()
requires a recent linux kernel (>= 5.17).
Depending on malloc() implementation, such memory areas will normally be
merged on the heap under MMAP_THRESHOLD (128 kB by default) and will
have a dedicated memory area once the threshold is exceeded. As such, when
large enough, they will appear like this in /proc/<pid>/maps:
7ec6b2d40000-7ec6b2d61000 rw-p 00000000 00:00 0 [anon:ev_poll:fd_evts_wr]
7ec6b2d61000-7ec6b2d82000 rw-p 00000000 00:00 0 [anon:ev_poll:fd_evts_rd]
Thanks to ("MINOR: tools: add vma_set_name() helper"), set a name hint
for user created memory-backed sinks (ring sections without backing-file)
so that they can be easily indentified in /proc/<pid>/maps.
Depending on malloc() implementation, such memory areas will normally be
merged on the heap under MMAP_THRESHOLD (128 kB by default) and will
have a dedicated memory area once the threshold is exceeded. As such, when
large enough, they will appear like this in /proc/<pid>/maps:
7b8e8ac00000-7b8e8bf13000 rw-p 00000000 00:00 0 [anon💍myring]
In 98d22f212 ("MEDIUM: shctx: Naming shared memory context"), David
implemented prctl/PR_SET_VMA support to give a name to shctx maps when
supported. Maps were named after "HAProxy $name". It turns out that it
is not relevant to include "HAProxy" in the map name, given that we're
already looking at maps for a given PID (and here it's HAProxy's pid).
Instead, let's name shctx maps by making use of the new vma_set_name()
helper introduced by previous commit. Resulting maps will be named
"shctx:$name", e.g.: "shctx:globalCache", they will appear like this in
/proc/<pid>/maps:
7ec6aab0f000-7ec6ac000000 rw-s 00000000 00:01 405 [anon_shmem:shctx:custom_name]
Following David Carlier's work in 98d22f21 ("MEDIUM: shctx: Naming shared
memory context"), let's provide an helper function to set a name hint on
a virtual memory area (ie: anonymous map created using mmap(), or memory
area returned by malloc()).
Naming will only occur if available, and naming errors will be ignored.
The function takes mandatory <type> and <name> parameterss to build the
map name as follow: "type:name". When looking at /proc/<pid>/maps, vma
named using this helper function will show up this way (provided that
the kernel has prtcl support for PR_SET_VMA_ANON_NAME):
example, using mmap + MAP_SHARED|MAP_ANONYMOUS:
7364c4fff000-736508000000 rw-s 00000000 00:01 3540 [anon_shmem:type:name]
Another example, using mmap + MAP_PRIVATE|MAP_ANONYMOUS or using
glibc/malloc() above MMAP_THRESHOLD:
7364c4fff000-736508000000 rw-s 00000000 00:01 3540 [anon:type:name]
The load keyword from the documentation has its own section to be
readable (like the server or bind options section).
The ocsp-update keyword was move from the bind section to the crt-list
load one.
Since 40d1c84bf0 ("BUG/MAJOR: ring: free the ring storage not the ring
itself when using maps"), munmap() call for startup_logs's ring and
file-backed rings fails to work (EINVAL) and causes memory leaks during
process cleanup.
munmap() fails because it is called with the ring's usable area pointer
which is an offset from the underlying original memory block allocated
using mmap(). Indeed, ring_area() helper function was misused because
it didn't explicitly mention that the returned address corresponds to
the usable storage's area, not the allocated one.
To fix the issue, we add an explicit ring_allocated_area() helper to
return the allocated area for the ring, just like we already have
ring_allocated_size() for the allocated size, and we properly use both
the allocated size and allocated area to manipulate them using munmap()
and msync().
No backport needed.
Check prev and new parameters in ckch_conf_cmp() so we don't dereference
a NULL ptr. There is no risk since it's not used with a NULL ptr yet.
Also remove the check that are done later, and do it at the beginning of
the function.
Should fix issue #2572.
Released version 3.0-dev12 with the following main changes :
- CI: drop asan.log umbrella completely
- BUG/MINOR: log: fix leak in add_sample_to_logformat_list() error path
- BUG/MINOR: log: smp_rgs array issues with inherited global log directives
- MINOR: rhttp: Don't require SSL when attach-srv name parsing
- REGTESTS: ssl: be more verbose with ocsp_compat_check.vtc
- DOC: Update UUID references to RFC 9562
- MINOR: hlua: add hlua_nb_instruction getter
- MEDIUM: hlua: take nbthread into account in hlua_get_nb_instruction()
- BUG/MEDIUM: server: clear purgeable conns before server deletion
- BUG/MINOR: mux-quic: fix error code on shutdown for non HTTP/3
- BUG/MINOR: qpack: fix error code reported on QPACK decoding failure
- BUG/MEDIUM: htx: mark htx_sl as packed since it may be realigned
- BUG/MEDIUM: stick-tables: properly mark stktable_data as packed
- SCRIPTS: run-regtests: fix a few occurrences of extended regexes
- BUG/MINOR: ssl_sock: fix xprt_set_used() to properly clear the TASK_F_USR1 bit
- MINOR: dynbuf: provide a b_dequeue() variant for multi-thread
- BUG/MEDIUM: muxes: enforce buf_wait check in takeover()
- BUG/MINOR: h1: Check authority for non-CONNECT methods only if a scheme is found
- BUG/MEDIUM: h1: Reject CONNECT request if the target has a scheme
- BUG/MAJOR: h1: Be stricter on request target validation during message parsing
- MINOR: qpack: prepare error renaming
- MINOR: h3/qpack: adjust naming for errors
- MINOR: h3: adjust error reporting on sending
- MINOR: h3: adjust error reporting on receive
- MINOR: mux-quic: support glitches
- MINOR: h3: report glitch on RFC violation
- BUILD: stick-tables: better mark the stktable_data as 32-bit aligned
- MINOR: ssl: rename tune.ssl.ocsp-update.mode in ocsp-update.mode
- REGTESTS: update the ocsp-update tests
- BUILD: stats: remove non portable getline() usage
- MEDIUM: ssl: add ocsp-update.mindelay and ocsp-update.maxdelay
- BUILD: log: get rid of non-portable strnlen() func
- BUG/MEDIUM: fd: prevent memory waste in fdtab array
- CLEANUP: compat: make the MIN/MAX macros more reliable
- Revert: MEDIUM: evports: permit to report multiple events at once"
- BUG/MINOR: stats: Don't state the 303 redirect response is chunked
- MINOR: mux-h1: Add a flag to ignore the request payload
- REORG: mux-h1: Group H1S_F_BODYLESS_* flags
- CLEANUP: mux-h1: Remove unused H1S_F_ERROR_MASK mask value
- MEDIUM: mux-h1: Support C-L/T-E header suppressions when sending messages
- MINOR: ssl: ckch_store_new_load_files_conf() loads filenames from ckch_conf
- MEDIUM: ssl/crtlist: loading crt-store keywords from a crt-list
- CLEANUP: ssl/ocsp: remove the deprecated parsing code for "ocsp-update"
- MINOR: ssl: pass ckch_store instead of ckch_data to ssl_sock_load_ocsp()
- MEDIUM: ssl: ckch_conf_parse() uses -1/0/1 for off/default/on
- MINOR: ssl: handle PARSE_TYPE_INT and PARSE_TYPE_ONOFF in ckch_store_load_files()
- MINOR: ssl/ocsp: use 'ocsp-update' in crt-store
- MINOR: ssl: ckch_conf_clean() utility function for ckch_conf
- MEDIUM: ssl: add ocsp-update.disable global option
- MEDIUM: ssl/cli: handle crt-store keywords in crt-list over the CLI
- MINOR: ssl: ckch_conf_cmp() compare multiple ckch_conf structures
- MEDIUM: ssl: temporarily load files by detecting their presence in crt-store
- REGTESTS: ocsp-update: change the reg-test to support the new crt-store mode
- DOC: capabilities: fix chapter header rendering
The header of a new management guide chapter, "13.1. Linux capabilities
support", is not rendered in HTML format in a proper way, because of missing
dots at the end of this chapter's number.
Update the ocsp-update tests for the recent changes:
- Incompatibilities check string changed to match the crt-store one
- The "good configurations" are not good anymore because the
ckch_conf_cmp() does not compare anymore with a global value.
crt-store is maint to be stricter than your common crt argument on a
bind line, and is supposed to be a declarative format.
However, since the 'ocsp-update' was migrated from ssl_conf to
ckch_conf, the .issuer file is not autodetected anymore when adding a
ocsp-update keyword in a crt-list file, which breaks retro-compatibility.
This patch is a quick fix that will disappear once we are able to be
strict on a crt-store and autodetect on a crt-list.
The ckch_conf_cmp() function allow to compare multiple ckch_conf
structures in order to check that multiple usage of the same crt in the
configuration uses the same ckch_conf definition.
A crt-list allows to use "crt-store" keywords that defines a ckch_store,
that can lead to inconsistencies when a crt is called multiple time with
different parameters.
This function compare and dump a list of differences in the err variable
to be output as error.
The variant ckch_conf_cmp_empty() compares the ckch_conf structure to an
empty one, which is useful for bind lines, that are not able to have
crt-store keywords.
These functions are used when a crt-store is already inialized and we
need to verify if the parameters are compatible.
ckch_conf_cmp() handles multiple cases:
- When the previous ckch_conf was declared with CKCH_CONF_SET_EMPTY, we
can't define any new keyword in the next initialisation
- When the previous ckch_conf was declared with keywords in a crtlist
(CKCH_CONF_SET_CRTLIST), the next initialisation must have the exact
same keywords.
- When the previous ckch_conf was declared in a "crt-store"
(CKCH_CONF_SET_CRTSTORE), the next initialisaton could use no keyword
at all or the exact same keywords.
This patch adds crt-store keywords from the crt-list on the CLI.
- keywords from crt-store can be used over the CLI when inserting
certificate in a crt-list
- keywords from crt-store are dumped when showing a crt-list content
over the CLI
The ckch_conf_kws.func function pointer needed a new "cli" parameter, in
order to differenciate loading that come from the CLI or from the
startup, as they don't behave the same. For example it must not try to
load a file on the filesystem when loading a crt-list line from the CLI.
dump_crtlist_sslconf() was renamed in dump_crtlist_conf() and takes a
new ckch_conf parameter in order to dump relevant crt-store keywords.
This option allow to disable completely the ocsp-update.
To achieve this, the ocsp-update.mode global keyword don't rely anymore
on SSL_SOCK_OCSP_UPDATE_OFF during parsing to call
ssl_create_ocsp_update_task().
Instead, we will inherit the SSL_SOCK_OCSP_UPDATE_* value from
ocsp-update.mode for each certificate which does not specify its own
mode.
To disable completely the ocsp without editing all crt entries,
ocsp-update.disable is used instead of "ocsp-update.mode" which is now
only used as the default value for crt.
Use the ocsp-update keyword in the crt-store section. This is not used
as an exception in the crtlist code anymore.
This patch introduces the "ocsp_update_mode" variable in the ckch_conf
structure.
The SSL_SOCK_OCSP_UPDATE_* enum was changed to a define to match the
ckch_conf on/off parser so we can have off to -1.
The callback used by ckch_store_load_files() only works with
PARSE_TYPE_STR.
This allows to use a callback which will use a integer type for PARSE_TYPE_INT
and PARSE_TYPE_ONOFF.
This require to change the type of the callback to void * to pass either
a char * or a int depending of the parsing type.
The ssl_sock_load_* functions were encapsuled in ckch_conf_load_*
function just to match the type.
This will allow to handle crt-store keywords that are ONOFF or INT
types.
ssl_sock_put_ckch_into_ctx() and ssl_sock_load_ocsp() need to take a
ckch_store in argument. Indeed the ocsp_update_mode is not stored
anymore in ckch_data, but in ckch_conf which is part of the ckch_store.
This is a minor change, but the function definition had to change.
Remove the "ocsp-update" keyword handling from the crt-list.
The code was made as an exception everywhere so we could activate the
ocsp-update for an individual certificate.
The feature will still exists but will be parsed as a "crt-store"
keyword which will still be usable in a "crt-list". This will appear in
future commits.
This commit also disable the reg-tests for now.
This patch allows the usage of "crt-store" keywords from a "crt-list".
The crtstore_parse_load() function was splitted into 2 functions, so the
keywords parsing is done in ckch_conf_parse().
With this patch, crt are loaded with ckch_store_new_load_files_conf() or
ckch_store_new_load_files_path() depending on weither or not there is a
"crt-store" keyword.
More checks need to be done on "crt" bind keywords to ensure that
keywords are compatible.
This patch does not introduce the feature on the CLI.
ckch_store_new_load_files_conf() is the equivalent of
new_ckch_store_load_files_path() but instead of trying to find the files
using a base filename, it will load them from a list of files.
During the 2.9 dev cycle, to be able to support zero-copy data forwarding, a
change on the H1 mux was performed to ignore the headers modifications about
payload representation (Content-Length and Transfer-Encoding headers).
It appears there are some use-cases where it could be handy to change values
of these headers or just remove them. For instance, we can imagine to remove
these headers on a server response to force the old HTTP/1.0 close mode
behavior. So thaks to this patch, the rules are relaxed. It is now possible
to remove these headers. When this happens, the following rules are applied:
* If "Content-Length" header is removed but a "Transfer-Encoding: chunked"
header is found, no special processing is performed. The message remains
chunked. However the close mode is not forced.
* If "Transfer-Encoding" header is removed but a "Content-Length" header is
found, no special processing is performed. The payload length must comply
to the specified content length.
* If one of them is removed and the other one is not found, a response is
switch the close mode and a "Content-Length: 0" header is forced on a
request.
With these rules, we fit the best to the user expectations.
This patch depends on the following commit:
* MINOR: mux-h1: Add a flag to ignore the request payload
This patch should fix the issue #2536. It should be backported it to 2.9
with the commit above.
This mask value is unused, so we can safely remove it. It is a chance
because its value was wrong. But there is no bug here, even in stable
versions, because it is no longer used in all versions.
There was a flag to skip the response payload on output, if any, by stating
it is bodyless. It is used for responses to HEAD requests or for 204/304
responses. This allow rewrites during analysis. For instance a HEAD request
can be rewrite to a GET request for any reason (ie, a server not supporting
HEAD requests). In this case, the server will send a response with a
payload. On frontend side, the payload will be skipped and a valid response
(without payload) will be sent to the client.
With this patch we introduce the corresponding flag for the request. It will
be used to skip the request payload. In addition, when payload must be
skipped for a request or a response, The zero-copy data forwarding is now
disabled.
Start-line flags for 303-See-Other response returned by the stats applet are
not properly set. Indeed, the reponse has a "content-length" header but both
HTX_SL_F_CHNK and HTX_SL_F_CLEN flags are set. Because of this bug, the
reponse is considered as chunked. So, let's remove HTX_SL_F_CHNK flag.
And also add HTX_SL_F_BODYLESS flag because there is no payload
("content-length" header is always set to 0).
This patch must be backported to all stable versions. On the 2.8 and lower
versions, the commit d0b04920d1 ("BUG/MINOR: htpp-ana/stats: Specify that
HTX redirect messages have a C-L header") must be backported first.
Tests have shown that switching nevlist to global.tune.maxpollevents
is totally unreliable when using evports, and that events seem to be
missed. A good reproducer seems to be QUIC. There are not enough
users of Solaris to warrant spending more time trying to get down to
this, and even the few that remain are by definition not interested
in performance, so let's just revert the commit that tried to lift the
value: e6662bf706 ("MEDIUM: evports: permit to report multiple events
at once").
No backport is needed.
After every release we say that MIN/MAX should be changed to be an
expression that only evaluates each operand once, and before every
version we forget to change it and we recheck that the code doesn't
misuse them. Let's fix them now.
In 97ea9c49f1 ("BUG/MEDIUM: fd: always align fdtab[] to 64 bytes"), the
patch doesn't do what the message says. The intent was only to align the
base fdtab addr on 64 bytes so that all fdtab entries are aligned and thus
don't share the same cache line. For that, fdtab pointer is adjusted from
fdtab_addr (unaligned) address after it is allocated. Thus, all we need
is an extra 64 bytes in the fdtab_addr array for the aligment. Because
we use calloc() to perform the allocation, a dumb mistake was made: the
'+64' was added on <size> calloc argument, which means EACH fdtab entry
is allocated with 64 extra bytes.
Given that a single fdtab entry is 64 bytes, since 97ea9c49f1 each fdtab
entry now takes 128 bytes! We doubled fdtab memory consumption.
To give you an idea, on my laptop, when looking at memory consumption
using 'ps -p `pidof haproxy` -o size' right after starting haproxy
process with default settings (no maxsock enforced):
before 97ea9c49f1:
-> 118440 (KB, ~= 118MB)
after 97ea9c49f1:
-> 183976 (KB, ~= 184MB)
To fix this, use calloc with 1 <nmemb> and manually provide the size with
<size> as we would do if we used malloc(). With this patch, we're back to
pre-97ea9c49f1 for fdtab memory consumption (with 64 extra bytes the
whole array, which is insignificant).
It should be backported to all stable versions.
In c614fd3b9 ("MINOR: log: add +cbor encoding option"), I wrongly used
strnlen() without noticing that the function is not portable (requires
_POSIX_C_SOURCE >= 2008) and that it was the first occurrence in the
entire project. In fact it is not a hard requirement since it's a pretty
simple function. Thus to restore build compatibility with minimal/older
build systems, let's actually get rid of it and use an equivalent portable
code where needed (we cannot simply rely on strlen() because the string
might not be NULL terminated, we must take upstream len into account).
No backport needed (unless c614fd3b9 gets backported)
getline() was used to read stats-file. However, this function is not
portable and may cause build issue on some systems. Replace it by
standard fgets().
No need to backport.
