Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gadget: allow content observer to have opinions about a change #9401

Merged
merged 7 commits into from Sep 28, 2020
Merged

gadget: allow content observer to have opinions about a change #9401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2099cc7
gadget: allow content observer to have opinions about a change
bboozzoo Sep 23, 2020
1771b37
many: update content observer interface
bboozzoo Sep 23, 2020
64da476
boot: check observer return values in unit tests
bboozzoo Sep 23, 2020
c8f1c3b
boot: verify install observer return values in unit tests
bboozzoo Sep 23, 2020
e8a0429
many: naming tweaks
bboozzoo Sep 23, 2020
7cc7387
Merge remote-tracking branch 'upstream/master' into bboozzoo/uc20-pre…
bboozzoo Sep 24, 2020
95aecd8
gadget: comment tweak
bboozzoo Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 32 additions & 32 deletions boot/assets.go
View file
Open in desktop
Expand Up @@ -227,36 +227,36 @@ type TrustedAssetsInstallObserver struct {
// of the secure boot.
//
// Implements gadget.ContentObserver.
func (o *TrustedAssetsInstallObserver) Observe(op gadget.ContentOperation, affectedStruct *gadget.LaidOutStructure, root, relativeTarget string, data *gadget.ContentChange) (bool, error) {
func (o *TrustedAssetsInstallObserver) Observe(op gadget.ContentOperation, affectedStruct *gadget.LaidOutStructure, root, relativeTarget string, data *gadget.ContentChange) (gadget.ContentChangeAction, error) {
if affectedStruct.Role != gadget.SystemBoot {
// only care about system-boot
return true, nil
return gadget.ChangeApply, nil
}

if o.blName == "" {
// we have no information about the bootloader yet
bl, err := bootloader.ForGadget(o.gadgetDir, root, &bootloader.Options{Role: bootloader.RoleRunMode, NoSlashBoot: true})
if err != nil {
return false, fmt.Errorf("cannot find bootloader: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot find bootloader: %v", err)
}
o.blName = bl.Name()
tbl, ok := bl.(bootloader.TrustedAssetsBootloader)
if !ok {
return true, nil
return gadget.ChangeApply, nil
}
trustedAssets, err := tbl.TrustedAssets()
if err != nil {
return false, fmt.Errorf("cannot list %q bootloader trusted assets: %v", bl.Name(), err)
return gadget.ChangeAbort, fmt.Errorf("cannot list %q bootloader trusted assets: %v", bl.Name(), err)
}
o.trustedAssets = trustedAssets
}
if len(o.trustedAssets) == 0 || !strutil.ListContains(o.trustedAssets, relativeTarget) {
// not one of the trusted assets
return true, nil
return gadget.ChangeApply, nil
}
ta, err := o.cache.Add(data.After, o.blName, filepath.Base(relativeTarget))
if err != nil {
return false, err
return gadget.ChangeAbort, err
}
// during installation, modeenv is written out later, at this point we
// only care that the same file may appear multiple times in gadget
Expand All @@ -266,11 +266,11 @@ func (o *TrustedAssetsInstallObserver) Observe(op gadget.ContentOperation, affec
o.trackedAssets = bootAssetsMap{}
}
if len(o.trackedAssets[ta.name]) > 0 {
return false, fmt.Errorf("cannot reuse asset name %q", ta.name)
return gadget.ChangeAbort, fmt.Errorf("cannot reuse asset name %q", ta.name)
}
o.trackedAssets[ta.name] = append(o.trackedAssets[ta.name], ta.hash)
}
return true, nil
return gadget.ChangeApply, nil
}

// ObserveExistingTrustedRecoveryAssets observes existing trusted assets of a
Expand Down Expand Up @@ -380,7 +380,7 @@ func findMaybeTrustedAssetsBootloader(root string, opts *bootloader.Options) (fo
// the bootloader binary which is measured as part of the secure boot.
//
// Implements gadget.ContentUpdateObserver.
func (o *TrustedAssetsUpdateObserver) Observe(op gadget.ContentOperation, affectedStruct *gadget.LaidOutStructure, root, relativeTarget string, data *gadget.ContentChange) (bool, error) {
func (o *TrustedAssetsUpdateObserver) Observe(op gadget.ContentOperation, affectedStruct *gadget.LaidOutStructure, root, relativeTarget string, data *gadget.ContentChange) (gadget.ContentChangeAction, error) {
var whichBootloader bootloader.Bootloader
var whichAssets []string
var err error
Expand All @@ -394,7 +394,7 @@ func (o *TrustedAssetsUpdateObserver) Observe(op gadget.ContentOperation, affect
NoSlashBoot: true,
})
if err != nil {
return false, err
return gadget.ChangeAbort, err
}
}
whichBootloader = o.bootBootloader
Expand All @@ -405,25 +405,25 @@ func (o *TrustedAssetsUpdateObserver) Observe(op gadget.ContentOperation, affect
Role: bootloader.RoleRecovery,
})
if err != nil {
return false, err
return gadget.ChangeAbort, err
}
}
whichBootloader = o.seedBootloader
whichAssets = o.seedTrustedAssets
isRecovery = true
default:
// only system-seed and system-boot are of interest
return true, nil
return gadget.ChangeApply, nil
}
if len(whichAssets) == 0 || !strutil.ListContains(whichAssets, relativeTarget) {
// not one of the trusted assets
return true, nil
return gadget.ChangeApply, nil
}
if o.modeenv == nil {
// we've hit a trusted asset, so a modeenv is needed now too
o.modeenv, err = ReadModeenv("")
if err != nil {
return false, fmt.Errorf("cannot load modeenv: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot load modeenv: %v", err)
}
}
switch op {
Expand All @@ -433,14 +433,14 @@ func (o *TrustedAssetsUpdateObserver) Observe(op gadget.ContentOperation, affect
return o.observeRollback(whichBootloader, isRecovery, root, relativeTarget, data)
default:
// we only care about update and rollback actions
return false, nil
return gadget.ChangeApply, nil
}
}

func (o *TrustedAssetsUpdateObserver) observeUpdate(bl bootloader.Bootloader, recovery bool, root, relativeTarget string, change *gadget.ContentChange) (bool, error) {
func (o *TrustedAssetsUpdateObserver) observeUpdate(bl bootloader.Bootloader, recovery bool, root, relativeTarget string, change *gadget.ContentChange) (gadget.ContentChangeAction, error) {
modeenvBefore, err := o.modeenv.Copy()
if err != nil {
return false, fmt.Errorf("cannot copy modeenv: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot copy modeenv: %v", err)
}

// we may be running after a mid-update reboot, where a successful boot
Expand All @@ -453,13 +453,13 @@ func (o *TrustedAssetsUpdateObserver) observeUpdate(bl bootloader.Bootloader, re
// it existed
taBefore, err = o.cache.Add(change.Before, bl.Name(), filepath.Base(relativeTarget))
if err != nil {
return false, err
return gadget.ChangeAbort, err
}
}

ta, err := o.cache.Add(change.After, bl.Name(), filepath.Base(relativeTarget))
if err != nil {
return false, err
return gadget.ChangeAbort, err
}

trustedAssets := &o.modeenv.CurrentTrustedBootAssets
Expand Down Expand Up @@ -490,21 +490,21 @@ func (o *TrustedAssetsUpdateObserver) observeUpdate(bl bootloader.Bootloader, re
// during an update; more entries indicates that the
// same asset name is used multiple times with different
// content
return false, fmt.Errorf("cannot reuse asset name %q", ta.name)
return gadget.ChangeAbort, fmt.Errorf("cannot reuse asset name %q", ta.name)
}
(*trustedAssets)[ta.name] = append((*trustedAssets)[ta.name], ta.hash)
}

if o.modeenv.deepEqual(modeenvBefore) {
return true, nil
return gadget.ChangeApply, nil
}
if err := o.modeenv.Write(); err != nil {
return false, fmt.Errorf("cannot write modeeenv: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot write modeeenv: %v", err)
}
return true, nil
return gadget.ChangeApply, nil
}

func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader, recovery bool, root, relativeTarget string, data *gadget.ContentChange) (bool, error) {
func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader, recovery bool, root, relativeTarget string, data *gadget.ContentChange) (gadget.ContentChangeAction, error) {
trustedAssets := &o.modeenv.CurrentTrustedBootAssets
otherTrustedAssets := o.modeenv.CurrentTrustedRecoveryBootAssets
if recovery {
Expand All @@ -516,7 +516,7 @@ func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader,
hashList, ok := (*trustedAssets)[assetName]
if !ok || len(hashList) == 0 {
// asset not tracked in modeenv
return true, nil
return gadget.ChangeApply, nil
}

// new assets are appended to the list
Expand All @@ -527,20 +527,20 @@ func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader,
if err != nil {
// file may not exist if it was added by the update, that's ok
if !os.IsNotExist(err) {
return false, fmt.Errorf("cannot calculate the digest of current asset: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot calculate the digest of current asset: %v", err)
}
newlyAdded = true
if len(hashList) > 1 {
// we have more than 1 hash of the asset, so we expected
// a previous revision to be restored, but got nothing
// instead
return false, fmt.Errorf("tracked asset %q is unexpectedly missing from disk",
return gadget.ChangeAbort, fmt.Errorf("tracked asset %q is unexpectedly missing from disk",
assetName)
}
} else {
if ondiskHash != expectedOldHash {
// this is unexpected, a different file exists on disk?
return false, fmt.Errorf("unexpected content of existing asset %q", relativeTarget)
return gadget.ChangeAbort, fmt.Errorf("unexpected content of existing asset %q", relativeTarget)
}
}

Expand All @@ -556,7 +556,7 @@ func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader,
// asset revision is not used used elsewhere, we can remove it from the cache
if err := o.cache.Remove(bl.Name(), assetName, newHash); err != nil {
// XXX: should this be a log instead?
return false, fmt.Errorf("cannot remove unused boot asset %v:%v: %v", assetName, newHash, err)
return gadget.ChangeAbort, fmt.Errorf("cannot remove unused boot asset %v:%v: %v", assetName, newHash, err)
}
}

Expand All @@ -568,10 +568,10 @@ func (o *TrustedAssetsUpdateObserver) observeRollback(bl bootloader.Bootloader,
}

if err := o.modeenv.Write(); err != nil {
return false, fmt.Errorf("cannot write modeeenv: %v", err)
return gadget.ChangeAbort, fmt.Errorf("cannot write modeeenv: %v", err)
}

return false, nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the intention expressed by false here (it was anyway not yet used) was wrong? will we even use the value of ContentChangeAction for rollback?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of false here was wrong, I tried to have the code return true for al the cases where it's fine to proceed with the change.

As for the rollback scenario, I don't see any need for inspecting the returned value at this point.

return gadget.ChangeApply, nil
}

// BeforeWrite is called when the update process has been staged for execution.
Expand Down