Remove the syscall sandbox #27896

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:remove_syscall_sandbox changing 28 files +5 −1175

fanquake commented at 9:41 am on June 16, 2023: member

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.

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.

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.

32e2ffc393

fanquake added the label Brainstorming on Jun 16, 2023
fanquake added the label Linux/Unix on Jun 16, 2023

DrahtBot commented at 9:41 am on June 16, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	dergoegge, davidgumberg, achow101
Concept ACK	TheCharlatan, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27861 (kernel: Add interrupt class by TheCharlatan)
#27854 ([WIP] add a stratum v2 template provider by ccdle12)
#27607 (index: make startup more efficient by furszy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

TheCharlatan commented at 11:28 am on June 16, 2023: contributor

Concept ACK
dergoegge commented at 12:14 pm on June 16, 2023: member

Concept ACK 🟥🟥🟥
hebasto commented at 11:10 am on June 24, 2023: member

Concept ACK.
dergoegge approved
dergoegge commented at 8:43 am on June 26, 2023: member

ACK 32e2ffc39374f61bb2435da507f285459985df9e
maflcko added the label DrahtBot Guix build requested on Jun 26, 2023
davidgumberg commented at 6:21 pm on June 26, 2023: contributor

crACK https://github.com/bitcoin/bitcoin/pull/27896/commits/32e2ffc39374f61bb2435da507f285459985df9e

Nit: We may want to remove BCLog::Util as well, since SetSyscallSandboxPolicy was its only user.

DrahtBot commented at 10:45 pm on June 26, 2023: contributor

Guix builds

File	commit 296735f7638749906243c9e203df7bd024493806(master)	commit 91706d1b5d3d7702ff3cadf8f0ae980604300ad2(master and this pull)
SHA256SUMS.part	`c65f335e7527365f...`	`47a237496379779c...`
*-aarch64-linux-gnu-debug.tar.gz	`5a0fe9c9d1efc225...`	`97fe9e1014216595...`
*-aarch64-linux-gnu.tar.gz	`05170c9b821d3d15...`	`1e8b8c30ec0c799c...`
*-arm-linux-gnueabihf-debug.tar.gz	`f7e39f68b22907e5...`	`634a0a685ce46367...`
*-arm-linux-gnueabihf.tar.gz	`e674a6f14867b5d4...`	`34dffe5b68e10ba4...`
*-arm64-apple-darwin-unsigned.dmg	`8380f18bd9bf08a5...`	`0592cad94c0701b8...`
*-arm64-apple-darwin-unsigned.tar.gz	`f4e10c4baf370d3c...`	`24470e1acdaf64e8...`
*-arm64-apple-darwin.tar.gz	`10683ff96986df42...`	`228517d5a4ec2cdc...`
*-powerpc64-linux-gnu-debug.tar.gz	`62ad0ba588090ff1...`	`14597f966dcf36d7...`
*-powerpc64-linux-gnu.tar.gz	`972edd6915d01cf1...`	`6848ed76574cea79...`
*-powerpc64le-linux-gnu-debug.tar.gz	`d49ee3a837144285...`	`ccdae1099b66398c...`
*-powerpc64le-linux-gnu.tar.gz	`189b13c7a4df3f82...`	`d374f8da1798a525...`
*-riscv64-linux-gnu-debug.tar.gz	`767845df189ec4e8...`	`c7ff5e0f08a7790f...`
*-riscv64-linux-gnu.tar.gz	`3a49a981a56cbc6b...`	`74e5aec8ef960691...`
*-x86_64-apple-darwin-unsigned.dmg	`946d54e9a8754e18...`	`696a73f6cf4b1908...`
*-x86_64-apple-darwin-unsigned.tar.gz	`9ee286d80a9134b4...`	`178caa484bcc938d...`
*-x86_64-apple-darwin.tar.gz	`af85c63c8384691d...`	`693f164d6235f0ea...`
*-x86_64-linux-gnu-debug.tar.gz	`9eaab59941d2989c...`	`8fb8ad47f07bc2cd...`
*-x86_64-linux-gnu.tar.gz	`448ab0cd5f64b0ba...`	`59a62838ee5c3181...`
*.tar.gz	`e55d3332038ad8e6...`	`a805c237d32cb819...`
guix_build.log	`d94426f4f609f4f9...`	`fd5013e8161d2c1c...`
guix_build.log.diff		`18742188c7b716ba...`

DrahtBot removed the label DrahtBot Guix build requested on Jun 26, 2023
achow101 commented at 9:48 pm on June 27, 2023: member

ACK 32e2ffc39374f61bb2435da507f285459985df9e

The syscall sandbox has a rather significant maintenance burden for rather limited benefit.
achow101 merged this on Jun 27, 2023
achow101 closed this on Jun 27, 2023
fanquake deleted the branch on Jun 28, 2023
sidhujag referenced this in commit 678a3d4350 on Jun 30, 2023
fanquake referenced this in commit b83d368eab on Nov 7, 2023
DavidKorczynski referenced this in commit 349a5d4793 on Nov 7, 2023
bitcoin locked this on Jun 27, 2024

Contributors
fanquake DrahtBot TheCharlatan dergoegge hebasto davidgumberg achow101

Labels
Brainstorming Linux/Unix