overlord/ifacestate: fix missing security setup for connected slot

interfaces/repo.go

@@ -747,35 +747,30 @@ func (r *Repository) RemoveSnap(snapName string) error {
 
 // DisconnectSnap disconnects all the connections to and from a given snap.
 //
-// The return value is a list of names that were affected.
-func (r *Repository) DisconnectSnap(snapName string) ([]string, error) {
+// The return value is a set of snap names that were affected.
+func (r *Repository) DisconnectSnap(snapName string) (map[string]bool, error) {
 	r.m.Lock()
 	defer r.m.Unlock()
 
-	seen := make(map[*snap.Info]bool)
+	affectedSnapSet := make(map[string]bool)
 
 	for _, plug := range r.plugs[snapName] {
 		for slot := range r.plugSlots[plug] {
 			r.disconnect(plug, slot)
-			seen[plug.Snap] = true
-			seen[slot.Snap] = true
+			affectedSnapSet[plug.Snap.Name()] = true
+			affectedSnapSet[slot.Snap.Name()] = true
 		}
 	}
 
 	for _, slot := range r.slots[snapName] {
 		for plug := range r.slotPlugs[slot] {
 			r.disconnect(plug, slot)
-			seen[plug.Snap] = true
-			seen[slot.Snap] = true
+			affectedSnapSet[plug.Snap.Name()] = true
+			affectedSnapSet[slot.Snap.Name()] = true
 		}
 	}
 
-	result := make([]string, 0, len(seen))
-	for info := range seen {
-		result = append(result, info.Name())
-	}
-	sort.Strings(result)
-	return result, nil
+	return affectedSnapSet, nil
 }
 
 // AutoConnectCandidates finds and returns viable auto-connection candidates
@@ -808,3 +803,18 @@ func (r *Repository) AutoConnectCandidates(plugSnapName, plugName string, policy
 	}
 	return candidates
 }
+
+// AddConnectedSnaps finds the set of snaps corresponding to the plugSlots of the given snapName
+// and adds them to the given snapSet
+func (r *Repository) AddConnectedSnaps(snapName string, snapSet map[string]bool) error {
+	r.m.Lock()
+	defer r.m.Unlock()
+
+	for _, plug := range r.plugs[snapName] {
+		for slot := range r.plugSlots[plug] {
+			snapSet[slot.Snap.Name()] = true
+		}
+	}
+
+	return nil
+}

interfaces/repo_test.go

@@ -1337,15 +1337,24 @@ func (s *DisconnectSnapSuite) TestNotConnected(c *C) {
 	c.Check(affected, HasLen, 0)
 }
 
+func keysFor(aMap map[string]bool) []string {
+	keys := make([]string, 0, len(aMap))
+	for key := range aMap {
+		keys = append(keys, key)
+	}
+	return keys
+}
+
 func (s *DisconnectSnapSuite) TestOutgoingConnection(c *C) {
 	connRef := ConnRef{PlugRef: PlugRef{Snap: "s1", Name: "iface-a"}, SlotRef: SlotRef{Snap: "s2", Name: "iface-a"}}
 	err := s.repo.Connect(connRef)
 	c.Assert(err, IsNil)
 	// Disconnect s1 with which has an outgoing connection to s2
 	affected, err := s.repo.DisconnectSnap("s1")
 	c.Assert(err, IsNil)
-	c.Check(affected, testutil.Contains, "s1")
-	c.Check(affected, testutil.Contains, "s2")
+	affectedNames := keysFor(affected)
+	c.Check(affectedNames, testutil.Contains, "s1")
+	c.Check(affectedNames, testutil.Contains, "s2")
 }
 
 func (s *DisconnectSnapSuite) TestIncomingConnection(c *C) {
@@ -1355,8 +1364,9 @@ func (s *DisconnectSnapSuite) TestIncomingConnection(c *C) {
 	// Disconnect s1 with which has an incoming connection from s2
 	affected, err := s.repo.DisconnectSnap("s1")
 	c.Assert(err, IsNil)
-	c.Check(affected, testutil.Contains, "s1")
-	c.Check(affected, testutil.Contains, "s2")
+	affectedNames := keysFor(affected)
+	c.Check(affectedNames, testutil.Contains, "s1")
+	c.Check(affectedNames, testutil.Contains, "s2")
 }
 
 func (s *DisconnectSnapSuite) TestCrossConnection(c *C) {
@@ -1370,8 +1380,9 @@ func (s *DisconnectSnapSuite) TestCrossConnection(c *C) {
 		c.Assert(err, IsNil)
 		affected, err := s.repo.DisconnectSnap(snapName)
 		c.Assert(err, IsNil)
-		c.Check(affected, testutil.Contains, "s1")
-		c.Check(affected, testutil.Contains, "s2")
+		affectedNames := keysFor(affected)
+		c.Check(affectedNames, testutil.Contains, "s1")
+		c.Check(affectedNames, testutil.Contains, "s2")
 	}
 }
 

overlord/ifacestate/handlers.go

@@ -44,11 +44,11 @@ func confinementOptions(flags snapstate.Flags) interfaces.ConfinementOptions {
 	}
 }
 
-func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap string, affectedSnaps []string) error {
+func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap string, affectedSnaps map[string]bool) error {
 	st := task.State()
 
 	// Setup security of the affected snaps.
-	for _, affectedSnapName := range affectedSnaps {
+	for affectedSnapName := range affectedSnaps {
 		// the snap that triggered the change needs to be skipped
 		if affectedSnapName == affectingSnap {
 			continue
@@ -127,6 +127,13 @@ func (m *InterfaceManager) setupProfilesForSnap(task *state.Task, _ *tomb.Tomb,
 	if err := m.autoConnect(task, snapName, nil); err != nil {
 		return err
 	}
+
+	// Now find all snaps affected by any autoconnection to make sure
+	// their security is also setup
+	if err := m.repo.AddConnectedSnaps(snapName, affectedSnaps); err != nil {
+		return err
+	}
+
 	if err := setupSnapSecurity(task, snapInfo, opts, m.repo); err != nil {
 		return err
 	}

overlord/ifacestate/ifacemgr_test.go

@@ -994,6 +994,46 @@ func (s *interfaceManagerSuite) TestSetupProfilesUsesFreshSnapInfo(c *C) {
 	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Revision, Equals, coreSnapInfo.Revision)
 }
 
+// setup-profiles needs to setup security for connected slots after autoconnection
+func (s *interfaceManagerSuite) TestAutoConnectSetupSecurityForConnectedSlots(c *C) {
+	// Add an OS snap.
+	coreSnapInfo := s.mockSnap(c, osSnapYaml)
+
+	// Initialize the manager. This registers the OS snap.
+	mgr := s.manager(c)
+
+	// Add a sample snap with a "network" plug which should be auto-connected.
+	snapInfo := s.mockSnap(c, sampleSnapYaml)
+
+	// Run the setup-snap-security task and let it finish.
+	change := s.addSetupSnapSecurityChange(c, &snapstate.SnapSetup{
+		SideInfo: &snap.SideInfo{
+			RealName: snapInfo.Name(),
+			Revision: snapInfo.Revision,
+		},
+	})
+	mgr.Ensure()
+	mgr.Wait()
+	mgr.Stop()
+
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	// Ensure that the task succeeded.
+	c.Assert(change.Err(), IsNil)
+	c.Assert(change.Status(), Equals, state.DoneStatus)
+
+	// Ensure that both snaps were setup correctly.
+	c.Assert(s.secBackend.SetupCalls, HasLen, 2)
+	c.Assert(s.secBackend.RemoveCalls, HasLen, 0)
+	// The sample snap was setup, with the correct new revision.
+	c.Check(s.secBackend.SetupCalls[0].SnapInfo.Name(), Equals, snapInfo.Name())
+	c.Check(s.secBackend.SetupCalls[0].SnapInfo.Revision, Equals, snapInfo.Revision)
+	// The OS snap was setup (because its connected to sample snap).
+	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Name(), Equals, coreSnapInfo.Name())
+	c.Check(s.secBackend.SetupCalls[1].SnapInfo.Revision, Equals, coreSnapInfo.Revision)
+}
+
 func (s *interfaceManagerSuite) TestDoDiscardConnsPlug(c *C) {
 	s.testDoDicardConns(c, "consumer")
 }