-
Notifications
You must be signed in to change notification settings - Fork 90
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
flow: Add test for excluding pkt recursion from flow #1370
flow: Add test for excluding pkt recursion from flow #1370
Conversation
Add tests for verifying matching packet flows when including and excluding pkt recursion from flow matching. Bug: #6260
--set "threshold-file=${SRCDIR}/threshold.config" \ | ||
--set "classification-file=${SRCDIR}`[ -f ${SRCDIR}/etc/classification.config ] && printf '/etc'`/classification.config" \ | ||
--set "reference-config-file=${SRCDIR}`[ -f ${SRCDIR}/etc/reference.config ] && printf '/etc'`/reference.config" \ | ||
--set 'decoder.recursion-level.use-for-tracking=false' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use args
instead of the whole command here
(and pcap:
to reuse another pcap)
match: | ||
event_type: flow | ||
proto: ICMP | ||
flow.pkts_toserver: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There does not seem to be a difference with flow-pkt-recursion-middleware-included/test.yaml
Is there ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, there is no difference in the expected flow matching result when the Suricata device is not a tunnel terminator. This is what flow-pkt-recursion-middleware-excluded and flow-pkt-recursion-middleware-included are showing, that for flows on the same recursion level there if no difference with this new config option decoder.recursion-level.use-for-tracking disabled (it is enabled by default).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Is this no difference in behavior not proved by existing tests already ?..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your comments. The existing test would need to be using tunneled unencrypted packets and explicitly check flow matching to already be proved. A test that matches this behavior might exist, but I thought it was specific enough traffic to add in a test for this feature. Happy to remove if you don't agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, let's keep it :-)
Thanks for the comments. Applied change in new pull request #1373. |
Add tests for verifying matching packet flows when including and excluding pkt recursion from flow matching.
Bug: #6260
Redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6260