summaryrefslogtreecommitdiff
diff options
authorZygmunt Krynicki <zygmunt.krynicki@canonical.com>2018-10-12 17:22:23 +0200
committerMichael Vogt <mvo@ubuntu.com>2018-10-15 08:02:57 +0200
commitcd933d6630fda84b01b610bc77828656b1c79518 (patch)
treee28df7bedd321dce4189da37720fdde3ff5df757
parent4098b8d7cde0ed7b758b638cb3e2fe2c0ee9810b (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.go5
-rw-r--r--interfaces/builtin/home_test.go2
-rw-r--r--tests/regression/lp-1797556/task.yaml19
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