pcrexform gave no signal when the regex did not match, when the capture
could not be extracted, or when there was no room for the capture. A
capture of nothing also left the input in place when the buffer had no
room yet, since InspectionBufferCopy then copies nothing.
Mark the transform SIGMATCH_TRANSFORM_CAN_FAIL and flag those failures
with SCInspectionBufferSetError. Set an empty capture's length to 0
directly, and leave an empty input alone, as from_base64 does.
Issue: 7313
from_base64 gave no signal when offset or bytes ran past the input or
nothing decoded, so a later keyword could not tell a failed decode from
a successful one.
It also skipped decoding when bytes equaled the input left after
offset, which the C transform before d8a99e1fdd decoded.
When an earlier transform had left the input in the buffer's own
memory, a strict decode that failed partway wrote decoded bytes over
it: after strip_whitespace, "/Zm9vYmFyYmF6cXV4!!" no longer matched.
Mark the transform SIGMATCH_TRANSFORM_CAN_FAIL and flag these failures,
and allocation failures, with SCInspectionBufferSetError. Decode when
bytes equals the remaining input. In place, decode into a separate
buffer and copy the result over only on success. Call base64_decode
instead of SCBase64Decode.
Issue: 7114
SCBase64Decode builds its output slice with the input length, whatever
the caller allocated, and the from_base64 transform called it through
unsafe after asking for only the decoded length.
Move the decoding into base64_decode(), which takes the input and output
as slices, and keep SCBase64Decode as a wrapper for C callers. The
transform switches to base64_decode in the next commit.
Issue: 7114
Add SCInspectionBufferSetError(), which sets a new
DETECT_CI_FLAGS_ERROR flag and leaves the buffer's data unchanged, so a
keyword after a failed transform can tell the failure from a success.
Add SIGMATCH_TRANSFORM_CAN_FAIL to mark transforms that call it.
Clear the flags in each buffer setup function, so an error from one use
of a buffer does not carry over to the next.
Issue: 7313
dcerpc_tx_get_stub_data set the DCE byte-order flag before
SCInspectionBufferSetupAndApplyTransforms. Once a later commit makes
buffer setup reset the flags, that order would clear the flag and
break byte_test's dce option (dcerpc/dcerpc-dcepayload-15).
Set the flag after the call.
Issue: 7313
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.
Jeff Lucovsky
Juliana Fajardini
Jason Ish
Lukas Sismis
Victor Julien
Philippe Antoine