extract: Make extracted output directories read-only

This is not a security feature or anything like that, but a hint
that the files are managed by the extract tool and should not be
modified manually.

3 files changed, 61 insertions, 32 deletions

diff --git a/tools/save.c b/tools/save.c
index 1f886ad..856a723 100644
--- a/tools/save.c
+++ b/tools/save.c
@@ -184,23 +184,18 @@ p11_save_finish_file (p11_save_file *file,
 		return true;
 	}
 
-#ifdef OS_UNIX
-	/* Set the mode of the file */
-	if (chmod (file->temp, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) < 0) {
-		p11_message ("couldn't set file permissions: %s: %s",
-		             file->temp, strerror (errno));
-		close (file->fd);
-		ret = false;
-
-	} else
-#endif /* OS_UNIX */
-
 	if (close (file->fd) < 0) {
 		p11_message ("couldn't write file: %s: %s",
 		             file->temp, strerror (errno));
 		ret = false;
 
 #ifdef OS_UNIX
+	/* Set the mode of the file, readable by everyone, but not writable */
+	} else if (chmod (file->temp, S_IRUSR | S_IRGRP | S_IROTH) < 0) {
+		p11_message ("couldn't set file permissions: %s: %s",
+		             file->temp, strerror (errno));
+		close (file->fd);
+		ret = false;
 
 	/* Atomically rename the tempfile over the filename */
 	} else if (file->flags & P11_SAVE_OVERWRITE) {
@@ -255,21 +250,42 @@ p11_save_open_directory (const char *path,
                          int flags)
 {
 #ifdef OS_UNIX
-	mode_t mode = S_IRWXU | S_IXGRP | S_IRGRP | S_IROTH | S_IXOTH;
+	struct stat sb;
 #endif
 	p11_save_dir *dir;
 
 	return_val_if_fail (path != NULL, NULL);
 
 #ifdef OS_UNIX
-	if (mkdir (path, mode) < 0) {
+	/* We update the permissions when we finish writing */
+	if (mkdir (path, S_IRWXU) < 0) {
 #else /* OS_WIN32 */
 	if (mkdir (path) < 0) {
 #endif
-		if (!(flags & P11_SAVE_OVERWRITE) || errno != EEXIST) {
+		/* Some random error, report it */
+		if (errno != EEXIST) {
+			p11_message ("couldn't create directory: %s: %s", path, strerror (errno));
+
+		/* The directory exists and we're not overwriting */
+		} else if (!(flags & P11_SAVE_OVERWRITE)) {
 			p11_message ("directory already exists: %s", path);
 			return NULL;
 		}
+#ifdef OS_UNIX
+		/*
+		 * If the directory exists on unix, we may have restricted
+		 * the directory permissions to read-only. We have to change
+		 * them back to writable in order for things to work.
+		 */
+		if (stat (path, &sb) >= 0) {
+			if ((sb.st_mode & S_IRWXU) != S_IRWXU &&
+			    chmod (path, S_IRWXU | sb.st_mode) < 0) {
+				p11_message ("couldn't make directory writable: %s: %s",
+				             path, strerror (errno));
+				return NULL;
+			}
+		}
+#endif /* OS_UNIX */
 	}
 
 	dir = calloc (1, sizeof (p11_save_dir));
@@ -498,8 +514,20 @@ p11_save_finish_directory (p11_save_dir *dir,
 	if (!dir)
 		return false;
 
-	if (commit && (dir->flags & P11_SAVE_OVERWRITE))
-		ret = cleanup_directory (dir->path, dir->cache);
+	if (commit) {
+		if (dir->flags & P11_SAVE_OVERWRITE)
+			ret = cleanup_directory (dir->path, dir->cache);
+
+#ifdef OS_UNIX
+		/* Try to set the mode of the directory to readable */
+		if (ret && chmod (dir->path, S_IRUSR | S_IXUSR | S_IRGRP |
+		                             S_IXGRP | S_IROTH | S_IXOTH) < 0) {
+			p11_message ("couldn't set directory permissions: %s: %s",
+			             dir->path, strerror (errno));
+			ret = false;
+		}
+#endif /* OS_UNIX */
+	}
 
 	p11_dict_free (dir->cache);
 	free (dir->path);
diff --git a/tools/tests/test-save.c b/tools/tests/test-save.c
index 88d1ecd..cd6cb44 100644
--- a/tools/tests/test-save.c
+++ b/tools/tests/test-save.c
@@ -304,7 +304,7 @@ test_directory_empty (CuTest *tc)
 
 	test_check_directory (tc, subdir, (NULL, NULL));
 
-	rmdir (subdir);
+	CuAssertTrue (tc, rmdir (subdir) >= 0);
 	free (subdir);
 
 	teardown (tc);
@@ -355,7 +355,7 @@ test_directory_files (CuTest *tc)
 	test_check_symlink (tc, subdir, "link.ext", "/the/destination");
 #endif
 
-	rmdir (subdir);
+	CuAssertTrue (tc, rmdir (subdir) >= 0);
 	free (subdir);
 
 	teardown (tc);
@@ -437,7 +437,7 @@ test_directory_dups (CuTest *tc)
 	test_check_symlink (tc, subdir, "link.1", "/destination2");
 #endif
 
-	rmdir (subdir);
+	CuAssertTrue (tc, rmdir (subdir) >= 0);
 	free (subdir);
 
 	teardown (tc);
@@ -487,18 +487,15 @@ test_directory_overwrite (CuTest *tc)
 	if (asprintf (&subdir, "%s/%s", test.directory, "extract-dir") < 0)
 		CuFail (tc, "asprintf() failed");
 
-#ifdef OS_UNIX
-	if (mkdir (subdir, S_IRWXU) < 0)
-#else
-	if (mkdir (subdir) < 0)
-#endif
-		CuFail (tc, "mkdir() failed");
-
 	/* Some initial files into this directory, which get overwritten */
-	write_zero_file (tc, subdir, "file.txt");
-	write_zero_file (tc, subdir, "another-file");
-	write_zero_file (tc, subdir, "third-file");
+	dir = p11_save_open_directory (subdir, 0);
+	ret = p11_save_write_and_finish (p11_save_open_file_in (dir, "file", ".txt", NULL), "", 0) &&
+	      p11_save_write_and_finish (p11_save_open_file_in (dir, "another-file", NULL, NULL), "", 0) &&
+	      p11_save_write_and_finish (p11_save_open_file_in (dir, "third-file", NULL, NULL), "", 0) &&
+	      p11_save_finish_directory (dir, true);
+	CuAssertTrue (tc, ret && dir);
 
+	/* Now the actual test, using the same directory */
 	dir = p11_save_open_directory (subdir, P11_SAVE_OVERWRITE);
 	CuAssertPtrNotNull (tc, dir);
 
@@ -525,7 +522,7 @@ test_directory_overwrite (CuTest *tc)
 	test_check_data (tc, subdir, "file.txt", test_text, strlen (test_text));
 	test_check_data (tc, subdir, "file.1.txt", test_text, 10);
 
-	rmdir (subdir);
+	CuAssertTrue (tc, rmdir (subdir) >= 0);
 	free (subdir);
 
 	teardown (tc);
diff --git a/tools/tests/test.c b/tools/tests/test.c
index 56e65cd..9407817 100644
--- a/tools/tests/test.c
+++ b/tools/tests/test.c
@@ -116,7 +116,7 @@ test_check_data_msg (CuTest *tc,
 	if (filelen != reflen || memcmp (filedata, refdata, reflen) != 0)
 		CuFail_Line (tc, file, line, "File contents not as expected", filename);
 
-	unlink (filename);
+	CuAssert_Line (tc, file, line, "couldn't remove file", unlink (filename) >= 0);
 	free (filename);
 	free (filedata);
 }
@@ -142,7 +142,7 @@ test_check_symlink_msg (CuTest *tc,
 
 	CuAssertStrEquals_LineMsg (tc, file, line, "symlink contents wrong", destination, buf);
 
-	unlink (filename);
+	CuAssert_Line (tc, file, line, "couldn't remove symlink", unlink (filename) >= 0);
 	free (filename);
 }
 
@@ -197,6 +197,10 @@ test_check_directory_msg (CuTest *tc,
 
 	closedir (dir);
 
+#if OS_UNIX
+	CuAssert_Line (tc, file, line, "couldn't chown directory", chmod (directory, S_IRWXU) >= 0);
+#endif
+
 	p11_dict_iterate (files, &iter);
 	while (p11_dict_next (&iter, (void **)&name, NULL))
 		CuFail_Line (tc, file, line, "Couldn't find file in directory", name);