Merge lp:~tyhicks/ecryptfs/test-with-xattr-metadata into lp:ecryptfs

[Tyler Hicks]

These patches contain updates to the tests that allow the tests to account for the ecryptfs_xattr_metadata option. Unfortunately, lp-872905.sh still fails on top of ext3. I'll have to look more into that failure.

[lp:~tyhicks/ecryptfs/test-with-xattr-metadata]

835. By Tyler Hicks on 2014-07-14

Update the xattr test to do the right thing with or without the
ecryptfs_xattr_metadata mount option.

836. By Tyler Hicks on 2014-07-14

Update the lp-872905 test to do the right thing with or without the
ecryptfs_xattr_metadata mount option.

[Tyler Hicks (tyhicks)]

I've pushed another commit, r836, which fixes up the lp-872905.sh test to work with the ecryptfs_xattr_metadata mount option.

[Colin Ian King (colin-king)]

This looks fine to me.

review: Approve

=== modified file 'tests/kernel/lp-509180.sh'
--- tests/kernel/lp-509180.sh	2012-11-02 23:20:18 +0000
+++ tests/kernel/lp-509180.sh	2014-07-14 07:15:25 +0000
@@ -22,6 +22,7 @@
 test_script_dir=$(dirname $0)
 rc=1
 test_dir=""
+xattr_opt=""
 
 . ${test_script_dir}/../lib/etl_funcs.sh
 
@@ -48,14 +49,18 @@
 old_sum=`md5sum $test_dir/test_file | cut -d ' ' -f 1`
 lower_file=`ls $ETL_MOUNT_SRC/ECRYPTFS*/*`
 
+if etl_is_mount_opt_set "ecryptfs_xattr_metadata" ; then
+	xattr_opt="-x"
+fi
+
 # Increment 9th byte so that eCryptfs marker fails validation
-${test_script_dir}/lp-509180/test -i $lower_file || exit
+${test_script_dir}/lp-509180/test -i $xattr_opt $lower_file || exit
 etl_umount
 
 etl_mount_i || exit
 cat $test_dir/test_file &> /dev/null
 # Decrement 9th byte so that eCryptfs marker passes validation
-${test_script_dir}/lp-509180/test -d $lower_file || exit
+${test_script_dir}/lp-509180/test -d $xattr_opt $lower_file || exit
 new_sum=`md5sum $test_dir/test_file | cut -d ' ' -f 1`
 
 # md5sums should be the same

=== modified file 'tests/kernel/lp-509180/test.c'
--- tests/kernel/lp-509180/test.c	2013-10-27 16:52:03 +0000
+++ tests/kernel/lp-509180/test.c	2014-07-14 07:15:25 +0000
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <attr/xattr.h>
 
 #define TEST_ERROR	(2)
 
@@ -33,11 +34,98 @@
 
 #define OPT_INC		(0x0001)
 #define OPT_DEC		(0x0002)
+#define OPT_XATTR	(0x0004)
 
 void usage(char *name)
 {
-	fprintf(stderr, "Usage: [-i | -d] file\n");
-}
+	fprintf(stderr, "Usage: [-i | -d] [-x] file\n");
+}
+
+static int test_with_metadata_in_header(int fd, int flags)
+{
+	unsigned char buffer[1];
+
+	if ((lseek(fd, (off_t)OFFSET, SEEK_SET)) < 0) {
+		fprintf(stderr, "Cannot lseek to offset %d: %s\n",
+			OFFSET, strerror(errno));
+		return TEST_ERROR;
+	}
+
+	if (read(fd, buffer, sizeof(buffer)) != sizeof(buffer)) {
+		fprintf(stderr, "Failed to read\n");
+		return TEST_ERROR;
+	}
+
+	if (flags & OPT_INC)
+		buffer[0]++;
+
+	if (flags & OPT_DEC)
+		buffer[0]--;
+
+	if ((lseek(fd, (off_t)OFFSET, SEEK_SET)) < 0) {
+		fprintf(stderr, "Cannot lseek to offset %d: %s\n",
+			OFFSET, strerror(errno));
+		return TEST_ERROR;
+	}
+
+	if (write(fd, buffer, sizeof(buffer)) != sizeof(buffer)) {
+		fprintf(stderr, "Failed to write\n");
+		return TEST_ERROR;
+	}
+
+	return 0;
+}
+
+static int test_with_metadata_in_xattr(int fd, int flags)
+{
+	const char *name = "user.ecryptfs";
+	unsigned char *value = NULL;
+	size_t nread, size = 0;
+	int rc = TEST_ERROR;
+
+	size = fgetxattr(fd, name, value, size);
+	if (size < 0) {
+		fprintf(stderr, "Cannot retrieve xattr size: %s\n",
+			strerror(errno));
+		goto out;
+	}
+
+	value = malloc(size);
+	if (!value) {
+		fprintf(stderr,
+			"Cannot allocate memory to store the xattr value\n");
+		goto out;
+	}
+
+	nread = fgetxattr(fd, name, value, size);
+	if (nread != size) {
+		if (nread < 0)
+			fprintf(stderr, "Cannot read xattr: %s\n",
+				strerror(errno));
+		else
+			fprintf(stderr, "Partial xattr read: %zu < %zu\n",
+				nread, size);
+		goto out;
+	}
+
+	if (flags & OPT_INC)
+		value[OFFSET]++;
+
+	if (flags & OPT_DEC)
+		value[OFFSET]--;
+
+	if (fsetxattr(fd, name, value, size, XATTR_REPLACE) < 0) {
+		fprintf(stderr, "Cannot write xattr: %s\n", strerror(errno));
+		goto out;
+	}
+
+	rc = 0;
+out:
+	free(value);
+
+	return rc;
+}
+
 
 /*
  *  https://bugs.launchpad.net/ecryptfs/+bug/509180
@@ -49,14 +137,13 @@
 	int opt, flags = 0;
 	int rc = 0;
 	char *file;
-	unsigned char buffer[1];
 
 	if (argc < 3) {
 		usage(argv[0]);
 		exit(TEST_ERROR);
 	}
 
-	while ((opt = getopt(argc, argv, "id")) != -1) {
+	while ((opt = getopt(argc, argv, "idx")) != -1) {
 		switch (opt) {
 		case 'i':
 			flags |= OPT_INC;
@@ -64,6 +151,9 @@
 		case 'd':
 			flags |= OPT_DEC;
 			break;
+		case 'x':
+			flags |= OPT_XATTR;
+			break;
 		default:
 			usage(argv[0]);
 			exit(TEST_ERROR);
@@ -82,36 +172,10 @@
 		exit(TEST_ERROR);
 	}
 
-	if ((lseek(fd, (off_t)OFFSET, SEEK_SET)) < 0) {
-		fprintf(stderr, "Cannot lseek to offset %d in %s : %s\n",
-			OFFSET, file, strerror(errno));
-		rc = TEST_ERROR;
-		goto tidy;
-	}
-
-	if (read(fd, buffer, sizeof(buffer)) != sizeof(buffer)) {
-		fprintf(stderr, "Failed to read\n");
-		rc = TEST_ERROR;
-		goto tidy;
-	}
-
-	if (flags & OPT_INC)
-		buffer[0]++;
-
-	if (flags & OPT_DEC)
-		buffer[0]--;
-
-	if ((lseek(fd, (off_t)OFFSET, SEEK_SET)) < 0) {
-		fprintf(stderr, "Cannot lseek to offset %d in %s : %s\n",
-			OFFSET, file, strerror(errno));
-		rc = TEST_ERROR;
-		goto tidy;
-	}
-
-	if (write(fd, buffer, sizeof(buffer)) != sizeof(buffer)) {
-		fprintf(stderr, "Failed to write\n");
-		rc = TEST_ERROR;
-	}
+	if (flags & OPT_XATTR)
+		rc = test_with_metadata_in_xattr(fd, flags);
+	else
+		rc = test_with_metadata_in_header(fd, flags);
 
 tidy:
 	if (close(fd) < 0) {

=== modified file 'tests/kernel/lp-872905.sh'
--- tests/kernel/lp-872905.sh	2012-11-02 23:20:18 +0000
+++ tests/kernel/lp-872905.sh	2014-07-14 07:15:25 +0000
@@ -66,11 +66,13 @@
 fi
 
 #
-# We shouldn't have a lower file created of zero bytes size if
-# the bug is fixed
+# If xattr metadata is not enabled, we shouldn't have a zero length lower file.
+# If xattr metadata is enabled, the expected lower file size is 0.
 # 
 sz=$(stat -c%s $lower_test_file)
-if [ $sz -ne 0 ]; then
+if ! etl_is_mount_opt_set "ecryptfs_xattr_metadata" && [ $sz -ne 0 ]; then
+	rc=0
+elif etl_is_mount_opt_set "ecryptfs_xattr_metadata" && [ $sz -eq 0 ]; then
 	rc=0
 fi
 

=== modified file 'tests/kernel/xattr.sh'
--- tests/kernel/xattr.sh	2013-01-24 17:33:42 +0000
+++ tests/kernel/xattr.sh	2014-07-14 07:15:25 +0000
@@ -23,6 +23,7 @@
 test_script_dir=$(dirname $0)
 rc=1
 test_dir=0
+xattr_opt=""
 
 . ${test_script_dir}/../lib/etl_funcs.sh
 
@@ -45,7 +46,11 @@
 
 echo "testing 1 2 3" > $test_file
 
-${test_script_dir}/xattr/test $test_file || exit
+if etl_is_mount_opt_set "ecryptfs_xattr_metadata" ; then
+	xattr_opt="-x"
+fi
+
+${test_script_dir}/xattr/test $xattr_opt $test_file || exit
 rc=$?
 
 rm -f $test_file1

=== modified file 'tests/kernel/xattr/test.c'
--- tests/kernel/xattr/test.c	2013-01-24 17:33:42 +0000
+++ tests/kernel/xattr/test.c	2014-07-14 07:15:25 +0000
@@ -26,6 +26,8 @@
 #include <errno.h>
 #include <sys/xattr.h>
 
+#define XATTR_METADATA_NAME	"user.ecryptfs"
+
 static const char *names[]  = {
 	"user.test1",
 	"user.test2",
@@ -40,20 +42,42 @@
 	NULL
 };
 
+static void usage(const char *name)
+{
+	fprintf(stderr, "Usage: %s [-x] file\n", name);
+}
+
 int main(int argc, char **argv)
 {
 	ssize_t len, names_len = 0;
-	int i, rc;
+	int i, rc, xattr_metadata = 0;
 	char buffer[1024];
-	char *ptr = buffer;
+	char *file, *ptr = buffer;
 
-	if (argc != 2) {
-		fprintf(stderr, "Usage: %s file\n", argv[0]);
+	if (argc < 2 || argc > 3) {
+		usage(argv[0]);
 		exit(EXIT_FAILURE);
 	}
 
+	file = argv[1];
+
+	if (argc == 3) {
+		if (strcmp(argv[1], "-x")) {
+			usage(argv[0]);
+			exit(EXIT_FAILURE);
+		}
+		file = argv[2];
+
+		/*
+		 * The XATTR_METADATA_NAME xattr is set. Account for it in
+		 * future sanity checks.
+		 */
+		xattr_metadata = 1;
+		names_len = 1 + strlen(XATTR_METADATA_NAME);
+	}
+
 	for (i = 0; names[i]; i++) {
-		if (setxattr(argv[1], names[i], values[i], strlen(values[i]), 0) < 0)
+		if (setxattr(file, names[i], values[i], strlen(values[i]), 0) < 0)
 			exit(EXIT_FAILURE);
 		names_len += 1 + strlen(names[i]);
 	}
@@ -61,28 +85,40 @@
 	/*
 	 *  Sanity check that listxattr returns correct length
 	 */
-	len = listxattr(argv[1], NULL, 0);
+	len = listxattr(file, NULL, 0);
+	if (len != names_len || len > sizeof(buffer))
+		exit(EXIT_FAILURE);
+
+	len = listxattr(file, buffer, sizeof(buffer));
 	if (len != names_len)
 		exit(EXIT_FAILURE);
 
-	len = listxattr(argv[1], buffer, sizeof(buffer));
-	if (len < 0)
-		exit(EXIT_FAILURE);
-
 	/*  
 	 *  Check listxattr names match what has been just set
 	 */
-	for (i = 0; names[i]; i++) {
-		if (strcmp(names[i], ptr))
+	for (ptr = buffer; *ptr; ptr += strlen(ptr) + 1) {
+		int matched = 0;
+
+		if (xattr_metadata && !strcmp(XATTR_METADATA_NAME, ptr))
+			continue;
+
+		for (i = 0; names[i]; i++) {
+			if (strcmp(names[i], ptr))
+				continue;
+
+			matched = 1;
+			break;
+		}
+
+		if (!matched)
 			exit(EXIT_FAILURE);
-		ptr += strlen(ptr) + 1;
 	}
 
 	/*
 	 *  Check contents of xattr
 	 */
 	for (i = 0; names[i]; i++) {
-		len = getxattr(argv[1], names[i], buffer, sizeof(buffer));
+		len = getxattr(file, names[i], buffer, sizeof(buffer));
 		if (len < 0)
 			exit(EXIT_FAILURE);
 		buffer[len] = '\0';
@@ -95,16 +131,18 @@
 	 *  Remove xattr
 	 */
 	for (i = 0; names[i]; i++) {
-		rc = removexattr(argv[1], names[i]);
+		rc = removexattr(file, names[i]);
 		if (rc < 0)
 			exit(EXIT_FAILURE);
+		names_len -= 1 + strlen(names[i]);
 	}
 
 	/*
-	 *  ..and there should be no xattrs left
+	 *  ..and the only xattrs that should be left are those that were
+	 *  already there when the test started
 	 */
-	len = listxattr(argv[1], NULL, 0);
-	if (len != 0)
+	len = listxattr(file, NULL, 0);
+	if (len != names_len)
 		exit(EXIT_FAILURE);
 		
 	exit(EXIT_SUCCESS);

=== modified file 'tests/lib/etl_funcs.sh'
--- tests/lib/etl_funcs.sh	2014-07-14 07:15:25 +0000
+++ tests/lib/etl_funcs.sh	2014-07-14 07:15:25 +0000
@@ -447,6 +447,38 @@
 	echo $blks
 }
 
+_etl_init_mount_opts()
+{
+	if [ -z "$ETL_MOUNT_OPTS" ]; then
+		opts=$default_mount_opts
+
+		if [ -n "$ETL_FNEK_SIG" ]; then
+			opts="$default_fne_mount_opts"
+		fi
+
+		if [ -n "$ETL_APPENDED_MOUNT_OPTS" ]; then
+			opts="${opts},${ETL_APPENDED_MOUNT_OPTS}"
+		fi
+
+		export ETL_MOUNT_OPTS=$(eval "echo $opts")
+	fi
+}
+
+etl_is_mount_opt_set()
+{
+	if [ -z "$1" ]; then
+		return 1
+	fi
+
+	_etl_init_mount_opts
+
+	if [[ ! $ETL_MOUNT_OPTS =~ (^|,)$1($|,) ]]; then
+		return 1
+	fi
+
+	return 0
+}
+
 #
 # etl_mount_i
 #
@@ -462,19 +494,7 @@
 		return 1
 	fi
 
-	if [ -z "$ETL_MOUNT_OPTS" ]; then
-		opts=$default_mount_opts
-
-		if [ -n "$ETL_FNEK_SIG" ]; then
-			opts="$default_fne_mount_opts"
-		fi
-
-		if [ -n "$ETL_APPENDED_MOUNT_OPTS" ]; then
-			opts="${opts},${ETL_APPENDED_MOUNT_OPTS}"
-		fi
-
-		export ETL_MOUNT_OPTS=$(eval "echo $opts")
-	fi
+	_etl_init_mount_opts
 
 	mount -it ecryptfs -o "$ETL_MOUNT_OPTS" \
 		"$ETL_MOUNT_SRC" "$ETL_MOUNT_DST"