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

interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers #6844

Merged
merged 2 commits into from May 9, 2019
Merged

interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers #6844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a12dd62
Special-case "snapd" in sanitizeSlotReservedForOSOrGadget and sanitiz…
stolowski May 9, 2019
a7b2273
Added a minimal test.
stolowski May 9, 2019
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
4 changes: 2 additions & 2 deletions interfaces/builtin/utils.go
View file
Open in desktop
Expand Up @@ -75,15 +75,15 @@ func plugAppLabelExpr(plug *interfaces.ConnectedPlug) string {

// sanitizeSlotReservedForOS checks if slot is of type os.
func sanitizeSlotReservedForOS(iface interfaces.Interface, slot *snap.SlotInfo) error {
if slot.Snap.Type != snap.TypeOS {
if slot.Snap.Type != snap.TypeOS && slot.Snap.InstanceName() != "snapd" {
Copy link
Collaborator

@zyga zyga May 9, 2019 •
edited

Choose a reason for hiding this comment

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

Should we use snap ID here instead? Though this will interfere with self-built snapd.
Is this in sync with how the policy checker does a similar verification?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. If I read it correctly, policy checker indeed uses snap ID (snapIDSnapd function in policy helpers.go). Note though, policy checker is in any case effective, and this temporary quickfix is to just stop custom sanitize from choking before it's removed completely. So If security is a concern, then policy checker takes care of it anyway. I hope that makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The policy checker is not considered when doing --dangerous installations so perhaps using snap name here is correct.

return fmt.Errorf("%s slots are reserved for the core snap", iface.Name())
}
return nil
}

// sanitizeSlotReservedForOSOrGadget checks if the slot is of type os or gadget.
func sanitizeSlotReservedForOSOrGadget(iface interfaces.Interface, slot *snap.SlotInfo) error {
if slot.Snap.Type != snap.TypeOS && slot.Snap.Type != snap.TypeGadget {
if slot.Snap.Type != snap.TypeOS && slot.Snap.Type != snap.TypeGadget && slot.Snap.InstanceName() != "snapd" {
return fmt.Errorf("%s slots are reserved for the core and gadget snaps", iface.Name())
}
return nil
Expand Down
4 changes: 4 additions & 0 deletions interfaces/builtin/utils_test.go
View file
Open in desktop
Expand Up @@ -35,26 +35,30 @@ type utilsSuite struct {
iface interfaces.Interface
slotOS *snap.SlotInfo
slotApp *snap.SlotInfo
slotSnapd *snap.SlotInfo
slotGadget *snap.SlotInfo
}

var _ = Suite(&utilsSuite{
iface: &ifacetest.TestInterface{InterfaceName: "iface"},
slotOS: &snap.SlotInfo{Snap: &snap.Info{Type: snap.TypeOS}},
slotApp: &snap.SlotInfo{Snap: &snap.Info{Type: snap.TypeApp}},
slotSnapd: &snap.SlotInfo{Snap: &snap.Info{Type: snap.TypeApp, SuggestedName: "snapd"}},
slotGadget: &snap.SlotInfo{Snap: &snap.Info{Type: snap.TypeGadget}},
})

func (s *utilsSuite) TestSanitizeSlotReservedForOS(c *C) {
errmsg := "iface slots are reserved for the core snap"
c.Assert(builtin.SanitizeSlotReservedForOS(s.iface, s.slotOS), IsNil)
c.Assert(builtin.SanitizeSlotReservedForOS(s.iface, s.slotSnapd), IsNil)
c.Assert(builtin.SanitizeSlotReservedForOS(s.iface, s.slotApp), ErrorMatches, errmsg)
c.Assert(builtin.SanitizeSlotReservedForOS(s.iface, s.slotGadget), ErrorMatches, errmsg)
}

func (s *utilsSuite) TestSanitizeSlotReservedForOSOrGadget(c *C) {
errmsg := "iface slots are reserved for the core and gadget snaps"
c.Assert(builtin.SanitizeSlotReservedForOSOrGadget(s.iface, s.slotOS), IsNil)
c.Assert(builtin.SanitizeSlotReservedForOSOrGadget(s.iface, s.slotSnapd), IsNil)
c.Assert(builtin.SanitizeSlotReservedForOSOrGadget(s.iface, s.slotApp), ErrorMatches, errmsg)
c.Assert(builtin.SanitizeSlotReservedForOSOrGadget(s.iface, s.slotGadget), IsNil)
}
Expand Down