From 9235256116574927657a93944ef1b21e255e771b Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Thu, 19 May 2022 16:16:51 +0600 Subject: [PATCH] Extract filesystem-related functions into fs.c MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit … and add unit tests. --- src/bsd/openbsd/tincd.c | 13 --- src/fs.c | 142 ++++++++++++++++++++++ src/fs.h | 23 ++++ src/invitation.c | 1 + src/keys.c | 2 +- src/meson.build | 1 + src/tincctl.c | 44 +------ src/tincctl.h | 9 -- src/tincd.c | 5 +- src/utils.c | 79 ------------- src/utils.h | 4 - test/unit/meson.build | 3 + test/unit/test_fs.c | 253 ++++++++++++++++++++++++++++++++++++++++ test/unit/test_utils.c | 52 --------- 14 files changed, 429 insertions(+), 202 deletions(-) create mode 100644 src/fs.c create mode 100644 src/fs.h create mode 100644 test/unit/test_fs.c diff --git a/src/bsd/openbsd/tincd.c b/src/bsd/openbsd/tincd.c index 0b49ea9e..2faf5448 100644 --- a/src/bsd/openbsd/tincd.c +++ b/src/bsd/openbsd/tincd.c @@ -21,12 +21,6 @@ static bool chrooted(void) { return !(confbase && *confbase); } -static void create_conf_subdir(const char *name, mode_t mode) { - char path[PATH_MAX]; - snprintf(path, sizeof(path), "%s/%s", confbase, name); - mkdir(path, mode); -} - static void open_conf_subdir(const char *name, const char *privs) { char path[PATH_MAX]; snprintf(path, sizeof(path), "%s/%s", confbase, name); @@ -37,13 +31,6 @@ static void open_common_paths(bool can_exec) { // Dummy device uses a fake path, skip it const char *dev = strcasecmp(device, DEVICE_DUMMY) ? device : NULL; - // These calls must be done before the first unveil() for two reasons: - // 1. the first unveil() blocks access to all other paths. - // 2. unveil() remembers the exact directory and won't allow access if it's (re)created. - create_conf_subdir("cache", 0777); - create_conf_subdir("hosts", 0777); - create_conf_subdir("invitations", 0700); - const unveil_path_t paths[] = { {"/dev/random", "r"}, {"/dev/urandom", "r"}, diff --git a/src/fs.c b/src/fs.c new file mode 100644 index 00000000..772fa576 --- /dev/null +++ b/src/fs.c @@ -0,0 +1,142 @@ +#include "system.h" + +#include "fs.h" +#include "names.h" +#include "xalloc.h" + +static bool makedir(const char *path, mode_t mode) { + assert(path); + + if(mkdir(path, mode)) { + if(errno == EEXIST) { + chmod(path, mode); + return true; + } + + fprintf(stderr, "Could not create directory %s: %s\n", path, strerror(errno)); + return false; + } + + return true; +} + +bool makedirs(tinc_dir_t dirs) { + if(!dirs) { + return false; + } + + char path[PATH_MAX]; + bool need_confbase = dirs & ~((unsigned)DIR_CONFDIR); + + if(need_confbase && (!confbase || !makedir(confbase, 0755))) { + return false; + } + + if(dirs & DIR_CONFDIR && !confbase_given && (!confdir || !makedir(confdir, 0755))) { + return false; + } + + if(dirs & DIR_INVITATIONS) { + snprintf(path, sizeof(path), "%s" SLASH "invitations", confbase); + + if(!makedir(path, 0700)) { + return false; + } + } + + if(dirs & DIR_CACHE) { + snprintf(path, sizeof(path), "%s" SLASH "cache", confbase); + + if(!makedir(path, 0755)) { + return false; + } + } + + if(dirs & DIR_HOSTS) { + snprintf(path, sizeof(path), "%s" SLASH "hosts", confbase); + + if(!makedir(path, 0755)) { + return false; + } + } + + return true; +} + + +/* Open a file with the desired permissions, minus the umask. + Also, if we want to create an executable file, we call fchmod() + to set the executable bits. */ + +FILE *fopenmask(const char *filename, const char *mode, mode_t perms) { + mode_t mask = umask(0); + perms &= ~mask; + umask(~perms & 0777); + FILE *f = fopen(filename, mode); + + if(!f) { + fprintf(stderr, "Could not open %s: %s\n", filename, strerror(errno)); + return NULL; + } + +#ifdef HAVE_FCHMOD + + if(perms & 0444) { + fchmod(fileno(f), perms); + } + +#endif + umask(mask); + return f; +} + +char *absolute_path(const char *path) { +#ifdef HAVE_WINDOWS + // Works for nonexistent paths + return _fullpath(NULL, path, 0); +#else + + if(!path || !*path) { + return NULL; + } + + // If an absolute path was passed, return its copy + if(*path == '/') { + return xstrdup(path); + } + + // Try using realpath. If it fails for any reason + // other than that the file was not found, bail out. + char *abs = realpath(path, NULL); + + if(abs || errno != ENOENT) { + return abs; + } + + // Since the file does not exist, we're forced to use a fallback. + // Get current working directory and concatenate it with the argument. + char cwd[PATH_MAX]; + + if(!getcwd(cwd, sizeof(cwd))) { + return NULL; + } + + // Remove trailing slash if present since we'll be adding our own + size_t cwdlen = strlen(cwd); + + if(cwdlen && cwd[cwdlen - 1] == '/') { + cwd[cwdlen - 1] = '\0'; + } + + // We don't do any normalization because it's complicated, and the payoff is small. + // If user passed something like '.././../foo' — that's their choice; fopen works either way. + xasprintf(&abs, "%s/%s", cwd, path); + + if(strlen(abs) >= PATH_MAX) { + free(abs); + abs = NULL; + } + + return abs; +#endif +} diff --git a/src/fs.h b/src/fs.h new file mode 100644 index 00000000..a35b5297 --- /dev/null +++ b/src/fs.h @@ -0,0 +1,23 @@ +#ifndef TINC_FS_H +#define TINC_FS_H + +#include "system.h" + +typedef enum { + DIR_CACHE = 1 << 0, + DIR_CONFBASE = 1 << 1, + DIR_CONFDIR = 1 << 2, + DIR_HOSTS = 1 << 3, + DIR_INVITATIONS = 1 << 4, +} tinc_dir_t; + +// Create one or multiple directories inside tincd configuration directory +extern bool makedirs(tinc_dir_t dirs); + +// Open file. If it does not exist, create a new file with the specified access mode. +extern FILE *fopenmask(const char *filename, const char *mode, mode_t perms) ATTR_DEALLOCATOR(fclose); + +// Get absolute path to a possibly nonexistent file or directory +extern char *absolute_path(const char *path) ATTR_MALLOC; + +#endif // TINC_FS_H diff --git a/src/invitation.c b/src/invitation.c index 08f053a9..e068ae89 100644 --- a/src/invitation.c +++ b/src/invitation.c @@ -36,6 +36,7 @@ #include "xalloc.h" #include "random.h" #include "pidfile.h" +#include "fs.h" #include "ed25519/sha512.h" diff --git a/src/keys.c b/src/keys.c index 5fe51ae6..1bd5907f 100644 --- a/src/keys.c +++ b/src/keys.c @@ -5,7 +5,7 @@ #include "names.h" #include "xalloc.h" #include "ecdsa.h" -#include "utils.h" +#include "fs.h" bool disable_old_keys(const char *filename, const char *what) { char tmpfile[PATH_MAX] = ""; diff --git a/src/meson.build b/src/meson.build index cc646288..6a9fb29a 100644 --- a/src/meson.build +++ b/src/meson.build @@ -119,6 +119,7 @@ src_lib_common = [ 'conf.c', 'console.c', 'dropin.c', + 'fs.c', 'keys.c', 'list.c', 'logger.c', diff --git a/src/tincctl.c b/src/tincctl.c index 46073b3a..2ed286a6 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -44,6 +44,7 @@ #include "sandbox.h" #include "pidfile.h" #include "console.h" +#include "fs.h" #ifndef MSG_NOSIGNAL #define MSG_NOSIGNAL 0 @@ -2189,49 +2190,6 @@ int check_port(const char *name) { return 0; } -static bool makedir(const char *path, mode_t mode) { - if(mkdir(path, mode) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", path, strerror(errno)); - return false; - } - - return true; -} - -bool makedirs(tincd_dir_t dirs) { - if(dirs & DIR_CONFBASE && !makedir(confbase, 0777)) { - return false; - } - - if(dirs & DIR_CONFDIR && !confbase_given && !makedir(confdir, 0755)) { - return false; - } - - if(dirs & DIR_HOSTS && !makedir(hosts_dir, 0777)) { - return false; - } - - char path[PATH_MAX]; - - if(dirs & DIR_INVITATIONS) { - snprintf(path, sizeof(path), "%s" SLASH "invitations", confbase); - - if(!makedir(path, 0700)) { - return false; - } - } - - if(dirs & DIR_CACHE) { - snprintf(path, sizeof(path), "%s" SLASH "%s", confbase, "cache"); - - if(!makedir(path, 0755)) { - return false; - } - } - - return true; -} - static int cmd_init(int argc, char *argv[]) { if(!access(tinc_conf, F_OK)) { fprintf(stderr, "Configuration file %s already exists!\n", tinc_conf); diff --git a/src/tincctl.h b/src/tincctl.h index ee5f056d..94df21d8 100644 --- a/src/tincctl.h +++ b/src/tincctl.h @@ -44,14 +44,6 @@ typedef struct { int type; } var_t; -typedef enum { - DIR_CACHE = 1 << 0, - DIR_CONFBASE = 1 << 1, - DIR_CONFDIR = 1 << 2, - DIR_HOSTS = 1 << 3, - DIR_INVITATIONS = 1 << 4, -} tincd_dir_t; - extern const var_t variables[]; extern size_t rstrip(char *value); @@ -60,6 +52,5 @@ extern bool connect_tincd(bool verbose); extern bool sendline(int fd, const char *format, ...) ATTR_FORMAT(printf, 2, 3); extern bool recvline(int fd, char *line, size_t len); extern int check_port(const char *name); -extern bool makedirs(tincd_dir_t dirs); #endif diff --git a/src/tincd.c b/src/tincd.c index 8a4c1f39..1c9b6ed6 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -57,6 +57,7 @@ #include "random.h" #include "sandbox.h" #include "watchdog.h" +#include "fs.h" /* If nonzero, display usage information and exit. */ static bool show_help = false; @@ -415,7 +416,9 @@ static bool drop_privs(void) { return false; } -#endif +#endif // HAVE_WINDOWS + + makedirs(DIR_CACHE | DIR_HOSTS | DIR_INVITATIONS); return sandbox_enter(); } diff --git a/src/utils.c b/src/utils.c index 8655e2be..3a3030d1 100644 --- a/src/utils.c +++ b/src/utils.c @@ -18,8 +18,6 @@ 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include - #include "logger.h" #include "system.h" #include "utils.h" @@ -81,57 +79,6 @@ size_t bin2hex(const void *vsrc, char *dst, size_t length) { return length * 2; } -char *absolute_path(const char *path) { -#ifdef HAVE_WINDOWS - // Works for nonexistent paths - return _fullpath(NULL, path, 0); -#else - - if(!path || !*path) { - return NULL; - } - - // If an absolute path was passed, return its copy - if(*path == '/') { - return xstrdup(path); - } - - // Try using realpath. If it fails for any reason - // other than that the file was not found, bail out. - char *abs = realpath(path, NULL); - - if(abs || errno != ENOENT) { - return abs; - } - - // Since the file does not exist, we're forced to use a fallback. - // Get current working directory and concatenate it with the argument. - char cwd[PATH_MAX]; - - if(!getcwd(cwd, sizeof(cwd))) { - return NULL; - } - - // Remove trailing slash if present since we'll be adding our own - size_t cwdlen = strlen(cwd); - - if(cwdlen && cwd[cwdlen - 1] == '/') { - cwd[cwdlen - 1] = '\0'; - } - - // We don't do any normalization because it's complicated, and the payoff is small. - // If user passed something like '.././../foo' — that's their choice; fopen works either way. - xasprintf(&abs, "%s/%s", cwd, path); - - if(strlen(abs) >= PATH_MAX) { - free(abs); - abs = NULL; - } - - return abs; -#endif -} - size_t b64decode_tinc(const char *src, void *dst, size_t length) { size_t i; uint32_t triplet = 0; @@ -347,32 +294,6 @@ char *replace_name(const char *name) { return ret_name; } -/* Open a file with the desired permissions, minus the umask. - Also, if we want to create an executable file, we call fchmod() - to set the executable bits. */ - -FILE *fopenmask(const char *filename, const char *mode, mode_t perms) { - mode_t mask = umask(0); - perms &= ~mask; - umask(~perms & 0777); - FILE *f = fopen(filename, mode); - - if(!f) { - fprintf(stderr, "Could not open %s: %s\n", filename, strerror(errno)); - return NULL; - } - -#ifdef HAVE_FCHMOD - - if(perms & 0444) { - fchmod(fileno(f), perms); - } - -#endif - umask(mask); - return f; -} - bool string_eq(const char *first, const char *second) { return !first == !second && !(first && second && strcmp(first, second)); diff --git a/src/utils.h b/src/utils.h index 487058ae..c33e28eb 100644 --- a/src/utils.h +++ b/src/utils.h @@ -74,10 +74,6 @@ extern bool check_id(const char *id); extern bool check_netname(const char *netname, bool strict); char *replace_name(const char *name) ATTR_MALLOC; -char *absolute_path(const char *path) ATTR_MALLOC; - -extern FILE *fopenmask(const char *filename, const char *mode, mode_t perms) ATTR_DEALLOCATOR(fclose); - // NULL-safe wrapper around strcmp(). extern bool string_eq(const char *first, const char *second); diff --git a/test/unit/meson.build b/test/unit/meson.build index 52005987..b91f6bae 100644 --- a/test/unit/meson.build +++ b/test/unit/meson.build @@ -53,6 +53,9 @@ tests = { 'utils': { 'code': 'test_utils.c', }, + 'fs': { + 'code': 'test_fs.c', + }, 'xalloc': { 'code': 'test_xalloc.c', }, diff --git a/test/unit/test_fs.c b/test/unit/test_fs.c new file mode 100644 index 00000000..95d2d09e --- /dev/null +++ b/test/unit/test_fs.c @@ -0,0 +1,253 @@ +#include "unittest.h" +#include "../../src/fs.h" +#include "../../src/names.h" +#include "../../src/xalloc.h" + +#ifndef HAVE_WINDOWS + +#define FAKE_PATH "nonexistentreallyfakepath" + +typedef struct { + const char *arg; + const char *want; +} testcase_t; + +static void test_absolute_path_on_absolute_returns_it(void **state) { + (void)state; + + const char *paths[] = {"/", "/foo", "/foo/./../bar"}; + + for(size_t i = 0; i < sizeof(paths) / sizeof(*paths); ++i) { + char *got = absolute_path(paths[i]); + assert_ptr_not_equal(paths[i], got); + assert_string_equal(paths[i], got); + free(got); + } +} + +static void test_absolute_path_on_empty_returns_null(void **state) { + (void)state; + assert_null(absolute_path(NULL)); + assert_null(absolute_path("\0")); +} + +static void test_absolute_path_relative(void **state) { + (void)state; + + testcase_t cases[] = { + {".", "/"}, + {"foo", "/foo"}, + {"./"FAKE_PATH, "/./"FAKE_PATH}, + {"../foo/./../"FAKE_PATH, "/../foo/./../"FAKE_PATH}, + }; + + for(size_t i = 0; i < sizeof(cases) / sizeof(*cases); ++i) { + char *got = absolute_path(cases[i].arg); + assert_string_equal(cases[i].want, got); + free(got); + } +} + +static int setup_path_unix(void **state) { + (void)state; + assert_int_equal(0, chdir("/")); + return 0; +} + +const char tmp_template[] = "/tmp/tinc.test.fs.XXXXXX"; +char tmp[sizeof(tmp_template)]; + +static int setup_temp_dir(void **state) { + (void)state; + strcpy(tmp, tmp_template); + assert_ptr_equal(tmp, mkdtemp(tmp)); + confdir = xstrdup(tmp); + xasprintf(&confbase, "%s/conf", tmp); + return 0; +} + +static int teardown_temp_dir(void **state) { + (void)state; + free(confdir); + free(confbase); + return 0; +} + +static void test_makedir(tinc_dir_t dir, bool exists) { + char path[PATH_MAX]; + char container[PATH_MAX] = {0}; + + switch(dir) { + case DIR_CONFDIR: + strcpy(path, tmp); + break; + + case DIR_CONFBASE: + sprintf(path, "%s/conf", tmp); + strcpy(container, tmp); + break; + + case DIR_CACHE: + sprintf(path, "%s/conf/cache", tmp); + sprintf(container, "%s/conf", tmp); + break; + + case DIR_HOSTS: + sprintf(path, "%s/conf/hosts", tmp); + sprintf(container, "%s/conf", tmp); + break; + + case DIR_INVITATIONS: + sprintf(path, "%s/conf/invitations", tmp); + sprintf(container, "%s/conf", tmp); + break; + } + + struct stat st; + + if(exists) { + assert_int_equal(0, stat(path, &st)); + } else { + assert_int_equal(-1, stat(path, &st)); + assert_int_equal(ENOENT, errno); + } + + // Deny write access and make sure makedirs() detects that + if(*container) { + assert_int_equal(0, chmod(tmp, 0)); + assert_false(makedirs(dir)); + assert_int_equal(0, chmod(tmp, 0755)); + } + + // Now test the happy path + assert_true(makedirs(dir)); + assert_int_equal(0, stat(path, &st)); + assert_true(S_ISDIR(st.st_mode)); + assert_int_equal(0, access(path, R_OK | W_OK)); + + // Make sure no other directories were created + if(*container) { + DIR *d = opendir(container); + assert_non_null(d); + + struct dirent *ent; + + while((ent = readdir(d))) { + if(strcmp(".", ent->d_name) && strcmp("..", ent->d_name)) { + assert_int_equal(st.st_ino, ent->d_ino); + assert_true(ent->d_type & DT_DIR); + } + } + + closedir(d); + } +} + +static void test_makedirs_cache(void **state) { + (void)state; + test_makedir(DIR_CACHE, false); +} + +static void test_makedirs_confbase(void **state) { + (void)state; + test_makedir(DIR_CONFBASE, false); +} + +static void test_makedirs_confdir(void **state) { + (void)state; + test_makedir(DIR_CONFDIR, true); +} + +static void test_makedirs_hosts(void **state) { + (void)state; + test_makedir(DIR_HOSTS, false); +} + +static void test_makedirs_invitations(void **state) { + (void)state; + test_makedir(DIR_INVITATIONS, false); +} + +static int setup_umask(void **state) { + (void)state; + umask(0); + return 0; +} + +static void test_fopenmask_existing(void **state) { + (void)state; + + struct stat st; + strcpy(tmp, tmp_template); + + int fd = mkstemp(tmp); + assert_int_not_equal(-1, fd); + close(fd); + + assert_int_equal(0, chmod(tmp, 0755)); + assert_int_equal(0, stat(tmp, &st)); + assert_int_equal(0755, st.st_mode & 0777); + + FILE *f = fopenmask(tmp, "r", 0700); + assert_non_null(f); + fclose(f); + + assert_int_equal(0, stat(tmp, &st)); + assert_int_equal(0700, st.st_mode & 0777); +} + +static void test_fopenmask_new(void **state) { + (void)state; + + struct stat st; + strcpy(tmp, tmp_template); + + // mktemp() nags about safety and using better alternatives + int fd = mkstemp(tmp); + assert_int_not_equal(-1, fd); + close(fd); + unlink(tmp); + + FILE *f = fopenmask(tmp, "w", 0750); + assert_non_null(f); + fclose(f); + + assert_int_equal(0, stat(tmp, &st)); + assert_int_equal(0750, st.st_mode & 0777); +} + +#endif // HAVE_WINDOWS + +static void test_makedirs_bad(void **state) { + (void)state; + + assert_false(makedirs(0)); + + confbase = NULL; // free not needed, just make it obvious that confbase is NULL + assert_false(makedirs(DIR_CACHE)); + assert_false(makedirs(DIR_CONFBASE)); + assert_false(makedirs(DIR_HOSTS)); + assert_false(makedirs(DIR_INVITATIONS)); + + confdir = NULL; // same + assert_false(makedirs(DIR_CONFDIR)); +} + +int main(void) { + const struct CMUnitTest tests[] = { +#ifndef HAVE_WINDOWS + cmocka_unit_test_setup(test_absolute_path_on_absolute_returns_it, setup_path_unix), + cmocka_unit_test_setup(test_absolute_path_on_empty_returns_null, setup_path_unix), + cmocka_unit_test_setup(test_absolute_path_relative, setup_path_unix), + cmocka_unit_test_setup_teardown(test_makedirs_cache, setup_temp_dir, teardown_temp_dir), + cmocka_unit_test_setup_teardown(test_makedirs_confbase, setup_temp_dir, teardown_temp_dir), + cmocka_unit_test_setup_teardown(test_makedirs_confdir, setup_temp_dir, teardown_temp_dir), + cmocka_unit_test_setup_teardown(test_makedirs_hosts, setup_temp_dir, teardown_temp_dir), + cmocka_unit_test_setup_teardown(test_makedirs_invitations, setup_temp_dir, teardown_temp_dir), + cmocka_unit_test_setup(test_fopenmask_existing, setup_umask), + cmocka_unit_test_setup(test_fopenmask_new, setup_umask), +#endif + cmocka_unit_test(test_makedirs_bad), + }; + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/test/unit/test_utils.c b/test/unit/test_utils.c index 51fc83ae..6fa543af 100644 --- a/test/unit/test_utils.c +++ b/test/unit/test_utils.c @@ -1,49 +1,6 @@ #include "unittest.h" #include "../../src/utils.h" -#define FAKE_PATH "nonexistentreallyfakepath" - -typedef struct { - const char *arg; - const char *want; -} testcase_t; - -static void test_unix_absolute_path_on_absolute_returns_it(void **state) { - (void)state; - - const char *paths[] = {"/", "/foo", "/foo/./../bar"}; - - for(size_t i = 0; i < sizeof(paths) / sizeof(*paths); ++i) { - char *got = absolute_path(paths[i]); - assert_ptr_not_equal(paths[i], got); - assert_string_equal(paths[i], got); - free(got); - } -} - -static void test_unix_absolute_path_on_empty_returns_null(void **state) { - (void)state; - assert_null(absolute_path(NULL)); - assert_null(absolute_path("\0")); -} - -static void test_unix_absolute_path_relative(void **state) { - (void)state; - - testcase_t cases[] = { - {".", "/"}, - {"foo", "/foo"}, - {"./"FAKE_PATH, "/./"FAKE_PATH}, - {"../foo/./../"FAKE_PATH, "/../foo/./../"FAKE_PATH}, - }; - - for(size_t i = 0; i < sizeof(cases) / sizeof(*cases); ++i) { - char *got = absolute_path(cases[i].arg); - assert_string_equal(cases[i].want, got); - free(got); - } -} - static void test_int_to_str(const char *ref, int num) { char *result = int_to_str(num); assert_string_equal(ref, result); @@ -99,12 +56,6 @@ static void test_is_decimal_pass_whitespace_prefix(void **state) { assert_true(is_decimal(" \r\n\t 777")); } -static int setup_path_unix(void **state) { - (void)state; - assert_int_equal(0, chdir("/")); - return 0; -} - static void test_string_eq(void **state) { (void)state; @@ -120,9 +71,6 @@ static void test_string_eq(void **state) { int main(void) { const struct CMUnitTest tests[] = { - cmocka_unit_test_setup(test_unix_absolute_path_on_absolute_returns_it, setup_path_unix), - cmocka_unit_test_setup(test_unix_absolute_path_on_empty_returns_null, setup_path_unix), - cmocka_unit_test_setup(test_unix_absolute_path_relative, setup_path_unix), cmocka_unit_test(test_int_to_str_return_expected), cmocka_unit_test(test_is_decimal_fail_empty), cmocka_unit_test(test_is_decimal_fail_hex), -- 2.20.1