vaultmgr: recover a lost vault key mode - #6451
Conversation
54aedc2 to
e94c7c4
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6451 +/- ##
==========================================
+ Coverage 25.87% 26.21% +0.33%
==========================================
Files 510 521 +11
Lines 93575 95338 +1763
==========================================
+ Hits 24212 24990 +778
- Misses 67494 68295 +801
- Partials 1869 2053 +184 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@claude review |
|
Claude finished @rene's task in 4m 49s —— View job Review complete
I reviewed the recovery logic end to end (
One low-priority observation posted inline on the wipe-and-recreate path ( No blocking issues. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| getAndPublishAllVaultStatuses(ctx) | ||
| return | ||
| } | ||
| recordVaultKeyMode(ctx) |
There was a problem hiding this comment.
The mode persisted on this recreate path is the inferred one, not a confirmed one. For a device that lost VaultConfig while a vault existed, vaultKeyMode() inferred TpmKeyOnly=false (merged). RemoveDefaultVault()+SetupDefaultVault() above then creates a brand-new empty vault via createVault, which stages the key with h.options.TpmKeyOnlyMode — still the inferred false, never cleared/overridden on the create path. So the recreated vault adopts the legacy merged derivation and recordVaultKeyMode persists it as authoritative, even though a fresh vault on a first-boot device would be created TPM-key-only.
Functionally harmless (the vault opens and the mode is now persisted) and largely pre-existing, so low priority — but it means the wipe-and-recreate recovery leaves a modern device on the deprecated key scheme. Worth considering forcing TpmKeyOnlyMode=true (and clearing Inferred) before the recreate SetupDefaultVault(), since a newly created vault is never a legacy one.
There was a problem hiding this comment.
It makes sense to use TpmKeyOnlyMode=true for any recreate.
There was a problem hiding this comment.
Updated to use true when recreating.
e94c7c4 to
8cb672a
Compare
/persist can come back from a boot only partially intact: a reset mid-write leaves a filesystem that e2fsck then repairs, and that repair is free to clear directory entries, strand subtrees in lost+found, or truncate a file. Every consumer of /persist has to survive its own state being partly absent, and most of EVE does. /persist/status/vaultmgr/VaultConfig does not -- losing that one file makes the vault permanently unopenable, and nothing on the device can reconstruct it. It records which derivation the vault was created with: the TPM key alone, or that key merged with a build-time constant. Without the file vaultmgr infers the mode from whether /persist/vault exists, which for an existing vault always names the merged form, so every device installed since 7.10.0 derives a key its own vault does not accept. The failure does not look like what it is -- the TPM unseal succeeds and only fscrypt refuses, so it reads as a broken seal, the controller-key recovery re-seals and fails identically, and the device parks in maintenance mode with MaintenanceModeReasonVaultLockedUp. Nothing ever tries the other derivation, and persisting the guess on sight turned one reboot into a permanent state. An inferred mode is now marked as such and no longer persisted until something has confirmed it. When it fails to open the vault the handler retries once with the other derivation, which is safe because a key that does not match changes nothing on disk, and only the mode that actually worked is written to VaultConfig -- so the guess happens at most once per device, and a genuine seal failure still reports both derivations rather than being attributed to either. ZFS retries only the key load, since a mount failure is not a key problem. The wipe-and-recreate fallback, taken when the controller has no key and vault cleanup is still allowed, creates the replacement vault TPM-key-only rather than inheriting the mode of the vault it just removed. A vault created today is never a legacy merged-key one, so inheriting an inferred mode there would have persisted the deprecated derivation as authoritative. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Exercises on a real device what the unit tests can only decide against a fake unlock: a device whose /persist came back without the file recording which key derivation its vault was created with must still open that vault, keep its contents, and remember the derivation that worked. The fault is injected by removing the persisted vault key mode and rebooting. Since the recovery recreates that file, the test proves the fault landed both before the reboot and from the device's own logs afterwards; a marker written into the vault beforehand distinguishes a recovered vault from a recreated empty one, and the reported unlock method distinguishes the local seal from the controller-key fallback. A second reboot then has to read the recorded mode with no probe at all. Evidence comes from the device's own /persist/newlog rather than from the controller, which receives these early-boot records far too late to assert on, and the vault seal is settled first: a device seals before onboarding writes the per-device /config files that measure-config records into PCR14, so an unsettled device recovers over the controller key and the local-seal assertion cannot be made. Runs once per /persist filesystem, since ext4 and ZFS are separate vault handlers. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8cb672a to
6eb1e37
Compare
Which derivation vaultmgr reports for the vault key, and whether it flags that answer as a guess, is what decides whether the handler may probe the other derivation before giving up. Cover both branches: a mode read back from the persisted vault config is reported as-is and never flagged, and with no config to read whatever the filesystem suggests is always flagged as inferred. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@eriknordmark , you have one conflict to fix.... |
2 participants
Description
/persistis mutable, and it can come back from a boot only partially intact.A power loss or a reset mid-write leaves a filesystem that
e2fsckthenrepairs, and that repair is free to clear directory entries, move whole
subtrees into
lost+found, or truncate a file to something that is no longerwhat it was. So every consumer of
/persisthas to survive its own statebeing partly or wholly absent on the next boot. Most of EVE does: pubsub state
gets rebuilt, caches get refetched,
onboot.shdeliberately discards several/persisttrees outright./persist/status/vaultmgr/VaultConfigdoes not. Losing that one file makes thevault permanently unopenable, and nothing on the device can reconstruct it.
It records which key derivation the vault was created with — the TPM key alone,
or that key merged with a build-time constant (the pre-7.10.0 form). Without
the file
vaultmgrhas to infer the mode, and its only signal is whether/persist/vaultexists. For an existing vault that always answers "merged", soevery device installed since 7.10.0 derives a key its own vault does not
accept. The failure then does not look like what it is:
fscryptrefuses, so it reads as a brokenseal (
Vault key unavailable,mismatchingPCRs=[]);MaintenanceModeReasonVaultLockedUp;nothing new to go on, so it stays there.
Worse, the guess was persisted on sight — so one reboot turned a recoverable
state into a permanent one.
This PR makes the mode survive a partial
/persistloss instead:HandlerOptions.TpmKeyOnlyModeInferred) and isno longer persisted until something has confirmed it.
the other derivation (
resolveKeyMode,pkg/pillar/vault/keymode.go). Thisis safe: a key that does not match changes nothing on disk, and
stageKeyshreds it. The retry runs only for an inferred mode, so a genuine seal
failure is not masked, and when neither derivation works the error names both
rather than blaming either.
VaultConfig, sothe guess happens at most once per device.
instead of inheriting the removed vault's (possibly inferred) mode. A vault
created today is never a legacy merged-key one, so inheriting an inferred
mode there would have persisted the deprecated derivation as authoritative.
ZFS retries just the key load, not the mount — a mount failure is not a key
problem.
Note on blast radius, since it is easy to over-read: the cost is a locked
vault, not a wipe. The
RemoveDefaultVault()+ recreate path needs anempty
EncryptedVaultKeyFromControllerand the/persist/status/allow-vault-cleansentinel, whichDisallowVaultCleanup()removes at the first content tree/volume or successful attest escrow — so it is
unreachable for any device with data to lose, exactly as designed.
Why the old key scheme has to stay
Collapsing the two derivations — assuming
TpmKeyOnly=trueand deleting thisclass of problem rather than making it recoverable — is not available. A small
but non-zero percentage of deployed production devices still unlock a vault
created with the merged key, which shows up in their logs as
Calling mergeKeys. The mode is sticky, since an existingfscryptprotector or ZFS keycannot be re-derived from a different key, so dropping support for the old
scheme would leave exactly those devices unable to open their vault. Both
derivations therefore stay supported, and the recovery in this PR is what makes
a lost mode survivable in the meantime.
How to test and validate this PR
Automated:
pkg/pillar/vault/keymode_test.gocovers the decision logic — aknown mode is never second-guessed, an inferred mode that works is not
second-guessed either, an inferred-wrong mode is recovered and the working mode
is the one reported, and a double failure reports both errors. Negative
control, now exercised: with the retry removed,
TestResolveKeyModeRecoversFromAWrongInferenceandTestResolveKeyModeReportsBothFailuresfail on their assertions while thethree retry-insensitive cases still pass.
pkg/pillar/cmd/vaultmgr/keymode_test.gocovers the branch feeding it: a moderead back from the persisted vault config is reported as-is and never flagged
inferred, and with no config to read whatever the filesystem suggests is always
flagged. Negative controls, both exercised: with the persisted-config branch
never taken the mode is reported wrong and flagged, and with it always taken a
guess is reported as authoritative — each failing exactly the one test that
should catch it.
On a device — done on amd64.
evetest/tests/security/vault_keymode_test.go(TestVaultKeyModeRecovery, onevariant per
/persistfilesystem) injects the fault and checks the recovery endto end: it removes the persisted mode, reboots through the controller, and
requires the device's own
/persist/newlogto show the mode being inferredwrong and the other derivation opening the vault. A marker written into the
vault beforehand separates a recovered vault from a recreated empty one,
VaultStatus.UnlockMethodhas to be the local TPM seal rather than thecontroller-key fallback, and a second reboot has to read the recorded mode
without probing.
The test settles the vault seal before injecting anything. A device seals on its
first boot, before onboarding writes the per-device
/configfiles thatmeasure-config records into PCR14, so boot 2 always unlocks over the controller
key with
mismatchingPCRs=[14]; injecting the fault there would measure thatre-seal rather than this change. Those settle boots double as the regression
check on the ordinary path — the mode is present, and nothing probes.
Observed on
kvm-amd64, ext4 and ZFS, both passing:ZFS is the same sequence through the other handler, where the wrong derivation
fails as
exit status 255fromzfs load-keyinstead ofexit status 1fromfscrypt. The next boot reads
Vault config inited with tpmkeyonly trueandprobes nothing.
Negative control on the device: with the retry removed from
resolveKeyModethesame test fails on its "vault must be unlocked" assertion, and the device
reproduces the original symptom exactly —
local TPM unseal FAILED; mismatching PCRs=[], vaultlocked,Vault key unavailable.Changelog notes
A device that lost its stored vault key-derivation mode — for example after a
filesystem repair of a partially corrupted
/persist— could no longer unlockits vault and stayed in maintenance mode. It now recovers on its own and
remembers the correct mode.
PR Backports
Not proposed for backport.
Checklist
And the last but not least:
check them.
Tested on amd64 (kvm, ext4 and ZFS) under evetest; not tested on arm64. No
labels set; in particular no
stablelabel, since this is not beingbackported.