[PATCH Version 2 0/4] GSSD: Do not fork when UID = 0

[PATCH Version 2 0/4] GSSD: Do not fork when UID = 0

[andros]

From: Andy Adamson <andros@netapp.com>

Version 2:
responded to comments.
- removed some printerr from 0003
- removed the SIGKILL call from 0004

Version 1:
Jeff Layton worked on this patch set with me.

patch 0001 and 0002 clean up process_krb5_upcall() by moving the two cases into
helper functions.

patch 0003 is the heart of this patch set.

commit f9cac65972da588d5218236de60a7be11247a8aa added the fork to
process_krb5_upcall so that the child assumes the uid of the principal
requesting service. This is good for the reasons listed in the commit.

When machine credentials are used, a gssd_k5_kt_princ entry is added to
a global list and used by future upcalls to note when valid machine credentials
have been obtained. When a child process performs this task, the entry to the
global list is lost upon exit, and all upcalls for machine credentials re-fetch
a TGT, even when a valid TGT is in the machine kerberos credential cache.

Since forking is not necessary when the principal has uid=0, solve the
gssd_k5_kt_princ_list issue by only forking when the uid != 0.

Please do more testing. Comments welcome.

-->Andy

Andy Adamson (4):
  GSSD: move process_krb5_upcall machine cred case to helper function
  GSSD: move process_krb5_updcall non machine cred case to helper
    function
  GSSD only fork when uid is not zeo
  GSSD: clean up machine credentials

 utils/gssd/gssd.c      |  11 ++-
 utils/gssd/gssd_proc.c | 239 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 150 insertions(+), 100 deletions(-)

-- 
1.9.3 (Apple Git-50)

[PATCH Version 2 1/4] GSSD: move process_krb5_upcall machine cred case to helper function

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
---
 utils/gssd/gssd_proc.c | 107 ++++++++++++++++++++++++++++---------------------
 1 file changed, 62 insertions(+), 45 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 03afc8b..f5a9ce1 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -482,6 +482,64 @@ change_identity(uid_t uid)
 	return 0;
 }
 
+AUTH *
+krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
+		    char *service, CLIENT **rpc_clnt)
+{
+	AUTH	*auth = NULL;
+	char	**credlist = NULL;
+	char	**ccname;
+	int	nocache = 0;
+	int	success = 0;
+
+	do {
+		gssd_refresh_krb5_machine_credential(clp->servername, NULL,
+						service);
+	/*
+	 * Get a list of credential cache names and try each
+	 * of them until one works or we've tried them all
+	 */
+		if (gssd_get_krb5_machine_cred_list(&credlist)) {
+			printerr(0, "ERROR: No credentials found "
+				"for connection to server %s\n",
+				clp->servername);
+			goto out;
+		}
+		for (ccname = credlist; ccname && *ccname; ccname++) {
+			gssd_setup_krb5_machine_gss_ccache(*ccname);
+			if ((create_auth_rpc_client(clp, tgtname, rpc_clnt,
+						&auth, uid,
+						AUTHTYPE_KRB5,
+						GSS_C_NO_CREDENTIAL)) == 0) {
+				/* Success! */
+				success++;
+				break;
+			}
+			printerr(2, "WARNING: Failed to create machine krb5"
+				"context with cred cache %s for server %s\n",
+				*ccname, clp->servername);
+		}
+		gssd_free_krb5_machine_cred_list(credlist);
+		if (!success) {
+			if(nocache == 0) {
+				nocache++;
+				printerr(2, "WARNING: Machine cache prematurely"					 "expired or corrupted trying to"
+					 "recreate cache for server %s\n",
+					clp->servername);
+			} else {
+				printerr(1, "WARNING: Failed to create machine"
+					 "krb5 context with any credentials"
+					 "cache for server %s\n",
+					clp->servername);
+				goto out;
+			}
+		}
+	} while(!success);
+
+out:
+	return auth;
+}
+
 /*
  * this code uses the userland rpcsec gss library to create a krb5
  * context on behalf of the kernel
@@ -494,8 +552,6 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 	AUTH			*auth = NULL;
 	struct authgss_private_data pd;
 	gss_buffer_desc		token;
-	char			**credlist = NULL;
-	char			**ccname;
 	char			**dirname;
 	int			create_resp = -1;
 	int			err, downcall_err = -EACCES;
@@ -587,49 +643,10 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 	if (create_resp != 0) {
 		if (uid == 0 && (root_uses_machine_creds == 1 ||
 				service != NULL)) {
-			int nocache = 0;
-			int success = 0;
-			do {
-				gssd_refresh_krb5_machine_credential(clp->servername,
-								     NULL, service);
-				/*
-				 * Get a list of credential cache names and try each
-				 * of them until one works or we've tried them all
-				 */
-				if (gssd_get_krb5_machine_cred_list(&credlist)) {
-					printerr(0, "ERROR: No credentials found "
-						 "for connection to server %s\n",
-						 clp->servername);
-					goto out_return_error;
-				}
-				for (ccname = credlist; ccname && *ccname; ccname++) {
-					gssd_setup_krb5_machine_gss_ccache(*ccname);
-					if ((create_auth_rpc_client(clp, tgtname, &rpc_clnt,
-								    &auth, uid,
-								    AUTHTYPE_KRB5,
-								    GSS_C_NO_CREDENTIAL)) == 0) {
-						/* Success! */
-						success++;
-						break;
-					}
-					printerr(2, "WARNING: Failed to create machine krb5 context "
-						 "with credentials cache %s for server %s\n",
-						 *ccname, clp->servername);
-				}
-				gssd_free_krb5_machine_cred_list(credlist);
-				if (!success) {
-					if(nocache == 0) {
-						nocache++;
-						printerr(2, "WARNING: Machine cache is prematurely expired or corrupted "
-						            "trying to recreate cache for server %s\n", clp->servername);
-					} else {
-						printerr(1, "WARNING: Failed to create machine krb5 context "
-						 "with any credentials cache for server %s\n",
-						 clp->servername);
-						goto out_return_error;
-					}
-				}
-			} while(!success);
+			auth =	krb5_use_machine_creds(clp, uid, tgtname,
+							service, &rpc_clnt);
+			if (auth == NULL)
+				goto out_return_error;
 		} else {
 			printerr(1, "WARNING: Failed to create krb5 context "
 				 "for user with uid %d for server %s\n",
-- 
1.9.3 (Apple Git-50)

[PATCH Version 2 2/4] GSSD: move process_krb5_updcall non machine cred case to helper function

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
---
 utils/gssd/gssd_proc.c | 74 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 27 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index f5a9ce1..0e04570 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -483,6 +483,49 @@ change_identity(uid_t uid)
 }
 
 AUTH *
+krb5_not_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
+			int *downcall_err, int *chg_err, CLIENT **rpc_clnt)
+{
+	AUTH		*auth = NULL;
+	gss_cred_id_t	gss_cred;
+	char		**dname;
+	int		err, resp = -1;
+
+	*chg_err = change_identity(uid);
+	if (*chg_err) {
+		printerr(0, "WARNING: failed to change identity: %s",
+			strerror(*chg_err));
+		goto out;
+	}
+
+	/** Tell krb5 gss which credentials cache to use.
+	 * Try first to acquire credentials directly via GSSAPI
+	 */
+	err = gssd_acquire_user_cred(&gss_cred);
+	if (err == 0)
+		resp = create_auth_rpc_client(clp, tgtname, rpc_clnt,
+						&auth, uid,
+						AUTHTYPE_KRB5, gss_cred);
+
+	/** if create_auth_rplc_client fails try the traditional
+	 * method of trolling for credentials
+	 */
+	for (dname = ccachesearch; resp != 0 && *dname != NULL; dname++) {
+		err = gssd_setup_krb5_user_gss_ccache(uid, clp->servername,
+						*dname);
+		if (err == -EKEYEXPIRED)
+			*downcall_err = -EKEYEXPIRED;
+		else if (err == 0)
+			resp = create_auth_rpc_client(clp, tgtname, rpc_clnt,
+						&auth, uid,AUTHTYPE_KRB5,
+						GSS_C_NO_CREDENTIAL);
+	}
+
+out:
+	return auth;
+}
+
+AUTH *
 krb5_use_machine_creds(struct clnt_info *clp, uid_t uid, char *tgtname,
 		    char *service, CLIENT **rpc_clnt)
 {
@@ -552,10 +595,7 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 	AUTH			*auth = NULL;
 	struct authgss_private_data pd;
 	gss_buffer_desc		token;
-	char			**dirname;
-	int			create_resp = -1;
 	int			err, downcall_err = -EACCES;
-	gss_cred_id_t		gss_cred;
 	OM_uint32		maj_stat, min_stat, lifetime_rec;
 	pid_t			pid;
 	gss_name_t		gacceptor = GSS_C_NO_NAME;
@@ -615,32 +655,12 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 		 service ? service : "<null>");
 	if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
 				service == NULL)) {
-
-		err = change_identity(uid);
-		if (err) {
-			printerr(0, "WARNING: failed to change identity: %s",
-				 strerror(err));
+		auth = krb5_not_machine_creds(clp, uid, tgtname, &downcall_err,
+						&err, &rpc_clnt);
+		if (err)
 			goto out_return_error;
-		}
-
-		/* Tell krb5 gss which credentials cache to use */
-		/* Try first to acquire credentials directly via GSSAPI */
-		err = gssd_acquire_user_cred(&gss_cred);
-		if (!err)
-			create_resp = create_auth_rpc_client(clp, tgtname, &rpc_clnt, &auth, uid,
-							     AUTHTYPE_KRB5, gss_cred);
-		/* if create_auth_rplc_client fails try the traditional method of
-		 * trolling for credentials */
-		for (dirname = ccachesearch; create_resp != 0 && *dirname != NULL; dirname++) {
-			err = gssd_setup_krb5_user_gss_ccache(uid, clp->servername, *dirname);
-			if (err == -EKEYEXPIRED)
-				downcall_err = -EKEYEXPIRED;
-			else if (!err)
-				create_resp = create_auth_rpc_client(clp, tgtname, &rpc_clnt, &auth, uid,
-							     AUTHTYPE_KRB5, GSS_C_NO_CREDENTIAL);
-		}
 	}
-	if (create_resp != 0) {
+	if (auth == NULL) {
 		if (uid == 0 && (root_uses_machine_creds == 1 ||
 				service != NULL)) {
 			auth =	krb5_use_machine_creds(clp, uid, tgtname,
-- 
1.9.3 (Apple Git-50)

[PATCH Version 2 3/4] GSSD only fork when uid is not zeo

[andros]

From: Andy Adamson <andros@netapp.com>

commit f9cac65972da588d5218236de60a7be11247a8aa added the fork to
process_krb5_upcall so that the child assumes the uid of the principal
requesting service.

When machine credentials are used, a gssd_k5_kt_princ entry is added to
a global list and used by future upcalls to note when valid machine credentials
have been obtained. When a child process performs this task, the entry to the
global list is lost upon exit, and all upcalls for machine credentials re-fetch
a TGT, even when a valid TGT is in the machine kerberos credential cache.

Since forking is not necessary when the principal has uid=0, solve the
gssd_k5_kt_princ_list issue by only forking when the uid != 0.

Signed-off-by: Andy Adamson <andros@netapp.com>
Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
---
 utils/gssd/gssd_proc.c | 62 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 0e04570..8501f38 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -597,33 +597,11 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 	gss_buffer_desc		token;
 	int			err, downcall_err = -EACCES;
 	OM_uint32		maj_stat, min_stat, lifetime_rec;
-	pid_t			pid;
+	pid_t			pid, childpid = -1;
 	gss_name_t		gacceptor = GSS_C_NO_NAME;
 	gss_OID			mech;
 	gss_buffer_desc		acceptor  = {0};
 
-	pid = fork();
-	switch(pid) {
-	case 0:
-		/* Child: fall through to rest of function */
-		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;
-	}
-
 	printerr(1, "handling krb5 upcall (%s)\n", clp->relpath);
 
 	token.length = 0;
@@ -655,6 +633,37 @@ process_krb5_upcall(struct clnt_info *clp, uid_t uid, int fd, char *tgtname,
 		 service ? service : "<null>");
 	if (uid != 0 || (uid == 0 && root_uses_machine_creds == 0 &&
 				service == NULL)) {
+
+		/* already running as uid 0 */
+		if (uid == 0)
+			goto no_fork;
+
+		printerr(1, "Forking\n");
+		pid = fork();
+		switch(pid) {
+		case 0:
+			/* Child: fall through to rest of function */
+			childpid = getpid();
+			printerr(1, "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)
@@ -721,7 +730,12 @@ out:
 		AUTH_DESTROY(auth);
 	if (rpc_clnt)
 		clnt_destroy(rpc_clnt);
-	exit(0);
+
+	pid = getpid();
+	if (pid == childpid)
+		exit(0);
+	else
+		return;
 
 out_return_error:
 	do_error_downcall(fd, uid, downcall_err);
-- 
1.9.3 (Apple Git-50)

[PATCH Version 2 4/4] GSSD: clean up machine credentials

[andros]

From: Andy Adamson <andros@netapp.com>

Since we no longer fork for uid 0, gssd_atexit() is only called when uid != 0,
and fails as permissions on the /tmp/krb5ccmachine_REALM file prohibit
the clean up of machine credentials (as it should).

Move the reaping of machine credentials back into a SIGINT sighandler so that
<Ctrl C> destroyes machine credentials.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 utils/gssd/gssd.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 2a768ea..e480349 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -729,10 +729,12 @@ found:
 }
 
 static void
-gssd_atexit(void)
+sig_die(int signal)
 {
 	if (root_uses_machine_creds)
 		gssd_destroy_krb5_machine_creds();
+	printerr(1, "exiting on signal %d\n", signal);
+	exit(0);
 }
 
 static void
@@ -892,17 +894,14 @@ main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	if (atexit(gssd_atexit)) {
-		printerr(1, "ERROR: atexit failed: %s\n", strerror(errno));
-		exit(EXIT_FAILURE);
-	}
-
 	inotify_fd = inotify_init1(IN_NONBLOCK);
 	if (inotify_fd == -1) {
 		printerr(1, "ERROR: inotify_init1 failed: %s\n", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
 
+	signal(SIGINT, sig_die);
+	signal(SIGTERM, sig_die);
 	signal_set(&sighup_ev, SIGHUP, gssd_scan_cb, NULL);
 	signal_add(&sighup_ev, NULL);
 	event_set(&inotify_ev, inotify_fd, EV_READ | EV_PERSIST, gssd_inotify_cb, NULL);
-- 
1.9.3 (Apple Git-50)

Re: [PATCH Version 2 0/4] GSSD: Do not fork when UID = 0

[Steve Dickson]

On 09/23/2015 10:30 AM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Version 2:
> responded to comments.
> - removed some printerr from 0003
> - removed the SIGKILL call from 0004
> 
> Version 1:
> Jeff Layton worked on this patch set with me.
> 
> patch 0001 and 0002 clean up process_krb5_upcall() by moving the two cases into
> helper functions.
> 
> patch 0003 is the heart of this patch set.
> 
> commit f9cac65972da588d5218236de60a7be11247a8aa added the fork to
> process_krb5_upcall so that the child assumes the uid of the principal
> requesting service. This is good for the reasons listed in the commit.
> 
> When machine credentials are used, a gssd_k5_kt_princ entry is added to
> a global list and used by future upcalls to note when valid machine credentials
> have been obtained. When a child process performs this task, the entry to the
> global list is lost upon exit, and all upcalls for machine credentials re-fetch
> a TGT, even when a valid TGT is in the machine kerberos credential cache.
> 
> Since forking is not necessary when the principal has uid=0, solve the
> gssd_k5_kt_princ_list issue by only forking when the uid != 0.
> 
> Please do more testing. Comments welcome.
> 
> -->Andy
> 
> Andy Adamson (4):
>   GSSD: move process_krb5_upcall machine cred case to helper function
>   GSSD: move process_krb5_updcall non machine cred case to helper
>     function
>   GSSD only fork when uid is not zeo
>   GSSD: clean up machine credentials
Committed all four of them... with some minor changes
in the debug statements and bug fixed in the third one... 

steved.

> 
>  utils/gssd/gssd.c      |  11 ++-
>  utils/gssd/gssd_proc.c | 239 ++++++++++++++++++++++++++++++-------------------
>  2 files changed, 150 insertions(+), 100 deletions(-)
>