From 5a782b1dd633369387a53ab58722980a2520202f Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Mon, 29 Jul 2024 14:42:30 +0200 Subject: [PATCH] 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 * sandbox/apparmor: more logging when checking apparmor_parser candidates Signed-off-by: Maciej Borzecki * 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 * cmd/snap: log errors when waiting for snapd to regenerate profiles Signed-off-by: Maciej Borzecki --------- Signed-off-by: Maciej Borzecki --- cmd/snap/cmd_run.go | 2 ++ sandbox/apparmor/apparmor.go | 4 ++++ secboot/encrypt_sb_test.go | 14 ++++++++------ snapdtool/tool_linux.go | 7 ------- snapdtool/tool_test.go | 4 ++-- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/cmd/snap/cmd_run.go b/cmd/snap/cmd_run.go index 62acb47b521..a887b31307b 100644 --- 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 index e08d9d4c325..895ece36441 100644 --- 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 index 9a7797b2a3d..2a271e4b503 100644 --- 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 index b696aa25a2b..2dd987b43c9 100644 --- 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 index f94e1f6989f..4604b6d372d 100644 --- 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) {