On PcapFileDispatch() failure, pv->current_file was left pointing at freed file
vars, which were then cleaned up again on thread teardown.
Generally unreachable, as no pcap-file pipeline slot fails on real input.
Ticket: #9162
Including decode.h pulls inline functions that use SCLogDebug and SCLogError
into the bindgen input, but util-debug.h hides those macros from bindgen,
breaking "make update-bindings". Only forward declarations are needed.
Action scopes for packet hook default policies were validated per hook
class, so `flow` was accepted for every packet hook. The pre_flow hook is
a special case as it is evaluated before Suricata obtains a flow record.
Firewall rules on the pre_flow hook were not validated at all, so `flow`
and `tx` scopes were accepted there as well.
This commit adds extra validation to scope parsing for the pre_flow
hook default policy and validates pre_flow firewall rules against the
same scopes.
Ticket: 8967
The packet default policy runtime only applied the action scope for
`accept`. For `drop:flow` and `reject:flow` it dropped the packet and left
the flow untouched, so the flow scope had no effect.
Ticket: 8969
Covers the killed-during-initial-pause case (mgmt thread returns so the
main loop runs once; non-mgmt returns false and skips it, THV_PAUSED
cleared on both paths) and the unpause-while-spinning path, driven from
a helper thread.
Ticket: #9168.
Every thread is created paused (THV_PAUSE) and the main thread unpauses
them all before the main loop. A thread that has not yet been scheduled
spins in TmThreadsWaitForUnpause() until it sees the unpause. Under
heavy thread oversubscription (many short offline runs in parallel) a
management thread can be descheduled across the whole run: by the time
it runs again the shutdown has set THV_KILL while it is still in the
initial pause. TmThreadsWaitForUnpause() then returned false, so the
thread skipped its main loop entirely and never did its final shutdown
work - for the stats thread that means the last stats event is never
written to eve.json. This intermittently fails suricata-verify tests
that assert on the final stats.
Mgmt threads (tv->type == TVT_MGMT) now return so the main loop still
runs once: it tests THV_KILL and exits after the pass, but the thread
gets the single pass it needs for final work (e.g. the last stats
output). THV_PAUSED is cleared on that path. RX, worker and other
non-mgmt threads keep the previous behaviour: their loops do not test
THV_KILL at the top - the RX loop exits only on the PktAcqLoop return
value and suricata_ctl_flags, and a pcap-file pass would dispatch
packets into a pipeline that is being torn down.
The log-maintenance thread breaks before its first sleep slice when it
is already being killed: on that pass no rotation or flush can happen
(their wait counts need a full second of slices), so the fixed 500 ms
sleep would be pure shutdown latency - the slice exists to react to
shutdown quickly, and a first wake into a KILL must not wait out a full
slice. A kill arriving after the first slice keeps the normal
behaviour, including the final rotation check.
Ticket: #9168.
A direction that hit stream.reassembly.depth stops collecting data, so
StreamTcpPruneSession() stops sliding its reassembly buffer and base_seq
freezes where the data stopped while the sequence numbers keep moving. The
base_seq bound on last_ack updates compares an ACK against that anchor with
SEQ_GT(), which is only meaningful within half the sequence space: 2^31 bytes
past the depth it reads a regular ACK as being behind base_seq and last_ack
stops moving, next_win with it, and every packet from then on is flagged
out-of-window or with an invalid ack. 2^32 bytes later both comparisons line up
again at once, last_ack takes one step of 4 GB and the reassembly engine
reports a gap over data the peer already acked - for TLS a decoder error, and
in IPS mode the exception policy drops the flow without RST or FIN.
Take the bound only while the direction is still reassembling, as that is when
base_seq tracks the stream; nothing is inserted once it stopped, so there is
nothing left for the bound to protect. Where base_seq is ahead of the ACK it
still refuses the update, so the check Redmine #6865 added stays in place for
streams that are still collected.
Ticket: #9141.
An empty SETTINGS frame (an ACK, or a SETTINGS without entries) opened
the settings array and closed it without items, producing a record the
eve schema rejects (settings has minItems 1). Skip the frame instead; a
tx whose only content is an empty SETTINGS frame is then not logged at
all, matching the empty headers handling.
Exposed by the flow-end flush of IDS-dropped flows: the dropped flow in
the drop-protocol-change test carries such a frame.
The '<' lower bound hook form is a firewall policy construct: the rule
covers its own hook and every prior one. For a threat detection rule the
form has no such meaning, and the flag it sets leaked into the firewall
policy walk, where the rule could influence the per-state decisions.
Reject the form at load for rules without SIG_FLAG_FIREWALL.
Ticket: #8944.
tls.version requires the connp's hello to have decoded (hello_decoded),
which only TLSDecodeHandshakeHello() set. The SSLv2 hello path sets the
client version without the flag, so an SSLv2-compatible ClientHello no
longer matched tls.version: on main it matched, so a firewall accept
rule using the keyword never accepted.
Set the flag in the SSLv2 client and server hello cases.
Ticket: #9047.
AppLayerParserGetTxEndState() replaces the remaining copies of the
transaction end state computation in the firewall tx setup, the
app-layer-state and app-layer-event keywords and the completion check.
One tolerated a missing tx data pointer, the others asserted or
dereferenced it, so they could disagree.
Move DetectTxCompleted() above its first use while there, so it no
longer needs a forward declaration, and shorten its comment.
Ticket: #8944.
Alerts and drops for firewall rules hooked at a sub-state (http2
stream/global) had no firewall.hook field: the logger resolved the state
name without the sub-state, so the lookup found nothing.
Keep the rule's sub-state on the Signature next to app_progress_hook,
set it from the hook and for the default policy signatures, resolve the
name in the sub-state table, and fall back to the plain state name when
that lookup fails.
Ticket: #8944.
An app-layer-state comparison that can never hold - mode '>' at or beyond
the completion state, or mode '<' at the starting state - can never match.
Both are fatal at rule load, so reject them at parsing.
The LTE (<hook) form at the first state (progress 0) is accepted and is
equivalent to the plain hook form: there are no prior states to auto-accept.
Derive the completion state from the direction the keyword resolved in
instead of the rule's flags, and document the new load time rejections.
Ticket: #8944.
eof is C > P, so an engine registered at a parser's completion state
never becomes complete: there is no next state to enter. A firewall
rule hooking such a state (e.g. tls:client_finished) stayed pending
forever and was optimistically accepted.
Treat progress == engine progress == tx end state as complete. This
also finalizes the buffer engines' eof on completion-state buffers, so
absent and bsize decisions now happen there, and it covers every parser
whose engine progress equals its completion state (dns, ntp, quic,
rdp-style two-state protocols), not only the firewall path that
motivated the change.
Ticket: #8944.
DetectEngineInspectGenericList returned NO_MATCH for every keyword miss,
also when the tx had moved past the phase the engine is registered at.
Unlike the buffer engines it never produced a terminal no match, so a
firewall LTE rule using such a keyword (tls.version on a hook) stayed
provisional and was accepted for the rest of the session.
Make a miss terminal at P + 1. This engine also backs the lua
on-request/on-response hooks and the tls_cert_*, dnp3/dnp3_ind and
ftpdata/ftp mode keywords, so a plain IDS rule using them keeps its
verdict once the tx has moved past the registered phase.
Export the parser's end state computation as AppLayerParserGetTxEndState()
here, so the version keyword can use it; the remaining call sites are
converted later.
Ticket: #8944.
A non-matching rule followed by an LTE rule (<hook) with an overlapping
hook can be decided by the wrong default policy. E.g.:
drop:flow tls:client_hello .. tls.sni; content:"match" \
content:"no-match";
accept:flow tls:<client_hello .. tls.sni; content:"match"
Here the `tls:client_started` state needs to be accepted by the second
rule, which acts as `accept:hook` on that state.
A failing `<` rule defers the per-state defaults of the states it
promised to cover; the last pending covering rule decides them, applying
its own hook default and sweeping forward from its own hook. There is no
backward sweep, so the states below that hook get no default of their
own.
The pending LTE coverage accounting was the problem: it counted
candidates before their header predicates were checked, compared against
a max that included the rule being evaluated, and kept rules that
resolved during the pass. Count only rules that pass the header checks,
keep per-hook coverage counts in per-thread scratch, drop a rule from
the counts once it resolves at or past its hook, and let the hook checks
consult the counts instead of the raw LTE flag.
Also fixes two latent issues in the same walk: the candidate list is now
sorted whenever the tx state store adds entries - the old condition
skipped the sort when the prefilter contributed nothing, so the
last-for-progress peek could see an unsorted list; and rule profiling
now receives `r == 1` instead of the raw return code, which used to
decrement the match counter on a no-match.
Ticket: #8944.
State table and failure/completion semantics for the
per-direction scale (banner/kex/session, done never reported),
the removed banner_wait_eol state and legacy hook names, the new
eve ssh error/state fields, and the transactions/lua/upgrade
examples - all updated to match.
Ticket: #9046.
banner_wait_eol only recorded that a banner line of 256 bytes or
more had arrived without EOL; it was not an active phase. The
direction kept reparsing the open line in place, so a rule or
firewall policy pointing at the state acted on a parked, not an
active, direction.
Move the condition to the event channel it already had (the
one-shot ssh.long_banner event) and key the continuation off an
explicit banner_published latch (set at the banner publish
sites): a direction with an open line stays in the banner state,
the line is consumed greedily until the EOL arrives, and the line
completion takes the normal banner-to-kex transition.
The state scale becomes dense: banner(0) kex(1) session(2),
completion done(3). request_banner_wait_eol /
response_banner_wait_eol no longer load: aliasing them to banner
would silently turn a long-banner rule into an any-banner rule.
Firewall rules that acted at the removed state must move to the
ssh.long_banner event or the banner state; and since the progress
gap between banner and kex is gone, the default-policy sweep no
longer drops a clean flow at the server banner.
Ticket: #9046.
A flow that hit an unrecoverable ssh parse error (invalid banner
or invalid record) was never visible in the eve ssh output: the
log condition only admitted a successful handshake (both
directions at kex or session), and the tx logger is one-shot, so
the successful-handshake log fired before a later failure could
be reported and the failure was then silent - the reason was
only visible through the app-layer events.
The log condition is now failure-only: the failure latches, so
the object is emitted exactly once - at the failure, carrying
the error field (invalid_banner / invalid_record) and the state
field (the state the failure occurred in - banner, kex, or
session if a later record fails after NewKeys) - and successful
flows are logged at the flow-end flush, where the engine logs
unconditionally. Logging on the success condition mid-flow would
consume the one-shot tx log and drop a later failure, so it is
not done. The state field is emitted only alongside the error.
The object may be error + state only, when no banner was
parsed. A unit test pins the condition for a failed, a
banner-only and a both-at-kex flow (the failure admits the log
on the failing delivery); the user guide and eve reference doc
commits that follow carry the field docs.
That flow-end flush is skipped for flows that end without a
further app-layer update, though: in IPS/firewall mode a drop
disables the app layer (StreamTcpCheckFlowDrops ->
StreamTcpDisableAppLayer), after which no app-layer update
reaches the tx loggers, and a bypassed flow gets no flow-end
packet either. A successful ssh record - e.g. one that only saw
the banners - was therefore never emitted for those flows. The
hook is the app-layer end: on the transition to app-layer EOF,
StreamTcpDisableAppLayer raises FLOW_APP_LAYER_FLUSH_PENDING,
and FlowUpdateState raises it on the transition to a bypassed
state; OutputTxLog runs the tx loggers once for the next packet,
also when that packet itself has no app-layer update and is not
marked established (the disabling packet returns from the
stream engine before that), and flow-worker clears the flag when
it frees the app layer of a bypassed flow. The flush is
deliberately not a flow-end flush: when the flush is the only
reason to run the loggers, eof is not forced, so each logger's
condition decides and no other parser starts logging records it
suppressed; a disabling packet that does carry an app-layer
update keeps the existing EOF behaviour. SSHTxLogCondition
admits an IPS drop or a bypass, so the pending record is emitted
there; an IDS drop does not end the app layer, so the condition
stays failure-only - a later failure still wins, and the success
record is emitted by the flow-end flush, which the flow timeout
path now also runs for IDS-dropped flows. The unrelated
schema-sort ordering of the new state keys is fixed up as well.
Ticket: #9046.
Test-only change. The parse-failure freeze was unit-tested for the
invalid first banner and the invalid record (Test28); the
banner_wait_eol failure and the oversized-no-EOL invalid banner were
only pinned by SV (ssh-events). Add a unit test driving both: a
direction parked in banner_wait_eol whose completed line is followed
by an invalid record freezes the direction at kex, and a >=256B
banner without EOL that fails the banner grammar freezes the
direction at banner at once. Both phases assert the toclient
direction is unaffected.
Ticket: #9046.
Jason Ish
Lukas Sismis
Victor Julien
Philippe Antoine