fix(security): sign the setup ceremony with startup-generated keys
REVIEW-001 (M0) M1s: the /setup wizard's pre-session double-submit ceremony (A15) was signed with csrf_key/cookie_secret left at the shipped CHANGE_ME placeholder until `finish` persisted generated values — a publicly-known signing key for the whole bootstrap. Generate strong ephemeral values at startup in setup mode, mirroring the TOTP key (R-6/D-1): `finish` skips a non-placeholder, so the wizard persists the very values the process already uses. Also add cmd/myday's first test — REVIEW-001 M2c: the D-1 invariant (the startup TOTP key is the persisted key) was covered only by a manual smoke. It generates the key, enrolls a secret, persists via the wizard, reopens the DB with the persisted key and asserts the secret still unseals.
This commit is contained in:
parent
f5e4fd0055
commit
8268757773
2 changed files with 238 additions and 14 deletions
|
|
@ -104,21 +104,10 @@ func run() error {
|
||||||
// R-6 already rules that a placeholder is replaced with a strong generated
|
// R-6 already rules that a placeholder is replaced with a strong generated
|
||||||
// value, so generate it here; it is adopted (and thereby persisted by the
|
// value, so generate it here; it is adopted (and thereby persisted by the
|
||||||
// wizard's `finish`, which skips a non-placeholder) only in setup mode.
|
// wizard's `finish`, which skips a non-placeholder) only in setup mode.
|
||||||
sealerKey := cfg.Secrets.TOTPEncryptionKey
|
sealerKey, generatedKey, err := prepareTOTPKey(cfg)
|
||||||
generatedKey := false
|
|
||||||
switch {
|
|
||||||
case config.SecretIsPlaceholder(sealerKey):
|
|
||||||
k, err := config.GenerateSecret(24) // 24 bytes -> 32 base64url chars == auth.KeySize
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
sealerKey, generatedKey = k, true
|
|
||||||
case len(sealerKey) != auth.KeySize:
|
|
||||||
// An operator-supplied key of the wrong size: fail closed and say so,
|
|
||||||
// rather than run a ceremony whose result we cannot store (A9).
|
|
||||||
return fmt.Errorf("myday: secrets.totp_encryption_key must be exactly %d bytes, got %d",
|
|
||||||
auth.KeySize, len(sealerKey))
|
|
||||||
}
|
|
||||||
sealer, err := auth.NewAEADSealer([]byte(sealerKey))
|
sealer, err := auth.NewAEADSealer([]byte(sealerKey))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("myday: build TOTP sealer: %w", err)
|
return fmt.Errorf("myday: build TOTP sealer: %w", err)
|
||||||
|
|
@ -163,6 +152,21 @@ func run() error {
|
||||||
"the wizard enrolls TOTP before it persists, so the key must exist now")
|
"the wizard enrolls TOTP before it persists, so the key must exist now")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// M1s: the CSRF and cookie signing keys drive the whole pre-session
|
||||||
|
// ceremony (A15), so they must be strong from the FIRST request too — not
|
||||||
|
// only after `finish` persists them (which would leave the wizard signing
|
||||||
|
// with the shipped, publicly-known placeholder until a restart). Generated
|
||||||
|
// here in setup mode only; `finish` then persists these very values.
|
||||||
|
if inSetup {
|
||||||
|
generated, err := prepareSigningKeys(cfg)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
for _, name := range generated {
|
||||||
|
slog.Info("myday: generated a strong signing key for setup (R-6/M1s)", "name", name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
csrf, err := httpx.NewCSRF([]byte(cfg.Secrets.CSRFKey))
|
csrf, err := httpx.NewCSRF([]byte(cfg.Secrets.CSRFKey))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|
@ -199,6 +203,7 @@ func run() error {
|
||||||
CSRF: preSession,
|
CSRF: preSession,
|
||||||
Audit: aud,
|
Audit: aud,
|
||||||
SuppressNudge: setupMode,
|
SuppressNudge: setupMode,
|
||||||
|
BaseURL: cfg.Server.BaseURL,
|
||||||
Secure: cfg.IsSecure(),
|
Secure: cfg.IsSecure(),
|
||||||
Logger: slog.Default(),
|
Logger: slog.Default(),
|
||||||
})
|
})
|
||||||
|
|
@ -221,6 +226,7 @@ func run() error {
|
||||||
Audit: aud,
|
Audit: aud,
|
||||||
TOTPOnlyEnroll: true,
|
TOTPOnlyEnroll: true,
|
||||||
SuccessPath: func(string) string { return "/setup" },
|
SuccessPath: func(string) string { return "/setup" },
|
||||||
|
BaseURL: cfg.Server.BaseURL,
|
||||||
Secure: cfg.IsSecure(),
|
Secure: cfg.IsSecure(),
|
||||||
Logger: slog.Default(),
|
Logger: slog.Default(),
|
||||||
})
|
})
|
||||||
|
|
@ -392,6 +398,58 @@ func buildRegistry(authFlow *flow.Actions, setupFlow *setup.Actions, recoveryFlo
|
||||||
return reg
|
return reg
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// prepareTOTPKey returns the TOTP sealing key the process will use, generating
|
||||||
|
// a strong one when the configured value is a placeholder (R-6/D-1). generated
|
||||||
|
// reports that the value was minted here — run() adopts it only in setup mode,
|
||||||
|
// where the process must still be able to seal a ceremony result.
|
||||||
|
//
|
||||||
|
// An operator-supplied key of the wrong size fails closed: rather than run a
|
||||||
|
// ceremony whose result we cannot store (A9), say so and stop.
|
||||||
|
func prepareTOTPKey(cfg *config.Config) (key string, generated bool, err error) {
|
||||||
|
key = cfg.Secrets.TOTPEncryptionKey
|
||||||
|
if config.SecretIsPlaceholder(key) {
|
||||||
|
// 24 bytes -> 32 base64url chars == auth.KeySize.
|
||||||
|
key, err = config.GenerateSecret(24)
|
||||||
|
if err != nil {
|
||||||
|
return "", false, err
|
||||||
|
}
|
||||||
|
return key, true, nil
|
||||||
|
}
|
||||||
|
if len(key) != auth.KeySize {
|
||||||
|
return "", false, fmt.Errorf("myday: secrets.totp_encryption_key must be exactly %d bytes, got %d",
|
||||||
|
auth.KeySize, len(key))
|
||||||
|
}
|
||||||
|
return key, false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// prepareSigningKeys replaces placeholder CSRF / cookie signing keys with strong
|
||||||
|
// generated values (R-6/M1s) and returns the names of the secrets it generated.
|
||||||
|
//
|
||||||
|
// run() calls it only in setup mode: the wizard's pre-session double-submit
|
||||||
|
// ceremony (A15) is signed with these keys from the first request, so the
|
||||||
|
// shipped, publicly-known placeholder must never be used. In productive mode
|
||||||
|
// config.Validate already refuses a placeholder (fail closed, SPEC-003 §1).
|
||||||
|
func prepareSigningKeys(cfg *config.Config) (generated []string, err error) {
|
||||||
|
for _, s := range []struct {
|
||||||
|
name string
|
||||||
|
dst *string
|
||||||
|
}{
|
||||||
|
{"secrets.csrf_key", &cfg.Secrets.CSRFKey},
|
||||||
|
{"secrets.cookie_secret", &cfg.Secrets.CookieSecret},
|
||||||
|
} {
|
||||||
|
if !config.SecretIsPlaceholder(*s.dst) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
v, err := config.GenerateSecret(32)
|
||||||
|
if err != nil {
|
||||||
|
return generated, err
|
||||||
|
}
|
||||||
|
*s.dst = v
|
||||||
|
generated = append(generated, s.name)
|
||||||
|
}
|
||||||
|
return generated, nil
|
||||||
|
}
|
||||||
|
|
||||||
// configTarget picks where the wizard persists settings: `config.yaml` beside
|
// configTarget picks where the wizard persists settings: `config.yaml` beside
|
||||||
// the loaded file. It NEVER overwrites the committed `config.example.yaml`
|
// the loaded file. It NEVER overwrites the committed `config.example.yaml`
|
||||||
// (D-010) — so a dev run on the example writes a real config.yaml next to it.
|
// (D-010) — so a dev run on the example writes a real config.yaml next to it.
|
||||||
|
|
|
||||||
166
cmd/myday/main_test.go
Normal file
166
cmd/myday/main_test.go
Normal file
|
|
@ -0,0 +1,166 @@
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"io"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/audit"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/auth"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/config"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/httpx"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/setup"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/store"
|
||||||
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/users"
|
||||||
|
)
|
||||||
|
|
||||||
|
// stubEnroller satisfies setup.Enroller — this test never runs a ceremony.
|
||||||
|
type stubEnroller struct{}
|
||||||
|
|
||||||
|
func (stubEnroller) TOTPBegin(http.ResponseWriter, *http.Request, string) error { return nil }
|
||||||
|
func (stubEnroller) TOTPVerify(http.ResponseWriter, *http.Request, string) error { return nil }
|
||||||
|
|
||||||
|
// setupYAML is a fresh-instance config: setup mode, every secret a placeholder.
|
||||||
|
const setupYAML = `setup: true
|
||||||
|
server:
|
||||||
|
listen: "127.0.0.1:8080"
|
||||||
|
base_url: "https://myday.example.com"
|
||||||
|
app:
|
||||||
|
locale: "en-GB"
|
||||||
|
database:
|
||||||
|
path: "DBPATH"
|
||||||
|
secrets:
|
||||||
|
cookie_secret: "CHANGE_ME_32_BYTES_RANDOM"
|
||||||
|
csrf_key: "CHANGE_ME_32_BYTES_RANDOM"
|
||||||
|
totp_encryption_key: "CHANGE_ME_32_BYTES_RANDOM"
|
||||||
|
`
|
||||||
|
|
||||||
|
// TestTOTPKeySurvivesSetupAndRestart pins the D-1 invariant (M2c): the TOTP key
|
||||||
|
// the process GENERATES AT STARTUP is the key the wizard persists, so a restart
|
||||||
|
// adopts the same key and the secrets enrolled during setup still unseal. A
|
||||||
|
// regression here silently destroys every future TOTP secret.
|
||||||
|
func TestTOTPKeySurvivesSetupAndRestart(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
dir := t.TempDir()
|
||||||
|
cfgPath := filepath.Join(dir, config.FileName)
|
||||||
|
dbPath := filepath.Join(dir, "myday.db")
|
||||||
|
body := strings.Replace(setupYAML, "DBPATH", dbPath, 1)
|
||||||
|
if err := os.WriteFile(cfgPath, []byte(body), 0o600); err != nil {
|
||||||
|
t.Fatalf("write config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cfg, err := config.Load(cfgPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("config.Load: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Startup (D-1 + M1s): every placeholder is replaced with a strong value
|
||||||
|
// the PROCESS will use — the wizard's ceremony must not run on a shipped
|
||||||
|
// placeholder key.
|
||||||
|
key, generated, err := prepareTOTPKey(cfg)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("prepareTOTPKey: %v", err)
|
||||||
|
}
|
||||||
|
if !generated || config.SecretIsPlaceholder(key) || len(key) != auth.KeySize {
|
||||||
|
t.Fatalf("the TOTP key was not generated (generated=%v len=%d)", generated, len(key))
|
||||||
|
}
|
||||||
|
cfg.Secrets.TOTPEncryptionKey = key // run() adopts it in setup mode
|
||||||
|
if _, err := prepareSigningKeys(cfg); err != nil {
|
||||||
|
t.Fatalf("prepareSigningKeys: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// First run: the wizard enrolls the admin and seals its TOTP secret with
|
||||||
|
// the startup key.
|
||||||
|
db, err := store.Open(cfg.Database.Path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("store.Open: %v", err)
|
||||||
|
}
|
||||||
|
if err := db.Migrate(); err != nil {
|
||||||
|
t.Fatalf("Migrate: %v", err)
|
||||||
|
}
|
||||||
|
aud := audit.New(db.DB)
|
||||||
|
us := users.New(db.DB, aud)
|
||||||
|
if _, err := us.Create(ctx, 0, "admin@example.com", "Claus", users.RoleAdmin); err != nil {
|
||||||
|
t.Fatalf("create admin: %v", err)
|
||||||
|
}
|
||||||
|
sealer, err := auth.NewAEADSealer([]byte(key))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewAEADSealer: %v", err)
|
||||||
|
}
|
||||||
|
ids := auth.New(db.DB, aud, auth.WithSealer(sealer)).Identities()
|
||||||
|
idID, _, err := ids.Mint(ctx, 1)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Mint: %v", err)
|
||||||
|
}
|
||||||
|
if err := ids.Activate(ctx, idID); err != nil {
|
||||||
|
t.Fatalf("Activate: %v", err)
|
||||||
|
}
|
||||||
|
const secret = "JBSWY3DPEHPK3PXP"
|
||||||
|
if err := ids.PutTOTPSecret(ctx, idID, []byte(secret)); err != nil {
|
||||||
|
t.Fatalf("PutTOTPSecret: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The wizard persists the config (`finish`).
|
||||||
|
pre, err := httpx.NewPreSessionCSRF([]byte(cfg.Secrets.CookieSecret), false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewPreSessionCSRF: %v", err)
|
||||||
|
}
|
||||||
|
wiz, err := setup.New(setup.Deps{
|
||||||
|
Config: cfg, ConfigPath: cfgPath, Users: us, Identities: ids,
|
||||||
|
Enroll: stubEnroller{}, CSRF: pre, Audit: aud, Printout: io.Discard,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("setup.New: %v", err)
|
||||||
|
}
|
||||||
|
form := url.Values{"app.locale": {"en-GB"}, "app.timezone": {"Europe/London"}}
|
||||||
|
req := httptest.NewRequest("POST", "/setup?do=setup&action=finish", strings.NewReader(form.Encode()))
|
||||||
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
if err := wiz.Finish(rec, req); err != nil {
|
||||||
|
t.Fatalf("Finish: %v", err)
|
||||||
|
}
|
||||||
|
if rec.Code != http.StatusSeeOther {
|
||||||
|
t.Fatalf("Finish = %d, want 303", rec.Code)
|
||||||
|
}
|
||||||
|
if err := db.Close(); err != nil {
|
||||||
|
t.Fatalf("close db: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Restart: the persisted config must carry the SAME TOTP key, and the
|
||||||
|
// sealed secret must still open with it (D-1 — the whole point).
|
||||||
|
restored, err := config.Load(cfgPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("the persisted config does not load after setup: %v", err)
|
||||||
|
}
|
||||||
|
if restored.Setup {
|
||||||
|
t.Error("the persisted config still says setup: true")
|
||||||
|
}
|
||||||
|
if restored.Secrets.TOTPEncryptionKey != key {
|
||||||
|
t.Fatalf("the persisted TOTP key differs from the runtime key (D-1):\n got %q\nwant %q",
|
||||||
|
restored.Secrets.TOTPEncryptionKey, key)
|
||||||
|
}
|
||||||
|
|
||||||
|
db2, err := store.Open(restored.Database.Path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reopen db: %v", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = db2.Close() }()
|
||||||
|
sealer2, err := auth.NewAEADSealer([]byte(restored.Secrets.TOTPEncryptionKey))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("restart sealer: %v", err)
|
||||||
|
}
|
||||||
|
ids2 := auth.New(db2.DB, nil, auth.WithSealer(sealer2)).Identities()
|
||||||
|
got, err := ids2.TOTPSecret(ctx, idID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("the TOTP secret no longer unseals after a restart (D-1/M2c): %v", err)
|
||||||
|
}
|
||||||
|
if string(got) != secret {
|
||||||
|
t.Errorf("TOTP secret after restart = %q, want %q", got, secret)
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue