Merge lp:~kyrofa/snap-confine/create_user_common_data into lp:~snappy-dev/snap-confine/trunk

[Kyle Fazzari]

Create user-specific common snap data directory.

The newest version of Snappy now supports a unversioned ("common") snap data directory for both system-wide and user-specific data. Since the launcher currently creates the other user-specific data, it makes sense to include the user-specific common data as well.

There's a caveat, though. While Snappy supports common data directories, they're as-yet unnamed (i.e. they're not represented by environment variables). They do, however, have a specific path relative to SNAP_DATA and SNAP_USER_DATA, so this PR utilizes that. The launcher will need to be updated again once a name is chosen for these paths.

[lp:~kyrofa/snap-confine/create_user_common_data]

136. By Kyle Fazzari on 2016-05-02

Should use calloc instead of malloc to zero-init string.

[Kyle Fazzari (kyrofa)]

Note the lack of profile changes here. This change seems to be covered under the current profile.

Unmerged revisions

136. By Kyle Fazzari on 2016-05-02

Should use calloc instead of malloc to zero-init string.

135. By Kyle Fazzari on 2016-05-02

Create user-specific common snap data directory.

The newest version of Snappy now supports a unversioned ("common") snap data
directory for both system-wide and user-specific data. Since the launcher
currently creates the other user-specific data, it makes sense to include
the user-specific common data as well.

=== modified file 'src/main.c'
--- src/main.c	2016-04-29 16:58:48 +0000
+++ src/main.c	2016-05-02 17:43:18 +0000
@@ -404,7 +404,7 @@
 	// we'll make a copy of it.
 	char *path_copy = strdup(path);
 	if (path_copy == NULL) {
-		die("failed to create user data directory");
+		die("failed to copy path when creating directory %s", path);
 	}
 	// Open flags to use while we walk the user data path:
 	// - Don't follow symlinks
@@ -421,7 +421,7 @@
 		fd = open("/", open_flags);
 		if (fd < 0) {
 			free(path_copy);
-			die("failed to create user data directory");
+			die("failed to open '/' when creating directory %s", path);
 		}
 	}
 	// strtok_r needs a pointer to keep track of where it is in the string.
@@ -435,7 +435,7 @@
 		if (mkdirat(fd, path_segment, 0755) < 0 && errno != EEXIST) {
 			close(fd);	// we die regardless of return code
 			free(path_copy);
-			die("failed to create user data directory");
+			die("failed to create directory %s", path_segment);
 		}
 		// Open the parent directory we just made (and close the
 		// previous one) so we can continue down the path.
@@ -443,11 +443,11 @@
 		fd = openat(fd, path_segment, open_flags);
 		if (close(previous_fd) != 0) {
 			free(path_copy);
-			die("could not close path segment");
+			die("failed to close previous path segment");
 		}
 		if (fd < 0) {
 			free(path_copy);
-			die("failed to create user data directory");
+			die("failed to open %s", path_segment);
 		}
 		// Obtain the next path segment.
 		path_segment = strtok_r(NULL, "/", &path_walker);
@@ -456,7 +456,7 @@
 	// Close the descriptor for the final directory in the path.
 	if (close(fd) != 0) {
 		free(path_copy);
-		die("could not close final directory");
+		die("failed to close final path segment");
 	}
 
 	free(path_copy);
@@ -476,6 +476,38 @@
 	mkpath(user_data);
 }
 
+void setup_user_common_data()
+{
+	// The common data directory is common across all versions of the
+	// snap. Note that it doesn't have its own variable yet, so it
+	// needs to be relative to SNAP_USER_DATA. FIXME when it has one.
+	const char *user_data = getenv("SNAP_USER_DATA");
+
+	if (user_data == NULL)
+		return;
+	// Only support absolute paths.
+	if (user_data[0] != '/') {
+		die("user common data directory must be an absolute path");
+	}
+
+	const char *common_folder_path = "/../common";
+	const size_t user_data_length = strlen(user_data);
+	const size_t common_folder_path_length = strlen(common_folder_path);
+
+	// +1 for NULL
+	const size_t common_user_length = user_data_length + common_folder_path_length + 1;
+	char *common_user_data = calloc(common_user_length, 1);
+	if (common_user_data == NULL) {
+		die("failed to create user common data directory");
+        }
+
+	strncpy(common_user_data, user_data, user_data_length);
+	strncat(common_user_data, common_folder_path, common_folder_path_length);
+	common_user_data[common_user_length-1] = '\0';
+	mkpath(common_user_data);
+	free(common_user_data);
+}
+
 int main(int argc, char **argv)
 {
 	const int NR_ARGS = 3;
@@ -543,6 +575,9 @@
 	// Ensure that the user data path exists.
 	setup_user_data();
 
+	// Ensure that the user data path common across versions exists.
+	setup_user_common_data();
+
 	// https://wiki.ubuntu.com/SecurityTeam/Specifications/SnappyConfinement
 
 	int rc = 0;

=== modified file 'tests/test_create_user_data'
--- tests/test_create_user_data	2016-04-15 20:20:28 +0000
+++ tests/test_create_user_data	2016-05-02 17:43:18 +0000
@@ -1,7 +1,5 @@
 #!/bin/sh
 
-set -e
-
 . $(pwd)/common.sh
 
 cat >$TMP/unrestricted <<EOF
@@ -43,24 +41,18 @@
    pass=false
    if $L snap.name.app $2 /bin/true 2>/dev/null; then
       if $3; then
-         if [ ! -d "$1" ]; then
-            pass=false
+         if [ -d "$1" ]; then
+            pass=true
          fi
-
-         pass=true
-      else
-         pass=false
       fi
    else
-      if $3; then
-         pass=false
-      else
+      if ! $3; then
          pass=true
       fi
    fi
 
    if [ -d "$1" ]; then
-      rmdir $1
+      rmdir -p $1 --ignore-fail-on-non-empty
    fi
 
    if $pass; then
@@ -70,53 +62,120 @@
    fi
 }
 
+# Check user data directory creation.
+#
 # $1 = $SNAP_USER_DATA definition
 # $2 = Profile name
-# $3 = True if success is expected, false otherwise
-run_current() {
+# $3 = Whether or not success is expected
+check_user_data() {
    export SNAP_USER_DATA=$1
    run_launcher $1 $2 $3
    pass=$?
    unset SNAP_USER_DATA
 
-   return $pass
+   if [ $pass -eq 0 ]; then
+      PASS
+   else
+      FAIL
+   fi
+}
+
+# Check user common data directory creation.
+#
+# $1 = $SNAP_USER_DATA definition
+# $2 = Profile name
+# $3 = Whether or not success is expected
+check_user_common_data() {
+   export SNAP_USER_DATA=$1
+
+   # There's no variable representing this right now, so we specify
+   # relative to $SNAP_USER_DATA. FIXME when this is no longer the
+   # case.
+   run_launcher "$SNAP_USER_DATA/../common" $2 $3
+   pass=$?
+   if [ -d "$SNAP_USER_DATA" ]; then
+      rmdir -p $SNAP_USER_DATA --ignore-fail-on-non-empty
+   fi
+   unset SNAP_USER_DATA
+
+   if [ $pass -eq 0 ]; then
+      PASS
+   else
+      FAIL
+   fi
 }
 
 printf "Test that an unrestricted launcher creates user data"
-run_current $TMP/user_data unrestricted true
+check_user_data $TMP/user_data unrestricted true
+
+printf "Test that an unrestricted launcher creates user common data"
+check_user_common_data $TMP/user_data unrestricted true
+
 
 printf "Test that a restricted launcher creates user data"
-run_current $TMP/user_data restricted true
+check_user_data $TMP/user_data restricted true
+
+printf "Test that a restricted launcher creates user common data"
+check_user_common_data $TMP/user_data restricted true
+
 
 printf "Test that an unrestricted launcher creates user data with parent directory"
-run_current $TMP/parent/user_data unrestricted true
+check_user_data $TMP/parent/user_data unrestricted true
+
+printf "Test that an unrestricted launcher creates user common data with parent directory"
+check_user_common_data $TMP/parent/user_data unrestricted true
+
 
 printf "Test that a restricted launcher creates user data with parent directory"
-run_current $TMP/parent/user_data restricted true
+check_user_data $TMP/parent/user_data restricted true
+
+printf "Test that a restricted launcher creates user common data with parent directory"
+check_user_common_data $TMP/parent/user_data restricted true
+
 
 printf "Test that user data can contain multiple path separators"
-run_current $TMP//user_data unrestricted true
+check_user_data $TMP//user_data unrestricted true
+
+printf "Test that user common data can contain multiple path separators"
+check_user_common_data $TMP//user_data unrestricted true
+
 
 printf "Test that user data must be absolute"
-run_current "../foo" unrestricted false
+check_user_data "../foo" unrestricted false
+
+printf "Test that user common data must be absolute"
+check_user_common_data "../foo" unrestricted false
+
+
+with_symlink() {
+   mkdir $TMP/nefarious_parent
+   ln -s $TMP/nefarious_parent $TMP/parent
+   $@
+   rm $TMP/parent
+   rmdir $TMP/nefarious_parent
+}
 
 printf "Testing that user data cannot be contained within a symlink"
-mkdir $TMP/nefarious_parent
-ln -s $TMP/nefarious_parent $TMP/parent
-run_current $TMP/parent/user_data unrestricted, false
-
-printf "Test that an unrestricted launcher works when user data exists (current)"
-mkdir $TMP/user_data
-if run_current $TMP/user_data unrestricted true; then
-   PASS
-else
-   FAIL
-fi
-
-printf "Test that an restricted launcher works when user data exists (current)"
-mkdir $TMP/user_data
-if run_current $TMP/user_data restricted true; then
-   PASS
-else
-   FAIL
-fi
+with_symlink check_user_data $TMP/parent/user_data unrestricted, false
+
+printf "Testing that user common data cannot be contained within a symlink"
+with_symlink check_user_common_data $TMP/parent/user_data unrestricted, false
+
+
+with_existing() {
+   mkdir $TMP/user_data
+   $@
+}
+
+printf "Test that an unrestricted launcher works when user data exists"
+with_existing check_user_data $TMP/user_data unrestricted true
+
+printf "Test that an unrestricted launcher works when user common data exists"
+with_existing check_user_common_data $TMP/user_data unrestricted true
+
+
+printf "Test that a restricted launcher works when user data exists"
+with_existing check_user_data $TMP/user_data restricted true
+
+printf "Test that a restricted launcher works with user common data exists"
+with_existing check_user_common_data $TMP/user_data restricted true