In case a keyword should work in firewall mode, with firewall rules
only. The engine errors out if threat detection rules use the given keyword.
Part of
Ticket #9041
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.
Juliana Fajardini