secboot: use uuid of luks2 instead of partition

`by-partuuid` does not make much sense because it uselessly assumes that it is a partition. Conceptually we should not care about it. It also makes the resolution more complex as we need to fetch information about the device which we do not really need at this point. It is more common to resolve by filesystem UUID than part UUID. For instance cryptsetup accepts path as `UUID=deadbeef-dead-dead-dead-deaddeafbeef`. But it does not accept this kind of syntax for partitions.
canonical · Oct 2, 2024 · 8df73a3 · 8df73a3
1 parent 5cc7d7a
commit 8df73a3
Show file tree

Hide file tree

Showing 11 changed files with 152 additions and 185 deletions.
diff --git a/osutil/disks/disks_darwin.go b/osutil/disks/disks_darwin.go
@@ -67,14 +67,6 @@ var diskFromPartitionDeviceNode = func(node string) (Disk, error) {
 	return nil, osutil.ErrDarwin
 }
 
-func PartitionUUIDFromMountPoint(mountpoint string, opts *Options) (string, error) {
-	return "", osutil.ErrDarwin
-}
-
-func PartitionUUID(node string) (string, error) {
-	return "", osutil.ErrDarwin
-}
-
 func SectorSize(devname string) (uint64, error) {
 	return 0, osutil.ErrDarwin
 }

diff --git a/osutil/disks/disks_linux.go b/osutil/disks/disks_linux.go
@@ -29,6 +29,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -963,41 +964,44 @@ func AllPhysicalDisks() ([]Disk, error) {
 	return disks, nil
 }
 
-// PartitionUUIDFromMountPoint returns the UUID of the partition which is a
-// source of a given mount point.
-func PartitionUUIDFromMountPoint(mountpoint string, opts *Options) (string, error) {
+var dmUUIDRe = regexp.MustCompile(`^CRYPT-(?P<type>.*)-(?P<uuid1>[0-9a-f]{8})(?P<uuid2>[0-9a-f]{4})(?P<uuid3>[0-9a-f]{4})(?P<uuid4>[0-9a-f]{4})(?P<uuid5>[0-9a-f]{12})-(?P<name>.*)$`)
+
+// DMCryptUUIDFromMountPoint finds the UUID of a device mapper device
+// mounted at mountpoint.
+func DMCryptUUIDFromMountPoint(mountpoint string) (string, error) {
 	_, props, err := partitionPropsFromMountPoint(mountpoint)
 	if err != nil {
 		return "", err
 	}
 
-	if opts != nil && opts.IsDecryptedDevice {
-		props, err = parentPartitionPropsForOptions(props)
-		if err != nil {
-			return "", err
-		}
+	dmUUID, hasDmUUID := props["DM_UUID"]
+	if !hasDmUUID {
+		return "", fmt.Errorf("device has no DM_UUID")
 	}
-	partUUID := props["ID_PART_ENTRY_UUID"]
-	if partUUID == "" {
-		partDev := filepath.Join("/dev", props["DEVNAME"])
-		return "", fmt.Errorf("cannot get required partition UUID udev property for device %s", partDev)
+	match := dmUUIDRe.FindStringSubmatchIndex(dmUUID)
+	if match == nil {
+		return "", fmt.Errorf("value of DM_UUID is not recognized")
 	}
-	return partUUID, nil
+
+	result := []byte{}
+	result = dmUUIDRe.ExpandString(result, "${uuid1}-${uuid2}-${uuid3}-${uuid4}-${uuid5}", dmUUID, match)
+	return string(result), nil
 }
 
-// PartitionUUID returns the UUID of a given partition
-func PartitionUUID(node string) (string, error) {
+// FilesystemUUID retrieves the UUID of the filesystem for a give node
+// path
+func FilesystemUUID(node string) (string, error) {
 	props, err := udevPropertiesForName(node)
 	if err != nil && props == nil {
 		// only fail here if props is nil, if it's available we validate it
 		// below
 		return "", fmt.Errorf("cannot process udev properties: %v", err)
 	}
-	partUUID := props["ID_PART_ENTRY_UUID"]
-	if partUUID == "" {
-		return "", fmt.Errorf("cannot get required udev partition UUID property")
+	uuid := props["ID_FS_UUID"]
+	if uuid == "" {
+		return "", fmt.Errorf("cannot get required udev ID_FS_UUID property")
 	}
-	return partUUID, nil
+	return uuid, nil
 }
 
 func SectorSize(devname string) (uint64, error) {

diff --git a/osutil/disks/disks_linux_test.go b/osutil/disks/disks_linux_test.go
@@ -1849,11 +1849,11 @@ func (s *diskSuite) TestAllPhysicalDisks(c *C) {
 	c.Assert(d[3].KernelDeviceNode(), Equals, "/dev/sdb")
 }
 
-func (s *diskSuite) TestPartitionUUIDFromMopuntPointErrs(c *C) {
+func (s *diskSuite) TestDMCryptUUIDFromMountPointErrs(c *C) {
 	restore := osutil.MockMountInfo(``)
 	defer restore()
 
-	_, err := disks.PartitionUUIDFromMountPoint("/run/mnt/blah", nil)
+	_, err := disks.DMCryptUUIDFromMountPoint("/run/mnt/blah")
 	c.Assert(err, ErrorMatches, "cannot find mountpoint \"/run/mnt/blah\"")
 
 	restore = osutil.MockMountInfo(`130 30 42:1 / /run/mnt/point rw,relatime shared:54 - ext4 /dev/vda4 rw
@@ -1870,30 +1870,25 @@ func (s *diskSuite) TestPartitionUUIDFromMopuntPointErrs(c *C) {
 	})
 	defer restore()
 
-	_, err = disks.PartitionUUIDFromMountPoint("/run/mnt/point", nil)
-	c.Assert(err, ErrorMatches, "cannot get required partition UUID udev property for device /dev/vda4")
-}
+	_, err = disks.DMCryptUUIDFromMountPoint("/run/mnt/point")
+	c.Assert(err, ErrorMatches, "device has no DM_UUID")
 
-func (s *diskSuite) TestPartitionUUIDFromMountPointPlain(c *C) {
-	restore := osutil.MockMountInfo(`130 30 42:1 / /run/mnt/point rw,relatime shared:54 - ext4 /dev/vda4 rw
-`)
-	defer restore()
 	restore = disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
 		c.Assert(typeOpt, Equals, "--name")
 		c.Assert(dev, Equals, "/dev/vda4")
 		return map[string]string{
-			"DEVTYPE":            "disk",
-			"ID_PART_ENTRY_UUID": "foo-uuid",
+			"DEVNAME": "vda4",
+			"prop":    "hello",
+			"DM_UUID": "garbage",
 		}, nil
 	})
 	defer restore()
 
-	uuid, err := disks.PartitionUUIDFromMountPoint("/run/mnt/point", nil)
-	c.Assert(err, IsNil)
-	c.Assert(uuid, Equals, "foo-uuid")
+	_, err = disks.DMCryptUUIDFromMountPoint("/run/mnt/point")
+	c.Assert(err, ErrorMatches, "value of DM_UUID is not recognized")
 }
 
-func (s *diskSuite) TestPartitionUUIDFromMopuntPointDecrypted(c *C) {
+func (s *diskSuite) TestDMCryptUUIDFromMountPoint(c *C) {
 	restore := osutil.MockMountInfo(`130 30 42:1 / /run/mnt/point rw,relatime shared:54 - ext4 /dev/mapper/something rw
 `)
 	defer restore()
@@ -1902,13 +1897,7 @@ func (s *diskSuite) TestPartitionUUIDFromMopuntPointDecrypted(c *C) {
 		switch dev {
 		case "/dev/mapper/something":
 			return map[string]string{
-				"DEVTYPE": "disk",
-				"MAJOR":   "242",
-				"MINOR":   "1",
-			}, nil
-		case "/dev/disk/by-uuid/5a522809-c87e-4dfa-81a8-8dc5667d1304":
-			return map[string]string{
-				"ID_PART_ENTRY_UUID": "foo-uuid",
+				"DM_UUID": "CRYPT-LUKS2-5a522809c87e4dfa81a88dc5667d1304-something",
 			}, nil
 		default:
 			c.Errorf("unexpected udev device properties requested: %s", dev)
@@ -1917,58 +1906,9 @@ func (s *diskSuite) TestPartitionUUIDFromMopuntPointDecrypted(c *C) {
 	})
 	defer restore()
 
-	// mock the sysfs dm uuid and name files
-	dmDir := filepath.Join(filepath.Join(dirs.SysfsDir, "dev", "block"), "242:1", "dm")
-	err := os.MkdirAll(dmDir, 0755)
+	uuid, err := disks.DMCryptUUIDFromMountPoint("/run/mnt/point")
 	c.Assert(err, IsNil)
-
-	b := []byte("something")
-	err = os.WriteFile(filepath.Join(dmDir, "name"), b, 0644)
-	c.Assert(err, IsNil)
-
-	b = []byte("CRYPT-LUKS2-5a522809c87e4dfa81a88dc5667d1304-something")
-	err = os.WriteFile(filepath.Join(dmDir, "uuid"), b, 0644)
-	c.Assert(err, IsNil)
-
-	uuid, err := disks.PartitionUUIDFromMountPoint("/run/mnt/point", &disks.Options{
-		IsDecryptedDevice: true,
-	})
-	c.Assert(err, IsNil)
-	c.Assert(uuid, Equals, "foo-uuid")
-}
-
-func (s *diskSuite) TestPartitionUUID(c *C) {
-	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
-		c.Assert(typeOpt, Equals, "--name")
-		switch dev {
-		case "/dev/vda4":
-			return map[string]string{
-				"ID_PART_ENTRY_UUID": "foo-uuid",
-			}, nil
-		case "/dev/no-uuid":
-			return map[string]string{
-				"no-uuid": "no-uuid",
-			}, nil
-		case "/dev/mock-failure":
-			return nil, fmt.Errorf("mock failure")
-		default:
-			c.Errorf("unexpected udev device properties requested: %s", dev)
-			return nil, fmt.Errorf("unexpected udev device: %s", dev)
-		}
-	})
-	defer restore()
-
-	uuid, err := disks.PartitionUUID("/dev/vda4")
-	c.Assert(err, IsNil)
-	c.Assert(uuid, Equals, "foo-uuid")
-
-	uuid, err = disks.PartitionUUID("/dev/no-uuid")
-	c.Assert(err, ErrorMatches, "cannot get required udev partition UUID property")
-	c.Check(uuid, Equals, "")
-
-	uuid, err = disks.PartitionUUID("/dev/mock-failure")
-	c.Assert(err, ErrorMatches, "cannot process udev properties: mock failure")
-	c.Check(uuid, Equals, "")
+	c.Assert(uuid, Equals, "5a522809-c87e-4dfa-81a8-8dc5667d1304")
 }
 
 func (s *diskSuite) TestFilesystemTypeForPartition(c *C) {
@@ -2048,3 +1988,33 @@ func (s *diskSuite) TestMockDisksChecking(c *C) {
 	}
 	c.Check(f, Panics, "mock error: duplicated kernel device nodes for partitions in disk mapping")
 }
+
+func (s *diskSuite) TestFilesystemUUID(c *C) {
+	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
+		c.Assert(typeOpt, Equals, "--name")
+		switch dev {
+		case "/dev/vda4":
+			return map[string]string{
+				"ID_FS_UUID": "the-expected-uuid",
+			}, nil
+		case "/dev/vda5":
+			return map[string]string{}, nil
+		case "/dev/vda6":
+			return nil, fmt.Errorf("some error")
+		default:
+			c.Errorf("unexpected udev device properties requested: %s", dev)
+			return nil, fmt.Errorf("unexpected udev device: %s", dev)
+		}
+	})
+	defer restore()
+
+	uuid, err := disks.FilesystemUUID("/dev/vda4")
+	c.Assert(err, IsNil)
+	c.Check(uuid, Equals, "the-expected-uuid")
+
+	_, err = disks.FilesystemUUID("/dev/vda5")
+	c.Check(err, ErrorMatches, `cannot get required udev ID_FS_UUID property`)
+
+	_, err = disks.FilesystemUUID("/dev/vda6")
+	c.Check(err, ErrorMatches, `cannot process udev properties: some error`)
+}
diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go
@@ -1643,7 +1643,7 @@ func (s *deviceMgrInstallModeSuite) doRunFactoryResetChange(c *C, model *asserts
 	var chosenBootstrapKey []byte
 	defer devicestate.MockSecbootAddBootstrapKeyOnExistingDisk(func(node string, newKey keys.EncryptionKey) error {
 		if tc.encrypt {
-			c.Check(node, Equals, "/dev/disk/by-partuuid/fbbb94fb-46ea-4e00-b830-afc72d202449")
+			c.Check(node, Equals, "/dev/disk/by-uuid/570faa3d-e3bc-49db-979b-e7814b6bd390")
 			chosenBootstrapKey = newKey
 			return nil
 		}
@@ -1653,7 +1653,7 @@ func (s *deviceMgrInstallModeSuite) doRunFactoryResetChange(c *C, model *asserts
 
 	defer devicestate.MockSecbootRenameOrDeleteKeys(func(node string, renames map[string]string) error {
 		if tc.encrypt {
-			c.Check(node, Equals, "/dev/disk/by-partuuid/fbbb94fb-46ea-4e00-b830-afc72d202449")
+			c.Check(node, Equals, "/dev/disk/by-uuid/570faa3d-e3bc-49db-979b-e7814b6bd390")
 			c.Check(renames, DeepEquals, map[string]string{
 				"default":          "factory-reset-old",
 				"default-fallback": "factory-reset-old-fallback",
@@ -1940,7 +1940,7 @@ echo "mock output of: $(basename "$0") $*"
 
 	defer disks.MockUdevPropertiesForDevice(func(string, string) (map[string]string, error) {
 		return map[string]string{
-			"ID_PART_ENTRY_UUID": "fbbb94fb-46ea-4e00-b830-afc72d202449",
+			"ID_FS_UUID": "570faa3d-e3bc-49db-979b-e7814b6bd390",
 		}, nil
 	})()
 
@@ -2010,7 +2010,7 @@ echo "mock output of: $(basename "$0") $*"
 
 	defer disks.MockUdevPropertiesForDevice(func(string, string) (map[string]string, error) {
 		return map[string]string{
-			"ID_PART_ENTRY_UUID": "fbbb94fb-46ea-4e00-b830-afc72d202449",
+			"ID_FS_UUID": "570faa3d-e3bc-49db-979b-e7814b6bd390",
 		}, nil
 	})()
 

diff --git a/overlord/devicestate/devicestate_test.go b/overlord/devicestate/devicestate_test.go
@@ -44,7 +44,6 @@ import (
 	"github.com/snapcore/snapd/kernel/fde"
 	"github.com/snapcore/snapd/logger"
 	"github.com/snapcore/snapd/osutil"
-	"github.com/snapcore/snapd/osutil/disks"
 	"github.com/snapcore/snapd/overlord"
 	"github.com/snapcore/snapd/overlord/assertstate"
 	"github.com/snapcore/snapd/overlord/assertstate/assertstatetest"
@@ -2199,15 +2198,15 @@ func (s *deviceMgrSuite) TestDeviceManagerEnsurePostFactoryResetEncrypted(c *C)
 	})
 	defer restore()
 
-	restore = devicestate.MockDisksPartitionUUIDFromMountPoint(func(mountpoint string, opts *disks.Options) (string, error) {
+	restore = devicestate.MockDisksDMCryptUUIDFromMountPoint(func(mountpoint string) (string, error) {
 		c.Check(mountpoint, Equals, boot.InitramfsUbuntuSaveDir)
 		return "FOOUUID", nil
 	})
 	defer restore()
 
 	deleteOldSaveKey := 0
 	restore = devicestate.MockSecbootDeleteKeys(func(node string, matches map[string]bool) error {
-		c.Check(node, Equals, "/dev/disk/by-partuuid/FOOUUID")
+		c.Check(node, Equals, "/dev/disk/by-uuid/FOOUUID")
 		c.Check(matches, DeepEquals, map[string]bool{
 			"factory-reset-old":          true,
 			"factory-reset-old-fallback": true,

diff --git a/overlord/devicestate/export_test.go b/overlord/devicestate/export_test.go
@@ -33,7 +33,6 @@ import (
 	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/kernel/fde"
 	"github.com/snapcore/snapd/osutil"
-	"github.com/snapcore/snapd/osutil/disks"
 	"github.com/snapcore/snapd/overlord/snapstate"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/overlord/storecontext"
@@ -628,10 +627,10 @@ func MockSecbootDeleteKeys(f func(node string, matches map[string]bool) error) (
 	}
 }
 
-func MockDisksPartitionUUIDFromMountPoint(f func(mountpoint string, opts *disks.Options) (string, error)) (restore func()) {
-	old := disksPartitionUUIDFromMountPoint
-	disksPartitionUUIDFromMountPoint = f
+func MockDisksDMCryptUUIDFromMountPoint(f func(mountpoint string) (string, error)) (restore func()) {
+	old := disksDMCryptUUIDFromMountPoint
+	disksDMCryptUUIDFromMountPoint = f
 	return func() {
-		disksPartitionUUIDFromMountPoint = old
+		disksDMCryptUUIDFromMountPoint = old
 	}
 }
diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go
@@ -65,6 +65,7 @@ var (
 	bootMakeRunnableStandalone           = boot.MakeRunnableStandaloneSystem
 	bootMakeRunnableAfterDataReset       = boot.MakeRunnableSystemAfterDataReset
 	bootEnsureNextBootToRunMode          = boot.EnsureNextBootToRunMode
+	disksDMCryptUUIDFromMountPoint       = disks.DMCryptUUIDFromMountPoint
 	installRun                           = install.Run
 	installFactoryReset                  = install.FactoryReset
 	installMountVolumes                  = install.MountVolumes
@@ -610,11 +611,11 @@ func (m *DeviceManager) doFactoryResetRunSystem(t *state.Task, _ *tomb.Tomb) err
 			return fmt.Errorf("internal error: no system-save device")
 		}
 
-		uuid, err := disks.PartitionUUID(saveNode)
+		uuid, err := disks.FilesystemUUID(saveNode)
 		if err != nil {
 			return fmt.Errorf("cannot find uuid for partition %s: %v", saveNode, err)
 		}
-		saveNode = fmt.Sprintf("/dev/disk/by-partuuid/%s", uuid)
+		saveNode = fmt.Sprintf("/dev/disk/by-uuid/%s", uuid)
 
 		saveBoostrapContainer, err := createSaveBootstrappedContainer(saveNode)
 		if err != nil {
@@ -1191,7 +1192,6 @@ var (
 	secbootRenameOrDeleteKeys            = secboot.RenameOrDeleteKeys
 	secbootCreateBootstrappedContainer   = secboot.CreateBootstrappedContainer
 	secbootDeleteKeys                    = secboot.DeleteKeys
-	disksPartitionUUIDFromMountPoint     = disks.PartitionUUIDFromMountPoint
 )
 
 func createSaveBootstrappedContainer(saveNode string) (secboot.BootstrappedContainer, error) {
@@ -1242,14 +1242,12 @@ func deleteOldSaveKey(saveMntPnt string) error {
 	// keys.
 
 	// FIXME: maybe there is better if we had a function returning the devname instead.
-	partUUID, err := disksPartitionUUIDFromMountPoint(saveMntPnt, &disks.Options{
-		IsDecryptedDevice: true,
-	})
+	uuid, err := disksDMCryptUUIDFromMountPoint(saveMntPnt)
 	if err != nil {
-		return fmt.Errorf("cannot partition save partition: %v", err)
+		return fmt.Errorf("cannot find save partition: %v", err)
 	}
 
-	diskPath := filepath.Join("/dev/disk/by-partuuid", partUUID)
+	diskPath := filepath.Join("/dev/disk/by-uuid", uuid)
 
 	toDelete := map[string]bool{
 		"factory-reset-old":          true,