Outcome of the syscall sandbox experiment

laanwj commented at 10:31 am on April 5, 2022: member

I think after having it integrated for a whlie (see #20487) it’s time to reflect on the status of the experimental syscall sandbox.

It was worth a try, but personally I have come to the conclusion that it’s unmaintainable, I don’t think this is something we can ever enable by default for end users.

In practice it’s haphazard whack-a-mole any time a syscall comes up and a crash happens. There’s no structure or plan to it, nor an end in sight. See e.g #24659 #24369 #24340 #23555 #23299 #23255 #23208 #23179.

The whole idea of a syscall sandbox is fundamentally a layer violation. It’s untenable when dynamic linking is involved, or dependencies we don’t know A to Z, with evolving user-kernel space APIs. See also discussion here: #24758 (comment)

I’m also not sure it addresses relevant risks in our case. The P2P thread communicates with the outside world and the scope of an exploit to gain full RCE can ostensibly be restricted by sandboxing it. However there are bigger risks within the bitcoind/-qt process itself: messing with consensus code, stealing wallet keys. A thread-based sandbox provides only minimal protection here.

It’s also single platform (Linux x86_64), and all the work would basically be duplicated for each platform added as the syscalls differ subtly.

laanwj added the label Brainstorming on Apr 5, 2022

laanwj added the label Linux/Unix on Apr 5, 2022

laanwj commented at 10:42 am on April 5, 2022: member

When releasing fully statically built binaries (e.g. #18110), the set of syscalls would be contained for that specific build. This would make the problem at least theoretically manageable.

Still I’m not sure bitcoin core should be in the business of micro-managing syscalls. It’s scope creep to Linux kernel internals.

ryanofsky commented at 12:39 pm on April 5, 2022: contributor

It seems to me problem is that practicalswift was made syscall list too restrictive, whitelisting specific calls that threads have made in the past, instead of whitelisting complete groups of calls. For example whitelisting fchown but not whitelisting chown fchownat or lchown.

If we whitelisted whole groups of syscalls like firejail (https://github.com/netblue30/firejail/blob/master/etc/templates/syscalls.txt) instead of individual syscalls, it seems like it would solve this problem. It looks like most or all of the linked issues would have been solved using firejail’s group definitions.

The whole idea of a syscall sandbox is fundamentally a layer violation. It’s untenable when dynamic linking is involved, or dependencies we don’t know A to Z, with evolving user-kernel space APIs.

I think it is pretty common for security features to be layer violations, canonical example being network firewalls. And I think it should be pretty easy to sandbox normal libraries we use like zmq or leveldb. But Qt is a special case and a massive library that can’t be reasonably sandboxed by aborting every time an unexpected syscall is made. A more reasonable approach to sandboxing Qt would be to return -1 for disallowed syscalls and log them, instead of aborting the application. It would also be reasonable to just not restrict what syscalls the GUI thread can make and rely on higher level sandboxing (bubblewrap, etc)

laanwj commented at 12:45 pm on April 5, 2022: member

Right. It would be fine to leave it up to the OS, or distribution, firejail, bubblewrap, apparmor, etc. What I’m less convinced of is having this in bitcoin core.

And I think it should be pretty easy to sandbox normal libraries we use like zmq or leveldb.

Maybe on some OSes. Linux has a huge number of system calls, though, and it’s completely unclear to me what is used when. It also varies over glibc versions, with some existing only for compatibility, for example.

If we whitelisted whole groups of syscalls like firejail (https://github.com/netblue30/firejail/blob/master/etc/templates/syscalls.txt) instead of individual syscalls

Sure, that’s the idea, the problem is that it’s a moving target.

Anyway, if people want to keep maintaining this as an experimental feature that’s fine with me. But I’d really not like to enable it by default, it would create many more issues than it would solve, I definitely don’t want to handle crash issues or network DoS issues created by someone forgetting to add the zillionth syscall.

nnmfnwl7 commented at 12:46 pm on April 20, 2022: none

Maybe little bit out of topic but related, and hopefully someone would be interested, also maybe usable later:

I was also many times thinking about not only bitcoin wallet sandboxing. For example: having multiple D/QT wallets running and if one is malicious/has-bug and process has access to user files, it could lead not only to loosing all wallet dat, also revealing some more sensitive data. As I was more and more interested in using direct wallet-to-wallet atomic-swap trading/exchange platforms, so securing those wallets was crucial, not only sandboxing wallets from each other also protecting users privacy. So im trying to create scripts that securely(sandbox)/privately(tor) way would be able to build wallets, manage wallets, run wallets, also create and manage atomic-swap based liquidity pools. Still working on finalization/tests to upload also source. For now only readme/overview and two pdfs presentations. https://github.com/nnmfnwl7/cc.setup.helper.debian

fanquake referenced this in commit 07a463d37d on May 31, 2023

fanquake referenced this in commit df1a886875 on Jun 14, 2023

fanquake referenced this in commit 29cd833d26 on Jun 16, 2023

fanquake referenced this in commit b1e8c6b4a9 on Jun 16, 2023

fanquake referenced this in commit 32e2ffc393 on Jun 16, 2023

achow101 closed this on Jun 27, 2023

achow101 referenced this in commit caff95a023 on Jun 27, 2023

bitcoin locked this on Jun 26, 2024

Outcome of the syscall sandbox experiment #24771