Merge master.seccomp-test-cleanup

8 files changed, 259 insertions, 20 deletions

diff --git a/cmd/libsnap-confine-private/snap-test.c b/cmd/libsnap-confine-private/snap-test.c
index 4387f119e6..707c16d501 100644
--- a/cmd/libsnap-confine-private/snap-test.c
+++ b/cmd/libsnap-confine-private/snap-test.c
@@ -61,7 +61,124 @@ static void test_verify_security_tag()
	g_assert_false(verify_security_tag("snap.name.app.."));
 }
 
+static void test_sc_snap_name_validate()
+{
+	struct sc_error *err = NULL;
+
+	// Smoke test, a valid snap name
+	sc_snap_name_validate("hello-world", &err);
+	g_assert_null(err);
+
+	// Smoke test: invalid character 
+	sc_snap_name_validate("hello world", &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==,
+	"snap name must use lower case letters, digits or dashes");
+	sc_error_free(err);
+
+	// Smoke test: no letters
+	sc_snap_name_validate("", &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==,
+	"snap name must contain at least one letter");
+	sc_error_free(err);
+
+	// Smoke test: leading dash
+	sc_snap_name_validate("-foo", &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==,
+	"snap name cannot start with a dash");
+	sc_error_free(err);
+
+	// Smoke test: trailing dash
+	sc_snap_name_validate("foo-", &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==,
+	"snap name cannot end with a dash");
+	sc_error_free(err);
+
+	// Smoke test: double dash
+	sc_snap_name_validate("f--oo", &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==,
+	"snap name cannot contain two consecutive dashes");
+	sc_error_free(err);
+
+	// Smoke test: NULL name is not valid
+	sc_snap_name_validate(NULL, &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	g_assert_cmpstr(sc_error_msg(err), ==, "snap name cannot be NULL");
+	sc_error_free(err);
+
+	const char *valid_names[] = {
+	"a", "aa", "aaa", "aaaa",
+	"a-a", "aa-a", "a-aa", "a-b-c",
+	"a0", "a-0", "a-0a",
+	"01game", "1-or-2"
+	};
+	for (int i = 0; i < sizeof valid_names / sizeof *valid_names; ++i) {
+	g_test_message("checking valid snap name: %s", valid_names[i]);
+	sc_snap_name_validate(valid_names[i], &err);
+	g_assert_null(err);
+	}
+	const char *invalid_names[] = {
+	// name cannot be empty
+	"",
+	// dashes alone are not a name
+	"-", "--",
+	// double dashes in a name are not allowed
+	"a--a",
+	// name should not end with a dash
+	"a-",
+	// name cannot have any spaces in it
+	"a ", " a", "a a",
+	// a number alone is not a name
+	"0", "123",
+	// identifier must be plain ASCII
+	"日本語", "한글", "ру́сский язы́к",
+	};
+	for (int i = 0; i < sizeof invalid_names / sizeof *invalid_names; ++i) {
+	g_test_message("checking invalid snap name: >%s<",
+ invalid_names[i]);
+	sc_snap_name_validate(invalid_names[i], &err);
+	g_assert_nonnull(err);
+	g_assert_true(sc_error_match
+ (err, SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME));
+	sc_error_free(err);
+	}
+}
+
+static void test_sc_snap_name_validate__respects_error_protocol()
+{
+	if (g_test_subprocess()) {
+	sc_snap_name_validate("hello world", NULL);
+	g_test_message("expected sc_snap_name_validate to return");
+	g_test_fail();
+	return;
+	}
+	g_test_trap_subprocess(NULL, 0, 0);
+	g_test_trap_assert_failed();
+	g_test_trap_assert_stderr
+ ("snap name must use lower case letters, digits or dashes\n");
+}
+
 static void __attribute__ ((constructor)) init()
 {
	g_test_add_func("/snap/verify_security_tag", test_verify_security_tag);
+	g_test_add_func("/snap/sc_snap_name_validate",
+	test_sc_snap_name_validate);
+	g_test_add_func("/snap/sc_snap_name_validate/respects_error_protocol",
+	test_sc_snap_name_validate__respects_error_protocol);
 }
diff --git a/cmd/libsnap-confine-private/snap.c b/cmd/libsnap-confine-private/snap.c
index 7059713b53..418b26435b 100644
--- a/cmd/libsnap-confine-private/snap.c
+++ b/cmd/libsnap-confine-private/snap.c
@@ -17,11 +17,15 @@
 #include "config.h"
 #include "snap.h"
 
+#include <errno.h>
+#include <regex.h>
 #include <stddef.h>
 #include <stdlib.h>
-#include <regex.h>
+#include <string.h>
 
-#include "../libsnap-confine-private/utils.h"
+#include "utils.h"
+#include "string-utils.h"
+#include "cleanup-funcs.h"
 
 bool verify_security_tag(const char *security_tag)
 {
@@ -43,3 +47,94 @@ bool verify_security_tag(const char *security_tag)
 
	return (status == 0);
 }
+
+static int skip_lowercase_letters(const char **p)
+{
+	int skipped = 0;
+	for (const char *c = *p; *c >= 'a' && *c <= 'z'; ++c) {
+	skipped += 1;
+	}
+	*p = (*p) + skipped;
+	return skipped;
+}
+
+static int skip_digits(const char **p)
+{
+	int skipped = 0;
+	for (const char *c = *p; *c >= '0' && *c <= '9'; ++c) {
+	skipped += 1;
+	}
+	*p = (*p) + skipped;
+	return skipped;
+}
+
+static int skip_one_char(const char **p, char c)
+{
+	if (**p == c) {
+	*p += 1;
+	return 1;
+	}
+	return 0;
+}
+
+void sc_snap_name_validate(const char *snap_name, struct sc_error **errorp)
+{
+	struct sc_error *err = NULL;
+
+	// Ensure that name is not NULL
+	if (snap_name == NULL) {
+	err = sc_error_init(SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME,
+ "snap name cannot be NULL");
+	goto out;
+	}
+	// This is a regexp-free routine hand-codes the following pattern:
+	//
+	// "^([a-z0-9]+-?)*[a-z](-?[a-z0-9])*$"
+	//
+	// The only motivation for not using regular expressions is so that we
+	// don't run untrusted input against a potentially complex regular
+	// expression engine.
+	const char *p = snap_name;
+	if (skip_one_char(&p, '-')) {
+	err = sc_error_init(SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME,
+ "snap name cannot start with a dash");
+	goto out;
+	}
+	bool got_letter = false;
+	for (; *p != '\0';) {
+	if (skip_lowercase_letters(&p) > 0) {
+	got_letter = true;
+	continue;
+	}
+	if (skip_digits(&p) > 0) {
+	continue;
+	}
+	if (skip_one_char(&p, '-') > 0) {
+	if (*p == '\0') {
+	err =
+ sc_error_init(SC_SNAP_DOMAIN,
+ SC_SNAP_INVALID_NAME,
+ "snap name cannot end with a dash");
+	goto out;
+	}
+	if (skip_one_char(&p, '-') > 0) {
+	err =
+ sc_error_init(SC_SNAP_DOMAIN,
+ SC_SNAP_INVALID_NAME,
+ "snap name cannot contain two consecutive dashes");
+	goto out;
+	}
+	continue;
+	}
+	err = sc_error_init(SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME,
+ "snap name must use lower case letters, digits or dashes");
+	goto out;
+	}
+	if (!got_letter) {
+	err = sc_error_init(SC_SNAP_DOMAIN, SC_SNAP_INVALID_NAME,
+ "snap name must contain at least one letter");
+	}
+
+ out:
+	sc_error_forward(errorp, err);
+}
diff --git a/cmd/libsnap-confine-private/snap.h b/cmd/libsnap-confine-private/snap.h
index 2876258e56..7e5fe29ae8 100644
--- a/cmd/libsnap-confine-private/snap.h
+++ b/cmd/libsnap-confine-private/snap.h
@@ -20,6 +20,29 @@
 
 #include <stdbool.h>
 
+#include "error.h"
+
+/**
+ * Error domain for errors related to the snap module.
+ **/
+#define SC_SNAP_DOMAIN "snap"
+
+enum {
+	/** The name of the snap is not valid. */
+	SC_SNAP_INVALID_NAME = 1,
+};
+
+/**
+ * Validate the given snap name.
+ *
+ * Valid name cannot be NULL and must match a regular expression describing the
+ * strict naming requirements. Please refer to snapd source code for details.
+ *
+ * The error protocol is observed so if the caller doesn't provide an outgoing
+ * error pointer the function will die on any error.
+ **/
+void sc_snap_name_validate(const char *snap_name, struct sc_error **errorp);
+
 bool verify_security_tag(const char *security_tag);
 
 #endif
diff --git a/cmd/snap-confine/snap-confine.c b/cmd/snap-confine/snap-confine.c
index 0a32b2ddcb..c22ec2f657 100644
--- a/cmd/snap-confine/snap-confine.c
+++ b/cmd/snap-confine/snap-confine.c
@@ -81,6 +81,9 @@ int main(int argc, char **argv)
	if (!verify_security_tag(security_tag))
	die("security tag %s not allowed", security_tag);
 
+	const char *snap_name = getenv("SNAP_NAME");
+	sc_snap_name_validate(snap_name, NULL);
+
 #ifndef CAPS_OVER_SETUID
	// this code always needs to run as root for the cgroup/udev setup,
	// however for the tests we allow it to run as non-root
@@ -120,7 +123,6 @@ int main(int argc, char **argv)
	debug
 ("skipping sandbox setup, classic confinement in use");
	} else {
-	const char *snap_name = getenv("SNAP_NAME");
	const char *group_name = snap_name;
	if (group_name == NULL) {
	die("SNAP_NAME is not set");
diff --git a/cmd/snap-confine/tests/common.sh b/cmd/snap-confine/tests/common.sh
index 8ffcae65db..dceed5718c 100644
--- a/cmd/snap-confine/tests/common.sh
+++ b/cmd/snap-confine/tests/common.sh
@@ -47,7 +47,7 @@ trap 'rm -rf $TMP' EXIT
 export SNAPPY_LAUNCHER_SECCOMP_PROFILE_DIR="$TMP"
 export SNAPPY_LAUNCHER_INSIDE_TESTS="1"
 export SNAP_CONFINE_NO_ROOT=1
-export SNAP_NAME=name.app
+export SNAP_NAME=name
 
 FAIL() {
 printf ": FAIL\n"
diff --git a/interfaces/backends/backends.go b/interfaces/backends/backends.go
index 44a981409f..1d60ebd6ae 100644
--- a/interfaces/backends/backends.go
+++ b/interfaces/backends/backends.go
@@ -28,6 +28,7 @@ import (
	"github.com/snapcore/snapd/interfaces/seccomp"
	"github.com/snapcore/snapd/interfaces/systemd"
	"github.com/snapcore/snapd/interfaces/udev"
+	"github.com/snapcore/snapd/logger"
	"github.com/snapcore/snapd/release"
 )
 
@@ -42,7 +43,9 @@ var All = []interfaces.SecurityBackend{
 }
 
 func init() {
-	if !release.ReleaseInfo.ForceDevMode() {
+	if release.ReleaseInfo.ForceDevMode() {
+	logger.Noticef("Cannot find sufficient apparmor support, disabling apparmor security backend")
+	} else {
	All = append(All, &apparmor.Backend{})
	}
 }
diff --git a/spread.yaml b/spread.yaml
index 22461cd908..aeb03cf03e 100644
--- a/spread.yaml
+++ b/spread.yaml
@@ -172,6 +172,10 @@ exclude:
 - "*.a"
 
 prepare-each: |
+ if journalctl |grep "Cannot find sufficient apparmor support"; then
+ echo "Insufficient apparmor supported detect, this should never happen"
+ exit 1
+ fi
 # systemd on 14.04 does not know about --rotate
 # or --vacuum-time.
 # TODO: Find a way to clean out systemd logs on
diff --git a/tests/main/classic-ubuntu-core-transition/task.yaml b/tests/main/classic-ubuntu-core-transition/task.yaml
index d0305d029c..92da7b3b09 100644
--- a/tests/main/classic-ubuntu-core-transition/task.yaml
+++ b/tests/main/classic-ubuntu-core-transition/task.yaml
@@ -8,7 +8,7 @@ warn-timeout: 1m
 kill-timeout: 5m
 
 restore: |
- rm -f prevValue state.json.copy
+ rm -f state.json.new
 
 execute: |
 wait_for_service() {
@@ -35,12 +35,11 @@ execute: |
 # modify daemon state to set ubuntu-core-transition-last-retry-time to the
 # current time to prevent the ubuntu-core transition before the test snap is
 # installed
- cat /var/lib/snapd/state.json | jq '.["ubuntu-core-transition-last-retry-time"]' > prevValue
- systemctl stop snapd.service snapd.socket
- cp /var/lib/snapd/state.json state.json.copy
- now=$(date --utc --rfc-3339=ns | tr ' ' T)
- cat state.json.copy | jq -c '. + {"ubuntu-core-transition-last-retry-time": "'"$now"'"}' > /var/lib/snapd/state.json
- systemctl start snapd.service snapd.socket
+ systemctl stop snapd.{service,socket}
+ now=$(date --utc -Ins)
+ cat /var/lib/snapd/state.json | jq -c '. + {data: (.data + {"ubuntu-core-transition-last-retry-time": "'"$now"'"})}' > state.json.new
+ mv state.json.new /var/lib/snapd/state.json
+ systemctl start snapd.{service,socket}
 
 snap install --${CORE_CHANNEL} ubuntu-core
 snap install test-snapd-python-webserver
@@ -51,14 +50,10 @@ execute: |
 curl http://localhost | MATCH "XKCD rocks"
 
 # restore ubuntu-core-transition-last-retry-time to its previous value and restart the daemon
- systemctl stop snapd.service snapd.socket
- cp /var/lib/snapd/state.json state.json.copy
- if [ "$(cat prevValue)" = null ]; then
- cat state.json.copy | jq -c 'del(.["ubuntu-core-transition-last-retry-time"])' > /var/lib/snapd/state.json
- else
- cat state.json.copy | jq -c '. + {"ubuntu-core-transition-last-retry-time": "'"$(cat prevValue)"'"}' > /var/lib/snapd/state.json
- fi
- systemctl start snapd.service snapd.socket
+ systemctl stop snapd.{service,socket}
+ cat /var/lib/snapd/state.json | jq -c 'del(.["data"]["ubuntu-core-transition-last-retry-time"])' > state.json.new
+ mv state.json.new /var/lib/snapd/state.json
+ systemctl start snapd.{service,socket}
 
 echo "Ensure transition is triggered"
 snap debug ensure-state-soon