rust-lang / libs-team

The home of the library team
Apache License 2.0
127 stars 19 forks source link

Parse IPv6 zone identifiers #476

Open famfo opened 2 weeks ago

famfo commented 2 weeks ago

Proposal

Extend the Ipv6Addr struct with support for zone identifiers (interface indexes and interface names) including string parsing.

Problem statement

Ipv6 addresses have zone identifiers (<address>%<zone_id>) for e.g. link-local and multicast addresses as per RFC4007. This is currently not supported by the core and std implementations of an Ipv6Addr.

Motivating examples or use cases

Use cases include addressing multicast IP addresses on specific interfaces for e.g. wake on LAN when the default route does not exit into the local network where devices are located or neighbor discovery over multicast on specific interfaces for dynamic routing protocols.

Other expected uses actively break when trying to parse a file that contains an Ipv6 address scoped using an interface name (e.g. hickory DNS and /etc/resolv.conf containing nameserver fe80::2a0:57ff:fe6f:42ee%wlp3s0).

Solution sketch

Alternatives

Links and related work

https://datatracker.ietf.org/doc/html/rfc3513 https://datatracker.ietf.org/doc/html/rfc4007 https://datatracker.ietf.org/doc/html/rfc3493 https://github.com/rust-lang/rust/issues/65976

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

scottmcm commented 2 weeks ago

Ipv6Addr is Copy -- how do you propose to store this additional string?

I didn't see any obvious length restriction in the RFC, and for a type in core it can't have a String.

Or were you thinking with

An implementation SHOULD support at least numerical indices that are non-negative decimal integers as .

that it could just be an unsigned integer of some kind? Though your example doesn't work with that.

But even then, it no longer being just 128-bit feels like it'd be a surprise to a bunch of people, and potentially bad.

TL/DR it feels like this should be a different type, not part of Ipv6Addr.

kennytm commented 2 weeks ago

I think the <core::net::Ipv6Addr>::from_str() impl should remain platform-agnostic and support just the numerical form fe80::abcd:1234%2. It is similar to that you can't parse 127.0.0.1:ssh into a SocketAddr even if you know it is port 22 through getservbyname.

ToSocketAddrs should be extended to recognize if_nametoindex on Linux since it already does DNS resolution.


@scottmcm if we follow C's sockaddr_in6 we only need to add an extra u32 field to represent the sin6_scope_id. The type remains Copy (actually the scope_id already existed in SocketAddrV6 and SocketAddrV6 is certainly Copy, this ACP wants to move the field from SocketAddrV6 directly into Ipv6Addr). The Linux device name "wlp3s0" is translated into the device index like 3 through if_nametoindex.

famfo commented 2 weeks ago

@scottmcm

that it could just be an unsigned integer of some kind? Though your example doesn't work with that.

Using an unsigned integer would work if converting interface names to interface indexes as @kennytm already noted.

But even then, it no longer being just 128-bit feels like it'd be a surprise to a bunch of people, and potentially bad.

TL/DR it feels like this should be a different type, not part of Ipv6Addr.

A scope identifier is part of an IPv6 address and I feel it more surprising that valid IPv6 addresses are not properly parsed. I don't see how a different type would solve the issue properly.


@kennytm

I think the ::from_str() impl should remain platform-agnostic and support just the numerical form fe80::abcd:1234%2. It is similar to that you can't parse 127.0.0.1:ssh into a SocketAddr even if you know it is port 22 through getservbyname.

Parsing interface names in core doesn't really make sense, that's why I proposed making the parser extendable for std (although I don't think getservbyname and if_nametoindex are a fair comparison).

kennytm commented 2 weeks ago

Parsing interface names in core doesn't really make sense, that's why I proposed making the parser extendable for std

I don't think it makes sense to produce a different answer for "fe80::1234%eth0".parse::<Ipv6Addr>() depending on whether the program is #![no_std].

famfo commented 2 weeks ago

That is a fair argument. I think this could be solved with a trait, similar to ToSocketAddrs, but that sadly doesn't give backwards comparability.

kennytm commented 2 weeks ago

Oh also, while the zone ID is required to disambiguate non-global IPv6 address, it is never sent on the wire. An IPv6 packet does not contain the scope_id at all.

So I think it does make sense that core::net::Ipv6Addr corresponds just to C's struct in6_addr (which is an unsigned char[16]), and core::net::SocketAddrV6 is the entire struct sockaddr_in6 where the scope_id lives.

scottmcm commented 2 weeks ago

I feel it more surprising that valid IPv6 addresses are not properly parsed

I think there's now multiple definitions of "address" and having those in different types would make sense. Because if you look at what an IPv6 packet header calls an address, there's no zone identifier. If I look at a X-Forwarded-For header, passing along a %3 because the proxy happened to have wan on its 3rd interface is irrelevant and not something I would want included.

I think that even if the type does get the field, the back compat you mention still probably often won't happen, as the docs say "IPv6 addresses are defined as 128-bit integers" and it wouldn't surprise me at all to have people passing them around via .octets()/from_octets, and thus they'd silently drop a zone anyway.

EDIT: maybe @kennytm just said this better as we typed at the same time.

famfo commented 2 weeks ago

Oh also, while the zone ID is required to disambiguate non-global IPv6 address, it is never sent on the wire. An IPv6 packet does not contain the scope_id at all.

Not send on the wire but parsed before by e.g. the Linux kernel to determine on which interface to send it out.

If I look at a X-Forwarded-For header, passing along a %3 because the proxy happened to have wan on its 3rd interface is irrelevant and not something I would want included.

Bad example in my opinion, if the peer is a link-local from the 2. interface it is a very important information. The zone identifier is explicitly passed with link-locals and multicast addresses, otherwise it is implicitly 0.

If proper Ipv6 parsing is only added to SockAddressV6 passing only IP addresses without a port is not possible e.g. for user input to a program and Ipv6 addresses have to be passed as [fe80::1%eth0]:0

kennytm commented 2 weeks ago

If proper Ipv6 parsing is only added to SockAddressV6 passing only IP addresses without a port is not possible e.g. for user input to a program and Ipv6 addresses have to be passed as [fe80::1%eth0]:0

No this works:

use std::net::ToSocketAddrs as _;

fn main() {
    let socket_addr = ("fe80::1%3", 0).to_socket_addrs().unwrap().next().unwrap();
    dbg!(socket_addr);
    // socket_addr = [fe80::1%3]:0
}