Skip to content

Commit

Permalink
secboot: look for unlock key in kernel keyring first (canonical#14977)
Browse files Browse the repository at this point in the history
There was a bug where the protector key file for the plainkey
of the ubuntu-save partition was incorrectly used as the unlock
key, the fix is to first check the unlock key from the keyring
if it is not there, then probably the key file is for the unlock
key and no plainkey is used.

Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>

* fixup! secboot: use default-fallback to be consistent with reality

Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>

---------

Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
  • Loading branch information
ZeyadYasser authored and pedronis committed Feb 7, 2025
1 parent a1c1781 commit 92724ce
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 7 deletions.
15 changes: 8 additions & 7 deletions secboot/encrypt_sb.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package secboot
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -165,18 +166,18 @@ func EnsureRecoveryKey(keyFile string, rkeyDevs []RecoveryKeyDevice) (keys.Recov

for _, device := range newDevices {
var unlockKey []byte
if device.keyFile != "" {
const defaultPrefix = "ubuntu-fde"
// always check keyring first for unlock key, otherwise fallback to using key file
key, err := sbGetDiskUnlockKeyFromKernel(defaultPrefix, device.node, false)
if errors.Is(err, sb.ErrKernelKeyNotFound) && device.keyFile != "" {
key, err := os.ReadFile(device.keyFile)
if err != nil {
return keys.RecoveryKey{}, fmt.Errorf("cannot get key from '%s': %v", device.keyFile, err)
return keys.RecoveryKey{}, fmt.Errorf("cannot get key from kernel keyring or %q: %v", device.keyFile, err)
}
unlockKey = key
} else if err != nil {
return keys.RecoveryKey{}, fmt.Errorf("cannot get key for unlocked disk: %v", err)
} else {
const defaultPrefix = "ubuntu-fde"
key, err := sbGetDiskUnlockKeyFromKernel(defaultPrefix, device.node, false)
if err != nil {
return keys.RecoveryKey{}, fmt.Errorf("cannot get key for unlocked disk: %v", err)
}
unlockKey = key
}

Expand Down
36 changes: 36 additions & 0 deletions secboot/encrypt_sb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,9 @@ func (s *keymgrSuite) TestEnsureRecoveryKey(c *C) {
defer secboot.MockListLUKS2ContainerRecoveryKeyNames(func(devicePath string) ([]string, error) {
return []string{}, nil
})()
keyringCalled := 0
defer secboot.MockGetDiskUnlockKeyFromKernel(func(prefix string, devicePath string, remove bool) (sb.DiskUnlockKey, error) {
keyringCalled++
return []byte{1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4}, nil
})()
defer secboot.MockAddLUKS2ContainerRecoveryKey(func(devicePath string, keyslotName string, existingKey sb.DiskUnlockKey, recoveryKey sb.RecoveryKey) error {
Expand All @@ -365,6 +367,8 @@ func (s *keymgrSuite) TestEnsureRecoveryKey(c *C) {
{Mountpoint: "/bar", AuthorizingKeyFile: keyFilePath},
})
c.Assert(err, IsNil)
// Make sure that keyring is checked first for the unlock keys
c.Check(keyringCalled, Equals, 2)
c.Check(udevadmCmd.Calls(), DeepEquals, [][]string{
{"udevadm", "info", "--query", "property", "--name", "/dev/mapper/foo"},
{"udevadm", "info", "--query", "property", "--name", "/dev/mapper/bar"},
Expand All @@ -388,13 +392,45 @@ func (s *keymgrSuite) TestEnsureRecoveryKey(c *C) {
{Mountpoint: "/bar", AuthorizingKeyFile: keyFilePath},
})
c.Assert(err, IsNil)
// Make sure that keyring is checked first for the unlock keys
c.Check(keyringCalled, Equals, 4)

recovery, err := os.ReadFile(filepath.Join(s.d, "recovery.key"))
c.Assert(err, IsNil)

c.Check(recovery, DeepEquals, originalRecovery)
}

func (s *keymgrSuite) TestEnsureRecoveryKeyFallback(c *C) {
s.mocksForDeviceMounts(c)

defer secboot.MockListLUKS2ContainerUnlockKeyNames(func(devicePath string) ([]string, error) {
return []string{"default-fallback"}, nil
})()
defer secboot.MockListLUKS2ContainerRecoveryKeyNames(func(devicePath string) ([]string, error) {
return []string{}, nil
})()
keyringCalled := 0
defer secboot.MockGetDiskUnlockKeyFromKernel(func(prefix string, devicePath string, remove bool) (sb.DiskUnlockKey, error) {
keyringCalled++
return nil, sb.ErrKernelKeyNotFound
})()
defer secboot.MockAddLUKS2ContainerRecoveryKey(func(devicePath string, keyslotName string, existingKey sb.DiskUnlockKey, recoveryKey sb.RecoveryKey) error {
// Verify unlock key directly came from keyfile
c.Assert(existingKey, DeepEquals, sb.DiskUnlockKey([]byte{9, 8, 7, 1, 2, 3}))
return nil
})()

keyFilePath := filepath.Join(c.MkDir(), "key.file")
err := os.WriteFile(keyFilePath, []byte{9, 8, 7, 1, 2, 3}, 0644)
c.Assert(err, IsNil)
_, err = secboot.EnsureRecoveryKey(filepath.Join(s.d, "recovery.key"), []secboot.RecoveryKeyDevice{
{Mountpoint: "/bar", AuthorizingKeyFile: keyFilePath},
})
c.Assert(err, IsNil)
c.Check(keyringCalled, Equals, 1)
}

func (s *keymgrSuite) TestEnsureRecoveryKeyLegacy(c *C) {
udevadmCmd := s.mocksForDeviceMounts(c)

Expand Down

0 comments on commit 92724ce

Please sign in to comment.