- (dtucker) [auth-pam.c monitor.c session.c sshd.c] Bug #926: Move

   pam_open_session and pam_close_session into the privsep monitor, which
   will ensure that pam_session_close is called as root.  Patch from Tomas
   Mraz.

5 files changed, 30 insertions, 32 deletions

diff --git a/ChangeLog b/ChangeLog
index 3057c2be5..4bb82cd29 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+20080312
+ - (dtucker) [auth-pam.c monitor.c session.c sshd.c] Bug #926: Move
+   pam_open_session and pam_close_session into the privsep monitor, which
+   will ensure that pam_session_close is called as root.  Patch from Tomas
+   Mraz.
+
 20080309
  - (dtucker) [configure.ac] It turns out gcc's -fstack-protector-all doesn't
    always work for all platforms and versions, so test what we can and
@@ -3712,4 +3718,4 @@
    OpenServer 6 and add osr5bigcrypt support so when someone migrates
    passwords between UnixWare and OpenServer they will still work. OK dtucker@
 
-$Id: ChangeLog,v 1.4864 2008/03/09 11:50:50 dtucker Exp $
+$Id: ChangeLog,v 1.4865 2008/03/11 11:58:25 dtucker Exp $
diff --git a/auth-pam.c b/auth-pam.c
index a07f1fe77..ccdb9937e 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -598,15 +598,17 @@ static struct pam_conv store_conv = { sshpam_store_conv, NULL };
 void
 sshpam_cleanup(void)
 {
-	debug("PAM: cleanup");
-	if (sshpam_handle == NULL)
+	if (sshpam_handle == NULL || (use_privsep && !mm_is_monitor()))
 		return;
+	debug("PAM: cleanup");
 	pam_set_item(sshpam_handle, PAM_CONV, (const void *)&null_conv);
 	if (sshpam_cred_established) {
+		debug("PAM: deleting credentials");
 		pam_setcred(sshpam_handle, PAM_DELETE_CRED);
 		sshpam_cred_established = 0;
 	}
 	if (sshpam_session_open) {
+		debug("PAM: closing session");
 		pam_close_session(sshpam_handle, PAM_SILENT);
 		sshpam_session_open = 0;
 	}
diff --git a/monitor.c b/monitor.c
index 2f23669cd..cc0e0fcac 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1547,6 +1547,11 @@ mm_answer_term(int sock, Buffer *req)
 	/* The child is terminating */
 	session_destroy_all(&mm_session_close);
 
+#ifdef USE_PAM
+	if (options.use_pam)
+		sshpam_cleanup();
+#endif
+
 	while (waitpid(pmonitor->m_pid, &status, 0) == -1)
 		if (errno != EINTR)
 			exit(1);
diff --git a/session.c b/session.c
index 3eba513de..f5eaa815c 100644
--- a/session.c
+++ b/session.c
@@ -428,11 +428,6 @@ do_exec_no_pty(Session *s, const char *command)
 
 	session_proctitle(s);
 
-#if defined(USE_PAM)
-	if (options.use_pam && !use_privsep)
-		do_pam_setcred(1);
-#endif /* USE_PAM */
-
 	/* Fork the child. */
 	if ((pid = fork()) == 0) {
 		is_child = 1;
@@ -563,14 +558,6 @@ do_exec_pty(Session *s, const char *command)
 	ptyfd = s->ptyfd;
 	ttyfd = s->ttyfd;
 
-#if defined(USE_PAM)
-	if (options.use_pam) {
-		do_pam_set_tty(s->tty);
-		if (!use_privsep)
-			do_pam_setcred(1);
-	}
-#endif
-
 	/* Fork the child. */
 	if ((pid = fork()) == 0) {
 		is_child = 1;
@@ -1373,16 +1360,8 @@ do_setusercontext(struct passwd *pw)
 # ifdef __bsdi__
 		setpgid(0, 0);
 # endif
-#ifdef GSSAPI
-		if (options.gss_authentication) {
-			temporarily_use_uid(pw);
-			ssh_gssapi_storecreds();
-			restore_uid();
-		}
-#endif
 # ifdef USE_PAM
 		if (options.use_pam) {
-			do_pam_session();
 			do_pam_setcred(use_privsep);
 		}
 # endif /* USE_PAM */
@@ -1410,13 +1389,6 @@ do_setusercontext(struct passwd *pw)
 			exit(1);
 		}
 		endgrent();
-# ifdef GSSAPI
-		if (options.gss_authentication) {
-			temporarily_use_uid(pw);
-			ssh_gssapi_storecreds();
-			restore_uid();
-		}
-# endif
 # ifdef USE_PAM
 		/*
 		 * PAM credentials may take the form of supplementary groups.
@@ -1424,7 +1396,6 @@ do_setusercontext(struct passwd *pw)
 		 * Reestablish them here.
 		 */
 		if (options.use_pam) {
-			do_pam_session();
 			do_pam_setcred(use_privsep);
 		}
 # endif /* USE_PAM */
diff --git a/sshd.c b/sshd.c
index 5ea87f0f9..5dfc2b185 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1847,6 +1847,20 @@ main(int ac, char **av)
 	audit_event(SSH_AUTH_SUCCESS);
 #endif
 
+#ifdef GSSAPI
+	if (options.gss_authentication) {
+		temporarily_use_uid(authctxt->pw);
+		ssh_gssapi_storecreds();
+		restore_uid();
+	}
+#endif
+#ifdef USE_PAM
+	if (options.use_pam) {
+		do_pam_setcred(1);
+		do_pam_session();
+	}
+#endif
+
 	/*
 	 * In privilege separation, we fork another child and prepare
 	 * file descriptor passing.