vaultmgr: recover a wrong vault key mode

evetest/tests/security/testsuite_test.go

@@ -21,6 +21,9 @@ import (
 //     signing API responses; device must recover config processing on its own.
 //   - TestControllerEncryptCertChange -- rotation of the controller ECDH
 //     certificate; object-encrypted configuration must be migrated and survive.
+//   - TestVaultKeyModeRecovery -- the vault still opens when the file recording
+//     which key derivation it was created with names the wrong one; run once per
+//     /persist filesystem, since ext4 and ZFS are separate vault handlers.
 func TestSecuritySuite(test *testing.T) {
 	evetest.Init(test)
 	defer evetest.Close()
@@ -45,5 +48,31 @@ func TestSecuritySuite(test *testing.T) {
 		evetest.TestCase{
 			Test: TestControllerEncryptCertChange,
 		},
+		// Last: the filesystem is part of the device requirements, so neither
+		// variant can share the device the tests above reuse, and their
+		// placement relative to those does not affect that reuse.
+		evetest.TestCase{
+			Test: TestVaultKeyModeRecovery,
+			Variants: []evetest.TestVariant{
+				{
+					Name: "TestVaultKeyModeRecoveryOnExt4",
+					Parameters: []evetest.TestParameterValue{
+						{
+							Key:   evetest.FilesystemParameterKey,
+							Value: evetest.FilesystemEXT4,
+						},
+					},
+				},
+				{
+					Name: "TestVaultKeyModeRecoveryOnZFS",
+					Parameters: []evetest.TestParameterValue{
+						{
+							Key:   evetest.FilesystemParameterKey,
+							Value: evetest.FilesystemZFS,
+						},
+					},
+				},
+			},
+		},
 	)
 }

pkg/pillar/cmd/vaultmgr/keymode_test.go

@@ -0,0 +1,149 @@
+// Copyright (c) 2026 Zededa, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package vaultmgr
+
+import (
+	"testing"
+
+	"github.com/lf-edge/eve/pkg/pillar/base"
+	"github.com/lf-edge/eve/pkg/pillar/pubsub"
+	"github.com/lf-edge/eve/pkg/pillar/types"
+	"github.com/lf-edge/eve/pkg/pillar/vault"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+)
+
+// initTest points the package logger at the test logger and restores the
+// package-level vault config the functions under test read, so the order the
+// tests run in cannot matter.
+func initTest(t *testing.T) {
+	log = base.NewSourceLogObject(logrus.StandardLogger(), "test", 0)
+	config, inited := vaultConfig, vaultConfigInited
+	t.Cleanup(func() {
+		vaultConfig, vaultConfigInited = config, inited
+	})
+}
+
+// A recorded mode is where unlocking starts. It is not treated as a fact --
+// the handler falls back to the other derivation -- but it is what gets tried
+// first, so a device that has one never pays for a wrong first guess.
+func TestVaultKeyModeUsesThePersistedConfig(t *testing.T) {
+	initTest(t)
+	for _, tpmKeyOnly := range []bool{true, false} {
+		vaultConfig = types.VaultConfig{TpmKeyOnly: tpmKeyOnly}
+		vaultConfigInited = true
+		assert.Equal(t, tpmKeyOnly, vaultKeyMode())
+	}
+}
+
+// With no recorded mode, the derivation a vault created today is keyed with is
+// the one to try first. It is wrong for a pre-7.10.0 merged-key vault whose
+// config was lost, which is exactly what the unlock fallback covers.
+func TestVaultKeyModeWithoutAPersistedConfig(t *testing.T) {
+	initTest(t)
+	// A value the persisted-config branch would return, so dropping the branch
+	// check below shows up as the wrong answer rather than as a coincidence.
+	vaultConfig = types.VaultConfig{TpmKeyOnly: false}
+	vaultConfigInited = false
+	assert.True(t, vaultKeyMode())
+}
+
+func TestKeyDerivationOf(t *testing.T) {
+	assert.Equal(t, types.VaultKeyDerivationTPMOnly, keyDerivationOf(true))
+	assert.Equal(t, types.VaultKeyDerivationTPMAndConstant, keyDerivationOf(false))
+}
+
+// fakeHandler reports a resolved key mode and nothing else; recordVaultKeyMode
+// reaches no other part of the interface.
+type fakeHandler struct {
+	vault.Handler
+	options vault.HandlerOptions
+}
+
+func (h fakeHandler) GetHandlerOptions() vault.HandlerOptions {
+	return h.options
+}
+
+func setHandler(t *testing.T, h vault.Handler) {
+	previous := handler
+	handler = h
+	t.Cleanup(func() { handler = previous })
+}
+
+// newKeyModeCtx returns a context whose VaultConfig publication is in memory,
+// so what recordVaultKeyMode writes can be read back.
+func newKeyModeCtx(t *testing.T, tpmEnabled bool) *vaultMgrContext {
+	t.Helper()
+	ps := pubsub.New(pubsub.NewMemoryDriver(), logrus.StandardLogger(), log)
+	pub, err := ps.NewPublication(pubsub.PublicationOptions{
+		AgentName: agentName,
+		TopicType: types.VaultConfig{},
+	})
+	if err != nil {
+		t.Fatalf("NewPublication: %v", err)
+	}
+	return &vaultMgrContext{pubVaultConfig: pub, tpmEnabled: tpmEnabled}
+}
+
+func recordedKeyMode(t *testing.T, ctx *vaultMgrContext) (types.VaultConfig, bool) {
+	t.Helper()
+	item, err := ctx.pubVaultConfig.Get(types.VaultConfig{}.Key())
+	if err != nil {
+		return types.VaultConfig{}, false
+	}
+	config, ok := item.(types.VaultConfig)
+	assert.True(t, ok, "VaultConfig publication holds %T", item)
+	return config, true
+}
+
+// What gets recorded is the derivation that opened the vault, read off the
+// handler. Recording the first guess instead is what made a lost mode
+// permanent.
+func TestRecordVaultKeyModeRecordsTheResolvedMode(t *testing.T) {
+	initTest(t)
+	setHandler(t, fakeHandler{options: vault.HandlerOptions{TpmKeyOnlyMode: true}})
+	vaultConfigInited = false
+	ctx := newKeyModeCtx(t, true)
+
+	recordVaultKeyMode(ctx)
+
+	config, recorded := recordedKeyMode(t, ctx)
+	assert.True(t, recorded)
+	assert.True(t, config.TpmKeyOnly)
+	assert.True(t, vaultConfigInited)
+}
+
+// A recorded mode the vault no longer uses has to be overwritten. The vault
+// recreate path is where the two come apart: it destroys the old vault and
+// keys the replacement TPM-key-only, leaving whatever was recorded describing
+// a vault that is gone. A boot that then trusted the record would start from
+// the wrong derivation.
+func TestRecordVaultKeyModeOverwritesAStaleMode(t *testing.T) {
+	initTest(t)
+	setHandler(t, fakeHandler{options: vault.HandlerOptions{TpmKeyOnlyMode: false}})
+	vaultConfigInited = false
+	ctx := newKeyModeCtx(t, true)
+	recordVaultKeyMode(ctx)
+
+	setHandler(t, fakeHandler{options: vault.HandlerOptions{TpmKeyOnlyMode: true}})
+	recordVaultKeyMode(ctx)
+
+	config, _ := recordedKeyMode(t, ctx)
+	assert.True(t, config.TpmKeyOnly)
+}
+
+// Without a TPM the vault key is not derived from one, so there is no mode to
+// record -- and none may be left behind for a later boot to read as its own.
+func TestRecordVaultKeyModeSkipsWithoutTpm(t *testing.T) {
+	initTest(t)
+	setHandler(t, fakeHandler{options: vault.HandlerOptions{TpmKeyOnlyMode: true}})
+	vaultConfigInited = false
+	ctx := newKeyModeCtx(t, false)
+
+	recordVaultKeyMode(ctx)
+
+	_, recorded := recordedKeyMode(t, ctx)
+	assert.False(t, recorded)
+	assert.False(t, vaultConfigInited)
+}

pkg/pillar/cmd/vaultmgr/vaultmgr.go

@@ -27,7 +27,6 @@ import (
 	"crypto/sha256"
 	"flag"
 	"fmt"
-	"os"
 	"sync"
 	"time"
 
@@ -42,7 +41,7 @@ import (
 	"github.com/lf-edge/eve/pkg/pillar/utils/persist"
 	"github.com/lf-edge/eve/pkg/pillar/utils/wait"
 	"github.com/lf-edge/eve/pkg/pillar/vault"
-	"github.com/lf-edge/eve/pkg/pillar/zfs"
+	"github.com/lf-edge/eve/pkg/pillar/zboot"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/proto"
 )
@@ -55,8 +54,11 @@ type vaultMgrContext struct {
 	subGlobalConfig           pubsub.Subscription
 	subVaultKeyFromController pubsub.Subscription
 	GCInitialized             bool // GlobalConfig initialized
-	defaultVaultUnlocked      bool
-	vaultUCDone               bool
+	// Whether this device derives the vault key from a TPM at all; there is no
+	// key mode to record or report without one.
+	tpmEnabled           bool
+	defaultVaultUnlocked bool
+	vaultUCDone          bool
 	// How the default vault was unlocked this boot, surfaced on VaultStatus.
 	unlockMethod types.VaultUnlockMethod
 	// How the default vault's key is derived, surfaced on VaultStatus. Stays
@@ -191,39 +193,62 @@ func initializeSelfPublishHandles(ps *pubsub.PubSub, ctx *vaultMgrContext) {
 	ctx.pubVaultConfig = pubVaultConfig
 }
 
-// checkAndPublishVaultConfig: If vault config is not yet initialized
-// Checks if defaultVault/defaultSecretDataset exists and if not publishes the vault config TmpKeyOnly = true
-// If those directories exists, then publishes the vault config TmpKeyOnly = false
-// Function returns the TmpKeyOnly value, plus whether the persist filesystem is
-// one that sets up a vault at all. On any other filesystem no vault key is
-// derived, so the TmpKeyOnly value describes nothing.
-func checkAndPublishVaultConfig(ctx *vaultMgrContext) (bool, bool) {
+// vaultKeyMode reports the TpmKeyOnly value to try deriving the vault key
+// with: the recorded mode when there is one, otherwise TPM-key-only, which is
+// how a vault created today is keyed.
+//
+// Neither answer is treated as a fact. A merged-key vault whose recorded mode
+// is gone gets the wrong value here, and so does one whose mode was recorded
+// by a boot that guessed. Unlocking falls back to the other derivation either
+// way, and the caller persists whichever opened the vault.
+func vaultKeyMode() bool {
+	if vaultConfigInited {
+		return vaultConfig.TpmKeyOnly
+	}
+	log.Notice("No persisted vault config; trying tpmKeyOnly first")
+	return true
+}
+
+// vaultSupported reports whether the persist filesystem is one that sets up a
+// vault at all. On any other filesystem no vault key is derived, so a key
+// derivation reported for it would describe nothing.
+func vaultSupported() bool {
 	persistFsType := persist.ReadPersistType()
-	vaultSupported := persistFsType == types.PersistExt4 ||
-		persistFsType == types.PersistZFS
-
-	// We do not have vault config, publish it
-	if vaultConfigInited == false {
-		tpmKeyOnly := false
-
-		switch persistFsType {
-		case types.PersistExt4:
-			_, err := os.Stat(types.SealedDirName)
-			if os.IsNotExist(err) {
-				tpmKeyOnly = true
-			}
-		case types.PersistZFS:
-			if _, err := zfs.GetDatasetKeyStatus(types.SealedDataset); err != nil {
-				tpmKeyOnly = true
-			}
-		default:
-			log.Noticef("unsupported %s filesystem, ignoring vault config setup",
-				persistFsType)
-		}
-		publishVaultConfig(ctx, tpmKeyOnly)
-		return tpmKeyOnly, vaultSupported
+	switch persistFsType {
+	case types.PersistExt4, types.PersistZFS:
+		return true
+	}
+	log.Noticef("unsupported %s filesystem, ignoring vault config setup",
+		persistFsType)
+	return false
+}
+
+// keyDerivationOf names a TpmKeyOnly value for VaultStatus.
+func keyDerivationOf(tpmKeyOnly bool) types.VaultKeyDerivation {
+	if tpmKeyOnly {
+		return types.VaultKeyDerivationTPMOnly
+	}
+	return types.VaultKeyDerivationTPMAndConstant
+}
+
+// recordVaultKeyMode persists the key-derivation mode the vault is actually
+// keyed with, so the next boot starts from it, and reports it on VaultStatus.
+// Called after an unlock or a create has succeeded. Persisting is skipped
+// while what is recorded already agrees -- what is left covers both a mode no
+// boot has recorded yet and one a recreate has since invalidated.
+func recordVaultKeyMode(ctx *vaultMgrContext) {
+	if !ctx.tpmEnabled {
+		return
+	}
+	tpmKeyOnly := handler.GetHandlerOptions().TpmKeyOnlyMode
+	if vaultSupported() {
+		ctx.keyDerivation = keyDerivationOf(tpmKeyOnly)
+	}
+	if vaultConfigInited && vaultConfig.TpmKeyOnly == tpmKeyOnly {
+		return
 	}
-	return vaultConfig.TpmKeyOnly, vaultSupported
+	publishVaultConfig(ctx, tpmKeyOnly)
+	vaultConfigInited = true
 }
 
 // runVaultOp runs a vault setup, unlock or removal on a separate goroutine and
@@ -351,18 +376,22 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar
 	// initialize publishing handles
 	initializeSelfPublishHandles(ps, &ctx)
 	tpmEnabled := etpm.IsTpmEnabled()
+	ctx.tpmEnabled = tpmEnabled
+	options := vault.HandlerOptions{
+		// Reading it once at startup is enough: a partition that is already
+		// committed cannot become uncommitted, and one committed later is
+		// picked up on the next boot, which is when leftovers matter again.
+		CurrentPartitionCommitted: zboot.IsCurrentPartitionStateActive(),
+	}
 	if tpmEnabled {
-		// TPM is enabled. Check if defaultVault directory exists, if not set vaultconfig
-		tpmKeyOnlyMode, vaultSupported := checkAndPublishVaultConfig(&ctx)
-		handler.SetHandlerOptions(vault.HandlerOptions{TpmKeyOnlyMode: tpmKeyOnlyMode})
-		if vaultSupported {
-			if tpmKeyOnlyMode {
-				ctx.keyDerivation = types.VaultKeyDerivationTPMOnly
-			} else {
-				ctx.keyDerivation = types.VaultKeyDerivationTPMAndConstant
-			}
+		options.TpmKeyOnlyMode = vaultKeyMode()
+		// The derivation reported until an unlock resolves it. A vault that
+		// never opens keeps this value, which names what was tried.
+		if vaultSupported() {
+			ctx.keyDerivation = keyDerivationOf(options.TpmKeyOnlyMode)
 		}
 	}
+	handler.SetHandlerOptions(options)
 
 	if tpmEnabled {
 		log.Noticef("about to setup the vault and fetch the disk key from TPM")
@@ -389,6 +418,7 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar
 		getAndPublishAllVaultStatuses(&ctx)
 	} else {
 		log.Noticef("vault is setup and unlocked successfully")
+		recordVaultKeyMode(&ctx)
 		ctx.defaultVaultUnlocked = true
 		if tpmEnabled {
 			ctx.unlockMethod = types.VaultUnlockTPMLocalSealed
@@ -585,6 +615,8 @@ func handleVaultKeyFromControllerImpl(ctxArg interface{}, key string,
 			return
 		}
 
+		recordVaultKeyMode(ctx)
+
 		// The local unseal had failed (that is why we are here); record that the
 		// unlock came from the controller key, so the distinction is visible on
 		// VaultStatus rather than only inferable from the log sequence.
@@ -615,11 +647,19 @@ func handleVaultKeyFromControllerImpl(ctxArg interface{}, key string,
 			return
 		}
 		log.Warnln("default vault removed")
+		// The replacement vault is brand new, so it is never one of the
+		// pre-7.10.0 merged-key vaults. Create it TPM-key-only rather than
+		// with the mode carried over from the vault just removed, which may
+		// itself have been wrong -- that is why this path was reached.
+		options := handler.GetHandlerOptions()
+		options.TpmKeyOnlyMode = true
+		handler.SetHandlerOptions(options)
 		if err := runVaultOp(ctx.ps, handler.SetupDefaultVault); err != nil {
 			log.Errorf("SetupDefaultVault failed, err: %v", err)
 			getAndPublishAllVaultStatuses(ctx)
 			return
 		}
+		recordVaultKeyMode(ctx)
 		ctx.defaultVaultUnlocked = true
 		ctx.unlockMethod = types.VaultUnlockRecreated
 		log.Noticef("%s re-created", types.DefaultVaultName)