diff options
author | Zygmunt Krynicki <zygmunt.krynicki@canonical.com> | 2018-10-12 17:22:23 +0200 |
---|---|---|
committer | Michael Vogt <mvo@ubuntu.com> | 2018-10-15 08:02:57 +0200 |
commit | cd933d6630fda84b01b610bc77828656b1c79518 (patch) | |
tree | e28df7bedd321dce4189da37720fdde3ff5df757 | |
parent | 4098b8d7cde0ed7b758b638cb3e2fe2c0ee9810b (diff) |
interfaces: don't allow snaps to write to $HOME/bin
The $HOME/bin directory is added to default PATH on some systems. To prevent sandbox escape in this specific case deny writing to this directory. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
-rw-r--r-- | interfaces/builtin/home.go | 5 | ||||
-rw-r--r-- | interfaces/builtin/home_test.go | 2 | ||||
-rw-r--r-- | tests/regression/lp-1797556/task.yaml | 19 |
3 files changed, 26 insertions, 0 deletions
diff --git a/interfaces/builtin/home.go b/interfaces/builtin/home.go index f44dfb03eb..812895aa8a 100644 --- a/interfaces/builtin/home.go +++ b/interfaces/builtin/home.go @@ -74,6 +74,11 @@ owner @{HOME}/snap/ r, # files; only allow writes to files, not the mount point). owner /run/user/[0-9]*/gvfs/{,**} r, owner /run/user/[0-9]*/gvfs/*/** w, + +# On some systems HOME/bin is in the default path in ahead of system +# directories. To prevent snaps from writing arbitrary executables waiting to +# be launched by the user, deny writes to the directory altogether. +deny @{HOME}/bin/{,**} w, ` const homeConnectedPlugAppArmorWithAllRead = ` diff --git a/interfaces/builtin/home_test.go b/interfaces/builtin/home_test.go index b3bfd4bf06..c69eaa6524 100644 --- a/interfaces/builtin/home_test.go +++ b/interfaces/builtin/home_test.go @@ -152,6 +152,7 @@ func (s *HomeInterfaceSuite) TestConnectedPlugAppArmorWithoutAttrib(c *C) { c.Assert(err, IsNil) c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.other.app"}) c.Check(apparmorSpec.SnippetForTag("snap.other.app"), testutil.Contains, `owner @{HOME}/ r,`) + c.Check(apparmorSpec.SnippetForTag("snap.other.app"), testutil.Contains, `deny @{HOME}/bin/{,**} w,`) c.Check(apparmorSpec.SnippetForTag("snap.other.app"), Not(testutil.Contains), `# Allow non-owner read`) } @@ -172,6 +173,7 @@ apps: err := apparmorSpec.AddConnectedPlug(s.iface, plug, s.slot) c.Assert(err, IsNil) c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.home-plug-snap.app2"}) + c.Check(apparmorSpec.SnippetForTag("snap.home-plug-snap.app2"), testutil.Contains, `deny @{HOME}/bin/{,**} w,`) c.Check(apparmorSpec.SnippetForTag("snap.home-plug-snap.app2"), testutil.Contains, `owner @{HOME}/ r,`) c.Check(apparmorSpec.SnippetForTag("snap.home-plug-snap.app2"), testutil.Contains, `# Allow non-owner read`) } diff --git a/tests/regression/lp-1797556/task.yaml b/tests/regression/lp-1797556/task.yaml new file mode 100644 index 0000000000..22362cf6bc --- /dev/null +++ b/tests/regression/lp-1797556/task.yaml @@ -0,0 +1,19 @@ +summary: snaps cannot write to ~/bin +details: | + On some distributions the ~/bin directory is in the default PATH for the + user. To avoid hijacking commands and allow sandbox escape, writing to this + directory is denied. +prepare: | + #shellcheck source=tests/lib/snaps.sh + . "$TESTSLIB/snaps.sh" + install_local test-snapd-sh + su -l -c 'mkdir -p /home/test/bin/' test +restore: | + su -l -c 'rmdir /home/test/bin/ || true' test +execute: | + test -d /home/test/bin + test ! -e /home/test/bin/evil + if snap debug sandbox-features | grep 'apparmor:.* kernel:file'; then + ! su -l -c 'test-snapd-sh.with-home-plug -c touch /home/test/bin/evil' test + fi + test ! -e /home/test/bin/evil |