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.

diff --git a/tools/save.c b/tools/save.c
index 1f886ad8a3f55283cb614fde87f8276c384db8c2..856a7238f1669de125eb2f45a121306d664f5063 100644 (file)
--- 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 88d1ecd27e24a7f398b50695e4a63f8ad76f0939..cd6cb4425c1773023050e5a46b551afdd1033cc3 100644 (file)
--- 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 56e65cd469bcca5b79ea8d821d3141bda673a8ae..94078178ec5f23e52938b00951cd368fbd6fe015 100644 (file)
--- 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);