priv: drop most privileges in monitor, only keep CAP_NET_RAW/ADMIN

On Linux, we mostly rely on CAP_NET_RAW. Only keep that one. However,
we also write to ifalias, which needs CAP_NET_ADMIN. We could let user
choose at runtime if they want to grant this capability or not.
Currently, a user can turn it on/off at any time.

Access to SNMP socket may also be problematic. We need some solid
solution about that before merging.

Is it safe to use the same UID for the monitored and the unprivileged
process? Signals are mostly harmless. As for ptrace, since the
monitored process as more capabilities, this will not be allowed by
Linux.

diff --git a/NEWS b/NEWS
index 76ad84953e6afc2747f603f32859abb18020ce5d..dd0e1e5a96804e8c40452a341e1d711a0612344f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,10 @@
+lldpd (1.0.2)
+  * Changes:
+    + On Linux, the monitor process will now drop its privileges
+      instead of running as root. It will keep CAP_NET_RAW and
+      CAP_NET_ADMIN capabilities. When using SNMP AgentX feature, the
+      access to the socket may require to grant access to _lldpd user.
+
 lldpd (1.0.1)
   * Fix:
     + Use "mkdir -p" instead of "mkdir" in systemd unit.

diff --git a/configure.ac b/configure.ac
index 97ae25582382b8e31bd3c8c08c5f495480e0094b..1af248b5ad82d675361f1f26a3031dc4777d7d77 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -226,6 +226,9 @@ PKG_CHECK_MODULES([check], [check >= 0.9.4], [have_check=yes], [have_check=no])
 
 # Third-party libraries
 lldp_CHECK_LIBEVENT
+PKG_CHECK_MODULES([libcap], [libcap >= 2], [
+  AC_DEFINE([HAVE_LINUX_CAPABILITIES], 1, [Define to indicate support of linux capabilities])
+], [:])
 
 # Compatibility with pkg.m4 < 0.27
 m4_ifdef([PKG_INSTALLDIR], [PKG_INSTALLDIR],

diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am
index 9c20e7fb8c927d7ce9f53c63a95ec3442ba0d0b3..2de820115d74e720085b61bfedad8383f5bb4442 100644 (file)
--- a/src/daemon/Makefile.am
+++ b/src/daemon/Makefile.am
@@ -28,11 +28,11 @@ liblldpd_la_SOURCES  = \
        protocols/sonmp.h \
        protocols/edp.c \
        protocols/edp.h
-liblldpd_la_CFLAGS   = $(AM_CFLAGS) @libevent_CFLAGS@
+liblldpd_la_CFLAGS   = $(AM_CFLAGS) @libevent_CFLAGS@ @libcap_CFLAGS@
 liblldpd_la_CPPFLAGS = $(AM_CPPFLAGS) -DSYSCONFDIR='"$(sysconfdir)"' -DLLDPCLI_PATH='"$(sbindir)/lldpcli"'
 liblldpd_la_LIBADD   = \
        $(top_builddir)/src/libcommon-daemon-client.la \
-       $(top_builddir)/src/libcommon-daemon-lib.la @libevent_LIBS@
+       $(top_builddir)/src/libcommon-daemon-lib.la @libevent_LIBS@ @libcap_LIBS@
 
 ## lldpd
 lldpd_SOURCES = main.c

diff --git a/src/daemon/priv.c b/src/daemon/priv.c
index 6db6d49f033e313fb75eabaa8a5ad8430f5151a9..5a062cf06a2457e6aea956ac7916331c2aa4f675 100644 (file)
--- a/src/daemon/priv.c
+++ b/src/daemon/priv.c
@@ -37,6 +37,11 @@
 #include <sys/ioctl.h>
 #include <netinet/if_ether.h>
 
+#ifdef HAVE_LINUX_CAPABILITIES
+#include <sys/capability.h>
+#include <sys/prctl.h>
+#endif
+
 #if defined HOST_OS_FREEBSD || HOST_OS_OSX || HOST_OS_DRAGONFLY
 # include <net/if_dl.h>
 #endif
@@ -595,6 +600,60 @@ sig_chld(int sig)
 
 #endif
 
+void
+priv_drop(uid_t uid, gid_t gid)
+{
+       gid_t gidset[1];
+       gidset[0] = gid;
+       log_debug("privsep", "dropping privileges");
+#ifdef HAVE_SETRESGID
+       if (setresgid(gid, gid, gid) == -1)
+               fatal("privsep", "setresgid() failed");
+#else
+       if (setregid(gid, gid) == -1)
+               fatal("privsep", "setregid() failed");
+#endif
+       if (setgroups(1, gidset) == -1)
+               fatal("privsep", "setgroups() failed");
+#ifdef HAVE_SETRESUID
+       if (setresuid(uid, uid, uid) == -1)
+               fatal("privsep", "setresuid() failed");
+#else
+       if (setreuid(uid, uid) == -1)
+               fatal("privsep", "setreuid() failed");
+#endif
+}
+
+void
+priv_caps(uid_t uid, gid_t gid)
+{
+#ifdef HAVE_LINUX_CAPABILITIES
+       cap_t caps;
+       log_debug("privsep", "getting CAP_NET_RAW/ADMIN privilege");
+       if (!(caps = cap_from_text("cap_net_raw,cap_net_admin,cap_setuid,cap_setgid=pe")))
+               fatal("privsep", "unable to convert caps");
+       if (cap_set_proc(caps) == -1) {
+               log_warn("privsep", "unable to drop privileges, monitor running as root");
+               cap_free(caps);
+               return;
+       }
+       cap_free(caps);
+
+       if (prctl(PR_SET_KEEPCAPS, 1L, 0L, 0L, 0L) == -1)
+               fatal("privsep", "cannot keep capabilities");
+       priv_drop(uid, gid);
+
+       log_debug("privsep", "dropping extra capabilities");
+       if (!(caps = cap_from_text("cap_net_raw,cap_net_admin=pe")))
+               fatal("privsep", "unable to convert caps");
+       if (cap_set_proc(caps) == -1)
+               fatal("privsep", "unable to drop extra privileges");
+       cap_free(caps);
+#else
+       log_info("privsep", "no libcap support, running monitor as root");
+#endif
+}
+
 void
 priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
 {
@@ -611,7 +670,6 @@ priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
        priv_privileged_fd(pair[1]);
 
 #ifdef ENABLE_PRIVSEP
-       gid_t gidset[1];
        /* Spawn off monitor */
        if ((monitored = fork()) < 0)
                fatal("privsep", "unable to fork monitor");
@@ -626,23 +684,7 @@ priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
                                fatal("privsep", "unable to chroot");
                        if (chdir("/") != 0)
                                fatal("privsep", "unable to chdir");
-                       gidset[0] = gid;
-#ifdef HAVE_SETRESGID
-                       if (setresgid(gid, gid, gid) == -1)
-                               fatal("privsep", "setresgid() failed");
-#else
-                       if (setregid(gid, gid) == -1)
-                               fatal("privsep", "setregid() failed");
-#endif
-                       if (setgroups(1, gidset) == -1)
-                               fatal("privsep", "setgroups() failed");
-#ifdef HAVE_SETRESUID
-                       if (setresuid(uid, uid, uid) == -1)
-                               fatal("privsep", "setresuid() failed");
-#else
-                       if (setreuid(uid, uid) == -1)
-                               fatal("privsep", "setreuid() failed");
-#endif
+                       priv_drop(uid, gid);
                }
                close(pair[1]);
                priv_ping();
@@ -654,6 +696,8 @@ priv_init(const char *chrootdir, int ctl, uid_t uid, gid_t gid)
                if (atexit(priv_exit) != 0)
                        fatal("privsep", "unable to set exit function");
 
+               priv_caps(uid, gid);
+
                /* Install signal handlers */
                const struct sigaction pass_to_child = {
                        .sa_handler = sig_pass_to_chld,

diff --git a/tests/ci/install.sh b/tests/ci/install.sh
index 843e9c9ef0e85559747bf7a46dd559d1f41ba782..b4e2a97fb4400a9042f0ce01a9c7e978f08209aa 100755 (executable)
--- a/tests/ci/install.sh
+++ b/tests/ci/install.sh
@@ -20,7 +20,7 @@ case "$(uname -s)" in
             libsnmp-dev libxml2-dev \
             libevent-dev libreadline-dev libbsd-dev \
             check libc6-dbg libevent-dbg libseccomp-dev \
-            libpcap-dev
+            libpcap-dev libcap-dev
         [ $CC != gcc ] || \
             sudo apt-get -qqy install gcc-5
         # For integration tests