REGTESTS: connection: test the NetScaler CIP protocol
Neither the "accept-netscaler-cip" bind keyword nor the
"tcp-request connection expect-netscaler-cip layer4" action had any
coverage, the notes listing them as doable now that the expect-proxy
test shows the pattern, the only obstacle being that the CIP header has
to be built by hand. It is built with sendhex here, in its standard
form, and the test follows the structure of the expect-proxy one.
The first frontend declares the magic number on its bind line, which
makes the header mandatory: a well-formed IPv4 header, then an IPv6 one,
replace the addresses of the session while fc_src keeps reporting the
real peer, whereas a wrong magic, an encapsulated protocol which is not
TCP, and a request with no header at all are all rejected.
The second frontend uses the action, which only expects the header from
the sources matching its condition. Note that the action reads the
magic number from the bind line, so without "accept-netscaler-cip"
there the expected magic is zero, which the test documents.
The action had no coverage, and the notes listed adding a fourth
backend here as the way to get it. That backend is a copy of the one
which retries on a 503, with "disable-l7-retry" in an http-request
rule: the 503 of the first server must then reach the client as it is,
with txn.conn_retries left at zero, and the second server must not be
contacted at all.
REGTESTS: http-rules: cover the rfc7239_nn and rfc7239_np converters
The test already covers rfc7239_field, rfc7239_is_valid, rfc7239_n2nn
and rfc7239_n2np, and the two converters which build a node name and a
node port were the only ones of the family left out. They are added to
the backend which already extracts the node of a "for" field, with one
IPv4, one IPv6, one string and one integer input, which covers both
branches of each: an address or a number is emitted as it is, an IPv6
is bracketed and a string becomes an obfuscated identifier prefixed
with an underscore.
REGTESTS: stick-table: cover the remaining gpc and gpt spellings
Thirteen spellings of the general purpose counters were still left out
of this test: the legacy sc1_ and sc2_ increments and clears, the sc1_
readers, sc0_inc_gpc0, sc0_clr_gpc0, sc_inc_gpc1(), sc_clr_gpc1() and
the generic sc_clr_gpc() of the array form, plus the whole src_inc_gpc*
and src_clr_gpc* family which the notes listed as a follow-up.
The first frontend cannot host them: its sc1 tracks the array table,
which the legacy fetches do not accept, and its counters already carry
the values the existing clients assert. A second frontend therefore
tracks one legacy table through its three counters, which makes the
aliasing visible since an increment through one of them is read back
through the other two, and a third frontend, whose counter tracks an
array table, is only there for the generic clear of the array form.
REGTESTS: stick-table: read the counters through sc1_ and sc2_ as well
The sc1_* and sc2_* families are pure aliases of the sc0_* ones with a
different hard-coded stick counter id, and none of their 29 entries was
covered. The notes listed extending this test as the way to close them
all at once, which is what this does.
Two more tables are added, one per extra stick counter, rather than
tracking the single existing one three times, which would have counted
every connection and every request three times and forced all the
existing values to be rewritten. Each counter therefore sees its own
copy of the same entry, and the sc1_ and sc2_ readings are compared to
the sc0_ ones rather than to hard-coded values. The generic form with
the ids 1 and 2 is read the same way.
sc0_bytes_in_rate and sc0_bytes_out_rate, which the first version had
missed, and src_updt_conn_cnt(), which creates its entry as it reads it
and therefore gets a table of its own, are covered too.
REGTESTS: ssl: test ssl_c_used, ssl_c_san and ssl_c_ca_err_depth
These three client certificate fetches were the ones the ssl_c_* test
did not report, the first two only because they were forgotten, the
last one because it needs a chain which does not verify.
The first client gets the two easy ones plus ssl_c_err and ssl_c_ca_err
which must both be zero for the certificate which does verify.
A second listener sends common.pem as a client certificate. It carries
a subjectAltName, which client1.pem does not, and its intermediate CA
is unknown to the ca-file of the bind line, so the chain breaks at
depth 1 and ssl_c_ca_err_depth reports it. Being a server certificate,
it also fails the purpose check at depth 0, hence the "ca-ignore-err
all" added next to the existing "crt-ignore-err all" so that the
handshake completes and the fetches can be read.
REGTESTS: sample_fetches: test the req.ssl_alpn fetch
req.ssl_alpn was the last fetch of the raw ClientHello family without a
test of its own, it only appeared as a condition in the tcp-check test.
It is also the only one of the family which returns several samples,
one per protocol name of the extension, so the test checks that a
log-format only sees the first name while an ACL matches any of them.
Five hand-crafted ClientHellos are used: one advertising "h2" and
"http/1.1" behind a server_name extension, one advertising the single
"acme-tls/1" name of the documentation example, one without any ALPN
extension, one whose ALPN extension is too short to hold a name, and
one announcing a name longer than the extension which carries it, the
last two checking that the fetch gives up instead of reading the
extension which follows.
REGTESTS: sample_fetches: test req.ssl_ver on an SSLv2 CLIENT-HELLO
req.ssl_ver is the only fetch of the family which accepts the SSLv2
record format in addition to the SSLv3 one, and that second half of
smp_fetch_req_ssl_ver() was not reached by the ClientHellos of the
first frontend. It is also the only one whose ACL parses its pattern
in dotted notation, which was not covered either.
A second frontend, which rejects everything and only reports the
fetches in its log, receives an SSLv2 CLIENT-HELLO with the short
record header announcing SSLv3.0, another one with the long record
header, its padding length, two ciphers and a 32-byte challenge,
announcing TLSv1.0, and finally a plain HTTP request which is neither.
The version is checked both as a number and through the "3.0" and
"gt 3.0" patterns of the two ACL spellings, while req.ssl_hello_type
and req.ssl_sni, which only know the SSLv3 record, return nothing.
REGTESTS: protobuf: cover all the field types of the ungrpc converter
The first message only used int32 leaves and a string, that is two of
the fifteen types the converter accepts as its second argument, and a
single gRPC message, while the converter is able to walk over as many
of them as the buffer holds.
A second frontend receives a message holding one field per type, the
field number being the same as the index of the type in the list of the
documentation, so that every branch of protobuf_smp_store_varint(),
protobuf_smp_store_32bit() and protobuf_smp_store_64bit() is visited,
including the zigzag decoding of sint32/sint64 and the little-endian
decoding of the fixed size types. Two fields are also extracted without
a type to check the binary form, and three others with a type which
does not match their wire type, which must return nothing.
The test records that a "float" or a "double" field is truncated to an
integer, which is what the documentation announces ("as an integer
sample") but is easy to overlook.
A third frontend receives a body made of two gRPC messages so that the
loop searching the field in the messages which follow is covered.
REGTESTS: http-messaging: test the options dropping the HTTP trailers
"option http-drop-request-trailers" and
"option http-drop-response-trailers" had no coverage.
vtest can send HTTP/1 trailers but cannot parse them, so neither end of
the chain may be a vtest HTTP peer when the trailers are expected to be
forwarded: the request side origin is a small haproxy instance, and the
client of the forwarded response side only reads the headers and then
the exact number of remaining bytes.
What actually leaves the proxy is observed at the byte level by two
extra haproxy instances running in TCP mode, one on each side, which
report through their log whether the trailer field name appears in what
they relayed. This gives a positive assertion for both the dropped and
the forwarded case, and the "no option" form is covered as well.
REGTESTS: protobuf: add a test for the ungrpc converter
ungrpc had no coverage while the protobuf converter is exercised by
nested_and_flat.vtc. It differs from protobuf by the 5-byte gRPC header
it skips first, and by the rule that only the first converter of a
chain may be an ungrpc, the following ones having to be protobuf.
The body is the "Rectangle" message of the documentation example, built
by hand with sendhex: a string field, and two nested messages holding
two varint leaves each. The test extracts the length-delimited field
without a type, the four leaves through their dotted paths, an
intermediary field as a binary sample, and checks that the three
documented equivalent spellings mixing ungrpc and protobuf all return
the same value. A field which is not in the message must yield nothing.
REGTESTS: ssl: test the ssl_fc_* and ssl_bc_* connection fetches
The existing SSL tests cover the certificate fetches (ssl_f_*, ssl_c_*
and ssl_s_*) but almost none of the ones describing the connection
itself. This test chains a clear frontend to an SSL backend and an SSL
frontend so that both families are read on the same exchange: the
ssl_bc_* ones from the client side of the SSL connection and the
ssl_fc_* ones from its server side, which lets the two be compared.
It covers ssl_bc, ssl_bc_alpn, ssl_bc_cipher, ssl_bc_protocol,
ssl_bc_alg_keysize, ssl_bc_use_keysize, ssl_bc_sni, ssl_bc_is_resumed,
ssl_bc_session_id, ssl_bc_unique_id, ssl_bc_client_random,
ssl_bc_server_random, ssl_bc_session_key, and on the frontend side
ssl_fc_protocol, ssl_fc_protocol_hello_id, ssl_fc_alg_keysize,
ssl_fc_use_keysize, ssl_fc_has_sni, ssl_fc_has_crt, ssl_fc_crtname,
ssl_fc_has_early, ssl_fc_session_id, ssl_fc_unique_id,
ssl_fc_client_random, ssl_fc_server_random, ssl_fc_session_key, plus
the captured ClientHello fetches ssl_fc_cipherlist_str, _hex, _bin,
_xxh, ssl_fc_eclist_bin, ssl_fc_ecformats_bin, ssl_fc_extlist_bin and
ssl_fc_sigalgs_bin.
A second pair of listeners pins TLSv1.3 to reach what only exists
there: ssl_fc_supported_versions_bin and the four traffic secrets plus
the exporter secret, on both ends.
The capture buffer is sized so that the whole ClientHello fits,
otherwise the extension lists which follow the cipher list are
silently reported as empty.
REGTESTS: connection: do not race with the reset in the expect-proxy test
The client which sends a malformed PROXY header also sent a request
behind it. The connection is reset as soon as the header line is
complete, so on a build where the reset comes back fast enough the
write of the request fails with ECONNRESET instead of succeeding, and
the test fails. Nothing needs to be sent after the bad header.
REGTESTS: sample_fetches: test the raw TLS hello fetches
req.ssl_hello_type, req.ssl_ver, req.ssl_ec_ext, req.ssl_st_ext and
res.ssl_hello_type had no coverage, nor had the deprecated
req_ssl_hello_type, req_ssl_ver, req_ssl_sni and rep_ssl_hello_type
aliases. They parse the raw buffer of a TCP proxy without any SSL data
layer, so the handshake messages are crafted by hand with sendhex the
same way the other ClientHello tests do, and a server answers each of
them with a ServerHello so that the response side is covered too.
Three ClientHellos are used: one carrying server_name, supported_groups
and an empty session ticket, one carrying a session ticket with data
and no supported_groups, and one carrying a single unrelated extension
so that req.ssl_st_ext reports its documented zero.
REGTESTS: connection: test the expect-proxy action
"tcp-request connection expect-proxy layer4" had no coverage. Unlike
the "accept-proxy" bind option, which makes the PROXY protocol header
mandatory for everybody, this action only expects it from the
connections matching its condition.
The test announces addresses which differ from the real ones so that
src and dst show which of the two was used, checks that fc_src keeps
reporting the real peer of the connection, that a connection from a
matching source is rejected when the header is missing or malformed,
and that a source which does not match the condition is served without
any header.
REGTESTS: cli: test the privilege levels of the CLI
The "operator" and "user" commands, which drop the privileges of the
current connection, had no coverage, and "show cli level" was only
reported by the smoke test.
The test walks down the three levels and checks both what each of them
still allows and what it refuses: reading the statistics and the
process information is allowed everywhere, clearing the counters
requires operator, and changing a global setting, a server state,
an ACL or entering the expert or experimental modes requires admin. It
also verifies that the levels only go down, and that a refused command
really left the setting untouched.
Each case chains the level change and the command it affects on a
single line because the level only applies to the connection it was set
on, and vtest opens a new connection for every "send".
REGTESTS: checks: test the health adjusting from observed traffic
The "observe", "error-limit" and "on-error" server settings had no
coverage. They adjust the health of a server from the traffic it serves
rather than from the health checks, which makes them testable without
any timer: the state changes exactly when the configured number of
consecutive errors has been served.
The test checks that a 404 is not counted as a layer 7 error while a
500 is, that the server is marked down once the error limit is reached
and that the backend then answers 503, and that a failed connection is
enough in layer 4 mode. It also verifies that the resulting state
change is attributed to the health analysis and not to a check result.
The health checks are directed at a second haproxy instance which
accepts the connections and never answers, with "option httpchk" so
that no check can complete and interfere with the observation.
REGTESTS: server: test the server settings reported by the runtime
A large part of the "server" keywords had no coverage: addr, port,
agent-addr, agent-port, rise, fall, init-state, minconn, maxqueue,
backup, non-stick, pool-purge-delay, pool-max-conn, pool-low-conn,
max-reuse and the explicit identifiers.
They are all visible through "show servers state", "show stat typed"
and "show servers conn", which makes them checkable without any traffic
and without waiting for a health check to complete. The checks are
directed at a second haproxy instance which accepts the connections and
never answers, so that no check can complete and the initial states
stay observable for the whole test, and the proxies and servers are
given explicit identifiers so that the field names of "show stat typed"
are predictable. Each dump is restricted to a single server because the
full typed dump of nine servers does not fit in one CLI response.
The four documented init-states are checked against the health counter
they set up, as is the fact that a server which says nothing inherits
the default-server value.
Note that "init-state none" is documented as restoring the default
behavior when the setting was inherited from default-server, but it
currently has no effect: srv_set_init_state() returns early for
SRV_INIT_STATE_NONE and therefore does not undo what the inherited
value had already applied. The test matches the current behavior.
REGTESTS: sample_fetches: pin one thread in the proxy state test
Same reason as for the connection address test: act_conn and fe_conn
are over-counted when several threads accept in parallel on a loaded
machine, which made the connection counters of this test unpredictable
when the suite is run with vtest -j.
REGTESTS: sample_fetches: pin one thread in the connection address test
The test asserted that dst_conn reports exactly one connection, which
holds when it runs alone but not when the reg-tests are run in parallel
on a loaded machine: it was reported failing with a value of 8.
The extra connections are not real. With several threads accepting in
parallel, the process ends up reporting current connections which were
never established: after the only client has left, "show info" reports
a non-zero CurrConns while CumConns has only counted the connections
which really happened, and "show stat" shows the same discrepancy
between scur and stot for the frontend. Forcing nbthread to 1 makes the
counters exact again, which is what this test needs since it checks the
address fetches, not the connection accounting.
req.len, req.payload and their "res." counterparts, the payload_lv
family and wait_end had little or no coverage. They are only usable in
TCP mode, on the raw buffers, so the test runs a plain TCP proxy and
reports what its rules see through the log.
The client sends a 4-byte big endian length, its payload and two
trailing bytes, and the server answers with the same framing, which
gives payload_lv() something to extract on both sides, including with a
relative second offset. A second client never sends enough data, which
lets the inspection delay expire so that wait_end becomes true and the
connection is rejected.
The log format ends with %B on purpose: a format made only of var()
fetches is emitted as soon as the server connection is established,
that is before the response side rules have run.
REGTESTS: http-rules: test the declared capture slots
The "declare capture" directive, the "id" form of the capture action
and the capture-req()/capture-res() converters had no coverage; only
"capture request header" and capture.req.hdr() were used elsewhere.
The test declares three request slots and two response slots in the
frontend, fills them from the frontend and from the backend, with the
action and with the converter, and reads them back with
capture.req.hdr() and capture.res.hdr(). It also checks that the
converters return their input unchanged, that an undeclared slot yields
nothing and that a slot whose input is missing stays empty.
REGTESTS: cli: test the commands which change the run-time settings
None of the CLI commands changing a run-time setting was covered: set
maxconn for the process, a frontend and a server, set rate-limit for
the connections and the sessions, set weight in absolute value and as a
percentage, get weight, set server state, disable/enable/shutdown
frontend, set profiling, set anon, set severity-output, set timeout,
set table, clear table and echo.
Each change is read back with the command which reports it, so that
both the setter and the reporter are exercised: "show info" for the
global limits, the "slim" and status columns of "show stat" for the
proxy and server limits and states, "get weight", "show profiling",
"show anon" and "show table".
REGTESTS: cli: add a smoke test for the read-only CLI commands
Most of the "show" commands of the CLI had no coverage at all, so a
crash or a broken output in one of their dump functions would go
unnoticed. This test issues show version, cli level, cli sockets, env,
startup-logs, dev, backend, defaults, default-server, servers state,
servers conn, stat, info, schema json, sess, errors, fd, pools,
activity, threads, tasks, profiling, anon, table, map and acl once on a
small but non-empty configuration.
The patterns are deliberately loose: the point is to verify that each
command is accepted and that its dump reaches its first line, not to
freeze output formats which are explicitly allowed to change.
REGTESTS: proxy: test monitor-uri and the monitor fail condition
"monitor fail" had no coverage and "monitor-uri" was only used as a
side setting of a health check test.
The test checks that the monitor URI is answered by the frontend
without reaching the backend while the other URIs are still forwarded,
that the status switches from 200 to 503 when the condition is met,
that the "unless" form inverts that, and that the monitor response
goes through the error files of the proxy as documented.
REGTESTS: log: test the log filtering options of a frontend
"option dontlognull", "option dontlog-normal" and "option logasap" had
no coverage, and "option log-separate-errors" was only used as a side
setting of an SSL test.
Each option gets its own frontend: a connection on which nothing is
sent must not be logged while the next real request is, a successful
request must not be logged while the one ending on a 503 is, the same
pair must both be logged but at different syslog levels, and the
message emitted early must carry the '+' prefix on the byte count and
on the total time.
Two syslog servers are needed since vtest only matches the messages
emitted at the level its server was declared with, which is precisely
what log-separate-errors changes.
REGTESTS: http-rules: test the tarpit, deny and silent-drop actions
The tarpit action was only referenced by the "set-timeout" test,
silent-drop was not covered at all, and deny was always used in its
default form, never with a customized status, body, content type or
extra header.
The test checks the default and the customized status of tarpit, the
"PT" termination state it produces in the log, the customized response
of deny, and that silent-drop closes the connection since we're using
rst-ttl here for portability reasons. The pause action is exercised
too, with a short delay.
"timeout tarpit" is set to a small value so that the test stays fast.
REGTESTS: proxy: test the connection retries, retry-on and redispatch
"retry-on" and "option redispatch" had no coverage, and the
txn.conn_retries and txn.redispatched fetches which report their effect
were not covered either.
Three backends are used: one whose only server is unreachable, which
must report as many retries as configured before giving up with a 503,
one which redispatches the retry to a healthy server, and one which
retries at the HTTP layer because the first server answers with a
status listed in "retry-on".
The connect timeout of the two backends which fail to connect is
shortened since haproxy spreads the retries over it, which would
otherwise make the test last as long as the default timeout.
REGTESTS: proxy: test the "option checkcache" response filter
"option checkcache" had no coverage. The test walks through the
conditions listed in the documentation: a cacheable 200 carrying a
Set-Cookie is blocked, a response made non cacheable by "cache-control:
private", "no-store", "no-cache=\"set-cookie\"" or "pragma: no-cache"
goes through, a status which is not cacheable by default goes through
unless the server declares the response public, and the response to a
POST is not filtered.
The origin is a second haproxy instance crafting one response per path,
because a blocked response makes the proxy close the server connection,
which a single vtest server session would not survive.
The test also documents one behavior which the documentation does not
mention: a response carrying neither freshness information nor a
validator is not considered cacheable.
REGTESTS: http-errorfiles: test the errorloc directives
The errorloc, errorloc302 and errorloc303 directives had no coverage
while errorfile and http-error do. The test checks the three forms on a
503 caused by an unreachable server and on a 403 returned by a deny
rule, verifies that "errorloc" is an alias for "errorloc302", that a
relative location is accepted, and that a status code for which no
errorloc was declared still gets the default error page.
REGTESTS: converter: add a test for the when() and debug() converters
Neither converter was covered. when() only lets its input through when
the condition is met, so the test builds a string made of one bracketed
field per condition and compares it for three kinds of transactions: one
forwarded to a healthy server, one whose server cannot be reached, and
one answered by the stats applet. This covers the error, normal,
forwarded, processed, toapplet, stopping and acl conditions as well as
the negated form.
debug() is checked to be transparent for the sample it is inserted in,
and its copy is read back from the default sink with "show events".
REGTESTS: sample_fetches: add a test for the constant and process fetches
The constant fetches always_true, always_false, str, int, bin, bool,
ipv4, ipv6 and meth, the process state ones pid, thread, tgroup, nbproc,
quic_enabled, cpu_calls, cpu_ns_tot, cpu_ns_avg, lat_ns_tot, lat_ns_avg,
cpu_usage_proc, cpu_usage_thr and cpu_usage_grp, plus date_us, env(),
rand() and dump_all_vars had little or no coverage.
The values which cannot be predicted are matched against their expected
shape, while pid is compared to the process identifier reported by
vtest, rand(10) must stay within its range, env() is looked up on the
HAPROXY_LOCALPEER variable which the process always exports, and
dump_all_vars must mention a variable set by an earlier rule.
REGTESTS: tcp-rules: test the set-src, set-dst and port rewriting actions
set-src-port and set-dst-port had no coverage at all, and set-src and
set-dst were only used incidentally by other tests.
The test applies them from the "tcp-request connection", "tcp-request
session", "tcp-request content" and "http-request" rule sets and checks
the documented behaviors: set-src and set-dst preserve the port while
set-src-port and set-dst-port preserve the address, and a later rule set
overrides what an earlier one did.
It also shows that a rewrite performed from a "tcp-request connection"
rule alters the connection itself, so that fc_src and fc_dst report the
new values, while a rewrite performed at the session level leaves them
untouched.
REGTESTS: http-rules: test replace-uri and the header replacement actions
replace-uri had no coverage at all, and replace-header and replace-value
were only used by a test checking their size limits, not their
semantics.
The test rewrites a request target in the origin form and in the
absolute form to show that replace-uri also sees the scheme and the
authority, rewrites the scheme of an absolute URI, checks that a non
matching regex leaves the target untouched, and compares the result with
replace-path and replace-pathq on the same target.
It also checks that replace-header replaces the whole header line while
replace-value only replaces the matching comma-delimited element, and
that back-references work in the replacement format.
REGTESTS: http-rules: test the binary header manipulation actions
The add-headers-bin, set-headers-bin and del-headers-bin actions had no
coverage at all. The test saves the incoming header block with
req.hdrs_bin, drops a few headers and restores them from the saved
block, with and without the "prefix" filter, and checks on the server
side which headers made it through.
The lists passed to del-headers-bin are built by hand with the bin()
fetch so that the varint encoding of the header names is exercised too,
with the default exact matching and with the "beg" one.
REGTESTS: log: test the transaction state fetches and the logging actions
The txn.sess_term_state, term_events, last_entity, waiting_entity,
txn.conn_retries, txn.redispatched, txn.id32, txn.timer.total,
txn.timer.user, req.timer.queue, res.timer.data, prio_class, prio_offset,
bc_be_queue, bc_srv_queue, fc_err_name and bc_err_name fetches were not
covered, and neither were the set-priority-class, set-priority-offset,
set-nice, set-log-level and do-log actions.
They are checked from a custom log format since most of them only reach
their final value once the transaction is over. The test verifies that
"set-log-level err" raises the syslog priority of the message, that
"set-log-level silent" suppresses it entirely, that "do-log" emits an
extra message before the response exists, and that a denied request is
reported with the "PR" termination state and the rule which took the
decision.
Two syslog servers are needed because vtest only matches the messages
emitted at the level its server was declared with.
Note that path, url and the req.timer.hdr/idle/tq fetches are not usable
at log time, hence the use of capture.req.uri here.
REGTESTS: http-rules: test the HTTP authentication action, fetches and ACL
Neither the "auth" action nor the http_auth_user, http_auth_pass,
http_auth_type and http_auth_group fetches were covered, and the
userlist section was only exercised through the stats.
The test declares a userlist with two users, one of them in a group,
and checks that the credentials are readable before being validated,
that http_auth() only matches with the right password, that
http_auth_group() only matches for the member of the group, and that
the "auth" action emits a 401 carrying the requested realm until valid
credentials are presented.
REGTESTS: stick-table: test the traffic counters of the stick counters
The connection, session, request, error, failure and byte counters of
the stick tables had no coverage, nor had the sc_*, sc0_*, src_* and
table_* families giving access to them, nor sc_key, sc_tracked,
sc_trackers, table_trackers, table_server_id, table_expire, table_idle,
in_table, table_avl and table_cnt.
A single client connection issues a normal request, a request denied
with a 403 and a request answered with a 500 by the server so that the
request, error and failure counters all move, and the three access
families are compared against each other at each step.
The values are grouped into a few headers because vtest only parses a
limited number of them.
REGTESTS: stick-table: test the general purpose counters and tags
The sc-add-gpc, sc-inc-gpc, sc-inc-gpc0, sc-inc-gpc1, sc-set-gpt and
sc-set-gpt0 actions had no coverage, and neither had the sc_*_gpc*,
sc_*_gpt*, sc0_*, sc1_*, sc2_* and src_* fetches nor the table_*gpc*
and table_*gpt* converters.
The array form of the general purpose storage (gpc, gpc_rate and gpt)
and the legacy scalar one (gpc0, gpc1 and gpt0) may not coexist in a
same table, so the test declares one table of each kind. The legacy one
is tracked by two stick counters at once, which lets the sc0_ and sc2_
shorthands be checked against each other and against the generic
sc_get_*(<sc-id>) form.
The three ways of reaching an entry are compared on each step: the stick
counters, the src_* fetches and the table_* converters, and the
increment, addition, reset and read variants are all exercised.
Note that among the shorthands of the array form, only sc1_clr_gpc() is
registered, sc0_clr_gpc() and sc2_clr_gpc() do not exist.
REGTESTS: sample_fetches: add a test for the proxy and server state fetches
None of the fetches reporting the internal state of the proxies and of
their servers was covered. This test checks fe_id, fe_name, fe_defbe,
be_id, be_name, srv_id, srv_name, nbsrv with and without an argument,
srv_is_up, srv_weight, srv_iweight, srv_uweight, act_conn, fe_conn,
be_conn, be_conn_free, srv_conn, srv_conn_free, connslots, queue,
avg_queue, srv_queue, fe_sess_rate, fe_req_rate, be_sess_rate,
srv_sess_rate, stopping, uptime and fe_client_timeout, plus the nbsrv,
srv_is_up, srv_queue and fe_exists converters.
The backend holds a usable server and a disabled one so that nbsrv() and
srv_is_up() have something to distinguish, and both servers have a
maxconn and a maxqueue so that connslots() returns a computable value
instead of the -1 it reports on an unbounded configuration.
The fetches whose implicit argument is the current proxy are placed in
the backend since they require the backend capability.
REGTESTS: sample_fetches: add a test for the connection address fetches
None of the address fetches of the frontend and backend connections was
covered. This test checks src, src_port, src_is_local, dst, dst_port,
dst_is_local, dst_conn, their fc_* equivalents, so_id, so_name,
fc_http_major, fc_nb_streams, fc_rcvd_proxy, fc_err_name, and on the
backend side bc_src, bc_src_port, bc_dst, bc_dst_port, bc_http_major,
bc_nb_streams, bc_be_queue, bc_srv_queue and bc_err_name.
Everything runs over the loopback so that both endpoints are known: the
port of the frontend and the address and port of the server are compared
against the macros exported by vtest, and the src/fc_src and dst/fc_dst
pairs are compared against each other.
However the _local tests fail on macos, and this reveals some weakenesses
in the addr_is_local() function (error checking might be too broad) and
its caller (since that function may return -1 which is taken as a bool).
So for now the local tests are disabled.
REGTESTS: sample_fetches: add a test for the HTTP request fetches
The request side header, URI and body fetches were largely uncovered.
This test checks req.hdr_val, req.hdr_ip, req.hdr_cnt with an occurrence
index, req.hdr_names with and without a custom delimiter, req.fhdr and
req.fhdr_cnt against req.hdr on a comma-delimited list, req.hdrs,
req.hdrs_bin, base, baseq, base32, base32+src, url32, url32+src, url_ip,
url_port, query, urlp_val, req.body_len, req.body_size, req.body_param
with and without the case-insensitive flag, req.timer.hdr,
req.timer.idle, req.ver, req.proto_http, the deprecated req_ver and
req_proto_http aliases, and the capture.req.method, capture.req.ver,
capture.req.uri and capture.req.hdr fetches.
The fetches are collected into variables from the http-request rule set
since most of them may not be used from the http-response one, and they
are reported in the response headers so that a single exchange suffices.
An absolute URI is used so that base, baseq, url_ip and url_port have
something to extract, which requires the Host header to match the
authority. A second request uses an address as the authority since
url_ip and url_port do not resolve host names.
REGTESTS: sample_fetches: add a test for the HTTP response fetches
Almost none of the res.* sample fetches was covered. This test checks
status, txn.status, res.ver, res.hdr, res.hdr_val, res.hdr_ip,
res.hdr_cnt with an occurrence index, res.hdr_names with and without a
custom delimiter, res.fhdr and res.fhdr_cnt against res.hdr on a
comma-delimited list, res.cook, res.cook_cnt, res.cook_val,
res.cook_names, res.body_size, res.hdrs, res.hdrs_bin, res.comp,
res.comp_algo and res.timer.hdr, as well as the deprecated shdr, shdr_val,
shdr_ip, shdr_cnt, scook, scook_cnt, scook_val and resp_ver aliases, and
the capture.res.hdr and capture.res.ver fetches fed by a response header
capture.
REGTESTS: http-rules: test the ACL and map management commands of the CLI
Out of the 13 CLI commands dealing with ACLs and maps, only "show map"
and "clear map" were used by another reg-test. This one covers them all
except the "#<ref>" form of "del" and "set" which requires capturing a
pointer from a previous dump: add acl, add map, del acl, del map, clear
acl, clear map, get acl, get map, set map, show acl, show map, and the
prepare/commit sequence for both.
The atomic replacement is verified from the data plane as well: the
entries added to the prepared version are invisible until the commit,
after which they have replaced the previous ones rather than being
merged with them.
Virtual lists are used so that the test needs no data file.
REGTESTS: http-rules: test the run-time ACL and map update actions
The add-acl, del-acl, set-map and del-map actions were not covered at
all, nor were the "file@", "opt@" and "virt@" name formats described in
section 2.7.
The test adds and removes entries from one request and observes them
from the next one, on an ACL loaded from a regular file and on three
maps designated respectively by a plain file name, by the same name
prefixed with "file@" to verify that both reference the same list, by
"virt@" which has no file at all, and by "opt@" pointing at a file which
does not exist. It also checks that updating an existing key replaces
its value instead of duplicating the entry, that the set-map action is
usable from the http-after-response rule set, and that the resulting
lists are visible from "show acl" and "show map".
REGTESTS: converter: cover all the map match methods and output types
Out of the 38 registered map converters, only map, map_str, map_dom,
map_beg and map_regm were exercised by the existing reg-tests, which
left the wiring of most of the entries of the keyword table unverified.
This test looks up every one of them at least once: the str, beg, sub,
dir, dom, end, int, ip, reg and regm match methods combined with the
default (string), _int, _ip and _key output types. It also covers an
integer range key, an IPv6 key, a network key, the difference between
map_reg and map_regm on a value holding a back-reference, the default
value argument and the failure of the converter when nothing matches
and no default is provided.
The output type is applied to the whole map at load time, so the same
key set is duplicated in three files holding string, integer and address
values, plus two smaller files for the integer and address keyed maps.
REGTESTS: converter: add a test for the date formatting converters
utime, ltime, ms_utime, ms_ltime, us_utime, us_ltime and http_date were
not covered at all. They are checked here against fixed input dates, so
that the expected output is constant, including the offset argument, the
%N specifier with its various widths and the unit argument of http_date.
The local time variants depend on the machine's timezone, so their
output is only matched loosely, which is still enough to verify that the
date and the sub-second part are correct.
The test also verifies that accept_date() and date() remain consistent
with each other when expressed in seconds, milliseconds and
microseconds. A test also checks that the ms_*/us_* converters apply
their offset in the right unit.
REGTESTS: converter: add a test for the string trimming and parsing converters
The ltrim, rtrim, cut_crlf, base2, port_only and language converters had
no coverage at all, and host_only was only used incidentally by other
tests. They are all checked here, including the corner cases of a string
entirely made of trimmed characters, of a host value without a port, of
an IPv6 host, and of an accept-language header matching nothing with and
without a default value. The "q=" test is also improved to test "q=0".
The input of cut_crlf comes from an URL-encoded query string parameter
since a header field may carry neither a CR nor a LF.
REGTESTS: converter: add a test for the arithmetic and bitwise converters
None of the integer converters was covered by a reg-test so far. This
test checks add, sub, mul, div, mod, neg, cpl, and, or, xor, not, bool,
odd, even, htonl and hex2i at once, using the "int()" fetch as an input
so that no server nor client data is needed.
The operand of add, sub and div is also passed as a variable so that
both the constant and the variable forms are covered, and the documented
behavior of a null divisor is verified for div() and mod().
Note that hex2i() currently fails on a non-hexadecimal input instead of
returning zero as documented, the test matches the implementation.
DOC: config: mention that url_ip and url_port fail when passed a name
url_ip and url_port both rely on a call to url2sa() which tries to parse
digits in the authority part. This means that neither is usable with a
host name. For example www.example.org:443 will not return an IP address
nor even port 443 since the IP address parsing fails early. It's worth
being said since it's not obvious.
DOC: config: mention other responses not blocked by option checkcache
The doc for "option checkcache" forgot to mention that responses with
no validator nor object freshness info are not considered and can pass
unfiltered to the client (it's because these info were developed with
the cache that didn't exist when the option was developed).
BUG/MINOR: sample: return the decoded JWT part when no path is given
jwt_header_query() and jwt_payload_query() are documented to return the
decoded header or payload of the token when they are called without any
argument. sample_conv_jwt_member_query() does fill the sample in that
case, but it jumps to its exit label while <retval> still holds the
zero it was initialized with, so the converter reports a failure and
the expression yields nothing at all.
Only the form taking a JSON path worked, which is the one the reg-test
used, hence the bug going unnoticed. The test now reads both parts of
the token without any argument as well.
The issue arrived in 2.5 with the original commit 0a72f5ee7c ("MINOR:
jwt: jwt_header_query and jwt_payload_query converters") so this fix
can be backported to all stable branches.
BUG/MEDIUM: tcpcheck: do not run a regex on an unallocated buffer
A "tcp-check expect rstring" rule can be evaluated while the input
buffer of the check is not allocated, in which case b_head() returns
NULL, which is not compatible with regex_exec2() that wants a writable
buffer to write a trailing zero, and dies.
By luck, the surrounding cases work by chance because my_memmem()
returns NULL on NULL, and the binary regex works on a trash buffer
(hence always allocated). When using PCRE/PCRE2 it doesn't crash
either because both return an error for a NULL subject, so only a
pure build with libc regex fails.
It's not easy to reproduce, it requires to run many (~2*nproc) parallel
instances of reg-tests/checks/tcp-check_min-recv.vtc several times
(~200) to see it, but it does happend.
The rule now reports no match when no buffer is allocated, which is
what the string and binary cases already do in the same situation
BUG/MINOR: http-fetch: do not count Set-Cookie attributes as cookies
A Set-Cookie header contains a single cookie, and other elements on
the same line are not cookies but attributes (which can easily be
mistaken for them by a parser). That's what happens for res.cook_cnt()
which uses the same parser as req.cook_cnt() and counts apparent cookie
values without figuring it's parsing attributes. Let's just do like for
res.cookie_names() and check for a request or response before looping,
so that we can now count a single cookie per set-cookie response.
BUG/MINOR: payload: always report a boolean from req.ssl_ec_ext
req.ssl_ec_ext is documented to return "a boolean identifying if client
sent the Supported Elliptic Curves Extension", but like a few other such
sample fetch functions, it fails to return false and incorrectly returns
a fetch failure instead. It must properly return false when the hello
parses correctly but doesn't have the EC extension instead of making the
sample fetch fail as if the hello was unparsable.
The bug was in the initial version in 1.6-dev3 with commit 5fc7d7e8ce
("MINOR: Add sample fetch to detect Supported Elliptic Curves Extension"),
so this fix can be backported to all stable branches.
BUG/MINOR: stick-table: do not count the lookup itself in table_trackers()
In 1.8 when adding multi-threading support with commit 819fc6f563
("MEDIUM: threads/stick-tables: handle multithreads on stick tables"),
the entry's ref_cnt was incremented by one on all trackers and accesses,
and smp_fetch_sc_trackers() learned to return one less than the visible
value so as not to count itself in it.
One of them was missed: table_trackers() returns the number of trackers
without subtracting one so it's always one above the real value. Let's
adjust it like other places do.
BUG/MINOR: http-ana: return a 502 when checkcache blocks a response
The documentation of "option checkcache" states that a response which
does not respect its requirements "will be blocked just as if it was
from an http-response deny rule, with an HTTP 502 bad gateway".
The code jumps to the "deny" label without touching txn->status, which
at that point still holds the status the server had returned. The
response delivered to the client is thus a 200 (or anything else that
the server returned), and the error files are not even used in this
case. Let's just properly set the status to 502 just like the other
rules such as "http-response deny" do. It now does what the doc says.
This has been there forever so it can be backported to all stable
branches.
BUG/MINOR: sample: make quic_enabled() always succeed
smp_fetch_quic_enabled() ends with "return smp->data.u.sint" instead of
"return 1", so it returns its boolean result as the success status of
the fetch as well. On a build without QUIC, or when the QUIC listeners
are disabled, the fetch therefore fails instead of reporting a false
boolean, and any expression using it fails to evaluate.
The issue was introduced in 2.8-dev2 with commit 33d11c464f ("MINOR:
sample: Add "quic_enabled" sample fetch") so this fix should be
backported to 2.8.
BUG/MINOR: sample: zero-pad the fractional part emitted by http_date()
When http_date() is given the "ms" or "us" unit, it appends the
sub-second part of the date with a plain "%d", so it is printed without
any padding: 123 ms gives ".123" but 23 ms gives ".23", which stands
for a completely different instant, and 23 us gives ".23" as well
although the two units must not produce the same string.
The fraction is now printed over the number of digits its unit
requires, three for milliseconds and six for microseconds. The test
which selects the format is also made to depend on that width rather
than on the unit argument, which is equivalent but clearer.
The bug was introduced in 2.1-dev4 with commit ae6f125c7b ("MINOR:
sample: add us/ms support to date/http_date"), so this fix should
be backported to all stable branches.
BUG/MINOR: sample: apply the ms_/us_ time offsets in the input unit
The documentation of ms_ltime(), ms_utime(), us_ltime() and us_utime()
states that their optional <offset> is expressed in milliseconds or in
microseconds, that is in the same unit as their input. The four of them
however split the input into a number of seconds and a sub-second part
first, and then add the offset to the number of seconds, so the offset
was really applied in seconds and could never shift the sub-second part.
With int(1234567890123),ms_utime("%H:%M:%S.%3N",877) the result was
23:46:07.123, that is 877 seconds later, instead of the 23:31:31.000
the documentation describes.
The offset is now added to the input before it is split, which also
lets it carry the sub-second part as intended. The plain ltime() and
utime() take their offset in seconds and are unaffected.
The bug was introduced with the converters in 2.9-dev3 with commit 5e63e1636a ("MEDIUM: sample: implement us and ms variant of utime and
ltime"), so the fix must be backported as far as 3.0.
BUG/MINOR: http: do not consume the delimiter of a truncated q-factor
http_parse_qvalue() increments its cursor before checking that the
character it just read is the '.' which must follow the integer part:
if (*qvalue++ != '.')
goto out;
so when the q-factor has no decimal part, as in "q=0", the cursor
reported through <end> points one byte past the character which
terminated it, that is past the ',' separating it from the next value.
sample_conv_q_preferred(), which implements the "language" converter,
resumes its scan at that position and then looks for the next ',' from
there, so it silently skips everything up to the following separator.
With "accept-language: fr;q=0, en;q=0.1" the "en" value is never
examined and the converter reports no match at all, while "q=0.0"
works as expected.
The other caller, the accept-encoding parsing of the compression
filter, passes NULL as <end> and is unaffected. Note that this bug
has been there since the introduction of the function in 1.5-dev23
via commit 0e9b1b4d1f ("MEDIUM: compression: consider the "q="
attribute in Accept-Encoding").
BUG/MINOR: startup: don't chroot by default when set-dumpable is set
In 3.5-dev7, the automatic chroot by default was applied with commit 79082c7c2b ("MAJOR: startup: use chroot auto by default when available"),
but it has the undesired effect to prevent us from getting a core dump
by default, even when set-dumpable is set.
Let's make sure set-dumpable disables this mechanism and document it.
This is cleaner than entering a "chroot /" because set-dumpable is
clearly identified as a debugging option that should be left commented
out after use.
Note: maybe we'll need to add a "chroot /" to all reg-tests now, as it
is no longer possible to get a core dumped by default, or we may also
use a command-line option to enforce set-dumpable.
BUG/MINOR: jwe: Avoid buffer overflow in fake cek (RSA1.5)
In commit 1a5a333 "BUG/MEDIUM: jwe: substitute random CEK on RSA1_5
decryption failure per RFC 7516 #11.5" we fill a fake CEK with random
data to avoid timing attacks but the size of the buffer was never
checked. If we happened to have a too little bufsize (less than 64
bytes) we could have out of bounds accesses.
BUG/MINOR: jwe: Buffer overflow when filling fake cek in case of RSA1.5
In commit 1a5a333 "BUG/MEDIUM: jwe: substitute random CEK on RSA1_5
decryption failure per RFC 7516 #11.5" we fill a fake CEK with random
data to avoid timing attacks. But the loop that fills the buffer does is
8 bytes at a time but the index was not properly incremented which could
result in a buffer overflow if the "struct buffer" size was too short.
BUG/MEDIUM: pattern: don't dereference static_pattern's data when not filling it
The match functions (pat_match_str() etc) may return a pointer to the
thread-local <static_pattern> without updating it when <fill> is not
set. However pattern_exec_match() then unconditionally duplicates the
returned pattern's sample data, dereferencing static_pattern.data, which
may still hold a stale pointer left there by an earlier lookup performed
on the same thread with <fill> set but without going through
pattern_exec_match(). This is what the CLI's "get map" does in
cli_io_handler_map_lookup(): it calls the match function directly, so
static_pattern.data points to the element's real sample data. If this
element is later freed (e.g. by "del map", "clear map", or the purge of
the old generation after "commit map"), the next ACL match on this thread
succeeding via the tree (ACLs use fill=0) reads freed memory in
pattern_exec_match().
This can be reproduced with an ACL and a map, both using virtual files,
with one thread:
acl known url_param(q) -f virt@cliacl
http-request set-var(txn.map) url_param(q),map(virt@climap)
then on the CLI: "add map virt@climap one first", "get map virt@climap
one", "prepare map virt@climap", "prepare acl virt@cliacl", "add map @1
virt@climap two third", "add acl @1 virt@cliacl two", "commit map @1
virt@climap", "commit acl @1 virt@cliacl", and finally a request to
"/?q=two" makes ASAN report a heap-use-after-free in pattern_exec_match().
Without ASAN, random garbage may be read, or the process may crash.
The fix consists in not touching the returned pattern at all when <fill>
is not set, since the caller only checks it against NULL. The problem was
introduced in 1.8 with commit b5997f740 ("MAJOR: threads/map: Make
acls/maps thread safe") so this must be backported to all stable versions.
BUG/MEDIUM: stick-table: restrict key sizes to what fits in a buffer
Dmitri Kaslov reported an interesting bug in the stick-table key exchange
over peers: the few extra bytes of overhead and data storage are not
accounted when building the key to be transmitted, while a message must
fit into a single buffer. This means in practise that a stick-table key
as large as a buffer can theoretically crash the sender. But it's not
that trivial since such a key has to be made (which wasn't the case in
1.4 when this was designed, rules and converters to do this weren't that
rich). For strings it doesn't happen by accident though, but nowadays
it's possible to do it by tracking too large data or even applying an
expansion converter (b64dec, hex etc). For binary it's easier because
the whole block is always padded with zeros and the whole size is sent.
It has become easier to trigger with large buffers because it's now
possible to wait for a large body and track it as a large key. During
local tests, mostly applet issues were noticed, with the peers applet
not being able to serialize its message and looping back there trying
again. But with ASAN it's visible that a few bytes are overwritten at
the end of the buffer.
In order to address this, here we apply a limit to what will be
serialized. For binary keys, that cannot be trimmed, we simply refuse
to start with too large a key + data storage for the buffer size,
explaining what to do to fix it (hint: nobody sane uses 16kB keys).
For strings, they're simply trimmed to the room left once data are
subtracted from the buffer size.
This is sufficient to stop this theoretical problem without affecting
peers transfers during an upgrade (no protocol change), nor affecting
anyone with a realistic configuration.
Thanks to Dmitri Kaslov for this report with a reproducer.
This should be backported to all stable releases, along with these two
previous commits:
MINOR: stick-table: provide a function to estimate on-wire data size
MINOR: stick-table: calculate the on-wire size of each key
MINOR: stick-table: calculate the on-wire size of each key
In stick_table->proto_size, we now store the estimated worse case for
the size of a key sent over the wire with all of its stored data. This
includes the key itself when it's fixed, the upper bound for stored
data, and some overhead (11 bytes measured). It will be used by next
commit.
MINOR: stick-table: provide a function to estimate on-wire data size
We'll need to estimate the on-wire size of the stored data as a majorant
to adjust key sizes, so let's provide a new function for this. It
considers all varint at their maximum size.
BUG/MINOR: mux-h1: Discard pending raw data after a tunnel rejection
When a backend rejects a CONNECT request, some early tunnel data may
still be pending in the request buffer. Once the response is complete,
output processing is unblocked with both messages in DONE state. Trying
to process the pending data then reports a processing error, although
these data may legitimately have been sent before the client learned
about the rejection.
Discard RAW_DATA blocks in this situation instead of reporting an error.
Only the backend path with both messages in DONE state does this, and
other block types are still rejected. No discarded bytes are written to
the server connection, so they cannot contaminate a subsequent request.
The discard operation respects the requested byte count. Also exempt
pending raw request data from the final DONE-state error check: a partial
discard, output congestion or allocation failure may leave such data to
be processed on a later call. Without this exception, consuming only part
of a raw block would still report the same processing error.
Successful tunnels and the response path keep their existing behavior.
This implementation relies on RAW_DATA blocks and is only intended for
3.5; it must not be applied as-is to older branches.
BUG/MEDIUM: mux-h1: Don't subscribe for sends while output is blocked
A wakup loop could be experienced when tunnel data remained blocked on
server side while waiting for the server response.
Indeed, after sending a CONNECT request, the backend H1 mux blocks output
processing until the server response decides whether a tunnel is
established. Early tunnel data must remain pending during this time.
When the stream connector subscribes for sends, h1_subscribe() normally
subscribes to the transport too if the mux has no pending send
subscription. However, when output processing is blocked, the socket may
already be writable. The transport then wakes the mux, which wakes the
stream, which cannot send and subscribes again. This causes a wakeup loop
while waiting for the response.
Do not add a transport send subscription while H1S_F_TX_BLK is set. Keep the
stream's subscription so that the receive path can wake it when the tunnel
is established, the rejection response completes, or EOS is reported. This
does not prevent flushing the mux's output buffer: h1_send() maintains its
own transport subscription for pending output.
This should be backported to stable versions after a period of observation.
BUG/MAJOR: mux-h2: Preserve raw data on aborted frontend tunnels
Since the commit 523112b72 ("MEDIUM: mux-h2: Use RAW_DATA blocks to store
tunneled data"), raw data received after a tunnel abort on client side were
not properly handled. These data were added as plain HTTP data, triggering a
BUG_ON() because it is invalid by definition. Instead they must be handled
as true raw data from the HTX point of view.
So now, we properly use RAW_DATA for aborted tunnels on the frontend. The
backend must continue to use regular DATA blocks for the rejection response,
whose payload and trailers remain valid. This matches the existing handling
of trailers and END_STREAM on aborted tunnels.
A regression test was added with early tunnel data left pending, then more
data received after the rejection headers while the response is still
pending. It reproduces the HTX assertion without this fix.
It is a 3.5-specific issue. No backport is needed.
This function is used and ambiguous. Indeed, it is not clear how to used
it. The caller is responsible to know what he does. But at the end, it is a
not so easy. Especially about the way the EOM flag is handled.
So to avoid bugs, it is safer to remove the function.
BUG/MINOR: chunk: Allow large chunks uses from large const buffers
When a chunk was retrieved via get_best_trash_chunk() or
alloc_best_trash_chunk(), it was not possible to get a large chunk when the
original buffer was a constant 0-sized buffer, independently on size of data
pointed by it.
With this patch, we relaxe the test. It is now possible to get a large
chunks from a 0-sized buffer if it points on large data. Indeed, in that
case, it means that data are hosted in a large buffer, so it is safe to get
a large chunk if possible.
This patch should be backported to 3.4 after a period of observation.
Revert "CLEANUP: proxy: mention that px->conn_retries isn't relevant in some cases"
This reverts commit bd48e26a747016acfddce2929443f84fdb9e6997.
Indeed, since commit b71d27e253 ("BUG/MINOR: stream: Fix custom max-retries
initialization when setting backend"), the proxy conn_retries parameter is
now taken again into account for proxies lacking the CAP_FE capability.
Let's remove comments that says otherwise.
It declares a ring whose server targets a UNIX socket bound by a
log-forward section, itself relaying to a vtest syslog receiver. It
fails to parse on an affected, unpatched build ("log server address
family not supported for log backend server").
The test pins "chroot /" in its global section: when haproxy runs as
root without an explicit "chroot" directive, its automatic "chroot
auto" feature (641fe4f11) isolates the process into an empty directory
right after startup. That breaks the ring's own UNIX-socket "server"
connect, a runtime, repeatedly-retried client connection, unlike the
log-forward listener's "bind unix@..." which is already open before
the chroot happens. Pinning "chroot /" makes the test behave the same
whether run as root or not.
This must be backported wherever the fix above is backported.
Yeonggi Kim [Tue, 22 Sep 2026 12:10:12 +0000 (21:10 +0900)]
BUG/MEDIUM: server: skip log backend addr checks for internal proxies
23e5f18b8 ("MEDIUM: sink: change the sink mode type to PR_MODE_SYSLOG")
tagged a sink's forward_px (the internal proxy backing a "ring" section)
with PR_MODE_SYSLOG "for compat checks" purposes, stating no behavior
change was expected. This unintentionally made ring's internal "server"
line subject to _srv_check_proxy_mode()'s log-backend compatibility
checks, which were only meant for real "mode log" backends.
Internal proxies (PR_CAP_INT) now fall through to the regular "else"
branch, i.e. the generic STREAM-transport check instead of the
family/DGRAM checks meant for log backends. Two independent
consequences follow from this, both regressions introduced by 23e5f18b8:
1. A "ring" section whose server targets a UNIX socket (a common
pattern to feed a local "log-forward" section) is rejected at parse
time with "log server address family not supported for log backend
server", even though this worked fine before 23e5f18b8.
2. Less visibly, a ring server given a DGRAM address (e.g.
"server s1 udp@127.0.0.1:1514") was, until now, let through the
log-backend family check (DGRAM is not rejected there) and reaches
connect_server() at runtime, which promptly segfaults in
conn_select_mux_be() -> proto_is_quic() on a NULL proto, since a
ring's forward_px only ever expects a STREAM transport. This patch
also closes that crash, by routing rings back through the regular
STREAM-only check that non-log backends already get.
40eb1dd13 ("BUG/MEDIUM: sink: fix unexpected double postinit of sink
backend") already gives the sink's forward_px the PR_CAP_INT capability
for a related reason (skip the automatic log-backend postinit, since
sink manages it manually). This reuses that same flag to also skip the
log-backend-specific address checks for the same class of proxies,
since they are never real user-configured log backends.
This should be backported up to 3.3, but only where 23e5f18b8 and 40eb1dd13 are backported too - without the latter (which sets
PR_CAP_INT on forward_px), this patch is a silent no-op.
BUG/MEDIUM: sink: reconnect attempt not working after session lasted more than ~25 days
As described in GH #3508, if a sink session lasted for more that 24.85
days and suddenly stops, the automatic sink session restart mechanism
won't try to reconnect to the server as it would normally. This regression
was introduced by commit 9561b9f ("BUG/MINOR: sink: add tempo between 2
connection attempts for sft servers"). The underlying cause is that we
rely on a 32 timer which is updated at the time of the last connection
attempt, which will eventually wrap when no connection attempt is
performed for more than 24.85 days (this is the case when the session
uptime reaches that time). In that case the timer ends up being in the
future and will not be considered as expired anymore when compared with
now_ms.
To fix the issue, we now look at the applet release/closing date instead
of its creation, this way we ensure that sft->last_close should not wrap
because the time between the applet release and a new attempt is relatively
short, since any new message queued in the forwarding queue will cause the
applet to perform a new attempt. Thus it is not expected that the timer
holds value greater than a few seconds, minutes or worst case hours.
However we need to mention a special case: if the syslog server becomes
unavailable for more that ~25 days, in that case stf->last_close could
wrap, causing the applet to stop trying to connect to the server, but
this is acceptable because a server that is not available for that long
can be considered as dead.
Thanks to GH user @ygkat for reporting the issue and providing very precise
analysis of the bug with a patch proposal, although the final patch followed
Willy's suggestion.
It should be backported to all stable versions (anywhere 9561b9f was
backported).
BUG/MAJOR: mux_quic: fix potential crash on RESET_STREAM receive
When a RESET_STREAM frame is received and upper layer is not allocated,
QCS instance is immediately locally closed and inserted into QCC purge
list. This operation has been performed since the following patch.
Before, the QCS would remain allocated and could possibly prevent the
connection closure if accounted as a request stream.
Sadly, this causes a crash if QCS is already inserted in QCC send list.
This is because <el_send> element is used both for send and purge list,
as a QCS cannot be present in both at the same time.
Fix this skipping insertion in purge list if QCS is already present in
the send list. QCS is not closed locally either in this case. This
allows the QCS to finalize its emission before being closed and purged.
This latter case requires to extend check in qcm_ctrl_send() for QCS
purging when everything is emitted. This is now also performed if a
RESET_STREAM has been received and upper stream layer is not present.
These modifications reveal that it is probably not valid to silently
close a QCS without notifying the peer. Reaction to a RESET_STREAM is in
fact dependent on the application layer protocol and the QUIC stream
type. For now, the current code base should ensure that a bidi HTTP/3
stream is not kept forever after peer reset. A more thoughtful approach
may be implemented later in a future patch.
MINOR: log: add the +utf8 encoding option to let valid UTF-8 pass
Since commit db9492bcf ("BUG/MEDIUM: log: encode the whole non-printable
range in the +json output"), the +json encoder escapes every byte outside
the 32..126 range. This guarantees a pure ASCII output that cannot corrupt
a terminal, and it also purposely affects valid UTF-8 text: U+00E9,
encoded as 0xC3 0xA9, is logged as "\u00C3\u00A9", which a JSON decoder
turns into U+00C3 U+00A9. Some users relying on +json to log non-ASCII
text now get unreadable records.
User @Wahnes reported in GH issue #3512 that it breaks their use case
where UTF-8 was expected to land as-is in the output, as permitted by
RFC 8259. However, the patch above purposely avoided this because a
number of valid UTF-8 sequences can have effects on the terminal, such
as direction overrides (RTL/LTR), zero-width characters and a few others
that could affect how the text is rendered, and be used to hide actvities
if retrieved verbatim from header field values for example.
This patch adds a new "+utf8" log option that explicitly lets valid
UTF-8 pass unencoded in JSON, and only that, so that non-utf-8 bytes
above 126 remain encoded. The C1 class of control chars (0x80..0x9f)
which may appear encoded in UTF-8 (e.g. 0xc2 0x9b for 0x9b or \u009b)
remain encoded to protect terminals. This flag is stored in a new flag
LOG_OPT_UTF8 that is only used by LOG_OPT_ENCODE_JSON for now.
The doc and the regtest were updated accordingly.
This may be backported wherever db9492bcf is backported, along with the
previous commit, so that users of versions where it gets backported can
restore the ability to log UTF-8 text.
MINOR: log: pass the input end to the _lf_encode_bytes() byte encoders
The byte encoding callbacks used by _lf_encode_bytes() only receive the
current byte, so they cannot look at the following ones. This will be
needed by the json encoder to detect multi-byte UTF-8 sequences. Let's
pass them <bytes_stop> as well as an end pointer, which is NULL for
NUL-terminated inputs.
BUG/MINOR: ssl: free curves, sigalgs and client_sigalgs in srv_parse_*()
When curves, sigalgs, or client-sigalgs are parsed on a server line,
the string allocated via strdup() was assigned directly to
newsrv->ssl_ctx without releasing any prior value.
If the server inherited these SSL settings from default-server via
srv_ssl_settings_cpy(), or if the keyword was specified multiple
times on the same server line, the previous string was overwritten
and leaked.
Free the previous pointers using ha_free() before assigning the new
strdup() value in srv_parse_curves(), srv_parse_sigalgs(), and
srv_parse_client_sigalgs().
This change should be backported to all supported branches.
BUG/MINOR: ssl: free curves, sigalgs and client_sigalgs in ssl_sock_free_srv_ctx()
The ssl-default-server-* keywords (curves, sigalgs, client_sigalgs)
allocate memory via strdup() that was never freed when servers are
destroyed. This causes a memory leak per server instance.
Add ha_free() calls guarded by the same OpenSSL API version checks
matching the allocation guards in cfgparse-ssl.c.
This change should be backported to all supported branches.
BUG/MINOR: ssl: copy curves, sigalgs and client_sigalgs in srv_ssl_settings_cpy()
When a server inherits SSL settings from default-server, the curves,
sigalgs and client_sigalgs fields were not copied despite being
allocated by ssl-default-server-* keywords. This causes use-after-free
when the original ssl_ctx is destroyed while another reference still
holds the pointer.
Add strdup() calls guarded by the same OpenSSL API version checks
used elsewhere in the copy function.
Fixes issue #3506.
This change is should be backported to all supported branches.
BUG/MINOR: acme: exact domain match in acme_challenge_ready()
acme_challenge_ready() compares the domain passed on the CLI (or via the
Lua ACME.challenge_ready() function) with strncmp() limited to the
length of the authorization's domain. This is a prefix match: with a
certificate covering "example.com" and "example.com.au", signaling
"example.com.au" also marks the "example.com" challenge as ready.
Use isteq() so that only the exact domain matches.
This should be backported to 3.3. The code was moved into
acme_challenge_ready() in 3.5, so the fix has to be applied to
cli_acme_chall_ready_parse() in older versions.
MINOR: init: also set the worker-id in file-less mode
The worker-id generation at boot, as done in commit 3639aac31f ("MINOR:
config: add a "worker-id" global keyword to identify the worker"), would
escape the file-less mode that is used with haterm and haload. There's
nothing dramatic in this, but it may end up being surprising with certain
corner-case configs that would want to exploit it (logging, header
contents etc), so better set it now for consistency between any worker
mode.
BUG/MINOR: h2: reject :protocol pseudo-header in H2 responses
RFC 7540#8.1.2.1 mandates that pseudo-headers defined for requests must not
appear in responses. When converting an HTTP/2 response to HTX, h2.c already
rejects a response carrying :authority, :method, :path or :scheme, but the
mask used for that check omits H2_PHDR_FND_PROT. As a result a backend HTTP/2
response that contains the request-only ":protocol" pseudo-header (defined by
RFC 8441 for Extended CONNECT) is accepted and forwarded to the client
instead of being rejected.
Add H2_PHDR_FND_PROT to the response-side rejection mask so that ":protocol"
is handled like the other request pseudo-headers and such a response yields a
502 instead of being forwarded.
There is no known security impact: the offending pseudo-header does not
appear in the emitted HTTP/1 response (which is byte-identical to that of
a valid one); the only effect is that a malformed response is tolerated
instead of being rejected.
This should be backported as far back as 2.4, where the ":protocol"
pseudo-header (H2_PHDR_FND_PROT) was introduced; the maintainer may narrow the
range to the currently maintained branches.
Prior to this patch, QUIC MUX timeout was only queued after receiving
the first bytes of data. Thus the timeout would be inactive after
initialization if nothing is received, with a risk of connection leak.
To properly address this, QUIC MUX task timeout is now queued
immediately after initialization has been performed.
The risk of connection leak caused by QUIC MUX is now greatly reduced.
For example, the previous issue on RESET_STREAM reception handling
cannot cause anymore such leak with the current code.
BUG/MAJOR: mux_quic: fix leak on RESET_STREAM reception
On QUIC MUX, <nb_hreq> counter serves to account for the currently
active requests in progress. While this number is non null, the
connection is considered active and cannot be released. The counter is
decremented only when the QCS instance is finally freed. This guarantees
that no truncated request could happen.
An issue happened on RESET_STREAM frame reception as the first frame for
a new QCS instance. In this case, <nb_hreq> is incremented. No progress
will occur as the upper stream layer is not instantiated, and QCS
instance will remains half-closed, which prevents its liberation.
The first issue is that it causes a leak of QCS during the connection
lifetime. Worst, this may prevent the whole connection closure even if
idle timeout as fired : this is because qcc_is_dead() checks <nb_hreq>
first before regarding transport layer status. Note that this only
happens if QUIC MUX timeout is not yet activated, which is the case if
qcc_io_cb() has not yet been executed.
This patch fixes this situation by improving RESET_STREAM receive
handler. Now, an extra check is performed to detect if the frame has
been received before the upper stream layer init. If true, QCS instance
is closed locally and immediately scheduled for purge, which ensures
that <nb_hreq> counter will be decremented as expected.
BUG/MEDIUM: h3: Set FIN when last DATA block is sent via zero-copy
Since commit ab7bb500a ("MEDIUM: h3: Use flags of HTX blocks to detect end
of message during sending"), the end of the message is detected via the
HTX_BLK_FL_EOM flag of consumed HTX blocks. But the zero-copy path in
h3_resp_data_send() jumps to the end without testing this flag. So when the
last DATA block of a message is transferred via zero-copy, "fin" is never
set and no FIN is emitted on the QUIC stream, leaving the peer waiting
indefinitely for the end of the stream.
Fix this by capturing the block's EOM flag before the buffer area swap and
setting "fin" accordingly.
REG-TESTS: http-messaging: Send one request per h2 client in h2_trailers.vtc
The script fails randomly depending on the load on the machine. C3 and C4 H2
clients are sending sending several invalid requests. However, instead of
received the expected RST_STREAM for some requests, a WINDOW_UPDATE is
received, making the script failed.
So, to avoid this issue, the clients are splitted to send only one h2
request per connection. It seems better this way.
BUG/MINOR: http-ana: restore the transaction status after early hints
While a 103 response is being built by consecutive early-hint rules, the
transaction status is set to 103 so that the next rules append their headers
to the same response. Since commit 4c3d3d2a6 ("BUG/MINOR: http-act: Properly
generate 103 responses when several rules are used"), once the response was
finished, or on error, the status was reset to 0.
But the initial status of a transaction is -1, and some code relies on it to
know whether a status was set. This is the case of the deny handling in
http_req_get_intercept_rule(), which only uses the default 403 status if the
status is still -1. So a Lua action returning act.DENY after an early-hint
rule makes haproxy answered with a 500 instead of a 403, and logged a null
status.
Let's restore -1 instead in http_early_hint_end() and on the error paths of
the early-hint action and of the cache early hints. A Lua reg-test is added.
This must be backported to all supported versions. The cache part only
exists in 3.5.
CLEANUP: mux-h2: replace a non-UTF-8 character in a comment
A comment in h2c_dec_hdrs() contains a section sign encoded in ISO-8859-1,
which is the only non-UTF-8 byte in the sources. Some tools consider the
whole file as binary because of it, e.g. "ugrep -I" silently skips it.
Let's spell it "section" instead.
BUG/MINOR: tcpcheck: refresh the start-line after updating the authority
When an HTTP health-check request was built with an absolute URI, each
"host" header set by "http-check send" updated the authority of the URI with
http_update_authority(). This rewrote the start-line, which could move it
elsewhere in the HTX message. So, in tcpcheck_eval_send(), the start-line
pointer <sl> must be renewed in that case.
Let's retrieve the start-line again after each authority update. A reg-test
was added.
This must be backported to all supported versions.
BUG/MINOR: mux-h1: only mark C-L and T-E as sent once the header is emitted
In h1_make_headers(), the H1S_F_HAVE_CLEN and H1S_F_HAVE_CHNK flags, and for
the content-length the message lengths, were set before the header was
formatted into the output buffer. If h1_format_htx_hdr() failed because the
buffer was full, the function stopped and left the header block in the HTX
message so that it was processed again on the next call, which was the
expected way to resume.
But then the flag was already set and the header was skipped as a
duplicate. h1_make_eoh() did not add it back either, since its check relied
on the same flags. The message would then be sent with a body but without
the header announcing its framing.
This requires the output buffer to be full in the middle of the headers. It
is not possible by construction for now. But this relies on the state of the
output buffer when a message starts.
Let's only set these flags and lengths once the header was emitted.
BUG/MINOR: http-fetch: fix the space check of http_auth_bearer(<hdr>)
When http_auth_bearer() was called with a custom header name, a test was
performed to know if the value contained a space after the scheme, by
comparing the length of the scheme with the length of the header value.
But istsplit() had already advanced <ctx.value> past the scheme and the
space at this point, so the comparison was made with the length of the token
instead:
- a token of exactly 6 characters, the length of "Bearer", was reported as
a mismatch, e.g. "Bearer abcdef";
- a lone "Bearer" without any space was accepted and returned an empty
token.
Let's compare with the length of the value before it is split. The reg-test
is extended with these two cases.
This should be backported to all supported versions.
BUG/MINOR: http-fetch: http_auth_bearer() must not match a missing header
When http_auth_bearer() was called with a custom header name and this header
was not found, the fetch still succeeded and returned an empty string. So a
condition like "{ http_auth_bearer(x-tok) -m found }" was true for a request
without any such header.
Commit 909578520 ("BUG/MINOR: http-fetch: Fix http_auth_bearer() when custom
header is used") intended to report a mismatch in this case, as its message
says, but only did it for a header with a wrong type.
Let's return 0 when the header is not found. A reg-test is added for the
fetch with and without a custom header.
projects / haproxy.git / log