Merge pull request #10820 from mvo5/ice/check-encryption-no-longer-bool-2

devicestate: use EncryptionType

8 files changed, 89 insertions, 54 deletions

diff --git a/gadget/install/install.go b/gadget/install/install.go
index 50502fd468..deae449bff 100644
--- a/gadget/install/install.go
+++ b/gadget/install/install.go
@@ -65,9 +65,7 @@ func roleOrLabelOrName(part gadget.OnDiskStructure) string {
 func Run(model gadget.Model, gadgetRoot, kernelRoot, device string, options Options, observer gadget.ContentObserver, perfTimings timings.Measurer) (*InstalledSystemSideData, error) {
	logger.Noticef("installing a new system")
	logger.Noticef(" gadget data from: %v", gadgetRoot)
-	if options.Encrypt {
-	logger.Noticef(" encryption: on")
-	}
+	logger.Noticef(" encryption: %v", options.EncryptionType)
	if gadgetRoot == "" {
	return nil, fmt.Errorf("cannot use empty gadget root directory")
	}
@@ -149,7 +147,8 @@ func Run(model gadget.Model, gadgetRoot, kernelRoot, device string, options Opti
	}
	logger.Noticef("created new partition %v for structure %v (size %v) %s",
	part.Node, part, part.Size.IECString(), roleFmt)
-	if options.Encrypt && roleNeedsEncryption(part.Role) {
+	encrypt := (options.EncryptionType != secboot.EncryptionTypeNone)
+	if encrypt && roleNeedsEncryption(part.Role) {
	var keys *EncryptionKeySet
	timings.Run(perfTimings, fmt.Sprintf("make-key-set[%s]", roleOrLabelOrName(part)), fmt.Sprintf("Create encryption key set for %s", roleOrLabelOrName(part)), func(timings.Measurer) {
	keys, err = makeKeySet()
diff --git a/gadget/install/params.go b/gadget/install/params.go
index a81f2d0c7e..3da142187b 100644
--- a/gadget/install/params.go
+++ b/gadget/install/params.go
@@ -26,8 +26,8 @@ import (
 type Options struct {
	// Also mount the filesystems after creation
	Mount bool
-	// Encrypt the data partition
-	Encrypt bool
+	// Encrypt the data/save partitions
+	EncryptionType secboot.EncryptionType
 }
 
 // EncryptionKeySet is a set of encryption keys.
diff --git a/overlord/devicestate/devicemgr.go b/overlord/devicestate/devicemgr.go
index 3bd3b42779..343112d4c4 100644
--- a/overlord/devicestate/devicemgr.go
+++ b/overlord/devicestate/devicemgr.go
@@ -49,6 +49,7 @@ import (
	"github.com/snapcore/snapd/overlord/storecontext"
	"github.com/snapcore/snapd/progress"
	"github.com/snapcore/snapd/release"
+	"github.com/snapcore/snapd/secboot"
	"github.com/snapcore/snapd/seed"
	"github.com/snapcore/snapd/snap"
	"github.com/snapcore/snapd/snap/snapfile"
@@ -1745,7 +1746,7 @@ func (m *DeviceManager) runFDESetupHook(req *fde.SetupRequest) ([]byte, error) {
	return hookOutput, nil
 }
 
-func (m *DeviceManager) checkFDEFeatures() error {
+func (m *DeviceManager) checkFDEFeatures() (et secboot.EncryptionType, err error) {
	// TODO: move most of this to kernel/fde.Features
	// Run fde-setup hook with "op":"features". If the hook
	// returns any {"features":[...]} reply we consider the
@@ -1756,22 +1757,28 @@ func (m *DeviceManager) checkFDEFeatures() error {
	}
	output, err := m.runFDESetupHook(req)
	if err != nil {
-	return err
+	return et, err
	}
	var res struct {
	Features []string `json:"features"`
	Error string `json:"error"`
	}
	if err := json.Unmarshal(output, &res); err != nil {
-	return fmt.Errorf("cannot parse hook output %q: %v", output, err)
+	return et, fmt.Errorf("cannot parse hook output %q: %v", output, err)
	}
	if res.Features == nil && res.Error == "" {
-	return fmt.Errorf(`cannot use hook: neither "features" nor "error" returned`)
+	return et, fmt.Errorf(`cannot use hook: neither "features" nor "error" returned`)
	}
	if res.Error != "" {
-	return fmt.Errorf("cannot use hook: it returned error: %v", res.Error)
+	return et, fmt.Errorf("cannot use hook: it returned error: %v", res.Error)
	}
-	return nil
+	if strutil.ListContains(res.Features, "device-setup") {
+	et = secboot.EncryptionTypeDeviceSetupHook
+	} else {
+	et = secboot.EncryptionTypeLUKS
+	}
+
+	return et, nil
 }
 
 func hasFDESetupHookInKernel(kernelInfo *snap.Info) bool {
diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go
index 4d50b338b8..de361c4c71 100644
--- a/overlord/devicestate/devicestate_install_mode_test.go
+++ b/overlord/devicestate/devicestate_install_mode_test.go
@@ -322,8 +322,8 @@ func (s *deviceMgrInstallModeSuite) doRunChangeTestWithEncryption(c *C, grade st
	c.Assert(brDevice, Equals, "")
	if tc.encrypt {
	c.Assert(brOpts, DeepEquals, install.Options{
-	Mount: true,
-	Encrypt: true,
+	Mount: true,
+	EncryptionType: secboot.EncryptionTypeLUKS,
	})
	} else {
	c.Assert(brOpts, DeepEquals, install.Options{
@@ -797,7 +797,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallSecuredBypassEncryption(c *C) {
 
 func (s *deviceMgrInstallModeSuite) TestInstallBootloaderVarSetFails(c *C) {
	restore := devicestate.MockInstallRun(func(mod gadget.Model, gadgetRoot, kernelRoot, device string, options install.Options, _ gadget.ContentObserver, _ timings.Measurer) (*install.InstalledSystemSideData, error) {
-	c.Check(options.Encrypt, Equals, false)
+	c.Check(options.EncryptionType, Equals, secboot.EncryptionTypeNone)
	// no keys set
	return &install.InstalledSystemSideData{}, nil
	})
@@ -863,7 +863,7 @@ func (s *deviceMgrInstallModeSuite) testInstallEncryptionSanityChecks(c *C, errM
 
 func (s *deviceMgrInstallModeSuite) TestInstallEncryptionSanityChecksNoKeys(c *C) {
	restore := devicestate.MockInstallRun(func(mod gadget.Model, gadgetRoot, kernelRoot, device string, options install.Options, _ gadget.ContentObserver, _ timings.Measurer) (*install.InstalledSystemSideData, error) {
-	c.Check(options.Encrypt, Equals, true)
+	c.Check(options.EncryptionType, Equals, secboot.EncryptionTypeLUKS)
	// no keys set
	return &install.InstalledSystemSideData{}, nil
	})
@@ -874,7 +874,7 @@ func (s *deviceMgrInstallModeSuite) TestInstallEncryptionSanityChecksNoKeys(c *C
 
 func (s *deviceMgrInstallModeSuite) TestInstallEncryptionSanityChecksNoSystemDataKey(c *C) {
	restore := devicestate.MockInstallRun(func(mod gadget.Model, gadgetRoot, kernelRoot, device string, options install.Options, _ gadget.ContentObserver, _ timings.Measurer) (*install.InstalledSystemSideData, error) {
-	c.Check(options.Encrypt, Equals, true)
+	c.Check(options.EncryptionType, Equals, secboot.EncryptionTypeLUKS)
	// no keys set
	return &install.InstalledSystemSideData{
	// empty map
@@ -1225,21 +1225,25 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncrypted(c *C) {
	deviceCtx := &snapstatetest.TrivialDeviceContext{DeviceModel: mockModel}
 
	for _, tc := range []struct {
-	hasFDESetupHook bool
-	hasTPM bool
-	encrypt bool
+	hasFDESetupHook bool
+	fdeSetupHookFeatures string
+
+	hasTPM bool
+	encryptionType secboot.EncryptionType
	}{
	// unhappy: no tpm, no hook
-	{false, false, false},
+	{false, "[]", false, secboot.EncryptionTypeNone},
	// happy: either tpm or hook or both
-	{false, true, true},
-	{true, false, true},
-	{true, true, true},
+	{false, "[]", true, secboot.EncryptionTypeLUKS},
+	{true, "[]", false, secboot.EncryptionTypeLUKS},
+	{true, "[]", true, secboot.EncryptionTypeLUKS},
+	// happy but device-setup hook
+	{true, `["device-setup"]`, true, secboot.EncryptionTypeDeviceSetupHook},
	} {
	hookInvoke := func(ctx *hookstate.Context, tomb *tomb.Tomb) ([]byte, error) {
	ctx.Lock()
	defer ctx.Unlock()
-	ctx.Set("fde-setup-result", []byte(`{"features":[]}`))
+	ctx.Set("fde-setup-result", []byte(fmt.Sprintf(`{"features":%s}`, tc.fdeSetupHookFeatures)))
	return nil, nil
	}
	rhk := hookstate.MockRunHook(hookInvoke)
@@ -1258,9 +1262,9 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncrypted(c *C) {
	})
	defer restore()
 
-	encrypt, err := devicestate.DeviceManagerCheckEncryption(s.mgr, st, deviceCtx)
+	encryptionType, err := devicestate.DeviceManagerCheckEncryption(s.mgr, st, deviceCtx)
	c.Assert(err, IsNil)
-	c.Check(encrypt, Equals, tc.encrypt, Commentf("%v", tc))
+	c.Check(encryptionType, Equals, tc.encryptionType, Commentf("%v", tc))
	}
 }
 
@@ -1311,8 +1315,9 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncryptedStorageSafety(c *C)
	})
	deviceCtx := &snapstatetest.TrivialDeviceContext{DeviceModel: mockModel}
 
-	encrypt, err := devicestate.DeviceManagerCheckEncryption(s.mgr, s.state, deviceCtx)
+	encryptionType, err := devicestate.DeviceManagerCheckEncryption(s.mgr, s.state, deviceCtx)
	c.Assert(err, IsNil)
+	encrypt := (encryptionType != secboot.EncryptionTypeNone)
	c.Check(encrypt, Equals, tc.expectedEncryption)
	}
 }
@@ -1391,22 +1396,27 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncryptedFDEHook(c *C) {
	for _, tc := range []struct {
	hookOutput string
	expectedErr string
+
+	encryptionType secboot.EncryptionType
	}{
	// invalid json
-	{"xxx", `cannot parse hook output "xxx": invalid character 'x' looking for beginning of value`},
+	{"xxx", `cannot parse hook output "xxx": invalid character 'x' looking for beginning of value`, secboot.EncryptionTypeNone},
	// no output is invalid
-	{"", `cannot parse hook output "": unexpected end of JSON input`},
+	{"", `cannot parse hook output "": unexpected end of JSON input`, secboot.EncryptionTypeNone},
	// specific error
-	{`{"error":"failed"}`, `cannot use hook: it returned error: failed`},
-	{`{}`, `cannot use hook: neither "features" nor "error" returned`},
+	{`{"error":"failed"}`, `cannot use hook: it returned error: failed`, secboot.EncryptionTypeNone},
+	{`{}`, `cannot use hook: neither "features" nor "error" returned`, secboot.EncryptionTypeNone},
	// valid
-	{`{"features":[]}`, ""},
-	{`{"features":["a"]}`, ""},
-	{`{"features":["a","b"]}`, ""},
+	{`{"features":[]}`, "", secboot.EncryptionTypeLUKS},
+	{`{"features":["a"]}`, "", secboot.EncryptionTypeLUKS},
+	{`{"features":["a","b"]}`, "", secboot.EncryptionTypeLUKS},
	// features must be list of strings
-	{`{"features":[1]}`, `cannot parse hook output ".*": json: cannot unmarshal number into Go struct.*`},
-	{`{"features":1}`, `cannot parse hook output ".*": json: cannot unmarshal number into Go struct.*`},
-	{`{"features":"1"}`, `cannot parse hook output ".*": json: cannot unmarshal string into Go struct.*`},
+	{`{"features":[1]}`, `cannot parse hook output ".*": json: cannot unmarshal number into Go struct.*`, secboot.EncryptionTypeNone},
+	{`{"features":1}`, `cannot parse hook output ".*": json: cannot unmarshal number into Go struct.*`, secboot.EncryptionTypeNone},
+	{`{"features":"1"}`, `cannot parse hook output ".*": json: cannot unmarshal string into Go struct.*`, secboot.EncryptionTypeNone},
+	// valid and switches to "device-setup"
+	{`{"features":["device-setup"]}`, "", secboot.EncryptionTypeDeviceSetupHook},
+	{`{"features":["a","device-setup","b"]}`, "", secboot.EncryptionTypeDeviceSetupHook},
	} {
	hookInvoke := func(ctx *hookstate.Context, tomb *tomb.Tomb) ([]byte, error) {
	ctx.Lock()
@@ -1417,11 +1427,12 @@ func (s *deviceMgrInstallModeSuite) TestInstallCheckEncryptedFDEHook(c *C) {
	rhk := hookstate.MockRunHook(hookInvoke)
	defer rhk()
 
-	err := devicestate.DeviceManagerCheckFDEFeatures(s.mgr, st)
+	et, err := devicestate.DeviceManagerCheckFDEFeatures(s.mgr, st)
	if tc.expectedErr != "" {
	c.Check(err, ErrorMatches, tc.expectedErr, Commentf("%v", tc))
	} else {
	c.Check(err, IsNil, Commentf("%v", tc))
+	c.Check(et, Equals, tc.encryptionType, Commentf("%v", tc))
	}
	}
 }
diff --git a/overlord/devicestate/export_test.go b/overlord/devicestate/export_test.go
index 714b0d7649..b6dc9c92ab 100644
--- a/overlord/devicestate/export_test.go
+++ b/overlord/devicestate/export_test.go
@@ -33,6 +33,7 @@ import (
	"github.com/snapcore/snapd/overlord/snapstate"
	"github.com/snapcore/snapd/overlord/state"
	"github.com/snapcore/snapd/overlord/storecontext"
+	"github.com/snapcore/snapd/secboot"
	"github.com/snapcore/snapd/sysconfig"
	"github.com/snapcore/snapd/timings"
 )
@@ -332,10 +333,10 @@ func DeviceManagerRunFDESetupHook(mgr *DeviceManager, req *fde.SetupRequest) ([]
	return mgr.runFDESetupHook(req)
 }
 
-func DeviceManagerCheckEncryption(mgr *DeviceManager, st *state.State, deviceCtx snapstate.DeviceContext) (bool, error) {
+func DeviceManagerCheckEncryption(mgr *DeviceManager, st *state.State, deviceCtx snapstate.DeviceContext) (secboot.EncryptionType, error) {
	return mgr.checkEncryption(st, deviceCtx)
 }
 
-func DeviceManagerCheckFDEFeatures(mgr *DeviceManager, st *state.State) error {
+func DeviceManagerCheckFDEFeatures(mgr *DeviceManager, st *state.State) (secboot.EncryptionType, error) {
	return mgr.checkFDEFeatures()
 }
diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go
index 9f4ee8735e..8a46e098de 100644
--- a/overlord/devicestate/handlers_install.go
+++ b/overlord/devicestate/handlers_install.go
@@ -231,11 +231,12 @@ func (m *DeviceManager) doSetupRunSystem(t *state.Task, _ *tomb.Tomb) error {
	bopts := install.Options{
	Mount: true,
	}
-	useEncryption, err := m.checkEncryption(st, deviceCtx)
+	encryptionType, err := m.checkEncryption(st, deviceCtx)
	if err != nil {
	return err
	}
-	bopts.Encrypt = useEncryption
+	bopts.EncryptionType = encryptionType
+	useEncryption := (encryptionType != secboot.EncryptionTypeNone)
 
	model := deviceCtx.Model()
 
@@ -457,7 +458,7 @@ var secbootCheckTPMKeySealingSupported = secboot.CheckTPMKeySealingSupported
 // checkEncryption verifies whether encryption should be used based on the
 // model grade and the availability of a TPM device or a fde-setup hook
 // in the kernel.
-func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.DeviceContext) (res bool, err error) {
+func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.DeviceContext) (res secboot.EncryptionType, err error) {
	model := deviceCtx.Model()
	secured := model.Grade() == asserts.ModelSecured
	dangerous := model.Grade() == asserts.ModelDangerous
@@ -466,7 +467,7 @@ func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.Dev
	// check if we should disable encryption non-secured devices
	// TODO:UC20: this is not the final mechanism to bypass encryption
	if dangerous && osutil.FileExists(filepath.Join(boot.InitramfsUbuntuSeedDir, ".force-unencrypted")) {
-	return false, nil
+	return res, nil
	}
 
	// check if the model prefers to be unencrypted
@@ -474,7 +475,7 @@ func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.Dev
	// if the install is unencrypted or encrypted
	if model.StorageSafety() == asserts.StorageSafetyPreferUnencrypted {
	logger.Noticef(`installing system unencrypted to comply with prefer-unencrypted storage-safety model option`)
-	return false, nil
+	return res, nil
	}
 
	// check if encryption is available
@@ -484,22 +485,25 @@ func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.Dev
	)
	if kernelInfo, err := snapstate.KernelInfo(st, deviceCtx); err == nil {
	if hasFDESetupHook = hasFDESetupHookInKernel(kernelInfo); hasFDESetupHook {
-	checkEncryptionErr = m.checkFDEFeatures()
+	res, checkEncryptionErr = m.checkFDEFeatures()
	}
	}
	// Note that having a fde-setup hook will disable the build-in
	// secboot encryption
	if !hasFDESetupHook {
	checkEncryptionErr = secbootCheckTPMKeySealingSupported()
+	if checkEncryptionErr == nil {
+	res = secboot.EncryptionTypeLUKS
+	}
	}
 
	// check if encryption is required
	if checkEncryptionErr != nil {
	if secured {
-	return false, fmt.Errorf("cannot encrypt device storage as mandated by model grade secured: %v", checkEncryptionErr)
+	return res, fmt.Errorf("cannot encrypt device storage as mandated by model grade secured: %v", checkEncryptionErr)
	}
	if encrypted {
-	return false, fmt.Errorf("cannot encrypt device storage as mandated by encrypted storage-safety model option: %v", checkEncryptionErr)
+	return res, fmt.Errorf("cannot encrypt device storage as mandated by encrypted storage-safety model option: %v", checkEncryptionErr)
	}
 
	if hasFDESetupHook {
@@ -509,11 +513,10 @@ func (m *DeviceManager) checkEncryption(st *state.State, deviceCtx snapstate.Dev
	}
 
	// not required, go without
-	return false, nil
+	return res, nil
	}
 
-	// encrypt
-	return true, nil
+	return res, nil
 }
 
 // RebootOptions can be attached to restart-system-to-run-mode tasks to control
diff --git a/secboot/encrypt.go b/secboot/encrypt.go
index 02eab315e6..0f30fb2011 100644
--- a/secboot/encrypt.go
+++ b/secboot/encrypt.go
@@ -116,3 +116,12 @@ func NewAuxKey() (AuxKey, error) {
	// On return, n == len(b) if and only if err == nil
	return key, err
 }
+
+// EncryptionType specifies what encryption backend should be used (if any)
+type EncryptionType string
+
+const (
+	EncryptionTypeNone EncryptionType = ""
+	EncryptionTypeLUKS EncryptionType = "cryptsetup"
+	EncryptionTypeDeviceSetupHook EncryptionType = "device-setup-hook"
+)
diff --git a/tests/lib/uc20-create-partitions/main.go b/tests/lib/uc20-create-partitions/main.go
index 57584db7c8..9079255b75 100644
--- a/tests/lib/uc20-create-partitions/main.go
+++ b/tests/lib/uc20-create-partitions/main.go
@@ -67,9 +67,14 @@ func main() {
 
	obs := &simpleObserver{}
 
+	var encryptionType secboot.EncryptionType
+	if args.Encrypt {
+	encryptionType = secboot.EncryptionTypeLUKS
+	}
+
	options := install.Options{
-	Mount: args.Mount,
-	Encrypt: args.Encrypt,
+	Mount: args.Mount,
+	EncryptionType: encryptionType,
	}
	installSideData, err := installRun(uc20Constraints{}, args.Positional.GadgetRoot, args.Positional.KernelRoot, args.Positional.Device, options, obs, timings.New(nil))
	if err != nil {