diff --git a/cmd/libsnap-confine-private/apparmor-support.c b/cmd/libsnap-confine-private/apparmor-support.c index eac0912d3e6..ae6fc048353 100644 --- a/cmd/libsnap-confine-private/apparmor-support.c +++ b/cmd/libsnap-confine-private/apparmor-support.c @@ -20,6 +20,8 @@ #endif #include "apparmor-support.h" +#include "string-utils.h" +#include "utils.h" #include #include @@ -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. diff --git a/cmd/libsnap-confine-private/tool.c b/cmd/libsnap-confine-private/tool.c index 008d53154ac..0c239538b91 100644 --- a/cmd/libsnap-confine-private/tool.c +++ b/cmd/libsnap-confine-private/tool.c @@ -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); diff --git a/cmd/libsnap-confine-private/utils-test.c b/cmd/libsnap-confine-private/utils-test.c index 14709179b8f..2440d722a7f 100644 --- a/cmd/libsnap-confine-private/utils-test.c +++ b/cmd/libsnap-confine-private/utils-test.c @@ -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", diff --git a/cmd/libsnap-confine-private/utils.c b/cmd/libsnap-confine-private/utils.c index 625bea1af74..635538a3dae 100644 --- a/cmd/libsnap-confine-private/utils.c +++ b/cmd/libsnap-confine-private/utils.c @@ -16,6 +16,7 @@ */ #include #include +#include #include #include #include @@ -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; +} diff --git a/cmd/libsnap-confine-private/utils.h b/cmd/libsnap-confine-private/utils.h index cc214e47fa5..fdd17f2db2f 100644 --- a/cmd/libsnap-confine-private/utils.h +++ b/cmd/libsnap-confine-private/utils.h @@ -111,4 +111,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 diff --git a/cmd/snap-confine/mount-support-test.c b/cmd/snap-confine/mount-support-test.c index 751873f9ed5..9f2d78afecb 100644 --- a/cmd/snap-confine/mount-support-test.c +++ b/cmd/snap-confine/mount-support-test.c @@ -21,6 +21,7 @@ #include "mount-support-nvidia.c" #include +#include 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); } diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c index 5ac1dcdac81..2a46fea4129 100644 --- a/cmd/snap-confine/mount-support.c +++ b/cmd/snap-confine/mount-support.c @@ -14,6 +14,7 @@ * along with this program. If not, see . * */ + #ifdef HAVE_CONFIG_H #include "config.h" #endif @@ -51,6 +52,91 @@ 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); } @@ -464,7 +529,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); diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index 3ed2508301e..664d585da9c 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -45,6 +45,17 @@ @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, @@ -403,10 +414,10 @@ # 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, diff --git a/cmd/snap/cmd_run.go b/cmd/snap/cmd_run.go index 44c18923a2d..9b7ae09e8dd 100644 --- a/cmd/snap/cmd_run.go +++ b/cmd/snap/cmd_run.go @@ -1031,6 +1031,8 @@ func (x *cmdRun) runSnapConfine(info *snap.Info, securityTag, snapApp, hook stri return fmt.Errorf(i18n.G("missing snap-confine: try updating your core/snapd package")) } + logger.Debugf("executing snap-confine from %s", snapConfine) + snapName, _ := snap.SplitSnapApp(snapApp) opts, err := getSnapDirOptions(snapName) if err != nil { diff --git a/interfaces/apparmor/apparmor_test.go b/interfaces/apparmor/apparmor_test.go index e5d76913905..56bc1c72912 100644 --- a/interfaces/apparmor/apparmor_test.go +++ b/interfaces/apparmor/apparmor_test.go @@ -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/usr/lib/snapd/snap-confine ... ) + // * exec( /snap/core//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}//usr/lib/snapd/snap-confine + %[2]s +`, usrLibSnapdConfineTransitionRule, nonBaseCoreTransitionSnippet) + case "###VAR###": return templateVariables(snapInfo, securityTag, cmdName) case "###PROFILEATTACH###": diff --git a/interfaces/apparmor/backend_test.go b/interfaces/apparmor/backend_test.go index 8548ecbb574..5b26b6af580 100644 --- a/interfaces/apparmor/backend_test.go +++ b/interfaces/apparmor/backend_test.go @@ -174,6 +174,9 @@ func (s *backendSuite) SetUpTest(c *C) { restore = apparmor_sandbox.MockFeatures(nil, nil, nil, nil) s.AddCleanup(restore) + + 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) @@ -1392,7 +1395,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyWithNFSNoProfileFiles( // The apparmor backend should not fail if the apparmor profile of // snap-confine is not present - err := (&apparmor.Backend{}).Initialize(nil) + err := (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts) c.Assert(err, IsNil) // Since there is no profile file, no call to apparmor were made c.Assert(cmd.Calls(), HasLen, 0) @@ -1430,7 +1433,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. @@ -1483,7 +1486,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. @@ -1528,7 +1531,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. @@ -1565,7 +1568,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. @@ -1604,7 +1607,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 @@ -1620,13 +1623,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") } @@ -1673,7 +1678,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. @@ -1699,7 +1704,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) @@ -1739,7 +1744,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyWithOverlayNoProfileFi // The apparmor backend should not fail if the apparmor profile of // snap-confine is not present - err := (&apparmor.Backend{}).Initialize(nil) + err := (&apparmor.Backend{}).Initialize(ifacetest.DefaultInitializeOpts) c.Assert(err, IsNil) // Since there is no profile file, no call to apparmor were made c.Assert(cmd.Calls(), HasLen, 0) @@ -1774,7 +1779,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. @@ -1826,7 +1831,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. @@ -1879,7 +1884,7 @@ func (s *backendSuite) testSetupSnapConfineGeneratedPolicyWithBPFCapability(c *C 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) // Capability bpf is supported by the parser, so an extra policy file @@ -1951,7 +1956,7 @@ func (s *backendSuite) TestSetupSnapConfineGeneratedPolicyWithBPFProbeError(c *C 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) // Probing apparmor_parser capabilities failed, so nothing gets written @@ -2404,7 +2409,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 { diff --git a/interfaces/apparmor/spec.go b/interfaces/apparmor/spec.go index ab5b6372dfd..0a01307d055 100644 --- a/interfaces/apparmor/spec.go +++ b/interfaces/apparmor/spec.go @@ -297,30 +297,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 } } @@ -374,7 +374,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()) } } @@ -392,33 +392,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) } } @@ -429,9 +429,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) @@ -447,7 +447,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) @@ -541,9 +541,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) } diff --git a/interfaces/apparmor/spec_test.go b/interfaces/apparmor/spec_test.go index 6a26d85da17..72a3eb0874c 100644 --- a/interfaces/apparmor/spec_test.go +++ b/interfaces/apparmor/spec_test.go @@ -283,72 +283,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. @@ -357,49 +357,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. @@ -408,46 +408,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. @@ -456,9 +456,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. diff --git a/interfaces/apparmor/template.go b/interfaces/apparmor/template.go index f5da8e0bfa6..f8271b67e89 100644 --- a/interfaces/apparmor/template.go +++ b/interfaces/apparmor/template.go @@ -463,6 +463,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 = ` diff --git a/interfaces/backend.go b/interfaces/backend.go index 4a37f4ae541..fb457114bda 100644 --- a/interfaces/backend.go +++ b/interfaces/backend.go @@ -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 diff --git a/interfaces/builtin/common_files.go b/interfaces/builtin/common_files.go index ef1804e8f43..219aa60d79e 100644 --- a/interfaces/builtin/common_files.go +++ b/interfaces/builtin/common_files.go @@ -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 } diff --git a/interfaces/builtin/content.go b/interfaces/builtin/content.go index afbc9cbccc4..b511fade88f 100644 --- a/interfaces/builtin/content.go +++ b/interfaces/builtin/content.go @@ -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" ) @@ -70,6 +71,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 { @@ -104,8 +115,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 @@ -124,8 +135,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 @@ -226,13 +237,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 @@ -251,15 +262,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) @@ -283,7 +294,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) } } diff --git a/interfaces/builtin/content_test.go b/interfaces/builtin/content_test.go index 0fd3b4e2044..02acdb8c051 100644 --- a/interfaces/builtin/content_test.go +++ b/interfaces/builtin/content_test.go @@ -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) } diff --git a/interfaces/builtin/cups_test.go b/interfaces/builtin/cups_test.go index 43ef814469a..00281aa683e 100644 --- a/interfaces/builtin/cups_test.go +++ b/interfaces/builtin/cups_test.go @@ -138,59 +138,59 @@ const expSnapUpdateNsSnippet = ` # Mount cupsd socket from cups snap to client mount options=(rw bind) "/var/snap/provider/common/foo-subdir/" -> /var/cups/, umount /var/cups/, # Writable directory /var/snap/provider/common/foo-subdir - /var/snap/provider/common/foo-subdir/ rw, - /var/snap/provider/common/ rw, - /var/snap/provider/ rw, + "/var/snap/provider/common/foo-subdir/" rw, + "/var/snap/provider/common/" rw, + "/var/snap/provider/" rw, # Writable mimic /var # .. 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 /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/*/", ` func (s *cupsSuite) TestAppArmorSpec(c *C) { diff --git a/interfaces/builtin/daemon_notify.go b/interfaces/builtin/daemon_notify.go index 4ac923a571a..f380f4b2ac0 100644 --- a/interfaces/builtin/daemon_notify.go +++ b/interfaces/builtin/daemon_notify.go @@ -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) } diff --git a/interfaces/ifacetest/backendtest.go b/interfaces/ifacetest/backendtest.go index c60331bd756..7d1836a7400 100644 --- a/interfaces/ifacetest/backendtest.go +++ b/interfaces/ifacetest/backendtest.go @@ -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) diff --git a/interfaces/seccomp/backend_test.go b/interfaces/seccomp/backend_test.go index 7fbc0bcce82..a28b56b8cc4 100644 --- a/interfaces/seccomp/backend_test.go +++ b/interfaces/seccomp/backend_test.go @@ -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") diff --git a/overlord/devicestate/firstboot_test.go b/overlord/devicestate/firstboot_test.go index b8dfcc71fa2..07e31393267 100644 --- a/overlord/devicestate/firstboot_test.go +++ b/overlord/devicestate/firstboot_test.go @@ -1336,7 +1336,14 @@ type: base` snapdYaml := `name: snapd version: 1.0 ` - snapdFname, snapdDecl, snapdRev := s.MakeAssertedSnap(c, snapdYaml, nil, snap.R(2), "canonical") + // the info file is needed by the Ensure() loop of snapstate manager + snapdSnapFiles := [][]string{ + {"usr/lib/snapd/info", ` +VERSION=2.54.3+git1.g479e745-dirty +SNAPD_APPARMOR_REEXEC=0 +`}, + } + snapdFname, snapdDecl, snapdRev := s.MakeAssertedSnap(c, snapdYaml, snapdSnapFiles, snap.R(2), "canonical") s.WriteAssertions("snapd.asserts", snapdRev, snapdDecl) var kernelFname string diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go index 555168665db..c12ba7c809f 100644 --- a/overlord/ifacestate/helpers.go +++ b/overlord/ifacestate/helpers.go @@ -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 diff --git a/overlord/managers_test.go b/overlord/managers_test.go index bda58830753..fb82e03091d 100644 --- a/overlord/managers_test.go +++ b/overlord/managers_test.go @@ -437,6 +437,21 @@ func (s *baseMgrsSuite) SetUpTest(c *C) { }, }) + // commonly used core and snapd revisions in tests + defaultInfoFile := ` +VERSION=2.54.3+git1.g479e745-dirty +SNAPD_APPARMOR_REEXEC=0 +` + for _, snapName := range []string{"snapd", "core"} { + for _, rev := range []string{"1", "11", "30"} { + infoFile := filepath.Join(dirs.GlobalRootDir, "snap", snapName, rev, dirs.CoreLibExecDir, "info") + err = os.MkdirAll(filepath.Dir(infoFile), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(infoFile, []byte(defaultInfoFile), 0644) + c.Assert(err, IsNil) + } + } + // don't actually try to talk to the store on snapstate.Ensure // needs doing after the call to devicestate.Manager (which happens in overlord.New) snapstate.CanAutoRefresh = nil @@ -656,8 +671,10 @@ hooks: snapdirs, err := filepath.Glob(filepath.Join(dirs.SnapMountDir, "*")) c.Assert(err, IsNil) - // just README and bin - c.Check(snapdirs, HasLen, 2) + // just README, bin, snapd, and core (snapd and core are there because we + // have info files for those snaps which need to be read from the snapstate + // Ensure loop) + c.Check(snapdirs, HasLen, 4) for _, d := range snapdirs { c.Check(filepath.Base(d), Not(Equals), "foo") } diff --git a/overlord/snapstate/snapmgr.go b/overlord/snapstate/snapmgr.go index 747e504c50e..4132fcd7488 100644 --- a/overlord/snapstate/snapmgr.go +++ b/overlord/snapstate/snapmgr.go @@ -25,6 +25,7 @@ import ( "fmt" "io" "os" + "path/filepath" "strings" "time" @@ -43,7 +44,9 @@ import ( "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/channel" "github.com/snapcore/snapd/snapdenv" + "github.com/snapcore/snapd/snapdtool" "github.com/snapcore/snapd/store" + "github.com/snapcore/snapd/strutil" ) var ( @@ -617,6 +620,120 @@ func (m *SnapManager) EnsureAutoRefreshesAreDelayed(delay time.Duration) ([]*sta return autoRefreshChgsInFlight, nil } +func (m *SnapManager) ensureVulnerableSnapRemoved(name string) error { + var removedYet bool + key := fmt.Sprintf("%s-snap-cve-2021-44731-vuln-removed", name) + if err := m.state.Get(key, &removedYet); err != nil && err != state.ErrNoState { + return err + } + if removedYet { + return nil + } + var snapSt SnapState + err := Get(m.state, name, &snapSt) + if err != nil && err != state.ErrNoState { + return err + } + if err == state.ErrNoState { + // not installed, nothing to do + return nil + } + + // check if the installed, active version is fixed + fixedVersionInstalled := false + inactiveVulnRevisions := []snap.Revision{} + for _, si := range snapSt.Sequence { + // check this version + s := snap.Info{SideInfo: *si} + ver, _, err := snapdtool.SnapdVersionFromInfoFile(filepath.Join(s.MountDir(), dirs.CoreLibExecDir)) + if err != nil { + return err + } + // res is < 0 if "ver" is lower than "2.54.3" + res, err := strutil.VersionCompare(ver, "2.54.3") + if err != nil { + return err + } + revIsVulnerable := (res < 0) + switch { + case !revIsVulnerable && si.Revision == snapSt.Current: + fixedVersionInstalled = true + case revIsVulnerable && si.Revision == snapSt.Current: + // the active installed revision is not fixed, we can break out + // early since we know we won't be able to remove old revisions + return nil + case revIsVulnerable && si.Revision != snapSt.Current: + // si revision is not fixed, but is not active, so it is a candidate + // for removal + inactiveVulnRevisions = append(inactiveVulnRevisions, si.Revision) + default: + // si revision is not active, but it is fixed, so just ignore it + } + } + + if !fixedVersionInstalled { + return nil + } + // TODO: should we use one change for removing all the snap revisions? + + // remove all the inactive vulnerable revisions + for _, rev := range inactiveVulnRevisions { + tss, err := Remove(m.state, name, rev, nil) + + if err != nil { + // in case of conflict, just trigger another ensure in a little + // bit and try again later + if _, ok := err.(*ChangeConflictError); ok { + m.state.EnsureBefore(time.Minute) + return nil + } + return fmt.Errorf("cannot make task set for removing %s snap: %v", name, err) + } + + msg := fmt.Sprintf(i18n.G("Remove vulnerable %q snap"), name) + + chg := m.state.NewChange("remove-snap", msg) + chg.AddAll(tss) + chg.Set("snap-names", []string{name}) + } + + // TODO: is it okay to set state here as done or should we do this + // elsewhere after the change is done somehow? + + // mark state as done + m.state.Set(key, true) + + // not strictly necessary, but does not hurt to ensure anyways + m.state.EnsureBefore(0) + + return nil +} + +func (m *SnapManager) ensureVulnerableSnapConfineVersionsRemovedOnClassic() error { + // only remove snaps on classic + if !release.OnClassic { + return nil + } + + m.state.Lock() + defer m.state.Unlock() + + // we have to remove vulnerable versions of both the core and snapd snaps + // only when we now have fixed versions installed / active + // the fixed version is 2.54.3, so if the version of the current core/snapd + // snap is that or higher, then we proceed (if we didn't already do this) + + if err := m.ensureVulnerableSnapRemoved("snapd"); err != nil { + return err + } + + if err := m.ensureVulnerableSnapRemoved("core"); err != nil { + return err + } + + return nil +} + // ensureForceDevmodeDropsDevmodeFromState undoes the forced devmode // in snapstate for forced devmode distros. func (m *SnapManager) ensureForceDevmodeDropsDevmodeFromState() error { @@ -916,6 +1033,7 @@ func (m *SnapManager) Ensure() error { m.refreshHints.Ensure(), m.catalogRefresh.Ensure(), m.localInstallCleanup(), + m.ensureVulnerableSnapConfineVersionsRemovedOnClassic(), } //FIXME: use firstErr helper diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index 55af99b1d51..b7be0f82b12 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -250,6 +250,21 @@ func (s *snapmgrBaseTest) SetUpTest(c *C) { SnapType: "os", }) + // commonly used revisions in tests + defaultInfoFile := ` +VERSION=2.54.3+git1.g479e745-dirty +SNAPD_APPARMOR_REEXEC=0 +` + for _, snapName := range []string{"snapd", "core"} { + for _, rev := range []string{"1", "11"} { + infoFile := filepath.Join(dirs.GlobalRootDir, "snap", snapName, rev, dirs.CoreLibExecDir, "info") + err = os.MkdirAll(filepath.Dir(infoFile), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(infoFile, []byte(defaultInfoFile), 0644) + c.Assert(err, IsNil) + } + } + repo := interfaces.NewRepository() ifacerepo.Replace(s.state, repo) s.state.Unlock() @@ -2779,6 +2794,228 @@ func (s *snapmgrTestSuite) TestErrreportDisable(c *C) { // no failure report was generated } +func (s *snapmgrTestSuite) TestEnsureRemovesVulnerableCoreSnap(c *C) { + s.testEnsureRemovesVulnerableSnap(c, "core") +} + +func (s *snapmgrTestSuite) TestEnsureRemovesVulnerableSnapdSnap(c *C) { + s.testEnsureRemovesVulnerableSnap(c, "snapd") +} + +func (s *snapmgrTestSuite) testEnsureRemovesVulnerableSnap(c *C, snapName string) { + // make the currently installed snap info file fixed but an old version + // vulnerable + fixedInfoFile := ` +VERSION=2.54.3+git1.g479e745-dirty +SNAPD_APPARMOR_REEXEC=0 +` + vulnInfoFile := ` +VERSION=2.54.2+git1.g479e745-dirty +SNAPD_APPARMOR_REEXEC=0 +` + + // revision 1 vulnerable + infoFile := filepath.Join(dirs.GlobalRootDir, "snap", snapName, "1", dirs.CoreLibExecDir, "info") + err := os.MkdirAll(filepath.Dir(infoFile), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(infoFile, []byte(vulnInfoFile), 0644) + c.Assert(err, IsNil) + + // revision 2 fixed + infoFile2 := filepath.Join(dirs.GlobalRootDir, "snap", snapName, "2", dirs.CoreLibExecDir, "info") + err = os.MkdirAll(filepath.Dir(infoFile2), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(infoFile2, []byte(fixedInfoFile), 0644) + c.Assert(err, IsNil) + + // revision 11 fixed + infoFile11 := filepath.Join(dirs.GlobalRootDir, "snap", snapName, "11", dirs.CoreLibExecDir, "info") + err = os.MkdirAll(filepath.Dir(infoFile11), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(infoFile11, []byte(fixedInfoFile), 0644) + c.Assert(err, IsNil) + + // use generic classic model + r := snapstatetest.UseFallbackDeviceModel() + defer r() + + st := s.state + st.Lock() + // ensure that only this specific snap is installed + snapstate.Set(s.state, "core", nil) + snapstate.Set(s.state, "snapd", nil) + + snapSt := &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{ + {RealName: snapName, Revision: snap.R(1)}, + {RealName: snapName, Revision: snap.R(2)}, + {RealName: snapName, Revision: snap.R(11)}, + }, + Current: snap.R(11), + SnapType: "os", + } + if snapName == "snapd" { + snapSt.SnapType = "snapd" + } + snapstate.Set(s.state, snapName, snapSt) + st.Unlock() + + // special policy only on classic + r = release.MockOnClassic(true) + defer r() + ensureErr := s.snapmgr.Ensure() + c.Assert(ensureErr, IsNil) + + // we should have created a single remove change for revision 1, revision 2 + // should have been left alone + st.Lock() + defer st.Unlock() + + allChgs := st.Changes() + c.Assert(allChgs, HasLen, 1) + removeChg := allChgs[0] + c.Assert(removeChg.Status(), Equals, state.DoStatus) + c.Assert(removeChg.Kind(), Equals, "remove-snap") + c.Assert(removeChg.Summary(), Equals, fmt.Sprintf(`Remove vulnerable %q snap`, snapName)) + + c.Assert(removeChg.Tasks(), HasLen, 2) + clearSnap := removeChg.Tasks()[0] + discardSnap := removeChg.Tasks()[1] + c.Assert(clearSnap.Kind(), Equals, "clear-snap") + c.Assert(discardSnap.Kind(), Equals, "discard-snap") + var snapsup snapstate.SnapSetup + err = clearSnap.Get("snap-setup", &snapsup) + c.Assert(err, IsNil) + c.Assert(snapsup.SideInfo.Revision, Equals, snap.R(1)) + + // and we set the appropriate key in the state + var removeDone bool + st.Get(snapName+"-snap-cve-2021-44731-vuln-removed", &removeDone) + c.Assert(removeDone, Equals, true) +} + +func (s *snapmgrTestSuite) TestEnsureChecksSnapdInfoFileOnClassicOnly(c *C) { + // delete the core/snapd snap info files - they should always exist in real + // devices, but deleting them here makes it so we can see the failure + // trying to read the files easily + + infoFile := filepath.Join(dirs.GlobalRootDir, "snap", "core", "1", dirs.CoreLibExecDir, "info") + err := os.Remove(infoFile) + c.Assert(err, IsNil) + + // special policy only on classic + r := release.MockOnClassic(true) + defer r() + ensureErr := s.snapmgr.Ensure() + c.Assert(ensureErr, ErrorMatches, "cannot open snapd info file.*") + + // if we are not on classic nothing happens + r = release.MockOnClassic(false) + defer r() + + ensureErr = s.snapmgr.Ensure() + c.Assert(ensureErr, IsNil) +} + +func (s *snapmgrTestSuite) TestEnsureSkipsCheckingSnapdSnapInfoFileWhenStateSet(c *C) { + // we default from SetUp to having the core snap installed, remove it so we + // only have the snapd snap available + s.state.Lock() + snapstate.Set(s.state, "core", nil) + snapstate.Set(s.state, "snapd", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{ + {RealName: "snapd", Revision: snap.R(1)}, + }, + Current: snap.R(1), + SnapType: "snapd", + }) + s.state.Unlock() + + s.testEnsureSkipsCheckingSnapdInfoFileWhenStateSet(c, "snapd") +} + +func (s *snapmgrTestSuite) TestEnsureSkipsCheckingCoreSnapInfoFileWhenStateSet(c *C) { + s.testEnsureSkipsCheckingSnapdInfoFileWhenStateSet(c, "core") +} + +func (s *snapmgrTestSuite) TestEnsureSkipsCheckingBothCoreAndSnapdSnapsInfoFileWhenStateSet(c *C) { + // special policy only on classic + r := release.MockOnClassic(true) + defer r() + + st := s.state + // also set snapd snapd as installed + st.Lock() + snapstate.Set(st, "snapd", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{ + {RealName: "snapd", Revision: snap.R(1)}, + }, + Current: snap.R(1), + SnapType: "snapd", + }) + st.Unlock() + + infoFileFor := func(snapName string) string { + return filepath.Join(dirs.GlobalRootDir, "snap", snapName, "1", dirs.CoreLibExecDir, "info") + } + + // delete both snapd and core snap info files + for _, snapName := range []string{"core", "snapd"} { + err := os.Remove(infoFileFor(snapName)) + c.Assert(err, IsNil) + } + + // make sure Ensure makes a whole hearted attempt to read both files - snapd + // is tried first + ensureErr := s.snapmgr.Ensure() + c.Assert(ensureErr, ErrorMatches, fmt.Sprintf(`cannot open snapd info file "%s".*`, infoFileFor("snapd"))) + + st.Lock() + st.Set("snapd-snap-cve-2021-44731-vuln-removed", true) + st.Unlock() + + // still unhappy about core file missing + ensureErr = s.snapmgr.Ensure() + c.Assert(ensureErr, ErrorMatches, fmt.Sprintf(`cannot open snapd info file "%s".*`, infoFileFor("core"))) + + // but with core state flag set too, we are now happy + st.Lock() + st.Set("core-snap-cve-2021-44731-vuln-removed", true) + st.Unlock() + + ensureErr = s.snapmgr.Ensure() + c.Assert(ensureErr, IsNil) +} + +func (s *snapmgrTestSuite) testEnsureSkipsCheckingSnapdInfoFileWhenStateSet(c *C, snapName string) { + // special policy only on classic + r := release.MockOnClassic(true) + defer r() + + // delete the snap info file for this snap - they should always exist in + // real devices, but deleting them here makes it so we can see the failure + // trying to read the files easily + infoFile := filepath.Join(dirs.GlobalRootDir, "snap", snapName, "1", dirs.CoreLibExecDir, "info") + err := os.Remove(infoFile) + c.Assert(err, IsNil) + + // make sure it makes a whole hearted attempt to read it + ensureErr := s.snapmgr.Ensure() + c.Assert(ensureErr, ErrorMatches, "cannot open snapd info file.*") + + // now it should stop trying to check if state says so + st := s.state + st.Lock() + st.Set(snapName+"-snap-cve-2021-44731-vuln-removed", true) + st.Unlock() + + ensureErr = s.snapmgr.Ensure() + c.Assert(ensureErr, IsNil) +} + func (s *snapmgrTestSuite) TestEnsureRefreshesAtSeedPolicy(c *C) { // special policy only on classic r := release.MockOnClassic(true) diff --git a/packaging/arch/PKGBUILD b/packaging/arch/PKGBUILD index 298a1d81af6..25041e164b0 100644 --- a/packaging/arch/PKGBUILD +++ b/packaging/arch/PKGBUILD @@ -11,7 +11,7 @@ pkgdesc="Service and tools for management of snap packages." depends=('squashfs-tools' 'libseccomp' 'libsystemd' 'apparmor') optdepends=('bash-completion: bash completion support' 'xdg-desktop-portal: desktop integration') -pkgver=2.54.2 +pkgver=2.54.3 pkgrel=1 arch=('x86_64' 'i686' 'armv7h' 'aarch64') url="https://github.com/snapcore/snapd" diff --git a/packaging/debian-sid/changelog b/packaging/debian-sid/changelog index 47667e0a441..6bd72ee2e17 100644 --- a/packaging/debian-sid/changelog +++ b/packaging/debian-sid/changelog @@ -1,3 +1,10 @@ +snapd (2.54.3-1) unstable; urgency=medium + + * New upstream release, LP: #1955137 + - bugfixes + + -- Michael Vogt Tue, 15 Feb 2022 12:55:24 +0100 + snapd (2.54.2-1) unstable; urgency=medium * New upstream release, LP: #1955137 diff --git a/packaging/fedora/snapd.spec b/packaging/fedora/snapd.spec index 7bc3d26b87a..a540c5910f6 100644 --- a/packaging/fedora/snapd.spec +++ b/packaging/fedora/snapd.spec @@ -102,7 +102,7 @@ %endif Name: snapd -Version: 2.54.2 +Version: 2.54.3 Release: 0%{?dist} Summary: A transactional software package manager License: GPLv3 @@ -980,6 +980,10 @@ fi %changelog +* Tue Feb 15 2022 Michael Vogt +- New upstream release 2.54.3 + - bugfixes + * Thu Jan 06 2022 Ian Johnson - New upstream release 2.54.2 - tests: exclude interfaces-kernel-module load on arm diff --git a/packaging/opensuse/snapd.changes b/packaging/opensuse/snapd.changes index b66a8d16b19..3876fa0e576 100644 --- a/packaging/opensuse/snapd.changes +++ b/packaging/opensuse/snapd.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Tue Feb 15 11:55:24 UTC 2022 - michael.vogt@ubuntu.com + +- Update to upstream release 2.54.3 + ------------------------------------------------------------------- Thu Jan 06 21:25:16 UTC 2022 - ian.johnson@canonical.com diff --git a/packaging/opensuse/snapd.spec b/packaging/opensuse/snapd.spec index 9b4e76f921d..66f8b8167e9 100644 --- a/packaging/opensuse/snapd.spec +++ b/packaging/opensuse/snapd.spec @@ -81,7 +81,7 @@ Name: snapd -Version: 2.54.2 +Version: 2.54.3 Release: 0 Summary: Tools enabling systems to work with .snap files License: GPL-3.0 diff --git a/packaging/ubuntu-14.04/changelog b/packaging/ubuntu-14.04/changelog index b2d8c7ff2e7..012ee88c493 100644 --- a/packaging/ubuntu-14.04/changelog +++ b/packaging/ubuntu-14.04/changelog @@ -1,3 +1,10 @@ +snapd (2.54.3~14.04) trusty; urgency=medium + + * New upstream release, LP: #1955137 + - bugfixes + + -- Michael Vogt Tue, 15 Feb 2022 12:55:24 +0100 + snapd (2.54.2~14.04) trusty; urgency=medium * New upstream release, LP: #1955137 diff --git a/packaging/ubuntu-16.04/changelog b/packaging/ubuntu-16.04/changelog index f49cd2bd579..a54ad269266 100644 --- a/packaging/ubuntu-16.04/changelog +++ b/packaging/ubuntu-16.04/changelog @@ -1,3 +1,10 @@ +snapd (2.54.3) xenial; urgency=medium + + * New upstream release, LP: #1955137 + - bugfixes + + -- Michael Vogt Tue, 15 Feb 2022 12:55:24 +0100 + snapd (2.54.2) xenial; urgency=medium * New upstream release, LP: #1955137 diff --git a/sandbox/apparmor/apparmor.go b/sandbox/apparmor/apparmor.go index bd67894aabc..1eed50dbd39 100644 --- a/sandbox/apparmor/apparmor.go +++ b/sandbox/apparmor/apparmor.go @@ -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 diff --git a/sandbox/apparmor/apparmor_test.go b/sandbox/apparmor/apparmor_test.go index 89e8deffcce..40374edaefb 100644 --- a/sandbox/apparmor/apparmor_test.go +++ b/sandbox/apparmor/apparmor_test.go @@ -359,3 +359,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/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 index 00000000000..576eec17ad1 --- /dev/null +++ b/tests/main/snap-confine-unexpected-path/task.yaml @@ -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 index 00000000000..9536e196477 --- /dev/null +++ b/tests/main/snap-run-devmode-classic/task.yaml @@ -0,0 +1,195 @@ +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 < /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 + + # 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 index 00000000000..321c4fe518b --- /dev/null +++ b/tests/nested/manual/core-seeding-devmode/task.yaml @@ -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 index 00000000000..755c84fac48 --- /dev/null +++ b/tests/nested/manual/devmode-snaps-can-run-other-snaps/task.yaml @@ -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 < /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/nested/manual/snapd-removes-vulnerable-snap-confine-revs/task.yaml b/tests/nested/manual/snapd-removes-vulnerable-snap-confine-revs/task.yaml new file mode 100644 index 00000000000..bc7fd05b3bb --- /dev/null +++ b/tests/nested/manual/snapd-removes-vulnerable-snap-confine-revs/task.yaml @@ -0,0 +1,110 @@ +summary: Check that refreshing snapd to a fixed version removes vulnerable revs + +# just focal is fine for this test - we only need to check that things happen on +# classic +systems: [ubuntu-20.04-*] + +environment: + # which snap snapd comes from in this test + SNAPD_SOURCE_SNAP/snapd: snapd + SNAPD_SOURCE_SNAP/core: core + + # needed to get a custom image + NESTED_IMAGE_ID: vuln-$SNAPD_SOURCE_SNAP-auto-removed + + # where we mount the image + IMAGE_MOUNTPOINT: /mnt/cloudimg + + # we don't actually use snapd from the branch in the seed of the VM initially, + # but we have to define this in order to get a custom image, see + # nested_get_image_name in nested.sh for this logic + NESTED_BUILD_SNAPD_FROM_CURRENT: true + + # meh snap-state doesn't have a consistent naming for these snaps, so we can't + # just do "$SNAPD_SOURCE_SNAP-from-deb.snap" + REPACKED_SNAP_NAME/core: core-from-snapd-deb.snap + REPACKED_SNAP_NAME/snapd: snapd-from-deb.snap + + # TODO: use more up to date, but still vulnerable revisions of core and snapd + # for this test to reduce the delta in functionality we end up testing here + + # a specific vulnerable snapd version we can base our image on + VULN_SNAP_REV_URL/snapd: https://storage.googleapis.com/snapd-spread-tests/snaps/snapd_2.49.1_11402.snap + + # a specific vulnerable core version we can base our image on + VULN_SNAP_REV_URL/core: https://storage.googleapis.com/snapd-spread-tests/snaps/core_2.45_9289.snap + +prepare: | + #shellcheck source=tests/lib/preseed.sh + . "$TESTSLIB/preseed.sh" + + # create a VM and mount a cloud image + tests.nested build-image classic + mkdir -p "$IMAGE_MOUNTPOINT" + IMAGE_NAME=$(tests.nested get image-name classic) + mount_ubuntu_image "$(tests.nested get images-path)/$IMAGE_NAME" "$IMAGE_MOUNTPOINT" + + # repack the deb into the snap we want + "$TESTSTOOLS"/snaps-state repack_snapd_deb_into_snap "$SNAPD_SOURCE_SNAP" + + # add the known vulnerable version of snapd into the seed, dangerously + curl -s -o "$SNAPD_SOURCE_SNAP-vuln.snap" "$VULN_SNAP_REV_URL" + + # repack to ensure it is a dangerous revision + unsquashfs -d "$SNAPD_SOURCE_SNAP-vuln" "$SNAPD_SOURCE_SNAP-vuln.snap" + rm "$SNAPD_SOURCE_SNAP-vuln.snap" + snap pack "$SNAPD_SOURCE_SNAP-vuln" --filename="$SNAPD_SOURCE_SNAP.snap" + + # inject the vulnerable snap into the seed + inject_snap_into_seed "$IMAGE_MOUNTPOINT" "$SNAPD_SOURCE_SNAP" + + # undo any preseeding, the images may have been preseeded without our snaps + # so we want to undo that to ensure our snaps are on them + SNAPD_DEBUG=1 /usr/lib/snapd/snap-preseed --reset "$IMAGE_MOUNTPOINT" + + # unmount the image and start the VM + umount_ubuntu_image "$IMAGE_MOUNTPOINT" + tests.nested create-vm classic + +execute: | + # check the current snapd snap is vulnerable + tests.nested exec cat /snap/$SNAPD_SOURCE_SNAP/current/usr/lib/snapd/info | MATCH '^VERSION=2\.4.*' + VULN_REV=$(tests.nested exec "snap list $SNAPD_SOURCE_SNAP" | tail -n +2 | awk '{print $3}') + + # now install our snapd deb from the branch - this is so we know the patched + # snapd is always executing, regardless of which snapd/core snap re-exec + # nonsense is going on + SNAPD_DEB_ARR=( "$SPREAD_PATH"/../snapd_*.deb ) + SNAPD_DEB=${SNAPD_DEB_ARR[0]} + tests.nested copy "$SNAPD_DEB" + tests.nested exec "sudo dpkg -i $(basename "$SNAPD_DEB")" + + # now send the snap version of snapd under test to the VM + tests.nested copy "$REPACKED_SNAP_NAME" + tests.nested exec "sudo snap install $REPACKED_SNAP_NAME --dangerous" + + # there is a race between the snap install finishing and removing the + # vulnerable revision, so we have to wait a bit + VULN_SNAP_REMOVED=false + #shellcheck disable=SC2034 + for i in $(seq 1 60); do + if tests.nested exec "snap list $SNAPD_SOURCE_SNAP --all" | NOMATCH "$VULN_REV"; then + VULN_SNAP_REMOVED=true + break + fi + sleep 1 + done + + if [ "$VULN_SNAP_REMOVED" != "true" ]; then + echo "vulnerable snap was not automatically removed" + exit 1 + fi + + # check that the current revision is not vulnerable + tests.nested exec cat /snap/$SNAPD_SOURCE_SNAP/current/usr/lib/snapd/info | NOMATCH '^VERSION=2\.4.*' + + # and there are no other revisions + if [ "$(tests.nested exec "snap list $SNAPD_SOURCE_SNAP" | tail -n +2 | wc -l)" != "1" ]; then + echo "unexpected extra revision of $SNAPD_SOURCE_SNAP installed" + 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 index 00000000000..4207f28b8a5 --- /dev/null +++ b/tests/regression/lp-1949368/bad-layout/meta/snap.yaml @@ -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 100755 index 00000000000..0f845e07c5a --- /dev/null +++ b/tests/regression/lp-1949368/content-consumer/bin/sh @@ -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 index 00000000000..f651398179b --- /dev/null +++ b/tests/regression/lp-1949368/content-consumer/meta/snap.yaml @@ -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 index 00000000000..46b3a530eb9 --- /dev/null +++ b/tests/regression/lp-1949368/content-provider/meta/snap.yaml @@ -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 index 00000000000..d1c3b17e386 --- /dev/null +++ b/tests/regression/lp-1949368/task.yaml @@ -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'