Handle hex-encoded IPv4-Mapped-in-IPv6 addresses #561
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
stop racing me to fixes :p |
f08cb69 to
0a016cd
Compare
There are several possible 'non-standard' forms of IPv4-mapped addresses. Identify them by the first 'f' in the 'ffff' so multiple leading zeros or 'f' in the hex-encoded address do not confuse. Signed-off-by: Tj <linux@iam.tj>
0a016cd to
44d337d
Compare
Signed-off-by: Tj <linux@iam.tj>
44d337d to
2ee0350
Compare
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 <pvorel@suse.cz>
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 <pvorel@suse.cz>
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 <pvorel@suse.cz>
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 <ronan@rjp.ie> Signed-off-by: Petr Vorel <pvorel@suse.cz>
|
Closing due reverting the original feature, see #567. |
|
Pity - despite not understanding many of the technical details, what I have seen in the discussion was a lot of flailing about and mostly personal reluctance to have this feature - there's not really been a conclusive argument(*) how IPv4 mapped in IPv6 (or hostnames that resolve to such an address form) should be interpreted other than as IPv4.
I still believe the feature is good and should eventually be reconsidered, possibly in a different form of implementation. |
|
There was a conflict with another issue, which we agreed is more important: #252 which resulted into 8ed7ffc. Once glibc is fixed (see https://public-inbox.org/libc-alpha/a26674d3f48af0f67b9abb3e54c1228854cc0bd5@rjp.ie/) 8ed7ffc can be reverted and that fix Anyway, feel free to create an issue so that we don't forget on it. |
|
Mhh - as you know, my proposed fix #562 works a bit different than this one here - I propose to do a proper interpretation of the result(s) returned by
Since my according pull request is not yet closed, I would propose to keep #562 open instead of creating a new issue, if that's okay with you? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hexadecimal encoding such as
::ffff:7f:1would break IPv4 handling; the hex value needs converting to dotted decimal.I've also added tests (I think correctly!) for both.