tests: add rule type check for flowbits v3 #1439
Conversation
Task #6309
There was a problem hiding this comment.
We're improving! 🎉
Running the tests locally now, I've noticed that some of the checks in the test would fail. I've added a comment so you can update them.
Once you've fixed the noalert rule issue, you should be able to run the tests locally, which should allow you to better test things - literally :P
Once you've run things locally and checked the output, it would be good to add more info to the test checks - the variable or ored variable names :)
| @@ -0,0 +1,8 @@ | |||
| alert ip any any -> any any (msg:"Flowbit noalert"; flowbits:noalert,fb1; sid:1;) | |||
There was a problem hiding this comment.
noalert is the single flowbits action that doesn't require a name. ;)
| 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;) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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. :)
There was a problem hiding this comment.
right, but should I create toggle, unset and other such rules for fb2?
| lists.packet.matches[0].name: "flowbits" | ||
| lists.packet.matches[0].flowbits.cmd: "toggle" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
should I also create one for "cmd": "set", "name": "fb2" then?
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
oh okay, thank you! I'll make the edits and see what output I get.
There was a problem hiding this comment.
@jufajardini where are you seeing this output? None of the files in my output directory display such information.
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
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":{}}
There was a problem hiding this comment.
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.
hadiqaalamdar
changed the title
|
new PR: #1441 |
Task #6309
Related to
Issue: #6309
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6309
Previous PR: #1438
Suricata PR: OISF/suricata#9688