secboot: UUID generator error instead of panic

The randutil.RandomKernelUUID() used to generate a panic on failure. The process of consolidating common code used by Pebble and Snapd (e.g. randutil) highlighted the fact that an error return, as relied on by Pebble, is a more flexibie approach and allows application code to decide the severity, instead of the library function. - Make secboot unlock code to deal with an error that can now be returned from the UUID generator. - Add unit tests to cover the new error paths in the code. Signed-off-by: Fred Lotter <[email protected]>
syncloud · May 25, 2023 · 9a5accc · 9a5accc
1 parent 0a488db
commit 9a5accc
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 33 deletions.
diff --git a/randutil/export_test.go b/randutil/export_test.go
@@ -19,12 +19,12 @@
 
 package randutil
 
+import "github.com/snapcore/snapd/testutil"
+
 var KernelUUIDPath = kernelUUIDPath
 
 func MockKernelUUIDPath(newPath string) (restore func()) {
-	kernelUUIDPathDefault := kernelUUIDPath
+	restore = testutil.Backup(&kernelUUIDPath)
 	kernelUUIDPath = newPath
-	return func() {
-		kernelUUIDPath = kernelUUIDPathDefault
-	}
+	return restore
 }
diff --git a/secboot/export_sb_test.go b/secboot/export_sb_test.go
@@ -169,7 +169,7 @@ func MockSbMeasureSnapModelToTPM(f func(tpm *sb_tpm2.Connection, pcrIndex int, m
 	}
 }
 
-func MockRandomKernelUUID(f func() string) (restore func()) {
+func MockRandomKernelUUID(f func() (string, error)) (restore func()) {
 	old := randutilRandomKernelUUID
 	randutilRandomKernelUUID = f
 	return func() {

diff --git a/secboot/secboot_sb.go b/secboot/secboot_sb.go
@@ -94,15 +94,23 @@ func UnlockVolumeUsingSealedKeyIfEncrypted(disk disks.Disk, name string, sealedE
 
 	if !res.IsEncrypted {
 		// if we didn't find an encrypted device just return, don't try to
-		// unlock it
-		// the filesystem device for the unencrypted case is the same as the
-		// partition device
+		// unlock it the filesystem device for the unencrypted case is the
+		// same as the partition device
 		res.PartDevice = partDevice
 		res.FsDevice = res.PartDevice
 		return res, nil
 	}
 
-	mapperName := name + "-" + randutilRandomKernelUUID()
+	uuid, err := randutilRandomKernelUUID()
+	if err != nil {
+		// We failed before we could generate the filsystem device path for
+		// the encrypted partition device, so we return FsDevice empty.
+		res.PartDevice = partDevice
+		return res, err
+	}
+
+	// make up a new name for the mapped device
+	mapperName := name + "-" + uuid
 	sourceDevice := partDevice
 	targetDevice := filepath.Join("/dev/mapper", mapperName)
 
@@ -131,8 +139,16 @@ func UnlockEncryptedVolumeUsingKey(disk disks.Disk, name string, key []byte) (Un
 	// we have a device
 	encdev := filepath.Join("/dev/disk/by-partuuid", partUUID)
 	unlockRes.PartDevice = encdev
+
+	uuid, err := randutilRandomKernelUUID()
+	if err != nil {
+		// We failed before we could generate the filsystem device path for
+		// the encrypted partition device, so we return FsDevice empty.
+		return unlockRes, err
+	}
+
 	// make up a new name for the mapped device
-	mapperName := name + "-" + randutilRandomKernelUUID()
+	mapperName := name + "-" + uuid
 	if err := unlockEncryptedPartitionWithKey(mapperName, encdev, key); err != nil {
 		return unlockRes, err
 	}

diff --git a/secboot/secboot_sb_test.go b/secboot/secboot_sb_test.go
@@ -451,6 +451,7 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncrypted(c *C) {
 		rkErr               error // recovery key unlock error, only relevant if TPM not available
 		activated           bool  // the activation operation succeeded
 		activateErr         error // the activation error
+		uuidFailure         bool  // failure to get valid uuid
 		err                 string
 		skipDiskEnsureCheck bool // whether to check to ensure the mock disk contains the device label
 		expUnlockMethod     secboot.UnlockMethod
@@ -462,6 +463,11 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncrypted(c *C) {
 			activated:       true,
 			disk:            mockDiskWithEncDev,
 			expUnlockMethod: secboot.UnlockedWithSealedKey,
+		}, {
+			// encrypted device: failure to generate uuid based target device name
+			tpmEnabled: true, hasEncdev: true, activated: true, uuidFailure: true,
+			disk: mockDiskWithEncDev,
+			err:  "mocked uuid error",
 		}, {
 			// device activation fails
 			tpmEnabled: true, hasEncdev: true,
@@ -539,13 +545,17 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncrypted(c *C) {
 			err: "error enumerating partitions for disk to find unencrypted device \"name\": filesystem label \"name\" not found",
 		},
 	} {
+		c.Logf("tc %v: %+v", idx, tc)
+
 		randomUUID := fmt.Sprintf("random-uuid-for-test-%d", idx)
-		restore := secboot.MockRandomKernelUUID(func() string {
-			return randomUUID
+		restore := secboot.MockRandomKernelUUID(func() (string, error) {
+			if tc.uuidFailure {
+				return "", errors.New("mocked uuid error")
+			}
+			return randomUUID, nil
 		})
 		defer restore()
 
-		c.Logf("tc %v: %+v", idx, tc)
 		_, restoreConnect := mockSbTPMConnection(c, tc.tpmErr)
 		defer restoreConnect()
 
@@ -1298,6 +1308,28 @@ func (s *secbootSuite) TestUnlockEncryptedVolumeUsingKeyBadDisk(c *C) {
 	c.Check(unlockRes, DeepEquals, secboot.UnlockResult{})
 }
 
+func (s *secbootSuite) TestUnlockEncryptedVolumeUsingKeyUUIDError(c *C) {
+	disk := &disks.MockDiskMapping{
+		Structure: []disks.Partition{
+			{
+				FilesystemLabel: "ubuntu-save-enc",
+				PartitionUUID:   "123-123-123",
+			},
+		},
+	}
+	restore := secboot.MockRandomKernelUUID(func() (string, error) {
+		return "", errors.New("mocked uuid error")
+	})
+	defer restore()
+
+	unlockRes, err := secboot.UnlockEncryptedVolumeUsingKey(disk, "ubuntu-save", []byte("fooo"))
+	c.Assert(err, ErrorMatches, "mocked uuid error")
+	c.Check(unlockRes, DeepEquals, secboot.UnlockResult{
+		PartDevice:  "/dev/disk/by-partuuid/123-123-123",
+		IsEncrypted: true,
+	})
+}
+
 func (s *secbootSuite) TestUnlockEncryptedVolumeUsingKeyHappy(c *C) {
 	disk := &disks.MockDiskMapping{
 		Structure: []disks.Partition{
@@ -1307,8 +1339,8 @@ func (s *secbootSuite) TestUnlockEncryptedVolumeUsingKeyHappy(c *C) {
 			},
 		},
 	}
-	restore := secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-123-123"
+	restore := secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-123-123", nil
 	})
 	defer restore()
 	restore = secboot.MockSbActivateVolumeWithKey(func(volumeName, sourceDevicePath string, key []byte,
@@ -1339,8 +1371,8 @@ func (s *secbootSuite) TestUnlockEncryptedVolumeUsingKeyErr(c *C) {
 			},
 		},
 	}
-	restore := secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-123-123"
+	restore := secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-123-123", nil
 	})
 	defer restore()
 	restore = secboot.MockSbActivateVolumeWithKey(func(volumeName, sourceDevicePath string, key []byte,
@@ -1414,8 +1446,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV1An
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1651,8 +1683,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV2(c
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1712,8 +1744,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV2Mo
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1767,8 +1799,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV2Mo
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1820,8 +1852,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV2Al
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1880,8 +1912,8 @@ func (s *secbootSuite) checkV2Key(c *C, keyFn string, prefixToDrop, expectedKey,
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -1948,8 +1980,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyV1(c
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()
 
@@ -2005,8 +2037,8 @@ func (s *secbootSuite) TestUnlockVolumeUsingSealedKeyIfEncryptedFdeRevealKeyBadJ
 	})
 	defer restore()
 
-	restore = secboot.MockRandomKernelUUID(func() string {
-		return "random-uuid-for-test"
+	restore = secboot.MockRandomKernelUUID(func() (string, error) {
+		return "random-uuid-for-test", nil
 	})
 	defer restore()