The app-layer firewall policy config resolves per-state keys
against the current state name table only, so keys renamed or
removed in this release are silently ignored: the state falls back
to the implicit default policy (drop in firewall mode) with
nothing logged. A suricata upgrade without updating
firewall.policies.app.tls would then drop traffic with no
diagnostic.
Walk the policy config at load time and compare every per-state
key against the state names of the protocol section it sits in,
mirroring the enumeration the policy loader consults (the plain
state tables and the sub-state sections). An unknown key logs a
warning with the config path. A key matching a state name renamed
in this release (a deliberately name-based, per-protocol hint
list) escalates to an error with the new name suggested, and the
load aborts under init-failure-fatal.
Ticket: #9047.
The server states were completion milestones: server_hello landed
when the server hello record was fully parsed and server_cert_done
when the certificate message was done, so the states named moments
at which the phase data was complete instead of the phase being
processed. The flight-completion state (server_hello_done) named
a moment at which no data is published and no decision can act.
The states now denote the record phase being processed:
server_hello is entered when the server hello message begins
(its header is read once, the message may span several
records) and server_cert when the certificate message begins,
so the state names the message being parsed and a message that
fails to decode leaves the track in the state that names it.
The record that completes a hello hands the track over to the
certificate phase before a single certificate byte arrives. The
enter/leave is the shared handshake phase mechanism, so the client
track follows: client_hello is entered at the start of the client
hello as well, and the completing client hello is decided at
client_cert.
The flight-done state is removed and renumbers the tail
(server_data, server_finished), leaving both tracks with the
same five-phase shape and an absorbing finished state.
Hello and certificate decode failures suppress the phase advance
of their own message only: the decoders keep their pre-existing
return contract (the message bytes are consumed and the error
events are raised as before) and set a per-connp suppression flag
that is reset at the start of the next handshake message and
checked at the phase advance. A decode failure is not an app-layer
parse error: the flow is neither dropped nor blinded, and the next
message that decodes cleanly still advances.
An undecodable certificate in a chain is skipped, not fatal: one
decoder event is raised for the bad certificate and the remaining
certificates are still decoded - the rest of the chain is kept and
the certificate data is filled from the first fully decodable
certificate. The certificate phase is suppressed for such a
message (the phase data is incomplete). The incomplete-data
condition is sticky per connp: it is set while the certificate
message is being decoded, cleared by a fully decoded certificate
message only, and consulted by the handshake-done heuristic that
would otherwise establish the data phase from the certificate
data - a later handshake message does not complete the data, so
the tracks do not cross the cert phase on partial data.
The certificate field extraction is commit-on-success: the fields
are extracted into stashes and published to the connp only when
the whole certificate yields every field, so a certificate that
fails a late extraction leaves no partial data behind and the next
certificate can fill the fields. An extraction failure raises its decoder event once per message: the message no longer aborts at the first failing certificate, and the event is not repeated for the later failing entries.
The tls.cert_chain_len match is gated on the chain being final (the
per-connp flag set by a fully decoded certificate message), so it
does not match while the track sits in server_cert with a chain that
is still arriving.
TOCLIENT keyword registrations and the tc log threshold follow the
renamed constants (values 1/2 unchanged; the threshold is
triggered by the same writes as before, so the log fires on the
same packets). The ja3s lua registration moves to server_data: lua keeps a numeric
hook, so it must follow the renamed state the old hook value pointed
at (server hello done was 3, server data is 3). The ja3s content is
final once the ServerHello parses, which is why the C keywords
(ja3s.string, ja3s.hash) stay registered at server_hello. Two string-based
generic buffer lookups and the user guide follow the renamed
states.
Ticket: #9047.
The client states were completion milestones: client_hello_done
landed only when the hello record was fully parsed, so during the
in-hello window (possibly multiple packets) the flow sat in
client_in_progress and no firewall state named the phase being
processed.
The states now denote the record phase being processed:
client_hello is entered when the hello message is fully parsed
(it may span several records) and client_cert when the
certificate message is fully parsed, so the phase data (SNI,
certificates) is available when its state is entered; a message
that fails to parse does not advance the track. The milestone
writes are removed and the name table, TOSERVER keyword
registrations (values unchanged) and the firewall user guide
follow the new names.
Ticket: #9047.
In TLS 1.2 and 1.3 the Finished message precedes the application
data on both directions. The DATA milestone write is keyed on the
parsed certificate subjects and runs on every record, so the first
record after the handshake - for a session that ends with a fatal
alert, the alert itself - moved the tracks backwards (client 4->3,
server 5->4). The completion states were therefore not absorbing:
the finished hook windows were transient in decryption mode, and a
keyword registered at progress >= 3 would have its eof revoked and
be suppressed by the stored CANT_MATCH flag.
The setters now only move a state forward, so post-terminal writes
are no-ops and the finished states absorb.
Ticket: #9047.
Ticket: 6028
memmove_s is a C11 safer version adding a fourth argument which
is the total size of the destination buffer, so as to check there
is no risk of overflow
block_scrutinee now warns on a block-wrapped if let scrutinee, since
the block changes when temporaries drop. The file config reads in the
nfs, smb and http2 gettxfiles handlers were wrapped in a pointless
single-expression block; remove it.
PostRunDeinit stops the flow manager, then runs the final flow sweeps. TCP
flows that a foreign thread times out during those sweeps are parked on the
flow hash row's evicted list for the manager to pick up, but the manager is
gone.
FlowRemoveHash only walked the row's head list, so those flows were later
recycled without FlowFinish, skipping the final reassembly and pseudo
packet pass. Any app layer tx that only completes at flow end is then
lost: typically a response arriving after the peer's FIN and never ACKed,
as that data only becomes usable at eof in non-inline mode.
The result is an intermittent count mismatch in the suricata-verify
suite, always exactly one short, e.g. "expected 28 matches; got 27" for
http-sticky-server and "expected 7 matches; got 6" for
smb2-ntlmssp-negotiateflags. The flow event is still logged, with reason
"shutdown", so the flow is not lost, only its tail transactions. The
lost records can be spotted in a reference eve.json by "pkt_src":
"stream (flow timeout)" and no pcap_cnt.
Stranding a flow needs a hash row collision plus a foreign thread sweep
inside the [last manager sweep, manager stopped] window, and the row
mapping is seeded with a per process random value (flow.c hash_rand).
That makes it a per run lottery at the default 65536 rows, and
deterministic with a small flow.hash-size such as 12 or 20. Unit tests
cannot cover it: FlowBelongsToUs returns true in unittests runmode, so
the evicted list is never populated there.
Also sweep the evicted list, with the same decision point as the head
walk: FlowNeedsReassembly sets ffr_ts/ffr_tc, which CheckWorkQueue
requires before running FlowFinish. CheckWorkQueue applies its own
bypass and drop guards, so a bypassed flow may now take a worker round
trip and have its flow event logged there rather than by the recycler.
The evicted list has never been swept on this path.
Fixes: 244dd11c34 ("flow/manager: fix flows not evicted & freed in time")
Ticket: #9146.
A response line carrying a code but no message, such as "221 \r\n" sent by
some servers in reply to QUIT, was logged with no completion_code, and with
the code itself in the reply field:
{"command": "QUIT", "reply": ["221"], "reply_received": "yes"}
parse_response_line split caused problems on no response message and prevented good logging of completion code
Ticket: #8966
Pygments 2.21 lexes "x < 0 or x > y" in Lua as a Lua 5.4 variable
attribute (e.g. "<const>"), producing an error token on the "0". Sphinx
reports this as a highlighting failure warning, and as the PDF build
treats warnings as errors, "make dist" fails while building the PDF.
Reorder the comparisons so the Lua example is lexed correctly. The logic
of the example is unchanged.
Example of warning by new clang:
runmodes.c:537:12: warning: variable 'file_logger_count' set but not used [-Wunused-but-set-global]
537 | static int file_logger_count = 0;
The internal type set for datasets of type IP is IPv6, however, the
expectation is that it'll support IPv6, standard IPv4 as well as IPv4 in
IPv6 notation. A regression happened during the conversion of datasets
parsing to rust wherein standard IPv4 was rejected with IP type. Fix
that by handling IPv4 standard notation in IPv6 handler.
Fixes: bc79300dfa ("datasets: move initial file reading to rust")
Ticket: 9006
Ticket: 9096
In case of a 100-continue response, we used to
- parse the response line, set progress to headers
- parse headers, set progress back to line
This caused FPs as the engine expects the progress to just increase
so that it skips signatures that did not match yet, and whose
progress is less than the tx progress.
Solution is to keep progress at line when we see a 100 response,
until we get to the body, or after a next response line which
is not 100.
The functions in util-ioctl.c use an AF_INET datagram socket to
perform interface ioctl operations. Most of them returned silently
when socket creation failed, while GetIfaceRSSQueuesNum() logged only
a warning.
Report socket creation failures as errors in all of these functions.
Such failures can indicate a system-level problem, such as file
descriptor exhaustion, and otherwise make startup and interface
reconfiguration failures difficult to diagnose.
Signed-off-by: Petr Vaganov <petrvaganoff@gmail.com>
Previously, on request failure, the prep stage fall-backed to the
default upstream branches of SV/SU due to not finding the PR body
in the error message.
This commit changes the behavior to fail the prep stage when
the description cannot be read.
Unauthenticated calls to api.github.com are limited to 60 requests per
hour, counted against the originating IP address. A GitHub-hosted
runner's egress IP is shared with jobs from unrelated repositories, so
that allowance is non-deterministic.
By authenticating the request with github.token we increase the limit
to 1000+ requests per hour counted against this repository.
The filedata inspect engine returned NO_MATCH for every non-matching
file, also when the tx had already moved past the engine's progress.
For streaming registrations such as the http2 request body this meant a
firewall LTE rule never became definitive, so the fail-closed default
policy was never applied.
Return CANT_MATCH_FILES once the tx progress moved beyond the engine,
mirroring the existing no file case. The per file state reset keeps the
rule matchable if the tx grows another file.
Ticket: #9122.
Success was decided by matching git-clang-format's stdout against two
known strings. When it fails, e.g. on an invalid revision, it writes to
stderr only and leaves stdout empty, so the check reported a formatting
violation where there is an error.
The reformatting commands had the opposite problem. git-clang-format
exits 1 when it did reformat files, which they treated as a failure, so
branch, commit, cached and rewrite-branch all reported an error whenever
they had work to do.
Keep stderr in its own file and compare against the exit code instead.
Exit code 1 means formatting changes were found or made, 2 and up are
errors.
Ticket: #9097
When a branch has no commits of its own, e.g. when it is the baseline
branch itself, git rev-list returns nothing and first_commit is empty.
The empty value was still used to build "$first_commit^", which git
rejects as a bad revision.
git-clang-format then failed without writing anything to stdout, which
the output check reads as "formatting required". Pushing the baseline
branch therefore failed the CI formatting check, reporting a style
violation that does not exist.
Report that there is nothing to format instead.
git rev-list also returns nothing when the baseline ref does not exist
as it only reports that on stderr, so verify the baseline separately to
keep that case a loud failure rather than a silent pass.
Ticket: #9097
Victor Julien
Philippe Antoine
dependabot[bot]
Amir Boussejra
William Ling
Jason Ish
Juliana Fajardini
Shivani Bhardwaj
Petr Vaganov
Lukas Sismis