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

flow: Add test for excluding pkt recursion from flow #1370

Conversation

coledishington
Copy link
Contributor

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

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'
Copy link
Collaborator

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
Copy link
Collaborator

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 ?

Copy link
Contributor Author

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).

Copy link
Collaborator

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 ?..

Copy link
Contributor Author

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.

Copy link
Collaborator

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 :-)

@coledishington
Copy link
Contributor Author

coledishington commented Sep 4, 2023

You can use args instead of the whole command here

(and pcap: to reuse another pcap)

Thanks for the comments. Applied change in new pull request #1373.

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.

2 participants