Hello,

As Alexander pointed out correctly the first suggested solution was not good 
enough
to solve the issue, but thanks to You @Alexander I could figure out the real 
issue.

Neither permission handling nor controlling the child process would resolve this
issue and also not assumed fixes in the implementation of TTY's Unix/Linux 
because
the issue it is simply the way how files are handled in a Unix system and a TTY 
is
just simply a file.

To demonstrate this just create a small program which is writing several text 
lines
to a file and change during execution ownership and permissions or even remove 
the
file and there will be no failure as long as the file handle is open.

In other words, if once a file descriptor is open and also if inherit from a 
forked
process, this process has all rights to do whatever it wants to do except if it
closes itself the file handle.

And that is exactly the issue with stdin in 'su' because after calling execve it
is out of our control if stdin gets closed or not.

In fact we need is the possibility to open a 'new' stdin just with the 
permissions
of the new session which we can close ourself at exit and this can be realised 
in
utilising pseudo terminal devices as implemented with the following patch.

Further the following patch was just tested roughly on Linux with PAM because 
of lack
of time but worked for me.


@Alexander:
I was using Your test procedure as You suggested but now if You want to have an
output You would have to redirect stdout and stderr to a file to see the result.


Another issue I realised during testing was the fact that the child process
cannot be killed with SIGTERM according to the blocked signal coded but which
might be a nice-to-have IMHO but I'm not sure if this behaviour is intentional
or not.

BTW: The patch is against shadow_4.1.5.1.orig.tar.gz which means without the
Debian patches.


___BEGIN_PATCH___
--- shadow-4.1.5.1.orig/src/su.c        2012-05-25 13:51:55.000000000 +0200
+++ shadow-4.1.5.1/src/su.c     2013-04-08 00:14:15.500412395 +0200
@@ -62,12 +62,10 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <unistd.h>
-#ifndef USE_PAM
 #include <sys/ioctl.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h>
-#endif                         /* !USE_PAM */
 #include "prototypes.h"
 #include "defines.h"
 #include "pwauth.h"
@@ -85,6 +83,7 @@ const char *Prog;
 static /*@observer@*/const char *caller_tty = NULL;    /* Name of tty SU is 
run from */
 static bool caller_is_root = false;
 static uid_t caller_uid;
+static bool have_tty = false;
 #ifndef USE_PAM
 static bool caller_on_console = false;
 #ifdef SU_ACCESS
@@ -106,10 +105,10 @@ static bool change_environment = true;

 #ifdef USE_PAM
 static pam_handle_t *pamh = NULL;
+#endif
 static int caught = 0;
 /* PID of the child, in case it needs to be killed */
 static pid_t pid_child = 0;
-#endif

 /*
  * External identifiers
@@ -123,10 +122,9 @@ extern size_t newenvc; /* libmisc/env.c
 static void execve_shell (const char *shellname,
                           char *args[],
                           char *const envp[]);
-#ifdef USE_PAM
 static RETSIGTYPE kill_child (int unused(s));
-static void prepare_pam_close_session (void);
-#else                          /* !USE_PAM */
+static void handle_session (void);
+#ifndef USE_PAM
 static RETSIGTYPE die (int);
 static bool iswheel (const char *);
 #endif                         /* !USE_PAM */
@@ -177,7 +175,7 @@ static bool iswheel (const char *usernam
        }
        return is_on_list (grp->gr_mem, username);
 }
-#else                          /* USE_PAM */
+#endif                         /* USE_PAM */
 static RETSIGTYPE kill_child (int unused(s))
 {
        if (0 != pid_child) {
@@ -189,7 +187,6 @@ static RETSIGTYPE kill_child (int unused
        }
        exit (255);
 }
-#endif                         /* USE_PAM */

 /* borrowed from GNU sh-utils' "su.c" */
 static bool restricted_shell (const char *shellname)
@@ -260,7 +257,6 @@ static void execve_shell (const char *sh
        }
 }

-#ifdef USE_PAM
 /* Signal handler for parent process later */
 static void catch_signals (int sig)
 {
@@ -268,19 +264,108 @@ static void catch_signals (int sig)
 }

 /*
- * prepare_pam_close_session - Fork and wait for the child to close the session
+ * handle_session - Fork and handle the session
  *
- *     Only the child returns. The parent will wait for the child to
+ *     Only the child returns. The parent will handle the session
+ *     or if not a controlling terminal then wait for the child to
  *     terminate and exit.
  */
-static void prepare_pam_close_session (void)
+static void handle_session (void)
 {
        sigset_t ourset;
        int status;
        int ret;
+       int fd_ptmx = -1;
+       int fd_pts = -1;
+       char *pts_name = NULL;  
+       struct termios termset_save;
+       struct termios termset_new;
+       fd_set inp_fds;
+       struct timeval sel_to;
+       char trbuf[BUFSIZ];
+       ssize_t bytes_r;
+       struct winsize winsz;
+       bool winsz_set = false;
+
+
+
+       if( isatty( 0) == 1) {
+               have_tty = true;
+
+               if( tcgetattr( STDIN_FILENO, &termset_save) == -1) {
+                       fprintf( stderr, _("%s: Cannot get termios 
attributes\n"), Prog);
+                       exit( 1);
+               }
+
+               if( ioctl( STDIN_FILENO, TIOCGWINSZ, &winsz) == -1 )
+                       fprintf( stderr, _("%s: Cannot get window size\n"), 
Prog);
+               else
+                       winsz_set = true;
+               
+               /*
+                * Open and prepare pseudo terminal master
+                */
+               if( (fd_ptmx = open( "/dev/ptmx", O_RDWR)) == -1) {
+                       fprintf( stderr, _("%s: Cannot open pt master\n"), 
Prog);
+                       exit( 1);
+               }
+
+               if( grantpt( fd_ptmx) == -1) {
+                       fprintf( stderr, _("%s: Cannot grant pt master 
permissions\n"), Prog);
+                       close( fd_ptmx);
+                       exit( 1);
+               }
+               if( unlockpt( fd_ptmx) == -1) {
+                       fprintf( stderr, _("%s: Cannot unlock pt master\n"), 
Prog);
+                       close( fd_ptmx);
+                       exit( 1);
+               }
+
+               if( (pts_name = ptsname( fd_ptmx)) == NULL) {
+                       fprintf( stderr, _("%s: Cannot get pt slave name\n"), 
Prog);
+                       close( fd_ptmx);
+                       exit( 1);
+               }
+
+               if( (fd_pts = open( pts_name, O_RDWR )) == -1) {
+                       fprintf( stderr, _("%s: Cannot open pt slave\n"), Prog);
+                       close( fd_ptmx);
+                       exit( 1);
+               }
+       }
+
+

        pid_child = fork ();
        if (pid_child == 0) {   /* child shell */
+
+               if( have_tty == true) {
+                       close( fd_ptmx);
+                       
+                       if( tcsetattr( fd_pts, TCSANOW, &termset_save) == -1) {
+                               fprintf( stderr, _("%s: Cannot set set termios 
attributes of sessiont\n"), Prog);
+                               close( fd_pts);
+                               exit (1);
+                       }
+
+                       if( winsz_set == true && ioctl( fd_pts, TIOCSWINSZ, 
&winsz) == -1)
+                               fprintf( stderr, _("%s: Cannot set window size 
of session %d\n"), Prog, errno);
+
+                       dup2( fd_pts, STDIN_FILENO);
+                       dup2( fd_pts, STDOUT_FILENO);
+                       dup2( fd_pts, STDERR_FILENO);
+
+                       if( STDIN_FILENO != fd_pts && STDOUT_FILENO != fd_pts
+                                       && STDERR_FILENO != fd_pts)
+                               close( fd_pts);
+
+                       if( setsid() == -1)
+                               fprintf( stderr, _("%s: Cannot set process 
group leader\n"), Prog);
+                       else
+                               if( ioctl( STDIN_FILENO, TIOCSCTTY, 1) == -1)
+                                       fprintf( stderr, _("%s: Cannot set 
controlling terminal\n"), Prog);
+
+               }
                return; /* Only the child will return from pam_create_session */
        } else if ((pid_t)-1 == pid_child) {
                (void) fprintf (stderr,
@@ -310,18 +395,9 @@ static void prepare_pam_close_session (v

                if (   (sigaddset (&ourset, SIGTERM) != 0)
                    || (sigaddset (&ourset, SIGALRM) != 0)
+                   || (sigaddset (&ourset, SIGWINCH) != 0)
                    || (sigaction (SIGTERM, &action, NULL) != 0)
-                   || (   !doshell /* handle SIGINT (Ctrl-C), SIGQUIT
-                                    * (Ctrl-\), and SIGTSTP (Ctrl-Z)
-                                    * since the child will not control
-                                    * the tty.
-                                    */
-                       && (   (sigaddset (&ourset, SIGINT)  != 0)
-                           || (sigaddset (&ourset, SIGQUIT) != 0)
-                           || (sigaddset (&ourset, SIGTSTP) != 0)
-                           || (sigaction (SIGINT,  &action, NULL) != 0)
-                           || (sigaction (SIGQUIT, &action, NULL) != 0)
-                           || (sigaction (SIGTSTP,  &action, NULL) != 0)))
+                   || (sigaction (SIGWINCH, &action, NULL) != 0)
                    || (sigprocmask (SIG_UNBLOCK, &ourset, NULL) != 0)
                    ) {
                        fprintf (stderr,
@@ -331,6 +407,13 @@ static void prepare_pam_close_session (v
                }
        }

+       if( have_tty == true) {
+               /* Set RAW mode  */
+               termset_new = termset_save;
+               cfmakeraw( &termset_new);
+               tcsetattr( STDIN_FILENO, TCSANOW, &termset_new);
+       }
+
        if (0 == caught) {
                bool stop = true;

@@ -338,7 +421,10 @@ static void prepare_pam_close_session (v
                        pid_t pid;
                        stop = true;

-                       pid = waitpid (-1, &status, WUNTRACED);
+                       if( have_tty == true)
+                               pid = waitpid (-1, &status, WUNTRACED |WNOHANG);
+                       else
+                               pid = waitpid (-1, &status, WUNTRACED);

                        /* When interrupted by signal, the signal will be
                         * forwarded to the child, and termination will be
@@ -354,7 +440,7 @@ static void prepare_pam_close_session (v
                                 */
                                kill (pid_child, SIGSTOP);
                                stop = false;
-                       } else if (   ((pid_t)-1 != pid)
+                       } else if (   ((pid_t)-1 != pid && have_tty == false)
                                   && (0 != WIFSTOPPED (status))) {
                                /* The child (shell) was suspended.
                                 * Suspend su. */
@@ -362,10 +448,68 @@ static void prepare_pam_close_session (v
                                /* wake child when resumed */
                                kill (pid, SIGCONT);
                                stop = false;
+                       } else if( pid == (pid_t)0 && have_tty == true) {
+                               stop = false;
+
+                               if( caught == SIGWINCH) {
+                                       caught = 0;
+                                       if( ioctl( STDIN_FILENO, TIOCGWINSZ, 
&winsz) != -1)
+                                               ioctl( fd_pts, TIOCSWINSZ, 
&winsz);
+                               }
+
+                   FD_ZERO( &inp_fds);
+               FD_SET( STDIN_FILENO, &inp_fds);
+                   FD_SET( fd_ptmx, &inp_fds);
+                               sel_to = (struct timeval){ 0, 10000};
+                               
+                   if( select( fd_ptmx + 1, &inp_fds, NULL, NULL, &sel_to) == 
-1) {
+                                       if( errno == EINTR)
+                                               continue;
+                                       stop = true;
+                               }
+               if( FD_ISSET( STDIN_FILENO, &inp_fds)) {
+                       bytes_r = read( STDIN_FILENO, trbuf, BUFSIZ);
+                       if(     bytes_r <= 0) {
+                                               if( errno == EINTR)
+                                                       continue;
+                                               fprintf( stderr, _("%s: Failure 
in reading from stdin\r\n"), Prog);
+                       stop = true;
+                                       }
+
+                       if( bytes_r > 0 && write( fd_ptmx, trbuf, bytes_r) != 
bytes_r) {
+                                               if( errno == EINTR || errno == 
EIO)
+                                                       continue;               
                                
+                                               fprintf( stderr, _("%s: Failure 
in writing to session\r\n"), Prog);
+                                               stop = true;
+                                       }
+               }
+
+               if( FD_ISSET( fd_ptmx, &inp_fds)) {
+                       bytes_r = read( fd_ptmx, trbuf, BUFSIZ);
+                       if( bytes_r <= 0) {
+                                               if( errno == EINTR || errno == 
EIO)
+                                                       continue;
+                                               fprintf( stderr, _("%s: Failure 
in reading from session %d %ld\r\n"), Prog, errno, bytes_r);
+                       stop = true;
+                                       }
+
+                       if( bytes_r > 0 && write( STDOUT_FILENO, trbuf, 
bytes_r) != bytes_r) {
+                                               fprintf( stderr, _("%s: Failure 
in writing to stdout\r\n"), Prog);
+                                               stop = true;
+                                       }
+               }                       
                        }
                } while (!stop);
        }

+
+       if( have_tty == true) {
+               close( fd_pts);
+               /* Reset RAW mode  */
+               if( tcsetattr( STDIN_FILENO, TCSANOW, &termset_save) == -1)
+                       fprintf( stderr, _("%s: Cannot reset termios 
attributes\n"), Prog);
+       }
+
        if (0 != caught) {
                (void) fputs ("\n", stderr);
                (void) fputs (_("Session terminated, terminating shell..."),
@@ -373,6 +517,7 @@ static void prepare_pam_close_session (v
                (void) kill (pid_child, caught);
        }

+#ifdef USE_PAM
        ret = pam_close_session (pamh, 0);
        if (PAM_SUCCESS != ret) {
                SYSLOG ((LOG_ERR, "pam_close_session: %s",
@@ -382,6 +527,7 @@ static void prepare_pam_close_session (v

        (void) pam_setcred (pamh, PAM_DELETE_CRED);
        (void) pam_end (pamh, PAM_SUCCESS);
+#endif                         /* USE_PAM */

        if (0 != caught) {
                (void) signal (SIGALRM, kill_child);
@@ -395,7 +541,6 @@ static void prepare_pam_close_session (v
                                        : WTERMSIG (status) + 128);
        /* Only the child returns. See above. */
 }
-#endif                         /* USE_PAM */

 /*
  * usage - print command line syntax and exit
@@ -1057,12 +1202,12 @@ int main (int argc, char **argv)
                exit (1);
        }

-       prepare_pam_close_session ();
-
        /* become the new user */
        if (change_uid (pw) != 0) {
                exit (1);
        }
+
+       handle_session ();
 #else                          /* !USE_PAM */
        /* no limits if su from root (unless su must fake login's behavior) */
        if (!caller_is_root || fakelogin) {
@@ -1072,39 +1217,12 @@ int main (int argc, char **argv)
        if (setup_uid_gid (pw, caller_on_console) != 0) {
                exit (1);
        }
-#endif                         /* !USE_PAM */

-       set_environment (pw);
-
-       if (!doshell) {
-               /* There is no need for a controlling terminal.
-                * This avoids the callee to inject commands on
-                * the caller's tty. */
-               int err = -1;
+       handle_session ();
+#endif                         /* !USE_PAM */

-#ifdef USE_PAM
-               /* When PAM is used, we are on the child */
-               err = setsid ();
-#else
-               /* Otherwise, we cannot use setsid */
-               int fd = open ("/dev/tty", O_RDWR);
-
-               if (fd >= 0) {
-                       err = ioctl (fd, TIOCNOTTY, (char *) 0);
-                       (void) close (fd);
-               } else if (ENXIO == errno) {
-                       /* There are no controlling terminal already */
-                       err = 0;
-               }
-#endif                         /* USE_PAM */

-               if (-1 == err) {
-                       (void) fprintf (stderr,
-                                       _("%s: Cannot drop the controlling 
terminal\n"),
-                                       Prog);
-                       exit (1);
-               }
-       }
+       set_environment (pw);

        /*
         * PAM_DATA_SILENT is not supported by some modules, and
___END_PATCH___




best regards

Wolf


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to