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

std::net::IpAddr: is_loopback failing on ipv4-in-ipv6 addresses #69772

Open
gh0st42 opened this issue Mar 6, 2020 · 5 comments
Open

std::net::IpAddr: is_loopback failing on ipv4-in-ipv6 addresses #69772

gh0st42 opened this issue Mar 6, 2020 · 5 comments
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@gh0st42
Copy link

gh0st42 commented Mar 6, 2020

Summary:

The issue arises when trying to determine if an IpAddr is coming from localhost in a mixed IPv4/IPV6 environment.
The is_loopback function should return true for loopback IPs such as 127.0.0.1 or [::1]. This fails if a socket is bound to [::] which responds to IPv4 as well as IPv6. Here, IPv4 is automatically wrapped in IPv6 and 127.0.0.1 is becoming [::ffff:127.0.0.1] which is not recognized as a loopback address.

Detailed story

If I bind a server to 0.0.0.0 or [::1] and telnet/curl from localhost, I can easily tell whether the connection came from a local user or not by using the is_loopback call on IpAddr, Ipv4Addr or Ipv6Addr.

Once I bind my server to [::] to work on IPv4 AND IPv6 at the same time and then connect to it via v4 to 127.0.0.1 the is_loopback call returns false.
I then have to manually try conversion of the Ipv6Addr into an Ipv4Addr (using to_ipv4) and perform a second check with is_loopback.

In my opinion, this behavior should either be clearly documented in the standard library or better yet should happen automatically (at least in IpAddr) since an ipv6 encapsulated ipv4 loopback address is still a perfectly valid loopback address.

The current documentation in Ipv6Addr states that it is a check for [::1] but a clear statement that IPv4 in IPv6 loopback addresses are not covered might help. I also guess that having the current minimal checks in both variants (v4 and v6) make sense to keep but the general is_loopback in IpAddr itself could provide the convenient conversion as it covers v4 and v6 anyways.

Example Code

use std::net::{Ipv4Addr, Ipv6Addr};

fn main() {
    let ipv4 = Ipv4Addr::new(127, 0, 0, 1); // regular 127.0.0.1
    let ipv6 = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1); // regular [::1] 
    let v4_in_v6 = Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0x7f00, 0x1); // if binding socket to [::] and connecting from ipv4 localhost
    
    println!("{} is loopback? {} ", ipv4, ipv4.is_loopback());
    println!("{} is loopback? {} ", ipv6, ipv6.is_loopback());
    println!("{} is loopback? {} ", v4_in_v6, v4_in_v6.is_loopback());
    println!("{} is loopback? {} ", v4_in_v6, v4_in_v6.to_ipv4().unwrap().is_loopback());
}

(Playground)

Output:

127.0.0.1 is loopback? true 
::1 is loopback? true 
::ffff:127.0.0.1 is loopback? false 
::ffff:127.0.0.1 is loopback? true 

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 6, 2020
@the8472
Copy link
Member

the8472 commented Mar 8, 2020

This isn't specific to is_loopback. Pretty much all of the helper methods make ipv6-specific checks.

CC #27709

@KodrAus
Copy link
Contributor

KodrAus commented Mar 11, 2020

This case seems like a bug to me, and I think both Ipv6Addr and IpAddr should correctly identify the ipv4 loopback in ipv6 as the loopback address.

@KodrAus
Copy link
Contributor

KodrAus commented Mar 11, 2020

As a datapoint, .NET does identify ::ffff:127.0.0.1 as loopback:

var addr = IPAddress.Parse("::ffff:127.0.0.1");

Assert(IPAddress.IsLoopback(addr));

@sminf
Copy link

sminf commented Dec 2, 2020

As a datapoint, .NET does identify ::ffff:127.0.0.1 as loopback:

var addr = IPAddress.Parse("::ffff:127.0.0.1");

Assert(IPAddress.IsLoopback(addr));

Same as Go. Playground

package main

import (
	"fmt"
	"net"
)

func main() {
	fmt.Println(net.ParseIP(`::ffff:127.0.0.1`).IsLoopback())
}

Golang check IP whether is IPv4 first, and not only .IsLoopback() do that, other functions on net.IP will do the same thing.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 3, 2021
Commit to not supporting IPv4-in-IPv6 addresses

Stabilization of the `ip` feature has for a long time been blocked on the question of whether Rust should support handling "IPv4-in-IPv6" addresses: should the various `Ipv6Address` property methods take IPv4-mapped or IPv4-compatible addresses into account. See also the IPv4-in-IPv6 Address Support issue rust-lang#85609 and rust-lang#69772 which originally asked the question.

# Overview

In the recent PR rust-lang#85655 I proposed changing `is_loopback` to take IPv4-mapped addresses into account, so `::ffff:127.0.0.1` would be recognized as a looback address. However, due to the points that came up in that PR, I alternatively propose the following: Keeping the current behaviour and commit to not assigning any special meaning for IPv4-in-IPv6 addresses, other than what the standards prescribe. This would apply to the stable method `is_loopback`, but also to currently unstable methods like `is_global` and `is_documentation` and any future methods. This is implemented in this PR as a change in documentation, specifically the following section:

> Both types of addresses are not assigned any special meaning by this implementation, other than what the relevant standards prescribe. This means that an address like `::ffff:127.0.0.1`, while representing an IPv4 loopback address, is not itself an IPv6 loopback address; only `::1` is. To handle these so called "IPv4-in-IPv6" addresses, they have to first be converted to their canonical IPv4 address.

# Discussion

In the discussion for or against supporting IPv4-in-IPv6 addresses the question what would be least surprising for users of other languages has come up several times. At first it seemed most big other languages supported IPv4-in-IPv6 addresses (or at least considered `::ffff:127.0.0.1` a loopback address). However after further investigation it appears that supporting IPv4-in-IPv6 addresses comes down to how a language represents addresses. .Net and Go do not have a separate type for IPv4 or IPv6 addresses, and do consider `::ffff:127.0.0.1` a loopback address. Java and Python, which do have separate types, do not consider `::ffff:127.0.0.1` a loopback address. Seeing as Rust has the separate `Ipv6Addr` type, it would make sense to also not support IPv4-in-IPv6 addresses. Note that this focuses on IPv4-mapped addresses, no other language handles IPv4-compatible addresses.

Another issue that was raised is how useful supporting these IPv4-in-IPv6 addresses would be in practice. Again with the example of `::ffff:127.0.0.1`, considering it a loopback address isn't too useful as to use it with most of the socket APIs it has to be converted to an IPv4 address anyway. From that perspective it would be better to instead provide better ways for doing this conversion like stabilizing `to_ipv4_mapped` or introducing a `to_canonical` method.

A point in favour of not supporting IPv4-in-IPv6 addresses is that that is the behaviour Rust has always had, and that supporting it would require changing already stable functions like `is_loopback`. This also keeps the documentation of these functions simpler, as we only have to refer to the relevant definitions in the IPv6 specification.

# Decision

To make progress on the `ip` feature, a decision needs to be made on whether or not to support IPv4-in-IPv6 addresses.
There are several options:

- Keep the current implementation and commit to never supporting IPv4-in-IPv6 addresses (accept this PR).
- Support IPv4-in-IPv6 addresses in some/all `IPv6Addr` methods (accept PR rust-lang#85655).
- Keep the current implementation and but not commit to anything yet (reject both this PR and PR rust-lang#85655), this entire issue will however come up again in the stabilization of several methods under the `ip` feature.

There are more options, like supporting IPv4-in-IPv6 addresses in `IpAddr` methods instead, but to my knowledge those haven't been seriously argued for by anyone.

There is currently an FCP ongoing on PR rust-lang#85655. I would ask the libs team for an alternative FCP on this PR as well, which if completed means the rejection of PR rust-lang#85655, and the decision to commit to not supporting IPv4-in-IPv6 addresses.

If anyone feels there is not enough evidence yet to make the decision for or against supporting IPv4-in-IPv6 addresses, let me know and I'll do whatever I can to resolve it.
@jstasiak
Copy link
Contributor

I think with #86335 in place this can be closed as working as expected?

paravoid added a commit to paravoid/cpython that referenced this issue Apr 5, 2024
While properties like is_private account for IPv4-mapped IPv6 addresses,
such as for example:

    >>> ipaddress.ip_address("192.168.0.1").is_private
    True
    >>> ipaddress.ip_address("::ffff:192.168.0.1").is_private
    True
...the same doesn't currently apply to the is_loopback property:
    >>> ipaddress.ip_address("127.0.0.1").is_loopback
    True
    >>> ipaddress.ip_address("::ffff:127.0.0.1").is_loopback
    False

At minimum, this inconsistency between different properties is
counter-intuitive. Moreover, ::ffff:127.0.0.0/112 is for all intents and
purposes a loopback address, and should be treated as such.

For the record, this will now match the behavior of other languages such
as Rust, Go and .NET, cf. rust-lang/rust#69772.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants