From 7d2f32ae7a571cacc7b51bf5988e47b2aae71d39 Mon Sep 17 00:00:00 2001 From: Benedikt Spies Date: Fri, 2 Oct 2026 16:53:12 +0200 Subject: [PATCH] Add --forward-signals option Forward signals sent to bwrap on to the sandboxed command, through the init process. Forwarding stops once init has reaped the command, as the kernel can then give its pid to an unrelated process. Signals the kernel sent to our process group are not forwarded, as the command received those itself unless it has left the group. Inspired by #402 and #586. Resolves: https://github.com/containers/bubblewrap/issues/369 Resolves: https://github.com/containers/bubblewrap/issues/573 Signed-off-by: Benedikt Spies Co-Authored-By: Claude Opus 5 --- bubblewrap.c | 120 ++++++++++++++++++++--- bwrap.xml | 13 +++ completions/bash/bwrap | 1 + completions/zsh/_bwrap | 1 + tests/meson.build | 2 + tests/test-forward-signals-tty.py | 109 +++++++++++++++++++++ tests/test-forward-signals.sh | 158 ++++++++++++++++++++++++++++++ 7 files changed, 391 insertions(+), 13 deletions(-) create mode 100644 tests/test-forward-signals-tty.py create mode 100755 tests/test-forward-signals.sh diff --git a/bubblewrap.c b/bubblewrap.c index 2ab88d2f..8611411f 100644 --- a/bubblewrap.c +++ b/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 (); diff --git a/bwrap.xml b/bwrap.xml index b48cf5b4..9e4487e2 100644 --- a/bwrap.xml +++ b/bwrap.xml @@ -592,6 +592,19 @@ See prctl, PR_SET_PDEATHSIG. + + + + 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 + , because it receives those itself. + + With , COMMAND must install a handler to + receive them, as for any PID=1. + + diff --git a/completions/bash/bwrap b/completions/bash/bwrap index e7a523c2..e9e8d102 100644 --- a/completions/bash/bwrap +++ b/completions/bash/bwrap @@ -47,6 +47,7 @@ _bwrap() { --exec-label --file --file-label + --forward-signals --gid --hostname --info-fd diff --git a/completions/zsh/_bwrap b/completions/zsh/_bwrap index fbddda43..bea4075a 100644 --- a/completions/zsh/_bwrap +++ b/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:' diff --git a/tests/meson.build b/tests/meson.build index fe45af0a..1c820b58 100644 --- a/tests/meson.build +++ b/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', diff --git a/tests/test-forward-signals-tty.py b/tests/test-forward-signals-tty.py new file mode 100644 index 00000000..775a456b --- /dev/null +++ b/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__]) diff --git a/tests/test-forward-signals.sh b/tests/test-forward-signals.sh new file mode 100755 index 00000000..28385bf9 --- /dev/null +++ b/tests/test-forward-signals.sh @@ -0,0 +1,158 @@ +#!/usr/bin/env bash + +set -xeuo pipefail + +srcd=$(cd $(dirname "$0") && pwd) +. "${srcd}/libtest.sh" + +if test -n "${bwrap_is_suid:-}"; then + skip "no setuid support for --unshare-user" +fi + +echo "1..8" +test_count=0 + +ok () { + test_count=$((test_count + 1)) + echo ok $test_count "$@" +} + +SANDBOX="$BWRAP --unshare-user --uid 0 --gid 0 --unshare-pid --bind / / --proc /proc --dev /dev" + +# wait_for SECONDS CMD...: poll until CMD succeeds, fail after SECONDS +wait_for () { + local deadline=$((SECONDS + $1)) + shift + until "$@"; do + test "$SECONDS" -lt "$deadline" || assert_not_reached "timed out waiting for: $*" + sleep 0.05 + done +} + +# exited PID: PID is gone or a zombie +exited () { + ! kill -0 "$1" 2>/dev/null || test "$(sed 's/.*) //' "/proc/$1/stat" 2>/dev/null | cut -c1)" = Z +} + +# child_pid FILE: the pid bwrap reported to --info-fd +child_pid () { + wait_for 5 grep -qs child-pid "$1" + sed -n 's/.*"child-pid": *\([0-9]*\).*/\1/p' "$1" +} + +# A command that reports the signals it receives and keeps running +cat > handler.sh <<'EOF' +trap 'echo got-term >> "$1"' TERM +trap 'echo got-usr1 >> "$1"' USR1 +echo ready >> "$1" +while :; do sleep 0.2; done +EOF + +# start OPTIONS...: run $SANDBOX in its own process group, as a job would be, +# and set BWRAP_PID and SANDBOX_PID once the command is ready +start () { + rm -f log info.json + set -m + $SANDBOX --info-fd 3 "$@" -- bash handler.sh "$(pwd)/log" 3> info.json > /dev/null 2>&1 & + BWRAP_PID=$! + set +m + SANDBOX_PID=$(child_pid info.json) + wait_for 5 grep -qs ready log +} + +start +kill -TERM "$BWRAP_PID" +wait_for 2 exited "$BWRAP_PID" +sleep 0.5 +assert_not_file_has_content log got-term +kill -KILL "$SANDBOX_PID" 2>/dev/null || true +ok "without --forward-signals, a signal is not forwarded" + +# a command without a handler, so the forwarded SIGTERM kills it +rm -f log +set -m +$SANDBOX --forward-signals -- sh -c "echo ready >> $(pwd)/log; exec sleep 60" > /dev/null 2>&1 & +bwrap_pid=$! +set +m +wait_for 5 grep -qs ready log +kill -TERM "$bwrap_pid" +status=0 +wait "$bwrap_pid" || status=$? +assert_streq "$status" 143 +ok "SIGTERM reaches the command, which bwrap reports as 128+15" + +start --forward-signals +kill -USR1 "$BWRAP_PID" +wait_for 2 grep -qs got-usr1 log +kill -0 "$BWRAP_PID" +kill -KILL "$SANDBOX_PID" +ok "a forwarded SIGUSR1 is handled and the sandbox keeps running" + +start --forward-signals +kill -TERM "$SANDBOX_PID" +wait_for 2 grep -qs got-term log +kill -KILL "$SANDBOX_PID" +ok "a signal sent to the init process reaches the command" + +# The command exits at once, a background process keeps the sandbox alive, and +# a new process takes the pid the command had. No --proc, as /proc/sys is +# read-only in the sandbox bwrap sets up. +rm -f log info.json +$SANDBOX --forward-signals --info-fd 3 -- sh -c 'sleep 60 > /dev/null 2>&1 &' 3> info.json +sandbox_pid=$(child_pid info.json) +set -m +$BWRAP --userns 11 --pidns 12 --cap-add ALL --bind / / -- \ + sh -c "echo 1 > /proc/sys/kernel/ns_last_pid && bash $(pwd)/handler.sh $(pwd)/log; true" \ + 11< "/proc/$sandbox_pid/ns/user" 12< "/proc/$sandbox_pid/ns/pid" > /dev/null 2>&1 & +reuse_pid=$! +set +m +wait_for 5 grep -qs ready log +kill -TERM "$sandbox_pid" +sleep 0.5 +assert_not_file_has_content log got-term +kill -KILL "$reuse_pid" "$sandbox_pid" 2>/dev/null || true +ok "once the command is reaped, its pid is not signalled again" + +# Without --unshare-pid there is no init process, so bwrap's child is the command +rm -f log +set -m +$BWRAP --forward-signals --unshare-user --uid 0 --gid 0 --bind / / --proc /proc --dev /dev \ + -- bash handler.sh "$(pwd)/log" > /dev/null 2>&1 & +bwrap_pid=$! +set +m +wait_for 5 grep -qs ready log +kill -TERM "$bwrap_pid" +wait_for 2 grep -qs got-term log +kill -KILL "$bwrap_pid" +ok "a signal is forwarded when there is no init process" + +# bwrap also runs an init process for --lock-file, where it is not pid 1 +rm -f log +touch lock +set -m +$BWRAP --forward-signals --lock-file "$(pwd)/lock" --bind / / --proc /proc --dev /dev \ + -- bash handler.sh "$(pwd)/log" > /dev/null 2>&1 & +bwrap_pid=$! +set +m +wait_for 5 grep -qs ready log +kill -TERM "$bwrap_pid" +wait_for 2 grep -qs got-term log +kill -KILL "$bwrap_pid" +ok "a signal is forwarded through the init process of --lock-file" + +# --block-fd holds the command back, so the signal arrives during setup +rm -f log info.json +mkfifo block +set -m +$SANDBOX --forward-signals --info-fd 3 --block-fd 4 -- sh -c "echo started >> $(pwd)/log" \ + 3> info.json 4<> block > /dev/null 2>&1 & +bwrap_pid=$! +set +m +sandbox_pid=$(child_pid info.json) +kill -TERM "$bwrap_pid" +echo go > block +status=0 +wait "$bwrap_pid" || status=$? +assert_streq "$status" 143 +test ! -e log +ok "a signal arriving during setup is not lost and the command does not start"