kami/Maven
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Derive the cold-start unlock key from the passkey PRF, not the public key (#14) #77

Closed
claude wants to merge 1 commits from overnight/coldstart-unlock into overnight/voice-barge-in
pull from: overnight/coldstart-unlock
merge into: kami:overnight/voice-barge-in
Conversation 2 Commits 1 Files Changed 13
+1349 -168
claude commented 2026-08-01 03:49:53 +02:00
Contributor
Copy Link
Copy Source

What changed

Cold-start unlock (passkey → L3 → encrypted data at rest) wrapped the database key under the credential public key. mavweb writes that key verbatim to passkeys.json, normally in the same state dir as db_key.wrapped, so anyone holding both files recovered the database key offline with no authenticator involved. The wrapped blob was a plaintext key with extra steps.

The wrapping secret is now the WebAuthn PRF extension output — 32 bytes the authenticator computes over a fixed salt and never stores. Versioned blob:

v2:  "MVNKW2\x00" || salt(16) || nonce(12) || AES-256-GCM(key)   magic as AAD
v1:  salt(16) || nonce(12) || AES-256-GCM(key)                   read-only

v1 still opens (no bricked deployment) and reports itself, so mavend logs a SECURITY: line telling him to re-enroll. Nothing writes v1. The magic is GCM additional data, so a v2 blob cannot be header-stripped and re-read as v1.

Four other defects on the same path:

  • Silent total data loss after a cold start. The locked-boot store is opened on an IPC goroutine inside UnlockFn and was never closed; Close is what re-encrypts the tmpfs working copy back over the ciphertext file. daemonLock now owns the store and seals it at shutdown.
  • MethodUnlock was reachable by anything on the box. The socket is same-uid and cannot authenticate its caller. Unlock now requires a passkey assertion mavweb verified cryptographically first.
  • Concurrent unlocks would each open a store and wire a full daemon, with the loser's goroutines running against a store nobody closes. Now serialized, and idempotent.
  • Hand-rolled HKDF keyed the expand step with the salt instead of the PRK — self-consistent but not RFC 5869. Replaced with stdlib crypto/hkdf.

Key wrapping moved from enrolment to the first assertion: create() does not produce a PRF result on most authenticators, only a support flag. An authenticator without PRF now writes no wrapped file rather than one that looks protected and is not, and the passkey page says so out loud instead of failing open.

Both configurations behave: with no wrapped key file the daemon boots normally and the key IPC answers ErrUnknownMethod; with one and no env key it boots locked, default-denies every method except AssertStepUp/Unlock, and comes up on assertion.

Files

  • internal/webauthn/keywrap.go — rewritten; v2 format, BlobVersion, ErrSecretLen, stdlib HKDF
  • internal/webauthn/prf.go — new; PRFSalt, DecodePRFResult, ErrNoPRF
  • internal/webauthn/webauthn.go — both option builders request the prf extension
  • internal/ipc/{api,client,server}.go — wire field public_key → secret
  • cmd/mavend/main.go — store ownership in daemonLock, seal on shutdown, unlockMu, assertion required
  • cmd/mavweb/webauthn.go — PRF posted with the assertion; wrap+unlock keyed on it; page JS reads getClientExtensionResults().prf.results.first

No config keys added. -wrapped-key-file (or DefaultWrappedKeyPath) is unchanged.

How verified

make build and make test both exit 0. New tests:

  • internal/webauthn/keywrap_test.go — v2 round trip, non-determinism, wrong secret, every single-bit flip in the blob, truncation, v2→v1 downgrade attempt, legacy v1 read, non-32-byte / all-zero / COSE-sized secrets refused on both paths
  • internal/webauthn/prf_test.go — salt stability and non-aliasing, padded/unpadded decode, ErrNoPRF, unusable results refused, both option builders request PRF with the right salt
  • internal/ipc/unlock_test.go — wire carries secret and not public_key, secret reaches the hook byte-for-byte, refusals propagate, unwired = ErrUnknownMethod, locked-mode default-deny with only the two unlock methods allowed
  • cmd/mavweb/passkey_prf_test.go — software authenticator; the PRF secret (not the public key) is what goes over IPC, no PRF means no unlock attempt at all, a failed unlock does not fail the assertion, a forged assertion never reaches the key IPC, and the page still asks for and posts the PRF
  • cmd/mavend/coldstart_test.go — seal-on-shutdown after a cold start survives a reboot (this fails without the fix), closeStore safe when never unlocked and safe twice, nothing in the state dir contains the plaintext key, wrong key does not open the store

Not verified here, and cannot be: the PRF round trip against real hardware. There is no authenticator on this box. Left as QA steps on the task.

Vikunja #14

## What changed Cold-start unlock (passkey → L3 → encrypted data at rest) wrapped the database key under the credential **public** key. mavweb writes that key verbatim to `passkeys.json`, normally in the same state dir as `db_key.wrapped`, so anyone holding both files recovered the database key offline with no authenticator involved. The wrapped blob was a plaintext key with extra steps. The wrapping secret is now the **WebAuthn PRF extension** output — 32 bytes the authenticator computes over a fixed salt and never stores. Versioned blob: ``` v2: "MVNKW2\x00" || salt(16) || nonce(12) || AES-256-GCM(key) magic as AAD v1: salt(16) || nonce(12) || AES-256-GCM(key) read-only ``` v1 still opens (no bricked deployment) and reports itself, so `mavend` logs a `SECURITY:` line telling him to re-enroll. Nothing writes v1. The magic is GCM additional data, so a v2 blob cannot be header-stripped and re-read as v1. Four other defects on the same path: - **Silent total data loss after a cold start.** The locked-boot store is opened on an IPC goroutine inside `UnlockFn` and was never closed; `Close` is what re-encrypts the tmpfs working copy back over the ciphertext file. `daemonLock` now owns the store and seals it at shutdown. - **`MethodUnlock` was reachable by anything on the box.** The socket is same-uid and cannot authenticate its caller. Unlock now requires a passkey assertion mavweb verified cryptographically first. - **Concurrent unlocks** would each open a store and wire a full daemon, with the loser's goroutines running against a store nobody closes. Now serialized, and idempotent. - **Hand-rolled HKDF** keyed the expand step with the salt instead of the PRK — self-consistent but not RFC 5869. Replaced with stdlib `crypto/hkdf`. Key wrapping moved from enrolment to the first assertion: `create()` does not produce a PRF result on most authenticators, only a support flag. An authenticator without PRF now writes **no** wrapped file rather than one that looks protected and is not, and the passkey page says so out loud instead of failing open. Both configurations behave: with no wrapped key file the daemon boots normally and the key IPC answers `ErrUnknownMethod`; with one and no env key it boots locked, default-denies every method except `AssertStepUp`/`Unlock`, and comes up on assertion. ### Files - `internal/webauthn/keywrap.go` — rewritten; v2 format, `BlobVersion`, `ErrSecretLen`, stdlib HKDF - `internal/webauthn/prf.go` — new; `PRFSalt`, `DecodePRFResult`, `ErrNoPRF` - `internal/webauthn/webauthn.go` — both option builders request the `prf` extension - `internal/ipc/{api,client,server}.go` — wire field `public_key` → `secret` - `cmd/mavend/main.go` — store ownership in `daemonLock`, seal on shutdown, `unlockMu`, assertion required - `cmd/mavweb/webauthn.go` — PRF posted with the assertion; wrap+unlock keyed on it; page JS reads `getClientExtensionResults().prf.results.first` No config keys added. `-wrapped-key-file` (or `DefaultWrappedKeyPath`) is unchanged. ## How verified `make build` and `make test` both exit 0. New tests: - `internal/webauthn/keywrap_test.go` — v2 round trip, non-determinism, wrong secret, **every single-bit flip** in the blob, truncation, v2→v1 downgrade attempt, legacy v1 read, non-32-byte / all-zero / COSE-sized secrets refused on both paths - `internal/webauthn/prf_test.go` — salt stability and non-aliasing, padded/unpadded decode, `ErrNoPRF`, unusable results refused, both option builders request PRF with the right salt - `internal/ipc/unlock_test.go` — wire carries `secret` and not `public_key`, secret reaches the hook byte-for-byte, refusals propagate, unwired = `ErrUnknownMethod`, locked-mode default-deny with only the two unlock methods allowed - `cmd/mavweb/passkey_prf_test.go` — software authenticator; the PRF secret (not the public key) is what goes over IPC, no PRF means no unlock attempt at all, a failed unlock does not fail the assertion, a **forged assertion never reaches the key IPC**, and the page still asks for and posts the PRF - `cmd/mavend/coldstart_test.go` — seal-on-shutdown after a cold start survives a reboot (this fails without the fix), `closeStore` safe when never unlocked and safe twice, nothing in the state dir contains the plaintext key, wrong key does not open the store **Not verified here, and cannot be:** the PRF round trip against real hardware. There is no authenticator on this box. Left as QA steps on the task. Vikunja #14
claude added 1 commit 2026-08-01 03:49:53 +02:00
Derive the cold-start unlock key from the passkey PRF, not the public key (#14) 4eca20bd94 
Cold-start unlock wrapped the database key under the credential *public* key.
A public key is public: mavweb writes it verbatim to passkeys.json, normally in
the same state dir as db_key.wrapped, so anyone holding both files recovered the
database key offline with no authenticator involved. The wrapped blob was a
plaintext key with extra steps.

The secret is now the WebAuthn PRF extension output — 32 bytes the authenticator
computes over a fixed salt and never stores anywhere. The blob gains a version:

  v2:  "MVNKW2\x00" || salt || nonce || AES-256-GCM(key), magic as AAD
  v1:  salt || nonce || AES-256-GCM(key)                  (read-only)

v1 still opens so an existing deployment is not bricked, and reports itself so
the daemon can log a SECURITY line telling him to re-enroll. Nothing writes v1.
The magic is authenticated, so a v2 blob cannot be stripped and re-read as v1.

Four other defects on the same path:

  - The locked-boot store was opened on an IPC goroutine inside UnlockFn and
    never closed. Close is what re-encrypts the tmpfs working copy back over
    the ciphertext, so every write of a cold-started session was lost silently
    on the next boot. daemonLock now owns the store and seals it at shutdown.
  - MethodUnlock was reachable by anything on the box; the socket is same-uid
    and cannot authenticate its caller. It now requires a passkey assertion
    that mavweb verified first.
  - Concurrent unlocks would each open a store and wire a daemon. One at a
    time, and never a second one.
  - The hand-rolled HKDF keyed the expand step with the salt instead of the
    PRK. Replaced with crypto/hkdf.

Key wrapping moves from enrolment to the first assertion, because create() does
not produce a PRF result on most authenticators — only a support flag. An
authenticator without PRF now writes no wrapped file at all rather than one
that looks protected and is not, and the page says so.

Verified: make build, make test. New tests cover the v2 round trip, a wrong
secret, every single-bit tamper, truncation, the v1 downgrade attempt, legacy
v1 reads, non-32-byte and all-zero secrets, the ipc wire field, locked-mode
default-deny, a forged assertion never reaching the unlock path, seal-on-
shutdown after a cold start, and that nothing in the state dir contains the
plaintext key. The PRF round trip against real hardware is a QA step.

Vikunja #14
claude reviewed 2026-08-01 11:38:43 +02:00
claude left a comment
Author
Contributor
Copy Link
Copy Source

The core change is right and the reasoning in the keywrap.go header is the clearest statement of the old bug I have read. Three details earn their keep. secretLen = 32 as a structural refusal, so a 77-byte COSE public key cannot be passed here again by accident. The magic authenticated as GCM additional data, so a v2 blob cannot be stripped and re-read as v1. Replacing the hand-rolled HKDF that keyed expand with the salt instead of the PRK. The daemonLock.st handoff is a separate real bug caught in passing. A cold-started daemon never called store.Close. Every write of that session was lost on the next boot, silently.

Then the failure modes.

1. Any box enrolled before this PR can never cold-start again

The header says v1 blobs stay readable "so an existing deployment opens and can be re-wrapped". Trace who supplies the v1 secret. UnwrapKey reads a v1 blob with wantSecretLen = 0, so it accepts the credential public key. The only caller is AssertFinish, and it now sends webauthn.DecodePRFResult(body.PRF). The public key is never sent again. h.store.Lookup(credID) was deleted from that path.

So on a box with a v1 blob:

  1. Cold boot, locked mode. He asserts his passkey.
  2. mavweb decodes a valid 32-byte PRF result and calls Unlock with it.
  3. UnwrapKey sees no magic, takes the v1 branch, derives under wrapInfoV1 from the PRF secret rather than the public key, and GCM open fails.
  4. unwrap key: decrypt failed (wrong credential?). The daemon stays locked.

There is no second attempt with the public key. The v1 read path is dead code from its only caller, and the SECURITY: warning in UnlockFn is unreachable. The escape hatch is gone too. Re-wrapping needs WrapKeyFn, wired only if envKeyBytes != nil. A locked boot is by definition the mode with no env key. The recovery path is to put MAVEN_DB_KEY back in the environment, which is the thing cold-start unlock exists to avoid.

If the deployed box has a v1 blob, this PR bricks its cold start. It needs one of two fixes before merge. Retry the unwrap with the public key inside AssertFinish when the PRF attempt fails. Or wire WrapKeyFn in locked mode after a successful unlock, so the unlock that used the v1 key rewrites the blob as v2.

2. Every assertion rewrites the blob, so only the last authenticator can cold-start

StoreEncryptionKey(ctx, secret) runs on every successful assertion, unconditionally, and WrapKeyFn does os.WriteFile(wp, blob, 0o600). Two consequences.

Last credential wins. AssertionOptions sends allowCredentials: [], and h.store.Save keeps more than one credential. Enrol a phone and a hardware key. Assert with the phone: the blob is wrapped under the phone's PRF output. Assert with the hardware key next week: the blob is rewritten under a completely different secret. The phone can no longer open the database. Nothing warns, and the log line says encryption key wrapped for credential <id> either way. The backup authenticator he enrolled for exactly this situation is the one thing that stops working.

Non-atomic rewrite of the only thing that opens the database. os.WriteFile truncates in place. A power cut or an OOM kill between the truncate and the write leaves a zero-length or half-written blob. The previous contents are gone. This is now on the path of every routine step-up, not only enrolment. Write to a temp file in the same directory, fsync, then rename. And skip the write entirely when a valid v2 blob already opens under this same secret.

3. The wrapping secret is whatever the browser says it is

body.PRF is 32 bytes chosen by the client. WebAuthn client extension outputs are not covered by the assertion signature. Nothing binds the PRF value to the credential just verified. AssertFinish decodes it, checks length and non-zero, and hands it to StoreEncryptionKey.

A page-level compromise of /auth/webauthn then converts one legitimate touch into permanent offline recovery of the database key. The script substitutes 32 bytes it knows. The daemon re-wraps the at-rest key under them. The attacker needs only the blob file afterwards. No authenticator, no second gesture. The real passkey is locked out at the same moment, so the failure is loud, but by then the key is gone.

TestForgedAssertionNeverUnlocks covers the forged-signature case. It does not cover a valid assertion carrying a substituted prf. Gating the rewrite as in finding 2 closes most of this. Write the blob only when no working v2 blob exists. A substituted secret then gets one shot at enrolment, not one per assertion.

4. The IsStepUp guard is not the boundary its comment claims

UnlockFn says:

the unlock path requires a passkey assertion that mavweb verified cryptographically first. Without this, MethodUnlock is reachable by anything on the box.

auth.Requirement(ipc.MethodAssertStepUp) returns AuthRead. Any process that can open the same-uid socket calls assert_step_up, gets passkeySess flipped, and then calls unlock. MethodUnlock is still reachable by anything on the box. What stops a local attacker is the 32-byte PRF output they do not have. That was already true before the guard.

The guard is worth keeping as depth. Say what it does. It stops an accidental unlock attempt from an unrelated local caller.

Smaller notes

  • The PRF secret is stable for the lifetime of the credential and it crosses HTTP in a request body. Unlike a signature it does not expire. One capture in a proxy log, a devtools HAR, or a crash dump is permanent. Worth a line in the header saying the blob's security now depends on that body never being logged.
  • In env-key mode UnlockFn is nil, so every assertion logs webauthn: unlock via credential <id>: unknown method. In locked mode after the first unlock, UnlockFn returns nil early, so every later assertion logs daemon unlocked via credential <id> when nothing happened. Both lines say the wrong thing on the common path.
  • UnwrapKey's v2 branch checks the secret length through wantSecretLen but not the all-zero case that checkSecret rejects on the wrap side. DecodePRFResult covers it for the one caller today, which makes the asymmetry harmless and easy to lose later.
  • WrapKey is documented as taking "the 32-byte WebAuthn PRF output for the enrolled credential". Nothing in internal/webauthn or mavend can tell which credential a secret came from. That is finding 3 restated at the API boundary.
The core change is right and the reasoning in the `keywrap.go` header is the clearest statement of the old bug I have read. Three details earn their keep. `secretLen = 32` as a structural refusal, so a 77-byte COSE public key cannot be passed here again by accident. The magic authenticated as GCM additional data, so a v2 blob cannot be stripped and re-read as v1. Replacing the hand-rolled HKDF that keyed expand with the salt instead of the PRK. The `daemonLock.st` handoff is a separate real bug caught in passing. A cold-started daemon never called `store.Close`. Every write of that session was lost on the next boot, silently. Then the failure modes. ## 1. Any box enrolled before this PR can never cold-start again The header says v1 blobs stay readable "so an existing deployment opens and can be re-wrapped". Trace who supplies the v1 secret. `UnwrapKey` reads a v1 blob with `wantSecretLen = 0`, so it accepts the credential public key. The only caller is `AssertFinish`, and it now sends `webauthn.DecodePRFResult(body.PRF)`. The public key is never sent again. `h.store.Lookup(credID)` was deleted from that path. So on a box with a v1 blob: 1. Cold boot, locked mode. He asserts his passkey. 2. mavweb decodes a valid 32-byte PRF result and calls `Unlock` with it. 3. `UnwrapKey` sees no magic, takes the v1 branch, derives under `wrapInfoV1` from the PRF secret rather than the public key, and GCM open fails. 4. `unwrap key: decrypt failed (wrong credential?)`. The daemon stays locked. There is no second attempt with the public key. The v1 read path is dead code from its only caller, and the `SECURITY:` warning in `UnlockFn` is unreachable. The escape hatch is gone too. Re-wrapping needs `WrapKeyFn`, wired only `if envKeyBytes != nil`. A locked boot is by definition the mode with no env key. The recovery path is to put `MAVEN_DB_KEY` back in the environment, which is the thing cold-start unlock exists to avoid. If the deployed box has a v1 blob, this PR bricks its cold start. It needs one of two fixes before merge. Retry the unwrap with the public key inside `AssertFinish` when the PRF attempt fails. Or wire `WrapKeyFn` in locked mode after a successful unlock, so the unlock that used the v1 key rewrites the blob as v2. ## 2. Every assertion rewrites the blob, so only the last authenticator can cold-start `StoreEncryptionKey(ctx, secret)` runs on every successful assertion, unconditionally, and `WrapKeyFn` does `os.WriteFile(wp, blob, 0o600)`. Two consequences. **Last credential wins.** `AssertionOptions` sends `allowCredentials: []`, and `h.store.Save` keeps more than one credential. Enrol a phone and a hardware key. Assert with the phone: the blob is wrapped under the phone's PRF output. Assert with the hardware key next week: the blob is rewritten under a completely different secret. The phone can no longer open the database. Nothing warns, and the log line says `encryption key wrapped for credential <id>` either way. The backup authenticator he enrolled for exactly this situation is the one thing that stops working. **Non-atomic rewrite of the only thing that opens the database.** `os.WriteFile` truncates in place. A power cut or an OOM kill between the truncate and the write leaves a zero-length or half-written blob. The previous contents are gone. This is now on the path of every routine step-up, not only enrolment. Write to a temp file in the same directory, `fsync`, then `rename`. And skip the write entirely when a valid v2 blob already opens under this same secret. ## 3. The wrapping secret is whatever the browser says it is `body.PRF` is 32 bytes chosen by the client. WebAuthn client extension outputs are not covered by the assertion signature. Nothing binds the PRF value to the credential just verified. `AssertFinish` decodes it, checks length and non-zero, and hands it to `StoreEncryptionKey`. A page-level compromise of `/auth/webauthn` then converts one legitimate touch into permanent offline recovery of the database key. The script substitutes 32 bytes it knows. The daemon re-wraps the at-rest key under them. The attacker needs only the blob file afterwards. No authenticator, no second gesture. The real passkey is locked out at the same moment, so the failure is loud, but by then the key is gone. `TestForgedAssertionNeverUnlocks` covers the forged-signature case. It does not cover a valid assertion carrying a substituted `prf`. Gating the rewrite as in finding 2 closes most of this. Write the blob only when no working v2 blob exists. A substituted secret then gets one shot at enrolment, not one per assertion. ## 4. The `IsStepUp` guard is not the boundary its comment claims `UnlockFn` says: > the unlock path requires a passkey assertion that mavweb verified cryptographically first. Without this, MethodUnlock is reachable by anything on the box. `auth.Requirement(ipc.MethodAssertStepUp)` returns `AuthRead`. Any process that can open the same-uid socket calls `assert_step_up`, gets `passkeySess` flipped, and then calls `unlock`. `MethodUnlock` is still reachable by anything on the box. What stops a local attacker is the 32-byte PRF output they do not have. That was already true before the guard. The guard is worth keeping as depth. Say what it does. It stops an *accidental* unlock attempt from an unrelated local caller. ## Smaller notes - The PRF secret is stable for the lifetime of the credential and it crosses HTTP in a request body. Unlike a signature it does not expire. One capture in a proxy log, a devtools HAR, or a crash dump is permanent. Worth a line in the header saying the blob's security now depends on that body never being logged. - In env-key mode `UnlockFn` is nil, so every assertion logs `webauthn: unlock via credential <id>: unknown method`. In locked mode after the first unlock, `UnlockFn` returns nil early, so every later assertion logs `daemon unlocked via credential <id>` when nothing happened. Both lines say the wrong thing on the common path. - `UnwrapKey`'s v2 branch checks the secret length through `wantSecretLen` but not the all-zero case that `checkSecret` rejects on the wrap side. `DecodePRFResult` covers it for the one caller today, which makes the asymmetry harmless and easy to lose later. - `WrapKey` is documented as taking "the 32-byte WebAuthn PRF output for the enrolled credential". Nothing in `internal/webauthn` or mavend can tell which credential a secret came from. That is finding 3 restated at the API boundary.
kami referenced this issue from a commit 2026-08-01 14:43:55 +02:00
ipc: gofmt unlock_test.go
kami closed this pull request 2026-08-01 14:52:03 +02:00
kami commented 2026-08-01 14:52:56 +02:00
Owner
Copy Link
Copy Source

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
claude
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kami
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#77
Delete Branch "overnight/coldstart-unlock"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?