Convert tincd path args to absolute paths

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.

diff --git a/src/tincd.c b/src/tincd.c
index 5bfeeab..06e6713 100644 (file)
--- 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;
 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);
 
                case OPT_CONFIG_FILE:
                        free(confbase);

-                       confbase = xstrdup(optarg);
+                       confbase = get_path_arg(optarg);

                        break;
 
                case OPT_NO_DETACH:
                        break;
 
                case OPT_NO_DETACH:


@@ -263,14 +274,14 @@ static bool parse_options(int argc, char **argv) {
 
                        if(optarg) {
                                free(logfilename);
 
                        if(optarg) {
                                free(logfilename);

-                               logfilename = xstrdup(optarg);
+                               logfilename = get_path_arg(optarg);

                        }
 
                        break;
 
                case OPT_PIDFILE:
                        free(pidfilename);
                        }
 
                        break;
 
                case OPT_PIDFILE:
                        free(pidfilename);

-                       pidfilename = xstrdup(optarg);
+                       pidfilename = get_path_arg(optarg);

                        break;
 
                default:
                        break;
 
                default:

diff --git a/src/utils.c b/src/utils.c
index c395169..5f40b8a 100644 (file)
--- 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;
 }
 
        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;
 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 0818047..bd5ce13 100644 (file)
--- 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);
 
 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
 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 531868f..931b6d4 100755 (executable)
--- a/test/integration/commandline.py
+++ b/test/integration/commandline.py
@@ -2,7 +2,12 @@
 
 """Test supported and unsupported commandline flags."""
 
 
 """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
 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)
 
     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 1d1dfb0..524ca62 100755 (executable)
--- 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."""
 
 """Simple assertions which print the expected and received values on failure."""
 

+import os.path

 import typing as T
 import typing as T

+from pathlib import Path

 
 from .log import log
 
 
 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")
 
     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 8102bca..f0849b9 100755 (executable)
--- 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
 import string
 import socket
 import typing as T

+from pathlib import Path

 
 from . import check
 from .log import log
 
 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)
 
 
             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))
 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 587a0e1..a66541e 100644 (file)
--- a/test/unit/test_utils.c
+++ b/test/unit/test_utils.c
@@ -1,6 +1,49 @@
 #include "unittest.h"
 #include "../../src/utils.h"
 
 #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);
 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"));
 }
 
        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[] = {
 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),
                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),
        };
                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);
 }
        return cmocka_run_group_tests(tests, NULL, NULL);
 }