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.

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;

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) {

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;
 };