From b6295dd63a8028ae0b239859406c477d779f4d5e Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Tue, 19 Mar 2013 17:24:47 +0100 Subject: 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. --- tools/save.c | 60 ++++++++++++++++++++++++++++++++++++------------- tools/tests/test-save.c | 25 +++++++++------------ tools/tests/test.c | 8 +++++-- 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); -- cgit v1.1