From c5e618930bdaed616c90e510e62b7439bcb229be Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: Dec 20 2018 18:58:19 +0000 Subject: [PATCH 1/3] Grant CAP_SYS_PTRACE for gssproxy non-privileged user Non-root user should have CAP_SYS_PTRACE capability to read '/proc/[PID]/exe'. Fixes: https://pagure.io/gssproxy/issue/239 Signed-off-by: Stanislav Levin --- diff --git a/Makefile.am b/Makefile.am index 5f3aeb0..3595963 100644 --- a/Makefile.am +++ b/Makefile.am @@ -88,7 +88,7 @@ AM_CPPFLAGS += \ -DSYSCONFDIR=\"$(sysconfdir)\" \ -DLOCALEDIR=\"$(localedir)\" -GSS_PROXY_LIBS = $(POPT_LIBS) $(KRB5_LIBS) $(VERTO_LIBS) $(INI_LIBS) $(GSSAPI_LIBS) $(GSSRPC_LIBS) +GSS_PROXY_LIBS = $(POPT_LIBS) $(KRB5_LIBS) $(VERTO_LIBS) $(INI_LIBS) $(GSSAPI_LIBS) $(GSSRPC_LIBS) $(CAP_LIBS) if BUILD_SELINUX GSS_PROXY_LIBS += $(SELINUX_LIBS) diff --git a/configure.ac b/configure.ac index 0af44ab..a7f6aaf 100644 --- a/configure.ac +++ b/configure.ac @@ -280,6 +280,13 @@ AC_CHECK_LIB(gssrpc, gssrpc_xdrmem_create,, [$GSSAPI_LIBS $GSSRPC_LIBS]) AC_SUBST([GSSRPC_LIBS]) +AC_CHECK_FUNC([prctl],,[AC_MSG_ERROR([Failed to find prctl])]) +AC_CHECK_LIB([cap], [cap_set_proc],[CAP_LIBS=-lcap], + [AC_MSG_ERROR(["Failed to find libcap symbols"])]) +AC_SUBST([CAP_LIBS]) +AC_CHECK_HEADERS([sys/capability.h],, + [AC_MSG_ERROR([Could not find libcap headers])]) + AC_CHECK_FUNCS([__secure_getenv secure_getenv]) WITH_INITSCRIPT diff --git a/contrib/gssproxy.spec.in b/contrib/gssproxy.spec.in index ccd3e50..3ce7113 100644 --- a/contrib/gssproxy.spec.in +++ b/contrib/gssproxy.spec.in @@ -40,6 +40,7 @@ BuildRequires: libselinux-devel BuildRequires: keyutils-libs-devel BuildRequires: libini_config-devel >= 1.2.0 BuildRequires: libverto-devel +BuildRequires: libcap-devel BuildRequires: popt-devel BuildRequires: findutils BuildRequires: systemd-units diff --git a/src/gp_init.c b/src/gp_init.c index f64e22c..537b7a5 100644 --- a/src/gp_init.c +++ b/src/gp_init.c @@ -1,17 +1,21 @@ /* Copyright (C) 2011,2015 the GSS-PROXY contributors, see COPYING for license */ -#include -#include -#include +#include +#include +#include +#include #include +#include #include -#include -#include +#include +#include #include +#include +#include +#include +#include #include -#include -#include -#include + #include "gp_proxy.h" void init_server(bool daemonize, int *wait_fd) @@ -223,6 +227,28 @@ int drop_privs(struct gp_config *cfg) return 0; } + /* When a thread that has a zero value for one or more of + * its user IDs resets all of these values to nonzero + * a permitted capability set is also cleared. + * + * Permitted capability set is used as a limiting superset + * for the effective capabilities, which in turn are used + * by the kernel to perform permission checks. + * + * This means that such a thread can never reacquire those + * capabilities then. + * + * To change the default behavior SECBIT_KEEP_CAPS flag can + * be used. This flag allows keeping permitted capability set + * during UID switching. */ + ret = prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); + if (ret) { + ret = errno; + GPDEBUG("Failed to set keep capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + return ret; + } + ret = getpwnam_r(cfg->proxy_user, &pws, buf, 2048, &pw); if (ret) { GPDEBUG("Failed to look up proxy user: '%s'! [%d:%s]\n", @@ -253,5 +279,162 @@ int drop_privs(struct gp_config *cfg) return ret; } + /* keep only CAP_SYS_PTRACE capability, + * all the other are redundant */ + ret = drop_caps(); + if (ret) { + return ret; + } + + /* restore SECBIT_KEEP_CAPS */ + if (prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0)) { + ret = errno; + GPDEBUG("Failed to reset keep capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + return ret; + } + return 0; } + +/* The capability bounding set is a security mechanism that is + * used to limit the file permitted capabilities. To prevent + * applying any of them capability bounding set has to be + * constrained. + * + * To drop capabilities from the bounding set a thread has to + * have CAP_SETPCAP capability. */ +int clear_bound_caps() +{ + cap_t caps = NULL; + cap_value_t cap = 0; + const cap_value_t setpcap_list[] = { CAP_SETPCAP }; + int ret; + + /* obtain a copy of current process capability sets + * to raise CAP_SETPCAP */ + caps = cap_get_proc(); + if (caps == NULL) { + ret = errno; + GPDEBUG("Failed to get current capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* raise CAP_SETPCAP within an effective set */ + if (cap_set_flag(caps, CAP_EFFECTIVE, 1, setpcap_list, CAP_SET) == -1) { + ret = errno; + GPDEBUG("Failed to raise setpcap capability flag: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* apply back our capability sets to the current process */ + if (cap_set_proc(caps) == -1) { + ret = errno; + GPDEBUG("Failed to set capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* having CAP_SETPCAP within an effective set completely drop + * the bounding set capability */ + while (CAP_IS_SUPPORTED(cap)) { + if (cap_drop_bound(cap) != 0) { + ret = errno; + GPDEBUG("Failed to drop bounding set capability: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + cap++; + } + ret = 0; + +done: + if (caps && cap_free(caps) == -1) { + ret = errno; + GPDEBUG("Failed to free capability state: [%d:%s]\n", + ret, gp_strerror(ret)); + } + return ret; +} + +/* To serve a 'program =' functionality ("If specified, this + * service will only match when the program being run is the + * specified string.") a non-privileged user has to have a + * read permission on "/proc/[PID]/exe". This can be achieved + * by CAP_SYS_PTRACE capability. + * + * For now thread has an effective capability set inherited from + * privileged user because of SECBIT_KEEP_CAPS flag. But required + * is only CAP_SYS_PTRACE within the effective and permitted sets. + * It needs to restrict redundant privileged capabilities of a + * non-privileged user. */ +int drop_caps() +{ + cap_t caps = NULL; + int ret; + const cap_value_t ptrace_list[] = { CAP_SYS_PTRACE }; + + /* to limit a set of file permitted capabilities completely + * drop the bounding set */ + ret = clear_bound_caps(); + if (ret) { + goto done; + } + + ret = CAP_IS_SUPPORTED(CAP_SYS_PTRACE); + if (ret == -1) { + ret = errno; + GPDEBUG("Failed to check if capability is supported: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } else if (!ret) { + GPDEBUG("Capability CAPS_SYS_PTRACE is not supported\n"); + ret = EINVAL; + goto done; + } + + /* allocates a clear capability state */ + caps = cap_init(); + if (caps == NULL) { + ret = errno; + GPDEBUG("Failed to init capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* to raise CAP_SYS_PTRACE within the effective set a same + * capability has to be present within the permitted one */ + if (cap_set_flag(caps, CAP_PERMITTED, 1, ptrace_list, CAP_SET) == -1) { + ret = errno; + GPDEBUG("Failed to set permitted ptrace capability flag: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* raise CAP_SYS_PTRACE within the effective set */ + if (cap_set_flag(caps, CAP_EFFECTIVE, 1, ptrace_list, CAP_SET) == -1) { + ret = errno; + GPDEBUG("Failed to set effective ptrace capability flag: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + + /* apply our new capability sets to the current process */ + if (cap_set_proc(caps) == -1) { + ret = errno; + GPDEBUG("Failed to set capabilities: [%d:%s]\n", + ret, gp_strerror(ret)); + goto done; + } + ret = 0; + +done: + if (caps && cap_free(caps) == -1) { + ret = errno; + GPDEBUG("Failed to free capability state: [%d:%s]\n", + ret, gp_strerror(ret)); + } + return ret; +} diff --git a/src/gp_proxy.h b/src/gp_proxy.h index 3e944ab..8763bcf 100644 --- a/src/gp_proxy.h +++ b/src/gp_proxy.h @@ -102,6 +102,8 @@ verto_ctx *init_event_loop(void); void init_proc_nfsd(struct gp_config *cfg); void write_pid(void); int drop_privs(struct gp_config *cfg); +int drop_caps(void); +int clear_bound_caps(void); /* from gp_socket.c */ void free_unix_socket(verto_ctx *ctx, verto_ev *ev); From 54e9c819a31c6d3f1e5d06444ef77ed93e023f88 Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: Jan 09 2019 09:22:17 +0000 Subject: [PATCH 2/3] Make build with capabilities optional The new configure option has been added to avoid build against libcap by default. Fixes: https://pagure.io/gssproxy/issue/239 Signed-off-by: Stanislav Levin --- diff --git a/Makefile.am b/Makefile.am index 3595963..408391d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -88,12 +88,16 @@ AM_CPPFLAGS += \ -DSYSCONFDIR=\"$(sysconfdir)\" \ -DLOCALEDIR=\"$(localedir)\" -GSS_PROXY_LIBS = $(POPT_LIBS) $(KRB5_LIBS) $(VERTO_LIBS) $(INI_LIBS) $(GSSAPI_LIBS) $(GSSRPC_LIBS) $(CAP_LIBS) +GSS_PROXY_LIBS = $(POPT_LIBS) $(KRB5_LIBS) $(VERTO_LIBS) $(INI_LIBS) $(GSSAPI_LIBS) $(GSSRPC_LIBS) if BUILD_SELINUX GSS_PROXY_LIBS += $(SELINUX_LIBS) endif +if HAVE_CAP + GSS_PROXY_LIBS += $(CAP_LIBS) +endif + GP_RPCGEN_OBJ = rpcgen/gp_rpc_xdr.c rpcgen/gss_proxy_xdr.c rpcgen/gp_xdr.c GP_RPCCLI_OBJ = \ src/client/gpm_display_status.c \ diff --git a/conf_macros.m4 b/conf_macros.m4 index ceb46a7..d81792b 100644 --- a/conf_macros.m4 +++ b/conf_macros.m4 @@ -274,3 +274,20 @@ AC_DEFUN([WITH_HARDENING], ) AM_CONDITIONAL([BUILD_HARDENING], [test x"$with_hardening" = xyes]) ]) + +AC_DEFUN([WITH_CAP], + [ AC_ARG_WITH([cap], + [AC_HELP_STRING([--with-cap], + [Whether to build with libcap [no]] + ) + ], + [], + with_cap=no + ) + if test x"$with_cap" = xyes; then + HAVE_CAP=1 + AC_SUBST(HAVE_CAP) + AC_DEFINE_UNQUOTED([HAVE_CAP], [1], [Build with capabilities support]) + fi + ]) +AM_CONDITIONAL([HAVE_CAP], [test x$with_cap = xyes]) diff --git a/configure.ac b/configure.ac index a7f6aaf..4fbe5bf 100644 --- a/configure.ac +++ b/configure.ac @@ -280,12 +280,15 @@ AC_CHECK_LIB(gssrpc, gssrpc_xdrmem_create,, [$GSSAPI_LIBS $GSSRPC_LIBS]) AC_SUBST([GSSRPC_LIBS]) -AC_CHECK_FUNC([prctl],,[AC_MSG_ERROR([Failed to find prctl])]) -AC_CHECK_LIB([cap], [cap_set_proc],[CAP_LIBS=-lcap], - [AC_MSG_ERROR(["Failed to find libcap symbols"])]) -AC_SUBST([CAP_LIBS]) -AC_CHECK_HEADERS([sys/capability.h],, - [AC_MSG_ERROR([Could not find libcap headers])]) +WITH_CAP +if test x$HAVE_CAP != x; then + AC_CHECK_FUNC([prctl],,[AC_MSG_ERROR([Failed to find prctl])]) + AC_CHECK_LIB([cap], [cap_set_proc],[CAP_LIBS=-lcap], + [AC_MSG_ERROR(["Failed to find libcap symbols"])]) + AC_SUBST([CAP_LIBS]) + AC_CHECK_HEADERS([sys/capability.h],, + [AC_MSG_ERROR([Could not find libcap headers])]) +fi AC_CHECK_FUNCS([__secure_getenv secure_getenv]) diff --git a/src/gp_init.c b/src/gp_init.c index 537b7a5..e1069bf 100644 --- a/src/gp_init.c +++ b/src/gp_init.c @@ -1,21 +1,28 @@ /* Copyright (C) 2011,2015 the GSS-PROXY contributors, see COPYING for license */ +#include + #include #include #include -#include #include #include #include #include #include #include -#include -#include #include #include #include +#ifdef HAVE_CAP + +#include +#include +#include + +#endif + #include "gp_proxy.h" void init_server(bool daemonize, int *wait_fd) @@ -227,6 +234,7 @@ int drop_privs(struct gp_config *cfg) return 0; } +#ifdef HAVE_CAP /* When a thread that has a zero value for one or more of * its user IDs resets all of these values to nonzero * a permitted capability set is also cleared. @@ -248,6 +256,7 @@ int drop_privs(struct gp_config *cfg) ret, gp_strerror(ret)); return ret; } +#endif ret = getpwnam_r(cfg->proxy_user, &pws, buf, 2048, &pw); if (ret) { @@ -279,6 +288,7 @@ int drop_privs(struct gp_config *cfg) return ret; } +#ifdef HAVE_CAP /* keep only CAP_SYS_PTRACE capability, * all the other are redundant */ ret = drop_caps(); @@ -293,10 +303,12 @@ int drop_privs(struct gp_config *cfg) ret, gp_strerror(ret)); return ret; } +#endif return 0; } +#ifdef HAVE_CAP /* The capability bounding set is a security mechanism that is * used to limit the file permitted capabilities. To prevent * applying any of them capability bounding set has to be @@ -438,3 +450,4 @@ done: } return ret; } +#endif diff --git a/src/gp_proxy.h b/src/gp_proxy.h index 8763bcf..8ccb5ab 100644 --- a/src/gp_proxy.h +++ b/src/gp_proxy.h @@ -102,8 +102,10 @@ verto_ctx *init_event_loop(void); void init_proc_nfsd(struct gp_config *cfg); void write_pid(void); int drop_privs(struct gp_config *cfg); +#ifdef HAVE_CAP int drop_caps(void); int clear_bound_caps(void); +#endif /* from gp_socket.c */ void free_unix_socket(verto_ctx *ctx, verto_ev *ev); From 568e6b5961086dc1e94cef2594e1ef18bd8f160b Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: Jan 09 2019 09:22:25 +0000 Subject: [PATCH 3/3] Check for gssproxy user in config before calling of "drop_privs" As for now, it's confusing that privileges are always dropped because the actual check of config happens within the "drop_privs" function. Fixes: https://pagure.io/gssproxy/issue/239 Signed-off-by: Stanislav Levin --- diff --git a/src/gp_init.c b/src/gp_init.c index e1069bf..712541c 100644 --- a/src/gp_init.c +++ b/src/gp_init.c @@ -229,11 +229,6 @@ int drop_privs(struct gp_config *cfg) struct passwd *pw, pws; int ret; - if (cfg->proxy_user == NULL) { - /* not dropping privs */ - return 0; - } - #ifdef HAVE_CAP /* When a thread that has a zero value for one or more of * its user IDs resets all of these values to nonzero diff --git a/src/gssproxy.c b/src/gssproxy.c index 93c1c1e..01d4ef9 100644 --- a/src/gssproxy.c +++ b/src/gssproxy.c @@ -269,10 +269,14 @@ int main(int argc, const char *argv[]) * so it can continue with dependencies and start nfsd */ init_done(wait_fd); - ret = drop_privs(gpctx->config); - if (ret) { - ret = EXIT_FAILURE; - goto cleanup; + /* if config option "run_as_user" is missing, then it's no need to + * drop privileges */ + if (gpctx->config->proxy_user) { + ret = drop_privs(gpctx->config); + if (ret) { + ret = EXIT_FAILURE; + goto cleanup; + } } ret = gp_workers_init(gpctx);