From cd49a62ea5d24e3b18ec0c44300fc2d006faebdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= Date: Mon, 13 Feb 2023 11:45:28 +0000 Subject: [PATCH] boot: introduce parameter for optional kernel command line in functions used to update it. --- boot/boot.go | 35 ++++++---- boot/boot_test.go | 85 ++++++++++++++--------- boot/cmdline.go | 30 ++++---- boot/cmdline_test.go | 33 +++++++-- boot/makebootable.go | 68 ++++++++++++++++-- boot/makebootable_test.go | 140 +++++++++++++++++++++++++++++++++++++- 6 files changed, 319 insertions(+), 72 deletions(-) diff --git a/boot/boot.go b/boot/boot.go index 4ec13412123..1441cafce4f 100644 --- a/boot/boot.go +++ b/boot/boot.go @@ -24,6 +24,7 @@ import ( "fmt" "github.com/snapcore/snapd/bootloader" + "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/snap" ) @@ -429,10 +430,12 @@ func SetRecoveryBootSystemAndMode(dev snap.Device, systemLabel, mode string) err return bl.SetBootVars(m) } -// UpdateManagedBootConfigs updates managed boot config assets if those are -// present for the ubuntu-boot bootloader. Returns true when an update was -// carried out. -func UpdateManagedBootConfigs(dev snap.Device, gadgetSnapOrDir string) (updated bool, err error) { +// UpdateManagedBootConfigs updates managed boot config assets if +// those are present for the ubuntu-boot bootloader. To do this it +// needs information from the model, the gadget we are updating to, +// and any additional kernel command line arguments coming from system +// options. Returns true when an update was carried out. +func UpdateManagedBootConfigs(dev snap.Device, gadgetSnapOrDir, cmdlineAppend string) (updated bool, err error) { if !dev.HasModeenv() { // only UC20 devices use managed boot config return false, nil @@ -440,10 +443,10 @@ func UpdateManagedBootConfigs(dev snap.Device, gadgetSnapOrDir string) (updated if !dev.RunMode() { return false, fmt.Errorf("internal error: boot config can only be updated in run mode") } - return updateManagedBootConfigForBootloader(dev, ModeRun, gadgetSnapOrDir) + return updateManagedBootConfigForBootloader(dev, ModeRun, gadgetSnapOrDir, cmdlineAppend) } -func updateManagedBootConfigForBootloader(dev snap.Device, mode, gadgetSnapOrDir string) (updated bool, err error) { +func updateManagedBootConfigForBootloader(dev snap.Device, mode, gadgetSnapOrDir, cmdlineAppend string) (updated bool, err error) { if mode != ModeRun { return false, fmt.Errorf("internal error: updating boot config of recovery bootloader is not supported yet") } @@ -461,18 +464,20 @@ func updateManagedBootConfigForBootloader(dev snap.Device, mode, gadgetSnapOrDir return false, err } // boot config update can lead to a change of kernel command line - _, err = observeCommandLineUpdate(dev.Model(), commandLineUpdateReasonSnapd, gadgetSnapOrDir) + _, err = observeCommandLineUpdate(dev.Model(), commandLineUpdateReasonSnapd, gadgetSnapOrDir, cmdlineAppend) if err != nil { return false, err } return tbl.UpdateBootConfig() } -// UpdateCommandLineForGadgetComponent handles the update of a gadget that -// contributes to the kernel command line of the run system. Returns true when a -// change in command line has been observed and a reboot is needed. The reboot, -// if needed, should be requested at the the earliest possible occasion. -func UpdateCommandLineForGadgetComponent(dev snap.Device, gadgetSnapOrDir string) (needsReboot bool, err error) { +// UpdateCommandLineForGadgetComponent handles the update of a gadget +// that contributes to the kernel command line of the run system +// (appending any additional kernel command line arguments coming from +// system options). Returns true when a change in command line has +// been observed and a reboot is needed. The reboot, if needed, should +// be requested at the the earliest possible occasion. +func UpdateCommandLineForGadgetComponent(dev snap.Device, gadgetSnapOrDir, cmdlineAppend string) (needsReboot bool, err error) { if !dev.HasModeenv() { // only UC20 devices are supported return false, fmt.Errorf("internal error: command line component cannot be updated on pre-UC20 devices") @@ -490,8 +495,9 @@ func UpdateCommandLineForGadgetComponent(dev snap.Device, gadgetSnapOrDir string } return false, err } + // gadget update can lead to a change of kernel command line - cmdlineChange, err := observeCommandLineUpdate(dev.Model(), commandLineUpdateReasonGadget, gadgetSnapOrDir) + cmdlineChange, err := observeCommandLineUpdate(dev.Model(), commandLineUpdateReasonGadget, gadgetSnapOrDir, cmdlineAppend) if err != nil { return false, err } @@ -500,10 +506,11 @@ func UpdateCommandLineForGadgetComponent(dev snap.Device, gadgetSnapOrDir string } // update the bootloader environment, maybe clearing the relevant // variables - cmdlineVars, err := bootVarsForTrustedCommandLineFromGadget(gadgetSnapOrDir) + cmdlineVars, err := bootVarsForTrustedCommandLineFromGadget(gadgetSnapOrDir, cmdlineAppend) if err != nil { return false, fmt.Errorf("cannot prepare bootloader variables for kernel command line: %v", err) } + logger.Debugf("updating boot vars: %v", cmdlineVars) if err := tbl.SetBootVars(cmdlineVars); err != nil { return false, fmt.Errorf("cannot set run system kernel command line arguments: %v", err) } diff --git a/boot/boot_test.go b/boot/boot_test.go index b372dc52622..c3e7391525c 100644 --- a/boot/boot_test.go +++ b/boot/boot_test.go @@ -42,6 +42,7 @@ import ( "github.com/snapcore/snapd/seed" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snaptest" + "github.com/snapcore/snapd/strutil" "github.com/snapcore/snapd/testutil" "github.com/snapcore/snapd/timings" ) @@ -3341,14 +3342,14 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyNoKeysNoReseal(c *C) { }) defer restore() - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, "") c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 1) c.Check(resealCalls, Equals, 0) } -func (s *bootConfigSuite) TestBootConfigUpdateHappyWithReseal(c *C) { +func (s *bootConfigSuite) testBootConfigUpdateHappyWithReseal(c *C, cmdlineAppend string) { s.stampSealedKeys(c, dirs.GlobalRootDir) coreDev := boottest.MockUC20Device("", nil) @@ -3384,20 +3385,22 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyWithReseal(c *C) { } c.Assert(m.WriteTo(""), IsNil) + newCmdline := strutil.JoinNonEmpty([]string{ + "snapd_recovery_mode=run mocked candidate panic=-1", cmdlineAppend}, " ") resealCalls := 0 restore := boot.MockSecbootResealKeys(func(params *secboot.ResealKeysParams) error { resealCalls++ c.Assert(params, NotNil) c.Assert(params.ModelParams, HasLen, 1) c.Check(params.ModelParams[0].KernelCmdlines, DeepEquals, []string{ - "snapd_recovery_mode=run mocked candidate panic=-1", + newCmdline, "snapd_recovery_mode=run this is mocked panic=-1", }) return nil }) defer restore() - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, cmdlineAppend) c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 1) @@ -3407,11 +3410,19 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyWithReseal(c *C) { c.Assert(err, IsNil) c.Assert(m2.CurrentKernelCommandLines, DeepEquals, boot.BootCommandLines{ "snapd_recovery_mode=run this is mocked panic=-1", - "snapd_recovery_mode=run mocked candidate panic=-1", + newCmdline, }) } -func (s *bootConfigSuite) TestBootConfigUpdateHappyNoChange(c *C) { +func (s *bootConfigSuite) TestBootConfigUpdateHappyWithReseal(c *C) { + s.testBootConfigUpdateHappyWithReseal(c, "") +} + +func (s *bootConfigSuite) TestBootConfigUpdateHappyCmdlineAppendWithReseal(c *C) { + s.testBootConfigUpdateHappyWithReseal(c, "foo bar") +} + +func (s *bootConfigSuite) testBootConfigUpdateHappyNoChange(c *C, cmdlineAppend string) { s.stampSealedKeys(c, dirs.GlobalRootDir) coreDev := boottest.MockUC20Device("", nil) @@ -3419,12 +3430,12 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyNoChange(c *C) { s.bootloader.StaticCommandLine = "mocked unchanged panic=-1" s.bootloader.CandidateStaticCommandLine = "mocked unchanged panic=-1" - s.mockCmdline(c, "snapd_recovery_mode=run mocked unchanged panic=-1") m := &boot.Modeenv{ Mode: "run", CurrentKernelCommandLines: boot.BootCommandLines{ - "snapd_recovery_mode=run mocked unchanged panic=-1", + strutil.JoinNonEmpty([]string{ + "snapd_recovery_mode=run mocked unchanged panic=-1", cmdlineAppend}, " "), }, } c.Assert(m.WriteTo(""), IsNil) @@ -3436,7 +3447,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyNoChange(c *C) { }) defer restore() - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, cmdlineAppend) c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 1) @@ -3447,10 +3458,18 @@ func (s *bootConfigSuite) TestBootConfigUpdateHappyNoChange(c *C) { c.Assert(m2.CurrentKernelCommandLines, HasLen, 1) } +func (s *bootConfigSuite) TestBootConfigUpdateHappyNoChange(c *C) { + s.testBootConfigUpdateHappyNoChange(c, "") +} + +func (s *bootConfigSuite) TestBootConfigUpdateHappyCmdlineAppendNoChange(c *C) { + s.testBootConfigUpdateHappyNoChange(c, "foo bar") +} + func (s *bootConfigSuite) TestBootConfigUpdateNonUC20DoesNothing(c *C) { nonUC20coreDev := boottest.MockDevice("pc-kernel") c.Assert(nonUC20coreDev.HasModeenv(), Equals, false) - updated, err := boot.UpdateManagedBootConfigs(nonUC20coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(nonUC20coreDev, s.gadgetSnap, "") c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 0) @@ -3459,7 +3478,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateNonUC20DoesNothing(c *C) { func (s *bootConfigSuite) TestBootConfigUpdateBadModeErr(c *C) { uc20Dev := boottest.MockUC20Device("recover", nil) c.Assert(uc20Dev.HasModeenv(), Equals, true) - updated, err := boot.UpdateManagedBootConfigs(uc20Dev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(uc20Dev, s.gadgetSnap, "") c.Assert(err, ErrorMatches, "internal error: boot config can only be updated in run mode") c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 0) @@ -3479,7 +3498,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateFailErr(c *C) { s.bootloader.UpdateErr = errors.New("update fail") - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, "") c.Assert(err, ErrorMatches, "update fail") c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 1) @@ -3496,7 +3515,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateCmdlineMismatchErr(c *C) { s.mockCmdline(c, "snapd_recovery_mode=run unexpected cmdline") - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, "") c.Assert(err, ErrorMatches, `internal error: current kernel command lines is unset`) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 0) @@ -3515,7 +3534,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateNotManagedErr(c *C) { } c.Assert(m.WriteTo(""), IsNil) - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, "") c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 0) @@ -3533,7 +3552,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateBootloaderFindErr(c *C) { } c.Assert(m.WriteTo(""), IsNil) - updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, s.gadgetSnap, "") c.Assert(err, ErrorMatches, "internal error: cannot find trusted assets bootloader under .*: mocked find error") c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 0) @@ -3593,7 +3612,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateWithGadgetAndReseal(c *C) { }) defer restore() - updated, err := boot.UpdateManagedBootConfigs(coreDev, gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, gadgetSnap, "") c.Assert(err, IsNil) c.Check(updated, Equals, false) c.Check(s.bootloader.UpdateCalls, Equals, 1) @@ -3641,7 +3660,7 @@ func (s *bootConfigSuite) TestBootConfigUpdateWithGadgetFullAndReseal(c *C) { }) defer restore() - updated, err := boot.UpdateManagedBootConfigs(coreDev, gadgetSnap) + updated, err := boot.UpdateManagedBootConfigs(coreDev, gadgetSnap, "") c.Assert(err, IsNil) c.Check(updated, Equals, true) c.Check(s.bootloader.UpdateCalls, Equals, 1) @@ -3745,7 +3764,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateNonUC20(c *C) { {"cmdline.extra", "foo"}, }) - reboot, err := boot.UpdateCommandLineForGadgetComponent(nonUC20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(nonUC20dev, sf, "") c.Assert(err, ErrorMatches, `internal error: command line component cannot be updated on pre-UC20 devices`) c.Assert(reboot, Equals, false) } @@ -3761,7 +3780,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20NotManagedBootload bl.SetErr = fmt.Errorf("unexpected call") s.forceBootloader(bl) - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, false) c.Check(bl.SetBootVarsCalls, Equals, 0) @@ -3777,7 +3796,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20ArgsAdded(c *C) { s.modeenvWithEncryption.CurrentKernelCommandLines = []string{"snapd_recovery_mode=run static mocked panic=-1"} c.Assert(s.modeenvWithEncryption.WriteTo(""), IsNil) - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) @@ -3823,7 +3842,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20ArgsSwitch(c *C) { c.Assert(err, IsNil) s.bootloader.SetBootVarsCalls = 0 - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, false) @@ -3849,7 +3868,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20ArgsSwitch(c *C) { {"cmdline.extra", "changed"}, }) - reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfChanged) + reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfChanged, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) @@ -3896,7 +3915,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20UnencryptedArgsRem c.Assert(err, IsNil) s.bootloader.SetBootVarsCalls = 0 - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) @@ -3936,7 +3955,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20SetError(c *C) { s.bootloader.SetErr = fmt.Errorf("set fails") - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, ErrorMatches, "cannot set run system kernel command line arguments: set fails") c.Assert(reboot, Equals, false) // set boot vars was called and failed @@ -3974,7 +3993,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateWithResealError(c *C) }) defer restore() - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, gadgetSnap) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, gadgetSnap, "") c.Assert(err, ErrorMatches, "cannot reseal the encryption key: reseal fails") c.Check(reboot, Equals, false) c.Check(s.bootloader.SetBootVarsCalls, Equals, 0) @@ -4006,7 +4025,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20TransitionFullExtr sf := snaptest.MakeTestSnapWithFiles(c, gadgetSnapYaml, [][]string{ {"cmdline.extra", "extra args"}, }) - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) c.Check(s.resealCalls, Equals, 1) @@ -4039,7 +4058,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20TransitionFullExtr sfFull := snaptest.MakeTestSnapWithFiles(c, gadgetSnapYaml, [][]string{ {"cmdline.full", "full args"}, }) - reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfFull) + reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfFull, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) c.Check(s.resealCalls, Equals, 2) @@ -4072,7 +4091,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20TransitionFullExtr // transition back to no arguments from the gadget sfNone := snaptest.MakeTestSnapWithFiles(c, gadgetSnapYaml, nil) - reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfNone) + reboot, err = boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sfNone, "") c.Assert(err, IsNil) c.Assert(reboot, Equals, true) c.Check(s.resealCalls, Equals, 3) @@ -4146,7 +4165,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20OverSpuriousReboot // let's panic on reseal first resealPanic = true c.Assert(func() { - boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") }, PanicMatches, "reseal panic") c.Check(s.resealCalls, Equals, 1) c.Check(s.resealCommandLines, DeepEquals, [][]string{{ @@ -4183,7 +4202,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20OverSpuriousReboot resealPanic = false // but panic in set c.Assert(func() { - boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") }, PanicMatches, "mocked reboot panic in SetBootVars") c.Check(s.resealCalls, Equals, 1) c.Check(s.resealCommandLines, DeepEquals, [][]string{{ @@ -4215,7 +4234,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20OverSpuriousReboot s.resealCalls = 0 s.resealCommandLines = nil restoreBootloaderNoPanic() - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) c.Check(reboot, Equals, true) c.Check(s.resealCalls, Equals, 1) @@ -4267,7 +4286,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20OverSpuriousReboot panic("mocked reboot panic after SetBootVars") } c.Assert(func() { - boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") }, PanicMatches, "mocked reboot panic after SetBootVars") c.Check(s.resealCalls, Equals, 1) c.Check(s.resealCommandLines, DeepEquals, [][]string{{ @@ -4311,7 +4330,7 @@ func (s *bootKernelCommandLineSuite) TestCommandLineUpdateUC20OverSpuriousReboot // try again, as if the task handler gets to run again s.resealCalls = 0 - reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf) + reboot, err := boot.UpdateCommandLineForGadgetComponent(s.uc20dev, sf, "") c.Assert(err, IsNil) // nothing changed now, we already booted with the new command line c.Check(reboot, Equals, false) diff --git a/boot/cmdline.go b/boot/cmdline.go index c1ce73f620a..370ce531b4d 100644 --- a/boot/cmdline.go +++ b/boot/cmdline.go @@ -104,23 +104,24 @@ func getBootloaderManagingItsAssets(where string, opts *bootloader.Options) (boo return mbl, nil } -// bootVarsForTrustedCommandLineFromGadget returns a set of boot variables that -// carry the command line arguments requested by the gadget. This is only useful +// bootVarsForTrustedCommandLineFromGadget returns a set of boot +// variables that carry the command line arguments defined by the +// gadget and some system options (cmdlineApped). This is only useful // if snapd is managing the boot config. -func bootVarsForTrustedCommandLineFromGadget(gadgetDirOrSnapPath string) (map[string]string, error) { +func bootVarsForTrustedCommandLineFromGadget(gadgetDirOrSnapPath, cmdlineAppend string) (map[string]string, error) { extraOrFull, full, err := gadget.KernelCommandLineFromGadget(gadgetDirOrSnapPath) if err != nil { - if err == gadget.ErrNoKernelCommandline { - // nothing set by the gadget, but we could have had - // arguments before, so make sure those are cleared now - clear := map[string]string{ - "snapd_extra_cmdline_args": "", - "snapd_full_cmdline_args": "", - } - return clear, nil + if err != gadget.ErrNoKernelCommandline { + return nil, fmt.Errorf("cannot use kernel command line from gadget: %v", err) } - return nil, fmt.Errorf("cannot use kernel command line from gadget: %v", err) + extraOrFull = "" + full = false } + logger.Debugf("trusted command line: from gadget: %q, from options: %q", + extraOrFull, cmdlineAppend) + + extraOrFull = strutil.JoinNonEmpty([]string{extraOrFull, cmdlineAppend}, " ") + // gadget has the kernel command line args := map[string]string{ "snapd_extra_cmdline_args": "", @@ -328,7 +329,7 @@ const ( // by an update of boot config or the gadget snap. When needed, the modeenv is // updated with a candidate command line and the encryption keys are resealed. // This helper should be called right before updating the managed boot config. -func observeCommandLineUpdate(model *asserts.Model, reason commandLineUpdateReason, gadgetSnapOrDir string) (updated bool, err error) { +func observeCommandLineUpdate(model *asserts.Model, reason commandLineUpdateReason, gadgetSnapOrDir, cmdlineOpt string) (updated bool, err error) { // TODO:UC20: consider updating a recovery system command line m, err := loadModeenv() @@ -355,6 +356,9 @@ func observeCommandLineUpdate(model *asserts.Model, reason commandLineUpdateReas if err != nil { return false, err } + // Add part coming from options + candidateCmdline = strutil.JoinNonEmpty( + []string{candidateCmdline, cmdlineOpt}, " ") if cmdline == candidateCmdline { // command line is the same or no actual change in modeenv return false, nil diff --git a/boot/cmdline_test.go b/boot/cmdline_test.go index b8df2761d64..29deb2708d6 100644 --- a/boot/cmdline_test.go +++ b/boot/cmdline_test.go @@ -370,9 +370,10 @@ func (s *kernelCommandLineSuite) TestComposeRecoveryCommandLineWithGadget(c *C) func (s *kernelCommandLineSuite) TestBootVarsForGadgetCommandLine(c *C) { for _, tc := range []struct { - errMsg string - files [][]string - expectedVars map[string]string + errMsg string + files [][]string + cmdlineAppend string + expectedVars map[string]string }{{ files: [][]string{ {"cmdline.extra", "foo bar baz"}, @@ -402,6 +403,30 @@ func (s *kernelCommandLineSuite) TestBootVarsForGadgetCommandLine(c *C) { "snapd_extra_cmdline_args": "", "snapd_full_cmdline_args": "full foo bar baz", }, + }, { + cmdlineAppend: "foo bar baz", + expectedVars: map[string]string{ + "snapd_extra_cmdline_args": "foo bar baz", + "snapd_full_cmdline_args": "", + }, + }, { + files: [][]string{ + {"cmdline.extra", "foo bar baz"}, + }, + cmdlineAppend: "x=y z", + expectedVars: map[string]string{ + "snapd_extra_cmdline_args": "foo bar baz x=y z", + "snapd_full_cmdline_args": "", + }, + }, { + files: [][]string{ + {"cmdline.full", "full foo bar baz"}, + }, + cmdlineAppend: "x=y z", + expectedVars: map[string]string{ + "snapd_extra_cmdline_args": "", + "snapd_full_cmdline_args": "full foo bar baz x=y z", + }, }, { // with no arguments boot variables should be cleared files: [][]string{}, @@ -413,7 +438,7 @@ func (s *kernelCommandLineSuite) TestBootVarsForGadgetCommandLine(c *C) { sf := snaptest.MakeTestSnapWithFiles(c, gadgetSnapYaml, append([][]string{ {"meta/snap.yaml", gadgetSnapYaml}, }, tc.files...)) - vars, err := boot.BootVarsForTrustedCommandLineFromGadget(sf) + vars, err := boot.BootVarsForTrustedCommandLineFromGadget(sf, tc.cmdlineAppend) if tc.errMsg == "" { c.Assert(err, IsNil) c.Assert(vars, DeepEquals, tc.expectedVars) diff --git a/boot/makebootable.go b/boot/makebootable.go index cd2eae71c09..fd8a6c85e2d 100644 --- a/boot/makebootable.go +++ b/boot/makebootable.go @@ -27,9 +27,12 @@ import ( "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/gadget" + "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/snapfile" + "github.com/snapcore/snapd/strutil" ) var sealKeyToModeenv = sealKeyToModeenvImpl @@ -77,7 +80,7 @@ func MakeBootableImage(model *asserts.Model, rootdir string, bootWith *BootableS if !bootWith.Recovery { return fmt.Errorf("internal error: MakeBootableImage called at runtime, use MakeRunnableSystem instead") } - return makeBootable20(rootdir, bootWith, bootFlags) + return makeBootable20(model, rootdir, bootWith, bootFlags) } // MakeBootablePartition configures a partition mounted on rootdir @@ -204,7 +207,7 @@ func configureBootloader(rootdir string, opts *bootloader.Options, bootWith *Boo return nil } -func makeBootable20(rootdir string, bootWith *BootableSet, bootFlags []string) error { +func makeBootable20(model *asserts.Model, rootdir string, bootWith *BootableSet, bootFlags []string) error { // we can only make a single recovery system bootable right now recoverySystems, err := filepath.Glob(filepath.Join(rootdir, "systems/*")) if err != nil { @@ -227,7 +230,7 @@ func makeBootable20(rootdir string, bootWith *BootableSet, bootFlags []string) e return fmt.Errorf("cannot install bootloader: %v", err) } - return MakeRecoverySystemBootable(rootdir, bootWith.RecoverySystemDir, &RecoverySystemBootableSet{ + return MakeRecoverySystemBootable(model, rootdir, bootWith.RecoverySystemDir, &RecoverySystemBootableSet{ Kernel: bootWith.Kernel, KernelPath: bootWith.KernelPath, GadgetSnapOrDir: bootWith.UnpackedGadgetDir, @@ -248,7 +251,7 @@ type RecoverySystemBootableSet struct { // MakeRecoverySystemBootable prepares a recovery system under a path relative // to recovery bootloader's rootdir for booting. -func MakeRecoverySystemBootable(rootdir string, relativeRecoverySystemDir string, bootWith *RecoverySystemBootableSet) error { +func MakeRecoverySystemBootable(model *asserts.Model, rootdir string, relativeRecoverySystemDir string, bootWith *RecoverySystemBootableSet) error { opts := &bootloader.Options{ // XXX: this is only needed by LK, it is unclear whether LK does // too much when extracting recovery kernel assets, in the end @@ -298,7 +301,13 @@ func MakeRecoverySystemBootable(rootdir string, relativeRecoverySystemDir string "snapd_recovery_kernel": filepath.Join("/", kernelPath), } if _, ok := bl.(bootloader.TrustedAssetsBootloader); ok { - recoveryCmdlineArgs, err := bootVarsForTrustedCommandLineFromGadget(bootWith.GadgetSnapOrDir) + // Look at gadget default values for system.kernel.*cmdline-append options + cmdlineAppend, err := buildOptionalKernelCommandLine(model, bootWith.GadgetSnapOrDir) + if err != nil { + return fmt.Errorf("while retrieving system.kernel.*cmdline-append defaults: %v", err) + } + // to set cmdlineAppend. + recoveryCmdlineArgs, err := bootVarsForTrustedCommandLineFromGadget(bootWith.GadgetSnapOrDir, cmdlineAppend) if err != nil { return fmt.Errorf("cannot obtain recovery system command line: %v", err) } @@ -487,7 +496,12 @@ func makeRunnableSystem(model *asserts.Model, bootWith *BootableSet, sealer *Tru } modeenv.CurrentKernelCommandLines = bootCommandLines{cmdline} - cmdlineVars, err := bootVarsForTrustedCommandLineFromGadget(bootWith.UnpackedGadgetDir) + // Look at gadget default values for system.kernel.*cmdline-append options + cmdlineAppend, err := buildOptionalKernelCommandLine(model, bootWith.UnpackedGadgetDir) + if err != nil { + return fmt.Errorf("while retrieving system.kernel.*cmdline-append defaults: %v", err) + } + cmdlineVars, err := bootVarsForTrustedCommandLineFromGadget(bootWith.UnpackedGadgetDir, cmdlineAppend) if err != nil { return fmt.Errorf("cannot prepare bootloader variables for kernel command line: %v", err) } @@ -529,6 +543,48 @@ func makeRunnableSystem(model *asserts.Model, bootWith *BootableSet, sealer *Tru return nil } +func buildOptionalKernelCommandLine(model *asserts.Model, gadgetSnapOrDir string) (string, error) { + sf, err := snapfile.Open(gadgetSnapOrDir) + if err != nil { + return "", fmt.Errorf("cannot open gadget snap: %v", err) + } + gadgetInfo, err := gadget.ReadInfoFromSnapFile(sf, nil) + if err != nil { + return "", fmt.Errorf("cannot read gadget data: %v", err) + } + + defaults := gadget.SystemDefaults(gadgetInfo.Defaults) + + var cmdlineAppend, cmdlineAppendDangerous string + + if cmdlineAppendIf, ok := defaults["system.kernel.cmdline-append"]; ok { + cmdlineAppend, ok = cmdlineAppendIf.(string) + if !ok { + return "", fmt.Errorf("system.kernel.cmdline-append is not a string") + } + } + + if cmdlineAppendIf, ok := defaults["system.kernel.dangerous-cmdline-append"]; ok { + cmdlineAppendDangerous, ok = cmdlineAppendIf.(string) + if !ok { + return "", fmt.Errorf("system.kernel.dangerous-cmdline-append is not a string") + } + if model.Grade() != asserts.ModelDangerous { + // Print a warning and ignore + logger.Noticef("WARNING: system.kernel.dangerous-cmdline-append ignored by non-dangerous models") + return "", nil + } + } + + if cmdlineAppend != "" { + // TODO perform validation against what is allowed by the gadget + } + + cmdlineAppend = strutil.JoinNonEmpty([]string{cmdlineAppend, cmdlineAppendDangerous}, " ") + + return cmdlineAppend, nil +} + // MakeRunnableSystem is like MakeBootableImage in that it sets up a system to // be able to boot, but is unique in that it is intended to be called from UC20 // install mode and makes the run system bootable (hence it is called diff --git a/boot/makebootable_test.go b/boot/makebootable_test.go index 5e573b4db96..fc9a5404c98 100644 --- a/boot/makebootable_test.go +++ b/boot/makebootable_test.go @@ -187,6 +187,28 @@ func (s *makeBootable20UbootSuite) SetUpTest(c *C) { s.forceBootloader(s.bootloader) } +const gadgetYaml = ` +volumes: + pc: + bootloader: grub + structure: + - name: ubuntu-seed + role: system-seed + filesystem: vfat + type: EF,C12A7328-F81F-11D2-BA4B-00A0C93EC93B + size: 1200M + - name: ubuntu-boot + role: system-boot + filesystem: ext4 + type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 + size: 750M + - name: ubuntu-data + role: system-data + filesystem: ext4 + type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 + size: 1G +` + func (s *makeBootable20Suite) TestMakeBootableImage20(c *C) { bootloader.Force(nil) model := boottest.MakeMockUC20Model() @@ -199,6 +221,7 @@ func (s *makeBootable20Suite) TestMakeBootableImage20(c *C) { {"grub-recovery.conf", grubRecoveryCfg}, {"grub.conf", grubCfg}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", []byte(grubRecoveryCfgAsset)) defer restore() @@ -275,6 +298,7 @@ func (s *makeBootable20Suite) TestMakeBootableImage20BootFlags(c *C) { {"grub-recovery.conf", grubRecoveryCfg}, {"grub.conf", grubCfg}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", []byte(grubRecoveryCfgAsset)) defer restore() @@ -339,6 +363,7 @@ func (s *makeBootable20Suite) testMakeBootableImage20CustomKernelArgs(c *C, whic snaptest.PopulateDir(unpackedGadgetDir, [][]string{ {"grub.conf", grubCfg}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, {whichFile, content}, }) @@ -519,6 +544,7 @@ func (s *makeBootable20Suite) testMakeSystemRunnable20(c *C, standalone, factory {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore = assets.MockInternal("grub-recovery.cfg", grubRecoveryCfgAsset) defer restore() @@ -1020,6 +1046,7 @@ func (s *makeBootable20Suite) TestMakeRunnableSystem20RunModeSealKeyErr(c *C) { {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", grubRecoveryCfgAsset) defer restore() @@ -1208,6 +1235,7 @@ func (s *makeBootable20Suite) testMakeSystemRunnable20WithCustomKernelArgs(c *C, {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, {whichFile, content}, } snaptest.PopulateDir(unpackedGadgetDir, gadgetFiles) @@ -1446,6 +1474,7 @@ func (s *makeBootable20Suite) TestMakeSystemRunnable20UnhappyMarkRecoveryCapable {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", grubRecoveryCfgAsset) defer restore() @@ -1813,6 +1842,7 @@ model_sign_key_id=Jv8_JiHiIzJVcO9M55pPdqSDWUvuhfDIBJUS-3VW7F_idjix7Ffn5qMxB21ZQu func (s *makeBootable20Suite) TestMakeRecoverySystemBootableAtRuntime20(c *C) { bootloader.Force(nil) + model := boottest.MakeMockUC20Model() // on uc20 the seed layout if different seedSnapsDirs := filepath.Join(s.rootdir, "/snaps") @@ -1835,6 +1865,7 @@ version: 5.0 {"grub.conf", ""}, {"meta/snap.yaml", gadgetSnapYaml}, {"cmdline.full", fmt.Sprintf("args from gadget rev %s", rev.String())}, + {"meta/gadget.yaml", gadgetYaml}, }) gadgetInSeed := filepath.Join(seedSnapsDirs, gadgetInfo.Filename()) err = os.Rename(gadgetFn, gadgetInSeed) @@ -1850,7 +1881,7 @@ version: 5.0 label := "20191209" recoverySystemDir := filepath.Join("/systems", label) - err = boot.MakeRecoverySystemBootable(s.rootdir, recoverySystemDir, &boot.RecoverySystemBootableSet{ + err = boot.MakeRecoverySystemBootable(model, s.rootdir, recoverySystemDir, &boot.RecoverySystemBootableSet{ Kernel: kernelInfo, KernelPath: kernelInSeed, // use gadget revision 1 @@ -1872,7 +1903,7 @@ version: 5.0 newLabel := "20210420" newRecoverySystemDir := filepath.Join("/systems", newLabel) // with a different gadget revision, but same kernel - err = boot.MakeRecoverySystemBootable(s.rootdir, newRecoverySystemDir, &boot.RecoverySystemBootableSet{ + err = boot.MakeRecoverySystemBootable(model, s.rootdir, newRecoverySystemDir, &boot.RecoverySystemBootableSet{ Kernel: kernelInfo, KernelPath: kernelInSeed, GadgetSnapOrDir: gadgets["5"], @@ -2008,6 +2039,7 @@ func (s *makeBootable20Suite) TestMakeRunnableSystemNoGoodRecoverySystems(c *C) {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", grubRecoveryCfgAsset) defer restore() @@ -2093,6 +2125,7 @@ func (s *makeBootable20Suite) TestMakeRunnableSystemStandaloneSnapsCopy(c *C) { {"bootx64.efi", "shim content"}, {"grubx64.efi", "grub content"}, {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml}, }) restore := assets.MockInternal("grub-recovery.cfg", grubRecoveryCfgAsset) defer restore() @@ -2184,3 +2217,106 @@ func (s *makeBootable20Suite) TestMakeStandaloneSystemRunnable20InstallOnClassic const classic = true s.testMakeSystemRunnable20(c, standalone, factoryReset, classic) } + +func (s *makeBootable20Suite) testMakeBootableImageOptionalKernelArgs(c *C, model *asserts.Model, options map[string]string, expectedCmdline, errMsg string) { + bootloader.Force(nil) + + defaults := "defaults:\n system:\n" + for k, v := range options { + defaults += fmt.Sprintf(" %s: %s\n", k, v) + } + + unpackedGadgetDir := c.MkDir() + grubCfg := "#grub cfg" + snaptest.PopulateDir(unpackedGadgetDir, [][]string{ + {"grub.conf", grubCfg}, + {"meta/snap.yaml", gadgetSnapYaml}, + {"meta/gadget.yaml", gadgetYaml + defaults}, + }) + + // on uc20 the seed layout is different + seedSnapsDirs := filepath.Join(s.rootdir, "/snaps") + err := os.MkdirAll(seedSnapsDirs, 0755) + c.Assert(err, IsNil) + + baseFn, baseInfo := makeSnap(c, "core22", `name: core22 +type: base +version: 5.0 +`, snap.R(3)) + baseInSeed := filepath.Join(seedSnapsDirs, baseInfo.Filename()) + err = os.Rename(baseFn, baseInSeed) + c.Assert(err, IsNil) + kernelFn, kernelInfo := makeSnapWithFiles(c, "pc-kernel", `name: pc-kernel +type: kernel +version: 5.0 +`, snap.R(5), [][]string{ + {"kernel.efi", "I'm a kernel.efi"}, + }) + kernelInSeed := filepath.Join(seedSnapsDirs, kernelInfo.Filename()) + err = os.Rename(kernelFn, kernelInSeed) + c.Assert(err, IsNil) + + label := "20191209" + recoverySystemDir := filepath.Join("/systems", label) + bootWith := &boot.BootableSet{ + Base: baseInfo, + BasePath: baseInSeed, + Kernel: kernelInfo, + KernelPath: kernelInSeed, + RecoverySystemDir: recoverySystemDir, + RecoverySystemLabel: label, + UnpackedGadgetDir: unpackedGadgetDir, + Recovery: true, + } + + err = boot.MakeBootableImage(model, s.rootdir, bootWith, nil) + if errMsg != "" { + c.Assert(err, ErrorMatches, errMsg) + return + } + c.Assert(err, IsNil) + + // ensure the correct recovery system configuration was set + seedGenv := grubenv.NewEnv(filepath.Join(s.rootdir, "EFI/ubuntu/grubenv")) + c.Assert(seedGenv.Load(), IsNil) + c.Check(seedGenv.Get("snapd_recovery_system"), Equals, label) + // and kernel command line + systemGenv := grubenv.NewEnv(filepath.Join(s.rootdir, recoverySystemDir, "grubenv")) + c.Assert(systemGenv.Load(), IsNil) + c.Check(systemGenv.Get("snapd_recovery_kernel"), Equals, "/snaps/pc-kernel_5.snap") + c.Check(systemGenv.Get("snapd_extra_cmdline_args"), Equals, expectedCmdline) +} + +func (s *makeBootable20Suite) TestMakeBootableImageOptionalKernelArgsHappy(c *C) { + model := boottest.MakeMockUC20Model() + const cmdline = "param1=val param2" + for _, opt := range []string{"system.kernel.cmdline-append", "system.kernel.dangerous-cmdline-append"} { + options := map[string]string{ + opt: cmdline, + } + s.testMakeBootableImageOptionalKernelArgs(c, model, options, cmdline, "") + } +} + +func (s *makeBootable20Suite) TestMakeBootableImageOptionalKernelArgsBothBootOptsSet(c *C) { + model := boottest.MakeMockUC20Model() + const cmdline = "param1=val param2" + const cmdlineDanger = "param3=val param4" + options := map[string]string{ + "system.kernel.cmdline-append": cmdline, + "system.kernel.dangerous-cmdline-append": cmdlineDanger, + } + s.testMakeBootableImageOptionalKernelArgs(c, model, options, cmdline+" "+cmdlineDanger, "") +} + +func (s *makeBootable20Suite) TestMakeBootableImageOptionalKernelArgsSignedAndDangerous(c *C) { + model := boottest.MakeMockUC20Model(map[string]interface{}{ + "grade": "signed", + }) + const cmdline = "param1=val param2" + options := map[string]string{ + "system.kernel.dangerous-cmdline-append": cmdline, + } + // The option is ignored if non-dangerous model + s.testMakeBootableImageOptionalKernelArgs(c, model, options, "", "") +}