2016-02-22 15:29:04

by Olga Kornievskaia

[permalink] [raw]
Subject: [RFC PATCH 0/3] adding pthread support to gssd

Adding support for multi-threaded gssd and allow for parallel upcalls.
For each upcall create a thread, detach to allow for automatic cleanup.
Communicate location of credential cache with gss api function instead
of an environmental variable. Use syscall api instead of libc setuid()
to change user identity.

Olga Kornievskaia (3):
gssd: use pthreads to handle upcalls
gssd: using syscalls directly to change thread's identity
gssd: always call gss_krb5_ccache_name

configure.ac | 3 +++
utils/gssd/Makefile.am | 3 ++-
utils/gssd/gssd.c | 28 ++++++++++++++++++++++++++--
utils/gssd/gssd.h | 2 ++
utils/gssd/gssd_proc.c | 42 +++++++++---------------------------------
utils/gssd/krb5_util.c | 35 ++++-------------------------------
utils/gssd/krb5_util.h | 2 +-
7 files changed, 47 insertions(+), 68 deletions(-)

--
1.8.3.1



2016-02-22 15:29:04

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/3] gssd: use pthreads to handle upcalls

Currently, to persevere global data over multiple mounts,
the root process does not fork when handling an upcall.
Instead on not-forking create a pthread to handle the
upcall since global data can be shared among threads.

Signed-off-by: Steve Dickson <[email protected]>
Signed-off-by: Olga Kornievskaia <[email protected]>
---
configure.ac | 3 +++
utils/gssd/Makefile.am | 3 ++-
utils/gssd/gssd.c | 34 +++++++++++++++++++++++++++++++---
utils/gssd/gssd.h | 2 ++
utils/gssd/gssd_proc.c | 30 ------------------------------
utils/gssd/krb5_util.c | 3 +++
6 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/configure.ac b/configure.ac
index 25d2ba4..e0ecd61 100644
--- a/configure.ac
+++ b/configure.ac
@@ -385,6 +385,9 @@ if test "$enable_gss" = yes; then
dnl Invoked after AC_KERBEROS_V5; AC_LIBRPCSECGSS needs to have KRBLIBS set
AC_LIBRPCSECGSS

+ dnl Check for pthreads
+ AC_LIBPTHREAD
+
dnl librpcsecgss already has a dependency on libgssapi,
dnl but we need to make sure we get the right version
if test "$enable_gss" = yes; then
diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am
index cb040b3..3f5f59a 100644
--- a/utils/gssd/Makefile.am
+++ b/utils/gssd/Makefile.am
@@ -49,7 +49,8 @@ gssd_LDADD = \
$(RPCSECGSS_LIBS) \
$(KRBLIBS) \
$(GSSAPI_LIBS) \
- $(LIBTIRPC)
+ $(LIBTIRPC) \
+ $(LIBPTHREAD)

gssd_LDFLAGS = \
$(KRBLDFLAGS)
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index e7cb07f..a480417 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -361,20 +361,48 @@ gssd_destroy_client(struct clnt_info *clp)

static void gssd_scan(void);

+/* For each upcall create a thread, detach from the main process so that
+ * resources are released back into the system without the need for a join.
+ * pthread_yield() is needed to allow the child thread to run right away
+ * and consume the upcall from the file descriptor. otherwise, if the main
+ * thread goes back into libevent code it will trigger another callback as
+ * the file descriptor is still "ready" and not consumed"
+ */
static void
gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data)
{
struct clnt_info *clp = data;
-
- handle_gssd_upcall(clp);
+ pthread_t th;
+ int ret;
+
+ ret = pthread_create(&th, NULL, (void *)handle_gssd_upcall,
+ (void *)clp);
+ if (ret != 0) {
+ printerr(0, "ERROR: pthread_create failed: ret %d: %s\n",
+ ret, strerror(errno));
+ return;
+ }
+ pthread_detach(th);
+ pthread_yield();
}

static void
gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void *data)
{
struct clnt_info *clp = data;
+ pthread_t th;
+ int ret;
+
+ ret = pthread_create(&th, NULL, (void *)handle_krb5_upcall,
+ (void *)clp);
+ if (ret != 0) {
+ printerr(0, "ERROR: pthread_create failed: ret %d: %s\n",
+ ret, strerror(errno));
+ return;
+ }

- handle_krb5_upcall(clp);
+ pthread_detach(th);
+ pthread_yield();
}

static struct clnt_info *
diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index c6937c5..8677329 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -36,6 +36,7 @@
#include <gssapi/gssapi.h>
#include <event.h>
#include <stdbool.h>
+#include <pthread.h>

#ifndef GSSD_PIPEFS_DIR
#define GSSD_PIPEFS_DIR "/var/lib/nfs/rpc_pipefs"
@@ -61,6 +62,7 @@ extern int root_uses_machine_creds;
extern unsigned int context_timeout;
extern unsigned int rpc_timeout;
extern char *preferred_realm;
+extern pthread_mutex_t ple_lock;

struct clnt_info {
TAILQ_ENTRY(clnt_info) list;
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 1ef68d8..616082e 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -629,36 +629,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
service == NULL)) {

- /* already running as uid 0 */
- if (uid == 0)
- goto no_fork;
-
- pid = fork();
- switch(pid) {
- case 0:
- /* Child: fall through to rest of function */
- childpid = getpid();
- unsetenv("KRB5CCNAME");
- printerr(2, "CHILD forked pid %d \n", childpid);
- break;
- case -1:
- /* fork() failed! */
- printerr(0, "WARNING: unable to fork() to handle"
- "upcall: %s\n", strerror(errno));
- return;
- default:
- /* Parent: just wait on child to exit and return */
- do {
- pid = wait(&err);
- } while(pid == -1 && errno != -ECHILD);
-
- if (WIFSIGNALED(err))
- printerr(0, "WARNING: forked child was killed"
- "with signal %d\n", WTERMSIG(err));
- return;
- }
-no_fork:
-
auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err,
&err, &rpc_clnt);
if (err)
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 8ef8184..3328696 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -128,6 +128,7 @@

/* Global list of principals/cache file names for machine credentials */
struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL;
+pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER;

#ifdef HAVE_SET_ALLOWABLE_ENCTYPES
int limit_to_legacy_enctypes = 0;
@@ -586,10 +587,12 @@ get_ple_by_princ(krb5_context context, krb5_principal princ)

/* Need to serialize list if we ever become multi-threaded! */

+ pthread_mutex_lock(&ple_lock);
ple = find_ple_by_princ(context, princ);
if (ple == NULL) {
ple = new_ple(context, princ);
}
+ pthread_mutex_unlock(&ple_lock);

return ple;
}
--
1.8.3.1


2016-02-22 15:29:05

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 2/3] gssd: using syscalls directly to change thread's identity

For the threaded version we have to set uid,gid per thread instead
of per process. glibc setresuid() when called from a thread, it'll
send a signal to all other threads to synchronize the uid in all
other threads. To bypass this, we have to call syscall() directly.

Signed-off-by: Olga Kornievskaia <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
---
utils/gssd/gssd_proc.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 616082e..fee72c6 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -69,6 +69,7 @@
#include <netdb.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <syscall.h>

#include "gssd.h"
#include "err_util.h"
@@ -457,7 +458,12 @@ change_identity(uid_t uid)
* Switch the GIDs. Note that we leave the saved-set-gid alone in an
* attempt to prevent attacks via ptrace()
*/
- if (setresgid(pw->pw_gid, pw->pw_gid, -1) != 0) {
+ /* For the threaded version we have to set uid,gid per thread instead
+ * of per process. glibc setresuid() when called from a thread, it'll
+ * send a signal to all other threads to synchronize the uid in all
+ * other threads. To bypass this, we have to call syscall() directly.
+ */
+ if (syscall(SYS_setresgid, pw->pw_gid) != 0) {
printerr(0, "WARNING: failed to set gid to %u!\n", pw->pw_gid);
return errno;
}
@@ -466,7 +472,7 @@ change_identity(uid_t uid)
* Switch UIDs, but leave saved-set-uid alone to prevent ptrace() by
* other processes running with this uid.
*/
- if (setresuid(uid, uid, -1) != 0) {
+ if (syscall(SYS_setresuid, uid) != 0) {
printerr(0, "WARNING: Failed to setuid for user with uid %u\n",
uid);
return errno;
--
1.8.3.1


2016-02-22 15:29:21

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 3/3] gssd: always call gss_krb5_ccache_name

Previously the location of the credential cache was passed in either
using environment variable KRB5CCNAME or gss_krb5_ccache_name() if
supported. For threaded-gssd, we can't use an environment variable
as it's shared among all thread. Thus always use the api call.

Signed-off-by: Olga Kornievskaia <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
---
utils/gssd/gssd_proc.c | 12 +++++++++--
utils/gssd/krb5_util.c | 56 +++++++++-----------------------------------------
utils/gssd/krb5_util.h | 1 -
3 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index fee72c6..bff29cf 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -554,7 +554,15 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
goto out;
}
for (ccname = credlist; ccname && *ccname; ccname++) {
- gssd_setup_krb5_machine_gss_ccache(*ccname);
+ u_int min_stat;
+
+ if (gss_krb5_ccache_name(&min_stat, *ccname, NULL) !=
+ GSS_S_COMPLETE) {
+ printerr(1, "WARNING: gss_krb5_ccache_name "
+ "with name '%s' failed (%s)\n",
+ *ccname, error_message(min_stat));
+ continue;
+ }
if ((create_auth_rpc_client(clp, tgtname, rpc_clnt,
&auth, uid,
AUTHTYPE_KRB5,
@@ -575,7 +583,7 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
"recreate cache for server %s\n",
clp->servername);
} else {
- printerr(1, "WARNING: Failed to create machine"
+ printerr(0, "ERROR: Failed to create machine"
"krb5 context with any credentials"
"cache for server %s\n",
clp->servername);
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 3328696..7b74ab3 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -468,37 +468,6 @@ gssd_get_single_krb5_cred(krb5_context context,
}

/*
- * Depending on the version of Kerberos, we either need to use
- * a private function, or simply set the environment variable.
- */
-static void
-gssd_set_krb5_ccache_name(char *ccname)
-{
-#ifdef USE_GSS_KRB5_CCACHE_NAME
- u_int maj_stat, min_stat;
-
- printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n",
- ccname);
- maj_stat = gss_krb5_ccache_name(&min_stat, ccname, NULL);
- if (maj_stat != GSS_S_COMPLETE) {
- printerr(0, "WARNING: gss_krb5_ccache_name with "
- "name '%s' failed (%s)\n",
- ccname, error_message(min_stat));
- }
-#else
- /*
- * Set the KRB5CCNAME environment variable to tell the krb5 code
- * which credentials cache to use. (Instead of using the private
- * function above for which there is no generic gssapi
- * equivalent.)
- */
- printerr(3, "using environment variable to select krb5 ccache %s\n",
- ccname);
- setenv("KRB5CCNAME", ccname, 1);
-#endif
-}
-
-/*
* Given a principal, find a matching ple structure
*/
static struct gssd_k5_kt_princ *
@@ -1094,6 +1063,7 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
const char *cctype;
struct dirent *d;
int err, i, j;
+ u_int maj_stat, min_stat;

printerr(3, "looking for client creds with uid %u for "
"server %s in %s\n", uid, servername, dirpattern);
@@ -1129,22 +1099,16 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)

printerr(2, "using %s as credentials cache for client with "
"uid %u for server %s\n", buf, uid, servername);
- gssd_set_krb5_ccache_name(buf);
- return 0;
-}

-/*
- * Let the gss code know where to find the machine credentials ccache.
- *
- * Returns:
- * void
- */
-void
-gssd_setup_krb5_machine_gss_ccache(char *ccname)
-{
- printerr(2, "using %s as credentials cache for machine creds\n",
- ccname);
- gssd_set_krb5_ccache_name(ccname);
+ printerr(3, "using gss_krb5_ccache_name to select krb5 ccache %s\n",
+ buf);
+ maj_stat = gss_krb5_ccache_name(&min_stat, buf, NULL);
+ if (maj_stat != GSS_S_COMPLETE) {
+ printerr(0, "ERROR: unable to get user cred cache '%s' "
+ "failed (%s)\n", buf, error_message(min_stat));
+ return maj_stat;
+ }
+ return 0;
}

/*
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index a319588..d3b0777 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -27,7 +27,6 @@ int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername,
char *dirname);
int gssd_get_krb5_machine_cred_list(char ***list);
void gssd_free_krb5_machine_cred_list(char **list);
-void gssd_setup_krb5_machine_gss_ccache(char *servername);
void gssd_destroy_krb5_machine_creds(void);
int gssd_refresh_krb5_machine_credential(char *hostname,
struct gssd_k5_kt_princ *ple,
--
1.8.3.1