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.

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

interfaces: add cifs-mount interface #5340

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Expand Up @@ -19,33 +19,33 @@

package builtin

const cifsMountControlSummary = `allows mounting and unmounting CIFS filesystems`
const cifsMountSummary = `allows mounting and unmounting CIFS filesystems`

const cifsMountControlBaseDeclarationSlots = `
cifs-mount-control:
const cifsMountBaseDeclarationSlots = `
cifs-mount:
allow-installation:
slot-snap-type:
- core
deny-auto-connection: true
`

const cifsMountControlConnectedPlugSecComp = `
const cifsMountConnectedPlugSecComp = `
# Description: Allow mount and umount syscall access.

Choose a reason for hiding this comment

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

Can you remove this newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

mount
umount
umount2
`

const cifsMountControlConnectedPlugAppArmor = `
const cifsMountConnectedPlugAppArmor = `
# Description: Allow mounting and unmounting CIFS filesystems.

# Required for mounts and unmounts
capability sys_admin,

# Allow mounts to our snap-specific writable directories
mount fstype=cifs ** -> /var/snap/@{SNAP_NAME}/@{SNAP_REVISION}/{,**},
mount fstype=cifs ** -> /var/snap/@{SNAP_NAME}/common/{,**},
mount fstype=cifs //** -> /var/snap/@{SNAP_NAME}/@{SNAP_REVISION}/{,**},
mount fstype=cifs //** -> /var/snap/@{SNAP_NAME}/common/{,**},

Choose a reason for hiding this comment

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

Can you comment that this actually works for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I tested it with my minimal test snap and everything seemed to work fine:

https://github.com/datenhahn/devtools-cifs-mount-control


# NOTE: due to LP: #1613403, fstype is not mediated and as such, these rules
# allow, for example, unmounting bind mounts from the content interface
Expand All @@ -58,17 +58,20 @@ umount /var/snap/@{SNAP_NAME}/common/{,**},
# decide to allow access (deny rules have precedence).
#

Choose a reason for hiding this comment

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

Can you remove this blank comment line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

# - https://github.com/snapcore/snapd/pull/5340#issuecomment-398071797
# - https://forum.snapcraft.io/t/namespace-awareness-of-run-mount-utab-and-libmount/5987`
# - https://forum.snapcraft.io/t/namespace-awareness-of-run-mount-utab-and-libmount/5987
#

Choose a reason for hiding this comment

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

Can you remove this blank comment line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

#deny /run/mount/utab w,
`

func init() {
registerIface(&commonInterface{
name: "cifs-mount-control",
summary: cifsMountControlSummary,
name: "cifs-mount",
summary: cifsMountSummary,
implicitOnCore: true,
implicitOnClassic: true,
baseDeclarationSlots: cifsMountControlBaseDeclarationSlots,
connectedPlugAppArmor: cifsMountControlConnectedPlugAppArmor,
connectedPlugSecComp: cifsMountControlConnectedPlugSecComp,
baseDeclarationSlots: cifsMountBaseDeclarationSlots,
connectedPlugAppArmor: cifsMountConnectedPlugAppArmor,
connectedPlugSecComp: cifsMountConnectedPlugSecComp,
reservedForOS: true,
})
}
Expand Up @@ -30,64 +30,64 @@ import (
"github.com/snapcore/snapd/testutil"
)

type CifsMountControlInterfaceSuite struct {
type CifsMountInterfaceSuite struct {
iface interfaces.Interface
slotInfo *snap.SlotInfo
slot *interfaces.ConnectedSlot
plugInfo *snap.PlugInfo
plug *interfaces.ConnectedPlug
}

const cifsMountControlConsumerYaml = `name: consumer
const cifsMountConsumerYaml = `name: consumer
version: 0
apps:
app:
plugs: [cifs-mount-control]
plugs: [cifs-mount]
`

const cifsMountControlCoreYaml = `name: core
const cifsMountCoreYaml = `name: core
version: 0
type: os
slots:
cifs-mount-control:
cifs-mount:
`

var _ = Suite(&CifsMountControlInterfaceSuite{
iface: builtin.MustInterface("cifs-mount-control"),
var _ = Suite(&CifsMountInterfaceSuite{
iface: builtin.MustInterface("cifs-mount"),
})

func (s *CifsMountControlInterfaceSuite) SetUpTest(c *C) {
s.plug, s.plugInfo = MockConnectedPlug(c, cifsMountControlConsumerYaml, nil, "cifs-mount-control")
s.slot, s.slotInfo = MockConnectedSlot(c, cifsMountControlCoreYaml, nil, "cifs-mount-control")
func (s *CifsMountInterfaceSuite) SetUpTest(c *C) {
s.plug, s.plugInfo = MockConnectedPlug(c, cifsMountConsumerYaml, nil, "cifs-mount")
s.slot, s.slotInfo = MockConnectedSlot(c, cifsMountCoreYaml, nil, "cifs-mount")
}

func (s *CifsMountControlInterfaceSuite) TestName(c *C) {
c.Assert(s.iface.Name(), Equals, "cifs-mount-control")
func (s *CifsMountInterfaceSuite) TestName(c *C) {
c.Assert(s.iface.Name(), Equals, "cifs-mount")
}

func (s *CifsMountControlInterfaceSuite) TestSanitizeSlot(c *C) {
func (s *CifsMountInterfaceSuite) TestSanitizeSlot(c *C) {
c.Assert(interfaces.BeforePrepareSlot(s.iface, s.slotInfo), IsNil)
slot := &snap.SlotInfo{
Snap: &snap.Info{SuggestedName: "some-snap"},
Name: "cifs-mount-control",
Interface: "cifs-mount-control",
Name: "cifs-mount",
Interface: "cifs-mount",
}
c.Assert(interfaces.BeforePrepareSlot(s.iface, slot), ErrorMatches,
"cifs-mount-control slots are reserved for the core snap")
"cifs-mount slots are reserved for the core snap")
}

func (s *CifsMountControlInterfaceSuite) TestSanitizePlug(c *C) {
func (s *CifsMountInterfaceSuite) TestSanitizePlug(c *C) {
c.Assert(interfaces.BeforePreparePlug(s.iface, s.plugInfo), IsNil)
}

func (s *CifsMountControlInterfaceSuite) TestAppArmorSpec(c *C) {
func (s *CifsMountInterfaceSuite) TestAppArmorSpec(c *C) {
spec := &apparmor.Specification{}
c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil)
c.Assert(spec.SecurityTags(), DeepEquals, []string{"snap.consumer.app"})
c.Assert(spec.SnippetForTag("snap.consumer.app"), testutil.Contains, `/run/mount/utab`)
c.Assert(spec.SnippetForTag("snap.consumer.app"), testutil.Contains, `#deny /run/mount/utab w,`)
}

func (s *CifsMountControlInterfaceSuite) TestSecCompSpec(c *C) {
func (s *CifsMountInterfaceSuite) TestSecCompSpec(c *C) {
spec := &seccomp.Specification{}
c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil)
c.Assert(spec.SecurityTags(), DeepEquals, []string{"snap.consumer.app"})
Expand All @@ -96,14 +96,14 @@ func (s *CifsMountControlInterfaceSuite) TestSecCompSpec(c *C) {
c.Assert(spec.SnippetForTag("snap.consumer.app"), testutil.Contains, "umount2\n")

Choose a reason for hiding this comment

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

Just one of these is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understood your comment correctly. I changed it and only kept the umount2 assert (it is most specific for the cifs mount util). If one assertion is true all settings were applied successfully anyway, that was your point, right?

}

func (s *CifsMountControlInterfaceSuite) TestStaticInfo(c *C) {
func (s *CifsMountInterfaceSuite) TestStaticInfo(c *C) {
si := interfaces.StaticInfoOf(s.iface)
c.Assert(si.ImplicitOnCore, Equals, true)
c.Assert(si.ImplicitOnClassic, Equals, true)
c.Assert(si.Summary, Equals, `allows to mount and unmount CIFS shares`)
c.Assert(si.BaseDeclarationSlots, testutil.Contains, "cifs-mount-control")
c.Assert(si.Summary, Equals, `allows mounting and unmounting CIFS filesystems`)
c.Assert(si.BaseDeclarationSlots, testutil.Contains, "cifs-mount")
}

func (s *CifsMountControlInterfaceSuite) TestInterfaces(c *C) {
func (s *CifsMountInterfaceSuite) TestInterfaces(c *C) {
c.Check(builtin.Interfaces(), testutil.DeepContains, s.iface)
}
4 changes: 2 additions & 2 deletions tests/lib/snaps/test-snapd-policy-app-consumer/meta/snap.yaml
Expand Up @@ -41,9 +41,9 @@ apps:
camera:
command: bin/run
plugs: [ camera ]
cifs-mount-control:
cifs-mount:
command: bin/run
plugs: [ cifs-mount-control ]
plugs: [ cifs-mount ]
classic-support:
command: bin/run
plugs: [ classic-support ]
Expand Down