[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[screen-devel] [PATCH v3 (screen master)] screen: handle pts devices in
|
From: |
Christian Brauner |
|
Subject: |
[screen-devel] [PATCH v3 (screen master)] screen: handle pts devices in different namespaces |
|
Date: |
Wed, 5 Apr 2017 14:24:21 +0200 |
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/<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>
---
Changelog: 2017-04-05
- remove attach_tty_is_in_new_ns from socket.c
This variable is unneeded and it does harm since it may end up falsely
initialized when not sent over the socket itself. The most promiment
instance of failure is when starting screen in detached mode
(screen -dm).
---
src/attacher.c | 6 ++--
src/screen.c | 93 +++++++++++++++++++++++++++++++++++++---------------------
src/screen.h | 4 +++
src/socket.c | 43 ++++++++++++++++++++++++---
src/tty.c | 24 +++++++++++++++
src/tty.h | 1 +
src/utmp.c | 4 +--
7 files changed, 133 insertions(+), 42 deletions(-)
diff --git a/src/attacher.c b/src/attacher.c
index 4db7243..e483c0b 100644
--- a/src/attacher.c
+++ b/src/attacher.c
@@ -137,7 +137,7 @@ int Attach(int how)
memset((char *)&m, 0, 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) {
@@ -328,7 +328,7 @@ void AttacherFinit(int sigsig)
/* Check if signal comes from backend */
if (stat(SocketPath, &statb) == 0 && (statb.st_mode & 0777) != 0600) {
memset((char *)&m, 0, 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;
m.m.detach.dpid = getpid();
m.type = MSG_HANGUP;
@@ -493,7 +493,7 @@ void SendCmdMessage(char *sty, char *match, char **av, int
query)
memset((char *)&m, 0, sizeof(m));
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/screen.c b/src/screen.c
index 6d06cf1..c741eb4 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -37,6 +37,8 @@
#include <pwd.h>
#include <signal.h>
#include <stdint.h>
+#include <stdbool.h>
+#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -76,6 +78,7 @@ static void logflush_fn(Event *, void *);
static int IsSymbol(char *, char *);
static char *ParseChar(char *, char *);
static int ParseEscape(char *);
+static void SetTtyname(bool fatal, struct stat *st);
int nversion; /* numerical version, used for secondary DA */
@@ -86,6 +89,10 @@ int attach_fd = -1;
char *attach_term;
char *LoginName;
struct mode attach_Mode;
+/* 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];
char SocketPath[MAXPATHLEN + 2 * MAXSTR];
char *SocketName; /* SocketName is pointer in SocketPath
*/
@@ -644,34 +651,6 @@ int main(int argc, char **argv)
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)
@@ -687,7 +666,7 @@ int main(int argc, char **argv)
int fl;
/* ttyname implies isatty */
- SET_TTYNAME(1);
+ SetTtyname(true, &st);
tty_mode = (int)st.st_mode & 0777;
fl = fcntl(0, F_GETFL, 0);
@@ -697,7 +676,16 @@ int main(int argc, char **argv)
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);
}
if ((attach_term = getenv("TERM")) == 0 || *attach_term == 0)
@@ -824,7 +812,7 @@ int main(int argc, char **argv)
xsignal(SIG_BYE, AttacherFinit); /* prevent races */
if (cmdflag) {
/* attach_tty is not mandatory */
- SET_TTYNAME(0);
+ SetTtyname(false, &st);
if (!*argv)
Panic(0, "Please specify a command.");
SET_GUID();
@@ -838,7 +826,7 @@ int main(int argc, char **argv)
if (multiattach)
Panic(0, "Can't create sessions of other users.");
} else if (dflag && !mflag) {
- SET_TTYNAME(0);
+ SetTtyname(false, &st);
Attach(MSG_DETACH);
Msg(0, "[%s %sdetached.]\n", SocketName, (dflag > 1 ? "power "
: ""));
eexit(0);
@@ -846,7 +834,7 @@ int main(int argc, char **argv)
}
if (!SocketMatch && !mflag && sty) {
/* attach_tty is not mandatory */
- SET_TTYNAME(0);
+ SetTtyname(false, &st);
SET_GUID();
nwin_options.args = argv;
SendCreateMsg(sty, &nwin);
@@ -1805,3 +1793,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/src/screen.h b/src/screen.h
index e0876b0..7486252 100644
--- a/src/screen.h
+++ b/src/screen.h
@@ -233,6 +233,8 @@ void setbacktick (int, int, int, char **);
/* global variables */
+/* Content of the tty symlink when attach_tty_is_in_new_ns == true. */
+extern char attach_tty_name_in_ns[];
extern char strnomem[];
extern char HostName[];
extern char SocketPath[];
@@ -274,6 +276,8 @@ extern bool lsflag;
extern bool quietflag;
extern bool wipeflag;
extern bool xflag;
+/* Indicator whether the current tty exists in another namespace. */
+extern bool attach_tty_is_in_new_ns;
extern int attach_fd;
extern int dflag;
diff --git a/src/socket.c b/src/socket.c
index cfb43fe..af47077 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -41,6 +41,8 @@
#include <utime.h>
#include <stdint.h>
#include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
#include <signal.h>
#include "screen.h"
@@ -574,12 +576,45 @@ static int CreateTempDisplay(Message *m, int recvfd,
Window *win)
return -1;
}
if (recvfd != -1) {
+ int ret;
+ char ttyname_in_ns[MAXPATHLEN] = {0};
char *myttyname;
i = recvfd;
- myttyname = ttyname(i);
- 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");
+ 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 its namespace (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);
Kill(pid, SIG_BYE);
return -1;
diff --git a/src/tty.c b/src/tty.c
index 0f18896..67702b6 100644
--- a/src/tty.c
+++ b/src/tty.c
@@ -1193,3 +1193,27 @@ int CheckTtyname(char *tty)
return rc;
}
+
+/* len(/proc/self/fd/) + len(max 64 bit int) */
+#define MAX_PTS_SYMLINK (14 + 21)
+char *GetPtsPathOrSymlink(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/src/tty.h b/src/tty.h
index ccb1e53..fd95b67 100644
--- a/src/tty.h
+++ b/src/tty.h
@@ -17,6 +17,7 @@ struct baud_values *lookup_baud (int bps);
int SetBaud (struct mode *, int, int);
int SttyMode (struct mode *, char *);
int CheckTtyname (char *);
+char *GetPtsPathOrSymlink (int);
/* global variables */
diff --git a/src/utmp.c b/src/utmp.c
index 75f7fc8..7657525 100644
--- a/src/utmp.c
+++ b/src/utmp.c
@@ -210,7 +210,7 @@ void RemoveLoginSlot()
struct stat stb;
char *tty;
D_loginttymode = 0;
- if ((tty = ttyname(D_userfd)) && stat(tty, &stb) == 0 &&
stb.st_uid == real_uid && !CheckTtyname(tty)
+ if ((tty = GetPtsPathOrSymlink(D_userfd)) && stat(tty, &stb) ==
0 && 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);
@@ -230,7 +230,7 @@ void RestoreLoginSlot()
Msg(errno, "Could not write %s", UtmpName);
}
D_loginslot = (slot_t) 0;
- if (D_loginttymode && (tty = ttyname(D_userfd)) && !CheckTtyname(tty))
+ if (D_loginttymode && (tty = GetPtsPathOrSymlink(D_userfd)) &&
!CheckTtyname(tty))
fchmod(D_userfd, D_loginttymode);
}
--
2.11.0
- [screen-devel] [PATCH v3 (screen master)] screen: handle pts devices in different namespaces,
Christian Brauner <=