really minimizing code running with root rights

2 files changed, 88 insertions, 72 deletions

diff --git a/src/nat/gnunet-helper-nat-server-windows.c b/src/nat/gnunet-helper-nat-server-windows.c
index b15752d8e..d970ffd70 100644
--- a/src/nat/gnunet-helper-nat-server-windows.c
+++ b/src/nat/gnunet-helper-nat-server-windows.c
@@ -71,6 +71,11 @@
 #define NAT_TRAV_PORT 22225
 
 /**
+ * Must match packet ID used by gnunet-helper-nat-client.c
+ */
+#define PACKET_ID 256
+
+/**
  * TTL to use for our outgoing messages.
  */
 #define IPDEFTTL 64
@@ -265,7 +270,7 @@ send_icmp_echo (const struct in_addr *my_ip)
   ip_pkt.vers_ihl = 0x45;
   ip_pkt.tos = 0;
   ip_pkt.pkt_len = htons (sizeof (packet));
-  ip_pkt.id = htons (256);
+  ip_pkt.id = htons (PACKET_ID);
   ip_pkt.flags_frag_offset = 0;
   ip_pkt.ttl = IPDEFTTL;
   ip_pkt.proto = IPPROTO_ICMP;
diff --git a/src/nat/gnunet-helper-nat-server.c b/src/nat/gnunet-helper-nat-server.c
index e57319d00..40860cb41 100644
--- a/src/nat/gnunet-helper-nat-server.c
+++ b/src/nat/gnunet-helper-nat-server.c
@@ -415,64 +415,28 @@ process_icmp_response ()
 
 
 /**
- * Create an ICMP raw socket for reading.
+ * Fully initialize the raw socket.
  *
- * @return -1 on error
+ * @return -1 on error, 0 on success
  */
 static int
-make_icmp_socket ()
-{
-  int ret;
-
-  ret = socket (AF_INET, SOCK_RAW, IPPROTO_ICMP);
-  if (-1 == ret)
-  {
-    fprintf (stderr, "Error opening RAW socket: %s\n", strerror (errno));
-    return -1;
-  }
-  if (ret >= FD_SETSIZE)
-  {
-    fprintf (stderr, "Socket number too large (%d > %u)\n", ret,
-             (unsigned int) FD_SETSIZE);
-    (void) close (ret);
-    return -1;
-  }
-  return ret;
-}
-
-
-/**
- * Create an ICMP raw socket for writing.
- *
- * @return -1 on error
- */
-static int
-make_raw_socket ()
+setup_raw_socket ()
 {
   const int one = 1;
-  int ret;
 
-  ret = socket (AF_INET, SOCK_RAW, IPPROTO_RAW);
-  if (-1 == ret)
-  {
-    fprintf (stderr, "Error opening RAW socket: %s\n", strerror (errno));
-    return -1;
-  }
   if (-1 ==
-      setsockopt (ret, SOL_SOCKET, SO_BROADCAST, (char *) &one, sizeof (one)))
+      setsockopt (rawsock, SOL_SOCKET, SO_BROADCAST, (char *) &one, sizeof (one)))
   {
     fprintf (stderr, "setsockopt failed: %s\n", strerror (errno));
-    (void) close (ret);
     return -1;
   }
   if (-1 ==
-      setsockopt (ret, IPPROTO_IP, IP_HDRINCL, (char *) &one, sizeof (one)))
+      setsockopt (rawsock, IPPROTO_IP, IP_HDRINCL, (char *) &one, sizeof (one)))
   {
     fprintf (stderr, "setsockopt failed: %s\n", strerror (errno));
-    (void) close (ret);
     return -1;
   }
-  return ret;
+  return 0;
 }
 
 
@@ -521,56 +485,97 @@ main (int argc, char *const *argv)
   struct timeval tv;
   uid_t uid;
   unsigned int alt;
+  int icmp_eno;
+  int raw_eno;
+  int global_ret;
+
+  /* Create an ICMP raw socket for reading (we'll check errors later) */
+  icmpsock = socket (AF_INET, SOCK_RAW, IPPROTO_ICMP);
+  icmp_eno = errno;
 
+  /* Create an (ICMP) raw socket for writing (we'll check errors later) */
+  rawsock = socket (AF_INET, SOCK_RAW, IPPROTO_RAW);
+  raw_eno = errno;
+  udpsock = -1;
+
+  /* drop root rights */
+  uid = getuid ();
+#ifdef HAVE_SETRESUID
+  if (0 != setresuid (uid, uid, uid))
+  {
+    fprintf (stderr, "Failed to setresuid: %s\n", strerror (errno));
+    global_ret = 1;
+    goto error_exit;
+  }
+#else
+  if (0 != (setuid (uid) | seteuid (uid)))
+  {
+    fprintf (stderr, "Failed to setuid: %s\n", strerror (errno));
+    global_ret = 2;
+    goto error_exit;
+  }
+#endif
+
+  /* Now that we run without root rights, we can do error checking... */
   if (2 != argc)
   {
     fprintf (stderr,
              "This program must be started with our (internal NAT) IP as the only argument.\n");
-    return 1;
+    global_ret = 3;
+    goto error_exit;
   }
   if (1 != inet_pton (AF_INET, argv[1], &external))
   {
     fprintf (stderr, "Error parsing IPv4 address: %s\n", strerror (errno));
-    return 1;
+    global_ret = 4;
+    goto error_exit;
   }
   if (1 != inet_pton (AF_INET, DUMMY_IP, &dummy))
   {
     fprintf (stderr, "Internal error converting dummy IP to binary.\n");
-    return 2;
+    global_ret = 5;
+    goto error_exit;
   }
-  if (-1 == (icmpsock = make_icmp_socket ()))
+
+  /* error checking icmpsock */
+  if (-1 == icmpsock)
   {
-    return 3;
+    fprintf (stderr, "Error opening RAW socket: %s\n", strerror (icmp_eno));
+    global_ret = 6;
+    goto error_exit;
   }
-  if (-1 == (rawsock = make_raw_socket ()))
+  if (icmpsock >= FD_SETSIZE)
   {
-    (void) close (icmpsock);
-    return 4;
+    /* this could happen if we were started with a large number of already-open
+       file descriptors... */
+    fprintf (stderr, "Socket number too large (%d > %u)\n", icmpsock,
+             (unsigned int) FD_SETSIZE);
+    global_ret = 7;
+    goto error_exit;
   }
-  uid = getuid ();
-#ifdef HAVE_SETRESUID
-  if (0 != setresuid (uid, uid, uid))
+
+  /* error checking rawsock */
+  if (-1 == rawsock)
   {
-    fprintf (stderr, "Failed to setresuid: %s\n", strerror (errno));
-    (void) close (icmpsock);
-    (void) close (rawsock);
-    return 5;
+    fprintf (stderr, "Error opening RAW socket: %s\n", strerror (raw_eno));
+    global_ret = 8;
+    goto error_exit;
   }
-#else
-  if (0 != (setuid (uid) | seteuid (uid)))
+  /* no need to check 'rawsock' against FD_SETSIZE as it is never used
+     with 'select' */
+
+  if (0 != setup_raw_socket ())
   {
-    fprintf (stderr, "Failed to setuid: %s\n", strerror (errno));
-    (void) close (icmpsock);
-    (void) close (rawsock);
-    return 6;
+    global_ret = 9;
+    goto error_exit;
   }
-#endif
+
   if (-1 == (udpsock = make_udp_socket (&external)))
   {
-    (void) close (icmpsock);
-    (void) close (rawsock);
-    return 7;
+    global_ret = 10;
+    goto error_exit;
   }
+
   alt = 0;
   while (1)
   {
@@ -594,11 +599,17 @@ main (int argc, char *const *argv)
     else
       send_udp ();
   }
+
   /* select failed (internal error or OS out of resources) */
-  (void) close (icmpsock);
-  (void) close (rawsock);
-  (void) close (udpsock);
-  return 8;
+  global_ret = 11; 
+error_exit:
+  if (-1 != icmpsock)
+    (void) close (icmpsock);
+  if (-1 != rawsock)
+    (void) close (rawsock);
+  if (-1 != udpsock)
+    (void) close (udpsock);
+  return global_ret;
 }