net: add IP.IsPrivate (RFC 1918 & RFC 4193)

[aaranmcguire]

The net package seems to have many helpers to report what an IP is. e.g:

• IsLoopback()
• IsMulticast()
• IsInterfaceLocalMulticast()

However there are no helpers to report if a IP address is in the private ranges (RFC 1918 & RFC 4193).

If no one has any objections I can make a method in net/ip.go named IsPrivate() that will check for the ranges listed in both RFCs using the same syntax the other helpers use.

[rsc]

How many more kinds of groups are there besides loopback, multicast, private, etc? That is, what else might we be missing? I'd like to understand if this is the last one or just the next on a very long sequence. It seems OK if we're almost done.

[rsc]

/cc @mikioh

[bitfield]

One problem with an isPrivate() method is that those private ranges only apply on the internet. Lots of code runs inside cloud VPCs and other private networks which use the whole IPv4 address space, so what would be a public network address on the internet is actually private.

[rsc]

ping @mikioh, @aaranmcguire: roughly speaking, are there 2 more of these IP address classes that we need to add, or 20 more?

[aaranmcguire]

I don't know any more classes of addresses, however I'm not a network person so there may be more I'm unaware of.

[rsc]

Since we think there are not many more of these, this seems OK to do.

IPv4 calls these Private; IPv6 calls these "Unique Local" at least in RFC 4193. One function should probably do both. Should it be IsPrivate or IsLocal?

[mikioh]

Will take a look, sorry my laptop is still broken (and my fingertips are not good for touch screens.)

[mikioh]

One roadblock is that we still have no concrete consensus on how to deal with IPv4 addresses including IPv4-IPv4(-IPv4) translation addresses and IPv6 addresses including IPv4-IPv6 transition addresses. For example, I'm still not sure whether reporting ::ffff:192.168.0.1 or <user-defined-translation-prefix>:c000:221:: as an IPv4 address is a good idea. A compromise might be:

// IsTranslator reports whether ip is IPv4 or IPv6 address translation
// available.
// When prefixes is nil, it deals with ip as an IPv4 address with the
// well-known prefix for IPv4-embedded IPv6 address defined in RFC
// 6052, an IPv4-mapped IPv6 address defined in RFC 4291, or an IPv4
// address in shared address space defined in RFC 6598.
method (IP) IsTranslator(prefixes []*IPNet) bool

// IsLocalUnicast reports whether ip is in the IPv4 private unicast
// address blocks, unique local IPv6 unicast address blocks, ...
method (IP) IsLocalUnicast() bool or
method (IP) IsLocalUnicast(prefixes []*IPNet) bool // including IsTranslator

If you think the above methods look confusing, that might be a clear signal that it's better to have an external package, like #11772.

The net package seems to have many helpers to report what an IP is: . e.g:

Yes, and the helper methods have an essential characteristic: each helper follows basic IP addressing architecture and never jumps across the line of complicated operational stuff; e.g., geographic things, translation things, and IANA-allocated special-purpose (e.g., for documentation, for AS112/honeypot) things.

IP address classes that we need to add, or 20 more?

People in some organization operating IPv4 private address blocks might want a method reporting organization-specific private/local IP addresses. I personally hope to solve this sort of issues with #18804.

[rsc]

It should be the case that net.IsPrivate(ip) == net.IsPrivate(ip.To16()) always, and net.IsPrivate(ip) == net.IsPrivate(ip.To4()) when ip.To4() != nil. We always treat IPv4-formatted and IPv4-in-IPv6-formatted addresss as equivalent in package net. That answers ::ffff:192.168.0.1.

I'm not familiar with <user-defined-translation-prefix>:c000:221:: but it seems like that would be outside the scope of IsPrivate. If you defined the prefix, it should be easy enough to write code to check for it (bytes.HasPrefix).

Does anyone object to adding net.IsPrivate?

[mikioh]

We always treat IPv4-formatted and IPv4-in-IPv6-formatted addresss

Are you talking about IPv4-mapped IPv6 address? Or something you or people in the Go team defined one? If the former, it's better to document why the prefix for IPv4-mapped IPv6 address ::ffff/96 is special and other translation addresses like IPv4-embedded addresses are ignored. If the latter, I have no clue.

Does anyone object to adding net.IsPrivate?

It should be IsLocalUnicast if it also covers IPv6 ULA.

[rsc]

It should be IsLocalUnicast if it also covers IPv6 ULA.

Why? We can easily include the "Unique Local Address" and "ULA" and "RFC 4193" in the docs, but is "private" an inaccurate description? These are addresses that don't route out to the internet, right? That's what private has always meant in V4.

[rsc]

It sounds like people are generally OK with IsPrivate, meaning either RFC 1918 (including with the standard IPv4-in-IPv6 embedding) or RFC 4193.

Let's stay away from IsTranslator and IsLocalUnicast for now.

[mikioh]

This is just a reply to @rsc for sharing the background.

Why? We can easily include the "Unique Local Address" and "ULA" and "RFC 4193" in the docs, but is "private" an inaccurate description?

Not so pedantic, two concerns:

1. the existing methods for IP address identification are based on addressing architecture, and the blocks described in RFC 1918 and RFC 4193 can be considered as globally (unique, sometimes non-unique) local unicast addresses which are detached from global routing operation and are subtypes of global unicast addresses as described in RFC 4291 (please don't say it's just for IPv6 instantly, IPv6 cannot ignore the transition to IPv6 from IPv4 as its nature). I'm not sure whether introducing APIs from the operational point of view is a good idea because I don't see any difference between this proposal and a proposal like net: add IsAmazonAZ, IsGoogleExternal and IsAzureElastic to IP.
2. privately use is one of the applications for the address blocks. I just wonder how do API users feel from !IP.IsPrivate().

[mikioh]

generally OK with IsPrivate

I'd recommend renaming IsPrivate with IsPrivateUnicast. IsPrivate looks a bit vague and we already have IsGlobalUnicast.

[gopherbot]

Change https://golang.org/cl/162998 mentions this issue: net: add IP.IsLocal()

[aaranmcguire]

What's the status of this? Can it be merged now?

[odeke-em]

Kindly paging @MaxSem the author of CL https://go-review.googlesource.com/c/go/+/162998/, @mikioh left some comments on your CL sometime ago, please take a look. Thank you!

[MaxSem]

As I see it, there were disagreements about the proposed function name among project members. I'm happy to change it, but I'd appreciate clarity as to the correct name :)

[6543]

I'm going to create a Pull ...

[6543]

If I have to do something just tell me :)

-> #42793

[gopherbot]

Change https://golang.org/cl/272668 mentions this issue: net: add IP.IsPrivate

[jkamdjou]

would also love to see this merged 👍

[perillo]

I think the comment in the IP.IsPrivate method is wrong.

For the local IPv4 addresses, the comment cites RFC 4193, but this document only specifies local IPv6 addresses.

[odeke-em]

@perillo, you are correct, thank you; it was just a typo. I had highlighted the change in my comment https://go-review.googlesource.com/c/go/+/272668/7..11/src/net/ip.go#b135

But I missed it in the later revision. Please feel free to send a change and tag me and I’ll review it.

[perillo]

@odeke-em thanks to you for the contribution.

I have also noted another small issue. Unlike RFC 1918, RFC 4193 in section 3 does not say "The Internet Assigned Numbers Authority (IANA) has reserved ...". Instead in section 8 (IANA Considerations) it just says "The IANA has assigned the FC00::/7 prefix to "Unique Local Unicast".

See https://tools.ietf.org/html/rfc4193#section-3 and https://tools.ietf.org/html/rfc4193#section-8.

Thanks

[6543]

Did I miss something?!?

[6543]

@perillo sorry it was me geting these typos in :/

should I send a pull with your suggestions?

[odeke-em]

@6543 yup, please send a PR for them. Thank you @perillo for the report, thank you @6543 for all your work and looking forward to more interactions with you when you send that change.

[gopherbot]

Change https://golang.org/cl/309249 mentions this issue: net: reference the correct RFCs and sections for IP.IsPrivate