[screen-devel] [PATCH (screen v4)] screen: handle pts devices in different namespaces

[Christian Brauner]

Various programs that deal with namespaces will use pty devices that exist in
another namespace. One obvious candiate are containers. So far ttyname() was
incorrectly handling this case because the pts device stems from the host and
thus cannot be found amongst the current namespace's /dev/pts/<n> entries.
Serge Hallyn and I recently upstreamed patches to glibc that allow
ttyname{_r}() to correctly handle this case. At a minimum, ttyname{_r}() will
set errno to ENODEV in case it finds that the /dev/pts/<n> device that the
symlink points to exists in another namespace.

(The next comment is a little longer but tries to ensure that one can still
understand what is going on after some time has passed.)
In case we detect that ttyname{_r}() returns NULL and sets errno to ENODEV we
have ample reason to assume that the pts device exists in a different
namespace. In this case, the code will set a global flag indicating this case
to true. Furthermore, all operations (e.g. chmod(), chown(), etc.) will now
need to operate on the symbolic link /proc/self/fd/0 directly. While this
sounds straightforward, it becomes difficult to handle this case correctly when
we reattach to an already existing screen session from a different pts device
than the original one. Let's look at the general reattach logic a little
closer:

Assume we are running a shell that uses a pts device from a different
namespace:

        address@hidden:~# ls -al /proc/self/fd/
        total 0
        dr-x------ 2 root root  0 Apr  2 20:22 .
        dr-xr-xr-x 9 root root  0 Apr  2 20:22 ..
        lrwx------ 1 root root 64 Apr  2 20:22 0 -> /dev/pts/6
        lrwx------ 1 root root 64 Apr  2 20:22 1 -> /dev/pts/6
        lrwx------ 1 root root 64 Apr  2 20:22 2 -> /dev/pts/6
        l-wx------ 1 root root 64 Apr  2 20:22 3 -> pipe:[3067913]
        lr-x------ 1 root root 64 Apr  2 20:22 4 -> /proc/27413/fd
        lrwx------ 1 root root 64 Apr  2 20:22 9 -> socket:[32944]

        address@hidden:~# ls -al /dev/pts/
        total 0
        drwxr-xr-x 2 root root      0 Mar 30 17:55 .
        drwxr-xr-x 8 root root    580 Mar 30 17:55 ..
        crw--w---- 1 root tty  136, 0 Mar 30 17:55 0
        crw--w---- 1 root tty  136, 1 Mar 30 17:55 1
        crw--w---- 1 root tty  136, 2 Mar 30 17:55 2
        crw--w---- 1 root tty  136, 3 Mar 30 17:55 3
        crw--w---- 1 root tty  136, 4 Mar 30 17:55 4
        crw-rw-rw- 1 root root   5, 2 Apr  2 20:22 ptmx

(As one can see /dev/pts/6 does not exist in the current namespace.)
Now, start a screen session in this shell. In this case this patch will have
screen directly operate on /proc/self/fd/0.
Let's look at the attach case. When we attach to an existing screen session
where the associated pts device lives in another namespace we need a way to
uniquely identify the pts device that is used and also need a way to get a
valid fd when we need one. This patch solves this by ensuring that a valid file
descriptor to the pts device is sent via a unix socket and SCM_RIGHTS to the
socket and display handling part of screen. However, screen also sends around
the name of the associated pts device or, in the case where the pts device
exists in another namespace, the symlink /proc/self/fd/0. But after having sent
the fd this part of the codebase cannot simply operate on /proc/self/fd/0 since
it very likely refers to a different file. So we need to operate on
/proc/self/fd/<fd-sent-via-SCM_RIGHTS> but also need to ensure that we haven't
been tricked into operating on a tampered with file or device. So we cannot
simply sent /proc/self/fd/0 via the unix socket. Instead we read the contents
of the symbolic link /proc/self/fd/0 in the main function and sent it via the
unix socket. Then in the socket and display handling part of screen, we read
the contents of the /proc/self/fd/<fd-sent-via-SCM_RIGHTS> as well and compare
the pts device names. If they match we know that everything is well. However,
now we also need to update any tty handling code to directly operate on
/proc/self/fd/<fd-sent-via-SCM_RIGHTS>.

Signed-off-by: Christian Brauner <address@hidden>
---
 src/attacher.c | 11 +++++---
 src/extern.h   |  2 +-
 src/pty.c      | 25 +++++++++++++++++-
 src/screen.c   | 81 +++++++++++++++++++++++++++++++++++++++++-----------------
 src/socket.c   | 41 +++++++++++++++++++++++++++--
 src/utmp.c     |  6 ++---
 6 files changed, 132 insertions(+), 34 deletions(-)

diff --git a/src/attacher.c b/src/attacher.c
index 1052549..ed0d6ea 100644
--- a/src/attacher.c
+++ b/src/attacher.c
@@ -27,6 +27,7 @@
  */
 
 #include "config.h"
+#include <stdbool.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
@@ -58,6 +59,10 @@ extern char *SockName, *SockMatch, SockPath[];
 extern char HostName[];
 extern struct passwd *ppp;
 extern char *attach_tty, *attach_term, *LoginName, *preselect;
+/* Indicator whether the current tty exists in another namespace. */
+extern bool attach_tty_is_in_new_ns;
+/* Content of the tty symlink when attach_tty_is_in_new_ns == true. */
+extern char attach_tty_name_in_ns[];
 extern int xflag, dflag, rflag, quietflag, adaptflag;
 extern struct mode attach_Mode;
 extern struct NewWindow nwin_options;
@@ -226,7 +231,7 @@ int how;
   bzero((char *) &m, sizeof(m));
   m.type = how;
   m.protocol_revision = MSG_REVISION;
-  strncpy(m.m_tty, attach_tty, sizeof(m.m_tty) - 1);
+  strncpy(m.m_tty, attach_tty_is_in_new_ns ? attach_tty_name_in_ns : 
attach_tty, sizeof(m.m_tty) - 1);
   m.m_tty[sizeof(m.m_tty) - 1] = 0;
 
   if (how == MSG_WINCH)
@@ -482,7 +487,7 @@ AttacherFinit SIGDEFARG
     {
       debug("Detaching backend!\n");
       bzero((char *) &m, sizeof(m));
-      strncpy(m.m_tty, attach_tty, sizeof(m.m_tty) - 1);
+      strncpy(m.m_tty, attach_tty_is_in_new_ns ? attach_tty_name_in_ns : 
attach_tty, sizeof(m.m_tty) - 1);
       m.m_tty[sizeof(m.m_tty) - 1] = 0;
       debug1("attach_tty is %s\n", attach_tty);
       m.m.detach.dpid = getpid();
@@ -1022,7 +1027,7 @@ int query;
   m.type = query ? MSG_QUERY : MSG_COMMAND;
   if (attach_tty)
     {
-      strncpy(m.m_tty, attach_tty, sizeof(m.m_tty) - 1);
+      strncpy(m.m_tty, attach_tty_is_in_new_ns ? attach_tty_name_in_ns : 
attach_tty, sizeof(m.m_tty) - 1);
       m.m_tty[sizeof(m.m_tty) - 1] = 0;
     }
   p = m.m.command.cmd;
diff --git a/src/extern.h b/src/extern.h
index 7966008..bd22ebb 100644
--- a/src/extern.h
+++ b/src/extern.h
@@ -111,7 +111,7 @@ extern struct baud_values *lookup_baud __P((int bps));
 extern int   SetBaud __P((struct mode *, int, int));
 extern int   SttyMode __P((struct mode *, char *));
 extern int   CheckTtyname __P((char *));
-
+extern char  *GetPtsPathOrSymlink __P((int));
 
 /* mark.c */
 extern int   GetHistory __P((void));
diff --git a/src/pty.c b/src/pty.c
index badf854..b670953 100644
--- a/src/pty.c
+++ b/src/pty.c
@@ -301,7 +301,7 @@ char **ttyn;
   strcpy (PtyName, "/dev/ptc");
   if ((f = open (PtyName, O_RDWR | O_NOCTTY)) < 0)
     return -1;
-  strncpy(TtyName, ttyname(f), sizeof(TtyName));
+  strncpy(TtyName, GetPtsPathOrSymlink(f), sizeof(TtyName));
   if (eff_uid && access(TtyName, R_OK | W_OK))
     {
       close(f);
@@ -390,3 +390,26 @@ char **ttyn;
 }
 #endif
 
+/* len(/proc/self/fd/) + len(max 64 bit int) */
+#define MAX_PTS_SYMLINK (14 + 21)
+char *GetPtsPathOrSymlink(fd)
+int fd;
+{
+  int ret;
+  char *tty_name;
+  static char tty_symlink[MAX_PTS_SYMLINK];
+  int saved_errno = 0;
+
+  errno = 0;
+  tty_name = ttyname(fd);
+  if (!tty_name && errno == ENODEV) {
+    saved_errno = errno;
+    ret = snprintf(tty_symlink, MAX_PTS_SYMLINK, "/proc/self/fd/%d", fd);
+    if (ret < 0 || ret >= MAX_PTS_SYMLINK)
+           return NULL;
+    errno = saved_errno;
+    return tty_symlink;
+  }
+
+  return tty_name;
+}
diff --git a/src/screen.c b/src/screen.c
index 14aaa33..2d12382 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -43,6 +43,8 @@
 #include <sys/socket.h>
 #endif
 #include <ctype.h>
+#include <stdbool.h>
+#include <unistd.h>
 #include <fcntl.h>
 
 #if defined(__sun)
@@ -172,6 +174,10 @@ int nversion;      /* numerical version, used for 
secondary DA */
 /* the attacher */
 struct passwd *ppp;
 char *attach_tty;
+/* Indicator whether the current tty exists in another namespace. */
+bool attach_tty_is_in_new_ns = false;
+/* Content of the tty symlink when attach_tty_is_in_new_ns == true. */
+char attach_tty_name_in_ns[MAXPATHLEN];
 int attach_fd = -1;
 char *attach_term;
 char *LoginName;
@@ -980,31 +986,49 @@ int main(int ac, char** av)
   } while (0)
 
 #define SET_TTYNAME(fatal) do \
-  { \
-    int saved_errno = 0; \
-    errno = 0; \
-    if (!(attach_tty = ttyname(0))) \
-    { \
-    /* stdin is a tty but it exists in another namespace. */ \
-    if (fatal && errno == ENODEV) \
-      attach_tty = ""; \
-    else if (fatal) \
-      Panic(0, "Must be connected to a terminal."); \
-    else \
-      attach_tty = ""; \
-    } \
-    else \
+{ \
+  int ret; \
+  int saved_errno = 0; \
+  attach_tty_is_in_new_ns = false; \
+  memset(&attach_tty_name_in_ns, 0, sizeof(attach_tty_name_in_ns)); \
+  errno = 0; \
+  attach_tty = ttyname(0); \
+  if (!attach_tty) \
     { \
-    saved_errno = errno; \
-    if (stat(attach_tty, &st)) \
-      Panic(errno, "Cannot access '%s'", attach_tty); \
-    /* Only call CheckTtyname() if the device does not exist in another \
-     * namespace. */ \
-    if (saved_errno != ENODEV && CheckTtyname(attach_tty)) \
-      Panic(0, "Bad tty '%s'", attach_tty); \
+      if (fatal && errno == ENODEV || !fatal && errno == ENODEV) \
+       { \
+         saved_errno = errno; \
+          attach_tty = "/proc/self/fd/0"; \
+          attach_tty_is_in_new_ns = true; \
+          ret = readlink(attach_tty, attach_tty_name_in_ns, 
sizeof(attach_tty_name_in_ns)); \
+          if (ret < 0 || (size_t)ret >= sizeof(attach_tty_name_in_ns)) \
+            { \
+              Panic(0, "Bad tty '%s'", attach_tty); \
+            } \
+       } \
+      else if (fatal) \
+        { \
+          Panic(0, "Must be connected to a terminal."); \
+        } \
+      else \
+        { \
+          attach_tty = ""; \
+        } \
     } \
-    if (strlen(attach_tty) >= MAXPATHLEN) \
-      Panic(0, "TtyName too long - sorry."); \
+\
+    if (attach_tty) \
+      { \
+        if (stat(attach_tty, &st)) \
+          Panic(errno, "Cannot access '%s'", attach_tty); \
+\
+       /* Only call CheckTtyname() if the device does not exist in another \
+        * namespace. */ \
+        if (saved_errno != ENODEV && CheckTtyname(attach_tty)) \
+          Panic(0, "Bad tty '%s'", attach_tty); \
+\
+        if (strlen(attach_tty) >= MAXPATHLEN) \
+          Panic(0, "TtyName too long - sorry."); \
+      } \
   } while (0)
 
   if (home == 0 || *home == '\0')
@@ -1041,7 +1065,16 @@ int main(int ac, char** av)
     if (attach_fd == -1) {
       if ((n = secopen(attach_tty, O_RDWR | O_NONBLOCK, 0)) < 0)
         Panic(0, "Cannot open your terminal '%s' - please check.", attach_tty);
-      close(n);
+      /* In case the pts device exists in another namespace we directly operate
+       * on the symbolic link itself. However, this means that we need to keep
+       * the fd open since we have no direct way of identifying the associated
+       * pts device accross namespaces. This is ok though since keeping fds 
open
+       * is done in the codebase already.
+       */
+      if (attach_tty_is_in_new_ns)
+       attach_fd = n;
+      else
+       close(n);
     }
 
     debug2("attach_tty is %s, attach_fd is %d\n", attach_tty, attach_fd);
diff --git a/src/socket.c b/src/socket.c
index e464a62..05011b8 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -27,6 +27,9 @@
  */
 
 #include "config.h"
+#include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -78,6 +81,8 @@ extern int ServerSocket, real_uid, real_gid, eff_uid, eff_gid;
 extern int dflag, iflag, rflag, lsflag, quietflag, wipeflag, xflag;
 extern int queryflag;
 extern char *attach_tty, *LoginName, HostName[];
+/* Indicator whether the current tty exists in another namespace. */
+extern bool attach_tty_is_in_new_ns;
 extern struct display *display, *displays;
 extern struct win *fore, **wtab, *console_window, *windows;
 extern struct layer *flayer;
@@ -869,11 +874,43 @@ struct win *wi;
     }
   if (recvfd != -1)
     {
+      int ret;
+      char ttyname_in_ns[MAXPATHLEN];
       char *myttyname;
+
       i = recvfd;
       recvfd = -1;
-      myttyname = ttyname(i);
-      if (myttyname == 0 || strcmp(myttyname, m->m_tty))
+      memset(&ttyname_in_ns, 0, sizeof(ttyname_in_ns));
+      errno = 0;
+      myttyname = GetPtsPathOrSymlink(i);
+      if (myttyname && errno == ENODEV && attach_tty_is_in_new_ns)
+        {
+          ret = readlink(myttyname, ttyname_in_ns, sizeof(ttyname_in_ns));
+          if (ret < 0 || (size_t)ret >= sizeof(ttyname_in_ns))
+            {
+             Msg(errno, "Could not perform necessary sanity checks on pts 
device.");
+             close(i);
+             Kill(pid, SIG_BYE);
+             return -1;
+            }
+          if (strcmp(ttyname_in_ns, m->m_tty))
+            {
+             Msg(errno, "Attach: passed fd does not match tty: %s - %s!", 
ttyname_in_ns, m->m_tty[0] != '\0' ? m->m_tty : "(null)");
+             close(i);
+             Kill(pid, SIG_BYE);
+             return -1;
+           }
+         /* m->m_tty so far contains the actual name of the pts device in the
+          * its (e.g. /dev/pts/0). This name however is not valid in the
+          * current namespace. So after we verified that the symlink returned
+          * by GetPtsPathOrSymlink() refers to the same pts device in this
+          * namespace we need to update m->m_tty to use that symlink for all
+          * future operations.
+          */
+          strncpy(m->m_tty, myttyname, sizeof(m->m_tty) - 1);
+          m->m_tty[sizeof(m->m_tty) - 1] = 0;
+        }
+      else if (myttyname == 0 || strcmp(myttyname, m->m_tty))
        {
          Msg(errno, "Attach: passed fd does not match tty: %s - %s!", 
m->m_tty, myttyname ? myttyname : "NULL");
          close(i);
diff --git a/src/utmp.c b/src/utmp.c
index fa8b87b..8ec2de4 100644
--- a/src/utmp.c
+++ b/src/utmp.c
@@ -361,7 +361,7 @@ RemoveLoginSlot()
       char *tty;
       debug("couln't zap slot -> do mesg n\n");
       D_loginttymode = 0;
-      if ((tty = ttyname(D_userfd)) && stat(tty, &stb) == 0 && (int)stb.st_uid 
== real_uid && !CheckTtyname(tty) && ((int)stb.st_mode & 0777) != 0666)
+      if ((tty = GetPtsPathOrSymlink(D_userfd)) && stat(tty, &stb) == 0 && 
(int)stb.st_uid == real_uid && !CheckTtyname(tty) && ((int)stb.st_mode & 0777) 
!= 0666)
        {
          D_loginttymode = (int)stb.st_mode & 0777;
          chmod(D_usertty, stb.st_mode & 0600);
@@ -387,7 +387,7 @@ RestoreLoginSlot()
     }
   UT_CLOSE;
   D_loginslot = (slot_t)0;
-  if (D_loginttymode && (tty = ttyname(D_userfd)) && !CheckTtyname(tty))
+  if (D_loginttymode && (tty = GetPtsPathOrSymlink(D_userfd)) && 
!CheckTtyname(tty))
     chmod(tty, D_loginttymode);
 }
 
@@ -851,7 +851,7 @@ getlogin()
   static char retbuf[sizeof(u.ut_user)+1];
   int fd;
 
-  for (fd = 0; fd <= 2 && (tty = ttyname(fd)) == NULL; fd++)
+  for (fd = 0; fd <= 2 && (tty = GetPtsPathOrSymlink(fd)) == NULL; fd++)
     ;
   if ((tty == NULL) || CheckTtyname(tty) || ((fd = open(UTMP_FILE, O_RDONLY)) 
< 0))
     return NULL;
-- 
2.11.0