df184b6e06 lnd: don't pass the admin macaroon in `curl` argv (Jonas Nick)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: d18fa7176c59f33a38222a2d3d44320769ac83e091104594a9572e44ce38205d341767ea598da1b5500208c1e9f6645ed5c3b9390438d95912e44d1c5f3a4752
f51460749a ci: disable the nixos-search test (Jonas Nick)
3b798f5ebf test: increase VM memory to 4 GiB (Jonas Nick)
17f0d98c22 update nixpkgs (Jonas Nick)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 0e40353d9d7937dd7a80c05416360c30ada7ee231d6263883072d60a707443e765d35f6a393e718ed1ef66f2e2cdbd1deadd6cd720dccb5c5e18711159cda38a
`lnd-create-macaroons` interpolated the hex-encoded admin macaroon into
`curl`'s argv, where any local user could read it from world-readable
`/proc/<pid>/cmdline`. The unit's `ProtectProc=invisible` doesn't apply,
because `nbLib.rootScript`'s `+` prefix disables sandboxing for the
process. Pass the header on a file descriptor instead, so the credential
never enters any argv (`printf` is a bash builtin); the request sent to
lnd is unchanged. Only configurations with a non-empty
`services.lnd.macaroons` were affected, which `services.charge-lnd` and
`services.btcpayserver` with `lightningBackend = "lnd"` set automatically.
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.
bitcoin: 30.2 -> 31.0
bitcoind: 30.2 -> 31.0
clboss: 0.15.1 -> 0.16.0
clightning: 25.12.1 -> 26.04.1
elementsd: 23.3.2 -> 23.3.3
lightning-loop: 0.31.5-beta -> 0.33.0-beta
lightning-loop 0.31.7+ blocks until lnd's chain notifier is ready
(lightninglabs/loop@bde49d7a), so the workaround sleep added in the
previous commit is no longer needed for lightning-loop. The
lightning-pool workaround stays — pool has no equivalent fix and
isn't actively maintained.
Note: nbxplorer test fails (presumed incompatibility with bitcoin 31).
bitcoin: 30.0 -> 30.2
bitcoind: 30.0 -> 30.2
bitcoind-knots: 29.2.knots20251110 -> 29.3.knots20260210
btcpayserver: 2.2.1 -> 2.3.4
clightning: 25.09.2 -> 25.12.1
elementsd: 23.2.4 -> 23.3.2
lnd: 0.19.3-beta -> 0.20.1-beta
lnd 0.20 makes its chain notifier subsystem ready later in startup
than its RPC server. lightning-loop and lightning-pool both subscribe
to chain notifications right after dialing lnd, which fails before
the notifier is up and crashes the daemon. Add a 5s ExecStartPre
sleep to both services as a temporary workaround.
This option is unneeded because `bitcoin-cli` auth and connection
settings are already provided by other options like `bitcoin-rpcpassword`.
In bitcoind versions prior to 29.0, option `datadir` (passed by clightning to `bitcoin-cli`)
was therefore ignored.
Since 29.0, option `datadir` is always checked for validity. This causes a
file permission error because service clighting has no access to the
bitcoin datadir. This is now fixed.
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`.
By wrapping the value in a list, users can add other `ExecStartPost` scripts.
The previous scalar value could only be replaced, but not merged with
other definitions.
These options are set by default in `jmclient/configure.py` and don't
need to be repeated.
Some options, like those in section `POLICY`, might be assigned more
suitable default values in future joinmarket releases, so don't override
them in nix-bitcoin.
Jonas Nick