diff --git a/pytest.ini b/pytest.ini index 9047220..9cd9e47 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,3 +5,6 @@ markers = setpriv: privilege-dependent tests (drop to an unprivileged user); excluded from CI because their result depends on the runner/container uid and the host mount permissions, but run locally as root + daemon_detach: real double-fork backgrounding path (--daemon without + --no-detach); slower/fragile, so it runs in the full suite but not the + fast PR gate diff --git a/src/server/server.c b/src/server/server.c index 122cf54..73eb02e 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -19,6 +19,8 @@ #include #include #include +#include +#include #include static char* authorized_root; @@ -459,9 +461,9 @@ static void print_server_usage(void) { * exists (or when HOME is set), otherwise /etc/fastsyncd.conf. Returns a * pointer to a static buffer (never NULL). */ static const char* default_daemon_config_path(void) { - static char user_path[PATH_MAX]; const char* home = getenv("HOME"); if (home && *home) { + static char user_path[PATH_MAX]; int n = snprintf(user_path, sizeof(user_path), "%s/.config/fastsync/fastsyncd.conf", home); if (n > 0 && (size_t)n < sizeof(user_path) && access(user_path, R_OK) == 0) return user_path; @@ -496,6 +498,12 @@ static bool daemonize(void) { if (devnull > STDERR_FILENO) close(devnull); } + /* Do not pin the launch CWD (module-relative 'path' entries would resolve + * against an unstable working directory) and drop the restrictive host umask + * so modules can create files/dirs with the modes the config requests. */ + if (chdir("/") != 0) + log_message(LOG_LEVEL_WARNING, "daemon: chdir to / failed: %s", strerror(errno)); + umask(0); return true; } diff --git a/src/shared/config.c b/src/shared/config.c index 61341fe..c7423da 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -1070,6 +1070,16 @@ static bool receive_daemon_module(int fd, Config* c) { char* module = receive_str(fd); if (!module) return false; + /* Guard against a hostile client flooding the log with an over-long module + * name: only an empty string (module-less) or a valid module name + * (bounded by DAEMON_MAX_MODULE_NAME) is accepted. This is an input + * guard, not a wire-format change. */ + if (*module != '\0' && !daemon_module_name_valid(module)) { + log_message(LOG_LEVEL_WARNING, "Daemon client sent an invalid or over-long module name"); + free(module); + send_status(fd, STATUS_ERROR); + return false; + } if (*module != '\0') { c->module = module; } else { diff --git a/src/shared/daemon_conf.c b/src/shared/daemon_conf.c index 31e38a3..60e3d27 100644 --- a/src/shared/daemon_conf.c +++ b/src/shared/daemon_conf.c @@ -122,7 +122,8 @@ static bool store_port(int* slot, const char* value, char* err, size_t err_size) /* Apply a global scalar key/value. Keys are case-insensitive. Returns false * (err filled) on an unknown key or an invalid value. */ -static bool apply_global_key(DaemonConf* conf, char* key, char* value, char* err, size_t err_size) { +static bool apply_global_key(DaemonConf* conf, char* key, const char* value, char* err, + size_t err_size) { if (key_equals(key, "port")) return store_port(&conf->global.port, value, err, err_size); if (key_equals(key, "motd file")) { @@ -177,7 +178,7 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char* } char* save = NULL; for (char* token = strtok_r(list, ",", &save); token; token = strtok_r(NULL, ",", &save)) { - char* user = trim_ws(token); + const char* user = trim_ws(token); if (*user == '\0') continue; char** grown = @@ -341,7 +342,7 @@ DaemonConf* daemon_conf_load(const char* path, char* err, size_t err_size) { } *close = '\0'; char* trailing = close + 1; - char* rest = trim_ws(trailing); + const char* rest = trim_ws(trailing); if (*rest != '\0') { set_error(err, err_size, "line %d: unexpected text after module header", line_no); ok = false; @@ -425,7 +426,7 @@ int daemon_conf_apply_dparam(DaemonConf* conf, const char* assignment, char* err } *eq = '\0'; char* key = trim_ws(copy); - char* value = trim_ws(eq + 1); + const char* value = trim_ws(eq + 1); if (*key == '\0') { free(copy); set_error(err, err_size, "--dparam '%s' has an empty key", assignment); diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index fb324f6..262802f 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -6,6 +6,7 @@ asks for a module with a host::module/path destination, and the transfer lands in the configured module root only. Read-only modules, unknown modules, and auth-required modules are all refused cleanly before any data moves. """ +import glob import os import shutil import signal @@ -34,6 +35,28 @@ FILES_MODULE = os.path.join(MODULE_ROOT, "files") READONLY_MODULE = os.path.join(MODULE_ROOT, "readonly") AUTH_MODULE = os.path.join(MODULE_ROOT, "auth") CONF_FILE = os.path.join(TEST_DATA_DIR, "fastsyncd.conf") +DETACH_MODULE = os.path.join(MODULE_ROOT, "detach") +DETACH_CONF = os.path.join(TEST_DATA_DIR, "fastsyncd_detach.conf") +DETACH_PORT = None + + +def _kill_by_cmdline_marker(marker): + """Send SIGTERM to every running process whose cmdline contains `marker` + (used to clean up the double-forked --daemon, which is orphaned to init and + no longer a child of the test's own process). Portable over /proc so the + tests do not depend on pgrep being present.""" + for proc_path in glob.glob("/proc/[0-9]*/cmdline"): + try: + with open(proc_path, "rb") as f: + data = f.read() + except OSError: + continue + if marker.encode() in data: + try: + os.kill(int(proc_path.split("/")[2]), signal.SIGTERM) + except (ProcessLookupError, ValueError): + pass + time.sleep(0.5) class DaemonManager: @@ -98,7 +121,7 @@ def _config_port(config_path): @pytest.fixture(scope="module", autouse=True) def daemon_env(): - for d in (MODULE_ROOT, FILES_MODULE, READONLY_MODULE, AUTH_MODULE): + for d in (MODULE_ROOT, FILES_MODULE, READONLY_MODULE, AUTH_MODULE, DETACH_MODULE): shutil.rmtree(d, ignore_errors=True) os.makedirs(d, exist_ok=True) generate_test_files(SOURCE_DIR, full=False) @@ -123,7 +146,16 @@ def daemon_env(): "path = %s\n" "auth users = alice\n" % (config_port, FILES_MODULE, READONLY_MODULE, AUTH_MODULE)) + + # A dedicated config for the real (double-fork) detach test: an unique path + # lets cleanup identify and kill the orphaned background daemon by cmdline. + global DETACH_PORT + DETACH_PORT = _find_free_port() + with open(DETACH_CONF, "w") as f: + f.write("port = %d\n\n[detach]\npath = %s\n" % (DETACH_PORT, DETACH_MODULE)) + yield + _kill_by_cmdline_marker(DETACH_CONF) shutil.rmtree(MODULE_ROOT, ignore_errors=True) shutil.rmtree(SOURCE_DIR, ignore_errors=True) @@ -165,22 +197,83 @@ class TestDaemonModuleSelection: class TestDaemonRejection: + def _tree_files(self): + """Snapshot every file path (module-relative) currently under the module + root tree, so confinement can be asserted by diff rather than by an + absolute 'empty' check (other tests legitimately populate modules).""" + files = set() + for root, _, names in os.walk(MODULE_ROOT): + for name in names: + full = os.path.join(root, name) + files.add(os.path.relpath(full, MODULE_ROOT)) + return files + def test_read_only_module_blocked(self, daemon): result = _push("127.0.0.1::readonly", daemon.port) assert result.returncode != 0 file_count = sum(len(files) for _, _, files in os.walk(READONLY_MODULE)) assert file_count == 0, "read-only module must not receive any file" + def test_read_only_no_write_anywhere(self, daemon): + """A refused read-only transfer must not add a single file anywhere under + the module root tree (negative confinement, not just the target).""" + before = self._tree_files() + result = _push("127.0.0.1::readonly", daemon.port) + assert result.returncode != 0 + assert self._tree_files() == before, "read-only rejection wrote under the module root" + def test_unknown_module_rejected(self, daemon): result = _push("127.0.0.1::no-such-module", daemon.port) assert result.returncode != 0 + def test_unknown_module_no_write_anywhere(self, daemon): + """An unknown module must be refused cleanly before any file lands + anywhere beneath the module root tree.""" + before = self._tree_files() + result = _push("127.0.0.1::no-such-module", daemon.port) + assert result.returncode != 0 + assert self._tree_files() == before, "unknown-module rejection wrote under the module root" + + def test_module_less_destination_rejected(self, daemon): + """A daemon destination with no module name (host::/path) is refused at + parse time, before any connection payload is sent.""" + result = _push("127.0.0.1::", daemon.port) + assert result.returncode != 0 + result = _push("127.0.0.1::/sub", daemon.port) + assert result.returncode != 0 + + def test_dotdot_destination_rejected(self, daemon): + """A '..' path expansion in the module-relative path is refused at parse + time so a client cannot escape the module root while it is still on the + client side of the wire.""" + result = _push("127.0.0.1::files/../..", daemon.port) + assert result.returncode != 0 + def test_auth_required_module_rejected(self, daemon): result = _push("127.0.0.1::locked", daemon.port) assert result.returncode != 0 file_count = sum(len(files) for _, _, files in os.walk(AUTH_MODULE)) assert file_count == 0 + @pytest.mark.daemon_detach + def test_real_detach_path(self): + """--daemon WITHOUT --no-detach double-forks a real background daemon; + a client can still transfer into the module root, and the orphaned + process is terminated cleanly (via SIGTERM after polling the port).""" + log_path = os.path.join(TEST_DATA_DIR, "fastsyncd_detach.log") + log = open(log_path, "w") + cmd = SERVER_CMD + ["--daemon", "--config", DETACH_CONF, "--allow-unauthenticated"] + proc = subprocess.Popen(cmd, stdout=log, stderr=log, stdin=subprocess.DEVNULL) + try: + _wait_for_port(DETACH_PORT, timeout=15) + result = _push("127.0.0.1::detach", DETACH_PORT) + assert result.returncode == 0, result.stderr or result.stdout + received = get_dest_received_dir(DETACH_MODULE, SOURCE_DIR) + _, missing = verify_transfer(SOURCE_DIR, received) + assert not missing, f"missing: {missing[:5]}" + finally: + _kill_by_cmdline_marker(DETACH_CONF) + def test_plaintext_requires_allow_unauthenticated(self): """Secure default: a daemon started WITHOUT --allow-unauthenticated must refuse a plaintext client (same posture as the standalone server).""" diff --git a/tests/test_daemon_conf.c b/tests/test_daemon_conf.c index 34b12be..c6a7211 100644 --- a/tests/test_daemon_conf.c +++ b/tests/test_daemon_conf.c @@ -148,7 +148,7 @@ static void test_daemon_conf_unknown_key_rejected() { char* path; char err[256]; EXPECT_EQ_INT(write_conf("bogus_key = 1\n", &path), 0); - DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); + const DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); free(path); EXPECT_NULL(conf); EXPECT_TRUE(strstr(err, "unknown global key") != NULL); @@ -163,7 +163,7 @@ static void test_daemon_conf_unknown_key_rejected() { static void test_daemon_conf_malformed_rejected() { char* path; char err[256]; - DaemonConf* conf; + const DaemonConf* conf; EXPECT_EQ_INT(write_conf("port 8734\n", &path), 0); conf = daemon_conf_load(path, err, sizeof(err)); @@ -235,7 +235,7 @@ static void test_daemon_conf_duplicate_module_rejected() { char* path; char err[256]; EXPECT_EQ_INT(write_conf("[m]\npath = /a\n[m]\npath = /b\n", &path), 0); - DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); + const DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); free(path); EXPECT_NULL(conf); EXPECT_TRUE(strstr(err, "duplicate module") != NULL); @@ -249,7 +249,7 @@ static void test_daemon_conf_long_line_rejected() { memcpy(body, "[m]\npath = /x\nport = ", 21); body[sizeof(body) - 1] = '\0'; EXPECT_EQ_INT(write_conf(body, &path), 0); - DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); + const DaemonConf* conf = daemon_conf_load(path, err, sizeof(err)); free(path); EXPECT_NULL(conf); EXPECT_TRUE(strstr(err, "exceeds the") != NULL); @@ -257,7 +257,8 @@ static void test_daemon_conf_long_line_rejected() { static void test_daemon_conf_missing_file_rejected() { char err[256]; - DaemonConf* conf = daemon_conf_load("/nonexistent/fastsync_daemon_conf_zzz", err, sizeof(err)); + const DaemonConf* conf = + daemon_conf_load("/nonexistent/fastsync_daemon_conf_zzz", err, sizeof(err)); EXPECT_NULL(conf); EXPECT_TRUE(strstr(err, "cannot open") != NULL); }