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

tests: add rule type check for flowbits v3 #1439

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions tests/rules/flowbits/test.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
alert ip any any -> any any (msg:"Flowbit noalert"; flowbits:noalert,fb1; sid:1;)
Copy link
Contributor

Choose a reason for hiding this comment

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

noalert is the single flowbits action that doesn't require a name. ;)

alert ip any any -> any any (msg:"Flowbit isset"; flowbits:isset,fb1; sid:2;)
alert ip any any -> any any (msg:"Flowbit isnotset"; flowbits:isnotset,fb1; sid:3;)
alert ip any any -> any any (msg:"Flowbit set"; flowbits:set,fb1; sid:4;)
alert ip any any -> any any (msg:"Flowbit unset"; flowbits:unset,fb1; sid:5;)
alert ip any any -> any any (msg:"Flowbit toggle"; flowbits:toggle,fb1; sid:6;)
alert ip any any -> any any (msg:"Flowbit isset ored flowbits"; flowbits:isset,fb1|fb2; sid:7;)
alert ip any any -> any any (msg:"Flowbit isnotset ored flowbits"; flowbits:isnotset, fb1|fb2 ; sid:8;)
Comment on lines +7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

As I've explained in the Suricata PR, the ored flowbits variables won't exist, nor be logged, if they have not been set with flowbits:set before. So, fb2 won't exist for the engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh so I should create another rule for fb2 as such and then use it in isset and isnotset rules:
alert ip any any -> any any (msg:"Flowbit set"; flowbits:set,fb2; sid:2;)
Am I correct to assume this and if so, should I also create duplicates of all other rules for fb2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you should create a rule for setting fb2. But there's no need to create isset or isnoset rules for it. It set will setup the variable name with Suri. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, but should I create toggle, unset and other such rules for fb2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not, there's no need.

64 changes: 64 additions & 0 deletions tests/rules/flowbits/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
requires:
min-version: 7.0.0
pcap: false

args:
- --engine-analysis

checks:
- filter:
filename: rules.json
count: 1
match:
id: 1
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "toggle"
Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

These checks slipped my prior review. Upon further inspection with with actual output, what we see is that the rules with ids 1, 4, 5 and 6 here actually don't have this output structure. It's:

 "lists": {
    "postmatch": {
      "matches": [
        {
          "name": "flowbits",
          "flowbits": {
            "variables": 0,
            "cmd": "toggle",
            "name": "fb1"
          }
        }
      ]
    }
  }

So, we must adjust the checks here accordingly.

Copy link
Contributor Author

@hadiqaalamdar hadiqaalamdar Oct 25, 2023

Choose a reason for hiding this comment

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

should I also create one for "cmd": "set", "name": "fb2" then?

Copy link
Contributor

Choose a reason for hiding this comment

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

The output that I shared indicates that instead of lists.patcket.matches..., for the rules that I indicated, the check must be for lists.postmatch.matches.

The cmd check will fail if we change the code to log action instead of cmd ;)

Last part: yes, we should have checks for all variable names that we have in the rules, and for the ored variables case, which will be different, since it will be an array. Check the output, locally, and you'll see :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh okay, thank you! I'll make the edits and see what output I get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jufajardini where are you seeing this output? None of the files in my output directory display such information.

Copy link
Contributor

@jufajardini jufajardini Oct 25, 2023

Choose a reason for hiding this comment

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

Can you be more specific in terms of which information, exactly?
In tests/rules/flowbits/output/rules.json
Some of the differences won't be there yet, unless you have already updated the code, that is ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This output, I don't see it anywhere. I've updated the test.rules and test.yaml files.

 "lists": {
    "postmatch": {
      "matches": [
        {
          "name": "flowbits",
          "flowbits": {
            "variables": 0,
            "cmd": "toggle",
            "name": "fb1"
          }
        }
      ]
    }
  }

My rules.json file looks like this:

{"raw":"alert ip any any -> any any (msg:\"Flowbit set\"; flowbits:set,fb1; sid:4;)","id":4,"gid":1,"rev":0,"msg":"Flowbit set","app_proto":"unknown","requirements":[],"type":"ip_only","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[],"frame_engines":[],"lists":{"postmatch":{"matches":[{"name":"flowbits","flowbits":{"cmd":"set","name":"fb1"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit set\"; flowbits:set,fb2; sid:5;)","id":5,"gid":1,"rev":0,"msg":"Flowbit set","app_proto":"unknown","requirements":[],"type":"ip_only","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[],"frame_engines":[],"lists":{"postmatch":{"matches":[{"name":"flowbits","flowbits":{"cmd":"set","name":"fb2"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit unset\"; flowbits:unset,fb1; sid:6;)","id":6,"gid":1,"rev":0,"msg":"Flowbit unset","app_proto":"unknown","requirements":[],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[],"frame_engines":[],"lists":{"postmatch":{"matches":[{"name":"flowbits","flowbits":{"cmd":"unset","name":"fb1"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit toggle\"; flowbits:toggle,fb1; sid:7;)","id":7,"gid":1,"rev":0,"msg":"Flowbit toggle","app_proto":"unknown","requirements":[],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[],"frame_engines":[],"lists":{"postmatch":{"matches":[{"name":"flowbits","flowbits":{"cmd":"toggle","name":"fb1"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit isset\"; flowbits:isset,fb1; sid:2;)","id":2,"gid":1,"rev":0,"msg":"Flowbit isset","app_proto":"unknown","requirements":["flow"],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","need_flowvar","toserver","toclient"],"pkt_engines":[{"name":"packet","is_mpm":false}],"frame_engines":[],"lists":{"packet":{"matches":[{"name":"flowbits","flowbits":{"cmd":"isset","name":"fb1"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit isnotset\"; flowbits:isnotset,fb1; sid:3;)","id":3,"gid":1,"rev":0,"msg":"Flowbit isnotset","app_proto":"unknown","requirements":["flow"],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[{"name":"packet","is_mpm":false}],"frame_engines":[],"lists":{"packet":{"matches":[{"name":"flowbits","flowbits":{"cmd":"isnotset","name":"fb1"}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit isset ored flowbits\"; flowbits:isset,fb1|fb2; sid:8;)","id":8,"gid":1,"rev":0,"msg":"Flowbit isset ored flowbits","app_proto":"unknown","requirements":["flow"],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","need_flowvar","toserver","toclient"],"pkt_engines":[{"name":"packet","is_mpm":false}],"frame_engines":[],"lists":{"packet":{"matches":[{"name":"flowbits","flowbits":{"cmd":"isset","ored_flowbits":2,"ored_variables":["fb1"]}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit isnotset ored flowbits\"; flowbits:isnotset, fb1|fb2 ; sid:9;)","id":9,"gid":1,"rev":0,"msg":"Flowbit isnotset ored flowbits","app_proto":"unknown","requirements":["flow"],"type":"pkt","flags":["src_any","dst_any","sp_any","dp_any","toserver","toclient"],"pkt_engines":[{"name":"packet","is_mpm":false}],"frame_engines":[],"lists":{"packet":{"matches":[{"name":"flowbits","flowbits":{"cmd":"isnotset","ored_flowbits":2,"ored_variables":["fb1"]}}]}}}
{"raw":"alert ip any any -> any any (msg:\"Flowbit noalert\"; flowbits:noalert; sid:1;)","id":1,"gid":1,"rev":0,"msg":"Flowbit noalert","app_proto":"unknown","requirements":[],"type":"ip_only","flags":["src_any","dst_any","sp_any","dp_any","noalert","toserver","toclient"],"pkt_engines":[],"frame_engines":[],"lists":{}}

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same type of output, the only difference is that mine is formatted ;)
If you search your json, you'll see the lists object there, too.

- filter:
filename: rules.json
count: 1
match:
id: 2
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "isset"
- filter:
filename: rules.json
count: 1
match:
id: 3
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "isnotset"
- filter:
filename: rules.json
count: 1
match:
id: 4
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "set"
- filter:
filename: rules.json
count: 1
match:
id: 5
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "unset"
- filter:
filename: rules.json
count: 1
match:
id: 6
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "toggle"
- filter:
filename: rules.json
count: 1
match:
id: 7
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "isset"
- filter:
filename: rules.json
count: 1
match:
id: 8
lists.packet.matches[0].name: "flowbits"
lists.packet.matches[0].flowbits.cmd: "isnotset"
Loading