Merge bitcoin/bitcoin#27896: Remove the syscall sandbox

32e2ffc393 Remove the syscall sandbox (fanquake)

Pull request description:

  After initially being merged in #20487, it's no-longer clear that an internal syscall sandboxing mechanism is something that Bitcoin Core should have/maintain, especially when compared to better maintained/supported alterantives, i.e [firejail](https://github.com/netblue30/firejail).

  There is more related discussion in #24771.

  Note that given where it's used, the sandbox also gets dragged into the kernel.

  If it's removed, this should not require any sort of deprecation, as this was only ever an opt-in, experimental feature.

  Closes #24771.

ACKs for top commit:
  davidgumberg:
     crACK 32e2ffc393
  achow101:
    ACK 32e2ffc393
  dergoegge:
    ACK 32e2ffc393

Tree-SHA512: 8cf71c5623bb642cb515531d4a2545d806e503b9d57bfc15a996597632b06103d60d985fd7f843a3c1da6528bc38d0298d6b8bcf0be6f851795a8040d71faf16

test/functional/feature_notifications.py

@@ -30,9 +30,6 @@ class NotificationsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.setup_clean_chain = True
-        # The experimental syscall sandbox feature (-sandbox) is not compatible with -alertnotify,
-        # -blocknotify, -walletnotify or -shutdownnotify (which all invoke execve).
-        self.disable_syscall_sandbox = True
 
     def setup_network(self):
         self.wallet = ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHARS_DISALLOWED)

test/functional/feature_startupnotify.py

@@ -18,7 +18,6 @@ FILE_NAME = "test.txt"
 class StartupNotifyTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        self.disable_syscall_sandbox = True
 
     def run_test(self):
         tmpdir_file = os.path.join(self.options.tmpdir, NODE_DIR, FILE_NAME)

test/functional/feature_syscall_sandbox.py

@@ -1,34 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (c) 2021-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test bitcoind aborts if a disallowed syscall is used when compiled with the syscall sandbox."""
-
-from test_framework.test_framework import BitcoinTestFramework, SkipTest
-
-
-class SyscallSandboxTest(BitcoinTestFramework):
-    def set_test_params(self):
-        self.num_nodes = 1
-
-    def skip_test_if_missing_module(self):
-        if not self.is_syscall_sandbox_compiled():
-            raise SkipTest("bitcoind has not been built with syscall sandbox enabled.")
-        if self.disable_syscall_sandbox:
-            raise SkipTest("--nosandbox passed to test runner.")
-
-    def run_test(self):
-        disallowed_syscall_terminated_bitcoind = False
-        expected_log_entry = 'ERROR: The syscall "getgroups" (syscall number 115) is not allowed by the syscall sandbox'
-        with self.nodes[0].assert_debug_log([expected_log_entry]):
-            self.log.info("Invoking disallowed syscall")
-            try:
-                self.nodes[0].invokedisallowedsyscall()
-            except ConnectionError:
-                disallowed_syscall_terminated_bitcoind = True
-        assert disallowed_syscall_terminated_bitcoind
-        self.nodes = []
-
-
-if __name__ == "__main__":
-    SyscallSandboxTest().main()

test/functional/feature_versionbits_warning.py

@@ -28,9 +28,6 @@ class VersionBitsWarningTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
-        # The experimental syscall sandbox feature (-sandbox) is not compatible with -alertnotify
-        # (which invokes execve).
-        self.disable_syscall_sandbox = True
 
     def setup_network(self):
         self.alert_filename = os.path.join(self.options.tmpdir, "alert.txt")

test/functional/rpc_signer.py

@@ -27,9 +27,6 @@ class RPCSignerTest(BitcoinTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 4
-        # The experimental syscall sandbox feature (-sandbox) is not compatible with -signer (which
-        # invokes execve).
-        self.disable_syscall_sandbox = True
 
         self.extra_args = [
             [],

test/functional/test_framework/test_framework.py

@@ -103,7 +103,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         self.supports_cli = True
         self.bind_to_localhost_only = True
         self.parse_args()
-        self.disable_syscall_sandbox = self.options.nosandbox or self.options.valgrind
         self.default_wallet_name = "default_wallet" if self.options.descriptors else ""
         self.wallet_data_filename = "wallet.dat"
         # Optional list of wallet names that can be set in set_test_params to
@@ -160,8 +159,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         parser = argparse.ArgumentParser(usage="%(prog)s [options]")
         parser.add_argument("--nocleanup", dest="nocleanup", default=False, action="store_true",
                             help="Leave bitcoinds and test.* datadir on exit or error")
-        parser.add_argument("--nosandbox", dest="nosandbox", default=False, action="store_true",
-                            help="Don't use the syscall sandbox")
         parser.add_argument("--noshutdown", dest="noshutdown", default=False, action="store_true",
                             help="Don't stop bitcoinds after the test execution")
         parser.add_argument("--cachedir", dest="cachedir", default=os.path.abspath(os.path.dirname(os.path.realpath(__file__)) + "/../../cache"),
@@ -188,7 +185,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         parser.add_argument("--perf", dest="perf", default=False, action="store_true",
                             help="profile running nodes with perf for the duration of the test")
         parser.add_argument("--valgrind", dest="valgrind", default=False, action="store_true",
-                            help="run nodes under the valgrind memory error detector: expect at least a ~10x slowdown. valgrind 3.14 or later required. Forces --nosandbox.")
+                            help="run nodes under the valgrind memory error detector: expect at least a ~10x slowdown. valgrind 3.14 or later required.")
         parser.add_argument("--randomseed", type=int,
                             help="set a random seed for deterministically reproducing a previous test run")
         parser.add_argument("--timeout-factor", dest="timeout_factor", type=float, help="adjust test timeouts by a factor. Setting it to 0 disables all timeouts")
@@ -497,11 +494,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             extra_args = [[]] * num_nodes
         if versions is None:
             versions = [None] * num_nodes
-        if self.is_syscall_sandbox_compiled() and not self.disable_syscall_sandbox:
-            for i in range(len(extra_args)):
-                # The -sandbox argument is not present in the v22.0 release.
-                if versions[i] is None or versions[i] >= 229900:
-                    extra_args[i] = extra_args[i] + ["-sandbox=log-and-abort"]
         if binary is None:
             binary = [get_bin_from_version(v, 'bitcoind', self.options.bitcoind) for v in versions]
         if binary_cli is None:
@@ -987,7 +979,3 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
     def is_bdb_compiled(self):
         """Checks whether the wallet module was compiled with BDB support."""
         return self.config["components"].getboolean("USE_BDB")
-
-    def is_syscall_sandbox_compiled(self):
-        """Checks whether the syscall sandbox was compiled."""
-        return self.config["components"].getboolean("ENABLE_SYSCALL_SANDBOX")

test/functional/test_runner.py

@@ -210,7 +210,6 @@ BASE_SCRIPTS = [
     'rpc_users.py',
     'rpc_whitelist.py',
     'feature_proxy.py',
-    'feature_syscall_sandbox.py',
     'wallet_signrawtransactionwithwallet.py --legacy-wallet',
     'wallet_signrawtransactionwithwallet.py --descriptors',
     'rpc_signrawtransactionwithkey.py',

test/functional/wallet_signer.py

@@ -45,9 +45,6 @@ class WalletSignerTest(BitcoinTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 2
-        # The experimental syscall sandbox feature (-sandbox) is not compatible with -signer (which
-        # invokes execve).
-        self.disable_syscall_sandbox = True
 
         self.extra_args = [
             [],