diff --git a/overlord/configstate/configcore/prompting.go b/overlord/configstate/configcore/prompting.go index 20b77b300df..6fff006fd83 100644 --- a/overlord/configstate/configcore/prompting.go +++ b/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 diff --git a/overlord/ifacestate/export_test.go b/overlord/ifacestate/export_test.go index 838800234a2..21449893ea4 100644 --- a/overlord/ifacestate/export_test.go +++ b/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 diff --git a/overlord/ifacestate/ifacemgr.go b/overlord/ifacestate/ifacemgr.go index 008846f9d64..db59ce80ab9 100644 --- a/overlord/ifacestate/ifacemgr.go +++ b/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() diff --git a/overlord/ifacestate/ifacestate.go b/overlord/ifacestate/ifacestate.go index ab2e5107233..bbfac2b6252 100644 --- a/overlord/ifacestate/ifacestate.go +++ b/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 +} diff --git a/overlord/ifacestate/ifacestate_test.go b/overlord/ifacestate/ifacestate_test.go index a20c2d11b76..f065cf7defb 100644 --- a/overlord/ifacestate/ifacestate_test.go +++ b/overlord/ifacestate/ifacestate_test.go @@ -374,6 +374,12 @@ func (s *interfaceManagerSuite) TestSmokeAppArmorPromptingEnabled(c *C) { return true }) defer restore() + checkCount := 0 + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + checkCount++ + return true, nil + }) + defer restore() createCount := 0 fakeManager := &apparmorprompting.InterfacesRequestsManager{} restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { @@ -397,6 +403,7 @@ func (s *interfaceManagerSuite) TestSmokeAppArmorPromptingEnabled(c *C) { defer restore() mgr := s.manager(c) + c.Check(checkCount, Equals, 1) c.Check(createCount, Equals, 1) c.Check(mgr.AppArmorPromptingRunning(), Equals, true) @@ -420,11 +427,19 @@ func (s *interfaceManagerSuite) TestSmokeAppArmorPromptingDisabled(c *C) { return false }) defer restore() + checkCount := 0 + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + c.Errorf("unexpectedly called m.interfacesRequestsControlHandlerServicePresent") + checkCount++ + return true, nil + }) + defer restore() createCount := 0 fakeManager := &apparmorprompting.InterfacesRequestsManager{} restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { + c.Errorf("unexpectedly called m.initInterfacesRequestsManager") createCount++ - return fakeManager, fmt.Errorf("should not have been called") + return fakeManager, nil }) defer restore() stopCount := 0 @@ -435,6 +450,7 @@ func (s *interfaceManagerSuite) TestSmokeAppArmorPromptingDisabled(c *C) { defer restore() mgr := s.manager(c) + c.Check(checkCount, Equals, 0) c.Check(createCount, Equals, 0) c.Check(mgr.AppArmorPromptingRunning(), Equals, false) @@ -7003,12 +7019,112 @@ func (s *interfaceManagerSuite) TestConnectHandlesAutoconnect(c *C) { }) } +func (s *interfaceManagerSuite) TestInterfacesRequestsManagerNoHandlerService(c *C) { + restore := ifacestate.MockAssessAppArmorPrompting(func(m *ifacestate.InterfaceManager) bool { + return true + }) + defer restore() + + checkCount := 0 + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + checkCount++ + return false, nil + }) + defer restore() + + createCount := 0 + fakeManager := &apparmorprompting.InterfacesRequestsManager{} + restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { + createCount++ + return fakeManager, nil + }) + defer restore() + + mgr, err := ifacestate.Manager(s.state, nil, s.o.TaskRunner(), nil, nil) + c.Assert(err, IsNil) + + logbuf, restore := logger.MockLogger() + defer restore() + + err = mgr.StartUp() + c.Check(err, IsNil) + + // Check that lack of handler services does not deactivate prompting + running := mgr.AppArmorPromptingRunning() + c.Check(running, Equals, true) + + c.Check(checkCount, Equals, 1) + c.Check(createCount, Equals, 1) + + logger.WithLoggerLock(func() { + logStr := logbuf.String() + c.Check(logStr, Not(testutil.Contains), "failed to check the presence of a interfaces-requests-control handler service") + c.Check(logStr, Not(testutil.Contains), "failed to start interfaces requests manager") + }) + + s.state.Lock() + defer s.state.Unlock() + warns := s.state.AllWarnings() + c.Check(warns, HasLen, 1) + c.Check(warns[0].String(), Matches, `"apparmor-prompting" feature flag enabled but no prompting client is present; requests will be auto-denied until a prompting client is installed`) +} + +func (s *interfaceManagerSuite) TestInterfacesRequestsManagerHandlerServicePresentError(c *C) { + restore := ifacestate.MockAssessAppArmorPrompting(func(m *ifacestate.InterfaceManager) bool { + return true + }) + defer restore() + + checkError := fmt.Errorf("custom error") + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + return false, checkError + }) + defer restore() + + createCount := 0 + fakeManager := &apparmorprompting.InterfacesRequestsManager{} + restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { + createCount++ + return fakeManager, nil + }) + defer restore() + + mgr, err := ifacestate.Manager(s.state, nil, s.o.TaskRunner(), nil, nil) + c.Assert(err, IsNil) + + logbuf, restore := logger.MockLogger() + defer restore() + + err = mgr.StartUp() + c.Check(err, IsNil) + + // Check that error while checking for handler services does not deactivate prompting + running := mgr.AppArmorPromptingRunning() + c.Check(running, Equals, true) + + c.Check(createCount, Equals, 1) + + logger.WithLoggerLock(func() { + c.Check(logbuf.String(), testutil.Contains, "failed to check the presence of a interfaces-requests-control handler service") + }) + + s.state.Lock() + defer s.state.Unlock() + warns := s.state.AllWarnings() + c.Check(warns, HasLen, 0) +} + func (s *interfaceManagerSuite) TestInitInterfacesRequestsManagerError(c *C) { restore := ifacestate.MockAssessAppArmorPrompting(func(m *ifacestate.InterfaceManager) bool { return true }) defer restore() + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + return true, nil + }) + defer restore() + createError := fmt.Errorf("custom error") restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { return nil, createError @@ -7044,6 +7160,10 @@ func (s *interfaceManagerSuite) TestStopInterfacesRequestsManagerError(c *C) { return true }) defer restore() + restore = ifacestate.MockInterfacesRequestsControlHandlerServicePresent(func(m *ifacestate.InterfaceManager) (bool, error) { + return true, nil + }) + defer restore() fakeManager := &apparmorprompting.InterfacesRequestsManager{} restore = ifacestate.MockCreateInterfacesRequestsManager(func(s *state.State) (*apparmorprompting.InterfacesRequestsManager, error) { return fakeManager, nil @@ -10853,3 +10973,175 @@ version: 1.0 c.Assert(calls[3].AppSet.Components(), HasLen, 1) c.Assert(calls[3].AppSet.Components()[0].Revision, Equals, snap.R(2)) } + +func (s *interfaceManagerSuite) TestInterfacesRequestsControlHandlerServicesNone(c *C) { + s.mockSnapd(c) + + s.state.Lock() + defer s.state.Unlock() + handlers, err := ifacestate.InterfacesRequestsControlHandlerServices(s.state) + c.Check(err, IsNil) + c.Check(handlers, HasLen, 0) + + present, err := ifacestate.CallInterfacesRequestsControlHandlerServicePresent(s.state) + c.Check(err, IsNil) + c.Check(present, Equals, false) +} + +func (s *interfaceManagerSuite) TestInterfacesRequestsControlHandlerServicesManyButNoHandlerApp(c *C) { + s.mockSnapd(c) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap1", hasHandler: false}) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap2", hasHandler: false}) + + s.state.Lock() + defer s.state.Unlock() + handlers, err := ifacestate.InterfacesRequestsControlHandlerServices(s.state) + c.Check(err, IsNil) + c.Check(handlers, HasLen, 0) + + present, err := ifacestate.CallInterfacesRequestsControlHandlerServicePresent(s.state) + c.Check(err, IsNil) + c.Check(present, Equals, false) +} + +func (s *interfaceManagerSuite) TestInterfacesRequestsControlHandlerServicesManyWithHandlerApp(c *C) { + s.mockSnapd(c) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap1", hasHandler: false}) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap2", hasHandler: true}) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap3", hasHandler: false}) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap4", hasHandler: true}) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap5", hasHandler: false}) + + s.state.Lock() + defer s.state.Unlock() + handlers, err := ifacestate.InterfacesRequestsControlHandlerServices(s.state) + c.Assert(err, IsNil) + c.Assert(handlers, HasLen, 2) + expected := map[string]bool{"test-snap2": true, "test-snap4": true} + result := map[string]bool{handlers[0].Snap.SuggestedName: true, handlers[1].Snap.SuggestedName: true} + c.Check(result, DeepEquals, expected) + + present, err := ifacestate.CallInterfacesRequestsControlHandlerServicePresent(s.state) + c.Check(err, IsNil) + c.Check(present, Equals, true) +} + +func (s *interfaceManagerSuite) TestInterfacesRequestsControlHandlerServicesDisconnected(c *C) { + s.mockSnapd(c) + s.mockPromptingHandler(c, mockPromptingHandlerOpts{snapName: "test-snap", hasHandler: true}) + + s.state.Lock() + defer s.state.Unlock() + s.state.Set("conns", map[string]interface{}{ + "test-snap:snap-interfaces-requests-control core:snap-interfaces-requests-control": map[string]interface{}{ + "interface": "snap-interfaces-requests-control", + "plug-static": map[string]interface{}{ + "handler-service": "prompts-handler", + }, + // manually disconnected + "undesired": true, + }, + }) + + handlers, err := ifacestate.InterfacesRequestsControlHandlerServices(s.state) + c.Check(err, IsNil) + c.Check(handlers, HasLen, 0) + + present, err := ifacestate.CallInterfacesRequestsControlHandlerServicePresent(s.state) + c.Check(err, IsNil) + c.Check(present, Equals, false) +} + +func (s *interfaceManagerSuite) mockSnapd(c *C) { + const snapdSnapYaml = ` +name: snapd +version: 1 +type: snapd +` + + si := &snap.SideInfo{RealName: "snapd", Revision: snap.R(1)} + snapdSnap := snaptest.MockSnap(c, snapdSnapYaml, si) + s.state.Lock() + defer s.state.Unlock() + snapstate.Set(s.state, "snapd", &snapstate.SnapState{ + Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{si}), + Current: snap.R(1), + Active: true, + SnapType: "snapd", + }) + + for _, iface := range builtin.Interfaces() { + if name := iface.Name(); name == "snap-interfaces-requests-control" { + // add implicit slot + // XXX copied from implicit.go + snapdSnap.Slots[name] = &snap.SlotInfo{ + Name: name, + Snap: snapdSnap, + Interface: name, + } + } + } +} + +type mockPromptingHandlerOpts struct { + snapName string + hasHandler bool +} + +func (s *interfaceManagerSuite) mockPromptingHandler(c *C, opts mockPromptingHandlerOpts) { + name := opts.snapName + + var mockSnapWithPromptshandlerFmt = `name: %s +version: 1.0 +apps: + +plugs: + snap-interfaces-requests-control: +` + + if opts.hasHandler { + mockSnapWithPromptshandlerFmt = `name: %s +version: 1.0 +apps: + prompts-handler: + daemon: simple + +plugs: + snap-interfaces-requests-control: + handler: prompts-handler +` + } + si := &snap.SideInfo{RealName: name, Revision: snap.R(1)} + snaptest.MockSnap(c, fmt.Sprintf(mockSnapWithPromptshandlerFmt, name), si) + s.state.Lock() + defer s.state.Unlock() + + snapstate.Set(s.state, name, &snapstate.SnapState{ + Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{si}), + Current: snap.R(1), + Active: true, + SnapType: "app", + }) + + plugStatic := map[string]interface{}{} + if opts.hasHandler { + plugStatic["handler-service"] = "prompts-handler" + } + + var conns map[string]interface{} + err := s.state.Get("conns", &conns) + if err != nil { + if errors.Is(err, state.ErrNoState) { + conns = map[string]interface{}{} + } else { + c.Fatalf("unexpected error: %v", err) + } + } + + conns[fmt.Sprintf("%s:snap-interfaces-requests-control core:snap-interfaces-requests-control", name)] = map[string]interface{}{ + "interface": "snap-interfaces-requests-control", + "plug-static": plugStatic, + } + + s.state.Set("conns", conns) +}