From 3d787920d51a35e74e442c7265be3b13b69ad8e4 Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Thu, 28 Apr 2022 21:05:36 +0600 Subject: [PATCH] Convert tincd path args to absolute paths MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Since tincd chdirs to its own configuration directory and only then resolves relative paths passed as command-line arguments (like --config and --pidfile), statements like these: $ tinc -c confdir init foo … $ tincd -c confdir -D didn't work properly. Now we resolve paths to absolute right when we receive them, and only then do chdir. --- src/tincd.c | 17 ++++++-- src/utils.c | 51 ++++++++++++++++++++++ src/utils.h | 2 + test/integration/commandline.py | 70 ++++++++++++++++++++++++++++++- test/integration/testlib/check.py | 8 ++++ test/integration/testlib/util.py | 10 +++++ test/unit/test_utils.c | 57 +++++++++++++++++++++++++ 7 files changed, 211 insertions(+), 4 deletions(-) diff --git a/src/tincd.c b/src/tincd.c index 5bfeeabb..06e67130 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -158,6 +158,17 @@ static void usage(bool status) { } } +// Try to resolve path to absolute, return a copy of the argument if this fails. +static char *get_path_arg(char *arg) { + char *result = absolute_path(arg); + + if(!result) { + result = xstrdup(arg); + } + + return result; +} + static bool parse_options(int argc, char **argv) { config_t *cfg; int r; @@ -175,7 +186,7 @@ static bool parse_options(int argc, char **argv) { case OPT_CONFIG_FILE: free(confbase); - confbase = xstrdup(optarg); + confbase = get_path_arg(optarg); break; case OPT_NO_DETACH: @@ -263,14 +274,14 @@ static bool parse_options(int argc, char **argv) { if(optarg) { free(logfilename); - logfilename = xstrdup(optarg); + logfilename = get_path_arg(optarg); } break; case OPT_PIDFILE: free(pidfilename); - pidfilename = xstrdup(optarg); + pidfilename = get_path_arg(optarg); break; default: diff --git a/src/utils.c b/src/utils.c index c395169d..5f40b8af 100644 --- a/src/utils.c +++ b/src/utils.c @@ -81,6 +81,57 @@ 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; diff --git a/src/utils.h b/src/utils.h index 08180477..bd5ce13c 100644 --- a/src/utils.h +++ b/src/utils.h @@ -74,6 +74,8 @@ extern bool check_id(const char *id); extern bool check_netname(const char *netname, bool strict); char *replace_name(const char *name); +char *absolute_path(const char *path) ATTR_MALLOC; + extern FILE *fopenmask(const char *filename, const char *mode, mode_t perms); #endif diff --git a/test/integration/commandline.py b/test/integration/commandline.py index 531868f6..931b6d40 100755 --- a/test/integration/commandline.py +++ b/test/integration/commandline.py @@ -2,7 +2,12 @@ """Test supported and unsupported commandline flags.""" -from testlib import check, util +import os +import signal +import subprocess as subp +import time + +from testlib import check, util, path from testlib.log import log from testlib.proc import Tinc, Script from testlib.test import Test @@ -83,3 +88,66 @@ with Test("commandline flags") as context: for code, flags in tinc_flags: node.cmd(*flags, code=code) + + +def test_relative_path(ctx: Test, chroot: bool) -> None: + """Test tincd with relative paths.""" + + foo = init(ctx) + + conf_dir = os.path.realpath(foo.sub(".")) + dirname = os.path.dirname(conf_dir) + basename = os.path.basename(conf_dir) + log.info("using confdir %s, dirname %s, basename %s", conf_dir, dirname, basename) + + args = [ + path.TINCD_PATH, + "-D", + "-c", + basename, + "--pidfile", + "pid", + "--logfile", + ".//./log", + ] + + if chroot: + args.append("-R") + + pidfile = os.path.join(dirname, "pid") + util.remove_file(pidfile) + + logfile = os.path.join(dirname, "log") + util.remove_file(logfile) + + with subp.Popen(args, stderr=subp.STDOUT, cwd=dirname) as tincd: + foo[Script.TINC_UP].wait(10) + + log.info("pidfile and logfile must exist at expected paths") + check.file_exists(pidfile) + check.file_exists(logfile) + + # chrooted tincd won't be able to reopen its log since in this + # test we put the log outside tinc's configuration directory. + if os.name != "nt" and not chroot: + log.info("test log file rotation") + time.sleep(1) + util.remove_file(logfile) + os.kill(tincd.pid, signal.SIGHUP) + time.sleep(1) + + log.info("pidfile and logfile must still exist") + check.file_exists(pidfile) + check.file_exists(logfile) + + log.info("stopping tinc through '%s'", pidfile) + foo.cmd("--pidfile", pidfile, "stop") + check.equals(0, tincd.wait()) + + +with Test("relative path to tincd dir") as context: + test_relative_path(context, chroot=False) + +if os.name != "nt" and not os.getuid(): + with Test("relative path to tincd dir (chroot)") as context: + test_relative_path(context, chroot=True) diff --git a/test/integration/testlib/check.py b/test/integration/testlib/check.py index 1d1dfb0f..524ca623 100755 --- a/test/integration/testlib/check.py +++ b/test/integration/testlib/check.py @@ -1,6 +1,8 @@ """Simple assertions which print the expected and received values on failure.""" +import os.path import typing as T +from pathlib import Path from .log import log @@ -86,3 +88,9 @@ def files_eq(path0: str, path1: str) -> None: if content0 != content1: raise ValueError(f"expected files {path0} and {path1} to match") + + +def file_exists(path: T.Union[str, Path]) -> None: + """Check that file or directory exists.""" + if not os.path.exists(path): + raise ValueError("expected path '{path}' to exist") diff --git a/test/integration/testlib/util.py b/test/integration/testlib/util.py index 8102bca2..f0849b95 100755 --- a/test/integration/testlib/util.py +++ b/test/integration/testlib/util.py @@ -7,6 +7,7 @@ import random import string import socket import typing as T +from pathlib import Path from . import check from .log import log @@ -31,6 +32,15 @@ def random_port() -> int: log.debug("could not bind to random port %d", port, exc_info=ex) +def remove_file(path: T.Union[str, Path]) -> bool: + """Try to remove file without failing if it does not exist.""" + try: + os.remove(path) + return True + except FileNotFoundError: + return False + + def random_string(k: int) -> str: """Generate a random alphanumeric string of length k.""" return "".join(random.choices(_ALPHA_NUMERIC, k=k)) diff --git a/test/unit/test_utils.c b/test/unit/test_utils.c index 587a0e1f..a66541e7 100644 --- a/test/unit/test_utils.c +++ b/test/unit/test_utils.c @@ -1,6 +1,49 @@ #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); @@ -56,8 +99,17 @@ 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; +} + 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), @@ -66,5 +118,10 @@ int main(void) { cmocka_unit_test(test_is_decimal_pass_signs), cmocka_unit_test(test_is_decimal_pass_whitespace_prefix), }; + +#ifdef HAVE_WINDOWS + cmocka_set_skip_filter("test_unix_*"); +#endif + return cmocka_run_group_tests(tests, NULL, NULL); } -- 2.20.1