- added many lots of new error checking to nat_auto and nat_mini. icmp_client server and client not contained in this commit.

2 files changed, 86 insertions, 32 deletions

diff --git a/src/nat/nat_auto.c b/src/nat/nat_auto.c
index 184f23538..b53fd0d47 100644
--- a/src/nat/nat_auto.c
+++ b/src/nat/nat_auto.c
@@ -141,6 +141,10 @@ struct GNUNET_NAT_AutoHandle
    */
   int have_v6;
 
+  /**
+   * Error code for better debugging and user feedback
+   */
+  enum GNUNET_NAT_FailureCode ret;
 };
 
 
@@ -166,6 +170,7 @@ fail_timeout (void *cls,
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
 
+  ah->ret = GNUNET_NAT_ERROR_NAT_TEST_TIMEOUT; 
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               _("NAT traversal with ICMP Server timed out.\n"));
   GNUNET_assert (NULL != ah->tst);
@@ -227,6 +232,7 @@ reversal_test (void *cls,
                                    &result_callback, ah);
   if (NULL == ah->tst)
   {
+    ah->ret = GNUNET_NAT_ERROR_NAT_TEST_START_FAILED;
     next_phase (ah);
     return;
   }
@@ -243,6 +249,10 @@ static void
 test_online (struct GNUNET_NAT_AutoHandle *ah)
 {
   // FIXME: not implemented
+  /*
+   * if (failure)
+   *  ah->ret = GNUNET_NAT_ERROR_NOT_ONLINE;
+   */
   next_phase (ah);
 }
 
@@ -263,7 +273,8 @@ set_external_ipv4 (void *cls,
   char buf[INET_ADDRSTRLEN];
 
   ah->eh = NULL;
-  if (NULL == addr)
+  ah->ret = ret;
+  if (GNUNET_NAT_ERROR_SUCCESS != ret)
   {
     next_phase (ah);
     return;
@@ -281,6 +292,11 @@ set_external_ipv4 (void *cls,
   if (NULL == inet_ntop (AF_INET, addr, buf, sizeof (buf)))
   {
     GNUNET_break (0);
+    /* actually, this should never happen, as the caller already executed just
+     * this check, but for consistency (eg: future changes in the caller) 
+     * we still need to report this error...
+     */
+    ah->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_ADDRESS_INVALID;
     next_phase (ah);
     return;
   }
@@ -298,6 +314,9 @@ set_external_ipv4 (void *cls,
 static void
 test_external_ip (struct GNUNET_NAT_AutoHandle *ah)
 {
+  if (GNUNET_NAT_ERROR_SUCCESS != ah->ret)
+    next_phase (ah);
+  
   // FIXME: CPS?
   /* try to detect external IP */
   ah->eh = GNUNET_NAT_mini_get_external_ipv4 (TIMEOUT,
@@ -319,7 +338,7 @@ test_external_ip (struct GNUNET_NAT_AutoHandle *ah)
  * @return GNUNET_OK to continue iteration, #GNUNET_SYSERR to abort
  */
 static int
-nipo (void *cls,
+process_if (void *cls,
       const char *name,
       int isDefault,
       const struct sockaddr *addr,
@@ -358,6 +377,7 @@ nipo (void *cls,
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               _("Detected internal network address `%s'.\n"),
               buf);
+  ah->ret = GNUNET_NAT_ERROR_SUCCESS;
   /* no need to continue iteration */
   return GNUNET_SYSERR;
 }
@@ -372,7 +392,9 @@ static void
 test_local_ip (struct GNUNET_NAT_AutoHandle *ah)
 {
   ah->have_v6 = GNUNET_NO;
-  GNUNET_OS_network_interfaces_list (&nipo, ah);
+  ah->ret = GNUNET_NAT_ERROR_NO_VALID_IF_IP_COMBO; // reset to success if any of the IFs in below iterator has a valid IP
+  GNUNET_OS_network_interfaces_list (&process_if, ah);
+  
   GNUNET_CONFIGURATION_set_value_string (ah->cfg, "nat", "DISABLEV6",
                                          (GNUNET_YES == ah->have_v6) ? "NO" : "YES");
   next_phase (ah);
@@ -387,7 +409,11 @@ test_local_ip (struct GNUNET_NAT_AutoHandle *ah)
 static void
 test_nat_punched (struct GNUNET_NAT_AutoHandle *ah)
 {
+  if (GNUNET_NAT_ERROR_SUCCESS != ah->ret)
+    next_phase (ah);
+  
   // FIXME: not implemented
+  
   next_phase (ah);
 }
 
@@ -402,6 +428,9 @@ test_upnpc (struct GNUNET_NAT_AutoHandle *ah)
 {
   int have_upnpc;
 
+  if (GNUNET_NAT_ERROR_SUCCESS != ah->ret)
+    next_phase (ah);
+  
   /* test if upnpc is available */
   have_upnpc = (GNUNET_SYSERR !=
                 GNUNET_OS_check_helper_binary ("upnpc", GNUNET_NO, NULL));
@@ -425,27 +454,44 @@ test_upnpc (struct GNUNET_NAT_AutoHandle *ah)
 static void
 test_icmp_server (struct GNUNET_NAT_AutoHandle *ah)
 {
-  int hns;
+  int ext_ip;
+  int nated;
+  int binary;
   char *tmp;
-  char *binary;
-
+  char *helper;
+  ext_ip = GNUNET_NO;
+  nated = GNUNET_NO;
+  binary = GNUNET_NO;
+  
   tmp = NULL;
-  binary = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-server");
-  hns =
-      ((GNUNET_OK ==
+  helper = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-server");
+  if ((GNUNET_OK ==
         GNUNET_CONFIGURATION_get_value_string (ah->cfg, "nat", "EXTERNAL_ADDRESS",
-                                               &tmp)) && (0 < strlen (tmp)) &&
-       (GNUNET_YES ==
-        GNUNET_CONFIGURATION_get_value_yesno (ah->cfg, "nat", "BEHIND_NAT")) &&
-       (GNUNET_YES ==
-        GNUNET_OS_check_helper_binary (binary, GNUNET_YES, "-d 127.0.0.1" ))); // use localhost as source for that one udp-port, ok for testing
+                                               &tmp)) && (0 < strlen (tmp))){
+    ext_ip = GNUNET_OK;
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("test_icmp_server not possible, as we have no public IPv4 address\n"));
+  }
+  else
+    goto err;
+    
+  if (GNUNET_YES ==
+        GNUNET_CONFIGURATION_get_value_yesno (ah->cfg, "nat", "BEHIND_NAT")){
+    nated = GNUNET_YES;
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("test_icmp_server not possible, as we are not behind NAT\n"));
+  }
+  else
+    goto err;
+  
+  if (GNUNET_YES ==
+        GNUNET_OS_check_helper_binary (helper, GNUNET_YES, "-d 127.0.0.1" )){
+    binary = GNUNET_OK; // use localhost as source for that one udp-port, ok for testing
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("No working gnunet-helper-nat-server found\n"));
+  }
+err:
   GNUNET_free_non_null (tmp);
-  GNUNET_free (binary);
-  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              (hns)
-              ? _("gnunet-helper-nat-server found, testing it\n")
-              : _("No working gnunet-helper-nat-server found\n"));
-  if (hns)
+  GNUNET_free (helper);
+
+  if (GNUNET_OK == ext_ip && GNUNET_YES == nated && GNUNET_OK == binary)
     ah->task = GNUNET_SCHEDULER_add_now (&reversal_test, ah);
   else
     next_phase (ah);
@@ -551,6 +597,7 @@ GNUNET_NAT_autoconfig_start (const struct GNUNET_CONFIGURATION_Handle *cfg,
   ah = GNUNET_new (struct GNUNET_NAT_AutoHandle);
   ah->fin_cb = cb;
   ah->fin_cb_cls = cb_cls;
+  ah->ret = GNUNET_NAT_ERROR_SUCCESS;
   ah->cfg = GNUNET_CONFIGURATION_dup (cfg);
   ah->initial_cfg = GNUNET_CONFIGURATION_dup (cfg);
 
diff --git a/src/nat/nat_mini.c b/src/nat/nat_mini.c
index 27a1361b2..ae15b5c13 100644
--- a/src/nat/nat_mini.c
+++ b/src/nat/nat_mini.c
@@ -98,6 +98,10 @@ struct GNUNET_NAT_ExternalHandle
    */
   char buf[17];
 
+  /**
+   * Error code for better debugging and user feedback
+   */
+  enum GNUNET_NAT_FailureCode ret;
 };
 
 
@@ -115,15 +119,16 @@ read_external_ipv4 (void *cls,
   struct GNUNET_NAT_ExternalHandle *eh = cls;
   ssize_t ret;
   struct in_addr addr;
-  int iret;
 
   eh->task = GNUNET_SCHEDULER_NO_TASK;
   if (GNUNET_YES == GNUNET_NETWORK_fdset_handle_isset (tc->read_ready, eh->r))
     ret =
         GNUNET_DISK_file_read (eh->r, &eh->buf[eh->off],
                                sizeof (eh->buf) - eh->off);
-  else
+  else {
+    eh->ret = GNUNET_NAT_ERROR_IPC_FAILURE;
     ret = -1;                   /* error reading, timeout, etc. */
+  }
   if (ret > 0)
   {
     /* try to read more */
@@ -134,23 +139,21 @@ read_external_ipv4 (void *cls,
                                         &read_external_ipv4, eh);
     return;
   }
-  iret = GNUNET_NO;
+  eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_OUTPUT_INVALID;
   if ((eh->off > 7) && (eh->buf[eh->off - 1] == '\n'))
   {
     eh->buf[eh->off - 1] = '\0';
     if (1 == inet_pton (AF_INET, eh->buf, &addr))
     {
-      if (0 == addr.s_addr)
-        iret = GNUNET_NO;       /* got 0.0.0.0 */
+      if (0 != addr.s_addr)
+        eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_ADDRESS_INVALID;       /* got 0.0.0.0 */
       else
-        iret = GNUNET_OK;
+        eh->ret = GNUNET_NAT_ERROR_SUCCESS;
     }
   }
   eh->cb (eh->cb_cls,
-          (GNUNET_OK == iret)
-          ? &addr :
-          NULL,
-          GNUNET_NAT_ERROR_EXTERNAL_IP_NO_VALID_ADDRESS_FOUND);
+          (GNUNET_NAT_ERROR_SUCCESS == eh->ret) ? &addr : NULL,
+          eh->ret);
   GNUNET_NAT_mini_get_external_ipv4_cancel (eh);
 }
 
@@ -170,7 +173,7 @@ signal_external_ip_error (void *cls,
   eh->task = GNUNET_SCHEDULER_NO_TASK;
   eh->cb (eh->cb_cls,
           NULL,
-          GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_FOUND);
+          eh->ret);
   GNUNET_free (eh);
 }
 
@@ -181,7 +184,7 @@ signal_external_ip_error (void *cls,
  * @param timeout when to fail
  * @param cb function to call with result
  * @param cb_cls closure for @a cb
- * @return handle for cancellation (can only be used until @a cb is called), NULL on error
+ * @return handle for cancellation (can only be used until @a cb is called), never NULL
  */
 struct GNUNET_NAT_ExternalHandle *
 GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
@@ -192,11 +195,13 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
   eh = GNUNET_new (struct GNUNET_NAT_ExternalHandle);
   eh->cb = cb;
   eh->cb_cls = cb_cls;
+  eh->ret = GNUNET_NAT_ERROR_SUCCESS;
   if (GNUNET_SYSERR ==
       GNUNET_OS_check_helper_binary ("external-ip", GNUNET_NO, NULL))
   {
     LOG (GNUNET_ERROR_TYPE_INFO,
          _("`external-ip' command not found\n"));
+    eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_FOUND;
     eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
                                          eh);
     return eh;
@@ -206,6 +211,7 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
   eh->opipe = GNUNET_DISK_pipe (GNUNET_YES, GNUNET_YES, GNUNET_NO, GNUNET_YES);
   if (NULL == eh->opipe)
   {
+    eh->ret = GNUNET_NAT_ERROR_IPC_FAILURE;
     eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
                                          eh);
     return eh;
@@ -217,6 +223,7 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
   if (NULL == eh->eip)
   {
     GNUNET_DISK_pipe_close (eh->opipe);
+    eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_EXECUTEABLE;
     eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
                                          eh);
     return eh;