diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go index 79a40b85171..d3b8a3fefd8 100644 --- a/overlord/devicestate/devicestate_install_mode_test.go +++ b/overlord/devicestate/devicestate_install_mode_test.go @@ -2333,7 +2333,7 @@ func (s *deviceMgrInstallModeSuite) doRunFactoryResetChange(c *C, model *asserts return fmt.Errorf("unexpected call") })() - defer devicestate.MockSecbootRenameOrDeleteKeys(func(node string, renames map[string]string) error { + defer devicestate.MockSecbootRenameKeys(func(node string, renames map[string]string) error { if tc.encrypt { c.Check(node, Equals, "/dev/disk/by-uuid/570faa3d-e3bc-49db-979b-e7814b6bd390") c.Check(renames, DeepEquals, map[string]string{ diff --git a/overlord/devicestate/export_test.go b/overlord/devicestate/export_test.go index 5dbf189cd6d..d6e24460c6a 100644 --- a/overlord/devicestate/export_test.go +++ b/overlord/devicestate/export_test.go @@ -610,11 +610,11 @@ func MockSecbootAddBootstrapKeyOnExistingDisk(f func(node string, newKey keys.En } } -func MockSecbootRenameOrDeleteKeys(f func(node string, renames map[string]string) error) (restore func()) { - old := secbootRenameOrDeleteKeys - secbootRenameOrDeleteKeys = f +func MockSecbootRenameKeys(f func(node string, renames map[string]string) error) (restore func()) { + old := secbootRenameKeys + secbootRenameKeys = f return func() { - secbootRenameOrDeleteKeys = old + secbootRenameKeys = old } } diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go index d0edec27aa7..0f42fd00537 100644 --- a/overlord/devicestate/handlers_install.go +++ b/overlord/devicestate/handlers_install.go @@ -1313,7 +1313,7 @@ func (m *DeviceManager) doInstallSetupStorageEncryption(t *state.Task, _ *tomb.T var ( secbootAddBootstrapKeyOnExistingDisk = secboot.AddBootstrapKeyOnExistingDisk - secbootRenameOrDeleteKeys = secboot.RenameOrDeleteKeys + secbootRenameKeys = secboot.RenameKeys secbootCreateBootstrappedContainer = secboot.CreateBootstrappedContainer secbootDeleteKeys = secboot.DeleteKeys ) @@ -1352,8 +1352,12 @@ func createSaveBootstrappedContainer(saveNode string) (secboot.BootstrappedConta "default": "factory-reset-old", "default-fallback": "factory-reset-old-fallback", } - if err := secbootRenameOrDeleteKeys(saveNode, renames); err != nil { - return nil, err + if err := secbootRenameKeys(saveNode, renames); err != nil { + return nil, fmt.Errorf("cannot rename existing keys: %w", err) + } + + if err := secboot.ConvertOldDisk(saveNode); err != nil { + return nil, fmt.Errorf("cannot convert old keys: %w", err) } return secbootCreateBootstrappedContainer(secboot.DiskUnlockKey(saveEncryptionKey), saveNode), nil @@ -1378,5 +1382,12 @@ func deleteOldSaveKey(saveMntPnt string) error { "factory-reset-old-fallback": true, } - return secbootDeleteKeys(diskPath, toDelete) + + if err := secbootDeleteKeys(diskPath, toDelete); err != nil { + return fmt.Errorf("cannot delete previous keys: %w", err) + } + if err := secboot.RemoveOldDiskKeys(diskPath); err != nil { + return fmt.Errorf("cannot remove old disk keys: %w", err) + } + return nil } diff --git a/secboot/export_sb_test.go b/secboot/export_sb_test.go index bf383000a9d..c34f96aab4f 100644 --- a/secboot/export_sb_test.go +++ b/secboot/export_sb_test.go @@ -330,6 +330,14 @@ func MockRenameLUKS2ContainerKey(f func(devicePath, keyslotName, renameTo string } } +func MockCopyAndRemoveLUKS2ContainerKey(f func(devicePath, keyslotName, renameTo string) error) (restore func()) { + old := sbCopyAndRemoveLUKS2ContainerKey + sbCopyAndRemoveLUKS2ContainerKey = f + return func() { + sbCopyAndRemoveLUKS2ContainerKey = old + } +} + func MockSbNewFileKeyDataReader(f func(path string) (*sb.FileKeyDataReader, error)) (restore func()) { old := sbNewFileKeyDataReader sbNewFileKeyDataReader = f diff --git a/secboot/secboot_dummy.go b/secboot/secboot_dummy.go index 260addd6f40..a21d226add8 100644 --- a/secboot/secboot_dummy.go +++ b/secboot/secboot_dummy.go @@ -79,7 +79,7 @@ func AddBootstrapKeyOnExistingDisk(node string, newKey keys.EncryptionKey) error return errBuildWithoutSecboot } -func RenameOrDeleteKeys(node string, renames map[string]string) error { +func RenameKeys(node string, renames map[string]string) error { return errBuildWithoutSecboot } diff --git a/secboot/secboot_sb.go b/secboot/secboot_sb.go index beb76acdcbf..d9de2d8fd33 100644 --- a/secboot/secboot_sb.go +++ b/secboot/secboot_sb.go @@ -44,14 +44,15 @@ func sbNewLUKS2KeyDataReaderImpl(device, slot string) (sb.KeyDataReader, error) } var ( - sbActivateVolumeWithKey = sb.ActivateVolumeWithKey - sbActivateVolumeWithKeyData = sb.ActivateVolumeWithKeyData - sbActivateVolumeWithRecoveryKey = sb.ActivateVolumeWithRecoveryKey - sbDeactivateVolume = sb.DeactivateVolume - sbAddLUKS2ContainerUnlockKey = sb.AddLUKS2ContainerUnlockKey - sbRenameLUKS2ContainerKey = sb.RenameLUKS2ContainerKey - sbNewLUKS2KeyDataReader = sbNewLUKS2KeyDataReaderImpl - sbSetProtectorKeys = sb_plainkey.SetProtectorKeys + sbActivateVolumeWithKey = sb.ActivateVolumeWithKey + sbActivateVolumeWithKeyData = sb.ActivateVolumeWithKeyData + sbActivateVolumeWithRecoveryKey = sb.ActivateVolumeWithRecoveryKey + sbDeactivateVolume = sb.DeactivateVolume + sbAddLUKS2ContainerUnlockKey = sb.AddLUKS2ContainerUnlockKey + sbRenameLUKS2ContainerKey = sb.RenameLUKS2ContainerKey + sbCopyAndRemoveLUKS2ContainerKey = sb.CopyAndRemoveLUKS2ContainerKey + sbNewLUKS2KeyDataReader = sbNewLUKS2KeyDataReaderImpl + sbSetProtectorKeys = sb_plainkey.SetProtectorKeys ) func init() { @@ -348,7 +349,7 @@ func AddBootstrapKeyOnExistingDisk(node string, newKey keys.EncryptionKey) error // Rename key slots on LUKS2 container. If the key slot does not // exist, it is ignored. If cryptsetup does not support renaming, then // the key slots are instead removed. -func RenameOrDeleteKeys(node string, renames map[string]string) error { +func RenameKeys(node string, renames map[string]string) error { targets := make(map[string]bool) for _, renameTo := range renames { @@ -378,8 +379,8 @@ func RenameOrDeleteKeys(node string, renames map[string]string) error { if found { if err := sbRenameLUKS2ContainerKey(node, slot, renameTo); err != nil { if errors.Is(err, sb.ErrMissingCryptsetupFeature) { - if err := sbDeleteLUKS2ContainerKey(node, slot); err != nil { - return fmt.Errorf("cannot remove old container key: %v", err) + if err := sbCopyAndRemoveLUKS2ContainerKey(node, slot, renameTo); err != nil { + return fmt.Errorf("cannot rename old container key: %v", err) } } else { return fmt.Errorf("cannot rename container key: %v", err) @@ -456,3 +457,25 @@ func (key *SealKeyRequest) getWriter() (sb.KeyDataWriter, error) { return key.BootstrappedContainer.GetTokenWriter(key.SlotName) } } + +func ConvertOldDisk(devicePath string) error { + if err := sb.NameLegacyLUKS2ContainerKey(devicePath, 0, "old-default-key"); err != nil && !errors.Is(err, sb.KeyslotAlreadyHasAName) { + return err + } + if err := sb.NameLegacyLUKS2ContainerKey(devicePath, 1, "old-recovery-key"); err != nil && !errors.Is(err, sb.KeyslotAlreadyHasAName) { + return err + } + if err := sb.NameLegacyLUKS2ContainerKey(devicePath, 2, "old-temporary-key"); err != nil && !errors.Is(err, sb.KeyslotAlreadyHasAName) { + return err + } + return nil +} + +func RemoveOldDiskKeys(devicePath string) error { + toDelete := map[string]bool{ + "old-default-key": true, + "old-recovery-key": true, + "old-temporary-key": true, + } + return DeleteKeys(devicePath, toDelete) +} diff --git a/secboot/secboot_sb_test.go b/secboot/secboot_sb_test.go index 58055d1e2d5..ae5632bd38e 100644 --- a/secboot/secboot_sb_test.go +++ b/secboot/secboot_sb_test.go @@ -2540,7 +2540,7 @@ func (s *secbootSuite) TestAddBootstrapKeyOnExistingDiskLUKS2Error(c *C) { c.Check(err, ErrorMatches, `cannot enroll new installation key: some error`) } -func (s *secbootSuite) TestRenameOrDeleteKeys(c *C) { +func (s *secbootSuite) TestRenameKeys(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return []string{"slot-a", "slot-b", "slot-c"}, nil @@ -2566,13 +2566,13 @@ func (s *secbootSuite) TestRenameOrDeleteKeys(c *C) { "slot-d": "new-slot-d", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, IsNil) c.Check(expectedRenames, HasLen, 0) } -func (s *secbootSuite) TestRenameOrDeleteKeysBadInput(c *C) { +func (s *secbootSuite) TestRenameKeysBadInput(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Errorf("unexpected call") return []string{"slot-a", "slot-b", "slot-c"}, nil @@ -2588,11 +2588,11 @@ func (s *secbootSuite) TestRenameOrDeleteKeysBadInput(c *C) { "slot-b": "slot-c", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, ErrorMatches, `internal error: keyslot name slot-b used as source and target of a rename`) } -func (s *secbootSuite) TestRenameOrDeleteKeysNameExists(c *C) { +func (s *secbootSuite) TestRenameKeysNameExists(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return []string{"slot-a", "slot-b", "slot-c"}, nil @@ -2607,19 +2607,19 @@ func (s *secbootSuite) TestRenameOrDeleteKeysNameExists(c *C) { "slot-a": "slot-b", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, ErrorMatches, `slot name slot-b is already in use`) } -func (s *secbootSuite) TestRenameOrDeleteKeysNoRename(c *C) { +func (s *secbootSuite) TestRenameKeysNoRename(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return []string{"slot-a", "slot-b", "slot-c"}, nil })() - expectedRemovals := map[string]bool{ - "slot-b": true, - "slot-c": true, + expectedRenames := map[string]string{ + "slot-b": "new-slot-b", + "slot-c": "new-slot-c", } defer secboot.MockRenameLUKS2ContainerKey(func(devicePath, slotName, renameTo string) error { @@ -2627,11 +2627,12 @@ func (s *secbootSuite) TestRenameOrDeleteKeysNoRename(c *C) { return sb.ErrMissingCryptsetupFeature })() - defer secboot.MockDeleteLUKS2ContainerKey(func(devicePath, slotName string) error { + defer secboot.MockCopyAndRemoveLUKS2ContainerKey(func(devicePath, slotName, renameTo string) error { c.Check(devicePath, Equals, "/dev/foo") - _, expected := expectedRemovals[slotName] - c.Check(expected, Equals, true) - delete(expectedRemovals, slotName) + expectedRename, expected := expectedRenames[slotName] + c.Assert(expected, Equals, true) + c.Check(renameTo, Equals, expectedRename) + delete(expectedRenames, slotName) return nil })() @@ -2641,13 +2642,13 @@ func (s *secbootSuite) TestRenameOrDeleteKeysNoRename(c *C) { "slot-d": "new-slot-d", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, IsNil) - c.Check(expectedRemovals, HasLen, 0) + c.Check(expectedRenames, HasLen, 0) } -func (s *secbootSuite) TestRenameOrDeleteKeysListError(c *C) { +func (s *secbootSuite) TestRenameKeysListError(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return nil, fmt.Errorf("some error") @@ -2669,11 +2670,11 @@ func (s *secbootSuite) TestRenameOrDeleteKeysListError(c *C) { "slot-d": "new-slot-d", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, ErrorMatches, `cannot list slots in partition save partition: some error`) } -func (s *secbootSuite) TestRenameOrDeleteKeysRenameError(c *C) { +func (s *secbootSuite) TestRenameKeysRenameError(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return []string{"slot-a", "slot-b", "slot-c"}, nil @@ -2683,7 +2684,7 @@ func (s *secbootSuite) TestRenameOrDeleteKeysRenameError(c *C) { return fmt.Errorf("some other error") })() - defer secboot.MockDeleteLUKS2ContainerKey(func(devicePath, slotName string) error { + defer secboot.MockCopyAndRemoveLUKS2ContainerKey(func(devicePath, slotName, renameTo string) error { c.Errorf("unexpected call") return nil })() @@ -2694,11 +2695,11 @@ func (s *secbootSuite) TestRenameOrDeleteKeysRenameError(c *C) { "slot-d": "new-slot-d", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) + err := secboot.RenameKeys("/dev/foo", toRename) c.Assert(err, ErrorMatches, `cannot rename container key: some other error`) } -func (s *secbootSuite) TestRenameOrDeleteKeysDeleteError(c *C) { +func (s *secbootSuite) TestRenameKeysDeleteError(c *C) { defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) { c.Check(devicePath, Equals, "/dev/foo") return []string{"slot-a", "slot-b", "slot-c"}, nil @@ -2708,7 +2709,7 @@ func (s *secbootSuite) TestRenameOrDeleteKeysDeleteError(c *C) { return sb.ErrMissingCryptsetupFeature })() - defer secboot.MockDeleteLUKS2ContainerKey(func(devicePath, slotName string) error { + defer secboot.MockCopyAndRemoveLUKS2ContainerKey(func(devicePath, slotName, renameTo string) error { return fmt.Errorf("some error") })() @@ -2718,8 +2719,8 @@ func (s *secbootSuite) TestRenameOrDeleteKeysDeleteError(c *C) { "slot-d": "new-slot-d", } - err := secboot.RenameOrDeleteKeys("/dev/foo", toRename) - c.Assert(err, ErrorMatches, `cannot remove old container key: some error`) + err := secboot.RenameKeys("/dev/foo", toRename) + c.Assert(err, ErrorMatches, `cannot rename old container key: some error`) } func (s *secbootSuite) TestDeleteKeys(c *C) {