fix(d5-daemon-core): cppcheck const-correctness, wire module length cap, daemonize chdir/umask, daemon confinement tests
- daemon_conf.c/server.c/test_daemon_conf.c: const-qualify parse/loop pointers; scope user_path static inside its block (clears the 9-wave-A cppcheck findings) - config.c receive_daemon_module: reject invalid/over-long wire module names (> DAEMON_MAX_MODULE_NAME) with a clean STATUS_ERROR; client side already enforced via daemon_module_name_valid in config_parse_daemon_dest - server.c daemonize: chdir(/) and umask(0) so module paths resolve from / and config-requested file modes are honored; PROTOCOL_VERSION stays 2.15.0 - test_daemon.py: confinement (read-only/unknown no-write anywhere), module-less and dot-dot destination refusal, real daemon_detach double-fork path
This commit is contained in:
@@ -5,3 +5,6 @@ markers =
|
|||||||
setpriv: privilege-dependent tests (drop to an unprivileged user); excluded
|
setpriv: privilege-dependent tests (drop to an unprivileged user); excluded
|
||||||
from CI because their result depends on the runner/container uid and the
|
from CI because their result depends on the runner/container uid and the
|
||||||
host mount permissions, but run locally as root
|
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
|
||||||
|
|||||||
+9
-1
@@ -19,6 +19,8 @@
|
|||||||
#include <stdlib.h>
|
#include <stdlib.h>
|
||||||
#include <string.h>
|
#include <string.h>
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
#include <errno.h>
|
||||||
|
#include <sys/stat.h>
|
||||||
#include <openssl/x509.h>
|
#include <openssl/x509.h>
|
||||||
|
|
||||||
static char* authorized_root;
|
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
|
* exists (or when HOME is set), otherwise /etc/fastsyncd.conf. Returns a
|
||||||
* pointer to a static buffer (never NULL). */
|
* pointer to a static buffer (never NULL). */
|
||||||
static const char* default_daemon_config_path(void) {
|
static const char* default_daemon_config_path(void) {
|
||||||
static char user_path[PATH_MAX];
|
|
||||||
const char* home = getenv("HOME");
|
const char* home = getenv("HOME");
|
||||||
if (home && *home) {
|
if (home && *home) {
|
||||||
|
static char user_path[PATH_MAX];
|
||||||
int n = snprintf(user_path, sizeof(user_path), "%s/.config/fastsync/fastsyncd.conf", home);
|
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)
|
if (n > 0 && (size_t)n < sizeof(user_path) && access(user_path, R_OK) == 0)
|
||||||
return user_path;
|
return user_path;
|
||||||
@@ -496,6 +498,12 @@ static bool daemonize(void) {
|
|||||||
if (devnull > STDERR_FILENO)
|
if (devnull > STDERR_FILENO)
|
||||||
close(devnull);
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1070,6 +1070,16 @@ static bool receive_daemon_module(int fd, Config* c) {
|
|||||||
char* module = receive_str(fd);
|
char* module = receive_str(fd);
|
||||||
if (!module)
|
if (!module)
|
||||||
return false;
|
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') {
|
if (*module != '\0') {
|
||||||
c->module = module;
|
c->module = module;
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -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
|
/* Apply a global scalar key/value. Keys are case-insensitive. Returns false
|
||||||
* (err filled) on an unknown key or an invalid value. */
|
* (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"))
|
if (key_equals(key, "port"))
|
||||||
return store_port(&conf->global.port, value, err, err_size);
|
return store_port(&conf->global.port, value, err, err_size);
|
||||||
if (key_equals(key, "motd file")) {
|
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;
|
char* save = NULL;
|
||||||
for (char* token = strtok_r(list, ",", &save); token; token = strtok_r(NULL, ",", &save)) {
|
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')
|
if (*user == '\0')
|
||||||
continue;
|
continue;
|
||||||
char** grown =
|
char** grown =
|
||||||
@@ -341,7 +342,7 @@ DaemonConf* daemon_conf_load(const char* path, char* err, size_t err_size) {
|
|||||||
}
|
}
|
||||||
*close = '\0';
|
*close = '\0';
|
||||||
char* trailing = close + 1;
|
char* trailing = close + 1;
|
||||||
char* rest = trim_ws(trailing);
|
const char* rest = trim_ws(trailing);
|
||||||
if (*rest != '\0') {
|
if (*rest != '\0') {
|
||||||
set_error(err, err_size, "line %d: unexpected text after module header", line_no);
|
set_error(err, err_size, "line %d: unexpected text after module header", line_no);
|
||||||
ok = false;
|
ok = false;
|
||||||
@@ -425,7 +426,7 @@ int daemon_conf_apply_dparam(DaemonConf* conf, const char* assignment, char* err
|
|||||||
}
|
}
|
||||||
*eq = '\0';
|
*eq = '\0';
|
||||||
char* key = trim_ws(copy);
|
char* key = trim_ws(copy);
|
||||||
char* value = trim_ws(eq + 1);
|
const char* value = trim_ws(eq + 1);
|
||||||
if (*key == '\0') {
|
if (*key == '\0') {
|
||||||
free(copy);
|
free(copy);
|
||||||
set_error(err, err_size, "--dparam '%s' has an empty key", assignment);
|
set_error(err, err_size, "--dparam '%s' has an empty key", assignment);
|
||||||
|
|||||||
@@ -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
|
in the configured module root only. Read-only modules, unknown modules, and
|
||||||
auth-required modules are all refused cleanly before any data moves.
|
auth-required modules are all refused cleanly before any data moves.
|
||||||
"""
|
"""
|
||||||
|
import glob
|
||||||
import os
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
import signal
|
import signal
|
||||||
@@ -34,6 +35,28 @@ FILES_MODULE = os.path.join(MODULE_ROOT, "files")
|
|||||||
READONLY_MODULE = os.path.join(MODULE_ROOT, "readonly")
|
READONLY_MODULE = os.path.join(MODULE_ROOT, "readonly")
|
||||||
AUTH_MODULE = os.path.join(MODULE_ROOT, "auth")
|
AUTH_MODULE = os.path.join(MODULE_ROOT, "auth")
|
||||||
CONF_FILE = os.path.join(TEST_DATA_DIR, "fastsyncd.conf")
|
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:
|
class DaemonManager:
|
||||||
@@ -98,7 +121,7 @@ def _config_port(config_path):
|
|||||||
|
|
||||||
@pytest.fixture(scope="module", autouse=True)
|
@pytest.fixture(scope="module", autouse=True)
|
||||||
def daemon_env():
|
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)
|
shutil.rmtree(d, ignore_errors=True)
|
||||||
os.makedirs(d, exist_ok=True)
|
os.makedirs(d, exist_ok=True)
|
||||||
generate_test_files(SOURCE_DIR, full=False)
|
generate_test_files(SOURCE_DIR, full=False)
|
||||||
@@ -123,7 +146,16 @@ def daemon_env():
|
|||||||
"path = %s\n"
|
"path = %s\n"
|
||||||
"auth users = alice\n"
|
"auth users = alice\n"
|
||||||
% (config_port, FILES_MODULE, READONLY_MODULE, AUTH_MODULE))
|
% (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
|
yield
|
||||||
|
_kill_by_cmdline_marker(DETACH_CONF)
|
||||||
shutil.rmtree(MODULE_ROOT, ignore_errors=True)
|
shutil.rmtree(MODULE_ROOT, ignore_errors=True)
|
||||||
shutil.rmtree(SOURCE_DIR, ignore_errors=True)
|
shutil.rmtree(SOURCE_DIR, ignore_errors=True)
|
||||||
|
|
||||||
@@ -165,22 +197,83 @@ class TestDaemonModuleSelection:
|
|||||||
|
|
||||||
|
|
||||||
class TestDaemonRejection:
|
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):
|
def test_read_only_module_blocked(self, daemon):
|
||||||
result = _push("127.0.0.1::readonly", daemon.port)
|
result = _push("127.0.0.1::readonly", daemon.port)
|
||||||
assert result.returncode != 0
|
assert result.returncode != 0
|
||||||
file_count = sum(len(files) for _, _, files in os.walk(READONLY_MODULE))
|
file_count = sum(len(files) for _, _, files in os.walk(READONLY_MODULE))
|
||||||
assert file_count == 0, "read-only module must not receive any file"
|
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):
|
def test_unknown_module_rejected(self, daemon):
|
||||||
result = _push("127.0.0.1::no-such-module", daemon.port)
|
result = _push("127.0.0.1::no-such-module", daemon.port)
|
||||||
assert result.returncode != 0
|
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):
|
def test_auth_required_module_rejected(self, daemon):
|
||||||
result = _push("127.0.0.1::locked", daemon.port)
|
result = _push("127.0.0.1::locked", daemon.port)
|
||||||
assert result.returncode != 0
|
assert result.returncode != 0
|
||||||
file_count = sum(len(files) for _, _, files in os.walk(AUTH_MODULE))
|
file_count = sum(len(files) for _, _, files in os.walk(AUTH_MODULE))
|
||||||
assert file_count == 0
|
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):
|
def test_plaintext_requires_allow_unauthenticated(self):
|
||||||
"""Secure default: a daemon started WITHOUT --allow-unauthenticated must
|
"""Secure default: a daemon started WITHOUT --allow-unauthenticated must
|
||||||
refuse a plaintext client (same posture as the standalone server)."""
|
refuse a plaintext client (same posture as the standalone server)."""
|
||||||
|
|||||||
@@ -148,7 +148,7 @@ static void test_daemon_conf_unknown_key_rejected() {
|
|||||||
char* path;
|
char* path;
|
||||||
char err[256];
|
char err[256];
|
||||||
EXPECT_EQ_INT(write_conf("bogus_key = 1\n", &path), 0);
|
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);
|
free(path);
|
||||||
EXPECT_NULL(conf);
|
EXPECT_NULL(conf);
|
||||||
EXPECT_TRUE(strstr(err, "unknown global key") != NULL);
|
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() {
|
static void test_daemon_conf_malformed_rejected() {
|
||||||
char* path;
|
char* path;
|
||||||
char err[256];
|
char err[256];
|
||||||
DaemonConf* conf;
|
const DaemonConf* conf;
|
||||||
|
|
||||||
EXPECT_EQ_INT(write_conf("port 8734\n", &path), 0);
|
EXPECT_EQ_INT(write_conf("port 8734\n", &path), 0);
|
||||||
conf = daemon_conf_load(path, err, sizeof(err));
|
conf = daemon_conf_load(path, err, sizeof(err));
|
||||||
@@ -235,7 +235,7 @@ static void test_daemon_conf_duplicate_module_rejected() {
|
|||||||
char* path;
|
char* path;
|
||||||
char err[256];
|
char err[256];
|
||||||
EXPECT_EQ_INT(write_conf("[m]\npath = /a\n[m]\npath = /b\n", &path), 0);
|
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);
|
free(path);
|
||||||
EXPECT_NULL(conf);
|
EXPECT_NULL(conf);
|
||||||
EXPECT_TRUE(strstr(err, "duplicate module") != NULL);
|
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);
|
memcpy(body, "[m]\npath = /x\nport = ", 21);
|
||||||
body[sizeof(body) - 1] = '\0';
|
body[sizeof(body) - 1] = '\0';
|
||||||
EXPECT_EQ_INT(write_conf(body, &path), 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);
|
free(path);
|
||||||
EXPECT_NULL(conf);
|
EXPECT_NULL(conf);
|
||||||
EXPECT_TRUE(strstr(err, "exceeds the") != NULL);
|
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() {
|
static void test_daemon_conf_missing_file_rejected() {
|
||||||
char err[256];
|
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_NULL(conf);
|
||||||
EXPECT_TRUE(strstr(err, "cannot open") != NULL);
|
EXPECT_TRUE(strstr(err, "cannot open") != NULL);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user