Print remote IPv4 address on a dual-stack v6 socket in IPv4 format

PATCH In review Gert Doering 2 participants 4 messages Dec 29, 2014 – Feb 15, 2015

Share & tools

Gert Doering

author

2014-12-29 17:48 UTC #0 lore raw

PATCH 1/1 Print remote IPv4 address on a dual-stack v6 socket in IPv4 format

Previously, the code would print IPv4-mapped format ("::ffff:1.2.3.4"), which is technically correct but adds no extra value, and is confusingly different from the output if using a v4 socket. Print "1.2.3.4" instead, whatever socket type is used.

Affects client IP address in log file, status output (mroute_addr_print_ex), and environment (setenv_sockaddr).

The fact that a dual-stack socket was used is still visible in the initial peer connect message in the log:
'... Peer Connection Initiated with [AF_INET6]::ffff:1.1.3.4:53779'

Signed-off-by Gert Doering <[email protected]...>

---
 src/openvpn/mroute.c | 12 ++++++++++--
 src/openvpn/socket.c | 16 +++++++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)

src/openvpn/mroute.c +10 -2

index ba4ef46..972f1dd 100644
@@ -426,8 +426,16 @@ mroute_addr_print_ex (const struct mroute_addr *ma,
 	  break;
 	case MR_ADDR_IPV6:
 	  {
-	    buf_printf (&out, "%s",
-		  print_in6_addr( *(struct in6_addr*)&maddr.addr, 0, gc)); 
+	    if ( IN6_IS_ADDR_V4MAPPED( (struct in6_addr*)&maddr.addr ) )
+	      {
+		buf_printf (&out, "%s",
+		     print_in_addr_t( *(in_addr_t*)(&maddr.addr[12]), IA_NET_ORDER, gc));
+	      }
+	    else
+	      {
+	        buf_printf (&out, "%s",
+		      print_in6_addr( *(struct in6_addr*)&maddr.addr, 0, gc));
+	      }
 	    if (maddr.type & MR_WITH_NETBITS)
 	      {
 		buf_printf (&out, "/%d", maddr.netbits);

src/openvpn/socket.c +13 -3

index 331a9d9..f5c740d 100644
@@ -2573,9 +2573,19 @@ setenv_sockaddr (struct env_set *es, const char *name_prefix, const struct openv
 	}
       break;
     case AF_INET6:
-      openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip6", name_prefix);
-      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-		  buf, sizeof(buf), NULL, 0, NI_NUMERICHOST);
+      if ( IN6_IS_ADDR_V4MAPPED( &addr->addr.in6.sin6_addr ))
+	{
+	  struct in_addr ia;
+	  ia.s_addr = *(in_addr_t *)&addr->addr.in6.sin6_addr.s6_addr[12] ;
+	  openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip", name_prefix);
+	  openvpn_snprintf (buf, sizeof(buf), "%s", inet_ntoa(ia) );
+	}
+      else
+        {
+	  openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip6", name_prefix);
+	  getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+		      buf, sizeof(buf), NULL, 0, NI_NUMERICHOST);
+	}
       setenv_str (es, name_buf, buf);
 
       if ((flags & SA_IP_PORT) && addr->addr.in6.sin6_port)

Signature · 1 line

2.1.2

Arne Schwabe

2015-02-09 09:04 UTC #1 lore raw

Am 29.12.14 um 18:48 schrieb Gert Doering:

Quoted 23 lines

Previously, the code would print IPv4-mapped format ("::ffff:1.2.3.4"), which is technically correct but adds no extra value, and is confusingly different from the output if using a v4 socket. Print "1.2.3.4" instead, whatever socket type is used.
Affects client IP address in log file, status output (mroute_addr_print_ex), and environment (setenv_sockaddr).
The fact that a dual-stack socket was used is still visible in the initial peer connect message in the log:
'... Peer Connection Initiated with [AF_INET6]::ffff:1.1.3.4:53779'
Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/mroute.c | 12 ++++++++++--
 src/openvpn/socket.c | 16 +++++++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)
src/openvpn/mroute.c +0 -0
index ba4ef46..972f1dd 100644

src/openvpn/mroute.c mroute_addr_print_ex (const struct mroute_addr *ma, · line 432

Quoted 9 lines

src/openvpn/mroute.c +4 -2

@@ -426,8 +426,16 @@ mroute_addr_print_ex (const struct mroute_addr *ma,
 	  break;
 	case MR_ADDR_IPV6:
 	  {
-	    buf_printf (&out, "%s",
-		  print_in6_addr( *(struct in6_addr*)&maddr.addr, 0, gc)); 
+	    if ( IN6_IS_ADDR_V4MAPPED( (struct in6_addr*)&maddr.addr ) )
+	      {
+		buf_printf (&out, "%s",
+		     print_in_addr_t( *(in_addr_t*)(&maddr.addr[12]), IA_NET_ORDER, gc));

Have you checked that this is endian safe?

Quoted 14 lines

src/openvpn/mroute.c +6 -0

+	      }
+	    else
+	      {
+	        buf_printf (&out, "%s",
+		      print_in6_addr( *(struct in6_addr*)&maddr.addr, 0, gc));
+	      }
 	    if (maddr.type & MR_WITH_NETBITS)
 	      {
 		buf_printf (&out, "/%d", maddr.netbits);

src/openvpn/socket.c +0 -0

index 331a9d9..f5c740d 100644

src/openvpn/socket.c setenv_sockaddr (struct env_set *es, const char *name_prefix, const struct openv · line 2579

Quoted 10 lines

src/openvpn/socket.c +4 -3

@@ -2573,9 +2573,19 @@ setenv_sockaddr (struct env_set *es, const char *name_prefix, const struct openv
 	}
       break;
     case AF_INET6:
-      openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip6", name_prefix);
-      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-		  buf, sizeof(buf), NULL, 0, NI_NUMERICHOST);
+      if ( IN6_IS_ADDR_V4MAPPED( &addr->addr.in6.sin6_addr ))
+	{
+	  struct in_addr ia;
+	  ia.s_addr = *(in_addr_t *)&addr->addr.in6.sin6_addr.s6_addr[12] ;

Same as above.

src/openvpn/socket.c setenv_sockaddr (struct env_set *es, const char *name_prefix, const struct openv · line 2591

Quoted 12 lines

src/openvpn/socket.c +9 -0

+	  openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip", name_prefix);
+	  openvpn_snprintf (buf, sizeof(buf), "%s", inet_ntoa(ia) );
+	}
+      else
+        {
+	  openvpn_snprintf (name_buf, sizeof (name_buf), "%s_ip6", name_prefix);
+	  getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+		      buf, sizeof(buf), NULL, 0, NI_NUMERICHOST);
+	}
       setenv_str (es, name_buf, buf);
 
       if ((flags & SA_IP_PORT) && addr->addr.in6.sin6_port)

Otherwise ACK from me.

Arne

Gert Doering

author

2015-02-15 20:39 UTC #2 lore raw

Hi,

Quoted 5 lines · Arne Schwabe

Quoted 5 lines

src/openvpn/mroute.c +0 -0

@@ -426,8 +426,16 @@ mroute_addr_print_ex (const struct mroute_addr *ma,
 	  break;
 	case MR_ADDR_IPV6:

Quoted 8 lines

src/openvpn/mroute.c mroute_addr_print_ex (const struct mroute_addr *ma, · line 432

Quoted 7 lines

src/openvpn/mroute.c +4 -2

 	  {
-	    buf_printf (&out, "%s",
-		  print_in6_addr( *(struct in6_addr*)&maddr.addr, 0, gc)); 
+	    if ( IN6_IS_ADDR_V4MAPPED( (struct in6_addr*)&maddr.addr ) )
+	      {
+		buf_printf (&out, "%s",
+		     print_in_addr_t( *(in_addr_t*)(&maddr.addr[12]), IA_NET_ORDER, gc));

Have you checked that this is endian safe?

Yep, both by running on i386 and on sparc64 - and then by staring at the code and wondering why it works.

Thing is that print_in_addr_t() takes a flag, IA_NET_ORDER, which tells it "that 32bit thingie you're printing now is in network byte order", so it will not call htonl() on it before passing to inet_ntoa(), which wants network byte order.

(And the IPv6 address is "just an array of 16 octets", no possible confusion here with 32bit integer representation)

Gert Doering

author

2015-02-15 22:04 UTC #3 lore raw

In reply to Gert Doering

Patch has been applied to the master branch.

commit 0b1a68fffa33e175c320c2828604cdc7dfb097e7 (master)

Author: Gert Doering
Date:   Mon Dec 29 18:48:45 2014 +0100

     Print remote IPv4 address on a dual-stack v6 socket in IPv4 format

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9363

--
kind regards,

Gert Doering