o/c/configcore,o/ifacestate: check for presence of handler-service on…

… startup (#14843)

* o/c/configcore,o/ifacestate: check for presence of handler-service on startup

During snapd startup, if the "apparmor-prompting" flag is enabled but
there is no snap with an interfaces-requests-control connection and an
app app declared as a "handler-service", then record a warning that
prompts will be auto-denied until a prompting client is installed.

Such a situation could arise if the user uninstalled the prompting
client snap without disabling the "apparmor-prompting" feature flag, or
if they disconnected the plug for the interfaces-requests-control
interface.

There is a check for the presence of a handler-service client when one
attempts to enable the "apparmor-prompting" feature flag in the first
place: when enabling the flag, if there is no handler-service app
present, or it cannot be successfully started, then the flag is left
disabled. Before this commit, however, there were no subsequent checks
for the presence of a handler-service, so the user would have no
indication of why snaps may be blocking on syscalls or having syscalls
denied. Now, we make this check again at snapd startup, but rather than
disabling prompting, we instead simply record a warning to the user.

Signed-off-by: Oliver Calder <[email protected]>

* o/ifacestate: simplify mocking of function to check handler service presence

Signed-off-by: Oliver Calder <[email protected]>

* o/ifacestate: improve comment around handler service presence error

Co-authored-by: Miguel Pires <[email protected]>

---------

Signed-off-by: Oliver Calder <[email protected]>
Co-authored-by: Miguel Pires <[email protected]>

overlord/configstate/configcore/prompting.go

@@ -26,12 +26,10 @@ import (
 
 	"github.com/snapcore/snapd/client"
 	"github.com/snapcore/snapd/features"
-	"github.com/snapcore/snapd/interfaces"
 	"github.com/snapcore/snapd/overlord/configstate/config"
 	"github.com/snapcore/snapd/overlord/ifacestate"
 	"github.com/snapcore/snapd/overlord/restart"
 	"github.com/snapcore/snapd/overlord/servicestate"
-	"github.com/snapcore/snapd/overlord/snapstate"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/release"
 	"github.com/snapcore/snapd/snap"
@@ -88,45 +86,7 @@ func findPromptingRequestsHandlers(st *state.State) ([]*snap.AppInfo, error) {
 	st.Lock()
 	defer st.Unlock()
 
-	conns, err := ifacestate.ConnectionStates(st)
-	if err != nil {
-		return nil, fmt.Errorf("internal error: cannot get connections: %w", err)
-	}
-
-	var handlers []*snap.AppInfo
-
-	for connId, connState := range conns {
-		if connState.Interface != "snap-interfaces-requests-control" || !connState.Active() {
-			continue
-		}
-
-		connRef, err := interfaces.ParseConnRef(connId)
-		if err != nil {
-			return nil, err
-		}
-
-		handler, ok := connState.StaticPlugAttrs["handler-service"].(string)
-		if !ok {
-			// does not have a handler service
-			continue
-		}
-
-		sn := connRef.PlugRef.Snap
-		si, err := snapstate.CurrentInfo(st, sn)
-		if err != nil {
-			return nil, err
-		}
-
-		// this should not fail as plug's before prepare should have validated that such app exists
-		app := si.Apps[handler]
-		if app == nil {
-			return nil, fmt.Errorf("internal error: cannot find app %q in snap %q", app, sn)
-		}
-
-		handlers = append(handlers, app)
-	}
-
-	return handlers, nil
+	return ifacestate.InterfacesRequestsControlHandlerServices(st)
 }
 
 // Trigger a security profile regeneration by restarting snapd if the

overlord/ifacestate/export_test.go

@@ -140,6 +140,17 @@ func MockAssessAppArmorPrompting(new func(m *InterfaceManager) bool) (restore fu
 	return testutil.Mock(&assessAppArmorPrompting, new)
 }
 
+func MockInterfacesRequestsControlHandlerServicePresent(new func(m *InterfaceManager) (bool, error)) (restore func()) {
+	return testutil.Mock(&interfacesRequestsControlHandlerServicePresent, new)
+}
+
+func CallInterfacesRequestsControlHandlerServicePresent(s *state.State) (bool, error) {
+	manager := &InterfaceManager{
+		state: s,
+	}
+	return interfacesRequestsControlHandlerServicePresent(manager)
+}
+
 func MockUDevInitRetryTimeout(t time.Duration) (restore func()) {
 	old := udevInitRetryTimeout
 	udevInitRetryTimeout = t

overlord/ifacestate/ifacemgr.go

@@ -205,6 +205,17 @@ func (m *InterfaceManager) StartUp() error {
 	}
 
 	if m.useAppArmorPrompting {
+		// Check if there is at least one snap on the system which has a
+		// connection using the "snap-interfaces-requests-control" plug
+		// with a "handler-service" attribute declared.
+		present, err := interfacesRequestsControlHandlerServicePresent(m)
+		if err != nil {
+			// Internal error, should not occur
+			logger.Noticef("failed to check the presence of a interfaces-requests-control handler service: %v", err)
+		} else if !present {
+			m.state.AddWarning(`"apparmor-prompting" feature flag enabled but no prompting client is present; requests will be auto-denied until a prompting client is installed`, nil)
+		}
+
 		func() {
 			// Must not hold state lock while starting interfaces requests
 			// manager, so that notices can be recorded if needed.
@@ -558,8 +569,28 @@ func (m *InterfaceManager) initUDevMonitor() error {
 	return nil
 }
 
-// initInterfacesRequestsManager should only be called if prompting is
-// supported and enabled.
+// interfacesRequestsControlHandlerServicePresent returns true if there is at
+// least one snap which has a "snap-interfaces-requests-control" connection
+// with an app declared by the "handler-service" attribute.
+//
+// The caller must ensure that the state lock is held.
+var interfacesRequestsControlHandlerServicePresent = func(m *InterfaceManager) (bool, error) {
+	handlers, err := InterfacesRequestsControlHandlerServices(m.state)
+	if err != nil {
+		return false, err
+	}
+	return len(handlers) > 0, nil
+}
+
+// initInterfacesRequestsManager initializes the prompting backends which make
+// up the interfaces requests manager.
+//
+// This function should only be called if prompting is supported and enabled,
+// and at least one installed snap has a "snap-interfaces-requests-control"
+// connection with the "handler-service" attribute declared.
+//
+// The state lock must not be held when this function is called, so that
+// notices can be recorded if necessary.
 func (m *InterfaceManager) initInterfacesRequestsManager() error {
 	m.interfacesRequestsManagerMu.Lock()
 	defer m.interfacesRequestsManagerMu.Unlock()

overlord/ifacestate/ifacestate.go

@@ -577,3 +577,50 @@ func OnSnapLinkageChanged(st *state.State, snapsup *snapstate.SnapSetup) error {
 	snapstate.Set(st, instanceName, &snapst)
 	return nil
 }
+
+// InterfacesRequestsControlHandlerServices returns the list of all apps which
+// are defined as "handler-service" for a snap which has a connected plug for
+// the "snap-interfaces-requests-control" interface.
+//
+// The caller must ensure that the given state is locked.
+func InterfacesRequestsControlHandlerServices(st *state.State) ([]*snap.AppInfo, error) {
+	conns, err := ConnectionStates(st)
+	if err != nil {
+		return nil, fmt.Errorf("internal error: cannot get connections: %w", err)
+	}
+
+	var handlers []*snap.AppInfo
+
+	for connId, connState := range conns {
+		if connState.Interface != "snap-interfaces-requests-control" || !connState.Active() {
+			continue
+		}
+
+		connRef, err := interfaces.ParseConnRef(connId)
+		if err != nil {
+			return nil, err
+		}
+
+		handler, ok := connState.StaticPlugAttrs["handler-service"].(string)
+		if !ok {
+			// does not have a handler service
+			continue
+		}
+
+		sn := connRef.PlugRef.Snap
+		si, err := snapstate.CurrentInfo(st, sn)
+		if err != nil {
+			return nil, err
+		}
+
+		// this should not fail as the plug's BeforePrepare should have validated that such an app exists
+		app := si.Apps[handler]
+		if app == nil {
+			return nil, fmt.Errorf("internal error: cannot find app %q in snap %q", app, sn)
+		}
+
+		handlers = append(handlers, app)
+	}
+
+	return handlers, nil
+}