From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Thu, 27 May 2010 18:46:39 +0000 (-0400)
Subject: Add init_session function to struct policy_plugin that gets called
X-Git-Tag: SUDO_1_8_0~578
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7e6d1d1f7d02f19906729f0d202e96c733181393;p=sudo

Add init_session function to struct policy_plugin that gets called
before the uid/gid/etc changes.  A struct passwd pointer is passed in,which may be NULL if the user does not exist in the passwd database.The sudoers module uses init_session to open the pam session as needed.
---

diff --git a/include/sudo_plugin.h b/include/sudo_plugin.h
index 442388346..0bbb7a35d 100644
--- a/include/sudo_plugin.h
+++ b/include/sudo_plugin.h
@@ -52,6 +52,7 @@ typedef int (*sudo_conv_t)(int num_msgs, const struct sudo_conv_message msgs[],
 typedef int (*sudo_printf_t)(int msg_type, const char *fmt, ...);
 
 /* Policy plugin type and defines */
+struct passwd;
 struct policy_plugin {
 #define SUDO_POLICY_PLUGIN     1
     unsigned int type; /* always SUDO_POLICY_PLUGIN */
@@ -68,6 +69,7 @@ struct policy_plugin {
 	const char *list_user);
     int (*validate)(void);
     void (*invalidate)(int remove);
+    int (*init_session)(struct passwd *pwd);
 };
 
 /* I/O plugin type and defines */
diff --git a/plugins/sudoers/auth/pam.c b/plugins/sudoers/auth/pam.c
index 62727defb..ea0a5e142 100644
--- a/plugins/sudoers/auth/pam.c
+++ b/plugins/sudoers/auth/pam.c
@@ -202,8 +202,22 @@ pam_begin_session(pw, auth)
     struct passwd *pw;
     sudo_auth *auth;
 {
-    int status;
+    int status = PAM_SUCCESS;
 
+    /*
+     * If there is no valid user we cannot open a PAM session.
+     * This is not an error as sudo can run commands with arbitrary
+     * uids, it just means we are done from a session management standpoint.
+     */
+    if (pw == NULL) {
+	if (pamh != NULL) {
+	    (void) pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+	    pamh = NULL;
+	}
+	goto done;
+    }
+
+    /* If the user did not have to authenticate there is no pam handle yet. */
     if (pamh == NULL)
 	pam_init(pw, NULL, NULL);
 
@@ -224,16 +238,14 @@ pam_begin_session(pw, auth)
     (void) pam_setcred(pamh, PAM_ESTABLISH_CRED);
 
 #ifndef NO_PAM_SESSION
-    /*
-     * To fully utilize PAM sessions we would need to keep a
-     * sudo process around until the command exits.  However, we
-     * can at least cause pam_limits to be run by opening and then
-     * immediately closing the session.
-     */
     status = pam_open_session(pamh, 0);
-    if (status != PAM_SUCCESS)
+    if (status != PAM_SUCCESS) {
 	(void) pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+	pamh = NULL;
+    }
 #endif
+
+done:
     return(status == PAM_SUCCESS ? AUTH_SUCCESS : AUTH_FAILURE);
 }
 
@@ -241,13 +253,14 @@ int
 pam_end_session(auth)
     sudo_auth *auth;
 {
-    int status;
+    int status = PAM_SUCCESS;
 
+    if (pamh) {
 #ifndef NO_PAM_SESSION
-    (void) pam_close_session(pamh, 0);
+	(void) pam_close_session(pamh, PAM_SILENT);
 #endif
-
-    status = pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+	status = pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+    }
     return(status == PAM_SUCCESS ? AUTH_SUCCESS : AUTH_FAILURE);
 }
 
diff --git a/plugins/sudoers/auth/sudo_auth.c b/plugins/sudoers/auth/sudo_auth.c
index 75436dbf6..e674ce4a0 100644
--- a/plugins/sudoers/auth/sudo_auth.c
+++ b/plugins/sudoers/auth/sudo_auth.c
@@ -234,7 +234,7 @@ cleanup:
     return rval;
 }
 
-int begin_session(struct passwd *pw)
+int auth_begin_session(struct passwd *pw)
 {
     sudo_auth *auth;
     int status;
@@ -251,7 +251,7 @@ int begin_session(struct passwd *pw)
     return TRUE;
 }
 
-int end_session(void)
+int auth_end_session(void)
 {
     sudo_auth *auth;
     int status;
diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c
index a729f2ade..77cf08c55 100644
--- a/plugins/sudoers/sudoers.c
+++ b/plugins/sudoers/sudoers.c
@@ -276,7 +276,20 @@ sudoers_policy_close(int exit_status, int error_code)
     /* We do not currently log the exit status. */
     if (error_code)
 	warningx("unable to execute %s: %s", safe_cmnd, strerror(error_code));
-    end_session();
+
+    /* Close the session we opened in sudoers_policy_init_session(). */
+    if (ISSET(sudo_mode, MODE_RUN|MODE_EDIT))
+	(void)auth_end_session();
+}
+
+/*
+ * The init_session function is called before executing the command
+ * and before uid/gid changes occur.
+ */
+static int
+sudoers_policy_init_session(struct passwd *pwd)
+{
+    return auth_begin_session(pwd);
 }
 
 static int
@@ -628,15 +641,6 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
 
     restore_perms();
 
-    /*
-     * Ideally we would like to do session setup (currently only PAM)
-     * from inside sudo itself, but this should be close enough.
-     */
-    if (ISSET(sudo_mode, MODE_RUN))
-	rval = begin_session(runas_pw);
-    if (ISSET(sudo_mode, MODE_EDIT))
-	rval = begin_session(sudo_user.pw);
-
 done:
     return rval;
 }
@@ -1438,7 +1442,8 @@ struct policy_plugin sudoers_policy = {
     sudoers_policy_check,
     sudoers_policy_list,
     sudoers_policy_validate,
-    sudoers_policy_invalidate
+    sudoers_policy_invalidate,
+    sudoers_policy_init_session
 };
 
 struct io_plugin sudoers_io = {
diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h
index c7252d052..9b738581b 100644
--- a/plugins/sudoers/sudoers.h
+++ b/plugins/sudoers/sudoers.h
@@ -210,8 +210,8 @@ int user_is_exempt(void);
 
 /* sudo_auth.c */
 int verify_user(struct passwd *, char *);
-int begin_session(struct passwd *);
-int end_session();
+int auth_begin_session(struct passwd *);
+int auth_end_session();
 
 #ifdef HAVE_LDAP
 /* ldap.c */
diff --git a/src/script.c b/src/script.c
index 4a0e351e0..48e933f73 100644
--- a/src/script.c
+++ b/src/script.c
@@ -639,7 +639,7 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
 	close(sv[0]);
 	fcntl(sv[1], F_SETFD, FD_CLOEXEC);
 	/* XXX - defer call to exec_setup() until my_execve()? */
-	if (exec_setup(details) == 0) {
+	if (exec_setup(details) == TRUE) {
 	    /* headed for execve() */
 	    if (log_io) {
 		/* Close the other end of the stdin/stdout/stderr pipes. */
diff --git a/src/sudo.c b/src/sudo.c
index 004617381..577397b83 100644
--- a/src/sudo.c
+++ b/src/sudo.c
@@ -651,13 +651,23 @@ cleanup(int gotsignal)
 
 /*
  * Setup the execution environment immediately prior to the call to execve()
+ * Returns TRUE on success and FALSE on failure.
  */
 int
 exec_setup(struct command_details *details)
 {
+    int rval = FALSE;
     struct passwd *pw;
 
     pw = getpwuid(details->euid);
+
+    /* Call policy plugin's session init before other setup occurs. */
+    if (policy_plugin.u.policy->init_session) {
+	/* The session init code is expected to print an error as needed. */
+	if (policy_plugin.u.policy->init_session(pw) != TRUE)
+	    goto done;
+    }
+
     if (pw != NULL) {
 #ifdef HAVE_GETUSERATTR
 	aix_setlimits(pw->pw_name);
@@ -732,15 +742,7 @@ exec_setup(struct command_details *details)
 	    goto done;
 	}
     }
-    if (details->cwd) {
-	/* cwd is relative to the new root, if any */
-	if (chdir(details->cwd) != 0) {
-	    warning("unable to change directory to %s", details->cwd);
-	    goto done;
-	}
-    }
 
-    /* Must set uids last */
 #ifdef HAVE_SETRESUID
     if (setresuid(details->uid, details->euid, details->euid) != 0) {
 	warning("unable to change to runas uid");
@@ -758,10 +760,19 @@ exec_setup(struct command_details *details)
     }
 #endif /* !HAVE_SETRESUID && !HAVE_SETREUID */
 
-    errno = 0;
+    /* Set cwd after uid to avoid permissions problems. */
+    if (details->cwd) {
+	/* Note: cwd is relative to the new root, if any. */
+	if (chdir(details->cwd) != 0) {
+	    warning("unable to change directory to %s", details->cwd);
+	    goto done;
+	}
+    }
+
+    rval = TRUE;
 
 done:
-    return errno;
+    return rval;
 }
 
 /*