improve/fix handling of NAT server logic for ICMP-based autonomous NAT traversal

7 files changed, 115 insertions, 46 deletions

diff --git a/src/include/gnunet_nat_service.h b/src/include/gnunet_nat_service.h
index 4df17b531..d9ce0e6f9 100644
--- a/src/include/gnunet_nat_service.h
+++ b/src/include/gnunet_nat_service.h
@@ -155,15 +155,11 @@ typedef void
  * reversal.
  *
  * @param cls closure
- * @param local_addr address where we received the request
- * @param local_addrlen actual length of the @a local_addr
  * @param remote_addr public IP address of the other peer
  * @param remote_addrlen actual length of the @a remote_addr
  */
 typedef void
 (*GNUNET_NAT_ReversalCallback) (void *cls,
-                                const struct sockaddr *local_addr,
-                                socklen_t local_addrlen,
                                 const struct sockaddr *remote_addr,
                                 socklen_t remote_addrlen);
 
diff --git a/src/nat/gnunet-nat.c b/src/nat/gnunet-nat.c
index 4d0ed5723..a145dc800 100644
--- a/src/nat/gnunet-nat.c
+++ b/src/nat/gnunet-nat.c
@@ -338,15 +338,11 @@ address_cb (void *cls,
  * reversal.
  *
  * @param cls closure, NULL
- * @param local_addr address where we received the request
- * @param local_addrlen actual length of the @a local_addr
  * @param remote_addr public IP address of the other peer
  * @param remote_addrlen actual length of the @a remote_addr
  */
 static void
 reversal_cb (void *cls,
-             const struct sockaddr *local_addr,
-             socklen_t local_addrlen,
              const struct sockaddr *remote_addr,
              socklen_t remote_addrlen)
 {
@@ -580,6 +576,14 @@ run (void *cls,
                               (listen_reversal) ? &reversal_cb : NULL,
                               NULL);
   }
+  else if (listen_reversal)
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_MESSAGE,
+                "Use of `-W` only effective in combination with `-i`\n");    
+    global_ret = 1;
+    GNUNET_SCHEDULER_shutdown ();
+    return;
+  }
 
   if (NULL != remote_addr)
   {
diff --git a/src/nat/gnunet-service-nat.c b/src/nat/gnunet-service-nat.c
index 762175437..b547e773a 100644
--- a/src/nat/gnunet-service-nat.c
+++ b/src/nat/gnunet-service-nat.c
@@ -1,6 +1,6 @@
 /*
   This file is part of GNUnet.
-  Copyright (C) 2016 GNUnet e.V.
+  Copyright (C) 2016, 2017 GNUnet e.V.
 
   GNUnet is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published
@@ -176,6 +176,12 @@ struct LocalAddressList
   struct LocalAddressList *prev;
 
   /**
+   * Context for a gnunet-helper-nat-server used to listen
+   * for ICMP messages to this client for connection reversal.
+   */
+  struct HelperContext *hc;
+  
+  /**
    * The address itself (i.e. `struct sockaddr_in` or `struct
    * sockaddr_in6`, in the respective byte order).
    */
@@ -395,6 +401,11 @@ destroy_lal ()
     GNUNET_CONTAINER_DLL_remove (lal_head,
                                  lal_tail,
                                  lal);
+    if (NULL != lal->hc)
+    {
+      GN_stop_gnunet_nat_server_ (lal->hc);
+      lal->hc = NULL;
+    }
     GNUNET_free (lal);
   }
 }
@@ -718,10 +729,12 @@ check_notify_client (struct LocalAddressList *delta,
     GNUNET_memcpy (&v4,
                    &delta->addr,
                    alen);
+    
+    /* Check for client notifications */
     for (unsigned int i=0;i<ch->num_caddrs;i++)
     {
       const struct sockaddr_in *c4;
-      
+
       if (AF_INET != ch->caddrs[i].ss.ss_family)
         return; /* IPv4 not relevant */
       c4 = (const struct sockaddr_in *) &ch->caddrs[i].ss;
@@ -890,7 +903,7 @@ check_notify_client_external_ipv4_change (const struct in_addr *v4,
   
   /* (3) notify client of change */
   notify_client (is_nat_v4 (v4)
-                 ? GNUNET_NAT_AC_EXTERN | GNUNET_NAT_AC_LAN_PRIVATE 
+                 ? GNUNET_NAT_AC_EXTERN | GNUNET_NAT_AC_LAN 
                  : GNUNET_NAT_AC_EXTERN | GNUNET_NAT_AC_GLOBAL,
                  ch,
                  add,
@@ -1006,6 +1019,65 @@ run_external_ip (void *cls)
 
 
 /**
+ * We got a connection reversal request from another peer.
+ * Notify applicable clients.
+ *
+ * @param cls closure with the `struct LocalAddressList` 
+ * @param ra IP address of the peer who wants us to connect to it 
+ */
+static void
+reversal_callback (void *cls,
+                   const struct sockaddr_in *ra)
+{
+  struct LocalAddressList *lal = cls;
+  const struct sockaddr_in *l4;
+
+  GNUNET_assert (AF_INET == lal->af);
+  l4 = (const struct sockaddr_in *) &lal->addr;
+  for (struct ClientHandle *ch = ch_head;
+       NULL != ch;
+       ch = ch->next)
+  {    
+    struct GNUNET_NAT_ConnectionReversalRequestedMessage *crrm;
+    struct GNUNET_MQ_Envelope *env;
+    int match;
+
+    /* Check if client is in applicable range for ICMP NAT traversal
+       for this local address */
+    if (! ch->natted_address)
+      continue;
+    match = GNUNET_NO;
+    for (unsigned int i=0;i<ch->num_caddrs;i++)
+    {
+      struct ClientAddress *ca = &ch->caddrs[i];
+      const struct sockaddr_in *c4;
+      
+      if (AF_INET != ca->ss.ss_family)
+        continue;
+      c4 = (const struct sockaddr_in *) &ca->ss;
+      if ( (0 != c4->sin_addr.s_addr) &&
+           (l4->sin_addr.s_addr != c4->sin_addr.s_addr) )
+        continue;
+      match = GNUNET_YES;
+      break;
+    }
+    if (! match)
+      continue;
+
+    /* Notify applicable client about connection reversal request */
+    env = GNUNET_MQ_msg_extra (crrm,
+                               sizeof (struct sockaddr_in),
+                               GNUNET_MESSAGE_TYPE_NAT_CONNECTION_REVERSAL_REQUESTED);
+    GNUNET_memcpy (&crrm[1],
+                   ra,
+                   sizeof (struct sockaddr_in));
+    GNUNET_MQ_send (ch->mq,
+                    env);
+  }
+}
+
+
+/**
  * Task we run periodically to scan for network interfaces.
  *
  * @param cls NULL
@@ -1041,7 +1113,11 @@ run_scan (void *cls)
                          (AF_INET == lal->af)
                          ? sizeof (struct sockaddr_in)
                          : sizeof (struct sockaddr_in6))) )
+      {
         found = GNUNET_YES;
+        pos->hc = lal->hc;
+        lal->hc = NULL;
+      }
     }
     if (GNUNET_NO == found)
       notify_clients (lal,
@@ -1072,6 +1148,17 @@ run_scan (void *cls)
     if (GNUNET_NO == found)
       notify_clients (pos,
                       GNUNET_YES);
+    if ( (AF_INET == pos->af) &&
+         (NULL == pos->hc) &&
+         (0 != (GNUNET_NAT_AC_LAN & pos->ac)) )
+    {
+      const struct sockaddr_in *s4
+        = (const struct sockaddr_in *) &pos->addr;
+      
+      pos->hc = GN_start_gnunet_nat_server_ (&s4->sin_addr,
+                                             &reversal_callback,
+                                             pos);
+    }
   }
   if ( (GNUNET_YES == have_nat) &&
        (GNUNET_YES == enable_upnp) &&
@@ -1164,12 +1251,12 @@ upnp_addr_change_cb (void *cls,
   {
   case AF_INET:
     ac = is_nat_v4 (&((const struct sockaddr_in *) addr)->sin_addr)
-      ? GNUNET_NAT_AC_LAN_PRIVATE
+      ? GNUNET_NAT_AC_LAN
       : GNUNET_NAT_AC_EXTERN;
     break;
   case AF_INET6:
     ac = is_nat_v6 (&((const struct sockaddr_in6 *) addr)->sin6_addr)
-      ? GNUNET_NAT_AC_LAN_PRIVATE
+      ? GNUNET_NAT_AC_LAN
       : GNUNET_NAT_AC_EXTERN;
     break;
   default:
diff --git a/src/nat/gnunet-service-nat_helper.c b/src/nat/gnunet-service-nat_helper.c
index 379603ae2..febc3c2dd 100644
--- a/src/nat/gnunet-service-nat_helper.c
+++ b/src/nat/gnunet-service-nat_helper.c
@@ -39,7 +39,7 @@ struct HelperContext
   /**
    * IP address we pass to the NAT helper.
    */
-  const char *internal_address;
+  struct in_addr internal_address;
 
   /**
    * Function to call if we receive a reversal request.
@@ -220,6 +220,7 @@ restart_nat_server (void *cls)
 {
   struct HelperContext *h = cls;
   char *binary;
+  char ia[INET_ADDRSTRLEN];
   
   h->server_read_task = NULL;
   h->server_stdout 
@@ -232,10 +233,15 @@ restart_nat_server (void *cls)
     try_again (h);
     return;
   }
+  GNUNET_assert (NULL !=
+                 inet_ntop (AF_INET,
+                            &h->internal_address,
+                            ia,
+                            sizeof (ia)));
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Starting `%s' at `%s'\n",
        "gnunet-helper-nat-server",
-       h->internal_address);
+       ia);
   /* Start the server process */
   binary
     = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-server");
@@ -247,7 +253,7 @@ restart_nat_server (void *cls)
                                NULL,
                                binary,
                                "gnunet-helper-nat-server",
-                               h->internal_address,
+                               ia,
                                NULL);
   GNUNET_free (binary);
   if (NULL == h->server_proc)
@@ -285,7 +291,7 @@ restart_nat_server (void *cls)
  * @return NULL on error
  */
 struct HelperContext *
-GN_start_gnunet_nat_server_ (const char *internal_address,
+GN_start_gnunet_nat_server_ (const struct in_addr *internal_address,
                              GN_ReversalCallback cb,
                              void *cb_cls)
 {
@@ -294,8 +300,7 @@ GN_start_gnunet_nat_server_ (const char *internal_address,
   h = GNUNET_new (struct HelperContext);
   h->cb = cb;
   h->cb_cls = cb_cls;
-  h->internal_address
-    = internal_address;
+  h->internal_address = *internal_address;
   if (NULL == h->server_stdout)
   {
     GN_stop_gnunet_nat_server_ (h);
diff --git a/src/nat/gnunet-service-nat_helper.h b/src/nat/gnunet-service-nat_helper.h
index d3f1a757c..026526b05 100644
--- a/src/nat/gnunet-service-nat_helper.h
+++ b/src/nat/gnunet-service-nat_helper.h
@@ -56,7 +56,7 @@ typedef void
  * @return NULL on error
  */
 struct HelperContext *
-GN_start_gnunet_nat_server_ (const char *internal_address,
+GN_start_gnunet_nat_server_ (const struct in_addr *internal_address,
                              GN_ReversalCallback cb,
                              void *cb_cls);
 
diff --git a/src/nat/nat.h b/src/nat/nat.h
index 3356b19ce..5cb1c1050 100644
--- a/src/nat/nat.h
+++ b/src/nat/nat.h
@@ -191,19 +191,7 @@ struct GNUNET_NAT_ConnectionReversalRequestedMessage
    */
   struct GNUNET_MessageHeader header;
 
-  /**
-   * Size of the local address where we received the request, in NBO.
-   */
-  uint16_t local_addr_size;
-
-  /**
-   * Size of the remote address making the request, in NBO.
-   */
-  uint16_t remote_addr_size;
-
-  /* followed by a `struct sockaddr` of @e local_addr_size bytes */
-
-  /* followed by a `struct sockaddr` of @e remote_addr_size bytes */
+  /* followed by a `struct sockaddr_in` */
   
 };
 
diff --git a/src/nat/nat_api.c b/src/nat/nat_api.c
index 481bc6fde..ab36d6162 100644
--- a/src/nat/nat_api.c
+++ b/src/nat/nat_api.c
@@ -176,14 +176,7 @@ check_connection_reversal_request (void *cls,
 {
   if (ntohs (crm->header.size) !=
       sizeof (*crm) +
-      ntohs (crm->local_addr_size) +
-      ntohs (crm->remote_addr_size) )
-  {
-    GNUNET_break (0);
-    return GNUNET_SYSERR;
-  }
-  if ( (sizeof (struct sockaddr_in) != ntohs (crm->local_addr_size)) ||
-       (sizeof (struct sockaddr_in) != ntohs (crm->remote_addr_size)) )
+      sizeof (struct sockaddr_in) )
   {
     GNUNET_break (0);
     return GNUNET_SYSERR;
@@ -203,13 +196,9 @@ handle_connection_reversal_request (void *cls,
                                     const struct GNUNET_NAT_ConnectionReversalRequestedMessage *crm)
 {
   struct GNUNET_NAT_Handle *nh = cls;
-  const struct sockaddr_in *local_sa = (const struct sockaddr_in *) &crm[1];
-  const struct sockaddr_in *remote_sa = &local_sa[1];
 
   nh->reversal_callback (nh->callback_cls,
-                         (const struct sockaddr *) local_sa,
-                         sizeof (struct sockaddr_in),
-                         (const struct sockaddr *) remote_sa,
+                         (const struct sockaddr *) &crm[1],
                          sizeof (struct sockaddr_in));
 }