From: Kirill Isakov <bootctl@gmail.com>
Date: Thu, 28 Apr 2022 15:05:36 +0000 (+0600)
Subject: Convert tincd path args to absolute paths
X-Git-Url: https://tinc-vpn.org/git/browse?a=commitdiff_plain;h=3d787920d51a35e74e442c7265be3b13b69ad8e4;p=tinc
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 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);
}