Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: Dual Listening on IPV4 & 6 in broker and listen modes
From: David Fifield <david () bamsoftware com>
Date: Thu, 23 Jun 2011 23:41:22 -0700

On Wed, Jun 22, 2011 at 04:58:43PM -0500, Colin L. Rice wrote:
Hello,

At the request of David I rewrote broker to allow clients using IPV4 and
IPV6 to communicate with each other using the default broker as well as
rewriting the whole dual listening system to make a little more sense
and be more maintainable.

However due to a weird feature of Linux where :: binds to 0.0.0.0 as
well we added a change where all IPV6 sockets now specify IPV6_V6ONLY so
on Linux networking stacks you can have a dual listening mode without
collisions. This means however that ncat -l -6 :: will not accept IPV4
connections in their IPV6 address format on Linux.

This isn't a feature of Linux only, it's part of the design of IPv6
sockets. See sections 3.7 "Compatibility with IPv4 Nodes" and 5.3
"IPV6_V6ONLY option for AF_INET6 Sockets" of RFC 3493.
http://tools.ietf.org/html/rfc3493
Some operating systems disable the "compatibility" part by default for
technical or security reasons.
http://tools.ietf.org/html/draft-itojun-v6ops-v4mapped-harmful-02

The patch is attached. Does anyone have any feedback? Would you prefer
that IPV6_V6ONLY only be set for ncat -l or ncat --broker and it be
turned off -6 is passed? 

I think that -6 mode should not accept any IPv4 connections, not even
from IPv4-mapped addresses like ::ffff:127.0.0.1. If I run "ncat -6 -l 80"
I expect to also be able to run a separate IPv4 web server. I think
we should turn on IPV6_V6ONLY for all IPv6 sockets.

Index: ncat_listen.c
===================================================================
--- ncat_listen.c     (revision 24247)
+++ ncat_listen.c     (working copy)
     /* setup the main listening socket */
-    listen_socket = do_listen(SOCK_STREAM, proto, &srcaddr);
+    listen_socket = do_listen(SOCK_STREAM, proto, &srcaddr[0]);

I think that wherever you refer to srcaddr[0], you need to refer to
srcaddr[1] equally. They should not be separated in the code. The best
way is to loop over num_srcaddrs. listen_socket should be an array of
the same size as srcaddr, not separate variables listen_socket and
listen_socket6.

@@ -545,7 +535,7 @@
 
     while (1) {
         /* create the UDP listen socket */
-        sockfd = do_listen(SOCK_DGRAM, proto, &srcaddr);
+        sockfd = do_listen(SOCK_DGRAM, proto, &srcaddr[0]);

This looks broken to me. The UDP mode only refers to srcaddr[0].

Index: ncat_main.c
===================================================================
--- ncat_main.c       (revision 24247)
+++ ncat_main.c       (working copy)
@@ -533,6 +533,7 @@
 
     if (o.verbose)
         print_banner();
+
     if(o.debug)
         nbase_set_log(loguser, logdebug);
     else
@@ -541,7 +542,7 @@
     /* Will be AF_INET or AF_INET6 when valid */
     memset(&targetss.storage, 0, sizeof(targetss.storage));
     targetss.storage.ss_family = AF_UNSPEC;
-    httpconnect.storage = socksconnect.storage = srcaddr.storage = targetss.storage;
+    httpconnect.storage = socksconnect.storage = srcaddr[0].storage = srcaddr[1].storage = targetss.storage;

I would prefer that you handle these things in a loop, instead of
hardcoding 0 and 1.

     if (proxyaddr) {
       if (!o.proxytype)
@@ -582,7 +583,7 @@
         if (o.listen)
             bye("-l and -s are incompatible.  Specify the address and port to bind to like you would a host to 
connect to.");
 
-        if (!resolve(source, 0, &srcaddr.storage, &srcaddrlen, o.af))
+        if (!resolve(source, 0, &srcaddr[0].storage, &srcaddrlen[0], o.af))
             bye("Could not resolve source address %s.", source);
     }
 
@@ -592,18 +593,18 @@
                compatibility. */
             o.portno = srcport;
         } else {
-            if (srcaddr.storage.ss_family == AF_UNSPEC)
-                srcaddr.storage.ss_family = o.af;
+            if (srcaddr[0].storage.ss_family == AF_UNSPEC)
+                srcaddr[0].storage.ss_family = o.af;
             if (o.af == AF_INET) {
-                srcaddr.in.sin_port = htons((unsigned short) srcport);
-                if (!srcaddrlen)
-                    srcaddrlen = sizeof(srcaddr.in);
+                srcaddr[0].in.sin_port = htons((unsigned short) srcport);
+                if (!srcaddrlen[0])
+                    srcaddrlen[0] = sizeof(srcaddr[0].in);
             }
 #ifdef HAVE_IPV6
             else {
-                srcaddr.in6.sin6_port = htons((unsigned short) srcport);
-                if (!srcaddrlen)
-                    srcaddrlen = sizeof(srcaddr.in6);
+                srcaddr[0].in6.sin6_port = htons((unsigned short) srcport);
+                if (!srcaddrlen[0])
+                    srcaddrlen[0] = sizeof(srcaddr[0].in6);
             }
 #endif
         }
@@ -626,6 +627,7 @@
             if (!resolve(o.target, 0, &targetss.storage, &targetsslen, o.af))
                 bye("Could not resolve hostname %s.", o.target);
             optind++;
+            o.inetset=1;

I think you can make this code a lot clearer. Instead of overloading the
meaning of the global srcaddr array (used for both option processing and
as data passed to ncat_listen), use a temporary variable for option
processing only. Then after the option loop, it can be very clear:

num_srcaddrs = 0;
if (tmp_addr.storage.ss_family == AF_UNSPEC) {
        if (o.af == AF_INET || o.af == AF_UNSPEC)
                srcaddr[num_srcaddrs++] = 0.0.0.0;
        if (o.af == AF_INET6 || o.af == AF_UNSPEC)
                srcaddr[num_srcaddrs++] = ::;
} else {
        srcaddr[num_srcaddrs++] = tmp_addr;
}

I think that better shows your intention of setting up two listening
addresses when no address and neither -4 nor -6 is given. This means
that you will have to change the default value of o.af in listen mode; I
think that's cleaner than the o.inetset variable.

This also can clear up a lot of confusion when someone is reading the
option processing loop while thinking about connect mode: "Why are they
using an array when you can only connect to one address?"

Index: ncat_core.c
===================================================================
--- ncat_core.c       (revision 24247)
+++ ncat_core.c       (working copy)
@@ -107,8 +107,8 @@
 #include <time.h>
 #include <assert.h>
 
-union sockaddr_u srcaddr;
-size_t srcaddrlen;
+union sockaddr_u srcaddr[2];
+size_t srcaddrlen[2];

Do we need srcaddrlen? Could we get rid of it?

This declaration needs a comment saying why the array has size 2: we set
up at most 2 listening sockets, one for IPv4 and one for IPv6.

David Fifield
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/


  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]