suricata icon indicating copy to clipboard operation
suricata copied to clipboard

flow: optionally use pkt recursion for hash

Open coledishington opened this issue 1 year ago • 3 comments

If a Suricata inline IPS device is routing traffic over a non-encrypted tunnel, like IPv6 tunnels, packets in a flow will be dropped and not be matched. e.g.

The following example is a Suricata inline IPS with an IPv6 tunnel:

request:  IPv4]ICMP] -> |IPS| -> IPv6]IPv4]ICMP]
reply:               <- |IPS| <- IPv6]IPv4]ICMP]

Both the IPv4 request and IPv6 reply will be seen by Suricata on ingress. The flows will not be matched due to flow recursion level.

Optionally use pkt recursion level in flow hash. Excluding recursion level in flow hash allows matching of packet flows and defrag on an inline IPS Suricata scenario where the IPS device is a tunnel terminator.

Bug: #6260

Make sure these boxes are signed before submitting your Pull Request -- thank you.

  • [✓] I have read the contributing guide lines at https://docs.suricata.io/en/latest/devguide/codebase/contributing/contribution-process.html
  • [✓] I have signed the Open Information Security Foundation contribution agreement at https://suricata.io/about/contribution-agreement/
  • [✓] I have updated the user guide (in doc/userguide/) to reflect the changes made (if applicable)

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6260

Describe changes: Add config (decoder.recursion-level.use-for-tracking) to control the use of packet recursion level in flow matching. This is to support scenarios where Suricata inline IPS device is routing traffic over a non-encrypted tunnel, like IPv6 tunnels, packets in a flow will be dropped and not be matched. e.g.

The following example is a Suricata inline IPS with an IPv6 tunnel: request: IPv4]ICMP] -> |IPS| -> IPv6]IPv4]ICMP] reply: <- |IPS| <- IPv6]IPv4]ICMP]

Both the IPv4 request and IPv6 reply will be seen by Suricata on ingress. The flows will not be matched due to flow recursion level.

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the pull request number.

Alternatively, SV_BRANCH may also be a link to an OISF/suricata-verify pull-request.

https://github.com/OISF/suricata-verify/pull/1597

SV_BRANCH=pr/1597

coledishington avatar Jan 21 '24 21:01 coledishington

Codecov Report

Attention: Patch coverage is 73.91304% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 82.20%. Comparing base (3cb7112) to head (21b6fc9). Report is 579 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10214      +/-   ##
==========================================
+ Coverage   82.18%   82.20%   +0.01%     
==========================================
  Files         977      977              
  Lines      271894   271902       +8     
==========================================
+ Hits       223465   223513      +48     
+ Misses      48429    48389      -40     
Flag Coverage Δ
fuzzcorpus 63.22% <65.21%> (+0.23%) :arrow_up:
suricata-verify 61.48% <73.91%> (-0.01%) :arrow_down:
unittests 62.80% <26.08%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Jan 21 '24 21:01 codecov[bot]

Thanks for the work

CI : ✅ Code : cool Commits segmentation : ok Commit messages : ok, (I would say Ticket rather than Bug as it is a feature) Git ID set : looks fine for me CLA : you already contributed Doc update : Could you please add some doc ? cc @jufajardini where should it be put ? Redmine ticket : ok Rustfmt : not needed Tests : ok Dependencies added: none

Thanks, @jufajardini do you have any suggestions where I should add some documentation?

coledishington avatar Jun 13 '24 01:06 coledishington

Thanks for the work CI : ✅ Code : cool Commits segmentation : ok Commit messages : ok, (I would say Ticket rather than Bug as it is a feature) Git ID set : looks fine for me CLA : you already contributed Doc update : Could you please add some doc ? cc @jufajardini where should it be put ? Redmine ticket : ok Rustfmt : not needed Tests : ok Dependencies added: none

Thanks, @jufajardini do you have any suggestions where I should add some documentation?

Considering we're adding a configuration option related to decoding, my suggestion is https://docs.suricata.io/en/latest/configuration/suricata-yaml.html#decoder Our docs for configuration usually explain what is the configuration option and possible values and outcomes, and can also add background explanation on the reason for that option, when that makes sense. :)

jufajardini avatar Jun 25 '24 20:06 jufajardini