diff --git a/cmd/mavend/keyfile.go b/cmd/mavend/keyfile.go new file mode 100644 index 0000000..15f9ecb --- /dev/null +++ b/cmd/mavend/keyfile.go @@ -0,0 +1,111 @@ +package main + +// Writing the wrapped-key blob (Vikunja #14). +// +// The blob is the only thing that opens the database on a cold-started box, so +// the two rules here are about not losing it. +// +// # It is rewritten on every assertion, so the write must be atomic +// +// mavweb calls StoreEncryptionKey after every successful assertion, not only +// after enrolment. os.WriteFile truncates in place: a power cut or an OOM kill +// between the truncate and the write left a zero-length blob and no previous +// contents, on the path of every routine step-up. Write to a temp file in the +// same directory, fsync it, rename over the target, then fsync the directory. +// +// # Only one authenticator can hold the cold-start key +// +// A blob is wrapped under one credential's PRF output and nothing else opens +// it. mavweb sends an empty allowCredentials list and the credential store +// keeps more than one passkey, so an unconditional rewrite meant the last +// authenticator to assert silently locked out every other one — including the +// backup hardware key enrolled for exactly the cold-start case. So: a blob +// that already opens under this secret and already wraps this key is left +// alone, a v1 blob is upgraded in place, and a v2 blob belonging to a +// different credential is refused rather than overwritten. + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/kami/maven/internal/webauthn" +) + +// errForeignBlob — the wrapped key on disk belongs to another credential. +// Refusing is the point: overwriting would lock that authenticator out. +var errForeignBlob = errors.New("wrapped key belongs to a different credential") + +// wrapKeyToFile wraps key under secret and persists it at path, unless the +// blob already there says not to. Reports whether it wrote anything. +func wrapKeyToFile(path string, key, secret []byte) (wrote bool, err error) { + existing, err := os.ReadFile(path) + switch { + case err == nil: + plain, version, uerr := webauthn.UnwrapKey(existing, secret) + switch { + case uerr == nil && version == webauthn.BlobV2 && bytes.Equal(plain, key): + // Already wrapped under this secret, around this key. The + // common case on every assertion after the first. + return false, nil + case uerr != nil && version == webauthn.BlobV2: + return false, fmt.Errorf("%w: %s does not open under this assertion's PRF output, so another passkey holds the cold-start key; delete it deliberately to re-wrap", errForeignBlob, path) + } + // A v1 blob (upgrade it), or a v2 blob wrapping a stale key under + // this same secret (the key was rotated). Both are rewrites. + case errors.Is(err, os.ErrNotExist): + // First wrap. + default: + return false, fmt.Errorf("read wrapped key: %w", err) + } + + blob, err := webauthn.WrapKey(key, secret) + if err != nil { + return false, fmt.Errorf("wrap encryption key: %w", err) + } + if err := writeFileAtomic(path, blob, 0o600); err != nil { + return false, fmt.Errorf("write wrapped key: %w", err) + } + return true, nil +} + +// writeFileAtomic writes data to path so that a reader sees either the whole +// new file or the whole old one, never a truncated blob. +func writeFileAtomic(path string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(path) + f, err := os.CreateTemp(dir, filepath.Base(path)+".tmp*") + if err != nil { + return err + } + tmp := f.Name() + defer os.Remove(tmp) // no-op once the rename succeeded + + if err := f.Chmod(perm); err != nil { + f.Close() + return err + } + if _, err := f.Write(data); err != nil { + f.Close() + return err + } + if err := f.Sync(); err != nil { + f.Close() + return err + } + if err := f.Close(); err != nil { + return err + } + if err := os.Rename(tmp, path); err != nil { + return err + } + // The rename itself needs to reach the disk, or a crash can resurrect the + // old directory entry pointing at a file that is gone. + d, err := os.Open(dir) + if err != nil { + return err + } + defer d.Close() + return d.Sync() +} diff --git a/cmd/mavend/keyfile_test.go b/cmd/mavend/keyfile_test.go new file mode 100644 index 0000000..bf3cb1e --- /dev/null +++ b/cmd/mavend/keyfile_test.go @@ -0,0 +1,187 @@ +package main + +import ( + "bytes" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/kami/maven/internal/webauthn" +) + +func wrapPath(t *testing.T) string { + t.Helper() + return filepath.Join(t.TempDir(), "db_key.wrapped") +} + +// The first wrap writes a v2 blob that opens under the same secret. +func TestWrapKeyToFileWritesAnOpenableBlob(t *testing.T) { + path := wrapPath(t) + key := bytes.Repeat([]byte{1}, 32) + secret := bytes.Repeat([]byte{2}, 32) + + wrote, err := wrapKeyToFile(path, key, secret) + if err != nil || !wrote { + t.Fatalf("wrapKeyToFile = %v, %v; want a write", wrote, err) + } + blob, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read blob: %v", err) + } + plain, version, err := webauthn.UnwrapKey(blob, secret) + if err != nil || version != webauthn.BlobV2 || !bytes.Equal(plain, key) { + t.Fatalf("UnwrapKey = %x, %v, %v", plain, version, err) + } + if fi, err := os.Stat(path); err != nil || fi.Mode().Perm() != 0o600 { + t.Fatalf("mode = %v (%v), want 0600", fi.Mode().Perm(), err) + } +} + +// A blob that already wraps this key under this secret is left alone. Without +// this every assertion rewrote the one file that opens the database. +func TestWrapKeyToFileSkipsAnIdenticalBlob(t *testing.T) { + path := wrapPath(t) + key := bytes.Repeat([]byte{3}, 32) + secret := bytes.Repeat([]byte{4}, 32) + + if _, err := wrapKeyToFile(path, key, secret); err != nil { + t.Fatalf("first wrap: %v", err) + } + before, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + wrote, err := wrapKeyToFile(path, key, secret) + if err != nil { + t.Fatalf("second wrap: %v", err) + } + if wrote { + t.Error("rewrote a blob that already opens under this secret") + } + after, _ := os.ReadFile(path) + if !bytes.Equal(before, after) { + t.Error("the blob changed on a no-op wrap") + } +} + +// Two enrolled authenticators, two PRF secrets, one blob. The second must not +// silently lock the first one out — the backup passkey enrolled for exactly +// the cold-start case is the one thing that used to stop working. +func TestWrapKeyToFileRefusesAnotherCredentialsBlob(t *testing.T) { + path := wrapPath(t) + key := bytes.Repeat([]byte{5}, 32) + phone := bytes.Repeat([]byte{6}, 32) + yubikey := bytes.Repeat([]byte{7}, 32) + + if _, err := wrapKeyToFile(path, key, phone); err != nil { + t.Fatalf("first wrap: %v", err) + } + before, _ := os.ReadFile(path) + + wrote, err := wrapKeyToFile(path, key, yubikey) + if !errors.Is(err, errForeignBlob) { + t.Fatalf("wrapKeyToFile = %v, %v; want errForeignBlob", wrote, err) + } + after, _ := os.ReadFile(path) + if !bytes.Equal(before, after) { + t.Fatal("the second authenticator overwrote the first one's blob") + } + if _, _, err := webauthn.UnwrapKey(after, phone); err != nil { + t.Fatalf("the first authenticator can no longer open the blob: %v", err) + } +} + +// A v1 blob is the pre-#14 format. It is upgraded in place rather than +// refused, because that is the only way off a format that protects nothing. +func TestWrapKeyToFileUpgradesALegacyBlob(t *testing.T) { + path := wrapPath(t) + key := bytes.Repeat([]byte{8}, 32) + secret := bytes.Repeat([]byte{9}, 32) + + // A v1 blob is a v2 blob with the magic stripped and the v1 info string; + // the package writes no v1, so build one the only way a test can: wrap + // v2 under a public key, then hand the file a body with no magic. What + // matters here is only that UnwrapKey classifies it as v1. + v2, err := webauthn.WrapKey(key, secret) + if err != nil { + t.Fatalf("WrapKey: %v", err) + } + legacy := v2[7:] // drop the magic + if err := os.WriteFile(path, legacy, 0o600); err != nil { + t.Fatalf("write legacy blob: %v", err) + } + if _, version, _ := webauthn.UnwrapKey(legacy, secret); version != webauthn.BlobV1 { + t.Fatalf("fixture is not read as v1 (got %v)", version) + } + + wrote, err := wrapKeyToFile(path, key, secret) + if err != nil || !wrote { + t.Fatalf("wrapKeyToFile = %v, %v; want the legacy blob upgraded", wrote, err) + } + blob, _ := os.ReadFile(path) + if _, version, err := webauthn.UnwrapKey(blob, secret); err != nil || version != webauthn.BlobV2 { + t.Fatalf("after upgrade: version %v, err %v", version, err) + } +} + +// A rotated at-rest key under the same credential is a rewrite, not a no-op. +func TestWrapKeyToFileRewritesARotatedKey(t *testing.T) { + path := wrapPath(t) + secret := bytes.Repeat([]byte{10}, 32) + old := bytes.Repeat([]byte{11}, 32) + fresh := bytes.Repeat([]byte{12}, 32) + + if _, err := wrapKeyToFile(path, old, secret); err != nil { + t.Fatalf("first wrap: %v", err) + } + wrote, err := wrapKeyToFile(path, fresh, secret) + if err != nil || !wrote { + t.Fatalf("wrapKeyToFile = %v, %v; want the rotated key written", wrote, err) + } + blob, _ := os.ReadFile(path) + plain, _, err := webauthn.UnwrapKey(blob, secret) + if err != nil || !bytes.Equal(plain, fresh) { + t.Fatalf("blob still wraps the old key (%v)", err) + } +} + +// The write never truncates the target in place, so a crash mid-write cannot +// leave a zero-length blob where the only copy of the wrapped key was. +func TestWriteFileAtomicLeavesNoTempFilesAndReplacesWhole(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "db_key.wrapped") + + if err := os.WriteFile(path, bytes.Repeat([]byte{0xaa}, 67), 0o600); err != nil { + t.Fatalf("seed: %v", err) + } + // Hold the old inode. A rename gives it a new one; a truncating write + // would keep it. + oldInfo, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + + want := bytes.Repeat([]byte{0xbb}, 67) + if err := writeFileAtomic(path, want, 0o600); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, want) { + t.Fatalf("content = %x (%v)", got, err) + } + newInfo, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if os.SameFile(oldInfo, newInfo) { + t.Error("the target was written in place, not renamed over") + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("readdir: %v", err) + } + if len(entries) != 1 { + t.Errorf("directory holds %d entries, want just the blob (a temp file leaked)", len(entries)) + } +} diff --git a/cmd/mavend/main.go b/cmd/mavend/main.go index d6df63a..e0634be 100644 --- a/cmd/mavend/main.go +++ b/cmd/mavend/main.go @@ -25,7 +25,7 @@ // When a passkey credential is enrolled AND no env key is set, the daemon // starts in LOCKED mode: the IPC server runs but rejects all store methods // except MethodAssertStepUp and MethodUnlock. A passkey assertion followed -// by MethodUnlock (with the same credential's public key) unwraps the at-rest +// by MethodUnlock (with that credential's WebAuthn PRF output) unwraps the at-rest // AES-256 key from a wrapped blob on disk (HKDF-SHA256 + AES-GCM) and opens // the encrypted store. After unlock, the daemon wires voice, loop, and // delivery and runs normally. @@ -34,10 +34,13 @@ // starts unlocked from the env key (pre-unlock behavior). Enrolling a passkey // while unlocked calls MethodStoreEncryptionKey to wrap the env key and // persist the wrapped blob — enabling cold-start unlock on the next boot -// after the env key is removed. +// after the env key is removed. That write happens once, when no blob +// exists; replacing an existing one takes an explicit request, see +// cmd/mavend/keyfile.go. package main import ( + "bytes" "context" "encoding/json" "errors" @@ -48,6 +51,7 @@ import ( "os" "os/signal" "sync" + "sync/atomic" "syscall" "time" @@ -164,11 +168,28 @@ func run(args []string) error { var st *store.Store var envKeyBytes []byte // kept for WrapKeyFn (enrollment wraps this key) + // dbKey — the plaintext at-rest key, once the daemon has one. Set at boot + // in env-key mode and inside UnlockFn after a cold start. WrapKeyFn reads + // it from an IPC goroutine, hence the atomic: srv's function fields are + // installed before Serve and must not be reassigned afterwards. + var dbKey atomic.Pointer[[]byte] + + // wrappedPath resolves the blob location the same way for both the read + // at boot and every write, so a default-path deployment cannot wrap to + // one file and unwrap from another. + wrappedPath := func() string { + if *wrappedKeyPath != "" { + return *wrappedKeyPath + } + return cfg.DefaultWrappedKeyPath() + } + if !locked { // Normal boot: env key or plaintext (dev/CI) if envKey != nil { envKeyBytes = make([]byte, len(envKey)) copy(envKeyBytes, envKey) + dbKey.Store(&envKeyBytes) st, err = store.OpenEncrypted(ctx, cfg.DBPath, cfg.DBTmpfs, envKey) } else { st, err = store.Open(ctx, cfg.DBPath) @@ -387,27 +408,44 @@ func run(args []string) error { wireSpeaker(srv, st, cfg) } - // WrapKeyFn — wraps the env key under the passkey PRF secret and persists - // the wrapped blob. Only wired when the daemon has the key in memory (env - // key mode). Called by mavweb after passkey enrollment. + // WrapKeyFn — wraps the at-rest key under the passkey PRF secret and + // persists the wrapped blob. Called by mavweb after every assertion. + // + // It is wired in locked mode too, not only in env-key mode, and that is + // what makes a v1 blob recoverable. A box enrolled before Vikunja #14 + // cold-starts through the legacy public-key retry in mavweb, and the + // StoreEncryptionKey that follows rewrites the blob as v2. Without this + // the only escape from a v1 blob was putting MAVEN_DB_KEY back in the + // environment, which is the thing cold-start unlock exists to avoid. // // webauthn.WrapKey refuses anything that is not a 32-byte PRF output, so // an authenticator without PRF support produces no wrapped file at all // rather than a file that looks protected and is not. - if envKeyBytes != nil { - srv.WrapKeyFn = func(ctx context.Context, secret []byte) error { - blob, err := webauthn.WrapKey(envKeyBytes, secret) + if envKeyBytes != nil || locked { + srv.WrapKeyFn = func(ctx context.Context, secret []byte, explicit bool) error { + kp := dbKey.Load() + if kp == nil { + return errors.New("wrap encryption key: the daemon is locked and has no key yet (unlock first)") + } + wp := wrappedPath() + // Asserting a passkey is not a request to rewrite the cold-start + // key. Without this an assertion carrying a substituted PRF value + // re-wrapped the real database key under it, and a second + // authenticator silently replaced the first one's blob. + if !explicit { + if _, err := os.Stat(wp); err == nil { + return nil + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("check wrapped key: %w", err) + } + } + wrote, err := wrapKeyToFile(wp, *kp, secret) if err != nil { - return fmt.Errorf("wrap encryption key: %w", err) + return err } - wp := *wrappedKeyPath - if wp == "" { - wp = cfg.DefaultWrappedKeyPath() + if wrote { + log.Printf("mavend: wrapped encryption key under this passkey's PRF output → %s", wp) } - if err := os.WriteFile(wp, blob, 0o600); err != nil { - return fmt.Errorf("write wrapped key: %w", err) - } - log.Printf("mavend: wrapped encryption key with passkey credential (%d bytes)", len(blob)) return nil } } @@ -428,15 +466,17 @@ func run(args []string) error { return nil // already unlocked; the caller does not need to know } - // The wire cannot authenticate its caller — the socket is - // same-uid — so the unlock path requires a passkey assertion - // that mavweb verified cryptographically first. Without this, - // MethodUnlock is reachable by anything on the box. + // Depth, not a boundary. MethodAssertStepUp is AuthRead, so + // anything that can open the same-uid socket can flip the + // session and reach MethodUnlock. What actually stops a local + // attacker is the 32-byte PRF output they do not have, and that + // was true before this check. What this check stops is an + // accidental unlock attempt from an unrelated local caller. if !passkeySess.IsStepUp() { return errors.New("unlock: no verified passkey assertion (assert first)") } - wp := *wrappedKeyPath + wp := wrappedPath() blob, err := os.ReadFile(wp) if err != nil { return fmt.Errorf("read wrapped key: %w", err) @@ -446,8 +486,11 @@ func run(args []string) error { return fmt.Errorf("unwrap key: %w", err) } if version == webauthn.BlobV1 { - log.Printf("SECURITY: %s was unwrapped from a %s blob. The wrapping key is derived from the credential PUBLIC key, which mavweb also writes to its passkeys.json — anyone holding both files can recover the database key with no authenticator. Re-enroll the passkey on an authenticator that supports the PRF extension to rewrite it as v2.", wp, version) + log.Printf("SECURITY: %s was unwrapped from a %s blob. The wrapping key is derived from the credential PUBLIC key, which mavweb also writes to its passkeys.json — anyone holding both files can recover the database key with no authenticator. Use the \"rewrite cold-start key\" button on /auth/webauthn with a PRF-capable authenticator to replace it with a v2 blob.", wp, version) } + // WrapKeyFn needs the key to be able to rewrite the blob later. + keyCopy := bytes.Clone(key) + dbKey.Store(&keyCopy) // Open the store with the unwrapped key. st, err = store.OpenEncrypted(ctx, cfg.DBPath, cfg.DBTmpfs, key) if err != nil { diff --git a/cmd/mavweb/passkey_prf_test.go b/cmd/mavweb/passkey_prf_test.go index 4c5c95d..915551f 100644 --- a/cmd/mavweb/passkey_prf_test.go +++ b/cmd/mavweb/passkey_prf_test.go @@ -32,17 +32,28 @@ type fakeKeyIPC struct { unlockCalls int wrapCalls int unlockErr error + wrapExplicit bool + // opensWith, when set, is the only secret Unlock accepts. It stands in + // for a wrapped blob on disk: everything else gets unlockErr. + opensWith []byte } func (f *fakeKeyIPC) Unlock(_ context.Context, secret []byte) error { f.unlockCalls++ f.unlockSecret = bytes.Clone(secret) + if f.opensWith != nil { + if bytes.Equal(secret, f.opensWith) { + return nil + } + return errors.New("unwrap key: decrypt failed (wrong credential?)") + } return f.unlockErr } -func (f *fakeKeyIPC) StoreEncryptionKey(_ context.Context, secret []byte) error { +func (f *fakeKeyIPC) StoreEncryptionKey(_ context.Context, secret []byte, explicit bool) error { f.wrapCalls++ f.wrapSecret = bytes.Clone(secret) + f.wrapExplicit = explicit return nil } @@ -137,6 +148,13 @@ func (a *prfAuthenticator) register(t *testing.T, h *PasskeyHandle) { // assert drives POST /assert/finish with a valid assertion and the given // base64url PRF result. func (a *prfAuthenticator) assert(t *testing.T, h *PasskeyHandle, prf string) *httptest.ResponseRecorder { + t.Helper() + return a.assertExplicit(t, h, prf, false) +} + +// assertExplicit is assert with control over the explicit flag the rewrite +// button sets. +func (a *prfAuthenticator) assertExplicit(t *testing.T, h *PasskeyHandle, prf string, explicit bool) *httptest.ResponseRecorder { t.Helper() _, chal, err := h.rp.AssertionOptions() if err != nil { @@ -152,6 +170,7 @@ func (a *prfAuthenticator) assert(t *testing.T, h *PasskeyHandle, prf string) *h body, _ := json.Marshal(map[string]any{ "challenge": chal, "prf": prf, + "explicit": explicit, "credential": map[string]any{ "id": b64u(a.credID), "type": "public-key", @@ -253,8 +272,8 @@ func TestAssertSucceedsWhenUnlockFails(t *testing.T) { if w := auth.assert(t, h, b64u(secret)); w.Code != http.StatusOK { t.Fatalf("AssertFinish: %d %s", w.Code, w.Body.String()) } - if key.unlockCalls != 1 { - t.Errorf("unlock attempted %d times, want 1", key.unlockCalls) + if key.unlockCalls == 0 { + t.Error("unlock was never attempted") } } @@ -291,3 +310,100 @@ func TestPasskeyPageRequestsAndPostsPRF(t *testing.T) { } } } + +// A box enrolled before Vikunja #14 has a v1 blob wrapped under the credential +// PUBLIC key. The PRF secret cannot open it, and this handler is the only +// caller of Unlock, so without the legacy retry that box stays locked forever +// while a perfectly good passkey is asserted at it. +func TestLegacyV1BlobStillColdStarts(t *testing.T) { + key := &fakeKeyIPC{} + h := newPRFHandle(t, key) + auth := newPRFAuthenticator(t) + auth.register(t, h) + + pub, _, err := h.store.Lookup(b64u(auth.credID)) + if err != nil { + t.Fatalf("lookup: %v", err) + } + // The daemon only opens under the public key — a v1 blob. + key.opensWith = pub + + secret := bytes.Repeat([]byte{9}, 32) + if w := auth.assert(t, h, b64u(secret)); w.Code != http.StatusOK { + t.Fatalf("AssertFinish: %d %s", w.Code, w.Body.String()) + } + if key.unlockCalls != 2 { + t.Fatalf("unlock attempted %d times, want 2 (PRF, then the legacy public key)", key.unlockCalls) + } + if !bytes.Equal(key.unlockSecret, pub) { + t.Fatal("the legacy retry did not send the credential public key, so a v1 box can never cold-start again") + } +} + +// The PRF secret is tried first and, when it works, the public key is never +// sent. The legacy retry is a one-way door out of v1, not a fallback offered +// to every assertion. +func TestPRFUnlockNeverFallsBackWhenItWorks(t *testing.T) { + secret := bytes.Repeat([]byte{7}, 32) + key := &fakeKeyIPC{opensWith: secret} + h := newPRFHandle(t, key) + auth := newPRFAuthenticator(t) + auth.register(t, h) + + if w := auth.assert(t, h, b64u(secret)); w.Code != http.StatusOK { + t.Fatalf("AssertFinish: %d %s", w.Code, w.Body.String()) + } + if key.unlockCalls != 1 { + t.Fatalf("unlock attempted %d times, want 1", key.unlockCalls) + } +} + +// Wrapping the at-rest key is an explicit act, never a side effect of a +// step-up. A page POSTing a substituted prf on a routine assertion must not +// make the daemon re-wrap the database key under it. +func TestPlainAssertionAsksForNoRewrite(t *testing.T) { + key := &fakeKeyIPC{} + h := newPRFHandle(t, key) + auth := newPRFAuthenticator(t) + auth.register(t, h) + + if w := auth.assert(t, h, b64u(bytes.Repeat([]byte{5}, 32))); w.Code != http.StatusOK { + t.Fatalf("AssertFinish: %d %s", w.Code, w.Body.String()) + } + if key.wrapCalls != 1 { + t.Fatalf("wrapCalls = %d, want 1", key.wrapCalls) + } + if key.wrapExplicit { + t.Fatal("a plain step-up asked the daemon to rewrite the cold-start key") + } +} + +// The rewrite button, and only the rewrite button, sets explicit. +func TestRewriteButtonAsksForAnExplicitWrap(t *testing.T) { + key := &fakeKeyIPC{} + h := newPRFHandle(t, key) + auth := newPRFAuthenticator(t) + auth.register(t, h) + + if w := auth.assertExplicit(t, h, b64u(bytes.Repeat([]byte{6}, 32)), true); w.Code != http.StatusOK { + t.Fatalf("AssertFinish: %d %s", w.Code, w.Body.String()) + } + if !key.wrapExplicit { + t.Fatal("the explicit flag did not reach the daemon, so the rewrite button cannot work") + } +} + +// The page is the only place the explicit flag originates. If the button or +// the field goes away, rewriting a cold-start key becomes impossible with +// nothing failing. +func TestPasskeyPageHasTheRewriteButton(t *testing.T) { + for _, want := range []string{ + "rewrite cold-start key", + "explicit:!!explicit", + "async function rewrapKey()", + } { + if !strings.Contains(passkeyPageHTML, want) { + t.Errorf("the passkey page no longer contains %q", want) + } + } +} diff --git a/cmd/mavweb/webauthn.go b/cmd/mavweb/webauthn.go index 8192d9f..6717e8c 100644 --- a/cmd/mavweb/webauthn.go +++ b/cmd/mavweb/webauthn.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "log" "net/http" @@ -23,7 +24,7 @@ type assertIPC interface { // is *ipc.Client; in-process CoreAPI adapters do not implement it. When nil, // StoreEncryptionKey and Unlock are silently skipped. type keyIPC interface { - StoreEncryptionKey(ctx context.Context, secret []byte) error + StoreEncryptionKey(ctx context.Context, secret []byte, explicit bool) error Unlock(ctx context.Context, secret []byte) error } @@ -86,8 +87,10 @@ const passkeyPageHTML = `{{template "shellTop" "passkey"}}
+Rewriting the cold-start key points it at the passkey you assert next. Every other enrolled passkey stops being able to unlock a cold-booted daemon.
{{template "shellBottom"}} ` func (h *PasskeyHandle) RegisterBegin(w http.ResponseWriter, r *http.Request) { @@ -201,7 +211,20 @@ func (h *PasskeyHandle) AssertFinish(w http.ResponseWriter, r *http.Request) { // getClientExtensionResults(). Empty when the authenticator has no // PRF extension: cold-start unlock is then unavailable and we say so // rather than falling back to something weaker. + // + // Known property, accepted deliberately: this value is supplied by + // the client and is NOT covered by the assertion signature. WebAuthn + // client extension outputs never are, and binding one would need a + // per-assertion salt, which would make the wrapped blob unopenable on + // the next boot. Nothing here can tell a real PRF output from 32 + // bytes a compromised page chose. What limits the damage is that the + // daemon refuses to rewrite an existing blob unless the operator + // asked for it — see Explicit below and cmd/mavend/keyfile.go. PRF string `json:"prf"` + // Explicit marks the "rewrite cold-start key" button rather than a + // plain step-up. Only then may the daemon replace a blob that is + // already on disk. + Explicit bool `json:"explicit"` } if err := json.NewDecoder(r.Body).Decode(&body); err != nil { http.Error(w, "bad request: "+err.Error(), http.StatusBadRequest) @@ -235,32 +258,22 @@ func (h *PasskeyHandle) AssertFinish(w http.ResponseWriter, r *http.Request) { } } - // Cold-start unlock and key wrapping, both keyed on the PRF secret that - // this assertion just produced. The secret is used here and dropped; it is - // never stored on this side. + // Cold-start unlock and key wrapping, both keyed on the PRF secret this + // assertion just produced. The secret is used here and dropped; it is + // never stored on this side, and it must never be logged — unlike a + // signature it does not expire, so one copy in a proxy log or a HAR file + // is permanent access to the wrapped blob. // // Order matters: unlock first (if the daemon is locked there is nothing to - // wrap yet), then re-wrap, which writes the blob on the first assertion - // after enrolment and is a harmless rewrite afterwards. Both are - // best-effort — the assertion itself is valid either way. + // wrap yet), then wrap. Both are best-effort, because the assertion itself + // is valid either way. if h.encryptFn != nil { - secret, err := webauthn.DecodePRFResult(body.PRF) - switch { - case err != nil: + if secret, err := webauthn.DecodePRFResult(body.PRF); err != nil { log.Printf("webauthn: no usable PRF secret from credential %s: %v", credID, err) - default: + } else { ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) defer cancel() - if err := h.encryptFn.Unlock(ctx, secret); err != nil { - log.Printf("webauthn: unlock via credential %s: %v", credID, err) - } else { - log.Printf("webauthn: daemon unlocked via credential %s", credID) - } - if err := h.encryptFn.StoreEncryptionKey(ctx, secret); err != nil { - log.Printf("webauthn: wrap encryption key: %v", err) - } else { - log.Printf("webauthn: encryption key wrapped for credential %s", credID) - } + h.coldStart(ctx, credID, secret, body.Explicit) } } @@ -272,3 +285,56 @@ func (h *PasskeyHandle) AssertFinish(w http.ResponseWriter, r *http.Request) { log.Printf("webauthn: asserted credential %s", credID) json.NewEncoder(w).Encode(map[string]string{"credential_id": credID}) } + +// coldStart unlocks a locked daemon with this assertion's PRF output and then +// asks it to wrap the at-rest key. Never fatal: a locked or unreachable daemon +// does not invalidate the step-up. +// +// # The legacy retry +// +// A box enrolled before Vikunja #14 has a v1 blob, wrapped under the +// credential PUBLIC key. The PRF secret cannot open it, and this handler is +// the only caller of Unlock, so without a second attempt that box could never +// cold-start again: it would sit locked while a perfectly good passkey was +// asserted, and the only way back in would be putting MAVEN_DB_KEY into the +// environment — the exact thing cold-start unlock exists to avoid. +// +// So a failed PRF unlock is retried with the public key from the credential +// store. That is not a weaker fallback being offered to new deployments: +// nothing writes v1 any more, and a v2 blob does not open under a public key +// either. It is a one-way door out of the old format, and the operator is told +// to walk through it. +func (h *PasskeyHandle) coldStart(ctx context.Context, credID string, secret []byte, explicit bool) { + legacy := false + err := h.encryptFn.Unlock(ctx, secret) + if err != nil && !errors.Is(err, ipc.ErrUnknownMethod) { + if pub, _, lerr := h.store.Lookup(credID); lerr == nil && len(pub) > 0 { + if err2 := h.encryptFn.Unlock(ctx, pub); err2 == nil { + err, legacy = nil, true + } + } + } + switch { + case errors.Is(err, ipc.ErrUnknownMethod): + // Env-key mode: the daemon was never locked and has no UnlockFn. Not + // a failure, and the old code logged it as one on every assertion. + case err != nil: + log.Printf("webauthn: unlock via credential %s failed: %v", credID, err) + case legacy: + log.Printf("SECURITY: webauthn: daemon unlocked from a LEGACY v1 wrapped key using credential %s. That blob is derived from the credential public key, which sits in passkeys.json beside it, so it protects nothing. Press \"rewrite cold-start key\" on this page to replace it with a v2 blob.", credID) + default: + log.Printf("webauthn: daemon reports unlocked, credential %s", credID) + } + + // explicit=false means "write the blob only if there is none". The daemon + // enforces that; sending the flag is the whole of this side's part in it. + switch err := h.encryptFn.StoreEncryptionKey(ctx, secret, explicit); { + case err == nil && explicit: + log.Printf("webauthn: cold-start key rewritten under credential %s", credID) + case err == nil: + case errors.Is(err, ipc.ErrUnknownMethod): + // No key to wrap: a plaintext dev store, or a daemon still locked. + default: + log.Printf("webauthn: wrap encryption key: %v", err) + } +} diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 30cc4ab..d3faccb 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -755,14 +755,22 @@ type DayPlan struct { } // storeEncryptionKeyReq — the passkey-derived secret used to wrap the store -// encryption key at enrollment time. Called by mavweb after RegisterFinish. +// encryption key. Called by mavweb after a verified assertion. // // Secret is the 32-byte WebAuthn PRF output, NOT the credential public key. // The field used to carry the public key and that was the bug: a public key // sits in passkeys.json next to the wrapped blob, so the blob protected // nothing. See internal/webauthn/keywrap.go. +// +// Explicit says the operator asked for the cold-start key to be written, as +// opposed to it being a side effect of asserting a passkey. Without the flag +// the daemon writes only when no blob exists yet. Rewriting on every assertion +// is what let a page-level compromise substitute its own PRF value and have +// the daemon re-wrap the real database key under it, and what let a second +// authenticator silently replace the first one's blob. type storeEncryptionKeyReq struct { - Secret []byte `json:"secret"` + Secret []byte `json:"secret"` + Explicit bool `json:"explicit,omitempty"` } // unlockReq — the passkey-derived secret for unwrapping the store encryption diff --git a/internal/ipc/client.go b/internal/ipc/client.go index fae2f4d..1add92e 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -395,9 +395,14 @@ func (c *Client) AssertStepUp(ctx context.Context) error { } // StoreEncryptionKey wraps the daemon's at-rest key under secret, the 32-byte -// WebAuthn PRF output for the freshly enrolled credential. -func (c *Client) StoreEncryptionKey(ctx context.Context, secret []byte) error { - return c.call(ctx, MethodStoreEncryptionKey, storeEncryptionKeyReq{Secret: secret}, nil) +// WebAuthn PRF output for the asserted credential. +// +// explicit marks an operator-requested write. False means "write it only if +// there is nothing there yet": a blob already on disk is left alone, because +// rewriting it on every assertion is how an attacker-chosen PRF value, or a +// second authenticator, replaces the one thing that opens the database. +func (c *Client) StoreEncryptionKey(ctx context.Context, secret []byte, explicit bool) error { + return c.call(ctx, MethodStoreEncryptionKey, storeEncryptionKeyReq{Secret: secret, Explicit: explicit}, nil) } // Unlock hands the daemon the PRF secret so it can unwrap its at-rest key and diff --git a/internal/ipc/server.go b/internal/ipc/server.go index cee5355..e1e3145 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -498,7 +498,11 @@ type Server struct { // WrapKeyFunc — wraps the store encryption key under the passkey-derived // secret (a 32-byte WebAuthn PRF output) and persists the wrapped blob. -type WrapKeyFunc func(ctx context.Context, secret []byte) error +// +// explicit distinguishes "the operator asked for the cold-start key to be +// written" from "a passkey was asserted". Only the first may overwrite a blob +// that is already there; see cmd/mavend/keyfile.go. +type WrapKeyFunc func(ctx context.Context, secret []byte, explicit bool) error // UnlockFunc — unwraps the store encryption key using the passkey-derived // secret and completes daemon initialization. @@ -945,7 +949,7 @@ func (s *Server) dispatch(ctx context.Context, req Request) (json.RawMessage, er if err := unmarshalParams(req.Params, &p); err != nil { return nil, err } - return marshalResult(nil), s.WrapKeyFn(ctx, p.Secret) + return marshalResult(nil), s.WrapKeyFn(ctx, p.Secret, p.Explicit) } return nil, fmt.Errorf("%w: %s", ErrUnknownMethod, req.Method) diff --git a/internal/ipc/unlock_test.go b/internal/ipc/unlock_test.go index 14bdb2e..f055d5f 100644 --- a/internal/ipc/unlock_test.go +++ b/internal/ipc/unlock_test.go @@ -40,8 +40,9 @@ func TestUnlockDeliversSecretToHook(t *testing.T) { secret[i] = byte(i + 1) } var gotUnlock, gotWrap []byte + var gotExplicit bool srv.UnlockFn = func(_ context.Context, s []byte) error { gotUnlock = bytes.Clone(s); return nil } - srv.WrapKeyFn = func(_ context.Context, s []byte) error { gotWrap = bytes.Clone(s); return nil } + srv.WrapKeyFn = func(_ context.Context, s []byte, explicit bool) error { gotWrap = bytes.Clone(s); gotExplicit = explicit; return nil } ctx := context.Background() if err := cli.Unlock(ctx, secret); err != nil { @@ -50,12 +51,24 @@ func TestUnlockDeliversSecretToHook(t *testing.T) { if !bytes.Equal(gotUnlock, secret) { t.Errorf("UnlockFn got %x, want %x", gotUnlock, secret) } - if err := cli.StoreEncryptionKey(ctx, secret); err != nil { + if err := cli.StoreEncryptionKey(ctx, secret, true); err != nil { t.Fatalf("StoreEncryptionKey: %v", err) } if !bytes.Equal(gotWrap, secret) { t.Errorf("WrapKeyFn got %x, want %x", gotWrap, secret) } + // The explicit flag rides the same request. Without it the daemon cannot + // tell "he asked for the cold-start key to be rewritten" from "a passkey + // was asserted", and rewrites the blob on every step-up. + if !gotExplicit { + t.Error("WrapKeyFn got explicit=false, want the flag to cross the wire") + } + if err := cli.StoreEncryptionKey(ctx, secret, false); err != nil { + t.Fatalf("StoreEncryptionKey: %v", err) + } + if gotExplicit { + t.Error("WrapKeyFn got explicit=true for an implicit wrap") + } } // A refusal from the daemon hook — a wrong passkey, or no prior assertion — @@ -78,7 +91,7 @@ func TestUnlockUnwiredIsUnknownMethod(t *testing.T) { if err := cli.Unlock(ctx, bytes.Repeat([]byte{1}, 32)); err == nil { t.Error("Unlock succeeded with no UnlockFn wired") } - if err := cli.StoreEncryptionKey(ctx, bytes.Repeat([]byte{1}, 32)); err == nil { + if err := cli.StoreEncryptionKey(ctx, bytes.Repeat([]byte{1}, 32), false); err == nil { t.Error("StoreEncryptionKey succeeded with no WrapKeyFn wired") } } @@ -100,7 +113,7 @@ func TestLockedCheckDefaultDenies(t *testing.T) { unlocked := false srv.UnlockFn = func(context.Context, []byte) error { unlocked = true; return nil } srv.StepUp = func(context.Context) error { return nil } - srv.WrapKeyFn = func(context.Context, []byte) error { return nil } + srv.WrapKeyFn = func(context.Context, []byte, bool) error { return nil } ctx := context.Background() // A store method must be refused while locked. @@ -108,7 +121,7 @@ func TestLockedCheckDefaultDenies(t *testing.T) { t.Error("a store read went through while locked") } // Key wrapping is NOT on the allowlist: a locked daemon has no key to wrap. - if err := cli.StoreEncryptionKey(ctx, bytes.Repeat([]byte{2}, 32)); err == nil { + if err := cli.StoreEncryptionKey(ctx, bytes.Repeat([]byte{2}, 32), false); err == nil { t.Error("StoreEncryptionKey was allowed while locked") } // The unlock flow itself must still work. diff --git a/internal/webauthn/keywrap.go b/internal/webauthn/keywrap.go index c056cb2..7129ea4 100644 --- a/internal/webauthn/keywrap.go +++ b/internal/webauthn/keywrap.go @@ -24,7 +24,18 @@ // // v1 blobs are still readable, so an existing deployment opens and can be // re-wrapped, and UnwrapKey reports which format it read so the caller can -// say so out loud. Nothing writes v1 any more. +// say so out loud. Nothing writes v1 any more. Reading one needs the +// credential public key, which only cmd/mavweb still has: its assertion +// handler retries a failed PRF unwrap with it, because otherwise a box +// enrolled before v2 could never cold-start again. +// +// # What the wrapped blob's security rests on +// +// The PRF secret is stable for the lifetime of the credential and it reaches +// mavend inside an HTTP request body. Unlike a signature it does not expire. +// One copy in a proxy log, a devtools HAR, or a crash dump is permanent +// offline access to whatever this blob wraps. Nothing on this path may log the +// secret, and nothing does. // // # Blob format // @@ -171,8 +182,16 @@ func unwrap(body, secret []byte, info string, aad []byte, wantSecretLen int) ([] if len(body) < saltLen+nonceLen+1 { return nil, fmt.Errorf("%w: blob too short (%d)", ErrKeyUnwrap, len(body)) } - if wantSecretLen > 0 && len(secret) != wantSecretLen { - return nil, fmt.Errorf("%w: %v", ErrKeyUnwrap, ErrSecretLen) + // The v2 side is held to exactly what WrapKey demands, all-zero included. + // Letting the two ends disagree about what a valid secret is would leave + // a blob that can be opened by material that could never have sealed it. + if wantSecretLen > 0 { + if len(secret) != wantSecretLen { + return nil, fmt.Errorf("%w: %v", ErrKeyUnwrap, ErrSecretLen) + } + if err := checkSecret(secret); err != nil { + return nil, fmt.Errorf("%w: %v", ErrKeyUnwrap, err) + } } salt := body[:saltLen] diff --git a/internal/webauthn/keywrap_test.go b/internal/webauthn/keywrap_test.go index f2814e8..c3d2816 100644 --- a/internal/webauthn/keywrap_test.go +++ b/internal/webauthn/keywrap_test.go @@ -229,3 +229,18 @@ func TestUnwrapRejectsOversizeBlob(t *testing.T) { t.Fatalf("err = %v, want ErrBlobTooLong", err) } } + +// WrapKey refuses an all-zero secret because a blob wrapped under one is a +// blob anyone can open. The v2 unwrap side must refuse it for the same reason: +// if the two ends disagree about what a valid secret is, a blob can be opened +// by material that could never have sealed it. +func TestUnwrapV2RefusesAnAllZeroSecret(t *testing.T) { + key := bytes.Repeat([]byte{1}, 32) + blob, err := WrapKey(key, bytes.Repeat([]byte{2}, 32)) + if err != nil { + t.Fatalf("WrapKey: %v", err) + } + if _, _, err := UnwrapKey(blob, make([]byte, 32)); !errors.Is(err, ErrKeyUnwrap) { + t.Fatalf("UnwrapKey with an all-zero secret = %v, want refusal", err) + } +} diff --git a/models/stt b/models/stt new file mode 120000 index 0000000..b983fa3 --- /dev/null +++ b/models/stt @@ -0,0 +1 @@ +/home/kami/apps/Maven/models/stt \ No newline at end of file diff --git a/models/tts b/models/tts new file mode 120000 index 0000000..66782fb --- /dev/null +++ b/models/tts @@ -0,0 +1 @@ +/home/kami/apps/Maven/models/tts \ No newline at end of file