overlord/devicestate, secboot: install setup encryption check must use cached context (canonical#16577)

ernestl · web-flow · commit 6fd1bba7660a · 2026-02-11T09:24:46.000+02:00
* overlord/devicestate, secboot: install setup encryption check must use cache from context

* fixup! overlord/devicestate, secboot: install setup encryption check must use cache from context

* fixup! overlord/devicestate, secboot: install setup encryption check must use cache from context
diff --git a/overlord/devicestate/devicestate_install_api_test.go b/overlord/devicestate/devicestate_install_api_test.go
@@ -864,7 +864,7 @@ func (s *deviceMgrInstallAPISuite) testInstallSetupStorageEncryption(c *C, isSup
 	gadgetSnapPath, kernelSnapPath, _, ginfo, mountCmd, _ := s.mockSystemSeedWithLabel(
 		c, label, seedCopyFn, seedOpts)
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, hasTPM)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, hasTPM, label)
 
 	// Mock encryption of partitions
 	encrytpPartCalls := 0
@@ -934,7 +934,7 @@ func (s *deviceMgrInstallAPISuite) testInstallSetupStorageEncryption(c *C, isSup
 
 	// ensure the expected encryption availability check was used
 	if isSupportedHybrid {
-		c.Assert(callCnt, DeepEquals, &callCounter{checkCnt: 1, checkActionCnt: 0, sealingSupportedCnt: 0})
+		c.Assert(callCnt, DeepEquals, &callCounter{checkCnt: 0, checkActionCnt: 1, sealingSupportedCnt: 0})
 	} else {
 		c.Assert(callCnt, DeepEquals, &callCounter{checkCnt: 0, checkActionCnt: 0, sealingSupportedCnt: 1})
 	}
@@ -1170,7 +1170,7 @@ func (s *deviceMgrInstallAPISuite) testInstallSetupStorageEncryptionPassphraseAu
 	s.state.Lock()
 	defer s.state.Unlock()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, true, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, true, true, "")
 
 	restore := devicestate.MockInstallEncryptPartitions(func(
 		onVolumes map[string]*gadget.Volume,
diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go
@@ -360,7 +360,7 @@ func (s *deviceMgrInstallModeSuite) SetUpTest(c *C) {
 	})
 	s.AddCleanup(restore)
 
-	mockHelperForEncryptionAvailabilityCheck(s, c, false, false)
+	mockHelperForEncryptionAvailabilityCheck(s, c, false, false, "")
 
 	s.state.Lock()
 	defer s.state.Unlock()
@@ -648,7 +648,7 @@ func (s *deviceMgrInstallModeSuite) doRunChangeTestWithEncryption(c *C, grade st
 	})
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.tpm)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.tpm, "")
 
 	if tc.trustedBootloader {
 		tab := bootloadertest.Mock("trusted", bootloaderRootdir).WithTrustedAssets()
@@ -1781,7 +1781,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallBootloaderVarSetFails(c *C) {
 	})
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false, "")
 
 	err := os.WriteFile(filepath.Join(dirs.GlobalRootDir, "/var/lib/snapd/modeenv"),
 		[]byte("mode=install\nrecovery_system=1234"), 0644)
@@ -1823,7 +1823,7 @@ func (s *deviceMgrInstallModeSuite) testInstallEncryptionValidityChecks(c *C, er
 	restore := release.MockOnClassic(false)
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true, "")
 
 	err := os.WriteFile(filepath.Join(dirs.GlobalRootDir, "/var/lib/snapd/modeenv"),
 		[]byte("mode=install\n"), 0644)
@@ -2010,7 +2010,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallWithEncryptionValidatesGadgetErr(
 	defer restore()
 
 	// pretend we have a TPM
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true, "")
 
 	// must be a model that requires encryption to error
 	s.testInstallGadgetNoSave(c, "secured")
@@ -2041,7 +2041,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallWithEncryptionValidatesGadgetWarn
 	defer restore()
 
 	// pretend we have a TPM
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true, "")
 
 	s.testInstallGadgetNoSave(c, "dangerous")
 
@@ -2067,7 +2067,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallWithoutEncryptionValidatesGadgetW
 	defer restore()
 
 	// pretend we have a TPM
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true, "")
 
 	s.testInstallGadgetNoSave(c, "dangerous")
 
@@ -2146,7 +2146,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncrypted(c *C) {
 			makeInstalledMockKernelSnap(c, st, kernelYamlNoFdeSetup)
 		}
 
-		callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.hasTPM)
+		callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.hasTPM, "")
 
 		encryptionType, err := devicestate.DeviceManagerCheckEncryption(s.mgr, st, deviceCtx, secboot.TPMProvisionFull)
 		c.Assert(callCnt.checkCnt, Equals, 0)
@@ -2314,7 +2314,7 @@ func (s *deviceMgrInstallModeSuite) doRunFactoryResetChange(c *C, model *asserts
 	})
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.tpm)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, tc.tpm, "")
 
 	if tc.trustedBootloader {
 		tab := bootloadertest.Mock("trusted", bootloaderRootdir).WithTrustedAssets()
@@ -3135,7 +3135,7 @@ func (s *deviceMgrInstallModeSuite) TestFactoryResetExpectedTasks(c *C) {
 	restore := release.MockOnClassic(false)
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false, "")
 
 	restore = devicestate.MockInstallFactoryReset(func(mod gadget.Model, gadgetRoot string, kernelSnapInfo *install.KernelSnapInfo, device string, options install.Options, obs gadget.ContentObserver, pertTimings timings.Measurer) (*install.InstalledSystemSideData, error) {
 		c.Assert(os.MkdirAll(dirs.SnapDeviceDirUnder(filepath.Join(dirs.GlobalRootDir, "/run/mnt/ubuntu-data/system-data")), 0755), IsNil)
@@ -3207,7 +3207,7 @@ func (s *deviceMgrInstallModeSuite) TestFactoryResetInstallDeviceHook(c *C) {
 	restore := release.MockOnClassic(false)
 	defer restore()
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, false, "")
 
 	hooksCalled := []*hookstate.Context{}
 	restore = hookstate.MockRunHook(func(ctx *hookstate.Context, tomb *tomb.Tomb) ([]byte, error) {
diff --git a/overlord/devicestate/devicestate_systems_test.go b/overlord/devicestate/devicestate_systems_test.go
@@ -2983,6 +2983,7 @@ var preinstallAction = &secboot.PreinstallAction{
 
 type suiteWithAddCleanup interface {
 	AddCleanup(func())
+	DeviceManager() *devicestate.DeviceManager
 }
 
 type callCounter struct {
@@ -2998,7 +2999,8 @@ type callCounter struct {
 //
 // isSupportedUbuntuHybrid: modify system release information and place current boot images to simulate supported Ubuntu hybrid install
 // hasTPM: indicates if we should simulate having a TPM (no error detected) or no TPM (some representative error)
-func mockHelperForEncryptionAvailabilityCheck(s suiteWithAddCleanup, c *C, isSupportedUbuntuHybrid, hasTPM bool) *callCounter {
+// cacheLabel: system label to use to cache check context where "" means do not cache check context
+func mockHelperForEncryptionAvailabilityCheck(s suiteWithAddCleanup, c *C, isSupportedUbuntuHybrid, hasTPM bool, cacheLabel string) *callCounter {
 	callCnt := &callCounter{}
 
 	releaseInfo := &release.OS{
@@ -3035,6 +3037,15 @@ func mockHelperForEncryptionAvailabilityCheck(s suiteWithAddCleanup, c *C, isSup
 		}
 	}
 
+	if isSupportedUbuntuHybrid && cacheLabel != "" {
+		// populate the cache with encryption support information that
+		// includes a check context similar to having performed an
+		// initial preinstall check
+		encInfo := &install.EncryptionSupportInfo{}
+		encInfo.SetAvailabilityCheckContext(&secboot.PreinstallCheckContext{})
+		s.DeviceManager().SetEncryptionSupportInfoInCacheUnlocked(cacheLabel, encInfo)
+	}
+
 	restore := install.MockSecbootPreinstallCheck(func(ctx context.Context, bootImagePaths []string) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) {
 		callCnt.checkCnt++
 		c.Assert(bootImagePaths, HasLen, 3)
@@ -3051,7 +3062,11 @@ func mockHelperForEncryptionAvailabilityCheck(s suiteWithAddCleanup, c *C, isSup
 		callCnt.checkActionCnt++
 		c.Assert(pcc, NotNil)
 		c.Assert(ctx, NotNil)
-		c.Assert(action, DeepEquals, preinstallAction)
+		if cacheLabel != "" {
+			c.Assert(action, DeepEquals, &secboot.PreinstallAction{Action: secboot.ActionNone})
+		} else {
+			c.Assert(action, DeepEquals, preinstallAction)
+		}
 		c.Assert(isSupportedUbuntuHybrid, Equals, true)
 
 		if hasTPM {
@@ -3160,7 +3175,7 @@ func (s *modelAndGadgetInfoSuite) TestSystemAndGadgetAndEncryptionInfoNotSupport
 		UnavailableWarning: "not encrypting device storage as checking TPM gave: cannot connect to TPM device",
 	}
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, false)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, false, "")
 
 	// basic availability check - fill empty info cache
 	encInfoFromCache := false
@@ -3211,7 +3226,7 @@ func (s *modelAndGadgetInfoSuite) TestSystemAndGadgetAndEncryptionInfoSupportedH
 	}
 	expectedEncInfo.SetAvailabilityCheckContext(preinstallCheckContext)
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, false)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, isSupportedHybrid, false, "")
 
 	// comprehensive preinstall check - fill empty info cache
 	encInfoFromCache := false
@@ -3298,7 +3313,7 @@ func (s *modelAndGadgetInfoSuite) testSystemAndGadgetAndEncryptionInfoPassphrase
 		PassphraseAuthAvailable: hasPassphraseSupport,
 	}
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, false, true, "")
 
 	// refresh empty cache
 	encInfoFromCache := false
@@ -3431,7 +3446,7 @@ func (s *modelAndGadgetInfoSuite) TestSystemAndGadgetInfoBadClassicGadget(c *C)
 	isClassic := true
 	s.makeMockUC20SeedWithGadgetYaml(c, "some-label", mockGadgetUCYamlNoBootRole, isClassic, nil)
 
-	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, true, true)
+	callCnt := mockHelperForEncryptionAvailabilityCheck(s, c, true, true, "")
 
 	_, _, _, err := s.mgr.SystemAndGadgetAndEncryptionInfo("some-label", false)
 	c.Assert(callCnt, DeepEquals, &callCounter{checkCnt: 1, checkActionCnt: 0, sealingSupportedCnt: 0})
diff --git a/overlord/devicestate/devicestate_test.go b/overlord/devicestate/devicestate_test.go
@@ -510,6 +510,10 @@ func (s *deviceMgrBaseSuite) addKeyToManagerInState(c *C) {
 	c.Assert(err, IsNil)
 }
 
+func (s *deviceMgrBaseSuite) DeviceManager() *devicestate.DeviceManager {
+	return s.mgr
+}
+
 func (s *deviceMgrSuite) SetUpTest(c *C) {
 	classic := false
 	s.setupBaseTest(c, classic)
diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go
@@ -1370,12 +1370,22 @@ func (m *DeviceManager) doInstallSetupStorageEncryption(t *state.Task, _ *tomb.T
 		return fmt.Errorf("reading gadget information: %v", err)
 	}
 
+	var checkAction *secboot.PreinstallAction
+	if m.readCacheEncryptionSupportInfoLocked(systemLabel) != nil {
+		// If a cached context is available, the preinstall check was previously run
+		// and must be repeated. Set an action to signal reuse of the preinstall check
+		// context. Use the inert "ActionNone" to repeat the check without requesting
+		// any modifications.
+		checkAction = &secboot.PreinstallAction{Action: secboot.ActionNone}
+	}
+
 	constraints := installLogic.EncryptionConstraints{
 		Model:         systemAndSeeds.Model,
 		Kernel:        systemAndSeeds.InfosByType[snap.TypeKernel],
 		Gadget:        gadgetInfo,
 		TPMMode:       secboot.TPMProvisionFull,
 		SnapdVersions: systemAndSeeds.SystemSnapdVersions,
+		CheckAction:   checkAction,
 	}
 
 	// do not use cached encryption information; perform a fresh encryption
diff --git a/secboot/preinstall_nosb.go b/secboot/preinstall_nosb.go
@@ -27,6 +27,8 @@ import (
 type PreinstallCheckContext struct{}
 type PreinstallCheckResult struct{}
 
+const ActionNone = ""
+
 func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallCheckContext, []PreinstallErrorDetails, error) {
 	return nil, nil, errBuildWithoutSecboot
 }
diff --git a/secboot/preinstall_sb.go b/secboot/preinstall_sb.go
@@ -61,6 +61,8 @@ var (
 	sbPreinstallRunChecks           = (*sb_preinstall.RunChecksContext).Run
 )
 
+const ActionNone = string(sb_preinstall.ActionNone)
+
 // PreinstallCheck runs preinstall checks using default check configuration and
 // TCG-compliant PCR profile generation options to evaluate whether the host
 // environment is an EFI system suitable for TPM-based Full Disk Encryption. The

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,8 @@ import (`
`27`	`27`	`type PreinstallCheckContext struct{}`
`28`	`28`	`type PreinstallCheckResult struct{}`
`29`	`29`
	`30`	`+const ActionNone = ""`
	`31`	`+`
`30`	`32`	`func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallCheckContext, []PreinstallErrorDetails, error) {`
`31`	`33`	`return nil, nil, errBuildWithoutSecboot`
`32`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,8 @@ var (`
`61`	`61`	`sbPreinstallRunChecks = (*sb_preinstall.RunChecksContext).Run`
`62`	`62`	`)`
`63`	`63`
	`64`	`+const ActionNone = string(sb_preinstall.ActionNone)`
	`65`	`+`
`64`	`66`	`// PreinstallCheck runs preinstall checks using default check configuration and`
`65`	`67`	`// TCG-compliant PCR profile generation options to evaluate whether the host`
`66`	`68`	`// environment is an EFI system suitable for TPM-based Full Disk Encryption. The`