Add support for bind profiles #43

Closed
wants to merge 10 commits into
from
@@ -69,6 +69,9 @@
# reading seccomp filters
/{tmp/snap.rootfs_*/,}var/lib/snapd/seccomp/profiles/* r,
+
+ # reading mount profiles
+ /{tmp/snap.rootfs_*/,}var/lib/snapd/mount/profiles/* r,
@zyga

zyga Jun 24, 2016

Collaborator

The profiles/ component was removed from the snapd side. Please update this here.

# set up snap-specific private /tmp dir
capability chown,
@@ -0,0 +1,10 @@
+summary: Check that basic install works
+restore: |
+ snap remove hello-world
+execute: |
+ echo Run some hello-world stuff
+ snap install hello-world
+ hello-world.echo | grep Hello
+ hello-world.env | grep SNAP_NAME=hello-world
+ echo Ensure that we get an error if hello-world.evil does not return an error
+ if hello-world.evil; then exit 1; fi
View
@@ -0,0 +1,38 @@
+project: snap-confine
@zyga

zyga Jun 21, 2016

Collaborator

Thank you for doing this :)

+
+environment:
+ REUSE_PROJECT: $(echo $REUSE_PROJECT)
+ PATH: /snap/bin:$PATH
+
+backends:
+ linode:
+ key: $(echo $SPREAD_LINODE_KEY)
+ systems:
+ - ubuntu-16.04-64-grub
+ - ubuntu-16.04-32-grub
+
+path: /spread/snap-confine
+
+exclude:
+ - .git
+
+prepare: |
+ [ "$REUSE_PROJECT" != 1 ] || exit 0
+
@zyga

zyga Jun 21, 2016

Collaborator

How can we differentiate prepare tasks for different distributions?

@mvo5

mvo5 Jun 21, 2016

Contributor

I don't know, sorry, I think it will have to be something like if grep ubuntu /etc/os-release and similar.

+ apt purge -y snap-confine || true
+ apt update
+ apt install -y snapd fakeroot
+ apt build-dep -y ./
+
+ test -d /home/test || adduser --quiet --disabled-password --gecos '' test
+ chown test.test -R /home/test /spread/
+ sudo -i -u test /bin/sh -c "cd $PWD && DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -tc -b -Zgzip"
+ apt install -y ../ubuntu-core-launcher_*.deb ../snap-confine_*.deb
+ rm -f ../snap-confine_*.deb
+
+suites:
+ spread-tests/:
+ summary: Full-system tests for snap-confinue
+ prepare: |
+ echo Ensure ubuntu-core is installed
+ sudo snap install ubuntu-core
View
@@ -90,6 +90,9 @@ int main(int argc, char **argv)
if (is_running_on_classic_distribution()) {
setup_snappy_os_mounts();
}
+ // setup the security backend bind mounts
+ setup_bind_mounts(appname);
+
#ifdef STRICT_CONFINEMENT
// set up private mounts
setup_private_mount(appname);
View
@@ -30,6 +30,7 @@
#include <errno.h>
#include <sched.h>
#include <string.h>
+#include <mntent.h>
#include "utils.h"
#include "snap.h"
@@ -293,3 +294,50 @@ void setup_slave_mount_namespace()
die("can not make make / rslave");
}
}
+
+void setup_bind_mounts(const char *appname)
+{
+ debug("%s: %s", __FUNCTION__, appname);
+
+ FILE *f = NULL;
+ const char *bind_profile_dir = "/var/lib/snapd/bind/profiles/";
@zyga

zyga Jun 24, 2016

Collaborator

This is stale, the correct value is /var/lib/snapd/mount/

+
+ char profile_path[PATH_MAX];
+ must_snprintf(profile_path, sizeof(profile_path), "%s/%s.bind",
@zyga

zyga Jun 24, 2016

Collaborator

The correct extension is .fstab

+ bind_profile_dir, appname);
+
+ f = fopen(profile_path, "r");
@zyga

zyga Jun 24, 2016

Collaborator

Can you please use __attribute__((cleanup(sc_cleanup_file))) and add sc_close_file to cleanup-funcs.[ch] with something like this inside:

void sc_cleanup_file(f **FILE) {
    if (*f != NULL) {
        fclose(*f);
    }
}

This should simplify the logic below.

+ // it is ok for the file to not exist
+ if (f == NULL && errno == ENOENT)
+ return;
+ // however any other error is a real error
+ if (f == NULL) {
+ die("cannot open %s", profile_path);
+ }
+
+ struct mntent *m = NULL;
+ while ((m = getmntent(f)) != NULL) {
+ int flags = MS_BIND | MS_RDONLY | MS_NODEV | MS_NOSUID;
+
+ if (strcmp(m->mnt_type, "none") != 0) {
+ die("only 'none' filesystemtype is supported");
+ }
+ if (hasmntopt(m, "bind") == NULL) {
+ die("need bind mount flag");
+ }
+ if (hasmntopt(m, "rw") != NULL) {
+ flags &= ~MS_RDONLY;
+ }
+
+ if (mount(m->mnt_fsname, m->mnt_dir, NULL, flags, NULL) != 0) {
+ die("unable to bind private /tmp");
+ }
+ }
+
@zyga

zyga Jun 21, 2016

Collaborator

I'd just fclose(...) it as it should be harmless

@mvo5

mvo5 Jun 21, 2016

Contributor

I took that from the seccomp code from jamie, I agree that it should be fine to just fclose()

+ if (f != NULL) {
+ if (fclose(f) != 0)
+ die("could not close bind mount file");
+ }
+
+ return;
+}
View
@@ -23,4 +23,6 @@ void setup_private_pts();
void setup_snappy_os_mounts();
void setup_slave_mount_namespace();
+void setup_bind_mounts(const char *appname);
@zyga

zyga Jun 24, 2016

Collaborator

I'd prefer this to be the security tag, the app name is not so true here AFAIK

+
#endif
View
@@ -150,12 +150,8 @@ void seccomp_load_filters(const char *filter_profile)
secure_getenv("SNAPPY_LAUNCHER_SECCOMP_PROFILE_DIR");
char profile_path[512]; // arbitrary path name limit
- int snprintf_rc = snprintf(profile_path, sizeof(profile_path), "%s/%s",
- filter_profile_dir, filter_profile);
- if (snprintf_rc < 0 || snprintf_rc >= 512) {
- errno = 0;
- die("snprintf returned unexpected value");
- }
+ must_snprintf(profile_path, sizeof(profile_path), "%s/%s",
+ filter_profile_dir, filter_profile);
f = fopen(profile_path, "r");
if (f == NULL) {