Skip to content

net: add IP.IsLocal #30278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

net: add IP.IsLocal #30278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/net/ip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ func (ip IP) IsMulticast() bool {
return len(ip) == IPv6len && ip[0] == 0xff
}

// IsLocal reports whether `ip' is a local address, according to
// RFC 1918 (IPv4 addresses) and RFC 4193 (IPv6 addresses).
func (ip IP) IsLocal() bool {
if ip4 := ip.To4(); ip4 != nil {
// Local IPv4 addresses are defined in https://tools.ietf.org/html/rfc1918
return ip4[0] == 10 ||
(ip4[0] == 172 && ip4[1]&0xf0 == 16) ||
(ip4[0] == 192 && ip4[1] == 168)
Copy link
Contributor

@msoedov msoedov May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. IsLocal name is miss-leading. Did you mean IsPrivate? I would interpret IsLocal as a private address in the same network
  2. How to handle cases 0.0.0.0 and 127.0.0.1 and ::1?

Copy link

@cypres cypres Jun 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I also think IsPrivate would be a better name.
  2. We do at least have IsUnspecified() and IsLoopback() to cover those cases with IsLocal you can do IsLocal() || IsUnspecified() || IsLoopback() if you want to cover all.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking about private addresses - 100.64.0.0/10 is private, 198.18.0.0/15 is private too.

This implementation is true only for RFC 1918 (IPv4 addresses) and RFC 4193 (IPv6 addresses).

Copy link

@cypres cypres Jul 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but that doesn't mean it would still be useful too have a group for RFC 1918 and RFC 4193... We're using this to filter webhook calls, so people can't setup a webhook that calls a private address in our internal networks. The carrier grade nat scope 100.64.0.0/10 and/or the benchmark test net 198.18.0.0/15 is not really any concern for us, protecting internal networks is.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was not clear, my points are:

  • IsLocal is a bad name, i am with you on it.
  • IsPrivate is ok name, but changing the name brings new problem - current implementation includes only part of private ip address blocks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a second thought about it. IsPrivate wont do, because there are a lot of other ip block in the private scope.

IP blocks from rfc1918 fall into private category but they have special purpose. rfc1918 doesnt name them local tho. But the gist is - local communication (inside enterprise). So the grouping is ok, naming is a little bit miss-leading. Proof - our comments 😄

But IsLocal makes more sense then IsPrivate and this logical block shouldnt cover

How to handle cases 0.0.0.0 and 127.0.0.1 and ::1?

Only blocks from RFC 1918 (IPv4 addresses) and RFC 4193 (IPv6 addresses) should be there.

}
// Local IPv6 addresses are defined in https://tools.ietf.org/html/rfc4193
return len(ip) == IPv6len && ip[0]&0xfe == 0xfc
}

// IsInterfaceLocalMulticast reports whether ip is
// an interface-local multicast address.
func (ip IP) IsInterfaceLocalMulticast() bool {
Expand Down
9 changes: 9 additions & 0 deletions src/net/ip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,15 @@ var ipAddrScopeTests = []struct {
{IP.IsMulticast, IP{0xff, 0x05, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, true},
{IP.IsMulticast, IP{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, false},
{IP.IsMulticast, nil, false},
{IP.IsLocal, nil, false},
{IP.IsLocal, IPv4(10, 0, 0, 0), true},
{IP.IsLocal, IPv4(11, 0, 0, 0), false},
{IP.IsLocal, IPv4(172, 16, 0, 0), true},
{IP.IsLocal, IPv4(172, 32, 0, 0), false},
{IP.IsLocal, IPv4(192, 168, 0, 0), true},
{IP.IsLocal, IPv4(192, 169, 0, 0), false},
{IP.IsLocal, IP{0xfc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, true},
{IP.IsLocal, IP{0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, false},
{IP.IsInterfaceLocalMulticast, IPv4(224, 0, 0, 0), false},
{IP.IsInterfaceLocalMulticast, IPv4(0xff, 0x01, 0, 0), false},
{IP.IsInterfaceLocalMulticast, IPv6interfacelocalallnodes, true},
Expand Down