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().
In h2_dump_h2s_info(), the tl.calls was being printed as signed instead
of unsigned, which is not correct but harmless (only used with "show
fd"). In the same function, we don't check if h2s->sd is valid while
dereferencing it. In practise it is valid since "show fd" is run under
thread isolation, but it's far from being obvious, and if conditions
would later change, we don't know it could be printed between h2s_new()
and h2s_frt_stream_new(). Finally in h2s_make_data() a wrong set of
H2_EV_RX_* flags were used instead of H2_EV_TX_* to emit traces.
In http_add_header() there are "type" variables of the same type at two
levels, which is a bit confusing. The inner one is for the "prev" block,
so let's rename it "ptype" by analogy with "pblk".
The definition of the PUBLISH message type indicates that the LSB are
independent, but uses a value of 0xF that clearly shows an attempt to
use a mask instead, but it results in all messages not having all flags
set to be rejected. A sane approach would have been to check for a mask
and an expected value. Let's just add a special case for it in function
mqtt_read_fixed_hdr() since that's for a single message type.
This can be backported anywhere.
In mqtt_parse_connack(), the MQTT_PROP_SUBSCRIPTION_IDENTIFIERS_AVAILABLE
case was checking and setting MQTT_FN_BIT_SUBSCRIPTION_IDENTIFIER instead
of MQTT_FN_BIT_SUBSCRIPTION_IDENTIFIERS_AVAILABLE, due to a copy-paste
mistake. This can be backported where needed.
In mqtt_parse_connect(), the MQTT_PROP_TOPIC_ALIAS_MAXIMUM case was checking
and setting MQTT_FN_BIT_TOPIC_ALIAS instead of MQTT_FN_BIT_TOPIC_ALIAS_MAXIMUM.
This means duplicate detection for Topic-Alias-Maximum property was using the
wrong bitmask, and the actual Topic-Alias-Maximum bit was never set, making
duplicate detection ineffective for this property. The CONNACK parser already
had this correct.
In mqtt_parse_connack(), the switch statement's default case for unknown
MQTT properties was using 'return 0' which returns MQTT_NEED_MORE_DATA.
This is misleading: an unknown property should be treated as an invalid
message (MQTT_INVALID_MESSAGE), like other functions do. This branches to
the "end" label without touching the preset return value instead. This can
be backported if needed.
In cfg_eval_cond_enabled(), the "KQUEUE" option incorrectly checks
GTUNE_USE_EPOLL instead of GTUNE_USE_KQUEUE. This is a copy-paste bug
from the preceding EPOLL case. It can be backported though it's harmless.
When the filter config (fconf) allocation fails in parse_cache_rule,
the previously allocated cache_flt_conf (cconf) and its strdup'd name
string are not freed. The error path only freed cconf but not
cconf->c.name, causing a memory leak.
No backport is needed.
When stats_add_scope() and stats_add_auth() fail to initialize a field,
they just leave a partially allocated and initialized structure behind
them that is leaked. The whole architecture doesn't provide clean
unrolling abilities since everything is shared and assigned unconditionally,
but let's at least release what was just allocated. The whole approach would
probably deserve being revisited if one day this becomes more dynamic.
No backport needed.
In userlist_postinit(), when an error occurs (missing group, missing user, or
allocation failure), the function returned immediately without freeing the
auth_groups_list linked lists that were built for all users in the first loop.
Each user's curuser->u.groups pointed to these allocated nodes, which leaked
on every error path.
Fix by replacing direct returns with a goto to a centralized cleanup label
that frees all users' groups lists before returning the error. Also fix a
trailing double space in one error return statement while refactoring.
Note that the impact is very low since we're supposed to fail to boo after
such errors.
When my_realloc2() fails in env_expand(), the code jumps to 'leave:' and
returns NULL, but the original input 'in' is never freed (it's only freed
at line 4919 in the success case). Given that callers typically pass it
the direct return of strdup(), it looks like it is expected to always be
freed. This can be backported everywhere.
action_http_set_status() checks for soft rewrite on the request message
by mistake instead of the response message. This could possibly cause a
rewrite failure when soft rewrite is enabled since it will not be seen
there, though the impact is extremely low. It can be backported.
smp_fetch_http_first_req() reads ->txn.http->flags without first
checking if txn.http is properly allocated. In theory if called from
the wrong context it could crash, even though tests where it's called
from "tcp-request content" don't seem to have any effect. Let's fix
it regardless, at least to dissipate the doubt. It can be backported
everywhere.
IPv6 addresses can be read enclosed in brackets, but the length of the
string is not checked before checking them. If by lack of luck, the
buffer is empty but already contains '[' in the first place, we'd read
the byte at position -1, possibly crashing (even though in practice it
will not since allocated blocks will be precedeed by the malloc meta-
data). At least it could make asan/valgrind unhappy.
This can be backported to all versions.
Since we can emit glitches during an H2 upgrade, we no longer have a
guaranteed h1s, so _h1_report_glitch() must check h1s before
dereferencing it. No backport is needed as this arrived in 3.4-dev11
with commit 72fd357814 ("MEDIUM: mux-h1: Return an error on h2 upgrade
attempts if not allowed").
Just a few minor user visible issues issues found in mux_h1 and http_htx
(traces, warnings and debug output). This may be backported though isn't
important at all.
This fixes typos and spelling mistakes in the following files:
channel-t.h channel.h filters-t.h http_htx.h htx-t.h tools.h
cfgcond.c channel.c flt_http_comp.c http_ana.c htx.c mqtt.c
mux_h1.c regex.c stats-proxy.c
Some functions such as tcp_parse_tcp_req() are able to emit their own
warnings by relying on warnif_misplaced_*() which directly prints the
warning. However when doing so they still increment the warning counter
which makes cfg_parse_listen() try to emit it, except that what's in the
variable is NULL, so we end up with:
[WARNING] (260) : config : parsing [/etc/haproxy/haproxy.cfg:17] : (null)
Let's just check the errmsg variable before printing the error. If it's
NULL, it's because the message was already printed.
This can be backported to all branches.
This lets lb_ops specify the conditions necessary to bind to this set of
ops. The condition is expressed as a list of mask and match fields on
the algorithm flags. This is then used in proxy_finalize() to locate the
lb_ops corresponding to the current configuration, by iterating over
the list of lb_ops structures. This list is implemented using the same
mechanisms used for configuration keywords: an INITCALL1 macro to a
registration function.
This also moves the lookup and property flags into the lb_ops structure
that were previously applied manually on a case by case basis.
Now the streams-elasticity limit applies to h2 frontend connections.
It allows to reduce the number of advertised streams based on the
number of concurrent connections.
This adds a new tune.streams-elasticity parameter. This parameter
indicates, as a percentage, the average number of streams per connection
at full load. It is used to calculate limits of the number of streams to
advertise on new connections. 0 means that no such limit is set.
When a limit is set, the new function conn_calc_max_streams() determines
the optimal number of streams to allow on a connection. It will assign at
least the ratio of streams left to connections left, and at least a fair
share of what's left times the number of desired streams. It will always
ensure that each connection gets at least 1 stream, and everything beyond
this will be evenly distributed. For now the function is not used.
In order to be able to enforce global streams limitations, we'll first
have to be able to account how many streams we promised to serve via
frontend muxes. We'll always need to support at least one stream, which
is why here we're only counting extra streams beyond the first one. It
also has the benefit of leaving H1 out of this, and save it from updating
a variable. Also in order to avoid an important update cost, we're storing
this value per thread group. For now only H2 is implemented, but QUIC
should follow shortly and should only count bidirectional streams.
In init_srv_requeue(), only attempt to run the tasklet if the server is
actually running, otherwise it will end up being queued a second time,
when the server is actually brought up, and that will lead to a
corrupted mt_list.
This can easily be reproduced by adding a dynamic server, as those start
disabled, and then enabling and disabling it a couple of times.
This should fix github issue #3360.
This should be backported up to 3.2.
Released version 3.4-dev11 with the following main changes :
- BUG/MEDIUM: acme: fix segfault on newOrder with empty authorizations
- BUG/MINOR: acme: skip auth/challenge steps when newOrder returns a certificate
- BUG/MINOR: sink: do not free existing sinks on allocation error
- CLEANUP: net_helper: fix incorrect const pointers in writev_n16()
- BUG/MINOR: vars: make parse_store() return error on var_set() failure
- BUG/MINOR: vars: don't store the variable twice with set-var-fmt
- BUG/MINOR: vars: only print first invalid char in fill_desc()
- BUG/MINOR: hpack: validate idx > 0 in hpack_valid_idx()
- MINOR: add an MPSC ring buffer implementation
- OPTIM: quic: rework the QUIC RX code
- MINOR: quic: store the DCID as an offset
- OPTIM: quic: reduce the size of struct quic_dgram
- BUG/MINOR: quic: handle cases where we don't have an address
- BUG/MEDIUM: cli: fix master CLI connection slot leak on client disconnect
- MEDIUM: mux-quic: extend shut to app proto layer
- MINOR: h3/hq_interop: implement stream reset on shut abort/kill-conn
- BUG/MINOR: acl: fix a possible arg corruption in smp_fetch_acl_parse()
- BUG/MINOR: map: do not leak a map descriptor on load error
- CLEANUP: map/cli: fix some map-related help messages
- BUG/MINOR: pattern: release the reference on failure to load from file
- CLEANUP: acl: remove duplicate test in parse_acl_expr() and unused variable
- CI: github: add DEBUG_STRICT=2 to ASAN jobs
- BUG/MINOR: quic: fix buffer overflow with sockaddr_in46
- BUG/MEDIUM: acme: fix stalled renewal when opportunistic DNS check fails
- BUG/MINOR: quic: fix trace crash on datagram receive
- MINOR: quic: fix trace spacing when datagram is displayed
- CLEANUP: mux-h2: remove the outdated condition to release h2c on timeout
- BUILD: add an EXTRA_MAKE option to build addons easily
- BUILD: otel: removed USE_OTEL, addon is now built via EXTRA_MAKE
- CLEANUP: otel: move opentelemetry outside haproxy sources
- BUG/MEDIUM: mux-h2: fix the body_len to check when parsing request trailers
- BUG/MAJOR: mux-h2: preset MSGF_BODY_CL on H2_SF_DATA_CLEN in h2c_dec_hdrs()
- DOC: otel: update the filter's status and URL in the docs
- DOC: acme: document missing acme-vars and provider-name keywords
- BUG/MINOR: dns: always validate the source address in responses
- BUG/MINOR: tcpcheck: Properly report error for http health-checks
- CLEANUP: resolvers: Remove duplicated line when resolvers proxy is initialized
- BUG/MINOR: resolvers: Free new requester on error when linking a resolution
- BUG/MINOR: resolvers: Fix lookup for a hostname in the state-file tree
- BUG/MINOR: resolvers: Free opts on parse error in resolv_parse_do_resolve()
- BUG/MAJOR: net_helper: also fix tcp_options_list for OOB write loop
- BUG/MEDIUM: ssl/sample: check output buffer size in aes_cbc_enc converter
- BUG/MAJOR: http-ana: fix private session retrieval on NTLM
- REGTESTS: add a regtest to validate various NTLM transitions
- BUG/MEDIUM: mworker/cli: fix user and operator permission via @@<pid> in master CLI
- BUG/MINOR: mworker/cli: check ci_insert() return value in pcli_parse_request()
- REGTESTS: http-messaging: always send RFC8441 client settings to use ext connect
- BUG/MINOR: h2: add decoding for :protocol in traces
- BUG/MINOR: mux-h2: condition the processing of 8441 extension to global setting
- MINOR: mux-h2: add a new message flag to indicate ext connect support
- BUG/MINOR: h2: only accept :protocol with extended CONNECT
- BUG/MINOR: acme: contact mail should be optional, don't pass ToS bool
- CLEANUP: http-fetch: Remove duplcated return statement in smp_fetch_stver()
- CLEANUP: http-fetch: Adjust smp_fetch_url32_src() comment
- CLEANUP: http-fetch: Fix indentation of sample_fetch_keywords
- BUG/MINOR: http_fetch: Check return values of unchecked buffer operations
- BUG/MINOR: http-fetch: Fix http_auth_bearer() when custom header is used
- BUG/MEDIUM: h1_htx: Remove reverved block on error during contig chunks parsing
- CLEANUP: haterm: Remove duplicated bloc to know if haterm must drain
- BUG/MINOR: haterm: Immediately report error when draining the request
- CLEANUP: haterm: Remove useless IS_HTX_SC() test
- BUG/MINOR: haterm: Fix a possible integer overflow on the request body length
- BUG/MEDIUM: haterm: Subscribe for receives until request was fully drained
- BUG/MINOR: haterm: Don't set HTX_FL_EOM flag on 100-Continue responses
- BUG/MEDIUM: haterm: Properly handle end of request and end of response
- BUG/MEDIUM: haterm: Properly handle client timeout
- BUG/MINOR: haterm: Fix condition to use direct data forwarding
- BUG/MINOR: haterm: Report a 400-bad-request error on receive error
- DEBUG: haterm: Add hstream flags in the trace messages
- MINOR: haterm: Remove now useless req_body field from hstream
- MINOR: mux_quic: reset stream after app shutdown for HTTP/0.9
- MINOR: mux_quic: do not perform unnecessary timeout handling on BE side
- BUG/MEDIUM: mux_quic: adjust qcc_is_dead() to account detached streams
- MINOR: mux_quic: simplify MUX_CTL_GET_NBSTRM
- MINOR: ssl: Export 'current_crtstore_name'
- MINOR: ssl: Factorize code from "new/set ssl cert" CLI command
- MINOR: ssl: Factorize ckch instance rebuild process
- MEDIUM: ssl: Refactorize "commit ssl cert"
- BUG/MINOR: ssl: Use the sequence number with kTLS and TLS 1.2
- BUG/MINOR: mux_quic: fix max stream ID reuse estimation
- MINOR: mux_quic: release BE conns if reuse definitely blocked
- BUG/MINOR: mux_quic: refresh timeout only if I/O performed
- MEDIUM: mux-h1: Return an error on h2 upgrade attempts if not allowed
- BUG/MEDIUM: mux-h2: Properly consume padding for DATA frames
- MEDIUM: tools: read_line_to_trash() handle empty files without \n
- MINOR: jws: support HMAC in jws_b64_protected(), make nonce optional
- MINOR: jws: introduce jws_b64_hmac_signature() function for HMAC signing
- MINOR: acme: implement EAB - external account binding
- MINOR: acme: allow specifying custom MAC alg for EAB
- REGTESTS: Fix h1_to_h2_upgrade.vtc to force h2 on first bind line
- MINOR: cli: allow specifying a tgid with show fd
- Revert "BUG/MEDIUM: cli: fix master CLI connection slot leak on client disconnect"
- BUILD: use Makefile.mk instead of Makefile.inc in EXTRA_MAKE
- Revert "BUG/MINOR: mux-h2: condition the processing of 8441 extension to global setting"
- BUG/MEDIUM: mux-h2: fix the detection of the ext connect support
- MINOR: jwe: Add option to enable/disable algorithms or encryption algorithms for jwt_decrypt
- MINOR: jwe: Disable 'RSA1_5' algorithm by default in jwt_decrypt converters
- BUG/MEDIUM: jwe: Fix jwt.decrypt_alg_list to work correctly
- BUG/MEDIUM: stick-table: properly check permissions on CLI's set/clear cmd
- DOC: acme: EAB is now supported
The "set stick-table" CLI command's permissions are checked a bit too
late in the I/O handler, because the lookups performed at parsing time
can already cause an entry to be created at level "user" even though the
user does not have the permission to go further and to fill the data in.
Note that the impact remains pretty low since the entry is created without
data being touchable, and all within the table's settings (max entries,
expire etc). In addition it cannot even be used to periodically refresh
an entry and prevent it from expiring because only a creation is handled
at this point.
Let's add the check in cli_parse_table_req() so that these privileged
commands are entirely denied past the table lookup. This way it remains
possible to know that the table doesn't exist, like for the "show" command
but not more.
This should be backported to all stable branches, because the bug right
now cannot result in an accidental use (entries are not properly created
and deletion does not work).
Thanks to Omkhar Arasaratnam for finding and reporting this.
