diff --git a/cmd/verdande/main.go b/cmd/verdande/main.go index ebb587f..acb4e5f 100644 --- a/cmd/verdande/main.go +++ b/cmd/verdande/main.go @@ -17,6 +17,7 @@ import ( "github.com/kristianwind/verdande/internal/config" "github.com/kristianwind/verdande/internal/httpapi" "github.com/kristianwind/verdande/internal/jobs" + "github.com/kristianwind/verdande/internal/secret" "github.com/kristianwind/verdande/internal/store" ) @@ -54,6 +55,15 @@ func run() error { defer db.Close() log.Info("database ready", "path", db.Path()) + // Before anything reads a setting. The key lives outside the database on + // purpose: backups can be downloaded through the interface, so a token stored + // in the clear leaves with every copy anyone takes. + box, err := secret.Open(cfg.SecretKey, cfg.DataDir) + if err != nil { + return err + } + db.UseSecrets(box) + httpapi.Version = version api := httpapi.New(cfg, db, log, webAssets(log)) diff --git a/internal/config/config.go b/internal/config/config.go index 2e85641..38526ad 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,6 +33,10 @@ type Config struct { GmailClientID string GmailClientSecret string + // SecretKey seals the tokens and passwords in the database, 32 bytes of base64. + // Empty means a key file beside the data — see internal/secret. + SecretKey string + // UpdateCheck asks GitHub whether a newer release exists. Off unless asked // for: a self-hosted app that reaches out without being told to has broken // the deal its operator made by self-hosting. @@ -100,6 +104,7 @@ func Load() (*Config, error) { PanelToken: env("VERDANDE_PANEL_TOKEN", ""), PanelServerID: env("VERDANDE_PANEL_SERVER_ID", ""), GmailClientSecret: env("VERDANDE_GMAIL_CLIENT_SECRET", ""), + SecretKey: env("VERDANDE_SECRET_KEY", ""), UpdateCheck: envBool("VERDANDE_UPDATE_CHECK", false), } diff --git a/internal/secret/secret.go b/internal/secret/secret.go new file mode 100644 index 0000000..d130ed1 --- /dev/null +++ b/internal/secret/secret.go @@ -0,0 +1,141 @@ +// Package secret seals the values that must not travel with a backup. +// +// The reason this exists is not the database on the server — it is the copies of +// it. Backups can be downloaded through the web interface, and every one taken so +// far carried a live Gmail refresh token in plain text. Anyone who ends up with a +// copy ends up with the mailbox. +// +// So the key must not live in the database. That rules out the pattern used for +// the VAPID pair, which the instance generates and stores in the same file it +// protects: a key kept beside what it locks is not a lock. It comes from the +// environment, or from a file next to the data that the backup does not include. +package secret + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "encoding/base64" + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +// prefix marks a sealed value, so a mixed column can be read without guessing and +// so the scheme can be replaced later without having to. +const prefix = "v1:" + +// KeyFile is where the key lands when the environment does not carry one. It sits +// beside the database rather than inside it, and the backup routine must never +// learn to include it. +const KeyFile = "secret.key" + +// Box seals and opens values. +type Box struct { + aead cipher.AEAD +} + +// Open loads the key, generating and saving one on first use. +// +// A key given in the environment wins: that is how a host keeps the secret out of +// its disks altogether. Otherwise a file, created 0600, and read back with its +// permissions checked — a key that anybody on the box can read is a key that is +// only ceremonially secret. +func Open(env, dataDir string) (*Box, error) { + if env != "" { + key, err := base64.StdEncoding.DecodeString(strings.TrimSpace(env)) + if err != nil || len(key) != 32 { + return nil, errors.New("VERDANDE_SECRET_KEY must be 32 bytes of base64 — " + + "generate one with: openssl rand -base64 32") + } + return newBox(key) + } + + path := filepath.Join(dataDir, KeyFile) + raw, err := os.ReadFile(path) + switch { + case err == nil: + key, decErr := base64.StdEncoding.DecodeString(strings.TrimSpace(string(raw))) + if decErr != nil || len(key) != 32 { + // Deliberately not regenerating. A new key here would not fix anything — + // it would silently make every stored token unreadable, which looks + // exactly like the mailbox having disconnected itself overnight. + return nil, fmt.Errorf("%s is not a 32-byte base64 key; move it aside only "+ + "if you accept that every connected mailbox must be reconnected", path) + } + if info, statErr := os.Stat(path); statErr == nil && info.Mode().Perm()&0o077 != 0 { + return nil, fmt.Errorf("%s is readable by others (%04o); chmod 600 it", path, info.Mode().Perm()) + } + return newBox(key) + + case errors.Is(err, os.ErrNotExist): + key := make([]byte, 32) + if _, randErr := rand.Read(key); randErr != nil { + return nil, randErr + } + encoded := base64.StdEncoding.EncodeToString(key) + if writeErr := os.WriteFile(path, []byte(encoded+"\n"), 0o600); writeErr != nil { + return nil, fmt.Errorf("write %s: %w", path, writeErr) + } + return newBox(key) + + default: + return nil, fmt.Errorf("read %s: %w", path, err) + } +} + +func newBox(key []byte) (*Box, error) { + block, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + aead, err := cipher.NewGCM(block) + if err != nil { + return nil, err + } + return &Box{aead: aead}, nil +} + +// Seal returns the sealed form of v. An empty string stays empty: there is nothing +// to hide, and a row full of ciphertext for "" only makes the column harder to read. +func (b *Box) Seal(v string) (string, error) { + if v == "" || IsSealed(v) { + return v, nil + } + nonce := make([]byte, b.aead.NonceSize()) + if _, err := rand.Read(nonce); err != nil { + return "", err + } + out := b.aead.Seal(nonce, nonce, []byte(v), nil) + return prefix + base64.StdEncoding.EncodeToString(out), nil +} + +// Unseal returns the plain value. Anything without the marker is returned as it +// is — the column holds values written before there was a key, and they are read +// as themselves until something writes them back sealed. +func (b *Box) Unseal(v string) (string, error) { + if !IsSealed(v) { + return v, nil + } + raw, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(v, prefix)) + if err != nil { + return "", fmt.Errorf("sealed value is not base64: %w", err) + } + if len(raw) < b.aead.NonceSize() { + return "", errors.New("sealed value is too short to contain a nonce") + } + nonce, body := raw[:b.aead.NonceSize()], raw[b.aead.NonceSize():] + out, err := b.aead.Open(nil, nonce, body, nil) + if err != nil { + // The overwhelmingly likely cause, and worth saying rather than "message + // authentication failed", which sends people looking for a corrupt row. + return "", errors.New("cannot read a stored secret: this is a different key " + + "than the one it was written with") + } + return string(out), nil +} + +// IsSealed reports whether v has already been through Seal. +func IsSealed(v string) bool { return strings.HasPrefix(v, prefix) } diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go new file mode 100644 index 0000000..3fd852a --- /dev/null +++ b/internal/secret/secret_test.go @@ -0,0 +1,149 @@ +package secret + +import ( + "encoding/base64" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestAValueSurvivesTheRoundTrip(t *testing.T) { + b := mustBox(t) + for _, in := range []string{"1//0abc-refresh", "kodeord med æøå", strings.Repeat("x", 5000)} { + sealed, err := b.Seal(in) + if err != nil { + t.Fatal(err) + } + if strings.Contains(sealed, in) { + t.Fatal("the sealed form still contains the value") + } + out, err := b.Unseal(sealed) + if err != nil { + t.Fatal(err) + } + if out != in { + t.Errorf("got %q, want %q", out, in) + } + } +} + +// Two seals of the same value must not look alike, or the column tells anyone +// reading it which accounts share a password. +func TestSealingTwiceGivesDifferentText(t *testing.T) { + b := mustBox(t) + first, _ := b.Seal("same") + second, _ := b.Seal("same") + if first == second { + t.Error("the same value sealed to the same text twice") + } +} + +// The column holds values written before there was a key. Reading must not fail +// on them, and sealing must not wrap an already-sealed value a second time. +func TestPlainTextIsReadAsItself(t *testing.T) { + b := mustBox(t) + out, err := b.Unseal("1//0abc-written-before-there-was-a-key") + if err != nil { + t.Fatal(err) + } + if out != "1//0abc-written-before-there-was-a-key" { + t.Errorf("an unsealed value came back as %q", out) + } + + sealed, _ := b.Seal("x") + again, _ := b.Seal(sealed) + if again != sealed { + t.Error("an already-sealed value was sealed a second time") + } +} + +func TestNothingIsStillNothing(t *testing.T) { + b := mustBox(t) + if out, _ := b.Seal(""); out != "" { + t.Errorf("an empty value sealed to %q", out) + } +} + +// The message somebody gets after restoring a database without its key file. It +// has to point at the key, because every other symptom points at the mailbox. +func TestTheWrongKeySaysItIsTheWrongKey(t *testing.T) { + sealed, err := mustBox(t).Seal("hemmelig") + if err != nil { + t.Fatal(err) + } + _, err = mustBox(t).Unseal(sealed) + if err == nil { + t.Fatal("another key opened it") + } + if !strings.Contains(err.Error(), "different key") { + t.Errorf("the error does not name the cause: %v", err) + } +} + +func TestTheKeyFileIsMadeOnceAndKeptPrivate(t *testing.T) { + dir := t.TempDir() + if _, err := Open("", dir); err != nil { + t.Fatal(err) + } + + path := filepath.Join(dir, KeyFile) + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("the key file is %04o, not 0600", perm) + } + + // Opening again must reuse it. A second key here would quietly make every + // stored token unreadable, and it would look like the mailboxes disconnecting + // themselves overnight. + first, _ := os.ReadFile(path) + if _, err := Open("", dir); err != nil { + t.Fatal(err) + } + second, _ := os.ReadFile(path) + if string(first) != string(second) { + t.Error("opening a second time replaced the key") + } +} + +func TestAKeyOthersCanReadIsRefused(t *testing.T) { + dir := t.TempDir() + if _, err := Open("", dir); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, KeyFile) + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + if _, err := Open("", dir); err == nil { + t.Error("a world-readable key was accepted") + } +} + +// The environment wins, so a host can keep the key off its disks altogether. +func TestTheEnvironmentBeatsTheFile(t *testing.T) { + dir := t.TempDir() + key := base64.StdEncoding.EncodeToString([]byte(strings.Repeat("k", 32))) + if _, err := Open(key, dir); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(dir, KeyFile)); !os.IsNotExist(err) { + t.Error("a key file was written even though the environment carried one") + } + + if _, err := Open("too-short", dir); err == nil { + t.Error("a key that is not 32 bytes was accepted") + } +} + +func mustBox(t *testing.T) *Box { + t.Helper() + b, err := Open("", t.TempDir()) + if err != nil { + t.Fatal(err) + } + return b +} diff --git a/internal/store/notifications.go b/internal/store/notifications.go index 918c94b..5eeed86 100644 --- a/internal/store/notifications.go +++ b/internal/store/notifications.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "errors" + "fmt" "time" "github.com/kristianwind/verdande/internal/auth" @@ -313,6 +314,65 @@ func (db *DB) UsersWithGmail(ctx context.Context) ([]User, error) { // UserSettings reads one scope of a person's integration settings. A missing row is // an empty map, not an error: nobody has settings until they set some. +// --- sealed values ---------------------------------------------------------------- + +// secretFields are the keys whose values never leave this process in the clear. +// +// A named set rather than "seal everything": the rest of a settings blob is what +// somebody reads when a mailbox misbehaves — which folder, which label, which +// address — and turning all of it into ciphertext would buy nothing and cost the +// ability to look. +var secretFields = map[string]bool{ + "refresh_token": true, + "access_token": true, + "password": true, + "client_secret": true, +} + +// seal returns a copy of values with the secret fields sealed. Without a key it +// returns them untouched, which is what the tests that never set one expect. +func (db *DB) seal(values map[string]any) (map[string]any, error) { + if db.box == nil { + return values, nil + } + out := make(map[string]any, len(values)) + for k, v := range values { + s, ok := v.(string) + if !secretFields[k] || !ok { + out[k] = v + continue + } + sealed, err := db.box.Seal(s) + if err != nil { + return nil, fmt.Errorf("seal %s: %w", k, err) + } + out[k] = sealed + } + return out, nil +} + +// unseal is seal's other direction. Values written before there was a key come +// back as themselves; the next write puts them away properly, so the column +// converts itself as it is used rather than in one migration that has to be right +// about every row at once. +func (db *DB) unseal(values map[string]any) (map[string]any, error) { + if db.box == nil { + return values, nil + } + for k, v := range values { + s, ok := v.(string) + if !secretFields[k] || !ok { + continue + } + plain, err := db.box.Unseal(s) + if err != nil { + return nil, fmt.Errorf("read %s: %w", k, err) + } + values[k] = plain + } + return values, nil +} + func (db *DB) UserSettings(ctx context.Context, userID, scope string) (map[string]any, error) { var raw string err := db.QueryRowContext(ctx, @@ -329,10 +389,14 @@ func (db *DB) UserSettings(ctx context.Context, userID, scope string) (map[strin if err := json.Unmarshal([]byte(raw), &values); err != nil { return map[string]any{}, nil } - return values, nil + return db.unseal(values) } func (db *DB) SetUserSettings(ctx context.Context, userID, scope string, values map[string]any) error { + values, err := db.seal(values) + if err != nil { + return err + } raw, err := json.Marshal(values) if err != nil { return err @@ -370,10 +434,14 @@ func (db *DB) InstanceSettings(ctx context.Context, scope string) (map[string]an if err := json.Unmarshal([]byte(raw), &values); err != nil { return map[string]any{}, nil } - return values, nil + return db.unseal(values) } func (db *DB) SetInstanceSettings(ctx context.Context, scope string, values map[string]any) error { + values, err := db.seal(values) + if err != nil { + return err + } raw, err := json.Marshal(values) if err != nil { return err diff --git a/internal/store/secrets_test.go b/internal/store/secrets_test.go new file mode 100644 index 0000000..cf3c1e9 --- /dev/null +++ b/internal/store/secrets_test.go @@ -0,0 +1,125 @@ +package store + +import ( + "context" + "encoding/json" + "path/filepath" + "strings" + "testing" + + "github.com/kristianwind/verdande/internal/secret" +) + +// The whole point of the exercise: what is written must not be readable in the +// file that a backup copies. +func TestATokenIsNotStoredInTheClear(t *testing.T) { + db, userID := sealedStore(t) + ctx := context.Background() + + const token = "1//0-a-real-looking-refresh-token" + if err := db.SetUserSettings(ctx, userID, "gmail", map[string]any{ + "refresh_token": token, + "label": "Verdande", + }); err != nil { + t.Fatal(err) + } + + var raw string + if err := db.QueryRowContext(ctx, + `SELECT values_json FROM user_settings WHERE user_id = ?`, userID).Scan(&raw); err != nil { + t.Fatal(err) + } + if strings.Contains(raw, token) { + t.Fatalf("the token is in the row as written: %s", raw) + } + // The rest stays readable, which is the reason for naming the secret fields + // rather than sealing the whole blob. + if !strings.Contains(raw, "Verdande") { + t.Errorf("the label was sealed too: %s", raw) + } + + back, err := db.UserSettings(ctx, userID, "gmail") + if err != nil { + t.Fatal(err) + } + if back["refresh_token"] != token { + t.Errorf("got %q back", back["refresh_token"]) + } +} + +// The migration that matters. A row written before there was a key must still be +// readable, and must put itself away sealed the next time it is written — nobody +// should have to reconnect a mailbox because the server learned to encrypt. +func TestATokenWrittenBeforeTheKeyStillWorks(t *testing.T) { + db, userID := sealedStore(t) + ctx := context.Background() + + const token = "1//0-written-when-there-was-no-key" + plain, _ := json.Marshal(map[string]any{"refresh_token": token}) + if _, err := db.ExecContext(ctx, ` + INSERT INTO user_settings (user_id, scope, values_json, updated_at) + VALUES (?, 'gmail', ?, 0)`, userID, string(plain)); err != nil { + t.Fatal(err) + } + + values, err := db.UserSettings(ctx, userID, "gmail") + if err != nil { + t.Fatalf("a row from before the key could not be read: %v", err) + } + if values["refresh_token"] != token { + t.Fatalf("got %q, want the plain token back", values["refresh_token"]) + } + + // Written back — and now sealed, without anything having asked for it. + if err := db.SetUserSettings(ctx, userID, "gmail", values); err != nil { + t.Fatal(err) + } + var raw string + if err := db.QueryRowContext(ctx, + `SELECT values_json FROM user_settings WHERE user_id = ?`, userID).Scan(&raw); err != nil { + t.Fatal(err) + } + if strings.Contains(raw, token) { + t.Error("the row is still in the clear after being written back") + } +} + +// UsersWithGmail finds people by the presence of the key, not its value, so it +// must keep working now that the value is ciphertext. +func TestTheSweepStillFindsAConnectedMailbox(t *testing.T) { + db, userID := sealedStore(t) + ctx := context.Background() + + if err := db.SetUserSettings(ctx, userID, "gmail", + map[string]any{"refresh_token": "1//0-token"}); err != nil { + t.Fatal(err) + } + users, err := db.UsersWithGmail(ctx) + if err != nil { + t.Fatal(err) + } + if len(users) != 1 { + t.Fatalf("the sweep found %d connected mailboxes, want 1", len(users)) + } +} + +func sealedStore(t *testing.T) (*DB, string) { + t.Helper() + db, err := Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + + box, err := secret.Open("", t.TempDir()) + if err != nil { + t.Fatal(err) + } + db.UseSecrets(box) + + user := &User{Email: "kristian@example.dk", Name: "Kristian"} + if err := db.CreateUser(context.Background(), user, "Indbakke"); err != nil { + t.Fatal(err) + } + return db, user.ID +} diff --git a/internal/store/store.go b/internal/store/store.go index 0187bba..de4d064 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -7,6 +7,7 @@ import ( "database/sql" "embed" "fmt" + "github.com/kristianwind/verdande/internal/secret" "io/fs" "path/filepath" "runtime" @@ -23,8 +24,20 @@ var migrationFS embed.FS type DB struct { *sql.DB path string + + // box seals the values that must not travel with a backup. Nil in the tests + // that do not care, where a secret is stored as itself — see Seal. + box *secret.Box } +// UseSecrets gives the store the key it seals with. Called once at startup, before +// anything reads or writes settings. +// +// Separate from Open because the key comes from the environment or from a file +// beside the data, and a database that cannot be opened without it would make a +// misplaced key look like a corrupt disk. +func (db *DB) UseSecrets(box *secret.Box) { db.box = box } + // dsn is the connection string. // // SQLite is a single file, but it is being driven by an HTTP server with concurrent diff --git a/rune/verdande.yaml b/rune/verdande.yaml index 8f59eb8..493fbdb 100644 --- a/rune/verdande.yaml +++ b/rune/verdande.yaml @@ -15,7 +15,7 @@ gameskill: # this: a manifest with new fields and an unchanged version is a manifest the # panel has no reason to re-read, so the fields never appear and the failure is # silent — the server's settings page simply does not offer them. - version: 3 + version: 4 icon: "app" # The image is distroless — no shell — so keep_entrypoint runs the binary the @@ -99,6 +99,11 @@ gameskill: name: "This server's id in the panel (the one in its URL)" type: string default: "" + - key: VERDANDE_SECRET_KEY + name: "Key for the tokens and mailbox passwords in the database — 32 bytes of base64, from: openssl rand -base64 32. Leave empty to keep one in a file beside the data. Change it and every mailbox must be connected again." + type: string + secret: true + default: "" install: # The image has no shell, so the install step borrows one. It only has to make diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index 830c92e..8642f08 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -360,6 +360,8 @@ export const da = { 'Slettede projekter, med deres opgaver. De bliver hentet tilbage præcis som de var — bortset fra opgaver, du havde slettet hver for sig først; dem var der en grund til.', 'data.backupsHint': 'Databasen kopieres hver nat og de seneste fjorten gemmes. Kopierne er talt, ikke dateret: en container, der har stået slukket en måned, må ikke komme tilbage og slette dem alle for at være for gamle.', + 'data.backupsSecrets': + 'Tokens og kodeord til postkasser ligger krypteret i kopien, og nøglen gør ikke: den ligger i en fil ved siden af databasen. Det er meningen — en kopi, der kommer på afveje, åbner ikke din mail. Men gendanner du en kopi et sted uden den fil, skal postkasserne forbindes forfra.', 'data.backupsAttachments': 'Vedhæftede filer er ikke med. De er indholdsadresserede og bliver kun lagt til, så fjorten ens kopier af dem ville fylde det drev op, de skulle beskytte — tag hele /data, hvis du vil have det hele.', 'data.restoreHint': diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index 2475d74..51ff391 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -355,6 +355,8 @@ export const en = { 'Deleted projects, with their tasks. They come back exactly as they were — except for tasks you had deleted individually first; there was a reason for those.', 'data.backupsHint': 'The database is copied every night and the most recent fourteen are kept. Counted, not dated: a container that has been off for a month must not come back and delete all of them for being too old.', + 'data.backupsSecrets': + 'Tokens and mailbox passwords are encrypted inside the copy, and the key is not: it sits in a file beside the database. That is the point — a copy that goes astray does not open your mail. But restore a copy somewhere without that file and the mailboxes must be connected again.', 'data.backupsAttachments': 'Attachments are not included. They are content-addressed and only ever added to, so fourteen identical copies would fill the volume they were meant to protect — take the whole of /data if you want everything.', 'data.restoreHint': diff --git a/web/src/routes/indstillinger/data/+page.svelte b/web/src/routes/indstillinger/data/+page.svelte index 03adf8b..a90e20d 100644 --- a/web/src/routes/indstillinger/data/+page.svelte +++ b/web/src/routes/indstillinger/data/+page.svelte @@ -267,6 +267,13 @@
{t('data.backupsAttachments')}
+ ++ {t('data.backupsSecrets')} +