cve-2021-44730-44731-4120
authorMichael Hudson-Doyle <mwhudson@debian.org>
Mon, 28 Nov 2022 10:37:00 +0000 (10:37 +0000)
committerAlex Murray <alex.murray@canonical.com>
Mon, 28 Nov 2022 10:37:00 +0000 (10:37 +0000)
===================================================================

Gbp-Pq: Name 0015-cve-2021-44730-44731-4120.patch

43 files changed:
cmd/libsnap-confine-private/apparmor-support.c
cmd/libsnap-confine-private/tool.c
cmd/libsnap-confine-private/utils-test.c
cmd/libsnap-confine-private/utils.c
cmd/libsnap-confine-private/utils.h
cmd/snap-confine/mount-support-test.c
cmd/snap-confine/mount-support.c
cmd/snap-confine/snap-confine.apparmor.in
interfaces/apparmor/apparmor.go
interfaces/apparmor/apparmor_test.go
interfaces/apparmor/backend.go
interfaces/apparmor/backend_test.go
interfaces/apparmor/spec.go
interfaces/apparmor/spec_test.go
interfaces/apparmor/template.go
interfaces/backend.go
interfaces/builtin/common_files.go
interfaces/builtin/content.go
interfaces/builtin/content_test.go
interfaces/builtin/daemon_notify.go
interfaces/ifacetest/backendtest.go
interfaces/seccomp/backend_test.go
overlord/devicestate/firstboot_test.go
overlord/ifacestate/helpers.go
overlord/managers_test.go
packaging/ubuntu-16.04/tests/integrationtests
sandbox/apparmor/apparmor.go
sandbox/apparmor/apparmor_test.go
snap/validate.go
snap/validate_test.go
spread.yaml
tests/lib/state.sh
tests/main/docker-smoke/task.yaml
tests/main/snap-confine-tmp-mount/task.yaml [new file with mode: 0644]
tests/main/snap-confine-unexpected-path/task.yaml [new file with mode: 0644]
tests/main/snap-run-devmode-classic/task.yaml [new file with mode: 0644]
tests/nested/manual/core-seeding-devmode/task.yaml [new file with mode: 0644]
tests/nested/manual/devmode-snaps-can-run-other-snaps/task.yaml [new file with mode: 0644]
tests/regression/lp-1949368/bad-layout/meta/snap.yaml [new file with mode: 0644]
tests/regression/lp-1949368/content-consumer/bin/sh [new file with mode: 0644]
tests/regression/lp-1949368/content-consumer/meta/snap.yaml [new file with mode: 0644]
tests/regression/lp-1949368/content-provider/meta/snap.yaml [new file with mode: 0644]
tests/regression/lp-1949368/task.yaml [new file with mode: 0644]

index eac0912d3e64992a2a23afd1fb155f2ba3e71613..ae6fc048353af6cc86e46c95dd84ec68b340215c 100644 (file)
@@ -20,6 +20,8 @@
 #endif
 
 #include "apparmor-support.h"
+#include "string-utils.h"
+#include "utils.h"
 
 #include <string.h>
 #include <errno.h>
@@ -53,18 +55,24 @@ void sc_init_apparmor_support(struct sc_apparmor *apparmor)
                        debug
                            ("apparmor is available on the system but has been disabled at boot");
                        break;
-               case ENOENT:
-                       debug
-                           ("apparmor is available but the interface but the interface is not available");
-                       break;
                case EPERM:
                        // NOTE: fall-through
                case EACCES:
                        debug
                            ("insufficient permissions to determine if apparmor is enabled");
-                       break;
+                       // since snap-confine is setuid root this should
+                       // never happen so likely someone is trying to
+                       // manipulate our execution environment - fail hard
+
+                       // fall-through
+               case ENOENT:
+               case ENOMEM:
                default:
-                       debug("apparmor is not enabled: %s", strerror(errno));
+                       // this shouldn't happen under normal usage so it
+                       // is possible someone is trying to manipulate our
+                       // execution environment - fail hard
+                       die("aa_is_enabled() failed unexpectedly (%s)",
+                           strerror(errno));
                        break;
                }
                apparmor->is_confined = false;
@@ -81,13 +89,13 @@ void sc_init_apparmor_support(struct sc_apparmor *apparmor)
        }
        debug("apparmor label on snap-confine is: %s", label);
        debug("apparmor mode is: %s", mode);
-       // The label has a special value "unconfined" that is applied to all
-       // processes without a dedicated profile. If that label is used then the
-       // current process is not confined. All other labels imply confinement.
-       if (label != NULL && strcmp(label, SC_AA_UNCONFINED_STR) == 0) {
-               apparmor->is_confined = false;
-       } else {
+       // expect to be confined by a profile with the name of a valid
+       // snap-confine binary since if not we may be executed under a
+       // profile with more permissions than expected
+       if (label != NULL && sc_streq(mode, SC_AA_ENFORCE_STR) && sc_is_expected_path(label)) {
                apparmor->is_confined = true;
+       } else {
+               apparmor->is_confined = false;
        }
        // There are several possible results for the confinement type (mode) that
        // are checked for below.
index 008d53154ac5533d417b90ac7029b9887e65e28f..0c239538b9115f307be92819810b61c75669db2e 100644 (file)
@@ -110,7 +110,7 @@ void sc_call_snap_update_ns_as_user(int snap_update_ns_fd,
                         snap_name);
 
        const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-       char xdg_runtime_dir_env[PATH_MAX + strlen("XDG_RUNTIME_DIR=")];
+       char xdg_runtime_dir_env[PATH_MAX + sizeof("XDG_RUNTIME_DIR=")] = { 0 };
        if (xdg_runtime_dir != NULL) {
                sc_must_snprintf(xdg_runtime_dir_env,
                                 sizeof(xdg_runtime_dir_env),
@@ -163,14 +163,21 @@ static int sc_open_snapd_tool(const char *tool_name)
        // want to store the terminating '\0'. The readlink system call doesn't add
        // terminating null, but our initialization of buf handles this for us.
        char buf[PATH_MAX + 1] = { 0 };
-       if (readlink("/proc/self/exe", buf, sizeof buf) < 0) {
+       if (readlink("/proc/self/exe", buf, sizeof(buf) - 1) < 0) {
                die("cannot readlink /proc/self/exe");
        }
        if (buf[0] != '/') {    // this shouldn't happen, but make sure have absolute path
                die("readlink /proc/self/exe returned relative path");
        }
+       // as we are looking up other tools relative to our own path, check
+       // we are located where we think we should be - otherwise we
+       // may have been hardlink'd elsewhere and then may execute the
+       // wrong tool as a result
+       if (!sc_is_expected_path(buf)) {
+               die("running from unexpected location: %s", buf);
+       }
        char *dir_name = dirname(buf);
-       int dir_fd SC_CLEANUP(sc_cleanup_close) = 1;
+       int dir_fd SC_CLEANUP(sc_cleanup_close) = -1;
        dir_fd = open(dir_name, O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
        if (dir_fd < 0) {
                die("cannot open path %s", dir_name);
index 14709179b8f8f1047f97715e22ab4eda3e4c0eb9..2440d722a7f5e371f0f7555c1d5d95b477634f26 100644 (file)
@@ -71,6 +71,37 @@ static void test_parse_bool(void)
        g_assert_cmpint(errno, ==, EFAULT);
 }
 
+static void test_sc_is_expected_path(void)
+{
+       struct {
+               const char *path;
+               bool expected;
+       } test_cases[] = {
+               {"/tmp/snap-confine", false},
+               {"/tmp/foo", false},
+               {"/home/ ", false},
+               {"/usr/lib/snapd/snap-confine1", false},
+               {"/usr/lib/snapd/snap—confine", false},
+               {"/snap/core/usr/lib/snapd/snap-confine", false},
+               {"/snap/core/x1x/usr/lib/snapd/snap-confine", false},
+               {"/snap/core/z1/usr/lib/snapd/snap-confine", false},
+               {"/snap/cꓳre/1/usr/lib/snapd/snap-confine", false},
+               {"/snap/snapd1/1/usr/lib/snapd/snap-confine", false},
+               {"/snap/core/current/usr/lib/snapd/snap-confine", false},
+               {"/usr/lib/snapd/snap-confine", true},
+               {"/usr/libexec/snapd/snap-confine", true},
+               {"/snap/core/1/usr/lib/snapd/snap-confine", true},
+               {"/snap/core/x1/usr/lib/snapd/snap-confine", true},
+               {"/snap/snapd/1/usr/lib/snapd/snap-confine", true},
+               {"/snap/snapd/1/usr/libexec/snapd/snap-confine", false},
+       };
+       size_t i;
+       for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) {
+               bool result = sc_is_expected_path(test_cases[i].path);
+               g_assert_cmpint(result, ==, test_cases[i].expected);
+       }
+}
+
 static void test_die(void)
 {
        if (g_test_subprocess()) {
@@ -194,6 +225,7 @@ static void test_sc_nonfatal_mkpath__absolute(void)
 static void __attribute__((constructor)) init(void)
 {
        g_test_add_func("/utils/parse_bool", test_parse_bool);
+       g_test_add_func("/utils/sc_is_expected_path", test_sc_is_expected_path);
        g_test_add_func("/utils/die", test_die);
        g_test_add_func("/utils/die_with_errno", test_die_with_errno);
        g_test_add_func("/utils/sc_nonfatal_mkpath/relative",
index 8cd9eb8752048a402e11e7d43ee5c9e3be047ce8..ea7d65f3978cd85648a5bea4d771186e69027f36 100644 (file)
@@ -16,6 +16,7 @@
  */
 #include <errno.h>
 #include <fcntl.h>
+#include <regex.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -237,3 +238,15 @@ int sc_nonfatal_mkpath(const char *const path, mode_t mode)
        }
        return 0;
 }
+
+bool sc_is_expected_path(const char *path)
+{
+       const char *expected_path_re =
+           "^(/snap/(snapd|core)/x?[0-9]+/usr/lib|/usr/lib(exec)?)/snapd/snap-confine$";
+       regex_t re;
+       if (regcomp(&re, expected_path_re, REG_EXTENDED | REG_NOSUB) != 0)
+               die("can not compile regex %s", expected_path_re);
+       int status = regexec(&re, path, 0, NULL, 0);
+       regfree(&re);
+       return status == 0;
+}
index 34e9ea635cffded45d47302acfc8d2b49239c785..b535ffe135c7d544d33c6737e20cdd4700de5f91 100644 (file)
@@ -101,4 +101,10 @@ void write_string_to_file(const char *filepath, const char *buf);
  **/
 __attribute__((warn_unused_result))
 int sc_nonfatal_mkpath(const char *const path, mode_t mode);
+
+/**
+ * Return true if path is a valid path for the snap-confine binary
+ **/
+__attribute__((warn_unused_result))
+bool sc_is_expected_path(const char *path);
 #endif
index 751873f9ed5decee9fa71c8d5291df07305742db..9f2d78afecb1779b6bc43281cabcc66474119344 100644 (file)
@@ -21,6 +21,7 @@
 #include "mount-support-nvidia.c"
 
 #include <glib.h>
+#include <glib/gstdio.h>
 
 static void replace_slashes_with_NUL(char *path, size_t len)
 {
@@ -91,10 +92,50 @@ static void test_is_subdir(void)
        g_assert_false(is_subdir("/", ""));
 }
 
+static void test_must_mkdir_and_open_with_perms(void)
+{
+       // make a directory with some contents and check we can
+       // must_mkdir_and_open_with_perms() to get control of it
+       GError *error = NULL;
+       GStatBuf st;
+       gchar *test_dir = g_dir_make_tmp("test-mkdir-XXXXXX", &error);
+       g_assert_no_error(error);
+       g_assert_nonnull(test_dir);
+       g_assert_cmpint(chmod(test_dir, 0755), ==, 0);
+       g_assert_true(g_file_test
+                     (test_dir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR));
+       g_assert_cmpint(g_stat(test_dir, &st), ==, 0);
+       g_assert_true(st.st_uid == getuid());
+       g_assert_true(st.st_gid == getgid());
+       g_assert_true(st.st_mode == (S_IFDIR | 0755));
+
+       gchar *test_subdir = g_build_filename(test_dir, "foo", NULL);
+       g_assert_cmpint(g_mkdir_with_parents(test_dir, 0755), ==, 0);
+       g_file_test(test_subdir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR);
+
+       // take over dir
+       int fd =
+           must_mkdir_and_open_with_perms(test_dir, getuid(), getgid(), 0700);
+       // check can unlink dir itself with no contents successfully and it
+       // still exists
+       g_assert_cmpint(fd, >=, 0);
+       g_assert_false(g_file_test
+                      (test_subdir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR));
+       g_assert_true(g_file_test
+                     (test_dir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR));
+       g_assert_cmpint(g_stat(test_dir, &st), ==, 0);
+       g_assert_true(st.st_uid == getuid());
+       g_assert_true(st.st_gid == getgid());
+       g_assert_true(st.st_mode == (S_IFDIR | 0700));
+       close(fd);
+}
+
 static void __attribute__((constructor)) init(void)
 {
        g_test_add_func("/mount/get_nextpath/typical",
                        test_get_nextpath__typical);
        g_test_add_func("/mount/get_nextpath/weird", test_get_nextpath__weird);
        g_test_add_func("/mount/is_subdir", test_is_subdir);
+       g_test_add_func("/mount/must_mkdir_and_open_with_perms",
+                       test_must_mkdir_and_open_with_perms);
 }
index 7f4a80a51f44b9a43e09fdcd4dc22b0711630cae..d6ec6746c8c35f60fa6822b114589da722a3592f 100644 (file)
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
+
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
 
 static void sc_detach_views_of_writable(sc_distro distro, bool normal_mode);
 
+static int must_mkdir_and_open_with_perms(const char *dir, uid_t uid, gid_t gid,
+                                         mode_t mode)
+{
+       int retries = 10;
+       int fd;
+
+ mkdir:
+       if (--retries == 0) {
+               die("lost race to create dir %s too many times", dir);
+       }
+       // Ignore EEXIST since we want to reuse and we will open with
+       // O_NOFOLLOW, below.
+       if (mkdir(dir, 0700) < 0 && errno != EEXIST) {
+               die("cannot create directory %s", dir);
+       }
+       fd = open(dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW);
+       if (fd < 0) {
+               // if is not a directory then remove it and try again
+               if (errno == ENOTDIR && unlink(dir) == 0) {
+                       goto mkdir;
+               }
+               die("cannot open directory %s", dir);
+       }
+       // ensure base_dir has the expected permissions since it may have
+       // already existed
+       struct stat st;
+       if (fstat(fd, &st) < 0) {
+               die("cannot stat base directory %s", dir);
+       }
+       if (st.st_uid != uid || st.st_gid != gid
+           || st.st_mode != (S_IFDIR | mode)) {
+               unsigned char random[10] = { 0 };
+               char random_dir[MAX_BUF] = { 0 };
+               int offset;
+               size_t i;
+
+               // base_dir isn't what we expect - create a random
+               // directory name and rename the existing erroneous
+               // base_dir to this then try recreating it again - NOTE we
+               // don't use mkdtemp() here since we don't want to actually
+               // create the directory yet as we want rename() to do that
+               // for us
+#ifdef SYS_getrandom
+               // use syscall(SYS_getrandom) since getrandom() is
+               // not available on older glibc
+               if (syscall(SYS_getrandom, random, sizeof(random), 0) !=
+                   sizeof(random)) {
+                       die("cannot get random bytes");
+               }
+#else
+               // use /dev/urandom on older systems which don't support
+               // SYS_getrandom
+               int rfd = open("/dev/urandom", O_RDONLY);
+               if (rfd < 0) {
+                       die("cannot open /dev/urandom");
+               }
+               if (read(rfd, random, sizeof(random)) != sizeof(random)) {
+                       die("cannot get random bytes");
+               }
+               close(rfd);
+#endif
+               offset =
+                   sc_must_snprintf(random_dir, sizeof(random_dir), "%s.",
+                                    dir);
+               for (i = 0; i < sizeof(random); i++) {
+                       offset +=
+                           sc_must_snprintf(random_dir + offset,
+                                            sizeof(random_dir) - offset,
+                                            "%02x", (unsigned int)random[i]);
+               }
+               // try and get dir which we own by renaming it to something
+               // else then creating it again
+
+               // TODO - change this to use renameat2(RENAME_EXCHANGE)
+               // once we can use a newer version of glibc for snapd
+               if (rename(dir, random_dir) < 0) {
+                       die("cannot rename base_dir to random_dir '%s'",
+                           random_dir);
+               }
+               close(fd);
+               goto mkdir;
+       }
+       return fd;
+}
+
 // TODO: simplify this, after all it is just a tmpfs
 // TODO: fold this into bootstrap
 static void setup_private_mount(const char *snap_name)
@@ -86,29 +172,8 @@ static void setup_private_mount(const char *snap_name)
        /* Switch to root group so that mkdir and open calls below create filesystem
         * elements that are not owned by the user calling into snap-confine. */
        sc_identity old = sc_set_effective_identity(sc_root_group_identity());
-       // Create /tmp/snap.$SNAP_NAME/ 0700 root.root. Ignore EEXIST since we want
-       // to reuse and we will open with O_NOFOLLOW, below.
-       if (mkdir(base_dir, 0700) < 0 && errno != EEXIST) {
-               die("cannot create base directory %s", base_dir);
-       }
-       base_dir_fd = open(base_dir,
-                          O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW);
-       if (base_dir_fd < 0) {
-               die("cannot open base directory %s", base_dir);
-       }
-       /* This seems redundant on first read but it has the non-obvious
-        * property of changing existing directories  that have already existed
-        * but had incorrect ownership or permission. This is possible due to
-        * earlier bugs in snap-confine and due to the fact that some systems
-        * use persistent /tmp directory and may not clean up leftover files
-        * for arbitrarily long. This comment applies the following two pairs
-        * of fchmod and fchown. */
-       if (fchmod(base_dir_fd, 0700) < 0) {
-               die("cannot chmod base directory %s to 0700", base_dir);
-       }
-       if (fchown(base_dir_fd, 0, 0) < 0) {
-               die("cannot chown base directory %s to root.root", base_dir);
-       }
+       // Create /tmp/snap.$SNAP_NAME/ 0700 root.root.
+       base_dir_fd = must_mkdir_and_open_with_perms(base_dir, 0, 0, 0700);
        // Create /tmp/snap.$SNAP_NAME/tmp 01777 root.root Ignore EEXIST since we
        // want to reuse and we will open with O_NOFOLLOW, below.
        if (mkdirat(base_dir_fd, "tmp", 01777) < 0 && errno != EEXIST) {
@@ -120,14 +185,14 @@ static void setup_private_mount(const char *snap_name)
        if (tmp_dir_fd < 0) {
                die("cannot open private tmp directory %s/tmp", base_dir);
        }
-       if (fchmod(tmp_dir_fd, 01777) < 0) {
-               die("cannot chmod private tmp directory %s/tmp to 01777",
-                   base_dir);
-       }
        if (fchown(tmp_dir_fd, 0, 0) < 0) {
                die("cannot chown private tmp directory %s/tmp to root.root",
                    base_dir);
        }
+       if (fchmod(tmp_dir_fd, 01777) < 0) {
+               die("cannot chmod private tmp directory %s/tmp to 01777",
+                   base_dir);
+       }
        sc_do_mount(tmp_dir, "/tmp", NULL, MS_BIND, NULL);
        sc_do_mount("none", "/tmp", NULL, MS_PRIVATE, NULL);
 }
@@ -448,7 +513,8 @@ static void sc_bootstrap_mount_namespace(const struct sc_mount_config *config)
        sc_identity old = sc_set_effective_identity(sc_root_group_identity());
        if (mkdir(SC_HOSTFS_DIR, 0755) < 0) {
                if (errno != EEXIST) {
-                       die("cannot perform operation: mkdir %s", SC_HOSTFS_DIR);
+                       die("cannot perform operation: mkdir %s",
+                           SC_HOSTFS_DIR);
                }
        }
        (void)sc_set_effective_identity(old);
index dd262c9cbe54e6026f832513ca3022449e55d99e..e9c224ee8a2e9ffcd13ac3aad5a8b6cf36902262 100644 (file)
 
     @LIBEXECDIR@/snap-confine mr,
 
+    # This rule is needed when executing from a "base: core" devmode snap on 
+    # UC18 and newer where the /usr/lib/snapd/snap-confine inside the 
+    # "base: core" mount namespace always comes from the snapd snap, and thus
+    # we will execute snap-confine via this path, and thus need to be able to
+    # read this path when executing. It's also necessary on classic where both
+    # the snapd and the core snap are installed at the same time.
+    # TODO: remove this rule when we stop supporting executing other snaps from
+    # inside devmode snaps, ideally even in the short term we would only include
+    # this rule on core only, and specifically uc18 and newer where we need it
+    #@VERBATIM_LIBEXECDIR_SNAP_CONFINE@ mr,
+
     /dev/null rw,
     /dev/full rw,
     /dev/zero rw,
     # stacked filesystems generally.
     # encrypted ~/.Private and old-style encrypted $HOME
     @{HOME}/.Private/ r,
-    @{HOME}/.Private/** mrixwlk,
+    @{HOME}/.Private/** mrwlk,
     # new-style encrypted $HOME
     @{HOMEDIRS}/.ecryptfs/*/.Private/ r,
-    @{HOMEDIRS}/.ecryptfs/*/.Private/** mrixwlk,
+    @{HOMEDIRS}/.ecryptfs/*/.Private/** mrwlk,
 
     # Allow snap-confine to move to the void, creating it if necessary.
     /var/lib/snapd/void/ rw,
index 011f606658d2473ad64c318f187d4e2ca2ca8014..6d1d35750f4a0eb77703a807c4932a0c8f1a051b 100644 (file)
@@ -38,17 +38,6 @@ import (
        "github.com/snapcore/snapd/osutil"
 )
 
-// ValidateNoAppArmorRegexp will check that the given string does not
-// contain AppArmor regular expressions (AARE), double quotes or \0.
-func ValidateNoAppArmorRegexp(s string) error {
-       const AARE = `?*[]{}^"` + "\x00"
-
-       if strings.ContainsAny(s, AARE) {
-               return fmt.Errorf("%q contains a reserved apparmor char from %s", s, AARE)
-       }
-       return nil
-}
-
 type aaParserFlags int
 
 const (
index 20e4b600b80aef09c9fdaa947021e66edaf83688..888460ffacbfb2b790fc9c7f93e76bb1187e3812 100644 (file)
@@ -217,22 +217,6 @@ func (s *appArmorSuite) TestLoadedApparmorProfilesHandlesParsingErrors(c *C) {
        c.Check(profiles, IsNil)
 }
 
-func (s *appArmorSuite) TestValidateFreeFromAAREUnhappy(c *C) {
-       var testCases = []string{"a?", "*b", "c[c", "dd]", "e{", "f}", "g^", `h"`, "f\000", "g\x00"}
-
-       for _, s := range testCases {
-               c.Check(apparmor.ValidateNoAppArmorRegexp(s), ErrorMatches, ".* contains a reserved apparmor char from .*", Commentf("%q is not raising an error", s))
-       }
-}
-
-func (s *appArmorSuite) TestValidateFreeFromAAREhappy(c *C) {
-       var testCases = []string{"foo", "BaR", "b-z", "foo+bar", "b00m!", "be/ep", "a%b", "a&b", "a(b", "a)b", "a=b", "a#b", "a~b", "a'b", "a_b", "a,b", "a;b", "a>b", "a<b", "a|b"}
-
-       for _, s := range testCases {
-               c.Check(apparmor.ValidateNoAppArmorRegexp(s), IsNil, Commentf("%q raised an error but shouldn't", s))
-       }
-}
-
 func (s *appArmorSuite) TestMaybeSetNumberOfJobs(c *C) {
        var cpus int
        restore := apparmor.MockRuntimeNumCPU(func() int {
index 73b9c3ade80918bda7843dbe1444afbf04cbea9b..8b29ff6ab5b41d49a21a61dbc4cc290ab83018bf 100644 (file)
@@ -69,6 +69,9 @@ var (
 // Backend is responsible for maintaining apparmor profiles for snaps and parts of snapd.
 type Backend struct {
        preseed bool
+
+       coreSnap  *snap.Info
+       snapdSnap *snap.Info
 }
 
 // Name returns the name of the backend.
@@ -81,6 +84,11 @@ func (b *Backend) Initialize(opts *interfaces.SecurityBackendOptions) error {
        if opts != nil && opts.Preseed {
                b.preseed = true
        }
+
+       if opts != nil {
+               b.coreSnap = opts.CoreSnapInfo
+               b.snapdSnap = opts.SnapdSnapInfo
+       }
        // NOTE: It would be nice if we could also generate the profile for
        // snap-confine executing from the core snap, right here, and not have to
        // do this in the Setup function below. I sadly don't think this is
@@ -211,7 +219,16 @@ func snapConfineFromSnapProfile(info *snap.Info) (dir, glob string, content map[
        patchedProfileText := bytes.Replace(
                vanillaProfileText, []byte("/usr/lib/snapd/snap-confine"), []byte(snapConfineInCore), -1)
 
-       // We need to add a uniqe prefix that can never collide with a
+       // Also replace the test providing access to verbatim
+       // /usr/lib/snapd/snap-confine, which is necessary because to execute snaps
+       // from strict snaps, we need to be able read and map
+       // /usr/lib/snapd/snap-confine from inside the strict snap mount namespace,
+       // even though /usr/lib/snapd/snap-confine from inside the strict snap mount
+       // namespace is actually a bind mount to the "snapConfineInCore"
+       patchedProfileText = bytes.Replace(
+               patchedProfileText, []byte("#@VERBATIM_LIBEXECDIR_SNAP_CONFINE@"), []byte("/usr/lib/snapd/snap-confine"), -1)
+
+       // We need to add a unique prefix that can never collide with a
        // snap on the system. Using "snap-confine.*" is similar to
        // "snap-update-ns.*" that is already used there
        //
@@ -564,12 +581,12 @@ func (b *Backend) deriveContent(spec *Specification, snapInfo *snap.Info, opts i
        // Add profile for each app.
        for _, appInfo := range snapInfo.Apps {
                securityTag := appInfo.SecurityTag()
-               addContent(securityTag, snapInfo, appInfo.Name, opts, spec.SnippetForTag(securityTag), content, spec)
+               b.addContent(securityTag, snapInfo, appInfo.Name, opts, spec.SnippetForTag(securityTag), content, spec)
        }
        // Add profile for each hook.
        for _, hookInfo := range snapInfo.Hooks {
                securityTag := hookInfo.SecurityTag()
-               addContent(securityTag, snapInfo, "hook."+hookInfo.Name, opts, spec.SnippetForTag(securityTag), content, spec)
+               b.addContent(securityTag, snapInfo, "hook."+hookInfo.Name, opts, spec.SnippetForTag(securityTag), content, spec)
        }
        // Add profile for snap-update-ns if we have any apps or hooks.
        // If we have neither then we don't have any need to create an executing environment.
@@ -610,7 +627,7 @@ func addUpdateNSProfile(snapInfo *snap.Info, opts interfaces.ConfinementOptions,
        }
 }
 
-func addContent(securityTag string, snapInfo *snap.Info, cmdName string, opts interfaces.ConfinementOptions, snippetForTag string, content map[string]osutil.FileState, spec *Specification) {
+func (b *Backend) addContent(securityTag string, snapInfo *snap.Info, cmdName string, opts interfaces.ConfinementOptions, snippetForTag string, content map[string]osutil.FileState, spec *Specification) {
        // If base is specified and it doesn't match the core snaps (not
        // specifying a base should use the default core policy since in this
        // case, the 'core' snap is used for the runtime), use the base
@@ -638,6 +655,151 @@ func addContent(securityTag string, snapInfo *snap.Info, cmdName string, opts in
        }
        policy = templatePattern.ReplaceAllStringFunc(policy, func(placeholder string) string {
                switch placeholder {
+               case "###DEVMODE_SNAP_CONFINE###":
+                       if !opts.DevMode {
+                               // nothing to add if we are not in devmode
+                               return ""
+                       }
+
+                       // otherwise we need to generate special policy to allow executing
+                       // snap-confine from inside a devmode snap
+
+                       // TODO: we should deprecate this and drop it in a future release
+
+                       // assumes coreSnapInfo is not nil
+                       coreProfileTarget := func() string {
+                               return fmt.Sprintf("/snap/core/%s/usr/lib/snapd/snap-confine", b.coreSnap.SnapRevision().String())
+                       }
+
+                       // assumes snapdSnapInfo is not nil
+                       snapdProfileTarget := func() string {
+                               return fmt.Sprintf("/snap/snapd/%s/usr/lib/snapd/snap-confine", b.snapdSnap.SnapRevision().String())
+                       }
+
+                       // There are 3 main apparmor exec transition rules we need to
+                       // generate:
+                       // * exec( /usr/lib/snapd/snap-confine ... )
+                       // * exec( /snap/snapd/<rev>/usr/lib/snapd/snap-confine ... )
+                       // * exec( /snap/core/<rev>/usr/lib/snapd/snap-confine ... )
+
+                       // The latter two can always transition to their respective
+                       // revisioned profiles unambiguously if each snap is installed.
+
+                       // The former rule for /usr/lib/snapd/snap-confine however is
+                       // more tricky. First, we can say that if only the snapd snap is
+                       // installed, to just transition to that profile and be done. If
+                       // just the core snap is installed, then we can deduce this
+                       // system is either UC16 or a classic one, in both cases though
+                       // we have /usr/lib/snapd/snap-confine defined as the profile to
+                       // transition to.
+                       // If both snaps are installed however, then we need to branch
+                       // and pick a profile that exists, we can't just arbitrarily
+                       // pick one profile because not all profiles will exist on all
+                       // systems actually, for example the snap-confine profile from
+                       // the core snap will not be generated/installed on UC18+. We
+                       // can simplify the logic however by realizing that no matter
+                       // the relative version numbers of snapd and core, when
+                       // executing a snap with base other than core (i.e. base core18
+                       // or core20), the snapd snap's version of snap-confine will
+                       // always be used for various reasons. This is also true for
+                       // base: core snaps, but only on non-classic systems. So we
+                       // essentially say that /usr/lib/snapd/snap-confine always
+                       // transitions to the snapd snap profile if the base is not
+                       // core or if the system is not classic. If the base is core and
+                       // the system is classic, then the core snap profile will be
+                       // used.
+
+                       usrLibSnapdConfineTransitionTarget := ""
+                       switch {
+                       case b.coreSnap != nil && b.snapdSnap == nil:
+                               // only core snap - use /usr/lib/snapd/snap-confine always
+                               usrLibSnapdConfineTransitionTarget = "/usr/lib/snapd/snap-confine"
+                       case b.snapdSnap != nil && b.coreSnap == nil:
+                               // only snapd snap - use snapd snap version
+                               usrLibSnapdConfineTransitionTarget = snapdProfileTarget()
+                       case b.snapdSnap != nil && b.coreSnap != nil:
+                               // both are installed - need to check which one to use
+                               // TODO: is snapInfo.Base sometimes unset for snaps w/o bases
+                               // these days? maybe this needs to be this instead ?
+                               // if release.OnClassic && (snapInfo.Base == "core" || snapInfo.Base == "")
+                               if release.OnClassic && snapInfo.Base == "core" {
+                                       // use the core snap as the target only if we are on
+                                       // classic and the base is core
+                                       usrLibSnapdConfineTransitionTarget = coreProfileTarget()
+                               } else {
+                                       // otherwise always use snapd
+                                       usrLibSnapdConfineTransitionTarget = snapdProfileTarget()
+                               }
+
+                       default:
+                               // neither of the snaps are installed
+
+                               // TODO: this panic is unfortunate, but we don't have time
+                               // to do any better for this security release
+                               // It is actually important that we panic here, the only
+                               // known circumstance where this happens is when we are
+                               // seeding during first boot of UC16 with a very new core
+                               // snap (i.e. with the security fix of 2.54.3) and also have
+                               // a devmode confined snap in the seed to prepare. In this
+                               // situation, when we panic(), we force snapd to exit, and
+                               // systemd will restart us and we actually recover the
+                               // initial seed change and continue on. This code will be
+                               // removed/adapted before it is merged to the main branch,
+                               // it is only meant to exist on the security release branch.
+                               msg := fmt.Sprintf("neither snapd nor core snap available while preparing apparmor profile for devmode snap %s, panicing to restart snapd to continue seeding", snapInfo.InstanceName())
+                               panic(msg)
+                       }
+
+                       // We use Pxr for all these rules since the snap-confine profile
+                       // is not a child profile of the devmode complain profile we are
+                       // generating right now.
+                       usrLibSnapdConfineTransitionRule := fmt.Sprintf("/usr/lib/snapd/snap-confine Pxr -> %s,\n", usrLibSnapdConfineTransitionTarget)
+
+                       coreSnapConfineSnippet := ""
+                       if b.coreSnap != nil {
+                               coreSnapConfineSnippet = fmt.Sprintf("/snap/core/*/usr/lib/snapd/snap-confine Pxr -> %s,\n", coreProfileTarget())
+                       }
+
+                       snapdSnapConfineSnippet := ""
+                       if b.snapdSnap != nil {
+                               snapdSnapConfineSnippet = fmt.Sprintf("/snap/snapd/*/usr/lib/snapd/snap-confine Pxr -> %s,\n", snapdProfileTarget())
+                       }
+
+                       nonBaseCoreTransitionSnippet := coreSnapConfineSnippet + "\n" + snapdSnapConfineSnippet
+
+                       // include both rules for the core snap and the snapd snap since
+                       // we can't know which one will be used at runtime (for example
+                       // SNAP_REEXEC could be set which affects which one is used)
+                       return fmt.Sprintf(`
+  # allow executing the snap command from either the rootfs (for base: core) or
+  # from the system snaps (all other bases) - this is very specifically only to
+  # enable proper apparmor profile transition to snap-confine below, if we don't
+  # include these exec rules, then when executing the snap command, apparmor 
+  # will create a new, unique sub-profile which then cannot be transitioned from
+  # to the actual snap-confine profile
+  /usr/bin/snap ixr,
+  /snap/{snapd,core}/*/usr/bin/snap ixr,
+
+  # allow transitioning to snap-confine to support executing strict snaps from
+  # inside devmode confined snaps
+
+  # this first rule is to handle the case of exec()ing 
+  # /usr/lib/snapd/snap-confine directly, the profile we transition to depends
+  # on whether we are classic or not, what snaps (snapd or core) are installed
+  # and also whether this snap is a base: core snap or a differently based snap.
+  # see the comment in interfaces/backend/apparmor.go where this snippet is
+  # generated for the full context
+  %[1]s
+
+  # the second (and possibly third if both core and snapd are installed) rule is
+  # to handle direct exec() of snap-confine from the respective snaps directly, 
+  # this happens mostly on non-core based snaps, wherein the base snap has a 
+  # symlink from /usr/bin/snap -> /snap/snapd/current/usr/bin/snap, which makes
+  # the snap command execute snap-confine directly from the associated system 
+  # snap in /snap/{snapd,core}/<rev>/usr/lib/snapd/snap-confine
+  %[2]s
+`, usrLibSnapdConfineTransitionRule, nonBaseCoreTransitionSnippet)
+
                case "###VAR###":
                        return templateVariables(snapInfo, securityTag, cmdName)
                case "###PROFILEATTACH###":
index 8414a4f228ea2d424e85f3252c2dd51701db9454..06108439391036f479d30dc3832bc9300669fd66 100644 (file)
@@ -169,6 +169,9 @@ func (s *backendSuite) SetUpTest(c *C) {
        s.parserCmd = testutil.MockCommand(c, "apparmor_parser", fakeAppArmorParser)
 
        apparmor.MockRuntimeNumCPU(func() int { return 99 })
+
+       err = s.Backend.Initialize(ifacetest.DefaultInitializeOpts)
+       c.Assert(err, IsNil)
 }
 
 func (s *backendSuite) TearDownTest(c *C) {
@@ -1352,7 +1355,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyNoNFS(c *C) {
        defer cmd.Restore()
 
        // Setup generated policy for snap-confine.
-       err := (&apparmor.Backend{}).Initialize(nil)
+       err := (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
        c.Assert(cmd.Calls(), HasLen, 0)
 
@@ -1408,7 +1411,7 @@ func (s *backendSuite) testSetupSnapConfineGeneratedPolicyWithNFS(c *C, profileF
        c.Assert(ioutil.WriteFile(profilePath, []byte(""), 0644), IsNil)
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
 
        // Because NFS is being used, we have the extra policy file.
@@ -1461,7 +1464,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyWithNFSAndReExec(c *C)
        defer restore()
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
 
        // Because NFS is being used, we have the extra policy file.
@@ -1506,7 +1509,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError1(c *C) {
        defer restore()
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        // NOTE: Errors in determining NFS are non-fatal to prevent snapd from
        // failing to operate. A warning message is logged but system operates as
        // if NFS was not active.
@@ -1543,7 +1546,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError2(c *C) {
        defer restore()
 
        // Setup generated policy for snap-confine.
-       err := (&apparmor.Backend{}).Initialize(nil)
+       err := (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, ErrorMatches, "cannot read .*corrupt-proc-self-exe: .*")
 
        // We didn't create the policy file.
@@ -1582,7 +1585,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError3(c *C) {
        c.Assert(ioutil.WriteFile(filepath.Join(apparmor_sandbox.ConfDir, "usr.lib.snapd.snap-confine"), []byte(""), 0644), IsNil)
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, ErrorMatches, "cannot reload snap-confine apparmor profile: .*\n.*\ntesting\n")
 
        // While created the policy file initially we also removed it so that
@@ -1598,13 +1601,15 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError3(c *C) {
 // Test behavior when MkdirAll fails
 func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError4(c *C) {
        // Create a file where we would expect to find the local policy.
-       err := os.MkdirAll(filepath.Dir(dirs.SnapConfineAppArmorDir), 0755)
+       err := os.RemoveAll(filepath.Dir(dirs.SnapConfineAppArmorDir))
+       c.Assert(err, IsNil)
+       err = os.MkdirAll(filepath.Dir(dirs.SnapConfineAppArmorDir), 0755)
        c.Assert(err, IsNil)
        err = ioutil.WriteFile(dirs.SnapConfineAppArmorDir, []byte(""), 0644)
        c.Assert(err, IsNil)
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, ErrorMatches, "*.: not a directory")
 }
 
@@ -1651,7 +1656,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyError5(c *C) {
        defer os.Chmod(dirs.SnapConfineAppArmorDir, 0755)
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, ErrorMatches, `cannot synchronize snap-confine policy: remove .*/generated-test: permission denied`)
 
        // The policy directory was unchanged.
@@ -1677,7 +1682,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyNoOverlay(c *C) {
        defer cmd.Restore()
 
        // Setup generated policy for snap-confine.
-       err := (&apparmor.Backend{}).Initialize(nil)
+       err := (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
        c.Assert(cmd.Calls(), HasLen, 0)
 
@@ -1730,7 +1735,7 @@ func (s *backendSuite) testSetupSnapConfineGeneratedPolicyWithOverlay(c *C, prof
        c.Assert(ioutil.WriteFile(profilePath, []byte(""), 0644), IsNil)
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
 
        // Because overlay is being used, we have the extra policy file.
@@ -1782,7 +1787,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyWithOverlayAndReExec(c
        defer restore()
 
        // Setup generated policy for snap-confine.
-       err = (&apparmor.Backend{}).Initialize(nil)
+       err = (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts)
        c.Assert(err, IsNil)
 
        // Because overlay is being used, we have the extra policy file.
@@ -2240,7 +2245,10 @@ func (s *backendSuite) TestSetupManyInPreseedMode(c *C) {
        aa, ok := s.Backend.(*apparmor.Backend)
        c.Assert(ok, Equals, true)
 
-       opts := interfaces.SecurityBackendOptions{Preseed: true}
+       opts := interfaces.SecurityBackendOptions{
+               Preseed:      true,
+               CoreSnapInfo: ifacetest.DefaultInitializeOpts.CoreSnapInfo,
+       }
        c.Assert(aa.Initialize(&opts), IsNil)
 
        for _, opts := range testedConfinementOpts {
index 18f4359aff1e922eaf99034b628e875e6847bd5c..d58fdc946d55f739219f32619fa1bdd3cae2fe26 100644 (file)
@@ -293,30 +293,30 @@ func (spec *Specification) AddLayout(si *snap.Info) {
                case l.Bind != "":
                        bind := si.ExpandSnapVariables(l.Bind)
                        // Allow bind mounting the layout element.
-                       emit("  mount options=(rbind, rw) %s/ -> %s/,\n", bind, path)
-                       emit("  mount options=(rprivate) -> %s/,\n", path)
-                       emit("  umount %s/,\n", path)
+                       emit("  mount options=(rbind, rw) \"%s/\" -> \"%s/\",\n", bind, path)
+                       emit("  mount options=(rprivate) -> \"%s/\",\n", path)
+                       emit("  umount \"%s/\",\n", path)
                        // Allow constructing writable mimic in both bind-mount source and mount point.
                        GenWritableProfile(emit, path, 2) // At least / and /some-top-level-directory
                        GenWritableProfile(emit, bind, 4) // At least /, /snap/, /snap/$SNAP_NAME and /snap/$SNAP_NAME/$SNAP_REVISION
                case l.BindFile != "":
                        bindFile := si.ExpandSnapVariables(l.BindFile)
                        // Allow bind mounting the layout element.
-                       emit("  mount options=(bind, rw) %s -> %s,\n", bindFile, path)
-                       emit("  mount options=(rprivate) -> %s,\n", path)
-                       emit("  umount %s,\n", path)
+                       emit("  mount options=(bind, rw) \"%s\" -> \"%s\",\n", bindFile, path)
+                       emit("  mount options=(rprivate) -> \"%s\",\n", path)
+                       emit("  umount \"%s\",\n", path)
                        // Allow constructing writable mimic in both bind-mount source and mount point.
                        GenWritableFileProfile(emit, path, 2)     // At least / and /some-top-level-directory
                        GenWritableFileProfile(emit, bindFile, 4) // At least /, /snap/, /snap/$SNAP_NAME and /snap/$SNAP_NAME/$SNAP_REVISION
                case l.Type == "tmpfs":
-                       emit("  mount fstype=tmpfs tmpfs -> %s/,\n", path)
-                       emit("  mount options=(rprivate) -> %s/,\n", path)
-                       emit("  umount %s/,\n", path)
+                       emit("  mount fstype=tmpfs tmpfs -> \"%s/\",\n", path)
+                       emit("  mount options=(rprivate) -> \"%s/\",\n", path)
+                       emit("  umount \"%s/\",\n", path)
                        // Allow constructing writable mimic to mount point.
                        GenWritableProfile(emit, path, 2) // At least / and /some-top-level-directory
                case l.Symlink != "":
                        // Allow constructing writable mimic to symlink parent directory.
-                       emit("  %s rw,\n", path)
+                       emit("  \"%s\" rw,\n", path)
                        GenWritableProfile(emit, path, 2) // At least / and /some-top-level-directory
                }
        }
@@ -370,7 +370,7 @@ func GenWritableMimicProfile(emit func(f string, args ...interface{}), path stri
        emit("  # .. permissions for traversing the prefix that is assumed to exist\n")
        for iter.Next() {
                if iter.Depth() < assumedPrefixDepth {
-                       emit("  %s r,\n", iter.CurrentPath())
+                       emit("  \"%s\" r,\n", iter.CurrentPath())
                }
        }
 
@@ -388,33 +388,33 @@ func GenWritableMimicProfile(emit func(f string, args ...interface{}), path stri
                mimicAuxPath := filepath.Join("/tmp/.snap", iter.CurrentPath()) + "/"
                emit("  # .. variant with mimic at %s\n", mimicPath)
                emit("  # Allow reading the mimic directory, it must exist in the first place.\n")
-               emit("  %s r,\n", mimicPath)
+               emit("  \"%s\" r,\n", mimicPath)
                emit("  # Allow setting the read-only directory aside via a bind mount.\n")
-               emit("  %s rw,\n", mimicAuxPath)
-               emit("  mount options=(rbind, rw) %s -> %s,\n", mimicPath, mimicAuxPath)
+               emit("  \"%s\" rw,\n", mimicAuxPath)
+               emit("  mount options=(rbind, rw) \"%s\" -> \"%s\",\n", mimicPath, mimicAuxPath)
                emit("  # Allow mounting tmpfs over the read-only directory.\n")
-               emit("  mount fstype=tmpfs options=(rw) tmpfs -> %s,\n", mimicPath)
+               emit("  mount fstype=tmpfs options=(rw) tmpfs -> \"%s\",\n", mimicPath)
                emit("  # Allow creating empty files and directories for bind mounting things\n" +
                        "  # to reconstruct the now-writable parent directory.\n")
-               emit("  %s*/ rw,\n", mimicAuxPath)
-               emit("  %s*/ rw,\n", mimicPath)
-               emit("  mount options=(rbind, rw) %s*/ -> %s*/,\n", mimicAuxPath, mimicPath)
-               emit("  %s* rw,\n", mimicAuxPath)
-               emit("  %s* rw,\n", mimicPath)
-               emit("  mount options=(bind, rw) %s* -> %s*,\n", mimicAuxPath, mimicPath)
+               emit("  \"%s*/\" rw,\n", mimicAuxPath)
+               emit("  \"%s*/\" rw,\n", mimicPath)
+               emit("  mount options=(rbind, rw) \"%s*/\" -> \"%s*/\",\n", mimicAuxPath, mimicPath)
+               emit("  \"%s*\" rw,\n", mimicAuxPath)
+               emit("  \"%s*\" rw,\n", mimicPath)
+               emit("  mount options=(bind, rw) \"%s*\" -> \"%s*\",\n", mimicAuxPath, mimicPath)
                emit("  # Allow unmounting the auxiliary directory.\n" +
                        "  # TODO: use fstype=tmpfs here for more strictness (LP: #1613403)\n")
-               emit("  mount options=(rprivate) -> %s,\n", mimicAuxPath)
-               emit("  umount %s,\n", mimicAuxPath)
+               emit("  mount options=(rprivate) -> \"%s\",\n", mimicAuxPath)
+               emit("  umount \"%s\",\n", mimicAuxPath)
                emit("  # Allow unmounting the destination directory as well as anything\n" +
                        "  # inside.  This lets us perform the undo plan in case the writable\n" +
                        "  # mimic fails.\n")
-               emit("  mount options=(rprivate) -> %s,\n", mimicPath)
-               emit("  mount options=(rprivate) -> %s*,\n", mimicPath)
-               emit("  mount options=(rprivate) -> %s*/,\n", mimicPath)
-               emit("  umount %s,\n", mimicPath)
-               emit("  umount %s*,\n", mimicPath)
-               emit("  umount %s*/,\n", mimicPath)
+               emit("  mount options=(rprivate) -> \"%s\",\n", mimicPath)
+               emit("  mount options=(rprivate) -> \"%s*\",\n", mimicPath)
+               emit("  mount options=(rprivate) -> \"%s*/\",\n", mimicPath)
+               emit("  umount \"%s\",\n", mimicPath)
+               emit("  umount \"%s*\",\n", mimicPath)
+               emit("  umount \"%s*/\",\n", mimicPath)
        }
 }
 
@@ -425,9 +425,9 @@ func GenWritableFileProfile(emit func(f string, args ...interface{}), path strin
        }
        if isProbablyWritable(path) {
                emit("  # Writable file %s\n", path)
-               emit("  %s rw,\n", path)
+               emit("  \"%s\" rw,\n", path)
                for p := parent(path); !isProbablyPresent(p); p = parent(p) {
-                       emit("  %s/ rw,\n", p)
+                       emit("  \"%s/\" rw,\n", p)
                }
        } else {
                parentPath := parent(path)
@@ -443,7 +443,7 @@ func GenWritableProfile(emit func(f string, args ...interface{}), path string, a
        if isProbablyWritable(path) {
                emit("  # Writable directory %s\n", path)
                for p := path; !isProbablyPresent(p); p = parent(p) {
-                       emit("  %s/ rw,\n", p)
+                       emit("  \"%s/\" rw,\n", p)
                }
        } else {
                parentPath := parent(path)
@@ -537,9 +537,9 @@ func (spec *Specification) UpdateNS() []string {
 func snippetFromLayout(layout *snap.Layout) string {
        mountPoint := layout.Snap.ExpandSnapVariables(layout.Path)
        if layout.Bind != "" || layout.Type == "tmpfs" {
-               return fmt.Sprintf("# Layout path: %s\n%s{,/**} mrwklix,", mountPoint, mountPoint)
+               return fmt.Sprintf("# Layout path: %s\n\"%s{,/**}\" mrwklix,", mountPoint, mountPoint)
        } else if layout.BindFile != "" {
-               return fmt.Sprintf("# Layout path: %s\n%s mrwklix,", mountPoint, mountPoint)
+               return fmt.Sprintf("# Layout path: %s\n\"%s\" mrwklix,", mountPoint, mountPoint)
        }
        return fmt.Sprintf("# Layout path: %s\n# (no extra permissions required for symlink)", mountPoint)
 }
index 796af910ecd3c48b8a55c0af5284ae802c8e1a0f..3cd2e676a9b0daf8c81fb0bddaa3cf2d87dc0601 100644 (file)
@@ -284,72 +284,72 @@ func (s *specSuite) TestApparmorSnippetsFromLayout(c *C) {
        s.spec.AddLayout(snapInfo)
        c.Assert(s.spec.Snippets(), DeepEquals, map[string][]string{
                "snap.vanguard.vanguard": {
-                       "# Layout path: /etc/foo.conf\n/etc/foo.conf mrwklix,",
-                       "# Layout path: /usr/foo\n/usr/foo{,/**} mrwklix,",
+                       "# Layout path: /etc/foo.conf\n\"/etc/foo.conf\" mrwklix,",
+                       "# Layout path: /usr/foo\n\"/usr/foo{,/**}\" mrwklix,",
                        "# Layout path: /var/cache/mylink\n# (no extra permissions required for symlink)",
-                       "# Layout path: /var/tmp\n/var/tmp{,/**} mrwklix,",
+                       "# Layout path: /var/tmp\n\"/var/tmp{,/**}\" mrwklix,",
                },
        })
        updateNS := s.spec.UpdateNS()
 
        profile0 := `  # Layout /etc/foo.conf: bind-file $SNAP/foo.conf
-  mount options=(bind, rw) /snap/vanguard/42/foo.conf -> /etc/foo.conf,
-  mount options=(rprivate) -> /etc/foo.conf,
-  umount /etc/foo.conf,
+  mount options=(bind, rw) "/snap/vanguard/42/foo.conf" -> "/etc/foo.conf",
+  mount options=(rprivate) -> "/etc/foo.conf",
+  umount "/etc/foo.conf",
   # Writable mimic /etc
   # .. permissions for traversing the prefix that is assumed to exist
-  / r,
+  "/" r,
   # .. variant with mimic at /etc/
   # Allow reading the mimic directory, it must exist in the first place.
-  /etc/ r,
+  "/etc/" r,
   # Allow setting the read-only directory aside via a bind mount.
-  /tmp/.snap/etc/ rw,
-  mount options=(rbind, rw) /etc/ -> /tmp/.snap/etc/,
+  "/tmp/.snap/etc/" rw,
+  mount options=(rbind, rw) "/etc/" -> "/tmp/.snap/etc/",
   # Allow mounting tmpfs over the read-only directory.
-  mount fstype=tmpfs options=(rw) tmpfs -> /etc/,
+  mount fstype=tmpfs options=(rw) tmpfs -> "/etc/",
   # Allow creating empty files and directories for bind mounting things
   # to reconstruct the now-writable parent directory.
-  /tmp/.snap/etc/*/ rw,
-  /etc/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/etc/*/ -> /etc/*/,
-  /tmp/.snap/etc/* rw,
-  /etc/* rw,
-  mount options=(bind, rw) /tmp/.snap/etc/* -> /etc/*,
+  "/tmp/.snap/etc/*/" rw,
+  "/etc/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/etc/*/" -> "/etc/*/",
+  "/tmp/.snap/etc/*" rw,
+  "/etc/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/etc/*" -> "/etc/*",
   # Allow unmounting the auxiliary directory.
   # TODO: use fstype=tmpfs here for more strictness (LP: #1613403)
-  mount options=(rprivate) -> /tmp/.snap/etc/,
-  umount /tmp/.snap/etc/,
+  mount options=(rprivate) -> "/tmp/.snap/etc/",
+  umount "/tmp/.snap/etc/",
   # Allow unmounting the destination directory as well as anything
   # inside.  This lets us perform the undo plan in case the writable
   # mimic fails.
-  mount options=(rprivate) -> /etc/,
-  mount options=(rprivate) -> /etc/*,
-  mount options=(rprivate) -> /etc/*/,
-  umount /etc/,
-  umount /etc/*,
-  umount /etc/*/,
+  mount options=(rprivate) -> "/etc/",
+  mount options=(rprivate) -> "/etc/*",
+  mount options=(rprivate) -> "/etc/*/",
+  umount "/etc/",
+  umount "/etc/*",
+  umount "/etc/*/",
   # Writable mimic /snap/vanguard/42
-  /snap/ r,
-  /snap/vanguard/ r,
+  "/snap/" r,
+  "/snap/vanguard/" r,
   # .. variant with mimic at /snap/vanguard/42/
-  /snap/vanguard/42/ r,
-  /tmp/.snap/snap/vanguard/42/ rw,
-  mount options=(rbind, rw) /snap/vanguard/42/ -> /tmp/.snap/snap/vanguard/42/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/vanguard/42/,
-  /tmp/.snap/snap/vanguard/42/*/ rw,
-  /snap/vanguard/42/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/vanguard/42/*/ -> /snap/vanguard/42/*/,
-  /tmp/.snap/snap/vanguard/42/* rw,
-  /snap/vanguard/42/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/vanguard/42/* -> /snap/vanguard/42/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/vanguard/42/,
-  umount /tmp/.snap/snap/vanguard/42/,
-  mount options=(rprivate) -> /snap/vanguard/42/,
-  mount options=(rprivate) -> /snap/vanguard/42/*,
-  mount options=(rprivate) -> /snap/vanguard/42/*/,
-  umount /snap/vanguard/42/,
-  umount /snap/vanguard/42/*,
-  umount /snap/vanguard/42/*/,
+  "/snap/vanguard/42/" r,
+  "/tmp/.snap/snap/vanguard/42/" rw,
+  mount options=(rbind, rw) "/snap/vanguard/42/" -> "/tmp/.snap/snap/vanguard/42/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/vanguard/42/",
+  "/tmp/.snap/snap/vanguard/42/*/" rw,
+  "/snap/vanguard/42/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/vanguard/42/*/" -> "/snap/vanguard/42/*/",
+  "/tmp/.snap/snap/vanguard/42/*" rw,
+  "/snap/vanguard/42/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/vanguard/42/*" -> "/snap/vanguard/42/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/vanguard/42/",
+  umount "/tmp/.snap/snap/vanguard/42/",
+  mount options=(rprivate) -> "/snap/vanguard/42/",
+  mount options=(rprivate) -> "/snap/vanguard/42/*",
+  mount options=(rprivate) -> "/snap/vanguard/42/*/",
+  umount "/snap/vanguard/42/",
+  umount "/snap/vanguard/42/*",
+  umount "/snap/vanguard/42/*/",
 `
        // Find the slice that describes profile0 by looking for the first unique
        // line of the next profile.
@@ -358,49 +358,49 @@ func (s *specSuite) TestApparmorSnippetsFromLayout(c *C) {
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile0)
 
        profile1 := `  # Layout /usr/foo: bind $SNAP/usr/foo
-  mount options=(rbind, rw) /snap/vanguard/42/usr/foo/ -> /usr/foo/,
-  mount options=(rprivate) -> /usr/foo/,
-  umount /usr/foo/,
+  mount options=(rbind, rw) "/snap/vanguard/42/usr/foo/" -> "/usr/foo/",
+  mount options=(rprivate) -> "/usr/foo/",
+  umount "/usr/foo/",
   # Writable mimic /usr
   # .. variant with mimic at /usr/
-  /usr/ r,
-  /tmp/.snap/usr/ rw,
-  mount options=(rbind, rw) /usr/ -> /tmp/.snap/usr/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /usr/,
-  /tmp/.snap/usr/*/ rw,
-  /usr/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/usr/*/ -> /usr/*/,
-  /tmp/.snap/usr/* rw,
-  /usr/* rw,
-  mount options=(bind, rw) /tmp/.snap/usr/* -> /usr/*,
-  mount options=(rprivate) -> /tmp/.snap/usr/,
-  umount /tmp/.snap/usr/,
-  mount options=(rprivate) -> /usr/,
-  mount options=(rprivate) -> /usr/*,
-  mount options=(rprivate) -> /usr/*/,
-  umount /usr/,
-  umount /usr/*,
-  umount /usr/*/,
+  "/usr/" r,
+  "/tmp/.snap/usr/" rw,
+  mount options=(rbind, rw) "/usr/" -> "/tmp/.snap/usr/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/usr/",
+  "/tmp/.snap/usr/*/" rw,
+  "/usr/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/usr/*/" -> "/usr/*/",
+  "/tmp/.snap/usr/*" rw,
+  "/usr/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/usr/*" -> "/usr/*",
+  mount options=(rprivate) -> "/tmp/.snap/usr/",
+  umount "/tmp/.snap/usr/",
+  mount options=(rprivate) -> "/usr/",
+  mount options=(rprivate) -> "/usr/*",
+  mount options=(rprivate) -> "/usr/*/",
+  umount "/usr/",
+  umount "/usr/*",
+  umount "/usr/*/",
   # Writable mimic /snap/vanguard/42/usr
   # .. variant with mimic at /snap/vanguard/42/usr/
-  /snap/vanguard/42/usr/ r,
-  /tmp/.snap/snap/vanguard/42/usr/ rw,
-  mount options=(rbind, rw) /snap/vanguard/42/usr/ -> /tmp/.snap/snap/vanguard/42/usr/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/vanguard/42/usr/,
-  /tmp/.snap/snap/vanguard/42/usr/*/ rw,
-  /snap/vanguard/42/usr/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/vanguard/42/usr/*/ -> /snap/vanguard/42/usr/*/,
-  /tmp/.snap/snap/vanguard/42/usr/* rw,
-  /snap/vanguard/42/usr/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/vanguard/42/usr/* -> /snap/vanguard/42/usr/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/vanguard/42/usr/,
-  umount /tmp/.snap/snap/vanguard/42/usr/,
-  mount options=(rprivate) -> /snap/vanguard/42/usr/,
-  mount options=(rprivate) -> /snap/vanguard/42/usr/*,
-  mount options=(rprivate) -> /snap/vanguard/42/usr/*/,
-  umount /snap/vanguard/42/usr/,
-  umount /snap/vanguard/42/usr/*,
-  umount /snap/vanguard/42/usr/*/,
+  "/snap/vanguard/42/usr/" r,
+  "/tmp/.snap/snap/vanguard/42/usr/" rw,
+  mount options=(rbind, rw) "/snap/vanguard/42/usr/" -> "/tmp/.snap/snap/vanguard/42/usr/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/vanguard/42/usr/",
+  "/tmp/.snap/snap/vanguard/42/usr/*/" rw,
+  "/snap/vanguard/42/usr/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/vanguard/42/usr/*/" -> "/snap/vanguard/42/usr/*/",
+  "/tmp/.snap/snap/vanguard/42/usr/*" rw,
+  "/snap/vanguard/42/usr/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/vanguard/42/usr/*" -> "/snap/vanguard/42/usr/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/vanguard/42/usr/",
+  umount "/tmp/.snap/snap/vanguard/42/usr/",
+  mount options=(rprivate) -> "/snap/vanguard/42/usr/",
+  mount options=(rprivate) -> "/snap/vanguard/42/usr/*",
+  mount options=(rprivate) -> "/snap/vanguard/42/usr/*/",
+  umount "/snap/vanguard/42/usr/",
+  umount "/snap/vanguard/42/usr/*",
+  umount "/snap/vanguard/42/usr/*/",
 `
        // Find the slice that describes profile1 by looking for the first unique
        // line of the next profile.
@@ -409,46 +409,46 @@ func (s *specSuite) TestApparmorSnippetsFromLayout(c *C) {
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile1)
 
        profile2 := `  # Layout /var/cache/mylink: symlink $SNAP_DATA/link/target
-  /var/cache/mylink rw,
+  "/var/cache/mylink" rw,
   # Writable mimic /var/cache
   # .. variant with mimic at /var/
-  /var/ r,
-  /tmp/.snap/var/ rw,
-  mount options=(rbind, rw) /var/ -> /tmp/.snap/var/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /var/,
-  /tmp/.snap/var/*/ rw,
-  /var/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/var/*/ -> /var/*/,
-  /tmp/.snap/var/* rw,
-  /var/* rw,
-  mount options=(bind, rw) /tmp/.snap/var/* -> /var/*,
-  mount options=(rprivate) -> /tmp/.snap/var/,
-  umount /tmp/.snap/var/,
-  mount options=(rprivate) -> /var/,
-  mount options=(rprivate) -> /var/*,
-  mount options=(rprivate) -> /var/*/,
-  umount /var/,
-  umount /var/*,
-  umount /var/*/,
+  "/var/" r,
+  "/tmp/.snap/var/" rw,
+  mount options=(rbind, rw) "/var/" -> "/tmp/.snap/var/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/var/",
+  "/tmp/.snap/var/*/" rw,
+  "/var/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/var/*/" -> "/var/*/",
+  "/tmp/.snap/var/*" rw,
+  "/var/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/var/*" -> "/var/*",
+  mount options=(rprivate) -> "/tmp/.snap/var/",
+  umount "/tmp/.snap/var/",
+  mount options=(rprivate) -> "/var/",
+  mount options=(rprivate) -> "/var/*",
+  mount options=(rprivate) -> "/var/*/",
+  umount "/var/",
+  umount "/var/*",
+  umount "/var/*/",
   # .. variant with mimic at /var/cache/
-  /var/cache/ r,
-  /tmp/.snap/var/cache/ rw,
-  mount options=(rbind, rw) /var/cache/ -> /tmp/.snap/var/cache/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /var/cache/,
-  /tmp/.snap/var/cache/*/ rw,
-  /var/cache/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/var/cache/*/ -> /var/cache/*/,
-  /tmp/.snap/var/cache/* rw,
-  /var/cache/* rw,
-  mount options=(bind, rw) /tmp/.snap/var/cache/* -> /var/cache/*,
-  mount options=(rprivate) -> /tmp/.snap/var/cache/,
-  umount /tmp/.snap/var/cache/,
-  mount options=(rprivate) -> /var/cache/,
-  mount options=(rprivate) -> /var/cache/*,
-  mount options=(rprivate) -> /var/cache/*/,
-  umount /var/cache/,
-  umount /var/cache/*,
-  umount /var/cache/*/,
+  "/var/cache/" r,
+  "/tmp/.snap/var/cache/" rw,
+  mount options=(rbind, rw) "/var/cache/" -> "/tmp/.snap/var/cache/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/var/cache/",
+  "/tmp/.snap/var/cache/*/" rw,
+  "/var/cache/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/var/cache/*/" -> "/var/cache/*/",
+  "/tmp/.snap/var/cache/*" rw,
+  "/var/cache/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/var/cache/*" -> "/var/cache/*",
+  mount options=(rprivate) -> "/tmp/.snap/var/cache/",
+  umount "/tmp/.snap/var/cache/",
+  mount options=(rprivate) -> "/var/cache/",
+  mount options=(rprivate) -> "/var/cache/*",
+  mount options=(rprivate) -> "/var/cache/*/",
+  umount "/var/cache/",
+  umount "/var/cache/*",
+  umount "/var/cache/*/",
 `
        // Find the slice that describes profile2 by looking for the first unique
        // line of the next profile.
@@ -457,9 +457,9 @@ func (s *specSuite) TestApparmorSnippetsFromLayout(c *C) {
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile2)
 
        profile3 := `  # Layout /var/tmp: type tmpfs, mode: 01777
-  mount fstype=tmpfs tmpfs -> /var/tmp/,
-  mount options=(rprivate) -> /var/tmp/,
-  umount /var/tmp/,
+  mount fstype=tmpfs tmpfs -> "/var/tmp/",
+  mount options=(rprivate) -> "/var/tmp/",
+  umount "/var/tmp/",
   # Writable mimic /var
 `
        // Find the slice that describes profile2 by looking till the end of the list.
index 187423e4d04b9daf5be5ac10ab63a8c32feae4b4..d07e71bf1c165baf5f26e10fb55f9500c510cd5c 100644 (file)
@@ -453,6 +453,9 @@ var templateCommon = `
   /run/lock/ r,
   /run/lock/snap.@{SNAP_INSTANCE_NAME}/ rw,
   /run/lock/snap.@{SNAP_INSTANCE_NAME}/** mrwklix,
+
+
+  ###DEVMODE_SNAP_CONFINE###
 `
 
 var templateFooter = `
index f635019e19332a4d96792fac23f25155797c5738..f731ff1103a7afb21185b17203386476b8d3c523 100644 (file)
@@ -70,6 +70,12 @@ type ConfinementOptions struct {
 type SecurityBackendOptions struct {
        // Preseed flag is set when snapd runs in preseed mode.
        Preseed bool
+       // CoreSnapInfo is the current revision of the core snap (if it is
+       // installed)
+       CoreSnapInfo *snap.Info
+       // SnapdSnapInfo is the current revision of the snapd snap (if it is
+       // installed)
+       SnapdSnapInfo *snap.Info
 }
 
 // SecurityBackend abstracts interactions between the interface system and the
index ef1804e8f43600f64e960672cb28ccec7b9847d5..219aa60d79e515750786422c1749cc104eb371af 100644 (file)
@@ -27,6 +27,7 @@ import (
 
        "github.com/snapcore/snapd/interfaces"
        "github.com/snapcore/snapd/interfaces/apparmor"
+       apparmor_sandbox "github.com/snapcore/snapd/sandbox/apparmor"
        "github.com/snapcore/snapd/snap"
 )
 
@@ -110,7 +111,7 @@ func (iface *commonFilesInterface) validateSinglePath(np string) error {
        if strings.Contains(p, "~") {
                return fmt.Errorf(`%q cannot contain "~"`, p)
        }
-       if err := apparmor.ValidateNoAppArmorRegexp(p); err != nil {
+       if err := apparmor_sandbox.ValidateNoAppArmorRegexp(p); err != nil {
                return err
        }
 
index fe0a1fdef2a0da26043cce91a5d9b169826ae175..99d639b8ccaf30812ed9efe756c1b6a69dfed212 100644 (file)
@@ -29,6 +29,7 @@ import (
        "github.com/snapcore/snapd/interfaces/apparmor"
        "github.com/snapcore/snapd/interfaces/mount"
        "github.com/snapcore/snapd/osutil"
+       apparmor_sandbox "github.com/snapcore/snapd/sandbox/apparmor"
        "github.com/snapcore/snapd/snap"
 )
 
@@ -68,6 +69,16 @@ func cleanSubPath(path string) bool {
        return filepath.Clean(path) == path && path != ".." && !strings.HasPrefix(path, "../")
 }
 
+func validatePath(path string) error {
+       if err := apparmor_sandbox.ValidateNoAppArmorRegexp(path); err != nil {
+               return fmt.Errorf("content interface path is invalid: %v", err)
+       }
+       if ok := cleanSubPath(path); !ok {
+               return fmt.Errorf("content interface path is not clean: %q", path)
+       }
+       return nil
+}
+
 func (iface *contentInterface) BeforePrepareSlot(slot *snap.SlotInfo) error {
        content, ok := slot.Attrs["content"].(string)
        if !ok || len(content) == 0 {
@@ -102,8 +113,8 @@ func (iface *contentInterface) BeforePrepareSlot(slot *snap.SlotInfo) error {
        paths := rpath
        paths = append(paths, wpath...)
        for _, p := range paths {
-               if !cleanSubPath(p) {
-                       return fmt.Errorf("content interface path is not clean: %q", p)
+               if err := validatePath(p); err != nil {
+                       return err
                }
        }
        return nil
@@ -122,8 +133,8 @@ func (iface *contentInterface) BeforePreparePlug(plug *snap.PlugInfo) error {
        if !ok || len(target) == 0 {
                return fmt.Errorf("content plug must contain target path")
        }
-       if !cleanSubPath(target) {
-               return fmt.Errorf("content interface target path is not clean: %q", target)
+       if err := validatePath(target); err != nil {
+               return err
        }
 
        return nil
@@ -224,13 +235,13 @@ func (iface *contentInterface) AppArmorConnectedPlug(spec *apparmor.Specificatio
 # directory.
 `)
                for i, w := range writePaths {
-                       fmt.Fprintf(contentSnippet, "%s/** mrwklix,\n",
+                       fmt.Fprintf(contentSnippet, "\"%s/**\" mrwklix,\n",
                                resolveSpecialVariable(w, slot.Snap()))
                        source, target := sourceTarget(plug, slot, w)
                        emit("  # Read-write content sharing %s -> %s (w#%d)\n", plug.Ref(), slot.Ref(), i)
-                       emit("  mount options=(bind, rw) %s/ -> %s{,-[0-9]*}/,\n", source, target)
-                       emit("  mount options=(rprivate) -> %s{,-[0-9]*}/,\n", target)
-                       emit("  umount %s{,-[0-9]*}/,\n", target)
+                       emit("  mount options=(bind, rw) \"%s/\" -> \"%s{,-[0-9]*}/\",\n", source, target)
+                       emit("  mount options=(rprivate) -> \"%s{,-[0-9]*}/\",\n", target)
+                       emit("  umount \"%s{,-[0-9]*}/\",\n", target)
                        // TODO: The assumed prefix depth could be optimized to be more
                        // precise since content sharing can only take place in a fixed
                        // list of places with well-known paths (well, constrained set of
@@ -249,15 +260,15 @@ func (iface *contentInterface) AppArmorConnectedPlug(spec *apparmor.Specificatio
 # read-only.
 `)
                for i, r := range readPaths {
-                       fmt.Fprintf(contentSnippet, "%s/** mrkix,\n",
+                       fmt.Fprintf(contentSnippet, "\"%s/**\" mrkix,\n",
                                resolveSpecialVariable(r, slot.Snap()))
 
                        source, target := sourceTarget(plug, slot, r)
                        emit("  # Read-only content sharing %s -> %s (r#%d)\n", plug.Ref(), slot.Ref(), i)
-                       emit("  mount options=(bind) %s/ -> %s{,-[0-9]*}/,\n", source, target)
-                       emit("  remount options=(bind, ro) %s{,-[0-9]*}/,\n", target)
-                       emit("  mount options=(rprivate) -> %s{,-[0-9]*}/,\n", target)
-                       emit("  umount %s{,-[0-9]*}/,\n", target)
+                       emit("  mount options=(bind) \"%s/\" -> \"%s{,-[0-9]*}/\",\n", source, target)
+                       emit("  remount options=(bind, ro) \"%s{,-[0-9]*}/\",\n", target)
+                       emit("  mount options=(rprivate) -> \"%s{,-[0-9]*}/\",\n", target)
+                       emit("  umount \"%s{,-[0-9]*}/\",\n", target)
                        // Look at the TODO comment above.
                        apparmor.GenWritableProfile(emit, source, 1)
                        apparmor.GenWritableProfile(emit, target, 1)
@@ -281,7 +292,7 @@ func (iface *contentInterface) AppArmorConnectedSlot(spec *apparmor.Specificatio
 `)
                for _, w := range writePaths {
                        _, target := sourceTarget(plug, slot, w)
-                       fmt.Fprintf(contentSnippet, "%s/** mrwklix,\n",
+                       fmt.Fprintf(contentSnippet, "\"%s/**\" mrwklix,\n",
                                target)
                }
        }
index cbbb74dba48288c5e60a2568eadf694d6f790bcd..7f05686069a68a905bc6b346e8a2e3d3ff676ed8 100644 (file)
@@ -194,7 +194,22 @@ plugs:
 `
        info := snaptest.MockInfo(c, mockSnapYaml, nil)
        plug := info.Plugs["content-plug"]
-       c.Assert(interfaces.BeforePreparePlug(s.iface, plug), ErrorMatches, "content interface target path is not clean:.*")
+       c.Assert(interfaces.BeforePreparePlug(s.iface, plug), ErrorMatches, "content interface path is not clean:.*")
+}
+
+func (s *ContentSuite) TestSanitizePlugApparmorInterpretedChar(c *C) {
+       const mockSnapYaml = `name: content-slot-snap
+version: 1.0
+plugs:
+ content-plug:
+  interface: content
+  content: mycont
+  target: foo"bar
+`
+       info := snaptest.MockInfo(c, mockSnapYaml, nil)
+       plug := info.Plugs["content-plug"]
+       c.Assert(interfaces.BeforePreparePlug(s.iface, plug), ErrorMatches,
+               `content interface path is invalid: "foo\\"bar" contains a reserved apparmor char.*`)
 }
 
 func (s *ContentSuite) TestSanitizePlugNilAttrMap(c *C) {
@@ -223,6 +238,22 @@ apps:
        c.Assert(interfaces.BeforePrepareSlot(s.iface, slot), ErrorMatches, "read or write path must be set")
 }
 
+func (s *ContentSuite) TestSanitizeSlotApparmorInterpretedChar(c *C) {
+       const mockSnapYaml = `name: content-slot-snap
+version: 1.0
+slots:
+ content-plug:
+  interface: content
+  source:
+   read: [$SNAP/shared]
+   write: ["$SNAP_DATA/foo}bar"]
+`
+       info := snaptest.MockInfo(c, mockSnapYaml, nil)
+       slot := info.Slots["content-plug"]
+       c.Assert(interfaces.BeforePrepareSlot(s.iface, slot), ErrorMatches,
+               `content interface path is invalid: "\$SNAP_DATA/foo}bar" contains a reserved apparmor char.*`)
+}
+
 func (s *ContentSuite) TestResolveSpecialVariable(c *C) {
        info := snaptest.MockInfo(c, "{name: name, version: 0}", &snap.SideInfo{Revision: snap.R(42)})
        c.Check(builtin.ResolveSpecialVariable("$SNAP/foo", info), Equals, filepath.Join(dirs.CoreSnapMountDir, "name/42/foo"))
@@ -310,143 +341,143 @@ slots:
 # In addition to the bind mount, add any AppArmor rules so that
 # snaps may directly access the slot implementation's files
 # read-only.
-/snap/producer/5/export/** mrkix,
+"/snap/producer/5/export/**" mrkix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
 
        updateNS := apparmorSpec.UpdateNS()
        profile0 := `  # Read-only content sharing consumer:content -> producer:content (r#0)
-  mount options=(bind) /snap/producer/5/export/ -> /snap/consumer/7/import{,-[0-9]*}/,
-  remount options=(bind, ro) /snap/consumer/7/import{,-[0-9]*}/,
-  mount options=(rprivate) -> /snap/consumer/7/import{,-[0-9]*}/,
-  umount /snap/consumer/7/import{,-[0-9]*}/,
+  mount options=(bind) "/snap/producer/5/export/" -> "/snap/consumer/7/import{,-[0-9]*}/",
+  remount options=(bind, ro) "/snap/consumer/7/import{,-[0-9]*}/",
+  mount options=(rprivate) -> "/snap/consumer/7/import{,-[0-9]*}/",
+  umount "/snap/consumer/7/import{,-[0-9]*}/",
   # Writable mimic /snap/producer/5
   # .. permissions for traversing the prefix that is assumed to exist
   # .. variant with mimic at /
   # Allow reading the mimic directory, it must exist in the first place.
-  / r,
+  "/" r,
   # Allow setting the read-only directory aside via a bind mount.
-  /tmp/.snap/ rw,
-  mount options=(rbind, rw) / -> /tmp/.snap/,
+  "/tmp/.snap/" rw,
+  mount options=(rbind, rw) "/" -> "/tmp/.snap/",
   # Allow mounting tmpfs over the read-only directory.
-  mount fstype=tmpfs options=(rw) tmpfs -> /,
+  mount fstype=tmpfs options=(rw) tmpfs -> "/",
   # Allow creating empty files and directories for bind mounting things
   # to reconstruct the now-writable parent directory.
-  /tmp/.snap/*/ rw,
-  /*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/*/ -> /*/,
-  /tmp/.snap/* rw,
-  /* rw,
-  mount options=(bind, rw) /tmp/.snap/* -> /*,
+  "/tmp/.snap/*/" rw,
+  "/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/*/" -> "/*/",
+  "/tmp/.snap/*" rw,
+  "/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/*" -> "/*",
   # Allow unmounting the auxiliary directory.
   # TODO: use fstype=tmpfs here for more strictness (LP: #1613403)
-  mount options=(rprivate) -> /tmp/.snap/,
-  umount /tmp/.snap/,
+  mount options=(rprivate) -> "/tmp/.snap/",
+  umount "/tmp/.snap/",
   # Allow unmounting the destination directory as well as anything
   # inside.  This lets us perform the undo plan in case the writable
   # mimic fails.
-  mount options=(rprivate) -> /,
-  mount options=(rprivate) -> /*,
-  mount options=(rprivate) -> /*/,
-  umount /,
-  umount /*,
-  umount /*/,
+  mount options=(rprivate) -> "/",
+  mount options=(rprivate) -> "/*",
+  mount options=(rprivate) -> "/*/",
+  umount "/",
+  umount "/*",
+  umount "/*/",
   # .. variant with mimic at /snap/
-  /snap/ r,
-  /tmp/.snap/snap/ rw,
-  mount options=(rbind, rw) /snap/ -> /tmp/.snap/snap/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/,
-  /tmp/.snap/snap/*/ rw,
-  /snap/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/*/ -> /snap/*/,
-  /tmp/.snap/snap/* rw,
-  /snap/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/* -> /snap/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/,
-  umount /tmp/.snap/snap/,
-  mount options=(rprivate) -> /snap/,
-  mount options=(rprivate) -> /snap/*,
-  mount options=(rprivate) -> /snap/*/,
-  umount /snap/,
-  umount /snap/*,
-  umount /snap/*/,
+  "/snap/" r,
+  "/tmp/.snap/snap/" rw,
+  mount options=(rbind, rw) "/snap/" -> "/tmp/.snap/snap/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/",
+  "/tmp/.snap/snap/*/" rw,
+  "/snap/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/*/" -> "/snap/*/",
+  "/tmp/.snap/snap/*" rw,
+  "/snap/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/*" -> "/snap/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/",
+  umount "/tmp/.snap/snap/",
+  mount options=(rprivate) -> "/snap/",
+  mount options=(rprivate) -> "/snap/*",
+  mount options=(rprivate) -> "/snap/*/",
+  umount "/snap/",
+  umount "/snap/*",
+  umount "/snap/*/",
   # .. variant with mimic at /snap/producer/
-  /snap/producer/ r,
-  /tmp/.snap/snap/producer/ rw,
-  mount options=(rbind, rw) /snap/producer/ -> /tmp/.snap/snap/producer/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/producer/,
-  /tmp/.snap/snap/producer/*/ rw,
-  /snap/producer/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/producer/*/ -> /snap/producer/*/,
-  /tmp/.snap/snap/producer/* rw,
-  /snap/producer/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/producer/* -> /snap/producer/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/producer/,
-  umount /tmp/.snap/snap/producer/,
-  mount options=(rprivate) -> /snap/producer/,
-  mount options=(rprivate) -> /snap/producer/*,
-  mount options=(rprivate) -> /snap/producer/*/,
-  umount /snap/producer/,
-  umount /snap/producer/*,
-  umount /snap/producer/*/,
+  "/snap/producer/" r,
+  "/tmp/.snap/snap/producer/" rw,
+  mount options=(rbind, rw) "/snap/producer/" -> "/tmp/.snap/snap/producer/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/producer/",
+  "/tmp/.snap/snap/producer/*/" rw,
+  "/snap/producer/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/producer/*/" -> "/snap/producer/*/",
+  "/tmp/.snap/snap/producer/*" rw,
+  "/snap/producer/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/producer/*" -> "/snap/producer/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/producer/",
+  umount "/tmp/.snap/snap/producer/",
+  mount options=(rprivate) -> "/snap/producer/",
+  mount options=(rprivate) -> "/snap/producer/*",
+  mount options=(rprivate) -> "/snap/producer/*/",
+  umount "/snap/producer/",
+  umount "/snap/producer/*",
+  umount "/snap/producer/*/",
   # .. variant with mimic at /snap/producer/5/
-  /snap/producer/5/ r,
-  /tmp/.snap/snap/producer/5/ rw,
-  mount options=(rbind, rw) /snap/producer/5/ -> /tmp/.snap/snap/producer/5/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/producer/5/,
-  /tmp/.snap/snap/producer/5/*/ rw,
-  /snap/producer/5/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/producer/5/*/ -> /snap/producer/5/*/,
-  /tmp/.snap/snap/producer/5/* rw,
-  /snap/producer/5/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/producer/5/* -> /snap/producer/5/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/producer/5/,
-  umount /tmp/.snap/snap/producer/5/,
-  mount options=(rprivate) -> /snap/producer/5/,
-  mount options=(rprivate) -> /snap/producer/5/*,
-  mount options=(rprivate) -> /snap/producer/5/*/,
-  umount /snap/producer/5/,
-  umount /snap/producer/5/*,
-  umount /snap/producer/5/*/,
+  "/snap/producer/5/" r,
+  "/tmp/.snap/snap/producer/5/" rw,
+  mount options=(rbind, rw) "/snap/producer/5/" -> "/tmp/.snap/snap/producer/5/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/producer/5/",
+  "/tmp/.snap/snap/producer/5/*/" rw,
+  "/snap/producer/5/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/producer/5/*/" -> "/snap/producer/5/*/",
+  "/tmp/.snap/snap/producer/5/*" rw,
+  "/snap/producer/5/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/producer/5/*" -> "/snap/producer/5/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/producer/5/",
+  umount "/tmp/.snap/snap/producer/5/",
+  mount options=(rprivate) -> "/snap/producer/5/",
+  mount options=(rprivate) -> "/snap/producer/5/*",
+  mount options=(rprivate) -> "/snap/producer/5/*/",
+  umount "/snap/producer/5/",
+  umount "/snap/producer/5/*",
+  umount "/snap/producer/5/*/",
   # Writable mimic /snap/consumer/7
   # .. variant with mimic at /snap/consumer/
-  /snap/consumer/ r,
-  /tmp/.snap/snap/consumer/ rw,
-  mount options=(rbind, rw) /snap/consumer/ -> /tmp/.snap/snap/consumer/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/consumer/,
-  /tmp/.snap/snap/consumer/*/ rw,
-  /snap/consumer/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/consumer/*/ -> /snap/consumer/*/,
-  /tmp/.snap/snap/consumer/* rw,
-  /snap/consumer/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/consumer/* -> /snap/consumer/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/consumer/,
-  umount /tmp/.snap/snap/consumer/,
-  mount options=(rprivate) -> /snap/consumer/,
-  mount options=(rprivate) -> /snap/consumer/*,
-  mount options=(rprivate) -> /snap/consumer/*/,
-  umount /snap/consumer/,
-  umount /snap/consumer/*,
-  umount /snap/consumer/*/,
+  "/snap/consumer/" r,
+  "/tmp/.snap/snap/consumer/" rw,
+  mount options=(rbind, rw) "/snap/consumer/" -> "/tmp/.snap/snap/consumer/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/consumer/",
+  "/tmp/.snap/snap/consumer/*/" rw,
+  "/snap/consumer/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/consumer/*/" -> "/snap/consumer/*/",
+  "/tmp/.snap/snap/consumer/*" rw,
+  "/snap/consumer/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/consumer/*" -> "/snap/consumer/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/consumer/",
+  umount "/tmp/.snap/snap/consumer/",
+  mount options=(rprivate) -> "/snap/consumer/",
+  mount options=(rprivate) -> "/snap/consumer/*",
+  mount options=(rprivate) -> "/snap/consumer/*/",
+  umount "/snap/consumer/",
+  umount "/snap/consumer/*",
+  umount "/snap/consumer/*/",
   # .. variant with mimic at /snap/consumer/7/
-  /snap/consumer/7/ r,
-  /tmp/.snap/snap/consumer/7/ rw,
-  mount options=(rbind, rw) /snap/consumer/7/ -> /tmp/.snap/snap/consumer/7/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/consumer/7/,
-  /tmp/.snap/snap/consumer/7/*/ rw,
-  /snap/consumer/7/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/consumer/7/*/ -> /snap/consumer/7/*/,
-  /tmp/.snap/snap/consumer/7/* rw,
-  /snap/consumer/7/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/consumer/7/* -> /snap/consumer/7/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/consumer/7/,
-  umount /tmp/.snap/snap/consumer/7/,
-  mount options=(rprivate) -> /snap/consumer/7/,
-  mount options=(rprivate) -> /snap/consumer/7/*,
-  mount options=(rprivate) -> /snap/consumer/7/*/,
-  umount /snap/consumer/7/,
-  umount /snap/consumer/7/*,
-  umount /snap/consumer/7/*/,
+  "/snap/consumer/7/" r,
+  "/tmp/.snap/snap/consumer/7/" rw,
+  mount options=(rbind, rw) "/snap/consumer/7/" -> "/tmp/.snap/snap/consumer/7/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/consumer/7/",
+  "/tmp/.snap/snap/consumer/7/*/" rw,
+  "/snap/consumer/7/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/consumer/7/*/" -> "/snap/consumer/7/*/",
+  "/tmp/.snap/snap/consumer/7/*" rw,
+  "/snap/consumer/7/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/consumer/7/*" -> "/snap/consumer/7/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/consumer/7/",
+  umount "/tmp/.snap/snap/consumer/7/",
+  mount options=(rprivate) -> "/snap/consumer/7/",
+  mount options=(rprivate) -> "/snap/consumer/7/*",
+  mount options=(rprivate) -> "/snap/consumer/7/*/",
+  umount "/snap/consumer/7/",
+  umount "/snap/consumer/7/*",
+  umount "/snap/consumer/7/*/",
 `
        c.Assert(strings.Join(updateNS[:], ""), Equals, profile0)
 }
@@ -493,25 +524,25 @@ slots:
 # to a limitation in the kernel's LSM hooks for AF_UNIX, these
 # are needed for using named sockets within the exported
 # directory.
-/var/snap/producer/5/export/** mrwklix,
+"/var/snap/producer/5/export/**" mrwklix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
 
        updateNS := apparmorSpec.UpdateNS()
        profile0 := `  # Read-write content sharing consumer:content -> producer:content (w#0)
-  mount options=(bind, rw) /var/snap/producer/5/export/ -> /var/snap/consumer/7/import{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/7/import{,-[0-9]*}/,
-  umount /var/snap/consumer/7/import{,-[0-9]*}/,
+  mount options=(bind, rw) "/var/snap/producer/5/export/" -> "/var/snap/consumer/7/import{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/7/import{,-[0-9]*}/",
+  umount "/var/snap/consumer/7/import{,-[0-9]*}/",
   # Writable directory /var/snap/producer/5/export
-  /var/snap/producer/5/export/ rw,
-  /var/snap/producer/5/ rw,
-  /var/snap/producer/ rw,
+  "/var/snap/producer/5/export/" rw,
+  "/var/snap/producer/5/" rw,
+  "/var/snap/producer/" rw,
   # Writable directory /var/snap/consumer/7/import
-  /var/snap/consumer/7/import/ rw,
-  /var/snap/consumer/7/ rw,
-  /var/snap/consumer/ rw,
+  "/var/snap/consumer/7/import/" rw,
+  "/var/snap/consumer/7/" rw,
+  "/var/snap/consumer/" rw,
   # Writable directory /var/snap/consumer/7/import-[0-9]*
-  /var/snap/consumer/7/import-[0-9]*/ rw,
+  "/var/snap/consumer/7/import-[0-9]*/" rw,
 `
        c.Assert(strings.Join(updateNS[:], ""), Equals, profile0)
 }
@@ -558,25 +589,25 @@ slots:
 # to a limitation in the kernel's LSM hooks for AF_UNIX, these
 # are needed for using named sockets within the exported
 # directory.
-/var/snap/producer/common/export/** mrwklix,
+"/var/snap/producer/common/export/**" mrwklix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
 
        updateNS := apparmorSpec.UpdateNS()
        profile0 := `  # Read-write content sharing consumer:content -> producer:content (w#0)
-  mount options=(bind, rw) /var/snap/producer/common/export/ -> /var/snap/consumer/common/import{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import{,-[0-9]*}/,
+  mount options=(bind, rw) "/var/snap/producer/common/export/" -> "/var/snap/consumer/common/import{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import{,-[0-9]*}/",
   # Writable directory /var/snap/producer/common/export
-  /var/snap/producer/common/export/ rw,
-  /var/snap/producer/common/ rw,
-  /var/snap/producer/ rw,
+  "/var/snap/producer/common/export/" rw,
+  "/var/snap/producer/common/" rw,
+  "/var/snap/producer/" rw,
   # Writable directory /var/snap/consumer/common/import
-  /var/snap/consumer/common/import/ rw,
-  /var/snap/consumer/common/ rw,
-  /var/snap/consumer/ rw,
+  "/var/snap/consumer/common/import/" rw,
+  "/var/snap/consumer/common/" rw,
+  "/var/snap/consumer/" rw,
   # Writable directory /var/snap/consumer/common/import-[0-9]*
-  /var/snap/consumer/common/import-[0-9]*/ rw,
+  "/var/snap/consumer/common/import-[0-9]*/" rw,
 `
        c.Assert(strings.Join(updateNS[:], ""), Equals, profile0)
 }
@@ -650,34 +681,34 @@ slots:
 # to a limitation in the kernel's LSM hooks for AF_UNIX, these
 # are needed for using named sockets within the exported
 # directory.
-/var/snap/producer/common/write-common/** mrwklix,
-/var/snap/producer/2/write-data/** mrwklix,
+"/var/snap/producer/common/write-common/**" mrwklix,
+"/var/snap/producer/2/write-data/**" mrwklix,
 
 # In addition to the bind mount, add any AppArmor rules so that
 # snaps may directly access the slot implementation's files
 # read-only.
-/var/snap/producer/common/read-common/** mrkix,
-/var/snap/producer/2/read-data/** mrkix,
-/snap/producer/2/read-snap/** mrkix,
+"/var/snap/producer/common/read-common/**" mrkix,
+"/var/snap/producer/2/read-data/**" mrkix,
+"/snap/producer/2/read-snap/**" mrkix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
 
        updateNS := apparmorSpec.UpdateNS()
        profile0 := `  # Read-write content sharing consumer:content -> producer:content (w#0)
-  mount options=(bind, rw) /var/snap/producer/common/write-common/ -> /var/snap/consumer/common/import/write-common{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import/write-common{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import/write-common{,-[0-9]*}/,
+  mount options=(bind, rw) "/var/snap/producer/common/write-common/" -> "/var/snap/consumer/common/import/write-common{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import/write-common{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import/write-common{,-[0-9]*}/",
   # Writable directory /var/snap/producer/common/write-common
-  /var/snap/producer/common/write-common/ rw,
-  /var/snap/producer/common/ rw,
-  /var/snap/producer/ rw,
+  "/var/snap/producer/common/write-common/" rw,
+  "/var/snap/producer/common/" rw,
+  "/var/snap/producer/" rw,
   # Writable directory /var/snap/consumer/common/import/write-common
-  /var/snap/consumer/common/import/write-common/ rw,
-  /var/snap/consumer/common/import/ rw,
-  /var/snap/consumer/common/ rw,
-  /var/snap/consumer/ rw,
+  "/var/snap/consumer/common/import/write-common/" rw,
+  "/var/snap/consumer/common/import/" rw,
+  "/var/snap/consumer/common/" rw,
+  "/var/snap/consumer/" rw,
   # Writable directory /var/snap/consumer/common/import/write-common-[0-9]*
-  /var/snap/consumer/common/import/write-common-[0-9]*/ rw,
+  "/var/snap/consumer/common/import/write-common-[0-9]*/" rw,
 `
        // Find the slice that describes profile0 by looking for the first unique
        // line of the next profile.
@@ -686,16 +717,16 @@ slots:
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile0)
 
        profile1 := `  # Read-write content sharing consumer:content -> producer:content (w#1)
-  mount options=(bind, rw) /var/snap/producer/2/write-data/ -> /var/snap/consumer/common/import/write-data{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import/write-data{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import/write-data{,-[0-9]*}/,
+  mount options=(bind, rw) "/var/snap/producer/2/write-data/" -> "/var/snap/consumer/common/import/write-data{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import/write-data{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import/write-data{,-[0-9]*}/",
   # Writable directory /var/snap/producer/2/write-data
-  /var/snap/producer/2/write-data/ rw,
-  /var/snap/producer/2/ rw,
+  "/var/snap/producer/2/write-data/" rw,
+  "/var/snap/producer/2/" rw,
   # Writable directory /var/snap/consumer/common/import/write-data
-  /var/snap/consumer/common/import/write-data/ rw,
+  "/var/snap/consumer/common/import/write-data/" rw,
   # Writable directory /var/snap/consumer/common/import/write-data-[0-9]*
-  /var/snap/consumer/common/import/write-data-[0-9]*/ rw,
+  "/var/snap/consumer/common/import/write-data-[0-9]*/" rw,
 `
        // Find the slice that describes profile1 by looking for the first unique
        // line of the next profile.
@@ -704,16 +735,16 @@ slots:
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile1)
 
        profile2 := `  # Read-only content sharing consumer:content -> producer:content (r#0)
-  mount options=(bind) /var/snap/producer/common/read-common/ -> /var/snap/consumer/common/import/read-common{,-[0-9]*}/,
-  remount options=(bind, ro) /var/snap/consumer/common/import/read-common{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import/read-common{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import/read-common{,-[0-9]*}/,
+  mount options=(bind) "/var/snap/producer/common/read-common/" -> "/var/snap/consumer/common/import/read-common{,-[0-9]*}/",
+  remount options=(bind, ro) "/var/snap/consumer/common/import/read-common{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import/read-common{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import/read-common{,-[0-9]*}/",
   # Writable directory /var/snap/producer/common/read-common
-  /var/snap/producer/common/read-common/ rw,
+  "/var/snap/producer/common/read-common/" rw,
   # Writable directory /var/snap/consumer/common/import/read-common
-  /var/snap/consumer/common/import/read-common/ rw,
+  "/var/snap/consumer/common/import/read-common/" rw,
   # Writable directory /var/snap/consumer/common/import/read-common-[0-9]*
-  /var/snap/consumer/common/import/read-common-[0-9]*/ rw,
+  "/var/snap/consumer/common/import/read-common-[0-9]*/" rw,
 `
        // Find the slice that describes profile2 by looking for the first unique
        // line of the next profile.
@@ -722,16 +753,16 @@ slots:
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile2)
 
        profile3 := `  # Read-only content sharing consumer:content -> producer:content (r#1)
-  mount options=(bind) /var/snap/producer/2/read-data/ -> /var/snap/consumer/common/import/read-data{,-[0-9]*}/,
-  remount options=(bind, ro) /var/snap/consumer/common/import/read-data{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import/read-data{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import/read-data{,-[0-9]*}/,
+  mount options=(bind) "/var/snap/producer/2/read-data/" -> "/var/snap/consumer/common/import/read-data{,-[0-9]*}/",
+  remount options=(bind, ro) "/var/snap/consumer/common/import/read-data{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import/read-data{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import/read-data{,-[0-9]*}/",
   # Writable directory /var/snap/producer/2/read-data
-  /var/snap/producer/2/read-data/ rw,
+  "/var/snap/producer/2/read-data/" rw,
   # Writable directory /var/snap/consumer/common/import/read-data
-  /var/snap/consumer/common/import/read-data/ rw,
+  "/var/snap/consumer/common/import/read-data/" rw,
   # Writable directory /var/snap/consumer/common/import/read-data-[0-9]*
-  /var/snap/consumer/common/import/read-data-[0-9]*/ rw,
+  "/var/snap/consumer/common/import/read-data-[0-9]*/" rw,
 `
        // Find the slice that describes profile3 by looking for the first unique
        // line of the next profile.
@@ -740,102 +771,102 @@ slots:
        c.Assert(strings.Join(updateNS[start:end], ""), Equals, profile3)
 
        profile4 := `  # Read-only content sharing consumer:content -> producer:content (r#2)
-  mount options=(bind) /snap/producer/2/read-snap/ -> /var/snap/consumer/common/import/read-snap{,-[0-9]*}/,
-  remount options=(bind, ro) /var/snap/consumer/common/import/read-snap{,-[0-9]*}/,
-  mount options=(rprivate) -> /var/snap/consumer/common/import/read-snap{,-[0-9]*}/,
-  umount /var/snap/consumer/common/import/read-snap{,-[0-9]*}/,
+  mount options=(bind) "/snap/producer/2/read-snap/" -> "/var/snap/consumer/common/import/read-snap{,-[0-9]*}/",
+  remount options=(bind, ro) "/var/snap/consumer/common/import/read-snap{,-[0-9]*}/",
+  mount options=(rprivate) -> "/var/snap/consumer/common/import/read-snap{,-[0-9]*}/",
+  umount "/var/snap/consumer/common/import/read-snap{,-[0-9]*}/",
   # Writable mimic /snap/producer/2
   # .. permissions for traversing the prefix that is assumed to exist
   # .. variant with mimic at /
   # Allow reading the mimic directory, it must exist in the first place.
-  / r,
+  "/" r,
   # Allow setting the read-only directory aside via a bind mount.
-  /tmp/.snap/ rw,
-  mount options=(rbind, rw) / -> /tmp/.snap/,
+  "/tmp/.snap/" rw,
+  mount options=(rbind, rw) "/" -> "/tmp/.snap/",
   # Allow mounting tmpfs over the read-only directory.
-  mount fstype=tmpfs options=(rw) tmpfs -> /,
+  mount fstype=tmpfs options=(rw) tmpfs -> "/",
   # Allow creating empty files and directories for bind mounting things
   # to reconstruct the now-writable parent directory.
-  /tmp/.snap/*/ rw,
-  /*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/*/ -> /*/,
-  /tmp/.snap/* rw,
-  /* rw,
-  mount options=(bind, rw) /tmp/.snap/* -> /*,
+  "/tmp/.snap/*/" rw,
+  "/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/*/" -> "/*/",
+  "/tmp/.snap/*" rw,
+  "/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/*" -> "/*",
   # Allow unmounting the auxiliary directory.
   # TODO: use fstype=tmpfs here for more strictness (LP: #1613403)
-  mount options=(rprivate) -> /tmp/.snap/,
-  umount /tmp/.snap/,
+  mount options=(rprivate) -> "/tmp/.snap/",
+  umount "/tmp/.snap/",
   # Allow unmounting the destination directory as well as anything
   # inside.  This lets us perform the undo plan in case the writable
   # mimic fails.
-  mount options=(rprivate) -> /,
-  mount options=(rprivate) -> /*,
-  mount options=(rprivate) -> /*/,
-  umount /,
-  umount /*,
-  umount /*/,
+  mount options=(rprivate) -> "/",
+  mount options=(rprivate) -> "/*",
+  mount options=(rprivate) -> "/*/",
+  umount "/",
+  umount "/*",
+  umount "/*/",
   # .. variant with mimic at /snap/
-  /snap/ r,
-  /tmp/.snap/snap/ rw,
-  mount options=(rbind, rw) /snap/ -> /tmp/.snap/snap/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/,
-  /tmp/.snap/snap/*/ rw,
-  /snap/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/*/ -> /snap/*/,
-  /tmp/.snap/snap/* rw,
-  /snap/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/* -> /snap/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/,
-  umount /tmp/.snap/snap/,
-  mount options=(rprivate) -> /snap/,
-  mount options=(rprivate) -> /snap/*,
-  mount options=(rprivate) -> /snap/*/,
-  umount /snap/,
-  umount /snap/*,
-  umount /snap/*/,
+  "/snap/" r,
+  "/tmp/.snap/snap/" rw,
+  mount options=(rbind, rw) "/snap/" -> "/tmp/.snap/snap/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/",
+  "/tmp/.snap/snap/*/" rw,
+  "/snap/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/*/" -> "/snap/*/",
+  "/tmp/.snap/snap/*" rw,
+  "/snap/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/*" -> "/snap/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/",
+  umount "/tmp/.snap/snap/",
+  mount options=(rprivate) -> "/snap/",
+  mount options=(rprivate) -> "/snap/*",
+  mount options=(rprivate) -> "/snap/*/",
+  umount "/snap/",
+  umount "/snap/*",
+  umount "/snap/*/",
   # .. variant with mimic at /snap/producer/
-  /snap/producer/ r,
-  /tmp/.snap/snap/producer/ rw,
-  mount options=(rbind, rw) /snap/producer/ -> /tmp/.snap/snap/producer/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/producer/,
-  /tmp/.snap/snap/producer/*/ rw,
-  /snap/producer/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/producer/*/ -> /snap/producer/*/,
-  /tmp/.snap/snap/producer/* rw,
-  /snap/producer/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/producer/* -> /snap/producer/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/producer/,
-  umount /tmp/.snap/snap/producer/,
-  mount options=(rprivate) -> /snap/producer/,
-  mount options=(rprivate) -> /snap/producer/*,
-  mount options=(rprivate) -> /snap/producer/*/,
-  umount /snap/producer/,
-  umount /snap/producer/*,
-  umount /snap/producer/*/,
+  "/snap/producer/" r,
+  "/tmp/.snap/snap/producer/" rw,
+  mount options=(rbind, rw) "/snap/producer/" -> "/tmp/.snap/snap/producer/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/producer/",
+  "/tmp/.snap/snap/producer/*/" rw,
+  "/snap/producer/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/producer/*/" -> "/snap/producer/*/",
+  "/tmp/.snap/snap/producer/*" rw,
+  "/snap/producer/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/producer/*" -> "/snap/producer/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/producer/",
+  umount "/tmp/.snap/snap/producer/",
+  mount options=(rprivate) -> "/snap/producer/",
+  mount options=(rprivate) -> "/snap/producer/*",
+  mount options=(rprivate) -> "/snap/producer/*/",
+  umount "/snap/producer/",
+  umount "/snap/producer/*",
+  umount "/snap/producer/*/",
   # .. variant with mimic at /snap/producer/2/
-  /snap/producer/2/ r,
-  /tmp/.snap/snap/producer/2/ rw,
-  mount options=(rbind, rw) /snap/producer/2/ -> /tmp/.snap/snap/producer/2/,
-  mount fstype=tmpfs options=(rw) tmpfs -> /snap/producer/2/,
-  /tmp/.snap/snap/producer/2/*/ rw,
-  /snap/producer/2/*/ rw,
-  mount options=(rbind, rw) /tmp/.snap/snap/producer/2/*/ -> /snap/producer/2/*/,
-  /tmp/.snap/snap/producer/2/* rw,
-  /snap/producer/2/* rw,
-  mount options=(bind, rw) /tmp/.snap/snap/producer/2/* -> /snap/producer/2/*,
-  mount options=(rprivate) -> /tmp/.snap/snap/producer/2/,
-  umount /tmp/.snap/snap/producer/2/,
-  mount options=(rprivate) -> /snap/producer/2/,
-  mount options=(rprivate) -> /snap/producer/2/*,
-  mount options=(rprivate) -> /snap/producer/2/*/,
-  umount /snap/producer/2/,
-  umount /snap/producer/2/*,
-  umount /snap/producer/2/*/,
+  "/snap/producer/2/" r,
+  "/tmp/.snap/snap/producer/2/" rw,
+  mount options=(rbind, rw) "/snap/producer/2/" -> "/tmp/.snap/snap/producer/2/",
+  mount fstype=tmpfs options=(rw) tmpfs -> "/snap/producer/2/",
+  "/tmp/.snap/snap/producer/2/*/" rw,
+  "/snap/producer/2/*/" rw,
+  mount options=(rbind, rw) "/tmp/.snap/snap/producer/2/*/" -> "/snap/producer/2/*/",
+  "/tmp/.snap/snap/producer/2/*" rw,
+  "/snap/producer/2/*" rw,
+  mount options=(bind, rw) "/tmp/.snap/snap/producer/2/*" -> "/snap/producer/2/*",
+  mount options=(rprivate) -> "/tmp/.snap/snap/producer/2/",
+  umount "/tmp/.snap/snap/producer/2/",
+  mount options=(rprivate) -> "/snap/producer/2/",
+  mount options=(rprivate) -> "/snap/producer/2/*",
+  mount options=(rprivate) -> "/snap/producer/2/*/",
+  umount "/snap/producer/2/",
+  umount "/snap/producer/2/*",
+  umount "/snap/producer/2/*/",
   # Writable directory /var/snap/consumer/common/import/read-snap
-  /var/snap/consumer/common/import/read-snap/ rw,
+  "/var/snap/consumer/common/import/read-snap/" rw,
   # Writable directory /var/snap/consumer/common/import/read-snap-[0-9]*
-  /var/snap/consumer/common/import/read-snap-[0-9]*/ rw,
+  "/var/snap/consumer/common/import/read-snap-[0-9]*/" rw,
 `
        // Find the slice that describes profile4 by looking till the end of the list.
        start = end
@@ -911,13 +942,13 @@ slots:
 # In addition to the bind mount, add any AppArmor rules so that
 # snaps may directly access the slot implementation's files
 # read-only.
-/snap/plugin-one/1/plugin/** mrkix,
+"/snap/plugin-one/1/plugin/**" mrkix,
 
 
 # In addition to the bind mount, add any AppArmor rules so that
 # snaps may directly access the slot implementation's files
 # read-only.
-/snap/plugin-two/1/plugin/** mrkix,
+"/snap/plugin-two/1/plugin/**" mrkix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.app.app"), Equals, expected)
 }
@@ -975,12 +1006,12 @@ slots:
 # to a limitation in the kernel's LSM hooks for AF_UNIX, these
 # are needed for using named sockets within the exported
 # directory.
-/var/snap/producer/2/directory/** mrwklix,
+"/var/snap/producer/2/directory/**" mrwklix,
 
 # In addition to the bind mount, add any AppArmor rules so that
 # snaps may directly access the slot implementation's files
 # read-only.
-/var/snap/producer/2/directory/** mrkix,
+"/var/snap/producer/2/directory/**" mrkix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
 }
@@ -1017,7 +1048,7 @@ apps:
 # implementation to access the slot's exported files at the plugging
 # snap's mountpoint to accommodate software where the plugging app
 # tells the slotting app about files to share.
-/var/snap/consumer/common/import/** mrwklix,
+"/var/snap/consumer/common/import/**" mrwklix,
 `
        c.Assert(apparmorSpec.SnippetForTag("snap.producer.app"), Equals, expected)
 }
index 4ac923a571ac0921ec256b98b8727a1e74fd3f5d..f380f4b2ac0eff615b576cca8ba3ec79b6fbbf5e 100644 (file)
@@ -26,6 +26,7 @@ import (
 
        "github.com/snapcore/snapd/interfaces"
        "github.com/snapcore/snapd/interfaces/apparmor"
+       apparmor_sandbox "github.com/snapcore/snapd/sandbox/apparmor"
 )
 
 const daemonNotifySummary = `allows sending daemon status changes to service manager`
@@ -64,7 +65,7 @@ func (iface *daemoNotifyInterface) AppArmorConnectedPlug(spec *apparmor.Specific
                // must be an absolute path or an abstract socket path
                return fmt.Errorf("cannot use %q as notify socket path: not absolute", notifySocket)
        }
-       if err := apparmor.ValidateNoAppArmorRegexp(notifySocket); err != nil {
+       if err := apparmor_sandbox.ValidateNoAppArmorRegexp(notifySocket); err != nil {
                return fmt.Errorf("cannot use %q as notify socket path: %s", notifySocket, err)
        }
 
index c60331bd7566ace1967182c3d2526c6db519cfcd..7d1836a7400120c9b7d86fd019e96239cd953281 100644 (file)
@@ -42,7 +42,15 @@ type BackendSuite struct {
        testutil.BaseTest
 }
 
+// CoreSnapInfo is set in SetupSuite
+var DefaultInitializeOpts = &interfaces.SecurityBackendOptions{}
+
 func (s *BackendSuite) SetUpTest(c *C) {
+       coreSnapPlaceInfo := snap.MinimalPlaceInfo("core", snap.Revision{N: 123})
+       snInfo, ok := coreSnapPlaceInfo.(*snap.Info)
+       c.Assert(ok, Equals, true)
+       DefaultInitializeOpts.CoreSnapInfo = snInfo
+
        // Isolate this test to a temporary directory
        s.RootDir = c.MkDir()
        dirs.SetRootDir(s.RootDir)
index 7fbc0bcce82bf8c0a8909b8d25aaa7195c540c03..a28b56b8cc4c035fb184a801dcc71932a6d55d8e 100644 (file)
@@ -175,7 +175,7 @@ fi`)
        c.Check(s.snapSeccomp.Calls(), HasLen, 0)
        // ensure the snap-seccomp from the core snap was used instead
        c.Check(snapSeccompOnCore.Calls(), DeepEquals, [][]string{
-               {"snap-seccomp", "version-info"},
+               {"snap-seccomp", "version-info"}, // from Initialize()
                {"snap-seccomp", "compile", profile + ".src", profile + ".bin"},
        })
        raw, err := ioutil.ReadFile(profile + ".src")
index 3c7cfda231983716cfb87c74a58ecee7f06910a7..6bdf6688eb953ef05eb11b0b1e83fa141a405f52 100644 (file)
@@ -24,6 +24,7 @@ import (
        "io/ioutil"
        "os"
        "path/filepath"
+       "runtime"
        "strconv"
        "strings"
        "time"
@@ -76,6 +77,11 @@ type firstBootBaseTest struct {
 func (t *firstBootBaseTest) setupBaseTest(c *C, s *seedtest.SeedSnaps) {
        t.BaseTest.SetUpTest(c)
 
+       // TODO: temporary: skip due to timeouts on riscv64
+       if runtime.GOARCH == "riscv64" || os.Getenv("SNAPD_SKIP_SLOW_TESTS") != "" {
+               c.Skip("skipping slow test")
+       }
+
        tempdir := c.MkDir()
        dirs.SetRootDir(tempdir)
        t.AddCleanup(func() { dirs.SetRootDir("/") })
index ddba39cc84fdc9afc18d08cb92b1fdd24500d867..77c4acc8db69fa07586397091fa057a4694e9c2f 100644 (file)
@@ -67,7 +67,39 @@ func (m *InterfaceManager) addInterfaces(extra []interfaces.Interface) error {
 }
 
 func (m *InterfaceManager) addBackends(extra []interfaces.SecurityBackend) error {
-       opts := interfaces.SecurityBackendOptions{Preseed: m.preseed}
+       // get the snapd snap info if it is installed
+       var snapdSnap snapstate.SnapState
+       var snapdSnapInfo *snap.Info
+       err := snapstate.Get(m.state, "snapd", &snapdSnap)
+       if err != nil && err != state.ErrNoState {
+               return fmt.Errorf("cannot access snapd snap state: %v", err)
+       }
+       if err == nil {
+               snapdSnapInfo, err = snapdSnap.CurrentInfo()
+               if err != nil && err != snapstate.ErrNoCurrent {
+                       return fmt.Errorf("cannot access snapd snap info: %v", err)
+               }
+       }
+
+       // get the core snap info if it is installed
+       var coreSnap snapstate.SnapState
+       var coreSnapInfo *snap.Info
+       err = snapstate.Get(m.state, "core", &coreSnap)
+       if err != nil && err != state.ErrNoState {
+               return fmt.Errorf("cannot access core snap state: %v", err)
+       }
+       if err == nil {
+               coreSnapInfo, err = coreSnap.CurrentInfo()
+               if err != nil && err != snapstate.ErrNoCurrent {
+                       return fmt.Errorf("cannot access core snap info: %v", err)
+               }
+       }
+
+       opts := interfaces.SecurityBackendOptions{
+               Preseed:       m.preseed,
+               CoreSnapInfo:  coreSnapInfo,
+               SnapdSnapInfo: snapdSnapInfo,
+       }
        for _, backend := range backends.All {
                if err := backend.Initialize(&opts); err != nil {
                        return err
index 94da9b2b89ffb18d68748d5788474c6738ddd19b..2ec269610aaa3dcd543ad2cb9727a9c8d0dc1f7f 100644 (file)
@@ -34,6 +34,7 @@ import (
        "net/url"
        "os"
        "path/filepath"
+       "runtime"
        "sort"
        "strings"
        "time"
@@ -140,6 +141,11 @@ func verifyLastTasksetIsRerefresh(c *C, tts []*state.TaskSet) {
 func (s *baseMgrsSuite) SetUpTest(c *C) {
        s.BaseTest.SetUpTest(c)
 
+       // TODO: temporary: skip due to timeouts on riscv64
+       if runtime.GOARCH == "riscv64" || os.Getenv("SNAPD_SKIP_SLOW_TESTS") != "" {
+               c.Skip("skipping slow tests")
+       }
+
        s.tempdir = c.MkDir()
        dirs.SetRootDir(s.tempdir)
        s.AddCleanup(func() { dirs.SetRootDir("") })
index b628478b27b1a4eb343f3461cfe431cf35c015b0..fc40ea66ff8e548127a6b3ec21385d3c37cba297 100644 (file)
@@ -74,6 +74,9 @@ backends:
         - adt-local:
             username: ubuntu
             password: ubuntu
+prepare: |
+    # Copy external tools from the subtree to the "$TESTSLIB"/tools directory
+    cp -f "$TESTSLIB"/external/snapd-testing-tools/tools/* "$TESTSTOOLS"
 suites:
         tests/smoke/:
             summary: Essenial system level tests for snapd
index 4d8c6129f690f81113c02ac1267c64bd9b374f56..95cf8657996f7a23bef6ef5f599ecb9bef10ad58 100644 (file)
@@ -35,6 +35,19 @@ import (
        "github.com/snapcore/snapd/strutil"
 )
 
+// ValidateNoAppArmorRegexp will check that the given string does not
+// contain AppArmor regular expressions (AARE), double quotes or \0.
+// Note that to check the inverse of this, that is that a string has
+// valid AARE, one should use interfaces/utils.NewPathPattern().
+func ValidateNoAppArmorRegexp(s string) error {
+       const AARE = `?*[]{}^"` + "\x00"
+
+       if strings.ContainsAny(s, AARE) {
+               return fmt.Errorf("%q contains a reserved apparmor char from %s", s, AARE)
+       }
+       return nil
+}
+
 // LevelType encodes the kind of support for apparmor
 // found on this system.
 type LevelType int
index b726c2b549a87de509b53768ff84668d43653b0a..ae6855197fdcd433bf1f800a8187162eab957926 100644 (file)
@@ -303,3 +303,19 @@ func (s *apparmorSuite) TestFeaturesProbedOnce(c *C) {
        _, err = apparmor.ParserFeatures()
        c.Assert(err, IsNil)
 }
+
+func (s *apparmorSuite) TestValidateFreeFromAAREUnhappy(c *C) {
+       var testCases = []string{"a?", "*b", "c[c", "dd]", "e{", "f}", "g^", `h"`, "f\000", "g\x00"}
+
+       for _, s := range testCases {
+               c.Check(apparmor.ValidateNoAppArmorRegexp(s), ErrorMatches, ".* contains a reserved apparmor char from .*", Commentf("%q is not raising an error", s))
+       }
+}
+
+func (s *apparmorSuite) TestValidateFreeFromAAREhappy(c *C) {
+       var testCases = []string{"foo", "BaR", "b-z", "foo+bar", "b00m!", "be/ep", "a%b", "a&b", "a(b", "a)b", "a=b", "a#b", "a~b", "a'b", "a_b", "a,b", "a;b", "a>b", "a<b", "a|b"}
+
+       for _, s := range testCases {
+               c.Check(apparmor.ValidateNoAppArmorRegexp(s), IsNil, Commentf("%q raised an error but shouldn't", s))
+       }
+}
index 4af57a35a7253f92187275e96d64590bfc67bc01..cc93de3fe3b844cebc3c887e3f38e0e6f057e0af 100644 (file)
@@ -31,6 +31,7 @@ import (
        "unicode/utf8"
 
        "github.com/snapcore/snapd/osutil"
+       "github.com/snapcore/snapd/sandbox/apparmor"
        "github.com/snapcore/snapd/snap/naming"
        "github.com/snapcore/snapd/spdx"
        "github.com/snapcore/snapd/strutil"
@@ -422,6 +423,9 @@ func ValidateLayoutAll(info *Info) error {
        // Validate that each source path is not a new top-level directory
        for _, layout := range info.Layout {
                cleanPathSrc := info.ExpandSnapVariables(filepath.Clean(layout.Path))
+               if err := apparmor.ValidateNoAppArmorRegexp(layout.Path); err != nil {
+                       return fmt.Errorf("invalid layout path: %v", err)
+               }
                elems := strings.SplitN(cleanPathSrc, string(os.PathSeparator), 3)
                switch len(elems) {
                // len(1) is either relative path or empty string, will be validated
@@ -1005,6 +1009,10 @@ func ValidateLayout(layout *Layout, constraints []LayoutConstraint) error {
                        !strings.HasPrefix(mountSource, si.ExpandSnapVariables("$SNAP_COMMON")) {
                        return fmt.Errorf("layout %q uses invalid bind mount source %q: must start with $SNAP, $SNAP_DATA or $SNAP_COMMON", layout.Path, mountSource)
                }
+               // Ensure that the path does not express an AppArmor pattern
+               if err := apparmor.ValidateNoAppArmorRegexp(mountSource); err != nil {
+                       return fmt.Errorf("layout %q uses invalid mount source: %s", layout.Path, err)
+               }
        }
 
        switch layout.Type {
@@ -1032,6 +1040,10 @@ func ValidateLayout(layout *Layout, constraints []LayoutConstraint) error {
                        !strings.HasPrefix(oldname, si.ExpandSnapVariables("$SNAP_COMMON")) {
                        return fmt.Errorf("layout %q uses invalid symlink old name %q: must start with $SNAP, $SNAP_DATA or $SNAP_COMMON", layout.Path, oldname)
                }
+               // Ensure that the path does not express an AppArmor pattern
+               if err := apparmor.ValidateNoAppArmorRegexp(oldname); err != nil {
+                       return fmt.Errorf("layout %q uses invalid symlink: %s", layout.Path, err)
+               }
        }
 
        // When new users and groups are supported those must be added to interfaces/mount/spec.go as well.
index 46f387f039795fc10e0376318ad764b330152cda..85ae1ccc0380e8b64852d6316453fb1044d9ef2d 100644 (file)
@@ -963,6 +963,21 @@ func (s *ValidateSuite) TestValidateLayout(c *C) {
        c.Check(ValidateLayout(&Layout{Snap: si, Path: "/tmp", Type: "tmpfs"}, nil),
                ErrorMatches, `layout "/tmp" in an off-limits area`)
 
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", Bind: "$SNAP/dev/sda[0123]"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid mount source: "/snap/foo/unset/dev/sda\[0123\]" contains a reserved apparmor char.*`)
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", Bind: "$SNAP/*"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid mount source: "/snap/foo/unset/\*" contains a reserved apparmor char.*`)
+
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", BindFile: "$SNAP/a\"quote"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid mount source: "/snap/foo/unset/a\\"quote" contains a reserved apparmor char.*`)
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", BindFile: "$SNAP/^invalid"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid mount source: "/snap/foo/unset/\^invalid" contains a reserved apparmor char.*`)
+
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", Symlink: "$SNAP/{here,there}"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid symlink: "/snap/foo/unset/{here,there}" contains a reserved apparmor char.*`)
+       c.Check(ValidateLayout(&Layout{Snap: si, Path: "$SNAP/evil", Symlink: "$SNAP/**"}, nil),
+               ErrorMatches, `layout "\$SNAP/evil" uses invalid symlink: "/snap/foo/unset/\*\*" contains a reserved apparmor char.*`)
+
        // Several valid layouts.
        c.Check(ValidateLayout(&Layout{Snap: si, Path: "/foo", Type: "tmpfs", Mode: 01755}, nil), IsNil)
        c.Check(ValidateLayout(&Layout{Snap: si, Path: "/usr", Bind: "$SNAP/usr"}, nil), IsNil)
@@ -1247,6 +1262,7 @@ layout:
     symlink: $SNAP/existent-dir
 `
 
+       // TODO: merge with the block below
        for _, testCase := range []struct {
                str         string
                topLevelDir string
@@ -1265,6 +1281,24 @@ layout:
                c.Assert(err, ErrorMatches, fmt.Sprintf(`layout %q defines a new top-level directory %q`, testCase.str, testCase.topLevelDir))
        }
 
+       for _, testCase := range []struct {
+               str           string
+               expectedError string
+       }{
+               {"$SNAP/with\"quote", "invalid layout path: .* contains a reserved apparmor char.*"},
+               {"$SNAP/myDir[0123]", "invalid layout path: .* contains a reserved apparmor char.*"},
+               {"$SNAP/here{a,b}", "invalid layout path: .* contains a reserved apparmor char.*"},
+               {"$SNAP/anywhere*", "invalid layout path: .* contains a reserved apparmor char.*"},
+       } {
+               // Layout adding a new top-level directory
+               strk = NewScopedTracker()
+               yaml14 := fmt.Sprintf(yaml14Pattern, testCase.str)
+               info, err = InfoFromSnapYamlWithSideInfo([]byte(yaml14), &SideInfo{Revision: R(42)}, strk)
+               c.Assert(err, IsNil)
+               c.Assert(info.Layout, HasLen, 1)
+               err = ValidateLayoutAll(info)
+               c.Assert(err, ErrorMatches, testCase.expectedError, Commentf("path: %s", testCase.str))
+       }
 }
 
 func (s *YamlSuite) TestValidateAppStartupOrder(c *C) {
index bb0f64d4e26bd0c461227625be0eb5d9c7b5cc76..f35e828901529ce7f977ed3d627384f4872dc0c7 100644 (file)
@@ -210,6 +210,10 @@ backends:
                   image: ubuntu-16.04-64
                   username: ubuntu
                   password: ubuntu
+            - ubuntu-core-18-32:
+                  image: ubuntu-18.04-32
+                  username: ubuntu
+                  password: ubuntu
             - ubuntu-core-18-64:
                   image: ubuntu-16.04-64
                   username: ubuntu
index c8bd1e22667de93e9caa3446669fefcd4908851e..42cdf32c4515d4a6f7984d42f38e2ece5532a826 100755 (executable)
@@ -132,3 +132,9 @@ restore_snapd_lib() {
     fi
     rsync -av --delete "$SNAPD_STATE_PATH"/snapd-lib/cache /var/lib/snapd
 }
+
+remove_disabled_snaps() {
+    snap list --all | grep disabled | while read -r name _ revision _ ; do
+        snap remove "$name" --revision="$revision"
+    done
+}
index 414389ab54775a10fa4814f440cb3c005928cb81..73754f7f454fe1fbb95c9bfb9fafa43820d318d1 100644 (file)
@@ -14,3 +14,5 @@ execute: |
   # the retry here is because there's a race between installing the docker snap
   # and dockerd to be "ready" enough such that docker can talk to it properly
   retry -n 30 --wait 1 docker run hello-world | MATCH "installation appears to be working correctly"
+
+  
diff --git a/tests/main/snap-confine-tmp-mount/task.yaml b/tests/main/snap-confine-tmp-mount/task.yaml
new file mode 100644 (file)
index 0000000..97d5101
--- /dev/null
@@ -0,0 +1,59 @@
+summary: ensure snap-confine controls private mount namespace
+
+details: |
+    Ensure that when creating the private mount namespace for a snap that
+    if it already exists but is not owned by root then any existing
+    contents within the private mount directory is first removed before the
+    mount is created.
+
+# ubuntu-14.04: the test sets up a user session, which requires more recent systemd
+systems: [-ubuntu-14.04-*]
+
+prepare: |
+    echo "Install a helper snap"
+    "$TESTSTOOLS"/snaps-state install-local test-snapd-sh
+
+    tests.session -u test prepare
+
+restore: |
+    tests.session -u test restore
+
+execute: |
+    rm -rf /tmp/snap.test-snapd-sh
+    # create /tmp/snap.test-snapd-sh as a regular user
+    tests.session -u test exec sh -c "mkdir /tmp/snap.test-snapd-sh"
+    test_umask=$(tests.session -u test exec sh -c "umask")
+    # check permissions are as expected
+    expected=$(printf "%o" $((0777-test_umask)))
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "test test $expected"
+    # and place other contents there
+    tests.session -u test exec sh -c "mkdir /tmp/snap.test-snapd-sh/tmp"
+    tests.session -u test exec sh -c "touch /tmp/snap.test-snapd-sh/tmp/foo"
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp | MATCH "test test $expected"
+    expected=$(printf "%o" $((0666-test_umask)))
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp/foo | MATCH "test test $expected"
+
+    # then execute snap-confine - this should take over our imposter base
+    # dir but execute id successfully - snap-confine outputs to stderr and
+    # id will output to stdout so capture each separately
+    SNAP_CONFINE=$(os.paths libexec-dir)/snapd/snap-confine
+    if os.query is-core; then
+        # on Ubuntu Core we need to use the correct path to ensure it is
+        # appropriately confined by apparmor as it may be from the snapd
+        # snap
+        SNAP_CONFINE=$(aa-status | grep "snap-confine$")
+    fi
+    tests.session -u test exec sh -c "env -i SNAPD_DEBUG=1 SNAP_INSTANCE_NAME=test-snapd-sh $SNAP_CONFINE --base core snap.test-snapd-sh.sh /bin/bash -c id 1>/tmp/snap-confine-stdout.log 2>/tmp/snap-confine-stderr.log"
+    tests.cleanup defer rm -f /tmp/snap-confine-stdout.log /tmp/snap-confine-stderr.log
+
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "root root 700"
+
+    # contents should have been removed and tmp dir recreated with root
+    # ownership but foo file should have been removed
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp | MATCH "root root 1777"
+    [ -f /tmp/snap.test-snapd-sh/tmp/foo ] && exit 1
+    # actual dir should be owned by root now
+    stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "root root 700"
+    # and snap-confine should ensure the target binary is executed as the test user
+    MATCH "uid=12345\(test\) gid=12345\(test\)" /tmp/snap-confine-stdout.log
+
diff --git a/tests/main/snap-confine-unexpected-path/task.yaml b/tests/main/snap-confine-unexpected-path/task.yaml
new file mode 100644 (file)
index 0000000..576eec1
--- /dev/null
@@ -0,0 +1,35 @@
+summary: ensure snap-confine denies operation when not in expected location
+
+details: |
+    Ensure that when running from an unexpected location, snap-confine will
+    not execute the snap-discard-ns helper from the same location since
+    this may not be the one which is expected.
+
+environment:
+    SNAP_CONFINE: $(os.paths libexec-dir)/snapd/snap-confine
+
+prepare: |
+    echo "Install a helper snap"
+    "$TESTSTOOLS"/snaps-state install-local test-snapd-sh
+
+execute: |
+    # copy snap-confine with full permissions to /tmp - ideally we would do
+    # this by hardlinking snap-confine into /tmp to make this a more
+    # realistic test (as this is something a regular user could do assuming
+    # fs.protected_hardlinks is disabled) but some spread systems have /tmp
+    # on a tmpfs and hence a different mount point so instead copy it as
+    # root for the test
+    echo "Copying snap-confine to /tmp"
+
+    cp -a "$SNAP_CONFINE" /tmp
+    tests.cleanup defer rm -f /tmp/snap-confine
+    # ensure has the correct permissions
+    diff <(stat -c "%U %G %a" "$SNAP_CONFINE") <(stat -c "%U %G %a" /tmp/snap-confine)
+
+    # then execute /tmp/snap-confine - this should fail since snap-confine
+    # is not in the location it expects to be when it goes to find the
+    # snap-discard-ns etc helper binaries
+    env -i SNAP_INSTANCE_NAME=test-snapd-sh /tmp/snap-confine --base snapd snap.test-snapd-sh.sh /nonexistent 2>/tmp/snap-confine-output.txt && exit 1
+    tests.cleanup defer rm -f /tmp/snap-confine-output.txt
+    MATCH "running from unexpected location: /tmp/snap-confine" /tmp/snap-confine-output.txt
+
diff --git a/tests/main/snap-run-devmode-classic/task.yaml b/tests/main/snap-run-devmode-classic/task.yaml
new file mode 100644 (file)
index 0000000..51f8c0a
--- /dev/null
@@ -0,0 +1,202 @@
+summary: Ensure snap run inside a devmode snap works (for now)
+
+details: |
+    This test ensures inside a devmode snap the "snap run" command
+    works.
+
+    Because of historic mistakes we allowed this and until we properly
+    deprecated it we need to ensure it works. We really do not want to
+    support running other snaps from devmode snaps as the use-case for
+    devmode is to get help on the way to confined snaps. But snaps can
+    not run other snaps so this does not make sense.
+
+systems:
+    # run the classic test on xenial so that we can build the snapd snap
+    # destructively without needing the lxd snap and thus execute much quicker
+    # NOTE: if this test is moved to classic impish or later before the snapd
+    # snap moves off of building based on xenial, then building with LXD will
+    # not work because xenial containers do not boot/get networking properly
+    # when the host has cgroupsv2 in it
+    - ubuntu-16.04-*
+
+environment:
+    # to build natively on the machine rather than with multipass or lxd
+    SNAPCRAFT_BUILD_ENVIRONMENT: host
+
+    # ensure that re-exec is on by default like it should be
+    SNAP_REEXEC: "1"
+
+    SNAP_TO_USE_FIRST/snapd_first: snapd
+    SNAP_TO_USE_FIRST/core_first: core
+
+    # TODO: we should probably have a smaller / simpler test-snapd-* snap for
+    # testing devmode confinement with base: core
+    BASE_CORE_DEVMODE_SNAP: godd
+    BASE_NON_CORE_DEVMODE_SNAP: test-snapd-tools-core18
+
+    BASE_CORE_STRICT_SNAP: test-snapd-sh
+    BASE_NON_CORE_STRICT_SNAP: test-snapd-sh-core18
+
+prepare: |
+    # much of what follows is copied from tests/main/snapd-snap
+
+    # install snapcraft snap
+
+    snap install snapcraft --channel=4.x/candidate --classic
+    tests.cleanup defer snap remove --purge snapcraft
+
+    # shellcheck disable=SC2164
+    pushd "$PROJECT_PATH"
+    echo "Build the snap"
+    snap run snapcraft snap --output snapd-from-branch.snap
+    popd
+
+    mv "$PROJECT_PATH/snapd-from-branch.snap" "$PWD/snapd-from-branch.snap"
+
+    # meh it doesn't work well to use quotas and "&&" in the arguments to sh -c
+    # with defer, so just put what we want to run in a script and execute that
+    cat >> snapcraft-cleanup.sh <<EOF
+    #!/bin/sh
+    cd $PROJECT_PATH
+    snap run snapcraft clean
+    EOF
+    chmod +x snapcraft-cleanup.sh
+    tests.cleanup defer sh -c "$PWD/snapcraft-cleanup.sh"
+
+    unsquashfs -d snapd-from-branch snapd-from-branch.snap
+    snapddir=snapd-from-branch
+
+    # now repack the core snap with this snapd snap
+    snap download core --edge --basename=core-from-edge
+    unsquashfs -d edge-core-snap core-from-edge.snap
+    coredir=edge-core-snap
+
+    # backup the meta dir
+    mv "$coredir/meta" "$coredir/meta-backup" 
+    # copy everything from the snapd snap into the core snap
+    cp -ar "$snapddir"/* "$coredir"
+
+    # restore the meta dir
+    rm -r "$coredir/meta"
+    mv "$coredir/meta-backup" "$coredir/meta" 
+
+    # set the version for the core snap to be the version from the snapd snap
+    SNAPD_SNAP_VERSION=$(grep -Po "version: \K.*" "$snapddir/meta/snap.yaml")
+    CORE_SNAP_VERSION=$(grep -Po "version: \K.*" "$coredir/meta/snap.yaml")
+    sed -i -e "s/$CORE_SNAP_VERSION/$SNAPD_SNAP_VERSION/" "$coredir/meta/snap.yaml"
+
+    # pack the core snap
+    snap pack --filename=core-from-branch.snap "$coredir"
+
+    rm -r "$coredir"
+    rm -r "$snapddir"
+
+execute: |
+    if [ "$SNAP_TO_USE_FIRST" = "core" ]; then
+        # first install our core snap because we don't have the snapd snap on 
+        # the system yet, so we don't need to do any shenanigans
+        snap install --dangerous core-from-branch.snap
+
+        snap install --devmode --beta "$BASE_CORE_DEVMODE_SNAP"
+        snap install "$BASE_CORE_STRICT_SNAP"
+
+        # umask is the command we execute to avoid yet another layer of quoting
+        OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_CORE_DEVMODE_SNAP}")
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+        snap install --dangerous snapd-from-branch.snap
+
+        # trigger profile re-generation because the same build-id for snapd is
+        # in the core and snapd snaps we are using, so profiles won't be 
+        # regenerated when we install the snapd snap above
+        systemctl stop snapd.socket snapd.service
+        rm /var/lib/snapd/system-key
+        systemctl start snapd.socket snapd.service
+
+        # also install the non-core base snap, note that we can install and use it
+        # even without the snapd snap, but we cannot execute other snaps from this 
+        # devmode snap without also installing the snapd snap, as inside non-core
+        # base snaps, there is a symlink 
+        # /usr/bin/snap -> /snap/snapd/current/usr/bin/snap
+        # which effectively requires the snapd snap to be installed to execute other
+        # snaps from inside the devmode non-core based snap
+        snap install --devmode "$BASE_NON_CORE_DEVMODE_SNAP"
+
+        # umask is the command we execute to avoid yet another layer of quoting
+        OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_CORE_DEVMODE_SNAP}")
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+        OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_NON_CORE_DEVMODE_SNAP}.sh")
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+    elif [ "$SNAP_TO_USE_FIRST" = "snapd" ]; then
+        # we already had the core snap installed, so we need to purge things
+        # and then install only the snapd snap to test this scenario
+
+        snap remove go snapcraft
+        snap remove core18
+        apt remove --purge -y snapd
+        apt install snapd -y
+
+        snap install --dangerous snapd-from-branch.snap
+
+        # snaps that don't depend on the core snap
+        snap install --devmode "$BASE_NON_CORE_DEVMODE_SNAP"
+        snap install "$BASE_NON_CORE_STRICT_SNAP"
+
+        # umask is the command we execute to avoid yet another layer of quoting
+        OUTPUT=$(echo "snap run ${BASE_NON_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_NON_CORE_DEVMODE_SNAP}.sh" )
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+        # now install the core snap and run those tests
+        echo "install the core snap"
+        snap install --dangerous core-from-branch.snap
+
+        # trigger profile re-generation because the same build-id for snapd is
+        # in the core and snapd snaps we are using, so profiles won't be 
+        # regenerated when we install the snapd snap above
+        systemctl stop snapd.socket snapd.service
+        rm /var/lib/snapd/system-key
+        systemctl start snapd.socket snapd.service
+
+        snap install --devmode --beta "$BASE_CORE_DEVMODE_SNAP"
+        snap install "$BASE_CORE_STRICT_SNAP"
+
+        OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_CORE_DEVMODE_SNAP}")
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+        OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | snap run --shell "${BASE_NON_CORE_DEVMODE_SNAP}.sh")
+        if [ "$OUTPUT" != "0022" ]; then
+            echo "test failed"
+            exit 1
+        fi
+
+        # docker can run from a devmode snap
+        snap install docker
+        echo "snap run docker run hello-world" | snap run --shell "${BASE_NON_CORE_DEVMODE_SNAP}.sh" | MATCH "Hello from Docker"
+
+        
+        # undo the purging
+        apt install -y "$PROJECT_PATH/../snapd_1337.2.54.2_amd64.deb"
+
+    else
+        echo "unknown variant $SNAP_TO_USE_FIRST"
+        exit 1
+    fi
+
+    
diff --git a/tests/nested/manual/core-seeding-devmode/task.yaml b/tests/nested/manual/core-seeding-devmode/task.yaml
new file mode 100644 (file)
index 0000000..321c4fe
--- /dev/null
@@ -0,0 +1,22 @@
+summary: Test that devmode snaps can be installed during seeding.
+
+# testing with core16 (no snapd snap) and core18 (with snapd snap) is enough
+systems: [ubuntu-16.04-64, ubuntu-18.04-64]
+
+environment:
+    NESTED_IMAGE_ID: core-seeding-devmode
+
+prepare: |
+    # seed a devmode snap
+    snap download --beta godd
+    GODD_SNAP=$(ls godd_*.snap)
+    mv "$GODD_SNAP" "$(tests.nested get extra-snaps-path)"
+
+    tests.nested build-image core
+    tests.nested create-vm core
+
+execute: |
+    tests.nested exec "sudo snap wait system seed.loaded"
+
+    # godd is installed
+    tests.nested exec "snap list godd" | MATCH "godd"
diff --git a/tests/nested/manual/devmode-snaps-can-run-other-snaps/task.yaml b/tests/nested/manual/devmode-snaps-can-run-other-snaps/task.yaml
new file mode 100644 (file)
index 0000000..755c84f
--- /dev/null
@@ -0,0 +1,220 @@
+summary: |
+  Test that devmode confined snaps can execute other snaps.
+
+systems:
+  - ubuntu-18.04-64
+  - ubuntu-16.04-64
+
+environment:
+  # not needed to build snapd from source to use here, we have to manually
+  # build it ourselves anyways
+  NESTED_BUILD_SNAPD_FROM_CURRENT: false
+
+  # TODO: we should probably have a smaller / simpler test-snapd-* snap for
+  # testing devmode confinement with base: core
+  BASE_CORE_DEVMODE_SNAP: godd
+  BASE_NON_CORE_DEVMODE_SNAP: test-snapd-tools-core18
+
+  BASE_CORE_STRICT_SNAP: test-snapd-sh
+  BASE_NON_CORE_STRICT_SNAP: test-snapd-sh-core18
+
+  # build the snap with lxd
+  SNAPCRAFT_BUILD_ENVIRONMENT: lxd
+
+prepare: |
+  # install lxd so we can build the snapd snap
+  snap install lxd --channel="$LXD_SNAP_CHANNEL"
+
+  snap install snapcraft --channel=4.x/candidate --classic
+  tests.cleanup defer snap remove --purge snapcraft
+
+  # much of what follows is copied from tests/main/snapd-snap
+
+  echo "Remove any installed debs (some images carry them) to ensure we test the snap"
+  # apt -v to test if apt is usable
+  if command -v apt && apt -v; then
+      # meh trusty's apt doesn't support -y, so use apt-get
+      apt-get autoremove -y lxd
+      if ! os.query is-debian-sid; then
+          # no lxd-client on debian sid
+          apt-get autoremove -y lxd-client
+      fi
+  fi
+
+  # load the fuse kernel module before installing lxd
+  modprobe fuse
+
+  snap set lxd waitready.timeout=240
+  lxd waitready
+  lxd init --auto
+
+  echo "Setting up proxy for lxc"
+  if [ -n "${http_proxy:-}" ]; then
+      lxd.lxc config set core.proxy_http "$http_proxy"
+  fi
+  if [ -n "${https_proxy:-}" ]; then
+      lxd.lxc config set core.proxy_https "$http_proxy"
+  fi
+
+  # TODO: do we need to address the spread system prepare shenanigans as 
+  # mentioned in tests/main/snapd-snap ?
+
+  # shellcheck disable=SC2164
+  pushd "$PROJECT_PATH"
+  echo "Build the snap"
+  snap run snapcraft snap --output snapd-from-branch.snap
+  popd
+
+  mv "$PROJECT_PATH/snapd-from-branch.snap" "$PWD/snapd-from-branch.snap"
+
+  # meh it doesn't work well to use quotas and "&&" in the arguments to sh -c
+  # with defer, so just put what we want to run in a script and execute that
+  cat >> snapcraft-cleanup.sh <<EOF
+  #!/bin/sh
+  cd $PROJECT_PATH
+  snap run snapcraft clean
+  EOF
+  chmod +x snapcraft-cleanup.sh
+  tests.cleanup defer sh -c "$PWD/snapcraft-cleanup.sh"
+
+  unsquashfs -d snapd-from-branch snapd-from-branch.snap
+  snapddir=snapd-from-branch
+
+  # now repack the core snap with this snapd snap
+  snap download core --edge --basename=core-from-edge
+  unsquashfs -d edge-core-snap core-from-edge.snap
+  coredir=edge-core-snap
+
+  # backup the meta dir
+  mv "$coredir/meta" "$coredir/meta-backup" 
+  # copy everything from the snapd snap into the core snap
+  cp -ar "$snapddir"/* "$coredir"
+
+  # restore the meta dir
+  rm -r "$coredir/meta"
+  mv "$coredir/meta-backup" "$coredir/meta" 
+
+  # set the version for the core snap to be the version from the snapd snap
+  SNAPD_SNAP_VERSION=$(grep -Po "version: \K.*" "$snapddir/meta/snap.yaml")
+  CORE_SNAP_VERSION=$(grep -Po "version: \K.*" "$coredir/meta/snap.yaml")
+  sed -i -e "s/$CORE_SNAP_VERSION/$SNAPD_SNAP_VERSION/" "$coredir/meta/snap.yaml"
+
+  # pack the core snap
+  snap pack --filename=core-from-branch.snap "$coredir"
+
+  rm -r "$coredir"
+  rm -r "$snapddir"
+
+  tests.nested build-image core 
+  tests.nested create-vm core
+
+execute: |
+  # TODO: should we also just test the classic cases on the system that is 
+  # driving the nested VM? That would save some time/resources
+
+  # wait for snap seeding to be done
+  tests.nested wait-for snap-command
+  tests.nested exec "sudo snap wait system seed.loaded"
+
+  # push both snaps to the vm
+  tests.nested copy core-from-branch.snap
+
+  tests.nested copy snapd-from-branch.snap
+
+  if os.query is-xenial; then
+    # on UC16, initially we will only have the core snap installed, run those
+    # tests first
+
+    # this will reboot as we refresh to our core snap
+    boot_id="$( tests.nested boot-id )"
+    REMOTE_CHG_ID="$(tests.nested exec sudo snap install --no-wait --dangerous core-from-branch.snap)"
+    tests.nested wait-for reboot "${boot_id}"
+    tests.nested exec sudo snap watch "${REMOTE_CHG_ID}"
+
+    tests.nested exec sudo snap install --devmode --beta "$BASE_CORE_DEVMODE_SNAP"
+    tests.nested exec sudo snap install "$BASE_CORE_STRICT_SNAP"
+
+    # umask is the command we execute to avoid yet another layer of quoting
+    OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_CORE_DEVMODE_SNAP}")
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+    # now install the snapd snap and run those tests
+    echo "install the snapd snap"
+    tests.nested exec sudo snap install --dangerous snapd-from-branch.snap
+
+    # trigger regeneration of profiles
+    tests.nested exec sudo systemctl stop snapd.socket snapd.service
+    tests.nested exec sudo rm -f /var/lib/snapd/system-key
+    tests.nested exec sudo systemctl start snapd.socket snapd.service
+
+    # also install the non-core base snap, note that we can install and use it
+    # even without the snapd snap, but we cannot execute other snaps from this 
+    # devmode snap without also installing the snapd snap, as inside non-core
+    # base snaps, there is a symlink 
+    # /usr/bin/snap -> /snap/snapd/current/usr/bin/snap
+    # which effectively requires the snapd snap to be installed to execute other
+    # snaps from inside the devmode non-core based snap
+    tests.nested exec sudo snap install --devmode "$BASE_NON_CORE_DEVMODE_SNAP"
+
+    # umask is the command we execute to avoid yet another layer of quoting
+    OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_CORE_DEVMODE_SNAP}")
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+    OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_NON_CORE_DEVMODE_SNAP}.sh")
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+  elif os.query is-bionic; then
+    # on UC18, initially we will only have the snapd snap installed, run those
+    # tests first
+    tests.nested exec sudo snap install  --dangerous snapd-from-branch.snap
+
+    # snaps that don't depend on the core snap
+    tests.nested exec sudo snap install --devmode "$BASE_NON_CORE_DEVMODE_SNAP"
+    tests.nested exec sudo snap install "$BASE_NON_CORE_STRICT_SNAP"
+
+
+    # umask is the command we execute to avoid yet another layer of quoting
+    OUTPUT=$(echo "snap run ${BASE_NON_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_NON_CORE_DEVMODE_SNAP}.sh" )
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+    # now install the core snap and run those tests
+    echo "install the core snap"
+    tests.nested exec sudo snap install --dangerous core-from-branch.snap
+
+    # trigger regeneration of profiles
+    tests.nested exec sudo systemctl stop snapd.socket snapd.service
+    tests.nested exec sudo rm -f /var/lib/snapd/system-key
+    tests.nested exec sudo systemctl start snapd.socket snapd.service
+
+    # snap that does depend on the core snap
+    tests.nested exec sudo snap install --devmode --beta "$BASE_CORE_DEVMODE_SNAP"
+    tests.nested exec sudo snap install "$BASE_CORE_STRICT_SNAP"
+
+    OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_CORE_DEVMODE_SNAP}")
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+    OUTPUT=$(echo "snap run ${BASE_CORE_STRICT_SNAP}.sh -c umask" | tests.nested exec "snap run --shell ${BASE_NON_CORE_DEVMODE_SNAP}.sh")
+    if [ "$OUTPUT" != "0002" ]; then
+      echo "test failed"
+      exit 1
+    fi
+
+  else
+    echo "unsupported system for this test"
+    exit 1
+  fi
diff --git a/tests/regression/lp-1949368/bad-layout/meta/snap.yaml b/tests/regression/lp-1949368/bad-layout/meta/snap.yaml
new file mode 100644 (file)
index 0000000..4207f28
--- /dev/null
@@ -0,0 +1,5 @@
+name: bad-layout
+version: 1.0
+layout:
+  /var/lib/foo:
+    bind: "$SNAP_DATA/var/*"
diff --git a/tests/regression/lp-1949368/content-consumer/bin/sh b/tests/regression/lp-1949368/content-consumer/bin/sh
new file mode 100644 (file)
index 0000000..0f845e0
--- /dev/null
@@ -0,0 +1,3 @@
+#!/bin/sh
+PS1='$ '
+exec /bin/sh "$@"
diff --git a/tests/regression/lp-1949368/content-consumer/meta/snap.yaml b/tests/regression/lp-1949368/content-consumer/meta/snap.yaml
new file mode 100644 (file)
index 0000000..f651398
--- /dev/null
@@ -0,0 +1,12 @@
+name: content-consumer
+version: 1.0
+apps:
+  sh:
+    command: bin/sh
+plugs:
+  quoting:
+    interface: content
+    target: "$SNAP_DATA/a,comma"
+  invalid-char:
+    interface: content
+    target: "$SNAP_DATA/{this,that}"
diff --git a/tests/regression/lp-1949368/content-provider/meta/snap.yaml b/tests/regression/lp-1949368/content-provider/meta/snap.yaml
new file mode 100644 (file)
index 0000000..46b3a53
--- /dev/null
@@ -0,0 +1,9 @@
+name: content-provider
+version: 1.0
+slots:
+  quoting:
+    interface: content
+    read: ["$SNAP_DATA/a,comma"]
+  invalid-char:
+    interface: content
+    read: ["$SNAP_DATA/{this,that}"]
diff --git a/tests/regression/lp-1949368/task.yaml b/tests/regression/lp-1949368/task.yaml
new file mode 100644 (file)
index 0000000..d1c3b17
--- /dev/null
@@ -0,0 +1,35 @@
+summary: Ensure that AppArmor paths are safe
+
+details: |
+    Some interfaces allow the developer to specify filesystem paths in their
+    plug sttributes, which then get encoded into the AppArmor profile of the
+    applications. We need to make sure that these paths are properly quoted,
+    and that snapd will refuse to connect a plug whose paths include some
+    invalid characters.
+
+prepare: |
+    echo "Creating the test snaps"
+    "$TESTSTOOLS"/snaps-state install-local content-provider
+    "$TESTSTOOLS"/snaps-state install-local content-consumer
+
+
+execute: |
+    echo "The plug is disconnected by default"
+    snap interfaces content-provider
+
+    echo "Verify that the plug with invalid characters raised a warning"
+    snap warnings |
+        tr -d '\n' | # remove newlines 
+        sed -e 's,\s\+, ,g' | # remove any extra spaces
+        MATCH 'snap "content-consumer" has bad plugs or slots: invalid-char'
+
+    echo "Connect a valid plug"
+    snap connect content-consumer:quoting content-provider:quoting
+
+    if [ "$(snap debug confinement)" = "strict" ]; then
+        echo "Verify that the AppArmor rule has proper quoting"
+        MATCH '"/var/snap/content-provider/x1/a,comma/\*\*" mrkix,' < /var/lib/snapd/apparmor/profiles/snap.content-consumer.sh
+    fi
+
+    echo "Attempt to install a snap with an invalid layout"
+    "$TESTSTOOLS"/snaps-state install-local bad-layout 2>&1 |MATCH 'cannot validate snap "bad-layout".*contains a reserved apparmor char'