Commit graph

17 commits

Author SHA1 Message Date
W. J. van der Laan
1884ce2f4c
Merge bitcoin/bitcoin#22937: refactor: Forbid calling unsafe fs::path(std::string) constructor and fs::path::string() method
6544ea5035 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec6 refactor: Add fs::PathToString, fs::PathFromString, u8string, u8path functions (Russell Yanofsky)

Pull request description:

  The `fs::path` class has a `std::string` constructor which will implicitly convert from strings. Implicit conversions like this are not great in general because they can hide complexity and inefficiencies in the code, but this case is especially bad, because after the transition from `boost::filesystem` to `std::filesystem` in #20744 the behavior of this constructor on windows will be more complicated and can mangle path strings. The `fs::path` class also has a `.string()` method which is inverse of the constructor and has the same problems.

  Fix this by replacing the unsafe method calls with `PathToString` and `PathFromString` function calls, and by forbidding unsafe method calls in the future.

ACKs for top commit:
  kiminuo:
    ACK 6544ea5035
  laanwj:
    Code review ACK 6544ea5035
  hebasto:
    re-ACK 6544ea5035, only added `fsbridge_stem` test case, updated comment, and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22937#pullrequestreview-765503126) review. Verified with the following command:

Tree-SHA512: c36324740eb4ee55151146626166c00d5ccc4b6f3df777e75c112bcb4d1db436c1d9cc8c29a1e7fb96051457d317961ab42e6c380c3be2771d135771b2b49fa0
2021-10-15 10:01:56 +02:00
Anthony Towns
31b2b802b5 blockstorage: use debug log category 2021-10-11 21:45:49 +10:00
Russell Yanofsky
6544ea5035 refactor: Block unsafe fs::path std::string conversion calls
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding calls
to methods which will be unsafe after the transaction to std::filesystem
to due lack of a boost::filesystem::path::imbue equivalent and inability
to set a predictable locale.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2021-10-05 11:10:47 -04:00
W. J. van der Laan
9e530c6352
Merge bitcoin/bitcoin#20487: Add syscall sandboxing using seccomp-bpf (Linux secure computing mode)
4747da3a5b Add syscall sandboxing (seccomp-bpf) (practicalswift)

Pull request description:

  Add experimental syscall sandboxing using seccomp-bpf (Linux secure computing mode).

  Enable filtering of system calls using seccomp-bpf: allow only explicitly allowlisted (expected) syscalls to be called.

  The syscall sandboxing implemented in this PR is an experimental feature currently available only under Linux x86-64.

  To enable the experimental syscall sandbox the `-sandbox=<mode>` option must be passed to `bitcoind`:

  ```
    -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.
  ```

  The allowed syscalls are defined on a per thread basis.

  I've used this feature since summer 2020 and I find it to be a helpful testing/debugging addition which makes it much easier to reason about the actual capabilities required of each type of thread in Bitcoin Core.

  ---

  Quick start guide:

  ```
  $ ./configure
  $ src/bitcoind -regtest -debug=util -sandbox=log-and-abort
  …
  2021-06-09T12:34:56Z Experimental syscall sandbox enabled (-sandbox=log-and-abort): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.
  …
  2021-06-09T12:34:56Z Syscall filter installed for thread "addcon"
  2021-06-09T12:34:56Z Syscall filter installed for thread "dnsseed"
  2021-06-09T12:34:56Z Syscall filter installed for thread "net"
  2021-06-09T12:34:56Z Syscall filter installed for thread "msghand"
  2021-06-09T12:34:56Z Syscall filter installed for thread "opencon"
  2021-06-09T12:34:56Z Syscall filter installed for thread "init"
  …
  # A simulated execve call to show the sandbox in action:
  2021-06-09T12:34:56Z ERROR: The syscall "execve" (syscall number 59) is not allowed by the syscall sandbox in thread "msghand". Please report.
  …
  Aborted (core dumped)
  $
  ```

  ---

  [About seccomp and seccomp-bpf](https://en.wikipedia.org/wiki/Seccomp):

  > In computer security, seccomp (short for secure computing mode) is a facility in the Linux kernel. seccomp allows a process to make a one-way transition into a "secure" state where it cannot make any system calls except exit(), sigreturn(), and read() and write() to already-open file descriptors. Should it attempt any other system calls, the kernel will terminate the process with SIGKILL or SIGSYS. In this sense, it does not virtualize the system's resources but isolates the process from them entirely.
  >
  > […]
  >
  > seccomp-bpf is an extension to seccomp that allows filtering of system calls using a configurable policy implemented using Berkeley Packet Filter rules. It is used by OpenSSH and vsftpd as well as the Google Chrome/Chromium web browsers on Chrome OS and Linux. (In this regard seccomp-bpf achieves similar functionality, but with more flexibility and higher performance, to the older systrace—which seems to be no longer supported for Linux.)

ACKs for top commit:
  laanwj:
    Code review and lightly tested ACK 4747da3a5b

Tree-SHA512: e1c28e323eb4409a46157b7cc0fc29a057ba58d1ee2de268962e2ade28ebd4421b5c2536c64a3af6e9bd3f54016600fec88d016adb49864b63edea51ad838e17
2021-10-04 22:45:43 +02:00
practicalswift
4747da3a5b Add syscall sandboxing (seccomp-bpf) 2021-10-01 13:51:10 +00:00
practicalswift
4343f114cc Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17)
test: Add test cases for LocaleIndependentAtoi

fuzz: Assert legacy atoi(s) == LocaleIndependentAtoi<int>(s)

fuzz: Assert legacy atoi64(s) == LocaleIndependentAtoi<int64_t>(s)
2021-09-30 14:21:17 +00:00
Jon Atack
350e034e64
consensus: don't call GetBlockPos in ReadBlockFromDisk without lock 2021-09-05 17:55:06 +02:00
MarcoFalke
faa54e3757
Move pblocktree global to BlockManager 2021-07-15 13:54:09 +02:00
MarcoFalke
fa0d9211ef
refactor: Remove chainparams arg from CChainState member functions
Passing this is confusing and redundant with the m_params member.
2021-06-13 09:43:54 +02:00
Carl Dong
6c3b5dc0c1 scripted-diff: tree-wide: Remove all review-only assertions
-BEGIN VERIFY SCRIPT-
find_regex='((assert|CHECK_NONFATAL)\(std::addressof|TODO: REVIEW-ONLY)' \
    && git grep -l -E "$find_regex" -- . \
        | xargs sed -i -E "/${find_regex}/d"
-END VERIFY SCRIPT-
2021-06-10 15:05:24 -04:00
MarcoFalke
fa09a9eac8
style: Add { } to multi-line if
Can be reviewed with --word-diff-regex=. --ignore-all-space
2021-04-27 10:36:41 +02:00
MarcoFalke
fadafab833
move-only: Move functions to blockstorage 2021-04-27 10:36:23 +02:00
MarcoFalke
fa247a327f
refactor: Move block storage globals to blockstorage
However, keep a declaration in validation to make it possible to move
smaller chunks to blockstorage without breaking compilation.

Also, expose AbortNode in the header.

Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-04-27 10:32:30 +02:00
MarcoFalke
fa81c30c6f
refactor: Move pruning/reindex/importing globals to blockstorage
Can be reviewed with --color-moved=dimmed-zebra
2021-04-27 10:32:24 +02:00
MarcoFalke
fa121b628d
blockstorage: [refactor] Use chainman reference where possible
Also, add missing { } for style.

Can be reviewed with `--word-diff-regex=.`
2021-04-05 20:26:32 +02:00
MarcoFalke
fa0c7d9ad2
move-only: Move *Disk functions to blockstorage
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-04-05 20:26:14 +02:00
MarcoFalke
fa413f07a1
move-only: Move ThreadImport to blockstorage
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-04-04 18:07:24 +02:00