From ea4ab661b4c9eb230646090f10ac453f13197852 Mon Sep 17 00:00:00 2001 From: TapTap Date: Tue, 15 Sep 2026 21:57:48 +0200 Subject: [PATCH] fix(parity): rsync 3.4.1 symlink and special-node semantics (#287, #288) #287: - --safe-links: keep safe in-tree links AS symlinks and skip unsafe (absolute or ".."-escaping) ones, mirroring rsync's unsafe_symlink(). Skipped links are recorded as delete-protected so --delete does not remove their destination mirror (no silent data loss). - --copy-unsafe-links: preserve safe links as symlinks and dereference only unsafe ones. - --munge-links: receiver-side rewrite storing /rsyncd-munged/-prefixed targets (rsync parity), replacing the no-op #SYMLINK sender prefix. - -l: store the target verbatim, including absolute and ".." targets (rsync -l parity); the old receiver containment silently dropped them. #288: - --specials: recreate unix-domain sockets via mknod(S_IFSOCK), which Linux permits unprivileged; keep EEXIST/EPERM skip behavior. - --copy-devices: copy a device's content into a regular file when requested; skip unrequested non-regular entries like rsync's default. --- src/client/scanner.c | 253 +++++++++++++++++---------- src/client/usage.c | 9 +- src/shared/file.c | 91 +++++++--- src/shared/file.h | 21 ++- src/shared/file_receive.c | 59 +++---- tests/integration/test_features.py | 270 ++++++++++++++++++++++------- tests/test_file.c | 97 ++++++++--- tests/test_server.c | 23 ++- 8 files changed, 573 insertions(+), 250 deletions(-) diff --git a/src/client/scanner.c b/src/client/scanner.c index 2614d1c..11074b6 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -98,17 +98,51 @@ static DirEntry* dir_entry_create(const char* path, int depth, FilterNode* conte return de; } -static bool safe_relative_link(const char* source_root, const char* containing_dir, - const char* link_target) { - char root[PATH_MAX]; - if (!realpath(source_root, root)) - return false; - char* joined = path_cat(containing_dir, link_target); - char resolved[PATH_MAX]; - bool safe = joined && realpath(joined, resolved) && strncmp(root, resolved, strlen(root)) == 0 && - (resolved[strlen(root)] == '\0' || resolved[strlen(root)] == '/'); - free(joined); - return safe; +/* How rsync's readlink_stat()/generator resolves one source symlink. */ +typedef enum { + LINK_ACTION_SKIP, /* not transferred (no link option) */ + LINK_ACTION_SKIP_PROTECTED, /* ignored as unsafe by --safe-links; rsync keeps + it in the transfer, so its destination mirror + must be protected from --delete */ + LINK_ACTION_DEREF, /* follow the referent (--copy-links, an unsafe + target under --copy-unsafe-links, or -k dir) */ + LINK_ACTION_CARRY, /* transmit the link itself (-l) */ +} LinkAction; + +/* Apply rsync's symlink-resolution precedence to one S_ISLNK entry: + * --copy-links dereferences every symlink; + * --copy-unsafe-links dereferences only targets unsafe_symlink() flags; + * -k/--copy-dirlinks dereferences only a symlink whose referent is a dir; + * --safe-links (receiver-side in rsync; modelled here) ignores an unsafe + * target that would otherwise be carried; with --munge-links + * every stored target becomes absolute, so --safe-links then + * ignores every symlink, exactly as rsync documents; + * -l/--links carries the link. + * `link_rel` is the symlink's transfer-relative path (incl. name) and is used + * only for the lexical unsafe test. `target` receives the raw link value. */ +static LinkAction scanner_link_action(const ScannerOptions* options, const char* path, + const char* link_rel, char* target, size_t target_size) { + if (!options->follow_symlinks && !options->copy_links && !options->safe_links && + !options->copy_unsafe_links && !options->copy_dirlinks) + return LINK_ACTION_SKIP; + ssize_t length = readlink(path, target, target_size - 1); + if (length < 0) + return LINK_ACTION_SKIP; + target[length] = '\0'; + + bool unsafe = file_symlink_unsafe(target, link_rel); + if (options->copy_links || (options->copy_unsafe_links && unsafe)) + return LINK_ACTION_DEREF; + if (options->copy_dirlinks) { + struct stat ref; + if (stat(path, &ref) == 0 && S_ISDIR(ref.st_mode)) + return LINK_ACTION_DEREF; + } + if (options->safe_links && (unsafe || options->munge_links)) + return LINK_ACTION_SKIP_PROTECTED; + if (!options->follow_symlinks || target[0] == '\0') + return LINK_ACTION_SKIP; + return LINK_ACTION_CARRY; } typedef struct { @@ -210,24 +244,35 @@ static void scanner_assign_hardlink(DirectoryScanner* scanner, HardLinkTable* ta } } -/* Phase 4 special/devices: detect a device (char/block), FIFO or socket entry - and, when the matching --devices/--specials flag asks it be preserved, - convert the File into a node to recreate (is_special, empty payload) with its - device rdev captured from the source stat. When the entry is not preserved - (or --copy-devices instead copies its content as an ordinary regular file) - the File is left as a normal data file. Returns true when converted. */ -static bool scanner_prepare_special(bool preserve_devices, bool preserve_specials, File* file, - const struct stat* stats) { +/* Phase 4 special/devices decision for one non-regular entry, matching rsync: + - a char/block device is RECREATED as a node under -D/--devices, unless + --copy-devices asks for its content to be copied into a regular file; + - a FIFO/socket is RECREATED under --specials; + - when the matching flag is absent the entry is SKIPPED ("skipping + non-regular file"), exactly like rsync's default, instead of being + silently copied as a zero-length regular file; + - anything else (regular/directory) is left to the normal data path. */ +typedef enum { + SCANNER_SPECIAL_REGULAR, /* ordinary file: transfer content */ + SCANNER_SPECIAL_RECREATE, /* is_special node to recreate on the receiver */ + SCANNER_SPECIAL_SKIP, /* non-regular entry not requested: skip */ +} ScannerSpecial; + +static ScannerSpecial scanner_prepare_special(bool preserve_devices, bool preserve_specials, + bool copy_devices, File* file, + const struct stat* stats) { if (!file || !stats) - return false; + return SCANNER_SPECIAL_REGULAR; bool is_device = S_ISCHR(stats->st_mode) || S_ISBLK(stats->st_mode); bool is_fifo = S_ISFIFO(stats->st_mode); bool is_socket = S_ISSOCK(stats->st_mode); if (!is_device && !is_fifo && !is_socket) - return false; + return SCANNER_SPECIAL_REGULAR; + if (is_device && copy_devices) + return SCANNER_SPECIAL_REGULAR; /* copy device content as a regular file */ bool preserve = is_device ? preserve_devices : preserve_specials; if (!preserve) - return false; + return SCANNER_SPECIAL_SKIP; file->is_special = true; file->data->size = 0; file->data->data = NULL; @@ -235,7 +280,7 @@ static bool scanner_prepare_special(bool preserve_devices, bool preserve_special file->rdev_major = (int32_t)major(stats->st_rdev); file->rdev_minor = (int32_t)minor(stats->st_rdev); } - return true; + return SCANNER_SPECIAL_RECREATE; } /* Append `rel` to the caller's exclusion sink, taking `mtx` when shared across @@ -306,10 +351,11 @@ static int open_directory_filter_context(DirectoryScanner* scanner, const Filter return 0; } -/* Inspect symlinks, resolve the entry type, and apply file filters once for both scanners. */ -static int scanner_inspect_entry(const ScannerOptions* options, const char* source_root, - const char* containing_dir, const char* name, - ScannerEntry* entry) { +/* Inspect symlinks, resolve the entry type, and apply file filters once for both scanners. + * `link_rel` is the entry's path relative to the transfer root (including its + * name), used for the lexical rsync unsafe-symlink test. */ +static int scanner_inspect_entry(const ScannerOptions* options, const char* containing_dir, + const char* link_rel, const char* name, ScannerEntry* entry) { entry->excluded = false; entry->is_symlink = false; entry->link_target = NULL; @@ -322,72 +368,49 @@ static int scanner_inspect_entry(const ScannerOptions* options, const char* sour free(entry->path); return 0; } - bool is_symlink = S_ISLNK(link_stats.st_mode); - if (!is_symlink) + if (!S_ISLNK(link_stats.st_mode)) goto regular; - /* Symlink: choose between dereferencing (---copy-links / --safe-links / - --copy-unsafe-links, plus -k for symlinks-to-directories) and carrying the - link through as a symlink (-l, and -k for symlinks-to-files). No link - option means the symlink is skipped entirely (pre-existing behavior). */ - const bool any_link_option = options->follow_symlinks || options->copy_links || - options->safe_links || options->copy_unsafe_links || - options->copy_dirlinks; - if (!any_link_option) - goto skip; - char link_target[4096]; - ssize_t length = readlink(entry->path, link_target, sizeof(link_target) - 1); - if (length < 0) + switch (scanner_link_action(options, entry->path, link_rel, link_target, sizeof(link_target))) { + case LINK_ACTION_SKIP: goto skip; - link_target[length] = '\0'; - - if (options->safe_links) { - if (link_target[0] == '/' || !safe_relative_link(source_root, containing_dir, link_target)) - goto skip; - } - if (options->copy_unsafe_links && !options->copy_links) { - if (link_target[0] != '/') - goto skip; - } - - bool emit_symlink = false; - if (options->copy_links) { - emit_symlink = false; /* --copy-links dereferences every referent */ - } else if (options->safe_links || options->copy_unsafe_links) { - emit_symlink = false; /* preserve pre-existing dereference behavior */ - } else if (options->copy_dirlinks) { - struct stat ref; - if (stat(entry->path, &ref) == 0 && S_ISDIR(ref.st_mode)) - emit_symlink = false; /* -k: symlink to a directory recurses as a dir */ - else - emit_symlink = true; /* -k: symlink to a file stays a symlink */ - } else if (options->follow_symlinks) { - emit_symlink = true; /* -l: copy symlink as symlink */ - } - - if (!emit_symlink) { - if (stat(entry->path, &entry->stats) != 0) + case LINK_ACTION_SKIP_PROTECTED: + /* --safe-links ignored the link, but rsync still counts it as present in + the transfer, so its destination mirror survives --delete. Record it as + an excluded path (the same delete-protection channel as a filter prune). */ + entry->excluded = true; + goto skip; + case LINK_ACTION_DEREF: + if (stat(entry->path, &entry->stats) != 0) { + /* rsync reports "symlink has no referent" and continues (exit 23); we + surface the same condition rather than silently dropping the entry. */ + char* escaped = output_escape(entry->path, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, "symlink has no referent: %s", + escaped ? escaped : ""); + free(escaped); goto skip; + } entry->is_directory = S_ISDIR(entry->stats.st_mode); if (entry->is_directory) return 1; goto apply_filters; + case LINK_ACTION_CARRY: + break; } - /* Carry the link as a symlink. --munge-links containment: a target that - could escape the receive root (absolute or containing "..") is never - transmitted -- the entry is merely skipped ("contained"). */ - if (link_target[0] == '\0' || - (options->munge_links && !file_symlink_target_contained(link_target))) - goto skip; + /* Carry the link as a symlink. --munge-links is applied by the RECEIVER (it + prefixes every stored target with /rsyncd-munged/); when the SOURCE already + holds a munged value the sender strips it so the receiver re-munges a clean + target, round-tripping a munged tree exactly like rsync. */ entry->is_symlink = true; entry->stats = link_stats; entry->is_directory = false; - entry->link_target = - options->munge_links ? file_symlink_munge(link_target) : str_dup(link_target); + entry->link_target = str_dup(link_target); if (!entry->link_target) goto skip; + if (options->munge_links) + file_symlink_unmunge(entry->link_target); goto apply_filters; regular: @@ -798,30 +821,57 @@ static File* dirs_file_for_entry(DirectoryScanner* scanner, const char* entry) { return NULL; } struct stat effective = link_stats; + bool emit_symlink = false; + char* symlink_target = NULL; if (S_ISLNK(link_stats.st_mode)) { - /* A symlink is transferred (following its referent) only when a link - resolution option is active, mirroring the regular scanner. */ - bool resolve = scanner->options.follow_symlinks || scanner->options.copy_links || - scanner->options.safe_links || scanner->options.copy_unsafe_links; - if (!resolve || stat(abs_path, &effective) != 0) { + /* Resolve the listed symlink with the same precedence as the recursive + scanner: dereference or carry the link. */ + char link_target[4096]; + LinkAction action = + scanner_link_action(&scanner->options, abs_path, entry, link_target, sizeof(link_target)); + if (action == LINK_ACTION_SKIP || action == LINK_ACTION_SKIP_PROTECTED) { free(abs_path); return NULL; } + if (action == LINK_ACTION_DEREF) { + if (stat(abs_path, &effective) != 0) { + free(abs_path); + return NULL; + } + } else { + emit_symlink = true; + symlink_target = str_dup(link_target); + if (!symlink_target) { + free(abs_path); + scanner->failed = true; + return NULL; + } + if (scanner->options.munge_links) + file_symlink_unmunge(symlink_target); + } } bool is_dir = S_ISDIR(effective.st_mode); bool is_file = S_ISREG(effective.st_mode); - if (!is_dir && !is_file) { + if (!emit_symlink && !is_dir && !is_file) { + free(symlink_target); free(abs_path); return NULL; } File* file = file_create(abs_path); free(abs_path); if (!file) { + free(symlink_target); scanner->failed = true; return NULL; } - file->is_dir = is_dir; - file->data->size = is_file ? (unsigned long long)effective.st_size : 0; + if (emit_symlink) { + file->is_symlink = true; + file->symlink_target = symlink_target; + symlink_target = NULL; + } else { + file->is_dir = is_dir; + file->data->size = is_file ? (unsigned long long)effective.st_size : 0; + } if (scanner->relative_mode) { file->send_path = str_dup(entry); if (!file->send_path) { @@ -978,8 +1028,14 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { continue; ScannerEntry inspected; - int inspection = scanner_inspect_entry(&scanner->options, scanner->current_path, - scanner->current_path, entry->d_name, &inspected); + char* link_rel = child_rel_path(scanner->current_rel, entry->d_name); + if (!link_rel) { + scanner->failed = true; + break; + } + int inspection = scanner_inspect_entry(&scanner->options, scanner->current_path, link_rel, + entry->d_name, &inspected); + free(link_rel); if (inspection < 0) { scanner->failed = true; break; @@ -1084,9 +1140,16 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { rel_copy = NULL; } /* --devices/--specials: a device/FIFO/socket entry marked for preservation - becomes a node to recreate (is_special, no data, rdev captured). */ - scanner_prepare_special(scanner->options.preserve_devices, scanner->options.preserve_specials, - file, &stats); + becomes a node to recreate (is_special, no data, rdev captured); an + unrequested non-regular entry is skipped (rsync default). */ + ScannerSpecial special = scanner_prepare_special(scanner->options.preserve_devices, + scanner->options.preserve_specials, + scanner->options.copy_devices, file, &stats); + if (special == SCANNER_SPECIAL_SKIP) { + free(rel_copy); + file_destroy(file); + continue; + } if (scanner->options.hardlinks && S_ISREG(stats.st_mode)) scanner_assign_hardlink(scanner, scanner->options.hardlinks, file, &stats); if (scanner->options.use_metadata) @@ -1335,7 +1398,7 @@ static void scan_root_entry(const ScannerOptions* options, const FilterNode* roo ParallelScanner* ps) { ScannerEntry inspected; int inspection = - scanner_inspect_entry(options, root_directory, root_directory, entry->d_name, &inspected); + scanner_inspect_entry(options, root_directory, entry->d_name, entry->d_name, &inspected); if (inspection < 0) { ps->failed = true; return; @@ -1415,7 +1478,13 @@ static void scan_root_entry(const ScannerOptions* options, const FilterNode* roo file->send_path = rel; rel = NULL; } - scanner_prepare_special(options->preserve_devices, options->preserve_specials, file, &st); + ScannerSpecial special = scanner_prepare_special( + options->preserve_devices, options->preserve_specials, options->copy_devices, file, &st); + if (special == SCANNER_SPECIAL_SKIP) { + free(rel); + file_destroy(file); + return; + } if (options->hardlinks && S_ISREG(st.st_mode)) { int gid; bool is_first; diff --git a/src/client/usage.c b/src/client/usage.c index b2bd9b4..c9e74e4 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -277,11 +277,11 @@ void print_usage(void) { printf(" Local receiver policy: never sent to the peer, off by default\n"); printf(" -l, --links Copy symlinks as symlinks\n"); printf(" -L, --copy-links Transform symlinks into referent files\n"); - printf(" --safe-links Skip symlinks that point outside transfer tree\n"); - printf(" --copy-unsafe-links Only transform unsafe symlinks into referent files\n"); + printf(" --safe-links Skip symlinks whose target points outside the tree\n"); + printf(" --copy-unsafe-links Copy unsafe symlinks (outside tree) as referent files\n"); printf(" -k, --copy-dirlinks Transform symlinks to directories into real dirs\n"); printf(" -K, --keep-dirlinks Keep an existing symlink-to-dir as that dir\n"); - printf(" --munge-links Munge symlink targets on the wire (sender)\n"); + printf(" --munge-links Munge stored symlink targets (/rsyncd-munged/) on the receiver\n"); printf(" -H, --hard-links Preserve hard-link relationships across the transfer\n"); printf(" -S, --sparse Handle sparse files efficiently\n"); printf( @@ -289,8 +289,7 @@ void print_usage(void) { printf( " --devices Recreate device nodes on the destination (privileged; skipped when\n"); printf(" the receiver lacks CAP_MKNOD)\n"); - printf(" --specials Recreate special files (FIFOs) on the destination (sockets " - "skipped)\n"); + printf(" --specials Recreate special files (FIFOs, sockets) on the destination\n"); printf(" --copy-devices Copy a source device's content as a regular file instead\n"); printf(" --write-devices Write received data into an existing destination device node\n"); printf(" --inplace Update files in-place (no temp+rename)\n"); diff --git a/src/shared/file.c b/src/shared/file.c index d3af701..4ecc007 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -415,10 +415,69 @@ bool file_get_trust_sender(void) { return file_trust_sender; } -/* True when `target` is a lexical symlink target that can never escape the - * receive root once created beneath it: relative (not absolute) and containing - * no ".." path component. Used by --munge-links' sender-side containment: an - * escaping target is never transmitted (the entry is skipped/contained). */ +/* rsync 3.4.1 unsafe_symlink(): true when `target` (the link's destination + * string) points outside the transfer tree rooted at the symlink's own + * location. `link_path` is the symlink's path relative to the top of the + * transfer (including its name). This is a purely lexical test matching + * rsync's util1.c: absolute/empty targets are always unsafe; leading "../" + * components are counted against the symlink's own directory depth; a ".." + * that would climb above the transfer root is unsafe. rsync 3.4.1 additionally + * rejects any INTERNAL "/../" component and a trailing "/..". */ +bool file_symlink_unsafe(const char* target, const char* link_path) { + if (!target || target[0] == '\0' || target[0] == '/') + return true; + const char* rest = target; + while (strncmp(rest, "../", 3) == 0) { + rest += 3; + while (*rest == '/') + rest++; + } + if (strstr(rest, "/../") != NULL) + return true; + size_t target_len = strlen(target); + if (target_len > 3 && strcmp(&target[target_len - 3], "/..") == 0) + return true; + + int depth = 0; + const char* name; + const char* slash; + const char* src = link_path ? link_path : ""; + for (name = src; (slash = strchr(name, '/')) != NULL; name = slash + 1) { + if (*name == '.' && (name[1] == '/' || (name[1] == '.' && name[2] == '/'))) { + if (name[1] == '.') + depth = 0; + } else { + depth++; + } + while (slash[1] == '/') + slash++; + } + if (*name == '.' && name[1] == '.' && name[2] == '\0') + depth = 0; + + for (name = target; (slash = strchr(name, '/')) != NULL; name = slash + 1) { + if (*name == '.' && (name[1] == '/' || (name[1] == '.' && name[2] == '/'))) { + if (name[1] == '.') { + if (--depth < 0) + return true; + } + } else { + depth++; + } + while (slash[1] == '/') + slash++; + } + if (*name == '.' && name[1] == '.' && name[2] == '\0') + depth--; + return depth < 0; +} + +/* Strict lexical helper: true when `target` is relative (not absolute) and + * contains no ".." component at all, so it can never escape the directory it + * is created in. This is stricter than rsync's unsafe_symlink() (which allows + * an in-tree ".."); the scanner/receiver use file_symlink_unsafe()/--safe-links + * for rsync parity, and this helper is retained for callers that want the + * ".."-free guarantee. */ bool file_symlink_target_contained(const char* target) { if (!target || target[0] == '\0' || target[0] == '/') return false; @@ -449,8 +508,9 @@ bool file_symlink_unmunge(char* target) { return true; } -/* Owned copy of `target` prefixed with SYMLINK_MUNGE_PREFIX (the sender-side - * --munge-links rewriting). Returns NULL on allocation failure. */ +/* Owned copy of `target` prefixed with SYMLINK_MUNGE_PREFIX (the receiver-side + * --munge-links rewriting, matching rsync's receiver). Returns NULL on + * allocation failure. */ char* file_symlink_munge(const char* target) { if (!target) return NULL; @@ -471,23 +531,14 @@ char* file_symlink_munge(const char* target) { * the target is ever followed. The final component is never dereferenced: an * existing non-directory entry at `path` is unlinked by name before the link is * placed; an existing directory there is left untouched (returns false, so a - * caller can treat it as a collision). As a receiver-side trust-boundary - * invariant, `target` must be file_symlink_target_contained() (relative and - * ".."-free): an absolute or escaping target is rejected outright (returns - * false) so a malicious sender can never materialize a symlink that points - * outside the receive root. */ + * caller can treat it as a collision). The link VALUE `target` is copied + * verbatim, matching rsync -l (which stores absolute and ".."-bearing targets + * as-is); target policy is the caller's job -- the scanner applies + * --safe-links/--copy-unsafe-links, and the receiver applies --munge-links. + * The PLACEMENT path is always confined below the authorized root. */ bool file_symlink_at_secure(const char* path, const char* target) { - /* The link itself (`path`) is always kept below the authorized root. The - TARGET may point anywhere: normally only a contained (relative, ".."-free) - target is permitted so a malicious sender can never plant a symlink that - later dereferences outside the root. Under --trust-sender that target - containment check is relaxed (the receiver trusts the sender and copies the - link verbatim, matching rsync -l), but path/leaf confinement is never - disabled, so the link still cannot be placed outside the tree. */ if (!path || !target || has_path_traversal(path)) return false; - if (!file_trust_sender && !file_symlink_target_contained(target)) - return false; char* leaf = NULL; int parent_fd = file_open_secure_parent(path, &leaf, true); if (parent_fd < 0) diff --git a/src/shared/file.h b/src/shared/file.h index 5333bc7..4ccc2ad 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -44,12 +44,20 @@ int file_open_for_read(const char* path); bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse); -/* Symlink trust-boundary helpers (Phase 4, symlink wave). --munge-links - * sender-side marker: every transmitted symlink target is prefixed with this - * while the flag is on; the receiver strips it to restore the real target. */ -#define SYMLINK_MUNGE_PREFIX "#SYMLINK/" +/* Symlink trust-boundary helpers (Phase 4, symlink wave; rsync parity). + * --munge-links is a RECEIVER-side rewrite: rsync prefixes every stored symlink + * target with this marker, making the link unusable while the referenced + * directory does not exist. A SENDER receiving a munged source strips it back + * off before transmitting (so a munged tree round-trips through the receiver's + * re-munging). */ +#define SYMLINK_MUNGE_PREFIX "/rsyncd-munged/" char* file_symlink_munge(const char* target); +/* rsync 3.4.1 unsafe_symlink(): true when `target` escapes the transfer tree + * rooted at `link_path` (the symlink's transfer-relative path incl. its name). + * Absolute/empty targets and targets climbing above the transfer root (via + * "..") are unsafe, as are internal "/../" components and trailing "/..". */ +bool file_symlink_unsafe(const char* target, const char* link_path); /* True when a lexical target is relative and contains no ".." component, so it * can never escape the receive root once created beneath it. */ bool file_symlink_target_contained(const char* target); @@ -57,8 +65,9 @@ bool file_symlink_target_contained(const char* target); * returns true when a marker was removed. */ bool file_symlink_unmunge(char* target); /* Create a symlink at `path` -> `target`, confined below the authorized root - * (O_NOFOLLOW parent walk, symlinkat; the target is never followed). Returns - * false when a directory already occupies `path`. */ + * (O_NOFOLLOW parent walk, symlinkat; the target is never followed). The link + * value is copied verbatim (rsync -l); only the placement path is confined. + * Returns false when a directory already occupies `path`. */ bool file_symlink_at_secure(const char* path, const char* target); /* --keep-dirlinks (-K) receiver process-wide policy: allow an in-root existing * symlink-to-directory to be followed as a directory. */ diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 39cd387..2776201 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -358,14 +358,6 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons log_message(LOG_LEVEL_ERROR, "Special node has no device/FIFO/socket mode"); return FILE_SAVE_ERROR; } - if (is_sock) { - /* No standard filesystem call recreates a socket; best-effort unsupported. */ - char* escaped_path = output_escape(file->path, log_get_8_bit_output()); - log_message(LOG_LEVEL_WARNING, "socket not recreated: %s (unsupported; skipped)", - escaped_path ? escaped_path : ""); - free(escaped_path); - return FILE_SAVE_SKIPPED; - } if (is_char || is_blk) { if (!config || !config->preserve_devices) return FILE_SAVE_SKIPPED; @@ -383,7 +375,10 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons free(escaped_path); return FILE_SAVE_SKIPPED; } - } else if (is_fifo) { + } else if (is_fifo || is_sock) { + /* FIFOs and unix sockets are recreated by --specials. mknod(S_IFSOCK) + works unprivileged on Linux (the node carries no live socket), so unlike + a socket bound to a live fd it can be materialized. */ if (!config || !config->preserve_specials) return FILE_SAVE_SKIPPED; } @@ -433,9 +428,12 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons } else if (is_blk) { create_mode = S_IFBLK; rdev = makedev((unsigned)file->rdev_major, (unsigned)file->rdev_minor); + } else if (is_sock) { + create_mode = S_IFSOCK; } else { create_mode = S_IFIFO; } + const char* node_kind = (is_char || is_blk) ? "device" : (is_fifo ? "FIFO" : "socket"); /* The creation permission bits come from the source only under -p/--perms; * otherwise a safe default (0644, group/other write never granted) keeps an * unprivileged no--p run from materializing a world-writable node. */ @@ -450,7 +448,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons struct stat st; if (fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) == 0 && ((is_char && S_ISCHR(st.st_mode)) || (is_blk && S_ISBLK(st.st_mode)) || - (is_fifo && S_ISFIFO(st.st_mode)))) { + (is_fifo && S_ISFIFO(st.st_mode)) || (is_sock && S_ISSOCK(st.st_mode)))) { close(parent_fd); free(leaf); free(destination); @@ -458,7 +456,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons } char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "refusing to replace existing entry with %s: %s (skipped)", - is_fifo ? "FIFO" : "device", escaped_path ? escaped_path : ""); + node_kind, escaped_path ? escaped_path : ""); free(escaped_path); } else if (errno == EPERM || errno == EACCES) { /* Missing CAP_MKNOD / parent write permission: the environment cannot @@ -467,14 +465,12 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons log_message(LOG_LEVEL_WARNING, "skipping %s: cannot create %s node (%s)\n" " --devices/--specials node creation needs privilege (CAP_MKNOD)", - escaped_path ? escaped_path : "", is_fifo ? "FIFO" : "device", - strerror(errno)); + escaped_path ? escaped_path : "", node_kind, strerror(errno)); free(escaped_path); } else { char* escaped_path = output_escape(file->path, log_get_8_bit_output()); - log_message(LOG_LEVEL_WARNING, "failed to create %s %s: %s (skipped)", - is_fifo ? "FIFO" : "device", escaped_path ? escaped_path : "", - strerror(errno)); + log_message(LOG_LEVEL_WARNING, "failed to create %s %s: %s (skipped)", node_kind, + escaped_path ? escaped_path : "", strerror(errno)); free(escaped_path); } close(parent_fd); @@ -710,26 +706,23 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi char* link_path = path_cat(root_directory, file->path); if (!link_path) return FILE_SAVE_ERROR; - /* Restore the real target by stripping the sender's --munge-links marker. - Only unmunge when the policy was negotiated: a plain -l run must preserve - a source symlink whose target genuinely begins with the marker verbatim. */ + /* The link value is stored verbatim (rsync -l parity: absolute and + ".."-bearing targets are preserved; the scanner's --safe-links / + --copy-unsafe-links decide which links are sent at all). --munge-links + is a RECEIVER-side rewrite: the stored target is prefixed with + /rsyncd-munged/, making the link unusable while the referenced directory + does not exist -- exactly as rsync's receiver munges. Only the link's + own placement path is confined below the receive root. */ + bool munge = config && config->munge_links; char* target = str_dup(file->symlink_target); bool ok = target != NULL; - if (ok && config && config->munge_links) - file_symlink_unmunge(target); - /* Receiver-side trust boundary (independent of the sender): a target that - could escape the receive root (absolute, or relative-with-"..") is never - materialized. It is contained (the entry is skipped) rather than failing - the whole transfer, so a hostile sender can inject a broken symlink but - can never redirect it outside the root. --trust-sender deliberately - relaxes this receiver-side re-validation: a trusted sender's escaping - symlink target is copied verbatim (rsync -l parity). The low-level - leaf/destination confinement in file_symlink_at_secure still ensures the - link itself is placed inside the authorized root. */ - if (ok && !file_get_trust_sender() && !file_symlink_target_contained(target)) - ok = false; + if (ok && munge) { + char* munged = file_symlink_munge(target); + free(target); + target = munged; + ok = target != NULL; + } if (!ok) { - /* Skip the escaping/empty target (contained) rather than abort. */ free(target); free(link_path); return FILE_SAVE_SKIPPED; diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index cf2f46f..04f0d29 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -92,54 +92,63 @@ class TestDeviceSpecial: assert stat.S_ISFIFO(os.stat(os.path.join(received, "pipe.fifo")).st_mode) @pytest.mark.ci - def test_specials_socket_source_skipped_safely(self): - """A socket cannot be recreated by any standard filesystem call, so - --specials must skip it with a note and still complete the run (the - adjacent regular file transfers normally; no socket node appears).""" + def test_specials_recreates_socket(self, shared_server): + """--specials recreates a unix-domain socket with mknod(S_IFSOCK), which + Linux permits unprivileged; the adjacent regular file still transfers.""" self._setup() sock_path = os.path.join(DEVICE_SOURCE, "source.sock") s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - server, port = _start_captured_server() try: s.bind(sock_path) result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, - flags=["--specials"], port=port) + flags=["--specials"], port=shared_server.port) finally: s.close() - out, err = _stop_captured_server(server) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) with open(os.path.join(received, "plain.txt")) as f: assert f.read() == "regular content\n" - assert not os.path.lexists(os.path.join(received, "source.sock")), ( - "socket source must be skipped, not materialized" - ) - assert "socket not recreated" in (out + err), ( - f"receiver did not log the documented socket skip: out={out!r} err={err!r}" + dest_sock = os.path.join(received, "source.sock") + assert os.path.lexists(dest_sock), "socket source was not recreated" + assert stat.S_ISSOCK(os.lstat(dest_sock).st_mode), ( + "socket source must be recreated as a socket node" ) + @pytest.mark.ci + def test_special_default_skips_non_regular(self, shared_server): + """Without --specials, rsync skips a FIFO/socket as a non-regular file; + FastSync must skip it (never copy it as an empty regular file).""" + self._setup() + os.mkfifo(os.path.join(DEVICE_SOURCE, "skip.fifo")) + sock_path = os.path.join(DEVICE_SOURCE, "skip.sock") + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + s.bind(sock_path) + result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, flags=[], port=shared_server.port) + finally: + s.close() + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + assert not os.path.lexists(os.path.join(received, "skip.fifo")) + assert not os.path.lexists(os.path.join(received, "skip.sock")) + with open(os.path.join(received, "plain.txt")) as f: + assert f.read() == "regular content\n" + @pytest.mark.ci @pytest.mark.parametrize("flags", [["--copy-devices"], ["--copy-devices", "--sendfile"]]) - def test_copy_devices_fifo_becomes_regular_file(self, shared_server, flags): - """--copy-devices treats a special source as an ordinary regular-file - copy: a FIFO (st_size 0) becomes a zero-length REGULAR file on the - destination (never a FIFO, never a hang), and the run succeeds. The - --sendfile variant previously blocked forever in the sendfile open(); - the non-regular source now falls back to the buffered read path, so it - must complete within the bounded-time assertion below.""" + def test_copy_devices_skips_fifo_without_specials(self, shared_server, flags): + """rsync's --copy-devices applies to device nodes only; a FIFO/socket is + a non-regular entry and is skipped unless --specials is also given. In + particular it must never hang in the sendfile open().""" self._setup() os.mkfifo(os.path.join(DEVICE_SOURCE, "device_copy.fifo")) result, dur = run_client(DEVICE_SOURCE, DEVICE_DEST, flags=flags, port=shared_server.port) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) - copied = os.path.join(received, "device_copy.fifo") - assert os.path.lexists(copied), "copy-devices source was not transferred" - st = os.lstat(copied) - assert stat.S_ISREG(st.st_mode), ( - f"copy-devices must produce a regular file, got mode {oct(st.st_mode)}" + assert not os.path.lexists(os.path.join(received, "device_copy.fifo")), ( + "a FIFO under --copy-devices alone must be skipped, not materialized" ) - assert st.st_size == 0, f"expected a size-bounded 0-byte copy, got {st.st_size}" assert dur < 60, f"{' '.join(flags)} hung on a FIFO source" def test_write_devices_non_crash(self, shared_server): @@ -220,6 +229,24 @@ class TestDeviceSpecial: assert stat.S_ISCHR(st.st_mode) assert os.major(st.st_rdev) == 1 and os.minor(st.st_rdev) == 3 + @pytest.mark.skipif(os.geteuid() != 0, reason="requires root to create device nodes") + def test_copy_devices_copies_device_as_regular(self, shared_server): + """Root-only: --copy-devices copies a device's content into an ordinary + regular file instead of recreating the node. /dev/null (1,3) has size 0, + so the result is a 0-byte REGULAR file.""" + self._setup() + src_dev = os.path.join(DEVICE_SOURCE, "copieddev") + os.mknod(src_dev, stat.S_IFCHR | 0o666, os.makedev(1, 3)) + result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, + flags=["-a", "--copy-devices"], port=shared_server.port) + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + st = os.lstat(os.path.join(received, "copieddev")) + assert stat.S_ISREG(st.st_mode), ( + f"--copy-devices must produce a regular file, got mode {oct(st.st_mode)}" + ) + assert st.st_size == 0 + def test_m_remove_source_files_keeps_recreated_fifo(self, shared_server): """--threads --remove-source-files --specials: a recreated FIFO must NOT be acknowledged as a removable source (its outcome must not shift the @@ -5150,8 +5177,10 @@ class TestSymlinkTrust: os.symlink("realfile.txt", os.path.join(source, "link_file")) os.symlink("realdir", os.path.join(source, "link_dir")) - result, _ = run_client(source, dest, flags=["-k"], port=shared_server.port) - assert result.returncode == 0, f"-k failed: {(result.stderr or result.stdout)[:300]}" + # -k only dereferences directory symlinks; file symlinks need -l to be + # carried as symlinks (rsync skips them otherwise). + result, _ = run_client(source, dest, flags=["-l", "-k"], port=shared_server.port) + assert result.returncode == 0, f"-l -k failed: {(result.stderr or result.stdout)[:300]}" received = get_dest_received_dir(dest, source) # link -> realdir dereferences into a real directory tree... @@ -5191,9 +5220,13 @@ class TestSymlinkTrust: # ... and the file is written beneath it, through to the referent dir. assert os.path.isfile(os.path.join(parent, "realdir", "file.txt")) - def test_munge_links_unmunged_target_and_containment(self, shared_server): - source = os.path.join(TEST_DATA_DIR, "symlink_trust_munge") - dest = os.path.join(TEST_DATA_DIR, "symlink_trust_munge_dst") + @pytest.mark.ci + def test_munge_links_prefixes_targets(self, shared_server): + # rsync's --munge-links is a RECEIVER-side rewrite: every stored target + # gets the /rsyncd-munged/ prefix, making the link unusable while that + # directory does not exist. + source = os.path.join(TEST_DATA_DIR, "symlink_munge") + dest = os.path.join(TEST_DATA_DIR, "symlink_munge_dst") clean_dir(source) clean_dir(dest) with open(os.path.join(source, "a.txt"), "wb") as f: @@ -5207,19 +5240,15 @@ class TestSymlinkTrust: assert result.returncode == 0, f"--munge-links failed: {(result.stderr or result.stdout)[:300]}" received = get_dest_received_dir(dest, source) - # The safe symlink is created with its correct (unmunged) target. - good = os.path.join(received, "good") - assert os.path.islink(good) - assert os.readlink(good) == "a.txt" - # A target that would escape the receive root is contained (skip: never - # transmitted, so nothing is created at the destination). - assert not os.path.lexists(os.path.join(received, "abs_escape")) - assert not os.path.lexists(os.path.join(received, "dotdot_escape")) + assert os.readlink(os.path.join(received, "good")) == "/rsyncd-munged/a.txt" + assert os.readlink(os.path.join(received, "abs_escape")) == "/rsyncd-munged//etc/passwd" + assert os.readlink(os.path.join(received, "dotdot_escape")) == "/rsyncd-munged/../../escape" assert os.path.isfile(os.path.join(received, "a.txt")) + @pytest.mark.ci def test_links_copies_symlinks_as_symlinks(self, shared_server): - source = os.path.join(TEST_DATA_DIR, "symlink_trust_links") - dest = os.path.join(TEST_DATA_DIR, "symlink_trust_links_dst") + source = os.path.join(TEST_DATA_DIR, "symlink_links") + dest = os.path.join(TEST_DATA_DIR, "symlink_links_dst") clean_dir(source) clean_dir(dest) os.makedirs(os.path.join(source, "realdir")) @@ -5238,48 +5267,157 @@ class TestSymlinkTrust: assert os.path.islink(os.path.join(received, "ld")) assert os.readlink(os.path.join(received, "ld")) == "realdir" - def test_receiver_contains_absolute_target_even_without_munge(self, shared_server): - # The trust boundary is symmetric and enforced receiver-side: a plain -l - # (no --munge-links) run must refuse to materialize an out-of-root - # absolute symlink target, while still copying a legitimate in-root one. - source = os.path.join(TEST_DATA_DIR, "symlink_trust_abs") - dest = os.path.join(TEST_DATA_DIR, "symlink_trust_abs_dst") + @pytest.mark.ci + def test_links_preserves_absolute_and_dotdot_targets(self, shared_server): + # rsync -l parity: -l stores a symlink target verbatim, including an + # absolute target and an in-tree ".." target (no silent drop). + source = os.path.join(TEST_DATA_DIR, "symlink_links_verbatim") + dest = os.path.join(TEST_DATA_DIR, "symlink_links_verbatim_dst") clean_dir(source) clean_dir(dest) with open(os.path.join(source, "a.txt"), "wb") as f: f.write(b"a\n") + os.makedirs(os.path.join(source, "sub")) os.symlink("a.txt", os.path.join(source, "good")) os.symlink("/etc/passwd", os.path.join(source, "unsafe_abs")) + os.symlink("../a.txt", os.path.join(source, "sub", "up")) result, _ = run_client(source, dest, flags=["-l"], port=shared_server.port) assert result.returncode == 0, f"-l failed: {(result.stderr or result.stdout)[:300]}" - received = get_dest_received_dir(dest, source) - good = os.path.join(received, "good") - assert os.path.islink(good) - assert os.readlink(good) == "a.txt" - # The absolute (non-contained) target was not materialized at the dest. - assert not os.path.lexists(os.path.join(received, "unsafe_abs")) + assert os.readlink(os.path.join(received, "good")) == "a.txt" + assert os.readlink(os.path.join(received, "unsafe_abs")) == "/etc/passwd" + assert os.readlink(os.path.join(received, "sub", "up")) == "../a.txt" - def test_links_does_not_strip_munge_prefix_without_munge(self, shared_server): - # A source symlink whose target genuinely begins with the #SYMLINK/ marker - # must round-trip verbatim under plain -l: the receiver only unmunges when - # the negotiated --munge-links policy is on, never unconditionally. - source = os.path.join(TEST_DATA_DIR, "symlink_trust_prefix") - dest = os.path.join(TEST_DATA_DIR, "symlink_trust_prefix_dst") + @pytest.mark.ci + def test_safe_links_keeps_safe_skips_unsafe(self, shared_server): + # --safe-links keeps symlinks that stay inside the transfer tree (even + # with a ".." that does not climb out) and drops absolute / escaping / + # internally-".."-bearing targets. + source = os.path.join(TEST_DATA_DIR, "symlink_safe") + dest = os.path.join(TEST_DATA_DIR, "symlink_safe_dst") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "a.txt"), "wb") as f: + f.write(b"a\n") + os.makedirs(os.path.join(source, "sub")) + os.symlink("a.txt", os.path.join(source, "safe_rel")) + os.symlink("../a.txt", os.path.join(source, "sub", "up")) + os.symlink("/etc/passwd", os.path.join(source, "abs")) + os.symlink("../outside.txt", os.path.join(source, "esc")) + os.symlink("sub/../a.txt", os.path.join(source, "internal")) + + result, _ = run_client(source, dest, flags=["-l", "--safe-links"], + port=shared_server.port) + assert result.returncode == 0, f"--safe-links failed: {(result.stderr or result.stdout)[:300]}" + received = get_dest_received_dir(dest, source) + assert os.readlink(os.path.join(received, "safe_rel")) == "a.txt" + assert os.readlink(os.path.join(received, "sub", "up")) == "../a.txt" + for unsafe in ("abs", "esc", "internal"): + assert not os.path.lexists(os.path.join(received, unsafe)), ( + f"{unsafe} must be skipped by --safe-links" + ) + + @pytest.mark.ci + def test_safe_links_protects_dest_from_delete(self): + # rsync counts an unsafe link ignored by --safe-links as present in the + # transfer, so its destination mirror survives --delete. FastSync must + # not delete it (no silent data loss). Own server: deletion needs + # --allow-delete, which the shared session server does not grant. + source = os.path.join(TEST_DATA_DIR, "symlink_safe_delete") + dest = os.path.join(TEST_DATA_DIR, "symlink_safe_delete_dst") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "keep.txt"), "wb") as f: + f.write(b"keep\n") + os.symlink("/etc/passwd", os.path.join(source, "unsafe_abs")) + + with ServerManager() as server: + server.start(extra_args=["--allow-delete"]) + received = get_dest_received_dir(dest, source) + os.makedirs(received, exist_ok=True) + mirror = os.path.join(received, "unsafe_abs") + with open(mirror, "wb") as f: + f.write(b"existing destination data\n") + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as f: + f.write(b"extra\n") + + result, _ = run_client(source, dest, flags=["-l", "--safe-links", "--delete"], + port=server.port) + assert result.returncode == 0, ( + f"--delete --safe-links failed: {(result.stderr or result.stdout)[:300]}" + ) + assert os.path.exists(mirror), ( + "a destination mirror of a --safe-links-skipped link must survive --delete" + ) + assert not os.path.exists(extra), "a genuine extra must still be deleted" + + @pytest.mark.ci + def test_copy_unsafe_links_derefs_only_unsafe(self, shared_server): + # --copy-unsafe-links keeps safe symlinks and dereferences unsafe ones + # (absolute or escaping) into regular files. + source = os.path.join(TEST_DATA_DIR, "symlink_copy_unsafe") + dest = os.path.join(TEST_DATA_DIR, "symlink_copy_unsafe_dst") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "a.txt"), "wb") as f: + f.write(b"a\n") + with open(os.path.join(source, "refer.txt"), "wb") as f: + f.write(b"refer\n") + external = os.path.join(TEST_DATA_DIR, "symlink_copy_unsafe_external.txt") + with open(external, "wb") as f: + f.write(b"external\n") + os.symlink("a.txt", os.path.join(source, "safe_rel")) + os.symlink("refer.txt", os.path.join(source, "from_rel")) + os.symlink("../symlink_copy_unsafe_external.txt", os.path.join(source, "esc")) + os.symlink("/etc/hostname", os.path.join(source, "abs")) + + result, _ = run_client(source, dest, flags=["-l", "--copy-unsafe-links"], + port=shared_server.port) + assert result.returncode == 0, ( + f"--copy-unsafe-links failed: {(result.stderr or result.stdout)[:300]}" + ) + received = get_dest_received_dir(dest, source) + assert os.path.islink(os.path.join(received, "safe_rel")) + assert os.readlink(os.path.join(received, "safe_rel")) == "a.txt" + assert os.path.islink(os.path.join(received, "from_rel")), ( + "a safe symlink must be preserved, not dereferenced" + ) + assert os.readlink(os.path.join(received, "from_rel")) == "refer.txt" + # An escaping (..) symlink and an absolute symlink are both dereferenced + # into regular files holding the referent's content. + assert not os.path.islink(os.path.join(received, "esc")) + with open(os.path.join(received, "esc"), "rb") as f: + assert f.read() == b"external\n" + assert not os.path.islink(os.path.join(received, "abs")) + assert os.path.isfile(os.path.join(received, "abs")) + + def test_munge_prefix_roundtrip(self, shared_server): + # A source target that already begins with /rsyncd-munged/ round-trips: + # plain -l stores it verbatim, and --munge-links strips on the sender + # then re-munges on the receiver, yielding the same stored value. + source = os.path.join(TEST_DATA_DIR, "symlink_munge_roundtrip") + dest = os.path.join(TEST_DATA_DIR, "symlink_munge_roundtrip_dst") clean_dir(source) clean_dir(dest) with open(os.path.join(source, "realfile.txt"), "wb") as f: f.write(b"real\n") - os.symlink("#SYMLINK/realfile.txt", os.path.join(source, "prefixed")) + os.symlink("/rsyncd-munged/realfile.txt", os.path.join(source, "prefixed")) + os.symlink("#SYMLINK/realfile.txt", os.path.join(source, "oldmarker")) - result, _ = run_client(source, dest, flags=["-l"], port=shared_server.port) - assert result.returncode == 0, f"-l failed: {(result.stderr or result.stdout)[:300]}" - - received = get_dest_received_dir(dest, source) - prefixed = os.path.join(received, "prefixed") - assert os.path.islink(prefixed) - assert os.readlink(prefixed) == "#SYMLINK/realfile.txt" + for flags, oldmarker_target in ( + (["-l"], "#SYMLINK/realfile.txt"), + (["-l", "--munge-links"], "/rsyncd-munged/#SYMLINK/realfile.txt"), + ): + clean_dir(dest) + result, _ = run_client(source, dest, flags=flags, port=shared_server.port) + assert result.returncode == 0, ( + f"{' '.join(flags)} failed: {(result.stderr or result.stdout)[:300]}" + ) + received = get_dest_received_dir(dest, source) + assert os.readlink(os.path.join(received, "prefixed")) == "/rsyncd-munged/realfile.txt" + assert os.readlink(os.path.join(received, "oldmarker")) == oldmarker_target def _xattr_supported(path): """True when the filesystem hosting `path` supports user xattrs.""" try: diff --git a/tests/test_file.c b/tests/test_file.c index c41050b..ae101e1 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -518,6 +518,20 @@ static void test_file_symlink_helpers() { EXPECT_FALSE(file_symlink_target_contained("../escape")); EXPECT_FALSE(file_symlink_target_contained("a/../b")); EXPECT_FALSE(file_symlink_target_contained("")); + + /* rsync 3.4.1 unsafe_symlink(): absolute/empty are unsafe; ".." is measured + against the symlink's own transfer-relative directory depth. */ + EXPECT_TRUE(file_symlink_unsafe("/etc/passwd", "link")); + EXPECT_TRUE(file_symlink_unsafe("", "link")); + EXPECT_FALSE(file_symlink_unsafe("a.txt", "link")); + EXPECT_FALSE(file_symlink_unsafe("./a.txt", "link")); + EXPECT_FALSE(file_symlink_unsafe("../real.txt", "a/up1")); + EXPECT_FALSE(file_symlink_unsafe("../../real.txt", "a/b/up3")); + EXPECT_TRUE(file_symlink_unsafe("../../../outside", "a/b/esc")); + EXPECT_TRUE(file_symlink_unsafe("../outside", "esc")); + /* Internal /../ and a trailing /.. are rejected by rsync 3.4.1. */ + EXPECT_TRUE(file_symlink_unsafe("a/b/../real.txt", "norm")); + EXPECT_TRUE(file_symlink_unsafe("dir/..", "link")); } static void test_file_symlink_at_secure() { @@ -1123,6 +1137,47 @@ static void test_special_fifo_mode_never_group_other_writable() { umask(saved_umask); } +/* --specials recreates a unix-domain socket via mknod(S_IFSOCK), which Linux + * permits unprivileged. Without --specials the entry is skipped. */ +static void test_special_socket_recreated() { + const char* root = "test_special_sock_tmp"; + const char* sock = "test_special_sock_tmp/source.sock"; + unlink(sock); + rmdir(root); + EXPECT_EQ_INT(mkdir(root, 0700), 0); + + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + FileMetadata meta; + memset(&meta, 0, sizeof(meta)); + meta.mode = S_IFSOCK | 0600; + meta.uid = geteuid(); + meta.gid = getegid(); + + File* f = file_create("source.sock"); + EXPECT_NOT_NULL(f); + f->is_special = true; + f->metadata = &meta; + cfg->preserve_specials = true; + cfg->use_metadata = true; + EXPECT_EQ_INT(file_save_to_disk_full(root, f, cfg), FILE_SAVE_WRITTEN); + struct stat st; + EXPECT_EQ_INT(lstat(sock, &st), 0); + EXPECT_TRUE(S_ISSOCK(st.st_mode)); + + /* Without --specials the same entry is skipped, never a regular file. */ + unlink(sock); + cfg->preserve_specials = false; + EXPECT_EQ_INT(file_save_to_disk_full(root, f, cfg), FILE_SAVE_SKIPPED); + EXPECT_EQ_INT(lstat(sock, &st), -1); + + f->metadata = NULL; + file_destroy(f); + config_delete(cfg); + unlink(sock); + rmdir(root); +} + static void test_inplace_overwrite_truncates_shorter_payload() { const char* root = "test_inplace_trunc_tmp"; const char* path = "test_inplace_trunc_tmp/big.txt"; @@ -1297,34 +1352,27 @@ static void test_dir_entry_save_to_disk() { * 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. */ + * only the redundant list-level re-validation 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. */ -static void test_trust_sender_relaxes_symlink_target() { - const char* root = "test_trust_sender_root"; - const char* link = "test_trust_sender_root/escape_link"; +static void test_symlink_target_verbatim() { + const char* root = "test_symlink_verbatim_root"; + const char* link = "test_symlink_verbatim_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. */ + /* rsync -l parity: a symlink target is stored verbatim, absolute or not; the + scanner's --safe-links/--copy-unsafe-links is what filters links. */ 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")); + struct stat st; 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); @@ -1335,10 +1383,10 @@ static void test_trust_sender_relaxes_symlink_target() { } unlink(link); - /* Same relaxation through the real save funnel (file_save_to_disk_full). */ + /* The same through the real save funnel: verbatim by default. */ Config* config = config_create(); EXPECT_NOT_NULL(config); - const char* save_link = "test_trust_sender_root/save_link"; + const char* save_link = "test_symlink_verbatim_root/save_link"; unlink(save_link); File* sym = file_create("save_link"); @@ -1348,10 +1396,6 @@ static void test_trust_sender_relaxes_symlink_target() { 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)); @@ -1477,7 +1521,7 @@ void test_trust_sender() { 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_symlink_target_verbatim(); test_trust_sender_confines_hostile_paths(); test_trust_sender_authorized_root_confinement(); file_set_trust_sender(false); @@ -1877,6 +1921,7 @@ void test_file() { test_atomic_no_perms_preserves_destination_mode(); test_new_file_mode_never_group_other_writable(); test_special_fifo_mode_never_group_other_writable(); + test_special_socket_recreated(); test_inplace_overwrite_truncates_shorter_payload(); test_inplace_refuses_fifo_destination(); test_inplace_refuses_device_destination(); diff --git a/tests/test_server.c b/tests/test_server.c index a43e605..26a2237 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -818,10 +818,24 @@ static void test_special_socket_path_log_escaped() { set_log_level(LOG_LEVEL_WARNING); log_set_8_bit_output(false); + const char* root = "test_special_sock_escape_root"; + const char* existing = "test_special_sock_escape_root/evil\npath"; + unlink(existing); + rmdir(root); + EXPECT_EQ_INT(mkdir(root, 0700), 0); + FILE* planted = fopen(existing, "wb"); + EXPECT_NOT_NULL(planted); + fclose(planted); + FILE* capture = tmpfile(); EXPECT_NOT_NULL(capture); log_set_file(capture); + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + cfg->preserve_specials = true; + cfg->use_metadata = true; + File* file = file_create("evil\npath"); EXPECT_NOT_NULL(file); file->is_special = true; @@ -829,7 +843,9 @@ static void test_special_socket_path_log_escaped() { EXPECT_NOT_NULL(file->metadata); file->metadata->mode = S_IFSOCK | 0644; - FileSaveResult result = file_save_to_disk_full("/tmp/dst", file, NULL); + /* A non-matching entry already occupies the path: the socket creation is + refused and the warning must escape the path's control byte. */ + FileSaveResult result = file_save_to_disk_full(root, file, cfg); EXPECT_EQ_INT(result, FILE_SAVE_SKIPPED); fflush(capture); @@ -841,8 +857,11 @@ static void test_special_socket_path_log_escaped() { log_set_file(NULL); fclose(capture); file_destroy(file); + config_delete(cfg); + unlink(existing); + rmdir(root); - EXPECT_NOT_NULL(strstr(output, "socket not recreated: evil\\#012path")); + EXPECT_NOT_NULL(strstr(output, "refusing to replace existing entry with socket: evil\\#012path")); } /* B1: a client-planted FIFO at the destination must not block the receiver's