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

analysis: report rule state altered by other rule - v1 #12286

Closed
wants to merge 1 commit into from

Conversation

jufajardini
Copy link
Contributor

Flowbits can make a rule such as a packet rule be treated as a stateful rule, without actually changing the rule type.

Add a flag to allow report such cases via the engine analysis.

Task #7456

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7456

Describe changes:

  • add a flag (bool) that is set when the flowbits analysis makes non stateful rules, such as a packet rule, a stateful one, so we can report that in the engine analysis report. Not a fan of the names used, but couldn't come up with something else...

Wasn't sure about using this as another SIG_INIT_FLAG or even as flag that could have other values, so decided to go with this as a proof of concept, sort of.

The output will also be tested with SV tests that should accompany the rule types doc.

Moved on with this work before the rule types documentation as this should also be present in the engine-analysis examples seen there.

Output example:

{
  "raw": "alert ip any any -> any any (msg:\"Flowbit isset ored flowbits\"; flowbits:isset,fb1|fb5; sid:1801;)",
  "id": 1801,
  "gid": 1,
  "rev": 0,
  "msg": "Flowbit isset ored flowbits",
  "app_proto": "unknown",
  "requirements": [
    "flow"
  ],
  "type": "pkt",
  "rule_state_dependency": true,
  "flags": [
    "src_any",
    "dst_any",
    "sp_any",
    "dp_any",
    "need_flowvar",
    "toserver",
    "toclient"
  ],
  "pkt_engines": [
    {
      "name": "packet",
      "is_mpm": false
    }
  ],
  "frame_engines": [],
  "engines": [],
  "lists": {
    "packet": {
      "matches": [
        {
          "name": "flowbits",
          "flowbits": {
            "cmd": "isset",
            "names": [
              "fb1",
              "fb5"
            ],
            "operator": "or"
          }
        }
      ]
    }
  }
}

Flowbits can make a rule such as a packet rule be treated as a stateful
rule, without actually changing the rule type.

Add a flag to allow report such cases via the engine analysis.

Task OISF#7456
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.23%. Comparing base (0e4faba) to head (3338880).
Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #12286    +/-   ##
========================================
  Coverage   83.22%   83.23%            
========================================
  Files         912      912            
  Lines      257311   257630   +319     
========================================
+ Hits       214154   214426   +272     
- Misses      43157    43204    +47     
Flag Coverage Δ
fuzzcorpus 61.08% <66.66%> (+0.01%) ⬆️
livemode 19.39% <33.33%> (-0.13%) ⬇️
pcap 44.44% <66.66%> (+0.07%) ⬆️
suricata-verify 62.85% <100.00%> (+0.01%) ⬆️
unittests 59.19% <33.33%> (+<0.01%) ⬆️

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

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23948

@@ -1047,6 +1047,8 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s)
break;
}

jb_set_bool(ctx.js, "rule_state_dependency", s->init_data->rule_state_dependency);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the immediate question ppl will have is "which rule?" followed by "why?". Perhaps more info can be added through the analyzer note part of the engine analysis

@jufajardini
Copy link
Contributor Author

Followed by #12311

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