From de96d36224f92ba92c82d0aec2514848e821e22c Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Apr 2017 14:32:46 +0200 Subject: [PATCH] screen: handle pts devices in different namespaces Various programs that deal with namespaces will use pty devices that exist in another namespace. One obvious candidate 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/ 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/ 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: root@zest1:~# 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] root@zest1:~# 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/ 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/ 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/. Signed-off-by: Christian Brauner --- attacher.c | 11 +++++-- debian/rules | 3 -- extern.h | 2 +- pty.c | 27 +++++++++++++++++- screen.c | 93 +++++++++++++++++++++++++++++++++++++++--------------------- socket.c | 38 +++++++++++++++++++++++-- utmp.c | 6 ++-- 7 files changed, 134 insertions(+), 46 deletions(-) diff --git a/attacher.c b/attacher.c index 1052549..ed0d6ea 100644 --- a/attacher.c +++ b/attacher.c @@ -27,6 +27,7 @@ */ #include "config.h" +#include #include #include #include @@ -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/debian/rules b/debian/rules index 772200d..d9477a9 100755 --- a/debian/rules +++ b/debian/rules @@ -36,9 +36,6 @@ override_dh_auto_configure: # To minimize the library dependencies, the .udeb binary is # built without "--enable-pam" dh_auto_configure -B build-udeb -- $(SCREEN_CONFIGURE) - # Assert the use of fifos instead of sockets - cd build; grep -q "define.*NAMEDPIPE.*1" config.h || echo "#define NAMEDPIPE 1" >> config.h - cd build-udeb; grep -q "define.*NAMEDPIPE.*1" config.h || echo "#define NAMEDPIPE 1" >> config.h override_dh_auto_build: dh_auto_build --builddirectory build diff --git a/extern.h b/extern.h index 2c9dd87..737b96a 100644 --- a/extern.h +++ b/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/pty.c b/pty.c index badf854..d629870 100644 --- a/pty.c +++ b/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,28 @@ 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]; + + errno = 0; + tty_name = ttyname(fd); + if (!tty_name && errno == ENODEV) + { + ret = snprintf(tty_symlink, MAX_PTS_SYMLINK, "/proc/self/fd/%d", fd); + if (ret < 0 || ret >= MAX_PTS_SYMLINK) + return NULL; + /* We are setting errno to ENODEV to allow callers to check whether the + * pts device exists in another namespace. + */ + errno = ENODEV; + return tty_symlink; + } + + return tty_name; +} diff --git a/screen.c b/screen.c index 3af5bcc..e0df6be 100644 --- a/screen.c +++ b/screen.c @@ -43,6 +43,8 @@ #include #endif #include +#include +#include #include #ifdef sgi @@ -159,6 +161,7 @@ static int IsSymbol __P((char *, char *)); static char *ParseChar __P((char *, char *)); static int ParseEscape __P((char *)); static char *pad_expand __P((char *, char *, int, int)); +static void SetTtyname __P((bool, struct stat *)); #ifdef DEBUG static void fds __P((void)); #endif @@ -168,6 +171,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; @@ -968,34 +975,6 @@ int main(int ac, char** av) eff_gid = real_gid; \ } 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 \ - { \ - 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 (strlen(attach_tty) >= MAXPATHLEN) \ - Panic(0, "TtyName too long - sorry."); \ - } while (0) - if (home == 0 || *home == '\0') home = ppp->pw_dir; if (strlen(LoginName) > MAXLOGINLEN) @@ -1016,7 +995,7 @@ int main(int ac, char** av) #endif /* ttyname implies isatty */ - SET_TTYNAME(1); + SetTtyname(true, &st); #ifdef MULTIUSER tty_mode = (int)st.st_mode & 0777; #endif @@ -1030,7 +1009,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); @@ -1204,7 +1192,7 @@ int main(int ac, char** av) signal(SIG_BYE, AttacherFinit); /* prevent races */ if (cmdflag) { /* attach_tty is not mandatory */ - SET_TTYNAME(0); + SetTtyname(false, &st); if (!*av) Panic(0, "Please specify a command."); SET_GUID(); @@ -1225,7 +1213,7 @@ int main(int ac, char** av) debug("screen -r: backend not responding -- still crying\n"); } else if (dflag && !mflag) { - SET_TTYNAME(0); + SetTtyname(false, &st); Attach(MSG_DETACH); Msg(0, "[%s %sdetached.]\n", SockName, (dflag > 1 ? "power " : "")); eexit(0); @@ -1233,7 +1221,7 @@ int main(int ac, char** av) } if (!SockMatch && !mflag && sty) { /* attach_tty is not mandatory */ - SET_TTYNAME(0); + SetTtyname(false, &st); SET_GUID(); nwin_options.args = av; SendCreateMsg(sty, &nwin); @@ -3398,3 +3386,42 @@ static int ParseEscape(char *p) return 0; } +void SetTtyname(bool fatal, struct stat *st) +{ + 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) { + if (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 (attach_tty) { + if (stat(attach_tty, st)) + Panic(errno, "Cannot access '%s'", attach_tty); + + if (strlen(attach_tty) >= MAXPATHLEN) + Panic(0, "TtyName too long - sorry."); + + /* 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); + } +} diff --git a/socket.c b/socket.c index 4d960f2..3472731 100644 --- a/socket.c +++ b/socket.c @@ -27,6 +27,9 @@ */ #include "config.h" +#include +#include +#include #include #include #include @@ -870,11 +873,42 @@ struct win *wi; } if (recvfd != -1) { + int ret; + char ttyname_in_ns[MAXPATHLEN] = {0}; char *myttyname; + i = recvfd; recvfd = -1; - myttyname = ttyname(i); - if (myttyname == 0 || strcmp(myttyname, m->m_tty)) + errno = 0; + myttyname = GetPtsPathOrSymlink(i); + if (myttyname && errno == ENODEV) + { + 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/utmp.c b/utmp.c index fa8b87b..8ec2de4 100644 --- a/utmp.c +++ b/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