tests: add rule to check for tcp/ack - v1 #1423
Conversation
Related to Issue: #6354
There was a problem hiding this comment.
Nice! One comment from my end.
I've linked this PR on your Suricata PR description, so that our CI checks will tests your changes against it. Let's see how that goes!
Meanwhile, I'll try to give some thought on naming for the ack value. Maybe we could have just number, to follow with our documentation uses. 🤔 I'll add that suggestion to the Suri PR, and then you can update the SV test accordingly :)
| count: 1 | ||
| match: | ||
| id: 1 | ||
| lists.packet.matches[0].name: "tcp.ack" |
There was a problem hiding this comment.
Would be good to have another line check here with the actual value that ack has in the line, (cf https://github.com/OISF/suricata-verify/tree/master/tests/rules/ipopts or other tests there, many of them check not only for the keyword itself, but also any value assigned to it.)
There was a problem hiding this comment.
Yes! I wanted to do that, but looking at the codes, I didn't see any parameter that would be used for it.
So I assumed it didn't exist.
There was a problem hiding this comment.
I don't think this test tests what you have done in your Suricata PR. That should have made this test fail. tcp.ack already exists in Suricata's matches which is why the rules as you have written work.
You should have checked for (alongwith what you already have) lists.packet....ack.ack: 273.
I didn't see any parameter that would be used for it.
Because you just added it in your Suricata PR ;)
Test for rule type for tcp-ack keyword.
Related to
Issue: #6354
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6354
Suricata PR: OISF/suricata#9608