* src/su.c (prepare_pam_close_session): Extract the creation of a

	child and listening for signal in the parent from run_shell().
	prepare_pam_close_session() is now executed before the creation of
	the pam session and before the UID is changed. This allows to
	close the session as root.

diff --git a/ChangeLog b/ChangeLog
index 7ef7ce6b180d29e28fadf71777d8f151d12ec34a..fcb31271237e7e22237a941ca35fd99edb569e79 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-06-13  Nicolas François  <nicolas.francois@centraliens.net>
+
+       * src/su.c (prepare_pam_close_session): Extract the creation of a
+       child and listening for signal in the parent from run_shell().
+       prepare_pam_close_session() is now executed before the creation of
+       the pam session and before the UID is changed. This allows to
+       close the session as root.
+
 2011-06-12  Nicolas François  <nicolas.francois@centraliens.net>
 
        * src/su.c (save_caller_context): Extract from main() the code

diff --git a/NEWS b/NEWS
index 6de516d82af720fbb10bae7b9455219616313f09..4a8c16088e251fc622b3cfda0226cc8338b80b3d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,8 @@ shadow-4.1.4.3 -> shadow-4.1.5                                        UNRELEASED
   * Do not forward the controlling terminal to commands executed with -c.
     This prevents tty hijacking which could lead to execution with the
     caller's privileges.
+  * Close PAM sessions as root. This will be more friendly to PAM modules
+    like pam_mount or pam_systemd.
 - newgrp, sg, groupmems
   * Fix parsing of gshadow entries.
 - useradd

diff --git a/src/su.c b/src/su.c
index bf3377c881733d97e90fb35607d7b15ac299adb6..869e0232bb6e9ee853c76a582ebb6138152075e4 100644 (file)
--- a/src/su.c
+++ b/src/su.c
@@ -257,49 +257,31 @@ static void catch_signals (int sig)
        caught = sig;
 }
 
-/* This I ripped out of su.c from sh-utils after the Mandrake pam patch
- * have been applied.  Some work was needed to get it integrated into
- * su.c from shadow.
+/*
+ * Create a session and fork.
+ * Only the child returns. The parent will wait for the child to terminate
+ * and exit.
  */
-static void run_shell (const char *shellstr, char *args[], bool doshell,
-                       char *const envp[])
+static void prepare_pam_close_session (void)
 {
-       pid_t child;
        sigset_t ourset;
        int status;
        int ret;
 
-       child = fork ();
-       if (child == 0) {       /* child shell */
-               /*
-                * PAM_DATA_SILENT is not supported by some modules, and
-                * there is no strong need to clean up the process space's
-                * memory since we will either call exec or exit.
-               pam_end (pamh, PAM_SUCCESS | PAM_DATA_SILENT);
-                */
-
-               if (doshell) {
-                       (void) shell (shellstr, (char *) args[0], envp);
-               } else {
-                       /* There is no need for a controlling terminal.
-                        * This avoids the callee to inject commands on
-                        * the caller's tty. */
-                       (void) setsid ();
-
-                       execve_shell (shellstr, (char **) args, envp);
-               }
-
-               exit (errno == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
-       } else if ((pid_t)-1 == child) {
+       pid_child = fork ();
+       if (pid_child == 0) {   /* child shell */
+               return; /* Only the child will return from pam_create_session */
+       } else if ((pid_t)-1 == pid_child) {
                (void) fprintf (stderr,
                                _("%s: Cannot fork user shell\n"),
                                Prog);
                SYSLOG ((LOG_WARN, "Cannot execute %s", shellstr));
                closelog ();
                exit (1);
+               /* Only the child returns. See above. */
        }
+
        /* parent only */
-       pid_child = child;
        sigfillset (&ourset);
        if (sigprocmask (SIG_BLOCK, &ourset, NULL) != 0) {
                (void) fprintf (stderr,
@@ -320,8 +302,8 @@ static void run_shell (const char *shellstr, char *args[], bool doshell,
                    || (sigaction (SIGTERM, &action, NULL) != 0)
                    || (   !doshell /* handle SIGINT (Ctrl-C), SIGQUIT
                                     * (Ctrl-\), and SIGTSTP (Ctrl-Z)
-                                    * since the child does not control
-                                    * the tty anymore.
+                                    * since the child will not control
+                                    * the tty.
                                     */
                        && (   (sigaddset (&ourset, SIGINT)  != 0)
                            || (sigaddset (&ourset, SIGQUIT) != 0)
@@ -359,7 +341,7 @@ static void run_shell (const char *shellstr, char *args[], bool doshell,
                                 * We will SIGSTOP ourself on the next
                                 * waitpid round.
                                 */
-                               kill (child, SIGSTOP);
+                               kill (pid_child, SIGSTOP);
                                stop = false;
                        } else if (   ((pid_t)-1 != pid)
                                   && (0 != WIFSTOPPED (status))) {
@@ -377,13 +359,13 @@ static void run_shell (const char *shellstr, char *args[], bool doshell,
                (void) fputs ("\n", stderr);
                (void) fputs (_("Session terminated, terminating shell..."),
                              stderr);
-               (void) kill (child, caught);
+               (void) kill (pid_child, caught);
        }
 
        ret = pam_close_session (pamh, 0);
        if (PAM_SUCCESS != ret) {
                SYSLOG ((LOG_ERR, "pam_close_session: %s",
-                        pam_strerror (pamh, ret)));
+                        pam_strerror (pamh, ret)));
                fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
                (void) pam_end (pamh, ret);
                exit (1);
@@ -401,8 +383,33 @@ static void run_shell (const char *shellstr, char *args[], bool doshell,
 
        exit ((0 != WIFEXITED (status)) ? WEXITSTATUS (status)
                                        : WTERMSIG (status) + 128);
+       /* Only the child returns. See above. */
 }
-#endif
+
+static void run_shell (const char *shellstr, char *args[], bool doshell,
+                       char *const envp[])
+{
+               /*
+                * PAM_DATA_SILENT is not supported by some modules, and
+                * there is no strong need to clean up the process space's
+                * memory since we will either call exec or exit.
+               pam_end (pamh, PAM_SUCCESS | PAM_DATA_SILENT);
+                */
+
+               if (doshell) {
+                       (void) shell (shellstr, (char *) args[0], envp);
+               } else {
+                       /* There is no need for a controlling terminal.
+                        * This avoids the callee to inject commands on
+                        * the caller's tty. */
+                       (void) setsid ();
+
+                       execve_shell (shellstr, (char **) args, envp);
+               }
+
+               exit (errno == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
+}
+#endif                         /* USE_PAM */
 
 /*
  * usage - print command line syntax and exit
@@ -929,9 +936,9 @@ int main (int argc, char **argv)
        ret = pam_start ("su", name, &conv, &pamh);
        if (PAM_SUCCESS != ret) {
                SYSLOG ((LOG_ERR, "pam_start: error %d", ret);
-                       fprintf (stderr,
-                                _("%s: pam_start: error %d\n"),
-                                Prog, ret));
+               fprintf (stderr,
+                        _("%s: pam_start: error %d\n"),
+                        Prog, ret));
                exit (1);
        }
 
@@ -941,7 +948,7 @@ int main (int argc, char **argv)
        }
        if (PAM_SUCCESS != ret) {
                SYSLOG ((LOG_ERR, "pam_set_item: %s",
-                        pam_strerror (pamh, ret)));
+                        pam_strerror (pamh, ret)));
                fprintf (stderr, _("%s: %s\n"), Prog, pam_strerror (pamh, ret));
                pam_end (pamh, ret);
                exit (1);
@@ -1020,6 +1027,8 @@ int main (int argc, char **argv)
                exit (1);
        }
 
+       prepare_pam_close_session ();
+
        /* become the new user */
        if (change_uid (pw) != 0) {
                pam_close_session (pamh, 0);