Convert tincd path args to absolute paths
authorKirill Isakov <bootctl@gmail.com>
Thu, 28 Apr 2022 15:05:36 +0000 (21:05 +0600)
committerKirill Isakov <bootctl@gmail.com>
Thu, 28 Apr 2022 15:05:36 +0000 (21:05 +0600)
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
src/utils.c
src/utils.h
test/integration/commandline.py
test/integration/testlib/check.py
test/integration/testlib/util.py
test/unit/test_utils.c

index 5bfeeab..06e6713 100644 (file)
@@ -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:
index c395169..5f40b8a 100644 (file)
@@ -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;
index 0818047..bd5ce13 100644 (file)
@@ -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
index 531868f..931b6d4 100755 (executable)
@@ -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)
index 1d1dfb0..524ca62 100755 (executable)
@@ -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")
index 8102bca..f0849b9 100755 (executable)
@@ -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))
index 587a0e1..a66541e 100644 (file)
@@ -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);
 }