diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index f88c863..006efbc 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -255,7 +255,7 @@ why plain `--append` works on the normal atomic path, not only with `--inplace`. | `-U`, `--atimes` | Preserve access times | ✅ Implemented | Captures the source access time (from the scanner's pre-read stat, so it is not clobbered by reading the file for transfer) and transmits it over the wire; the receiver restores it together with the mtime via `futimens`/`utimensat`. Implies metadata transmission (the times travel inside the `-M` metadata payload), but does not enable ownership application (that stays opt-in via the identity flags). Wire: new `atime` fields on the metadata frame + a `preserve_atimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** | | `-N`, `--crtimes` | Preserve create times | ⚠️ Partial | Captures the source birth time via `statx(STATX_BTIME)` on Linux and transmits it (recorded as a wire field), but there is **no portable way to set a birth time** (`utimensat` can only set atime/mtime), so the receiver explicitly does NOT apply it: it logs a debug note and continues — never failing the transfer and never pretending it worked. On platforms without `statx` it parses as a documented no-op (flag accepted; nothing is captured). Implies metadata transmission. Wire: new `crtime` fields + a `preserve_crtimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** (see the Phase-4 metadata-time notes) | | `-O`, `--omit-dir-times` | Omit dirs from --times | 🔄 Compatibility No-op | Accepted and parsed for CLI compatibility, and the config boolean crosses the wire, but it has **no effect**: FastSync never preserves directory mtimes in the first place (directories are created via `mkdir` with no metadata, a documented divergence under `-d`/recursive), so there is nothing for an "omit" to suppress. It never breaks a normal run | -| `-J`, `--omit-link-times` | Omit symlinks from --times | 🔄 Compatibility No-op | Accepted and parsed for CLI compatibility, and the config boolean crosses the wire, but it has **no effect**: FastSync never sets symlink times (`-l`/`--links` still only includes symlinks without transmitting a target; `--copy-links` dereferences), so there is nothing for an "omit" to suppress. It never breaks a normal run | +| `-J`, `--omit-link-times` | Omit symlinks from --times | 🔄 Compatibility No-op | Accepted and parsed for CLI compatibility, and the config boolean crosses the wire, but it has **no effect**: FastSync never sets symlink times (`-l`/`--links` copies symlinks as symlinks but the receiver does not apply timestamps/owner to symlink entries), so there is nothing for an "omit" to suppress. It never breaks a normal run | | `--super` | Receiver attempts super-user activities | ❌ Not Implemented | | | `--fake-super` | Store/recover privileged attrs via xattrs | ❌ Not Implemented | | | `--open-noatime` | Avoid changing access time when opening files | ✅ Implemented | Sender-side policy: the sender opens source files with `O_NOATIME` (Linux) when reading them for transfer, so the open/read does NOT bump the source's on-disk access time. Degrades safely when `O_NOATIME` is unavailable (not defined) or refused (`EPERM`, since it needs `CAP_FOWNER` or file ownership): the code falls back to a normal open, so the data always transfers — only the atime-bump is skipped. It does not itself capture/preserve atime; it only avoids modifying it. **Client-only, never crosses the wire.** Exposed as `file_open_for_read()` and applied to both the buffered data path and the sendfile path | @@ -396,13 +396,83 @@ with `--append`/`--append-verify` (a payload-less sibling cannot be tail-resumed | Flag | Rsync Description | FastSync Status | Notes | |------|-------------------|-----------------|-------| -| `-l`, `--links` | Copy symlinks as symlinks | ⚠️ Partial | Scanner includes symlinks; target path not transmitted | +| `-l`, `--links` | Copy symlinks as symlinks | ✅ Implemented | A symlink is transmitted as a real symlink: its target string crosses the wire (a new `STATUS_SYMLINK` frame / chunk entry type) and the receiver creates it with `symlinkat` beneath the receive root. This makes the previously-`-l`-included-but-targetless symlink handling complete. See the Phase-4 symlink-trust notes | | `-L`, `--copy-links` | Transform symlink to referent | ✅ Implemented | `copy_links` config field | | `--copy-unsafe-links` | Transform unsafe symlinks | ✅ Implemented | `copy_unsafe_links` config field | | `--safe-links` | Ignore symlinks outside tree | ✅ Implemented | `safe_links` config field | -| `--munge-links` | Munge symlinks for safety | ❌ Not Implemented | | -| `-k`, `--copy-dirlinks` | Transform symlink to dir | ❌ Not Implemented | | -| `-K`, `--keep-dirlinks` | Treat symlinked dir as dir | ❌ Not Implemented | | +| `--munge-links` | Munge symlinks for safety | ✅ Implemented | Sender rewrites each transmitted symlink target with a `#SYMLINK/` marker; a target that could escape the receive root (absolute or containing `..`) is never transmitted (contained/skipped); the receiver strips the marker to restore the real target. See the Phase-4 symlink-trust notes | +| `-k`, `--copy-dirlinks` | Transform symlink to dir | ✅ Implemented | A symlink whose referent is a directory is dereferenced and recursed as a real directory; a symlink to a regular file stays a symlink. Sender-side only. See the Phase-4 symlink-trust notes | +| `-K`, `--keep-dirlinks` | Treat symlinked dir as dir | ✅ Implemented | On the receiver, an existing destination symlink-to-a-directory is used as that directory (followed) instead of being replaced; it is followed only when it resolves to a directory that stays beneath the receive root. See the Phase-4 symlink-trust notes | + +**Phase-4 symlink-trust notes:** `-l/--links`, `-k/--copy-dirlinks`, +`-K/--keep-dirlinks`, and `--munge-links` form the "symlink trust boundaries" +row. Making all three new flags have an observable, security-sane effect +required transmitting symlink targets, so FastSync's `-l/--links` is now real: +a symlink-type entry carries its target on the wire (a new `STATUS_SYMLINK` +frame for the per-file path, and a new entry type `2` in the `-s` chunk +serializer) and the receiver creates it with `symlinkat` under an `O_NOFOLLOW` +parent walk, never following the target. Wire changes: `STATUS_SYMLINK`, +the chunk entry type `2`, a per-entry symlink-target string, and two new config +booleans that CROSS the wire — `munge_links` and `keep_dirlinks`; `PROTOCOL_VERSION` +was bumped **2.12.0 → 2.13.0** (peers must match, exactly as prior phases did). + +**Per-flag semantics and divergences.** +- **`-l/--links`** copies a symlink as a symlink: the scanner `readlink`s the + target, the sender transmits it, and the receiver `symlinkat`s it. FastSync + `-l` never preserved symlink targets before (the flag was documented partial + and, in fact, tried to read the referent as file data); it now does, matching + rsync. Divergences: because the receiver enforces the symlink containment + predicate unconditionally, a plain `-l` sync **refuses to round-trip a + legitimate absolute symlink target** (it is dropped, never created pointing + outside the root — see the `--munge-links` note for the symmetric trust + boundary); a relative in-root target is copied as-is. FastSync also does not + set timestamps/owner on symlinks (no symlink-mode metadata application), + matching its existing no-op `--omit-link-times`. +- **`-k/--copy-dirlinks`** (sender): a symlink whose referent is a directory is + dereferenced and recursed into as a real directory; a symlink to a regular + file (or any non-directory) is kept as a symlink. This is rsync's `-k`. When + `-L/--copy-links` or `--safe-links`/`--copy-unsafe-links` are active, their + (dereference) semantics take precedence, so `-k` is subsumed exactly as in + rsync. +- **`-K/--keep-dirlinks`** (receiver, crosses the wire): when a directory is to + be created (on-demand parent creation for a child write) and the destination + path is already an existing symlink that resolves to a directory *within* the + receive root, that symlinked directory is used (followed) instead of being + replaced by a real directory; new entries are written beneath it. The follow + is confined: it only happens where `realpath` of the symlink resolves to a + still-within-root real directory, so a malicious link pointing outside the + root is never followed. Scope: `-K` acts on the write path (parent/`mkdir` + creation); the delete walker still never follows symlinks (a documented + divergence for `--delete` over an existing symlinked dir). Without `-K` the + destination symlink is not followed (the O_NOFOLLOW walk fails the write), + which is the safe default. +- **`--munge-links`** (sender security rewrite; crosses the wire so the receiver + unmunges): every transmitted symlink target is prefixed with the marker + `#SYMLINK/`; the receiver strips the marker (only when the negotiated + `munge_links` policy is on — a plain `-l` run never strips the prefix, so a + source symlink that genuinely begins with `#SYMLINK/` round-trips verbatim) + and restores the exact real target. The trust boundary is **symmetric and + enforced receiver-side**, independent of the sender: `file_symlink_at_secure` + refuses any target that `file_symlink_target_contained` rejects (absolute + `/...` or relative with a `..` component), and `file_save_to_disk_full` + contains such an entry (skipped) rather than materializing it. A deliberate confinement trade-off: because the receiver + enforces containment unconditionally, a plain `-l` (no `--munge-links`) sync + *refuses to round-trip a legitimate absolute symlink target* — such target is + dropped, never created pointing outside the root. This is a stricter subset of + rsync: rsync stores munged targets on the RECEIVING side and depends on both + ends running `--munge-links`; FastSync additionally enforces the containment + predicate at the receiver regardless of what the sender transmitted. When no + symlink is being transmitted (`-l`/`-k`/`-a` off) `--munge-links` has nothing + to rewrite and is inert. -*K/`--keep-dirlinks` policy is installed per + connection at config-accept (stable for the whole transfer, never racy under + `-m`), and only ever follows an in-root symlink-to-directory.* + +**Compatibility (byte-identical when all three are absent):** `-k`, `-K` and +`--munge-links` are opt-in. Without them the scanner's link handling, the wire +frames, and the receiver's writes are unchanged for every other option set, so a +run that previously worked continues to behave identically. `-l/--links` itself +now transmits targets (the prior behavior was broken/partial); its status moved +`⚠️ Partial → ✅ Implemented`. ## 10. Sparse & Device diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 5ff1a78..1b45440 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -476,6 +476,9 @@ static const OptionEntry OPTION_TABLE[] = { {"--copy-links", NULL, OPT_FLAG, offsetof(Config, copy_links)}, {"--safe-links", NULL, OPT_FLAG, offsetof(Config, safe_links)}, {"--copy-unsafe-links", NULL, OPT_FLAG, offsetof(Config, copy_unsafe_links)}, + {"--copy-dirlinks", "-k", OPT_FLAG, offsetof(Config, copy_dirlinks)}, + {"--keep-dirlinks", "-K", OPT_FLAG, offsetof(Config, keep_dirlinks)}, + {"--munge-links", NULL, OPT_FLAG, offsetof(Config, munge_links)}, {"--hard-links", "-H", OPT_FLAG, offsetof(Config, preserve_hard_links)}, {"--sparse", "-S", OPT_FLAG, offsetof(Config, preserve_sparse)}, {"--inplace", NULL, OPT_FLAG, offsetof(Config, inplace)}, diff --git a/src/client/client_send.c b/src/client/client_send.c index 3dbf2a7..612865e 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -102,6 +102,8 @@ static bool prepare_scanner(const Config* config, int num_threads, PreparedScann options->copy_links = config->copy_links; options->safe_links = config->safe_links; options->copy_unsafe_links = config->copy_unsafe_links; + options->copy_dirlinks = config->copy_dirlinks; + options->munge_links = config->munge_links; options->checksum = config->checksum; options->one_file_system = config->one_file_system; options->file_list = (const FileListSet*)config->files_from_set; @@ -1067,6 +1069,20 @@ static bool send_directory_entry(Client* client, File* file) { return send_str(client->file_descriptor, file_wire_path(file)); } +/* Transmit one symlink entry: a STATUS_SYMLINK frame carrying the destination + * path, the (sender-munged, if --munge-links) target string, and metadata when + * negotiated. The receiver unmunges the target and creates the symlink beneath + * its root. Symlinks never need an incremental check or data payload. */ +static bool send_symlink_entry(const Client* client, File* file, const Config* config) { + if (!file || !file_wire_path(file) || !file->symlink_target) + return false; + int fd = client->file_descriptor; + if (!send_status(fd, STATUS_SYMLINK) || !send_str(fd, file_wire_path(file)) || + !send_str(fd, file->symlink_target)) + return false; + return !config->use_metadata || metadata_send(fd, file->metadata); +} + // Send a single file directly via sendfile (non-incremental path). static bool send_file_direct_sendfile(File* file, int fd, bool use_metadata, const Config* config) { if (!send_status(fd, STATUS_NEXT)) @@ -1248,6 +1264,13 @@ static int send_chunk_with_removal(Client* client, Chunk* chunk, Config* config, change_emit_file_sent(config, f); continue; } + /* Symlink entry (-l / -k keep-as-symlink): only the target rides the wire. */ + if (f->is_symlink) { + if (!send_symlink_entry(client, f, config)) + return -1; + change_emit_file_sent(config, f); + continue; + } bool stream = f->data->data == NULL && f->data->size > 0; bool use_sendfile = (config->use_sendfile && !config->use_compression) || (stream && !config->use_compression); diff --git a/src/client/scanner.c b/src/client/scanner.c index 0662823..b410c7c 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -112,6 +112,12 @@ typedef struct { char* path; struct stat stats; bool is_directory; + /* True when the entry should be carried through as a SYMLINK (is_symlink) + rather than a dereferenced file/directory. When true, `link_target` holds + the owned target string to transmit (sender-munged under --munge-links); + ownership transfers to the File built from this entry. */ + bool is_symlink; + char* link_target; /* True when the entry was pruned by a user selection rule (--filter/-C/per-dir rules, the --exclude/--include layer, or --max-size/--min-size) rather than skipped for another reason (unreadable, symlink policy, not applicable). */ @@ -263,6 +269,8 @@ static int scanner_inspect_entry(const ScannerOptions* options, const char* sour const char* containing_dir, const char* name, ScannerEntry* entry) { entry->excluded = false; + entry->is_symlink = false; + entry->link_target = NULL; entry->path = path_cat(containing_dir, name); if (!entry->path) return -1; @@ -273,38 +281,81 @@ static int scanner_inspect_entry(const ScannerOptions* options, const char* sour return 0; } bool is_symlink = S_ISLNK(link_stats.st_mode); - if (is_symlink && !options->follow_symlinks && !options->copy_links && !options->safe_links && - !options->copy_unsafe_links) + if (!is_symlink) + 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; - if (is_symlink && options->safe_links) { - char link_target[4096]; - ssize_t length = readlink(entry->path, link_target, sizeof(link_target) - 1); - if (length < 0) - goto skip; - link_target[length] = '\0'; + char link_target[4096]; + ssize_t length = readlink(entry->path, link_target, sizeof(link_target) - 1); + if (length < 0) + 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 (is_symlink && options->copy_unsafe_links && !options->copy_links) { - char link_target[4096]; - ssize_t length = readlink(entry->path, link_target, sizeof(link_target) - 1); - if (length < 0) - goto skip; - link_target[length] = '\0'; + if (options->copy_unsafe_links && !options->copy_links) { if (link_target[0] != '/') goto skip; } - if (is_symlink && options->follow_symlinks && !options->copy_links) - entry->stats = link_stats; - else if (stat(entry->path, &entry->stats) != 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) + goto skip; + entry->is_directory = S_ISDIR(entry->stats.st_mode); + if (entry->is_directory) + return 1; + goto apply_filters; + } + + /* 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; + 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); + if (!entry->link_target) + goto skip; + goto apply_filters; + +regular: + if (stat(entry->path, &entry->stats) != 0) + goto skip; entry->is_directory = S_ISDIR(entry->stats.st_mode); if (entry->is_directory) return 1; + +apply_filters: for (int i = 0; i < options->exclude_count; i++) if (glob_match(options->exclude_patterns[i], name)) { entry->excluded = true; @@ -330,6 +381,8 @@ static int scanner_inspect_entry(const ScannerOptions* options, const char* sour skip: free(entry->path); entry->path = NULL; + free(entry->link_target); + entry->link_target = NULL; return 0; } @@ -363,6 +416,8 @@ DirectoryScanner* directory_scanner_create_with_options(const char* root_directo scanner->copy_links = options->copy_links; scanner->safe_links = options->safe_links; scanner->copy_unsafe_links = options->copy_unsafe_links; + scanner->copy_dirlinks = options->copy_dirlinks; + scanner->munge_links = options->munge_links; scanner->checksum = options->checksum; scanner->one_file_system = options->one_file_system; scanner->failed = false; @@ -822,6 +877,8 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { .copy_links = scanner->copy_links, .safe_links = scanner->safe_links, .copy_unsafe_links = scanner->copy_unsafe_links, + .copy_dirlinks = scanner->copy_dirlinks, + .munge_links = scanner->munge_links, .checksum = scanner->checksum, .one_file_system = scanner->one_file_system, .file_list = scanner->file_list, @@ -917,10 +974,18 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { free(cur_path); if (file == NULL) { free(rel_copy); + free(inspected.link_target); + inspected.link_target = NULL; scanner->failed = true; continue; } - file->data->size = stats.st_size; + if (inspected.is_symlink) { + file->is_symlink = true; + file->symlink_target = inspected.link_target; + inspected.link_target = NULL; + } else { + file->data->size = stats.st_size; + } if (scanner->relative_mode) { file->send_path = rel_copy; rel_copy = NULL; @@ -1235,10 +1300,18 @@ static void scan_root_entry(const ScannerOptions* options, const FilterNode* roo free(cur_path); if (!file) { free(rel); + free(inspected.link_target); + inspected.link_target = NULL; ps->failed = true; return; } - file->data->size = st.st_size; + if (inspected.is_symlink) { + file->is_symlink = true; + file->symlink_target = inspected.link_target; + inspected.link_target = NULL; + } else { + file->data->size = st.st_size; + } if (use_rel) { file->send_path = rel; rel = NULL; diff --git a/src/client/scanner.h b/src/client/scanner.h index 4f755f4..82d846d 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -32,6 +32,14 @@ typedef struct { bool copy_links; bool safe_links; bool copy_unsafe_links; + /* Phase 4 symlink-trust sender options: -k/--copy-dirlinks (dereference a + * symlink to a directory as a directory, keeping symlinks-to-files as + * symlinks) and --munge-links (rewrite each transmitted symlink target with a + * marker; escaping targets are never transmitted). Both are client/sender + * side only and never serialized to the wire (keep_dirlinks is the + * receiver-side counterpart). */ + bool copy_dirlinks; + bool munge_links; bool checksum; bool one_file_system; /* Phase 2 (files-from / filter layer). All pointers are shared read-only @@ -100,6 +108,8 @@ typedef struct { bool copy_links; bool safe_links; bool copy_unsafe_links; + bool copy_dirlinks; + bool munge_links; bool checksum; bool one_file_system; dev_t root_dev; diff --git a/src/client/usage.c b/src/client/usage.c index 453e5c1..36dfd0c 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -180,6 +180,9 @@ void print_usage(void) { printf(" --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(" -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(" -H, --hard-links Preserve hard-link relationships across the transfer\n"); printf(" -S, --sparse Handle sparse files efficiently\n"); printf(" --inplace Update files in-place (no temp+rename)\n"); diff --git a/src/server/receiver.c b/src/server/receiver.c index 548b090..c61dba7 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -154,7 +154,8 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver DeleteManifest* deferred_manifest = NULL; while (status == STATUS_NEXT || status == STATUS_CHUNK || status == STATUS_CHECK || status == STATUS_KEEPALIVE || status == STATUS_ABORT || status == STATUS_CHECK_BATCH || - status == STATUS_MKDIR || status == STATUS_MANIFEST || status == STATUS_HARDLINK) { + status == STATUS_MKDIR || status == STATUS_MANIFEST || status == STATUS_HARDLINK || + status == STATUS_SYMLINK) { if (status == STATUS_KEEPALIVE) { if (!send_status(file_descriptor, STATUS_KEEPALIVE)) goto fail; @@ -185,6 +186,10 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver File* file = file_receive_hardlink(file_descriptor); if (!file || !sink->store_file(file, sink->context)) goto receive_error; + } else if (status == STATUS_SYMLINK) { + File* sym = file_receive_symlink(file_descriptor, config); + if (!sym || !sink->store_file(sym, sink->context)) + goto receive_error; } else if (status == STATUS_MANIFEST) { DeleteManifest* manifest = receive_manifest_entries(file_descriptor); if (!manifest) diff --git a/src/server/server.c b/src/server/server.c index 7ca8afc..f5bfd98 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -215,6 +215,11 @@ void handler(int file_descriptor) { apply path. Each connection is its own forked process, so this per-process snapshot never races another connection. */ identity_set_active(config); + /* Persist the negotiated --keep-dirlinks policy once, here at config-accept, + before any multithreaded 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). */ + file_set_keep_dirlinks(config->keep_dirlinks); if (config->use_multithreading) { Queue* q = queue_create(100, file_destroy); if (q == NULL) { diff --git a/src/shared/chunk.c b/src/shared/chunk.c index 4a40487..a318504 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -74,7 +74,8 @@ static unsigned long long per_file_serialize_size(File* file, bool use_metadata) if (metadata_size > ULLONG_MAX - size) return 0; size += metadata_size; - /* Entry type marker: 0 = regular file, 1 = explicit directory entry. */ + /* Entry type marker: 0 = regular file, 1 = explicit directory entry, + 2 = symlink entry (carries its target string). */ if (sizeof(int) > ULLONG_MAX - size) return 0; size += sizeof(int); @@ -83,7 +84,18 @@ static unsigned long long per_file_serialize_size(File* file, bool use_metadata) size += sizeof(size_t); if ((unsigned long long)file->data->size > ULLONG_MAX - size) return 0; - return size + file->data->size; + size += file->data->size; + /* Symlink entries append the target string (length-prefixed). */ + if (file->is_symlink) { + size_t target_len = file->symlink_target ? strlen(file->symlink_target) : 0; + if (sizeof(size_t) > ULLONG_MAX - size) + return 0; + size += sizeof(size_t); + if ((unsigned long long)target_len > ULLONG_MAX - size) + return 0; + size += target_len; + } + return size; } Data* chunk_serialize(Chunk* chunk, bool use_metadata) { @@ -116,7 +128,7 @@ Data* chunk_serialize(Chunk* chunk, bool use_metadata) { memcpy(data_pointer, wire_path, path_len); data_pointer += path_len; - int entry_type = file->is_dir ? 1 : 0; + int entry_type = file->is_symlink ? 2 : (file->is_dir ? 1 : 0); memcpy(data_pointer, &entry_type, sizeof(int)); data_pointer += sizeof(int); @@ -129,6 +141,15 @@ Data* chunk_serialize(Chunk* chunk, bool use_metadata) { if (file_data_size > 0) memcpy(data_pointer, file->data->data, file_data_size); data_pointer += file_data_size; + + if (file->is_symlink) { + size_t target_len = file->symlink_target ? strlen(file->symlink_target) : 0; + memcpy(data_pointer, &target_len, sizeof(size_t)); + data_pointer += sizeof(size_t); + if (target_len > 0) + memcpy(data_pointer, file->symlink_target, target_len); + data_pointer += target_len; + } } return data; } @@ -206,13 +227,14 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { } int entry_type; memcpy(&entry_type, data_pointer, sizeof(int)); - if (entry_type != 0 && entry_type != 1) { + if (entry_type != 0 && entry_type != 1 && entry_type != 2) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: bad entry type"); file_destroy(file); array_list_delete(files); return NULL; } file->is_dir = entry_type == 1; + file->is_symlink = entry_type == 2; data_pointer += sizeof(int); remaining_size -= sizeof(int); @@ -293,6 +315,43 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { data_pointer += file_data_size; remaining_size -= file_data_size; + if (file->is_symlink) { + if (remaining_size < sizeof(size_t)) { + log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for symlink target"); + file_destroy(file); + array_list_delete(files); + return NULL; + } + size_t target_len; + memcpy(&target_len, data_pointer, sizeof(size_t)); + data_pointer += sizeof(size_t); + remaining_size -= sizeof(size_t); + if (target_len == 0 || remaining_size < target_len) { + log_message(LOG_LEVEL_ERROR, "Invalid chunk format: bad symlink target"); + file_destroy(file); + array_list_delete(files); + return NULL; + } + char* target = protocol_alloc(target_len + 1); + if (!target) { + log_perror("Could not allocate memory for symlink target"); + file_destroy(file); + array_list_delete(files); + return NULL; + } + memcpy(target, data_pointer, target_len); + target[target_len] = '\0'; + if (memchr(target, '\0', target_len) != NULL) { + free(target); + file_destroy(file); + array_list_delete(files); + return NULL; + } + file->symlink_target = target; + data_pointer += target_len; + remaining_size -= target_len; + } + if (!array_list_add(files, file)) { file_destroy(file); array_list_delete(files); diff --git a/src/shared/config.c b/src/shared/config.c index a524a25..a3e62b0 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -71,6 +71,9 @@ static void config_set_defaults(Config* config) { config->copy_links = false; config->safe_links = false; config->copy_unsafe_links = false; + config->copy_dirlinks = false; + config->munge_links = false; + config->keep_dirlinks = false; config->preserve_hard_links = false; config->preserve_acls = false; config->preserve_xattrs = false; @@ -204,6 +207,7 @@ static bool validate_received_config(const Config* config) { !(config->preserve_hard_links && (config->append || config->append_verify)) && valid_wire_bool(config->preserve_atimes) && valid_wire_bool(config->preserve_crtimes) && valid_wire_bool(config->omit_dir_times) && valid_wire_bool(config->omit_link_times) && + valid_wire_bool(config->munge_links) && valid_wire_bool(config->keep_dirlinks) && (!config->use_compression || (config->compression_level >= 1 && config->compression_level <= 22)) && config->chunk_size > 0 && config->chunk_size <= MAX_CHUNK_SIZE && @@ -771,6 +775,18 @@ static bool receive_metadata_times_options(int fd, Config* c) { receive_wire_bool(fd, &c->omit_link_times); } +/* Phase 4 symlink-trust: --munge-links and -K/--keep-dirlinks. Both CROSS the + * wire (the receiver unmunges symlink targets and, with -K, follows an in-root + * destination symlink-to-directory). -k/--copy-dirlinks is sender-only and is + * never serialized. Trailing fields; protocol 2.13.0. */ +static bool send_symlink_trust_options(int fd, const Config* c) { + return send_int(fd, c->munge_links) && send_int(fd, c->keep_dirlinks); +} + +static bool receive_symlink_trust_options(int fd, Config* c) { + return receive_wire_bool(fd, &c->munge_links) && receive_wire_bool(fd, &c->keep_dirlinks); +} + bool config_send(int file_descriptor, const Config* config) { protocol_session_set_max_alloc(NULL, config->max_alloc); if (!send_core_fields(file_descriptor, config) || !send_delta_fields(file_descriptor, config) || @@ -780,7 +796,8 @@ bool config_send(int file_descriptor, const Config* config) { !send_basis_options(file_descriptor, config) || !send_fuzzy_option(file_descriptor, config) || !send_checksum_options(file_descriptor, config) || !send_identity_options(file_descriptor, config) || - !send_metadata_times_options(file_descriptor, config)) + !send_metadata_times_options(file_descriptor, config) || + !send_symlink_trust_options(file_descriptor, config)) return false; Status status; if (!receive_status(file_descriptor, &status)) @@ -817,7 +834,8 @@ Config* config_receive(int file_descriptor) { !receive_fuzzy_option(file_descriptor, config) || !receive_checksum_options(file_descriptor, config) || !receive_identity_options(file_descriptor, config) || - !receive_metadata_times_options(file_descriptor, config)) + !receive_metadata_times_options(file_descriptor, config) || + !receive_symlink_trust_options(file_descriptor, config)) goto error; if (config->compress_choice[0] != '\0' && strcmp(config->compress_choice, "zstd") != 0 && strcmp(config->compress_choice, "none") != 0) { diff --git a/src/shared/config.h b/src/shared/config.h index 15d5aae..35ee916 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -108,6 +108,15 @@ typedef struct Config { bool copy_links; bool safe_links; bool copy_unsafe_links; + /* Phase 4 symlink-trust. -k/--copy-dirlinks and --munge-links are + * CLIENT/sender-side only (they decide how the SENDER scans and rewrites + * symlinks; the receiver never reads them), so they never cross the wire. + * -K/--keep-dirlinks is a RECEIVER-side policy (follow an in-root destination + * symlink-to-directory as a directory) and CROSSES the wire along with + * --munge-links (so the receiver knows to unmunge). */ + bool copy_dirlinks; /* client-only, sender-side (-k) */ + bool munge_links; /* crosses the wire */ + bool keep_dirlinks; /* crosses the wire (-K) */ // Issue #121: Extended metadata preservation bool preserve_hard_links; @@ -312,7 +321,7 @@ typedef struct Config { bool open_noatime; } Config; -#define PROTOCOL_VERSION "2.12.0" +#define PROTOCOL_VERSION "2.13.0" #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024) /* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */ #define MAX_BASIS_DIRS 64 diff --git a/src/shared/file.c b/src/shared/file.c index f80e5e3..8f4dd00 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -114,6 +114,8 @@ File* file_create(const char* path) { file->link_group = 0; file->link_first = false; file->hardlink_target = NULL; + file->is_symlink = false; + file->symlink_target = NULL; return file; } @@ -133,6 +135,8 @@ void file_destroy(void* item) { file->basis_link = NULL; free(file->hardlink_target); file->hardlink_target = NULL; + free(file->symlink_target); + file->symlink_target = NULL; free(file); } @@ -336,6 +340,108 @@ bool file_destination_is_newer_secure(const char* path, const FileMetadata* meta return file_stat_secure(path, &st) && stat_is_newer(&st, metadata); } +/* --keep-dirlinks (-K) receiver process-wide policy: when set, a destination + * path component that is itself a symlink to an in-root directory is followed + * (used as that directory) instead of failing the O_NOFOLLOW walk. Only ever + * honoured when the resolved target is a directory that stays beneath the + * authorized root, so a malicious symlink can never redirect the write outside + * it. Client of record is the server's receiver. */ +static bool file_keep_dirlinks = false; + +void file_set_keep_dirlinks(bool enable) { + file_keep_dirlinks = enable; +} + +bool file_get_keep_dirlinks(void) { + return file_keep_dirlinks; +} + +/* 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). */ +bool file_symlink_target_contained(const char* target) { + if (!target || target[0] == '\0' || target[0] == '/') + return false; + const char* p = target; + while (*p) { + const char* slash = strchr(p, '/'); + size_t comp_len = slash ? (size_t)(slash - p) : strlen(p); + if (comp_len == 2 && p[0] == '.' && p[1] == '.') + return false; + if (!slash) + break; + p = slash + 1; + } + return true; +} + +/* Remove a leading symlink munge marker (if present); returns true when the + * marker was stripped. `target` is a mutable NUL-terminated buffer. */ +bool file_symlink_unmunge(char* target) { + if (!target) + return false; + static const char* const marker = SYMLINK_MUNGE_PREFIX; + size_t marker_len = strlen(marker); + if (strncmp(target, marker, marker_len) != 0) + return false; + size_t rest = strlen(target + marker_len) + 1; + memmove(target, target + marker_len, rest); + return true; +} + +/* Owned copy of `target` prefixed with SYMLINK_MUNGE_PREFIX (the sender-side + * --munge-links rewriting). Returns NULL on allocation failure. */ +char* file_symlink_munge(const char* target) { + if (!target) + return NULL; + static const char* const marker = SYMLINK_MUNGE_PREFIX; + size_t marker_len = strlen(marker); + size_t target_len = strlen(target); + char* out = malloc(marker_len + target_len + 1); + if (!out) + return NULL; + memcpy(out, marker, marker_len); + memcpy(out + marker_len, target, target_len + 1); + return out; +} + +/* Create a symlink at `path` pointing to `target`, confined below the + * authorized root: the parent directory is opened with an O_NOFOLLOW fd walk + * and the link is created with symlinkat so neither the destination chain nor + * 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. */ +bool file_symlink_at_secure(const char* path, const char* target) { + if (!path || !target || has_path_traversal(path) || !file_symlink_target_contained(target)) + return false; + char* leaf = NULL; + int parent_fd = file_open_secure_parent(path, &leaf, true); + if (parent_fd < 0) + return false; + bool ok = false; + struct stat st; + bool exists = fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) == 0; + if (exists && S_ISDIR(st.st_mode)) { + /* A directory already at this path cannot be replaced atomically with a + symlink without --force semantics; leave it and report the collision. */ + ok = false; + } else { + if (exists && unlinkat(parent_fd, leaf, 0) != 0 && errno != ENOENT) + goto out; + ok = symlinkat(target, parent_fd, leaf) == 0; + } +out: + close(parent_fd); + free(leaf); + return ok; +} + int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) { char* copy = str_dup(path); if (!copy) @@ -383,6 +489,7 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) } char* save = NULL; char* component = strtok_r(parent, "/", &save); + char rel_buf[PATH_MAX] = ""; while (component) { if (strcmp(component, "..") == 0) { close(fd); @@ -392,10 +499,45 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) } if (strcmp(component, ".") != 0) { int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - if (create_dirs && next < 0 && errno == ENOENT) { + if (next < 0 && create_dirs && errno == ENOENT) { if (mkdirat(fd, component, 0755) == 0 || errno == EEXIST) next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); } + /* --keep-dirlinks (-K): a path component that is an existing symlink to + an in-root directory is used as THAT directory rather than failing the + O_NOFOLLOW walk. Only honoured when the symlink resolves to a + directory that stays beneath the authorized root, so a malicious link + can never redirect the write outside it. */ + if (next < 0 && file_keep_dirlinks && authorized_root_path != NULL && + (errno == ELOOP || errno == ENOTDIR || errno == EACCES)) { + struct stat lst; + if (fstatat(fd, component, &lst, AT_SYMLINK_NOFOLLOW) == 0 && S_ISLNK(lst.st_mode)) { + char candidate[PATH_MAX]; + char root[PATH_MAX]; + if (realpath(authorized_root_path, root) && + snprintf(candidate, sizeof(candidate), "%s%s/%s", root, rel_buf, component) < + (int)sizeof(candidate)) { + char resolved[PATH_MAX]; + if (realpath(candidate, resolved) && strcmp(resolved, root) != 0 && + strncmp(root, resolved, strlen(root)) == 0 && + (resolved[strlen(root)] == '/' || resolved[strlen(root)] == '\0')) { + struct stat rst; + if (stat(resolved, &rst) == 0 && S_ISDIR(rst.st_mode)) { + /* Re-open the resolved directory WITHOUT following a symlink and + re-verify it is still a directory inode, so a symlink swapped + in between realpath() and open() (TOCTOU) cannot redirect this + fd outside the root. */ + next = open(resolved, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + struct stat ofst; + if (next >= 0 && (fstat(next, &ofst) != 0 || !S_ISDIR(ofst.st_mode))) { + close(next); + next = -1; + } + } + } + } + } + } if (next < 0) { close(fd); free(copy); @@ -404,6 +546,20 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) } close(fd); fd = next; + /* Track the walked relative prefix so the -K candidate path can be + reconstructed. An overflow while building it means the whole path is + at the PATH_MAX edge, so fail hard rather than silently building a + wrong (truncated) candidate for a later -K follow. */ + size_t need = strlen(rel_buf) + strlen(component) + 2; + if (need <= sizeof(rel_buf)) { + strcat(rel_buf, "/"); + strcat(rel_buf, component); + } else if (file_keep_dirlinks) { + close(fd); + free(copy); + free(leaf); + return -1; + } } component = strtok_r(NULL, "/", &save); } diff --git a/src/shared/file.h b/src/shared/file.h index 0d72343..84878b9 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -33,6 +33,27 @@ 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/" + +char* file_symlink_munge(const char* target); +/* 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); +/* Strip a leading SYMLINK_MUNGE_PREFIX from `target` (mutable, in place); + * 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`. */ +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. */ +void file_set_keep_dirlinks(bool enable); +bool file_get_keep_dirlinks(void); + /* A configured fd without a canonical identity deliberately rejects paths. */ bool file_set_authorized_root(int fd, const char* canonical_path); diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 07754a0..a85a80a 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -332,6 +332,49 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi return ok ? FILE_SAVE_WRITTEN : FILE_SAVE_ERROR; } + /* Symlink entry. (The process-wide --keep-dirlinks policy is set once by the + connection handler from the negotiated config, before any receiver/writer + threads start, so it is stable throughout this walk.) */ + + if (file->is_symlink) { + if (!file->symlink_target || file->path[0] == '\0' || has_path_traversal(file->path)) { + log_message(LOG_LEVEL_ERROR, "Invalid symlink entry received"); + return FILE_SAVE_ERROR; + } + 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. */ + 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. */ + if (ok && !file_symlink_target_contained(target)) + ok = false; + if (!ok) { + /* Skip the escaping/empty target (contained) rather than abort. */ + free(target); + free(link_path); + return FILE_SAVE_SKIPPED; + } + char* parent = str_dup(link_path); + if (parent) { + file_ensure_directory_secure(dirname(parent)); + free(parent); + } + ok = file_symlink_at_secure(link_path, target); + free(target); + free(link_path); + return ok ? FILE_SAVE_WRITTEN : FILE_SAVE_ERROR; + } + /* --hard-links/-H sibling: a later member of a link group arrives with no payload and is installed as a hard link to (or, on link() failure, a byte-identical copy of) the group's first member. Handled entirely here, @@ -1868,6 +1911,59 @@ File* file_receive_hardlink(int file_descriptor) { return file; } +/* Receive a symlink entry (the leading STATUS_SYMLINK code has already been + consumed): the destination path and the (sender-munged, if --munge-links) + symlink target string, then metadata when negotiated. The created File is + routed through the regular store_file sink, which creates the link beneath + the receive root (unmungeing the target first). */ +File* file_receive_symlink(int file_descriptor, const Config* config) { + char* path = receive_str(file_descriptor); + if (path == NULL) + return NULL; + if (path[0] == '\0' || has_path_traversal(path)) { + char* escaped_path = output_escape(path, log_get_8_bit_output()); + log_message(LOG_LEVEL_ERROR, "Invalid received symlink path: %s", + escaped_path ? escaped_path : ""); + free(escaped_path); + free(path); + send_status(file_descriptor, STATUS_ERROR); + return NULL; + } + char* target = receive_str(file_descriptor); + if (!target) { + free(path); + return NULL; + } + if (target[0] == '\0') { + char* escaped = output_escape(target, log_get_8_bit_output()); + log_message(LOG_LEVEL_ERROR, "Invalid received symlink target: %s", + escaped ? escaped : ""); + free(escaped); + free(target); + free(path); + send_status(file_descriptor, STATUS_ERROR); + return NULL; + } + File* file = file_create(path); + free(path); + if (!file) { + free(target); + return NULL; + } + if (config && config->use_metadata) { + int meta_ok = 1; + file->metadata = metadata_receive(file_descriptor, &meta_ok); + if (!meta_ok) { + file_destroy(file); + free(target); + return NULL; + } + } + file->is_symlink = true; + file->symlink_target = target; + return file; +} + /* Read a delete-manifest frame (the STATUS_MANIFEST leading code has already been consumed): a keep-set entry count followed by that many destination-relative paths, then a protected-prefix count followed by that diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index 3ecee9c..01846e7 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -10,6 +10,7 @@ File* file_receive(const Config* config, int file_descriptor); File* file_receive_directory(int file_descriptor); File* file_receive_hardlink(int file_descriptor); +File* file_receive_symlink(int file_descriptor, const Config* config); File* receive_incremental_check(int fd, const Config* config, bool* skipped); /* A received delete-manifest frame: the keep-set (`keeps`, destination-relative diff --git a/src/shared/file_types.h b/src/shared/file_types.h index 8e3548f..89e3e42 100644 --- a/src/shared/file_types.h +++ b/src/shared/file_types.h @@ -56,6 +56,13 @@ typedef struct { int link_group; bool link_first; char* hardlink_target; + /* Symlink-type entry (-l/--links, or -k/--copy-dirlinks' keep-as-symlink + * branch). When true, `symlink_target` holds the (sender-munged, if + * --munge-links) target string that is carried on the wire; the receiver + * creates a symlink to (an unmunged) target instead of writing regular-file + * data. `data` is empty for a symlink entry. Sender + receiver state. */ + bool is_symlink; + char* symlink_target; } File; /* The path that should be sent on the wire and used for the receiver-side diff --git a/src/shared/protocol.h b/src/shared/protocol.h index 92e32f4..a43fdc2 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -90,7 +90,13 @@ enum NET_STATUS { * first (data-carrying) member's destination-relative wire path; the receiver * creates this entry as a hard link to the first member's installed file * (falling back to a byte-identical copy if link() fails). Protocol 2.12.0. */ - STATUS_HARDLINK + STATUS_HARDLINK, + /* A symlink-type entry (-l/--links, -k/--copy-dirlinks' keep-as-symlink + * branch). The sender transmits the destination path, the (sender-munged, + * if --munge-links) symlink target, and optional metadata; the receiver + * creates a symlink to the unmunged target beneath the receive root (see + * file_receive_symlink). Protocol 2.13.0. */ + STATUS_SYMLINK }; void io_set_fds(int read_fd, int write_fd); diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index e21c8d4..acb2053 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -4128,3 +4128,153 @@ class TestOmitTimes: received = get_dest_received_dir(dest, source) mismatches, missing = verify_transfer(source, received) assert not missing and not mismatches, f"missing={missing} mismatches={mismatches}" + + +class TestSymlinkTrust: + """Phase-4 symlink trust boundaries: -k/--copy-dirlinks, -K/--keep-dirlinks + and --munge-links. Destination paths mirror the absolute source path below + the destination root (run_client uses absolute --source-dir/--dest-dir).""" + + def test_copy_dirlinks_dereferences_dir_symlink(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "symlink_trust_copy_dirlinks") + dest = os.path.join(TEST_DATA_DIR, "symlink_trust_copy_dirlinks_dst") + clean_dir(source) + clean_dir(dest) + os.makedirs(os.path.join(source, "realdir")) + with open(os.path.join(source, "realfile.txt"), "wb") as f: + f.write(b"real file\n") + with open(os.path.join(source, "realdir", "inside.txt"), "wb") as f: + f.write(b"inside dir\n") + 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]}" + + received = get_dest_received_dir(dest, source) + # link -> realdir dereferences into a real directory tree... + link_dir = os.path.join(received, "link_dir") + assert os.path.isdir(link_dir) + assert not os.path.islink(link_dir) + assert os.path.isfile(os.path.join(link_dir, "inside.txt")) + # ... while a symlink to a regular file stays a symlink. + link_file = os.path.join(received, "link_file") + assert os.path.islink(link_file) + assert os.readlink(link_file) == "realfile.txt" + + def test_keep_dirlinks_keeps_dest_symlink_to_dir(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "symlink_trust_keep_dirlinks") + dest = os.path.join(TEST_DATA_DIR, "symlink_trust_keep_dirlinks_dst") + clean_dir(source) + clean_dir(dest) + os.makedirs(os.path.join(source, "sub")) + with open(os.path.join(source, "sub", "file.txt"), "wb") as f: + f.write(b"under the kept symlinked dir\n") + + # Plant the destination's symlink-to-directory at the exact mirror path: + # sub -> realdir (relative, both siblings under the mirror parent). + parent = os.path.join(dest, os.path.abspath(source).lstrip(os.sep)) + os.makedirs(parent) + os.makedirs(os.path.join(parent, "realdir")) + os.symlink("realdir", os.path.join(parent, "sub")) + + result, _ = run_client(source, dest, flags=["-K"], port=shared_server.port) + assert result.returncode == 0, f"-K failed: {(result.stderr or result.stdout)[:300]}" + + received = get_dest_received_dir(dest, source) + sub = os.path.join(received, "sub") + # sub stays a symlink to the directory rather than being replaced... + assert os.path.islink(sub) + assert os.readlink(sub) == "realdir" + # ... 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") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "a.txt"), "wb") as f: + f.write(b"a\n") + os.symlink("a.txt", os.path.join(source, "good")) + os.symlink("/etc/passwd", os.path.join(source, "abs_escape")) + os.symlink("../../escape", os.path.join(source, "dotdot_escape")) + + result, _ = run_client(source, dest, flags=["-l", "--munge-links"], + port=shared_server.port) + 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.path.isfile(os.path.join(received, "a.txt")) + + 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") + clean_dir(source) + clean_dir(dest) + os.makedirs(os.path.join(source, "realdir")) + with open(os.path.join(source, "realfile.txt"), "wb") as f: + f.write(b"real\n") + with open(os.path.join(source, "realdir", "x.txt"), "wb") as f: + f.write(b"x\n") + os.symlink("realfile.txt", os.path.join(source, "lf")) + os.symlink("realdir", os.path.join(source, "ld")) + + 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) + assert os.path.islink(os.path.join(received, "lf")) + assert os.readlink(os.path.join(received, "lf")) == "realfile.txt" + 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") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "a.txt"), "wb") as f: + f.write(b"a\n") + os.symlink("a.txt", os.path.join(source, "good")) + os.symlink("/etc/passwd", os.path.join(source, "unsafe_abs")) + + 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")) + + 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") + 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")) + + 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" diff --git a/tests/test_chunk.c b/tests/test_chunk.c index 8271b8f..6ce2e0b 100644 --- a/tests/test_chunk.c +++ b/tests/test_chunk.c @@ -159,8 +159,66 @@ static void test_chunk_dir_entry_roundtrip() { rmdir(dir_path); } +static void test_chunk_symlink_roundtrip() { + const char* file_path = "temp_chunk_symlink_file.txt"; + const char* link_path = "temp_chunk_symlink"; + const char* content = "regular payload"; + const char* target = "temp_chunk_symlink_file.txt"; + + rmdir(link_path); + unlink(file_path); + + file_write_to_disk(file_path, content, strlen(content), false, false); + + for (int use_metadata = 0; use_metadata <= 1; use_metadata++) { + struct stat st; + EXPECT_EQ_INT(stat(file_path, &st), 0); + + File* reg = file_create(file_path); + EXPECT_NOT_NULL(reg); + reg->data->size = (unsigned long long)st.st_size; + EXPECT_TRUE(file_load_data(reg)); + + File* link = file_create(link_path); + EXPECT_NOT_NULL(link); + link->is_symlink = true; + link->symlink_target = str_dup(target); + EXPECT_NOT_NULL(link->symlink_target); + + if (use_metadata) { + reg->metadata = file_metadata_create(file_path, &st, false, false); + EXPECT_NOT_NULL(reg->metadata); + link->metadata = file_metadata_create(file_path, &st, false, false); + EXPECT_NOT_NULL(link->metadata); + } + + File* files[2] = {reg, link}; + Chunk* chunk = chunk_create(files, 2); + EXPECT_NOT_NULL(chunk); + + Data* serialized = chunk_serialize(chunk, use_metadata != 0); + EXPECT_NOT_NULL(serialized); + Chunk* deserialized = chunk_deserialize(serialized, use_metadata != 0); + EXPECT_NOT_NULL(deserialized); + EXPECT_EQ_INT(deserialized->element_count, 2); + EXPECT_FALSE(deserialized->items[0]->is_symlink); + EXPECT_TRUE(deserialized->items[1]->is_symlink); + EXPECT_NULL(deserialized->items[0]->symlink_target); + EXPECT_EQ_STR(deserialized->items[1]->symlink_target, target); + EXPECT_EQ_INT((int)deserialized->items[1]->data->size, 0); + + data_destroy(serialized); + chunk_destroy(deserialized); + chunk_destroy(chunk); /* frees reg and link */ + } + + unlink(file_path); + rmdir(link_path); +} + void test_chunk() { test_file_operations(); test_chunk_operations(); test_chunk_dir_entry_roundtrip(); + test_chunk_symlink_roundtrip(); } diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 1aa40fb..4ba7f81 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -1219,6 +1219,40 @@ static void test_parse_args_short_s_remains_chunk_serialization() { config_delete(cfg); } +/* Phase 4 symlink-trust flags: -k/--copy-dirlinks, -K/--keep-dirlinks and + --munge-links must parse into their Config fields. */ +static void test_parse_args_symlink_trust() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "-k", "-K", "--munge-links", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + + EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->copy_dirlinks); + EXPECT_TRUE(cfg->keep_dirlinks); + EXPECT_TRUE(cfg->munge_links); + config_delete(cfg); + + cfg = config_create(); + char* long_argv[] = {"fastsync", "--copy-dirlinks", "--keep-dirlinks", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, long_argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->copy_dirlinks); + EXPECT_TRUE(cfg->keep_dirlinks); + EXPECT_FALSE(cfg->munge_links); + config_delete(cfg); + + /* Without any of the flags they stay off (additive, opt-in). */ + cfg = config_create(); + char* plain_argv[] = {"fastsync", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 3, plain_argv, positional_args, &positional_count), 0); + EXPECT_FALSE(cfg->copy_dirlinks); + EXPECT_FALSE(cfg->keep_dirlinks); + EXPECT_FALSE(cfg->munge_links); + config_delete(cfg); +} + static void test_parse_args_8_bit_output() { Config* cfg = config_create(); char* long_argv[] = {"fastsync", "--8-bit-output", "/src", "/dst"}; @@ -2429,6 +2463,7 @@ void test_client_cli() { test_parse_args_rejects_unsupported_stderr_modes(); test_parse_args_secluded_args(); test_parse_args_short_s_remains_chunk_serialization(); + test_parse_args_symlink_trust(); test_parse_args_whole_file(); test_parse_args_fuzzy_implies_delta(); test_parse_args_fuzzy_negation(); diff --git a/tests/test_config.c b/tests/test_config.c index 9770b84..2401985 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -622,9 +622,60 @@ static void test_config_delete_policy_wire_roundtrip() { } } -/* --delete-missing-args crosses the wire (the receiver executes the exact-path - deletions) while --ignore-missing-args is client-only: the receiver must - observe delete_missing_args unchanged and ignore_missing_args always false. */ +/* Phase 4 symlink-trust wire split: --munge-links and -K/--keep-dirlinks CROSS + the wire (the receiver unmunges targets and follows an in-root dir-link), + while -k/--copy-dirlinks is client/sender-only and must NOT reach the + receiver (it would observe it false). */ +static void test_config_symlink_trust_wire_roundtrip() { + if (is_running_under_valgrind()) + return; + + struct { + bool munge_links, keep_dirlinks, copy_dirlinks; + } cases[] = { + {false, false, false}, + {true, false, false}, + {false, true, false}, + {true, true, true}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + int p[2]; + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + pid_t pid = fork(); + if (pid == 0) { + close(p[1]); + io_set_fds(p[0], p[0]); + Config* recv = config_receive(p[0]); + bool ok = recv != NULL; + if (ok) { + ok = recv->munge_links == cases[i].munge_links && + recv->keep_dirlinks == cases[i].keep_dirlinks && + /* copy_dirlinks never crosses the wire. */ + recv->copy_dirlinks == false; + } + config_delete(recv); + close(p[0]); + _exit(ok ? 0 : 1); + } else { + close(p[0]); + io_set_fds(p[1], p[1]); + Config* send_cfg = config_create(); + EXPECT_NOT_NULL(send_cfg); + send_cfg->send_directory = str_dup("/src"); + send_cfg->receive_root_directory = str_dup("/dst"); + send_cfg->munge_links = cases[i].munge_links; + send_cfg->keep_dirlinks = cases[i].keep_dirlinks; + send_cfg->copy_dirlinks = cases[i].copy_dirlinks; + bool sent = config_send(p[1], send_cfg); + int status; + waitpid(pid, &status, 0); + close(p[1]); + config_delete(send_cfg); + EXPECT_TRUE(sent); + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } + } +} static void test_config_delete_missing_args_wire_roundtrip() { if (is_running_under_valgrind()) return; @@ -1107,6 +1158,7 @@ void test_config() { test_config_delete_timing_wire_roundtrip(); test_config_delete_timing_conflict_rejected(); test_config_delete_policy_wire_roundtrip(); + test_config_symlink_trust_wire_roundtrip(); test_config_delete_missing_args_wire_roundtrip(); test_config_append_wire_roundtrip(); test_config_basis_roundtrip(); diff --git a/tests/test_file.c b/tests/test_file.c index 0495bd2..2ad5c18 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -468,6 +468,50 @@ static void test_file_write_to_disk_does_not_follow_symlink() { unlink(link); } +static void test_file_symlink_helpers() { + /* Munge/unmunge round-trip restores the original target. */ + char* munged = file_symlink_munge("target.txt"); + EXPECT_NOT_NULL(munged); + EXPECT_EQ_INT(memcmp(munged, SYMLINK_MUNGE_PREFIX, strlen(SYMLINK_MUNGE_PREFIX)), 0); + EXPECT_TRUE(file_symlink_unmunge(munged)); + EXPECT_EQ_STR(munged, "target.txt"); + free(munged); + + char noop[] = "plain-target"; + EXPECT_FALSE(file_symlink_unmunge(noop)); + EXPECT_EQ_STR(noop, "plain-target"); + + /* Containment: relative targets without ".." are safe; absolute or + ".."-escaping targets are not. */ + EXPECT_TRUE(file_symlink_target_contained("a.txt")); + EXPECT_TRUE(file_symlink_target_contained("sub/dir/file")); + EXPECT_FALSE(file_symlink_target_contained("/etc/passwd")); + EXPECT_FALSE(file_symlink_target_contained("../escape")); + EXPECT_FALSE(file_symlink_target_contained("a/../b")); + EXPECT_FALSE(file_symlink_target_contained("")); +} + +static void test_file_symlink_at_secure() { + const char* link = "test_symlink_at_secure_link"; + const char* outside = "test_symlink_at_secure_outside.txt"; + unlink(link); + unlink(outside); + EXPECT_TRUE(file_write_to_disk(outside, "out", 3, false, false)); + + EXPECT_TRUE(file_symlink_at_secure(link, "outside.text")); + struct stat st; + EXPECT_EQ_INT(lstat(link, &st), 0); + EXPECT_TRUE(S_ISLNK(st.st_mode)); + + /* Replacing an existing non-directory entry is fine. */ + EXPECT_TRUE(file_symlink_at_secure(link, "other.txt")); + EXPECT_EQ_INT(lstat(link, &st), 0); + EXPECT_TRUE(S_ISLNK(st.st_mode)); + + unlink(link); + unlink(outside); +} + static void test_file_content_to_buffer() { const char* content = "Buffer content test"; EXPECT_TRUE(file_write_to_disk("test_buffer_file.txt", content, strlen(content), false, false)); @@ -978,6 +1022,8 @@ void test_file() { test_file_write_to_disk_creates_dirs(); test_file_write_to_disk_does_not_follow_symlink(); test_file_content_to_buffer(); + test_file_symlink_helpers(); + test_file_symlink_at_secure(); test_file_save_to_disk_path_traversal(); test_file_save_to_disk_deep_traversal(); test_dir_entry_save_to_disk();