Detect rule hook/v8 #12628

victorjulien · 2025-02-19T13:07:01Z

Continuing hook work from #12422.

Adds a first packet hook: flow_start. Allows for rules like

alert tcp:flow_start any any -> any any (seq:123; sid:1;)
alert tcp:flow_start any any -> any any (dsize:0; sid:2;)
alert tcp:flow_start any any -> any any (sid:3;)
alert ip:flow_start any any -> any any (sid:4;)
alert ip:flow_start any any -> any any (flow:to_server; sid:5;)

Builds on #12567, in that every rule is now part of a "prefilter engine", and which engines are run depends on the hooks. Packet has a new field Packet::pkt_hooks, which will hold a bit array of hooks that should be activated by this packet. Right now only flow_start.

SV_BRANCH=OISF/suricata-verify#2308

codecov · 2025-02-19T17:43:57Z

Codecov Report

Attention: Patch coverage is 87.07182% with 117 lines in your changes missing coverage. Please review.

Project coverage is 80.77%. Comparing base (80dbaac) to head (2a2e188).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12628      +/-   ##
==========================================
+ Coverage   80.75%   80.77%   +0.01%     
==========================================
  Files         934      934              
  Lines      259594   259999     +405     
==========================================
+ Hits       209634   210010     +376     
- Misses      49960    49989      +29

Flag	Coverage Δ
fuzzcorpus	`57.05% <69.98%> (+0.06%)`	⬆️
livemode	`19.52% <41.36%> (+0.15%)`	⬆️
pcap	`44.20% <47.50%> (+0.03%)`	⬆️
suricata-verify	`63.62% <85.39%> (+0.09%)`	⬆️
unittests	`58.33% <60.22%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

suricata-qa · 2025-02-21T10:05:03Z

Information:

ERROR: QA failed on report_test.

field	baseline	test	%
	SURI_TLPW2_single_stats_chk
.uptime	750	775	103.33%
	SURI_TLPW2_autofp_stats_chk
.uptime	143	166	116.08%
	SURI_TLPW1_stats_chk
.uptime	180	202	112.22%
	SURI_TLPR1_stats_chk
.uptime	637	663	104.08%

Pipeline 24831

suricata-qa · 2025-02-23T02:05:03Z

Information: QA ran without warnings.

Pipeline 24866

Several rules matched on both directions even if events are set in a single direction.

Instead of having a per detection engine list of rule that couldn't be prefiltered, put those into special "prefilter" engines. For packet and frame rules this doesn't change much, it just removes some hard coded logic from the detect engine. For the packet non-prefilter rules in the "non-prefilter" special prefilter engine, add additional filtering for the packet variant. It can prefilter on alproto, dsize and dest port. The frame non-prefilter rules are added to a single engine, that per rule checks the alproto and the type. For app-layer, there is an engine per progress value, per app-layer protocol and per direction. This hooks app-layer non-prefilter rules into the app inspect logic at the correct "progress" hook. e.g. a rule like dns.query; bsize:1; Negated MPM rules will also fall into this category: dns.query; content:!"abc"; Are part of a special "generic list" app engine for dns, at the same progress hook as `dns.query`. This all results in a lot fewer checks: previous: -------------------------------------------------------------------------- Date: 1/29/2025 -- 10:22:25. Sorted by: number of checks. -------------------------------------------------------------------------- Num Rule Gid Rev Ticks % Checks Matches Max Ticks Avg Ticks Avg Match Avg No Match -------- ------------ -------- -------- ------------ ------ -------- -------- ----------- ----------- ----------- -------------- 1 20 1 0 181919672 11.85 588808 221 60454 308.96 2691.46 308.07 2 50 1 0 223455914 14.56 453104 418 61634 493.17 3902.59 490.02 3 60 1 0 185990683 12.12 453104 418 60950 410.48 1795.40 409.20 4 51 1 0 192436011 12.54 427028 6084 61223 450.64 2749.12 417.42 5 61 1 0 180401533 11.75 427028 6084 61093 422.46 2177.04 397.10 6 70 1 0 153899099 10.03 369836 0 61282 416.13 0.00 416.13 7 71 1 0 123389405 8.04 369836 12833 44921 333.63 2430.23 258.27 8 41 1 0 63889876 4.16 155824 12568 39138 410.01 1981.97 272.10 9 40 1 0 64149724 4.18 155818 210 39792 411.70 4349.57 406.38 10 10 1 0 70848850 4.62 65558 0 39544 1080.70 0.00 1080.70 11 11 1 0 94743878 6.17 65558 32214 60547 1445.19 2616.14 313.92 this commit: -------------------------------------------------------------------------- Date: 1/29/2025 -- 10:15:46. Sorted by: number of checks. -------------------------------------------------------------------------- Num Rule Gid Rev Ticks % Checks Matches Max Ticks Avg Ticks Avg Match Avg No Match -------- ------------ -------- -------- ------------ ------ -------- -------- ----------- ----------- ----------- -------------- 1 50 1 0 138776766 19.23 95920 418 167584 1446.80 3953.11 1435.83 2 60 1 0 97988084 13.58 95920 418 182817 1021.56 1953.63 1017.48 3 51 1 0 105318318 14.60 69838 6084 65649 1508.04 2873.38 1377.74 4 61 1 0 89571260 12.41 69838 6084 164632 1282.56 2208.41 1194.20 5 11 1 0 91132809 12.63 32779 32214 373569 2780.22 2785.58 2474.45 6 10 1 0 66095303 9.16 32779 0 56704 2016.39 0.00 2016.39 7 70 1 0 48107573 6.67 12928 0 42832 3721.19 0.00 3721.19 8 71 1 0 32308792 4.48 12928 12833 39565 2499.13 2510.05 1025.09 9 41 1 0 25546837 3.54 12886 12470 41479 1982.53 1980.84 2033.05 10 40 1 0 26069992 3.61 12886 210 38495 2023.13 4330.05 1984.91 11 20 1 0 639025 0.09 221 221 14750 2891.52 2891.52 0.00

To support hook based buffer names.

e.g. server hello done has no data

Per direction track progress to be able to have more fine grained control over where the detection engines and logging hooks in.

Generic: <app_proto>:request_done and <app_proto>:response_done Per protocol, it uses the registered progress (state) values. E.g. tls:client_hello_done A rule ruleset could be: pass tls:client_hello_done any any -> any any (tls.sni; content:"www.google.com"; sid:21; alert;) drop tls:client_hello_done any any -> any any (sid:22;) The pass rule is evaluated when the client hello is parsed, and if it doesn't match the drop rule will be evaluated. Registers each generic lists as "<alproto>:<progress state>:generic" (e.g. "tls:client_hello_done:generic"). Ticket: OISF#7485.

For registration of app-layer inspection, no longer use the 'needs' table from the script, but instead use the rule hook setting. Ticket: OISF#4783.

WIP packet: track hooks that this packet triggers hooks: set flow_start hook in packet detect: pack prefilter engine struct For future expansion of the fields. detect/prefilter: put pkt mask in struct In preparation for adding more pkt fields. WIP detect: implement pkt:flow_start hook SQUASH detect flow start hook

suricata-qa · 2025-02-27T07:10:02Z

Information: QA ran without warnings.

Pipeline 24922

victorjulien mentioned this pull request Feb 19, 2025

Detect rule hook/v7 #12422

Closed

victorjulien force-pushed the detect-rule-hook/v8 branch from 587343c to 6524605 Compare February 19, 2025 15:50

victorjulien force-pushed the detect-rule-hook/v8 branch from 10b6154 to 049311c Compare February 20, 2025 16:54

victorjulien force-pushed the detect-rule-hook/v8 branch 2 times, most recently from 437044e to 81ab9fa Compare February 21, 2025 21:56

victorjulien added 19 commits February 26, 2025 11:17

smtp/events: set direction on rules

aaf673d

Several rules matched on both directions even if events are set in a single direction.

detect: don't register duplicate app inspect engines

d95d2e0

detect/nfs: don't double register nfq_request generic list

90a7b59

detect/tls: don't double register tls_validity generic list

f97d5b8

app-layer: constify AppLayerGetProtoByName

9ed7196

detect: allow longer buffer names

60704bc

To support hook based buffer names.

tls: fix handshake handling being too strict

06d7358

e.g. server hello done has no data

tls: introduce per direction progress tracking

08bea92

Per direction track progress to be able to have more fine grained control over where the detection engines and logging hooks in.

app-layer: API for mapping progress name vs id

5b3905a

tls: expose progress by name

4fb93ac

http1: register progress state names

7326c50

detect/lua: use rule hook instead of init() 'needs' table

9cac879

For registration of app-layer inspection, no longer use the 'needs' table from the script, but instead use the rule hook setting. Ticket: OISF#4783.

detect: reuse hook based generic lists

9939e8f

detect: constify rule file and lines in parsing and analyzer

e39c223

detect/loader: minor code cleanup

c30c647

WIP start of firewall rules

2a2e188

victorjulien force-pushed the detect-rule-hook/v8 branch from 81ab9fa to 2a2e188 Compare February 26, 2025 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect rule hook/v8 #12628

Detect rule hook/v8 #12628

victorjulien commented Feb 19, 2025

codecov bot commented Feb 19, 2025 •

edited

Loading

suricata-qa commented Feb 21, 2025

suricata-qa commented Feb 23, 2025

suricata-qa commented Feb 27, 2025

Detect rule hook/v8 #12628

Are you sure you want to change the base?

Detect rule hook/v8 #12628

Conversation

victorjulien commented Feb 19, 2025

codecov bot commented Feb 19, 2025 • edited Loading

Codecov Report

suricata-qa commented Feb 21, 2025

suricata-qa commented Feb 23, 2025

suricata-qa commented Feb 27, 2025

codecov bot commented Feb 19, 2025 •

edited

Loading