Cover split END_STREAM headers in both directions, with and without an
already-closed peer, intermediate continuations, normal headers and DATA,
and duplicate end-of-stream handling.
Require a matching stream before inheriting the reassembly buffer's saved
END_STREAM flag. Otherwise an orphan continuation on stream B can inherit
stream A's flag and prematurely close B. Test that the orphan raises the
anomaly without consuming A's pending flag, then complete A normally.
Ticket: #9218
Ticket: 9217
Backport note: Feed the saved END_STREAM flag from CONTINUATION frames
through the existing 8.0.x half-closed/closed transaction-state transitions,
without importing main's per-direction progress tracking. Retain the
upstream header-reassembly and continuation-parser changes unchanged.
(cherry picked from commit 620dff6759)
As more continuation frames are expected.
Next commit will update progress for those continuation frames.
Ticket: 9217
Backport note: Gate the existing 8.0.x transaction-state transition on
END_HEADERS for HEADERS frames, preserving DATA and half-closed handling
instead of importing main's per-direction progress state machines.
(cherry picked from commit ef7184012a)
Change the byte order of vlan TCI when extracting it
from vlan_hdr in xdp_filter.c. This fixes xdp bypass for vlan tagged
packets, as without the fix flows that are supposed to be bypassed by
the xdp filter are not bypassed.
Ticket: 8699
(cherry picked from commit 5cb7a0d7be)
Ticket: 8684
The behavior of the rule
alert ip any any -> any any (pcre:"/.*mc|\H+$/"; sid:4;)
depended upon the presence of another rule such as
alert ip any any -> any any (content:"*mcH"; sid:5;)
With the sid 5 content rule, the code went through PrefilterPktStream
which updated raw_stream_progress, which ended up updating raw_progress_rel
which was used on later packets to know where to start.
Without the sid 5 content rule, there was no update of these progresses.
It led to quadratic complexity, as for each new packet, we ran
the pcre engine on the whole tcp stream from the beginning, with
the newly added payload data.
Fix is to process sid 4 pcre rule the same way whether there is
or there is not a sid 5 content rule.
So, in case we do not have a mpm store for a signature group head,
we register a dummy stream prefilter, whose goal will just be
to update raw_stream_progress
(cherry picked from commit f87b8e64a5)
In order to avoid breaking ABI on 64-bit, move the fields to existing
holes that do not alter the offset of any of the fields or the size of
the struct SSLStateConnp.
So far the SSL parser returned an error if there were less than 4 bytes
available for the handshake header. This meant that a fragmented
handshake header could result in the SSL parser erroring out and thus
disabling any further inspection for the protocol.
The parser follows a multi-record parsing scheme which means that the
fragment of the header may exist in the next record. Use internal
buffering to assess and process the records just like the handshake
buffer.
Ticket: 9167
(cherry picked from commit bea218e7b1)
The elephant flow backport from main changes Flow flag bits and the layout of
public structures. Main can do this as 9.0 breaks the ABI, but 8.0.x plugins
built against an earlier 8.0 release are still loaded.
Adjust the backport to keep the existing ABI:
- keep FLOW_IS_ELEPHANT at bit 3 with its 8.0 meaning of an elephant flow in
either direction, and track the direction in the aux flags as
FLOW_AUX_IS_ELEPHANT_TOSERVER and FLOW_AUX_IS_ELEPHANT_TOCLIENT
- leave FLOW_ACTION_ACCEPT at bit 4 and bit 5 vacant, so Flow::flags is
unchanged
- add the per direction elephant counters at the end of DecodeThreadVars
- add DETECT_FLOW_ELEPHANT at the end of the static keyword IDs
Ticket: #8117
Expose elephant flow flags on a flow via rule language. The rule keyword
is defined as "flow.elephant: <dir>;" where the <dir> can expand to:
- toserver
- toclient
- either
- both
Feature 8117
(cherry picked from commit 33a3448f45)
The tracking for elephant flow is done per direction however, the flag
was set on the flow whenever either of the directions crossed the
rate-limit defined in the settings. Given that the tracking was already
split, it makes sense to split the detection tracking per direction as
well and allow user to have a better control via rule language.
(cherry picked from commit 0f92583702)
The functionality of "both" can already be easily achieved by using both
"toclient" and "toserver" in a rule. This just adds the ease of
expression in rule. As it is added now, check the direction for the
pre-existing users of direction as well.
(cherry picked from commit 06eafb79d6)
Backport adaptations: The `qa/ci/Makefile.am` hunk is left out, as
8.0.x does not ship `qa/ci` in the dist; the job builds from the git
checkout and does not need it.
Ticket: 9238
(cherry picked from commit 3a6ecab098)
Several C loggers are registered through a cast to
EveJsonSimpleTxLogFunc, which has an incompatible signature. Clang's
function sanitizer reports this as a call through a pointer to an
incorrect function type.
Change these loggers to the generic callback signature and register them
without casts.
Ticket: 9226
Backport note: Keep the main-8.0.x ALPROTO_DNS inner protocol in DoH2
callbacks and omit the LLMNR logger, which is absent on this branch.
Restore detect-dce-opnum.h after spurious rename detection. These
resolutions match the earlier backport in PR 16458.
(cherry picked from commit 9451642a28)
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)
Jason Ish
Philippe Antoine
Adam Kiripolsky
Shivani Bhardwaj
Victor Julien
Lukas Sismis
Juliana Fajardini
Amir Boussejra
William Bland