[hipl-commit] [trunk] Rev 4457: Major overhaul of the privilege separation code.

  • From: Mircea Gherzan <mircea.gherzan@xxxxxxxxxxxxxx>
  • To: hipl-commit@xxxxxxxxxxxxx
  • Date: Mon, 3 May 2010 19:51:16 +0300

Committer: Mircea Gherzan <mircea.gherzan@xxxxxxxxxxxxxx>
Date: 03/05/2010 at 19:51:16
Revision: 4457
Revision-id: mircea.gherzan@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Branch nick: trunk

Log:
  Major overhaul of the privilege separation code.
  
  The code based on libcap was completely removed, because
  of the low/obsolescent availability of libcap (OpenWRT 
  doesnt have it, Maemo has an ancient version). The
  "altsep" code (based on straight syscalls) is much more
  "portable" across Linux boxes: it just relies on some
  kernel headers, no libraries involved.
  
  Furthermore, privilege separation is no longer a config
  option: it does not make any sense. It belongs to the 
  _runtime_. Therefore it's now enabled by default and can
  be disabled in hipd via the -p command line switch.
  
  This overhaul was approved by Rene.

Modified:
  M  Makefile.am
  M  agent/agent.c
  M  configure.ac
  M  debian/rules
  M  doc/HACKING
  M  doc/HOWTO.xml
  M  firewall/firewall.c
  M  hipd/hipd.c
  M  hipd/init.c
  M  hipd/init.h
  M  lib/core/hip_capability.c
  M  patches/openwrt/package/Makefile
  M  tools/hipl_autobuild.sh

=== modified file 'Makefile.am'
--- Makefile.am 2010-05-03 04:24:28 +0000
+++ Makefile.am 2010-05-03 16:43:45 +0000
@@ -230,6 +230,7 @@
                                  lib/core/hashchain_store.c \
                                  lib/core/hashtable.c       \
                                  lib/core/hashtree.c        \
+                                 lib/core/hip_capability.c  \
                                  lib/core/hip_statistics.c  \
                                  lib/core/hip_udp.c         \
                                  lib/core/hit.c             \
@@ -261,10 +262,6 @@
 lib_core_libhipcore_la_SOURCES += lib/core/performance.c
 endif
 
-if HIP_PRIVSEP
-lib_core_libhipcore_la_SOURCES += lib/core/hip_capability.c
-endif
-
 # don't use libipsec, but xfrmapi
 if HIP_PFKEY
 lib_core_libhipcore_la_SOURCES += lib/tool/pfkeysadb.c

=== modified file 'agent/agent.c'
--- agent/agent.c       2010-04-09 18:50:26 +0000
+++ agent/agent.c       2010-05-03 16:43:45 +0000
@@ -149,9 +149,8 @@
 
     /* Open socket to communicate with daemon, then drop from root to user */
     HIP_IFE(connhipd_init_sock(), -1);
-#ifdef CONFIG_HIP_PRIVSEP
+
     HIP_IFEL(hip_set_lowcapability(1), -1, "Failed to reduce priviledges\n");
-#endif /* CONFIG_HIP_PRIVSEP */
 
     HIP_IFEL(str_var_init(), -1, "str_var_init() failed!\n");
     /* Create config path. */

=== modified file 'configure.ac'
--- configure.ac        2010-04-29 09:27:16 +0000
+++ configure.ac        2010-05-03 16:43:45 +0000
@@ -165,39 +165,6 @@
     AH_TEMPLATE(CONFIG_HIP_HIPPROXY, [Defined to 1 if HIP Proxy is enabled.])
 fi
 
-AC_ARG_ENABLE(altsep,
-               AS_HELP_STRING([--enable-altsep],
-                              [Alternative privilege separation (default is 
NO)]),
-               [ac_cv_use_altsep=$enableval],
-               [ac_cv_use_altsep=no])
-AC_CACHE_CHECK([whether to use Alternative privilege separation],
-               [ac_cv_use_altsep],
-               [ac_cv_use_altsep=no])
-if test x"$ac_cv_use_altsep" = x"yes"; then
-    AC_DEFINE(CONFIG_HIP_ALTSEP)
-    AH_TEMPLATE(CONFIG_HIP_ALTSEP,
-                [Defined to 1 if alternative privilege separation is enabled.])
-fi
-
-AC_ARG_ENABLE(privsep,
-               AS_HELP_STRING([--enable-privsep],
-                              [Privilege separation (default is YES)]),
-               [ac_cv_use_privsep=$enableval],
-               [ac_cv_use_privsep=yes])
-AC_CACHE_CHECK([whether to use privsep],
-               [ac_cv_use_privsep],
-               [ac_cv_use_privsep=yes])
-if test x"$ac_cv_use_privsep" = x"yes"; then
-    if test x"$ac_cv_use_altsep" != x"yes"; then
-        AC_CHECK_LIB(cap, cap_get_proc,,
-                     AC_MSG_ERROR(libcap-dev developer headers not found))
-    fi
-    AC_DEFINE(CONFIG_HIP_PRIVSEP)
-    AH_TEMPLATE(CONFIG_HIP_PRIVSEP,
-                [Defined to 1 if privilege separation is enabled.])
-fi
-AM_CONDITIONAL(HIP_PRIVSEP, test x"$ac_cv_use_privsep" = x"yes")
-
 AC_ARG_ENABLE(i3,
                AS_HELP_STRING([--enable-i3],
                [HIP i3  (default is NO)]),

=== modified file 'debian/rules'
--- debian/rules        2010-04-08 15:10:18 +0000
+++ debian/rules        2010-05-03 16:43:45 +0000
@@ -35,7 +35,6 @@
                          --enable-shared \
                          --disable-dht \
                          --disable-agent \
-                         --disable-privsep \
                          --disable-i3 \
                          --disable-debug
 

=== modified file 'doc/HACKING'
--- doc/HACKING 2010-04-29 14:59:33 +0000
+++ doc/HACKING 2010-05-03 16:43:45 +0000
@@ -1194,14 +1194,6 @@
    * rpmbuild --target i686 -ba --without kabichk 
/usr/src/redhat/SPECS/kernel-2.6.16.9.spec
 
 
-VALGRIND AND HIPD
-=================
-
-On some machines, valgrind does not work without the following:
-
-  ./configure --disable-privsep && make clean all
-
-
 TEST MATRIX
 ===========
 

=== modified file 'doc/HOWTO.xml'
--- doc/HOWTO.xml       2010-04-28 17:49:19 +0000
+++ doc/HOWTO.xml       2010-05-03 16:43:45 +0000
@@ -367,7 +367,7 @@
       ./configure &amp;&amp; make install
     </para>
     <para>
-      Notes: you can optionally compile a binary package with "make deb" or 
"make rpm". If you want to use valgrind with hipd, you may need execute 
./configure --disable-privsep (and make clean all)
+      Notes: you can optionally compile a binary package with "make deb" or 
"make rpm".
     </para>
     <para>
       Some features, like the HIP firewall are not compiled by default.
@@ -1566,7 +1566,7 @@
       commands (assuming a clean source tree): </para>
 
       <para><programlisting>
-        ./configure --enable-maemo --disable-firewall --enable-altsep
+        ./configure --enable-maemo --disable-firewall
         make
       </programlisting></para>
 

=== modified file 'firewall/firewall.c'
--- firewall/firewall.c 2010-04-27 16:38:06 +0000
+++ firewall/firewall.c 2010-05-03 16:43:45 +0000
@@ -2341,11 +2341,9 @@
         return err;
     }
 
-#ifdef CONFIG_HIP_PRIVSEP
     if (limit_capabilities) {
         HIP_IFEL(hip_set_lowcapability(0), -1, "Failed to reduce priviledges");
     }
-#endif
 
 #ifdef CONFIG_HIP_HIPPROXY
     //send hipproxy status request before the control thread running.

=== modified file 'hipd/hipd.c'
--- hipd/hipd.c 2010-05-03 15:00:11 +0000
+++ hipd/hipd.c 2010-05-03 16:43:45 +0000
@@ -351,7 +351,7 @@
 {
     int c;
 
-    while ((c = getopt(argc, argv, ":bi:kNchafV")) != -1) {
+    while ((c = getopt(argc, argv, ":bi:kNchafVdp")) != -1) {
         switch (c) {
         case 'b':
             /* run in the "background" */
@@ -384,6 +384,10 @@
         case 'd':
             hip_set_logdebug(LOGDEBUG_ALL);
             break;
+        case 'p':
+            /* do _not_ use low capabilies ("privilege separation") */
+            *flags &= ~HIPD_START_LOWCAP;
+            break;
         case 'V':
             hip_print_version("hipd");
         case '?':
@@ -824,7 +828,7 @@
  */
 int main(int argc, char *argv[])
 {
-    uint64_t sflags = HIPD_START_FOREGROUND;
+    uint64_t sflags = HIPD_START_FOREGROUND | HIPD_START_LOWCAP;
 
     /* The flushing is enabled by default. The reason for this is that
      * people are doing some very experimental features on some branches

=== modified file 'hipd/init.c'
--- hipd/init.c 2010-05-03 14:00:58 +0000
+++ hipd/init.c 2010-05-03 16:43:45 +0000
@@ -10,6 +10,7 @@
 #define _BSD_SOURCE
 
 #include <limits.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
@@ -65,6 +66,9 @@
 #endif
 /** end ICMPV6_FILTER related stuff */
 
+/* Startup flags of the HIPD. Keep the around, for they will be used at exit */
+static uint64_t sflags;
+
 
/******************************************************************************/
 /**
  * Catch SIGCHLD.
@@ -331,7 +335,6 @@
     return 0;
 }
 
-#if defined(CONFIG_HIP_PRIVSEP) || defined(CONFIG_HIP_ALTSEP)
 /**
  * Remove a single module from the kernel, rmmod style (not modprobe).
  * @param name  name of the module
@@ -342,8 +345,6 @@
     return syscall(__NR_delete_module, name, O_NONBLOCK);
 }
 
-#endif
-
 /**
  * Cleanup/unload the kernel modules on hipd exit.
  * Unused for now because of unprivileged executions.
@@ -354,30 +355,29 @@
     /* some net modules depend on crypto, so keep net modules first */
     const char **mods[] = {kernel_net_mod, kernel_crypto_mod};
     int count[2], type, i, ret;
-#if ! (defined(CONFIG_HIP_PRIVSEP) || defined(CONFIG_HIP_ALTSEP))
     char cmd[MODPROBE_MAX_LINE];
-#endif
 
     count[0] = sizeof(kernel_net_mod)    / sizeof(kernel_net_mod[0]);
     count[1] = sizeof(kernel_crypto_mod) / sizeof(kernel_crypto_mod[0]);
 
     for (type = 0; type < 2; type++) {
         for (i = 0; i < count[type]; i++) {
-#if defined(CONFIG_HIP_PRIVSEP) || defined(CONFIG_HIP_ALTSEP)
-            /* Although we still retain the CAP_SYS_MODULE capability,
-             * because of the new (post-2.6.24) rules governing capability
-             * inheritance (i.e. "permitted" and "effective") on execve
-             * we cannot call modprobe or rmmod, because their thread
-             * will run without CAP_SYS_MODULE and thus fail to do the job.
-             * Not as good as 'modprobe -r', but (way) better that nothing.
-             */
-            ret = hip_rmmod(mods[type][i]);
-#else
-            /* no privilege separation whatsoever, we are almighty */
-            snprintf(cmd, sizeof(cmd), "/sbin/modprobe -r %s 2> /dev/null",
-                     mods[type][i]);
-            ret = system(cmd);
-#endif
+            if (sflags & HIPD_START_LOWCAP) {
+                /* Although we still retain the CAP_SYS_MODULE capability,
+                 * because of the new (post-2.6.24) rules governing capability
+                 * inheritance (i.e. "permitted" and "effective") on execve
+                 * we cannot call modprobe or rmmod, because their thread
+                 * will run without CAP_SYS_MODULE and thus fail to do the job.
+                 * Not as good as 'modprobe -r', but (way) better that nothing.
+                 */
+                ret = hip_rmmod(mods[type][i]);
+            } else {
+                /* no privilege separation whatsoever, we are almighty */
+                snprintf(cmd, sizeof(cmd), "/sbin/modprobe -r %s 2> /dev/null",
+                         mods[type][i]);
+                ret = system(cmd);
+            }
+
             if (ret) {
                 /* the errno is relevant in the hip_rmmod() case */
                 HIP_DEBUG("Unable to remove the %s module: %s.\n",
@@ -989,6 +989,9 @@
     char mtu[16];
     struct sockaddr_in6 daemon_addr;
 
+    /* Keep the flags around: they will be used at kernel module removal */
+    sflags = flags;
+
     memset(str, 0, 64);
     memset(mtu, 0, 16);
 
@@ -1162,10 +1165,9 @@
     hip_relay_init();
 #endif
 
-#ifdef CONFIG_HIP_PRIVSEP
-    HIP_IFEL(hip_set_lowcapability(0), -1, "Failed to set capabilities\n");
-#endif /* CONFIG_HIP_PRIVSEP */
-
+    if (flags & HIPD_START_LOWCAP) {
+        HIP_IFEL(hip_set_lowcapability(0), -1, "Failed to set capabilities\n");
+    }
 
 #ifdef CONFIG_HIP_HI3
     if (hip_use_i3) {

=== modified file 'hipd/init.h'
--- hipd/init.h 2010-05-03 14:00:58 +0000
+++ hipd/init.h 2010-05-03 16:43:45 +0000
@@ -30,6 +30,7 @@
 #define HIPD_START_FLUSH_IPSEC              (1 << 2)
 #define HIPD_START_KILL_OLD                 (1 << 3)
 #define HIPD_START_FIX_ALIGNMENT            (1 << 4)
+#define HIPD_START_LOWCAP                   (1 << 5)
 
 /*
  * HIP daemon initialization functions.

=== modified file 'lib/core/hip_capability.c'
--- lib/core/hip_capability.c   2010-05-02 00:49:12 +0000
+++ lib/core/hip_capability.c   2010-05-03 16:43:45 +0000
@@ -8,9 +8,7 @@
  * the damage of a exploit to the software. The code is Linux
  * specific.
  *
- * The capability code has been problematic with valgrind, the memory leak
- * detector. If you experience problems with valgrind, you can disable
- * capability code with ./configure --disable-privsep && make clean all
+ * This code causes problems with valgrind, because of setpwent(3).
  *
  * @brief Functionality to lower the privileges of a daemon
  *
@@ -29,12 +27,8 @@
 #include "ife.h"
 #include "hip_capability.h"
 
-#ifdef CONFIG_HIP_ALTSEP
 #include <linux/capability.h>
 #include <linux/unistd.h>
-#else
-#include <sys/capability.h>
-#endif /* CONFIG_HIP_ALTSEP */
 
 #define USER_NOBODY "nobody"
 #define USER_HIPD "hipd"
@@ -69,8 +63,6 @@
     return uid;
 }
 
-#ifdef CONFIG_HIP_ALTSEP
-
 /**
  * Wrapper for the capget system call.
  * @param hdrp  pointer to a __user_cap_header_struct
@@ -93,8 +85,6 @@
     return syscall(__NR_capset, hdrp, datap);
 }
 
-#endif /* CONFIG_HIP_ALTSEP */
-
 /**
  * Lower the privileges of the currently running process.
  *
@@ -107,7 +97,6 @@
     int err   = 0;
     uid_t uid = -1;
 
-#ifdef CONFIG_HIP_ALTSEP
     struct __user_cap_header_struct header;
     struct __user_cap_data_struct data;
 
@@ -159,76 +148,5 @@
               data.effective, data.permitted, data.inheritable);
 
 out_err:
-
-#else /* ! ALTSEP */
-
-    cap_value_t cap_list[] = {CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_MODULE};
-    int ncap_list          = sizeof(cap_list) / sizeof(cap_list[0]);
-    cap_t cap_p            = NULL;
-    char *cap_s            = NULL;
-    char *name             = NULL;
-
-    /* @todo: does this work when you start hipd as root (without sudo) */
-
-    if (run_as_sudo) {
-        HIP_IFEL(!(name = getenv("SUDO_USER")), -1,
-                 "Failed to determine current username\n");
-    } else {
-        /* Check if user "hipd" exists if it does use it
-         * otherwise use "nobody" */
-        if (hip_user_to_uid(USER_HIPD) >= 0) {
-            name = USER_HIPD;
-        } else if (hip_user_to_uid(USER_NOBODY) >= 0) {
-            name = USER_NOBODY;
-        } else {
-            HIP_IFEL(1, -1, "System does not have nobody account\n");
-        }
-    }
-
-    HIP_IFEL(prctl(PR_SET_KEEPCAPS, 1), -1, "prctl err\n");
-
-    HIP_DEBUG("Now PR_SET_KEEPCAPS=%d\n", prctl(PR_GET_KEEPCAPS));
-
-    uid = hip_user_to_uid(name);
-    HIP_IFEL((uid < 0), -1,
-             "Error while retrieving USER '%s' uid\n", name);
-
-    HIP_IFEL(!(cap_p = cap_get_proc()), -1,
-             "Error getting capabilities\n");
-    HIP_DEBUG("cap_p %s\n", cap_s = cap_to_text(cap_p, NULL));
-    /* It would be better to use #if DEBUG */
-    if (cap_s != NULL) {
-        cap_free(cap_s);
-        cap_s = NULL;
-    }
-
-    HIP_DEBUG("Before setreuid UID=%d and EFF_UID=%d\n",
-              getuid(), geteuid());
-
-    HIP_IFEL(setreuid(uid, uid), -1, "setruid failed\n");
-
-    HIP_DEBUG("After setreuid UID=%d and EFF_UID=%d\n",
-              getuid(), geteuid());
-
-    HIP_DEBUG("Going to clear all capabilities except the ones needed\n");
-    HIP_IFEL(cap_clear(cap_p) < 0, -1, "Error clearing capabilities\n");
-
-    HIP_IFEL(cap_set_flag(cap_p, CAP_EFFECTIVE, ncap_list, cap_list, CAP_SET) 
< 0,
-             -1, "Error setting capability flags\n");
-    HIP_IFEL(cap_set_flag(cap_p, CAP_PERMITTED, ncap_list, cap_list, CAP_SET) 
< 0,
-             -1, "Error setting capability flags\n");
-    HIP_IFEL(cap_set_proc(cap_p) < 0, -1, "Error modifying capabilities\n");
-    HIP_DEBUG("UID=%d EFF_UID=%d\n", getuid(), geteuid());
-    HIP_DEBUG("cap_p %s\n", cap_s = cap_to_text(cap_p, NULL));
-    /* It would be better to use #if DEBUG */
-    if (cap_s != NULL) {
-        cap_free(cap_s);
-        cap_s = NULL;
-    }
-
-out_err:
-    cap_free(cap_p);
-
-#endif
     return err;
 }

=== modified file 'patches/openwrt/package/Makefile'
--- patches/openwrt/package/Makefile    2010-04-09 14:05:52 +0000
+++ patches/openwrt/package/Makefile    2010-05-03 16:43:45 +0000
@@ -101,7 +101,6 @@
             --enable-shared \
             --disable-dht \
             --disable-agent \
-            --disable-privsep \
             --disable-i3 \
             --disable-debug \
     );

=== modified file 'tools/hipl_autobuild.sh'
--- tools/hipl_autobuild.sh     2010-04-28 23:41:10 +0000
+++ tools/hipl_autobuild.sh     2010-05-03 16:43:45 +0000
@@ -109,10 +109,10 @@
 run_program "make -j17 distcheck"
 
 # PISA configuration
-compile --enable-firewall --disable-agent --disable-pfkey --disable-rvs 
--disable-hipproxy --disable-altsep --enable-privsep --disable-i3 
--disable-opportunistic --disable-dht --disable-blind --disable-profiling 
--enable-debug --enable-midauth --disable-performance --disable-demo
+compile --enable-firewall --disable-agent --disable-pfkey --disable-rvs 
--disable-hipproxy --disable-i3 --disable-opportunistic --disable-dht 
--disable-blind --disable-profiling --enable-debug --enable-midauth 
--disable-performance --disable-demo
 
 # Alternative path to vanilla
-compile --enable-firewall --enable-agent --enable-pfkey --disable-rvs 
--disable-hipproxy --enable-altsep --disable-privsep --enable-i3 
--disable-opportunistic --disable-dht --enable-blind --enable-profiling 
--disable-debug --enable-midauth --enable-performance --enable-demo
+compile --enable-firewall --enable-agent --enable-pfkey --disable-rvs 
--disable-hipproxy --enable-i3 --disable-opportunistic --disable-dht 
--enable-blind --enable-profiling --disable-debug --enable-midauth 
--enable-performance --enable-demo
 
 # Compile HIPL within an OpenWrt checkout
 CONFIGURATION="OpenWrt ARM crosscompile"

Other related posts:

  • » [hipl-commit] [trunk] Rev 4457: Major overhaul of the privilege separation code. - Mircea Gherzan