From c04e949c40bdfbd1b7329a1ec5c1bf82cc77a41f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 16 Oct 2026 14:18:41 -0400 Subject: [PATCH] Stored credentials are encrypted at rest (#5927) #5927 "Adopt the family idea secret.Box": the SMTP password and the backup bucket's secret key were kept as plain text in the settings table, so a copy of the database alone could read them. - New sealing.py: AES-GCM under an HKDF-derived key, each value bound to its setting's name. The key is a random file, /var/inkwell/sealing.key (0600), made on first use. It lives on the data volume, not in the database, which is where the signing secret sits by default. - set_settings seals a secret before storing it; get_setting opens it. A value that no longer opens (key file lost) reads as not set, so Settings asks for it again. - At startup, credentials stored before this are sealed in place, so nothing has to be re-entered. A data folder that can't be written never stops the server; saving a password then answers with the reason. - New dependency: cryptography (approved by the operator). Also #1919 "Deprecate / remove the PWA": kept. Its service worker caches only the offline page, so an installed copy always runs the live build. One stale comment in sw.js is corrected. Co-Authored-By: Claude Opus 5.5 --- docs/backups.md | 3 +- docs/public-hosting.md | 7 +- frontend/public/sw.js | 5 +- pyproject.toml | 1 + src/inkwell/app.py | 22 +++++- src/inkwell/sealing.py | 132 ++++++++++++++++++++++++++++++++++++ src/inkwell/settings.py | 35 +++++++++- src/inkwell/settings_api.py | 7 +- tests/test_integration.py | 33 ++++++++- tests/test_sealing.py | 55 +++++++++++++++ 10 files changed, 290 insertions(+), 10 deletions(-) create mode 100644 src/inkwell/sealing.py create mode 100644 tests/test_sealing.py diff --git a/docs/backups.md b/docs/backups.md index 6e072da..e649a84 100644 --- a/docs/backups.md +++ b/docs/backups.md @@ -35,7 +35,8 @@ In practice a day's new backup adds roughly the size of that day's edits and upl to the off-site repository, not the size of the whole file. A backup **never** holds a credential: not the SMTP password, not the bucket's secret -key, and not the server's own signing key. A copy kept off-site cannot be used to +key, and not the server's own signing key. (On the server those two are stored +encrypted, under a key file on the data volume, `/var/inkwell/sealing.key`.) A copy kept off-site cannot be used to reach the server or the bucket it came from. One-hour password reset links are left out too. diff --git a/docs/public-hosting.md b/docs/public-hosting.md index 8bb0408..2400c72 100644 --- a/docs/public-hosting.md +++ b/docs/public-hosting.md @@ -138,8 +138,11 @@ The public address is required because the emailed link has to point somewhere t server can trust. Built from the request instead, a forged `Host` header would mail someone a reset link to a site the forger controls. -The SMTP password is kept in the database and never sent back to the browser; the -field shows only that one is saved. The forgot-password page answers the same way, +The SMTP password is kept in the database encrypted, and never sent back to the +browser; the field shows only that one is saved. The key it is encrypted with is +`/var/inkwell/sealing.key`, on the data volume rather than in the database, so a copy +of the database alone can't read it. Keep the data volume: if that file is lost, the +password reads as not set and has to be entered again. The forgot-password page answers the same way, at the same speed, whether or not an address has an account, and `reset_emails_per_account` (Settings → Security) caps how many reset emails one address can be sent. diff --git a/frontend/public/sw.js b/frontend/public/sw.js index 503da01..5ec5afa 100644 --- a/frontend/public/sw.js +++ b/frontend/public/sw.js @@ -2,10 +2,11 @@ // // This exists so the web app satisfies the PWA install criteria ("Add to Home // Screen") and shows a friendly page when a navigation happens with no network. -// The real offline client is the future Android app (M5); to stay always-fresh +// The offline clients are the desktop and Android apps; to stay always-fresh // and avoid deploy staleness, this SW does NOT cache the app shell, hashed build // assets, or any /api response — everything but the offline fallback goes -// straight to the network. +// straight to the network. So an installed copy is the live web app, never an +// older one (#1919). const CACHE = "inkwell-shell-v1"; const OFFLINE_URL = "/offline.html"; diff --git a/pyproject.toml b/pyproject.toml index 230c15f..a7f06e7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,6 +13,7 @@ dependencies = [ "asyncpg>=0.29", "alembic>=1.13", "bcrypt>=4.0", + "cryptography>=42", "hypercorn>=0.17", ] diff --git a/src/inkwell/app.py b/src/inkwell/app.py index 393020b..013a472 100644 --- a/src/inkwell/app.py +++ b/src/inkwell/app.py @@ -27,7 +27,16 @@ from .responses import json_error, not_found from .retention import run_sweeper from .backup_api import bp as backups_bp from .backup_job import run_scheduler as run_backup_scheduler -from .settings import MAX_BODY_MB, apply_session_ttl, get_public_config, get_setting, load_or_create_secret_key, refresh_live +from .sealing import SealError +from .settings import ( + MAX_BODY_MB, + apply_session_ttl, + get_public_config, + get_setting, + load_or_create_secret_key, + refresh_live, + seal_stored_secrets, +) from .settings_api import bp as settings_bp from .shares_api import bp as shares_bp from .sync import bp as sync_bp, protocol_advertisement @@ -43,6 +52,7 @@ logging.basicConfig( level=os.environ.get("INKWELL_LOG_LEVEL", "INFO").upper(), format="%(asctime)s %(levelname)s %(name)s %(message)s", ) +logger = logging.getLogger(__name__) STATIC_DIR = os.path.join(os.path.dirname(__file__), "static") @@ -125,6 +135,16 @@ def create_app() -> Quart: # hold their registry defaults, which is the correct behaviour for a # server that has not finished starting. await refresh_live(db) + # Credentials stored before sealing existed are sealed now (#5927). A data + # folder that can't be written leaves them as they are; it never stops the + # server starting. + try: + if count := await seal_stored_secrets(db): + await db.commit() + logger.info("sealed %d stored credential(s)", count) + except SealError as exc: + await db.rollback() + logger.warning("stored credentials left unsealed: %s", exc) # Expire old trash in the background (retention.py). One task per process is # correct because the image serves with a single hypercorn worker (Dockerfile); # if that ever gains `--workers`, this needs a lock so N workers don't each diff --git a/src/inkwell/sealing.py b/src/inkwell/sealing.py new file mode 100644 index 0000000..22e0f57 --- /dev/null +++ b/src/inkwell/sealing.py @@ -0,0 +1,132 @@ +"""Credentials at rest: the SMTP password and the backup bucket's secret key (#5927). + +The family's secret.Box (#5185): a stored credential is encrypted under a key the +database does not hold, so a copy of the database (a dump, a copied volume) cannot +read the keys to anything else. + +- **Where the key lives.** A random key in a file under DATA_DIR, made on first use. + Not the signing secret: by default that is kept in the settings table (rule 25, + no env var needed), so a key derived from it would sit beside what it seals. The + data volume and the database volume are separate, and backups take only the + attachment files from DATA_DIR, never this file. +- **Per purpose, per row.** The cipher key is HKDF-derived from the file's key with + a purpose label, and each value is bound to its setting's name, so a sealed value + copied onto another setting does not open there. +- **Unreadable means "enter it again".** If the key file is lost or replaced, a + sealed value no longer opens; it reads as not set and Settings asks for it again. + It never stops the server starting. +""" +from __future__ import annotations + +import base64 +import logging +import os +import secrets +from pathlib import Path + +from cryptography.exceptions import InvalidTag +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.ciphers.aead import AESGCM +from cryptography.hazmat.primitives.kdf.hkdf import HKDF + +from .config import Config + +logger = logging.getLogger(__name__) + +KEY_FILE_NAME = "sealing.key" +# Long and specific on purpose: a password that happened to start with "v1." must not +# read as already sealed. +PREFIX = "inkwell-sealed:v1:" +_PURPOSE = b"inkwell/settings/v1" +_NONCE_BYTES = 12 + + +class SealError(Exception): + """The key file could not be read or made, so nothing can be sealed.""" + + +def key_path() -> Path: + return Config.data_root() / KEY_FILE_NAME + + +def _read_key(path: Path) -> bytes: + raw = base64.urlsafe_b64decode(path.read_bytes().strip()) + if len(raw) != 32: + raise SealError(f"{path} isn't a sealing key.") + return raw + + +def _master(*, create: bool) -> bytes | None: + path = key_path() + try: + return _read_key(path) + except FileNotFoundError: + if not create: + return None + except (OSError, ValueError) as exc: + raise SealError(f"Couldn't read {path}: {exc}") from None + key = secrets.token_bytes(32) + try: + path.parent.mkdir(parents=True, exist_ok=True) + except OSError as exc: + raise SealError(f"Couldn't create {path.parent}: {exc}") from None + try: + # O_EXCL: two writers racing on first use both end up with the winner's key. + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + except FileExistsError: + try: + return _read_key(path) + except (OSError, ValueError) as exc: + raise SealError(f"Couldn't read {path}: {exc}") from None + except OSError as exc: + raise SealError(f"Couldn't create {path}: {exc}") from None + with os.fdopen(fd, "wb") as f: + f.write(base64.urlsafe_b64encode(key) + b"\n") + f.flush() + os.fsync(f.fileno()) + logger.info("created the key for stored credentials at %s", path) + return key + + +def _cipher(*, create: bool) -> AESGCM | None: + master = _master(create=create) + if master is None: + return None + derived = HKDF(algorithm=hashes.SHA256(), length=32, salt=None, info=_PURPOSE).derive(master) + return AESGCM(derived) + + +def is_sealed(value: str) -> bool: + return value.startswith(PREFIX) + + +def seal(plain: str, owner: str) -> str: + """Encrypt `plain` for the setting named `owner`. Raises SealError when the key + file can't be made (DATA_DIR not writable).""" + cipher = _cipher(create=True) + assert cipher is not None + nonce = secrets.token_bytes(_NONCE_BYTES) + data = cipher.encrypt(nonce, plain.encode(), owner.encode()) + return PREFIX + base64.urlsafe_b64encode(nonce + data).decode() + + +def unseal(stored: str, owner: str) -> str | None: + """The plain value, or None when it can't be opened (key file lost or replaced, + or the value was copied from another setting). A value stored before sealing + existed comes back as it is; startup seals it (seal_stored_secrets).""" + if not is_sealed(stored): + return stored + try: + cipher = _cipher(create=False) + except SealError as exc: + logger.warning("%s", exc) + return None + if cipher is None: + return None + try: + data = base64.urlsafe_b64decode(stored[len(PREFIX):]) + if len(data) <= _NONCE_BYTES: + return None + return cipher.decrypt(data[:_NONCE_BYTES], data[_NONCE_BYTES:], owner.encode()).decode() + except (InvalidTag, ValueError): + return None diff --git a/src/inkwell/settings.py b/src/inkwell/settings.py index ce8889d..fdb7f46 100644 --- a/src/inkwell/settings.py +++ b/src/inkwell/settings.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import logging import secrets from dataclasses import dataclass from datetime import datetime, timedelta, timezone @@ -8,6 +9,9 @@ from typing import Any, Literal from .common import coerce_bool from .models.settings import Setting +from .sealing import is_sealed, seal, unseal + +logger = logging.getLogger(__name__) SettingType = Literal["string", "bool", "int"] @@ -413,7 +417,17 @@ async def get_setting(db, key: str) -> Any: if defn is None: raise KeyError(key) raw = await _load_raw(db, key) - return defn.default if raw is None else _coerce(defn, raw) + if raw is None: + return defn.default + if defn.secret and isinstance(raw, str) and raw: + plain = unseal(raw, key) + if plain is None: + # The key file was lost or replaced. Read as not set, so Settings asks for + # it again, rather than handing a sealed blob to SMTP or S3. + logger.warning("the stored %s can't be decrypted; enter it again in Settings", key) + return defn.default + raw = plain + return _coerce(defn, raw) async def get_public_config(db) -> dict: @@ -556,10 +570,29 @@ def validate_updates(updates: dict) -> tuple[dict, str | None]: async def set_settings(db, updates: dict) -> None: + """Store validated values. A credential is sealed first (sealing.py); raises + SealError if it can't be.""" for key, value in updates.items(): + defn = _BY_KEY.get(key) + if defn is not None and defn.secret and isinstance(value, str) and value: + value = seal(value, key) await _upsert(db, key, value) +async def seal_stored_secrets(db) -> int: + """Seal any credential still stored in plain text, from before sealing existed. + Run at startup; returns how many it sealed. The caller commits.""" + sealed = 0 + for defn in REGISTRY: + if not defn.secret: + continue + raw = await _load_raw(db, defn.key) + if isinstance(raw, str) and raw and not is_sealed(raw): + await _upsert(db, defn.key, seal(raw, defn.key)) + sealed += 1 + return sealed + + async def load_or_create_secret_key(db) -> str: """Return the persisted cookie-signing secret, generating + storing one on first run. Keeps sessions valid across restarts with no env var or volume required.""" diff --git a/src/inkwell/settings_api.py b/src/inkwell/settings_api.py index 57a5d73..9c5bb6c 100644 --- a/src/inkwell/settings_api.py +++ b/src/inkwell/settings_api.py @@ -10,6 +10,7 @@ from . import mail_templates from .mailer import mail_settings, send from .responses import json_error from .models.user import User +from .sealing import SealError from .settings import apply_session_ttl, get_admin_settings, get_setting, refresh_live, set_settings, validate_updates logger = logging.getLogger(__name__) @@ -39,7 +40,11 @@ async def update_settings(): return json_error(error, 400) async with session_scope() as db: - await set_settings(db, clean) + try: + await set_settings(db, clean) + except SealError as exc: + # Nothing is saved: a credential is never stored unsealed by a save. + return json_error(f"Couldn't encrypt the password or key to store it. {exc}", 500) await db.commit() # Re-read the cached security values so a saved limit or hop count applies to # the very next request. Unconditional: cheap, and a conditional here would be diff --git a/tests/test_integration.py b/tests/test_integration.py index 72196da..0684cca 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -26,7 +26,7 @@ import pytest import pytest_asyncio from sqlalchemy import delete, func, select, text, update -from inkwell import audit, mailer, ratelimit +from inkwell import audit, mailer, ratelimit, sealing from inkwell.app import create_app from inkwell.config import Config from inkwell.db import dispose_engine, session_scope @@ -40,7 +40,7 @@ from inkwell.models.note import Note from inkwell.models.note_attachment import NoteAttachment from inkwell.models.user import User from inkwell.notes.tags import _lift_and_reconcile_tags -from inkwell.settings import get_setting, live, refresh_live, reset_live, set_settings +from inkwell.settings import get_setting, live, refresh_live, reset_live, seal_stored_secrets, set_settings from inkwell.notes.checklist import parse_items, set_item_checked from inkwell.notes.helpers import derive_display_title from inkwell.models.note_link_preview import NoteLinkPreview @@ -2004,11 +2004,40 @@ async def test_the_smtp_password_never_leaves_the_server(app_client, db): assert (await app_client.patch("/api/settings", json={"smtp_password": ""})).status_code == 200 async with session_scope() as fresh: assert await get_setting(fresh, "smtp_password") == "hunter2" + # And the row itself holds it sealed, not as text (#5927). + row = await fresh.get(Setting, "smtp_password") + assert "hunter2" not in row.value and sealing.PREFIX in row.value assert (await app_client.patch("/api/settings", json={"smtp_security": "ssl3"})).status_code == 400 assert (await app_client.patch("/api/settings", json={"public_url": "notes.example.test"})).status_code == 400 +async def test_a_credential_stored_before_sealing_is_sealed_and_a_lost_key_asks_again(app_client, db): + await _mail_off() + await _admin_with_invite(app_client) + # As an instance from before #5927 left it: the password as plain JSON. + async with session_scope() as fresh: + fresh.add(Setting(key="smtp_password", value='"hunter2"')) + await fresh.commit() + async with session_scope() as fresh: + assert await get_setting(fresh, "smtp_password") == "hunter2" + assert await seal_stored_secrets(fresh) == 1 + await fresh.commit() + async with session_scope() as fresh: + assert "hunter2" not in (await fresh.get(Setting, "smtp_password")).value + assert await get_setting(fresh, "smtp_password") == "hunter2" + assert await seal_stored_secrets(fresh) == 0 # once only + + # The key file lost and a new one made: the value no longer opens, and reads as + # not set, so Settings asks for it again. + sealing.key_path().unlink() + sealing.seal("anything", "other") + async with session_scope() as fresh: + assert await get_setting(fresh, "smtp_password") == "" + rows = {r["key"]: r for r in (await (await app_client.get("/api/settings")).get_json())["settings"]} + assert rows["smtp_password"]["is_set"] is False + + async def test_a_forgotten_password_is_reset_from_an_emailed_link(app_client, db, monkeypatch): await _mail_off() outbox = _outbox(monkeypatch) diff --git a/tests/test_sealing.py b/tests/test_sealing.py new file mode 100644 index 0000000..9f558dc --- /dev/null +++ b/tests/test_sealing.py @@ -0,0 +1,55 @@ +import stat + +import pytest + +from inkwell import sealing +from inkwell.config import Config + + +def test_a_sealed_value_opens_for_its_own_setting_only(): + stored = sealing.seal("hunter2", "smtp_password") + assert stored.startswith(sealing.PREFIX) and "hunter2" not in stored + assert sealing.unseal(stored, "smtp_password") == "hunter2" + # Copied onto another setting's row, it does not open there. + assert sealing.unseal(stored, "backup_s3_secret_key") is None + + +def test_sealing_the_same_value_twice_gives_different_text(): + assert sealing.seal("x", "k") != sealing.seal("x", "k") + + +def test_the_key_file_is_made_once_readable_only_by_the_server(): + sealing.seal("x", "k") + path = sealing.key_path() + assert path.parent == Config.data_root() + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + first = path.read_bytes() + sealing.seal("y", "k") + assert path.read_bytes() == first + + +def test_a_value_from_before_sealing_reads_as_it_is(): + assert sealing.unseal("hunter2", "smtp_password") == "hunter2" + assert sealing.unseal("v1.looks-versioned", "smtp_password") == "v1.looks-versioned" + + +def test_reading_never_creates_a_key(): + assert sealing.unseal(sealing.PREFIX + "AAAA", "k") is None + assert not sealing.key_path().exists() + + +def test_a_replaced_key_or_a_damaged_value_reads_as_unreadable(): + stored = sealing.seal("hunter2", "k") + assert sealing.unseal(stored[:-4] + "AAAA", "k") is None + assert sealing.unseal(sealing.PREFIX + "not base64!", "k") is None + sealing.key_path().unlink() + sealing.seal("other", "k") # a new key + assert sealing.unseal(stored, "k") is None + + +def test_a_data_folder_that_cannot_be_written_refuses_to_seal(tmp_path, monkeypatch): + blocked = tmp_path / "file-not-dir" + blocked.write_text("") + monkeypatch.setattr(Config, "DATA_DIR", str(blocked)) + with pytest.raises(sealing.SealError): + sealing.seal("hunter2", "k")