Add --forward-signals option

bubblewrap.c

@@ -77,6 +77,7 @@ static bool opt_unshare_cgroup_try = false;
 static bool opt_needs_devpts = false;
 static bool opt_new_session = false;
 static bool opt_die_with_parent = false;
+static bool opt_forward_signals = false;
 static uid_t opt_sandbox_uid = -1;
 static gid_t opt_sandbox_gid = -1;
 static int opt_sync_fd = -1;
@@ -352,6 +353,7 @@ usage (int ecode, FILE *out)
            "    --json-status-fd FD          Write container status to FD as multiple JSON documents\n"
            "    --new-session                Create a new terminal session\n"
            "    --die-with-parent            Kills with SIGKILL child process (COMMAND) when bwrap or bwrap's parent dies.\n"
+           "    --forward-signals            Forward SIGHUP, SIGINT, SIGQUIT, SIGTERM, SIGUSR1, SIGUSR2 and SIGWINCH to COMMAND\n"
            "    --as-pid-1                   Do not install a reaper process with PID=1\n"
            "    --cap-add CAP                Add cap CAP when running as privileged user\n"
            "    --cap-drop CAP               Drop cap CAP when running as privileged user\n"
@@ -374,14 +376,39 @@ handle_die_with_parent (void)
     die_with_error ("prctl");
 }
 
+/* Signals that --forward-signals passes on to the command */
+static const int forwarded_signals[] = { SIGHUP, SIGINT, SIGQUIT, SIGTERM, SIGUSR1, SIGUSR2, SIGWINCH };
+
+static void
+add_forwarded_signals (sigset_t *mask)
+{
+  size_t i;
+
+  if (!opt_forward_signals)
+    return;
+
+  for (i = 0; i < N_ELEMENTS (forwarded_signals); i++)
+    sigaddset (mask, forwarded_signals[i]);
+}
+
+/* SIGCHLD, plus the signals --forward-signals passes on */
+static void
+monitored_signals (sigset_t *mask)
+{
+  sigemptyset (mask);
+  sigaddset (mask, SIGCHLD);
+  add_forwarded_signals (mask);
+}
+
+/* The sandbox inherits the mask, so a signal arriving during setup stays
+ * pending until the command is ready for it. */
 static void
-block_sigchild (void)
+block_signals (void)
 {
   sigset_t mask;
   int status;
 
-  sigemptyset (&mask);
-  sigaddset (&mask, SIGCHLD);
+  monitored_signals (&mask);
 
   if (sigprocmask (SIG_BLOCK, &mask, NULL) == -1)
     die_with_error ("sigprocmask");
@@ -392,17 +419,72 @@ block_sigchild (void)
 }
 
 static void
-unblock_sigchild (void)
+unblock_signals (void)
 {
   sigset_t mask;
 
-  sigemptyset (&mask);
-  sigaddset (&mask, SIGCHLD);
+  monitored_signals (&mask);
 
   if (sigprocmask (SIG_UNBLOCK, &mask, NULL) == -1)
     die_with_error ("sigprocmask");
 }
 
+/* A signal the kernel sent to our process group has already reached pid if it
+ * is in that group, so forwarding it would deliver it twice. Also called from
+ * a signal handler: getpgid() is a bare system call on Linux. */
+static bool
+should_forward_signal (pid_t pid, int code)
+{
+  pid_t pgid;
+
+  if (pid <= 0)
+    return false;
+
+  if (code != SI_KERNEL)
+    return true;
+
+  pgid = getpgid (pid);
+  return pgid != -1 && pgid != getpgrp ();
+}
+
+/* The command, while init forwards to it; 0 once init has reaped it, as its
+ * pid can be reused after that */
+static volatile sig_atomic_t forward_signals_pid = 0;
+
+static void
+forward_signal_handler (int sig, siginfo_t *info, void *context UNUSED)
+{
+  int saved_errno = errno;
+  pid_t pid = forward_signals_pid;
+
+  if (should_forward_signal (pid, info->si_code))
+    kill (pid, sig);
+
+  errno = saved_errno;
+}
+
+/* Used in init: as pid 1 of a pid namespace it only receives signals it has a
+ * handler for, so it cannot use a signalfd like the monitor does. */
+static void
+install_forward_signal_handlers (pid_t pid)
+{
+  struct sigaction act = { 0 };
+  sigset_t mask;
+  size_t i;
+
+  forward_signals_pid = pid;
+  act.sa_sigaction = forward_signal_handler;
+  act.sa_flags = SA_SIGINFO | SA_RESTART;
+  for (i = 0; i < N_ELEMENTS (forwarded_signals); i++)
+    if (sigaction (forwarded_signals[i], &act, NULL) != 0)
+      die_with_error ("sigaction");
+
+  sigemptyset (&mask);
+  add_forwarded_signals (&mask);
+  if (sigprocmask (SIG_UNBLOCK, &mask, NULL) == -1)
+    die_with_error ("sigprocmask");
+}
+
 /* Closes all fd:s except 0,1,2 and the passed in array of extra fds */
 static int
 close_extra_fds (void *data, int fd)
@@ -507,8 +589,7 @@ monitor_child (int event_fd, pid_t child_pid, int setup_finished_fd)
   assert (j < sizeof(dont_close)/sizeof(*dont_close));
   fdwalk (close_extra_fds, dont_close);
 
-  sigemptyset (&mask);
-  sigaddset (&mask, SIGCHLD);
+  monitored_signals (&mask);
 
   signal_fd = signalfd (-1, &mask, SFD_CLOEXEC | SFD_NONBLOCK);
   if (signal_fd == -1)
@@ -548,12 +629,17 @@ monitor_child (int event_fd, pid_t child_pid, int setup_finished_fd)
         }
 
       /* We need to read the signal_fd, or it will keep polling as read,
-       * however we ignore the details as we get them from waitpid
+       * however we ignore the details of SIGCHLD as we get them from waitpid
        * below anyway */
       s = read (signal_fd, &fdsi, sizeof (struct signalfd_siginfo));
       if (s == -1 && errno != EINTR && errno != EAGAIN)
         die_with_error ("read signalfd");
 
+      /* With --forward-signals, pass the signal on */
+      if (s == (ssize_t) sizeof (struct signalfd_siginfo) && fdsi.ssi_signo != SIGCHLD &&
+          should_forward_signal (child_pid, fdsi.ssi_code))
+        kill (child_pid, fdsi.ssi_signo);
+
       /* We may actually get several sigchld compressed into one
          SIGCHLD, so we have to handle all of them. */
       while ((died_pid = waitpid (-1, &died_status, WNOHANG)) > 0)
@@ -610,6 +696,9 @@ do_init (int event_fd, pid_t initial_pid)
   /* Optionally bind our lifecycle to that of the caller */
   handle_die_with_parent ();
 
+  if (opt_forward_signals)
+    install_forward_signal_handlers (initial_pid);
+
   seccomp_programs_apply ();
 
   while (true)
@@ -620,6 +709,7 @@ do_init (int event_fd, pid_t initial_pid)
       child = TEMP_FAILURE_RETRY (wait (&status));
       if (child == initial_pid)
         {
+          forward_signals_pid = 0;
           initial_exit_status = propagate_exit_status (status);
 
           if(event_fd != -1)
@@ -2575,6 +2665,10 @@ parse_args_recurse (int          *argcp,
         {
           opt_die_with_parent = true;
         }
+      else if (strcmp (arg, "--forward-signals") == 0)
+        {
+          opt_forward_signals = true;
+        }
       else if (strcmp (arg, "--as-pid-1") == 0)
         {
           opt_as_pid_1 = true;
@@ -3052,8 +3146,8 @@ main (int    argc,
         die_with_error ("eventfd()");
     }
 
-  /* We block sigchild here so that we can use signalfd in the monitor. */
-  block_sigchild ();
+  /* We block sigchild (and forwarded signals) here so that we can use signalfd in the monitor. */
+  block_signals ();
 
   clone_flags = SIGCHLD | CLONE_NEWNS;
   if (opt_unshare_user)
@@ -3526,8 +3620,8 @@ main (int    argc,
         close (opt_sync_fd);
     }
 
-  /* We want sigchild in the child */
-  unblock_sigchild ();
+  /* We want sigchild and forwarded signals in the child */
+  unblock_signals ();
 
   /* Optionally bind our lifecycle */
   handle_die_with_parent ();

bwrap.xml

@@ -592,6 +592,19 @@
         See prctl, PR_SET_PDEATHSIG.
       </para></listitem>
     </varlistentry>
+    <varlistentry>
+      <term><option>--forward-signals</option></term>
+      <listitem><para>
+        Forwards SIGHUP, SIGINT, SIGQUIT, SIGTERM, SIGUSR1, SIGUSR2 and
+        SIGWINCH sent to bwrap on to the child process (COMMAND), so that it
+        can handle them. Signals generated by the terminal are not forwarded
+        unless COMMAND has left bwrap's process group, for example with
+        <option>--new-session</option>, because it receives those itself.
+        </para><para>
+        With <option>--as-pid-1</option>, COMMAND must install a handler to
+        receive them, as for any PID=1.
+      </para></listitem>
+    </varlistentry>
     <varlistentry>
       <term><option>--as-pid-1</option></term>
       <listitem><para>

completions/bash/bwrap

@@ -47,6 +47,7 @@ _bwrap() {
 		--exec-label
 		--file
 		--file-label
+		--forward-signals
 		--gid
 		--hostname
 		--info-fd

completions/zsh/_bwrap

@@ -46,6 +46,7 @@ _bwrap_args=(
     '--disable-userns[Disable further use of user namespaces inside sandbox]'
     '--exec-label[Exec label for the sandbox]:SELinux label:_selinux_contexts'
     '--file-label[File label for temporary sandbox content]:SELinux label:_selinux_contexts'
+    '--forward-signals[Forward SIGHUP, SIGINT, SIGQUIT, SIGTERM, SIGUSR1, SIGUSR2 and SIGWINCH to COMMAND]'
     '--gid[Custom gid in the sandbox (requires --unshare-user or --userns)]: :_guard "[0-9]#" "numeric group ID"'
     '--help[Print help and exit]'
     '--hostname[Custom hostname in the sandbox (requires --unshare-uts)]:hostname:'

tests/meson.build

@@ -16,6 +16,8 @@ executable(
 )
 
 test_scripts = [
+  'test-forward-signals-tty.py',
+  'test-forward-signals.sh',
   'test-run.sh',
   'test-sandbox.py',
   'test-seccomp.py',

tests/test-forward-signals-tty.py

@@ -0,0 +1,109 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# The parts of --forward-signals that need a terminal, as the kernel sends
+# SIGINT and SIGWINCH to the foreground process group of one.
+
+import fcntl
+import importlib.util
+import os
+import pty
+import signal
+import struct
+import sys
+import tempfile
+import termios
+import time
+import unittest
+
+_spec = importlib.util.spec_from_file_location(
+    'test_helper',
+    os.path.join(os.path.dirname(os.path.abspath(__file__)), 'test-helper.py'),
+)
+_helper = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_helper)
+
+BWRAP = _helper.BWRAP
+can_run_bwrap = _helper.can_run_bwrap
+run_tap_tests = _helper.run_tap_tests
+
+# Reports the signals it receives and keeps running
+COMMAND = r'''
+trap 'echo got-int >> "$1"' INT
+trap 'echo got-winch >> "$1"' WINCH
+echo ready >> "$1"
+while :; do sleep 0.2; done
+'''
+
+
+@unittest.skipUnless(can_run_bwrap(), 'bwrap not available or not functional')
+class TestForwardSignalsTty(unittest.TestCase):
+    def sandbox(self, *extra_args):
+        """bwrap in a pseudo-terminal, as its own session and foreground group"""
+        d = tempfile.TemporaryDirectory(prefix='bwrap-tty-test.')
+        self.addCleanup(d.cleanup)
+        self.log = os.path.join(d.name, 'log')
+        script = os.path.join(d.name, 'command.sh')
+        with open(script, 'w') as f:
+            f.write(COMMAND)
+        self.pid, self.terminal = pty.fork()
+        if self.pid == 0:
+            try:
+                os.execv(BWRAP, [
+                    BWRAP, '--forward-signals', '--unshare-user',
+                    '--uid', '0', '--gid', '0', '--bind', '/', '/',
+                    '--proc', '/proc', '--dev', '/dev', *extra_args,
+                    '--', '/bin/sh', script, self.log,
+                ])
+            finally:
+                os._exit(127)
+        self.addCleanup(self.kill)
+        self.assertTrue(self.wait_for('ready'), 'command did not start')
+
+    def kill(self):
+        try:
+            os.kill(self.pid, signal.SIGKILL)
+            os.waitpid(self.pid, 0)
+        except OSError:
+            pass
+        os.close(self.terminal)
+
+    def wait_for(self, text, timeout=10):
+        deadline = time.monotonic() + timeout
+        while time.monotonic() < deadline:
+            try:
+                with open(self.log) as f:
+                    if text in f.read():
+                        return True
+            except FileNotFoundError:
+                pass
+            time.sleep(0.02)
+        return False
+
+    def check_ctrl_c(self, *extra_args):
+        self.sandbox(*extra_args)
+        os.write(self.terminal, b'\x03')
+        self.assertTrue(self.wait_for('got-int', 3), 'SIGINT did not arrive')
+        # the command handles it, so bwrap must not have died in its place
+        self.assertEqual(os.waitpid(self.pid, os.WNOHANG), (0, 0))
+
+    def test_ctrl_c(self):
+        self.check_ctrl_c('--unshare-pid')
+
+    def test_ctrl_c_as_pid_1(self):
+        self.check_ctrl_c('--unshare-pid', '--as-pid-1')
+
+    def resize(self):
+        fcntl.ioctl(self.terminal, termios.TIOCSWINSZ,
+                    struct.pack('HHHH', 40, 100, 0, 0))
+
+    def test_resize(self):
+        """With --new-session the command is in a session of its own, so a
+        resize only reaches it if bwrap forwards SIGWINCH."""
+        self.sandbox('--unshare-pid', '--new-session')
+        self.resize()
+        self.assertTrue(self.wait_for('got-winch', 3), 'SIGWINCH did not arrive')
+
+
+if __name__ == '__main__':
+    run_tap_tests(sys.modules[__name__])