-
Notifications
You must be signed in to change notification settings - Fork 251
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
Conversation
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]>
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. I find this somewhat unfavorable because it permanently changes the output. |
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. |
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
|
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. |
@pevik, I am curious, what is your use case for this functionality? |
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. |
This is described on the BusyBox mailing list: https://lists.busybox.net/pipermail/busybox/2024-October/090979.html |
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. |
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). |
Agree, let's close this. I'm sorry for wasting your time. |
It was certainly not time wasted. Thank you for your contribution! |
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
These posts contain the interesting observation that IPv4-mapped IPv6 addresses are accepted as IPv4 addresses. This also works with
Currently, the default behavior of
As such |
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]>
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]>
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]>
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]>
IP addresses with ::ffff: prefix are IPv6 addresses which are mapped to IPv4. Therefore ping resulting IPv4 address.
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.