From 0ea431e3929705ce6c17beb5c1337d2cce565bf9 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Wed, 18 May 2022 07:29:55 -0600 Subject: [PATCH] Move code to suspend sudo when no pty is in use to separate file. Use this in test_ptrace.c to be able to suspend just like sudo does. --- MANIFEST | 1 + src/Makefile.in | 90 +++++++++++++--------- src/exec_nopty.c | 80 +------------------- src/exec_pty.c | 8 +- src/regress/intercept/test_ptrace.c | 15 +++- src/sudo_exec.h | 3 + src/suspend_nopty.c | 111 ++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 118 deletions(-) create mode 100644 src/suspend_nopty.c diff --git a/MANIFEST b/MANIFEST index 527092c4f..ccd3aa01e 100644 --- a/MANIFEST +++ b/MANIFEST @@ -1222,6 +1222,7 @@ src/sudo_intercept_common.c src/sudo_noexec.c src/sudo_plugin_int.h src/sudo_usage.h.in +src/suspend_nopty.c src/tcsetpgrp_nobg.c src/tgetpass.c src/ttyname.c diff --git a/src/Makefile.in b/src/Makefile.in index 3efb98725..f6ff40532 100644 --- a/src/Makefile.in +++ b/src/Makefile.in @@ -144,7 +144,7 @@ OBJS = conversation.o copy_file.o edit_open.o env_hooks.o exec.o exec_common.o \ exec_intercept.o exec_monitor.o exec_nopty.o exec_preload.o \ exec_ptrace.o exec_pty.o get_pty.o hooks.o limits.o load_plugins.o \ net_ifs.o parse_args.o preserve_fds.o signal.o sudo.o sudo_edit.o \ - tcsetpgrp_nobg.o tgetpass.o ttyname.o utmp.o @SUDO_OBJS@ + suspend_nopty.o tcsetpgrp_nobg.o tgetpass.o ttyname.o utmp.o @SUDO_OBJS@ IOBJS = $(OBJS:.o=.i) sesh.i @@ -161,7 +161,7 @@ CHECK_NOEXEC_OBJS = check_noexec.o exec_common.o exec_preload.o CHECK_TTYNAME_OBJS = check_ttyname.o ttyname.o -TEST_PTRACE_OBJS = test_ptrace.o +TEST_PTRACE_OBJS = suspend_nopty.o tcsetpgrp_nobg.o test_ptrace.o LIBOBJDIR = $(top_builddir)/@ac_config_libobj_dir@/ @@ -614,23 +614,25 @@ exec_preload.o: $(srcdir)/exec_preload.c $(incdir)/compat/stdbool.h \ $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ $(top_builddir)/config.h $(top_builddir)/pathnames.h $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/exec_preload.c -exec_ptrace.o: $(srcdir)/exec_ptrace.c $(incdir)/compat/stdbool.h \ - $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ - $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ - $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ - $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ - $(incdir)/sudo_util.h $(srcdir)/exec_intercept.h \ - $(srcdir)/exec_ptrace.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ - $(top_builddir)/config.h $(top_builddir)/pathnames.h +exec_ptrace.o: $(srcdir)/exec_ptrace.c $(incdir)/compat/endian.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_event.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/exec_intercept.h $(srcdir)/exec_ptrace.h \ + $(srcdir)/sudo.h $(srcdir)/sudo_exec.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/exec_ptrace.c -exec_ptrace.i: $(srcdir)/exec_ptrace.c $(incdir)/compat/stdbool.h \ - $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ - $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ - $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ - $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ - $(incdir)/sudo_util.h $(srcdir)/exec_intercept.h \ - $(srcdir)/exec_ptrace.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ - $(top_builddir)/config.h $(top_builddir)/pathnames.h +exec_ptrace.i: $(srcdir)/exec_ptrace.c $(incdir)/compat/endian.h \ + $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ + $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_event.h $(incdir)/sudo_fatal.h \ + $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ + $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/exec_intercept.h $(srcdir)/exec_ptrace.h \ + $(srcdir)/sudo.h $(srcdir)/sudo_exec.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h $(CC) -E -o $@ $(CPPFLAGS) $< exec_ptrace.plog: exec_ptrace.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/exec_ptrace.c --i-file $< --output-file $@ @@ -953,6 +955,24 @@ sudo_intercept_common.i: $(srcdir)/sudo_intercept_common.c \ $(CC) -E -o $@ $(CPPFLAGS) $< sudo_intercept_common.plog: sudo_intercept_common.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/sudo_intercept_common.c --i-file $< --output-file $@ +suspend_nopty.o: $(srcdir)/suspend_nopty.c $(incdir)/compat/stdbool.h \ + $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ + $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ + $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ + $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ + $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ + $(top_builddir)/config.h $(top_builddir)/pathnames.h + $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/suspend_nopty.c +suspend_nopty.i: $(srcdir)/suspend_nopty.c $(incdir)/compat/stdbool.h \ + $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ + $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ + $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ + $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ + $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ + $(top_builddir)/config.h $(top_builddir)/pathnames.h + $(CC) -E -o $@ $(CPPFLAGS) $< +suspend_nopty.plog: suspend_nopty.i + rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/suspend_nopty.c --i-file $< --output-file $@ tcsetpgrp_nobg.o: $(srcdir)/tcsetpgrp_nobg.c $(incdir)/compat/stdbool.h \ $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ @@ -972,24 +992,26 @@ tcsetpgrp_nobg.i: $(srcdir)/tcsetpgrp_nobg.c $(incdir)/compat/stdbool.h \ tcsetpgrp_nobg.plog: tcsetpgrp_nobg.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/tcsetpgrp_nobg.c --i-file $< --output-file $@ test_ptrace.o: $(srcdir)/regress/intercept/test_ptrace.c \ - $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ - $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ - $(incdir)/sudo_event.h $(incdir)/sudo_fatal.h \ - $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ - $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ - $(srcdir)/exec_intercept.h $(srcdir)/exec_ptrace.c \ - $(srcdir)/exec_ptrace.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ - $(top_builddir)/config.h $(top_builddir)/pathnames.h + $(incdir)/compat/endian.h $(incdir)/compat/stdbool.h \ + $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ + $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ + $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ + $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ + $(incdir)/sudo_util.h $(srcdir)/exec_intercept.h \ + $(srcdir)/exec_ptrace.c $(srcdir)/exec_ptrace.h \ + $(srcdir)/sudo.h $(srcdir)/sudo_exec.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/regress/intercept/test_ptrace.c test_ptrace.i: $(srcdir)/regress/intercept/test_ptrace.c \ - $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ - $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ - $(incdir)/sudo_event.h $(incdir)/sudo_fatal.h \ - $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \ - $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ - $(srcdir)/exec_intercept.h $(srcdir)/exec_ptrace.c \ - $(srcdir)/exec_ptrace.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \ - $(top_builddir)/config.h $(top_builddir)/pathnames.h + $(incdir)/compat/endian.h $(incdir)/compat/stdbool.h \ + $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \ + $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \ + $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ + $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ + $(incdir)/sudo_util.h $(srcdir)/exec_intercept.h \ + $(srcdir)/exec_ptrace.c $(srcdir)/exec_ptrace.h \ + $(srcdir)/sudo.h $(srcdir)/sudo_exec.h $(top_builddir)/config.h \ + $(top_builddir)/pathnames.h $(CC) -E -o $@ $(CPPFLAGS) $< test_ptrace.plog: test_ptrace.i rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/intercept/test_ptrace.c --i-file $< --output-file $@ diff --git a/src/exec_nopty.c b/src/exec_nopty.c index a9fc72408..7bd720ac5 100644 --- a/src/exec_nopty.c +++ b/src/exec_nopty.c @@ -558,9 +558,7 @@ handle_sigchld_nopty(struct exec_closure_nopty *ec) } if (WIFSTOPPED(status)) { - struct sigaction sa, osa; - pid_t saved_pgrp = -1; - int fd, signo = WSTOPSIG(status); + const int signo = WSTOPSIG(status); if (sig2str(signo, signame) == -1) (void)snprintf(signame, sizeof(signame), "%d", signo); @@ -573,79 +571,9 @@ handle_sigchld_nopty(struct exec_closure_nopty *ec) continue; } - /* Special handling for job control in the main command only. */ - if (pid != ec->cmnd_pid) - continue; - - /* - * Save the controlling terminal's process group so we can restore - * it after we resume, if needed. Most well-behaved shells change - * the pgrp back to its original value before suspending so we must - * not try to restore in that case, lest we race with the command - * upon resume, potentially stopping sudo with SIGTTOU while the - * command continues to run. - */ - fd = open(_PATH_TTY, O_RDWR); - if (fd != -1) { - saved_pgrp = tcgetpgrp(fd); - if (saved_pgrp == -1) { - close(fd); - fd = -1; - } - } - if (saved_pgrp != -1) { - /* - * Command was stopped trying to access the controlling - * terminal. If the command has a different pgrp and we - * own the controlling terminal, give it to the command's - * pgrp and let it continue. - */ - if (signo == SIGTTOU || signo == SIGTTIN) { - if (saved_pgrp == ec->ppgrp) { - pid_t cmnd_pgrp = getpgid(ec->cmnd_pid); - if (cmnd_pgrp != ec->ppgrp) { - if (tcsetpgrp_nobg(fd, cmnd_pgrp) == 0) { - if (killpg(cmnd_pgrp, SIGCONT) != 0) { - sudo_warn("kill(%d, SIGCONT)", - (int)cmnd_pgrp); - } - close(fd); - continue; - } - } - } - } - } - if (signo == SIGTSTP) { - memset(&sa, 0, sizeof(sa)); - sigemptyset(&sa.sa_mask); - sa.sa_flags = SA_RESTART; - sa.sa_handler = SIG_DFL; - if (sudo_sigaction(SIGTSTP, &sa, &osa) != 0) { - sudo_warn(U_("unable to set handler for signal %d"), - SIGTSTP); - } - } - if (kill(getpid(), signo) != 0) - sudo_warn("kill(%d, SIG%s)", (int)getpid(), signame); - if (signo == SIGTSTP) { - if (sudo_sigaction(SIGTSTP, &osa, NULL) != 0) { - sudo_warn(U_("unable to restore handler for signal %d"), - SIGTSTP); - } - } - if (saved_pgrp != -1) { - /* - * On resume, restore foreground process group, if different. - * Otherwise, we cannot resume some shells (pdksh). - * - * It is possible that we are no longer the foreground process, - * use tcsetpgrp_nobg() to prevent sudo from receiving SIGTTOU. - */ - if (saved_pgrp != ec->ppgrp) - tcsetpgrp_nobg(fd, saved_pgrp); - close(fd); - } + /* If the main command is suspended, suspend sudo too. */ + if (pid == ec->cmnd_pid) + suspend_sudo_nopty(signo, ec->ppgrp, ec->cmnd_pid); } else { if (WIFSIGNALED(status)) { if (sig2str(WTERMSIG(status), signame) == -1) { diff --git a/src/exec_pty.c b/src/exec_pty.c index 4e9b71d3d..36f176580 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -528,12 +528,12 @@ check_foreground(struct exec_closure_pty *ec) * foreground or SIGCONT_BG if it is a background process. */ static int -suspend_sudo(struct exec_closure_pty *ec, int signo) +suspend_sudo_pty(struct exec_closure_pty *ec, int signo) { char signame[SIG2STR_MAX]; struct sigaction sa, osa; int ret = 0; - debug_decl(suspend_sudo, SUDO_DEBUG_EXEC); + debug_decl(suspend_sudo_pty, SUDO_DEBUG_EXEC); switch (signo) { case SIGTTOU: @@ -1015,7 +1015,7 @@ backchannel_cb(int fd, int what, void *v) /* Suspend parent and tell monitor how to resume on return. */ sudo_debug_printf(SUDO_DEBUG_INFO, "command stopped, suspending parent"); - signo = suspend_sudo(ec, WSTOPSIG(cstat.val)); + signo = suspend_sudo_pty(ec, WSTOPSIG(cstat.val)); schedule_signal(ec, signo); /* Re-enable I/O events */ add_io_events(ec->evbase); @@ -1111,7 +1111,7 @@ handle_sigchld_pty(struct exec_closure_pty *ec) */ sudo_debug_printf(SUDO_DEBUG_INFO, "monitor stopped, suspending sudo"); - n = suspend_sudo(ec, WSTOPSIG(status)); + n = suspend_sudo_pty(ec, WSTOPSIG(status)); kill(pid, SIGCONT); schedule_signal(ec, n); /* Re-enable I/O events */ diff --git a/src/regress/intercept/test_ptrace.c b/src/regress/intercept/test_ptrace.c index 704222a18..36b52eef4 100644 --- a/src/regress/intercept/test_ptrace.c +++ b/src/regress/intercept/test_ptrace.c @@ -97,6 +97,12 @@ init_debug_files(struct sudo_conf_debug_file_list *file_list, debug_return; } +int +sudo_sigaction(int signo, struct sigaction *sa, struct sigaction *osa) +{ + return sigaction(signo, sa, osa); +} + int main(int argc, char *argv[]) { @@ -107,7 +113,7 @@ main(int argc, char *argv[]) const char *errstr; sigset_t blocked, empty; struct sigaction sa; - pid_t child, pid; + pid_t child, pid, ppgrp; int ch, status; debug_decl_vars(main, SUDO_DEBUG_MAIN); @@ -159,6 +165,7 @@ main(int argc, char *argv[]) sigaction(SIGUSR1, &sa, NULL); /* Fork a shell. */ + ppgrp = getpgrp(); child = fork(); switch (child) { case -1: @@ -212,9 +219,9 @@ main(int argc, char *argv[]) } else if (WIFSTOPPED(status)) { if (exec_ptrace_stopped(pid, status, &closure)) { if (pid == child) { - /* TODO - terminal pgrp switching like exec_nopty.c. */ - kill(getpid(), WSTOPSIG(status)); - kill(child, SIGCONT); + suspend_sudo_nopty(WSTOPSIG(status), ppgrp, child); + if (kill(child, SIGCONT) != 0) + sudo_warn("kill(%d, SIGCONT)", (int)child); } } } else { diff --git a/src/sudo_exec.h b/src/sudo_exec.h index 9c58cd2ab..c34172e59 100644 --- a/src/sudo_exec.h +++ b/src/sudo_exec.h @@ -151,4 +151,7 @@ bool exec_ptrace_stopped(pid_t pid, int status, void *intercept); bool set_exec_filter(void); int exec_ptrace_seize(pid_t child); +/* suspend_nopty.c */ +void suspend_sudo_nopty(int signo, pid_t ppgrp, pid_t cmnd_pid); + #endif /* SUDO_EXEC_H */ diff --git a/src/suspend_nopty.c b/src/suspend_nopty.c new file mode 100644 index 000000000..babeae29b --- /dev/null +++ b/src/suspend_nopty.c @@ -0,0 +1,111 @@ +/* + * SPDX-License-Identifier: ISC + * + * Copyright (c) 2009-2022 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +/* + * This is an open source non-commercial project. Dear PVS-Studio, please check it. + * PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "sudo.h" +#include "sudo_exec.h" + +void +suspend_sudo_nopty(int signo, pid_t ppgrp, pid_t cmnd_pid) +{ + struct sigaction sa, osa; + pid_t saved_pgrp = -1; + int fd; + debug_decl(suspend_sudo_nopty, SUDO_DEBUG_EXEC); + + /* + * Save the controlling terminal's process group so we can restore + * it after we resume, if needed. Most well-behaved shells change + * the pgrp back to its original value before suspending so we must + * not try to restore in that case, lest we race with the command + * upon resume, potentially stopping sudo with SIGTTOU while the + * command continues to run. + */ + fd = open(_PATH_TTY, O_RDWR); + if (fd != -1) { + saved_pgrp = tcgetpgrp(fd); + if (saved_pgrp == -1) { + close(fd); + fd = -1; + } + } + + if (saved_pgrp != -1) { + /* + * Command was stopped trying to access the controlling + * terminal. If the command has a different pgrp and we + * own the controlling terminal, give it to the command's + * pgrp and let it continue. + */ + if (signo == SIGTTOU || signo == SIGTTIN) { + if (saved_pgrp == ppgrp) { + pid_t cmnd_pgrp = getpgid(cmnd_pid); + if (cmnd_pgrp != ppgrp) { + if (tcsetpgrp_nobg(fd, cmnd_pgrp) == 0) { + if (killpg(cmnd_pgrp, SIGCONT) != 0) + sudo_warn("kill(%d, SIGCONT)", (int)cmnd_pgrp); + close(fd); + debug_return; + } + } + } + } + } + + if (signo == SIGTSTP) { + memset(&sa, 0, sizeof(sa)); + sigemptyset(&sa.sa_mask); + sa.sa_flags = SA_RESTART; + sa.sa_handler = SIG_DFL; + if (sudo_sigaction(SIGTSTP, &sa, &osa) != 0) + sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP); + } + if (kill(getpid(), signo) != 0) + sudo_warn("kill(%d, %d)", (int)getpid(), signo); + if (signo == SIGTSTP) { + if (sudo_sigaction(SIGTSTP, &osa, NULL) != 0) + sudo_warn(U_("unable to restore handler for signal %d"), SIGTSTP); + } + if (saved_pgrp != -1) { + /* + * On resume, restore foreground process group, if different. + * Otherwise, we cannot resume some shells (pdksh). + * + * It is possible that we are no longer the foreground process, + * use tcsetpgrp_nobg() to prevent sudo from receiving SIGTTOU. + */ + if (saved_pgrp != ppgrp) + tcsetpgrp_nobg(fd, saved_pgrp); + close(fd); + } + + debug_return; +}