From 944d18b9b2568ee95d6c00edb7af137bbf284757 Mon Sep 17 00:00:00 2001 From: Valentin David Date: Wed, 18 Feb 2026 15:36:20 +0100 Subject: [PATCH] overlord/fdestate: run post install checks during auto repair --- overlord/devicestate/devicemgr.go | 16 +- .../devicestate/devicestate_cloudinit_test.go | 2 +- .../devicestate/devicestate_systems_test.go | 4 +- overlord/devicestate/devicestate_test.go | 54 ++++- overlord/devicestate/export_test.go | 2 +- overlord/fdestate/autorepair.go | 146 ++++++++++++- overlord/fdestate/autorepair_test.go | 195 +++++++++++++++++- overlord/fdestate/export_test.go | 14 ++ overlord/install/install.go | 13 +- overlord/install/install_test.go | 20 +- secboot/preinstall_nosb.go | 8 +- secboot/preinstall_sb.go | 55 +++-- secboot/preinstall_sb_test.go | 21 +- 13 files changed, 491 insertions(+), 59 deletions(-) diff --git a/overlord/devicestate/devicemgr.go b/overlord/devicestate/devicemgr.go index 070652bc559..cc1784d55f7 100644 --- a/overlord/devicestate/devicemgr.go +++ b/overlord/devicestate/devicemgr.go @@ -1266,6 +1266,20 @@ func (m *DeviceManager) ensureFDE() error { logger.Trace("ensure", "manager", "DeviceManager", "func", "ensureFDE") + model, err := m.Model() + if err != nil { + if errors.Is(err, state.ErrNoState) { + logger.Debugf("no model is available, skipping ensureFDE") + return nil + } + return err + } + + runPostInstallChecks, err := install.CheckHybridQuestingRelease(model) + if err != nil { + return err + } + // FIXME: we should rename to something like "reset lockout" lockoutResetErr := secbootMarkSuccessful() @@ -1276,7 +1290,7 @@ func (m *DeviceManager) ensureFDE() error { // FIXME: we need to check that a try kernel was attempted here and not attempt // repair in that case. - if err := fdestateAttemptAutoRepairIfNeeded(m.state, lockoutResetErr); err != nil { + if err := fdestateAttemptAutoRepairIfNeeded(m.state, lockoutResetErr, runPostInstallChecks); err != nil { return err } diff --git a/overlord/devicestate/devicestate_cloudinit_test.go b/overlord/devicestate/devicestate_cloudinit_test.go index 3e2a99f1088..26b918808ac 100644 --- a/overlord/devicestate/devicestate_cloudinit_test.go +++ b/overlord/devicestate/devicestate_cloudinit_test.go @@ -43,7 +43,7 @@ func (s *cloudInitBaseSuite) SetUpTest(c *C) { r := release.MockOnClassic(false) defer r() - s.AddCleanup(devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error) error { + s.AddCleanup(devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { return nil })) diff --git a/overlord/devicestate/devicestate_systems_test.go b/overlord/devicestate/devicestate_systems_test.go index 8ec3f66605d..a646bb0037b 100644 --- a/overlord/devicestate/devicestate_systems_test.go +++ b/overlord/devicestate/devicestate_systems_test.go @@ -3445,9 +3445,9 @@ func mockHelperForEncryptionAvailabilityCheck(s suiteWithAddCleanup, c *C, isSup s.DeviceManager().SetEncryptionSupportInfoInCacheUnlocked(cacheLabel, encInfo) } - restore := install.MockSecbootPreinstallCheck(func(ctx context.Context, bootImagePaths []string) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { + restore := install.MockSecbootPreinstallCheck(func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { callCnt.checkCnt++ - c.Assert(bootImagePaths, HasLen, 3) + c.Assert(bootImageFiles, HasLen, 3) c.Assert(isSupportedUbuntuHybrid, Equals, true) if hasTPM { return preinstallCheckContext, nil, nil diff --git a/overlord/devicestate/devicestate_test.go b/overlord/devicestate/devicestate_test.go index 4cfa393933f..b9a67c3b4cb 100644 --- a/overlord/devicestate/devicestate_test.go +++ b/overlord/devicestate/devicestate_test.go @@ -3797,12 +3797,62 @@ func (s *deviceMgrSuite) TestDeviceManagerStartupCallbacks(c *C) { } func (s *deviceMgrSuite) TestDeviceManagerEnsureFDE(c *C) { + s.setHybridModelInState(c) + defer release.MockReleaseInfo(&release.OS{ID: "ubuntu", VersionID: "26.04"})() + defer release.MockOnClassic(true)() + + defer devicestate.MockSecbootMarkSuccessful(func() error { + return fmt.Errorf("MarkSuccessful did not work") + })() + + called := 0 + defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { + c.Check(runPostInstallChecks, Equals, true) + c.Check(lockoutResetErr, ErrorMatches, `MarkSuccessful did not work`) + called++ + return nil + })() + + devicestate.SetSystemMode(s.mgr, "run") + err := devicestate.EnsureFDE(s.mgr) + c.Assert(err, IsNil) + c.Check(called, Equals, 1) +} + +func (s *deviceMgrSuite) TestDeviceManagerEnsureFDEClassicNoPostInstallChecks(c *C) { + s.setHybridModelInState(c) + defer release.MockReleaseInfo(&release.OS{ID: "ubuntu", VersionID: "25.04"})() + defer release.MockOnClassic(true)() + + defer devicestate.MockSecbootMarkSuccessful(func() error { + return fmt.Errorf("MarkSuccessful did not work") + })() + + called := 0 + defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { + c.Check(runPostInstallChecks, Equals, false) + c.Check(lockoutResetErr, ErrorMatches, `MarkSuccessful did not work`) + called++ + return nil + })() + + devicestate.SetSystemMode(s.mgr, "run") + err := devicestate.EnsureFDE(s.mgr) + c.Assert(err, IsNil) + c.Check(called, Equals, 1) +} + +func (s *deviceMgrSuite) TestDeviceManagerEnsureFDENoPostInstallChecks(c *C) { + s.setUC20PCModelInState(c) + defer release.MockOnClassic(false)() + defer devicestate.MockSecbootMarkSuccessful(func() error { return fmt.Errorf("MarkSuccessful did not work") })() called := 0 - defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error) error { + defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { + c.Check(runPostInstallChecks, Equals, false) c.Check(lockoutResetErr, ErrorMatches, `MarkSuccessful did not work`) called++ return nil @@ -3820,7 +3870,7 @@ func (s *deviceMgrSuite) TestDeviceManagerEnsureFDEInstall(c *C) { return fmt.Errorf("unexpected call") })() - defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error) error { + defer devicestate.MockFdestateAttemptAutoRepairIfNeeded(func(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { c.Errorf("unexpected call") return fmt.Errorf("unexpected call") })() diff --git a/overlord/devicestate/export_test.go b/overlord/devicestate/export_test.go index 45f38b8b9cf..54f01f9cc15 100644 --- a/overlord/devicestate/export_test.go +++ b/overlord/devicestate/export_test.go @@ -751,6 +751,6 @@ func MockOsutilBootID(bootID string) (restore func()) { return testutil.Mock(&osutilBootID, func() (string, error) { return bootID, nil }) } -func MockFdestateAttemptAutoRepairIfNeeded(f func(st *state.State, locktoutResetErr error) error) (restore func()) { +func MockFdestateAttemptAutoRepairIfNeeded(f func(st *state.State, locktoutResetErr error, runPostInstallChecks bool) error) (restore func()) { return testutil.Mock(&fdestateAttemptAutoRepairIfNeeded, f) } diff --git a/overlord/fdestate/autorepair.go b/overlord/fdestate/autorepair.go index b00ed24ca16..d97fd5c6dd7 100644 --- a/overlord/fdestate/autorepair.go +++ b/overlord/fdestate/autorepair.go @@ -20,11 +20,16 @@ package fdestate import ( + "context" "errors" "fmt" "os" + "path/filepath" + "strings" + "time" "github.com/snapcore/snapd/boot" + "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/gadget/device" "github.com/snapcore/snapd/logger" @@ -34,8 +39,13 @@ import ( ) var ( + bootloaderFind = bootloader.Find + + bootReadModeenv = boot.ReadModeenv + secbootProvisionTPM = secboot.ProvisionTPM secbootShouldAttemptRepair = secboot.ShouldAttemptRepair + secbootPostinstallCheck = secboot.PostinstallCheck osutilBootID = osutil.BootID ) @@ -51,6 +61,10 @@ const ( AutoRepairSuccess AutoRepairResult = "success" ) +const ( + postInstallCheckTimeout = 2 * time.Minute +) + type repairState struct { Result AutoRepairResult `json:"result"` } @@ -97,7 +111,109 @@ func getRepairAttemptResult(st *state.State) (*repairState, error) { return rs.State, nil } -func autoRepair(st *state.State) (AutoRepairResult, error) { +func getRunBootChain() ([]bootloader.BootFile, error) { + modeenv, err := bootReadModeenv(dirs.GlobalRootDir) + if err != nil { + return nil, fmt.Errorf("cannot read modeenv: %w", err) + } + + rbl, err := bootloaderFind(boot.InitramfsUbuntuSeedDir, &bootloader.Options{ + Role: bootloader.RoleRecovery, + }) + if err != nil { + return nil, fmt.Errorf("cannot find recovery bootloader: %w", err) + } + + tbl, ok := rbl.(bootloader.TrustedAssetsBootloader) + if !ok { + return nil, fmt.Errorf("internal error: recovery bootloader does not support trusted assets") + } + + bl, err := bootloaderFind(boot.InitramfsUbuntuBootDir, &bootloader.Options{ + Role: bootloader.RoleRunMode, + NoSlashBoot: true, + }) + if err != nil { + return nil, fmt.Errorf("cannot find run bootloader: %w", err) + } + + ebl, ok := bl.(bootloader.ExtractedRunKernelImageBootloader) + if !ok { + return nil, fmt.Errorf("internal error: run bootloader does not support kernel extraction") + } + + info, err := ebl.TryKernel() + if err != nil { + if err == bootloader.ErrNoTryKernelRef { + info, err = ebl.Kernel() + } + if err != nil { + return nil, err + } + } + + trustedAssets, err := tbl.TrustedAssets() + if err != nil { + return nil, err + } + + kernelPath := info.MountFile() + + runModeBootChains, err := tbl.BootChains(bl, kernelPath) + if err != nil { + return nil, err + } + + // runModeBootChains is all possible run boot chains, but only one should exist (there + // are legacy boot chains before we registered UEFI boot entries). + // The "BootFile"s for the gadget part points to identifier names instead of real path, so we + // need to resolve those. To resolve those we need to cross check with the modeenv, and then + // find the file in the cache. The last one, is the kernel and should be pointing to the right place. + for _, runModeBootChain := range runModeBootChains { + var chain []bootloader.BootFile + + if len(runModeBootChain) == 0 { + // That is not possible for a boot chain to be size 0, because that would mean there is no + // kernel. We should not ignore this, there are bigger problems. + return nil, fmt.Errorf("internal error: no file in boot chain") + } + + ignoreChain := false + for _, bf := range runModeBootChain[:len(runModeBootChain)-1] { + path := bf.Path + name, ok := trustedAssets[path] + if !ok { + return nil, fmt.Errorf("internal error: unknown trusted asset %s from boot chain", path) + } + var hashes []string + if bf.Role == bootloader.RoleRecovery { + hashes, ok = modeenv.CurrentTrustedRecoveryBootAssets[name] + } else { + hashes, ok = modeenv.CurrentTrustedBootAssets[name] + } + if !ok { + ignoreChain = true + break + } + + // In theory we should only have one hash here. Multiple would be when we are trying + // a boot chain, and this should have been cleaned. It should be safe to take the last one (newest). + if len(hashes) > 1 { + logger.Noticef("WARNING: multiple hashes for a trusted boot file were found.") + } + hash := hashes[len(hashes)-1] + p := filepath.Join(dirs.SnapBootAssetsDir, bl.Name(), fmt.Sprintf("%s-%s", name, hash)) + chain = append(chain, bootloader.NewBootFile("", p, bf.Role)) + } + if !ignoreChain { + return append(chain, runModeBootChain[len(runModeBootChain)-1]), nil + } + } + + return nil, fmt.Errorf("cannot find the active boot chain") +} + +func autoRepair(st *state.State, runPostInstallChecks bool) (AutoRepairResult, error) { method, err := device.SealedKeysMethod(dirs.GlobalRootDir) if err != nil { return AutoRepairNotAttempted, err @@ -106,8 +222,28 @@ func autoRepair(st *state.State) (AutoRepairResult, error) { switch method { case device.SealingMethodFDESetupHook: case device.SealingMethodTPM, device.SealingMethodLegacyTPM: - // FIXME: re-run platform checks (post install checks?) - // Then maybe return AutoRepairFailedEncryptionSupport + if runPostInstallChecks { + images, err := getRunBootChain() + if err != nil { + return AutoRepairNotAttempted, err + } + + ctx, cancel := context.WithTimeout(context.Background(), postInstallCheckTimeout) + defer cancel() + + if _, details, err := secbootPostinstallCheck(ctx, images); len(details) > 0 || err != nil { + if err != nil { + logger.Noticef("WARNING: could not auto repair keyslots due to failed platform initialization: %v", err) + } else { + var messages []string + for _, detail := range details { + messages = append(messages, fmt.Sprintf("- %s", detail.Message)) + } + logger.Noticef("WARNING: could not auto repair keyslots due to failed platform initialization:\n%s", strings.Join(messages, "\n")) + } + return AutoRepairFailedPlatformInit, nil + } + } lockoutAuthFile := device.TpmLockoutAuthUnder(boot.InstallHostFDESaveDir) if err := secbootProvisionTPM(secboot.TPMPartialReprovision, lockoutAuthFile); err != nil { @@ -143,7 +279,7 @@ func autoRepair(st *state.State) (AutoRepairResult, error) { // of lockout reset and may attempt to repair keyslots. If the // auto-repair attempted has already occurred during the current boot, // this will do nothing. -func AttemptAutoRepairIfNeeded(st *state.State, lockoutResetErr error) error { +func AttemptAutoRepairIfNeeded(st *state.State, lockoutResetErr error, runPostInstallChecks bool) error { if lockoutResetErr != nil { // FIXME: we need to either try repair in some cases and save the // error for the status API @@ -188,7 +324,7 @@ func AttemptAutoRepairIfNeeded(st *state.State, lockoutResetErr error) error { } } - result, err := autoRepair(st) + result, err := autoRepair(st, runPostInstallChecks) if err != nil { return err } diff --git a/overlord/fdestate/autorepair_test.go b/overlord/fdestate/autorepair_test.go index 3afc24cbe4b..8a1298a96b5 100644 --- a/overlord/fdestate/autorepair_test.go +++ b/overlord/fdestate/autorepair_test.go @@ -21,6 +21,7 @@ package fdestate_test import ( + "context" "fmt" "os" @@ -28,12 +29,15 @@ import ( sb "github.com/snapcore/secboot" "github.com/snapcore/snapd/boot" + "github.com/snapcore/snapd/bootloader" + "github.com/snapcore/snapd/bootloader/bootloadertest" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/gadget/device" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/overlord/fdestate" "github.com/snapcore/snapd/overlord/fdestate/backend" "github.com/snapcore/snapd/secboot" + "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" ) @@ -54,6 +58,58 @@ func (s *autoRepairSuite) SetUpTest(c *C) { })) } +func (s *autoRepairSuite) mockPostInstallChecks(c *C) { + recoveryBl := bootloadertest.Mock("recovery", "").WithTrustedAssets() + recoveryBl.TrustedAssetsMap = map[string]string{ + "EFI/ubuntu/shim.efi": "ubuntu:shim", + "EFI/ubuntu/grub.efi": "ubuntu:grub", + } + recoveryBl.KernelBootFileBuilder = func(kernelPath string) bootloader.BootFile { + return bootloader.NewBootFile("some-kernel", "kernel.efi", bootloader.RoleRunMode) + } + recoveryBl.BootChainList = []bootloader.BootFile{ + bootloader.NewBootFile("", "EFI/ubuntu/shim.efi", bootloader.RoleRecovery), + bootloader.NewBootFile("", "EFI/ubuntu/grub.efi", bootloader.RoleRecovery), + bootloader.NewBootFile("", "EFI/ubuntu/grub.efi", bootloader.RoleRunMode), + } + + runBl := bootloadertest.Mock("run", "").WithExtractedRunKernelImage() + runBl.SetEnabledKernel(&snap.Info{SuggestedName: "some-kernel", InstanceKey: "x1", SnapType: snap.TypeKernel}) + + s.AddCleanup(fdestate.MockBootloaderFind(func(rootdir string, opts *bootloader.Options) (bootloader.Bootloader, error) { + if opts.Role == bootloader.RoleRecovery { + return recoveryBl, nil + } else if opts.Role == bootloader.RoleRunMode { + return runBl, nil + } else { + c.Errorf("unexpected") + return nil, fmt.Errorf("unexpected") + } + })) + + s.AddCleanup(fdestate.MockBootReadModeenv(func(rootdir string) (*boot.Modeenv, error) { + return &boot.Modeenv{ + CurrentTrustedBootAssets: map[string][]string{ + "ubuntu:grub": { + "hash-grub-run", + }, + }, + CurrentTrustedRecoveryBootAssets: map[string][]string{ + "ubuntu:shim": { + "hash-shim-recovery", + }, + "ubuntu:grub": { + "hash-grub-recovery", + }, + }, + }, nil + })) + + s.AddCleanup(fdestate.MockSecbootPostinstallCheck(func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { + return nil, nil, nil + })) +} + func (s *autoRepairSuite) TestAttemptAutoRepairNeeded(c *C) { const onClassic = false s.startedManager(c, onClassic) @@ -85,7 +141,10 @@ func (s *autoRepairSuite) TestAttemptAutoRepairNeeded(c *C) { return nil })() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + s.mockPostInstallChecks(c) + + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) c.Check(reprovisioned, Equals, 1) @@ -97,7 +156,7 @@ func (s *autoRepairSuite) TestAttemptAutoRepairNeeded(c *C) { c.Check(result.Result, Equals, fdestate.AutoRepairResult("success")) // Try again it should do nothing - err = fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + err = fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) c.Check(reprovisioned, Equals, 1) @@ -137,7 +196,8 @@ func (s *autoRepairSuite) TestAttemptAutoRepairNotNeeded(c *C) { return fmt.Errorf("Unexpected call") })() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) result, err := fdestate.GetRepairAttemptResult(s.st) @@ -174,7 +234,10 @@ func (s *autoRepairSuite) TestAttemptAutoRepairNeededBadReprovision(c *C) { return fmt.Errorf("Unexpected call") })() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + s.mockPostInstallChecks(c) + + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) c.Check(reprovisioned, Equals, 1) @@ -202,7 +265,8 @@ func (s *autoRepairSuite) TestAttemptAutoRepairErrorNoActivateState(c *C) { logbuf, restore := logger.MockLogger() defer restore() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) result, err := fdestate.GetRepairAttemptResult(s.st) @@ -251,10 +315,13 @@ func (s *autoRepairSuite) TestAttemptAutoRepairErrorNoActivateStateRecovery(c *C }, nil })() + s.mockPostInstallChecks(c) + logbuf, restore := logger.MockLogger() defer restore() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) result, err := fdestate.GetRepairAttemptResult(s.st) @@ -285,7 +352,8 @@ func (s *autoRepairSuite) TestAttemptAutoRepairErrorActivateState(c *C) { logbuf, restore := logger.MockLogger() defer restore() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) result, err := fdestate.GetRepairAttemptResult(s.st) @@ -313,7 +381,8 @@ func (s *autoRepairSuite) TestAttemptAutoRepairErrorNoFileActivateState(c *C) { logbuf, restore := logger.MockLogger() defer restore() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) result, err := fdestate.GetRepairAttemptResult(s.st) @@ -355,7 +424,10 @@ func (s *autoRepairSuite) TestAttemptAutoRepairNeededBadReseal(c *C) { return fmt.Errorf("some error") })() - err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil) + s.mockPostInstallChecks(c) + + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) c.Assert(err, IsNil) c.Check(reprovisioned, Equals, 1) @@ -390,3 +462,108 @@ func (s *autoRepairSuite) TestIgnoreOldAutoRepairResult(c *C) { c.Assert(err, IsNil) c.Check(result.Result, Equals, fdestate.AutoRepairResult("failed-platform-init")) } + +func (s *autoRepairSuite) TestAttemptAutoRepairFailedPostinstallChecks(c *C) { + const onClassic = false + s.startedManager(c, onClassic) + + s.st.Lock() + defer s.st.Unlock() + + c.Assert(device.StampSealedKeys(dirs.GlobalRootDir, device.SealingMethodTPM), IsNil) + + s.createUnlockedState(c, sb.ActivationSucceededWithPlatformKey) + + defer fdestate.MockSecbootProvisionTPM(func(mode secboot.TPMProvisionMode, lockoutAuthFile string) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + })() + + defer fdestate.MockSecbootShouldAttemptRepair(func(as *secboot.ActivateState) bool { + return true + })() + + s.mockBootAssetsStateForModeenv(c) + + defer fdestate.MockBackendResealKeyForBootChains(func(manager backend.FDEStateManager, method device.SealingMethod, rootdir string, params *boot.ResealKeyForBootChainsParams) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + })() + + s.mockPostInstallChecks(c) + defer fdestate.MockSecbootPostinstallCheck(func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { + return nil, nil, fmt.Errorf("some error") + })() + + logbuf, restore := logger.MockLogger() + defer restore() + + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) + c.Assert(err, IsNil) + + result, err := fdestate.GetRepairAttemptResult(s.st) + c.Assert(err, IsNil) + + c.Check(result.Result, Equals, fdestate.AutoRepairResult("failed-platform-init")) + + c.Check(logbuf.String(), testutil.Contains, `WARNING: could not auto repair keyslots due to failed platform initialization: some error`) +} + +func (s *autoRepairSuite) TestAttemptAutoRepairFailedPostinstallChecksWithDetails(c *C) { + const onClassic = false + s.startedManager(c, onClassic) + + s.st.Lock() + defer s.st.Unlock() + + c.Assert(device.StampSealedKeys(dirs.GlobalRootDir, device.SealingMethodTPM), IsNil) + + s.createUnlockedState(c, sb.ActivationSucceededWithPlatformKey) + + defer fdestate.MockSecbootProvisionTPM(func(mode secboot.TPMProvisionMode, lockoutAuthFile string) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + })() + + defer fdestate.MockSecbootShouldAttemptRepair(func(as *secboot.ActivateState) bool { + return true + })() + + s.mockBootAssetsStateForModeenv(c) + + defer fdestate.MockBackendResealKeyForBootChains(func(manager backend.FDEStateManager, method device.SealingMethod, rootdir string, params *boot.ResealKeyForBootChainsParams) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + })() + + s.mockPostInstallChecks(c) + defer fdestate.MockSecbootPostinstallCheck(func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { + var details = []secboot.PreinstallErrorDetails{ + { + Kind: "kind-1", + Message: "error-1", + }, + { + Kind: "kind-2", + Message: "error-2", + }, + } + + return nil, details, nil + })() + + logbuf, restore := logger.MockLogger() + defer restore() + + const runPostInstallChecks = true + err := fdestate.AttemptAutoRepairIfNeeded(s.st, nil, runPostInstallChecks) + c.Assert(err, IsNil) + + result, err := fdestate.GetRepairAttemptResult(s.st) + c.Assert(err, IsNil) + + c.Check(result.Result, Equals, fdestate.AutoRepairResult("failed-platform-init")) + + c.Check(logbuf.String(), testutil.Contains, "WARNING: could not auto repair keyslots due to failed platform initialization:\n- error-1\n- error-2\n") +} diff --git a/overlord/fdestate/export_test.go b/overlord/fdestate/export_test.go index 08cf94932b9..f37b3ec3363 100644 --- a/overlord/fdestate/export_test.go +++ b/overlord/fdestate/export_test.go @@ -20,9 +20,11 @@ package fdestate import ( + "context" "time" "github.com/snapcore/snapd/boot" + "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/gadget" "github.com/snapcore/snapd/gadget/device" "github.com/snapcore/snapd/overlord/fdestate/backend" @@ -191,3 +193,15 @@ func MockSecbootShouldAttemptRepair(f func(as *secboot.ActivateState) bool) (res func MockSecbootGetPrimaryKey(f func(devices []string, fallbackKeyFiles []string) ([]byte, error)) (restore func()) { return testutil.Mock(&secbootGetPrimaryKey, f) } + +func MockBootloaderFind(f func(rootdir string, opts *bootloader.Options) (bootloader.Bootloader, error)) (restore func()) { + return testutil.Mock(&bootloaderFind, f) +} + +func MockBootReadModeenv(f func(rootdir string) (*boot.Modeenv, error)) (restore func()) { + return testutil.Mock(&bootReadModeenv, f) +} + +func MockSecbootPostinstallCheck(f func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error)) (restore func()) { + return testutil.Mock(&secbootPostinstallCheck, f) +} diff --git a/overlord/install/install.go b/overlord/install/install.go index 0a940f67a4b..ed2837e2398 100644 --- a/overlord/install/install.go +++ b/overlord/install/install.go @@ -37,6 +37,7 @@ import ( "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/boot" + "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/gadget" "github.com/snapcore/snapd/gadget/device" @@ -207,7 +208,7 @@ func MockSecbootCheckTPMKeySealingSupported(f func(tpmMode secboot.TPMProvisionM } // MockSecbootPreinstallCheck mocks secbootPreinstallCheck usage by the package for testing. -func MockSecbootPreinstallCheck(f func(ctx context.Context, bootImagePaths []string) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error)) (restore func()) { +func MockSecbootPreinstallCheck(f func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error)) (restore func()) { osutil.MustBeTestBinary("secbootPreinstallCheck can only be mocked in tests") old := secbootPreinstallCheck secbootPreinstallCheck = f @@ -464,7 +465,7 @@ func CheckHybridQuestingRelease(model *asserts.Model) (bool, error) { return cmp >= 0, nil } -func orderedCurrentBootImages(model *asserts.Model) ([]string, error) { +func orderedCurrentBootImages(model *asserts.Model) ([]bootloader.BootFile, error) { if model.HybridClassic() { images, err := orderedCurrentBootImagesHybrid() if err != nil { @@ -476,7 +477,7 @@ func orderedCurrentBootImages(model *asserts.Model) ([]string, error) { return nil, nil } -func orderedCurrentBootImagesHybrid() ([]string, error) { +func orderedCurrentBootImagesHybrid() ([]bootloader.BootFile, error) { imageInfo := []struct { name string glob string @@ -486,7 +487,7 @@ func orderedCurrentBootImagesHybrid() ([]string, error) { {"kernel", filepath.Join(dirs.GlobalRootDir, "cdrom/casper/vmlinuz")}, } - var bootImagePaths []string + var bootImageFiles []bootloader.BootFile for _, info := range imageInfo { matches, err := filepath.Glob(info.glob) if err != nil { @@ -498,10 +499,10 @@ func orderedCurrentBootImagesHybrid() ([]string, error) { if len(matches) > 1 { return nil, fmt.Errorf("unexpected multiple matches for installer %s obtained using globbing pattern %q", info.name, info.glob) } - bootImagePaths = append(bootImagePaths, matches[0]) + bootImageFiles = append(bootImageFiles, bootloader.NewBootFile("", matches[0], bootloader.RoleRunMode)) } - return bootImagePaths, nil + return bootImageFiles, nil } func hasFDESetupHookInKernel(kernelInfo *snap.Info) bool { diff --git a/overlord/install/install_test.go b/overlord/install/install_test.go index 48a6dc6b47b..40ebd4c3cc1 100644 --- a/overlord/install/install_test.go +++ b/overlord/install/install_test.go @@ -329,14 +329,14 @@ func (s *installSuite) TestOrderedCurrentBootImagesHybrid(c *C) { } { s.mockHelperForOrderedCurrentBootImagesHybrid(c, true, tc.imageError, tc.errorBootImage) - bootImagePaths, err := install.OrderedCurrentBootImagesHybrid() + bootImageFiles, err := install.OrderedCurrentBootImagesHybrid() if tc.expectedError != "" { c.Assert(err, ErrorMatches, tc.expectedError) } else { c.Assert(err, IsNil) - for i, path := range bootImagePaths { - c.Assert(path, Matches, "*/"+relBootImagePaths[i]) + for i, path := range bootImageFiles { + c.Assert(path.Path, Matches, "*/"+relBootImagePaths[i]) } } } @@ -385,15 +385,15 @@ func (s *installSuite) TestOrderedCurrentBootImages(c *C) { } modelMock := s.mockModel(modelMods) - bootImagePaths, err := install.OrderedCurrentBootImages(modelMock) + bootImageFiles, err := install.OrderedCurrentBootImages(modelMock) if tc.expectedError != "" { c.Assert(err, ErrorMatches, tc.expectedError) } else { c.Assert(err, IsNil) } - for i, path := range bootImagePaths { - c.Assert(path, Matches, "*/"+relBootImagePaths[i]) + for i, path := range bootImageFiles { + c.Assert(path.Path, Matches, "*/"+relBootImagePaths[i]) } } } @@ -616,12 +616,12 @@ func (s *installSuite) mockHelperForEncryptionAvailabilityCheck(c *C, isSupporte } // mock secboot.PreinstallCheck for Supported Ubuntu hybrid systems - restore := install.MockSecbootPreinstallCheck(func(ctx context.Context, bootImagePaths []string) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { + restore := install.MockSecbootPreinstallCheck(func(ctx context.Context, bootImageFiles []bootloader.BootFile) (*secboot.PreinstallCheckContext, []secboot.PreinstallErrorDetails, error) { c.Assert(ctx, NotNil) c.Assert(isSupportedUbuntuHybrid, Equals, true) - c.Assert(bootImagePaths, HasLen, len(relBootImagePaths)) - for i, path := range bootImagePaths { - c.Assert(path, Matches, "*/"+relBootImagePaths[i]) + c.Assert(bootImageFiles, HasLen, len(relBootImagePaths)) + for i, path := range bootImageFiles { + c.Assert(path.Path, Matches, "*/"+relBootImagePaths[i]) } if checkFailErrors == ErrorSecbootPreinstall { diff --git a/secboot/preinstall_nosb.go b/secboot/preinstall_nosb.go index 383c8f1b4cc..f7e27f54dd0 100644 --- a/secboot/preinstall_nosb.go +++ b/secboot/preinstall_nosb.go @@ -22,6 +22,8 @@ package secboot import ( "context" + + "github.com/snapcore/snapd/bootloader" ) type PreinstallCheckContext struct{} @@ -29,7 +31,11 @@ type PreinstallCheckResult struct{} const ActionNone = "" -func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { +func PreinstallCheck(ctx context.Context, bootImageFiles []bootloader.BootFile) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { + return nil, nil, errBuildWithoutSecboot +} + +func PostinstallCheck(ctx context.Context, bootImageFiles []bootloader.BootFile) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { return nil, nil, errBuildWithoutSecboot } diff --git a/secboot/preinstall_sb.go b/secboot/preinstall_sb.go index 3b4d8056b62..e8f8c804800 100644 --- a/secboot/preinstall_sb.go +++ b/secboot/preinstall_sb.go @@ -30,6 +30,7 @@ import ( sb_efi "github.com/snapcore/secboot/efi" sb_preinstall "github.com/snapcore/secboot/efi/preinstall" + "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/snapdenv" @@ -64,19 +65,7 @@ var ( 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 -// caller must supply the current boot images in boot order via bootImagePaths. -// On success, it returns the preinstall check context required for follow-up -// preinstall checks with actions, and a list with details on all errors -// identified by secboot (or nil if no errors were found). Any warnings -// contained in the secboot result are logged. On failure, it returns the error -// encountered while interpreting the secboot error. -// -// To support testing, when the system is running in a Virtual Machine, the check -// configuration is modified to permit this to avoid an error. -func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { +func preinstallCheck(ctx context.Context, postInstall bool, bootImageFiles []bootloader.BootFile) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { // allow value-added-retailer drivers that are: // - listed as Driver#### load options // - referenced in the DriverOrder UEFI variable @@ -87,13 +76,21 @@ func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallC checkFlags |= sb_preinstall.PermitVirtualMachine } + if postInstall { + checkFlags |= sb_preinstall.PostInstallChecks + } + // do not customize TCG compliant PCR profile generation profileOptionFlags := sb_preinstall.PCRProfileOptionsDefault - // create boot file images from provided paths + // create boot file images from provided boot image files var bootImages []sb_efi.Image - for _, image := range bootImagePaths { - bootImages = append(bootImages, sb_efi.NewFileImage(image)) + for _, image := range bootImageFiles { + fileImage, err := efiImageFromBootFile(&image) + if err != nil { + return nil, nil, err + } + bootImages = append(bootImages, fileImage) } checkContext := &PreinstallCheckContext{sbPreinstallNewRunChecksContext(checkFlags, bootImages, profileOptionFlags)} @@ -104,7 +101,7 @@ func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallC if err != nil { return nil, errorDetails, err } - return checkContext, errorDetails, err + return checkContext, errorDetails, nil } if result.Warnings != nil { @@ -116,6 +113,30 @@ func PreinstallCheck(ctx context.Context, bootImagePaths []string) (*PreinstallC return checkContext, nil, nil } +// 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 +// caller must supply the current boot images in boot order via bootImageFiles. +// On success, it returns the preinstall check context required for follow-up +// preinstall checks with actions, and a list with details on all errors +// identified by secboot (or nil if no errors were found). Any warnings +// contained in the secboot result are logged. On failure, it returns the error +// encountered while interpreting the secboot error. +// +// To support testing, when the system is running in a Virtual Machine, the check +// configuration is modified to permit this to avoid an error. +func PreinstallCheck(ctx context.Context, bootImageFiles []bootloader.BootFile) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { + const postInstall bool = false + return preinstallCheck(ctx, postInstall, bootImageFiles) +} + +// PostinstallCheck re-runs almost the same checks as PreinstallCheck +// but for an already installed instance. +func PostinstallCheck(ctx context.Context, bootImageFiles []bootloader.BootFile) (*PreinstallCheckContext, []PreinstallErrorDetails, error) { + const postInstall bool = true + return preinstallCheck(ctx, postInstall, bootImageFiles) +} + // PreinstallCheckAction runs a follow-up preinstall check using the specified // action to evaluate whether a previously reported issue can be resolved. It // reuses the check configuration and boot image state from the preinstall check diff --git a/secboot/preinstall_sb_test.go b/secboot/preinstall_sb_test.go index be12c2bb6f9..330ac152b6d 100644 --- a/secboot/preinstall_sb_test.go +++ b/secboot/preinstall_sb_test.go @@ -33,6 +33,7 @@ import ( sb_preinstall "github.com/snapcore/secboot/efi/preinstall" . "gopkg.in/check.v1" + "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/secboot" "github.com/snapcore/snapd/testutil" @@ -304,10 +305,22 @@ func (s *preinstallSuite) TestPreinstallCheckConfig(c *C) { // testPreinstallCheckAndAction is a helper to test PreinstallCheck and PreinstallCheckAction func (s *preinstallSuite) testPreinstallCheckAndAction(c *C, checkAction *secboot.PreinstallAction, detectErrors, failUnwrap bool) { + rootdir := c.MkDir() + bootImagePaths := []string{ - "/cdrom/EFI/boot/bootXXX.efi", - "/cdrom/EFI/boot/grubXXX.efi", - "/cdrom/casper/vmlinuz", + filepath.Join(rootdir, "/cdrom/EFI/boot/bootXXX.efi"), + filepath.Join(rootdir, "/cdrom/EFI/boot/grubXXX.efi"), + filepath.Join(rootdir, "/cdrom/casper/vmlinuz"), + } + + var bootImageFiles []bootloader.BootFile + for _, path := range bootImagePaths { + err := os.MkdirAll(filepath.Dir(path), 0755) + c.Assert(err, IsNil) + err = os.WriteFile(path, []byte{}, 0644) + c.Assert(err, IsNil) + // role is not important + bootImageFiles = append(bootImageFiles, bootloader.NewBootFile("", path, bootloader.RoleRecovery)) } systemdCmd := testutil.MockCommand(c, "systemd-detect-virt", "exit 1") @@ -387,7 +400,7 @@ func (s *preinstallSuite) testPreinstallCheckAndAction(c *C, checkAction *secboo if checkAction == nil { // test PreinstallCheck expectedAction = sb_preinstall.ActionNone - checkContext, errorDetails, err = secboot.PreinstallCheck(context.Background(), bootImagePaths) + checkContext, errorDetails, err = secboot.PreinstallCheck(context.Background(), bootImageFiles) if failUnwrap { c.Assert(checkContext, IsNil) } else {