From db3d867b61840c54515e2ae294bf9ecf7cc01f47 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Sun, 15 Mar 2015 20:54:18 +0100 Subject: [PATCH 01/40] Initial attempt at libcap - untested! --- configure.in | 16 ++++++++++++++++ src/gateway.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/configure.in b/configure.in index 9fceb5f0..74a99a11 100644 --- a/configure.in +++ b/configure.in @@ -84,6 +84,22 @@ AC_SUBST(enable_latex_docs) # Acutally perform the doxygen check BB_ENABLE_DOXYGEN + +# Enable cyassl? +AC_DEFUN([BB_LIBCAP], +[ +AC_ARG_ENABLE(libcap, [ --enable-cyassl enable documentation generation with doxygen (no)], [enable_libcap=yes], [enable_libcap=no]) +if test "x$enable_libcap" = xyes; then + AC_CHECK_HEADERS(sys/capability.h) + AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ + AC_MSG_ERROR([unable to find the cap_get_proc function.]) + ]) + AC_DEFINE(USE_LIBCAP,, Compile with libcap support) +fi +]) +# Actually perform the cyassl check +BB_LIBCAP + # check for pthread AC_CHECK_HEADER(pthread.h, , AC_MSG_ERROR(You need the pthread headers) ) AC_CHECK_LIB(pthread, pthread_create, , AC_MSG_ERROR(You need the pthread library) ) diff --git a/src/gateway.c b/src/gateway.c index 060a2bf2..c9996546 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -59,6 +59,7 @@ #include "ping_thread.h" #include "httpd_thread.h" #include "util.h" +#include "config.h" /** XXX Ugly hack * We need to remember the thread IDs of threads that simulate wait with pthread_cond_timedwait @@ -497,6 +498,52 @@ main_loop(void) /* never reached */ } + +#ifdef USE_LIBCAP +#include +#include +#include +/* For getpwnam */ +#include +/* For getgrnam */ +#include +#endif + + +#ifdef USE_LIBCAP +void drop_privileges(const char *user, const char *group) { + cap_value_t cap_values[] = { CAP_NET_ADMIN}; + cap_t caps; + + // TODO: what happens on reload? + caps = cap_get_proc(); + /* Hopefully clear all caps but cap_values */ + cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); + cap_set_proc(caps); + /* About to switch uid. This is necessary because the process can + still read everyting owned by root - IIRC. + However, we need to have our capabilities survive setuid */ + prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); + cap_free(caps); + + struct passwd *pwd = getpwnam(user); /* don't free, see getpwnam() for details */ + if (pwd) { + uid_t uid = pwd->pw_uid; + setuid(uid); + } + struct group *grp = getgrnam(group); + if (grp) { + gid_t gid = grp->gr_gid; + setgid(gid); + } + + caps = cap_get_proc(); + cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); + cap_set_proc(caps); + cap_free(caps); +} +#endif /* USE_LIBCAP */ + /** Reads the configuration file and then starts the main loop */ int main(int argc, char **argv) { @@ -509,6 +556,10 @@ int main(int argc, char **argv) { config_read(config->configfile); config_validate(); +#ifdef USE_LIBCAP + drop_privileges("nobody", "nogroup"); +#endif /* USE LIBCAP */ + /* Initializes the linked list of connected clients */ client_list_init(); From 6a2e30692fbc502702ac567e1978e4ef44bc9896 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 16 Mar 2015 08:18:51 +0100 Subject: [PATCH 02/40] Fix: wifidog requires CAP_NET_RAW --- src/gateway.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gateway.c b/src/gateway.c index c9996546..022e6445 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -512,7 +512,7 @@ main_loop(void) #ifdef USE_LIBCAP void drop_privileges(const char *user, const char *group) { - cap_value_t cap_values[] = { CAP_NET_ADMIN}; + cap_value_t cap_values[] = { CAP_NET_ADMIN, CAP_NET_RAW }; cap_t caps; // TODO: what happens on reload? From 5f71eb7556f0925fc1ba283aa606cfd60864d90c Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 16 Mar 2015 08:43:48 +0100 Subject: [PATCH 03/40] Cherry-pick SOCK_PACKET -> SOCK_RAW https://github.com/acv/wifidog-gateway/commit/d1fec068f2150833213d914b92202d6008cf33e5.diff --- src/util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util.c b/src/util.c index 1794d953..8930417a 100644 --- a/src/util.c +++ b/src/util.c @@ -169,7 +169,7 @@ get_iface_ip(const char *ifname) u_int32_t ip; /* Create a socket */ - if ((sockd = socket (AF_INET, SOCK_PACKET, htons(0x8086))) < 0) { + if ((sockd = socket (AF_INET, SOCK_RAW, htons(0x8086))) < 0) { debug(LOG_ERR, "socket(): %s", strerror(errno)); return NULL; } @@ -226,7 +226,7 @@ get_iface_mac(const char *ifname) strcpy(ifr.ifr_name, ifname); - s = socket(PF_INET, SOCK_DGRAM, 0); + s = socket(AF_INET, SOCK_DGRAM, 0); if (-1 == s) { debug(LOG_ERR, "get_iface_mac socket: %s", strerror(errno)); return NULL; From 9326edfa3a7b939d02d3da70717f277e20da3448 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Mon, 16 Mar 2015 08:47:42 +0100 Subject: [PATCH 04/40] Fix: drop group first, regain privileges --- src/gateway.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/gateway.c b/src/gateway.c index 022e6445..bb065319 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -526,18 +526,21 @@ void drop_privileges(const char *user, const char *group) { prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); cap_free(caps); - struct passwd *pwd = getpwnam(user); /* don't free, see getpwnam() for details */ - if (pwd) { - uid_t uid = pwd->pw_uid; - setuid(uid); - } struct group *grp = getgrnam(group); if (grp) { gid_t gid = grp->gr_gid; setgid(gid); } + struct passwd *pwd = getpwnam(user); /* don't free, see getpwnam() for details */ + if (pwd) { + uid_t uid = pwd->pw_uid; + setuid(uid); + } caps = cap_get_proc(); + /* Re-gain privileges */ + cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); + /* Child processes get the same privileges */ cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); cap_set_proc(caps); cap_free(caps); From 6f6f526653ed53a212d82af0cc665560f0c173e1 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 25 Mar 2015 11:13:03 +0100 Subject: [PATCH 05/40] Move drop_privileges to capabilities.c --- src/Makefile.am | 7 ++++-- src/capabilities.c | 45 +++++++++++++++++++++++++++++++++++++ src/capabilities.h | 35 +++++++++++++++++++++++++++++ src/gateway.c | 55 +++++----------------------------------------- 4 files changed, 91 insertions(+), 51 deletions(-) create mode 100644 src/capabilities.c create mode 100644 src/capabilities.h diff --git a/src/Makefile.am b/src/Makefile.am index a1bd709f..d3b057df 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -29,7 +29,9 @@ wifidog_SOURCES = commandline.c \ safe.c \ httpd_thread.c \ simple_http.c \ - pstring.c + pstring.c \ + capabilities.c +# TODO: capabilities.c is always compiled, protect with ifdef? noinst_HEADERS = commandline.h \ common.h \ @@ -49,6 +51,7 @@ noinst_HEADERS = commandline.h \ safe.h \ httpd_thread.h \ simple_http.h \ - pstring.h + pstring.h \ + capabilities.h wdctl_SOURCES = wdctl.c diff --git a/src/capabilities.c b/src/capabilities.c new file mode 100644 index 00000000..1ba464e3 --- /dev/null +++ b/src/capabilities.c @@ -0,0 +1,45 @@ +#include +#include +#include +/* For getpwnam */ +#include +/* For getgrnam */ +#include + +void +drop_privileges(const char *user, const char *group) +{ + cap_value_t cap_values[] = { CAP_NET_ADMIN, CAP_NET_RAW }; + cap_t caps; + + // TODO: what happens on reload? + caps = cap_get_proc(); + /* Hopefully clear all caps but cap_values */ + cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); + cap_set_proc(caps); + /* About to switch uid. This is necessary because the process can + still read everyting owned by root - IIRC. + However, we need to have our capabilities survive setuid */ + prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); + cap_free(caps); + + struct group *grp = getgrnam(group); + if (grp) { + gid_t gid = grp->gr_gid; + setgid(gid); + } + /* don't free, see getpwnam() for details */ + struct passwd *pwd = getpwnam(user); + if (pwd) { + uid_t uid = pwd->pw_uid; + setuid(uid); + } + + caps = cap_get_proc(); + /* Re-gain privileges */ + cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); + /* Child processes get the same privileges */ + cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); + cap_set_proc(caps); + cap_free(caps); +} diff --git a/src/capabilities.h b/src/capabilities.h new file mode 100644 index 00000000..14dd8fe2 --- /dev/null +++ b/src/capabilities.h @@ -0,0 +1,35 @@ +/* vim: set et sw=4 ts=4 sts=4 : */ +/********************************************************************\ + * This program is free software; you can redistribute it and/or * + * modify it under the terms of the GNU General Public License as * + * published by the Free Software Foundation; either version 2 of * + * the License, or (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License* + * along with this program; if not, contact: * + * * + * Free Software Foundation Voice: +1-617-542-5942 * + * 59 Temple Place - Suite 330 Fax: +1-617-542-2652 * + * Boston, MA 02111-1307, USA gnu@gnu.org * + * * +\********************************************************************/ + +/* $Id$ */ +/** @file capabilities.h + @author Copyright (C) 2015 Michael Haas +*/ + +#ifndef _CAPABILITIES_H_ +#define _CAPABILITIES_H_ + +void +drop_privileges(const char*, const char*); + +void +regain_privileges(); +#endif /* _CAPABILITIES_H_ */ diff --git a/src/gateway.c b/src/gateway.c index 373e5ba4..a4c4b261 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -62,6 +62,11 @@ #include "util.h" #include "config.h" + +#ifdef USE_LIBCAP +#include "capabilities.h" +#endif /* USE LIBCAP */ + /** XXX Ugly hack * We need to remember the thread IDs of threads that simulate wait with pthread_cond_timedwait * so we can explicitly kill them in the termination handler @@ -491,54 +496,6 @@ main_loop(void) /* never reached */ } -#ifdef USE_LIBCAP -#include -#include -#include -/* For getpwnam */ -#include -/* For getgrnam */ -#include -#endif - -#ifdef USE_LIBCAP -void -drop_privileges(const char *user, const char *group) -{ - cap_value_t cap_values[] = { CAP_NET_ADMIN, CAP_NET_RAW }; - cap_t caps; - -// TODO: what happens on reload? - caps = cap_get_proc(); -/* Hopefully clear all caps but cap_values */ - cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); - cap_set_proc(caps); -/* About to switch uid. This is necessary because the process can -still read everyting owned by root - IIRC. -However, we need to have our capabilities survive setuid */ - prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); - cap_free(caps); - - struct group *grp = getgrnam(group); - if (grp) { - gid_t gid = grp->gr_gid; - setgid(gid); - } - struct passwd *pwd = getpwnam(user); /* don't free, see getpwnam() for details */ - if (pwd) { - uid_t uid = pwd->pw_uid; - setuid(uid); - } - - caps = cap_get_proc(); -/* Re-gain privileges */ - cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); -/* Child processes get the same privileges */ - cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); - cap_set_proc(caps); - cap_free(caps); -} -#endif /* USE_LIBCAP */ /** Reads the configuration file and then starts the main loop */ int @@ -551,7 +508,7 @@ main(int argc, char **argv) parse_commandline(argc, argv); #ifdef USE_LIBCAP drop_privileges("nobody", "nogroup"); -#endif /* USE LIBCAP */ +#endif /* USE LIBCAP */ /* Initialize the config */ config_read(config->configfile); From c0ef73ce3a505dcd2ca0fbed0791e9d3f8c034a9 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 25 Mar 2015 12:14:53 +0100 Subject: [PATCH 06/40] Add debug output Also protect capabilities with ifdefs --- src/capabilities.c | 34 ++++++++++++++++++++++++++++++---- src/capabilities.h | 5 ++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 1ba464e3..b8678efb 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -1,3 +1,7 @@ + +#ifdef USE_LIBCAP + +#include #include #include #include @@ -6,40 +10,62 @@ /* For getgrnam */ #include +#include "debug.h" void drop_privileges(const char *user, const char *group) { - cap_value_t cap_values[] = { CAP_NET_ADMIN, CAP_NET_RAW }; + debug(LOG_DEBUG, "Entered drop_privileges"); + /* The capabilities we want. + * CAP_NET_RAW is used for our socket handling. + * We actually require CAP_NET_ADMIN for iptables. However, + * iptables must be run as root so we do that instead. */ + cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; cap_t caps; // TODO: what happens on reload? + caps = cap_get_proc(); + debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); /* Hopefully clear all caps but cap_values */ cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); cap_set_proc(caps); + caps = cap_get_proc(); + debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); /* About to switch uid. This is necessary because the process can still read everyting owned by root - IIRC. However, we need to have our capabilities survive setuid */ + // TODO: also see SECBIT_KEEP_CAPS prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); cap_free(caps); - + debug(LOG_DEBUG, "Switching to group %s", group); struct group *grp = getgrnam(group); if (grp) { gid_t gid = grp->gr_gid; setgid(gid); + } else { + debug(LOG_ERR, "Failed to look up GID for group %s", group); } /* don't free, see getpwnam() for details */ + debug(LOG_DEBUG, "Switching to user %s", user); struct passwd *pwd = getpwnam(user); if (pwd) { uid_t uid = pwd->pw_uid; setuid(uid); + } else { + debug(LOG_ERR, "Failed to look up UID for user %s", user); } - caps = cap_get_proc(); + debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); + debug(LOG_DEBUG, "Regaining privileges."); /* Re-gain privileges */ cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); - /* Child processes get the same privileges */ + /* Child processes get the same privileges. In particular, + * iptables */ cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); cap_set_proc(caps); + debug(LOG_DEBUG, "Regained: %s", cap_to_text(caps, NULL)); + caps = cap_get_proc(); cap_free(caps); } + +#endif /* USE_LIBCAP */ diff --git a/src/capabilities.h b/src/capabilities.h index 14dd8fe2..cb8a5eed 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -19,11 +19,12 @@ * * \********************************************************************/ -/* $Id$ */ /** @file capabilities.h @author Copyright (C) 2015 Michael Haas */ +#ifdef USE_LIBCAP + #ifndef _CAPABILITIES_H_ #define _CAPABILITIES_H_ @@ -33,3 +34,5 @@ drop_privileges(const char*, const char*); void regain_privileges(); #endif /* _CAPABILITIES_H_ */ + +#endif /* USE_LIBCAP */ From 6c18dafb09ea9f8b81bfa6e3c42751db66e855a1 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 25 Mar 2015 13:15:14 +0100 Subject: [PATCH 07/40] Error handling in capabilities.c --- src/capabilities.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index b8678efb..a38b7026 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -1,4 +1,7 @@ + +#include "../config.h" + #ifdef USE_LIBCAP #include @@ -11,9 +14,11 @@ #include #include "debug.h" + void drop_privileges(const char *user, const char *group) { + int ret = 0; debug(LOG_DEBUG, "Entered drop_privileges"); /* The capabilities we want. * CAP_NET_RAW is used for our socket handling. @@ -28,7 +33,10 @@ drop_privileges(const char *user, const char *group) debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); /* Hopefully clear all caps but cap_values */ cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); - cap_set_proc(caps); + ret = cap_set_proc(caps); + if (ret == -1) { + debug(LOG_ERR, "Could not set capabilities!"); + } caps = cap_get_proc(); debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); /* About to switch uid. This is necessary because the process can @@ -62,9 +70,12 @@ drop_privileges(const char *user, const char *group) /* Child processes get the same privileges. In particular, * iptables */ cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); - cap_set_proc(caps); - debug(LOG_DEBUG, "Regained: %s", cap_to_text(caps, NULL)); + ret = cap_set_proc(caps); + if (ret == -1) { + debug(LOG_ERR, "Could not set capabilities!"); + } caps = cap_get_proc(); + debug(LOG_DEBUG, "Regained: %s", cap_to_text(caps, NULL)); cap_free(caps); } From fbf355663467abd2120cb2bb98994984658f6d65 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 25 Mar 2015 13:15:30 +0100 Subject: [PATCH 08/40] Fix compilation --- src/capabilities.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/capabilities.h b/src/capabilities.h index cb8a5eed..d06c81bb 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -23,6 +23,8 @@ @author Copyright (C) 2015 Michael Haas */ +#include "../config.h" + #ifdef USE_LIBCAP #ifndef _CAPABILITIES_H_ From c0d284c6a72c807aa7b4476b9ea55570fa02929b Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 10:31:07 +0200 Subject: [PATCH 09/40] Tabs -> spaces --- configure.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/configure.in b/configure.in index 6c0f231b..a24af039 100644 --- a/configure.in +++ b/configure.in @@ -90,11 +90,11 @@ AC_DEFUN([BB_LIBCAP], [ AC_ARG_ENABLE(libcap, [ --enable-cyassl enable documentation generation with doxygen (no)], [], [enable_libcap=no]) if test "x$enable_libcap" = xyes; then - AC_CHECK_HEADERS(sys/capability.h) - AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ - AC_MSG_ERROR([unable to find the cap_get_proc function.]) - ]) - AC_DEFINE(USE_LIBCAP,, Compile with libcap support) + AC_CHECK_HEADERS(sys/capability.h) + AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ + AC_MSG_ERROR([unable to find the cap_get_proc function.]) + ]) + AC_DEFINE(USE_LIBCAP,, Compile with libcap support) fi ]) # Actually perform the libcap check From bd19dc8aba9cc2bb192af4d7210531fdf2eed2ee Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 11:03:55 +0200 Subject: [PATCH 10/40] Capabilities: Fix unnecessary calls The new strategy is to use seteuid(), in which case we don't need to care about the capabilities of child processes. --- src/capabilities.c | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index a38b7026..e8019c62 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -27,8 +27,6 @@ drop_privileges(const char *user, const char *group) cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; cap_t caps; - // TODO: what happens on reload? - caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); /* Hopefully clear all caps but cap_values */ @@ -39,40 +37,59 @@ drop_privileges(const char *user, const char *group) } caps = cap_get_proc(); debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); - /* About to switch uid. This is necessary because the process can - still read everyting owned by root - IIRC. - However, we need to have our capabilities survive setuid */ - // TODO: also see SECBIT_KEEP_CAPS - prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); cap_free(caps); + /* We are about to drop our effective UID to a non-privileged user. + * This clears the EFFECTIVE capabilities set, so we will have to + * re-enable these. We can do that because they are not cleared from + * the PERMITTED set. + * Note: if we used setuid() instead of seteuid(), we would have lost the + * PERMITTED set as well. In this case, we would need to call prctl + * with PR_SET_KEEPCAPS. + */ debug(LOG_DEBUG, "Switching to group %s", group); + /* don't free grp, see getpwnam() for details */ struct group *grp = getgrnam(group); if (grp) { gid_t gid = grp->gr_gid; - setgid(gid); + setegid(gid); } else { debug(LOG_ERR, "Failed to look up GID for group %s", group); + exit(1); } - /* don't free, see getpwnam() for details */ debug(LOG_DEBUG, "Switching to user %s", user); + /* don't pwd, see getpwnam() for details */ struct passwd *pwd = getpwnam(user); if (pwd) { uid_t uid = pwd->pw_uid; - setuid(uid); + seteuid(uid); } else { debug(LOG_ERR, "Failed to look up UID for user %s", user); + exit(1); } caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); - debug(LOG_DEBUG, "Regaining privileges."); + debug(LOG_DEBUG, "Regaining capabilities."); /* Re-gain privileges */ cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); - /* Child processes get the same privileges. In particular, - * iptables */ - cap_set_flag(caps, CAP_INHERITABLE, 2, cap_values, CAP_SET); + /* Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE + * would be useful to execve iptables as non-root. In practice, Wifidog + * often runs on embedded systems (OpenWrt) where the required file-based + * capabilities are not available as the underlying file system does not + * support extended attributes. + * + * The linux capabilities implementation requires that the executable is + * specifically marked as being able to inherit capabilities from a calling + * process. This can be done by setting the Inheritable+Effective file + * capabilities on the executable. Alas, it's not relevant here. + * + * This is also the main reason why we only seteuid() instead of setuid(): + * iptables will be called as root (with ALL capabilities!) and thus continue + * to work as before. + */ ret = cap_set_proc(caps); if (ret == -1) { debug(LOG_ERR, "Could not set capabilities!"); + exit(1); } caps = cap_get_proc(); debug(LOG_DEBUG, "Regained: %s", cap_to_text(caps, NULL)); From 1d324c551792aec676edb2a91e68c4fe0db2920f Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 11:39:09 +0200 Subject: [PATCH 11/40] Add user/group config option --- contrib/load-tester/wifidog-mock.conf | 3 +++ src/capabilities.c | 23 +++++++++++++++++++++++ src/conf.c | 22 ++++++++++++++++++++++ src/conf.h | 4 ++++ src/gateway.c | 7 ++++--- wifidog.conf | 23 +++++++++++++++++++++++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/contrib/load-tester/wifidog-mock.conf b/contrib/load-tester/wifidog-mock.conf index 0a488f80..86cf2849 100644 --- a/contrib/load-tester/wifidog-mock.conf +++ b/contrib/load-tester/wifidog-mock.conf @@ -1,3 +1,6 @@ +User nobody +Group nobody + ExternalInterface eth0 GatewayInterface internal0 diff --git a/src/capabilities.c b/src/capabilities.c index e8019c62..0eb47db2 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -8,6 +8,8 @@ #include #include #include +/* For exit */ +#include /* For getpwnam */ #include /* For getgrnam */ @@ -15,6 +17,26 @@ #include "debug.h" +/** + * Switches to non-privileged user and drops unneeded capabilities. + * + * Wifidog does not need to run as root. The only capabilities required + * are: + * - CAP_NET_RAW: get IP addresses from sockets, ICMP ping + * - CAP_NET_ADMIN: set up iptables + * + * This function drops all other capabilities. As only the effective + * user id is set, it is theoretically possible for an attacker to + * regain root privileges. + * Any processes started with execve will + * have UID0. This is a convenient side effect to allow for proper + * operation of iptables. + * + * Any error is considered fatal and exit() is called. + * + * @param user Non-privileged user + * @param Non-privileged group + */ void drop_privileges(const char *user, const char *group) { @@ -34,6 +56,7 @@ drop_privileges(const char *user, const char *group) ret = cap_set_proc(caps); if (ret == -1) { debug(LOG_ERR, "Could not set capabilities!"); + exit(1); } caps = cap_get_proc(); debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); diff --git a/src/conf.c b/src/conf.c index bf103f05..24380366 100644 --- a/src/conf.c +++ b/src/conf.c @@ -101,6 +101,8 @@ typedef enum { oSSLPeerVerification, oSSLCertPath, oSSLAllowedCipherList, + oUser, + oGroup, } OpCodes; /** @internal @@ -145,6 +147,8 @@ static const struct { "sslpeerverification", oSSLPeerVerification}, { "sslcertpath", oSSLCertPath}, { "sslallowedcipherlist", oSSLAllowedCipherList}, { + "user", oUser}, { + "group", oGroup}, { NULL, oBadOption},}; static void config_notnull(const void *parm, const char *parmname); @@ -197,6 +201,8 @@ config_init(void) config.ssl_verify = DEFAULT_AUTHSERVSSLPEERVER; config.ssl_cipher_list = NULL; config.arp_table_path = safe_strdup(DEFAULT_ARPTABLE); + config.user = safe_strdup(DEFAULT_USER); + config.group = safe_strdup(DEFAULT_GROUP); } /** @@ -764,6 +770,22 @@ config_read(const char *filename) debug(LOG_WARNING, "SSLAllowedCipherList is set but no SSL compiled in. Ignoring!"); #endif break; + case oUser: +#ifndef USE_LIBCAP + debug(LOG_WARNING, "Non-privileged user is set but not compiled with libcap. Bailing out!"); + exit(1); +#endif + free(config.user); + config.user = safe_strdup(p1); + break; + case oGroup: +#ifndef USE_LIBCAP + debug(LOG_WARNING, "Non-privileged group is set but not compiled with libcap. Bailing out!"); + exit(1); +#endif + free(config.group); + config.group = safe_strdup(p1); + break; case oBadOption: /* FALL THROUGH */ default: diff --git a/src/conf.h b/src/conf.h index c27dd4f2..a3bc8afe 100644 --- a/src/conf.h +++ b/src/conf.h @@ -72,6 +72,8 @@ /** Note that DEFAULT_AUTHSERVSSLNOPEERVER must be 0 or 1, even if the config file syntax is yes or no */ #define DEFAULT_AUTHSERVSSLPEERVER 1 /* 0 means: Enable peer verification */ #define DEFAULT_ARPTABLE "/proc/net/arp" +#define DEFAULT_USER "nobody" /* Unprivileged user, used if compiled with capabilities */ +#define DEFAULT_GROUP "nobody" /* Unprivileged group, used if compiled with capabilities */ /*@}*/ /*@{*/ @@ -193,6 +195,8 @@ typedef struct { t_trusted_mac *trustedmaclist; /**< @brief list of trusted macs */ char *arp_table_path; /**< @brief Path to custom ARP table, formatted like /proc/net/arp */ + char *user; /**< @brief Name of non-privileged user */ + char *group; /**< @brief Name of non-privileged group */ } s_config; /** @brief Get the current gateway configuration */ diff --git a/src/gateway.c b/src/gateway.c index a71ca398..fab6c0ab 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -502,14 +502,15 @@ gw_main(int argc, char **argv) config_init(); parse_commandline(argc, argv); -#ifdef USE_LIBCAP - drop_privileges("nobody", "nogroup"); -#endif /* USE LIBCAP */ /* Initialize the config */ config_read(config->configfile); config_validate(); +#ifdef USE_LIBCAP + drop_privileges(config->user, config->group); +#endif /* USE LIBCAP */ + /* Initializes the linked list of connected clients */ client_list_init(); diff --git a/wifidog.conf b/wifidog.conf index 4a9809f4..3432c064 100644 --- a/wifidog.conf +++ b/wifidog.conf @@ -1,6 +1,29 @@ # $Id$ # WiFiDog Configuration file +# Parameter: User +# Default: nobody +# Optional +# +# Set this to the name of a non-privileged user. Wifidog will change to this +# user instead of running as root, providing a slight security benefit. +# +# Only set this option if Wifidog is compiled with --enable-libcap. Setting +# this option otherwise is an error and Wifidog will not start. + +# User nobody + + +# Parameter: Group +# Default: nobody +# Optional +# +# Set this to the name of a non-privileged group. Wifidog will change to this +# group instead of running as root, providing a slight security benefit. +# +# Only set this option if Wifidog is compiled with --enable-libcap. Setting +# this option otherwise is an error and Wifidog will not start. + # Parameter: GatewayID # Default: default # Optional From 2131aae0af210943600d3c6368d4764923169191 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 12:41:32 +0200 Subject: [PATCH 12/40] Fix: CAP_NET_ADMIN is actually required, add back I made some wrong assumptions about execve() in a real-uid 0 context. Only the file capabilities are all enabled, which still requires the caller to have the appropriate capabilities. I will add a comment clarifying the behaviour here. --- src/capabilities.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 0eb47db2..8155bb75 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -23,7 +23,7 @@ * Wifidog does not need to run as root. The only capabilities required * are: * - CAP_NET_RAW: get IP addresses from sockets, ICMP ping - * - CAP_NET_ADMIN: set up iptables + * - CAP_NET_ADMIN: modify firewall rules * * This function drops all other capabilities. As only the effective * user id is set, it is theoretically possible for an attacker to @@ -44,14 +44,16 @@ drop_privileges(const char *user, const char *group) debug(LOG_DEBUG, "Entered drop_privileges"); /* The capabilities we want. * CAP_NET_RAW is used for our socket handling. - * We actually require CAP_NET_ADMIN for iptables. However, - * iptables must be run as root so we do that instead. */ + * CAP_NET_ADMIN is not used directly by iptables which + * is called by Wifidog + */ cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; cap_t caps; caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); - /* Hopefully clear all caps but cap_values */ + /* Clear all caps and then set the caps we desire */ + cap_clear(caps); cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); ret = cap_set_proc(caps); if (ret == -1) { @@ -93,7 +95,7 @@ drop_privileges(const char *user, const char *group) debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); debug(LOG_DEBUG, "Regaining capabilities."); /* Re-gain privileges */ - cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); + cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_values, CAP_SET); /* Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE * would be useful to execve iptables as non-root. In practice, Wifidog * often runs on embedded systems (OpenWrt) where the required file-based @@ -106,8 +108,13 @@ drop_privileges(const char *user, const char *group) * capabilities on the executable. Alas, it's not relevant here. * * This is also the main reason why we only seteuid() instead of setuid(): - * iptables will be called as root (with ALL capabilities!) and thus continue - * to work as before. + * When an executable is called as root (real UID == 0), the INHERITED + * and PERMITTED file capabilities are implicitly marked as enabled. + * + * TODO: what about EFFECTIVE? What is set-user-id-root in this context? + * Iptables probably works because of (F(permitted) & cap_bset) + * So, what is the bounding set after we're done? + * */ ret = cap_set_proc(caps); if (ret == -1) { From 05af333ecb39322f74a0047860c1b1c17c0ea66b Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 12:43:41 +0200 Subject: [PATCH 13/40] Add TODO - need to check return values! --- src/capabilities.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/capabilities.c b/src/capabilities.c index 8155bb75..ce115d56 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -76,6 +76,7 @@ drop_privileges(const char *user, const char *group) struct group *grp = getgrnam(group); if (grp) { gid_t gid = grp->gr_gid; + // TODO: check retval setegid(gid); } else { debug(LOG_ERR, "Failed to look up GID for group %s", group); @@ -86,6 +87,7 @@ drop_privileges(const char *user, const char *group) struct passwd *pwd = getpwnam(user); if (pwd) { uid_t uid = pwd->pw_uid; + // TODO: check retval seteuid(uid); } else { debug(LOG_ERR, "Failed to look up UID for user %s", user); From 6302dda6f06fd357b10c59f6168bf608e62253ae Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 18:49:24 +0200 Subject: [PATCH 14/40] Capabilities: error handling, comment --- src/capabilities.c | 57 +++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index ce115d56..f88fb29f 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -4,10 +4,14 @@ #ifdef USE_LIBCAP +#include + #include #include #include #include +/* For strerror */ +#include /* For exit */ #include /* For getpwnam */ @@ -35,7 +39,7 @@ * Any error is considered fatal and exit() is called. * * @param user Non-privileged user - * @param Non-privileged group + * @param group Non-privileged group */ void drop_privileges(const char *user, const char *group) @@ -47,6 +51,7 @@ drop_privileges(const char *user, const char *group) * CAP_NET_ADMIN is not used directly by iptables which * is called by Wifidog */ + const int num_caps = 2; cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; cap_t caps; @@ -54,7 +59,7 @@ drop_privileges(const char *user, const char *group) debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); /* Clear all caps and then set the caps we desire */ cap_clear(caps); - cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); + cap_set_flag(caps, CAP_PERMITTED, num_caps, cap_values, CAP_SET); ret = cap_set_proc(caps); if (ret == -1) { debug(LOG_ERR, "Could not set capabilities!"); @@ -74,30 +79,34 @@ drop_privileges(const char *user, const char *group) debug(LOG_DEBUG, "Switching to group %s", group); /* don't free grp, see getpwnam() for details */ struct group *grp = getgrnam(group); - if (grp) { - gid_t gid = grp->gr_gid; - // TODO: check retval - setegid(gid); - } else { - debug(LOG_ERR, "Failed to look up GID for group %s", group); + if (NULL == grp) { + debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); + exit(1); + } + gid_t gid = grp->gr_gid; + ret = setegid(gid); + if (ret != 0) { + debug(LOG_ERR, "Failed to setegid() for group %s: %s", group, strerror(errno)); exit(1); } debug(LOG_DEBUG, "Switching to user %s", user); - /* don't pwd, see getpwnam() for details */ + /* don't free pwd, see getpwnam() for details */ struct passwd *pwd = getpwnam(user); - if (pwd) { - uid_t uid = pwd->pw_uid; - // TODO: check retval - seteuid(uid); - } else { + if (NULL == pwd) { debug(LOG_ERR, "Failed to look up UID for user %s", user); exit(1); } + uid_t uid = pwd->pw_uid; + ret = seteuid(uid); + if (ret != 0) { + debug(LOG_ERR, "Failed to seteuid() for user %s: %s", user, strerror(errno)); + exit(1); + } caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); debug(LOG_DEBUG, "Regaining capabilities."); /* Re-gain privileges */ - cap_set_flag(caps, CAP_EFFECTIVE, 1, cap_values, CAP_SET); + cap_set_flag(caps, CAP_EFFECTIVE, num_caps, cap_values, CAP_SET); /* Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE * would be useful to execve iptables as non-root. In practice, Wifidog * often runs on embedded systems (OpenWrt) where the required file-based @@ -110,13 +119,19 @@ drop_privileges(const char *user, const char *group) * capabilities on the executable. Alas, it's not relevant here. * * This is also the main reason why we only seteuid() instead of setuid(): - * When an executable is called as root (real UID == 0), the INHERITED - * and PERMITTED file capabilities are implicitly marked as enabled. + * When an executable is called as root (real UID == 0), the INHERITABLE + * and PERMITTED file capability sets are implicitly marked as enabled. + * + * TODO: + * It is not clear to me why iptables has the necessary capabilities in its + * EFFECTIVE set. This should only happen for SUID0 executables. I also do + * not understand why iptables receives the necessary capabilities at all: * - * TODO: what about EFFECTIVE? What is set-user-id-root in this context? - * Iptables probably works because of (F(permitted) & cap_bset) - * So, what is the bounding set after we're done? + * P'(permitted) = (P(inheritable) & F(inheritable)) | + * (F(permitted) & cap_bset) * + * Since P(inheritable) is empty, P'(permitted) should be empty as well + * unless cap_bset is automatically populated. */ ret = cap_set_proc(caps); if (ret == -1) { @@ -124,7 +139,7 @@ drop_privileges(const char *user, const char *group) exit(1); } caps = cap_get_proc(); - debug(LOG_DEBUG, "Regained: %s", cap_to_text(caps, NULL)); + debug(LOG_INFO, "Final capabilities: %s", cap_to_text(caps, NULL)); cap_free(caps); } From a0a78a8b6c43f71a9575f8739a7320a247a68438 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 21:44:30 +0200 Subject: [PATCH 15/40] Refactor capabilities.c and hook up fw_iptables.c Turns out I was mistaken about some things - I had accidentally still ran iptables as root. The current code now drops from UID0 to a non-privileged user at startup. There are two occasions where the effective user ID is set back to 0: * in execute() in util.c * in fw_iptables.c with a wrapper around popen --- src/capabilities.c | 162 ++++++++++++++++++++++++++++++++------------- src/capabilities.h | 9 ++- src/fw_iptables.c | 11 ++- src/util.c | 6 ++ 4 files changed, 140 insertions(+), 48 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index f88fb29f..435fa1fc 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -10,6 +10,8 @@ #include #include #include +/* FILE and popen */ +#include /* For strerror */ #include /* For exit */ @@ -19,6 +21,7 @@ /* For getgrnam */ #include +#include "capabilities.h" #include "debug.h" /** @@ -46,6 +49,17 @@ drop_privileges(const char *user, const char *group) { int ret = 0; debug(LOG_DEBUG, "Entered drop_privileges"); + + /* + * We are about to drop our effective UID to a non-privileged user. + * This clears the EFFECTIVE capabilities set, so we later re-enable + * re-enable these. We can do that because they are not cleared from + * the PERMITTED set. + * Note: if we used setuid() instead of seteuid(), we would have lost the + * PERMITTED set as well. In this case, we would need to call prctl + * with PR_SET_KEEPCAPS. + */ + set_user_group(user, group); /* The capabilities we want. * CAP_NET_RAW is used for our socket handling. * CAP_NET_ADMIN is not used directly by iptables which @@ -68,46 +82,14 @@ drop_privileges(const char *user, const char *group) caps = cap_get_proc(); debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); cap_free(caps); - /* We are about to drop our effective UID to a non-privileged user. - * This clears the EFFECTIVE capabilities set, so we will have to - * re-enable these. We can do that because they are not cleared from - * the PERMITTED set. - * Note: if we used setuid() instead of seteuid(), we would have lost the - * PERMITTED set as well. In this case, we would need to call prctl - * with PR_SET_KEEPCAPS. - */ - debug(LOG_DEBUG, "Switching to group %s", group); - /* don't free grp, see getpwnam() for details */ - struct group *grp = getgrnam(group); - if (NULL == grp) { - debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); - exit(1); - } - gid_t gid = grp->gr_gid; - ret = setegid(gid); - if (ret != 0) { - debug(LOG_ERR, "Failed to setegid() for group %s: %s", group, strerror(errno)); - exit(1); - } - debug(LOG_DEBUG, "Switching to user %s", user); - /* don't free pwd, see getpwnam() for details */ - struct passwd *pwd = getpwnam(user); - if (NULL == pwd) { - debug(LOG_ERR, "Failed to look up UID for user %s", user); - exit(1); - } - uid_t uid = pwd->pw_uid; - ret = seteuid(uid); - if (ret != 0) { - debug(LOG_ERR, "Failed to seteuid() for user %s: %s", user, strerror(errno)); - exit(1); - } caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); debug(LOG_DEBUG, "Regaining capabilities."); /* Re-gain privileges */ cap_set_flag(caps, CAP_EFFECTIVE, num_caps, cap_values, CAP_SET); - /* Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE + cap_set_flag(caps, CAP_INHERITABLE, num_caps, cap_values, CAP_SET); + /* + * Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE * would be useful to execve iptables as non-root. In practice, Wifidog * often runs on embedded systems (OpenWrt) where the required file-based * capabilities are not available as the underlying file system does not @@ -121,17 +103,6 @@ drop_privileges(const char *user, const char *group) * This is also the main reason why we only seteuid() instead of setuid(): * When an executable is called as root (real UID == 0), the INHERITABLE * and PERMITTED file capability sets are implicitly marked as enabled. - * - * TODO: - * It is not clear to me why iptables has the necessary capabilities in its - * EFFECTIVE set. This should only happen for SUID0 executables. I also do - * not understand why iptables receives the necessary capabilities at all: - * - * P'(permitted) = (P(inheritable) & F(inheritable)) | - * (F(permitted) & cap_bset) - * - * Since P(inheritable) is empty, P'(permitted) should be empty as well - * unless cap_bset is automatically populated. */ ret = cap_set_proc(caps); if (ret == -1) { @@ -143,4 +114,103 @@ drop_privileges(const char *user, const char *group) cap_free(caps); } + +/** + * Switches the effective user ID to 0 (root). + * + * If the underlying seteuid call fails, an error message is logged. + * No other error handling is performed. + * + */ +void switch_to_root() { + int ret = 0; + ret = seteuid(0); + /* Not being able to raise privileges is not fatal. */ + if (ret != 0) { + debug(LOG_ERR, "execute: Could not seteuid(0): %s", strerror(errno)); + } + ret = setegid(0); + if (ret != 0) { + debug(LOG_ERR, "execute: Could not setegid(0): %s", strerror(errno)); + } + debug(LOG_DEBUG, "execute: Switched to UID 0!");; +} + + +/** + * Switches user and group, typically to a non-privileged user. + * + * If either user or group switching fails, this is considered fatal + * and exit() is called. + * + * @param user name of the user + * @param group name of the group + * + */ +void set_user_group(const char* user, const char* group) { + debug(LOG_DEBUG, "Switching to group %s", group); + /* don't free grp, see getpwnam() for details */ + struct group *grp = getgrnam(group); + if (NULL == grp) { + debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); + exit(1); + } + gid_t gid = grp->gr_gid; + struct passwd *pwd = getpwnam(user); + if (NULL == pwd) { + debug(LOG_ERR, "Failed to look up UID for user %s", user); + exit(1); + } + uid_t uid = pwd->pw_uid; + set_uid_gid(uid, gid); + +} + +/** + * Switches user ID and group ID, typically to a non-privileged user. + * + * If either user or group switching fails, this is considered fatal + * and exit() is called. + * + * @param uid the ID of the user + * @param gid the ID of the group + * + */ +void set_uid_gid(uid_t uid, gid_t gid) { + int ret; + ret = setegid(gid); + if (ret != 0) { + debug(LOG_ERR, "Failed to setegid() %s", strerror(errno)); + exit(1); + } + ret = seteuid(uid); + if (ret != 0) { + debug(LOG_ERR, "Failed to seteuid(): %s", strerror(errno)); + exit(1); + } +} + + +/** + * Calls popen with root privileges. + * + * This method is a wrapper around popen(). The effective + * user and group IDs of the current process are temporarily set + * to 0 (root) and then reset to the original, typically non-privileged, + * values before returning. + * + * @param command First popen parameter + * @param type Second popen parameter + * @returns File handle pointer returned by popen + */ +FILE *popen_as_root(const char *command, const char *type) { + FILE *p = NULL; + uid_t uid = getuid(); + gid_t gid = getgid(); + switch_to_root(); + p = popen(command, type); + set_uid_gid(uid, gid); + return p; +} + #endif /* USE_LIBCAP */ diff --git a/src/capabilities.h b/src/capabilities.h index d06c81bb..74ccf2f1 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -34,7 +34,14 @@ void drop_privileges(const char*, const char*); void -regain_privileges(); +switch_to_root(); + +FILE *popen_as_root(const char*, const char*); + +void set_user_group(const char*, const char*); + +void set_uid_gid(uid_t, gid_t); + #endif /* _CAPABILITIES_H_ */ #endif /* USE_LIBCAP */ diff --git a/src/fw_iptables.c b/src/fw_iptables.c index f526aa8a..b8d8b880 100644 --- a/src/fw_iptables.c +++ b/src/fw_iptables.c @@ -48,6 +48,9 @@ #include "debug.h" #include "util.h" #include "client_list.h" +#include "capabilities.h" + +#include "../config.h" static int iptables_do_command(const char *format, ...); static char *iptables_compile(const char *, const char *, const t_firewall_rule *); @@ -514,7 +517,13 @@ iptables_fw_destroy_mention(const char *table, const char *chain, const char *me safe_asprintf(&command, "iptables -t %s -L %s -n --line-numbers -v", table, chain); iptables_insert_gateway_id(&command); - if ((p = popen(command, "r"))) { + /* TODO: execute() already has privilege handling code */ +#ifdef USE_LIBCAP + p = popen_as_root(command, "r"); +#else + p = open(command, "r"); +#endif /* USE_LIBCAP */ + if (p) { /* Skip first 2 lines */ while (!feof(p) && fgetc(p) != '\n') ; while (!feof(p) && fgetc(p) != '\n') ; diff --git a/src/util.c b/src/util.c index b604227c..0028eebf 100644 --- a/src/util.c +++ b/src/util.c @@ -65,6 +65,7 @@ #include "pstring.h" #include "gateway.h" #include "commandline.h" +#include "capabilities.h" #include "../config.h" @@ -105,6 +106,11 @@ execute(const char *cmd_line, int quiet) /* We don't want to see any errors if quiet flag is on */ if (quiet) close(2); + +#ifdef USE_LIBCAP + /* There is no need to lower privileges again; we're exiting anyways */ + switch_to_root(); +#endif if (execvp("/bin/sh", (char *const *)new_argv) == -1) { /* execute the command */ debug(LOG_ERR, "execvp(): %s", strerror(errno)); } else { From 8ad25bc4cf3df241ffaacc9916586cdcb6476b0a Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 22:04:45 +0200 Subject: [PATCH 16/40] Remove TODO --- src/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index d83894a3..7d6fd398 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -35,7 +35,6 @@ libgateway_a_SOURCES = commandline.c \ simple_http.c \ pstring.c \ capabilities.c -# TODO: capabilities.c is always compiled, protect with ifdef? noinst_HEADERS = commandline.h \ common.h \ From 6f6cdac7ed919ae7a01051524d7e615e6b81bb7c Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 22:06:36 +0200 Subject: [PATCH 17/40] Add license header, fix formatting --- src/capabilities.c | 25 ++++++++++++++++++++++++- src/capabilities.h | 9 ++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 435fa1fc..55515a06 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -1,4 +1,27 @@ - +/* vim: set et sw=4 ts=4 sts=4 : */ +/********************************************************************\ + * This program is free software; you can redistribute it and/or * + * modify it under the terms of the GNU General Public License as * + * published by the Free Software Foundation; either version 2 of * + * the License, or (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License* + * along with this program; if not, contact: * + * * + * Free Software Foundation Voice: +1-617-542-5942 * + * 59 Temple Place - Suite 330 Fax: +1-617-542-2652 * + * Boston, MA 02111-1307, USA gnu@gnu.org * + * * +\********************************************************************/ + +/** @file capabilities.c + @author Copyright (C) 2015 Michael Haas +*/ #include "../config.h" diff --git a/src/capabilities.h b/src/capabilities.h index 74ccf2f1..d0bb47ac 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -36,11 +36,14 @@ drop_privileges(const char*, const char*); void switch_to_root(); -FILE *popen_as_root(const char*, const char*); +FILE* +popen_as_root(const char*, const char*); -void set_user_group(const char*, const char*); +void +set_user_group(const char*, const char*); -void set_uid_gid(uid_t, gid_t); +void +set_uid_gid(uid_t, gid_t); #endif /* _CAPABILITIES_H_ */ From be3e0199c7400881c3e2ed35721bfbef02565e7c Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 22:10:35 +0200 Subject: [PATCH 18/40] Fix typo: open -> popen --- src/fw_iptables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fw_iptables.c b/src/fw_iptables.c index b8d8b880..dccb3d88 100644 --- a/src/fw_iptables.c +++ b/src/fw_iptables.c @@ -521,7 +521,7 @@ iptables_fw_destroy_mention(const char *table, const char *chain, const char *me #ifdef USE_LIBCAP p = popen_as_root(command, "r"); #else - p = open(command, "r"); + p = popen(command, "r"); #endif /* USE_LIBCAP */ if (p) { /* Skip first 2 lines */ From ef379f3157ee00ef45175acd248a5806e2f48d31 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Fri, 3 Apr 2015 22:13:52 +0200 Subject: [PATCH 19/40] Fix help string for --enable-libcap --- configure.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index a24af039..f6e12f95 100644 --- a/configure.in +++ b/configure.in @@ -88,13 +88,13 @@ BB_ENABLE_DOXYGEN # Enable capabilities? AC_DEFUN([BB_LIBCAP], [ -AC_ARG_ENABLE(libcap, [ --enable-cyassl enable documentation generation with doxygen (no)], [], [enable_libcap=no]) +AC_ARG_ENABLE(libcap, [ --enable-libcap enable privilege dropping with capabilities (no)], [], [enable_libcap=no]) if test "x$enable_libcap" = xyes; then AC_CHECK_HEADERS(sys/capability.h) AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ AC_MSG_ERROR([unable to find the cap_get_proc function.]) ]) - AC_DEFINE(USE_LIBCAP,, Compile with libcap support) + AC_DEFINE(USE_LIBCAP,, "Compile with libcap support") fi ]) # Actually perform the libcap check From 79e60f112aa8037e89e819b505f0960a321a1a70 Mon Sep 17 00:00:00 2001 From: Alexandre Carmel-Veilleux Date: Fri, 3 Apr 2015 16:23:59 -0400 Subject: [PATCH 20/40] Fix alignment of configure help --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index f6e12f95..d2abf704 100644 --- a/configure.in +++ b/configure.in @@ -88,7 +88,7 @@ BB_ENABLE_DOXYGEN # Enable capabilities? AC_DEFUN([BB_LIBCAP], [ -AC_ARG_ENABLE(libcap, [ --enable-libcap enable privilege dropping with capabilities (no)], [], [enable_libcap=no]) +AC_ARG_ENABLE(libcap, [ --enable-libcap enable privilege dropping with capabilities (no)], [], [enable_libcap=no]) if test "x$enable_libcap" = xyes; then AC_CHECK_HEADERS(sys/capability.h) AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ From 9af82159c2032ef295b623d61b65dd3b712b1606 Mon Sep 17 00:00:00 2001 From: Alexandre Carmel-Veilleux Date: Fri, 3 Apr 2015 16:26:42 -0400 Subject: [PATCH 21/40] OCD realignment --- configure.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index d2abf704..e8efcae9 100644 --- a/configure.in +++ b/configure.in @@ -88,7 +88,7 @@ BB_ENABLE_DOXYGEN # Enable capabilities? AC_DEFUN([BB_LIBCAP], [ -AC_ARG_ENABLE(libcap, [ --enable-libcap enable privilege dropping with capabilities (no)], [], [enable_libcap=no]) +AC_ARG_ENABLE(libcap, [ --enable-libcap enable privilege dropping with capabilities (no)], [], [enable_libcap=no]) if test "x$enable_libcap" = xyes; then AC_CHECK_HEADERS(sys/capability.h) AC_SEARCH_LIBS([cap_get_proc], [cap], [], [ @@ -103,7 +103,7 @@ BB_LIBCAP # Enable cyassl? AC_DEFUN([BB_CYASSL], [ -AC_ARG_ENABLE(cyassl, [ --enable-cyassl enable TLS support for auth server communication (no)], [], [enable_cyassl=no]) +AC_ARG_ENABLE(cyassl, [ --enable-cyassl enable TLS support for auth server communication (no)], [], [enable_cyassl=no]) if test "x$enable_cyassl" = xyes; then AC_CHECK_HEADERS(cyassl/ssl.h) AC_SEARCH_LIBS([CyaSSLv23_client_method], [cyassl wolfssl], [], [ From 05c04c7b7dbc562cbab1444acddc43ea81e65988 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 10:18:36 +0200 Subject: [PATCH 22/40] Rename build type cyassl -> full "full" build type also builds with --enable-libcap --- .travis.yml | 2 +- .travis_configure_wrapper.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index b4dfdc47..5e07da5f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ env: - secure: fiVVKcMM8Cz8WAj6PB6eD/b+Y77klXOe9jbpehf6QwjFwf6paEHoMsrZ0aFXogm2Uej47GlTdRb3UkBqonbK4ANbu0ewsWCW0RGClZz5ghaSnfwdxEhuXsrFIax7DvJCStk2V84Keb+tSVemx4opxqZAlZ/Nen28S91KSDoJeRA= matrix: - BUILD_TYPE=normal - - CYASSL="3.3.2" BUILD_TYPE=cyassl + - CYASSL="3.3.2" BUILD_TYPE="full" cache: directories: - dependencies-src diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 241dab90..0db0edd7 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -10,7 +10,7 @@ if [[ "$BUILD_TYPE" == "normal" ]]; then echo "Running Wifidog configure" ./configure $@ -elif [[ "$BUILD_TYPE" == "cyassl" ]]; then +elif [[ "$BUILD_TYPE" == "full" ]]; then if [[ -z "$CYASSL" ]]; then echo "CYASSL not set." exit 1 @@ -50,7 +50,7 @@ elif [[ "$BUILD_TYPE" == "cyassl" ]]; then echo "Running Wifidog configure" export CFLAGS="-I${CUR}/dependencies-installed/include/" export LDFLAGS="-L${CUR}/dependencies-installed/lib/" - ./configure --enable-cyassl $@ + ./configure --enable-cyassl --enable-libcap $@ else echo "Unknow BUILD_TYPE $BUILD_TYPE" exit 1 From 9c263261ef41b7f77a2ea3001c2a7b807a57a8b9 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 10:19:29 +0200 Subject: [PATCH 23/40] Respect $CFLAGS in travis configure wrapper --- .travis_configure_wrapper.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 0db0edd7..cf047e5d 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -48,8 +48,8 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then echo "Cached CyaSSL install found." fi echo "Running Wifidog configure" - export CFLAGS="-I${CUR}/dependencies-installed/include/" - export LDFLAGS="-L${CUR}/dependencies-installed/lib/" + export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" + export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" ./configure --enable-cyassl --enable-libcap $@ else echo "Unknow BUILD_TYPE $BUILD_TYPE" From 58984c36f01078fe8f1d5cbbe166d7d28efc3e69 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 10:33:28 +0200 Subject: [PATCH 24/40] Travis: build libcap --- .travis.yml | 2 +- .travis_configure_wrapper.sh | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5e07da5f..9fc18bd4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ env: - secure: fiVVKcMM8Cz8WAj6PB6eD/b+Y77klXOe9jbpehf6QwjFwf6paEHoMsrZ0aFXogm2Uej47GlTdRb3UkBqonbK4ANbu0ewsWCW0RGClZz5ghaSnfwdxEhuXsrFIax7DvJCStk2V84Keb+tSVemx4opxqZAlZ/Nen28S91KSDoJeRA= matrix: - BUILD_TYPE=normal - - CYASSL="3.3.2" BUILD_TYPE="full" + - LIBCAP="2.24" CYASSL="3.3.2" BUILD_TYPE="full" cache: directories: - dependencies-src diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index cf047e5d..44453c70 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -18,6 +18,9 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then CUR=`pwd` mkdir -p dependencies-src || true mkdir -p dependencies-installed || true + # TODO: changing $CYASSL version number will not invalidate this check + # Need to remove full cache in travis interface if we want to upgrade + # CyaSSL if [[ ! -f dependencies-installed/include/cyassl/ssl.h ]]; then echo "Cached CyaSSL install not found. Installing." cd dependencies-src @@ -47,6 +50,28 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then else echo "Cached CyaSSL install found." fi + if [[ ! -f dependencies-installed/include/sys/capability.h ]]; then + echo "Cached libcap not found. Installing." + cd dependencies-src + if [[ -f libcap-${LIBCAP}/Makefile ]]; then + echo "Found cached libcap package" + else + echo "No cache, downloading libcap" + wget https://www.kernel.org/pub/linux/libs/security/linux-privs/libcap2/libcap-${LIBCAP}.tar.gz \ + -O libcap-${LIBCAP}.tar.gz + tar -xvzf libcap-${LIBCAP}.tar.gz + fi + cd libcap-${LIBCAP} + echo "Content of libcap-${LIBCAP}" + ls + echo "Running libcap make" + make + echo "Running libcap make install" + make install DESTDIR="$CUR"/dependencies-installed/ + cd $CUR + else + echo "Cached libcap install found." + fi echo "Running Wifidog configure" export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" From b7507175a86b786a0c71b4974d0dfa6013680612 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 10:47:21 +0200 Subject: [PATCH 25/40] Also build libattr as a libcap dependency --- .travis.yml | 2 +- .travis_configure_wrapper.sh | 46 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9fc18bd4..4a0bfb1b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ env: - secure: fiVVKcMM8Cz8WAj6PB6eD/b+Y77klXOe9jbpehf6QwjFwf6paEHoMsrZ0aFXogm2Uej47GlTdRb3UkBqonbK4ANbu0ewsWCW0RGClZz5ghaSnfwdxEhuXsrFIax7DvJCStk2V84Keb+tSVemx4opxqZAlZ/Nen28S91KSDoJeRA= matrix: - BUILD_TYPE=normal - - LIBCAP="2.24" CYASSL="3.3.2" BUILD_TYPE="full" + - LIBATTR="2.4.47" LIBCAP="2.24" CYASSL="3.3.2" BUILD_TYPE="full" cache: directories: - dependencies-src diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 44453c70..ea573b51 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -72,6 +72,52 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then else echo "Cached libcap install found." fi + if [[ ! -f dependencies-installed/include/attr/libattr.h ]]; then + echo "Cached libattr not found. Installing." + cd dependencies-src + if [[ -f libattr-${LIBATTR}/configure ]]; then + echo "Found cached libattr package" + else + echo "No cache, downloading libattr" + wget http://download.savannah.gnu.org/releases/attr/attr-${LIBATTR}.src.tar.gz \ + -O attr-${LIBATTR}.tar.gz + tar -xvzf attr-${LIBATTR}.tar.gz + fi + cd attr-${LIBATTR} + echo "Content of attr-${LIBATTR}" + ls + echo "Running attr configure" + ./configure --prefix="$CUR"/dependencies-installed/ + echo "Running attr make install" + make install + cd $CUR + else + echo "Cached attr install found." + fi + if [[ ! -f dependencies-installed/include/sys/capability.h ]]; then + echo "Cached libcap not found. Installing." + cd dependencies-src + if [[ -f libcap-${LIBCAP}/Makefile ]]; then + echo "Found cached libcap package" + else + echo "No cache, downloading libcap" + wget https://www.kernel.org/pub/linux/libs/security/linux-privs/libcap2/libcap-${LIBCAP}.tar.gz \ + -O libcap-${LIBCAP}.tar.gz + tar -xvzf libcap-${LIBCAP}.tar.gz + fi + cd libcap-${LIBCAP} + echo "Content of libcap-${LIBCAP}" + ls + echo "Running libcap make" + make + echo "Running libcap make install" + make install DESTDIR="$CUR"/dependencies-installed/ + cd $CUR + else + echo "Cached libcap install found." + fi + + echo "Running Wifidog configure" export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" From f5070787a6fd1fc871cbe9088a5d7b3b72acd39b Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 11:05:54 +0200 Subject: [PATCH 26/40] Refactor travis_configure_wrapper Also fix build and install of libattr --- .travis_configure_wrapper.sh | 94 ++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index ea573b51..ce7dd6eb 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -1,26 +1,44 @@ #!/usr/bin/env bash -if [[ -z "$BUILD_TYPE" ]]; then - echo "BUILD_TYPE not set. Bye." - exit 1 -fi -if [[ "$BUILD_TYPE" == "normal" ]]; then +function main { + if [[ -z "$BUILD_TYPE" ]]; then + echo "BUILD_TYPE not set. Bye." + exit 1 + fi + + if [[ "$BUILD_TYPE" == "normal" ]]; then - echo "Running Wifidog configure" - ./configure $@ + echo "Running Wifidog configure" + ./configure $@ + + elif [[ "$BUILD_TYPE" == "full" ]]; then + if [[ -z "$CYASSL" || -z "$LIBCAP" || -z "$LIBATTR" ]]; then + echo "Make sure that CYASSL, LIBCAP and LIBATTR are set." + exit 1 + fi + mkdir -p dependencies-src || true + mkdir -p dependencies-installed || true + build_cyassl + build_libattr + build_libcap -elif [[ "$BUILD_TYPE" == "full" ]]; then - if [[ -z "$CYASSL" ]]; then - echo "CYASSL not set." + echo "Running Wifidog configure" + export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" + export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" + ./configure --enable-cyassl --enable-libcap $@ + else + echo "Unknow BUILD_TYPE $BUILD_TYPE" exit 1 fi - CUR=`pwd` - mkdir -p dependencies-src || true - mkdir -p dependencies-installed || true + +} + +function build_cyassl { # TODO: changing $CYASSL version number will not invalidate this check # Need to remove full cache in travis interface if we want to upgrade # CyaSSL + CUR=`pwd` if [[ ! -f dependencies-installed/include/cyassl/ssl.h ]]; then echo "Cached CyaSSL install not found. Installing." cd dependencies-src @@ -50,7 +68,11 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then else echo "Cached CyaSSL install found." fi - if [[ ! -f dependencies-installed/include/sys/capability.h ]]; then +} + +function build_libcap { + CUR=`pwd` + if [[ ! -f dependencies-installed/usr/include/sys/capability.h ]]; then echo "Cached libcap not found. Installing." cd dependencies-src if [[ -f libcap-${LIBCAP}/Makefile ]]; then @@ -67,11 +89,16 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then echo "Running libcap make" make echo "Running libcap make install" - make install DESTDIR="$CUR"/dependencies-installed/ - cd $CUR + make install DESTDIR="$CUR"/dependencies-installed/ RAISE_SETFCAP=no + cd "$CUR" else echo "Cached libcap install found." fi + +} + +function build_libattr { + CUR=`pwd` if [[ ! -f dependencies-installed/include/attr/libattr.h ]]; then echo "Cached libattr not found. Installing." cd dependencies-src @@ -89,40 +116,11 @@ elif [[ "$BUILD_TYPE" == "full" ]]; then echo "Running attr configure" ./configure --prefix="$CUR"/dependencies-installed/ echo "Running attr make install" - make install + make install install-dev install-lib cd $CUR else echo "Cached attr install found." fi - if [[ ! -f dependencies-installed/include/sys/capability.h ]]; then - echo "Cached libcap not found. Installing." - cd dependencies-src - if [[ -f libcap-${LIBCAP}/Makefile ]]; then - echo "Found cached libcap package" - else - echo "No cache, downloading libcap" - wget https://www.kernel.org/pub/linux/libs/security/linux-privs/libcap2/libcap-${LIBCAP}.tar.gz \ - -O libcap-${LIBCAP}.tar.gz - tar -xvzf libcap-${LIBCAP}.tar.gz - fi - cd libcap-${LIBCAP} - echo "Content of libcap-${LIBCAP}" - ls - echo "Running libcap make" - make - echo "Running libcap make install" - make install DESTDIR="$CUR"/dependencies-installed/ - cd $CUR - else - echo "Cached libcap install found." - fi - +} - echo "Running Wifidog configure" - export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" - export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" - ./configure --enable-cyassl --enable-libcap $@ -else - echo "Unknow BUILD_TYPE $BUILD_TYPE" - exit 1 -fi +main From 5dee3cff897de259da2fec221c50c8f21b067e36 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 11:13:59 +0200 Subject: [PATCH 27/40] Reset CFLAGS, libattr fails due to -Werror --- .travis_configure_wrapper.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index ce7dd6eb..7a6d9139 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -19,13 +19,20 @@ function main { fi mkdir -p dependencies-src || true mkdir -p dependencies-installed || true + # reset CFLAGS because some dependencies generate warnings + OLD_CFLAGS="${CFLAGS}" + OLD_CXXFLAGS="${CXXFLAGS}" + OLD_LDFLAGS="${LDFLAGS}" + export CFLAGS="-I${CUR}/dependencies-installed/include/" + export CXXFLAGS="-I${CUR}/dependencies-installed/include/" + export LDFLAGS="-L${CUR}/dependencies-installed/lib/" build_cyassl build_libattr build_libcap - echo "Running Wifidog configure" - export CFLAGS="${CFLAGS} -I${CUR}/dependencies-installed/include/" - export LDFLAGS="${CFLAGS} -L${CUR}/dependencies-installed/lib/" + export CFLAGS="${OLD_CFLAGS} ${CFLAGS}" + export CXXFLAGS="${OLD_CXXFLAGS} ${LDFLAGS}" + export LDFLAGS="${OLD_LDFLAGS} ${LDFLAGS}" ./configure --enable-cyassl --enable-libcap $@ else echo "Unknow BUILD_TYPE $BUILD_TYPE" From 3e26b420e8c90f18e0d6016db441b07ee2b83f67 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:01:33 +0200 Subject: [PATCH 28/40] Free memory, use thread-safe getpwnam_r --- src/capabilities.c | 65 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 55515a06..e8914f35 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -46,6 +46,7 @@ #include "capabilities.h" #include "debug.h" +#include "safe.h" /** * Switches to non-privileged user and drops unneeded capabilities. @@ -93,6 +94,10 @@ drop_privileges(const char *user, const char *group) cap_t caps; caps = cap_get_proc(); + if (NULL == caps) { + debug(LOG_ERR, "cap_get_proc failed, exiting!"); + exit(1); + } debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); /* Clear all caps and then set the caps we desire */ cap_clear(caps); @@ -102,11 +107,20 @@ drop_privileges(const char *user, const char *group) debug(LOG_ERR, "Could not set capabilities!"); exit(1); } + cap_free(caps), caps = cap_get_proc(); + if (NULL == caps) { + debug(LOG_ERR, "cap_get_proc failed, exiting!"); + exit(1); + } debug(LOG_DEBUG, "Dropped caps, now: %s", cap_to_text(caps, NULL)); cap_free(caps); caps = cap_get_proc(); debug(LOG_DEBUG, "Current capabilities: %s", cap_to_text(caps, NULL)); + if (NULL == caps) { + debug(LOG_ERR, "cap_get_proc failed, exiting!"); + exit(1); + } debug(LOG_DEBUG, "Regaining capabilities."); /* Re-gain privileges */ cap_set_flag(caps, CAP_EFFECTIVE, num_caps, cap_values, CAP_SET); @@ -132,7 +146,12 @@ drop_privileges(const char *user, const char *group) debug(LOG_ERR, "Could not set capabilities!"); exit(1); } + cap_free(caps); caps = cap_get_proc(); + if (NULL == caps) { + debug(LOG_ERR, "cap_get_proc failed, exiting!"); + exit(1); + } debug(LOG_INFO, "Final capabilities: %s", cap_to_text(caps, NULL)); cap_free(caps); } @@ -172,20 +191,46 @@ void switch_to_root() { */ void set_user_group(const char* user, const char* group) { debug(LOG_DEBUG, "Switching to group %s", group); - /* don't free grp, see getpwnam() for details */ - struct group *grp = getgrnam(group); - if (NULL == grp) { - debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); + struct passwd *pwd = NULL; + struct passwd *pwdresult = NULL; + struct group *grp = NULL; + struct group *grpresult = NULL; + char *buf; + ssize_t bufsize; + int s; + + bufsize = sysconf(_SC_GETPW_R_SIZE_MAX); + + if (bufsize == -1) { + /* Suggested by man getgrnam_r */ + bufsize = 16384; + } + buf = safe_malloc(bufsize); + + s = getgrnam_r(group, grp, buf, bufsize, &grpresult); + + if (grpresult == NULL) { + if (s == 0) { + debug(LOG_ERR, "GID for group %s not found!", group); + } + else { + debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); + } exit(1); } - gid_t gid = grp->gr_gid; - struct passwd *pwd = getpwnam(user); - if (NULL == pwd) { - debug(LOG_ERR, "Failed to look up UID for user %s", user); + + s = getpwnam_r(user, pwd, buf, bufsize, &pwdresult); + if (pwdresult == NULL) { + if (s == 0) { + debug(LOG_ERR, "UID for user %s not found!", user); + } + else { + debug(LOG_ERR, "Failed to look up UID for user %s: %s", user, strerror(errno)); + } exit(1); } - uid_t uid = pwd->pw_uid; - set_uid_gid(uid, gid); + + set_uid_gid(pwd->pw_uid, grp->gr_gid); } From 3cc6acab5b93bb2d77c46ced2f43e65eff47a1ae Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:02:42 +0200 Subject: [PATCH 29/40] Fix unprivileged group example --- wifidog.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wifidog.conf b/wifidog.conf index 3432c064..50894844 100644 --- a/wifidog.conf +++ b/wifidog.conf @@ -24,6 +24,8 @@ # Only set this option if Wifidog is compiled with --enable-libcap. Setting # this option otherwise is an error and Wifidog will not start. +# User nobody + # Parameter: GatewayID # Default: default # Optional From b471afbffdc96e8a4103e56de574043283001904 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:11:19 +0200 Subject: [PATCH 30/40] Re-order declarations --- src/capabilities.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index e8914f35..0c614414 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -71,9 +71,17 @@ void drop_privileges(const char *user, const char *group) { + const int num_caps = 2; + /* The capabilities we want. + * CAP_NET_RAW is used for our socket handling. + * CAP_NET_ADMIN is not used directly by iptables which + * is called by Wifidog + */ + cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; + cap_t caps; int ret = 0; + debug(LOG_DEBUG, "Entered drop_privileges"); - /* * We are about to drop our effective UID to a non-privileged user. * This clears the EFFECTIVE capabilities set, so we later re-enable @@ -84,15 +92,6 @@ drop_privileges(const char *user, const char *group) * with PR_SET_KEEPCAPS. */ set_user_group(user, group); - /* The capabilities we want. - * CAP_NET_RAW is used for our socket handling. - * CAP_NET_ADMIN is not used directly by iptables which - * is called by Wifidog - */ - const int num_caps = 2; - cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; - cap_t caps; - caps = cap_get_proc(); if (NULL == caps) { debug(LOG_ERR, "cap_get_proc failed, exiting!"); From e445b38b947d6608bf5902e5f9c53f5fce6420d3 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:14:45 +0200 Subject: [PATCH 31/40] Fix missing variable in travis configure wrapper --- .travis_configure_wrapper.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 7a6d9139..628e5164 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -23,6 +23,7 @@ function main { OLD_CFLAGS="${CFLAGS}" OLD_CXXFLAGS="${CXXFLAGS}" OLD_LDFLAGS="${LDFLAGS}" + CUR=`pwd` export CFLAGS="-I${CUR}/dependencies-installed/include/" export CXXFLAGS="-I${CUR}/dependencies-installed/include/" export LDFLAGS="-L${CUR}/dependencies-installed/lib/" From 87c5572d13907ddd1a444d6464f493f88a9904e7 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:48:05 +0200 Subject: [PATCH 32/40] Fix libcap include paths --- .travis_configure_wrapper.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 628e5164..2796a348 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -94,10 +94,8 @@ function build_libcap { cd libcap-${LIBCAP} echo "Content of libcap-${LIBCAP}" ls - echo "Running libcap make" - make echo "Running libcap make install" - make install DESTDIR="$CUR"/dependencies-installed/ RAISE_SETFCAP=no + make install DESTDIR="$CUR"/dependencies-installed/ IPATH="${CFLAGS} -fPIC -I\$(topdir)/libcap/include/uapi -I\$(topdir)/libcap/include" LDFLAGS=${LDFLAGS} RAISE_SETFCAP=no cd "$CUR" else echo "Cached libcap install found." From 56c8fd2c69edb011e8263df82537cc9a547bf9fb Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 12:58:18 +0200 Subject: [PATCH 33/40] Fix libcap install location Do not install into lib64/, prefer lib/ --- .travis_configure_wrapper.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index 2796a348..ec0fdb1a 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -95,7 +95,7 @@ function build_libcap { echo "Content of libcap-${LIBCAP}" ls echo "Running libcap make install" - make install DESTDIR="$CUR"/dependencies-installed/ IPATH="${CFLAGS} -fPIC -I\$(topdir)/libcap/include/uapi -I\$(topdir)/libcap/include" LDFLAGS=${LDFLAGS} RAISE_SETFCAP=no + make install DESTDIR="$CUR"/dependencies-installed/ IPATH="${CFLAGS} -fPIC -I\$(topdir)/libcap/include/uapi -I\$(topdir)/libcap/include" LDFLAGS=${LDFLAGS} RAISE_SETFCAP=no lib=lib cd "$CUR" else echo "Cached libcap install found." From c78e25cd4f780dd1ac5eecfd3912be04c72699d5 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 13:05:37 +0200 Subject: [PATCH 34/40] Fix prefix for libcap --- .travis_configure_wrapper.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis_configure_wrapper.sh b/.travis_configure_wrapper.sh index ec0fdb1a..96d07370 100755 --- a/.travis_configure_wrapper.sh +++ b/.travis_configure_wrapper.sh @@ -95,7 +95,7 @@ function build_libcap { echo "Content of libcap-${LIBCAP}" ls echo "Running libcap make install" - make install DESTDIR="$CUR"/dependencies-installed/ IPATH="${CFLAGS} -fPIC -I\$(topdir)/libcap/include/uapi -I\$(topdir)/libcap/include" LDFLAGS=${LDFLAGS} RAISE_SETFCAP=no lib=lib + make install DESTDIR="$CUR"/dependencies-installed/ IPATH="${CFLAGS} -fPIC -I\$(topdir)/libcap/include/uapi -I\$(topdir)/libcap/include" LDFLAGS=${LDFLAGS} RAISE_SETFCAP=no lib=lib prefix=/ cd "$CUR" else echo "Cached libcap install found." From 3031faad4edcede691f8ae0c131fe5b65e14652d Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 13:13:33 +0200 Subject: [PATCH 35/40] Free some memory --- src/capabilities.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/capabilities.c b/src/capabilities.c index 0c614414..aabf6af7 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -230,6 +230,12 @@ void set_user_group(const char* user, const char* group) { } set_uid_gid(pwd->pw_uid, grp->gr_gid); + free(pwd); + free(pwdresult); + free(grp); + free(grpresult); + free(buf); + } From a83394c5e190725ab1daa3bd3845c8b302b110ca Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 13:28:58 +0200 Subject: [PATCH 36/40] Run indent --- src/capabilities.c | 33 +++++++++++++++++---------------- src/capabilities.h | 13 ++++++------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index aabf6af7..ce3e7b4c 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -80,7 +80,7 @@ drop_privileges(const char *user, const char *group) cap_value_t cap_values[] = { CAP_NET_RAW, CAP_NET_ADMIN }; cap_t caps; int ret = 0; - + debug(LOG_DEBUG, "Entered drop_privileges"); /* * We are about to drop our effective UID to a non-privileged user. @@ -106,8 +106,7 @@ drop_privileges(const char *user, const char *group) debug(LOG_ERR, "Could not set capabilities!"); exit(1); } - cap_free(caps), - caps = cap_get_proc(); + cap_free(caps), caps = cap_get_proc(); if (NULL == caps) { debug(LOG_ERR, "cap_get_proc failed, exiting!"); exit(1); @@ -155,7 +154,6 @@ drop_privileges(const char *user, const char *group) cap_free(caps); } - /** * Switches the effective user ID to 0 (root). * @@ -163,7 +161,9 @@ drop_privileges(const char *user, const char *group) * No other error handling is performed. * */ -void switch_to_root() { +void +switch_to_root() +{ int ret = 0; ret = seteuid(0); /* Not being able to raise privileges is not fatal. */ @@ -177,7 +177,6 @@ void switch_to_root() { debug(LOG_DEBUG, "execute: Switched to UID 0!");; } - /** * Switches user and group, typically to a non-privileged user. * @@ -188,7 +187,9 @@ void switch_to_root() { * @param group name of the group * */ -void set_user_group(const char* user, const char* group) { +void +set_user_group(const char *user, const char *group) +{ debug(LOG_DEBUG, "Switching to group %s", group); struct passwd *pwd = NULL; struct passwd *pwdresult = NULL; @@ -211,8 +212,7 @@ void set_user_group(const char* user, const char* group) { if (grpresult == NULL) { if (s == 0) { debug(LOG_ERR, "GID for group %s not found!", group); - } - else { + } else { debug(LOG_ERR, "Failed to look up GID for group %s: %s", group, strerror(errno)); } exit(1); @@ -222,8 +222,7 @@ void set_user_group(const char* user, const char* group) { if (pwdresult == NULL) { if (s == 0) { debug(LOG_ERR, "UID for user %s not found!", user); - } - else { + } else { debug(LOG_ERR, "Failed to look up UID for user %s: %s", user, strerror(errno)); } exit(1); @@ -235,7 +234,6 @@ void set_user_group(const char* user, const char* group) { free(grp); free(grpresult); free(buf); - } @@ -249,7 +247,9 @@ void set_user_group(const char* user, const char* group) { * @param gid the ID of the group * */ -void set_uid_gid(uid_t uid, gid_t gid) { +void +set_uid_gid(uid_t uid, gid_t gid) +{ int ret; ret = setegid(gid); if (ret != 0) { @@ -263,7 +263,6 @@ void set_uid_gid(uid_t uid, gid_t gid) { } } - /** * Calls popen with root privileges. * @@ -276,7 +275,9 @@ void set_uid_gid(uid_t uid, gid_t gid) { * @param type Second popen parameter * @returns File handle pointer returned by popen */ -FILE *popen_as_root(const char *command, const char *type) { +FILE * +popen_as_root(const char *command, const char *type) +{ FILE *p = NULL; uid_t uid = getuid(); gid_t gid = getgid(); @@ -286,4 +287,4 @@ FILE *popen_as_root(const char *command, const char *type) { return p; } -#endif /* USE_LIBCAP */ +#endif /* USE_LIBCAP */ diff --git a/src/capabilities.h b/src/capabilities.h index d0bb47ac..78a753ae 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -31,20 +31,19 @@ #define _CAPABILITIES_H_ void -drop_privileges(const char*, const char*); + drop_privileges(const char *, const char *); void -switch_to_root(); + switch_to_root(); -FILE* -popen_as_root(const char*, const char*); +FILE *popen_as_root(const char *, const char *); void -set_user_group(const char*, const char*); + set_user_group(const char *, const char *); void -set_uid_gid(uid_t, gid_t); + set_uid_gid(uid_t, gid_t); #endif /* _CAPABILITIES_H_ */ -#endif /* USE_LIBCAP */ +#endif /* USE_LIBCAP */ From 070435ac4fdd3f8daec69106bb3be256588ed03e Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 13:29:30 +0200 Subject: [PATCH 37/40] Fix superfluous newlines in capabilities.h decl --- src/capabilities.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/capabilities.h b/src/capabilities.h index 78a753ae..abc27644 100644 --- a/src/capabilities.h +++ b/src/capabilities.h @@ -30,19 +30,15 @@ #ifndef _CAPABILITIES_H_ #define _CAPABILITIES_H_ -void - drop_privileges(const char *, const char *); +void drop_privileges(const char *, const char *); -void - switch_to_root(); +void switch_to_root(); FILE *popen_as_root(const char *, const char *); -void - set_user_group(const char *, const char *); +void set_user_group(const char *, const char *); -void - set_uid_gid(uid_t, gid_t); +void set_uid_gid(uid_t, gid_t); #endif /* _CAPABILITIES_H_ */ From 42e534183652039d5588f28523e80fd4fc0224ce Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Wed, 8 Apr 2015 13:30:13 +0200 Subject: [PATCH 38/40] Fix typo: , -> ; --- src/capabilities.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/capabilities.c b/src/capabilities.c index ce3e7b4c..c7fed47c 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -106,7 +106,8 @@ drop_privileges(const char *user, const char *group) debug(LOG_ERR, "Could not set capabilities!"); exit(1); } - cap_free(caps), caps = cap_get_proc(); + cap_free(caps); + caps = cap_get_proc(); if (NULL == caps) { debug(LOG_ERR, "cap_get_proc failed, exiting!"); exit(1); From 897a6955e50185f150b16365f31cd01d0ec74e36 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Thu, 9 Apr 2015 12:53:44 +0200 Subject: [PATCH 39/40] Remove erroneous comment The comment explained a behaviour which no longer reflected the code --- src/capabilities.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index c7fed47c..fd0d4fc7 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -124,22 +124,6 @@ drop_privileges(const char *user, const char *group) /* Re-gain privileges */ cap_set_flag(caps, CAP_EFFECTIVE, num_caps, cap_values, CAP_SET); cap_set_flag(caps, CAP_INHERITABLE, num_caps, cap_values, CAP_SET); - /* - * Note that we keep CAP_INHERITABLE empty. In theory, CAP_INHERITABLE - * would be useful to execve iptables as non-root. In practice, Wifidog - * often runs on embedded systems (OpenWrt) where the required file-based - * capabilities are not available as the underlying file system does not - * support extended attributes. - * - * The linux capabilities implementation requires that the executable is - * specifically marked as being able to inherit capabilities from a calling - * process. This can be done by setting the Inheritable+Effective file - * capabilities on the executable. Alas, it's not relevant here. - * - * This is also the main reason why we only seteuid() instead of setuid(): - * When an executable is called as root (real UID == 0), the INHERITABLE - * and PERMITTED file capability sets are implicitly marked as enabled. - */ ret = cap_set_proc(caps); if (ret == -1) { debug(LOG_ERR, "Could not set capabilities!"); From 7a24ddba33f782ac3bd1d2b538ac4f19f8253fe4 Mon Sep 17 00:00:00 2001 From: Michael Haas Date: Thu, 9 Apr 2015 12:54:24 +0200 Subject: [PATCH 40/40] Fix typo --- src/capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/capabilities.c b/src/capabilities.c index fd0d4fc7..e4a6a0f4 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -85,7 +85,7 @@ drop_privileges(const char *user, const char *group) /* * We are about to drop our effective UID to a non-privileged user. * This clears the EFFECTIVE capabilities set, so we later re-enable - * re-enable these. We can do that because they are not cleared from + * these. We can do that because they are not cleared from * the PERMITTED set. * Note: if we used setuid() instead of seteuid(), we would have lost the * PERMITTED set as well. In this case, we would need to call prctl