On Sun, Apr 17, 2016 at 10:37:26PM +0200, Christian Grothoff wrote:
> Hi Daniel,
> 
> I think a command-line argument is fine, just don't introduce
> getopt()-style parsing into the SUID binaries ;-).

That's why I thought of a configure option to just not include
that functionality in the SUID binary in first place.
The diffstat to introduce that option at runtime also isn't as small
as I was hoping it would end up being...

As gnunet-helper-exit already got a parameter to specify the NAT
interface I reused that parameter and also skip sysctl for v4/v6
ip_forwarding if it isn't supplied.
For gnunet-helper-dns I needed to introduce a new parameter and pass
it from gnunet-service-dns which checks if the config option
SKIP_ROUTING_SETUP is set.

Find the patches for dns and exit attached, I'd be glad if you can
review them before I commit my changes.

Cheers

Daniel

> 
> Happy hacking!
> 
> -Christian
> 
> On 04/17/2016 10:21 PM, Daniel Golle wrote:
> > Hi!
> > 
> > I'm currently working on improving IPvX-over-GNUnet on OpenWrt.
> > I believe that providing v4/v6/DNS exit service using an OpenWrt box
> > is a quite good idea.
> > On OpenWrt it doesn't make so much sense to mess around with routing,
> > sysctl and iptables rules in the helpers as networking and firewall are
> > managed by OpenWrt's services. The situation is also different from a
> > desktop system because on an embedded device (think e.g.:
> > IPvX-over-GNUnet router) the networking and firewall configuration
> > corresponds to a specific use (think: tunneling all traffic through
> > GNUnet) and do exactly that. To me it seems desirable to have an
> > additional parameter (or even a compile-time configure argument!) for
> > the dns- and exit-helpers to make them stay away from routing, sysctl
> > and firewall stuff and just assume that an external service will handle
> > all that once the interface comes up (because that's what netifd does
> > on OpenWrt).
> > Depending on your preference (additional cmdline parameter vs.
> > compile-time), I'd like to introduce that option, so EXIT will be more
> > useful to provide gateways to the ARPA internet in community mesh
> > networks -- that's the main application for most of them and GNUnet
> > could already offer a decentralized and more secure way to do that.
> > 
> > Cheers
> > 
> > Daniel
> > 
> > _______________________________________________
> > GNUnet-developers mailing list
> > [email protected]
> > https://lists.gnu.org/mailman/listinfo/gnunet-developers
> > 
> 




> _______________________________________________
> GNUnet-developers mailing list
> [email protected]
> https://lists.gnu.org/mailman/listinfo/gnunet-developers

>From 84087abe25a5aefddbc7d90de10eca948b2c1964 Mon Sep 17 00:00:00 2001
From: Daniel Golle <[email protected]>
Date: Mon, 18 Apr 2016 02:22:38 +0200
Subject: [PATCH 1/2] dns: add support for skipping the routing setup

---
 src/dns/gnunet-helper-dns.c  | 168 ++++++++++++++++++++++---------------------
 src/dns/gnunet-service-dns.c |  15 +++-
 2 files changed, 100 insertions(+), 83 deletions(-)

diff --git a/src/dns/gnunet-helper-dns.c b/src/dns/gnunet-helper-dns.c
index b36972a..58876b2 100644
--- a/src/dns/gnunet-helper-dns.c
+++ b/src/dns/gnunet-helper-dns.c
@@ -709,6 +709,7 @@ PROCESS_BUFFER:
  *             3: IPv6 netmask length in bits ("64")
  *             4: IPv4 address for the tunnel ("1.2.3.4")
  *             5: IPv4 netmask ("255.255.0.0")
+ *             6: skip sysctl, routing and iptables setup ("0")
  * @return 0 on success, otherwise code indicating type of error:
  *         1 wrong number of arguments
  *         2 invalid arguments (i.e. port number / prefix length wrong)
@@ -733,8 +734,9 @@ main (int argc, char *const*argv)
   char mygid[32];
   int fd_tun;
   uid_t uid;
+  int nortsetup = 0;
 
-  if (6 != argc)
+  if (7 != argc)
   {
     fprintf (stderr, "Fatal: must supply 6 arguments!\n");
     return 1;
@@ -755,40 +757,44 @@ main (int argc, char *const*argv)
     return 254;
   }
 #endif
-
-  /* verify that the binaries were care about are executable */
-  if (0 == access ("/sbin/iptables", X_OK))
-    sbin_iptables = "/sbin/iptables";
-  else if (0 == access ("/usr/sbin/iptables", X_OK))
-    sbin_iptables = "/usr/sbin/iptables";
-  else
-  {
-    fprintf (stderr,
-	     "Fatal: executable iptables not found in approved directories: %s\n",
-	     strerror (errno));
-    return 3;
-  }
-  if (0 == access ("/sbin/ip", X_OK))
-    sbin_ip = "/sbin/ip";
-  else if (0 == access ("/usr/sbin/ip", X_OK))
-    sbin_ip = "/usr/sbin/ip";
-  else
-  {
-    fprintf (stderr,
-	     "Fatal: executable ip not found in approved directories: %s\n",
-	     strerror (errno));
-    return 4;
-  }
-  if (0 == access ("/sbin/sysctl", X_OK))
-    sbin_sysctl = "/sbin/sysctl";
-  else if (0 == access ("/usr/sbin/sysctl", X_OK))
-    sbin_sysctl = "/usr/sbin/sysctl";
-  else
-  {
-    fprintf (stderr,
-             "Fatal: executable sysctl not found in approved directories: %s\n",
-             strerror (errno));
-    return 5;
+  if (0 == strncmp(argv[6], "1", 2))
+    nortsetup = 1;
+
+  if (0 == nortsetup) {
+    /* verify that the binaries we care about are executable */
+    if (0 == access ("/sbin/iptables", X_OK))
+      sbin_iptables = "/sbin/iptables";
+    else if (0 == access ("/usr/sbin/iptables", X_OK))
+      sbin_iptables = "/usr/sbin/iptables";
+    else
+    {
+      fprintf (stderr,
+	       "Fatal: executable iptables not found in approved directories: %s\n",
+	       strerror (errno));
+      return 3;
+    }
+    if (0 == access ("/sbin/ip", X_OK))
+      sbin_ip = "/sbin/ip";
+    else if (0 == access ("/usr/sbin/ip", X_OK))
+      sbin_ip = "/usr/sbin/ip";
+    else
+    {
+      fprintf (stderr,
+	       "Fatal: executable ip not found in approved directories: %s\n",
+	       strerror (errno));
+      return 4;
+    }
+    if (0 == access ("/sbin/sysctl", X_OK))
+      sbin_sysctl = "/sbin/sysctl";
+    else if (0 == access ("/usr/sbin/sysctl", X_OK))
+      sbin_sysctl = "/usr/sbin/sysctl";
+    else
+    {
+      fprintf (stderr,
+               "Fatal: executable sysctl not found in approved directories: %s\n",
+               strerror (errno));
+      return 5;
+    }
   }
 
   /* setup 'mygid' string */
@@ -858,7 +864,7 @@ main (int argc, char *const*argv)
   dev[IFNAMSIZ - 1] = '\0';
 
   /* Disable rp filtering */
-  {
+  if (0 == nortsetup) {
     char *const sysctl_args[] = {"sysctl", "-w",
       "net.ipv4.conf.all.rp_filter=0", NULL};
     char *const sysctl_args2[] = {"sysctl", "-w",
@@ -921,46 +927,48 @@ main (int argc, char *const*argv)
   /* Forward everything from our EGID (which should only be held
      by the 'gnunet-service-dns') and with destination
      to port 53 on UDP, without hijacking */
-  r = 8; /* failed to fully setup routing table */
-  {
-    char *const mangle_args[] =
-      {
-	"iptables", "-m", "owner", "-t", "mangle", "-I", "OUTPUT", "1", "-p",
-	"udp", "--gid-owner", mygid, "--dport", DNS_PORT, "-j",
-	"ACCEPT", NULL
-      };
-    if (0 != fork_and_exec (sbin_iptables, mangle_args))
-      goto cleanup_rest;
-  }
-  /* Mark all of the other DNS traffic using our mark DNS_MARK */
-  {
-    char *const mark_args[] =
-      {
-	"iptables", "-t", "mangle", "-I", "OUTPUT", "2", "-p",
-	"udp", "--dport", DNS_PORT, "-j", "MARK", "--set-mark", DNS_MARK,
-	NULL
-      };
-    if (0 != fork_and_exec (sbin_iptables, mark_args))
-      goto cleanup_mangle_1;
-  }
-  /* Forward all marked DNS traffic to our DNS_TABLE */
-  {
-    char *const forward_args[] =
-      {
-	"ip", "rule", "add", "fwmark", DNS_MARK, "table", DNS_TABLE, NULL
-      };
-    if (0 != fork_and_exec (sbin_ip, forward_args))
-      goto cleanup_mark_2;
-  }
-  /* Finally, add rule in our forwarding table to pass to our virtual interface */
-  {
-    char *const route_args[] =
-      {
-	"ip", "route", "add", "default", "dev", dev,
-	"table", DNS_TABLE, NULL
-      };
-    if (0 != fork_and_exec (sbin_ip, route_args))
-      goto cleanup_forward_3;
+  if (0 == nortsetup) {
+    r = 8; /* failed to fully setup routing table */
+    {
+      char *const mangle_args[] =
+        {
+	 "iptables", "-m", "owner", "-t", "mangle", "-I", "OUTPUT", "1", "-p",
+	 "udp", "--gid-owner", mygid, "--dport", DNS_PORT, "-j",
+	 "ACCEPT", NULL
+        };
+      if (0 != fork_and_exec (sbin_iptables, mangle_args))
+        goto cleanup_rest;
+    }
+    /* Mark all of the other DNS traffic using our mark DNS_MARK */
+    {
+      char *const mark_args[] =
+        {
+	 "iptables", "-t", "mangle", "-I", "OUTPUT", "2", "-p",
+	 "udp", "--dport", DNS_PORT, "-j", "MARK", "--set-mark", DNS_MARK,
+	 NULL
+        };
+      if (0 != fork_and_exec (sbin_iptables, mark_args))
+        goto cleanup_mangle_1;
+    }
+    /* Forward all marked DNS traffic to our DNS_TABLE */
+    {
+      char *const forward_args[] =
+        {
+	 "ip", "rule", "add", "fwmark", DNS_MARK, "table", DNS_TABLE, NULL
+        };
+      if (0 != fork_and_exec (sbin_ip, forward_args))
+        goto cleanup_mark_2;
+    }
+    /* Finally, add rule in our forwarding table to pass to our virtual interface */
+    {
+      char *const route_args[] =
+        {
+	 "ip", "route", "add", "default", "dev", dev,
+	 "table", DNS_TABLE, NULL
+        };
+      if (0 != fork_and_exec (sbin_ip, route_args))
+        goto cleanup_forward_3;
+    }
   }
 
   /* drop privs *except* for the saved UID; this is not perfect, but better
@@ -1007,7 +1015,7 @@ main (int argc, char *const*argv)
   /* update routing tables again -- this is why we could not fully drop privs */
   /* now undo updating of routing tables; normal exit or clean-up-on-error case */
  cleanup_route_4:
-  {
+  if (0 == nortsetup) {
     char *const route_clean_args[] =
       {
 	"ip", "route", "del", "default", "dev", dev,
@@ -1017,7 +1025,7 @@ main (int argc, char *const*argv)
       r += 1;
   }
  cleanup_forward_3:
-  {
+  if (0 == nortsetup) {
     char *const forward_clean_args[] =
       {
 	"ip", "rule", "del", "fwmark", DNS_MARK, "table", DNS_TABLE, NULL
@@ -1026,7 +1034,7 @@ main (int argc, char *const*argv)
       r += 2;
   }
  cleanup_mark_2:
-  {
+  if (0 == nortsetup) {
     char *const mark_clean_args[] =
       {
 	"iptables", "-t", "mangle", "-D", "OUTPUT", "-p", "udp",
@@ -1036,7 +1044,7 @@ main (int argc, char *const*argv)
       r += 4;
   }
  cleanup_mangle_1:
-  {
+  if (0 == nortsetup) {
     char *const mangle_clean_args[] =
       {
 	"iptables", "-m", "owner", "-t", "mangle", "-D", "OUTPUT", "-p", "udp",
diff --git a/src/dns/gnunet-service-dns.c b/src/dns/gnunet-service-dns.c
index 0f975e8..972c96f 100644
--- a/src/dns/gnunet-service-dns.c
+++ b/src/dns/gnunet-service-dns.c
@@ -219,7 +219,7 @@ static struct GNUNET_HELPER_Handle *hijacker;
 /**
  * Command-line arguments we are giving to the hijacker process.
  */
-static char *helper_argv[7];
+static char *helper_argv[8];
 
 /**
  * Head of DLL of clients we consult.
@@ -284,7 +284,7 @@ cleanup_task (void *cls GNUNET_UNUSED)
     GNUNET_HELPER_stop (hijacker, GNUNET_NO);
     hijacker = NULL;
   }
-  for (i=0;i<7;i++)
+  for (i=0;i<8;i++)
     GNUNET_free_non_null (helper_argv[i]);
   for (i=0;i<=UINT16_MAX;i++)
     cleanup_rr (&requests[i]);
@@ -1040,6 +1040,7 @@ run (void *cls, struct GNUNET_SERVER_Handle *server,
   struct in6_addr dns_exit6;
   char *dns_exit;
   char *binary;
+  int nortsetup;
 
   cfg = cfg_;
   stats = GNUNET_STATISTICS_create ("dns", cfg);
@@ -1136,7 +1137,15 @@ run (void *cls, struct GNUNET_SERVER_Handle *server,
     return;
   }
   helper_argv[5] = ipv4mask;
-  helper_argv[6] = NULL;
+
+  nortsetup = GNUNET_CONFIGURATION_get_value_yesno (cfg, "dns",
+                                                     "SKIP_ROUTING_SETUP");
+  if (GNUNET_YES == nortsetup)
+    helper_argv[6] = GNUNET_strdup("1");
+  else
+    helper_argv[6] = GNUNET_strdup("0");
+
+  helper_argv[7] = NULL;
   hijacker = GNUNET_HELPER_start (GNUNET_NO,
 				  "gnunet-helper-dns",
 				  helper_argv,
-- 
2.8.0

>From 2ad5df4a359fc2305599bc4d067f9671acfba174 Mon Sep 17 00:00:00 2001
From: Daniel Golle <[email protected]>
Date: Mon, 18 Apr 2016 02:54:08 +0200
Subject: [PATCH 2/2] exit: fully skip routing setup if no interface is
 specified

---
 src/exit/gnunet-helper-exit.c | 84 +++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

diff --git a/src/exit/gnunet-helper-exit.c b/src/exit/gnunet-helper-exit.c
index 7427b5b..2aa9ec9 100644
--- a/src/exit/gnunet-helper-exit.c
+++ b/src/exit/gnunet-helper-exit.c
@@ -667,27 +667,30 @@ main (int argc, char **argv)
     fprintf (stderr, "Fatal: disabling both IPv4 and IPv6 makes no sense.\n");
     return 1;
   }
-  if (0 == access ("/sbin/iptables", X_OK))
-    sbin_iptables = "/sbin/iptables";
-  else if (0 == access ("/usr/sbin/iptables", X_OK))
-    sbin_iptables = "/usr/sbin/iptables";
-  else
+  if (0 != strcmp (argv[2], "-"))
   {
-    fprintf (stderr,
-	     "Fatal: executable iptables not found in approved directories: %s\n",
-	     strerror (errno));
-    return 1;
-  }
-  if (0 == access ("/sbin/sysctl", X_OK))
-    sbin_sysctl = "/sbin/sysctl";
-  else if (0 == access ("/usr/sbin/sysctl", X_OK))
-    sbin_sysctl = "/usr/sbin/sysctl";
-  else
-  {
-    fprintf (stderr,
-	     "Fatal: executable sysctl not found in approved directories: %s\n",
-	     strerror (errno));
-    return 1;
+    if (0 == access ("/sbin/iptables", X_OK))
+      sbin_iptables = "/sbin/iptables";
+    else if (0 == access ("/usr/sbin/iptables", X_OK))
+      sbin_iptables = "/usr/sbin/iptables";
+    else
+    {
+      fprintf (stderr,
+	       "Fatal: executable iptables not found in approved directories: %s\n",
+	       strerror (errno));
+      return 1;
+    }
+    if (0 == access ("/sbin/sysctl", X_OK))
+      sbin_sysctl = "/sbin/sysctl";
+    else if (0 == access ("/usr/sbin/sysctl", X_OK))
+      sbin_sysctl = "/usr/sbin/sysctl";
+    else
+    {
+      fprintf (stderr,
+	       "Fatal: executable sysctl not found in approved directories: %s\n",
+	       strerror (errno));
+      return 1;
+    }
   }
 
   strncpy (dev, argv[1], IFNAMSIZ);
@@ -718,6 +721,7 @@ main (int argc, char **argv)
       }
       set_address6 (dev, address, prefix_len);
     }
+    if (0 != strcmp (argv[2], "-"))
     {
       char *const sysctl_args[] =
 	{
@@ -740,29 +744,31 @@ main (int argc, char **argv)
 
       set_address4 (dev, address, mask);
     }
+    if (0 != strcmp (argv[2], "-"))
     {
-      char *const sysctl_args[] =
-	{
-	  "sysctl", "-w", "net.ipv4.ip_forward=1", NULL
-	};
-      if (0 != fork_and_exec (sbin_sysctl,
-			      sysctl_args))
       {
-	fprintf (stderr,
-		 "Failed to enable IPv4 forwarding.  Will continue anyway.\n");
+        char *const sysctl_args[] =
+	  {
+	    "sysctl", "-w", "net.ipv4.ip_forward=1", NULL
+	  };
+        if (0 != fork_and_exec (sbin_sysctl,
+			        sysctl_args))
+        {
+	  fprintf (stderr,
+		   "Failed to enable IPv4 forwarding.  Will continue anyway.\n");
+        }
       }
-    }
-    if (0 != strcmp (argv[2], "-"))
-    {
-      char *const iptables_args[] =
-	{
-	  "iptables", "-t", "nat", "-A", "POSTROUTING", "-o", argv[2], "-j", "MASQUERADE", NULL
-	};
-      if (0 != fork_and_exec (sbin_iptables,
-			      iptables_args))
       {
-	fprintf (stderr,
-		 "Failed to enable IPv4 masquerading (NAT).  Will continue anyway.\n");
+        char *const iptables_args[] =
+	  {
+	    "iptables", "-t", "nat", "-A", "POSTROUTING", "-o", argv[2], "-j", "MASQUERADE", NULL
+	  };
+        if (0 != fork_and_exec (sbin_iptables,
+			        iptables_args))
+        {
+	  fprintf (stderr,
+		   "Failed to enable IPv4 masquerading (NAT).  Will continue anyway.\n");
+        }
       }
     }
   }
-- 
2.8.0

_______________________________________________
GNUnet-developers mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/gnunet-developers

Reply via email to