snapdtool, sandbox/apparmor: fix apparmor_parser lookup (#14240)

* snapdtool: disable lookup of development binaries The lookup of development binaries from the current process execution location proves to be problematic as it adds ambiguity to what locations are being considered. Specifically, when /usr/bin/snap asks about an internal tool, the lookup may incorrectly return /usr/bin/foo instead of /usr/lib/snapd/foo if /usr/bin/foo happens to exist and is executable. Signed-off-by: Maciej Borzecki <[email protected]> * sandbox/apparmor: more logging when checking apparmor_parser candidates Signed-off-by: Maciej Borzecki <[email protected]> * secboot: improve mocking of snap-fde-keymgr Improve the mocking of snap-fde-keymgr such that the tool is indeed mocked at the right location. Signed-off-by: Maciej Borzecki <[email protected]> * cmd/snap: log errors when waiting for snapd to regenerate profiles Signed-off-by: Maciej Borzecki <[email protected]> --------- Signed-off-by: Maciej Borzecki <[email protected]>
canonical · Jul 29, 2024 · 5a782b1 · 5a782b1
1 parent 364da01
commit 5a782b1
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 15 deletions.
diff --git a/cmd/snap/cmd_run.go b/cmd/snap/cmd_run.go
@@ -206,6 +206,8 @@ func maybeWaitForSecurityProfileRegeneration(cli *client.Client) error {
 		// the user or what do we do if we know snapd is down for maintenance?
 		if _, err := cli.SysInfo(); err == nil {
 			return nil
+		} else {
+			logger.Debugf("cannot obtain system info: %v", err)
 		}
 		// sleep a little bit for good measure
 		time.Sleep(1 * time.Second)

diff --git a/sandbox/apparmor/apparmor.go b/sandbox/apparmor/apparmor.go
@@ -395,6 +395,8 @@ func ParserMtime() int64 {
 		if fi, err := os.Stat(cmd.Path); err == nil {
 			mtime = fi.ModTime().Unix()
 		}
+	} else {
+		logger.Debugf("apparmor parser err: %v", err)
 	}
 	return mtime
 }
@@ -790,6 +792,7 @@ func AppArmorParser() (cmd *exec.Cmd, internal bool, err error) {
 	// installed snapd-apparmor support re-exec
 	if path, err := snapdtool.InternalToolPath("apparmor_parser"); err == nil {
 		if osutil.IsExecutable(path) && snapdAppArmorSupportsReexec() && !systemAppArmorLoadsSnapPolicy() {
+			logger.Debugf("checking internal apparmor_parser candidate at %v", path)
 			prefix := strings.TrimSuffix(path, "apparmor_parser")
 			snapdAbi30File := filepath.Join(prefix, "/apparmor.d/abi/3.0")
 			snapdAbi40File := filepath.Join(prefix, "/apparmor.d/abi/4.0")
@@ -828,6 +831,7 @@ func AppArmorParser() (cmd *exec.Cmd, internal bool, err error) {
 	for _, dir := range filepath.SplitList(parserSearchPath) {
 		path := filepath.Join(dir, "apparmor_parser")
 		if _, err := os.Stat(path); err == nil {
+			logger.Debugf("checking distro apparmor_parser at %v", path)
 			// Detect but ignore apparmor 4.0 ABI support.
 			//
 			// At present this causes some bugs with mqueue mediation that can

diff --git a/secboot/encrypt_sb_test.go b/secboot/encrypt_sb_test.go
@@ -90,6 +90,7 @@ func (s *encryptSuite) TestFormatEncryptedDeviceInvalidEncType(c *C) {
 type keymgrSuite struct {
 	testutil.BaseTest
 
+	rootDir       string
 	d             string
 	keymgrCmd     *testutil.MockCmd
 	udevadmCmd    *testutil.MockCmd
@@ -101,6 +102,10 @@ var _ = Suite(&keymgrSuite{})
 func (s *keymgrSuite) SetUpTest(c *C) {
 	s.BaseTest.SetUpTest(c)
 
+	s.rootDir = c.MkDir()
+	s.AddCleanup(func() { dirs.SetRootDir(dirs.GlobalRootDir) })
+	dirs.SetRootDir(s.rootDir)
+
 	s.d = c.MkDir()
 	s.systemdRunCmd = testutil.MockCommand(c, "systemd-run", `
 while true; do
@@ -115,7 +120,7 @@ while true; do
 done
 `)
 	s.AddCleanup(s.systemdRunCmd.Restore)
-	s.keymgrCmd = testutil.MockCommand(c, "snap-fde-keymgr", fmt.Sprintf(`
+	s.keymgrCmd = testutil.MockCommand(c, filepath.Join(dirs.DistroLibExecDir, "snap-fde-keymgr"), fmt.Sprintf(`
 set -e
 if [ "$1" = "change-encryption-key" ]; then
     cat > %s/input
@@ -205,7 +210,7 @@ func (s *keymgrSuite) TestStageEncryptionKeyBadUdev(c *C) {
 }
 
 func (s *keymgrSuite) TestStageTransitionEncryptionKeyBadKeymgr(c *C) {
-	keymgrCmd := testutil.MockCommand(c, "snap-fde-keymgr", `echo keymgr very unhappy; exit 1`)
+	keymgrCmd := testutil.MockCommand(c, filepath.Join(dirs.DistroLibExecDir, "snap-fde-keymgr"), `echo keymgr very unhappy; exit 1`)
 	defer keymgrCmd.Restore()
 	// update where /proc/self/exe resolves to
 	restore := snapdtool.MockOsReadlink(func(string) (string, error) {
@@ -332,10 +337,7 @@ done
 `)
 	s.AddCleanup(udevadmCmd.Restore)
 
-	s.AddCleanup(func() { dirs.SetRootDir(dirs.GlobalRootDir) })
-	dirs.SetRootDir(s.d)
-
-	snaptest.PopulateDir(s.d, [][]string{
+	snaptest.PopulateDir(s.rootDir, [][]string{
 		{"/sys/dev/block/600:3/dm/uuid", "CRYPT-LUKS2-5a522809c87e4dfa81a88dc5667d1304-foo"},
 		{"/sys/dev/block/600:3/dm/name", "foo"},
 		{"/sys/dev/block/600:4/dm/uuid", "CRYPT-LUKS2-5a522809c87e4dfa81a88dc5667d1305-bar"},

diff --git a/snapdtool/tool_linux.go b/snapdtool/tool_linux.go
@@ -129,13 +129,6 @@ func InternalToolPath(tool string) (string, error) {
 			if osutil.IsExecutable(maybeTool) {
 				return maybeTool, nil
 			}
-		} else {
-			// or perhaps some other random location, make sure the tool
-			// exists there and is an executable
-			maybeTool := filepath.Join(filepath.Dir(exe), tool)
-			if osutil.IsExecutable(maybeTool) {
-				return maybeTool, nil
-			}
 		}
 	}
 

diff --git a/snapdtool/tool_test.go b/snapdtool/tool_test.go
@@ -286,7 +286,7 @@ func (s *toolSuite) TestInternalToolPathWithDevLocationFallback(c *C) {
 	c.Check(path, Equals, filepath.Join(dirs.DistroLibExecDir, "potato"))
 }
 
-func (s *toolSuite) TestInternalToolPathWithOtherDevLocationWhenExecutable(c *C) {
+func (s *toolSuite) TestInternalToolPathWithOtherDevLocationWhenExecutableFallback(c *C) {
 	restore := snapdtool.MockOsReadlink(func(string) (string, error) {
 		return filepath.Join(dirs.GlobalRootDir, "/tmp/snapd"), nil
 	})
@@ -300,7 +300,7 @@ func (s *toolSuite) TestInternalToolPathWithOtherDevLocationWhenExecutable(c *C)
 
 	path, err := snapdtool.InternalToolPath("potato")
 	c.Check(err, IsNil)
-	c.Check(path, Equals, filepath.Join(dirs.GlobalRootDir, "/tmp/potato"))
+	c.Check(path, Equals, filepath.Join(dirs.DistroLibExecDir, "potato"))
 }
 
 func (s *toolSuite) TestInternalToolPathWithOtherDevLocationNonExecutable(c *C) {