fix issue with NAT logic overwriting port specified by user in manual hole punch; adding some logging

4 files changed, 96 insertions, 57 deletions

diff --git a/src/nat/gnunet-nat.c b/src/nat/gnunet-nat.c
index 80bfb5726..1c5850592 100644
--- a/src/nat/gnunet-nat.c
+++ b/src/nat/gnunet-nat.c
@@ -34,9 +34,9 @@
 static int global_ret;
 
 /**
- * Name of section in configuration file to use for 
+ * Name of section in configuration file to use for
  * additional options.
- */ 
+ */
 static char *section_name;
 
 /**
@@ -81,7 +81,7 @@ static struct GNUNET_NAT_Handle *nh;
 
 /**
  * Listen socket for STUN processing.
- */ 
+ */
 static struct GNUNET_NETWORK_Handle *ls;
 
 /**
@@ -110,7 +110,7 @@ test_finished ()
  * a function to call whenever our set of 'valid' addresses changes.
  *
  * @param cls closure, NULL
- * @param add_remove #GNUNET_YES to add a new public IP address, 
+ * @param add_remove #GNUNET_YES to add a new public IP address,
  *                   #GNUNET_NO to remove a previous (now invalid) one
  * @param ac address class the address belongs to
  * @param addr either the previous or the new public IP address
@@ -123,12 +123,12 @@ address_cb (void *cls,
             const struct sockaddr *addr,
             socklen_t addrlen)
 {
-  GNUNET_log (GNUNET_ERROR_TYPE_MESSAGE,
-              "%s %s (%d)\n",
-              add_remove ? "+" : "-",
-              GNUNET_a2s (addr,
-                          addrlen),
-              (int) ac);
+  fprintf (stdout,
+           "%s %s (%d)\n",
+           add_remove ? "+" : "-",
+           GNUNET_a2s (addr,
+                       addrlen),
+           (int) ac);
 }
 
 
@@ -186,7 +186,7 @@ static void
 stun_read_task (void *cls)
 {
   ssize_t size;
-  
+
   rtask = GNUNET_SCHEDULER_add_read_net (GNUNET_TIME_UNIT_FOREVER_REL,
                                          ls,
                                          &stun_read_task,
@@ -204,7 +204,7 @@ stun_read_task (void *cls)
     struct sockaddr_storage sa;
     socklen_t salen = sizeof (sa);
     ssize_t ret;
-    
+
     ret = GNUNET_NETWORK_socket_recvfrom (ls,
                                           buf,
                                           size + 1,
@@ -318,7 +318,7 @@ run (void *cls,
   else if (listen_reversal)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_MESSAGE,
-                "Use of `-W` only effective in combination with `-i`\n");    
+                "Use of `-W` only effective in combination with `-i`\n");
     global_ret = 1;
     GNUNET_SCHEDULER_shutdown ();
     return;
@@ -327,7 +327,7 @@ run (void *cls,
   if (NULL != remote_addr)
   {
     int ret;
-    
+
     if ( (NULL == nh) ||
          (sizeof (struct sockaddr_in) != local_len) )
     {
@@ -365,7 +365,7 @@ run (void *cls,
       break;
     }
   }
-  
+
   if (do_stun)
   {
     if (NULL == local_addr)
diff --git a/src/nat/gnunet-service-nat.c b/src/nat/gnunet-service-nat.c
index bfe212308..0695c7ac7 100644
--- a/src/nat/gnunet-service-nat.c
+++ b/src/nat/gnunet-service-nat.c
@@ -662,6 +662,11 @@ notify_client (enum GNUNET_NAT_AddressClass ac,
   struct GNUNET_MQ_Envelope *env;
   struct GNUNET_NAT_AddressChangeNotificationMessage *msg;
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Notifying client about %s of IP %s\n",
+              add ? "addition" : "removal",
+              GNUNET_a2s (addr,
+                          addr_len));
   env = GNUNET_MQ_msg_extra (msg,
                              addr_len,
                              GNUNET_MESSAGE_TYPE_NAT_ADDRESS_CHANGE);
@@ -693,7 +698,11 @@ check_notify_client (struct LocalAddressList *delta,
   struct sockaddr_in6 v6;
 
   if (0 == (ch->flags & GNUNET_NAT_RF_ADDRESSES))
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Not notifying client as it does not care about addresses\n");
     return;
+  }
   switch (delta->af)
   {
   case AF_INET:
@@ -715,25 +724,24 @@ check_notify_client (struct LocalAddressList *delta,
            (! match_ipv4 ("127.0.0.1", &v4.sin_addr, 8)) )
         continue; /* bound to loopback, but this is not loopback */
       if ( (! match_ipv4 ("127.0.0.1", &c4->sin_addr, 8) ) &&
-           (0 != c4->sin_addr.s_addr) &&
            match_ipv4 ("127.0.0.1", &v4.sin_addr, 8) )
         continue; /* bound to non-loopback, but this is loopback */
-      if ( (0 != (ch->flags & GNUNET_NAT_AC_EXTERN)) &&
-           (0 != c4->sin_addr.s_addr) &&
-           (! is_nat_v4 (&v4.sin_addr)) )
-        continue; /* based on external-IP, but this IP is not
-                     from private address range. */
+      if ( (0 != (delta->ac & GNUNET_NAT_AC_EXTERN)) &&
+           (0 != c4->sin_addr.s_addr) &&
+           (! is_nat_v4 (&v4.sin_addr)) )
+       continue; /* based on external-IP, but this IP is not
+                    from private address range. */
       if ( (0 != memcmp (&v4.sin_addr,
-                         &c4->sin_addr,
-                         sizeof (struct in_addr))) &&
-           (0 != c4->sin_addr.s_addr) &&
-           ( (! is_nat_v4 (&c4->sin_addr)) ||
-             (0 == (ch->flags & GNUNET_NAT_AC_EXTERN))) )
+                         &c4->sin_addr,
+                         sizeof (struct in_addr))) &&
+           (0 != c4->sin_addr.s_addr) &&
+           (! is_nat_v4 (&c4->sin_addr)) )
         continue; /* this IP is not from private address range,
                      and IP does not match. */
 
       /* OK, IP seems relevant, notify client */
-      v4.sin_port = c4->sin_port;
+      if (0 == htons (v4.sin_port))
+        v4.sin_port = c4->sin_port;
       notify_client (delta->ac,
                      ch,
                      add,
@@ -760,13 +768,10 @@ check_notify_client (struct LocalAddressList *delta,
            (! match_ipv6 ("::1", &v6.sin6_addr, 128)) )
         continue; /* bound to loopback, but this is not loopback */
       if ( (! match_ipv6 ("::1", &c6->sin6_addr, 128) ) &&
-           (0 != memcmp (&c6->sin6_addr,
-                         &in6addr_any,
-                         sizeof (struct in6_addr))) &&
            match_ipv6 ("::1", &v6.sin6_addr, 128) )
         continue; /* bound to non-loopback, but this is loopback */
-      if ( (0 != (ch->flags & GNUNET_NAT_AC_EXTERN)) &&
-           (0 != memcmp (&c6->sin6_addr,
+      if ( (0 != (delta->ac & GNUNET_NAT_AC_EXTERN)) &&
+           (0 != memcmp (&c6->sin6_addr,
                          &in6addr_any,
                          sizeof (struct in6_addr))) &&
            (! is_nat_v6 (&v6.sin6_addr)) )
@@ -793,7 +798,8 @@ check_notify_client (struct LocalAddressList *delta,
                      does not match and is not an external IP */
 
       /* OK, IP seems relevant, notify client */
-      v6.sin6_port = c6->sin6_port;
+      if (0 == htons (v6.sin6_port))
+        v6.sin6_port = c6->sin6_port;
       notify_client (delta->ac,
                      ch,
                      add,
@@ -853,6 +859,10 @@ notify_client_external_ipv4_change (void *cls,
     struct LocalAddressList lal;
     struct sockaddr_in *s4;
 
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Detected eternal IP, can now back-fill AUTO:%u in hole punching specification of `%s'\n",
+                (unsigned int) ch->ext_dns_port,
+                ch->section_name);
     memset (&lal, 0, sizeof (lal));
     s4 = (struct sockaddr_in *) &lal.addr;
     s4->sin_family = AF_INET;
@@ -868,8 +878,6 @@ notify_client_external_ipv4_change (void *cls,
   /* (1) check if client cares. */
   if (! ch->natted_address)
     return;
-  if (0 == (GNUNET_NAT_RF_ADDRESSES & ch->flags))
-    return;
   have_v4 = GNUNET_NO;
   for (unsigned int i=0;i<ch->num_caddrs;i++)
   {
@@ -891,6 +899,10 @@ notify_client_external_ipv4_change (void *cls,
   sa.sin_addr = *v4;
   sa.sin_port = htons (0);
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Detected eternal IP %s, notifying client of external IP (without port)\n",
+              GNUNET_a2s ((const struct sockaddr *) &sa,
+                          sizeof (sa)));
   /* (3) notify client of change */
   notify_client (is_nat_v4 (v4)
                  ? GNUNET_NAT_AC_EXTERN | GNUNET_NAT_AC_LAN
@@ -1055,7 +1067,8 @@ run_scan (void *cls)
 
         GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                     "Found NATed local address %s, starting NAT server\n",
-                    GNUNET_a2s ((void *) &pos->addr, sizeof (*s4)));
+                    GNUNET_a2s ((const struct sockaddr *) &pos->addr,
+                                sizeof (*s4)));
         pos->hc = GN_start_gnunet_nat_server_ (&s4->sin_addr,
                                                &reversal_callback,
                                                pos);
@@ -1280,6 +1293,11 @@ dyndns_lookup (void *cls)
   struct ClientHandle *ch = cls;
   struct LocalAddressList *lal;
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Performing DNS lookup for punched hole given for `%s' as `%s:%u'\n",
+              ch->section_name,
+              ch->hole_external,
+              (unsigned int) ch->ext_dns_port);
   for (lal = ch->ext_addr_head; NULL != lal; lal = lal->next)
     lal->old = GNUNET_YES;
   ch->ext_dns_task = NULL;
@@ -1374,6 +1392,11 @@ lookup_hole_external (struct ClientHandle *ch)
                       ch->hole_external,
                       &s4->sin_addr))
   {
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "IPv4 punched hole given for `%s' via `%s:%u'\n",
+                ch->section_name,
+                ch->hole_external,
+                (unsigned int) ch->ext_dns_port);
     s4->sin_port = htons (ch->ext_dns_port);
     lal->af = AF_INET;
     lal->ac = GNUNET_NAT_AC_GLOBAL | GNUNET_NAT_AC_MANUAL;
@@ -1423,8 +1446,6 @@ handle_register (void *cls,
     GNUNET_SERVICE_client_drop (ch->client);
     return;
   }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Received REGISTER message from client\n");
   ch->flags = message->flags;
   ch->proto = message->proto;
   ch->num_caddrs = ntohs (message->num_addrs);
@@ -1512,6 +1533,9 @@ handle_register (void *cls,
   ch->section_name
     = GNUNET_strndup (off,
                       ntohs (message->str_len));
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Received REGISTER message from client for subsystem `%s'\n",
+              ch->section_name);
   if (GNUNET_OK ==
       GNUNET_CONFIGURATION_get_value_string (cfg,
                                              ch->section_name,
diff --git a/src/nat/gnunet-service-nat_helper.c b/src/nat/gnunet-service-nat_helper.c
index e91f63beb..de6531fa8 100644
--- a/src/nat/gnunet-service-nat_helper.c
+++ b/src/nat/gnunet-service-nat_helper.c
@@ -221,6 +221,26 @@ restart_nat_server (void *cls)
   char ia[INET_ADDRSTRLEN];
 
   h->server_read_task = NULL;
+  GNUNET_assert (NULL !=
+                 inet_ntop (AF_INET,
+                            &h->internal_address,
+                            ia,
+                            sizeof (ia)));
+  /* Start the server process */
+  binary
+    = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-server");
+  if (GNUNET_YES !=
+      GNUNET_OS_check_helper_binary (binary,
+                                     GNUNET_YES,
+                                     ia))
+  {
+    /* move instantly to max delay, as this is unlikely to be fixed */
+    h->server_retry_delay
+      = GNUNET_TIME_STD_EXPONENTIAL_BACKOFF_THRESHOLD;
+    GNUNET_free (binary);
+    try_again (h);
+    return;
+  }
   h->server_stdout
     = GNUNET_DISK_pipe (GNUNET_YES, GNUNET_YES,
                         GNUNET_NO, GNUNET_YES);
@@ -228,21 +248,14 @@ restart_nat_server (void *cls)
   {
     GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR,
                          "pipe");
+    GNUNET_free (binary);
     try_again (h);
     return;
   }
-  GNUNET_assert (NULL !=
-                 inet_ntop (AF_INET,
-                            &h->internal_address,
-                            ia,
-                            sizeof (ia)));
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Starting `%s' at `%s'\n",
               "gnunet-helper-nat-server",
               ia);
-  /* Start the server process */
-  binary
-    = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-server");
   h->server_proc
     = GNUNET_OS_start_process (GNUNET_NO,
                                0,
diff --git a/src/nat/nat_api.c b/src/nat/nat_api.c
index eec5d3968..dfe7defc9 100644
--- a/src/nat/nat_api.c
+++ b/src/nat/nat_api.c
@@ -52,7 +52,7 @@ struct AddrEntry
    * Address class of the address.
    */
   enum GNUNET_NAT_AddressClass ac;
-  
+
   /**
    * Number of bytes that follow.
    */
@@ -70,7 +70,7 @@ struct GNUNET_NAT_Handle
    * Configuration we use.
    */
   const struct GNUNET_CONFIGURATION_Handle *cfg;
-  
+
   /**
    * Message queue for communicating with the NAT service.
    */
@@ -80,7 +80,7 @@ struct GNUNET_NAT_Handle
    * Our registration message.
    */
   struct GNUNET_MessageHeader *reg;
-  
+
   /**
    * Head of address DLL.
    */
@@ -95,12 +95,12 @@ struct GNUNET_NAT_Handle
    * Function to call when our addresses change.
    */
   GNUNET_NAT_AddressCallback address_callback;
-  
+
   /**
    * Function to call when another peer requests connection reversal.
    */
   GNUNET_NAT_ReversalCallback reversal_callback;
-  
+
   /**
    * Closure for the various callbacks.
    */
@@ -136,7 +136,7 @@ static void
 reconnect (struct GNUNET_NAT_Handle *nh)
 {
   struct AddrEntry *ae;
-  
+
   if (NULL != nh->mq)
   {
     GNUNET_MQ_destroy (nh->mq);
@@ -184,7 +184,7 @@ check_connection_reversal_request (void *cls,
   return GNUNET_OK;
 }
 
-  
+
 /**
  * Handle connection reversal request.
  *
@@ -247,7 +247,7 @@ check_address_change_notification (void *cls,
   return GNUNET_OK;
 }
 
-  
+
 /**
  * Handle connection reversal request.
  *
@@ -264,6 +264,8 @@ handle_address_change_notification (void *cls,
   enum GNUNET_NAT_AddressClass ac;
   struct AddrEntry *ae;
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Received address change notification\n");
   ac = (enum GNUNET_NAT_AddressClass) ntohl (acn->addr_class);
   if (GNUNET_YES == ntohl (acn->add_remove))
   {
@@ -395,7 +397,7 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg,
   size_t len;
   size_t str_len;
   char *off;
-  
+
   len = 0;
   for (unsigned int i=0;i<num_addrs;i++)
     len += addrlens[i];
@@ -569,7 +571,7 @@ test_stun_packet (const void *data,
  *
  * The function does some basic sanity checks on packet size and
  * content, try to extract a bit of information.
- * 
+ *
  * At the moment this only processes BIND requests, and returns the
  * externally visible address of the request to the rest of the
  * NAT logic.
@@ -663,7 +665,7 @@ GNUNET_NAT_test_address (struct GNUNET_NAT_Handle *nh,
  * @param nh handle (used for configuration)
  * @param local_sa our local address of the peer (IPv4-only)
  * @param remote_sa the remote address of the peer (IPv4-only)
- * @return #GNUNET_SYSERR on error, 
+ * @return #GNUNET_SYSERR on error,
  *         #GNUNET_NO if connection reversal is unavailable,
  *         #GNUNET_OK otherwise (presumably in progress)
  */