[PATCH 1/1] ping: Add support for IPv4-mapped IPv6 address format

[David Leonard]

On Sun, 20 Oct 2024, Lars Uffmann wrote:
> As for the points David brings up:

> On 2024-10-20 13:51:32, Petr Vorel wrote:
>>>> I'm not a fan of this patch because ping is a low-level diagnostic tool.
> While I would acknowlege the low-level diagnostic part, I do think it has 
> uses that go beyond that - since most operating systems can be configured to

I spent some time with your application trying to understand what
networking it's doing and the motivating context... and I would
recommend you look at getaddrinfo() and getnodeinfo(). These are
supported by cygwin64 (mentioned in file comments). My general advice
to you is to try and make your application address agnostic.

It seems clear that this patch to ping exists only because of your
application's deliberate construction of IPv4-mapped IPv6
addresses to fit a poor choice of type/represenation.

I see that the code had previously been using uint32_t for addresses,
and switched to in6_addr for IPv6 support. The incorrect assumption is
that in6_addr is simply a superspace of IPv4. (It's not.) You would be
better off using something to track address families, such as
a struct sockaddr_storage or struct addrinfo (latter is more portable).

> However, I also believe that the usefulness of this patch is greater than the 
> usefulness of
>>>> the case where an advanced user wants ping to send a packet with an 
>>>> IPv4-mapped IPv6 addresses

The problem remains that this patch will still carelessly break that case
by inserting an unwanted translation into an old and reliable tool used for
careful diagnostics.

> So @David: does that mean when you send such a packet with a destination 
> ::ffff::<ipv4>, it would actually match some network route so that it could 
> reach a firewall?

Yes, the default route ::/0.

>>>> 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.
> How about
>  ping -4 -6 ::ffff::<ipv4>

We're possibly getting off topic, but the following usage patterns might be
widely acceptable.

The exsiting cases I want preserved are:

   ping 192.0.2.1            -> ICMPv4 to 192.0.2.1
   ping 2001:db8::           -> ICMPv6 to 201:db8::
   ping ::ffff:192.0.2.1     -> ICMPv6 to ::ffff:192.0.2.1

The new mapped cases would be:

   ping -4 ::ffff:192.0.2.1  -> ICMPv4 to 192.0.2.1
   ping -6 192.0.2.1         -> ICMPv6 to ::ffff:192.0.2.1

Currently the first mapped case (-4 IPv6addr) ALREADY WORKS for
busybox's ping, while the last case (-6 IPv4addr) gives "bad address":

     # ./busybox ping -4 ::ffff:1.1.1.1
     PING ::ffff:1.1.1.1 (1.1.1.1): 56 data bytes
     64 bytes from 1.1.1.1: seq=0 ttl=59 time=2.260 ms
     ...
     # ./busybox ping -6 1.1.1.1
     ping: bad address '1.1.1.1'

Interestingly, iputils' ping (20240117) reports "Address family for hostname
not supported" for both.

> For my own application, I might end up implementing an ICMP socket directly, 
> mostly to avoid the unnecessary system call in the exec routine.

I don't understand your application's needs, but if you have to coordinate two
L4s for connectivity testing, then it's probably more reliable if done together.

d