fix(p5-remote-option): wire server --trust-sender, align save-layer gates, add hostile-sender test

This commit is contained in:
2026-09-09 14:23:59 +02:00
parent 5e79d7d76b
commit ad228db915
5 changed files with 218 additions and 10 deletions
+12 -5
View File
@@ -22,6 +22,7 @@
static char* authorized_root; static char* authorized_root;
static int authorized_root_fd = -1; static int authorized_root_fd = -1;
static bool allow_delete; static bool allow_delete;
static bool trust_sender;
static bool allow_unauthenticated; static bool allow_unauthenticated;
static const char* required_client_cn; static const char* required_client_cn;
@@ -221,11 +222,14 @@ void handler(int file_descriptor) {
across the per-connection forked processes). */ across the per-connection forked processes). */
file_set_keep_dirlinks(config->keep_dirlinks); file_set_keep_dirlinks(config->keep_dirlinks);
/* --trust-sender is a LOCAL receiver policy: it never crosses the wire (so a /* --trust-sender is a LOCAL receiver policy: it never crosses the wire (so a
wire peer can never enable it), the receiving process applies it here from wire peer can never enable it). The standalone server only honours it when
its own config. Set before any multithreaded receiver/writer threads are its own CLI was started with --trust-sender (the client forwards that switch
spawned so the fd-walk reads a stable value during the whole transfer, and into the remote argv via --remote-option=--trust-sender; the server then
never bleeds across the per-connection forked processes. Off by default. */ parses it here and applies the policy below). Set before any multithreaded
file_set_trust_sender(config->trust_sender); receiver/writer threads are spawned so the fd-walk reads a stable value
during the whole transfer, and never bleeds across the per-connection
forked processes. Off by default. */
file_set_trust_sender(trust_sender);
if (config->use_multithreading) { if (config->use_multithreading) {
Queue* q = queue_create(100, file_destroy); Queue* q = queue_create(100, file_destroy);
if (q == NULL) { if (q == NULL) {
@@ -348,6 +352,7 @@ static void print_server_usage(void) {
printf(" --client-cn <name> Required TLS client certificate CN\n"); printf(" --client-cn <name> Required TLS client certificate CN\n");
printf(" --destination-root <path> Authorized destination root (default: .)\n"); printf(" --destination-root <path> Authorized destination root (default: .)\n");
printf(" --allow-delete Permit manifest deletion\n"); printf(" --allow-delete Permit manifest deletion\n");
printf(" --trust-sender Trust the remote sender's file list\n");
printf(" --allow-unauthenticated Allow plaintext/anonymous network clients\n"); printf(" --allow-unauthenticated Allow plaintext/anonymous network clients\n");
printf(" -v, --verbose Enable debug logging\n"); printf(" -v, --verbose Enable debug logging\n");
printf(" --help Show this help\n"); printf(" --help Show this help\n");
@@ -384,6 +389,8 @@ int main(int argc, char* argv[]) {
destination_root = argv[++i]; destination_root = argv[++i];
} else if (strcmp(argv[i], "--allow-delete") == 0) { } else if (strcmp(argv[i], "--allow-delete") == 0) {
allow_delete = true; allow_delete = true;
} else if (strcmp(argv[i], "--trust-sender") == 0) {
trust_sender = true;
} else if (strcmp(argv[i], "--allow-unauthenticated") == 0) { } else if (strcmp(argv[i], "--allow-unauthenticated") == 0) {
allow_unauthenticated = true; allow_unauthenticated = true;
} else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) { } else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) {
+10 -5
View File
@@ -329,8 +329,12 @@ bool file_special_rdev_valid(int32_t major, int32_t minor, mode_t mode) {
*/ */
static FileSaveResult file_save_special_to_disk(const char* root_directory, const File* file, static FileSaveResult file_save_special_to_disk(const char* root_directory, const File* file,
const Config* config) { const Config* config) {
/* The empty-path and structural checks stay unconditional; the redundant
".." list-path re-check is skipped under --trust-sender exactly like the
receive layer (confinement is deferred to the secure parent walk below,
which is never disabled). */
if (!root_directory || !file || !file->path || file->path[0] == '\0' || if (!root_directory || !file || !file->path || file->path[0] == '\0' ||
has_path_traversal(file->path) || !file->metadata) (!file_get_trust_sender() && has_path_traversal(file->path)) || !file->metadata)
return FILE_SAVE_ERROR; return FILE_SAVE_ERROR;
mode_t mode = file->metadata->mode; mode_t mode = file->metadata->mode;
@@ -461,7 +465,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons
* entry is skipped), never aborts. */ * entry is skipped), never aborts. */
static FileSaveResult file_save_write_device(const char* root_directory, const File* file) { static FileSaveResult file_save_write_device(const char* root_directory, const File* file) {
if (!root_directory || !file || !file->path || file->path[0] == '\0' || if (!root_directory || !file || !file->path || file->path[0] == '\0' ||
has_path_traversal(file->path)) (!file_get_trust_sender() && has_path_traversal(file->path)))
return FILE_SAVE_ERROR; return FILE_SAVE_ERROR;
if (!file->data) if (!file->data)
return FILE_SAVE_ERROR; return FILE_SAVE_ERROR;
@@ -538,7 +542,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
char *backup_path = NULL, *parent_copy = NULL; char *backup_path = NULL, *parent_copy = NULL;
if (!file || !file->path || !file->data || (file->data->size != 0 && !file->data->data) || if (!file || !file->path || !file->data || (file->data->size != 0 && !file->data->data) ||
has_path_traversal(file->path) || (!file_get_trust_sender() && has_path_traversal(file->path)) ||
(backup_enabled && (backup_enabled &&
(!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL || (!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL ||
strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) { strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) {
@@ -560,7 +564,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
immediately (they are never staged by --delay-updates, matching rsync, immediately (they are never staged by --delay-updates, matching rsync,
where directory creation is not delayed). */ where directory creation is not delayed). */
if (file->is_dir) { if (file->is_dir) {
if (file->path[0] == '\0' || has_path_traversal(file->path)) { if (file->path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(file->path))) {
log_message(LOG_LEVEL_ERROR, "Invalid directory path received"); log_message(LOG_LEVEL_ERROR, "Invalid directory path received");
return FILE_SAVE_ERROR; return FILE_SAVE_ERROR;
} }
@@ -577,7 +581,8 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
threads start, so it is stable throughout this walk.) */ threads start, so it is stable throughout this walk.) */
if (file->is_symlink) { if (file->is_symlink) {
if (!file->symlink_target || file->path[0] == '\0' || has_path_traversal(file->path)) { if (!file->symlink_target || file->path[0] == '\0' ||
(!file_get_trust_sender() && has_path_traversal(file->path))) {
log_message(LOG_LEVEL_ERROR, "Invalid symlink entry received"); log_message(LOG_LEVEL_ERROR, "Invalid symlink entry received");
return FILE_SAVE_ERROR; return FILE_SAVE_ERROR;
} }
+1
View File
@@ -55,6 +55,7 @@ int main() {
RUN_TEST(test_metadata); RUN_TEST(test_metadata);
RUN_TEST(test_glob); RUN_TEST(test_glob);
RUN_TEST(test_file); RUN_TEST(test_file);
RUN_TEST(test_trust_sender);
RUN_TEST(test_delay_updates); RUN_TEST(test_delay_updates);
RUN_TEST(test_file_sendfile); RUN_TEST(test_file_sendfile);
RUN_TEST(test_multiprocessing); RUN_TEST(test_multiprocessing);
+194
View File
@@ -6,6 +6,7 @@
#include "protocol.h" #include "protocol.h"
#include "test_utils.h" #include "test_utils.h"
#include <fcntl.h> #include <fcntl.h>
#include <limits.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <sys/stat.h> #include <sys/stat.h>
@@ -1022,6 +1023,199 @@ static void test_dir_entry_save_to_disk() {
rmdir(root); rmdir(root);
} }
/* ---- Phase 5 (--trust-sender) safety-floor tests ----
*
* --trust-sender is a receiver-local policy that never crosses the wire: a real
* receiver enables it from its own process (the standalone server's --trust-
* sender CLI switch, which a client forwards as --remote-option=--trust-sender),
* so these tests force file_set_trust_sender(true) directly. Trust must RELAX
* only the redundant list-level re-validation (an escaping symlink TARGET is
* copied verbatim, rsync -l parity) and must NEVER disable the low-level
* fd-relative confinement floor: file_open_secure_parent's ".." rejection, the
* O_NOFOLLOW parent walk, leaf/destination confinement, and the ungated
* has_path_traversal on the link's own placement path in file_symlink_at_secure
* stay hard. A hostile sender therefore still cannot place a file, directory
* or symlink outside the receive root even with trust on. */
static void test_trust_sender_relaxes_symlink_target() {
const char* root = "test_trust_sender_root";
const char* link = "test_trust_sender_root/escape_link";
unlink(link);
rmdir(root);
EXPECT_EQ_INT(mkdir(root, 0755), 0);
/* Control: without trust an absolute (escaping) target is refused and the
link is never placed. */
file_set_trust_sender(false);
EXPECT_FALSE(file_symlink_at_secure(link, "/etc/passwd"));
struct stat st;
EXPECT_EQ_INT(lstat(link, &st), -1);
/* Trust ON: the escaping target is copied verbatim (rsync -l parity) ... */
file_set_trust_sender(true);
EXPECT_TRUE(file_symlink_at_secure(link, "/etc/passwd"));
EXPECT_EQ_INT(lstat(link, &st), 0);
EXPECT_TRUE(S_ISLNK(st.st_mode));
/* ...but the link itself still lands beneath the receive root. */
char target[128];
ssize_t target_len = readlink(link, target, sizeof(target) - 1);
EXPECT_TRUE(target_len > 0);
// cppcheck-suppress knownConditionTrueFalse
if (target_len > 0) {
target[target_len] = '\0';
EXPECT_EQ_STR(target, "/etc/passwd");
}
unlink(link);
/* Same relaxation through the real save funnel (file_save_to_disk_full). */
Config* config = config_create();
EXPECT_NOT_NULL(config);
const char* save_link = "test_trust_sender_root/save_link";
unlink(save_link);
File* sym = file_create("save_link");
EXPECT_NOT_NULL(sym);
sym->is_symlink = true;
sym->symlink_target = str_dup("/etc/passwd");
EXPECT_NOT_NULL(sym->symlink_target);
file_set_trust_sender(false);
EXPECT_EQ_INT(file_save_to_disk_full(root, sym, config), FILE_SAVE_SKIPPED);
EXPECT_EQ_INT(lstat(save_link, &st), -1);
file_set_trust_sender(true);
EXPECT_EQ_INT(file_save_to_disk_full(root, sym, config), FILE_SAVE_WRITTEN);
EXPECT_EQ_INT(lstat(save_link, &st), 0);
EXPECT_TRUE(S_ISLNK(st.st_mode));
file_destroy(sym);
config_delete(config);
unlink(save_link);
rmdir(root);
}
static void test_trust_sender_confines_hostile_paths() {
const char* root = "test_trust_sender_root";
const char* escaped_file = "../test_trust_sender_escaped_file.txt";
const char* escaped_dir = "../test_trust_sender_escaped_dir";
const char* escaped_link = "../test_trust_sender_escaped_link";
unlink(escaped_file);
rmdir(escaped_dir);
unlink(escaped_link);
unlink(root);
rmdir(root);
EXPECT_EQ_INT(mkdir(root, 0755), 0);
Config* config = config_create();
EXPECT_NOT_NULL(config);
file_set_trust_sender(true);
struct stat st;
/* A hostile regular-file path that would escape the root is contained: the
save-layer ".." re-check is relaxed under trust, so the attempt reaches the
secure floor, which refuses the walk -- nothing appears outside. */
File* file = file_create(escaped_file);
EXPECT_NOT_NULL(file);
file->data->data = malloc(5);
EXPECT_NOT_NULL(file->data->data);
memcpy(file->data->data, "evil", 4);
file->data->size = 4;
EXPECT_EQ_INT(file_save_to_disk_full(root, file, config), FILE_SAVE_ERROR);
file_destroy(file);
EXPECT_EQ_INT(lstat(escaped_file, &st), -1);
/* A hostile directory entry is contained the same way. */
File* dir = file_create(escaped_dir);
EXPECT_NOT_NULL(dir);
dir->is_dir = true;
EXPECT_EQ_INT(file_save_to_disk_full(root, dir, config), FILE_SAVE_ERROR);
file_destroy(dir);
EXPECT_EQ_INT(lstat(escaped_dir, &st), -1);
/* A hostile symlink whose OWN placement path escapes the root is refused even
under trust: the ungated has_path_traversal in file_symlink_at_secure never
turns off. */
EXPECT_FALSE(file_symlink_at_secure("test_trust_sender_root/../escaped_link", "/etc/passwd"));
EXPECT_EQ_INT(lstat(escaped_link, &st), -1);
/* file_open_secure_parent still refuses a ".." component outright. */
char* leaf = NULL;
EXPECT_EQ_INT(file_open_secure_parent("test_trust_sender_root/../../etc/passwd", &leaf, true),
-1);
free(leaf);
config_delete(config);
rmdir(root);
}
/* The same guarantees under a configured authorized root: a within-root link
with an escaping target is created (relaxed), while a placement path that is
a clean absolute path OUTSIDE the authorized root (no ".." anywhere) is
refused by the leaf/destination confinement. */
static void test_trust_sender_authorized_root_confinement() {
const char* root = "test_trust_sender_root";
const char* sibling = "test_trust_sender_sibling";
unlink(root);
rmdir(root);
rmdir(sibling);
EXPECT_EQ_INT(mkdir(root, 0755), 0);
EXPECT_EQ_INT(mkdir(sibling, 0755), 0);
char root_abs[PATH_MAX];
char sibling_abs[PATH_MAX];
EXPECT_NOT_NULL(realpath(root, root_abs));
EXPECT_NOT_NULL(realpath(sibling, sibling_abs));
int root_fd = open(root_abs, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
EXPECT_TRUE(root_fd >= 0);
// cppcheck-suppress knownConditionTrueFalse
if (root_fd < 0) {
rmdir(root);
rmdir(sibling);
return;
}
EXPECT_TRUE(file_set_authorized_root(root_fd, root_abs));
file_set_trust_sender(true);
struct stat st;
/* Within the authorized root, an escaping symlink TARGET is copied verbatim. */
char* inside_link = path_cat(root_abs, "authorized_escape_link");
EXPECT_NOT_NULL(inside_link);
unlink(inside_link);
EXPECT_TRUE(file_symlink_at_secure(inside_link, "/etc/passwd"));
EXPECT_EQ_INT(lstat(inside_link, &st), 0);
EXPECT_TRUE(S_ISLNK(st.st_mode));
unlink(inside_link);
/* A clean absolute path in a sibling directory (outside the authorized root)
is still refused even under trust. */
char* outside_link = path_cat(sibling_abs, "test_trust_sender_outside_link");
EXPECT_NOT_NULL(outside_link);
unlink(outside_link);
EXPECT_FALSE(file_symlink_at_secure(outside_link, "/etc/passwd"));
EXPECT_EQ_INT(lstat(outside_link, &st), -1);
free(outside_link);
free(inside_link);
file_set_authorized_root(-1, NULL);
close(root_fd);
unlink("test_trust_sender_outside_link");
rmdir(sibling);
rmdir(root);
}
void test_trust_sender() {
/* The final reset lines always run (a failing EXPECT only returns from the
helper), so a later group never inherits a stray trust/authorized-root
policy. */
file_set_trust_sender(false);
test_trust_sender_relaxes_symlink_target();
test_trust_sender_confines_hostile_paths();
test_trust_sender_authorized_root_confinement();
file_set_trust_sender(false);
file_set_authorized_root(-1, NULL);
}
void test_file() { void test_file() {
test_file_create(); test_file_create();
test_file_special_rdev_valid(); test_file_special_rdev_valid();
+1
View File
@@ -2,5 +2,6 @@
#define TEST_FILE_H #define TEST_FILE_H
void test_file(); void test_file();
void test_trust_sender();
#endif #endif