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

ping: Add support for IPv4-mapped IPv6 address format #356

Closed
wants to merge 1 commit into from

Conversation

pevik
Copy link
Contributor

@pevik pevik commented Oct 18, 2024

IP addresses with ::ffff: prefix are IPv6 addresses which are mapped to IPv4. Therefore ping resulting IPv4 address.

$ ./src/fping -c1 ::ffff:127.0.0.1
IPv4-Mapped-in-IPv6 address, using IPv4 127.0.0.1
 [<- 127.0.0.1]127.0.0.1        : [0], 64 bytes, 0.066 ms (0.066 avg, 0% loss)

Based on iputils feature iputils/iputils@8ed7ffc, which I sent also to busybox https://lists.busybox.net/pipermail/busybox/2024-October/090974.html.

NOTE: CI failure is unrelated, addressed in #355. Once you merge fix I'll rebase to get CI green.

IP addresses with ::ffff: prefix are IPv6 addresses which are mapped to
IPv4. Therefore ping resulting IPv4 address.

    $ ./src/fping -c1 ::ffff:127.0.0.1
    IPv4-Mapped-in-IPv6 address, using IPv4 127.0.0.1
     [<- 127.0.0.1]127.0.0.1        : [0], 64 bytes, 0.066 ms (0.066 avg, 0% loss)

Signed-off-by: Petr Vorel <[email protected]>
@coveralls
Copy link

Coverage Status

coverage: 87.796% (+0.2%) from 87.614%
when pulling e6e9fbe on pevik:IPv4-Mapped-in-IPv6
into b32852c on schweikert:develop.

@gsnw-sebast
Copy link
Collaborator

gsnw-sebast commented Oct 20, 2024

Unfortunately, I can't test it directly at the moment.

But the first time I looked over it, I noticed that the output always appears when this constellation occurs.
No matter whether the variable verbose_flag = 0 or quiet_flag = 1.

I find this somewhat unfavorable because it permanently changes the output.
I would recommend setting at least verbose_flag = 1 for printf.

@pevik
Copy link
Contributor Author

pevik commented Oct 20, 2024

Thanks for having a look, no rush with the PR.

Sure, I'll modify it. Also code contains bug, which needs to be fixed: iputils/iputils@988cadd. I'll have look into it next week.

@pevik
Copy link
Contributor Author

pevik commented Oct 20, 2024

Given the following discussion in the busybox mailing list it might not be a good idea to implement this feature. Feel free to contribute there to the discussion (ML requires a subscription in https://lists.busybox.net/mailman/listinfo/busybox).

https://lists.busybox.net/pipermail/busybox/2024-October/090975.html

I can understand why anyone would think this translation is normal, because
the socket API normally unmaps IPv4-mapped IPv6 addresses for compatibility:

https://www.rfc-editor.org/rfc/rfc3493.html#section-3.7

Applications may use AF_INET6 sockets to open TCP connections to IPv4
nodes, or send UDP packets to IPv4 nodes, by simply encoding the
destination's IPv4 address as an IPv4-mapped IPv6 address, and
passing that address, within a sockaddr_in6 structure, in the
connect() or sendto() call.

But this is not the case with ping because ping uses IPv6 raw sockets,
and that seems to stop sendto() from performing the convenience unmapping.

Sadly, this patch breaks the case where an advanced user wants ping
to send a packet with an IPv4-mapped IPv6 addresses. For example, you
might be testing or diagosing firewall rules in that subnet.

Additionally, I can't see any combination of -4 or -6 option flags that
could be used as a guard to make the unmapping behaviour expected.

@auerswal
Copy link
Collaborator

Since the current pull request contains at least one bug (see #356 (comment)) and there seems to be some controversy about this functionality on the BusyBox mailing list, I'll convert this PR to a "draft" for now. @pevik, feel free to change the state back after addressing the bug you mentioned.

@auerswal
Copy link
Collaborator

@pevik, I am curious, what is your use case for this functionality?

@auerswal
Copy link
Collaborator

But the first time I looked over it, I noticed that the output always appears when this constellation occurs. No matter whether the variable verbose_flag = 0 or quiet_flag = 1.

I find this somewhat unfavorable because it permanently changes the output. I would recommend setting at least verbose_flag = 1 for printf.

After a first glance at the added test (thanks, BTW!), I also tend to think this extra message might be too verbose. Anyway, I still need to read the BusyBox mailing list thread and think about use cases.

@auerswal auerswal marked this pull request as draft October 20, 2024 17:09
@auerswal
Copy link
Collaborator

@pevik, I am curious, what is your use case for this functionality?

This is described on the BusyBox mailing list: https://lists.busybox.net/pipermail/busybox/2024-October/090979.html

@hmh
Copy link
Contributor

hmh commented Oct 21, 2024

I understand the point of fping being able to bypass the appropriate meaning of ::ffff:ipv4, since it is a low-level network testing tool: it makes complete sense that one might want to use fping to generate invalid IPv6 packets where the source and/or destination addresses are in the ::ffff:0/96 range, and validate that they are correctly dropped as soon as possible by the network path.

However IP addresses on the ::ffff:ipv4/96 range are IPv4. One could say that address range was "namespaced away" from IPv6 by the current IPv6 standards. This whole thing is already confusing enough to the non-initiated, let's not make that confusion pit any worse, please. On the grounds of the principle-of-least-surprise, I too would prefer that one should have to explicitly request the non-standard behavior of being able to create IPv6 packets with addresses in the IPv4-mapped range (which are invalid IPv6 packets).

I think it is fine (if somewhat annoying) to actually require an explicit -4 (send it as IPv4) or -6 (send it as IPv6 -- non-standard) if an IP address literal on the ::ffff:ipv4/96 range is specified as the target, and output an error message otherwise.

And FWLIW, I strongly recommend that whichever behavior is chosen, it must be explicitly documented in the fping documentation and manpage.

@schweikert
Copy link
Owner

I am also not sure whether this should be merged. Unless somebody can provide a testimony of why this would be effectively useful, I wouldn't add this code (with potential bugs, etc.). Also, I can see how there could be downsides as well (i.e. the provided firewall testing use case).

@pevik
Copy link
Contributor Author

pevik commented Oct 26, 2024

Agree, let's close this. I'm sorry for wasting your time.

@pevik pevik closed this Oct 26, 2024
@schweikert
Copy link
Owner

It was certainly not time wasted. Thank you for your contribution!

@auerswal
Copy link
Collaborator

I'd like to collect some more information just in case some similar issue comes up in the future.

Two posts on the BusyBox mailing list look further into the behavior of different ping implementations when using IPv4-mapped IPv6 addresses:

  1. https://lists.busybox.net/pipermail/busybox/2024-October/090981.html
  2. https://lists.busybox.net/pipermail/busybox/2024-October/090985.html

These posts contain the interesting observation that IPv4-mapped IPv6 addresses are accepted as IPv4 addresses. This also works with fping when using -4, for both raw and datagram sockets:

$ ls -l src/fping
-rwxr-xr-x 1 auerswald auerswald 203232 Okt 28 14:54 src/fping
$ ./src/fping -4 ::ffff:127.0.0.1
::ffff:127.0.0.1 is alive
$ sudo ./src/fping -4 ::ffff:127.0.0.1
::ffff:127.0.0.1 is alive

Currently, the default behavior of fping with IPv4-mapped IPv6 addresses is identical to using -6. It results in an error with datagram sockets, and creates an ICMPv6 packet with with raw sockets (IPv6 addresses and interface names are obscured):

$ ./src/fping ::ffff:127.0.0.1
::ffff:127.0.0.1: error while sending ping: Invalid argument
::ffff:127.0.0.1 is unreachable
$ ./src/fping -6 ::ffff:127.0.0.1
::ffff:127.0.0.1: error while sending ping: Invalid argument
::ffff:127.0.0.1 is unreachable
$ sudo tcpdump -ieth0 -lnv 'icmp6[icmp6type] = icmp6-echo or icmp6[icmp6type] = icmp6-echoreply' &
[3] 9761
$ sudo ./src/fping -r0 ::ffff:127.0.0.1
15:18:12.743619 IP6 (flowlabel 0x450e4, hlim 64, next-header ICMPv6 (58) payload length: 64) 2001:db8::6007:b3fa:cb3e:132f > ::ffff:127.0.0.1: [icmp6 sum ok] ICMP6, echo request, seq 0
::ffff:127.0.0.1 is unreachable
$ sudo ./src/fping -r0 -6 ::ffff:127.0.0.1
15:18:18.852266 IP6 (flowlabel 0x450e4, hlim 64, next-header ICMPv6 (58) payload length: 64) 2001:db8:::6007:b3fa:cb3e:132f > ::ffff:127.0.0.1: [icmp6 sum ok] ICMP6, echo request, seq 0
::ffff:127.0.0.1 is unreachable

As such fping currently works similarly to other common ping implementations regarding IPv4-mapped IPv6 addresses.

pevik added a commit to pevik/iputils that referenced this pull request Nov 15, 2024
This reverts commit 8ed7ffc.

This is an incomplete implementation. While there are two attempts
(PR#562 [1] PR#562 [2] and PR#563 [3]) to fix it, it's probably not a
good idea to support IPv4-Mapped-in-IPv6 target addresses.

There were concerns [4] about supporting this when discussion on BusyBox
mailing list about patch bringing this functionality to BusyBox ping
implementation:

* ping is a low-level diagnostic tool, maybe it's not good to assume
  things like this, because IPv4-mapped IPv6 address format is not
  supported by raw sockets
* it's not clear how -4 -6 switches should behave

Although inetutils ping at least partly supports the feature [5]:

- explicit ipv4 ping:
% ping -c1 ::ffff:127.0.0.1
PING ::ffff:127.0.0.1 (127.0.0.1): 56 data bytes
64 bytes from 127.0.0.1: icmp_seq=0 ttl=64 time=0,100 ms

- explicit ipv6 ping:
% ping6 -c1 ::ffff:127.0.0.1
PING ::ffff:127.0.0.1 (::ffff:127.0.0.1): 56 data bytes
./ping/ping6: sending packet: Network is unreachable

Because also fping did not want to support it [6], let's revert the
feature.

[1] iputils#561
[2] iputils#563
[3] iputils#562
[4] https://lists.busybox.net/pipermail/busybox/2024-October/090975.html
[5] https://lists.busybox.net/pipermail/busybox/2024-October/090985.html
[6] schweikert/fping#356
Signed-off-by: Petr Vorel <[email protected]>
pevik added a commit to pevik/iputils that referenced this pull request Nov 17, 2024
This reverts commit 8ed7ffc.

This is an incomplete implementation. While there are two attempts
(PR#562 [1] PR#562 [2] and PR#563 [3]) to fix it, it's probably not a
good idea to support IPv4-Mapped-in-IPv6 target addresses.

There were concerns [4] about supporting this when discussion on BusyBox
mailing list about patch bringing this functionality to BusyBox ping
implementation:

* ping is a low-level diagnostic tool, maybe it's not good to assume
  things like this, because IPv4-mapped IPv6 address format is not
  supported by raw sockets
* it's not clear how -4 -6 switches should behave

NOTE: next commit fixes IPv4-Mapped-in-IPv6 target addresses via -4
flag.

Because also fping did not want to support it [6], let's revert the
feature.

[1] iputils#561
[2] iputils#563
[3] iputils#562
[4] https://lists.busybox.net/pipermail/busybox/2024-October/090975.html
[5] https://lists.busybox.net/pipermail/busybox/2024-October/090985.html
[6] schweikert/fping#356

Signed-off-by: Petr Vorel <[email protected]>
pevik added a commit to pevik/iputils that referenced this pull request Nov 26, 2024
This reverts commit 8ed7ffc.

This is an incomplete implementation. While there are two attempts
(PR#562 [1] PR#562 [2] and PR#563 [3]) to fix it, it's probably not a
good idea to support IPv4-Mapped-in-IPv6 target addresses.

There were concerns [4] about supporting this when discussion on BusyBox
mailing list about patch bringing this functionality to BusyBox ping
implementation:

* ping is a low-level diagnostic tool, maybe it's not good to assume
  things like this, because IPv4-mapped IPv6 address format is not
  supported by raw sockets
* it's not clear how -4 -6 switches should behave

NOTE: next commit fixes IPv4-Mapped-in-IPv6 target addresses via -4
flag.

Because also fping did not want to support it [6], let's revert the
feature.

[1] iputils#561
[2] iputils#563
[3] iputils#562
[4] https://lists.busybox.net/pipermail/busybox/2024-October/090975.html
[5] https://lists.busybox.net/pipermail/busybox/2024-October/090985.html
[6] schweikert/fping#356

Closes: iputils#567
Signed-off-by: Petr Vorel <[email protected]>
pevik added a commit to pevik/iputils that referenced this pull request Nov 27, 2024
This reverts commit 8ed7ffc.

This is an incomplete implementation. While there are two attempts
(PR#562 [1] PR#562 [2] and PR#563 [3]) to fix it, it's probably not a
good idea to support IPv4-Mapped-in-IPv6 target addresses.

There were concerns [4] about supporting this when discussion on BusyBox
mailing list about patch bringing this functionality to BusyBox ping
implementation:

* ping is a low-level diagnostic tool, maybe it's not good to assume
  things like this, because IPv4-mapped IPv6 address format is not
  supported by raw sockets
* it's not clear how -4 -6 switches should behave

NOTE: next commit fixes IPv4-Mapped-in-IPv6 target addresses via -4
flag.

Because also fping did not want to support it [6], let's revert the
feature.

[1] iputils#561
[2] iputils#563
[3] iputils#562
[4] https://lists.busybox.net/pipermail/busybox/2024-October/090975.html
[5] https://lists.busybox.net/pipermail/busybox/2024-October/090985.html
[6] schweikert/fping#356

Closes: iputils#567
Reviewed-by: Ronan Pigott <[email protected]>
Signed-off-by: Petr Vorel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants