Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/flowbit prefilter/v27 #12681

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

victorjulien
Copy link
Member

victorjulien and others added 4 commits February 26, 2025 20:32
Allow for more efficient rules that 'prefilter' on flowbits with 'isset' logic.

This prefilter is enabled by default, which means that if no mpm is present or
no explicit prefilter is used, the flowbits prefilter will be set up for a rule.

flowbits 'isset' prefilter

For rules that have a 'flowbits:isset,<bit>' statement, a "regular" prefilter
facility is created. It means that the rules are removed from the normal
match list(s) and added to a prefilter engine that runs prior to the individual
rule inspection stage.

Implementation: the prefilter is implemented as an RB_TREE of flowbits, with the
rule id's they "enable" stored per tree node. The matching logic is walking the
list of bits set in the flow and looking each of them up in the RB_TREE, adding
the rule ids of each of the matching bits to the list of rule candidates.

The 'isset' prefilter has one important corner case, which is that bits can in
fact be set during the rule evaluation stage. This is different from all other
prefilter engines, that evaluate an immutable state (for the lifetime of the
packets inspection).

flowbits 'set' post-match prefilter

For flowbits 'set' action, special post-match 'prefilter' facilities deal with
this corner case. The high level logic is that these track which 'isset' sigs
depend on them, and add these dependencies to the candidates list when a 'set'
action occurs.

This is implemented in a few steps:

1. flowbits 'set' is flagged
2. when 'set' action occurs the flowbit is added to a "post rule
   match work queue"
3. when the rule evaluation ends, the post-match "prefilter" engine is run
   on each of the flowbits in the "post rule match work queue"
4. these engines ammend the candidates list with the rule id dependencies
   for the flowbit
5. the candidates list is sorted to make sure within the execution for that
   packet the inspection order is maintained

Ticket: OISF#2486.
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 81.77258% with 109 lines in your changes missing coverage. Please review.

Project coverage is 80.75%. Comparing base (80dbaac) to head (03e395c).

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #12681    +/-   ##
========================================
  Coverage   80.75%   80.75%            
========================================
  Files         934      934            
  Lines      259594   260065   +471     
========================================
+ Hits       209634   210026   +392     
- Misses      49960    50039    +79     
Flag Coverage Δ
fuzzcorpus 56.82% <19.96%> (-0.17%) ⬇️
livemode 19.31% <1.35%> (-0.06%) ⬇️
pcap 44.07% <15.56%> (-0.10%) ⬇️
suricata-verify 63.58% <81.27%> (+0.05%) ⬆️
unittests 58.22% <16.41%> (-0.10%) ⬇️

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

@catenacyber
Copy link
Contributor

I feel a bit like this post-prefilter logic could be useful more generally in the detect engine, in the sense that we run not only one prefilter stage.

Like

  1. we take the signature group head
  2. prefilter add sids to the candidates to match

And then, instead of evaluating each rule individually, we use the remaining prefilter engines.

Like if my candidates have rules with http.ua content (after having http.uri prefilter)

@catenacyber
Copy link
Contributor

Assertion failed: (!(DeStateSearchState(state, direction, s->num))), function DeStateSignatureAppend, file detect-engine-state.c, line 127.

with http-all-headers SV test and added rules

alert http any any -> any any (http.uri; content:"down"; flowbits:set,uritest; sid:11;)
alert http any any -> any any (http.user_agent; content:"Mozilla"; flowbits:isset, headtest; prefilter; flowbits:set,moz; sid:10;)
alert http any any -> any any (http.method; content:"GET"; flowbits:isset,uritest; prefilter; flowbits:set,headtest; sid:12;)
alert http any any -> any any (http.host; content:"ether"; flowbits:isset,moz; prefilter; sid:14;)

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants