diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 00000000..9817eaa7 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,9 @@ +# The Docker build context is the repository root (docker/compose.yml sets +# `context: ..`) and both Dockerfiles pull it into the builder stage with +# `COPY . .`. `make docker-build` stages the LND cert and admin macaroon in +# docker/config/lnd/ right before `docker compose build`, so without this entry +# a spend-capable credential would be shipped into the build context and left +# in the builder layer and its cache. .gitignore already keeps it out of git, +# but Docker does not read .gitignore. +docker/config/ +docker/.env diff --git a/AGENTS.md b/AGENTS.md index 3ccd7e3c..19823e07 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,6 @@ - Add a language specifier to every fenced code block. Static analysis (markdownlint MD040) flags blocks without a language identifier. Example: ` ```flutter test ` instead of bare ` ``` `. ## Security & Configuration Tips -- Do not commit populated `settings.toml`. Copy from `settings.tpl.toml` to `~/.mostro/settings.toml` for local runs. -- Protect LND credentials before `make docker-build`. +- Do not commit populated `settings.toml`. Install it from `settings.tpl.toml` with `install -d -m 700 ~/.mostro && install -m 600 settings.tpl.toml ~/.mostro/settings.toml` for local runs: the file carries `nsec_privkey` and the directory also holds `mostro.db`. +- Protect LND credentials before `make docker-build`. The admin macaroon is spend-capable: copy it with `install -m 600` (never plain `cp`, which inherits the source or destination mode) and keep its directory at `0700`. - Scrub logs that might leak invoices or Nostr keys; rotate secrets promptly if exposed. diff --git a/INSTALL.md b/INSTALL.md index b63748b7..a56d8c96 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -92,10 +92,11 @@ adduser --disabled-login mostro # keep pressing enter until it ends cd /opt/mostro ``` -Create a new settings file from `/opt/mostro/mostro/settings.tpl.toml` and save it to `/opt/mostro`: +Create a new settings file from `/opt/mostro/mostro/settings.tpl.toml` and save it to `/opt/mostro`. `/opt/mostro` holds `settings.toml`, whose `nsec_privkey` is the daemon's identity, and the `mostro.db` created later, so both the directory and the file are restricted to the service account — `install -m` rather than `cp`, which keeps whatever mode the template happens to have: ```bash -cp /opt/mostro/mostro/settings.tpl.toml /opt/mostro/settings.toml +install -d -m 700 -o mostro -g mostro /opt/mostro +install -m 600 -o mostro -g mostro /opt/mostro/mostro/settings.tpl.toml /opt/mostro/settings.toml ``` Update the file `/opt/mostro/settings.toml` with your favourite editor. @@ -108,30 +109,55 @@ Here some parameters you might want to change: - **nsec_privkey** : Your mostro private key - **relays** : List of relays you want to connect to +### Protect the admin macaroon + +The admin macaroon is a spend-capable credential: any user who can read it has full control of the LND node, including the funds escrowed in Mostro's hold invoices. Access should reach no further than the `mostro` service account created above and the LND account the node already runs as. + +If LND runs on this same VPS, grant access through the node's group instead of loosening the file (LND creates `admin.macaroon` with mode `0640`): + +```bash +usermod -aG lnd mostro +``` + +If you copy the macaroon into `/opt/mostro` instead, install it owner-readable only and hand it to the service account — do not use plain `cp`, which keeps whatever mode the source file or an existing destination happens to have: + +```bash +install -d -m 700 -o mostro -g mostro /opt/mostro/lnd +install -m 600 -o mostro -g mostro /path/to/lnd/admin.macaroon /opt/mostro/lnd/admin.macaroon +``` + +Then point `lnd_macaroon_file` at `/opt/mostro/lnd/admin.macaroon`. + ## Database The data is saved in a sqlite db file named by default `mostro.db`, this file is saved on the root directory of the project and can be change just editing the `url` var on the `[database]` section in `settings.toml` file. -Before start building you can initialize the database manually with `sqlx-cli` (optional — `mostrod` creates the file and runs migrations on first connect): +Before start building you can initialize the database manually with `sqlx-cli` (optional — `mostrod` creates the file and runs migrations on first connect). + +These commands run as root, so the database ends up owned by root. Hand it to the service account right away: SQLite writes the `-shm`/`-wal` sidecars next to the database, so `mostrod` needs to own the files *and* be able to create new ones in the directory. ```bash cargo install sqlx-cli --version 0.9.0 --no-default-features --features sqlite +cd /opt/mostro export DATABASE_URL=sqlite://mostro.db sqlx database create sqlx migrate run +chown mostro:mostro /opt/mostro/mostro.db* ``` -Check the DB files are there +Check the DB files are there, and that they belong to `mostro`: ```bash ls -al /opt/mostro -drwxrwxr-x root root 4.0 KB Fri Jun 14 15:52:07 2024 . -drwxr-x--- root root 4.0 KB Sat Jun 15 15:50:32 2024 .. -.rw-r--r-- root root 52 KB Fri May 31 16:35:34 2024 mostro.db -.rw-r--r-- root root 32 KB Sat Jun 15 15:28:23 2024 mostro.db-shm -.rw-r--r-- root root 16 KB Fri Jun 14 15:57:24 2024 mostro.db-wal +drwx------ mostro mostro 4.0 KB Fri Jun 14 15:52:07 2024 . +drwxr-x--- root root 4.0 KB Sat Jun 15 15:50:32 2024 .. +.rw-r--r-- mostro mostro 52 KB Fri May 31 16:35:34 2024 mostro.db +.rw-r--r-- mostro mostro 32 KB Sat Jun 15 15:28:23 2024 mostro.db-shm +.rw-r--r-- mostro mostro 16 KB Fri Jun 14 15:57:24 2024 mostro.db-wal ``` +If you skip this step, `mostrod` creates the database itself on first connect — as root during the foreground test below. Either way, the `chown -R mostro:mostro /opt/mostro` further down is the backstop that puts the ownership right before the service starts. + ## Clean compilation artifacts Since the instance you are using has little disk space you don't want to waste valuable disk space. Once successfully compiled the compilation artifacts can use up to **2Gb** of space. diff --git a/Makefile b/Makefile index 348bfa2f..1defb378 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,47 @@ SHELL := $(shell which bash) VERSION := $(shell grep "^version = " Cargo.toml | sed "s/version = \"\(.*\)\"/\1/") +# Notes on the docker/config handling below, since none of it is obvious: +# +# - `install -d -m 700` applies the mode to an existing directory too, so it is +# guarded with `[ -d config ]` everywhere. Only the run that first creates +# the directory gets to decide its mode; an operator who deliberately opened +# `config` up to a group keeps it across every later `make docker-build` or +# `make docker-up`. `config/lnd` is unguarded because docker-build owns it: +# it holds nothing but the credentials this target installs. +# - `install -m 600` onto an existing 0644 macaroon is safe as it stands: it +# unlinks the destination and creates it with the owner-only bits already +# applied, rather than truncating in place and chmod'ing at the end. Measured +# by polling the mode throughout a 300 MB copy — only 0600 is ever observed. +# - The mostro container has to run as whoever owns `config`: the macaroon +# there is 0600 and the daemon also writes mostro.db beside it. Deriving the +# uid:gid from the directory covers both the default (docker-build created it +# as you) and the documented `chown -R 1000:1000` handover. uid 0 is refused +# rather than used: a root-owned `config` (a `sudo make docker-build`, say) +# would otherwise drop the unprivileged user the image runs as. +# - Which is also why docker-build derives `-o/-g` from `config` when it runs +# as root over a directory that is not. `install` unlinks the destination and +# recreates it as the invoking user, so `sudo make docker-build` over a +# `config` handed to 1000 would leave a `root:root 0600` macaroon that +# docker-up then starts a uid 1000 container against — mostrod failing at the +# LND connection with nothing pointing at ownership. A `config` that is +# itself root-owned is left alone: docker-up refuses that case outright. +# - That refusal matches both spellings compose takes for uid 0, the numeric 0 +# and the name `root`, since MOSTRO_CONTAINER_USER is passed through to the +# `user:` key verbatim. Any other name is resolved inside the image, where +# mostrouser is the only account besides root. + docker-build: @set -o pipefail; \ cd docker && \ - mkdir -p config/lnd && \ + { [ -d config ] || install -d -m 700 config; } && \ + config_owner="$$(stat -c '%u:%g' config 2>/dev/null || stat -f '%u:%g' config)" && \ + install_owner="" && \ + if [ "$$(id -u)" = 0 ] && [ "$${config_owner%%:*}" != 0 ]; then \ + install_owner="-o $${config_owner%%:*} -g $${config_owner##*:}"; \ + echo "Running as root: installing config/lnd as $${config_owner}, the owner of config"; \ + fi && \ + install -d -m 700 $${install_owner} config/lnd && \ echo "Checking LND files..." && \ echo "LND_CERT_FILE=$${LND_CERT_FILE}" && \ echo "LND_MACAROON_FILE=$${LND_MACAROON_FILE}" && \ @@ -27,8 +64,15 @@ docker-build: exit 1; \ fi && \ echo "Copying LND cert and macaroon to docker config" && \ - cp -v $${LND_CERT_FILE} config/lnd/tls.cert && \ - cp -v $${LND_MACAROON_FILE} config/lnd/admin.macaroon && \ + install -m 644 $${install_owner} "$${LND_CERT_FILE}" config/lnd/tls.cert && \ + install -m 600 $${install_owner} "$${LND_MACAROON_FILE}" config/lnd/admin.macaroon && \ + echo "Wrote config/lnd/tls.cert (mode 644) and config/lnd/admin.macaroon (mode 600)" && \ + echo "config/lnd is mode 700, and config keeps the mode it was created with (700 unless you" && \ + echo "changed it): settings.toml holds nsec_privkey and mostro.db lands there too" && \ + echo "The mostro container runs as the owner of docker/config, which make docker-up derives" && \ + echo "and prints. Set MOSTRO_CONTAINER_USER=uid:gid to override it. A root-owned config" && \ + echo "directory is refused there; under sudo this target installs the credentials as the" && \ + echo "owner of config, so a directory already handed over stays readable by the container." && \ echo "Building docker image" && \ docker compose build @@ -36,8 +80,20 @@ docker-up: @set -o pipefail; \ cd docker && \ echo "Copying Nostr relay config" && \ + { [ -d config ] || install -d -m 700 config; } && \ mkdir -p config/relay && \ cp -v ./relay_config.toml config/relay/config.toml && \ + export MOSTRO_CONTAINER_USER="$${MOSTRO_CONTAINER_USER:-$$(stat -c '%u:%g' config 2>/dev/null || stat -f '%u:%g' config)}" && \ + case "$${MOSTRO_CONTAINER_USER%%:*}" in \ + 0|root) \ + echo "Error: refusing to run the mostro container as root." >&2; \ + echo "MOSTRO_CONTAINER_USER is $${MOSTRO_CONTAINER_USER}. It defaults to the owner of" >&2; \ + echo "docker/config, which a run under sudo leaves as root." >&2; \ + echo "Hand that directory to an unprivileged account (chown -R 1000:1000 config)," >&2; \ + echo "or export MOSTRO_CONTAINER_USER=uid:gid with a non-zero uid that can read it." >&2; \ + exit 1;; \ + esac && \ + echo "Running mostro as $${MOSTRO_CONTAINER_USER} (MOSTRO_CONTAINER_USER; defaults to the owner of docker/config)" && \ echo "Starting services" && \ docker compose up -d @@ -45,6 +101,7 @@ docker-relay-up: @set -o pipefail; \ cd docker && \ echo "Copying Nostr relay config" && \ + { [ -d config ] || install -d -m 700 config; } && \ mkdir -p config/relay && \ cp -v ./relay_config.toml config/relay/config.toml && \ echo "Starting Nostr relay" && \ diff --git a/README.md b/README.md index fb70f370..28c2ee11 100644 --- a/README.md +++ b/README.md @@ -443,9 +443,10 @@ cargo build --release # Install to system sudo install target/release/mostrod /usr/local/bin -# Setup configuration -mkdir -p ~/.mostro -cp settings.tpl.toml ~/.mostro/settings.toml +# Setup configuration (0700/0600: settings.toml holds nsec_privkey, and +# mostro.db lands in the same directory) +install -d -m 700 ~/.mostro +install -m 600 settings.tpl.toml ~/.mostro/settings.toml # Edit ~/.mostro/settings.toml (see Configuration section) # Initialize database (optional — mostrod also migrates on first connect) @@ -479,9 +480,10 @@ Best for: Local testing, development environments, quick experiments git clone https://github.com/MostroP2P/mostro.git cd mostro -# Setup configuration -mkdir -p docker/config -cp settings.tpl.toml docker/config/settings.toml +# Setup configuration (0700/0600: the config dir ends up holding nsec_privkey, +# the LND credentials and mostro.db) +install -d -m 700 docker/config +install -m 600 settings.tpl.toml docker/config/settings.toml # Edit docker/config/settings.toml # Build and run (provide LND paths as environment variables) @@ -495,6 +497,10 @@ This starts: - Mostro daemon (exposed via configured relays) - Local Nostr relay (port 7000 by default) +`make docker-build` installs the LND admin macaroon into `docker/config/lnd/` with mode `0600`, since it grants full control of your node, and sets `docker/config/lnd` to mode `0700` on every run. It sets `docker/config` to `0700` only when it has to create the directory — that one holds `settings.toml` and `mostro.db` as well, so a mode you chose for it deliberately is left as it is. + +`make docker-up` then runs the container as the owner of `docker/config`, so it can read those files whatever your uid is; `export MOSTRO_CONTAINER_USER=uid:gid` to override that. It refuses to start the container as root, which is what a root-owned `docker/config` would otherwise mean. Under `sudo`, `make docker-build` installs the credentials as the owner of `docker/config` rather than as root, so a directory already handed over to uid 1000 stays readable by the container across rebuilds. + **Stop**: `make docker-down` For detailed Docker setup, see [docker/README.md](docker/README.md). @@ -565,6 +571,8 @@ payment_retries_interval = 60 # seconds between retries **Required**: LND connection details. Mostro needs admin macaroon for hold invoice management. +**Permissions**: the admin macaroon is a spend-capable credential — anyone who can read it controls the node, including the funds escrowed in Mostro's hold invoices. Keep it readable only by the account running `mostrod`, or by that account and LND's group (`chmod 600` for a private copy, `chmod o=` to keep group access). mostrod logs a warning at startup when the file's `other` permission bits are set. That check runs in Lightning mode only, right before the LND connection is opened, so a node running Cashu escrow never reaches it. + --- #### Nostr Configuration @@ -590,6 +598,8 @@ rana --vanity mostro **Important**: Never reuse keys between Mostro instances. Each daemon needs a unique identity. +**Permissions**: `settings.toml` holds `nsec_privkey` in plaintext unless you move it to the environment (see below), and `mostro.db` sits in the same directory. Keep both owner-only: `chmod 700` on the settings directory and `chmod 600` on `settings.toml`. mostrod already creates them that way when it has to — both through the interactive setup wizard and through the template copy it makes on a non-interactive first run — but a directory or file you create yourself with `mkdir`, `cp` or `curl` inherits your umask instead, so use `install -d -m 700` and `install -m 600`. At startup mostrod logs a warning when `settings.toml` or `/.env` has its `other` permission bits set; unlike the macaroon check, this one runs in both Lightning and Cashu mode, because the nsec is the instance's identity either way. + ##### Providing the nsec via environment variable For better separation of secrets from config, Mostro can read the nsec from the diff --git a/docker/Dockerfile b/docker/Dockerfile index 3eb7968e..56b48c1f 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -26,8 +26,12 @@ FROM debian:bookworm-slim # Install dependencies RUN apt-get update && apt-get install -y --reinstall ca-certificates -# Add a non-root user -RUN useradd -m mostrouser +# Add a non-root user. uid/gid are pinned because compose defaults +# MOSTRO_CONTAINER_USER to 1000:1000 and the deployment docs have operators +# hand ./config over with `chown -R 1000:1000`: letting useradd pick the next +# free id would break both the moment the base image grows an account of its +# own. +RUN groupadd -g 1000 mostrouser && useradd -m -u 1000 -g 1000 mostrouser # Copy built binary from build stage COPY --from=builder /mostro/target/release/mostrod /usr/local/bin/mostrod diff --git a/docker/ENV_VARIABLES.md b/docker/ENV_VARIABLES.md index 3d4be287..384811e3 100644 --- a/docker/ENV_VARIABLES.md +++ b/docker/ENV_VARIABLES.md @@ -10,7 +10,9 @@ This document describes the environment variables used by the Docker setup. - `LND_MACAROON_FILE`: Path to the LND admin macaroon file on your host system - Example: `~/.polar/networks/1/volumes/lnd/alice/data/chain/bitcoin/regtest/admin.macaroon` -These files are copied to `docker/config/lnd/` during the build process. +These files are copied to `docker/config/lnd/` during the build process: the cert with mode `0644`, the admin macaroon with mode `0600`, both inside a directory with mode `0700`. The macaroon grants full control of your LND node, so it is never left readable by other users on the host. The `docker/config` root is set to mode `0700` too when the build has to create it, since `settings.toml` (which carries `nsec_privkey`) and `mostro.db` live beside the credentials; an existing `docker/config` keeps the mode you gave it. + +The copies belong to the user that ran the command, and `make docker-up` runs the container as the owner of `docker/config` — that user — so a host account that is not uid 1000 needs no extra step. Under `sudo`, the build installs `config/lnd` and the credentials as the owner of `docker/config` instead of as root, so a directory handed over with `chown -R 1000:1000` stays readable by the container across rebuilds. Set the optional variable below to override the container user. ## Optional Variables @@ -18,6 +20,12 @@ These files are copied to `docker/config/lnd/` during the build process. - Used in `compose.yml` for port mapping - Example: `export MOSTRO_RELAY_LOCAL_PORT=7000` +- `MOSTRO_CONTAINER_USER`: uid/gid the `mostro` container runs as + - `make docker-up` defaults it to the owner of `docker/config`, the account that can read the `0600` macaroon and write `mostro.db` there, and prints what it picked. A bare `docker compose up` falls back to `1000:1000`, the image's `mostrouser`. + - root is refused, whether derived or set explicitly, and in both spellings compose accepts for it: the numeric `0` and the name `root`. `make docker-up` stops rather than run the daemon as root, which is what a root-owned `docker/config` (left by a `sudo make docker-build` on a fresh tree) would otherwise mean. Hand the directory to an unprivileged account, or set this variable to a non-zero uid/gid that can read it. Any other name is resolved inside the image, where `mostrouser` (uid/gid 1000) is the only account besides root. + - Set it to run as someone else — for instance uid/gid 1000 on a config directory you handed over with `chown -R 1000:1000` + - Example: `export MOSTRO_CONTAINER_USER=$(id -u):$(id -g)` + - `MOSTRO_DB_PASSWORD`: Not used (database encryption was removed). Kept in `compose.yml` for backward compatibility; can be omitted or left empty. ## Usage Examples diff --git a/docker/README.md b/docker/README.md index ebacc628..f8d4bb21 100644 --- a/docker/README.md +++ b/docker/README.md @@ -33,10 +33,12 @@ To build and run the Docker container using Docker Compose, follow these steps: ```sh cd docker - mkdir -p config - cp ../settings.tpl.toml config/settings.toml + install -d -m 700 config + install -m 600 ../settings.tpl.toml config/settings.toml ``` + Mode `0700` on `config` and `0600` on `settings.toml` because that directory ends up holding every secret this deployment has: `nsec_privkey` in `settings.toml`, the LND credentials in `config/lnd/`, and the `mostro.db` the daemon writes. `install -d -m 700` also tightens a `config` directory an earlier `mkdir -p` left at `0755` — this command is you deciding the mode. Neither `make docker-build` nor `make docker-up` will decide it again: both only create `config` when it is missing, so a directory you deliberately opened up to a group later on keeps that mode. (`config/lnd` is the exception: `make docker-build` sets it to `0700` on every run, since nothing but the LND credentials it installs lives there.) + _Don't forget to edit `lnd_grpc_host`, `nsec_privkey` and `relays` fields in the `config/settings.toml` file. Note that paths in `settings.toml` refer to paths **inside the container**, so use `/config/lnd/tls.cert` and `/config/lnd/admin.macaroon` for the LND certificate and macaroon files (these will be copied there by `make docker-build`)._ 3. Build the docker image. You need to provide the `LND_CERT_FILE` and `LND_MACAROON_FILE` environment variables with the paths to your LND TLS certificate and macaroon files. These files will be copied to the `docker/config/lnd` directory by the `make docker-build` command. The build process will validate that these variables are set and that the files exist before proceeding. @@ -62,6 +64,20 @@ To build and run the Docker container using Docker Compose, follow these steps: make docker-build ``` + The admin macaroon grants full control of your LND node, so `make docker-build` writes it to `config/lnd/admin.macaroon` with mode `0600` (owner only) inside a `config/lnd` directory with mode `0700`. The `config` root is set to `0700` too when this command has to create it, and left alone when it is already there (step 2). The directories and files this command creates belong to the user who ran it; `mostro.db` is created later by the container and belongs to whoever the container runs as. + + Under `sudo` there is one extra step, taken for you: `install` recreates its destination as the invoking user, so a plain `sudo make docker-build` would leave a `root:root` macaroon inside a `config` you had already handed to uid 1000, and the container would fail at the LND connection with nothing pointing at ownership. When it runs as root over a `config` that is not root-owned, the command derives `-o`/`-g` from that directory and installs `config/lnd` and both credentials as its owner. A `config` that is root-owned itself is left as it is: `make docker-up` refuses that case outright rather than run the daemon as root. + + `make docker-up` runs the container as the owner of `docker/config` and prints the uid/gid it picked. That is the account that can actually read the `0600` macaroon and write `mostro.db` there, whether the directory belongs to you (the usual case after `make docker-build`) or was handed to uid/gid 1000. To pick a different one, export the variable `compose.yml` reads in the same shell: + + ```sh + export MOSTRO_CONTAINER_USER=$(id -u):$(id -g) + ``` + + A bare `docker compose up` does not derive anything and falls back to `1000:1000`, the image's `mostrouser`. + + `make docker-up` refuses to start when that uid comes out as root — both the numeric `0` and the name `root`, the two spellings compose accepts — which is what a `sudo make docker-build` on a fresh tree leaves behind: the container would run as root, dropping the one privilege boundary the image has. Either run both targets as the account that owns `docker/config`, or hand the directory over with `sudo chown -R 1000:1000 config` (`docker compose up` then matches, since the image's `mostrouser` is pinned to uid/gid 1000). + 4. [Optional] Set the `MOSTRO_RELAY_LOCAL_PORT` environment variable to the port you want to use for the local relay (defaults to 7000 if not set). This can be set before running `make docker-up`: ```sh @@ -89,19 +105,23 @@ You can run the plain Mostro image without building locally. Use a single **conf **Option A — download the template** (from the [settings.tpl.toml](https://github.com/MostroP2P/mostro/blob/main/settings.tpl.toml) repo file): ```sh - mkdir -p ~/mostro-config/lnd - curl -sL https://raw.githubusercontent.com/MostroP2P/mostro/main/settings.tpl.toml -o ~/mostro-config/settings.toml + install -d -m 700 ~/mostro-config ~/mostro-config/lnd + (umask 077 && curl -fsSL https://raw.githubusercontent.com/MostroP2P/mostro/main/settings.tpl.toml -o ~/mostro-config/settings.toml) ``` - **Option B — use the entrypoint default:** run the container once with an empty config dir; the entrypoint copies a default `settings.toml` (from the image, built from `settings.tpl.toml`) into `/config`. Stop the container, edit the file on the host (e.g. `~/mostro-config/settings.toml`), then start the container again. + The config root is `0700` and `settings.toml` is `0600` because both `nsec_privkey` and, later, `mostro.db` live there. `curl` creates the file under the umask in force, typically `0644`; setting the umask in a subshell around it means the file is never world-readable, not even for the moment a subsequent `chmod` would take. -2. Copy your LND TLS cert and macaroon into the config dir (so they appear at `/config/lnd/` in the container): + **Option B — use the entrypoint default:** create the config dir with `install -d -m 700 ~/mostro-config ~/mostro-config/lnd`, then run the container once against it; the entrypoint installs a default `settings.toml` (from the image, built from `settings.tpl.toml`) into `/config` with mode `0600`. Stop the container, edit the file on the host (e.g. `~/mostro-config/settings.toml`), then start the container again. + +2. Copy your LND TLS cert and macaroon into the config dir (so they appear at `/config/lnd/` in the container). Use `install` rather than `cp`: `cp` keeps whatever mode the source file (or an already existing destination file) happens to have, while `install -m` sets the mode explicitly. The admin macaroon grants full control of your LND node, so it must not be readable by other users on the host: ```sh - cp /path/to/your/tls.cert ~/mostro-config/lnd/tls.cert - cp /path/to/your/admin.macaroon ~/mostro-config/lnd/admin.macaroon + install -m 644 /path/to/your/tls.cert ~/mostro-config/lnd/tls.cert + install -m 600 /path/to/your/admin.macaroon ~/mostro-config/lnd/admin.macaroon ``` + Mode `0600` on the macaroon inside a `0700` directory means only their owner can reach the file, and the container runs as uid/gid 1000 by default (the image's `mostrouser`, pinned to those ids). If your user is not uid 1000, run the container as yourself by adding `--user $(id -u):$(id -g)` to the `docker run` command in step 4 — that also lets it write `mostro.db` into your config directory. + 3. Edit `~/mostro-config/settings.toml`: set `nsec_privkey`, `relays`, and for Docker set `lnd_cert_file` / `lnd_macaroon_file` to `/config/lnd/...`, `lnd_grpc_host` (e.g. `https://host.docker.internal:10009`), and `[database]` `url = "sqlite:///config/mostro.db"`. 4. Run the container. On Linux, add `--add-host=host.docker.internal:host-gateway` so the container can reach LND on the host: @@ -123,28 +143,36 @@ Steps to run the plain Mostro image on a VPS (no repo clone; image from Docker H 1. **Install Docker** on the VPS (e.g. [Docker Engine](https://docs.docker.com/engine/install/)). -2. **Create a config directory** (e.g. `/opt/mostro` or `~/mostro-config`): +2. **Create a config directory** at `/opt/mostro`: ```sh - mkdir -p /opt/mostro/lnd + install -d -m 700 -o 1000 -g 1000 /opt/mostro + install -d -m 700 -o 1000 -g 1000 /opt/mostro/lnd ``` + These steps run as root, while the container runs as uid/gid 1000, so both directories are handed to the container's user: it needs to write `mostro.db` into the config directory. Both are owner-only because of what goes in them — `nsec_privkey` in `settings.toml` and the database in the config root, the LND credentials in `lnd` (step 4). + 3. **Get the settings template** into that directory as `settings.toml`: - - Either run the container once with an empty config dir; the entrypoint will copy the default template to `/config/settings.toml`. Stop the container, then edit the file on the host. + - Either run the container once with an empty config dir; the entrypoint installs the default template at `/config/settings.toml` with mode `0600`. Stop the container, then edit the file on the host. - Or download the template and copy it: ```sh - curl -sL https://raw.githubusercontent.com/MostroP2P/mostro/main/settings.tpl.toml -o /opt/mostro/settings.toml + (umask 077 && curl -fsSL https://raw.githubusercontent.com/MostroP2P/mostro/main/settings.tpl.toml -o /opt/mostro/settings.toml) + chown 1000:1000 /opt/mostro/settings.toml ``` -4. **Put LND files** in the config dir so they appear at `/config/lnd/` in the container: + `curl` writes the file under root's umask, typically `0644` and root-owned. The umask in the subshell settles the mode as the file is created, so it is never world-readable; the owner still has to be handed over afterwards, because the file receives `nsec_privkey` in step 5 and the container reads it as uid/gid 1000. + +4. **Put LND files** in the config dir so they appear at `/config/lnd/` in the container. Use `install -m` rather than `cp`, which would keep whatever mode the source file (or an already existing destination file) happens to have: ```sh - cp /path/to/lnd/tls.cert /opt/mostro/lnd/tls.cert - cp /path/to/lnd/admin.macaroon /opt/mostro/lnd/admin.macaroon + install -m 644 /path/to/lnd/tls.cert /opt/mostro/lnd/tls.cert + install -m 600 -o 1000 -g 1000 /path/to/lnd/admin.macaroon /opt/mostro/lnd/admin.macaroon ``` + The admin macaroon grants full control of your LND node — anyone who reads it can move the funds escrowed in Mostro's hold invoices — so it is installed owner-readable only, and `-o 1000 -g 1000` hands it to the container's user (as the `0700` directory from step 2 already was). Without that ownership, mode `0600` would leave mostrod unable to read the macaroon. (`-o`/`-g` require root; as a non-root user, drop them, run the steps as the account that owns the config dir, and add `--user $(id -u):$(id -g)` to the `docker run` commands in step 6, so the container runs as that account rather than as the image default `1000:1000` — which could not read the `0600` macaroon you just installed.) + (If LND is on another host, you only need the cert and macaroon copied here; point `lnd_grpc_host` at that host in step 5.) 5. **Edit `/opt/mostro/settings.toml`**: diff --git a/docker/compose.yml b/docker/compose.yml index 7bb9f1d0..caf796b3 100644 --- a/docker/compose.yml +++ b/docker/compose.yml @@ -5,6 +5,12 @@ services: dockerfile: docker/Dockerfile volumes: - ./config:/config # settings.toml and mostro.db + # The image pins mostrouser to uid/gid 1000, so the default keeps the + # previous behaviour. Hosts whose user is not uid 1000 export + # MOSTRO_CONTAINER_USER=$(id -u):$(id -g) instead of handing ./config over + # to uid 1000 — the LND macaroon is installed 0600, and the daemon also + # needs to write mostro.db in the same directory. + user: "${MOSTRO_CONTAINER_USER:-1000:1000}" platform: linux/amd64 networks: - default diff --git a/docker/start.sh b/docker/start.sh index c09cdde4..2cf56430 100644 --- a/docker/start.sh +++ b/docker/start.sh @@ -1,10 +1,17 @@ #!/bin/sh set -e -# Check if the settings.toml file exists, if not, create a new one -if [ ! -f /config/settings.toml ]; then +# Check if the settings.toml file exists, if not, create a new one. +# `install -m 600` rather than `cp`: the file receives nsec_privkey once edited, +# and `cp` would keep whatever mode the template in the image happens to have. +# +# `-e` plus `-L` rather than `-f`: `-f` is false for a dangling symlink, and +# `install` would then write the template through it. Leaving anything that is +# already at the path alone means mostrod reports it a moment later, with more +# context than this script has. +if [ ! -e /config/settings.toml ] && [ ! -L /config/settings.toml ]; then echo "settings.toml not found, creating a new one from template (default)." - cp /mostro/settings.toml /config/settings.toml + install -m 600 /mostro/settings.toml /config/settings.toml fi # Run application (Mostro creates mostro.db at startup if missing) diff --git a/docs/STARTUP_AND_CONFIG.md b/docs/STARTUP_AND_CONFIG.md index 2efad905..ecb94014 100644 --- a/docs/STARTUP_AND_CONFIG.md +++ b/docs/STARTUP_AND_CONFIG.md @@ -29,9 +29,10 @@ Before settings initialization, the daemon performs (see `src/main.rs`): ### Settings Initialization Details **Directory setup**: -- Creates `~/.mostro/` directory if not exists +- Creates `~/.mostro/` directory if not exists, owner-only (mode `0700` on Unix) - Checks for existing `~/.mostro/settings.toml` -- If missing: copies `settings.tpl.toml` to `~/.mostro/settings.toml` +- If missing: copies `settings.tpl.toml` to `~/.mostro/settings.toml`, owner-only + (mode `0600` on Unix), since the file receives `nsec_privkey` once edited - On first run after creating the file, the process exits so the user can edit `settings.toml`, then restart Mostro. - Overrides database URL: `~/.mostro/mostro.db` **Settings loading**: @@ -104,6 +105,26 @@ Configuration is loaded from `~/.mostro/settings.toml` (template: `settings.tpl. **Lightning** (`src/config/types.rs:27-46`): - `lnd_cert_file` (String): Path to LND TLS certificate - `lnd_macaroon_file` (String): Path to LND macaroon auth file + - The admin macaroon is spend-capable: any account that can read it controls + the node, including the funds escrowed in Mostro's hold invoices. Keep it + readable only by the user running `mostrod` (`chmod 600`). + - At startup (Lightning mode only, right before the LND connection is opened) + `src/config/permissions.rs`, `fn warn_if_other_accessible` logs a warning when the + file's `other` permission bits are set, and suggests `chmod o=`. The check + is advisory — the daemon still starts — and tolerates `0640`, the mode LND + itself writes the macaroon with, so reading it through the node's group + stays supported. + - The same check runs over `settings.toml`, `/.env` and + `/mostro.db` right after the settings load, in both Lightning and + Cashu mode: the first two carry `nsec_privkey`, which is the instance's identity + regardless of which escrow it uses, and the database holds the trade history, the + disputes and the hold-invoice preimages. SQLite creates the database under the + umask, so a settings directory that predates the `0700` default is where a `0644` + database comes from. A missing file is silent: `.env` is optional, and the database + does not exist yet on a first boot. + - Only the mode bits are inspected: a POSIX ACL can grant a named user access + without setting them, so a quiet startup is not proof that no other account + can read the file. - `lnd_grpc_host` (String): LND gRPC endpoint URL - `invoice_expiration_window` (u32): Required invoice validity window in seconds (default: 3600) - `hold_invoice_cltv_delta` (u32): Hold invoice CLTV delta in blocks (default: 144) @@ -187,3 +208,54 @@ There is **no** database password or separate global for SQLite; the daemon open ## Security - Do not commit populated `settings.toml`. - Keep templates in `settings.tpl.toml`; place runtime config at `~/.mostro/settings.toml`. +- `settings.toml` carries `nsec_privkey` in plaintext unless it is supplied through + `MOSTRO_NSEC_PRIVKEY`, and the settings directory also holds `mostro.db` and, in the + Docker flows, the LND credentials under `lnd/`. Keep the directory at mode `0700` and + the file at `0600`. +- Every path that creates them does so already, through one of three primitives in + `src/config/permissions.rs`: `fn create_settings_dir` for the directory, + `fn create_owner_only` for a file that must not already exist, and + `fn write_owner_only_atomic` for one that is rewritten. A directory or file created + outside the daemon — `mkdir`, `cp`, `curl` — inherits the umask instead, so the + deployment guides use `install -d -m 700` and `install -m 600`. +- `create_owner_only`'s three callers are the non-interactive template copy + (`src/config/util.rs`, `fn init_configuration_file`), the manual template copy + (`src/config/wizard.rs`, `fn run_setup_menu`) and the guided wizard save + (`src/config/wizard.rs`, `fn save_settings`). `write_owner_only_atomic` has one: + the wizard's `.env` write (`src/config/wizard.rs`, `fn write_env_file`). +- An existing settings directory is left as it is, so a deliberately group-readable + deployment keeps working. `0700` also applies to the settings directory alone — any + missing parents are created under the umask, the way `mkdir -p` would, so + `mostrod -d /srv/apps/mostro/conf` on a fresh tree closes off `conf` without closing + off `/srv/apps` for anything else that lives there. +- `create_owner_only` uses `O_CREAT | O_EXCL`, so it fails rather than write through + whatever already occupies the path. This covers initial creation only: on a settings + directory another local account can write to, a symlink planted between the caller's + existence check and the create would otherwise have its target truncated and its mode + reset to `0600`. A write that fails partway removes the file it created, so a full + disk cannot leave a truncated `settings.toml` behind for the next boot to reject as + malformed TOML. +- `write_env_file` cannot refuse an existing path the way `create_owner_only` does — + rewriting an existing `.env` is a supported thing to do — so + `write_owner_only_atomic` stages the line in a fresh `O_EXCL` temporary beside the + target and `rename`s it into place. `rename` never opens the destination, so a + planted symlink is replaced rather than followed and its target keeps both its + contents and its mode. The temporary is `fsync`ed before the rename and the directory + after it (`src/config/permissions.rs`, `fn sync_dir`), so the replacement is durable + and not merely atomic — a power loss right after an unflushed rename can leave the + directory entry pointing at the old file, which for `.env` means losing an + `nsec_privkey` the wizard reported as saved. A failed flush is logged and not + propagated: the contents are already in place. `.env` matters here as much as + `settings.toml`: it carries the same `nsec_privkey`, and in the wizard flow it is + written first. +- It is not a check on startup as a whole. A `settings.toml` that already exists is read + and loaded normally, symlink or not — `fn init_configuration_file` only reaches the + creation path when it finds no settings file at all. +- The guarantee covers the final entry, not the directory path leading to it. The + parents `fn create_settings_dir` has to invent are made with `fs::create_dir_all`, + which resolves symlinked components on the way, so a settings directory + reached through a symlinked parent is created at whatever that link points to. This is + deliberate: operators do symlink a config directory onto another volume, and planting + such a link on the default `~/.mostro` path means write access to `$HOME`, which is a + compromise of the account already. A settings directory that is itself a symlink to an + existing directory is used as-is, and a dangling one is refused. diff --git a/src/cli.rs b/src/cli.rs index a79df533..f6463d9f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -4,6 +4,7 @@ use crate::config::util::init_configuration_file; use clap::Parser; +use std::path::PathBuf; #[derive(Parser)] #[command( @@ -33,20 +34,22 @@ pub struct Cli { /// Default folder is HOME but user can specify a custom folder with dirsettings (-d ) parameter from CLI /// Example: mostro p2p -d /user_folder/mostro /// -pub fn settings_init() -> Result<(), Box> { +/// Returns the settings directory in use, which the caller needs to check the +/// permissions of the secret-bearing files inside it. +pub fn settings_init() -> Result> { // Parse CLI arguments let cli = Cli::parse(); // Select config file from CLI or default to HOME/.mostro // create config file if it doesn't exist - if let Some(path) = cli.dirsettings.as_deref() { + let settings_dir = if let Some(path) = cli.dirsettings.as_deref() { init_configuration_file(Some(path.to_string()))? } else { init_configuration_file(None)? }; // Mostro settings are initialized - Ok(()) + Ok(settings_dir) } #[cfg(test)] @@ -123,7 +126,7 @@ mod tests { // In a real implementation, we would need dependency injection for testing // Test that the function signature is correct - let _: fn() -> Result<(), Box> = settings_init; + let _: fn() -> Result> = settings_init; // Verify function exists and has correct return type // No-op: type check above is sufficient diff --git a/src/config/constants.rs b/src/config/constants.rs index ce5d3b5f..74f576fd 100644 --- a/src/config/constants.rs +++ b/src/config/constants.rs @@ -27,6 +27,14 @@ pub const NOSTR_EXCHANGE_RATES_EVENT_KIND: u16 = 30078; /// startup. Shared between the wizard (writes it) and the loader (reads it). pub const ENV_FILENAME: &str = ".env"; +/// Name of the settings file inside the settings directory. +pub const SETTINGS_FILENAME: &str = "settings.toml"; + +/// Name of the SQLite database inside the settings directory. The loader +/// overrides `database.url` with it unconditionally, so this is the only path +/// the daemon ever opens. +pub const DB_FILENAME: &str = "mostro.db"; + /// Environment variable name used to override the Nostr private key from the /// process environment. Shared between the wizard and the loader. pub const NSEC_ENV_VAR: &str = "MOSTRO_NSEC_PRIVKEY"; diff --git a/src/config/mod.rs b/src/config/mod.rs index d141f552..4661325f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,7 +1,10 @@ // Mostro module for configurataion settings pub mod constants; +pub mod permissions; pub mod secret; pub mod settings; +#[cfg(test)] +pub(crate) mod test_support; /// This module provides functionality to manage and initialize settings for the Mostro application. /// It includes structures for database, lightning, Nostr, and Mostro settings, as well as functions to initialize and access these settings. pub mod types; diff --git a/src/config/permissions.rs b/src/config/permissions.rs new file mode 100644 index 00000000..88b61cfb --- /dev/null +++ b/src/config/permissions.rs @@ -0,0 +1,692 @@ +//! Filesystem permissions of the files mostrod owns or reads: the startup +//! check on the credential and secret files named by the settings, and the +//! primitives that create the settings directory, `settings.toml` and `.env` +//! owner-only. +//! +//! Kept in its own module — rather than folded into `config::util` — because +//! these are about the files themselves, not about loading the settings. Both +//! `config::util` (non-interactive template copy) and `config::wizard` (manual +//! template copy, guided wizard save, `.env` write) bring those files into +//! existence through the primitives here, so a single place decides how they +//! come into being. + +use mostro_core::error::MostroError::{self, MostroInternalErr}; +use mostro_core::error::ServiceError; +use std::ffi::{OsStr, OsString}; +use std::fs; +use std::path::{Path, PathBuf}; + +/// How many temporary sibling names `write_owner_only_atomic` tries before it +/// gives up. Each attempt only fails when the name is already taken, so a +/// handful is plenty; the bound is there so a directory seeded with every +/// candidate name is an error rather than a hang. +const TEMP_NAME_ATTEMPTS: u32 = 16; + +/// Warn when a file holding a secret has any of its "other" permission bits +/// set, which puts it within reach of every account on the host. +/// +/// The check is advisory: a node whose macaroon is `0644` still starts, it +/// just says so out loud once per boot. Refusing to start would turn a +/// hardening gap into an outage on the next upgrade for every operator who +/// already runs that way. +/// +/// Empty paths and files that cannot be stat'ed are ignored: an unset or +/// unreadable path is not a permissions problem, and the real failure is +/// reported with far more context by whoever opens the file (for the macaroon, +/// `LndConnector::new`). `/.env` is optional and usually absent, +/// and `mostro.db` does not exist yet on a first boot, so a missing file has +/// to stay silent. +pub fn warn_if_other_accessible(path: &Path, label: &str) { + if path.as_os_str().is_empty() { + return; + } + + if let Some(mode) = other_accessible_mode(path) { + // `chmod o=` rather than `chmod 600`: the file may legitimately be + // owned by another account and read by mostrod through a shared group + // — the LND macaroon through the node's group is the documented case — + // and following advice that drops the group bits would leave the + // daemon unable to authenticate on its next restart. + tracing::warn!( + "{label} ({}) has permissions {mode:04o}: its \"other\" bits are set, so every \ + account on this host can reach it. Clear them with: chmod o= {}", + path.display(), + path.display() + ); + } +} + +/// The file's permission bits when the "other" class has any of them set, +/// `None` otherwise. +/// +/// Group access is deliberately tolerated: LND itself creates +/// `admin.macaroon` with mode `0640`, and granting a service account access +/// through the node's group is a legitimate deployment, not a finding. +/// +/// Only the mode bits are read. A POSIX ACL can widen access without touching +/// them, so a quiet startup means "the mode bits are sane", not "no other +/// account can read this file" — the check is a cheap guard against the +/// documented copy-it-into-place flows, not an audit. +/// +/// `metadata` follows symlinks on purpose — pointing `lnd_macaroon_file` at a +/// link is common, and what matters is the mode of the file that is actually +/// read. +#[cfg(unix)] +fn other_accessible_mode(path: &Path) -> Option { + use std::os::unix::fs::PermissionsExt; + + let mode = fs::metadata(path).ok()?.permissions().mode() & 0o777; + (mode & 0o007 != 0).then_some(mode) +} + +/// Non-Unix platforms have no POSIX permission bits to inspect. +#[cfg(not(unix))] +fn other_accessible_mode(_path: &Path) -> Option { + None +} + +/// Create the settings directory owner-only (`0700` on Unix). +/// +/// The directory holds `settings.toml` with a plaintext `nsec_privkey`, the +/// `mostro.db` database and, in the Docker flows, the LND credentials under +/// `lnd/`. `create_dir_all` would apply the process umask instead, which on a +/// typical host leaves the directory at `0755`. +/// +/// `0700` applies to the settings directory itself and to nothing else. Any +/// missing parents are created under the umask, the way `mkdir -p` would: with +/// `mostrod -d /srv/apps/mostro/conf` on a fresh tree, only `conf` is closed +/// off, while `/srv/apps` and `/srv/apps/mostro` stay reachable by whatever +/// else lives under them. +/// +/// An existing settings directory is returned as it is, so an operator who +/// already set one up with deliberate group access keeps it. +/// +/// The parents are created with `fs::create_dir_all`, which resolves symlinked +/// components on the way. That is deliberate: symlinking a config directory +/// onto another volume is a legitimate setup, and planting a link on the +/// default `~/.mostro` path takes write access to `$HOME`, which already owns +/// the account. A settings directory that is itself a symlink to an +/// existing directory never reaches here — the caller finds it and uses it. +pub(crate) fn create_settings_dir(settings_dir: &Path) -> Result<(), MostroError> { + if settings_dir.is_dir() { + return Ok(()); + } + + if let Some(parent) = settings_dir.parent() { + if !parent.as_os_str().is_empty() && !parent.is_dir() { + fs::create_dir_all(parent) + .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; + } + } + + // Non-recursive on purpose: the mode below must reach the settings + // directory and no ancestor, and `create_dir_all` has no way to say that. + let mut builder = fs::DirBuilder::new(); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + builder.mode(0o700); + } + builder + .create(settings_dir) + .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string()))) +} + +/// Create `path` and write `contents` to it with owner-only permissions +/// (`0600` on Unix). Fails if anything already exists at `path`. +/// +/// Every path that brings an initial `settings.toml` into existence goes +/// through here, so the file that later carries `nsec_privkey` is never left +/// at the process umask and never reached through a symlink. +/// +/// `create_new` maps to `O_CREAT | O_EXCL`, which POSIX requires to fail with +/// `EEXIST` when the path names a symbolic link, whatever it points at. Callers +/// only reach this after finding no settings file, but that check and this call +/// cannot be one operation: on a settings directory another local account can +/// write to, a symlink planted in between would otherwise have its target +/// truncated and its mode reset to `0600` by the two steps below. +/// +/// A write that fails partway takes the file with it. What is left behind +/// otherwise is a truncated `settings.toml` that the next boot parses and +/// rejects as malformed TOML instead of recreating — the file was created with +/// `O_EXCL` a moment earlier, so removing it cannot touch anything else. +pub(crate) fn create_owner_only(path: &Path, contents: &[u8]) -> Result<(), MostroError> { + use std::io::Write; + + let mut file = open_owner_only_new(path).map_err(|e| { + MostroInternalErr(ServiceError::IOError(format!( + "Could not create {}: {}", + path.display(), + e + ))) + })?; + + let written = file.write_all(contents); + // Closed before the cleanup below, which Windows would refuse on an open + // handle. + drop(file); + + written.map_err(|e| { + let _ = fs::remove_file(path); + MostroInternalErr(ServiceError::IOError(format!( + "Could not write {}: {}", + path.display(), + e + ))) + }) +} + +/// Replace `path` with `contents`, owner-only (`0600` on Unix), atomically. +/// +/// For the files mostrod rewrites rather than creates once — today +/// `/.env`, which carries the same `nsec_privkey` as +/// `settings.toml`. [`create_owner_only`] cannot serve them: it refuses a path +/// that already exists, which is exactly what a rewrite has to do. +/// +/// The contents go to a fresh temporary file in the same directory, created +/// with `O_CREAT | O_EXCL` and chmod'ed through its descriptor, and are then +/// moved onto `path` with `rename`. That buys two things at once: +/// +/// - `rename` replaces whatever `path` names without ever opening it, so a +/// symlink another local account planted there is unlinked rather than +/// written through — its target keeps both its contents and its mode. +/// `create_owner_only` refuses in that situation; here refusing is not an +/// option, and replacing the link gives the target the same protection. A +/// deliberately symlinked `.env` is not a supported setup: it is a file the +/// daemon writes, in a directory it created `0700`. +/// - The file at `path` is never observed half-written. A `.env` truncated by +/// a full disk would otherwise leave the daemon with no `nsec_privkey` at +/// all on the next boot. +/// +/// The contents are `fsync`ed before the rename and the directory after it, so +/// the replacement survives a crash and not only a process exit. Without the +/// second one the rename is atomic but not durable: a power loss right after +/// it can leave the directory entry still pointing at the old file, which for +/// the wizard's `.env` means an `nsec_privkey` the operator was told was +/// saved. +pub(crate) fn write_owner_only_atomic(path: &Path, contents: &[u8]) -> Result<(), MostroError> { + use std::io::Write; + + let dir = match path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent, + _ => Path::new("."), + }; + let file_name = path.file_name().ok_or_else(|| { + MostroInternalErr(ServiceError::IOError(format!( + "{} does not name a file", + path.display() + ))) + })?; + + let (temp_path, mut temp_file) = create_temp_sibling(dir, file_name)?; + + // The temporary is only ever left behind on a failure, and never with the + // secret still in it. + let staged = temp_file + .write_all(contents) + .and_then(|()| temp_file.sync_all()); + drop(temp_file); + + if let Err(e) = staged.and_then(|()| fs::rename(&temp_path, path)) { + let _ = fs::remove_file(&temp_path); + return Err(MostroInternalErr(ServiceError::IOError(format!( + "Could not write {}: {}", + path.display(), + e + )))); + } + + sync_dir(dir); + + Ok(()) +} + +/// Flush the directory entry the `rename` above just replaced. +/// +/// `fsync` on a directory descriptor is the POSIX way to make a rename +/// durable; the data blocks are already on disk from the `sync_all` on the +/// temporary. Windows has no equivalent, and its rename does not need one. +/// +/// Failures are logged rather than propagated. The new contents are in place +/// and visible either way — only the durability of the directory entry is in +/// question — so returning an error here would fail a write that succeeded and +/// send the caller into a rollback path with nothing to roll back. +#[cfg(unix)] +fn sync_dir(dir: &Path) { + match fs::File::open(dir).and_then(|dir_file| dir_file.sync_all()) { + Ok(()) => {} + Err(e) => tracing::warn!( + "Wrote the file but could not flush {}: {e}. The contents are in place; only a \ + crash before the filesystem catches up on its own could still lose them.", + dir.display() + ), + } +} + +#[cfg(not(unix))] +fn sync_dir(_dir: &Path) {} + +/// Create an owner-only temporary file next to the target and return it with +/// its path. +/// +/// `O_EXCL` again, so a stale temporary left behind by a killed run — or one +/// planted deliberately — is never written through; the suffix is bumped until +/// a free name is found. The temporary is a sibling rather than something +/// under `/tmp` because `rename` only works within a filesystem, and because +/// the settings directory is already `0700`. +fn create_temp_sibling(dir: &Path, file_name: &OsStr) -> Result<(PathBuf, fs::File), MostroError> { + let mut last_error = None; + + for attempt in 0..TEMP_NAME_ATTEMPTS { + let candidate = dir.join(temp_sibling_name(file_name, attempt)); + + match open_owner_only_new(&candidate) { + Ok(file) => return Ok((candidate, file)), + // Only a taken name is worth another attempt. An unwritable or + // missing directory, or a full disk, fails the same way sixteen + // times over and would be reported as a name collision. + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => last_error = Some(e), + Err(e) => { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "Could not create a temporary file in {}: {e}", + dir.display() + )))) + } + } + } + + Err(MostroInternalErr(ServiceError::IOError(format!( + "Could not create a temporary file in {} after {TEMP_NAME_ATTEMPTS} attempts: {}", + dir.display(), + last_error + .map(|e| e.to_string()) + .unwrap_or_else(|| "unknown error".to_string()) + )))) +} + +/// The name `create_temp_sibling` tries for a given attempt: a dotfile next to +/// the target, so a temporary left behind by a killed run is not mistaken for +/// a settings file. +/// +/// Shared with the tests, which seed one of these names to exercise the retry +/// and would otherwise silently stop matching if the format changed here. +fn temp_sibling_name(file_name: &OsStr, attempt: u32) -> OsString { + let mut name = OsString::from("."); + name.push(file_name); + name.push(format!(".tmp-{}-{attempt}", std::process::id())); + name +} + +/// Open a brand-new file owner-only, failing if anything already occupies the +/// path. +/// +/// `OpenOptionsExt::mode` is masked by the process umask, so the mode is set +/// again through the file descriptor — never through the path, which would +/// reintroduce the symlink `create_new` just refused to follow. +fn open_owner_only_new(path: &Path) -> std::io::Result { + #[cfg(unix)] + let file = { + use std::os::unix::fs::OpenOptionsExt; + fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(path)? + }; + #[cfg(not(unix))] + let file = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(path)?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + file.set_permissions(fs::Permissions::from_mode(0o600))?; + } + + Ok(file) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use crate::config::test_support::temp_dir; + use std::os::unix::fs::PermissionsExt; + use std::path::PathBuf; + + fn macaroon_with_mode(tag: &str, mode: u32) -> PathBuf { + let path = temp_dir("permissions", tag).join("admin.macaroon"); + std::fs::write(&path, b"macaroon").expect("write macaroon"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)) + .expect("set permissions"); + path + } + + #[test] + fn owner_only_is_accepted() { + let path = macaroon_with_mode("owner-only", 0o600); + assert_eq!(other_accessible_mode(&path), None); + } + + #[test] + fn group_readable_is_accepted() { + // 0640 is the mode LND writes admin.macaroon with, and reaching it + // through the node's group is a supported setup. + let path = macaroon_with_mode("group-read", 0o640); + assert_eq!(other_accessible_mode(&path), None); + } + + #[test] + fn world_readable_is_reported() { + let path = macaroon_with_mode("world-read", 0o644); + assert_eq!(other_accessible_mode(&path), Some(0o644)); + } + + #[test] + fn other_read_without_group_read_is_reported() { + let path = macaroon_with_mode("other-read", 0o604); + assert_eq!(other_accessible_mode(&path), Some(0o604)); + } + + #[test] + fn world_writable_is_reported() { + // Not a disclosure by itself, but a local account that can replace the + // credential mostrod authenticates with is the same class of problem. + let path = macaroon_with_mode("world-write", 0o602); + assert_eq!(other_accessible_mode(&path), Some(0o602)); + } + + #[test] + fn symlink_reports_the_target_mode() { + let dir = temp_dir("permissions", "symlink"); + let target = dir.join("real.macaroon"); + std::fs::write(&target, b"macaroon").expect("write macaroon"); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o644)) + .expect("set permissions"); + + let link = dir.join("linked.macaroon"); + std::os::unix::fs::symlink(&target, &link).expect("create symlink"); + + assert_eq!(other_accessible_mode(&link), Some(0o644)); + } + + #[test] + fn missing_file_is_ignored() { + assert_eq!( + other_accessible_mode(Path::new("/definitely/not/here.macaroon")), + None + ); + } + + #[test] + fn empty_and_missing_paths_do_not_panic() { + warn_if_other_accessible(Path::new(""), "LND admin macaroon"); + warn_if_other_accessible( + Path::new("/definitely/not/here.macaroon"), + "Mostro env file", + ); + } +} + +#[cfg(test)] +mod owner_only_tests { + use super::*; + use crate::config::test_support::{assert_mode, mode_of, set_mode, temp_dir}; + + fn temp_root(tag: &str) -> PathBuf { + temp_dir("owner-only", tag) + } + + #[test] + fn settings_dir_is_created_owner_only() { + let root = temp_root("dir"); + let settings_dir = root.join(".mostro"); + create_settings_dir(&settings_dir).expect("create settings dir"); + assert_mode(&settings_dir, 0o700); + } + + #[test] + fn settings_dir_creation_closes_off_the_leaf_but_not_its_ancestors() { + let root = temp_root("dir-nested"); + let settings_dir = root.join("apps").join("mostro").join("conf"); + create_settings_dir(&settings_dir).expect("create nested settings dir"); + assert_mode(&settings_dir, 0o700); + + // The parents `mkdir -p` had to invent keep whatever the umask gives + // them, so `mostrod -d /srv/apps/mostro/conf` does not close off + // `/srv/apps` for everything else that lives under it. Compared + // against a reference tree rather than a literal mode, because the + // umask is the environment's to choose. + let reference = root.join("reference").join("inner"); + std::fs::create_dir_all(&reference).expect("create reference tree"); + assert_eq!( + mode_of(&root.join("apps")), + mode_of(&root.join("reference")) + ); + assert_eq!( + mode_of(&root.join("apps").join("mostro")), + mode_of(&reference) + ); + } + + #[test] + fn settings_dir_creation_leaves_an_existing_directory_alone() { + let root = temp_root("dir-existing"); + let settings_dir = root.join(".mostro"); + std::fs::create_dir(&settings_dir).expect("create settings dir"); + set_mode(&settings_dir, 0o750); + // A deliberate group-readable directory must survive this rather than + // be tightened or reported as an error. + create_settings_dir(&settings_dir).expect("existing directory is not an error"); + assert_mode(&settings_dir, 0o750); + } + + #[test] + fn template_is_written_owner_only() { + let root = temp_root("file"); + let config_file = root.join("settings.toml"); + create_owner_only(&config_file, b"nsec_privkey = 'nsec1...'\n").expect("write template"); + assert_mode(&config_file, 0o600); + assert_eq!( + std::fs::read_to_string(&config_file).expect("read back"), + "nsec_privkey = 'nsec1...'\n" + ); + } + + #[test] + fn a_preexisting_file_is_refused_instead_of_truncated() { + let root = temp_root("file-existing"); + let config_file = root.join("settings.toml"); + std::fs::write(&config_file, "operator contents").expect("seed file"); + set_mode(&config_file, 0o644); + assert!(create_owner_only(&config_file, b"fresh\n").is_err()); + // Neither the contents nor the mode of what was already there change. + assert_eq!( + std::fs::read_to_string(&config_file).expect("read back"), + "operator contents" + ); + assert_mode(&config_file, 0o644); + } + + #[test] + fn writing_to_an_unwritable_path_is_an_error() { + let root = temp_root("file-error"); + // A directory cannot be opened for writing, so this exercises the + // error branch instead of silently succeeding. + let config_file = root.join("settings.toml"); + std::fs::create_dir(&config_file).expect("create dir in the file's place"); + assert!(create_owner_only(&config_file, b"x").is_err()); + } + + #[test] + fn atomic_write_creates_a_missing_file_owner_only() { + let root = temp_root("atomic-new"); + let env_file = root.join(".env"); + write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n") + .expect("write env file"); + assert_eq!( + std::fs::read_to_string(&env_file).expect("read back"), + "MOSTRO_NSEC_PRIVKEY=nsec1...\n" + ); + assert_mode(&env_file, 0o600); + } + + #[test] + fn atomic_write_replaces_an_existing_file_and_tightens_its_mode() { + let root = temp_root("atomic-existing"); + let env_file = root.join(".env"); + std::fs::write(&env_file, "OLD=stale\n").expect("seed env file"); + set_mode(&env_file, 0o644); + + write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1replaced\n") + .expect("rewrite env file"); + + assert_eq!( + std::fs::read_to_string(&env_file).expect("read back"), + "MOSTRO_NSEC_PRIVKEY=nsec1replaced\n" + ); + assert_mode(&env_file, 0o600); + } + + #[test] + fn atomic_write_leaves_no_temporary_behind() { + let root = temp_root("atomic-clean"); + let env_file = root.join(".env"); + write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n").expect("write"); + + let leftovers: Vec<_> = std::fs::read_dir(&root) + .expect("read dir") + .map(|entry| entry.expect("dir entry").file_name()) + .filter(|name| name != ".env") + .collect(); + assert!( + leftovers.is_empty(), + "the temporary must be renamed away, found {leftovers:?}" + ); + } + + #[test] + fn atomic_write_to_an_unwritable_path_is_an_error() { + let root = temp_root("atomic-error"); + let env_file = root.join(".env"); + std::fs::create_dir(&env_file).expect("create dir in the file's place"); + // `rename` cannot replace a non-empty directory, so this must not + // report success. + std::fs::write(env_file.join("occupied"), b"x").expect("occupy the directory"); + assert!(write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n").is_err()); + } + + #[test] + fn a_failure_that_is_not_a_name_collision_is_reported_as_itself() { + let root = temp_root("atomic-missing-dir"); + let env_file = root.join("absent").join(".env"); + let err = write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n") + .expect_err("a missing directory must not be written to"); + // Sixteen identical failures in a directory that does not exist only + // bury the cause, so the message must not blame a name collision. + let message = format!("{err:?}"); + assert!( + !message.contains("attempts"), + "expected the underlying error, got {message}" + ); + } +} + +#[cfg(all(test, unix))] +mod symlink_tests { + use super::*; + use crate::config::test_support::{assert_mode, set_mode, temp_dir}; + + fn victim_in(root: &Path) -> PathBuf { + let victim = root.join("victim"); + std::fs::write(&victim, "victim contents").expect("seed victim"); + set_mode(&victim, 0o644); + victim + } + + #[test] + fn a_symlink_in_the_settings_path_is_refused_and_its_target_untouched() { + let root = temp_dir("owner-only", "file-symlink"); + let victim = victim_in(&root); + + // What another local account could plant in a settings directory it can + // write to, in the window between the caller's existence check and this + // call. Following it would truncate the victim and reset it to 0600. + let config_file = root.join("settings.toml"); + std::os::unix::fs::symlink(&victim, &config_file).expect("plant symlink"); + + assert!(create_owner_only(&config_file, b"template\n").is_err()); + assert_eq!( + std::fs::read_to_string(&victim).expect("read back"), + "victim contents" + ); + assert_mode(&victim, 0o644); + // The symlink itself is left in place; nothing was written through it. + assert!(std::fs::symlink_metadata(&config_file) + .expect("symlink metadata") + .file_type() + .is_symlink()); + } + + #[test] + fn a_dangling_symlink_is_refused_rather_than_created_through() { + let root = temp_dir("owner-only", "file-dangling"); + let config_file = root.join("settings.toml"); + // `Path::exists` follows symlinks, so the caller's check reports false + // here and this call is what has to refuse. + std::os::unix::fs::symlink(root.join("does-not-exist"), &config_file) + .expect("plant dangling symlink"); + assert!(!config_file.exists()); + assert!(create_owner_only(&config_file, b"template\n").is_err()); + assert!(!root.join("does-not-exist").exists()); + } + + #[test] + fn atomic_write_replaces_a_planted_symlink_instead_of_following_it() { + let root = temp_dir("owner-only", "atomic-symlink"); + let victim = victim_in(&root); + + let env_file = root.join(".env"); + std::os::unix::fs::symlink(&victim, &env_file).expect("plant symlink"); + + write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n") + .expect("write env file"); + + // The nsec landed in a regular file that replaced the link, and the + // target kept both its contents and its mode. + assert_eq!( + std::fs::read_to_string(&victim).expect("read victim"), + "victim contents" + ); + assert_mode(&victim, 0o644); + assert!(std::fs::symlink_metadata(&env_file) + .expect("symlink metadata") + .file_type() + .is_file()); + assert_mode(&env_file, 0o600); + } + + #[test] + fn atomic_write_steps_over_a_stale_temporary() { + let root = temp_dir("owner-only", "atomic-stale"); + let env_file = root.join(".env"); + // What a run killed mid-write leaves behind. Writing through it would + // be harmless here, but the same name could just as well be a symlink + // another account planted, so the first free suffix is used instead. + let stale = root.join(temp_sibling_name(OsStr::new(".env"), 0)); + std::fs::write(&stale, "stale").expect("seed stale temporary"); + + write_owner_only_atomic(&env_file, b"MOSTRO_NSEC_PRIVKEY=nsec1...\n").expect("write"); + + assert_eq!( + std::fs::read_to_string(&env_file).expect("read back"), + "MOSTRO_NSEC_PRIVKEY=nsec1...\n" + ); + assert_eq!( + std::fs::read_to_string(&stale).expect("read stale"), + "stale", + "the stale temporary must not be written through" + ); + } +} diff --git a/src/config/test_support.rs b/src/config/test_support.rs new file mode 100644 index 00000000..98c2e8f2 --- /dev/null +++ b/src/config/test_support.rs @@ -0,0 +1,90 @@ +//! Shared helpers for the tests in this module that touch the filesystem. +//! +//! Collected here rather than repeated per test module so that how a test +//! temporary directory comes into being is decided in one place. These tests +//! are about permissions, and on a shared CI host `/tmp` is world-writable, so +//! the answer matters: a predictable name is fine, running inside a directory +//! this process did not create is not. + +use std::path::{Path, PathBuf}; + +/// A fresh, owner-only temporary directory named after the calling module and +/// a per-test tag. +/// +/// The name stays deterministic (module, tag and the pid) so repeated runs +/// reuse the same path instead of piling up under `/tmp`. Creation is a plain +/// non-recursive `mkdir`, which fails if anything already occupies the path — +/// including a symlink another local account planted between the cleanup below +/// and this call. Failing the test is the point: the alternative is a +/// permissions test that quietly runs inside someone else's directory. +pub(crate) fn temp_dir(module: &str, tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("mostro-{module}-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + + let mut builder = std::fs::DirBuilder::new(); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + builder.mode(0o700); + } + builder.create(&dir).unwrap_or_else(|e| { + panic!( + "could not create the test directory {}: {e}. It is never reused when something \ + already occupies the path — remove whatever is there and rerun.", + dir.display() + ) + }); + dir +} + +/// The path's permission bits, or `None` on platforms that have none. +/// +/// `Option` rather than `u32` so assertions compile everywhere and the tests +/// that only care about content stay portable; the mode assertions read +/// `Some(0o600)`. +#[cfg(unix)] +pub(crate) fn mode_of(path: &Path) -> Option { + use std::os::unix::fs::PermissionsExt; + + let metadata = + std::fs::metadata(path).unwrap_or_else(|e| panic!("stat {}: {e}", path.display())); + Some(metadata.permissions().mode() & 0o777) +} + +/// Non-Unix platforms have no POSIX permission bits to report. +#[cfg(not(unix))] +pub(crate) fn mode_of(_path: &Path) -> Option { + None +} + +/// Assert a path's permission bits, where the platform has any. +/// +/// A no-op elsewhere, so the tests around it — that a file is created, refused +/// or replaced — stay portable instead of being compiled only on Unix. +#[cfg(unix)] +pub(crate) fn assert_mode(path: &Path, expected: u32) { + assert_eq!( + mode_of(path), + Some(expected), + "unexpected mode on {}", + path.display() + ); +} + +/// Non-Unix platforms have no POSIX permission bits to assert on. +#[cfg(not(unix))] +pub(crate) fn assert_mode(_path: &Path, _expected: u32) {} + +/// Loosen (or otherwise set) a path's permission bits, where the platform has +/// any. A no-op elsewhere. +#[cfg(unix)] +pub(crate) fn set_mode(path: &Path, mode: u32) { + use std::os::unix::fs::PermissionsExt; + + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)) + .unwrap_or_else(|e| panic!("set mode {mode:04o} on {}: {e}", path.display())); +} + +/// Non-Unix platforms have no POSIX permission bits to set. +#[cfg(not(unix))] +pub(crate) fn set_mode(_path: &Path, _mode: u32) {} diff --git a/src/config/util.rs b/src/config/util.rs index 635a5ce3..4176fb25 100644 --- a/src/config/util.rs +++ b/src/config/util.rs @@ -2,7 +2,10 @@ /// This module provides utility functions for the config module. /// It includes functions to initialize the default settings directory and create a settings file from the template if it doesn't exist. /// It also includes functions to add a trailing slash to a path if it doesn't already have one. -use crate::config::constants::{ENV_FILENAME, MAX_DEV_FEE_PERCENTAGE, MIN_DEV_FEE_PERCENTAGE}; +use crate::config::constants::{ + DB_FILENAME, ENV_FILENAME, MAX_DEV_FEE_PERCENTAGE, MIN_DEV_FEE_PERCENTAGE, SETTINGS_FILENAME, +}; +use crate::config::permissions::{create_owner_only, create_settings_dir}; use crate::config::secret::read_nsec_env_var; use crate::config::wizard; use crate::config::{init_mostro_settings, Settings}; @@ -13,8 +16,6 @@ use std::io::IsTerminal; use std::path::PathBuf; use zeroize::Zeroizing; -const DB_FILENAME: &str = "mostro.db"; - /// Loads the optional `/.env` file so that values placed there /// become available through `std::env::var`. Variables already set in the /// process environment take precedence and are never overwritten. @@ -132,7 +133,13 @@ fn validate_cashu_settings( /// Initialize the default settings directory and create a settings file from the template if it doesn't exist. /// Checks if the directory already exists, and if not, creates it and writes the template file. /// If a custom config path is provided, it uses that instead of the default `~/.mostro` directory. -pub fn init_configuration_file(config_path: Option) -> Result<(), MostroError> { +/// +/// Returns the settings directory that was used. The caller needs it because +/// the files in there — `settings.toml` and the optional `.env`, both carrying +/// `nsec_privkey` in plaintext — are checked for over-broad permissions at +/// startup, and the path is otherwise not recoverable from the loaded +/// `Settings`. +pub fn init_configuration_file(config_path: Option) -> Result { let settings_dir = if let Some(user_path) = config_path { PathBuf::from(user_path) } else { @@ -147,15 +154,14 @@ pub fn init_configuration_file(config_path: Option) -> Result<(), Mostro // Check if /.mostro directory exists if !settings_dir.exists() { - std::fs::create_dir_all(&settings_dir) - .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; + create_settings_dir(&settings_dir)?; } // Load `/.env` so MOSTRO_NSEC_PRIVKEY (and any future env // overrides) can be read from it. Real env vars keep precedence. load_env_file(&settings_dir); - let config_file_path = settings_dir.join("settings.toml"); + let config_file_path = settings_dir.join(SETTINGS_FILENAME); if !config_file_path.exists() { let mut settings = if std::io::stdin().is_terminal() { @@ -163,8 +169,7 @@ pub fn init_configuration_file(config_path: Option) -> Result<(), Mostro wizard::run_setup_menu(&settings_dir, &config_file_path)? } else { // Non-interactive (Docker, CI, systemd): copy template and exit - std::fs::write(&config_file_path, include_bytes!("../../settings.tpl.toml")) - .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; + create_owner_only(&config_file_path, include_bytes!("../../settings.tpl.toml"))?; println!( "Created settings file from template at {} - Edit it to configure your Mostro instance", config_file_path.display() @@ -176,7 +181,7 @@ pub fn init_configuration_file(config_path: Option) -> Result<(), Mostro validate_mostro_settings(&settings)?; init_mostro_settings(settings)?; tracing::info!("Settings correctly loaded!"); - return Ok(()); + return Ok(settings_dir); } // Read the file content into a zeroizing buffer so TOML plaintext is wiped @@ -205,7 +210,7 @@ pub fn init_configuration_file(config_path: Option) -> Result<(), Mostro tracing::info!("Settings correctly loaded!"); - Ok(()) + Ok(settings_dir) } #[cfg(test)] @@ -497,11 +502,7 @@ mod env_file_tests { use super::*; fn temp_dir(tag: &str) -> std::path::PathBuf { - let dir = - std::env::temp_dir().join(format!("mostro-config-util-{tag}-{}", std::process::id())); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).expect("create temp dir"); - dir + crate::config::test_support::temp_dir("config-util", tag) } #[test] @@ -542,11 +543,7 @@ mod init_configuration_file_tests { use super::*; fn temp_config_dir(tag: &str) -> std::path::PathBuf { - let dir = - std::env::temp_dir().join(format!("mostro-init-config-{tag}-{}", std::process::id())); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).expect("create temp dir"); - dir + crate::config::test_support::temp_dir("init-config", tag) } // NOTE: the success path (valid settings.toml) calls diff --git a/src/config/wizard.rs b/src/config/wizard.rs index e20f0bda..8e7cd3ef 100644 --- a/src/config/wizard.rs +++ b/src/config/wizard.rs @@ -1,4 +1,3 @@ -use std::io::Write; use std::path::{Path, PathBuf}; use dialoguer::{Confirm, Input, Password, Select}; @@ -8,7 +7,8 @@ use nostr_sdk::prelude::*; use secrecy::{ExposeSecret, SecretString}; use zeroize::Zeroizing; -use super::constants::{ENV_FILENAME, NSEC_ENV_VAR}; +use super::constants::{DB_FILENAME, ENV_FILENAME, NSEC_ENV_VAR}; +use super::permissions::{create_owner_only, write_owner_only_atomic}; use super::settings::Settings; use super::types::{ DatabaseSettings, LightningSettings, MostroSettings, NostrSettings, RpcSettings, @@ -43,8 +43,7 @@ pub fn run_setup_menu( Ok(settings) } _ => { - std::fs::write(config_file_path, TEMPLATE_BYTES) - .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; + create_owner_only(config_file_path, TEMPLATE_BYTES)?; println!( "Created settings file from template at {} - Edit it to configure your Mostro instance", config_file_path.display() @@ -79,38 +78,13 @@ fn run_setup_wizard(settings_dir: &Path, config_file_path: &Path) -> Result` to the given path with 0o600 permissions on Unix. +/// Serialize `settings` and create `config_file_path` from it, owner-only. /// -/// `OpenOptionsExt::mode(0o600)` only applies when the file is created, so for -/// preexisting files we must explicitly tighten permissions after opening to -/// avoid leaving a previously-broader mode in place. -fn write_env_file(path: &Path, nsec: &str) -> Result<(), MostroError> { - #[cfg(unix)] - let file = { - use std::os::unix::fs::OpenOptionsExt; - std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(0o600) - .open(path) - }; - #[cfg(not(unix))] - let file = { - std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(path) - }; - let mut file = file.map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; - - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let permissions = std::fs::Permissions::from_mode(0o600); - file.set_permissions(permissions) - .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; - } +/// Shares `create_owner_only` with the manual template copy here and the +/// non-interactive one in `config::util`, so every path that brings an initial +/// `settings.toml` into existence gets the same mode and the same refusal to +/// follow a symlink — this file is about to hold `nsec_privkey`. +fn save_settings(config_file_path: &Path, settings: &Settings) -> Result<(), MostroError> { + let toml_content = Zeroizing::new( + toml::to_string_pretty(settings) + .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?, + ); + create_owner_only(config_file_path, toml_content.as_bytes()) +} - writeln!(file, "{}={}", NSEC_ENV_VAR, nsec) - .map_err(|e| MostroInternalErr(ServiceError::IOError(e.to_string())))?; - Ok(()) +/// Write `MOSTRO_NSEC_PRIVKEY=` to the given path, owner-only (`0600` on +/// Unix), replacing whatever is already there. +/// +/// Goes through `write_owner_only_atomic` rather than opening the path +/// directly. `.env` holds the same `nsec_privkey` as `settings.toml` and, in +/// the wizard flow, is written first — so opening it with +/// `create(true).truncate(true)` would follow a symlink another local account +/// planted in the settings directory, truncating its target and resetting it +/// to `0600`. That is the one thing `create_owner_only` exists to prevent for +/// `settings.toml`, and this file is worth exactly as much. `create_owner_only` +/// itself cannot serve here: it refuses a path that already exists, and +/// rewriting an existing `.env` is a supported thing to do. +/// +/// The line goes through a `Zeroizing` buffer so the plaintext nsec is wiped +/// once handed off, the way `save_settings` treats the serialized TOML. +fn write_env_file(path: &Path, nsec: &str) -> Result<(), MostroError> { + let line = Zeroizing::new(format!("{}={}\n", NSEC_ENV_VAR, nsec)); + write_owner_only_atomic(path, line.as_bytes()) } fn prompt_mostro_settings() -> Result { @@ -386,6 +355,7 @@ fn expand_tilde(path: &str) -> PathBuf { #[cfg(test)] mod tests { use super::*; + use crate::config::test_support::{assert_mode, set_mode}; #[test] fn test_validate_nsec_valid() { @@ -433,10 +403,7 @@ mod tests { } fn temp_dir(tag: &str) -> PathBuf { - let dir = std::env::temp_dir().join(format!("mostro-wizard-{tag}-{}", std::process::id())); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).expect("create temp dir"); - dir + crate::config::test_support::temp_dir("wizard", tag) } #[test] @@ -470,16 +437,6 @@ mod tests { assert!(resolve_file_path("/definitely/not/here.macaroon").is_err()); } - #[cfg(unix)] - fn mode_of(path: &Path) -> u32 { - use std::os::unix::fs::PermissionsExt; - std::fs::metadata(path) - .expect("stat env file") - .permissions() - .mode() - & 0o777 - } - #[test] fn test_write_env_file_creates_file_with_owner_only_permissions() { let dir = temp_dir("env-new"); @@ -489,8 +446,7 @@ mod tests { let contents = std::fs::read_to_string(&env_path).expect("read env file"); assert_eq!(contents, format!("{}=nsec1testvalue\n", NSEC_ENV_VAR)); - #[cfg(unix)] - assert_eq!(mode_of(&env_path), 0o600); + assert_mode(&env_path, 0o600); } #[test] @@ -498,19 +454,136 @@ mod tests { let dir = temp_dir("env-existing"); let env_path = dir.join(ENV_FILENAME); std::fs::write(&env_path, "OLD=stale\n").expect("seed env file"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&env_path, std::fs::Permissions::from_mode(0o644)) - .expect("loosen permissions"); - } + set_mode(&env_path, 0o644); write_env_file(&env_path, "nsec1replaced").expect("rewrite env file"); - // Old content truncated, permissions tightened back to 0600. + // Old content replaced, permissions tightened back to 0600. let contents = std::fs::read_to_string(&env_path).expect("read env file"); assert_eq!(contents, format!("{}=nsec1replaced\n", NSEC_ENV_VAR)); - #[cfg(unix)] - assert_eq!(mode_of(&env_path), 0o600); + assert_mode(&env_path, 0o600); + } + + #[cfg(unix)] + #[test] + fn test_write_env_file_does_not_write_the_nsec_through_a_planted_symlink() { + let dir = temp_dir("env-symlink"); + let victim = dir.join("victim"); + std::fs::write(&victim, "victim contents").expect("seed victim"); + set_mode(&victim, 0o644); + + // The wizard writes `.env` before `settings.toml`, so on a settings + // directory another local account can write to this is the first shot + // it gets at the nsec. + let env_path = dir.join(ENV_FILENAME); + std::os::unix::fs::symlink(&victim, &env_path).expect("plant symlink"); + + write_env_file(&env_path, "nsec1secret").expect("write env file"); + + assert_eq!( + std::fs::read_to_string(&victim).expect("read victim"), + "victim contents", + "the nsec must not be written through the link" + ); + assert_mode(&victim, 0o644); + assert_mode(&env_path, 0o600); + } +} + +#[cfg(test)] +mod save_settings_tests { + use super::*; + use crate::config::test_support::{assert_mode, set_mode}; + + fn temp_root(tag: &str) -> PathBuf { + crate::config::test_support::temp_dir("wizard-save", tag) + } + + fn sample_settings() -> Settings { + Settings { + database: DatabaseSettings::default(), + lightning: LightningSettings::default(), + nostr: NostrSettings { + nsec_privkey: "nsec13as48eum93hkg7plv526r9gjpa0uc52zysqm93pmnkca9e69x6tsdjmdxd" + .to_string() + .into(), + relays: vec!["wss://relay.mostro.network".to_string()], + }, + mostro: MostroSettings::default(), + rpc: RpcSettings::default(), + expiration: None, + anti_abuse_bond: None, + cashu: None, + price: None, + } + } + + #[test] + fn wizard_save_creates_the_file_owner_only() { + let root = temp_root("ok"); + let config_file = root.join("settings.toml"); + save_settings(&config_file, &sample_settings()).expect("save settings"); + assert_mode(&config_file, 0o600); + let written = std::fs::read_to_string(&config_file).expect("read back"); + assert!(written.contains("nsec_privkey")); + } + + #[test] + fn wizard_save_refuses_a_preexisting_file() { + let root = temp_root("existing"); + let config_file = root.join("settings.toml"); + std::fs::write(&config_file, "operator contents").expect("seed file"); + assert!(save_settings(&config_file, &sample_settings()).is_err()); + assert_eq!( + std::fs::read_to_string(&config_file).expect("read back"), + "operator contents" + ); + } + + #[cfg(unix)] + #[test] + fn wizard_save_leaves_a_symlink_target_untouched() { + let root = temp_root("symlink"); + let victim = root.join("victim"); + std::fs::write(&victim, "victim contents").expect("seed victim"); + set_mode(&victim, 0o644); + + let config_file = root.join("settings.toml"); + std::os::unix::fs::symlink(&victim, &config_file).expect("plant symlink"); + + // The nsec must not be written through a link another local account + // could have planted in the settings directory. + assert!(save_settings(&config_file, &sample_settings()).is_err()); + assert_eq!( + std::fs::read_to_string(&victim).expect("read back"), + "victim contents" + ); + assert_mode(&victim, 0o644); + } + + #[test] + fn manual_template_copy_is_owner_only() { + let root = temp_root("template"); + let config_file = root.join("settings.toml"); + // The manual branch of `run_setup_menu` writes TEMPLATE_BYTES through + // the same primitive; the menu itself needs a terminal, so exercise + // the write it performs. + create_owner_only(&config_file, TEMPLATE_BYTES).expect("write template"); + assert_mode(&config_file, 0o600); + } + + #[cfg(unix)] + #[test] + fn manual_template_copy_refuses_a_symlink() { + let root = temp_root("template-symlink"); + let victim = root.join("victim"); + std::fs::write(&victim, "victim contents").expect("seed victim"); + let linked = root.join("linked.toml"); + std::os::unix::fs::symlink(&victim, &linked).expect("plant symlink"); + assert!(create_owner_only(&linked, TEMPLATE_BYTES).is_err()); + assert_eq!( + std::fs::read_to_string(&victim).expect("read back"), + "victim contents" + ); } } diff --git a/src/main.rs b/src/main.rs index 6a636497..4c8578e3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,8 +24,10 @@ pub type Result> = std::result::Result; use crate::app::context::AppContext; use crate::app::{run, run_cashu}; use crate::cli::settings_init; +use crate::config::constants::{DB_FILENAME, ENV_FILENAME, SETTINGS_FILENAME}; use crate::config::{ - get_db_pool, Settings, DB_POOL, LN_STATUS, MESSAGE_QUEUES, MOSTRO_CONFIG, NOSTR_CLIENT, + get_db_pool, permissions, Settings, DB_POOL, LN_STATUS, MESSAGE_QUEUES, MOSTRO_CONFIG, + NOSTR_CLIENT, }; use crate::db::find_held_invoices; use crate::lightning::LnStatus; @@ -59,7 +61,30 @@ async fn main() -> Result<()> { .init(); // Init MOSTRO_SETTINGS oncelock with all settings variables from TOML file - settings_init()?; + let settings_dir = settings_init()?; + + // `settings.toml` carries `nsec_privkey` in plaintext unless it was moved + // to `/.env`, which carries it instead — either way the key + // that signs every event this instance publishes sits in one of these two + // files. Both are what the deployment guides have operators create by hand + // with `cp`, `curl` or an editor, all of which apply the umask, so a `0644` + // settings file is the normal accident. `mostro.db` is the same directory + // and the same accident: SQLite creates it under the umask, and it holds + // the trade history, the disputes and the hold-invoice preimages. Checked + // in both Lightning and Cashu mode, unlike the macaroon further down, + // because none of the three is specific to either. + // + // The database is checked before it exists on a first boot, which is + // silent; a fresh install is not the case this catches. The one it does + // catch is the deployment that has been running on a `0755` settings + // directory since before any of this existed. + for (name, label) in [ + (SETTINGS_FILENAME, "Mostro settings file"), + (ENV_FILENAME, "Mostro env file"), + (DB_FILENAME, "Mostro database"), + ] { + permissions::warn_if_other_accessible(&settings_dir.join(name), label); + } // Build and install the multi-source price manager (spec §9 Phase 1). // Done immediately after settings load so every later subsystem @@ -221,6 +246,17 @@ async fn main() -> Result<()> { return run_cashu(ctx).await; } + // The admin macaroon is spend-capable: whoever reads it controls the node, + // including the funds escrowed in the hold invoices mostrod manages. Say so + // once per boot when the file is left reachable by other local accounts — + // the documented deployment flows copy it around, and a bad mode is + // otherwise invisible until it is abused. + let macaroon_file = &Settings::get_ln().lnd_macaroon_file; + permissions::warn_if_other_accessible( + std::path::Path::new(macaroon_file), + "LND admin macaroon", + ); + let mut ln_client = LndConnector::new().await?; let ln_status = ln_client.get_node_info().await?; let ln_status = LnStatus::from_get_info_response(ln_status);