The resolver 'r' is appended to the global sec_resolvers list, but upon failure
later, pointers are released but the element remains in the list, corrupting it,
and possibly causing a crash during deinit() when releasing remaining ones.
Adding a LIST_DEL_INIT() on the error unrolling path is sufficient.
Note that the issue will only happen on failure to allocate memory via
strdup() so the risk is low. The bug was introduced in 2.6 by commit
e7f5776800 ("MINOR: resolvers: resolvers_new() create a resolvers with
default values"), so the fix may be backported to several releases, but
does not necessarily have to go that far.
In cli_parse_set_server()'s 'ssl' branch, the server lock is taken,
and not released in case srv_set_ssl() fails, resulting in a dead lock
and a panic the next time an attempt to touch this server is made. The
lock must be released on all error paths.
This was introduced in 3.3 by commit f8f94ffc9 ("BUG/MEDIUM: server:
Use sni as pool connection name for SSL server only") which was marked
for backporting to 3.0, so this must likely be backported that far.
The condition 'if (srv->pool_conn_name)' was checking the destination
instead of the source 'src->pool_conn_name', meaning the strdup() would
never fire (since newly calloc'd servers start with NULL pool_conn_name),
and the pool_conn_name setting from default-server was silently ignored.
Introduced in 3.2 with commit f0f1816f1 ("MINOR: check: implement
check-pool-conn-name srv keyword") when pool_conn_name support was added
to srv_settings_cpy(). The bug caused any 'pool-conn-name' setting in a
'default-server' line to be lost for all servers inheriting from it.
Note that it's not the first time this function induces such a bug due
to the poor choice of "srv" vs "src" that should be renamed to avoid
keyboard mistakes and visual confusion.
This needs to be backported to 3.2.
This function may face an OOM on strdup() in the middle of the hostname
or hostname_dn replacement, leaving NULLs in either or both of the server's
fields, which is definitely not good for other call places.
Let's perform a safe replacement instead: we first allocate the new
values, and only if they are successful, then we release the previous
ones and replace them.
It is not necessary to backport this unless the issue is reported (it
was found via code review).
All task allocation errors report "memory allocation error initializing
the ring" when the actual failure was task_new_anywhere() returning NULL.
This clearly is a copy-paste. Let's fix the error messages to help when
debugging. Since it's only about allocation failures during init, there
is probably no point in backporting this.
One is s/keyworld/keyword in the retry-on parser. The other one is a
wrong argument "len" being printed in case of parse error for
"declare capture" instead of the length itself.
These can be backported though they are not important.
In dns_session_init(), sockaddr_alloc() allocates 'addr' from the sockaddr
pool, but on failure of appctx_finalize_startup() we jump to the error label
without calling sockaddr_free(&addr), leaking the allocation. Let's add the
missing sockaddr_free() on the error branch.
This must be backported to 2.6.
The original DNS code would only use the 8 lower bits of the compression
offset. This was fixed in 2.0 with commit 2fa66c3b9 ("BUG/MEDIUM: dns:
overflowed dns name start position causing invalid dns error") but it was
not sufficient because the anti-loop check continues to use only 8 of the
14 bits, thus a crafted response where the 8 lower bits pass the check and
the 6 higher should fail it would be accepted. The impacts remains limited
thanks to the bounds check and the recursion limits, but such invalid
responses could still cost a lot to process. Let's compute the 14-bit
offset once for all and use it everywhere.
In resolv_validate_dns_response(), the additional records loop calls
resolv_read_name(). When it returns zero due to a bad response, the main
loop does a "continue" without making the "reader" pointer progress, so it
evaluates the exact same field again and again. Fortunately this is limited
by arcount which is 16 bits, but it means it can still iterate 65535 times
there, allocating and releasing an answer_record at each turn. Let's just
jump to the invalid_resp label that handles the cleaning. There was the
same pattern (without the allocation) with nscount a few lines above BTW.
These can possibly explain some situations where a high CPU usage observed
processing responses.
Seems like these were introduced in 2.2 with commit 37950c8d2
("BUG/MEDIUM: dns: improper parsing of aditional records")
This must be backported to stable versions.
In cfg_parse_resolvers(), two duplicate name checks set err_code but lacked
'goto out', allowing execution to fall through and create the duplicate entry.
This would result in new resolvers and nameservers to be created after the
error was displayed, and a leak of the previous one. It's mostly harmless
since we're exiting after such errors. This can be backported if desired.
In the inner while loop that validates each character of a POST parameter
value, the code checks *p via HTTP_IS_TOKEN() and HTTP_IS_LWS() instead
of *end, while the loop condition only advances "end", so only the first
character of each value is validated.
This means spaces or binary data embedded in parameter values after the
first character goes undetected. Fix by replacing both references to *p
with *end to properly scan through all characters as intended.
This bug was introduced in 1.5-dev20 by commit 98634f0c7 ("MEDIUM:
backend: Enhance hash-type directive with an algorithm options") so
the fix must be backported to all versions.
Released version 3.4-dev12 with the following main changes :
- SCRIPTS: announce-release: add a link to the OpenTelemetry filter
- BUG/MEDIUM: servers: Only requeue servers if they are up
- MINOR: tinfo: store the number of committed extra streams in the tgroup
- MINOR: connection: add a function to calculate elastic streams limit
- MINOR: mux-h2: consider the elastic streams limit on frontend
- MINOR: lb: make LB initialization even more declarative
- BUG/MINOR: cfgparse-listen: do not emit extraneous line in rule order warnings
- CLEANUP: tree-wide: fix typos in non user-visible comments in 15 files
- CLEANUP: h1/htx: fix a few typos in warning, debug and trace messages
- BUG/MINOR: mux-h1: only check h1s if not NULL
- BUG/MINOR: http-fetch: fix smp_fetch_hdr_ip()'s handling of brackets for IPv6
- BUG/MINOR: http-fetch: make http_first_req() check for HTTP first
- BUG/MINOR: http-act: set-status() must check the response message, not the request
- BUG/MINOR: tools: fix memory leak in env_expand() error path
- BUG/MINOR: auth: free user groups on error paths in userlist_postinit()
- BUG/MINOR: uri-auth: avoid leaks on initialization error
- BUG/MINOR: cache: fix memory leak in parse_cache_rule error path
- BUG/MINOR: cfgcond: make KQUEUE check for GTUNE_USE_KQUEUE not GTUNE_USE_EPOLL
- BUG/MINOR: mqtt: connack parser returns MQTT_NEED_MORE_DATA on unknown property
- BUG/MINOR: mqtt: connect parser uses wrong bit field for TOPIC_ALIAS_MAXIMUM
- BUG/MINOR: mqtt: connack parser uses wrong bit for SUBSCRIPTION_IDENTIFIERS_AVAILABLE
- BUG/MINOR: mqtt: fix PUBLISH flags validation that want all bits to be set
- CLEANUP: http_htx: rename inner 'type' to 'ptype' to avoid variable shadowing
- CLEANUP: mux-h2: fix minor output debugging format issues
- CLEANUP: http-rules: fix a few '&' vs '&&' checks for clarity
- CLEANUP: auth: remove undeclared auth_resolve_groups() from auth.h
- CLEANUP: cache: remove redundant res_htx assignment in http_cache_io_handler()
- CLEANUP: channel: remove bogus and unused definition of channel_empty()
- CLEANUP: flt_http_comp: remove duplicate rate limit and CPU usage checks
- CLEANUP: mqtt: remove duplicate MQTT_FN_BIT_USER_PROPERTY in CONNECT fields
- BUG/MINOR: uri-auth: fix possible null-deref in latest fix for leaks
- BUG/MEDIUM: tasks: Keep the TASK_RUNNING flag until queued
- CLEANUP: mqtt: fix spelling of shared_subscription_available
- CLEANUP: regex: pre-initialize error variable in regex_comp() to calm analysis
- BUILD: compiler: fix redefinition of __nonstring
- CLEANUP: defaults: adjust MAX_THREADS multiplier number in comment
- CLEANUP: src/cpuset.c: fix missing return in functions returning int
- REGTESTS: Use ${tmpdir} instead of hardcoded /tmp/
- REGTESTS: Don't try to use real nameservers for testcases
- CLEANUP: tree-wide: fix typos in non user-visible comments in 3 more files
- MINOR: cli: improve forward compatibility for show fd
- DOC: management: document the <tgid>/<fd> form of show fd
- CLEANUP: tree-wide: fix more typos and outdated explanations in comments
- BUG/MEDIUM: dict: hold read lock while incrementing refcount in dict_insert
- BUG/MEDIUM: http-client: Only consume input buffer when hc one is empty
- BUG/MINOR: xprt_qstrm: fix conflicting prototype
- REORG: mux_quic: use newer qcm prefix for legacy qmux files
- MINOR: mux_quic: use qcm prefix for mux callbacks
- MINOR: mux_quic: use qcm prefix for mux functions
- MINOR: mux_quic: use qcm prefix for traces functions/structs
- MINOR: mux_quic: rename qstrm files to qmux
- MINOR: mux_quic: remove qstrm naming in QUIC MUX
- MINOR: connection: rename QMux related flags
- MINOR: xprt_qmux: use qmux instead of qstrm naming
- MINOR: trace: implement source alias
- MEDIUM: mux_quic: rename qmux traces to qcm
- MINOR: sample: add a generic reverse converter
- MINOR: sample: add a reverse_dom converter
- DOC: proxy-protocol: clarify UDP usage
- BUILD: 51d.c: cleanup, fix preprocessor ifdefs
- CLEANUP: tree-wide: fix typos in user-invisible files
- CLEANUP: htx: Adjust numbering of HTX blocks' types in the description
Support of pseudo-headers was removed as unused, but mention of it
in the description remains and disrupt the numbering in comment, which
can be confusing.
The ifdef spans over function boundaries, making some combinations produce
code that does not build:
addons/51degrees/51d.c:638:1: error: Unmatched '}'. Configuration: ''. [syntaxError]
the proxy protocol spec didn't specify UDP and therefore most
implementations treat it as a TCP connection and re-use the last send
information for a ip/port pair.
This change makes it more clear.
In domain-based routing and policy rules, suffix matching on hostnames is
often easier to express as a prefix match on reversed labels. A dedicated
converter makes this convenient with existing fetches and matchers.
This also has a performance benefit for large maps. Prefix string matches use
the prefix-tree index (PAT_MATCH_BEG with pat_idx_tree_pfx), while end matches
use the string-list index (PAT_MATCH_END with pat_idx_list_str), so
reversed-label lookups can avoid linear suffix scans.
This patch adds "reverse_dom", a string converter that reverses domain labels,
ignores one optional trailing dot on input, and rejects empty labels. It
intentionally leaves trailing-dot handling to the caller so configurations can
choose between exact matches, subdomain-only matches, or an explicit dotted
form built with "concat(.)" for prefix lookups.
Examples:
example.com -> com.example
mail.example.com -> com.example.mail
The documentation is updated and a reg-test covers the converter itself, the
explicit dotted form for "map_beg()", and the subdomain-only "-m beg" case.
Some use cases benefit from reversing a string before passing it to other
converters or lookups. While reverse_dom addresses domain-specific label
reversal, a generic byte-wise string reversal remains useful on its own and can
also be combined with other converters such as concat().
A common lookup use case is turning a suffix match on the original string into
a prefix match on the reversed string. Prefix string matches use the
prefix-tree index (PAT_MATCH_BEG with pat_idx_tree_pfx), while end matches use
the string-list index (PAT_MATCH_END with pat_idx_list_str), so reversing
before map_beg can avoid linear suffix scans for large maps.
This patch adds a new string converter named "reverse". It reverses the input
string byte by byte and returns the resulting string unchanged otherwise. It
does not apply any domain-specific semantics or character-encoding semantics.
The documentation is updated and a reg-test is added to cover the basic
conversion as well as a simple composition with concat(.).
This patch is part of a renaming affecting mux_quic and related files.
Naming "qcm" will become the new marker for common mux_quic stuffs,
shared both on QUIC and QMux protocols. The final objective is to limit
"qmux" naming for stuffs related to the new experimental protocol of the
same name.
The current patch renames mux_quic traces token to "qcm". This is the
only change with external visible impacts in the whole renaming series.
However, to preserve compatibility, trace source alias is defined with
the older name "qmux". This relies on the previous patch which
introduced "alias" new trace_source member.
Add a new "alias" member in trace_source structure. Its purpose is to be
an alternative to the member "name". This will be used in the next patch
to allow renaming of QUIC mux traces while preserving compatibility.
This new member is only used in trace_find_source() which is the helper
used to retrieve a trace source from its name.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming in xprt_qmux layer. Older "qstrm"
identifier is replaced by the new name "qmux". Every remaining functions
and structures in xprt_qmux are changed. Outside effects are only
present in QUIC MUX which directly uses some of these functions.
This is a follow-up on the QUIC MUX renaming process.
The current patch performs renaming of "qstrm" to "qmux" in connection
flags. These flags are only used in linked with the xprt_qmux layer.
This has an impact on every files which manipulates these flags, namely
backend, session and ssl_sock sources.
Also, internal xprt identifier is renamed from XPRT_QSTRM to XPRT_QMUX,
This is a follow-up on the QUIC MUX renaming process.
The current patch replaces "qstrm" naming with "qmux" in QUIC MUX source
file. Some members are impacted in qcc and qcs structures, as well as
some internal functions used for QMux receive/send. Internal mux_ops is
also rename to qmux_ops. This is not a breaking change as its externally
visible name was already set to "qmux" originally.
This is a follow-up on the QUIC MUX renaming process. Now most of "qmux"
generic usages as been replaced in favor of "qcm" naming. The next part
of the renaming is to replace "qstrm" naming with "qmux" for stuffs
related to the new QMux protocol specifically.
This is first applied on filenames. As with the previous renaming,
Makefile and include statements are updated as well to prevent
compilation issues.
This is a follow-up on the QUIC MUX renaming process.
This patch renames several definitions in QUIC MUX traces source code.
This is only an internal change. Trace source name is kept to "qmux" for
now, but will be changed in a dedicated patch.
This is a follow-up on the QUIC MUX renaming process.
A previous patch already renames MUX ops callbacks. The current patch
proceed to a similar operation for the rest of the MUX functions.
This is a follow-up on the QUIC MUX renaming process.
The current patch renames all MUX functions used as stream ops
callbacks. Also, internally defined mux_ops is also renamed from
"qmux_ops" to "quic_ops". There is no breaking change as mux_ops name
field remain set to "QUIC".
This patch is the first one of the renaming serie, affecting the QUIC
MUX module. The objective is to remove older "qmux" naming which was
used as a generic identifier. Now it should be restricted to the QMux
experimental protocol. A new "qcm" naming will replace the generic
usage.
The current patch renames the files themselves. Token "qmux" is replaced
by the new "qcm" identifier. Makefile and include statements are
adjusted as required.
This patch adds the missing include of xprt_qstrm header into its
companion source file. This helped to detect an incoherence in the
xprt_qstrm_xfer_rxbuf() prototype which is now fixed.
Header files is also updated with mandatory include statements and
forward declaration.
No backport needed.
Since http-client applet uses its own buffers, it is possible to have data
stuck in the applet input buffer while the http-client response buffer is
full, preventing the applet to consume these data. If this happens on the
last part of the response payload, the upper stream can decide to shut the
applet. In this case, the applet using the http client will not be able to
retrieve these last data because they will never be move into the hc
response buffer.
The main reason for this bug is that, for now, the applets cannot survive
the upper stream unlike multiplexers. It could be a good improvement for the
3.5. However, some applets still uses the stream-connector and the upper
stream (peer and stat applets for instance). So it is not an easy task.
In the mean time, to fix the issue on stable branches, the http-client
applet now stops to consume data when the hc response buffer is not empty.
This way, the applet shut will be deferred. Data will be consumed when they
can be fully moved in the httpclient response buffer.
This patch should fix the issue #3366. It must be backported to 3.3.
In dict_insert(), the read lock on d->rwlock was released before
incrementing the entry's refcount. Between the RDUNLOCK and the
HA_ATOMIC_INC, another thread could call dict_entry_unref() to drop
the refcount to zero, acquire the write lock, delete the entry from
the tree, and free it. The subsequent HA_ATOMIC_INC would then be a
use-after-free on freed memory.
The fix moves the HA_ATOMIC_INC inside the read lock, matching the
pattern used in stick_table.c for identical refcount-then-unlock
sequences.
It can be backported to the branches where this is relevant.
Some outdated comments, as well as typos were fixed in the following files:
dgram.h protocol.h queue-t.h cpu_topo.c debug.c dict.c
protocol.c queue.c raw_sock.c trace.c wdt.c
Add the syntax description, including the wildcard forms and the
note that <tgid> is currently parsed but ignored pending future
support for per-thread-group fd tables.
The "<tgid>/" and "/" wildcard forms previously produced no output.
This isn't a bug since they are new, but a script written for future
versions (where the slash form will gain per-thread-group semantics)
would not work the same on 3.4. Make them produce output by dropping
the redundant ctx->fd = -1 wildcard sentinel; also tighten tgid
validation to reject values <= 0.
The test doesn't need a real nameserver and in a isolated, restricted
test environment it might not be able to reach one at all, like with a
network sandbox. So lets just use 127.0.0.1:53. Even if there is none,
that's not a problem for this particular test.
Signed-off-by: Christian Ruppert <idl0r@qasl.de>
Tests may be excuted in sandboxed or minimalistic / restricted
environments, so incosistencies might cause trouble, like missing
permissions. So lets use the tmpdir variable instead, so the user might
define some path
Signed-off-by: Christian Ruppert <idl0r@qasl.de>
Cppcheck found the issue described in github #2124, which can cause these
errors if no CPUSET implementation is supported (and CPUSET_USE_ULONG is
not enabled):
src/cpuset.c:21:11: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:36:11: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c💯1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:124:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:152:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
src/cpuset.c:163:1: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn]
This can be backported.
Dmitry Sivachenko reported a build warning on FreeBSD -dev, where
__nonstring is apparently already defined. Let's guard our own
definition to avoid such issues. It could make sense to backport
this to recent stable versions which may soon be exposed to modern
compilers.
In regex_comp(), the error variable is either a const char* (USE_PCRE)
or a a uchar[] (USE_PCRE2), and navigating through the ifdefs is quite a
mess, making it hard to figure if it's always properly initialized when
printing an error message. Let's just preset it to NULL to clarify what
comes from where.
In task_schedule(), it is not enough to get the TASK_RUNNING flag before
setting the expire field, we also have to keep it while queueing the
taks, otherwise the task may run in the meanwhile and set expire to 0,
triggering the BUG_ON() in __task_queue() again. So now, only drop the
running flag once it's done.
This should be backported up to 2.8.
Latest commit 2dfbc311a8 ("BUG/MINOR: uri-auth: avoid leaks on
initialization error") left a possible null-deref case which was
surprisingly only detected by certain compiler combinations. No
backport needed.
In comp_prepare_compress_request(), the compression rate limit and CPU
usage checks were duplicated. The first set runs before selecting the
algorithm, and the second set runs after. That's definitely a copy-paste
issue or a patch being applied twice. Let's just drop one.
In http_re{q,s}_get_intercept_rule(), there are two occurrences of '&'
being used instead of '&&' which fortunately work thanks to the tests
being negations (hence 0/1 on each branch). Let's fix that and take this
opportunity for adding explicit precedence in http_apply_redirect_rule().
