Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"su --pty -" fails to pass through the characteristics of the terminal, special keypresses, etc. #767

Open
dkg opened this issue Mar 5, 2019 · 10 comments

Comments

@dkg
Copy link
Contributor

dkg commented Mar 5, 2019

when using --pty with su, the width of the terminal is not transmitted to the child process:

0 dkg@alice:~$ su --pty - -c 'ls /'
Password: 
bin   home	      lib32	  media  proc  srv  var
boot  initrd.img      lib64	  mnt	 root  sys  vmlinuz
dev   initrd.img.old  libx32	  mnt2	 run   tmp  vmlinuz.old
etc   lib	      lost+found  opt	 sbin  usr
0 dkg@alice:~$ su - -c 'ls /'
Password: 
bin   dev  home        initrd.img.old  lib32  libx32	  media  mnt2  proc  run   srv	tmp  var      vmlinuz.old
boot  etc  initrd.img  lib	       lib64  lost+found  mnt	 opt   root  sbin  sys	usr  vmlinuz
0 dkg@alice:~$ 

Additionally, when using --pty without an explicit command (just running a shell), the "up" and "down" arrow keys don't seem to work any more (i'm using rxvt-unicode, in case that matters). Instead, i see the following characters emitted directly in the session: ^[[A^[[B. Likewise, pressing ctrl-A doesn't do what i expect, instead it just prints ^A. This makes it very difficult to use the command line under su --pty - the way one would expect.

@dkg
Copy link
Contributor Author

dkg commented Mar 5, 2019

Here's another way that the --pty behavior doesn't work cleanly as a drop-in fix:

0 root@alice:~# echo hello | runuser -u dkg -- tr h d
dello
0 root@alice:~# echo hello | runuser --pty -u dkg -- tr h d
dello
^C
Session terminated, killing shell... ...killed.
1 root@alice:~# 

(the problem is that the second command hangs forever unless i send it a ctrl-C)

Note that in such a pipeline, file descriptor 0 isn't connected to a tty, but fd 1 and 2 are still connected, and therefore the first command is still vulnerable to TIOCSTI abuse.

I plan to continue experimenting with switching in --pty and reporting what i run into -- i'll report them here on this thread, unless @karelzak tells me they'd rather have them as independent bug reports.

karelzak added a commit that referenced this issue Mar 6, 2019
* use proper winsize rather than uninitialized variable (Oops...)

* set the current terminal to the raw mode

* disable ECHO for non-terminal execution to be compatible with
  non-pty output

Addresses: #767
Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Collaborator

karelzak commented Mar 6, 2019

The problem with special keys and winsize is fixed. I have also fixed signal handling on kill and dumps.

Not sure what do you mean with pipeline and the file descriptors:

echo "ps af" | ./runuser --pty -u kzak -- bash
    PID TTY      STAT   TIME COMMAND
 7266 pts/11   Ss+    0:00 -bash
 6791 pts/8    Ss+    0:00 -bash
 9141 pts/12   Ss     0:00 bash
 9206 pts/12   S      0:00  \_ sudo su -
 9207 pts/12   S      0:00      \_ su -
 9210 pts/12   S      0:00          \_ -bash
15979 pts/12   S+     0:00              \_ ./runuser --pty -u kzak -- bash
15980 pts/13   Ss     0:00                  \_ bash                                   <<<<
16044 pts/13   R+     0:00                      \_ ps af                            

in this case bash (15980) uses another terminal.

@karelzak
Copy link
Collaborator

karelzak commented Mar 6, 2019

Anyway, thanks for testing!

@dkg
Copy link
Contributor Author

dkg commented Mar 6, 2019

thanks for the quick fixes, @karelzak !

the issue with the pipeline is that this command produces the correct output, and then terminates immediately:

echo hello | runuser -u dkg -- tr h d

But this command produces the correct output and then hangs indefinitely:

echo hello | runuser --pty -u dkg -- tr h d

does it not hang for you? I'm running with util-linux 2.33.1-0.1 on debian testing/unstable.

@dkg
Copy link
Contributor Author

dkg commented Mar 6, 2019

hm, testing from git master, i see that it now does not hang, that's good :) Bisecting with git, it looks like the hang was fixed by 282ca3d

@dkg
Copy link
Contributor Author

dkg commented Mar 6, 2019

hm, the text added in 64a87be suggests:

This feature is mostly designed for interactive sessions.

that makes it likely to be ignored in places deeper in the infrastructure that don't even know whether they'll be getting a terminal (like update-ieee-data), but those are the very places where the pty isolation is important to defend the superuser's terminal against compromise by the non-privileged process.

@dkg
Copy link
Contributor Author

dkg commented Mar 7, 2019

Testing against git master (64a87be), i see a significant delay when i use --pty and stdin is reading from a pipe.

I captured an strace to see what was happening like so:

0 root@sid:~# time echo | strace -f -T -tt -o runuser.strace.txt /home/dkg/src/util-linux/util-linux/runuser --pty -u dkg -- echo hello
hello

real	0m2.175s
user	0m0.027s
sys	0m0.046s
0 root@sid:~# 

I'm attaching the generated runuser.strace.txt.

It looks like the parent runuser process cycles through 2 seconds of quarter-second nanosleep() interleaved with poll()ing on fd 5 (the pty slave gathered from TIOCGPTPEER) before sending the fd 4 (the pty master) an EOT char (\x04), and then realizing that it should poll() on both fd 4 and fd 6 (its signalfd). The response from this final poll() gives the parent process what it needs to start shutting down and to transfer the remaining text back to the outer terminal:

[…]
4528  09:26:45.555754 exit_group(0)     = ?
4528  09:26:45.555831 +++ exited with 0 +++
4525  09:26:45.801574 <... nanosleep resumed> NULL) = 0 <0.250423>
4525  09:26:45.801898 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000163>
4525  09:26:45.802355 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250421>
4525  09:26:46.053174 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000163>
4525  09:26:46.053665 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250520>
4525  09:26:46.304540 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000155>
4525  09:26:46.305055 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250463>
4525  09:26:46.555892 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000165>
4525  09:26:46.556386 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250517>
4525  09:26:46.807266 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000171>
4525  09:26:46.807797 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250484>
4525  09:26:47.058656 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000167>
4525  09:26:47.059152 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250536>
4525  09:26:47.310040 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000163>
4525  09:26:47.310532 nanosleep({tv_sec=0, tv_nsec=250000000}, NULL) = 0 <0.250200>
4525  09:26:47.560890 poll([{fd=5, events=POLLIN}], 1, 10) = 1 ([{fd=5, revents=POLLIN}]) <0.000045>
4525  09:26:47.561059 write(4, "\4", 1) = 1 <0.000031>
4525  09:26:47.561150 poll([{fd=6, events=POLLIN|POLLERR|POLLHUP}, {fd=4, events=POLLIN|POLLERR|POLLHUP}, {fd=-1}], 3, -1) = 2 ([{fd=6, revents=POLLIN}, {fd=4, revents=POLLIN}]) <0.000025>
4525  09:26:47.561247 read(6, "\21\0\0\0\0\0\0\0\1\0\0\0\260\21\0\0\350\3\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 128) = 128 <0.000025>
4525  09:26:47.561332 wait4(4528, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], WSTOPPED, NULL) = 4528 <0.000031>
4525  09:26:47.561413 read(4, "hello\r\n", 8192) = 7 <0.000017>
4525  09:26:47.561475 write(1, "hello\r\n", 7) = 7 <0.000112>
[…]

@dkg
Copy link
Contributor Author

dkg commented Mar 7, 2019

I should mention that i don't see this delay when runuser is invoked without --pty in such a pipeline. I also don't see it when stdin is a terminal, or redirected from /dev/null.

@dkg
Copy link
Contributor Author

dkg commented Mar 7, 2019

The delay appears to be in write_eof_to_child() in login-utils/su-common.c, as invoked from pty_proxy_master() in the same file.

@karelzak
Copy link
Collaborator

karelzak commented Mar 8, 2019

Yes, it's necessary otherwise we will send data and EOF to child before it's ready to read it. All this is from script(1) where we spent years to implement "pipe to interactive session".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants