Skip to content

Commit

Permalink
many: remove all device-setup fde-setup code (#12618)
Browse files Browse the repository at this point in the history 
* many: remove all `device-setup` fde-setup code

When we initially worked on the inline-cryto-engine (ICE) code we
had a design based on the fde-setup hook that would return a
`device-setup` feature as a hint that ICE should be used. It
turned out this design was impractical and we moved to a much
better approach that got merged in
#12589
and relies on support for ICE in the kernel and cryptsetup.

With that new approach all the code that was supporting the
`device-setup` approach can be removed now.

* daemon: consider again the fde-setup hook case

* client: consider the ICE encryption type

* osu/disks,kernel: remove device-unlock support as well

---------

Co-authored-by: Samuele Pedroni <[email protected]>
  • Loading branch information
@mvo5 @pedronis
mvo5 and pedronis authored Apr 5, 2023
1 parent bd81886 commit ec48421
Show file tree
Hide file tree
Showing 20 changed files with 22 additions and 868 deletions.
6 changes: 3 additions & 3 deletions client/systems.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2020 Canonical Ltd
* Copyright (C) 2020-2023 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
Expand Down Expand Up @@ -160,8 +160,8 @@ type StorageEncryption struct {
// StorageSafety can have values of asserts.StorageSafety
StorageSafety string `json:"storage-safety,omitempty"`

// Type has values of secboot.Type: "", "cryptsetup",
// "device-setup-hook"
// Type has values of secboot.EncryptionType: "", "cryptsetup",
// "cryptsetup-with-inline-crypto-engine"
Type string `json:"encryption-type,omitempty"`

// UnavailableReason describes why the encryption is not
Expand Down
15 changes: 8 additions & 7 deletions daemon/api_debug.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2015-2021 Canonical Ltd
* Copyright (C) 2015-2023 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
Expand Down Expand Up @@ -301,14 +301,15 @@ func getGadgetDiskMapping(st *state.State) Response {
}
// no sealed keys, so no encryption
} else {
// TODO: is there a better way to find the encType
// than indirectly via the sealedKeyMethods? does not
// matter right now because there really is only one
// encryption type
switch sealingMethod {
case device.SealingMethodLegacyTPM, device.SealingMethodTPM:
case device.SealingMethodLegacyTPM, device.SealingMethodTPM, device.SealingMethodFDESetupHook:
// LUKS and LUKS-with-ICE are the same for what is
// required here
encType = secboot.EncryptionTypeLUKS
case device.SealingMethodFDESetupHook:
// TODO:ICE: device setup hook support goes away
// XXX: this also seems to be broken already, this sealing
// method should not imply ICE
encType = secboot.EncryptionTypeDeviceSetupHook
default:
return InternalError("unknown sealing method: %s", sealingMethod)
}
Expand Down
9 changes: 0 additions & 9 deletions gadget/gadget_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3217,15 +3217,6 @@ func (s *gadgetYamlTestSuite) TestLayoutCompatibilityWithLUKSEncryptedPartitions

// unsupported encryption types
invalidEncOptions := &gadget.VolumeCompatibilityOptions{
AssumeCreatablePartitionsCreated: true,
ExpectedStructureEncryption: map[string]gadget.StructureEncryptionParameters{
"Writable": {Method: gadget.EncryptionICE},
},
}
err = gadget.EnsureVolumeCompatibility(gadgetVolume, &deviceLayout, invalidEncOptions)
c.Assert(err, ErrorMatches, `cannot find disk partition /dev/node2 \(starting at 2097152\) in gadget: Inline Crypto Engine encrypted partitions currently unsupported`)

invalidEncOptions = &gadget.VolumeCompatibilityOptions{
AssumeCreatablePartitionsCreated: true,
ExpectedStructureEncryption: map[string]gadget.StructureEncryptionParameters{
"Writable": {Method: "other"},
Expand Down
69 changes: 0 additions & 69 deletions gadget/install/encrypt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@ import (
"fmt"
"os/exec"

"github.com/snapcore/snapd/boot"
"github.com/snapcore/snapd/gadget"
"github.com/snapcore/snapd/kernel/fde"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/osutil/disks"
"github.com/snapcore/snapd/secboot"
"github.com/snapcore/snapd/secboot/keys"
)
Expand Down Expand Up @@ -101,69 +98,3 @@ func cryptsetupClose(name string) error {
}
return nil
}

// encryptedDeviceWithSetupHook represents a block device that is setup using
// the "device-setup" hook.
type encryptedDeviceWithSetupHook struct {
parent *gadget.OnDiskStructure
name string
node string
}

// expected interface is implemented
var _ = encryptedDevice(&encryptedDeviceWithSetupHook{})

// createEncryptedDeviceWithSetupHook creates an encrypted device in the
// existing partition using the specified key using the fde-setup hook
func createEncryptedDeviceWithSetupHook(part *gadget.OnDiskStructure, key keys.EncryptionKey, name string) (encryptedDevice, error) {
// for roles requiring encryption, the filesystem label is always set to
// either the implicit value or a value that has been validated
if part.Name != name || part.PartitionFSLabel != name {
return nil, fmt.Errorf("cannot use partition name %q for an encrypted structure with partition label %q or filesystem label %q",
name, part.Name, part.PartitionFSLabel)
}

// 1. create linear mapper device with 1Mb of reserved space
uuid := ""
offset := fde.DeviceSetupHookPartitionOffset
sizeMinusOffset := uint64(part.Size) - offset
mapperDevice, err := disks.CreateLinearMapperDevice(part.Node, name, uuid, offset, sizeMinusOffset)
if err != nil {
return nil, err
}

// 2. run fde-setup "device-setup" on it
// TODO: We may need a different way to run the fde-setup hook
// here. The hook right now runs with a locked state. But
// when this runs the state will be unlocked but our hook
// mechanism needs a locked state. This means we either need
// something like "boot.RunFDE*Device*SetupHook" or we run
// the entire install with the state locked (which may not
// be as terrible as it sounds as this is a rare situation).
runHook := boot.RunFDESetupHook
params := &fde.DeviceSetupParams{
Key: key,
Device: mapperDevice,
PartitionName: name,
}
if err := fde.DeviceSetup(runHook, params); err != nil {
return nil, err
}

return &encryptedDeviceWithSetupHook{
parent: part,
name: name,
node: mapperDevice,
}, nil
}

func (dev *encryptedDeviceWithSetupHook) Close() error {
if output, err := exec.Command("dmsetup", "remove", dev.name).CombinedOutput(); err != nil {
return osutil.OutputErr(output, err)
}
return nil
}

func (dev *encryptedDeviceWithSetupHook) Node() string {
return dev.node
}
105 changes: 0 additions & 105 deletions gadget/install/encrypt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/snapcore/snapd/gadget"
"github.com/snapcore/snapd/gadget/install"
"github.com/snapcore/snapd/gadget/quantity"
"github.com/snapcore/snapd/kernel/fde"
"github.com/snapcore/snapd/secboot"
"github.com/snapcore/snapd/secboot/keys"
"github.com/snapcore/snapd/testutil"
Expand Down Expand Up @@ -128,107 +127,3 @@ func (s *encryptSuite) TestNewEncryptedDeviceLUKS(c *C) {
})
}
}

var mockDeviceStructureForDeviceSetupHook = gadget.OnDiskStructure{
Name: "ubuntu-data",
PartitionFSLabel: "ubuntu-data",
StartOffset: 0,
Size: 3 * quantity.SizeMiB,
Node: "/dev/node1",
}

func (s *encryptSuite) TestCreateEncryptedDeviceWithSetupHook(c *C) {

for _, tc := range []struct {
mockedOpenErr string
mockedRunFDESetupHookErr error
expectedErr string
}{
{
mockedOpenErr: "",
mockedRunFDESetupHookErr: nil,
expectedErr: "",
},
{
mockedRunFDESetupHookErr: errors.New("fde-setup hook error"),
mockedOpenErr: "",
expectedErr: "device setup failed with: fde-setup hook error",
},

{
mockedOpenErr: "open error",
mockedRunFDESetupHookErr: nil,
expectedErr: `cannot create mapper "ubuntu-data" on /dev/node1: open error`,
},
} {
script := ""
if tc.mockedOpenErr != "" {
script = fmt.Sprintf("echo '%s'>&2; exit 1", tc.mockedOpenErr)

}

restore := install.MockBootRunFDESetupHook(func(req *fde.SetupRequest) ([]byte, error) {
return nil, tc.mockedRunFDESetupHookErr
})
defer restore()

mockDmsetup := testutil.MockCommand(c, "dmsetup", script)
s.AddCleanup(mockDmsetup.Restore)

dev, err := install.CreateEncryptedDeviceWithSetupHook(&mockDeviceStructureForDeviceSetupHook,
s.mockedEncryptionKey, "ubuntu-data")
if tc.expectedErr == "" {
c.Assert(err, IsNil)
} else {
c.Assert(err, ErrorMatches, tc.expectedErr)
continue
}
c.Check(dev.Node(), Equals, "/dev/mapper/ubuntu-data")

err = dev.Close()
c.Assert(err, IsNil)

c.Check(mockDmsetup.Calls(), DeepEquals, [][]string{
// Caculation is in 512 byte blocks. The total
// size of the mock device is 3Mb: 2Mb
// (4096*512) length if left and the offset is
// 1Mb (2048*512) at the start
{"dmsetup", "create", "ubuntu-data", "--table", "0 4096 linear /dev/node1 2048"},
{"dmsetup", "remove", "ubuntu-data"},
})
}
}

func (s *encryptSuite) TestCreateEncryptedDeviceWithSetupHookPartitionNameCheck(c *C) {
mockDeviceStructureBadName := gadget.OnDiskStructure{
Name: "ubuntu-data",
PartitionFSLabel: "ubuntu-data",
StartOffset: 0,
Size: 3 * quantity.SizeMiB,
Node: "/dev/node1",
}
restore := install.MockBootRunFDESetupHook(func(req *fde.SetupRequest) ([]byte, error) {
c.Error("unexpected call")
return nil, fmt.Errorf("unexpected call")
})
defer restore()

mockDmsetup := testutil.MockCommand(c, "dmsetup", `echo "unexpected call" >&2; exit 1`)
s.AddCleanup(mockDmsetup.Restore)

// pass a name that does not match partition name
dev, err := install.CreateEncryptedDeviceWithSetupHook(&mockDeviceStructureBadName,
s.mockedEncryptionKey, "some-name")
c.Assert(err, ErrorMatches, `cannot use partition name "some-name" for an encrypted structure with partition label "ubuntu-data" or filesystem label "ubuntu-data"`)
c.Check(dev, IsNil)
c.Check(mockDmsetup.Calls(), HasLen, 0)
// make structure name different than the label, which is set to either
// the implicit value or has already been validated and matches what is
// expected for the particular role
mockDeviceStructureBadName.Name = "bad-name"
dev, err = install.CreateEncryptedDeviceWithSetupHook(&mockDeviceStructureBadName,
s.mockedEncryptionKey, "bad-name")
c.Assert(err, ErrorMatches, `cannot use partition name "bad-name" for an encrypted structure with partition label "bad-name" or filesystem label "ubuntu-data"`)
c.Check(dev, IsNil)
c.Check(mockDmsetup.Calls(), HasLen, 0)
}
5 changes: 2 additions & 3 deletions gadget/install/export_secboot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import (
)

var (
DiskWithSystemSeed = diskWithSystemSeed
NewEncryptedDeviceLUKS = newEncryptedDeviceLUKS
CreateEncryptedDeviceWithSetupHook = createEncryptedDeviceWithSetupHook
DiskWithSystemSeed = diskWithSystemSeed
NewEncryptedDeviceLUKS = newEncryptedDeviceLUKS
)

func MockSecbootFormatEncryptedDevice(f func(key keys.EncryptionKey, encType secboot.EncryptionType, label, node string) error) (restore func()) {
Expand Down
18 changes: 3 additions & 15 deletions gadget/install/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,8 @@ func maybeEncryptPartition(laidOut *gadget.LaidOutStructure, encryptionType secb
if err != nil {
return nil, nil, err
}

//TODO:ICE: device-setup hook support goes away
case secboot.EncryptionTypeDeviceSetupHook:
timings.Run(perfTimings, fmt.Sprintf("new-encrypted-device-setup-hook[%s]", laidOut.Role()),
fmt.Sprintf("Create encryption device for %s using device-setup-hook", laidOut.Role()),
func(timings.Measurer) {
dataPart, err = createEncryptedDeviceWithSetupHook(&laidOut.OnDiskStructure, encryptionKey, laidOut.Name())
})
if err != nil {
return nil, nil, err
}
default:
return nil, nil, fmt.Errorf("internal error: unknown encryption type: %v", encryptionType)
}

// update the encrypted device node, such that subsequent steps
Expand Down Expand Up @@ -292,12 +283,9 @@ func createEncryptionParams(encTyp secboot.EncryptionType) gadget.StructureEncry
switch encTyp {
case secboot.EncryptionTypeLUKS, secboot.EncryptionTypeLUKSWithICE:
return gadget.StructureEncryptionParameters{
// TODO:ICE: remove "Method" entirely, there is only LUKS
Method: gadget.EncryptionLUKS,
}
case secboot.EncryptionTypeDeviceSetupHook:
return gadget.StructureEncryptionParameters{
Method: gadget.EncryptionICE,
}
}
logger.Noticef("internal error: unknown encryption parameter %q", encTyp)
return gadget.StructureEncryptionParameters{}
Expand Down
5 changes: 0 additions & 5 deletions gadget/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,6 @@ func EnsureVolumeCompatibility(gadgetVolume *Volume, diskVolume *OnDiskVolume, o
}

switch encTypeParams.Method {
case EncryptionICE:
return false, "Inline Crypto Engine encrypted partitions currently unsupported"
case EncryptionLUKS:
// then this partition is expected to have been encrypted, the
// filesystem label on disk will need "-enc" appended
Expand Down Expand Up @@ -587,9 +585,6 @@ const (
// standard LUKS as it is used for automatic FDE using SecureBoot and TPM
// 2.0 in UC20+
EncryptionLUKS DiskEncryptionMethod = "LUKS"

// TODO:ICE: remove this
EncryptionICE DiskEncryptionMethod = "ICE"
)

// DiskVolumeValidationOptions is a set of options on how to validate a disk to
Expand Down
Loading

0 comments on commit ec48421

Please sign in to comment.