From 7106dfe330ffa1a41de903479e89dfc60cb3dad0 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Mon, 13 Jul 2026 17:29:36 +0200 Subject: [PATCH] fix(vault): provision a master key on upgrade instead of refusing to boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A node with a legacy plaintext nsec but no ROUTSTR_SECRET_KEY refused to boot: bootstrap_secrets raised and vault.encrypt required the env key. That turned encryption at rest into a hard breaking change on auto-upgrade. Encryption stays mandatory — the nsec is never persisted in plaintext — but key custody becomes flexible. When no ROUTSTR_SECRET_KEY is set, encrypt() generates a Fernet key, writes it owner-only (0600) to a key file, and prints a one-time back-it-up notice, so an upgrading node keeps running. The read path stays strict: decrypt()/get_fernet() never mint a key (a fresh key could not match existing ciphertext) and fail fast with the generation command when none is configured. A malformed env key still fails fast rather than silently self-provisioning a different key. The key file defaults beside the SQLite database (ROUTSTR_SECRET_KEY_FILE overrides), so it rides whatever volume already persists the data instead of a working-directory path a container recreate would drop. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 1 + routstr/core/settings.py | 14 +- routstr/core/vault.py | 167 ++++++++++++++++++--- tests/integration/test_secret_bootstrap.py | 40 +++-- tests/unit/test_vault.py | 135 ++++++++++++++++- 5 files changed, 315 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index 0d6c7236..903d526e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ __pycache__ .env keys.db +routstr_secret.key wallet.sqlite3 # Python build artifacts diff --git a/routstr/core/settings.py b/routstr/core/settings.py index 542aa80e..133496f6 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -516,16 +516,10 @@ async def bootstrap_secrets(db_session: AsyncSession) -> None: legacy_nsec = _legacy_plaintext(raw_blob, "NSEC", "nsec") if legacy_nsec: # The node has a Nostr identity to protect. Encryption at rest is - # mandatory: without a key we fail fast (clear, actionable boot - # error) rather than silently persisting the nsec in plaintext. - if not os.environ.get("ROUTSTR_SECRET_KEY"): - raise RuntimeError( - "An nsec is configured but ROUTSTR_SECRET_KEY is not set. " - "The key is required to encrypt the Nostr identity at rest. " - "Generate one and set it in the environment:\n" - ' python -c "from cryptography.fernet import Fernet; ' - 'print(Fernet.generate_key().decode())"' - ) + # mandatory, but a missing key is provisioned, not fatal: + # vault.encrypt generates and persists a master key (with a loud + # one-time operator notice) when none was supplied, so an upgrading + # node keeps running. The nsec is never persisted in plaintext. secret.encrypted_nsec = vault.encrypt(legacy_nsec) secret.nsec_managed = True settings.nsec = legacy_nsec diff --git a/routstr/core/vault.py b/routstr/core/vault.py index 8bc04ff3..674d74c2 100644 --- a/routstr/core/vault.py +++ b/routstr/core/vault.py @@ -4,15 +4,22 @@ Thin wrapper over ``cryptography`` so nothing else in the codebase touches Fernet/scrypt/HMAC directly: - :func:`encrypt`/:func:`decrypt` — Fernet symmetric encryption, keyed by the - mandatory ``ROUTSTR_SECRET_KEY``. Ciphertext is self-describing (``fernet:v1:`` - prefix) so a value can be told apart from legacy plaintext and so reading it - under the wrong key surfaces as a hard error rather than silent corruption. + mandatory master key. Ciphertext is self-describing (``fernet:v1:`` prefix) so + a value can be told apart from legacy plaintext and so reading it under the + wrong key surfaces as a hard error rather than silent corruption. - :func:`hash_password`/:func:`verify_password` — salted scrypt hashing. This is - *key-independent*: it never reads ``ROUTSTR_SECRET_KEY``, so password login and - the recovery script keep working even when the key is missing. + *key-independent*: it never reads the master key, so password login and the + recovery script keep working even when the key is missing. -A missing or malformed ``ROUTSTR_SECRET_KEY`` fails fast with the generation -command in the message. +Key custody is flexible but encryption is not optional. The key comes from the +``ROUTSTR_SECRET_KEY`` env var, else a persisted key file +(``ROUTSTR_SECRET_KEY_FILE``, defaulting beside the SQLite database so it persists +on the same volume as the data); when neither is set, :func:`encrypt` generates +one to the key file and prints a one-time notice, so an existing node upgrades +without breaking instead of refusing to boot. Reading is strict — :func:`decrypt` +never generates a key (a new key could not match existing ciphertext) and fails +fast with the generation command when none is configured. A malformed +``ROUTSTR_SECRET_KEY`` is an operator error and always fails fast. """ import base64 @@ -20,8 +27,11 @@ import hashlib import hmac import os import secrets +from pathlib import Path from cryptography.fernet import Fernet +from sqlalchemy.engine import make_url +from sqlalchemy.exc import ArgumentError _PREFIX = "fernet:v1:" _GEN_COMMAND = ( @@ -29,6 +39,15 @@ _GEN_COMMAND = ( 'print(Fernet.generate_key().decode())"' ) +# Where an auto-generated master key is persisted when the operator supplies no +# ``ROUTSTR_SECRET_KEY``. Defaults beside the SQLite database so it rides whatever +# volume already persists the data (a container recreate would otherwise generate +# a fresh key and be unable to decrypt existing secrets); falls back to the +# working directory when the DB location is unknown. Override the exact path with +# ``ROUTSTR_SECRET_KEY_FILE``. +_KEY_FILE_ENV = "ROUTSTR_SECRET_KEY_FILE" +_DEFAULT_KEY_FILE = "routstr_secret.key" + # Minimum admin-password length, enforced wherever a password is set/changed # (admin endpoints + the recovery script) so the policy lives in one place. MIN_PASSWORD_LENGTH = 8 @@ -41,19 +60,106 @@ _SCRYPT_DKLEN = 32 _SCRYPT_SALT_BYTES = 16 -def _require_secret_key() -> str: - key = os.environ.get("ROUTSTR_SECRET_KEY") - if not key: - raise RuntimeError( - "ROUTSTR_SECRET_KEY is not set. It is required to encrypt secrets at " - "rest. Generate one with:\n " + _GEN_COMMAND - ) +def _database_dir() -> Path | None: + """Directory of the SQLite database file, or ``None`` when it has no on-disk + location (a non-SQLite URL or ``:memory:``). + + Read from ``DATABASE_URL`` at call time and parsed here rather than importing + ``routstr.core.db`` — that module builds the engine at import, which the + crypto layer must not drag in. Mirrors db.py's ``DATABASE_URL`` default. + """ + url_str = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") + try: + url = make_url(url_str) + except ArgumentError: + return None + if url.get_backend_name() != "sqlite" or not url.database: + return None + if url.database == ":memory:": + return None + return Path(url.database).parent + + +def _key_file_path() -> Path: + """Where the auto-generated master key is read from / written to. + + ``ROUTSTR_SECRET_KEY_FILE`` wins; otherwise the key sits beside the SQLite + database so it persists on the same volume as the data, falling back to the + working directory when the DB location is unknown. + """ + override = os.environ.get(_KEY_FILE_ENV) + if override: + return Path(override) + directory = _database_dir() + return (directory or Path()) / _DEFAULT_KEY_FILE + + +def _read_key_file(path: Path) -> str | None: + try: + stored = path.read_text().strip() + except OSError: + return None + return stored or None + + +def _load_secret_key() -> str | None: + """The configured key without provisioning: env var, then the key file.""" + return os.environ.get("ROUTSTR_SECRET_KEY") or _read_key_file(_key_file_path()) + + +def _warn_generated_key(path: Path, key: str) -> None: + # stdout, not the logger: the operator must see this once (e.g. in + # ``docker compose logs``), but it must never be persisted into the on-disk + # log files the logger also writes. Mirrors the generated-admin-password + # notice so an upgrade cannot silently create an unbacked key. + print( + "No ROUTSTR_SECRET_KEY was set; generated one to encrypt node secrets at " + f"rest and saved it to {path}.\n" + "!! BACK UP THIS FILE. If it is lost, the encrypted secrets cannot be " + "recovered and will have to be re-entered.\n" + "To manage the key yourself (e.g. from a secrets manager) set it in the " + f"environment instead:\n ROUTSTR_SECRET_KEY={key}", + flush=True, + ) + + +def _generate_and_persist_key(path: Path) -> str: + """Generate a Fernet key, persist it owner-only, and warn once.""" + key = Fernet.generate_key().decode() + path.parent.mkdir(parents=True, exist_ok=True) + try: + fd = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + except FileExistsError: + # Another worker won the race and wrote the key first; adopt theirs + # rather than clobber a key that secrets may already be encrypted under. + existing = _read_key_file(path) + if existing: + return existing + raise + with os.fdopen(fd, "w") as handle: + handle.write(key) + _warn_generated_key(path, key) return key -def get_fernet() -> Fernet: - """Build a :class:`Fernet` from ``ROUTSTR_SECRET_KEY`` (fails fast).""" - key = _require_secret_key() +def ensure_secret_key() -> str: + """Return the master key, provisioning one if the operator supplied none. + + Precedence: the ``ROUTSTR_SECRET_KEY`` env var, then the persisted key file, + otherwise a freshly generated key written to the key file (with a one-time + operator notice). This keeps encryption at rest mandatory while letting an + existing node upgrade without setting a key first. A malformed env key is + left to fail at :func:`get_fernet` — it is an operator error, not an unset + key, so it must not trigger silent self-provisioning. + """ + env_key = os.environ.get("ROUTSTR_SECRET_KEY") + if env_key: + return env_key + path = _key_file_path() + return _read_key_file(path) or _generate_and_persist_key(path) + + +def _fernet_from_key(key: str) -> Fernet: try: return Fernet(key.encode()) except (ValueError, TypeError) as exc: @@ -63,10 +169,31 @@ def get_fernet() -> Fernet: ) from exc +def get_fernet() -> Fernet: + """Build a :class:`Fernet` from the configured key (env var or key file). + + Strict: this never generates a key, so already-encrypted ciphertext is never + shadowed by a fresh key. A read with no key configured fails fast with the + generation command. + """ + key = _load_secret_key() + if not key: + raise RuntimeError( + "ROUTSTR_SECRET_KEY is not set. It is required to encrypt secrets at " + "rest. Generate one with:\n " + _GEN_COMMAND + ) + return _fernet_from_key(key) + + def encrypt(plaintext: str) -> str: - """Encrypt ``plaintext`` into a self-describing ``fernet:v1:`` token.""" - token = get_fernet().encrypt(plaintext.encode()).decode() - return _PREFIX + token + """Encrypt ``plaintext`` into a self-describing ``fernet:v1:`` token. + + Provisions a master key (env var, key file, or a freshly generated one) so an + upgrading node never has to set one before its first secret is stored; the + value is always encrypted, never persisted in plaintext. + """ + fernet = _fernet_from_key(ensure_secret_key()) + return _PREFIX + fernet.encrypt(plaintext.encode()).decode() def is_encrypted(value: str) -> bool: diff --git a/tests/integration/test_secret_bootstrap.py b/tests/integration/test_secret_bootstrap.py index 18899dea..4a177ee2 100644 --- a/tests/integration/test_secret_bootstrap.py +++ b/tests/integration/test_secret_bootstrap.py @@ -10,6 +10,7 @@ silently corrupting state. import json from contextlib import asynccontextmanager +from pathlib import Path from typing import Any, AsyncGenerator import pytest @@ -166,25 +167,46 @@ async def test_fail_fast_when_nsec_encrypted_with_different_key( await bootstrap_secrets(integration_session) -# --- encryption is mandatory: a node with an nsec needs ROUTSTR_SECRET_KEY ----- +# --- encryption is mandatory, key custody is not: upgrade without a key -------- @pytest.mark.asyncio -async def test_legacy_nsec_without_secret_key_fails_fast( +async def test_legacy_nsec_without_secret_key_generates_and_encrypts( clean_secret_env: None, integration_session: AsyncSession, monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], ) -> None: - # A node with a Nostr identity must not boot without a key to encrypt it: - # encryption at rest is mandatory, not opt-in. The failure names the missing - # key and hands over the generation command rather than crashing opaquely or - # silently persisting the nsec in plaintext. No env/blob copy is dropped — the - # node refuses until the operator sets the key. + # A node upgrading with a legacy plaintext NSEC but no ROUTSTR_SECRET_KEY must + # NOT break. Encryption at rest stays mandatory (the nsec is never persisted + # in plaintext), but the key custody is flexible: bootstrap generates a master + # key, persists it to the key file, warns loudly, and encrypts the identity — + # so the node keeps running instead of refusing to boot. monkeypatch.delenv("ROUTSTR_SECRET_KEY", raising=False) + key_file = tmp_path / "routstr_secret.key" + monkeypatch.setenv("ROUTSTR_SECRET_KEY_FILE", str(key_file)) monkeypatch.setenv("NSEC", NSEC_HEX) - with pytest.raises(RuntimeError, match="ROUTSTR_SECRET_KEY"): - await bootstrap_secrets(integration_session) + await bootstrap_secrets(integration_session) + + # A master key was generated and persisted... + assert key_file.exists() + # ...the nsec is encrypted at rest under it, never stored in plaintext... + secret = await get_secret(integration_session) + assert secret.encrypted_nsec is not None + assert vault.is_encrypted(secret.encrypted_nsec) is True + assert vault.decrypt(secret.encrypted_nsec) == NSEC_HEX + assert secret.nsec_managed is True + # ...the node holds the live identity (npub derived from it)... + assert settings.nsec == NSEC_HEX + assert settings.npub == derive_npub_from_nsec(NSEC_HEX) + # ...and the operator is loudly told a key was generated and must be backed up + # (path + value shown) so an upgrade cannot silently create an unbacked key. + out = capsys.readouterr().out + assert str(key_file) in out + assert key_file.read_text().strip() in out + assert "BACK UP" in out.upper() # --- boot ordering: rescue legacy blob secrets before they are stripped ---- diff --git a/tests/unit/test_vault.py b/tests/unit/test_vault.py index 21621d54..52f1d9fd 100644 --- a/tests/unit/test_vault.py +++ b/tests/unit/test_vault.py @@ -14,6 +14,8 @@ builds on, independent of any database or app wiring: command in the message. """ +from pathlib import Path + import pytest from cryptography.fernet import InvalidToken @@ -124,12 +126,17 @@ def test_password_hashing_is_key_independent( # --- fail-fast on missing/malformed key ------------------------------------ -def test_missing_key_fails_fast_with_generation_command( - monkeypatch: pytest.MonkeyPatch, +def test_decrypt_without_any_key_fails_fast_with_generation_command( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: + # Reading secrets is strict: with no key in env AND no key file, decrypt + # fails fast with the generation command rather than silently minting a new + # key (a fresh key could never match already-encrypted ciphertext). Only the + # encrypt path auto-provisions; the read path never does. monkeypatch.delenv("ROUTSTR_SECRET_KEY", raising=False) + monkeypatch.setenv("ROUTSTR_SECRET_KEY_FILE", str(tmp_path / "absent.key")) with pytest.raises(RuntimeError) as exc: - vault.encrypt("x") + vault.decrypt("fernet:v1:not-real-ciphertext") msg = str(exc.value) assert "ROUTSTR_SECRET_KEY" in msg assert "Fernet.generate_key" in msg @@ -139,3 +146,125 @@ def test_malformed_key_fails_fast(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("ROUTSTR_SECRET_KEY", "not-a-valid-fernet-key") with pytest.raises(RuntimeError): vault.encrypt("x") + + +def test_malformed_env_key_does_not_self_provision( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # A malformed env key is an operator mistake, not an unset key: it must fail + # loudly, never silently generate a different key to a file (which would hide + # the mistake and could brick secrets the operator meant to key differently). + monkeypatch.setenv("ROUTSTR_SECRET_KEY", "not-a-valid-fernet-key") + key_file = tmp_path / "routstr_secret.key" + monkeypatch.setenv("ROUTSTR_SECRET_KEY_FILE", str(key_file)) + with pytest.raises(RuntimeError): + vault.encrypt("x") + assert not key_file.exists() + + +# --- auto-provisioned key file (non-breaking upgrade path) ----------------- + + +@pytest.fixture +def generated_key_file(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: + """No env key; the key file points at a fresh, empty tmp location. + + Exercises what an existing node hits when it upgrades without setting + ROUTSTR_SECRET_KEY: the master key is auto-generated and persisted here so + boot does not break, while secrets are still never written in plaintext. + """ + monkeypatch.delenv("ROUTSTR_SECRET_KEY", raising=False) + key_file = tmp_path / "routstr_secret.key" + monkeypatch.setenv("ROUTSTR_SECRET_KEY_FILE", str(key_file)) + return key_file + + +def test_encrypt_without_key_generates_and_persists_key_file( + generated_key_file: Path, +) -> None: + # Encryption at rest stays mandatory, but a missing key is provisioned rather + # than fatal: encrypt generates a key, writes it to the key file (owner-only), + # and the value round-trips — decrypt, still with no env key, reads the same + # file key back. + key_file = generated_key_file + assert not key_file.exists() + + token = vault.encrypt("nsec1secret") + + assert token.startswith("fernet:v1:") + assert key_file.exists() + assert key_file.stat().st_mode & 0o077 == 0 # not group/other-accessible + assert vault.decrypt(token) == "nsec1secret" + + +def test_generated_key_warns_operator_with_path_and_value( + generated_key_file: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # The whole point of auto-generating is that an upgrading operator MUST NOT + # miss it: the notice names the file to back up, prints the key so it can be + # promoted into a secrets manager, and shouts the back-up imperative. + key_file = generated_key_file + vault.encrypt("x") + + out = capsys.readouterr().out + assert "ROUTSTR_SECRET_KEY" in out + assert str(key_file) in out + assert key_file.read_text().strip() in out + assert "BACK UP" in out.upper() + + +def test_existing_key_file_is_reused_and_warns_only_once( + generated_key_file: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # Once the key exists, later encrypts reuse it (never rotate a key that + # secrets were already encrypted under) and stay silent (no repeated notice). + key_file = generated_key_file + vault.encrypt("a") + first_key = key_file.read_text() + capsys.readouterr() # drain the one-time notice + + vault.encrypt("b") + + assert key_file.read_text() == first_key + assert capsys.readouterr().out == "" + + +def test_env_key_takes_precedence_over_key_file( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # An explicit env key wins over a persisted file key (an operator-supplied + # key from a secrets manager overrides the auto-generated one), and the file + # is left untouched. + key_file = tmp_path / "routstr_secret.key" + key_file.write_text(KEY_B) + monkeypatch.setenv("ROUTSTR_SECRET_KEY_FILE", str(key_file)) + monkeypatch.setenv("ROUTSTR_SECRET_KEY", KEY_A) + + token = vault.encrypt("x") + + assert vault.decrypt(token) == "x" # env key (A) decrypts it + monkeypatch.setenv("ROUTSTR_SECRET_KEY", KEY_B) + with pytest.raises(InvalidToken): + vault.decrypt(token) # the file key (B) does not + assert key_file.read_text() == KEY_B # file key never used or overwritten + + +def test_generated_key_defaults_beside_the_database( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # With no env key and no explicit key-file path, the key is provisioned next + # to the SQLite database, so it rides whatever volume already persists the + # data instead of landing in the working directory (where a container + # recreate would lose it and brick decryption). + monkeypatch.delenv("ROUTSTR_SECRET_KEY", raising=False) + monkeypatch.delenv("ROUTSTR_SECRET_KEY_FILE", raising=False) + monkeypatch.chdir(tmp_path) # isolate the working-dir fallback from the repo + db_dir = tmp_path / "data" + db_dir.mkdir() + monkeypatch.setenv("DATABASE_URL", f"sqlite+aiosqlite:///{db_dir}/routstr.db") + + token = vault.encrypt("beside-the-db") + + assert (db_dir / "routstr_secret.key").exists() + assert not (tmp_path / "routstr_secret.key").exists() # not the working dir + assert vault.decrypt(token) == "beside-the-db"