output/ipv6: Add configuration option to shorten IPv6 IP addresses#14458
output/ipv6: Add configuration option to shorten IPv6 IP addresses#14458
Conversation
Issue: 7399 Use shortened IPv6 addresses in all output when configured. IPv6 addresses are shortened per RFC5952 By default, IPv6 addresses are never shortened; set logging.ipv6-addr-shorten=yes to shorten. Added Rust utility function to create shortened IPv6 address.
Document the configuration variable logging.ipv6-addr-shorten Issue: 7399
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14458 +/- ##
==========================================
- Coverage 84.20% 84.17% -0.03%
==========================================
Files 1013 1014 +1
Lines 262383 262498 +115
==========================================
+ Hits 220936 220959 +23
- Misses 41447 41539 +92
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| let bytes = std::slice::from_raw_parts(addr, 16); | ||
|
|
||
| // Convert &[u8] → Ipv6Addr | ||
| let ipv6 = match <&[u8; 16]>::try_from(bytes) { |
There was a problem hiding this comment.
how might this fail? If we have 16 bytes we have a valid ipv6 address, right?
There was a problem hiding this comment.
As reported in #14433 (comment)
I think you should use a from method that is MSRV-compatible
There was a problem hiding this comment.
Can we unwrap this somehow then? Now the error check is just dead code AFAICS
| # Shorten IPv6 addresses per RFC 5952 before logging. The default is no | ||
| #ipv6-addr-shorten: no |
There was a problem hiding this comment.
This section logging typically has no effect on outputs. I wonder if this should be specified per eve logger?
There was a problem hiding this comment.
I went down that rabbit hole (per eve logger) but decided on this approach to catch the output of any IPv6 address anywhere.
Perhaps logging isn't the correct section but I feel this is a global Suricata config setting, not per-output.
There was a problem hiding this comment.
@jasonish Suggestions on how to specify the global config value?
There was a problem hiding this comment.
Unfortunately not. Perhaps at the top level? This is an issue with Eve output currently; there is no way to specify global things. A goal for 9 is to remove globals as well. I feel it more naturally belongs on the eve instance? Maybe the idea of a global section is not a bad idea?
There was a problem hiding this comment.
I'll start a global section with the IPv6 option being the first member.
|
Information: QA ran without warnings. Pipeline = 28672 |
| E.g., previously, ``ether_type`` values were logged in host order; an ethertype value of ``0xfbb7`` | ||
| (network order) was logged as `47099`` (``0xb7fb``). This ethertype value will be logged as ``64439``. | ||
|
|
||
| - The output format for IPv6 addresses can be configured. By default, they are presented in their |
There was a problem hiding this comment.
I don't think this belongs here unless we change the default.
There was a problem hiding this comment.
I'll remove mention of this from the upgrade unless you suggest changing the default value to "always shorten"
There was a problem hiding this comment.
@victorjulien Should the default be yes (shorten)?
catenacyber
left a comment
There was a problem hiding this comment.
Some changes were requested inline if I understand correctly
|
Continued in #14636 |
Continuation of #14449
Add a configuration option for outputting shortened IPv6 addresses per RFC-5952
The configuration option:
logging.ipv6-addr-shortenhas a default value ofno.When set to
yes, IPv6 addresses will be shortened everywhere they are output. E.g., the IPv6 addressfe80:0000:0000:0000:020c:29ff:faf2:ab42will be output asfe80::20c:29ff:faf2:ab42Link to ticket: https://redmine.openinfosecfoundation.org/issues/7399
Describe changes:
Updates:
min-versionrustfmtProvide values to any of the below to override the defaults.
link to the pull request in the respective
_BRANCHvariable.SV_REPO=
SV_BRANCH=OISF/suricata-verify#2789
SU_REPO=
SU_BRANCH=