Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

proposal: get rid of Unmap and *Raw #64

Open
danderson opened this issue Dec 24, 2020 · 4 comments
Open

proposal: get rid of Unmap and *Raw #64

danderson opened this issue Dec 24, 2020 · 4 comments

Comments

@danderson
Copy link
Member

In a twitter thread (end of the thread: https://twitter.com/dave_universetf/status/1341977567658536960), the question came up of why handle IPv4-mapped IPv6 addresses specially. After much consideration, I can't identify any good use cases for unmapping these addresses automatically in the general case. I think we should remove Unmap, and make the *Raw constructors the default and only constructors.

In more detail: in theory, IPv4-mapped IPv6 addresses are a way to cram an IPv4 address into the IPv6 address space. Notionally, the benefit of this is that you can represent both internets with a single address type. However, in practice we still distinguish the two internets in our code, and require our callers to care (via Is4 et al.).

Per https://tools.ietf.org/html/rfc4038#section-4.2, the original purpose was to allow APIs and applications to wield v6 addresses only, and generate IPv4 packets further down in the network stack. In practice, this really hasn't happened, and the network stacks have remained dual-stack all the way up to the application layer.

Meanwhile, automatically unmapping introduces confusions: you might parse an IPv6 address, and find that Is4()==true. In the extreme, this could cause security issues if you're, say, writing a firewall. Alternatively, assuming that if you gave us an IPv6 address, you meant for it to be IPv6, offers a very consistent behavior.

Given that I'm failing to find a use for IPv4-mapped IPv6 at all, I think we should stop featuring it prominently in our API, and just remove it. If we were to support v4-in-v6 encoding, it would be much more useful to support the transition mechanisms that are actually used (but I also think we shouldn't put that in netaddr).

/cc @bradfitz @mdlayher would y'all miss IPv4-mapped IPv6 if it went away?

If we do this, one question is what the FromStd constructors should do, since they treat the v4-mapped 16 byte and the 4 byte representations of IPv4 interchangeably. I think for that case, given how people wield the stdlib, those constructors should always unmap (or maybe we keep offering a Raw constructor for the stdlib type only?). That way, IPs that would pass To4() in the stdlib become pure IPv4 in netaddr, and the rest become IPv6.

Past discussions of unmapping: #34

@mdlayher
Copy link
Member

would y'all miss IPv4-mapped IPv6 if it went away?

Not one bit! I've always found the stdlib behaviors annoying.

That way, IPs that would pass To4() in the stdlib become pure IPv4 in netaddr, and the rest become IPv6.

This seems like the correct choice to me.

@danderson
Copy link
Member Author

Pausing until after winter break, but once the uint64 representation change is merged, I may put together a draft PR to see what this would look like, and figure out what makes sense for the stdlib constructors.

@bradfitz
Copy link
Contributor

In addition to the standard library, a large motivation for the auto-unmapping was wgcfg.IP (a [16]byte IP type), but that's now gone.

In any case, if we keep the As16() [16]byte, it'd be nice to have a way to map back. Today that's IPFrom16. If we ditch As16, then what about As4? Or do you make As16 panic like As4 does if the family is wrong?

@danderson
Copy link
Member Author

I suppose we could keep .Map and .Unmap to support explicit translation from mapped to unmapped, for those few applications where it matters.

For As16... Hmm. I think I would make it panic unless explicitly Mapped first, just for consistency. Given that we have converters for the stdlib types, by the time you're using As4 or As16, you're doing something specific enough that I think it's okay to require you to care about the address family.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants