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.

Sign up for GitHub

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

many: move notify.SysPath to apparmor.NotifySocketPath #15131

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

many: move notify.SysPath to apparmor.NotifySocketPath #15131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea2bf42
many: move notify.SysPath to apparmor.NotifySocketPath
olivercalder Feb 25, 2025
ba6dff8
s/apparmor: improve testing and documentation of NotifySocketPath
olivercalder Feb 25, 2025
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
3 changes: 1 addition & 2 deletions overlord/configstate/configcore/prompting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import (
"github.com/snapcore/snapd/overlord/state"
"github.com/snapcore/snapd/release"
"github.com/snapcore/snapd/sandbox/apparmor"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
"github.com/snapcore/snapd/snap"
"github.com/snapcore/snapd/snap/snaptest"
)
Expand All @@ -70,7 +69,7 @@ func (s *promptingSuite) SetUpTest(c *C) {
[]string{"prompt"}, nil,
))
// mock the presence of the notification socket
os.MkdirAll(notify.SysPath, 0o755)
os.MkdirAll(apparmor.NotifySocketPath, 0o755)
// mock the presence of permstable32_version with supported version
s.AddCleanup(apparmor.MockFsRootPath(dirs.GlobalRootDir))
os.MkdirAll(filepath.Join(dirs.GlobalRootDir, "sys/kernel/security/apparmor/features/policy"), 0o755)
Expand Down
33 changes: 24 additions & 9 deletions sandbox/apparmor/apparmor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
"github.com/snapcore/snapd/snapdtool"
"github.com/snapcore/snapd/strutil"
)
Expand Down Expand Up @@ -305,6 +304,22 @@ const (
Full
)

var (
Copy link
Contributor

Choose a reason for hiding this comment

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

Could those carry documentation (follow up is fine)

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably should. The other ones already existed, but I moved their declaration up to before they're set, since that made more sense to me. Just the last one is new, and it's the path to the socket over which communication can occur with kernel apparmor.

// ConfDir is the path to the directory holding AppArmor configuration.
ConfDir string
// CacheDir is the path to the cache directory for AppArmor.
CacheDir string
// SystemCacheDir is the path to the system cache directory for AppArmor,
// which may or may not be different from CacheDir.
SystemCacheDir string
// SnapConfineAppArmorDir is the path to the AppArmor snap confine
// directory.
SnapConfineAppArmorDir string
// NotifySocketPath is the path to the socket over which listeners can
// communicate with AppArmor in the kernel.
NotifySocketPath string
)

func setupConfCacheDirs(newrootdir string) {
ConfDir = filepath.Join(newrootdir, "/etc/apparmor.d")
CacheDir = filepath.Join(newrootdir, "/var/cache/apparmor")
Expand All @@ -329,17 +344,17 @@ func setupConfCacheDirs(newrootdir string) {
SnapConfineAppArmorDir = filepath.Join(dirs.SnapdStateDir(newrootdir), "apparmor", snapConfineDir)
}

func setupNotifySocketPath(newrootdir string) {
NotifySocketPath = filepath.Join(newrootdir, "/sys/kernel/security/apparmor/.notify")
}

func init() {
dirs.AddRootDirCallback(setupConfCacheDirs)
setupConfCacheDirs(dirs.GlobalRootDir)
}

var (
ConfDir string
CacheDir string
SystemCacheDir string
SnapConfineAppArmorDir string
)
dirs.AddRootDirCallback(setupNotifySocketPath)
setupNotifySocketPath(dirs.GlobalRootDir)
}

func (level LevelType) String() string {
switch level {
Expand Down Expand Up @@ -464,7 +479,7 @@ func PromptingSupportedByFeatures(apparmorFeatures *FeaturesSupported) (bool, st
return false, "apparmor kernel features do not support prompting for file access"
}
}
if !notify.SupportAvailable() {
if !osutil.FileExists(NotifySocketPath) {
return false, "apparmor kernel notification socket required by prompting listener is not present"
}
version, err := probeKernelFeaturesPermstable32Version()
Expand Down
12 changes: 10 additions & 2 deletions sandbox/apparmor/apparmor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/sandbox/apparmor"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
"github.com/snapcore/snapd/snapdtool"
"github.com/snapcore/snapd/testutil"
)
Expand Down Expand Up @@ -765,7 +764,7 @@ func (s *apparmorSuite) TestPromptingSupported(c *C) {

// Create a file at the notify path, doesn't matter what kind of file.
// The actual file is a socket, but a directory will do here for convenience.
c.Assert(os.MkdirAll(notify.SysPath, 0o755), IsNil)
c.Assert(os.MkdirAll(apparmor.NotifySocketPath, 0o755), IsNil)
restore = apparmor.MockFeatures(goodKernelFeatures, nil, goodParserFeatures, nil)
defer restore()

Expand Down Expand Up @@ -902,6 +901,15 @@ func (s *apparmorSuite) TestSetupConfCacheDirsWithInternalApparmor(c *C) {
c.Check(apparmor.SnapConfineAppArmorDir, Equals, "/newdir/var/lib/snapd/apparmor/snap-confine.internal")
}

func (s *apparmorSuite) TestSetupNotifySocketPath(c *C) {
apparmor.SetupNotifySocketPath("/newdir")
c.Check(apparmor.NotifySocketPath, Equals, "/newdir/sys/kernel/security/apparmor/.notify")

newRoot := c.MkDir()
dirs.SetRootDir(newRoot)
c.Check(apparmor.NotifySocketPath, Equals, filepath.Join(newRoot, "/sys/kernel/security/apparmor/.notify"))
}

func (s *apparmorSuite) TestSystemAppArmorLoadsSnapPolicyErr(c *C) {
fakeroot := c.MkDir()
dirs.SetRootDir(fakeroot)
Expand Down
5 changes: 3 additions & 2 deletions sandbox/apparmor/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ import (
)

var (
NumberOfJobsParam = numberOfJobsParam
SetupConfCacheDirs = setupConfCacheDirs
NumberOfJobsParam = numberOfJobsParam
SetupConfCacheDirs = setupConfCacheDirs
SetupNotifySocketPath = setupNotifySocketPath
)

func MockRuntimeNumCPU(new func() int) (restore func()) {
Expand Down
3 changes: 2 additions & 1 deletion sandbox/apparmor/notify/listener/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"golang.org/x/sys/unix"

"github.com/snapcore/snapd/osutil/epoll"
"github.com/snapcore/snapd/sandbox/apparmor"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
"github.com/snapcore/snapd/testutil"
)
Expand Down Expand Up @@ -56,7 +57,7 @@ func MockOsOpenWithSocket() (restore func()) {
if err != nil {
return nil, err
}
notifyFile := os.NewFile(uintptr(socket), notify.SysPath)
notifyFile := os.NewFile(uintptr(socket), apparmor.NotifySocketPath)
return notifyFile, nil
}
restore = MockOsOpen(f)
Expand Down
3 changes: 2 additions & 1 deletion sandbox/apparmor/notify/listener/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil/epoll"
"github.com/snapcore/snapd/sandbox/apparmor"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
)

Expand Down Expand Up @@ -186,7 +187,7 @@ const (
//
// If the kernel does not support the notification mechanism the error is ErrNotSupported.
func Register() (listener *Listener, err error) {
path := notify.SysPath
path := apparmor.NotifySocketPath
if override := os.Getenv("PROMPT_NOTIFY_PATH"); override != "" {
path = override
}
Expand Down
11 changes: 6 additions & 5 deletions sandbox/apparmor/notify/listener/listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/snapcore/snapd/arch"
"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/osutil/epoll"
"github.com/snapcore/snapd/sandbox/apparmor"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
"github.com/snapcore/snapd/sandbox/apparmor/notify/listener"
"github.com/snapcore/snapd/testutil"
Expand Down Expand Up @@ -150,7 +151,7 @@ func (*listenerSuite) TestRegisterOverridePath(c *C) {
l, err := listener.Register()
c.Assert(err, IsNil)

c.Assert(outputOverridePath, Equals, notify.SysPath)
c.Assert(outputOverridePath, Equals, apparmor.NotifySocketPath)

err = l.Close()
c.Assert(err, IsNil)
Expand Down Expand Up @@ -191,7 +192,7 @@ func (*listenerSuite) TestRegisterErrors(c *C) {

l, err = listener.Register()
c.Assert(l, IsNil)
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot open %q: %v", notify.SysPath, customError))
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot open %q: %v", apparmor.NotifySocketPath, customError))

restoreOpen = listener.MockOsOpen(func(name string) (*os.File, error) {
placeholderSocket, err := unix.Socket(unix.AF_UNIX, unix.SOCK_STREAM, 0)
Expand Down Expand Up @@ -230,7 +231,7 @@ func (*listenerSuite) TestRegisterErrors(c *C) {

l, err = listener.Register()
c.Assert(l, IsNil)
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot register epoll on %q: bad file descriptor", notify.SysPath))
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot register epoll on %q: bad file descriptor", apparmor.NotifySocketPath))
}

// An expedient abstraction over notify.MsgNotificationFile to allow defining
Expand Down Expand Up @@ -449,11 +450,11 @@ func (*listenerSuite) TestRunMultipleRequestsInBuffer(c *C) {
func (*listenerSuite) TestRunEpoll(c *C) {
sockets, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0)
c.Assert(err, IsNil)
notifyFile := os.NewFile(uintptr(sockets[0]), notify.SysPath)
notifyFile := os.NewFile(uintptr(sockets[0]), apparmor.NotifySocketPath)
kernelSocket := sockets[1]

restoreOpen := listener.MockOsOpen(func(name string) (*os.File, error) {
c.Assert(name, Equals, notify.SysPath)
c.Assert(name, Equals, apparmor.NotifySocketPath)
return notifyFile, nil
})
defer restoreOpen()
Expand Down
21 changes: 0 additions & 21 deletions sandbox/apparmor/notify/notify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,10 @@ package notify
import (
"errors"
"fmt"
"path/filepath"

"golang.org/x/sys/unix"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/osutil"
)

var SysPath string

// SupportAvailable returns true if SysPath exists, indicating that apparmor
// prompting messages may be received from SysPath.
func SupportAvailable() bool {
return osutil.FileExists(SysPath)
}

var doIoctl = Ioctl

// RegisterFileDescriptor registers a notification socket using the given file
Expand Down Expand Up @@ -56,12 +44,3 @@ func RegisterFileDescriptor(fd uintptr) (ProtocolVersion, error) {
return protocolVersion, nil
}
}

func setupSysPath(newrootdir string) {
SysPath = filepath.Join(newrootdir, "/sys/kernel/security/apparmor/.notify")
}

func init() {
dirs.AddRootDirCallback(setupSysPath)
setupSysPath(dirs.GlobalRootDir)
}
21 changes: 0 additions & 21 deletions sandbox/apparmor/notify/notify_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package notify_test

import (
"fmt"
"os"
"path/filepath"
"testing"

. "gopkg.in/check.v1"
Expand All @@ -30,25 +28,6 @@ func (s *notifySuite) SetUpTest(c *C) {
s.AddCleanup(func() { dirs.SetRootDir("") })
}

func (*notifySuite) TestSysPathBehavior(c *C) {
newRoot := c.MkDir()
newSysPath := filepath.Join(newRoot, "/sys/kernel/security/apparmor/.notify")
dirs.SetRootDir(newRoot)
c.Assert(notify.SysPath, Equals, newSysPath)
}

func (*notifySuite) TestSupportAvailable(c *C) {
newRoot := c.MkDir()
dirs.SetRootDir(newRoot)
c.Assert(notify.SupportAvailable(), Equals, false)
err := os.MkdirAll(filepath.Dir(notify.SysPath), 0755)
c.Assert(err, IsNil)
c.Assert(notify.SupportAvailable(), Equals, false)
_, err = os.Create(notify.SysPath)
c.Assert(err, IsNil)
c.Assert(notify.SupportAvailable(), Equals, true)
}

var fakeNotifyVersions = []notify.VersionAndCheck{
{
Version: 2,
Expand Down
6 changes: 5 additions & 1 deletion sandbox/apparmor/notify/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@
// on the notify socket with that version, in which case we'll need to try
// the next version in the list.
versionLikelySupportedChecks = map[ProtocolVersion]func() bool{
3: SupportAvailable,
3: func() bool {
Copy link
Contributor

@zyga zyga Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is this intentional?

I was asking about timing/staging this change in the patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, since SupportAvailable is going away, we can't call it here anymore. This was only ever really a placeholder anyway, since any caller interested in protocol version support is being called because prompting itself is supported, and prompting support first checks that the notify socket already exists. So this check was not necessary, just something we could do to make extra sure, until we introduce more interesting checks (e.g. in #15089)

Copy link
Member Author

Choose a reason for hiding this comment

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

This patch removes SupportAvailable (since notify.SysPath is being replaced by apparmor.NotifySocketPath), so I remove the call to it here as well, and in #15089 replace it with a proper check.

// If prompting is supported, version 3 is always assumed to be
// supported.
return true
},

Check warning on line 48 in sandbox/apparmor/notify/version.go

View check run for this annotation

Codecov / codecov/patch

sandbox/apparmor/notify/version.go#L44-L48 

Added lines #L44 - L48 were not covered by tests
}

// versionKnown returns true if the given protocol version is known by
Expand Down
Loading