34a18f92ee netns-isolation: fix netns-exec being executable by all normal users (Jonas Nick)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 3aa359bb61967b76299918b893b85f5d92909029237cba892b367f94c82643268dceaf2928157b47d4973a8af81e5e9ef29651bc45db0e3b149966b16c41bc8b
The `netns-exec` wrapper was created with `group = ""`, which makes the
`security.wrappers` activation run `chown ${allowedUser}:`. GNU chown
resolves the trailing colon to the owner's login group, and because the
operator is defined with `isNormalUser = true`, that group is the shared
group `users`. Together with mode 550, this made the wrapper executable
by every normal user on the host instead of only by `allowedUser`.
The wrapper carries `cap_sys_admin=ep` and the NixOS security wrapper
performs no owner check, so any normal user could enter `nb-joinmarket`,
the only netns that `netns-exec` permits. This exposes joinmarketd's
unauthenticated control port on 127.0.0.1 inside that netns and allows
sending packets from the joinmarket netns address. This can only ever become a
problem if there's a second, normal user on the host.
Use mode 500 so that only `allowedUser` can execute the wrapper. Also
set the group to `root`, so that `users` doesn't silently become an
authorized group again if the mode is ever loosened.
The netns-isolation test asserted this property with
`runuser -u clightning -- netns-exec nb-bitcoind ip a`, which fails
regardless of the file mode, because `clightning` is a system user with
its own group and `nb-bitcoind` is not in netns-exec's allowlist.
Replace it with a check that a normal user is denied by the exec
permissions, and assert the reason for each failure instead of only the
exit status.
Let A be a service that depends on another service B.
When A can gracefully handle failures and restarts of B, use
```
wants = [ "B.service" ];
after = [ "B.service" ];
```
instead of
```
requires = [ "B.service" ];
after = [ "B.service" ];
```
in the definition of A.
This way, A keeps running when B is stopped or restarted after a failure.
With `requires`, A is instead stopped when B is stopped or restarted due to a failure.
This brings two benefits:
1. Improved uptime
Examples:
- RTL keeps running when one lightning node has failed
- btcpayserver keeps running and accepting on-chain payments when the lightning node has crashed
2. Avoids a systemd bug where depending units (`A.service` in the
above example) are not restarted when their dependency fails
(issue github/systemd#18856, no full link to avoid spamming the issue).
In real world nix-bitcoin deployments, this issue was only likely to
appear when clightning failed during activation, causing depending
units (like `RTL`) to stop and not be restarted.
All services depending on `clightning` have now been changed to use
`wants`, thereby avoiding the bug.
Services `electrs` and `lightning-loop` fail when their respective
dependencies stop, so these services have not been changed.
I also haven't changed services `joinmarket` and
`joinmarket-yieldgenerator`. Further manual testing is needed to
determine if they can be switched to `wants`.
For both lnd and clightning-rest, `lndconnectOnion` is replaced by
options `lndconnect.enable` and `lndconnect.onion`.
This allows using lndconnect without Tor.
This makes scripts fail early on request errors.
Previously, curl exited with status 0 when enountering HTTP error status
codes.
`-fsS` equals `--fail --silent --show-error`.
Option `services.lnd.restOnionService.package` has been removed.
There's not much use in overriding the [lndconnect pkg](https://github.com/LN-Zap/lndconnect).
- Rename `services.rtl.cl-rest` to `services.clightning-rest`.
`clightning-rest` is generally useful for connecting external REST clients
to clightning.
- Add a dedicated network namespace in netns-isolation.
- Add nodeinfo entry.
- Add datadir (which contains REST auth data) to backups.
Previously, in case of btcpayserver.lbtc on regtest, nbxplorer just
printed a log error while trying to generate liquidd regtest blocks.
Now nbxplorer exits with an error, so disable lbtc.
This is simpler and more memory-efficient.
We've also changed other services to use this appraoch.
Also remove unneded `wait_for_unit` in the electrs regtest test.
- README: add matrix room
- examples/configuration.nix: explain why bitcoind is enabled by default
- btcpayserver: group lnd service settings
- clightning:
Use public onion port only when the onion service is public
This allows users to enable the onion service while announcing a
non-onion public address.
- netns-isolation: move `readOnly` attr to the top
- tests: use mkDefault to allow for easier overriding
- tests/btcpayserver: test web server response
- `waitfornewblock` was previously not included in the public RPC
whitelist because it's reserved for testing and marked as hidden
in bitcoind.
- electrs changed its verbosity settings. `-vv` is now the best choice
for normal usage.
- bitcoind option `dataDirReadableByGroup` is now unused.
Because it can be valuable for other use cases and implementing
it is intricate, we're keeping it for now.
- test: keep `nc` connection open because otherwise the electrs
RPC server would now close the connection before sending a response.
joinmarket-ob-watcher now makes extensive use of bitcoind because of
JoinMarket's new fidelity bond functionality. Therefore it fails on
non-synced nodes, as those in the test suite. We now test that the
service fails with the correct error, rather than asserting that it is
running.
NixOS option `security.hideProcessInformation` for globally restricting
access to /proc has been removed.
Use per-service restrictions via 'ProtectProc' instead.
Rename
`nix-bitcoin.security.hideProcessInformation` to
`nix-bitcoin.security.dbusHideProcessInformation`
because this option now only implements the dbus restriction.
Jonas Nick