output/ipv6: Add configuration option for shortened IPv6 IP addresses #14433
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
jlucovsky
requested review from
a team,
jasonish,
jufajardini and
victorjulien
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14433 +/- ##
==========================================
- Coverage 84.20% 84.18% -0.03%
==========================================
Files 1013 1014 +1
Lines 262383 262420 +37
==========================================
- Hits 220936 220913 -23
- Misses 41447 41507 +60
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 28610 |
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Thanks for the work, fine enough for me, but some nits
CI : ✅
Code : good
Commits segmentation : I would squash but ok
Commit messages : good
Git ID set : looks fine for me
CLA : you already contributed
Doc update : looks nice
Redmine ticket : ok
Rustfmt :
Tests : some remarks on SV there but ok
Dependencies added: none
| pub unsafe extern "C" fn SCIPv6Shorten( | ||
| addr: *const c_uchar, // pointer to 16-byte IPv6 | ||
| out_buf: *mut c_char, // out buffer allocated by caller | ||
| out_len: usize // size of out buffer |
There was a problem hiding this comment.
Is there a MAX_IP6_ADDRESS size ? If so, could we use it a fixed size array with it ?
There was a problem hiding this comment.
Per RFC 8200, IP v6 address sizes are 128 bits.
A constant/define can be added -- do you think it'll aid readability?
There was a problem hiding this comment.
here I meant the max size of a ip6 as a string
|
|
||
| unsafe { | ||
| // get 16-byte IPv6 address | ||
| let bytes = std::slice::from_raw_parts(addr, 16); |
There was a problem hiding this comment.
could rename 16 to IP6_ADDR_SIZE or similar
| // Convert &[u8] → Ipv6Addr | ||
| let ipv6 = match <&[u8; 16]>::try_from(bytes) { | ||
| Ok(b) => Ipv6Addr::from(*b), | ||
| Err(_) => return 0, |
There was a problem hiding this comment.
It cannot fail, according to the documentation, as it will always generate a result based on the 16-byte bit pattern it's presented with
There was a problem hiding this comment.
Is there not a method that does not return a Result ?
|
|
||
| // Sufficient room? | ||
| if ipv6_str.len() + 1 > out_len { | ||
| return 0; |
There was a problem hiding this comment.
There was a problem hiding this comment.
for both success and failure
There was a problem hiding this comment.
Will modify to return 0 on error and the actual length on success.
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
SCIPv6Shorten always returns 0 for both failure and success
Guess this underlines lack of test or unreachable code...
I'll add some test cases. |
|
Continued in #14449 |
4 participants
Continuation of #14426
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-versionProvide 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=