feat: stored credentials are sealed at rest, so a copy of the database alone reveals none of them (#6767)

Minstrel keeps credentials it has to read back: indexer, Prowlarr and
download-client keys and passwords, the AcoustID key, cover-art and tag
provider keys, the SMTP password, and each user's ListenBrainz token and
Subsonic password. They were stored in the clear; they are now sealed with
AES-256-GCM under a per-column HKDF key derived from the stream secret, each
value bound to its row.

- internal/secret: Seal/Open/Reveal, and Columns, the one list of sealed
  columns (a backup will leave them out). Install takes the master key at
  startup; SealStored seals any value still in the clear, so an existing
  database is brought over in place on the first boot.
- Every write seals and every read opens: acquire (one read path, getService
  / usableServices), acoustid settings, SMTP, ListenBrainz, Subsonic auth and
  password generation, cover-art and tag provider settings.
- A value that does not open (the stream secret changed) is logged and reads
  as unset, to be entered again in Settings. docs/security.md says so.
- Tests: secret unit tests (wrong owner/purpose/key, malformed, no key), a
  SealStored integration test over all eight columns plus a schema check that
  each listed column exists, and the existing tests now assert sealed storage.
  dbtest installs a test key for integration tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/secret/secret.go

@@ -0,0 +1,179 @@
+// Package secret seals the credentials Minstrel keeps for outside systems
+// (indexer and download-client keys, the SMTP password, provider API keys,
+// ListenBrainz tokens, Subsonic passwords) so the database holds ciphertext,
+// not the credential.
+//
+// Each column has its own key, derived from the install's master key with
+// HKDF-SHA256 and the column as the info string, so a value sealed for one
+// column cannot be opened as another, and the master key never touches a
+// cipher. Each value is also bound to the row it belongs to (its owner), so a
+// sealed value copied onto another row does not open there.
+//
+// The master key is the stream secret (MINSTREL_STREAM_SECRET, or the
+// stream_secret file in the data directory). It lives outside the database,
+// so a copy of the database alone opens nothing. Changing it makes every
+// sealed value unreadable: each one reads as unset and has to be entered
+// again in Settings.
+package secret
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/hkdf"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
+	"errors"
+	"fmt"
+	"log/slog"
+	"strings"
+	"sync"
+)
+
+// ErrNoKey is a seal or open before Install has given the package a key.
+var ErrNoKey = errors.New("secret: no master key installed")
+
+// ErrCorrupt is a stored value that does not open: the master key changed,
+// the value was edited by hand, or it belongs to a different row.
+var ErrCorrupt = errors.New("secret: the stored value does not open")
+
+// sealedPrefix versions the format, so a future change of cipher can still
+// read what this one wrote. Nothing a person types starts with it by chance
+// often enough to matter, and Open rejects anything without it.
+const sealedPrefix = "v1."
+
+var b64 = base64.RawURLEncoding
+
+var (
+	mu     sync.RWMutex
+	master []byte
+	boxes  map[string]cipher.AEAD
+)
+
+// Install sets the master key every Seal and Open derives from. The server
+// calls it once at startup, before anything reads a credential.
+func Install(key []byte) {
+	mu.Lock()
+	defer mu.Unlock()
+	master = append([]byte(nil), key...)
+	boxes = map[string]cipher.AEAD{}
+}
+
+// aead returns purpose's cipher, deriving it on first use.
+func aead(purpose string) (cipher.AEAD, error) {
+	mu.RLock()
+	a, ok := boxes[purpose]
+	key := master
+	mu.RUnlock()
+	if ok {
+		return a, nil
+	}
+	if len(key) == 0 {
+		return nil, ErrNoKey
+	}
+	derived, err := hkdf.Key(sha256.New, key, nil, "minstrel/"+purpose, 32)
+	if err != nil {
+		return nil, fmt.Errorf("secret: derive: %w", err)
+	}
+	block, err := aes.NewCipher(derived)
+	if err != nil {
+		return nil, fmt.Errorf("secret: %w", err)
+	}
+	a, err = cipher.NewGCM(block)
+	if err != nil {
+		return nil, fmt.Errorf("secret: %w", err)
+	}
+	mu.Lock()
+	if boxes != nil {
+		boxes[purpose] = a
+	}
+	mu.Unlock()
+	return a, nil
+}
+
+// IsSealed reports whether stored is in the sealed format.
+func IsSealed(stored string) bool { return strings.HasPrefix(stored, sealedPrefix) }
+
+// Seal encrypts plain for purpose (the column it is stored in) and owner (the
+// row it belongs to). The empty string stays empty, so "is one set?" stays a
+// test for the empty string.
+func Seal(purpose, owner, plain string) (string, error) {
+	if plain == "" {
+		return "", nil
+	}
+	a, err := aead(purpose)
+	if err != nil {
+		return "", err
+	}
+	nonce := make([]byte, a.NonceSize())
+	if _, err := rand.Read(nonce); err != nil {
+		return "", fmt.Errorf("secret: nonce: %w", err)
+	}
+	out := a.Seal(nonce, nonce, []byte(plain), []byte(owner))
+	return sealedPrefix + b64.EncodeToString(out), nil
+}
+
+// Open decrypts what Seal stored for the same purpose and owner. The empty
+// string opens to itself.
+func Open(purpose, owner, stored string) (string, error) {
+	if stored == "" {
+		return "", nil
+	}
+	a, err := aead(purpose)
+	if err != nil {
+		return "", err
+	}
+	raw, ok := strings.CutPrefix(stored, sealedPrefix)
+	if !ok {
+		return "", ErrCorrupt
+	}
+	data, err := b64.DecodeString(raw)
+	n := a.NonceSize()
+	if err != nil || len(data) < n {
+		return "", ErrCorrupt
+	}
+	plain, err := a.Open(nil, data[:n], data[n:], []byte(owner))
+	if err != nil {
+		return "", ErrCorrupt
+	}
+	return string(plain), nil
+}
+
+// Reveal is Open for a caller about to use the credential: a value that does
+// not open is logged and read as unset, which is what it has become. The
+// admin sees "not set" in Settings and enters it again.
+func Reveal(purpose, owner, stored string) string {
+	plain, err := Open(purpose, owner, stored)
+	if err != nil {
+		slog.Warn("secret: a stored credential does not open; it reads as unset until it is entered again",
+			"column", purpose, "err", err)
+		return ""
+	}
+	return plain
+}
+
+// RevealPtr is Reveal for a nullable column: nil stays nil, and a value that
+// does not open reads as nil.
+func RevealPtr(purpose, owner string, stored *string) *string {
+	if stored == nil {
+		return nil
+	}
+	plain := Reveal(purpose, owner, *stored)
+	if plain == "" {
+		return nil
+	}
+	return &plain
+}
+
+// SealPtr is Seal for a nullable column: nil and the empty string both store
+// NULL.
+func SealPtr(purpose, owner string, plain *string) (*string, error) {
+	if plain == nil || *plain == "" {
+		return nil, nil
+	}
+	sealed, err := Seal(purpose, owner, *plain)
+	if err != nil {
+		return nil, err
+	}
+	return &sealed, nil
+}

internal/secret/secret_test.go

@@ -0,0 +1,110 @@
+package secret
+
+import (
+	"errors"
+	"strings"
+	"testing"
+)
+
+func withKey(t *testing.T, key string) {
+	t.Helper()
+	mu.RLock()
+	prev := master
+	mu.RUnlock()
+	Install([]byte(key))
+	t.Cleanup(func() { Install(prev) })
+}
+
+func TestSealOpensForTheSamePurposeAndOwner(t *testing.T) {
+	withKey(t, "test master key")
+	sealed, err := Seal("t.col", "row-1", "hunter2")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !IsSealed(sealed) || strings.Contains(sealed, "hunter2") {
+		t.Fatalf("sealed = %q, want an opaque v1. value", sealed)
+	}
+	plain, err := Open("t.col", "row-1", sealed)
+	if err != nil || plain != "hunter2" {
+		t.Fatalf("Open = %q, %v; want hunter2", plain, err)
+	}
+	again, _ := Seal("t.col", "row-1", "hunter2")
+	if again == sealed {
+		t.Error("two seals of one value are identical; the nonce is not random")
+	}
+}
+
+func TestOpenRefusesAnythingElse(t *testing.T) {
+	withKey(t, "test master key")
+	sealed, _ := Seal("t.col", "row-1", "hunter2")
+
+	cases := map[string]func() (string, error){
+		"another owner":   func() (string, error) { return Open("t.col", "row-2", sealed) },
+		"another purpose": func() (string, error) { return Open("t.other", "row-1", sealed) },
+		"plain text":      func() (string, error) { return Open("t.col", "row-1", "hunter2") },
+		"truncated":       func() (string, error) { return Open("t.col", "row-1", sealed[:len(sealed)-4]) },
+		"not base64":      func() (string, error) { return Open("t.col", "row-1", "v1.!!!") },
+		"too short":       func() (string, error) { return Open("t.col", "row-1", "v1.AAAA") },
+	}
+	for name, open := range cases {
+		if _, err := open(); !errors.Is(err, ErrCorrupt) {
+			t.Errorf("%s: err = %v, want ErrCorrupt", name, err)
+		}
+	}
+
+	Install([]byte("a different key"))
+	if _, err := Open("t.col", "row-1", sealed); !errors.Is(err, ErrCorrupt) {
+		t.Errorf("another master key: err = %v, want ErrCorrupt", err)
+	}
+}
+
+func TestEmptyStaysEmpty(t *testing.T) {
+	withKey(t, "test master key")
+	if s, err := Seal("t.col", "o", ""); s != "" || err != nil {
+		t.Errorf("Seal(\"\") = %q, %v; want the empty string", s, err)
+	}
+	if s, err := Open("t.col", "o", ""); s != "" || err != nil {
+		t.Errorf("Open(\"\") = %q, %v; want the empty string", s, err)
+	}
+	empty := ""
+	if p, err := SealPtr("t.col", "o", &empty); p != nil || err != nil {
+		t.Errorf("SealPtr(\"\") = %v, %v; want nil", p, err)
+	}
+	if p := RevealPtr("t.col", "o", nil); p != nil {
+		t.Errorf("RevealPtr(nil) = %v, want nil", p)
+	}
+}
+
+func TestNoKeyIsAnErrorNotAPanic(t *testing.T) {
+	withKey(t, "")
+	if _, err := Seal("t.col", "o", "x"); !errors.Is(err, ErrNoKey) {
+		t.Errorf("Seal: err = %v, want ErrNoKey", err)
+	}
+	if _, err := Open("t.col", "o", "v1.AAAA"); !errors.Is(err, ErrNoKey) {
+		t.Errorf("Open: err = %v, want ErrNoKey", err)
+	}
+}
+
+func TestRevealReadsAnUnopenableValueAsUnset(t *testing.T) {
+	withKey(t, "test master key")
+	if got := Reveal("t.col", "o", "not sealed"); got != "" {
+		t.Errorf("Reveal = %q, want the empty string", got)
+	}
+	bad := "not sealed"
+	if got := RevealPtr("t.col", "o", &bad); got != nil {
+		t.Errorf("RevealPtr = %q, want nil", *got)
+	}
+}
+
+func TestEveryColumnHasADistinctPurpose(t *testing.T) {
+	seen := map[string]bool{}
+	for _, c := range Columns {
+		if seen[c.Purpose()] {
+			t.Errorf("%s is listed twice", c.Purpose())
+		}
+		seen[c.Purpose()] = true
+	}
+	if !seen[AcquisitionAPIKey.Purpose()] || !seen[SubsonicPassword.Purpose()] {
+		t.Error("Columns is missing a declared column")
+	}
+}

internal/secret/stored.go

@@ -0,0 +1,112 @@
+package secret
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgxpool"
+)
+
+// Column is one database column that holds a sealed credential.
+//
+// Its Purpose ("table.column") is the key-derivation label every Seal and
+// Open of that column passes. The owner a value is bound to is the row's key
+// as text (KeyColumn::text), or "" for a single-row settings table.
+type Column struct {
+	Table     string
+	Column    string
+	KeyColumn string
+	Singleton bool
+}
+
+// Purpose is the label Seal and Open take for this column.
+func (c Column) Purpose() string { return c.Table + "." + c.Column }
+
+// The sealed columns, and the purpose strings their callers pass. Every
+// credential Minstrel stores for an outside system, or that must be read back
+// in the clear, is listed here.
+var (
+	AcquisitionAPIKey   = Column{"acquisition_services", "api_key", "code", false}
+	AcquisitionPassword = Column{"acquisition_services", "password", "code", false}
+	AcoustIDAPIKey      = Column{"acoustid_settings", "api_key", "id", true}
+	SMTPPassword        = Column{"smtp_config", "password", "id", true}
+	ListenBrainzToken   = Column{"users", "listenbrainz_token", "id", false}
+	SubsonicPassword    = Column{"users", "subsonic_password", "id", false}
+	CoverArtAPIKey      = Column{"cover_art_provider_settings", "api_key", "provider_id", false}
+	TagProviderAPIKey   = Column{"tag_provider_settings", "api_key", "provider_id", false}
+)
+
+// Columns is every sealed column, for SealStored and for anything that has to
+// know which columns hold credentials (a backup leaves them out).
+var Columns = []Column{
+	AcquisitionAPIKey, AcquisitionPassword, AcoustIDAPIKey, SMTPPassword,
+	ListenBrainzToken, SubsonicPassword, CoverArtAPIKey, TagProviderAPIKey,
+}
+
+// Owner is the owner string for a row whose key is key. A single-row table's
+// rows are owned by "".
+func (c Column) Owner(key string) string {
+	if c.Singleton {
+		return ""
+	}
+	return key
+}
+
+// SealStored seals every value in Columns that is still stored in the clear,
+// and returns how many it sealed. The server runs it at startup, after
+// migrations and before anything reads a credential, so a database written
+// before sealing existed is brought over in place. Each row is updated only
+// if it still holds the value that was read, so a concurrent write wins.
+func SealStored(ctx context.Context, pool *pgxpool.Pool) (int, error) {
+	total := 0
+	for _, c := range Columns {
+		n, err := sealColumn(ctx, pool, c)
+		total += n
+		if err != nil {
+			return total, fmt.Errorf("seal %s: %w", c.Purpose(), err)
+		}
+	}
+	return total, nil
+}
+
+func sealColumn(ctx context.Context, pool *pgxpool.Pool, c Column) (int, error) {
+	table := pgx.Identifier{c.Table}.Sanitize()
+	col := pgx.Identifier{c.Column}.Sanitize()
+	key := pgx.Identifier{c.KeyColumn}.Sanitize()
+
+	rows, err := pool.Query(ctx, fmt.Sprintf(
+		`SELECT %s::text, %s FROM %s WHERE %s IS NOT NULL AND %s <> '' AND %s NOT LIKE 'v1.%%'`,
+		key, col, table, col, col, col))
+	if err != nil {
+		return 0, err
+	}
+	type pending struct{ key, plain string }
+	var todo []pending
+	for rows.Next() {
+		var p pending
+		if err := rows.Scan(&p.key, &p.plain); err != nil {
+			rows.Close()
+			return 0, err
+		}
+		todo = append(todo, p)
+	}
+	rows.Close()
+	if err := rows.Err(); err != nil {
+		return 0, err
+	}
+
+	update := fmt.Sprintf(`UPDATE %s SET %s = $1 WHERE %s::text = $2 AND %s = $3`, table, col, key, col)
+	sealed := 0
+	for _, p := range todo {
+		v, err := Seal(c.Purpose(), c.Owner(p.key), p.plain)
+		if err != nil {
+			return sealed, err
+		}
+		if _, err := pool.Exec(ctx, update, v, p.key, p.plain); err != nil {
+			return sealed, err
+		}
+		sealed++
+	}
+	return sealed, nil
+}

internal/secret/stored_test.go

@@ -0,0 +1,154 @@
+package secret_test
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"log/slog"
+	"os"
+	"testing"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/secret"
+)
+
+func newPool(t *testing.T) *pgxpool.Pool {
+	t.Helper()
+	if testing.Short() {
+		t.Skip("skipping integration test in -short mode")
+	}
+	dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
+	if dsn == "" {
+		t.Skip("MINSTREL_TEST_DATABASE_URL not set")
+	}
+	if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	pool, err := pgxpool.New(context.Background(), dsn)
+	if err != nil {
+		t.Fatalf("pool: %v", err)
+	}
+	t.Cleanup(pool.Close)
+	dbtest.ResetDB(t, pool)
+	return pool
+}
+
+// Every listed column exists, so a renamed or dropped column fails here
+// rather than leaving a credential unsealed.
+func TestColumnsExistInTheSchema_Integration(t *testing.T) {
+	pool := newPool(t)
+	for _, c := range secret.Columns {
+		var n int
+		err := pool.QueryRow(context.Background(),
+			`SELECT count(*) FROM information_schema.columns
+			  WHERE table_schema = 'public' AND table_name = $1 AND column_name IN ($2, $3)`,
+			c.Table, c.Column, c.KeyColumn).Scan(&n)
+		if err != nil || n != 2 {
+			t.Errorf("%s (key %s): found %d of 2 columns, err %v", c.Purpose(), c.KeyColumn, n, err)
+		}
+	}
+}
+
+// A database written before sealing existed is sealed in place at startup,
+// every value opens to what it held, and a second pass has nothing to do.
+func TestSealStoredSealsWhatIsInTheClear_Integration(t *testing.T) {
+	pool := newPool(t)
+	ctx := context.Background()
+	q := dbq.New(pool)
+
+	u, err := q.CreateUser(ctx, dbq.CreateUserParams{
+		Username: dbtest.TestUserPrefix + "sealer", PasswordHash: "x", ApiTokenHash: "sealer", IsAdmin: false,
+	})
+	if err != nil {
+		t.Fatalf("user: %v", err)
+	}
+	if _, err := q.CreateAcquisitionService(ctx, dbq.CreateAcquisitionServiceParams{
+		Code: "seal-test", Kind: "search", Protocol: "prowlarr", Label: "Seal test",
+		Category: "", TimeoutMs: 1000, SearchesPerHour: 0,
+	}); err != nil {
+		t.Fatalf("acquisition row: %v", err)
+	}
+	seed := []string{
+		`UPDATE acquisition_services SET api_key = 'acq-key', password = 'acq-pw' WHERE code = 'seal-test'`,
+		`UPDATE acoustid_settings SET api_key = 'acoustid-key' WHERE id = true`,
+		`UPDATE smtp_config SET password = 'smtp-pw' WHERE id = true`,
+		fmt.Sprintf(`UPDATE users SET listenbrainz_token = 'lb-token', subsonic_password = 'sub-pw' WHERE id = '%s'`, u.ID.String()),
+		`INSERT INTO cover_art_provider_settings (provider_id, enabled, display_order, api_key)
+		 VALUES ('seal-test', false, 99, 'cover-key')
+		 ON CONFLICT (provider_id) DO UPDATE SET api_key = EXCLUDED.api_key`,
+		`INSERT INTO tag_provider_settings (provider_id, enabled, display_order, api_key)
+		 VALUES ('seal-test', false, 99, 'tag-key')
+		 ON CONFLICT (provider_id) DO UPDATE SET api_key = EXCLUDED.api_key`,
+	}
+	for _, s := range seed {
+		if _, err := pool.Exec(ctx, s); err != nil {
+			t.Fatalf("seed %q: %v", s, err)
+		}
+	}
+	t.Cleanup(func() {
+		for _, s := range []string{
+			`DELETE FROM cover_art_provider_settings WHERE provider_id = 'seal-test'`,
+			`DELETE FROM tag_provider_settings WHERE provider_id = 'seal-test'`,
+			`DELETE FROM acquisition_services WHERE code = 'seal-test'`,
+			`UPDATE acoustid_settings SET api_key = NULL WHERE id = true`,
+			`UPDATE smtp_config SET password = '' WHERE id = true`,
+		} {
+			_, _ = pool.Exec(context.Background(), s)
+		}
+	})
+
+	want := []struct {
+		col   secret.Column
+		key   string
+		plain string
+	}{
+		{secret.AcquisitionAPIKey, "seal-test", "acq-key"},
+		{secret.AcquisitionPassword, "seal-test", "acq-pw"},
+		{secret.AcoustIDAPIKey, "true", "acoustid-key"},
+		{secret.SMTPPassword, "true", "smtp-pw"},
+		{secret.ListenBrainzToken, u.ID.String(), "lb-token"},
+		{secret.SubsonicPassword, u.ID.String(), "sub-pw"},
+		{secret.CoverArtAPIKey, "seal-test", "cover-key"},
+		{secret.TagProviderAPIKey, "seal-test", "tag-key"},
+	}
+	if len(want) != len(secret.Columns) {
+		t.Fatalf("the test covers %d columns, secret.Columns lists %d", len(want), len(secret.Columns))
+	}
+
+	n, err := secret.SealStored(ctx, pool)
+	if err != nil {
+		t.Fatalf("SealStored: %v", err)
+	}
+	if n < len(want) {
+		t.Errorf("sealed %d values, want at least %d", n, len(want))
+	}
+
+	for _, w := range want {
+		var stored string
+		query := fmt.Sprintf(`SELECT %s FROM %s WHERE %s::text = $1`,
+			pgx.Identifier{w.col.Column}.Sanitize(), pgx.Identifier{w.col.Table}.Sanitize(),
+			pgx.Identifier{w.col.KeyColumn}.Sanitize())
+		if err := pool.QueryRow(ctx, query, w.key).Scan(&stored); err != nil {
+			t.Errorf("%s: read: %v", w.col.Purpose(), err)
+			continue
+		}
+		if !secret.IsSealed(stored) {
+			t.Errorf("%s = %q, want it sealed", w.col.Purpose(), stored)
+			continue
+		}
+		got, err := secret.Open(w.col.Purpose(), w.col.Owner(w.key), stored)
+		if err != nil || got != w.plain {
+			t.Errorf("%s opens to %q (err %v), want %q", w.col.Purpose(), got, err, w.plain)
+		}
+	}
+
+	again, err := secret.SealStored(ctx, pool)
+	if err != nil || again != 0 {
+		t.Errorf("second pass sealed %d (err %v), want 0", again, err)
+	}
+}