-
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
Closed
coledishington
wants to merge
1
commit into
OISF:master
from
coledishington:flow-optional-recur-limit-6260-v3
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Test Purpose | ||
|
||
Tests comparing flows with and without recursion level set. Ignoring | ||
recursion level in flows is useful for devices that run inline IPS and | ||
terminate an unencrypted tunnel, like an IPv6 tunnel. Terminating the | ||
tunnel causes ingress request and reply traffic to have different | ||
headers. e.g. | ||
|
||
request: IPv4]ICMP] -> |IPS| -> IPv6]IPv4]ICMP] | ||
reply: <- |IPS| <- IPv6]IPv4]ICMP] | ||
|
||
The terminated tests are checking when the suricata is an inline IPS | ||
device that is terminating a tunnel. Both flow directions are tested, | ||
first packet ingress on non-tunneled interface and first packet ingress | ||
on tunneled interface. | ||
This is the case where recursion level can affect the flows when IPS is | ||
running inline IPS and terminating a tunnel. | ||
|
||
Middleware tests check when the suricata device is analysing tunneled | ||
packets and is not a tunnel terminator. | ||
This case should not be affected by recursion level in flows. | ||
|
||
## PCAP | ||
|
||
This PCAP was generated with scapy. |
18 changes: 18 additions & 0 deletions
18
tests/flow-pkt-recursion/flow-pkt-recursion-middleware-excluded/test.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# Exclude recursion-level from flow matching | ||
# Re-use the pcap from the middleware-included test. | ||
command: | | ||
${SRCDIR}/src/suricata -c "${SRCDIR}/suricata.yaml" -l "${OUTPUT_DIR}" \ | ||
-r "${TEST_DIR}/../flow-pkt-recursion-middleware-included/test.pcap" \ | ||
--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' | ||
|
||
checks: | ||
- filter: | ||
count: 1 | ||
match: | ||
event_type: flow | ||
proto: ICMP | ||
flow.pkts_toserver: 1 | ||
flow.pkts_toclient: 1 |
Binary file added
BIN
+320 Bytes
tests/flow-pkt-recursion/flow-pkt-recursion-middleware-included/test.pcap
Binary file not shown.
8 changes: 8 additions & 0 deletions
8
tests/flow-pkt-recursion/flow-pkt-recursion-middleware-included/test.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
checks: | ||
- filter: | ||
count: 1 | ||
match: | ||
event_type: flow | ||
proto: ICMP | ||
flow.pkts_toserver: 1 | ||
flow.pkts_toclient: 1 |
19 changes: 19 additions & 0 deletions
19
tests/flow-pkt-recursion/flow-pkt-recursion-terminated-excluded/test.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# Exclude recursion-level from flow matching | ||
# Re-use the pcap from the middleware-included test. | ||
command: | | ||
${SRCDIR}/src/suricata -c "${SRCDIR}/suricata.yaml" -l "${OUTPUT_DIR}" \ | ||
-r "${TEST_DIR}/../flow-pkt-recursion-terminated-included/test.pcap" \ | ||
--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 commentThe reason will be displayed to describe this comment to others. Learn more. You can use (and |
||
|
||
checks: | ||
# All packets should be caught as being in one flow | ||
- filter: | ||
count: 2 | ||
match: | ||
event_type: flow | ||
proto: ICMP | ||
flow.pkts_toserver: 1 | ||
flow.pkts_toclient: 1 |
Binary file added
BIN
+536 Bytes
tests/flow-pkt-recursion/flow-pkt-recursion-terminated-included/test.pcap
Binary file not shown.
9 changes: 9 additions & 0 deletions
9
tests/flow-pkt-recursion/flow-pkt-recursion-terminated-included/test.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
checks: | ||
# None of the flows are joined due to different recursion levels | ||
- filter: | ||
count: 4 | ||
match: | ||
event_type: flow | ||
proto: ICMP | ||
flow.pkts_toserver: 1 | ||
flow.pkts_toclient: 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
from pathlib import Path | ||
|
||
from scapy.all import ICMP, IP, Ether, IPv6, PcapWriter, Raw | ||
|
||
mac_1, mac_2 = 'cb:cf:2b:50:a7:61', '49:a2:25:1a:07:4a' | ||
|
||
request = Ether(src=mac_1, dst=mac_2) | ||
reply = Ether(src=mac_2, dst=mac_1) | ||
|
||
ip_1, ip_2 = '1.1.1.1', '2.2.2.2' | ||
ipv6_1, ipv6_2 = 'fd01::1.1.1.1', 'fd02::2.2.2.2' | ||
|
||
payload = Raw(b'#JSb[abR^79aV(kDAN,(C\n\\A+p V+MF7\rd9Z&&9D31.;T%\x0ct.#') | ||
icmp_echo = ICMP(type=8, seq=1) / payload | ||
icmp_reply = ICMP(type=0, seq=1) / payload | ||
|
||
test = 'flow-pkt-recursion' | ||
|
||
middleware_pcap = Path.cwd() / f'{test}-middleware-included' / 'test.pcap' | ||
with PcapWriter(str(middleware_pcap)) as pcap: | ||
# Flow of IPv6 tunneled packets in both directions | ||
pcap.write(request / IPv6(src=ipv6_1, dst=ipv6_2) / IP(src=ip_1, dst=ip_2) / icmp_echo) | ||
pcap.write(reply / IPv6(src=ipv6_2, dst=ipv6_1) / IP(src=ip_2, dst=ip_1) / icmp_reply) | ||
|
||
terminated_pcap = Path.cwd() / f'{test}-terminated-included' / 'test.pcap' | ||
with PcapWriter(str(terminated_pcap)) as pcap: | ||
# Flow of tunnel terminated on Suricata device, echo originates | ||
# from Suricata device | ||
pcap.write(request / IP(src=ip_1, dst=ip_2) / icmp_echo) | ||
pcap.write(reply / IPv6(src=ipv6_2, dst=ipv6_1) / IP(src=ip_2, dst=ip_1) / icmp_reply) | ||
|
||
# Flow of tunnel terminated on Suricata device, reply originates | ||
# from Suricata device | ||
pcap.write(reply / IPv6(src=ipv6_2, dst=ipv6_1) / IP(src=ip_2, dst=ip_1) / icmp_echo) | ||
pcap.write(request / IP(src=ip_1, dst=ip_2) / icmp_reply) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 :-)