diff --git a/cmd/snap-fde-keymgr/export_test.go b/cmd/snap-fde-keymgr/export_test.go index a08a308ba7e..e8dc63bc4ac 100644 --- a/cmd/snap-fde-keymgr/export_test.go +++ b/cmd/snap-fde-keymgr/export_test.go @@ -51,9 +51,15 @@ func MockRemoveRecoveryKeyFromLUKSUsingKey(f func(key keys.EncryptionKey, dev st return restore } -func MockChangeLUKSEncryptionKey(f func(newKey keys.EncryptionKey, dev string) error) (restore func()) { - restore = testutil.Backup(&keymgrChangeLUKSDeviceEncryptionKey) - keymgrChangeLUKSDeviceEncryptionKey = f +func MockStageLUKSEncryptionKeyChange(f func(newKey keys.EncryptionKey, dev string) error) (restore func()) { + restore = testutil.Backup(&keymgrStageLUKSDeviceEncryptionKeyChange) + keymgrStageLUKSDeviceEncryptionKeyChange = f + return restore +} + +func MockTransitionLUKSEncryptionKeyChange(f func(newKey keys.EncryptionKey, dev string) error) (restore func()) { + restore = testutil.Backup(&keymgrTransitionLUKSDeviceEncryptionKeyChange) + keymgrTransitionLUKSDeviceEncryptionKeyChange = f return restore } diff --git a/cmd/snap-fde-keymgr/main.go b/cmd/snap-fde-keymgr/main.go index 934e3468d71..5b176b611d2 100644 --- a/cmd/snap-fde-keymgr/main.go +++ b/cmd/snap-fde-keymgr/main.go @@ -52,7 +52,9 @@ type cmdRemoveRecoveryKey struct { } type cmdChangeEncryptionKey struct { - Device string `long:"device" description:"encrypted device" required:"yes"` + Device string `long:"device" description:"encrypted device" required:"yes"` + Stage bool `long:"stage" description:"stage the new key"` + Transition bool `long:"transition" description:"replace the old key, unstage the new"` } type options struct { @@ -66,7 +68,8 @@ var ( keymgrAddRecoveryKeyToLUKSDeviceUsingKey = keymgr.AddRecoveryKeyToLUKSDeviceUsingKey keymgrRemoveRecoveryKeyFromLUKSDevice = keymgr.RemoveRecoveryKeyFromLUKSDevice keymgrRemoveRecoveryKeyFromLUKSDeviceUsingKey = keymgr.RemoveRecoveryKeyFromLUKSDeviceUsingKey - keymgrChangeLUKSDeviceEncryptionKey = keymgr.ChangeLUKSDeviceEncryptionKey + keymgrStageLUKSDeviceEncryptionKeyChange = keymgr.StageLUKSDeviceEncryptionKeyChange + keymgrTransitionLUKSDeviceEncryptionKeyChange = keymgr.TransitionLUKSDeviceEncryptionKeyChange ) func validateAuthorizations(authorizations []string) error { @@ -201,13 +204,32 @@ type newKey struct { } func (c *cmdChangeEncryptionKey) Execute(args []string) error { + if c.Stage && c.Transition { + return fmt.Errorf("cannot both stage and transition the encryption key change") + } + if !c.Stage && !c.Transition { + return fmt.Errorf("cannot change encryption key without stage or transition request") + } + var newEncryptionKeyData newKey dec := json.NewDecoder(osStdin) if err := dec.Decode(&newEncryptionKeyData); err != nil { return fmt.Errorf("cannot obtain new encryption key: %v", err) } - if err := keymgrChangeLUKSDeviceEncryptionKey(newEncryptionKeyData.Key, c.Device); err != nil { - return fmt.Errorf("cannot change LUKS device encryption key: %v", err) + switch { + case c.Stage: + // staging the key change authorizes the operation using a key + // from the keyring + if err := keymgrStageLUKSDeviceEncryptionKeyChange(newEncryptionKeyData.Key, c.Device); err != nil { + return fmt.Errorf("cannot stage LUKS device encryption key change: %v", err) + } + case c.Transition: + // transitioning the key change authorizes the operation using + // the currently provided key (which must have been staged + // before hence the op will be authorized successfully) + if err := keymgrTransitionLUKSDeviceEncryptionKeyChange(newEncryptionKeyData.Key, c.Device); err != nil { + return fmt.Errorf("cannot transition LUKS device encryption key change: %v", err) + } } return nil } diff --git a/cmd/snap-fde-keymgr/main_test.go b/cmd/snap-fde-keymgr/main_test.go index 3ceefa8ab0f..191091d770a 100644 --- a/cmd/snap-fde-keymgr/main_test.go +++ b/cmd/snap-fde-keymgr/main_test.go @@ -338,28 +338,115 @@ func (s *mainSuite) TestRemoveKeyRequiresAuthz(c *C) { c.Assert(err, ErrorMatches, `cannot remove recovery keys with invalid authorizations: authorization file .*/authz.key does not exist`) } +// 1 in ASCII repeated 32 times +const all1sKey = `{"key":"MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE="}` + func (s *mainSuite) TestChangeEncryptionKey(c *C) { - const all1sKey = `{"key":"MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE="}` + b := bytes.NewBufferString(all1sKey) + restore := main.MockOsStdin(b) + defer restore() + unexpectedCall := func(newKey keys.EncryptionKey, luksDev string) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + } + defer main.MockStageLUKSEncryptionKeyChange(unexpectedCall) + defer main.MockTransitionLUKSEncryptionKeyChange(unexpectedCall) + + err := main.Run([]string{ + "change-encryption-key", + "--device", "/dev/vda4", + }) + c.Assert(err, ErrorMatches, "cannot change encryption key without stage or transition request") + + err = main.Run([]string{ + "change-encryption-key", + "--device", "/dev/vda4", + "--stage", "--transition", + }) + c.Assert(err, ErrorMatches, "cannot both stage and transition the encryption key change") +} + +func (s *mainSuite) TestStageEncryptionKey(c *C) { b := bytes.NewBufferString(all1sKey) restore := main.MockOsStdin(b) defer restore() dev := "" - changeCalls := 0 + stageCalls := 0 var key []byte - restore = main.MockChangeLUKSEncryptionKey(func(newKey keys.EncryptionKey, luksDev string) error { - changeCalls++ + var stageErr error + restore = main.MockStageLUKSEncryptionKeyChange(func(newKey keys.EncryptionKey, luksDev string) error { + stageCalls++ dev = luksDev key = newKey - return nil + return stageErr + }) + defer restore() + restore = main.MockTransitionLUKSEncryptionKeyChange(func(newKey keys.EncryptionKey, luksDev string) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") }) defer restore() err := main.Run([]string{ "change-encryption-key", "--device", "/dev/vda4", + "--stage", }) c.Assert(err, IsNil) - c.Check(changeCalls, Equals, 1) + c.Check(stageCalls, Equals, 1) c.Check(dev, Equals, "/dev/vda4") // secboot encryption key size c.Check(key, DeepEquals, bytes.Repeat([]byte("1"), 32)) + + restore = main.MockOsStdin(bytes.NewBufferString(all1sKey)) + defer restore() + stageErr = fmt.Errorf("mock stage error") + err = main.Run([]string{ + "change-encryption-key", + "--device", "/dev/vda4", + "--stage", + }) + c.Assert(err, ErrorMatches, "cannot stage LUKS device encryption key change: mock stage error") +} + +func (s *mainSuite) TestTransitionEncryptionKey(c *C) { + b := bytes.NewBufferString(all1sKey) + restore := main.MockOsStdin(b) + defer restore() + dev := "" + transitionCalls := 0 + var key []byte + var transitionErr error + restore = main.MockStageLUKSEncryptionKeyChange(func(newKey keys.EncryptionKey, luksDev string) error { + c.Errorf("unexpected call") + return fmt.Errorf("unexpected call") + }) + defer restore() + restore = main.MockTransitionLUKSEncryptionKeyChange(func(newKey keys.EncryptionKey, luksDev string) error { + transitionCalls++ + dev = luksDev + key = newKey + return transitionErr + }) + defer restore() + defer restore() + err := main.Run([]string{ + "change-encryption-key", + "--device", "/dev/vda4", + "--transition", + }) + c.Assert(err, IsNil) + c.Check(transitionCalls, Equals, 1) + c.Check(dev, Equals, "/dev/vda4") + // secboot encryption key size + c.Check(key, DeepEquals, bytes.Repeat([]byte("1"), 32)) + + restore = main.MockOsStdin(bytes.NewBufferString(all1sKey)) + defer restore() + transitionErr = fmt.Errorf("mock transition error") + err = main.Run([]string{ + "change-encryption-key", + "--device", "/dev/vda4", + "--transition", + }) + c.Assert(err, ErrorMatches, "cannot transition LUKS device encryption key change: mock transition error") } diff --git a/cmd/snap/cmd_quota.go b/cmd/snap/cmd_quota.go index 09bc3400420..e661abdae48 100644 --- a/cmd/snap/cmd_quota.go +++ b/cmd/snap/cmd_quota.go @@ -165,23 +165,19 @@ func parseCpuQuota(cpuMax string) (count int, percentage int, err error) { return count, percentage, nil } -func parseQuotas(maxMemory string, cpuMax string, cpuSet string, threadsMax string) (*client.QuotaValues, error) { - var mem int64 - var cpuCount int - var cpuPercentage int - var cpus []int - var threads int - - if maxMemory != "" { - value, err := strutil.ParseByteSize(maxMemory) +func (x *cmdSetQuota) parseQuotas() (*client.QuotaValues, error) { + var quotaValues client.QuotaValues + + if x.MemoryMax != "" { + value, err := strutil.ParseByteSize(x.MemoryMax) if err != nil { return nil, err } - mem = value + quotaValues.Memory = quantity.Size(value) } - if cpuMax != "" { - countValue, percentageValue, err := parseCpuQuota(cpuMax) + if x.CPUMax != "" { + countValue, percentageValue, err := parseCpuQuota(x.CPUMax) if err != nil { return nil, err } @@ -189,12 +185,15 @@ func parseQuotas(maxMemory string, cpuMax string, cpuSet string, threadsMax stri return nil, fmt.Errorf("cannot use value %v: cpu quota percentage must be between 1 and 100", percentageValue) } - cpuCount = countValue - cpuPercentage = percentageValue + quotaValues.CPU = &client.QuotaCPUValues{ + Count: countValue, + Percentage: percentageValue, + } } - if cpuSet != "" { - cpuTokens := strutil.CommaSeparatedList(cpuSet) + if x.CPUSet != "" { + var cpus []int + cpuTokens := strutil.CommaSeparatedList(x.CPUSet) for _, cpuToken := range cpuTokens { cpu, err := strconv.ParseUint(cpuToken, 10, 32) if err != nil { @@ -202,31 +201,29 @@ func parseQuotas(maxMemory string, cpuMax string, cpuSet string, threadsMax stri } cpus = append(cpus, int(cpu)) } + + quotaValues.CPUSet = &client.QuotaCPUSetValues{ + CPUs: cpus, + } } - if threadsMax != "" { - value, err := strconv.ParseUint(threadsMax, 10, 32) + if x.ThreadsMax != "" { + value, err := strconv.ParseUint(x.ThreadsMax, 10, 32) if err != nil { - return nil, fmt.Errorf("cannot use threads value %q", threadsMax) + return nil, fmt.Errorf("cannot use threads value %q", x.ThreadsMax) } - threads = int(value) + quotaValues.Threads = int(value) } - return &client.QuotaValues{ - Memory: quantity.Size(mem), - CPU: &client.QuotaCPUValues{ - Count: cpuCount, - Percentage: cpuPercentage, - }, - CPUSet: &client.QuotaCPUSetValues{ - CPUs: cpus, - }, - Threads: threads, - }, nil + return "aValues, nil +} + +func (x *cmdSetQuota) hasQuotaSet() bool { + return x.MemoryMax != "" || x.CPUMax != "" || x.CPUSet != "" || x.ThreadsMax != "" } func (x *cmdSetQuota) Execute(args []string) (err error) { - quotaProvided := x.MemoryMax != "" || x.CPUMax != "" || x.CPUSet != "" || x.ThreadsMax != "" + quotaProvided := x.hasQuotaSet() names := installedSnapNames(x.Positional.Snaps) @@ -267,7 +264,7 @@ func (x *cmdSetQuota) Execute(args []string) (err error) { // we have a limits to set for this group, so specify that along // with whatever snaps may have been provided and whatever parent may // have been specified - quotaValues, err := parseQuotas(x.MemoryMax, x.CPUMax, x.CPUSet, x.ThreadsMax) + quotaValues, err := x.parseQuotas() if err != nil { return err } diff --git a/cmd/snap/cmd_quota_test.go b/cmd/snap/cmd_quota_test.go index 6e0eec0cad3..9170599e09d 100644 --- a/cmd/snap/cmd_quota_test.go +++ b/cmd/snap/cmd_quota_test.go @@ -212,11 +212,11 @@ func (s *quotaSuite) TestParseQuotas(c *check.C) { quotas string err string }{ - {maxMemory: "12KB", quotas: `{"memory":12000,"cpu":{},"cpu-set":{}}`}, - {cpuMax: "12x40%", quotas: `{"cpu":{"count":12,"percentage":40},"cpu-set":{}}`}, - {cpuMax: "40%", quotas: `{"cpu":{"percentage":40},"cpu-set":{}}`}, - {cpuSet: "1,3", quotas: `{"cpu":{},"cpu-set":{"cpus":[1,3]}}`}, - {threadsMax: "2", quotas: `{"cpu":{},"cpu-set":{},"threads":2}`}, + {maxMemory: "12KB", quotas: `{"memory":12000}`}, + {cpuMax: "12x40%", quotas: `{"cpu":{"count":12,"percentage":40}}`}, + {cpuMax: "40%", quotas: `{"cpu":{"percentage":40}}`}, + {cpuSet: "1,3", quotas: `{"cpu-set":{"cpus":[1,3]}}`}, + {threadsMax: "2", quotas: `{"threads":2}`}, // Error cases {cpuMax: "ASD", err: `cannot parse cpu quota string "ASD"`}, {cpuMax: "0x100%", err: `cannot parse cpu quota string "0x100%"`}, @@ -230,7 +230,7 @@ func (s *quotaSuite) TestParseQuotas(c *check.C) { {threadsMax: "xxx", err: `cannot use threads value "xxx"`}, {threadsMax: "-3", err: `cannot use threads value "-3"`}, } { - quotas, err := main.ParseQuotas(testData.maxMemory, testData.cpuMax, testData.cpuSet, testData.threadsMax) + quotas, err := main.ParseQuotaValues(testData.maxMemory, testData.cpuMax, testData.cpuSet, testData.threadsMax) testLabel := check.Commentf("%v", testData) if testData.err == "" { c.Check(err, check.IsNil, testLabel) diff --git a/cmd/snap/export_test.go b/cmd/snap/export_test.go index 868ea267b0e..0f72ddf5f5b 100644 --- a/cmd/snap/export_test.go +++ b/cmd/snap/export_test.go @@ -95,8 +95,6 @@ var ( IsStopping = isStopping GetSnapDirOptions = getSnapDirOptions - - ParseQuotas = parseQuotas ) func HiddenCmd(descr string, completeHidden bool) *cmdInfo { @@ -459,3 +457,14 @@ func MockAutostartSessionApps(f func(string) error) func() { autostartSessionApps = old } } + +func ParseQuotaValues(maxMemory, cpuMax, cpuSet, threadsMax string) (*client.QuotaValues, error) { + var quotas cmdSetQuota + + quotas.MemoryMax = maxMemory + quotas.CPUMax = cpuMax + quotas.CPUSet = cpuSet + quotas.ThreadsMax = threadsMax + + return quotas.parseQuotas() +} diff --git a/overlord/ifacestate/export_test.go b/overlord/ifacestate/export_test.go index 9f73e79a11a..c7c2d9f9d44 100644 --- a/overlord/ifacestate/export_test.go +++ b/overlord/ifacestate/export_test.go @@ -62,6 +62,7 @@ var ( BatchConnectTasks = batchConnectTasks FirstTaskAfterBootWhenPreseeding = firstTaskAfterBootWhenPreseeding + BuildConfinementOptions = buildConfinementOptions ) type ConnectOpts = connectOpts diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index e23209e1d6b..dd2d01e123e 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -22,6 +22,7 @@ package ifacestate import ( "errors" "fmt" + "path" "reflect" "sort" "strings" @@ -29,26 +30,68 @@ import ( "gopkg.in/tomb.v2" + "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/i18n" "github.com/snapcore/snapd/interfaces" "github.com/snapcore/snapd/interfaces/hotplug" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/overlord/hookstate" + "github.com/snapcore/snapd/overlord/servicestate" "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/overlord/state" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/quota" "github.com/snapcore/snapd/timings" ) var snapstateFinishRestart = snapstate.FinishRestart -// confinementOptions returns interfaces.ConfinementOptions from snapstate.Flags. -func confinementOptions(flags snapstate.Flags) interfaces.ConfinementOptions { - return interfaces.ConfinementOptions{ - DevMode: flags.DevMode, - JailMode: flags.JailMode, - Classic: flags.Classic, +// journalQuotaLayout returns the necessary journal quota mount layouts +// to mimick what systemd does for services with log namespaces. +func journalQuotaLayout(quotaGroup *quota.Group) []snap.Layout { + if quotaGroup.JournalLimit == nil { + return nil + } + + // bind mount the journal namespace folder on top of the journal folder + // /etc/systemd/journal. -> /etc/systemd/journal + layouts := []snap.Layout{{ + Bind: path.Join(dirs.SnapSystemdDir, fmt.Sprintf("journal.snap-%s", quotaGroup.Name)), + Path: path.Join(dirs.SnapSystemdDir, "journal"), + Mode: 0755, + }} + return layouts +} + +// getExtraLayouts helper function to dynamically calculate the extra mount layouts for +// a snap instance. These are the layouts which can change during the lifetime of a snap +// like for instance mimicking systemd journal namespace mount layouts. +func getExtraLayouts(st *state.State, snapInstanceName string) ([]snap.Layout, error) { + snapOpts, err := servicestate.SnapServiceOptions(st, snapInstanceName, nil) + if err != nil { + return nil, err } + + var extraLayouts []snap.Layout + if snapOpts.QuotaGroup != nil { + extraLayouts = append(extraLayouts, journalQuotaLayout(snapOpts.QuotaGroup)...) + } + + return extraLayouts, nil +} + +func buildConfinementOptions(st *state.State, snapInstanceName string, flags snapstate.Flags) (interfaces.ConfinementOptions, error) { + extraLayouts, err := getExtraLayouts(st, snapInstanceName) + if err != nil { + return interfaces.ConfinementOptions{}, fmt.Errorf("cannot get extra mount layouts of snap %q: %s", snapInstanceName, err) + } + + return interfaces.ConfinementOptions{ + DevMode: flags.DevMode, + JailMode: flags.JailMode, + Classic: flags.Classic, + ExtraLayouts: extraLayouts, + }, nil } func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap string, affectedSnaps []string, tm timings.Measurer) error { @@ -72,7 +115,10 @@ func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap st if err := addImplicitSlots(st, affectedSnapInfo); err != nil { return err } - opts := confinementOptions(snapst.Flags) + opts, err := buildConfinementOptions(st, affectedSnapInfo.InstanceName(), snapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, affectedSnapInfo, opts, tm); err != nil { return err } @@ -115,7 +161,10 @@ func (m *InterfaceManager) doSetupProfiles(task *state.Task, tomb *tomb.Tomb) er return nil } - opts := confinementOptions(snapsup.Flags) + opts, err := buildConfinementOptions(task.State(), snapInfo.InstanceName(), snapsup.Flags) + if err != nil { + return err + } return m.setupProfilesForSnap(task, tomb, snapInfo, opts, perfTimings) } @@ -205,8 +254,13 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb, if err := addImplicitSlots(st, snapInfo); err != nil { return err } + opts, err := buildConfinementOptions(st, snapInfo.InstanceName(), snapst.Flags) + if err != nil { + return err + } + affectedSnaps = append(affectedSnaps, snapInfo) - confinementOpts = append(confinementOpts, confinementOptions(snapst.Flags)) + confinementOpts = append(confinementOpts, opts) } return m.setupSecurityByBackend(task, affectedSnaps, confinementOpts, tm) @@ -297,7 +351,10 @@ func (m *InterfaceManager) undoSetupProfiles(task *state.Task, tomb *tomb.Tomb) if err != nil { return err } - opts := confinementOptions(snapst.Flags) + opts, err := buildConfinementOptions(task.State(), snapInfo.InstanceName(), snapst.Flags) + if err != nil { + return err + } return m.setupProfilesForSnap(task, tomb, snapInfo, opts, perfTimings) } } @@ -499,12 +556,18 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) (err error) }() if !delayedSetupProfiles { - slotOpts := confinementOptions(slotSnapst.Flags) + slotOpts, err := buildConfinementOptions(st, slotSnapst.InstanceName(), slotSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, slot.Snap, slotOpts, perfTimings); err != nil { return err } - plugOpts := confinementOptions(plugSnapst.Flags) + plugOpts, err := buildConfinementOptions(st, plugSnapst.InstanceName(), plugSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, plug.Snap, plugOpts, perfTimings); err != nil { return err } @@ -605,7 +668,10 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { if err != nil { return err } - opts := confinementOptions(snapst.Flags) + opts, err := buildConfinementOptions(st, snapInfo.InstanceName(), snapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, snapInfo, opts, perfTimings); err != nil { return err } @@ -711,11 +777,18 @@ func (m *InterfaceManager) undoDisconnect(task *state.Task, _ *tomb.Tomb) error return err } - slotOpts := confinementOptions(slotSnapst.Flags) + slotOpts, err := buildConfinementOptions(st, slotSnapst.InstanceName(), slotSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, slot.Snap, slotOpts, perfTimings); err != nil { return err } - plugOpts := confinementOptions(plugSnapst.Flags) + + plugOpts, err := buildConfinementOptions(st, plugSnapst.InstanceName(), plugSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, plug.Snap, plugOpts, perfTimings); err != nil { return err } @@ -794,11 +867,19 @@ func (m *InterfaceManager) undoConnect(task *state.Task, _ *tomb.Tomb) error { if err != nil { return err } - slotOpts := confinementOptions(slotSnapst.Flags) + + slotOpts, err := buildConfinementOptions(st, slotSnapst.InstanceName(), slotSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, slot.Snap, slotOpts, perfTimings); err != nil { return err } - plugOpts := confinementOptions(plugSnapst.Flags) + + plugOpts, err := buildConfinementOptions(st, plugSnapst.InstanceName(), plugSnapst.Flags) + if err != nil { + return err + } if err := m.setupSnapSecurity(task, plug.Snap, plugOpts, perfTimings); err != nil { return err } diff --git a/overlord/ifacestate/handlers_test.go b/overlord/ifacestate/handlers_test.go index b87ba029832..59c8869299b 100644 --- a/overlord/ifacestate/handlers_test.go +++ b/overlord/ifacestate/handlers_test.go @@ -20,25 +20,49 @@ package ifacestate_test import ( + "path" + . "gopkg.in/check.v1" + "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/overlord/configstate/config" "github.com/snapcore/snapd/overlord/ifacestate" + "github.com/snapcore/snapd/overlord/servicestate/servicestatetest" + "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/overlord/state" + "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/quota" + "github.com/snapcore/snapd/snap/snaptest" ) -type handlersSuite struct{} +const snapAyaml = `name: snap-a +type: app +base: base-snap-a +` + +type handlersSuite struct { + st *state.State +} var _ = Suite(&handlersSuite{}) +func (s *handlersSuite) SetUpTest(c *C) { + s.st = state.New(nil) + dirs.SetRootDir(c.MkDir()) +} + +func (s *handlersSuite) TearDownTest(c *C) { + dirs.SetRootDir("") +} + func (s *handlersSuite) TestInSameChangeWaitChain(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() + s.st.Lock() + defer s.st.Unlock() // no wait chain (yet) - startT := st.NewTask("start", "...start") - intermediateT := st.NewTask("intermediateT", "...intermediateT") - searchT := st.NewTask("searchT", "...searchT") + startT := s.st.NewTask("start", "...start") + intermediateT := s.st.NewTask("intermediateT", "...intermediateT") + searchT := s.st.NewTask("searchT", "...searchT") c.Check(ifacestate.InSameChangeWaitChain(startT, searchT), Equals, false) // add (indirect) wait chain @@ -48,16 +72,15 @@ func (s *handlersSuite) TestInSameChangeWaitChain(c *C) { } func (s *handlersSuite) TestInSameChangeWaitChainDifferentChanges(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() + s.st.Lock() + defer s.st.Unlock() - t1 := st.NewTask("t1", "...") - chg1 := st.NewChange("chg1", "...") + t1 := s.st.NewTask("t1", "...") + chg1 := s.st.NewChange("chg1", "...") chg1.AddTask(t1) - t2 := st.NewTask("t2", "...") - chg2 := st.NewChange("chg2", "...") + t2 := s.st.NewTask("t2", "...") + chg2 := s.st.NewChange("chg2", "...") chg2.AddTask(t2) // add a cross change wait chain @@ -66,23 +89,79 @@ func (s *handlersSuite) TestInSameChangeWaitChainDifferentChanges(c *C) { } func (s *handlersSuite) TestInSameChangeWaitChainWithCycles(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() + s.st.Lock() + defer s.st.Unlock() // cycles like this are unexpected in practice but are easier to test than // the exponential paths situation that e.g. seed changes present. - startT := st.NewTask("start", "...start") - task1 := st.NewTask("task1", "...") + startT := s.st.NewTask("start", "...start") + task1 := s.st.NewTask("task1", "...") task1.WaitFor(startT) - task2 := st.NewTask("task2", "...") + task2 := s.st.NewTask("task2", "...") task2.WaitFor(task1) - task3 := st.NewTask("task3", "...") + task3 := s.st.NewTask("task3", "...") task3.WaitFor(task2) startT.WaitFor(task2) startT.WaitFor(task3) - unrelated := st.NewTask("unrelated", "...") + unrelated := s.st.NewTask("unrelated", "...") c.Check(ifacestate.InSameChangeWaitChain(startT, unrelated), Equals, false) } + +func mockInstalledSnap(c *C, st *state.State, snapYaml string) *snap.Info { + snapInfo := snaptest.MockSnap(c, snapYaml, &snap.SideInfo{ + Revision: snap.R(1), + }) + + snapName := snapInfo.SnapName() + si := &snap.SideInfo{RealName: snapName, SnapID: snapName + "-id", Revision: snap.R(1)} + snapstate.Set(st, snapName, &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: si.Revision, + SnapType: string(snapInfo.Type()), + }) + return snapInfo +} + +func (s *handlersSuite) TestBuildConfinementOptions(c *C) { + s.st.Lock() + defer s.st.Unlock() + + snapInfo := mockInstalledSnap(c, s.st, snapAyaml) + flags := snapstate.Flags{} + opts, err := ifacestate.BuildConfinementOptions(s.st, snapInfo.InstanceName(), snapstate.Flags{}) + + c.Check(err, IsNil) + c.Check(len(opts.ExtraLayouts), Equals, 0) + c.Check(opts.Classic, Equals, flags.Classic) + c.Check(opts.DevMode, Equals, flags.DevMode) + c.Check(opts.JailMode, Equals, flags.JailMode) +} + +func (s *handlersSuite) TestBuildConfinementOptionsWithLogNamespace(c *C) { + s.st.Lock() + defer s.st.Unlock() + + tr := config.NewTransaction(s.st) + tr.Set("core", "experimental.quota-groups", true) + tr.Commit() + + snapInfo := mockInstalledSnap(c, s.st, snapAyaml) + + // Create a new quota group with a journal quota + err := servicestatetest.MockQuotaInState(s.st, "foo", "", []string{snapInfo.InstanceName()}, quota.NewResourcesBuilder().WithJournalNamespace().Build()) + c.Assert(err, IsNil) + + flags := snapstate.Flags{} + opts, err := ifacestate.BuildConfinementOptions(s.st, snapInfo.InstanceName(), snapstate.Flags{}) + + c.Check(err, IsNil) + c.Assert(len(opts.ExtraLayouts), Equals, 1) + c.Check(opts.ExtraLayouts[0].Bind, Equals, path.Join(dirs.SnapSystemdDir, "journal.snap-foo")) + c.Check(opts.ExtraLayouts[0].Path, Equals, path.Join(dirs.SnapSystemdDir, "journal")) + c.Check(opts.Classic, Equals, flags.Classic) + c.Check(opts.DevMode, Equals, flags.DevMode) + c.Check(opts.JailMode, Equals, flags.JailMode) +} diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go index 97fa6db882a..ec9a4a55a2e 100644 --- a/overlord/ifacestate/helpers.go +++ b/overlord/ifacestate/helpers.go @@ -190,7 +190,11 @@ func (m *InterfaceManager) regenerateAllSecurityProfiles(tm timings.Measurer) er if err := snapstate.Get(m.state, snapName, &snapst); err != nil { logger.Noticef("cannot get state of snap %q: %s", snapName, err) } - return confinementOptions(snapst.Flags) + opts, err := buildConfinementOptions(m.state, snapst.InstanceName(), snapst.Flags) + if err != nil { + logger.Noticef("cannot get confinement options for snap %q: %s", snapName, err) + } + return opts } // For each backend: diff --git a/secboot/keymgr/export_test.go b/secboot/keymgr/export_test.go index 2607f5658cc..95802e732b2 100644 --- a/secboot/keymgr/export_test.go +++ b/secboot/keymgr/export_test.go @@ -30,10 +30,4 @@ func MockGetDiskUnlockKeyFromKernel(f func(prefix, devicePath string, remove boo return restore } -func MockAddKeyToUserKeyring(f func(key []byte, devicePath, purpose, prefix string) error) (restore func()) { - restore = testutil.Backup(&keyringAddKeyToUserKeyring) - keyringAddKeyToUserKeyring = f - return restore -} - var RecoveryKDF = recoveryKDF diff --git a/secboot/keymgr/keymgr_luks2.go b/secboot/keymgr/keymgr_luks2.go index 74a5c2d2933..8dc4d2989d1 100644 --- a/secboot/keymgr/keymgr_luks2.go +++ b/secboot/keymgr/keymgr_luks2.go @@ -27,7 +27,6 @@ import ( sb "github.com/snapcore/secboot" "github.com/snapcore/snapd/osutil" - "github.com/snapcore/snapd/secboot/keyring" "github.com/snapcore/snapd/secboot/keys" "github.com/snapcore/snapd/secboot/luks2" ) @@ -43,7 +42,6 @@ const ( var ( sbGetDiskUnlockKeyFromKernel = sb.GetDiskUnlockKeyFromKernel - keyringAddKeyToUserKeyring = keyring.AddKeyToUserKeyring ) func getEncryptionKeyFromUserKeyring(dev string) ([]byte, error) { @@ -59,7 +57,9 @@ func getEncryptionKeyFromUserKeyring(dev string) ([]byte, error) { return currKey, err } -var keyslotFull = regexp.MustCompile(`^.* cryptsetup failed with: Key slot [0-9]+ is full, please select another one\.$`) +// TODO rather than inspecting the error messages, parse the LUKS2 headers + +var keyslotFull = regexp.MustCompile(`^.*cryptsetup failed with: Key slot [0-9]+ is full, please select another one\.$`) // IsKeyslotAlreadyUsed returns true if the error indicates that the keyslot // attempted for a given key is already used @@ -160,26 +160,24 @@ func RemoveRecoveryKeyFromLUKSDeviceUsingKey(currKey keys.EncryptionKey, dev str return nil } -// ChangeLUKSDeviceEncryptionKey changes the main encryption key of the device. -// Uses an existing unlock key of that device, which is present in the kernel -// user keyring. Once complete the user keyring contains the new encryption key. -func ChangeLUKSDeviceEncryptionKey(newKey keys.EncryptionKey, dev string) error { +// StageLUKSDeviceEncryptionKeyChange stages a new encryption key with the goal +// of changing the main encryption key referenced in keyslot 0. The operation is +// authorized using the key that unlocked the device and is stored in the +// keyring (as it happens during factory reset). +func StageLUKSDeviceEncryptionKeyChange(newKey keys.EncryptionKey, dev string) error { if len(newKey) != keys.EncryptionKeySize { return fmt.Errorf("cannot use a key of size different than %v", keys.EncryptionKeySize) } + // the key to authorize the device is in the keyring currKey, err := getEncryptionKeyFromUserKeyring(dev) if err != nil { return err } - // we only have the current key, we cannot add a key to an occupied - // keyslot, and cannot start with killing its keyslot as that would make - // the device unusable, so instead add the new key to an auxiliary - // keyslot, then use the new key to authorize removal of keyslot 0 - // (which refers to the old key), add the new key again, but this time - // to keyslot 0, lastly kill the aux keyslot + // TODO rather than inspecting the errors, parse the LUKS2 headers + // free up the temp slot if err := luks2.KillSlot(dev, tempKeySlot, currKey); err != nil { if !isKeyslotNotActive(err) { return fmt.Errorf("cannot kill the temporary keyslot: %v", err) @@ -193,35 +191,87 @@ func ChangeLUKSDeviceEncryptionKey(newKey keys.EncryptionKey, dev string) error if err := luks2.AddKey(dev, currKey[:], newKey, &options); err != nil { return fmt.Errorf("cannot add temporary key: %v", err) } + return nil +} - // now it should be possible to kill the original keyslot by using the - // new key for authorization +// TransitionLUKSDeviceEncryptionKeyChange completes the main encryption key +// change to the new key provided in the parameters. The new key must have been +// staged before, thus it can authorize LUKS operations. Lastly, the unlock key +// in the keyring is updated to the new key. +func TransitionLUKSDeviceEncryptionKeyChange(newKey keys.EncryptionKey, dev string) error { + if len(newKey) != keys.EncryptionKeySize { + return fmt.Errorf("cannot use a key of size different than %v", keys.EncryptionKeySize) + } + + // the expected state is as follows: + // key slot 0 - the old encryption key + // key slot 2 - the new encryption key (added during --stage) + // the desired state is: + // key slot 0 - the new encryption key + // key slot 2 - empty + // it is possible that the system was rebooted right after key slot 0 was + // populated with the new key and key slot 2 was emptied + + // there is no state information on disk which would tell if the + // scenario 1 above occurred and to which stage it was executed, but we + // need to find out if key slot 2 is in use (as the caller believes that + // a key was staged earlier); do this indirectly by trying to add a key + // to key slot 2 + + // TODO rather than inspecting the errors, parse the LUKS2 headers + + tempKeyslotAlreadyUsed := true + + options := luks2.AddKeyOptions{ + KDFOptions: luks2.KDFOptions{TargetDuration: 100 * time.Millisecond}, + Slot: tempKeySlot, + } + err := luks2.AddKey(dev, newKey, newKey, &options) + if err == nil { + // key slot is not in use, so we are dealing with unexpected reboot scenario + tempKeyslotAlreadyUsed = false + } else if err != nil && !IsKeyslotAlreadyUsed(err) { + return fmt.Errorf("cannot add new encryption key: %v", err) + } + + if !tempKeyslotAlreadyUsed { + // since the key slot was not used, it means that the transition + // was already carried out (since it got authorized by the new + // key), so now all is needed is to remove the added key + if err := luks2.KillSlot(dev, tempKeySlot, newKey); err != nil { + return fmt.Errorf("cannot kill temporary key slot: %v", err) + } + + return nil + } + + // first kill the main encryption key slot, authorize the operation + // using the new key which must have been added to the temp keyslot in + // the stage operation if err := luks2.KillSlot(dev, encryptionKeySlot, newKey); err != nil { if !isKeyslotNotActive(err) { - return fmt.Errorf("cannot kill existing slot: %v", err) + return fmt.Errorf("cannot kill the encryption key slot: %v", err) } } - options.Slot = encryptionKeySlot - // add the new key to keyslot 0 + + options = luks2.AddKeyOptions{ + KDFOptions: luks2.KDFOptions{TargetDuration: 100 * time.Millisecond}, + Slot: encryptionKeySlot, + } if err := luks2.AddKey(dev, newKey, newKey, &options); err != nil { - return fmt.Errorf("cannot add key: %v", err) + return fmt.Errorf("cannot add new encryption key: %v", err) } - // and kill the aux slot + + // now it should be possible to kill the temporary keyslot by using the + // new key for authorization if err := luks2.KillSlot(dev, tempKeySlot, newKey); err != nil { - return fmt.Errorf("cannot kill temporary key slot: %v", err) + if !isKeyslotNotActive(err) { + return fmt.Errorf("cannot kill temporary key slot: %v", err) + } } // TODO needed? if err := luks2.SetSlotPriority(dev, encryptionKeySlot, luks2.SlotPriorityHigh); err != nil { - return fmt.Errorf("cannot change keyslot priority: %v", err) - } - - const keyringPurposeDiskUnlock = "unlock" - const keyringPrefix = "ubuntu-fde" - // TODO: make the key permanent in the keyring, investigate why timeout - // is set to a very large number of weeks, but not tagged as perm in - // /proc/keys - if err := keyringAddKeyToUserKeyring(newKey, dev, keyringPurposeDiskUnlock, keyringPrefix); err != nil { - return fmt.Errorf("cannot add key to user keyring: %v", err) + return fmt.Errorf("cannot change key slot priority: %v", err) } return nil } diff --git a/secboot/keymgr/keymgr_luks2_test.go b/secboot/keymgr/keymgr_luks2_test.go index a84afc5c8b0..31072e74fd3 100644 --- a/secboot/keymgr/keymgr_luks2_test.go +++ b/secboot/keymgr/keymgr_luks2_test.go @@ -327,7 +327,7 @@ done c.Assert(filepath.Join(s.rootDir, "unlock.key"), testutil.FileEquals, key) } -func (s *keymgrSuite) TestChangeEncryptionKeyHappy(c *C) { +func (s *keymgrSuite) TestStageEncryptionKeyHappy(c *C) { unlockKey := "1234abcd" getCalls := 0 restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { @@ -339,17 +339,6 @@ func (s *keymgrSuite) TestChangeEncryptionKeyHappy(c *C) { }) defer restore() - addCalls := 0 - restore = keymgr.MockAddKeyToUserKeyring(func(key []byte, devicePath, purpose, prefix string) error { - addCalls++ - c.Assert(key, DeepEquals, bytes.Repeat([]byte{1}, 32)) - c.Check(devicePath, Equals, "/dev/foobar") - c.Check(purpose, Equals, "unlock") - c.Check(prefix, Equals, "ubuntu-fde") - return nil - }) - defer restore() - cmd := testutil.MockCommand(c, "cryptsetup", ` while [ "$#" -gt 1 ]; do case "$1" in @@ -366,16 +355,15 @@ done defer cmd.Restore() // try a too short key key := bytes.Repeat([]byte{1}, 12) - err := keymgr.ChangeLUKSDeviceEncryptionKey(key, "/dev/foobar") + err := keymgr.StageLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") c.Assert(err, ErrorMatches, "cannot use a key of size different than 32") key = bytes.Repeat([]byte{1}, 32) - err = keymgr.ChangeLUKSDeviceEncryptionKey(key, "/dev/foobar") + err = keymgr.StageLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") c.Assert(err, IsNil) c.Assert(getCalls, Equals, 1) - c.Assert(addCalls, Equals, 1) calls := cmd.Calls() - c.Assert(calls, HasLen, 6) + c.Assert(calls, HasLen, 2) c.Assert(calls[0], DeepEquals, []string{ "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", }) @@ -391,32 +379,61 @@ done "--key-slot", "2", "/dev/foobar", "-", }) - c.Assert(calls[2], DeepEquals, []string{ - "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "0", +} + +func (s *keymgrSuite) TestStageEncryptionKeyKilledSlotAlreadyEmpty(c *C) { + unlockKey := "1234abcd" + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + return []byte(unlockKey), nil }) - c.Assert(calls[3], HasLen, 14) - c.Assert(calls[3][5], testutil.Contains, dirs.RunDir) - calls[3][5] = "" - // actual new key - c.Assert(calls[3], DeepEquals, []string{ + defer restore() + + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + luksKillSlot) + killslot=1 + shift + ;; + --key-file) + cat "$2" > /dev/null + shift 2 + ;; + *) + shift + ;; + esac +done +if [ "$killslot" = "1" ]; then + echo "Keyslot 1 is not active." >&2 + exit 1 +fi +`) + defer cmd.Restore() + + key := bytes.Repeat([]byte{1}, 32) + err := keymgr.StageLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, IsNil) + calls := cmd.Calls() + c.Assert(calls, HasLen, 2) + c.Assert(calls[0], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", + }) + c.Assert(calls[1], HasLen, 14) + c.Assert(calls[1][5], testutil.Contains, dirs.RunDir) + calls[1][5] = "" + // temporary key + c.Assert(calls[1], DeepEquals, []string{ "cryptsetup", "luksAddKey", "--type", "luks2", "--key-file", "", "--pbkdf", "argon2i", "--iter-time", "100", - "--key-slot", "0", + "--key-slot", "2", "/dev/foobar", "-", }) - // kill the temp key - c.Assert(calls[4], DeepEquals, []string{ - "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", - }) - // set priority - c.Assert(calls[5], DeepEquals, []string{ - "cryptsetup", "config", "--priority", "prefer", "--key-slot", "0", "/dev/foobar", - }) } -func (s *keymgrSuite) TestChangeEncryptionKeyGetUnlockFail(c *C) { +func (s *keymgrSuite) TestStageEncryptionKeyGetUnlockFail(c *C) { getCalls := 0 restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { getCalls++ @@ -425,46 +442,73 @@ func (s *keymgrSuite) TestChangeEncryptionKeyGetUnlockFail(c *C) { }) defer restore() - restore = keymgr.MockAddKeyToUserKeyring(func(key []byte, devicePath, purpose, prefix string) error { - c.Fatalf("unexpected call") - return fmt.Errorf("unexpected call") - }) - defer restore() - key := bytes.Repeat([]byte{1}, 32) - err := keymgr.ChangeLUKSDeviceEncryptionKey(key, "/dev/foobar") + err := keymgr.StageLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") c.Assert(err, ErrorMatches, "cannot obtain current unlock key for /dev/foobar: cannot find key in kernel keyring") c.Assert(s.cryptsetupCmd.Calls(), HasLen, 0) } -func (s *keymgrSuite) TestChangeEncryptionKeyAddKeyringFails(c *C) { +func (s *keymgrSuite) TestChangeEncryptionTempKeyFails(c *C) { unlockKey := "1234abcd" getCalls := 0 restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { getCalls++ - c.Check(devicePath, Equals, "/dev/foobar") - c.Check(remove, Equals, false) - c.Check(prefix, Equals, "ubuntu-fde") return []byte(unlockKey), nil }) defer restore() - addCalls := 0 - restore = keymgr.MockAddKeyToUserKeyring(func(key []byte, devicePath, purpose, prefix string) error { - addCalls++ - c.Assert(key, DeepEquals, bytes.Repeat([]byte{1}, 32)) - c.Check(devicePath, Equals, "/dev/foobar") - c.Check(purpose, Equals, "unlock") - c.Check(prefix, Equals, "ubuntu-fde") - return fmt.Errorf("add keyring fails") + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + --key-file) + cat "$2" > /dev/null + shift + ;; + luksAddKey) + add=1 + ;; + --key-slot) + if [ "$add" = "1" ] && [ "$2" = "2" ]; then + echo "mock failure" >&2 + exit 1 + fi + ;; + *) + ;; + esac + shift +done +`) + defer cmd.Restore() + + key := bytes.Repeat([]byte{1}, 32) + err := keymgr.StageLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot add temporary key: cryptsetup failed with: mock failure") + c.Assert(getCalls, Equals, 1) + calls := cmd.Calls() + c.Assert(calls, HasLen, 2) +} + +func (s *keymgrSuite) TestTransitionEncryptionKeyExpectedHappy(c *C) { + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") }) defer restore() cmd := testutil.MockCommand(c, "cryptsetup", ` while [ "$#" -gt 1 ]; do case "$1" in + luksAddKey) + keyadd=1 + shift + ;; + --key-slot) + keyslot="$2" + shift 2 + ;; --key-file) - cat "$2" + cat "$2" > /dev/null shift 2 ;; *) @@ -472,29 +516,265 @@ while [ "$#" -gt 1 ]; do ;; esac done +if [ "$keyadd" = "1" ] && [ "$keyslot" = "2" ]; then + echo "Key slot 2 is full, please select another one." >&2 + exit 1 +fi `) defer cmd.Restore() + // try a too short key + key := bytes.Repeat([]byte{1}, 12) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot use a key of size different than 32") + + key = bytes.Repeat([]byte{1}, 32) + err = keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, IsNil) + calls := cmd.Calls() + c.Assert(calls, HasLen, 5) + // probing the key slot use + c.Assert(calls[0], HasLen, 14) + c.Assert(calls[0][5], testutil.Contains, dirs.RunDir) + calls[0][5] = "" + // temporary key + c.Assert(calls[0], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "2", + "/dev/foobar", "-", + }) + // killing the encryption key + c.Assert(calls[1], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "0", + }) + // adding the new encryption key + c.Assert(calls[2], HasLen, 14) + c.Assert(calls[2][5], testutil.Contains, dirs.RunDir) + calls[2][5] = "" + c.Assert(calls[2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "0", + "/dev/foobar", "-", + }) + // kill the temp key + c.Assert(calls[3], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", + }) + // set priority + c.Assert(calls[4], DeepEquals, []string{ + "cryptsetup", "config", "--priority", "prefer", "--key-slot", "0", "/dev/foobar", + }) +} + +func (s *keymgrSuite) TestTransitionEncryptionKeyHappyKillSlotsInactive(c *C) { + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") + }) + defer restore() + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + luksKillSlot) + killslot=1 + shift + ;; + luksAddKey) + keyadd=1 + shift + ;; + --key-slot) + keyslot="$2" + shift 2 + ;; + --key-file) + cat "$2" > /dev/null + shift 2 + ;; + *) + shift 1 + ;; + esac +done +if [ "$keyadd" = "1" ] && [ "$keyslot" = "2" ]; then + echo "Key slot 2 is full, please select another one." >&2 + exit 1 +elif [ "$killslot" = "1" ]; then + echo "Keyslot 2 is not active." >&2 + exit 1 +fi +`) + defer cmd.Restore() + // try a too short key + key := bytes.Repeat([]byte{1}, 12) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot use a key of size different than 32") + + key = bytes.Repeat([]byte{1}, 32) + err = keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, IsNil) + calls := cmd.Calls() + c.Assert(calls, HasLen, 5) + // probing the key slot use + c.Assert(calls[0], HasLen, 14) + // temporary key + c.Assert(calls[0][:2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", + }) + // killing the encryption key + c.Assert(calls[1], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "0", + }) + // adding the new encryption key + c.Assert(calls[2], HasLen, 14) + c.Assert(calls[2][5], testutil.Contains, dirs.RunDir) + calls[2][5] = "" + c.Assert(calls[2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "0", + "/dev/foobar", "-", + }) + // kill the temp key + c.Assert(calls[3], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", + }) + // set priority + c.Assert(calls[4], DeepEquals, []string{ + "cryptsetup", "config", "--priority", "prefer", "--key-slot", "0", "/dev/foobar", + }) +} + +func (s *keymgrSuite) TestTransitionEncryptionKeyHappyOtherErrs(c *C) { + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") + }) + defer restore() + + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + luksAddKey) + keyadd=1 + shift + ;; + --key-slot) + keyslot="$2" + shift 2 + ;; + --key-file) + cat "$2" > /dev/null + shift 2 + ;; + *) + shift 1 + ;; + esac +done +if [ "$keyadd" = "1" ] && [ "$keyslot" = "2" ]; then + echo "Key slot 2 is full, please select another one." >&2 + exit 1 +elif [ "$keyadd" = "1" ] && [ "$keyslot" = "0" ]; then + echo "mock error" >&2 + exit 1 +fi +`) + defer cmd.Restore() key := bytes.Repeat([]byte{1}, 32) - err := keymgr.ChangeLUKSDeviceEncryptionKey(key, "/dev/foobar") - c.Assert(err, ErrorMatches, "cannot add key to user keyring: add keyring fails") - c.Assert(getCalls, Equals, 1) - c.Assert(addCalls, Equals, 1) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot add new encryption key: cryptsetup failed with: mock error") calls := cmd.Calls() - c.Assert(calls, HasLen, 6) + c.Assert(calls, HasLen, 3) + // probing the key slot use + c.Assert(calls[0], HasLen, 14) + // temporary key + c.Assert(calls[0][:2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", + }) + // killing the encryption key + c.Assert(calls[1], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "0", + }) + // adding the new encryption key + c.Assert(calls[2], HasLen, 14) + c.Assert(calls[2][5], testutil.Contains, dirs.RunDir) + calls[2][5] = "" + c.Assert(calls[2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "0", + "/dev/foobar", "-", + }) } -func (s *keymgrSuite) TestChangeEncryptionTempKeyFails(c *C) { - unlockKey := "1234abcd" - getCalls := 0 +func (s *keymgrSuite) TestTransitionEncryptionKeyCannotAddKeyNotStaged(c *C) { + // conditions like when the encryption key has not been previously + // staged + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { - getCalls++ - return []byte(unlockKey), nil + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") }) defer restore() - restore = keymgr.MockAddKeyToUserKeyring(func(key []byte, devicePath, purpose, prefix string) error { - c.Fatalf("unexpected call") - return fmt.Errorf("add keyring fails") + + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + luksAddKey) + keyadd=1 + shift + ;; + --key-file) + cat "$2" > /dev/null + shift 2 + ;; + *) + shift 1 + ;; + esac +done +if [ "$keyadd" = "1" ] ; then + echo "No key available with this passphrase." >&2 + exit 1 +fi +`) + defer cmd.Restore() + + key := bytes.Repeat([]byte{1}, 32) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot add new encryption key: cryptsetup failed with: No key available with this passphrase.") + calls := cmd.Calls() + c.Assert(calls, HasLen, 1) + // probing the key slot use + c.Assert(calls[0], HasLen, 14) + c.Assert(calls[0][5], testutil.Contains, dirs.RunDir) + calls[0][5] = "" + // temporary key + c.Assert(calls[0], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "2", + "/dev/foobar", "-", + }) +} + +func (s *keymgrSuite) TestTransitionEncryptionKeyPostRebootHappy(c *C) { + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") }) defer restore() @@ -503,31 +783,85 @@ while [ "$#" -gt 1 ]; do case "$1" in --key-file) cat "$2" > /dev/null + shift 2 + ;; + *) shift ;; - luksAddKey) - add=1 + esac +done +`) + defer cmd.Restore() + + key := bytes.Repeat([]byte{1}, 32) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, IsNil) + calls := cmd.Calls() + c.Assert(calls, HasLen, 2) + c.Assert(calls[0], HasLen, 14) + c.Assert(calls[0][5], testutil.Contains, dirs.RunDir) + calls[0][5] = "" + // adding to a temporary key slot is successful, indicating a previously + // successful transition + c.Assert(calls[0], DeepEquals, []string{ + "cryptsetup", "luksAddKey", "--type", "luks2", + "--key-file", "", + "--pbkdf", "argon2i", + "--iter-time", "100", + "--key-slot", "2", + "/dev/foobar", "-", + }) + // an a cleanup of the temp key slot + c.Assert(calls[1], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", + }) +} + +func (s *keymgrSuite) TestTransitionEncryptionKeyPostRebootCannotKillSlot(c *C) { + // a post reboot scenario in which the luksKillSlot fails unexpectedly + + restore := keymgr.MockGetDiskUnlockKeyFromKernel(func(prefix, devicePath string, remove bool) (sb.DiskUnlockKey, error) { + c.Errorf("unepected call") + return nil, fmt.Errorf("unexpected call") + }) + defer restore() + + cmd := testutil.MockCommand(c, "cryptsetup", ` +while [ "$#" -gt 1 ]; do + case "$1" in + luksKillSlot) + killslot=1 + shift ;; - --key-slot) - if [ "$add" = "1" ] && [ "$2" = "2" ]; then - echo "mock failure" >&2 - exit 1 - fi + --key-file) + cat "$2" > /dev/null + shift 2 ;; *) + shift ;; esac - shift done +if [ "$killslot" = "1" ]; then + echo "mock error" >&2 + exit 5 +fi `) defer cmd.Restore() key := bytes.Repeat([]byte{1}, 32) - err := keymgr.ChangeLUKSDeviceEncryptionKey(key, "/dev/foobar") - c.Assert(err, ErrorMatches, "cannot add temporary key: cryptsetup failed with: mock failure") - c.Assert(getCalls, Equals, 1) + err := keymgr.TransitionLUKSDeviceEncryptionKeyChange(key, "/dev/foobar") + c.Assert(err, ErrorMatches, "cannot kill temporary key slot: cryptsetup failed with: mock error") calls := cmd.Calls() c.Assert(calls, HasLen, 2) + c.Assert(calls[0], HasLen, 14) + c.Assert(calls[0][:2], DeepEquals, []string{ + "cryptsetup", "luksAddKey", + }) + // an a cleanup of the temp key slot + c.Assert(calls[1], DeepEquals, []string{ + "cryptsetup", "luksKillSlot", "--type", "luks2", "--key-file", "-", "/dev/foobar", "2", + }) } func (s *keymgrSuite) TestRecoveryKDF(c *C) { diff --git a/tests/main/uc20-create-partitions-encrypt/task.yaml b/tests/main/uc20-create-partitions-encrypt/task.yaml index 801fa21a0b3..e5a10a393af 100644 --- a/tests/main/uc20-create-partitions-encrypt/task.yaml +++ b/tests/main/uc20-create-partitions-encrypt/task.yaml @@ -71,6 +71,11 @@ restore: | losetup -d "$LOOP" losetup -l | NOMATCH "$LOOP" fi + + # purge the key we added + systemd-run --pipe --wait --collect -p KeyringMode=inherit -- \ + /usr/bin/keyctl purge -s user "ubuntu-fde:${LOOP}p4:unlock" || true + apt autoremove -y cryptsetup rm -Rf /run/mnt @@ -79,6 +84,11 @@ debug: | cat /proc/partitions execute: | + channel=20 + if os.query is-jammy; then + channel=22 + fi + # this test simulates a reinstall, to clear the TPM this requires # a reboot so the losetup has to be redone if [ "$SPREAD_REBOOT" = 1 ]; then @@ -126,8 +136,13 @@ execute: | sfdisk -d "$LOOP" | MATCH "^${LOOP}p1 .*size=\s*2048, type=21686148-6449-6E6F-744E-656564454649,.*BIOS Boot" sfdisk -d "$LOOP" | MATCH "^${LOOP}p2 .*size=\s*2457600, type=C12A7328-F81F-11D2-BA4B-00A0C93EC93B,.*ubuntu-seed" sfdisk -d "$LOOP" | MATCH "^${LOOP}p3 .*size=\s*1536000, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-boot" - sfdisk -d "$LOOP" | MATCH "^${LOOP}p4 .*size=\s*32768, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-save" - sfdisk -d "$LOOP" | MATCH "^${LOOP}p5 .*size=\s*15500753, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-data" + if [ "$channel" = "20" ]; then + sfdisk -d "$LOOP" | MATCH "^${LOOP}p4 .*size=\s*32768, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-save" + sfdisk -d "$LOOP" | MATCH "^${LOOP}p5 .*size=\s*15500753, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-data" + else + sfdisk -d "$LOOP" | MATCH "^${LOOP}p4 .*size=\s*65536, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-save" + sfdisk -d "$LOOP" | MATCH "^${LOOP}p5 .*size=\s*15467985, type=0FC63DAF-8483-4772-8E79-3D69D8477DE4,.*ubuntu-data" + fi not cryptsetup isLuks "${LOOP}p1" not cryptsetup isLuks "${LOOP}p2" @@ -218,6 +233,44 @@ execute: | not cryptsetup open --key-file recovery-key "${LOOP}p5" test-data not cryptsetup open --key-file recovery-key "${LOOP}p4" test-save + echo "Verify encryption key change" + # first get the relevant ubuntu-save key to the user keyring + systemd-run --pipe --wait --collect -p KeyringMode=inherit -- \ + /usr/bin/keyctl padd user "ubuntu-fde:${LOOP}p4:unlock" @u < save-key + + # the new key is all ones + printf "11111111111111111111111111111111" > new-save-key + echo '{"key":"MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE="}' | \ + systemd-run --pipe --wait --collect -p KeyringMode=inherit -- \ + /usr/lib/snapd/snap-fde-keymgr change-encryption-key \ + --device "${LOOP}p4" --stage + # now it's possible to open save with both the old and new keys + cryptsetup open --key-file save-key "${LOOP}p4" test-save + cryptsetup close /dev/mapper/test-save + cryptsetup open --key-file new-save-key "${LOOP}p4" test-save + cryptsetup close /dev/mapper/test-save + + # now transition the key + echo '{"key":"MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE="}' | \ + systemd-run --pipe --wait --collect -p KeyringMode=inherit -- \ + /usr/lib/snapd/snap-fde-keymgr change-encryption-key \ + --device "${LOOP}p4" --transition + + # the old key no longer works + not cryptsetup open --key-file save-key "${LOOP}p4" test-save + # but the new key does + cryptsetup open --key-file new-save-key "${LOOP}p4" test-save + cryptsetup close /dev/mapper/test-save + + # try to transition once more, such that we execute a reboot like scenario + echo '{"key":"MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE="}' | \ + systemd-run --pipe --wait --collect -p KeyringMode=inherit -- \ + /usr/lib/snapd/snap-fde-keymgr change-encryption-key \ + --device "${LOOP}p4" --transition + # and opening should still work + cryptsetup open --key-file new-save-key "${LOOP}p4" test-save + cryptsetup close /dev/mapper/test-save + LOOP_BASENAME="$(basename "$LOOP")" # disk things @@ -272,8 +325,13 @@ execute: | jq -r '.pc.structure[3]."filesystem-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-save-enc jq -r '.pc.structure[3]."partition-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-save jq -r '.pc.structure[3].id' < "$DISK_MAPPING_JSON" | MATCH "" - jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 - jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 16777216 + if [ "$channel" = "20" ]; then + jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 + jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 16777216 + else + jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 + jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 33554432 + fi # fifth structure - ubuntu-data-enc jq -r '.pc.structure[4]."device-path"' < "$DISK_MAPPING_JSON" | MATCH "/sys/devices/virtual/block/$LOOP_BASENAME/${LOOP_BASENAME}p5" @@ -281,5 +339,10 @@ execute: | jq -r '.pc.structure[4]."filesystem-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-data-enc jq -r '.pc.structure[4]."partition-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-data jq -r '.pc.structure[4].id' < "$DISK_MAPPING_JSON" | MATCH "" - jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2063597568 - jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 7936385536 + if [ "$channel" = "20" ]; then + jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2063597568 + jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 7936385536 + else + jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2080374784 + jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 7919608320 + fi diff --git a/tests/main/uc20-create-partitions/task.yaml b/tests/main/uc20-create-partitions/task.yaml index 6de18a33622..e264d8d28be 100644 --- a/tests/main/uc20-create-partitions/task.yaml +++ b/tests/main/uc20-create-partitions/task.yaml @@ -74,6 +74,11 @@ debug: | udevadm info --query property "${LOOP}p5" || true execute: | + channel=20 + if os.query is-jammy; then + channel=22 + fi + LOOP="$(cat loop.txt)" echo "Run the snap-bootstrap tool" @@ -164,8 +169,13 @@ execute: | sfdisk -l "$LOOP" | MATCH "${LOOP}p1 .* 1M\s* BIOS boot" sfdisk -l "$LOOP" | MATCH "${LOOP}p2 .* 1\.2G\s* EFI System" sfdisk -l "$LOOP" | MATCH "${LOOP}p3 .* 750M\s* Linux filesystem" - sfdisk -l "$LOOP" | MATCH "${LOOP}p4 .* 16M\s* Linux filesystem" - sfdisk -l "$LOOP" | MATCH "${LOOP}p5 .* 16\.7G\s* Linux filesystem" + if [ "$channel" = "20" ]; then + sfdisk -l "$LOOP" | MATCH "${LOOP}p4 .* 16M\s* Linux filesystem" + sfdisk -l "$LOOP" | MATCH "${LOOP}p5 .* 16\.7G\s* Linux filesystem" + else + sfdisk -l "$LOOP" | MATCH "${LOOP}p4 .* 32M\s* Linux filesystem" + sfdisk -l "$LOOP" | MATCH "${LOOP}p5 .* 16\.7G\s* Linux filesystem" + fi echo "check that the filesystems are created and mounted" mount | MATCH /run/mnt/ubuntu-boot @@ -226,8 +236,13 @@ execute: | jq -r '.pc.structure[3]."filesystem-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-save jq -r '.pc.structure[3]."partition-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-save jq -r '.pc.structure[3].id' < "$DISK_MAPPING_JSON" | MATCH "" - jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 - jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 16777216 + if [ "$channel" = "20" ]; then + jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 + jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 16777216 + else + jq -r '.pc.structure[3].offset' < "$DISK_MAPPING_JSON" | MATCH 2046820352 + jq -r '.pc.structure[3].size' < "$DISK_MAPPING_JSON" | MATCH 33554432 + fi # fifth structure - ubuntu-data jq -r '.pc.structure[4]."device-path"' < "$DISK_MAPPING_JSON" | MATCH "/sys/devices/virtual/block/$LOOP_BASENAME/${LOOP_BASENAME}p5" @@ -235,5 +250,10 @@ execute: | jq -r '.pc.structure[4]."filesystem-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-data jq -r '.pc.structure[4]."partition-label"' < "$DISK_MAPPING_JSON" | MATCH ubuntu-data jq -r '.pc.structure[4].id' < "$DISK_MAPPING_JSON" | MATCH "" - jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2063597568 - jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 17936385536 + if [ "$channel" = "20" ]; then + jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2063597568 + jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 17936385536 + else + jq -r '.pc.structure[4].offset' < "$DISK_MAPPING_JSON" | MATCH 2080374784 + jq -r '.pc.structure[4].size' < "$DISK_MAPPING_JSON" | MATCH 17919608320 + fi