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,overlord: allow access to kernel snap in profiles #14899

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions interfaces/apparmor/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,15 @@ func (b *Backend) addContent(securityTag string, snapInfo *snap.Info, cmdName st
}
policy = templatePattern.ReplaceAllStringFunc(policy, func(placeholder string) string {
switch placeholder {
case "###KERNEL_MODULES_AND_FIRMWARE###":
if opts.KernelSnap != "" {
return fmt.Sprintf(`
# Allow access to kernel modules and firmware from the kernel snap
/snap/%[1]s/*/{modules,firmware}/{,**} r,
pedronis marked this conversation as resolved.
Show resolved Hide resolved
/snap/%[1]s/components/mnt/*/*/{modules,firmware}/{,**} r,
/var/snap/%[1]s/*/{modules,firmware}/{,**} r,`, opts.KernelSnap)
}
return ""
case "###DEVMODE_SNAP_CONFINE###":
if !opts.DevMode {
// nothing to add if we are not in devmode
Expand Down
36 changes: 36 additions & 0 deletions interfaces/apparmor/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2986,3 +2986,39 @@ func (s *backendSuite) TestRemoveAllSnapAppArmorProfiles(c *C) {
c.Check(os.IsNotExist(err), Equals, true)
}
}

func (s *backendSuite) TestKernelModulesAndFwRule(c *C) {
restoreTemplate := apparmor.MockTemplate("template\n###KERNEL_MODULES_AND_FIRMWARE###\n")
defer restoreTemplate()
restore := apparmor_sandbox.MockLevel(apparmor_sandbox.Full)
defer restore()
restore = osutil.MockIsHomeUsingRemoteFS(func() (bool, error) { return false, nil })
defer restore()

for _, tc := range []struct {
opts interfaces.ConfinementOptions
suppress bool
expected Checker
}{
{
opts: interfaces.ConfinementOptions{},
expected: Not(testutil.Contains),
},
{
opts: interfaces.ConfinementOptions{KernelSnap: "mykernel"},
expected: testutil.Contains,
},
} {
snapInfo := s.InstallSnap(c, tc.opts, "", ifacetest.SambaYamlV1, 1)
profile := filepath.Join(dirs.SnapAppArmorDir, "snap.samba.smbd")
data, err := os.ReadFile(profile)
c.Assert(err, IsNil)

c.Assert(string(data), tc.expected, `
# Allow access to kernel modules and firmware from the kernel snap
/snap/mykernel/*/{modules,firmware}/{,**} r,
/snap/mykernel/components/mnt/*/*/{modules,firmware}/{,**} r,
/var/snap/mykernel/*/{modules,firmware}/{,**} r,`)
s.RemoveSnap(c, snapInfo)
}
}
1 change: 1 addition & 0 deletions interfaces/apparmor/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ var templateCommon = `
#include <abstractions/base>
#include <abstractions/consoles>
#include <abstractions/openssl>
###KERNEL_MODULES_AND_FIRMWARE###

# While in later versions of the base abstraction, include this explicitly
# for series 16 and cross-distro
Expand Down
3 changes: 3 additions & 0 deletions interfaces/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ type ConfinementOptions struct {
// AppArmorPrompting indicates whether the prompt prefix should be used in
// relevant rules when generating AppArmor security profiles.
AppArmorPrompting bool
// KernelSnap is the name of the kernel snap in the system
// (empty for classic systems).
KernelSnap string
}

// SecurityBackendOptions carries extra flags that affect initialization of the
Expand Down
4 changes: 2 additions & 2 deletions overlord/ifacestate/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ func NewInterfaceManagerWithAppArmorPrompting(useAppArmorPrompting bool) *Interf
return m
}

func (m *InterfaceManager) BuildConfinementOptions(st *state.State, snapInfo *snap.Info, flags snapstate.Flags) (interfaces.ConfinementOptions, error) {
return m.buildConfinementOptions(st, snapInfo, flags)
func (m *InterfaceManager) BuildConfinementOptions(st *state.State, task *state.Task, snapInfo *snap.Info, flags snapstate.Flags) (interfaces.ConfinementOptions, error) {
return m.buildConfinementOptions(st, task, snapInfo, flags)
}

type ConnectOpts = connectOpts
Expand Down
31 changes: 19 additions & 12 deletions overlord/ifacestate/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,25 @@ func getExtraLayouts(st *state.State, snapInfo *snap.Info) ([]snap.Layout, error
return extraLayouts, nil
}

func (m *InterfaceManager) buildConfinementOptions(st *state.State, snapInfo *snap.Info, flags snapstate.Flags) (interfaces.ConfinementOptions, error) {
func (m *InterfaceManager) buildConfinementOptions(st *state.State, task *state.Task, snapInfo *snap.Info, flags snapstate.Flags) (interfaces.ConfinementOptions, error) {
extraLayouts, err := getExtraLayouts(st, snapInfo)
if err != nil {
return interfaces.ConfinementOptions{}, fmt.Errorf("cannot get extra mount layouts of snap %q: %s", snapInfo.InstanceName(), err)
}

kernelSnap := ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

the code here probably needs to take the task so we can do the right thin during remodel

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, fixed now

deviceCtx, err := snapstate.DeviceCtx(st, task, nil)
if err == nil {
kernelSnap = deviceCtx.Kernel()
}

return interfaces.ConfinementOptions{
DevMode: flags.DevMode,
JailMode: flags.JailMode,
Classic: flags.Classic,
ExtraLayouts: extraLayouts,
AppArmorPrompting: m.useAppArmorPrompting,
KernelSnap: kernelSnap,
}, nil
}

Expand Down Expand Up @@ -124,7 +131,7 @@ func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap st
return fmt.Errorf("building app set for snap %q: %v", affectingSnap, err)
}

opts, err := m.buildConfinementOptions(st, affectedSnapInfo, snapst.Flags)
opts, err := m.buildConfinementOptions(st, task, affectedSnapInfo, snapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -170,7 +177,7 @@ func (m *InterfaceManager) doSetupProfiles(task *state.Task, tomb *tomb.Tomb) er
return nil
}

opts, err := m.buildConfinementOptions(task.State(), snapInfo, snapsup.Flags)
opts, err := m.buildConfinementOptions(task.State(), task, snapInfo, snapsup.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -340,7 +347,7 @@ func (m *InterfaceManager) setupProfilesForAppSet(task *state.Task, appSet *inte
return fmt.Errorf("building app set for snap %q: %v", name, err)
}

opts, err := m.buildConfinementOptions(st, snapInfo, snapst.Flags)
opts, err := m.buildConfinementOptions(st, task, snapInfo, snapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -456,7 +463,7 @@ func (m *InterfaceManager) undoSetupProfiles(task *state.Task, tomb *tomb.Tomb)
if err != nil {
return err
}
opts, err := m.buildConfinementOptions(task.State(), snapInfo, snapst.Flags)
opts, err := m.buildConfinementOptions(task.State(), task, snapInfo, snapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -691,7 +698,7 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) (err error)
if err != nil {
return err
}
slotOpts, err := m.buildConfinementOptions(st, slotSnapInfo, slotSnapst.Flags)
slotOpts, err := m.buildConfinementOptions(st, task, slotSnapInfo, slotSnapst.Flags)
if err != nil {
return err
}
Expand All @@ -703,7 +710,7 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) (err error)
if err != nil {
return err
}
plugOpts, err := m.buildConfinementOptions(st, plugSnapInfo, plugSnapst.Flags)
plugOpts, err := m.buildConfinementOptions(st, task, plugSnapInfo, plugSnapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -813,7 +820,7 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error {
return fmt.Errorf("building app set for snap %q: %v", snapInfo.InstanceName(), err)
}

opts, err := m.buildConfinementOptions(st, snapInfo, snapst.Flags)
opts, err := m.buildConfinementOptions(st, task, snapInfo, snapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -937,7 +944,7 @@ func (m *InterfaceManager) undoDisconnect(task *state.Task, _ *tomb.Tomb) error
if err != nil {
return err
}
slotOpts, err := m.buildConfinementOptions(st, slotSnapInfo, slotSnapst.Flags)
slotOpts, err := m.buildConfinementOptions(st, task, slotSnapInfo, slotSnapst.Flags)
if err != nil {
return err
}
Expand All @@ -949,7 +956,7 @@ func (m *InterfaceManager) undoDisconnect(task *state.Task, _ *tomb.Tomb) error
if err != nil {
return err
}
plugOpts, err := m.buildConfinementOptions(st, plugSnapInfo, plugSnapst.Flags)
plugOpts, err := m.buildConfinementOptions(st, task, plugSnapInfo, plugSnapst.Flags)
if err != nil {
return err
}
Expand Down Expand Up @@ -1047,7 +1054,7 @@ func (m *InterfaceManager) undoConnect(task *state.Task, _ *tomb.Tomb) error {
if err != nil {
return err
}
slotOpts, err := m.buildConfinementOptions(st, slotSnapInfo, slotSnapst.Flags)
slotOpts, err := m.buildConfinementOptions(st, task, slotSnapInfo, slotSnapst.Flags)
if err != nil {
return err
}
Expand All @@ -1059,7 +1066,7 @@ func (m *InterfaceManager) undoConnect(task *state.Task, _ *tomb.Tomb) error {
if err != nil {
return err
}
plugOpts, err := m.buildConfinementOptions(st, plugSnapInfo, plugSnapst.Flags)
plugOpts, err := m.buildConfinementOptions(st, task, plugSnapInfo, plugSnapst.Flags)
if err != nil {
return err
}
Expand Down
51 changes: 49 additions & 2 deletions overlord/ifacestate/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
package ifacestate_test

import (
"errors"
"path"

. "gopkg.in/check.v1"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/overlord/configstate/config"
"github.com/snapcore/snapd/overlord/devicestate"
"github.com/snapcore/snapd/overlord/ifacestate"
"github.com/snapcore/snapd/overlord/servicestate/servicestatetest"
"github.com/snapcore/snapd/overlord/snapstate"
Expand All @@ -34,6 +36,7 @@ import (
"github.com/snapcore/snapd/snap"
"github.com/snapcore/snapd/snap/quota"
"github.com/snapcore/snapd/snap/snaptest"
"github.com/snapcore/snapd/testutil"
)

const snapAyaml = `name: snap-a
Expand All @@ -42,14 +45,22 @@ base: base-snap-a
`

type handlersSuite struct {
testutil.BaseTest
st *state.State
}

var _ = Suite(&handlersSuite{})

func (s *handlersSuite) mockModel() func() {
old := snapstate.DeviceCtx
snapstate.DeviceCtx = devicestate.DeviceCtx
return func() { snapstate.DeviceCtx = old }
}

func (s *handlersSuite) SetUpTest(c *C) {
s.st = state.New(nil)
dirs.SetRootDir(c.MkDir())
s.AddCleanup(s.mockModel())
}

func (s *handlersSuite) TearDownTest(c *C) {
Expand Down Expand Up @@ -136,14 +147,50 @@ func (s *handlersSuite) TestBuildConfinementOptions(c *C) {

snapInfo := mockInstalledSnap(c, s.st, snapAyaml)
flags := snapstate.Flags{}
opts, err := m.BuildConfinementOptions(s.st, snapInfo, snapstate.Flags{})
opts, err := m.BuildConfinementOptions(s.st, nil, snapInfo, snapstate.Flags{})

c.Check(err, IsNil)
c.Check(len(opts.ExtraLayouts), Equals, 0)
c.Check(opts.Classic, Equals, flags.Classic)
c.Check(opts.DevMode, Equals, flags.DevMode)
c.Check(opts.JailMode, Equals, flags.JailMode)
c.Check(opts.AppArmorPrompting, Equals, testAppArmorPrompting)
c.Check(opts.KernelSnap, Equals, "")
}
}

func (s *handlersSuite) TestBuildConfinementOptionsWithTask(c *C) {
s.st.Lock()
defer s.st.Unlock()

// This test is to check that the task is actually passed down to snapstate.DeviceCtx(),
// and that errors there are handled fine.
t := s.st.NewTask("foo", "description")
s.AddCleanup(func() func() {
old := snapstate.DeviceCtx
snapstate.DeviceCtx = func(st *state.State, task *state.Task,
providedDeviceCtx snapstate.DeviceContext) (snapstate.DeviceContext, error) {
c.Check(task, DeepEquals, t)
return nil, errors.New("classic, no context")
}
return func() { snapstate.DeviceCtx = old }
}())

for _, testAppArmorPrompting := range []bool{true, false} {
// Create fake InterfaceManager to hold fake AppArmor Prompting value
m := ifacestate.NewInterfaceManagerWithAppArmorPrompting(testAppArmorPrompting)

snapInfo := mockInstalledSnap(c, s.st, snapAyaml)
flags := snapstate.Flags{}
opts, err := m.BuildConfinementOptions(s.st, t, snapInfo, snapstate.Flags{})

c.Check(err, IsNil)
c.Check(len(opts.ExtraLayouts), Equals, 0)
c.Check(opts.Classic, Equals, flags.Classic)
c.Check(opts.DevMode, Equals, flags.DevMode)
c.Check(opts.JailMode, Equals, flags.JailMode)
c.Check(opts.AppArmorPrompting, Equals, testAppArmorPrompting)
c.Check(opts.KernelSnap, Equals, "")
}
}

Expand All @@ -166,7 +213,7 @@ func (s *handlersSuite) TestBuildConfinementOptionsWithLogNamespace(c *C) {
c.Assert(err, IsNil)

flags := snapstate.Flags{}
opts, err := m.BuildConfinementOptions(s.st, snapInfo, snapstate.Flags{})
opts, err := m.BuildConfinementOptions(s.st, nil, snapInfo, snapstate.Flags{})

c.Check(err, IsNil)
c.Assert(len(opts.ExtraLayouts), Equals, 1)
Expand Down
2 changes: 1 addition & 1 deletion overlord/ifacestate/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (m *InterfaceManager) regenerateAllSecurityProfiles(tm timings.Measurer) er
logger.Noticef("cannot get current info for snap %q: %s", snapName, err)
return interfaces.ConfinementOptions{}
}
opts, err := m.buildConfinementOptions(m.state, snapInfo, snapst.Flags)
opts, err := m.buildConfinementOptions(m.state, nil, snapInfo, snapst.Flags)
if err != nil {
logger.Noticef("cannot get confinement options for snap %q: %s", snapName, err)
}
Expand Down
Loading
Loading