Skip to content
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.

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

net: Unable to reliably distinguish IPv4-mapped-IPv6 addresses from regular IPv4 addresses #37921

Open
andrewlouisx opened this issue Mar 18, 2020 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@andrewlouisx
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.13.8 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/alouis/Library/Caches/go-build"
GOENV="/Users/alouis/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/alouis/Documents/go_playground"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gf/xt8nljp959z9gyd_qdvr68gc0000gp/T/go-build940989106=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/gGY8VPwVm2J

What did you expect to see?

Some way to parse an IPv4 mapped IPv6 address (e.g. 0:0:0:0:0:ffff:ac15:0006 or ::ffff:172.21.0.6) and know that it is an IPv4 mapped IPv6 address.

The net package is bit prohibitive here, given that the ParseIP method is pretty opinionated. See here:

ip := net.ParseIP("::ffff:ac15:0006") //  -> 172.21.0.6 (the ip4 equivalent of ac15:0006)

len(ip) // -> 16, this is because internally, all IPs seem to be 16 byte net.IP{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xac, 0x15, 0x0, 0x6}

What did you see instead?

For any given IPv4-mapped-IPv6 address IP, all the IP parsing in the net package only indicate that the input is an IPv4 address.

This is also perhaps, further complicated by the internal representation of IPv4 addresses being 16 bytes, bearing the ::ffff prefix internally.

However, if the user is using the net package for IP sanitization/validation, not being able to tell the difference seems to be a shortcoming. In order to be implement this on my own, I had to copy/paste the parseIPv6 method from the net package, and do something like the following:

func isIPv4MappedIPv6Address(in string) bool {
	in = strings.Split(in, "/")[0]

	ip := parseIPv6(in)
	if ip == nil {
		// not an ipv6 address
		return false
	}

	if ip.To4() == nil {
		// couldn't map our ip6 to an ip4
		return false
	}

	return true
}

I would propose here to expose the parseIPv6 and parseIPv6 methods. Exposing the Parse helpers would help multiple use-cases I'm sure, without increasing the API footprint significantly of the net package


For more parsing fun related to IPv4-mapped-IPv6 addresses, here's another instance where things look weird: https://play.golang.org/p/lsVp472VG4E

	ip, network, _ := net.ParseCIDR("::ffff:ac15:0006/32")
	fmt.Printf("ip: %v\n", ip) // ip: 172.21.0.6
	fmt.Printf("network: %v\n", network) // network: ::/32

Since ip at this point is the 16 byte form, it does have the prefix, but then again any other IPv4 address would have as we see. However, we get an IPv6 network, which will leave the onus on the user to make sense of this result.

@andrewlouisx
Copy link
Author

cc @toothrot @mikioh @bradfitz @ianlancetaylor

This one seems to have fallen between the cracks and hasn't been triaged yet, tagging you folks for visibility.

@toothrot
Copy link
Contributor

toothrot commented Mar 20, 2020

I wouldn't say it fell through the cracks. Sometimes it takes us more than a couple days to triage issues when we're doing things like releasing Go 1.14.1! For the record, it was still in our triage queue.

Thanks for filing.

@toothrot toothrot added this to the Backlog milestone Mar 20, 2020
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 20, 2020
@toothrot
Copy link
Contributor

toothrot commented Mar 20, 2020

Would it be more accurate to do something like the guard close in To4() in your detection?

go/src/net/ip.go

Lines 189 to 200 in 20a838a

func (ip IP) To4() IP {
if len(ip) == IPv4len {
return ip
}
if len(ip) == IPv6len &&
isZeros(ip[0:10]) &&
ip[10] == 0xff &&
ip[11] == 0xff {
return ip[12:16]
}
return nil
}

@andrewlouisx
Copy link
Author

Hey @toothrot! Sorry for the late reply here :)

Unfortunately, this will not work. Since under the hood, whenever you call ParseIP on anything, we always end up with a 16 byte representation. If it is an IPv4 address, the first 8 bytes of the IP type result is still 0xffff!

Here's an example: https://play.golang.org/p/GlBb81pAPIB

You can see there, that even though we have an IPv4 string, the internal representation is still 16 bytes - and then the reason we still get a non-nil result there from the To4() is because we wind up in this condition:

go/src/net/ip.go

Lines 193 to 198 in 20a838a

if len(ip) == IPv6len &&
isZeros(ip[0:10]) &&
ip[10] == 0xff &&
ip[11] == 0xff {
return ip[12:16]
}

Does that makes sense? And now we wind up in a situation again where we can't distinguish after parsing whether we were given 127.0.0.1 or ::ffff:127.0.0.1

@bradfitz
Copy link
Contributor

bradfitz commented Apr 1, 2020

@andrewlouis93, I've addressed this issue in https://godoc.org/inet.af/netaddr (https://github.com/inetaf/netaddr). Perhaps that'll work for you in the meantime?

@andrewlouisx
Copy link
Author

Just had a look at this, I think this will work great. Looking forward to seeing that package evolve, thanks @bradfitz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants