213 lines
7.1 KiB
Diff
213 lines
7.1 KiB
Diff
From 41978a56e042977923c1a55191b887218c536145 Mon Sep 17 00:00:00 2001
|
|
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
|
|
Date: Sat, 27 Apr 2024 18:53:50 -0600
|
|
Subject: [PATCH] If user's tty goes away, tell monitor to revoke the tty in
|
|
its session.
|
|
|
|
Previously, we would simply close the pty leader in the main sudo
|
|
process. This had the effect of revoking the pty, but the foreground
|
|
process would not necessarily receive SIGHUP. By using TIOCNOTTY
|
|
in the monitor, the running command has a better chance of getting
|
|
SIGHUP. Once the monitor has revoked the pty, the main sudo process
|
|
will close the pty leader, invalidating the pty. GitHub issue #367.
|
|
|
|
Reference:https://github.com/sudo-project/sudo/commit/41978a56e042977923c1a55191b887218c536145
|
|
Conflict:NA
|
|
|
|
---
|
|
src/exec_monitor.c | 48 +++++++++++++++++++++++++++++++++++--
|
|
src/exec_pty.c | 60 ++++++++++++++++++++++++++++++++++++----------
|
|
src/sudo.h | 1 +
|
|
3 files changed, 95 insertions(+), 14 deletions(-)
|
|
|
|
diff --git a/src/exec_monitor.c b/src/exec_monitor.c
|
|
index 05f5f8cd1..c570b5d86 100644
|
|
--- a/src/exec_monitor.c
|
|
+++ b/src/exec_monitor.c
|
|
@@ -118,6 +118,8 @@ deliver_signal(struct monitor_closure *mc, int signo, bool from_parent)
|
|
/* NOTREACHED */
|
|
default:
|
|
/* Relay signal to command. */
|
|
+ sudo_debug_printf(SUDO_DEBUG_NOTICE, "%s: killpg(%d, %d)",
|
|
+ __func__, (int)mc->cmnd_pid, signo);
|
|
killpg(mc->cmnd_pid, signo);
|
|
break;
|
|
}
|
|
@@ -334,11 +336,53 @@ mon_backchannel_cb(int fd, int what, void *v)
|
|
mc->cstat->val = n ? EIO : ECONNRESET;
|
|
sudo_ev_loopbreak(mc->evbase);
|
|
} else {
|
|
- if (cstmp.type == CMD_SIGNO) {
|
|
+ switch (cstmp.type) {
|
|
+ case CMD_IOCTL:
|
|
+ if (cstmp.val != TIOCNOTTY) {
|
|
+ sudo_warnx(U_("unexpected ioctl on backchannel: %d"),
|
|
+ cstmp.val);
|
|
+ } else if (io_fds[SFD_FOLLOWER] != -1) {
|
|
+ int result, ttyfd;
|
|
+
|
|
+ /*
|
|
+ * Parent asks us to revoke the terminal when the
|
|
+ * user's terminal goes away. Doing this in the
|
|
+ * monitor allows the foreground command to receive
|
|
+ * SIGHUP before the terminal is revoked.
|
|
+ */
|
|
+ result = ioctl(io_fds[SFD_FOLLOWER], TIOCNOTTY, NULL);
|
|
+ if (result == -1) {
|
|
+ sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
|
|
+ "%s: unable to revoke follower pty", __func__);
|
|
+ ttyfd = open(_PATH_TTY, O_RDWR);
|
|
+ if (ttyfd != -1) {
|
|
+ result = ioctl(ttyfd, TIOCNOTTY, NULL);
|
|
+ if (result == -1) {
|
|
+ sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
|
|
+ "%s: unable to revoke controlling tty",
|
|
+ __func__);
|
|
+ }
|
|
+ close(ttyfd);
|
|
+ } else {
|
|
+ sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
|
|
+ "%s: unable to open %s", __func__, _PATH_TTY);
|
|
+ }
|
|
+ }
|
|
+ if (result == 0) {
|
|
+ sudo_debug_printf(SUDO_DEBUG_INFO,
|
|
+ "%s: revoked controlling tty for session", __func__);
|
|
+ }
|
|
+ /* Now tell the parent to close the pty leader. */
|
|
+ send_status(fd, &cstmp);
|
|
+ }
|
|
+ break;
|
|
+ case CMD_SIGNO:
|
|
deliver_signal(mc, cstmp.val, true);
|
|
- } else {
|
|
+ break;
|
|
+ default:
|
|
sudo_warnx(U_("unexpected reply type on backchannel: %d"),
|
|
cstmp.type);
|
|
+ break;
|
|
}
|
|
}
|
|
debug_return;
|
|
diff --git a/src/exec_pty.c b/src/exec_pty.c
|
|
index 6c0f7583e..fff9b8f1e 100644
|
|
--- a/src/exec_pty.c
|
|
+++ b/src/exec_pty.c
|
|
@@ -61,6 +61,7 @@ static struct exec_closure pty_ec;
|
|
|
|
static void sync_ttysize(struct exec_closure *ec);
|
|
static void schedule_signal(struct exec_closure *ec, int signo);
|
|
+static void send_command_status(struct exec_closure *ec, int type, int val);
|
|
|
|
/*
|
|
* Allocate a pty if /dev/tty is a tty.
|
|
@@ -383,8 +384,18 @@ read_callback(int fd, int what, void *v)
|
|
ev_free_by_fd(evbase, fd);
|
|
/* If writer already consumed the buffer, close it too. */
|
|
if (iob->wevent != NULL && iob->off == iob->len) {
|
|
- safe_close(sudo_ev_get_fd(iob->wevent));
|
|
- ev_free_by_fd(evbase, sudo_ev_get_fd(iob->wevent));
|
|
+ /*
|
|
+ * Don't close the pty leader, it will invalidate the pty.
|
|
+ * We ask the monitor to revoke the pty nicely using TIOCNOTTY.
|
|
+ */
|
|
+ const int wfd = sudo_ev_get_fd(iob->wevent);
|
|
+ if (wfd == io_fds[SFD_LEADER]) {
|
|
+ sudo_debug_printf(SUDO_DEBUG_NOTICE, "user's tty revoked");
|
|
+ send_command_status(iob->ec, CMD_IOCTL, TIOCNOTTY);
|
|
+ } else {
|
|
+ safe_close(wfd);
|
|
+ }
|
|
+ ev_free_by_fd(evbase, wfd);
|
|
iob->off = iob->len = 0;
|
|
}
|
|
break;
|
|
@@ -461,8 +472,18 @@ write_callback(int fd, int what, void *v)
|
|
iob->len - iob->off, fd);
|
|
/* Close reader if there is one. */
|
|
if (iob->revent != NULL) {
|
|
- safe_close(sudo_ev_get_fd(iob->revent));
|
|
- ev_free_by_fd(evbase, sudo_ev_get_fd(iob->revent));
|
|
+ /*
|
|
+ * Don't close the pty leader, it will invalidate the pty.
|
|
+ * We ask the monitor to revoke the pty nicely using TIOCNOTTY.
|
|
+ */
|
|
+ const int rfd = sudo_ev_get_fd(iob->revent);
|
|
+ if (rfd == io_fds[SFD_LEADER]) {
|
|
+ sudo_debug_printf(SUDO_DEBUG_NOTICE, "user's tty revoked");
|
|
+ send_command_status(iob->ec, CMD_IOCTL, TIOCNOTTY);
|
|
+ } else {
|
|
+ safe_close(rfd);
|
|
+ }
|
|
+ ev_free_by_fd(evbase, rfd);
|
|
}
|
|
safe_close(fd);
|
|
ev_free_by_fd(evbase, fd);
|
|
@@ -656,6 +677,28 @@ backchannel_cb(int fd, int what, void *v)
|
|
case sizeof(cstat):
|
|
/* Check command status. */
|
|
switch (cstat.type) {
|
|
+ case CMD_ERRNO:
|
|
+ /* Monitor was unable to execute command or broken pipe. */
|
|
+ sudo_debug_printf(SUDO_DEBUG_INFO, "errno from monitor: %s",
|
|
+ strerror(cstat.val));
|
|
+ sudo_ev_loopbreak(ec->evbase);
|
|
+ *ec->cstat = cstat;
|
|
+ break;
|
|
+ case CMD_IOCTL:
|
|
+ if (cstat.val != TIOCNOTTY) {
|
|
+ sudo_warnx(U_("unexpected ioctl on backchannel: %d"),
|
|
+ cstat.val);
|
|
+ } else if (io_fds[SFD_LEADER] != -1) {
|
|
+ /*
|
|
+ * Monitor requests that we revoke the user's terminal.
|
|
+ * This must happen after the monitor has used TIOCNOTTY
|
|
+ * to invalidate the session and gracefully kill the
|
|
+ * controlling terminal's process group.
|
|
+ */
|
|
+ close(io_fds[SFD_LEADER]);
|
|
+ io_fds[SFD_LEADER] = -1;
|
|
+ }
|
|
+ break;
|
|
case CMD_PID:
|
|
ec->cmnd_pid = cstat.val;
|
|
sudo_debug_printf(SUDO_DEBUG_INFO, "executed %s, pid %d",
|
|
@@ -693,13 +736,6 @@ backchannel_cb(int fd, int what, void *v)
|
|
*ec->cstat = cstat;
|
|
}
|
|
break;
|
|
- case CMD_ERRNO:
|
|
- /* Monitor was unable to execute command or broken pipe. */
|
|
- sudo_debug_printf(SUDO_DEBUG_INFO, "errno from monitor: %s",
|
|
- strerror(cstat.val));
|
|
- sudo_ev_loopbreak(ec->evbase);
|
|
- *ec->cstat = cstat;
|
|
- break;
|
|
}
|
|
/* Keep reading command status messages until EAGAIN or EOF. */
|
|
break;
|
|
@@ -1382,7 +1418,7 @@ exec_pty(struct command_details *details,
|
|
if (sudo_ev_dispatch(ec->evbase) == -1)
|
|
sudo_warn("%s", U_("error in event loop"));
|
|
if (sudo_ev_got_break(ec->evbase)) {
|
|
- /* error from callback or monitor died */
|
|
+ /* error from callback */
|
|
sudo_debug_printf(SUDO_DEBUG_ERROR, "event loop exited prematurely");
|
|
/* XXX: no good way to know if we should terminate the command. */
|
|
if (cstat->val == CMD_INVALID && ec->cmnd_pid != -1) {
|
|
diff --git a/src/sudo.h b/src/sudo.h
|
|
index a7450dca9..ca245ca68 100644
|
|
--- a/src/sudo.h
|
|
+++ b/src/sudo.h
|
|
@@ -225,6 +225,7 @@ struct command_status {
|
|
#define CMD_WSTATUS 2
|
|
#define CMD_SIGNO 3
|
|
#define CMD_PID 4
|
|
+#define CMD_IOCTL 5
|
|
int type;
|
|
int val;
|
|
};
|
|
--
|
|
2.33.0
|
|
|