Skip to content

Commit 32e2ffc

Browse files
committed
Remove the syscall sandbox
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. Note that given where it's used, the sandbox also gets dragged into the kernel. There is some related discussion in #24771. This should not require any sort of deprecation, as this was only ever an opt-in, experimental feature. Closes #24771.
1 parent b3db18a commit 32e2ffc

28 files changed

+5
-1175
lines changed

ci/test/00_setup_env_i686_multiprocess.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,3 @@ export GOAL="install"
1515
export BITCOIN_CONFIG="--enable-debug CC='clang -m32' CXX='clang++ -m32' \
1616
LDFLAGS='--rtlib=compiler-rt -lgcc_s' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'"
1717
export TEST_RUNNER_ENV="BITCOIND=bitcoin-node"
18-
export TEST_RUNNER_EXTRA="--nosandbox"

ci/test/00_setup_env_native_valgrind.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export CONTAINER_NAME=ci_native_valgrind
1111
export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libsqlite3-dev"
1212
export USE_VALGRIND=1
1313
export NO_DEPENDS=1
14-
export TEST_RUNNER_EXTRA="--nosandbox --exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
14+
export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
1515
export GOAL="install"
1616
# Temporarily pin dwarf 4, until using Valgrind 3.20 or later
1717
export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++ CFLAGS='-gdwarf-4' CXXFLAGS='-gdwarf-4'" # TODO enable GUI

configure.ac

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,6 @@ case $host in
9696
;;
9797
esac
9898

99-
AC_ARG_WITH([seccomp],
100-
[AS_HELP_STRING([--with-seccomp],
101-
[enable experimental syscall sandbox feature (-sandbox), default is yes if seccomp-bpf is detected under Linux x86_64])],
102-
[seccomp_found=$withval],
103-
[seccomp_found=auto])
104-
10599
AC_ARG_ENABLE([c++20],
106100
[AS_HELP_STRING([--enable-c++20],
107101
[enable compilation in c++20 mode (disabled by default)])],
@@ -1539,36 +1533,6 @@ if test "$use_external_signer" != "no"; then
15391533
fi
15401534
AM_CONDITIONAL([ENABLE_EXTERNAL_SIGNER], [test "$use_external_signer" = "yes"])
15411535

1542-
dnl Do not compile with syscall sandbox support when compiling under the sanitizers.
1543-
dnl The sanitizers introduce use of syscalls that are not typically used in bitcoind
1544-
dnl (such as execve when the sanitizers execute llvm-symbolizer).
1545-
if test "$use_sanitizers" != ""; then
1546-
AC_MSG_WARN([Specifying --with-sanitizers forces --without-seccomp since the sanitizers introduce use of syscalls not allowed by the bitcoind syscall sandbox (-sandbox=<mode>).])
1547-
seccomp_found=no
1548-
fi
1549-
if test "$seccomp_found" != "no"; then
1550-
AC_MSG_CHECKING([for seccomp-bpf (Linux x86-64)])
1551-
AC_PREPROC_IFELSE([AC_LANG_PROGRAM([[
1552-
@%:@include <linux/seccomp.h>
1553-
]], [[
1554-
#if !defined(__x86_64__)
1555-
# error Syscall sandbox is an experimental feature currently available only under Linux x86-64.
1556-
#endif
1557-
]])],[
1558-
AC_MSG_RESULT([yes])
1559-
seccomp_found="yes"
1560-
AC_DEFINE([USE_SYSCALL_SANDBOX], [1], [Define this symbol to build with syscall sandbox support.])
1561-
],[
1562-
AC_MSG_RESULT([no])
1563-
seccomp_found="no"
1564-
])
1565-
fi
1566-
dnl Currently only enable -sandbox=<mode> feature if seccomp is found.
1567-
dnl In the future, sandboxing could be also be supported with other
1568-
dnl sandboxing mechanisms besides seccomp.
1569-
use_syscall_sandbox=$seccomp_found
1570-
AM_CONDITIONAL([ENABLE_SYSCALL_SANDBOX], [test "$use_syscall_sandbox" != "no"])
1571-
15721536
dnl Check for reduced exports
15731537
if test "$use_reduce_exports" = "yes"; then
15741538
AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [CORE_CXXFLAGS="$CORE_CXXFLAGS -fvisibility=hidden"],
@@ -2008,7 +1972,6 @@ echo
20081972
echo "Options used to compile and link:"
20091973
echo " external signer = $use_external_signer"
20101974
echo " multiprocess = $build_multiprocess"
2011-
echo " with experimental syscall sandbox support = $use_syscall_sandbox"
20121975
echo " with libs = $build_bitcoin_libs"
20131976
echo " with wallet = $enable_wallet"
20141977
if test "$enable_wallet" != "no"; then

src/Makefile.am

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ BITCOIN_CORE_H = \
313313
util/sock.h \
314314
util/spanparsing.h \
315315
util/string.h \
316-
util/syscall_sandbox.h \
317316
util/syserror.h \
318317
util/thread.h \
319318
util/threadinterrupt.h \
@@ -741,7 +740,6 @@ libbitcoin_util_a_SOURCES = \
741740
util/spanparsing.cpp \
742741
util/strencodings.cpp \
743742
util/string.cpp \
744-
util/syscall_sandbox.cpp \
745743
util/time.cpp \
746744
util/tokenpipe.cpp \
747745
$(BITCOIN_CORE_H)
@@ -976,7 +974,6 @@ libbitcoinkernel_la_SOURCES = \
976974
util/serfloat.cpp \
977975
util/strencodings.cpp \
978976
util/string.cpp \
979-
util/syscall_sandbox.cpp \
980977
util/syserror.cpp \
981978
util/thread.cpp \
982979
util/threadnames.cpp \

src/bitcoind.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include <util/check.h>
2525
#include <util/exception.h>
2626
#include <util/strencodings.h>
27-
#include <util/syscall_sandbox.h>
2827
#include <util/syserror.h>
2928
#include <util/threadnames.h>
3029
#include <util/tokenpipe.h>
@@ -242,7 +241,6 @@ static bool AppInit(NodeContext& node)
242241
daemon_ep.Close();
243242
}
244243
#endif
245-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::SHUTOFF);
246244
return fRet;
247245
}
248246

src/checkqueue.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include <sync.h>
99
#include <tinyformat.h>
10-
#include <util/syscall_sandbox.h>
1110
#include <util/threadnames.h>
1211

1312
#include <algorithm>
@@ -149,7 +148,6 @@ class CCheckQueue
149148
for (int n = 0; n < threads_num; ++n) {
150149
m_worker_threads.emplace_back([this, n]() {
151150
util::ThreadRename(strprintf("scriptch.%i", n));
152-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::VALIDATION_SCRIPT_CHECK);
153151
Loop(false /* worker thread */);
154152
});
155153
}

src/httpserver.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include <shutdown.h>
1919
#include <sync.h>
2020
#include <util/strencodings.h>
21-
#include <util/syscall_sandbox.h>
2221
#include <util/threadnames.h>
2322
#include <util/translation.h>
2423

@@ -297,7 +296,6 @@ static void http_reject_request_cb(struct evhttp_request* req, void*)
297296
static void ThreadHTTP(struct event_base* base)
298297
{
299298
util::ThreadRename("http");
300-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET_HTTP_SERVER);
301299
LogPrint(BCLog::HTTP, "Entering http event loop\n");
302300
event_base_dispatch(base);
303301
// Event loop will be interrupted by InterruptHTTPServer()
@@ -350,7 +348,6 @@ static bool HTTPBindAddresses(struct evhttp* http)
350348
static void HTTPWorkQueueRun(WorkQueue<HTTPClosure>* queue, int worker_num)
351349
{
352350
util::ThreadRename(strprintf("httpworker.%i", worker_num));
353-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::NET_HTTP_SERVER_WORKER);
354351
queue->Run();
355352
}
356353

src/index/base.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#include <node/interface_ui.h>
1515
#include <shutdown.h>
1616
#include <tinyformat.h>
17-
#include <util/syscall_sandbox.h>
1817
#include <util/thread.h>
1918
#include <util/translation.h>
2019
#include <validation.h> // For g_chainman
@@ -167,7 +166,6 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain&
167166

168167
void BaseIndex::ThreadSync()
169168
{
170-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::TX_INDEX);
171169
// Wait for a possible reindex-chainstate to finish until continuing
172170
// with the index sync
173171
while (!g_indexes_ready_to_sync) {

src/init.cpp

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@
7979
#include <util/moneystr.h>
8080
#include <util/strencodings.h>
8181
#include <util/string.h>
82-
#include <util/syscall_sandbox.h>
8382
#include <util/syserror.h>
8483
#include <util/thread.h>
8584
#include <util/threadnames.h>
@@ -627,10 +626,6 @@ void SetupServerArgs(ArgsManager& argsman)
627626
hidden_args.emplace_back("-daemonwait");
628627
#endif
629628

630-
#if defined(USE_SYSCALL_SANDBOX)
631-
argsman.AddArg("-sandbox=<mode>", "Use the experimental syscall sandbox in the specified mode (-sandbox=log-and-abort or -sandbox=abort). Allow only expected syscalls to be used by bitcoind. Note that this is an experimental new feature that may cause bitcoind to exit or crash unexpectedly: use with caution. In the \"log-and-abort\" mode the invocation of an unexpected syscall results in a debug handler being invoked which will log the incident and terminate the program (without executing the unexpected syscall). In the \"abort\" mode the invocation of an unexpected syscall results in the entire process being killed immediately by the kernel without executing the unexpected syscall.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
632-
#endif // USE_SYSCALL_SANDBOX
633-
634629
// Add the hidden options
635630
argsman.AddHiddenArgs(hidden_args);
636631
}
@@ -841,7 +836,7 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic<int>& exit_status)
841836
return true;
842837
}
843838

844-
bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandbox)
839+
bool AppInitParameterInteraction(const ArgsManager& args)
845840
{
846841
const CChainParams& chainparams = Params();
847842
// ********************************************************* Step 2: parameter interactions
@@ -986,40 +981,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
986981
if (args.GetIntArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1)
987982
return InitError(Untranslated("Unknown rpcserialversion requested."));
988983

989-
#if defined(USE_SYSCALL_SANDBOX)
990-
if (args.IsArgSet("-sandbox") && !args.IsArgNegated("-sandbox")) {
991-
const std::string sandbox_arg{args.GetArg("-sandbox", "")};
992-
bool log_syscall_violation_before_terminating{false};
993-
if (sandbox_arg == "log-and-abort") {
994-
log_syscall_violation_before_terminating = true;
995-
} else if (sandbox_arg == "abort") {
996-
// log_syscall_violation_before_terminating is false by default.
997-
} else {
998-
return InitError(Untranslated("Unknown syscall sandbox mode (-sandbox=<mode>). Available modes are \"log-and-abort\" and \"abort\"."));
999-
}
1000-
// execve(...) is not allowed by the syscall sandbox.
1001-
const std::vector<std::string> features_using_execve{
1002-
"-alertnotify",
1003-
"-blocknotify",
1004-
"-signer",
1005-
"-startupnotify",
1006-
"-walletnotify",
1007-
};
1008-
for (const std::string& feature_using_execve : features_using_execve) {
1009-
if (!args.GetArg(feature_using_execve, "").empty()) {
1010-
return InitError(Untranslated(strprintf("The experimental syscall sandbox feature (-sandbox=<mode>) is incompatible with %s (which uses execve).", feature_using_execve)));
1011-
}
1012-
}
1013-
if (!SetupSyscallSandbox(log_syscall_violation_before_terminating)) {
1014-
return InitError(Untranslated("Installation of the syscall sandbox failed."));
1015-
}
1016-
if (use_syscall_sandbox) {
1017-
SetSyscallSandboxPolicy(SyscallSandboxPolicy::INITIALIZATION);
1018-
}
1019-
LogPrintf("Experimental syscall sandbox enabled (-sandbox=%s): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.\n", sandbox_arg);
1020-
}
1021-
#endif // USE_SYSCALL_SANDBOX
1022-
1023984
// Also report errors from parsing before daemonization
1024985
{
1025986
KernelNotifications notifications{};

src/init.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic<int>& exit_status);
4444
* @note This can be done before daemonization. Do not call Shutdown() if this function fails.
4545
* @pre Parameters should be parsed and config file should be read, AppInitBasicSetup should have been called.
4646
*/
47-
bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandbox = true);
47+
bool AppInitParameterInteraction(const ArgsManager& args);
4848
/**
4949
* Initialization sanity checks.
5050
* @note This can be done before daemonization. Do not call Shutdown() if this function fails.

0 commit comments

Comments
 (0)