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

Parse IPv6 zone identifiers #476

Open
famfo opened this issue Nov 5, 2024 · 9 comments
Open

Parse IPv6 zone identifiers #476

famfo opened this issue Nov 5, 2024 · 9 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@famfo
Copy link

famfo commented Nov 5, 2024

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

  • move Ipv6 address zone identifier from SocketAddrV6 to Ipv6Addr
  • add libc::if_nametoindex wrappers to platform specific APIs in std
  • add the ability to extend the core::net::parser Ipv6 parser with custom zone identifier parsers
  • extend the Ipv6 parser to resolve zone identifier by name in std

Alternatives

  • only adding interface name resolution to SocketAddrV6 in std
    • cons: this will not work when parsing IP addresses, only socket addresses

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
rust-lang/rust#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):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@famfo famfo added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Nov 5, 2024
@famfo famfo changed the title (My API Change Proposal) Parse IPv6 zone identifiers Nov 5, 2024
@scottmcm
Copy link
Member

scottmcm commented Nov 5, 2024

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 <zone_id>.

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
Copy link
Member

kennytm commented Nov 5, 2024

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
Copy link
Author

famfo commented Nov 5, 2024

@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 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.

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
Copy link
Member

kennytm commented Nov 5, 2024

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
Copy link
Author

famfo commented Nov 5, 2024

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
Copy link
Member

kennytm commented Nov 5, 2024

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
Copy link
Member

scottmcm commented Nov 5, 2024

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
Copy link
Author

famfo commented Nov 5, 2024

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
Copy link
Member

kennytm commented Nov 6, 2024

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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants