Merge remote-tracking branch 'origin/master' into bboozzoo/selinux-policy-refactor

7 files changed, 214 insertions, 61 deletions

diff --git a/cmd/snap-confine/ns-support.c b/cmd/snap-confine/ns-support.c
index 36b2fc8e8f..129c7d15a1 100644
--- a/cmd/snap-confine/ns-support.c
+++ b/cmd/snap-confine/ns-support.c
@@ -68,12 +68,6 @@
 **/
 static const char *sc_ns_dir = SC_NS_DIR;
 
-/**
- * Name of the preserved mount namespace associated with SC_NS_DIR
- * and a given group identifier (typically SNAP_NAME).
- **/
-#define SC_NS_MNT_FILE ".mnt"
-
 enum {
	HELPER_CMD_EXIT,
	HELPER_CMD_CAPTURE_MOUNT_NS,
@@ -458,8 +452,7 @@ int sc_join_preserved_ns(struct sc_mount_ns *group, struct sc_apparmor
 {
	// Open the mount namespace file.
	char mnt_fname[PATH_MAX] = { 0 };
-	sc_must_snprintf(mnt_fname, sizeof mnt_fname, "%s%s", group->name,
- SC_NS_MNT_FILE);
+	sc_must_snprintf(mnt_fname, sizeof mnt_fname, "%s.mnt", group->name);
	int mnt_fd SC_CLEANUP(sc_cleanup_close) = -1;
	// NOTE: There is no O_EXCL here because the file can be around but
	// doesn't have to be a mounted namespace.
@@ -622,11 +615,10 @@ static void helper_main(struct sc_mount_ns *group, struct sc_apparmor *apparmor,
 {
	// This is the child process which will capture the mount namespace.
	//
-	// It will do so by bind-mounting the SC_NS_MNT_FILE after the parent
-	// process calls unshare() and finishes setting up the namespace
-	// completely.
-	// Change the hat to a sub-profile that has limited permissions
-	// necessary to accomplish the capture of the mount namespace.
+	// It will do so by bind-mounting the .mnt after the parent process calls
+	// unshare() and finishes setting up the namespace completely. Change the
+	// hat to a sub-profile that has limited permissions necessary to
+	// accomplish the capture of the mount namespace.
	sc_maybe_aa_change_hat(apparmor, "mount-namespace-capture-helper", 0);
	// Configure the child to die as soon as the parent dies. In an odd
	// case where the parent is killed then we don't want to complete our
@@ -688,7 +680,7 @@ static void helper_capture_ns(struct sc_mount_ns *group, pid_t parent)
 
	debug("capturing per-snap mount namespace");
	sc_must_snprintf(src, sizeof src, "/proc/%d/ns/mnt", (int)parent);
-	sc_must_snprintf(dst, sizeof dst, "%s%s", group->name, SC_NS_MNT_FILE);
+	sc_must_snprintf(dst, sizeof dst, "%s.mnt", group->name);
 
	/* Ensure the bind mount destination exists. */
	int fd = open(dst, O_CREAT | O_CLOEXEC | O_NOFOLLOW | O_RDONLY, 0600);
@@ -796,8 +788,7 @@ void sc_discard_preserved_mount_ns(struct sc_mount_ns *group)
	}
	// Unmount ${group_name}.mnt which holds the preserved namespace
	char mnt_fname[PATH_MAX] = { 0 };
-	sc_must_snprintf(mnt_fname, sizeof mnt_fname, "%s%s", group->name,
- SC_NS_MNT_FILE);
+	sc_must_snprintf(mnt_fname, sizeof mnt_fname, "%s.mnt", group->name);
	debug("unmounting preserved mount namespace file %s", mnt_fname);
	if (umount2(mnt_fname, UMOUNT_NOFOLLOW) < 0) {
	switch (errno) {
diff --git a/data/selinux/snappy.if b/data/selinux/snappy.if
index ad1cc55a19..36a16e27f1 100644
--- a/data/selinux/snappy.if
+++ b/data/selinux/snappy.if
@@ -218,20 +218,20 @@ interface(`snappy_stream_connect',`
 ## </param>
 ## <rolecap/>
 #
-interface(`snappy_admin',
- gen_require(`
- type snappy_t, snappy_config_t; 
- type snappy_var_run_t;
- ')
+interface(`snappy_admin',`
+	gen_require(`
+	type snappy_t, snappy_config_t;
+	type snappy_var_run_t;
+	')
 
- allow $1 snappy_t:process signal_perms;
+	allow $1 snappy_t:process signal_perms;
 
- ps_process_pattern($1, snappy_t);
+	ps_process_pattern($1, snappy_t);
 
- admin_pattern($1, snappy_config_t);
+	admin_pattern($1, snappy_config_t);
 
- files_list_pids($1, snappy_var_run_t);
- admin_pattern($1, snappy_var_run_t);
+	files_list_pids($1, snappy_var_run_t);
+	admin_pattern($1, snappy_var_run_t);
 ')
 
 ########################################
diff --git a/snap/info.go b/snap/info.go
index ca933112fb..e52aa49740 100644
--- a/snap/info.go
+++ b/snap/info.go
@@ -225,6 +225,9 @@ type Info struct {
	Plugs map[string]*PlugInfo
	Slots map[string]*SlotInfo
 
+	toplevelPlugs []*PlugInfo
+	toplevelSlots []*SlotInfo
+
	// Plugs or slots with issues (they are not included in Plugs or Slots)
	BadInterfaces map[string]string // slot or plug => message
 
@@ -800,6 +803,8 @@ type HookInfo struct {
 
	Environment strutil.OrderedMap
	CommandChain []string
+
+	Explicit bool
 }
 
 // File returns the path to the *.socket file
@@ -1007,6 +1012,8 @@ func ReadInfo(name string, si *SideInfo) (*Info, error) {
	return nil, &invalidMetaError{Snap: name, Revision: si.Revision, Msg: err.Error()}
	}
 
+	bindImplicitHooks(info)
+
	_, instanceKey := SplitInstanceName(name)
	info.InstanceKey = instanceKey
 
@@ -1071,6 +1078,8 @@ func ReadInfoFromSnapFile(snapf Container, si *SideInfo) (*Info, error) {
	return nil, err
	}
 
+	bindImplicitHooks(info)
+
	err = Validate(info)
	if err != nil {
	return nil, err
diff --git a/snap/info_snap_yaml.go b/snap/info_snap_yaml.go
index ec7e21be90..87c3ed27c5 100644
--- a/snap/info_snap_yaml.go
+++ b/snap/info_snap_yaml.go
@@ -146,16 +146,14 @@ func InfoFromSnapYaml(yamlData []byte) (*Info, error) {
	}
 
	// At this point snap.Plugs and snap.Slots only contain globally-declared
-	// plugs and slots. We're about to change that, but we need to remember the
-	// global ones for later, so save their names.
-	globalPlugNames := make([]string, 0, len(snap.Plugs))
-	for plugName := range snap.Plugs {
-	globalPlugNames = append(globalPlugNames, plugName)
+	// plugs and slots, so copy them for later.
+	snap.toplevelPlugs = make([]*PlugInfo, 0, len(snap.Plugs))
+	snap.toplevelSlots = make([]*SlotInfo, 0, len(snap.Slots))
+	for _, plug := range snap.Plugs {
+	snap.toplevelPlugs = append(snap.toplevelPlugs, plug)
	}
-
-	globalSlotNames := make([]string, 0, len(snap.Slots))
-	for slotName := range snap.Slots {
-	globalSlotNames = append(globalSlotNames, slotName)
+	for _, slot := range snap.Slots {
+	snap.toplevelSlots = append(snap.toplevelSlots, slot)
	}
 
	// Collect all apps, their aliases and hooks
@@ -165,10 +163,10 @@ func InfoFromSnapYaml(yamlData []byte) (*Info, error) {
	setHooksFromSnapYaml(y, snap)
 
	// Bind unbound plugs to all apps and hooks
-	bindUnboundPlugs(globalPlugNames, snap)
+	bindUnboundPlugs(snap)
 
	// Bind unbound slots to all apps and hooks
-	bindUnboundSlots(globalSlotNames, snap)
+	bindUnboundSlots(snap)
 
	// Collect layout elements.
	if y.Layout != nil {
@@ -421,6 +419,7 @@ func setHooksFromSnapYaml(y snapYaml, snap *Info) {
	Name: hookName,
	Environment: yHook.Environment,
	CommandChain: yHook.CommandChain,
+	Explicit: true,
	}
	if len(y.Plugs) > 0 || len(yHook.PlugNames) > 0 {
	hook.Plugs = make(map[string]*PlugInfo)
@@ -468,13 +467,8 @@ func setHooksFromSnapYaml(y snapYaml, snap *Info) {
	}
 }
 
-func bindUnboundPlugs(plugNames []string, snap *Info) error {
-	for _, plugName := range plugNames {
-	plug, ok := snap.Plugs[plugName]
-	if !ok {
-	return fmt.Errorf("no plug named %q", plugName)
-	}
-
+func bindUnboundPlugs(snap *Info) {
+	for plugName, plug := range snap.Plugs {
	// A plug is considered unbound if it isn't being used by any apps
	// or hooks. In which case we bind them to all apps and hooks.
	if len(plug.Apps) == 0 && len(plug.Hooks) == 0 {
@@ -489,17 +483,10 @@ func bindUnboundPlugs(plugNames []string, snap *Info) error {
	}
	}
	}
-
-	return nil
 }
 
-func bindUnboundSlots(slotNames []string, snap *Info) error {
-	for _, slotName := range slotNames {
-	slot, ok := snap.Slots[slotName]
-	if !ok {
-	return fmt.Errorf("no slot named %q", slotName)
-	}
-
+func bindUnboundSlots(snap *Info) {
+	for slotName, slot := range snap.Slots {
	// A slot is considered unbound if it isn't being used by any apps
	// or hooks. In which case we bind them to all apps and hooks.
	if len(slot.Apps) == 0 && len(slot.Hooks) == 0 {
@@ -513,8 +500,41 @@ func bindUnboundSlots(slotNames []string, snap *Info) error {
	}
	}
	}
+}
 
-	return nil
+// bindImplicitHooks binds all global plugs and slots to implicit hooks
+func bindImplicitHooks(snap *Info) {
+	for _, plug := range snap.toplevelPlugs {
+	for hookName, hook := range snap.Hooks {
+	if hook.Explicit {
+	continue
+	}
+	if hook.Plugs == nil {
+	hook.Plugs = make(map[string]*PlugInfo)
+	}
+	hook.Plugs[plug.Name] = plug
+	if plug.Hooks == nil {
+	plug.Hooks = make(map[string]*HookInfo)
+	}
+	plug.Hooks[hookName] = hook
+	}
+	}
+
+	for _, slot := range snap.toplevelSlots {
+	for hookName, hook := range snap.Hooks {
+	if hook.Explicit {
+	continue
+	}
+	if hook.Slots == nil {
+	hook.Slots = make(map[string]*SlotInfo)
+	}
+	hook.Slots[slot.Name] = slot
+	if slot.Hooks == nil {
+	slot.Hooks = make(map[string]*HookInfo)
+	}
+	slot.Hooks[hookName] = hook
+	}
+	}
 }
 
 func convertToSlotOrPlugData(plugOrSlot, name string, data interface{}) (iface, label string, attrs map[string]interface{}, err error) {
diff --git a/snap/info_snap_yaml_test.go b/snap/info_snap_yaml_test.go
index d1e71bff9d..ac9e229995 100644
--- a/snap/info_snap_yaml_test.go
+++ b/snap/info_snap_yaml_test.go
@@ -783,6 +783,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Slots: map[string]*snap.SlotInfo{slot.Name: slot},
+
+	Explicit: true,
	})
 }
 
@@ -816,6 +818,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Slots: map[string]*snap.SlotInfo{slot.Name: slot},
+
+	Explicit: true,
	})
 }
 
@@ -908,6 +912,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Plugs: nil,
+
+	Explicit: true,
	})
 }
 
@@ -956,6 +962,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Plugs: nil,
+
+	Explicit: true,
	})
 }
 
@@ -989,6 +997,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Plugs: map[string]*snap.PlugInfo{plug.Name: plug},
+
+	Explicit: true,
	})
 }
 
@@ -1023,6 +1033,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Plugs: map[string]*snap.PlugInfo{plug.Name: plug},
+
+	Explicit: true,
	})
 }
 
@@ -1057,11 +1069,15 @@ hooks:
	Snap: info,
	Name: "with-plug",
	Plugs: map[string]*snap.PlugInfo{plug.Name: plug},
+
+	Explicit: true,
	})
	c.Assert(withoutPlugHook, DeepEquals, &snap.HookInfo{
	Snap: info,
	Name: "without-plug",
	Plugs: map[string]*snap.PlugInfo{},
+
+	Explicit: true,
	})
 }
 
@@ -1097,6 +1113,8 @@ hooks:
	Snap: info,
	Name: "test-hook",
	Plugs: map[string]*snap.PlugInfo{plug.Name: plug},
+
+	Explicit: true,
	})
 }
 
@@ -1133,6 +1151,8 @@ apps:
	Snap: info,
	Name: "test-hook",
	Plugs: map[string]*snap.PlugInfo{plug.Name: plug},
+
+	Explicit: true,
	})
	c.Assert(app, DeepEquals, &snap.AppInfo{
	Snap: info,
diff --git a/snap/info_test.go b/snap/info_test.go
index 895c47f146..51e550fbf9 100644
--- a/snap/info_test.go
+++ b/snap/info_test.go
@@ -711,7 +711,7 @@ version: 1.0`
	// Verify that the `test-hook` hook has now been loaded, and that it has
	// no associated plugs.
	c.Check(info.Hooks, HasLen, 1)
-	verifyImplicitHook(c, info, "test-hook")
+	verifyImplicitHook(c, info, "test-hook", nil)
	})
 }
 
@@ -722,8 +722,8 @@ version: 1.0`
	// Verify that both hooks have now been loaded, and that neither have any
	// associated plugs.
	c.Check(info.Hooks, HasLen, 2)
-	verifyImplicitHook(c, info, "foo")
-	verifyImplicitHook(c, info, "bar")
+	verifyImplicitHook(c, info, "foo", nil)
+	verifyImplicitHook(c, info, "bar", nil)
	})
 }
 
@@ -736,7 +736,7 @@ version: 1.0`
	s.checkInstalledSnapAndSnapFile(c, yaml, "SNAP", []string{"foo", "bar"}, func(c *C, info *snap.Info) {
	// Verify that only foo has been loaded, not bar
	c.Check(info.Hooks, HasLen, 1)
-	verifyImplicitHook(c, info, "foo")
+	verifyImplicitHook(c, info, "foo", nil)
	})
 }
 
@@ -752,16 +752,125 @@ hooks:
	// no associated plugs. Also verify that the `explicit` hook is still
	// valid.
	c.Check(info.Hooks, HasLen, 2)
-	verifyImplicitHook(c, info, "implicit")
+	verifyImplicitHook(c, info, "implicit", nil)
	verifyExplicitHook(c, info, "explicit", []string{"test-plug"}, []string{"test-slot"})
	})
 }
 
-func verifyImplicitHook(c *C, info *snap.Info, hookName string) {
+func (s *infoSuite) TestReadInfoExplicitHooks(c *C) {
+	yaml := `name: foo
+version: 1.0
+plugs:
+ test-plug:
+slots:
+ test-slot:
+hooks:
+ explicit:
+`
+	s.checkInstalledSnapAndSnapFile(c, yaml, "SNAP", []string{"explicit"}, func(c *C, info *snap.Info) {
+	c.Check(info.Hooks, HasLen, 1)
+	verifyExplicitHook(c, info, "explicit", []string{"test-plug"}, []string{"test-slot"})
+	})
+}
+
+func (s *infoSuite) TestReadInfoImplicitHookWithTopLevelPlugSlots(c *C) {
+	yaml := `name: foo
+version: 1.0
+plugs:
+ test-plug:
+slots:
+ test-slot:
+hooks:
+ explicit:
+ plugs: [test-plug,other-plug]
+ slots: [test-slot,other-slot]
+`
+
+	yaml2 := `name: foo
+version: 1.0
+plugs:
+ test-plug:
+slots:
+ test-slot:
+`
+	s.checkInstalledSnapAndSnapFile(c, yaml, "SNAP", []string{"implicit"}, func(c *C, info *snap.Info) {
+	c.Check(info.Hooks, HasLen, 2)
+	implicitHook := info.Hooks["implicit"]
+	c.Assert(implicitHook, NotNil)
+	c.Assert(implicitHook.Explicit, Equals, false)
+	c.Assert(implicitHook.Plugs, HasLen, 1)
+	c.Assert(implicitHook.Slots, HasLen, 1)
+
+	c.Check(info.Plugs, HasLen, 2)
+	c.Check(info.Slots, HasLen, 2)
+
+	plug := info.Plugs["test-plug"]
+	c.Assert(plug, NotNil)
+	c.Assert(implicitHook.Plugs["test-plug"], DeepEquals, plug)
+
+	slot := info.Slots["test-slot"]
+	c.Assert(slot, NotNil)
+	c.Assert(implicitHook.Slots["test-slot"], DeepEquals, slot)
+
+	explicitHook := info.Hooks["explicit"]
+	c.Assert(explicitHook, NotNil)
+	c.Assert(explicitHook.Explicit, Equals, true)
+	c.Assert(explicitHook.Plugs, HasLen, 2)
+	c.Assert(explicitHook.Slots, HasLen, 2)
+
+	plug = info.Plugs["test-plug"]
+	c.Assert(plug, NotNil)
+	c.Assert(explicitHook.Plugs["test-plug"], DeepEquals, plug)
+
+	slot = info.Slots["test-slot"]
+	c.Assert(slot, NotNil)
+	c.Assert(explicitHook.Slots["test-slot"], DeepEquals, slot)
+	})
+
+	s.checkInstalledSnapAndSnapFile(c, yaml2, "SNAP", []string{"implicit"}, func(c *C, info *snap.Info) {
+	c.Check(info.Hooks, HasLen, 1)
+	implicitHook := info.Hooks["implicit"]
+	c.Assert(implicitHook, NotNil)
+	c.Assert(implicitHook.Explicit, Equals, false)
+	c.Assert(implicitHook.Plugs, HasLen, 1)
+	c.Assert(implicitHook.Slots, HasLen, 1)
+
+	c.Check(info.Plugs, HasLen, 1)
+	c.Check(info.Slots, HasLen, 1)
+
+	plug := info.Plugs["test-plug"]
+	c.Assert(plug, NotNil)
+	c.Assert(implicitHook.Plugs["test-plug"], DeepEquals, plug)
+
+	slot := info.Slots["test-slot"]
+	c.Assert(slot, NotNil)
+	c.Assert(implicitHook.Slots["test-slot"], DeepEquals, slot)
+	})
+
+}
+
+func verifyImplicitHook(c *C, info *snap.Info, hookName string, plugNames []string) {
	hook := info.Hooks[hookName]
	c.Assert(hook, NotNil, Commentf("Expected hooks to contain %q", hookName))
	c.Check(hook.Name, Equals, hookName)
-	c.Check(hook.Plugs, IsNil)
+
+	if len(plugNames) == 0 {
+	c.Check(hook.Plugs, IsNil)
+	}
+
+	for _, plugName := range plugNames {
+	// Verify that the HookInfo and PlugInfo point to each other
+	plug := hook.Plugs[plugName]
+	c.Assert(plug, NotNil, Commentf("Expected hook plugs to contain %q", plugName))
+	c.Check(plug.Name, Equals, plugName)
+	c.Check(plug.Hooks, HasLen, 1)
+	hook = plug.Hooks[hookName]
+	c.Assert(hook, NotNil, Commentf("Expected plug to be associated with hook %q", hookName))
+	c.Check(hook.Name, Equals, hookName)
+
+	// Verify also that the hook plug made it into info.Plugs
+	c.Check(info.Plugs[plugName], DeepEquals, plug)
+	}
 }
 
 func verifyExplicitHook(c *C, info *snap.Info, hookName string, plugNames []string, slotNames []string) {
diff --git a/store/details_v2_test.go b/store/details_v2_test.go
index 16aded1062..085ad48b7e 100644
--- a/store/details_v2_test.go
+++ b/store/details_v2_test.go
@@ -304,6 +304,10 @@ func (s *detailsV2Suite) TestInfoFromStoreSnap(c *C) {
	t := x.Type()
	for i := 0; i < x.NumField(); i++ {
	f := t.Field(i)
+	if f.PkgPath != "" {
+	// not exported, ignore
+	continue
+	}
	v := x.Field(i)
	if f.Anonymous {
	checker(pfx+f.Name+".", v)