Support IPv6 in in_private_net function, reduce noise on errors #157
Conversation
|
/rebase |
extended the ip subnet check to ignore all IPv6 addresses be returning "false" for all of them, even unique local addresses subnet check is not faster only non-IP addresses will lead to a log message now, IPv6 checks will be silent fixes #156 fixes Graylog2/graylog2-server#4624 related to #33
| if(!InetAddresses.isInetAddress(ip)) { | ||
| InetAddress inetAddress = InetAddresses.forString(ip); | ||
| if (inetAddress instanceof Inet6Address) { | ||
| // we don't deal with IPv6 unique local addresses currently. | ||
| return false; |
There was a problem hiding this comment.
I'm not sure we should be doing this, since it changes the behavior of the lookup function.
If we can't or won't answer the question if a v6 address is private, we shouldn't return a boolean value.
There was a problem hiding this comment.
How about we return a boxed Boolean here, and keep returning null for v6 addresses?
There was a problem hiding this comment.
null could be an option, but I'd like to avoid doing it if possible.
An alternative is to use https://www.rfc-editor.org/rfc/rfc4193.txt ranges (FC00::/7), which I think is technically correct, but probably rarely used in practice.
There was a problem hiding this comment.
that's what isSiteLocalAddress would be doing for v6. But I'm wondering if it wouldn't be more practical to also include link local addresses fe80::/10 as well.
There was a problem hiding this comment.
I think link-local addresses are out of scope for this function: the original version also didn't consider link-local IPv4 address, because they shouldn't be routed anyway.
For the purposes of this function, I'll now update the PR to include the unique local addresses in IPv6 and properly return true for those.
suppress harmless API stability warning
kroepke
changed the title
|
Updated the description and code. Please have another look. |
kroepke
changed the title
|
@kroepke do we still want a 3.3 backport of this? |
|
I guess it doesn't hurt to backport it, in case we do another release, yes. |
|
ok, i'll update my existing backport pr then |
The
in_private_netfunction now supports unique local addresses in IPv6 (entireFC00::/7subnet)When something else than an IP address is passed in, the function is now less chatty and does not log the stacktrace anymore. For IP addresses it should never log anything.
fixes #156
fixes Graylog2/graylog2-server#4624
related to #33