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
Backport note: main-8.0.x has 16-bit sigmatch flags defined with BIT_U16() in
src/detect.h and no SIGMATCH_* constants in rust/sys, and lacks the
SIGMATCH_BAN_FIREWALL_RULE/MODE flags that surround this change on main.
SIGMATCH_BAN_TD_FIREWALL_MODE is defined as BIT_U16(14) in src/detect.h, the
rust/sys hunk is dropped, and only the new feature-list and parser checks are
added.
(cherry picked from commit d4aa40df61)
For calculating the address space to be matched against, the CIDR
netmask is taken to account by the IP Only Engine. However, this netmask
was not set properly for dotted netmasks. Fix it.
Ticket: 9166
(cherry picked from commit 8d46226a59)
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.
Backport adaptation: the `suricata.yaml.in` hunk is left out, as the
8.0.x yaml has no `firewall.policies` example to extend.
Ticket: 8968
(cherry picked from commit a329e0a3b9)
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.
Backport adaptations: the flow drop sets `FLOW_ACTION_DROP` and
`FLOW_AUX_ACTION_BY_FIREWALL` directly, like the other 8.0.x flow drops,
as 8.0.x has no `FlowSetDropAction()`. The debug log reads `p->pcap_cnt`,
as 8.0.x has no `PcapPacketCntGet()`.
Ticket: 8970
(cherry picked from commit 6fe3a3dfc3)
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.
(cherry picked from commit f2e3a2e2a7)
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
(cherry picked from commit 23ff7feebb)
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
(cherry picked from commit f9f3c2082a)
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.
(cherry picked from commit 8998fbacc5)
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.
Backport note: on 8.0.x the log thread is log-flush.c (log-maintenance.c on
main) and has no rotation, so the first-slice change was applied there.
(cherry picked from commit 1ff8de029a)
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.
(cherry picked from commit e465600c9f)
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
(cherry picked from commit 8b379a3b04)
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.
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.
(cherry picked from commit 65d2df795f)
Disable the logging to console, so that we can parse more reliably
the list of app-layer protos, that we will use to create the list
of app-layer fuzz targets.
As it may be unexpected that removing app-layer.protocols.pgsql.enabled
or the pgsql section altogether from the settings actually enables the
parser, inform users of this when it happens.
Ticket: #8350
EngineAnalysisFP(), CleanupFPAnalyzer(), and CleanupRuleAnalyzer() were writing
to and closing the wrong FILE* pointers. Fast pattern output was written to
rules_analysis.txt instead of rules_fast_pattern.txt, leaving the latter
effectively empty.
Restore the behavior from before c8615bcd4, which inadvertently swapped the
pointers.
Bug: #9007
(cherry picked from commit fe8b2f3d2a)
The filter range excluded $first_commit itself, so the first commit on
the branch was never rewritten and single-commit branches failed.
Ticket: #9097
(cherry picked from commit b778e8c06d)
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
(cherry picked from commit 4f47e43a33)
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
(cherry picked from commit 6aa97aa8a9)
Prevent out-of-bounds read of protocol field when PPPoE session header is
exactly PPPOE_SESSION_HEADER_MIN_LEN bytes. Inspect first protocol octet
only to decide single vs double octet form, and read second octet only after
len >= 8 verification.
Add unit tests for the 7-byte boundary case for both the double-octet
(rejected as too small) and single-octet (decoded as PPP_IP) protocols.
Ticket: #8856.
(cherry picked from commit e41ccf837e)
SCACTileSearchLarge uses 32-bit next-state cells holding a 24-bit state
index. When there are more than 65536 states, CheckMatch was called with
(uint16_t)state, silently discarding the bits beyond 16.
Address this by widening CheckMatch's state parameter.
Ticket: #8841.
(cherry picked from commit c3b9cad5dd)
The TLV loop for the IPv6 hop-by-hop and destination option headers
gates each option on (offset + 1 + ip6_optlen) > optslen. An option is
two header bytes (type, length) plus ip6_optlen data bytes, so its data
ends at offset + 1 + ip6_optlen and the bound has to be
offset + 2 + ip6_optlen.
Before: an option that declares its data running one byte past the
option area is accepted, and the Router Alert / Jumbo / Home Address
memcpy then copies sizeof(value) bytes ending one byte past the area;
when the header sits at the tail of the IPv6 payload that read goes
past the payload.
After: the option is rejected with IPV6_EXTHDR_INVALID_OPTLEN, matching
how the IPv4 option loop already bounds a full option against the
remaining length. The check stays next to the read so each option type
does not need its own guard.
Ticket: #8817.
(cherry picked from commit 38b4ad79a3)
Tickete: 8265
The traffic "USER CAPABILITY" could be interpreted as both ftp
or imap.
Add a probing parser to imap after detection of the " CAPABILITY"
pattern to try do disambiguate.
(cherry picked from commit e9bf04e9e7)
The byte backstop leaves many tiny OOO chunks unbounded, and the scan
only inspected the in-progress file, which an interleaved second
handle could hide. Add a chunk-count backstop (configured count, 1024
hard cap when disabled) and check every open file tx.
Ticket: #8791.
(cherry picked from commit a4e4c3cd7b)
Replace the hardcoded 2 MiB gap backstop, which silently truncated
OOO file data, with the configured queue size per direction (1 GiB
hard cap when disabled), raising SMBEvent::TruncatedFileData. It runs
ahead of the chunk_left == 0 early return, and a gap reaching the
tracker tags the session gapped even when fully consumed.
Ticket: #8791.
(cherry picked from commit 3c9883b37c)
Philippe Antoine
Victor Julien
Juliana Fajardini
Shivani Bhardwaj
Lukas Sismis
Jason Ish
Amir Boussejra
William Bland
Uwez Khan
Giuseppe Longo