diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 78adf43..5ebd10e 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -1885,6 +1885,75 @@ static bool cli_handle_checksum_options(CliParseCtx* ctx) { return false; } +/* Determine whether a --chown spec sets the owner and/or group side, honoring + * the same escape-aware splitting as identity_parse_chown(): a `\:` is a literal + * colon, not a field separator. */ +static void chown_spec_sides(const char* value, bool* has_owner, bool* has_group) { + *has_owner = false; + *has_group = false; + if (!value) + return; + bool split = false; + for (const char* p = value; *p; p++) { + if (*p == '\\' && p[1] == ':') { + p++; + continue; + } + if (*p == ':') { + split = true; + continue; + } + if (split) + *has_group = true; + else + *has_owner = true; + } +} + +/* rsync refuses to mix --chown with --usermap/--groupmap on the SAME side + * ("--usermap conflicts with prior --chown"). `chown_value` is non-NULL only + * for the --chown option itself. Returns true and records a parse error when + * the new option conflicts with one already seen. */ +static bool mapping_option_conflicts(CliParseCtx* ctx, const char* optname, bool is_group, + const char* chown_value) { + const Config* config = ctx->config; + if (chown_value) { + bool has_owner; + bool has_group; + chown_spec_sides(chown_value, &has_owner, &has_group); + if (has_owner && config->usermap_count > 0) { + log_message(LOG_LEVEL_ERROR, "%s conflicts with prior --usermap", optname); + ctx->exit_code = -1; + return true; + } + if (has_group && config->groupmap_count > 0) { + log_message(LOG_LEVEL_ERROR, "%s conflicts with prior --groupmap", optname); + ctx->exit_code = -1; + return true; + } + return false; + } + if (!is_group && config->chown_uid_set) { + log_message(LOG_LEVEL_ERROR, "%s conflicts with prior --chown", optname); + ctx->exit_code = -1; + return true; + } + if (is_group && config->chown_gid_set) { + log_message(LOG_LEVEL_ERROR, "%s conflicts with prior --chown", optname); + ctx->exit_code = -1; + return true; + } + return false; +} + +static bool usermap_conflicts_with_chown(CliParseCtx* ctx, const char* optname, bool is_group) { + return mapping_option_conflicts(ctx, optname, is_group, NULL); +} + +static bool chown_conflicts_with_map(CliParseCtx* ctx, const char* optname, const char* value) { + return mapping_option_conflicts(ctx, optname, false, value); +} + /* Remote-option, basis-directory and identity-mapping options. Returns true * when the argument was consumed. */ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { @@ -1957,6 +2026,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { return true; } if (strncmp(arg, "--usermap=", 10) == 0) { + if (usermap_conflicts_with_chown(ctx, "--usermap", false)) + return true; if (identity_parse_map(config, arg + 10, false) != 0) { ctx->exit_code = -1; return true; @@ -1970,6 +2041,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { ctx->exit_code = -1; return true; } + if (usermap_conflicts_with_chown(ctx, "--usermap", false)) + return true; if (identity_parse_map(config, ctx->argv[++ctx->i], false) != 0) { ctx->exit_code = -1; return true; @@ -1978,6 +2051,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { return true; } if (strncmp(arg, "--groupmap=", 11) == 0) { + if (usermap_conflicts_with_chown(ctx, "--groupmap", true)) + return true; if (identity_parse_map(config, arg + 11, true) != 0) { ctx->exit_code = -1; return true; @@ -1991,6 +2066,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { ctx->exit_code = -1; return true; } + if (usermap_conflicts_with_chown(ctx, "--groupmap", true)) + return true; if (identity_parse_map(config, ctx->argv[++ctx->i], true) != 0) { ctx->exit_code = -1; return true; @@ -1999,6 +2076,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { return true; } if (strncmp(arg, "--chown=", 8) == 0) { + if (chown_conflicts_with_map(ctx, "--chown", arg + 8)) + return true; if (identity_parse_chown(config, arg + 8) != 0) { ctx->exit_code = -1; return true; @@ -2015,6 +2094,8 @@ static bool cli_handle_remote_basis_options(CliParseCtx* ctx) { ctx->exit_code = -1; return true; } + if (chown_conflicts_with_map(ctx, "--chown", ctx->argv[ctx->i + 1])) + return true; if (identity_parse_chown(config, ctx->argv[++ctx->i]) != 0) { ctx->exit_code = -1; return true; diff --git a/src/client/client_send.c b/src/client/client_send.c index 1c6702b..418f86b 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -1464,7 +1464,11 @@ static bool send_directory_entry(const Client* client, File* file, const Config* if (!send_status(client->file_descriptor, STATUS_MKDIR) || !send_wire_str(client->file_descriptor, file_wire_path(file))) return false; - return !config->use_metadata || metadata_send(client->file_descriptor, file->metadata); + if (config->use_metadata && !metadata_send(client->file_descriptor, file->metadata)) + return false; + /* Directory xattrs/ACLs (-X/-A) ride the same trailing block as regular files + when the xattr transport was negotiated. */ + return !config->use_xattrs || xattr_send(client->file_descriptor, file->xattrs); } /* P7 Wave D: transmit every captured source directory's metadata in terminal @@ -1496,6 +1500,9 @@ static bool send_dir_times(const Client* client, const Config* config, ArrayList return false; if (!send_wire_str(fd, file_wire_path(file)) || !metadata_send(fd, file->metadata)) return false; + /* Directory xattrs/ACLs travel with the deferred directory metadata. */ + if (config->use_xattrs && !xattr_send(fd, file->xattrs)) + return false; } index += chunk; } diff --git a/src/client/scanner.c b/src/client/scanner.c index caef32e..06ffd6a 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -645,7 +645,8 @@ static Chunk* chunk_data_to_chunk(ArrayList* chunk_data) { * allocation failure is fatal and reported to the caller. */ static bool scanner_capture_dir_time(ArrayList* dir_entries, mtx_t* mutex, const char* root_path, const char* fs_path, bool relative_mode, bool preserve_atimes, - bool preserve_crtimes) { + bool preserve_crtimes, bool preserve_xattrs, + bool preserve_acls) { if (!dir_entries || !root_path || !fs_path) return true; struct stat st; @@ -672,6 +673,11 @@ static bool scanner_capture_dir_time(ArrayList* dir_entries, mtx_t* mutex, const file_destroy(file); return false; } + /* Directory xattrs/ACLs (-X/-A): captured here so the deferred + STATUS_DIR_TIMES frame can carry them and the receiver can re-apply them + fd-relative (a regular file's per-file block never covered directories). */ + if (preserve_xattrs || preserve_acls) + file->xattrs = xattr_capture_path(fs_path, preserve_acls); if (relative_mode) { file->send_path = rel; rel = NULL; @@ -771,10 +777,11 @@ static int open_next_directory(DirectoryScanner* scanner) { return -1; } if (scanner->options.capture_dir_times && - !scanner_capture_dir_time(scanner->options.dir_entries, scanner->options.dir_entries_mutex, - scanner->root_path, scanner->current_path, scanner->relative_mode, - scanner->options.preserve_atimes, - scanner->options.preserve_crtimes)) { + !scanner_capture_dir_time( + scanner->options.dir_entries, scanner->options.dir_entries_mutex, scanner->root_path, + scanner->current_path, scanner->relative_mode, scanner->options.preserve_atimes, + scanner->options.preserve_crtimes, scanner->options.preserve_xattrs, + scanner->options.preserve_acls)) { closedir(scanner->current_dir); scanner->current_dir = NULL; free(scanner->current_path); @@ -824,6 +831,7 @@ static File* dirs_root_dir_file(DirectoryScanner* scanner) { return NULL; } } + scanner_capture_xattrs(scanner, file); return file; } @@ -937,6 +945,7 @@ static File* dirs_file_for_entry(DirectoryScanner* scanner, const char* entry) { return NULL; } } + scanner_capture_xattrs(scanner, file); return file; } @@ -1760,7 +1769,8 @@ ParallelScanner* parallel_scanner_create_with_options(const char* root_directory if (options->capture_dir_times && !scanner_capture_dir_time(options->dir_entries, options->dir_entries_mutex, root_directory, root_directory, options->relative && options->file_list != NULL, - options->preserve_atimes, options->preserve_crtimes)) { + options->preserve_atimes, options->preserve_crtimes, + options->preserve_xattrs, options->preserve_acls)) { array_list_delete(root_files); array_list_delete(subdirs); parallel_scanner_destroy(ps); diff --git a/src/client/usage.c b/src/client/usage.c index ef4517a..95eab0b 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -197,17 +197,19 @@ void print_usage(void) { printf(" within the confined receive root. Never elevates\n"); printf(" privileges and never bypasses confinement; ownership\n"); printf(" is still applied only with -o/--owner, -g/--group, or an\n"); - printf(" explicit identity flag (--numeric-ids/--chown/--usermap/\n"); - printf(" --groupmap/--copy-as)\n"); + printf(" explicit identity flag (--chown/--usermap/--groupmap/\n"); + printf(" --copy-as); --numeric-ids only changes how ids map\n"); printf(" --no-super Forbid those super-user activities even when the\n"); printf(" receiver is running as root\n"); printf(" --chmod Modify transferred permissions (rsync syntax)\n"); - printf(" --numeric-ids Do not map uid/gid by name: use the source numeric\n"); - printf(" ids directly when applying ownership\n"); + printf(" --numeric-ids Map uid/gid by id instead of by name (a modifier, not\n"); + printf(" an ownership request: combine with -o/-g or a map)\n"); printf(" --usermap=MAP Map usernames when applying ownership: comma-separated\n"); - printf(" FROM:TO rules, first match wins. FROM/TO are names\n"); - printf(" (resolved on the source machine), * (match any /\n"); - printf(" current user), or @N numeric ids. e.g. *:nobody\n"); + printf(" FROM:TO rules, first match wins. FROM is a name (from\n"); + printf(" the source), an id, an inclusive LOW-HIGH range, *\n"); + printf(" (any id), or empty (ids with no name). TO is an id, *\n"); + printf(" (current user), or a name resolved on the receiver.\n"); + printf(" e.g. 0-99:nobody,*:normal (cannot mix with --chown)\n"); printf(" --groupmap=MAP Map group names when applying ownership (same syntax)\n"); printf(" --chown=USER:GROUP Override the ownership of transferred files. Forms:\n"); printf(" USER:GROUP, USER (owner only), :GROUP (group only); a\n"); diff --git a/src/server/receiver.c b/src/server/receiver.c index 51b935c..3d43890 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -487,7 +487,7 @@ static bool receiver_save_file(File* file, void* context_pointer) { receiver_send_success_frame). */ if (result != FILE_SAVE_ERROR && file->is_dir && file->metadata && dir_metadata_should_capture(context->config) && - !dir_time_list_add(&context->dir_times, file->path, file->metadata)) { + !dir_time_list_add(&context->dir_times, file->path, file->metadata, file->xattrs)) { file_destroy(file); return false; } diff --git a/src/server/receiver_pipeline.c b/src/server/receiver_pipeline.c index c983adf..0828db8 100644 --- a/src/server/receiver_pipeline.c +++ b/src/server/receiver_pipeline.c @@ -232,7 +232,7 @@ int write_thread(void* pipeline_context) { caller apply it once every writer has drained. */ if (!dry_run && result != FILE_SAVE_ERROR && file->is_dir && file->metadata && dir_metadata_should_capture(context->config) && - !dir_time_list_add(&context->dir_times, file->path, file->metadata)) { + !dir_time_list_add(&context->dir_times, file->path, file->metadata, file->xattrs)) { file_destroy(file); pipeline_context_receiver_note_bytes_released(context, file_bytes); mtx_lock(&context->mutex); diff --git a/src/shared/batch.c b/src/shared/batch.c index 0ec2103..42dab5e 100644 --- a/src/shared/batch.c +++ b/src/shared/batch.c @@ -172,7 +172,7 @@ int batch_read_apply(int fd, const Config* config, const char* dest_root) { * destroyed; applied once the whole stream has been consumed. */ if (save != FILE_SAVE_ERROR && file->is_dir && file->metadata && dir_metadata_should_capture(config) && - !dir_time_list_add(&dir_times, file->path, file->metadata)) { + !dir_time_list_add(&dir_times, file->path, file->metadata, file->xattrs)) { file_destroy(file); chunk_destroy(chunk); goto done; diff --git a/src/shared/config.c b/src/shared/config.c index 7c968f5..88145e7 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -749,10 +749,18 @@ void config_delete(Config* config) { free(config->skip_compress_suffixes[i]); free(config->skip_compress_suffixes); } - free(config->usermap); + if (config->usermap) { + for (int i = 0; i < config->usermap_count; i++) + free(config->usermap[i].to_name); + free(config->usermap); + } config->usermap = NULL; config->usermap_count = 0; - free(config->groupmap); + if (config->groupmap) { + for (int i = 0; i < config->groupmap_count; i++) + free(config->groupmap[i].to_name); + free(config->groupmap); + } config->groupmap = NULL; config->groupmap_count = 0; if (config->filters) { @@ -986,7 +994,8 @@ static bool receive_basis_entries(int fd, Config* c, ConfigStringBudget* budget) static bool send_identity_entries(int fd, const IdentityMap* map, int count) { for (int i = 0; i < count; i++) { - if (!send_int(fd, map[i].from) || !send_int(fd, map[i].to)) + if (!send_int(fd, map[i].from) || !send_int(fd, map[i].from_hi) || !send_int(fd, map[i].to) || + !send_str(fd, map[i].to_name ? map[i].to_name : "")) return false; } return true; @@ -994,20 +1003,32 @@ static bool send_identity_entries(int fd, const IdentityMap* map, int count) { static bool receive_identity_entries(int fd, ConfigStringBudget* budget, int count, IdentityMap** out) { - (void)budget; if (count <= 0) return true; IdentityMap* map = calloc((size_t)count, sizeof(IdentityMap)); if (!map) return false; for (int i = 0; i < count; i++) { - if (!receive_int(fd, &map[i].from) || !receive_int(fd, &map[i].to)) { - free(map); - return false; + if (!receive_int(fd, &map[i].from) || !receive_int(fd, &map[i].from_hi) || + !receive_int(fd, &map[i].to)) + goto fail; + char* name = config_receive_str(fd, budget); + if (!name) + goto fail; + if (name[0] == '\0') { + free(name); + map[i].to_name = NULL; + } else { + map[i].to_name = name; } } *out = map; return true; +fail: + for (int i = 0; i < count; i++) + free(map[i].to_name); + free(map); + return false; } /* --------------------------------------------------------------------------- diff --git a/src/shared/config.h b/src/shared/config.h index 575b0b8..677972a 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -39,15 +39,20 @@ typedef struct BasisDest { char* path; /* relative to the destination root (receiver-confined) */ } BasisDest; -/* One resolved FROM:TO identity-mapping rule (--usermap / --groupmap). Both - * fields are numeric ids. IDENTITY_MATCH_ANY (-1) in `from` is rsync's '*' - * wildcard (matches any transmitted id); IDENTITY_CURRENT (-1) in `to` makes - * the receiver resolve the receiving process's own current euid/egid at apply - * time. Names are resolved to numbers at parse time on the client (see - * identity.h for the exact subset). */ +/* One FROM:TO identity-mapping rule (--usermap / --groupmap). `from`/`from_hi` + * describe the sender-side FROM matcher (a single id when from_hi == from, an + * inclusive LOW-HIGH range, IDENTITY_MATCH_ANY for rsync's '*', or + * IDENTITY_MATCH_UNNAMED for rsync's empty FROM). `to` is the receiver-side TO + * numeric id (IDENTITY_CURRENT = the receiving process's own euid/egid) UNLESS + * `to_name` is non-NULL, in which case the receiver resolves the name against + * its own account database at apply time (rsync resolves TO names on the + * receiver) and `to` is ignored. FROM names/ranges/globs are resolved on the + * client (the sender) exactly as rsync matches them against sender names. */ typedef struct { int32_t from; + int32_t from_hi; int32_t to; + char* to_name; } IdentityMap; /* --sockopts=OPTIONS allowlist. Only these option names are accepted; anything @@ -577,13 +582,17 @@ typedef struct Config { * targets and, with -K, follows an in-root destination symlink-to-directory); * -k/--copy-dirlinks is sender-only and is never serialized. */ /* numeric_ids */ - /* --numeric-ids: no name lookup, use the transmitted numeric ids raw. */ + /* --numeric-ids: a mapping MODIFIER only -- no name lookup, use the + * transmitted numeric ids raw. It does NOT by itself request ownership. */ /* chown_uid_set */ /* --chown USER (owner) override; IDENTITY_CURRENT = the receiver's euid. */ /* chown_gid_set */ /* --chown :GROUP (group) override; IDENTITY_CURRENT = the receiver's egid. */ /* usermap */ - /* --usermap / --groupmap entries, in order (first match wins). */ + /* --usermap / --groupmap entries, in order (first match wins). Each entry's + * from/from_hi are a single id, an inclusive range, IDENTITY_MATCH_ANY ('*'), + * or IDENTITY_MATCH_UNNAMED (empty FROM); to_name carries a receiver-resolved + * TO name (rsync resolves TO names on the receiving side). */ /* preserve_atimes */ /* -U/--atimes: preserve source access times on the destination. */ /* preserve_crtimes */ @@ -611,8 +620,11 @@ typedef struct Config { * --copy-as) imply it. */ /* fake_super */ /* --fake-super: receiver-only. When set, each written file additionally gets - * a reserved user.fastsync.stat xattr recording the source uid/gid/mode/mtime - * so a later privileged restore could re-apply them. Crosses the wire. */ + * a reserved user.fastsync.stat xattr recording the RESOLVED uid/gid (the + * source's own when no ownership request is active, else the --chown/--usermap + * result) plus mode/mtime so a later privileged restore could re-apply them. + * It NEVER real-chowns: the point is to record the source ownership on an + * unprivileged receiver. Crosses the wire. */ /* module */ /* Daemon module selection (Wave A, protocol 2.15.0). Client-composed from a * host::module/path destination; NULL or "" means "no module" (the ordinary @@ -852,22 +864,35 @@ typedef struct Config { * UNCHANGED: the receiver still gates attribute application on use_metadata, * which is now DERIVED from these attributes by config_derived_use_metadata(). * - * Delete-Semantics Wave (#290): 2.22.0 -> 2.23.0. + * Rsync-Parity Wave: 2.22.0 -> 2.23.0. * - * WHY the bump, grounded in the wire: the delete-manifest frame gains a fourth - * trailing section (protocol 2.23.0): a synchronized-directory count followed by - * that many destination-relative directory paths (the receive root is "."). - * The receiver confines its extras walk to these directories, so `--files-from` - * with `--delete` only removes inside listed directory subtrees (rsync parity) + * WHY the bump, grounded in the wire. Several independent changes land in this + * protocol version: + * + * (1) Ownership parity (#286/#294): each --usermap/--groupmap wire entry grows + * from two int32s to [from][from_hi][to][to_name]; `from_hi` carries an + * inclusive LOW-HIGH range (== from for a single/any/unnamed matcher) and the + * trailing string carries a TO NAME for the receiver to resolve (rsync resolves + * TO names on the receiving side). The STATUS_MKDIR and STATUS_DIR_TIMES frames + * also gain a bounded per-entry xattr block when -X/-A is negotiated, so + * directory xattrs/ACLs (including default ACLs) are preserved like regular-file + * xattrs. + * + * (2) Delete semantics (#290): the delete-manifest frame gains a fourth trailing + * section -- a synchronized-directory count followed by that many + * destination-relative directory paths (the receive root is "."). The receiver + * confines its extras walk to these directories, so `--files-from` with + * `--delete` only removes inside listed directory subtrees (rsync parity) * instead of deleting every untransmitted path under the receive root. The * frame stream also gains STATUS_DELETE_LIMIT, the terminal success status sent * instead of STATUS_OK when a --max-delete commit removes up to the bound and - * skips the rest (the sender then exits 25 like rsync). The config-frame LAYOUT - * is unchanged. Any manifest/frame-sequence change must bump the protocol - * version: a 2.22 peer would desynchronize on the extra trailing section or the - * unknown status, and the strict same-version handshake (config_receive rejects - * a mismatched version before parsing anything else) is what keeps a 2.23 client - * and a 2.22 server from ever reaching that state. */ + * skips the rest (the sender then exits 25 like rsync). + * + * Any config-frame layout or frame-sequence change must bump the protocol + * version: a 2.22 peer would desynchronize on the new entry bytes, the extra + * trailing section or the unknown status, and the strict same-version handshake + * (config_receive rejects a mismatched version before parsing anything else) is + * what keeps a 2.23 client and a 2.22 server from ever reaching that state. */ #define PROTOCOL_VERSION "2.23.0" #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024) /* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */ @@ -893,9 +918,11 @@ typedef struct Config { /* Identity-mapping sentinels and bounds (see identity.h for semantics). * IDENTITY_MATCH_ANY is a usermap/groupmap FROM '*' (matches any id); - * IDENTITY_CURRENT is a chown / map TO '*' (resolve to the receiver's current - * euid/egid at apply time). */ + * IDENTITY_MATCH_UNNAMED is a FROM with an empty token (rsync's "ids with no + * name on the sender"); IDENTITY_CURRENT is a chown / map TO '*' (resolve to + * the receiver's current euid/egid at apply time). */ #define IDENTITY_MATCH_ANY (-1) +#define IDENTITY_MATCH_UNNAMED (-2) #define IDENTITY_CURRENT (-1) #define MAX_IDENTITY_MAP 128 diff --git a/src/shared/file.c b/src/shared/file.c index 4b33d86..7a636f4 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -986,13 +986,18 @@ static void restore_extra_fd(int fd, const FileMetadata* metadata, const FileXat bool fake_super, FileAttrPolicy policy) { xattr_apply_fd(fd, xattrs); if (fake_super && metadata) { - fake_super_store_fd(fd, (uint32_t)metadata->uid, (uint32_t)metadata->gid, - (uint32_t)metadata->mode, metadata->mtime_sec, metadata->mtime_nsec); - /* Replay: re-apply the recorded uid/gid/mode/mtime fd-relative so a save - under --fake-super restores the attrs (when privileged) instead of only - recording them. Best-effort; fake_super_restore_fd silently skips a - non-root fchown EPERM/EACCES and never fatal. The replayed mode/mtime - honor the per-attribute policy so fake-super cannot bypass the split. */ + /* Record the ownership that WOULD have been applied: when an explicit + ownership request (--chown/--usermap/--groupmap/--copy-as or -o/-g) is + active, the resolved mapping; otherwise the source's own id. The real + chown is suppressed (identity_apply_ownership early-returns under + --fake-super) so recording never defeats the flag. Mode/mtime are still + replayed (policy-gated) so unprivileged --fake-super keeps working. */ + uint32_t store_uid; + uint32_t store_gid; + identity_resolve_storage_ids((int32_t)metadata->uid, (int32_t)metadata->gid, &store_uid, + &store_gid); + fake_super_store_fd(fd, store_uid, store_gid, (uint32_t)metadata->mode, metadata->mtime_sec, + metadata->mtime_nsec); fake_super_restore_fd(fd, policy); } } diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 94e7d2c..8d30f30 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -2434,11 +2434,14 @@ File* file_receive(const Config* config, int file_descriptor) { bool dir_metadata_should_capture(const Config* config) { /* Directory metadata is captured when a directory attribute is actually - * requested: -p/--perms (directory modes) or -t/--times (directory mtimes, - * unless -O/--omit-dir-times suppresses them). --atimes/-U alone does not - * pull directory metadata (matching the original dir-time bundle). */ + * requested: -p/--perms (directory modes), -t/--times (directory mtimes, + * unless -O/--omit-dir-times suppresses them), -o/-g (directory ownership), + * or -X/-A (directory xattrs/ACLs). --atimes/-U alone does not pull + * directory metadata (matching the original dir-time bundle). */ return config && config->use_metadata && - (config->preserve_perms || (config->preserve_times && !config->omit_dir_times)); + (config->preserve_perms || (config->preserve_times && !config->omit_dir_times) || + config->preserve_owner || config->preserve_group || config->preserve_xattrs || + config->preserve_acls); } void dir_time_list_init(DirTimeList* list) { @@ -2446,6 +2449,7 @@ void dir_time_list_init(DirTimeList* list) { return; list->paths = NULL; list->entries = NULL; + list->xattrs = NULL; list->count = 0; list->capacity = 0; list->bytes = 0; @@ -2454,18 +2458,23 @@ void dir_time_list_init(DirTimeList* list) { void dir_time_list_free(DirTimeList* list) { if (!list) return; - for (size_t i = 0; i < list->count; i++) + for (size_t i = 0; i < list->count; i++) { free(list->paths[i]); + xattr_list_free(list->xattrs ? list->xattrs[i] : NULL); + } free(list->paths); free(list->entries); + free(list->xattrs); list->paths = NULL; list->entries = NULL; + list->xattrs = NULL; list->count = 0; list->capacity = 0; list->bytes = 0; } -bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata) { +bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata, + const FileXattrList* xattrs) { if (!list || !wire_path || !metadata) return true; /* nothing to remember; never a hard error */ /* Cumulative, not per-frame: the sender may stream a tree across unbounded @@ -2474,9 +2483,14 @@ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetad transfer, which becomes a clean protocol error). */ size_t path_len = strlen(wire_path); /* Charge the whole per-entry cost (path copy + pointer slot + metadata - struct), not just the path, so the array growth is bounded by the same - cumulative budget. */ - size_t entry_cost = path_len + sizeof(FileMetadata) + sizeof(char*); + struct + captured xattrs), not just the path, so the array growth is + bounded by the same cumulative budget. */ + size_t xattr_cost = 0; + if (xattrs) { + for (int i = 0; i < xattrs->count; i++) + xattr_cost += strlen(xattrs->items[i].name) + xattrs->items[i].value_len + sizeof(FileXattr); + } + size_t entry_cost = path_len + sizeof(FileMetadata) + 2 * sizeof(char*) + xattr_cost; if (list->count >= MAX_DIR_TIME_ENTRIES || entry_cost > MAX_DIR_TIME_BYTES - list->bytes) return false; if (list->count == list->capacity) { @@ -2485,10 +2499,9 @@ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetad return false; /* Assign each grown array as soon as its realloc succeeds: the old block is already freed by then, so discarding the pointer would dangle. capacity - is advanced only after BOTH reallocs succeed, so a partial failure leaves - capacity no larger than the entries allocation (the paths array may be - over-allocated, which is harmless) -- never a mismatched list the next - add could write past. */ + is advanced only after ALL reallocs succeed, so a partial failure leaves + capacity no larger than the smallest allocation -- never a mismatched + list the next add could write past. */ char** grown_paths = realloc(list->paths, new_capacity * sizeof(char*)); if (!grown_paths) return false; @@ -2497,13 +2510,23 @@ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetad if (!grown_entries) return false; list->entries = grown_entries; + FileXattrList** grown_xattrs = realloc(list->xattrs, new_capacity * sizeof(FileXattrList*)); + if (!grown_xattrs) + return false; + list->xattrs = grown_xattrs; list->capacity = new_capacity; } char* copy = str_dup(wire_path); if (!copy) return false; + FileXattrList* xattr_copy = xattr_list_clone(xattrs); + if (xattrs && !xattr_copy) { + free(copy); + return false; + } list->paths[list->count] = copy; list->entries[list->count] = *metadata; + list->xattrs[list->count] = xattr_copy; list->count++; list->bytes += entry_cost; return true; @@ -2515,7 +2538,12 @@ void dir_metadata_list_apply(const DirTimeList* list, const char* root_directory return; bool apply_times = config->preserve_times && !config->omit_dir_times; bool apply_mode = config->preserve_perms; - if (!apply_times && !apply_mode) + bool apply_xattrs = config->use_xattrs; + /* Ownership is applied through the active identity snapshot (which no-ops + * unless an ownership request is active), and xattrs only when -X/-A was + * negotiated. Times/mode keep their own per-attribute gates. */ + bool have_any = apply_times || apply_mode || apply_xattrs || identity_active_enabled(); + if (!have_any) return; for (size_t i = 0; i < list->count; i++) { char* dir_path = path_cat(root_directory, list->paths[i]); @@ -2544,6 +2572,13 @@ void dir_metadata_list_apply(const DirTimeList* list, const char* root_directory free(dir_path); continue; } + /* One O_DIRECTORY|O_NOFOLLOW fd drives ownership/mode/xattr application so + none of them can follow a same-named symlink planted after the fstatat. */ + int dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + /* Ownership first: a chown clears setuid/setgid, so it must precede mode. */ + if (dir_fd >= 0) + identity_apply_ownership(dir_fd, (int32_t)list->entries[i].uid, + (int32_t)list->entries[i].gid); if (apply_times) { struct timespec times[2] = { {.tv_sec = 0, .tv_nsec = UTIME_OMIT}, @@ -2573,28 +2608,28 @@ void dir_metadata_list_apply(const DirTimeList* list, const char* root_directory if (mode_ready) { /* Route the directory mode through the SAME sanitization as the * regular-file policy: a client-supplied mode never grants group/other - * write. Open the directory with O_DIRECTORY|O_NOFOLLOW (never - * following a same-named symlink) and fchmod the fd, avoiding the - * fchmodat(..., 0) TOCTOU/symlink-follow hole. */ + * write. */ mode_t safe_mode = (dir_mode & 0777 & ~(S_IWGRP | S_IWOTH)) | (dir_mode & (S_ISGID | S_ISVTX)); - int dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (dir_fd < 0) { char* escaped_path = output_escape(dir_path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "Failed to open directory %s to set its mode: %s", escaped_path ? escaped_path : "", strerror(errno)); free(escaped_path); - } else { - if (fchmod(dir_fd, safe_mode) != 0) { - char* escaped_path = output_escape(dir_path, log_get_8_bit_output()); - log_message(LOG_LEVEL_WARNING, "Failed to set directory mode on %s: %s", - escaped_path ? escaped_path : "", strerror(errno)); - free(escaped_path); - } - close(dir_fd); + } else if (fchmod(dir_fd, safe_mode) != 0) { + char* escaped_path = output_escape(dir_path, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, "Failed to set directory mode on %s: %s", + escaped_path ? escaped_path : "", strerror(errno)); + free(escaped_path); } } } + /* xattrs/ACLs last: a mode change can rewrite the ACL mask, so the ACL + xattrs must be (re)applied after fchmod. */ + if (apply_xattrs && dir_fd >= 0 && list->xattrs) + xattr_apply_fd(dir_fd, list->xattrs[i]); + if (dir_fd >= 0) + close(dir_fd); close(parent_fd); free(leaf); free(dir_path); @@ -2634,6 +2669,11 @@ File* file_receive_directory(int file_descriptor, const Config* config) { return NULL; } } + /* Directory xattrs/ACLs (-X/-A) ride after the metadata when negotiated. */ + if (config && !receive_file_xattrs(file, file_descriptor, config)) { + file_destroy(file); + return NULL; + } return file; } @@ -2670,6 +2710,12 @@ File* file_receive_dir_time(int file_descriptor, const Config* config) { return NULL; } } + /* Directory xattrs/ACLs (-X/-A) ride after the metadata, mirroring the + sender's send_dir_times(). */ + if (config && !receive_file_xattrs(file, file_descriptor, config)) { + file_destroy(file); + return NULL; + } return file; } diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index c88cdee..4f2b281 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -40,8 +40,9 @@ File* receive_incremental_check_ex(int fd, const Config* config, bool* skipped, * parent's mtime). -O/--omit-dir-times skips the application entirely. The * list owns deep copies of the paths and metadata; freed on every path. */ typedef struct { - char** paths; /* owned, destination-relative wire paths */ - FileMetadata* entries; /* owned, parallel to paths */ + char** paths; /* owned, destination-relative wire paths */ + FileMetadata* entries; /* owned, parallel to paths */ + FileXattrList** xattrs; /* owned, parallel to paths; NULL when none */ size_t count; size_t capacity; size_t bytes; /* cumulative strlen of every retained path */ @@ -57,17 +58,19 @@ bool dir_metadata_should_capture(const Config* config); void dir_time_list_init(DirTimeList* list); void dir_time_list_free(DirTimeList* list); -/* Deep-copy one directory's path + metadata into the list. Returns false on - * allocation failure OR when the cumulative entry/byte caps would be exceeded - * (the caller fails the transfer). */ -bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata); +/* Deep-copy one directory's path + metadata (and, when non-NULL, its captured + * xattr/ACL block) into the list. Returns false on allocation failure OR when + * the cumulative entry/byte caps would be exceeded (the caller fails the + * transfer). */ +bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata, + const FileXattrList* xattrs); /* Apply every accumulated directory's metadata beneath `root_directory`, - * confined fd-relative. Times (mtime, plus atime when -U captured one) are - * applied only when config->preserve_times && !config->omit_dir_times; the mode - * (through --chmod when configured) is applied only when config->preserve_perms. - * Best-effort per entry: an absent directory (an empty/pruned source dir that - * was deliberately not created) or a non-directory at the path is skipped - * QUIETLY, an unreachable one with a warning, and never fatal. */ + * confined fd-relative: ownership through the negotiated identity policy, + * times (mtime, plus atime when -U captured one under -t), the mode (through + * --chmod when configured, under -p), and the captured xattrs/ACLs (under + * -X/-A). Best-effort per entry: an absent directory (an empty/pruned source + * dir that was deliberately not created) or a non-directory at the path is + * skipped QUIETLY, an unreachable one with a warning, and never fatal. */ void dir_metadata_list_apply(const DirTimeList* list, const char* root_directory, const Config* config); diff --git a/src/shared/identity.c b/src/shared/identity.c index 6e44252..c806789 100644 --- a/src/shared/identity.c +++ b/src/shared/identity.c @@ -43,14 +43,27 @@ typedef struct { * so they are tracked separately from the explicit ownership gate. */ bool preserve_owner; bool preserve_group; + /* --fake-super: when active the receiver must only RECORD the (resolved) + * ownership in the reserved xattr, never perform a real chown. Snapshotted + * so the fd-relative ownership helpers can suppress the chown without a + * Config argument. */ + bool fake_super; bool set; } IdentityActive; static IdentityActive g_identity; static void identity_active_reset(void) { - free(g_identity.usermap); - free(g_identity.groupmap); + if (g_identity.usermap) { + for (int i = 0; i < g_identity.usermap_count; i++) + free(g_identity.usermap[i].to_name); + free(g_identity.usermap); + } + if (g_identity.groupmap) { + for (int i = 0; i < g_identity.groupmap_count; i++) + free(g_identity.groupmap[i].to_name); + free(g_identity.groupmap); + } g_identity.usermap = NULL; g_identity.groupmap = NULL; g_identity.usermap_count = 0; @@ -66,6 +79,7 @@ static void identity_active_reset(void) { g_identity.copy_as_gid = 0; g_identity.preserve_owner = false; g_identity.preserve_group = false; + g_identity.fake_super = false; g_identity.set = false; } @@ -88,20 +102,35 @@ bool identity_set_active(const Config* config) { g_identity.copy_as_gid = config->copy_as_gid; g_identity.preserve_owner = config->preserve_owner; g_identity.preserve_group = config->preserve_group; + g_identity.fake_super = config->fake_super; if (config->usermap_count > 0) { g_identity.usermap = calloc((size_t)config->usermap_count, sizeof(IdentityMap)); if (!g_identity.usermap) goto alloc_failed; - memcpy(g_identity.usermap, config->usermap, - (size_t)config->usermap_count * sizeof(IdentityMap)); + for (int i = 0; i < config->usermap_count; i++) { + g_identity.usermap[i] = config->usermap[i]; + g_identity.usermap[i].to_name = + config->usermap[i].to_name ? str_dup(config->usermap[i].to_name) : NULL; + if (config->usermap[i].to_name && !g_identity.usermap[i].to_name) { + g_identity.usermap_count = i; /* free only the entries already duplicated */ + goto alloc_failed; + } + } g_identity.usermap_count = config->usermap_count; } if (config->groupmap_count > 0) { g_identity.groupmap = calloc((size_t)config->groupmap_count, sizeof(IdentityMap)); if (!g_identity.groupmap) goto alloc_failed; - memcpy(g_identity.groupmap, config->groupmap, - (size_t)config->groupmap_count * sizeof(IdentityMap)); + for (int i = 0; i < config->groupmap_count; i++) { + g_identity.groupmap[i] = config->groupmap[i]; + g_identity.groupmap[i].to_name = + config->groupmap[i].to_name ? str_dup(config->groupmap[i].to_name) : NULL; + if (config->groupmap[i].to_name && !g_identity.groupmap[i].to_name) { + g_identity.groupmap_count = i; + goto alloc_failed; + } + } g_identity.groupmap_count = config->groupmap_count; } g_identity.set = true; @@ -157,30 +186,28 @@ bool privilege_super_mode_permitted(SuperMode mode) { } bool identity_active_enabled(void) { - /* numeric_ids is included: this set only gates identity_apply_ownership, - which runs only when metadata is present (a -M/--preserve transfer). A - standalone --numeric-ids (no ownership-affecting flag) carries no - metadata, never reaches identity_apply_ownership, and therefore correctly - stays inert; combined with -M it activates raw-id application. --super / - --no-super does NOT enable ownership: it only permits or forbids the - already-requested super-user activities, so a --super with no explicit - identity flag must never silently apply client-chosen ownership. */ + /* --numeric-ids is deliberately NOT included: it is a mapping MODIFIER (use + * the transmitted numeric id raw instead of a name lookup), not a request to + * change ownership. rsync's --numeric-ids on its own never chowns anything; + * it only changes how an already-requested -o/-g/map resolves. Ownership is + * activated only by an explicit request: --chown/--usermap/--groupmap/ + * --copy-as or a preserve-source -o/--owner / -g/--group. --super/--no-super + * likewise does NOT enable ownership: it only permits or forbids the + * already-requested super-user activities. */ return g_identity.set && - (g_identity.numeric_ids || g_identity.chown_uid_set || g_identity.chown_gid_set || - g_identity.usermap_count > 0 || g_identity.groupmap_count > 0 || g_identity.copy_as_set || - g_identity.preserve_owner || g_identity.preserve_group); + (g_identity.chown_uid_set || g_identity.chown_gid_set || g_identity.usermap_count > 0 || + g_identity.groupmap_count > 0 || g_identity.copy_as_set || g_identity.preserve_owner || + g_identity.preserve_group); } bool identity_owner_requested(void) { - return g_identity.set && - (g_identity.copy_as_set || g_identity.chown_uid_set || g_identity.numeric_ids || - g_identity.preserve_owner || g_identity.usermap_count > 0); + return g_identity.set && (g_identity.copy_as_set || g_identity.chown_uid_set || + g_identity.preserve_owner || g_identity.usermap_count > 0); } bool identity_group_requested(void) { - return g_identity.set && - (g_identity.copy_as_set || g_identity.chown_gid_set || g_identity.numeric_ids || - g_identity.preserve_group || g_identity.groupmap_count > 0); + return g_identity.set && (g_identity.copy_as_set || g_identity.chown_gid_set || + g_identity.preserve_group || g_identity.groupmap_count > 0); } bool identity_ownership_requested(const Config* config) { @@ -228,6 +255,29 @@ bool identity_copy_as_refused(const Config* config) { return geteuid() != 0 || config->super_mode == SUPER_MODE_OFF; } +/* Validate one received FROM:TO map rule. `from` is a single id, the LOW end + * of an inclusive range, IDENTITY_MATCH_ANY, or IDENTITY_MATCH_UNNAMED; a + * sentinel FROM must carry the same value in from_hi. `to` is a non-negative + * id, IDENTITY_CURRENT, or ignored when a bounded receiver-resolved `to_name` + * is present. */ +static bool identity_wire_map_valid(const IdentityMap* map) { + if (!map) + return false; + if (map->from < IDENTITY_MATCH_UNNAMED) + return false; + if (map->from < 0) { + if (map->from_hi != map->from) + return false; + } else if (map->from_hi < map->from) { + return false; + } + if (map->to < IDENTITY_CURRENT) + return false; + if (map->to_name && strlen(map->to_name) > 255) + return false; + return true; +} + bool identity_wire_valid(const Config* config) { if (!config) return false; @@ -239,11 +289,11 @@ bool identity_wire_valid(const Config* config) { if (config->chown_gid_set && config->chown_gid < IDENTITY_MATCH_ANY) return false; for (int i = 0; i < config->usermap_count; i++) { - if (config->usermap[i].from < IDENTITY_MATCH_ANY || config->usermap[i].to < IDENTITY_CURRENT) + if (!identity_wire_map_valid(&config->usermap[i])) return false; } for (int i = 0; i < config->groupmap_count; i++) { - if (config->groupmap[i].from < IDENTITY_MATCH_ANY || config->groupmap[i].to < IDENTITY_CURRENT) + if (!identity_wire_map_valid(&config->groupmap[i])) return false; } /* Defense-in-depth: a --copy-as block must never carry a negative (sentinel) @@ -301,15 +351,137 @@ static int identity_resolve_token(const char* token, bool is_group, int32_t* out return 0; } -static int identity_append_rule(IdentityMap** map, int* count, int32_t from, int32_t to) { +static bool identity_all_digits(const char* token) { + if (!token || *token == '\0') + return false; + for (const char* p = token; *p; p++) + if (*p < '0' || *p > '9') + return false; + return true; +} + +static bool identity_token_has_glob(const char* token) { + return token && (strchr(token, '*') || strchr(token, '?') || strchr(token, '[')); +} + +/* Parse a --usermap/--groupmap FROM token into a matcher (from/from_hi). rsync + * accepts a name, a numeric id, an inclusive LOW-HIGH range, '*' (any id), or an + * empty token (ids with no name on the sender). Returns 0 on success, -1 on a + * malformed token or an unresolvable sender-side name. */ +static int identity_parse_from(const char* token, bool is_group, int32_t* out_from, + int32_t* out_hi) { + if (token[0] == '\0') { + *out_from = IDENTITY_MATCH_UNNAMED; + *out_hi = IDENTITY_MATCH_UNNAMED; + return 0; + } + if (strcmp(token, "*") == 0) { + *out_from = IDENTITY_MATCH_ANY; + *out_hi = IDENTITY_MATCH_ANY; + return 0; + } + const char* num = token[0] == '@' ? token + 1 : token; + if (identity_all_digits(num)) { + int32_t id; + if (identity_resolve_token(token, is_group, &id) != 0) + return -1; + *out_from = id; + *out_hi = id; + return 0; + } + /* An inclusive LOW-HIGH numeric range. */ + const char* dash = strchr(num, '-'); + if (dash && dash != num && dash[1] != '\0' && strchr(dash + 1, '-') == NULL) { + size_t lo_len = (size_t)(dash - num); + size_t hi_len = strlen(dash + 1); + char low[16]; + char high[16]; + if (lo_len < sizeof(low) && hi_len < sizeof(high)) { + memcpy(low, num, lo_len); + low[lo_len] = '\0'; + memcpy(high, dash + 1, hi_len); + high[hi_len] = '\0'; + if (identity_all_digits(low) && identity_all_digits(high)) { + char* endptr = NULL; + errno = 0; + long lo = strtol(low, &endptr, 10); + if (errno != 0 || !endptr || *endptr != '\0') + return -1; + errno = 0; + long hi = strtol(high, &endptr, 10); + if (errno != 0 || !endptr || *endptr != '\0' || hi < lo || hi > INT32_MAX) + return -1; + *out_from = (int32_t)lo; + *out_hi = (int32_t)hi; + return 0; + } + } + /* Not a numeric LOW-HIGH range: fall through and treat as a name (a + * hyphenated account name like "wayne-smith" must still resolve). */ + } + /* A sender-side name. A wildcard other than the bare '*' is matched by rsync + * against the sender's names; because FastSync transmits numeric ids only, the + * receiver cannot evaluate it, so reject rather than silently mis-match. */ + if (identity_token_has_glob(token)) { + log_message(LOG_LEVEL_ERROR, + "%smap FROM '%s': name wildcards other than '*' are not supported " + "(FastSync transmits numeric ids, so sender names are unavailable on the " + "receiver)", + is_group ? "--group" : "--user", token); + return -1; + } + int32_t id; + if (identity_resolve_token(token, is_group, &id) != 0) + return -1; + *out_from = id; + *out_hi = id; + return 0; +} + +/* Parse a --usermap/--groupmap TO token. '*', a bare numeric id, or an @N id is + * stored numerically; every other non-empty token is a NAME resolved on the + * RECEIVER at apply time (rsync resolves TO names against the receiving side). + * Returns 0 on success, -1 on an empty/malformed token. */ +static int identity_parse_to(const char* token, bool is_group, int32_t* out_to, char** out_name) { + if (token[0] == '\0') { + log_message(LOG_LEVEL_ERROR, "%smap TO value is missing", is_group ? "--group" : "--user"); + return -1; + } + if (strcmp(token, "*") == 0) { + *out_to = IDENTITY_CURRENT; + *out_name = NULL; + return 0; + } + const char* num = token[0] == '@' ? token + 1 : token; + if (identity_all_digits(num)) { + int32_t id; + if (identity_resolve_token(token, is_group, &id) != 0) + return -1; + *out_to = id; + *out_name = NULL; + return 0; + } + if (identity_token_has_glob(token)) { + log_message(LOG_LEVEL_ERROR, "%smap TO '%s' may not contain a wildcard", + is_group ? "--group" : "--user", token); + return -1; + } + char* name = str_dup(token); + if (!name) + return -1; + *out_to = 0; + *out_name = name; + return 0; +} + +static int identity_append_rule(IdentityMap** map, int* count, const IdentityMap* rule) { if (*count >= MAX_IDENTITY_MAP) return -1; IdentityMap* grown = realloc(*map, (size_t)(*count + 1) * sizeof(IdentityMap)); if (!grown) return -1; *map = grown; - (*map)[*count].from = from; - (*map)[*count].to = to; + (*map)[*count] = *rule; (*count)++; return 0; } @@ -326,7 +498,7 @@ int identity_parse_map(Config* config, const char* value, bool is_group) { char* saveptr = NULL; for (char* rule = strtok_r(list, ",", &saveptr); rule; rule = strtok_r(NULL, ",", &saveptr)) { char* colon = strchr(rule, ':'); - if (!colon || colon == rule) { + if (!colon) { /* Log before freeing: `rule` points into the str_dup'd list. */ log_message(LOG_LEVEL_ERROR, "%s rules must be FROM:TO (got '%s')", optname, rule); free(list); @@ -335,25 +507,25 @@ int identity_parse_map(Config* config, const char* value, bool is_group) { *colon = '\0'; char* from_token = rule; char* to_token = colon + 1; - if (*to_token == '\0') { + IdentityMap parsed; + memset(&parsed, 0, sizeof(parsed)); + if (identity_parse_from(from_token, is_group, &parsed.from, &parsed.from_hi) != 0) { + log_message(LOG_LEVEL_ERROR, + "%s could not resolve FROM '%s' in '%s' (a name must exist on the " + "source; use @N for a numeric id)", + optname, from_token, value); free(list); - log_message(LOG_LEVEL_ERROR, "%s rule 'FROM:' is missing the TO value (got '%s')", optname, - value); return -1; } - int32_t from_id, to_id; - if (identity_resolve_token(from_token, is_group, &from_id) != 0 || - identity_resolve_token(to_token, is_group, &to_id) != 0) { + if (identity_parse_to(to_token, is_group, &parsed.to, &parsed.to_name) != 0) { + log_message(LOG_LEVEL_ERROR, "%s could not parse TO '%s' in '%s'", optname, to_token, value); free(list); - log_message(LOG_LEVEL_ERROR, - "%s could not resolve '%s' (name must exist on the source; use " - "@N for a numeric id)", - optname, value); return -1; } if (identity_append_rule(is_group ? &config->groupmap : &config->usermap, - is_group ? &config->groupmap_count : &config->usermap_count, from_id, - to_id) != 0) { + is_group ? &config->groupmap_count : &config->usermap_count, + &parsed) != 0) { + free(parsed.to_name); free(list); log_message(LOG_LEVEL_ERROR, "%s has too many rules (max %d)", optname, MAX_IDENTITY_MAP); return -1; @@ -628,17 +800,108 @@ done: /* ---- Receiver-side ownership application ---- */ -static bool identity_map_lookup(const IdentityMap* map, int count, int32_t source_id, +/* True when a map rule's FROM matcher accepts `id`. A sentinel FROM never + * carries a range. IDENTITY_MATCH_UNNAMED mirrors rsync's empty FROM: it + * matches only ids that have no name in the account database (rsync matches the + * sender's names; FastSync transmits numeric ids only, so it approximates this + * with the receiver's database -- documented in RSYNC_COMPAT.md). */ +static bool identity_map_from_matches(const IdentityMap* map, int32_t id, bool is_group) { + if (map->from == IDENTITY_MATCH_ANY) + return true; + if (map->from == IDENTITY_MATCH_UNNAMED) + return is_group ? (getgrgid((gid_t)id) == NULL) : (getpwuid((uid_t)id) == NULL); + return id >= map->from && id <= map->from_hi; +} + +/* First matching rule wins. A rule whose TO is a receiver-side name resolves it + * against the receiver's account database here; an unresolvable TO name is + * skipped with a warning and the next rule is considered (rsync prints "Unknown + * --usermap name on receiver" and leaves the id unmapped rather than aborting). */ +static bool identity_map_lookup(const IdentityMap* map, int count, int32_t source_id, bool is_group, int32_t* out_to) { for (int i = 0; i < count; i++) { - if (map[i].from == IDENTITY_MATCH_ANY || map[i].from == source_id) { + if (!identity_map_from_matches(&map[i], source_id, is_group)) + continue; + if (map[i].to_name) { + if (is_group) { + struct group* gr = getgrnam(map[i].to_name); + if (!gr) { + log_message(LOG_LEVEL_WARNING, "Unknown --groupmap name on receiver: %s", map[i].to_name); + continue; + } + *out_to = (int32_t)gr->gr_gid; + } else { + struct passwd* pw = getpwnam(map[i].to_name); + if (!pw) { + log_message(LOG_LEVEL_WARNING, "Unknown --usermap name on receiver: %s", map[i].to_name); + continue; + } + *out_to = (int32_t)pw->pw_uid; + } + } else { *out_to = map[i].to; - return true; } + return true; } return false; } +/* Resolve the owner side from the negotiated policy. Sets *out and returns + * true when an owner-affecting request is active (a usermap, --chown USER, or + * -o/--owner); returns false (leaving *out untouched) when the owner side is + * not requested, so callers can pass (uid_t)-1 to fchown and leave it as-is. + * --numeric-ids only changes the RESOLUTION (raw id instead of a name lookup); + * it never makes the side requested. */ +static bool identity_resolve_owner(int32_t source_uid, uid_t* out) { + if (!(g_identity.chown_uid_set || g_identity.preserve_owner || g_identity.usermap_count > 0)) + return false; + int32_t target; + if (identity_map_lookup(g_identity.usermap, g_identity.usermap_count, source_uid, false, + &target)) { + *out = target == IDENTITY_CURRENT ? geteuid() : (uid_t)target; + } else if (g_identity.chown_uid_set) { + *out = g_identity.chown_uid == IDENTITY_CURRENT ? geteuid() : (uid_t)g_identity.chown_uid; + } else if (g_identity.numeric_ids) { + *out = (uid_t)source_uid; + } else { + /* Best-effort name mapping against the receiver's own database. When the + * transmitted (numeric) id has no name here, fall back to the raw numeric id + * so -o still preserves the source owner. */ + struct passwd* pw = getpwuid((uid_t)source_uid); + if (pw) { + const struct passwd* mapped = getpwnam(pw->pw_name); + *out = mapped ? mapped->pw_uid : (uid_t)source_uid; + } else { + *out = (uid_t)source_uid; + } + } + return true; +} + +/* Group-side counterpart of identity_resolve_owner(). */ +static bool identity_resolve_group(int32_t source_gid, gid_t* out) { + if (!(g_identity.chown_gid_set || g_identity.preserve_group || g_identity.groupmap_count > 0)) + return false; + int32_t target; + if (identity_map_lookup(g_identity.groupmap, g_identity.groupmap_count, source_gid, true, + &target)) { + *out = target == IDENTITY_CURRENT ? getegid() : (gid_t)target; + } else if (g_identity.chown_gid_set) { + *out = g_identity.chown_gid == IDENTITY_CURRENT ? getegid() : (gid_t)g_identity.chown_gid; + } else if (g_identity.numeric_ids) { + *out = (gid_t)source_gid; + } else { + struct group* gr = getgrgid((gid_t)source_gid); + if (gr) { + const struct group* mapped = getgrnam(gr->gr_name); + *out = mapped ? mapped->gr_gid : (gid_t)source_gid; + } else { + *out = (gid_t)source_gid; + } + } + return true; +} + /* Resolve the target ownership from the negotiated policy against the entry's * current stat. Shared by the fd (regular file) and no-follow (symlink) apply * paths. Returns false when no side is to be changed. */ @@ -662,58 +925,12 @@ static bool identity_resolve_targets(const struct stat* st, int32_t source_uid, * request the owner/group respectively, and a side that is NOT requested must * be left exactly as it is (`-1` to fchown on that side). This is what lets * plain -g change only the group, or -o only the owner. */ - bool owner_requested = g_identity.chown_uid_set || g_identity.numeric_ids || - g_identity.preserve_owner || g_identity.usermap_count > 0; - bool group_requested = g_identity.chown_gid_set || g_identity.numeric_ids || - g_identity.preserve_group || g_identity.groupmap_count > 0; - if (!owner_requested && !group_requested) - return false; - - int32_t target; uid_t uid = (uid_t)-1; gid_t gid = (gid_t)-1; - - /* Priority (unchanged): usermap/groupmap > --chown > --numeric-ids (raw) > - * name mapping on the transmitted numeric id, with a raw-id fallback when the - * receiver has no name for that id. */ - if (owner_requested) { - if (identity_map_lookup(g_identity.usermap, g_identity.usermap_count, source_uid, &target)) { - uid = target == IDENTITY_CURRENT ? geteuid() : (uid_t)target; - } else if (g_identity.chown_uid_set) { - uid = g_identity.chown_uid == IDENTITY_CURRENT ? geteuid() : (uid_t)g_identity.chown_uid; - } else if (g_identity.numeric_ids) { - uid = (uid_t)source_uid; - } else { - /* Best-effort name mapping against the receiver's own database. When the - * transmitted (numeric) id has no name here, fall back to the raw numeric - * id so -o still preserves the source owner. */ - struct passwd* pw = getpwuid((uid_t)source_uid); - if (pw) { - const struct passwd* mapped = getpwnam(pw->pw_name); - uid = mapped ? mapped->pw_uid : (uid_t)source_uid; - } else { - uid = (uid_t)source_uid; - } - } - } - - if (group_requested) { - if (identity_map_lookup(g_identity.groupmap, g_identity.groupmap_count, source_gid, &target)) { - gid = target == IDENTITY_CURRENT ? getegid() : (gid_t)target; - } else if (g_identity.chown_gid_set) { - gid = g_identity.chown_gid == IDENTITY_CURRENT ? getegid() : (gid_t)g_identity.chown_gid; - } else if (g_identity.numeric_ids) { - gid = (gid_t)source_gid; - } else { - struct group* gr = getgrgid((gid_t)source_gid); - if (gr) { - const struct group* mapped = getgrnam(gr->gr_name); - gid = mapped ? mapped->gr_gid : (gid_t)source_gid; - } else { - gid = (gid_t)source_gid; - } - } - } + bool owner_requested = identity_resolve_owner(source_uid, &uid); + bool group_requested = identity_resolve_group(source_gid, &gid); + if (!owner_requested && !group_requested) + return false; /* Only change ownership when a requested side actually differs (avoid * needless syscalls and any chance of clearing setuid/setgid on an @@ -726,6 +943,30 @@ static bool identity_resolve_targets(const struct stat* st, int32_t source_uid, return true; } +/* --fake-super storage resolution: the receiver records the ownership it WOULD + * have applied. A requested side uses the resolved mapping (--copy-as / + * usermap / --chown / -o/-g, with --numeric-ids as the raw-id modifier); a side + * that was not requested keeps the source's own id, so a plain --fake-super run + * records the source owner untouched. */ +void identity_resolve_storage_ids(int32_t source_uid, int32_t source_gid, uint32_t* out_uid, + uint32_t* out_gid) { + if (g_identity.copy_as_set) { + *out_uid = (uint32_t)g_identity.copy_as_uid; + *out_gid = (uint32_t)g_identity.copy_as_gid; + return; + } + uid_t uid = (uid_t)source_uid; + gid_t gid = (gid_t)source_gid; + uid_t resolved_uid; + gid_t resolved_gid; + if (identity_resolve_owner(source_uid, &resolved_uid)) + uid = resolved_uid; + if (identity_resolve_group(source_gid, &resolved_gid)) + gid = resolved_gid; + *out_uid = (uint32_t)uid; + *out_gid = (uint32_t)gid; +} + static void identity_log_chown_failure(const char* what, uid_t uid, gid_t gid) { /* EPERM/EACCES are expected when the receiver is not privileged (e.g. the CI * `nobody` user): warn and continue, never abort the transfer. Any other @@ -760,8 +1001,12 @@ bool identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid) { /* Ownership application is OFF unless the client requested an identity flag. * This is the controlled gate: a default (or plain -M) transfer never changes * ownership, byte-for-byte preserving FastSync's existing behavior. --no-super - * additionally forbids it even when the receiver is root. */ - if (!identity_active_enabled() || !privilege_super_permitted() || fd < 0) + * additionally forbids it even when the receiver is root. --fake-super never + * performs a REAL chown: that would defeat the point of the flag (record the + * source ownership on an unprivileged receiver for a later privileged + * restore); the resolved ownership is stored in the reserved xattr instead by + * fake_super_store_fd(). */ + if (!identity_active_enabled() || g_identity.fake_super || !privilege_super_permitted() || fd < 0) return true; struct stat st; if (fstat(fd, &st) != 0) @@ -781,7 +1026,8 @@ bool identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid) { bool identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t source_uid, int32_t source_gid) { - if (!identity_active_enabled() || !privilege_super_permitted() || parent_fd < 0 || !leaf) + if (!identity_active_enabled() || g_identity.fake_super || !privilege_super_permitted() || + parent_fd < 0 || !leaf) return true; struct stat st; if (fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) != 0) diff --git a/src/shared/identity.h b/src/shared/identity.h index 6b50b35..327205c 100644 --- a/src/shared/identity.h +++ b/src/shared/identity.h @@ -127,6 +127,14 @@ bool identity_explicit_ownership_requested(const Config* config); * is active returns true. */ bool identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid); +/* Resolve the ownership that --fake-super should RECORD in the reserved xattr + * (rather than chown for real). A requested side (--copy-as / usermap / + * --chown / -o / -g, with --numeric-ids as the raw-id modifier) yields the + * resolved target; a side that was not requested keeps the transmitted source + * id. Must be called after identity_set_active(). */ +void identity_resolve_storage_ids(int32_t source_uid, int32_t source_gid, uint32_t* out_uid, + uint32_t* out_gid); + /* P7 Wave D: the no-follow (symlink) counterpart. Resolves the same * usermap/groupmap/chown/numeric-ids/copy-as policy but applies it with * fchownat(..., AT_SYMLINK_NOFOLLOW) so a symlink's own ownership is changed diff --git a/src/shared/xattr.c b/src/shared/xattr.c index 91b139d..8c1e73c 100644 --- a/src/shared/xattr.c +++ b/src/shared/xattr.c @@ -38,6 +38,22 @@ void xattr_list_free(FileXattrList* list) { free(list); } +FileXattrList* xattr_list_clone(const FileXattrList* list) { + if (!list) + return NULL; + FileXattrList* clone = xattr_list_new(); + if (!clone) + return NULL; + for (int i = 0; i < list->count; i++) { + if (!xattr_list_append(clone, list->items[i].name, list->items[i].value, + list->items[i].value_len)) { + xattr_list_free(clone); + return NULL; + } + } + return clone; +} + bool xattr_list_append(FileXattrList* list, const char* name, const void* value, size_t value_len) { if (!list || !name || (!value && value_len != 0)) return false; @@ -365,29 +381,11 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6 } } -/* --fake-super replay: read the freshly-stored record and re-apply the source - * stat fd-relative. A privileged (root) run can actually change the owner; - * a non-root run silently skips the fchown on EPERM/EACCES (never fatal, - * mirroring the normal metadata identity path; other errors are logged) and - * still applies mode/mtime where permitted. - * - * The OWNER leg additionally honors three policies: - * - an ownership identity policy must be active: the explicit flags - * (--numeric-ids / --chown / --usermap / --groupmap / --copy-as) OR the - * preserve-source -o/--owner / -g/--group requests. --fake-super on its own - * only RECORDS the source owner; replaying that owner as a live chown - * without an ownership opt-in would be an un-gated client-chosen-ownership - * primitive. The owner and group sides are applied INDEPENDENTLY (through - * identity_owner_requested()/identity_group_requested()), so a plain -o or - * -g touches only the requested side and passes (uid_t)-1 / (gid_t)-1 for - * the other. - * - --no-super (privilege_super_permitted() false) suppresses it even for a - * root receiver, exactly like the normal metadata identity path. - * - an active --copy-as is AUTHORITATIVE: the identity path already forced the - * target owner, so replaying the recorded source owner here would silently - * override it. The xattr record is still stored/replayed for a later - * privileged restore; only the live chown is skipped. Mode/mtime remain - * applied either way so unprivileged --fake-super still works. */ +/* --fake-super replay: read the freshly-stored record and re-apply mode/mtime + * fd-relative. The recorded uid/gid are retained for a later privileged + * restore but are NEVER chowned here: --fake-super only RECORDS ownership, it + * must not real-chown the recorded (resolved) owner. Mode/mtime still apply so + * unprivileged --fake-super keeps working. */ bool fake_super_restore_fd(int fd, FileAttrPolicy policy) { if (fd < 0) return false; @@ -403,22 +401,12 @@ bool fake_super_restore_fd(int fd, FileAttrPolicy policy) { 5) return false; /* malformed record: skip, never fatal */ - /* Owner is applied best-effort only: a non-root process cannot chown and - must not abort the transfer for that reason (FastSync identity philosophy). - EPERM/EACCES (expected for a non-root receiver) are skipped silently; a - genuine EINVAL (an impossible stored id) is logged so the corruption is - not hidden. --no-super suppresses the owner leg even for root, and an - active --copy-as is authoritative so its forced owner must not be - overwritten by the recorded source owner. */ - if (identity_active_enabled() && privilege_super_permitted() && !identity_copy_as_active()) { - /* Apply only the requested side(s): an unchosen side is passed as -1 so the - * kernel leaves it exactly as-is. */ - uid_t owner = identity_owner_requested() ? (uid_t)ul_uid : (uid_t)-1; - gid_t group = identity_group_requested() ? (gid_t)ul_gid : (gid_t)-1; - if (fchown(fd, owner, group) != 0 && errno != EPERM && errno != EACCES) - log_message(LOG_LEVEL_WARNING, - "--fake-super: could not restore owner on destination file: %s", strerror(errno)); - } + /* --fake-super NEVER performs a real chown: that would defeat the whole + point of the flag (record privileged ownership on an unprivileged receiver + for a later privileged restore). The uid/gid parsed above are retained in + the record for that later restore, but no ownership change happens here. */ + (void)ul_uid; + (void)ul_gid; /* Mode is applied only when the per-attribute policy asks for it, through the SAME shared helper the normal metadata path uses (metadata_mode_for_policy): group/other write bits are never granted, so a recorded source mode of 0666 diff --git a/src/shared/xattr.h b/src/shared/xattr.h index 6f3c538..c61cd89 100644 --- a/src/shared/xattr.h +++ b/src/shared/xattr.h @@ -56,6 +56,8 @@ typedef struct { FileXattrList* xattr_list_new(void); void xattr_list_free(FileXattrList* list); +/* Deep-copy `list` (NULL in, NULL out). Returns NULL on allocation failure. */ +FileXattrList* xattr_list_clone(const FileXattrList* list); /* Append one entry (deep copy). Returns false on allocation failure. */ bool xattr_list_append(FileXattrList* list, const char* name, const void* value, size_t value_len); @@ -96,17 +98,16 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6 int64_t mtime_nsec); /* --fake-super replay: parse the FAKESUPER_XATTR record previously written on - * `fd` by fake_super_store_fd and re-apply uid/gid/mode/mtime fd-relative. - * Best-effort: absence of the xattr or a malformed record is a silent no-op - * that never fails the transfer. The OWNER leg is applied only when an explicit - * ownership identity policy is active (numeric-ids/chown/usermap/groupmap/ - * copy-as/-o/-g), when super-user activities are permitted, and when --copy-as - * is not authoritative; a non-root EPERM/EACCES is skipped silently, matching - * FastSync's identity philosophy. The MODE leg is applied only when - * policy.perms||policy.executability and the MTIME leg only when policy.times, - * so the fake-super replay cannot bypass the per-attribute split; the mode is - * sanitized exactly like the normal metadata path (group/other write bits never - * granted). Returns true when the xattr was present and parsed. */ + * `fd` by fake_super_store_fd and re-apply mode/mtime fd-relative. The + * recorded uid/gid are deliberately NOT chowned for real: --fake-super only + * RECORDS ownership (the caller stores the resolved mapping via + * identity_resolve_storage_ids), it never performs a real chown. Best-effort: + * absence of the xattr or a malformed record is a silent no-op that never fails + * the transfer. The MODE leg is applied only when policy.perms||policy. + * executability and the MTIME leg only when policy.times, so the fake-super + * replay cannot bypass the per-attribute split; the mode is sanitized exactly + * like the normal metadata path (group/other write bits never granted). + * Returns true when the xattr was present and parsed. */ bool fake_super_restore_fd(int fd, FileAttrPolicy policy); #endif \ No newline at end of file diff --git a/tests/fuzz/fuzz_config_receive.c b/tests/fuzz/fuzz_config_receive.c index 12c9c61..95a01d3 100644 --- a/tests/fuzz/fuzz_config_receive.c +++ b/tests/fuzz/fuzz_config_receive.c @@ -115,7 +115,9 @@ static void build_canonical_frame(void) { if (cfg->usermap) { cfg->usermap_count = 1; cfg->usermap[0].from = MAP_FROM; + cfg->usermap[0].from_hi = MAP_FROM; cfg->usermap[0].to = MAP_TO; + cfg->usermap[0].to_name = NULL; } if (!cfg->send_directory || !cfg->receive_root_directory || !cfg->usermap) { config_delete(cfg); diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index a975fce..f6da785 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -5009,9 +5009,11 @@ class TestIdentityMapping: clean_dir(dest) with open(os.path.join(source, "f.txt"), "wb") as f: f.write(b"mapped") + # #294: --chown cannot be mixed with --usermap/--groupmap on the same + # side, so the maps travel together and --chown is exercised separately. result, _ = run_client( source, dest, - flags=["--preserve", "--usermap=@1000:@1001", "--groupmap=@100:@101", "--chown=@2000:@2001"], + flags=["--preserve", "--usermap=@1000:@1001", "--groupmap=@100:@101"], port=shared_server.port) assert result.returncode == 0, \ f"exit {result.returncode}: {(result.stderr or '')[:200]}" @@ -5019,8 +5021,14 @@ class TestIdentityMapping: with open(os.path.join(received, "f.txt"), "rb") as f: assert f.read() == b"mapped" + result, _ = run_client(source, dest, flags=["--preserve", "--chown=@2000:@2001"], + port=shared_server.port) + assert result.returncode == 0, \ + f"chown exit {result.returncode}: {(result.stderr or '')[:200]}" + @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") - def test_numeric_ids_applies_ownership_as_root(self, shared_server): + def test_numeric_ids_alone_does_not_apply_ownership_as_root(self, shared_server): + # #286.1: --numeric-ids is a mapping modifier, not an ownership request. source = os.path.join(TEST_DATA_DIR, "identity_root_source") dest = os.path.join(TEST_DATA_DIR, "identity_root_dest") clean_dir(source) @@ -5038,8 +5046,8 @@ class TestIdentityMapping: dst_file = os.path.join(received, "f.txt") assert os.path.exists(dst_file) st = os.stat(dst_file) - assert st.st_uid == 12345 and st.st_gid == 12346, \ - f"owner not applied: uid={st.st_uid} gid={st.st_gid}" + assert st.st_uid != 12345, \ + f"--numeric-ids alone must not chown: uid={st.st_uid} gid={st.st_gid}" @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") def test_chown_overrides_ownership_as_root(self, shared_server): @@ -5126,21 +5134,21 @@ class TestSuperPrivilege: f"--super alone must not apply ownership (uid={st.st_uid} gid={st.st_gid})" @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") - def test_super_with_numeric_ids_applies_ownership_as_root(self, shared_server): - """Control: an explicit identity policy is what enables ownership, so - --numeric-ids --super still applies the raw ids as root (the very - ownership --no-super suppresses).""" + def test_super_with_owner_numeric_ids_applies_ownership_as_root(self, shared_server): + """Control: an explicit ownership request is what enables ownership, so + -a --numeric-ids --super applies the raw ids as root (the very ownership + --no-super suppresses). --numeric-ids itself is only the modifier.""" source, dest = self._seed("supernumeric") os.chown(os.path.join(source, "f.txt"), 12345, 12346) result, _ = run_client(source, dest, - flags=["--preserve", "--numeric-ids", "--super"], + flags=["-a", "--numeric-ids", "--super"], port=shared_server.port) assert result.returncode == 0, \ f"exit {result.returncode}: {(result.stderr or '')[:300]}" received = get_dest_received_dir(dest, source) st = os.stat(os.path.join(received, "f.txt")) assert (st.st_uid, st.st_gid) == (12345, 12346), \ - f"--numeric-ids --super should apply raw ids: uid={st.st_uid} gid={st.st_gid}" + f"-a --numeric-ids --super should apply raw ids: uid={st.st_uid} gid={st.st_gid}" @pytest.mark.ci @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") @@ -6063,6 +6071,79 @@ class TestExtendedAttributes: assert len(fields) == 5 assert fields[0] == str(uid), f"reserved uid field {fields[0]} != source uid {uid}" + @pytest.mark.ci + def test_fake_super_records_resolved_chown_without_real_chown(self, shared_server): + """#294: --fake-super must NOT real-chown the recorded owner; it records + the RESOLVED ownership (here a --chown mapping) in the reserved xattr.""" + source, dest = self._source_and_dest("fakesuper_chown") + f = os.path.join(source, "data.txt") + with open(f, "wb") as fh: + fh.write(b"fake-super chown\n") + if not _xattr_supported(f): + pytest.skip("filesystem does not support xattrs") + + result, _ = run_client(source, dest, + flags=["--fake-super", "--chown=@33333:@44444"], + port=shared_server.port) + assert result.returncode == 0, \ + f"--fake-super --chown sync failed: {(result.stderr or result.stdout)[:300]}" + dst = os.path.join(get_dest_received_dir(dest, source), "data.txt") + record = os.getxattr(dst, "user.fastsync.stat").decode().split(":") + assert record[0] == "33333", f"recorded owner {record[0]} != resolved 33333" + assert record[1] == "44444", f"recorded group {record[1]} != resolved 44444" + st = os.stat(dst) + assert st.st_uid != 33333, "--fake-super must not real-chown the recorded owner" + + @pytest.mark.ci + def test_directory_xattrs_preserved(self, shared_server): + """#286.3: -aX must preserve user.* xattrs on DIRECTORIES, not just files.""" + source, dest = self._source_and_dest("dirxattr") + os.makedirs(os.path.join(source, "sub")) + if not _xattr_supported(source): + pytest.skip("filesystem does not support user xattrs") + os.setxattr(source, "user.rootdir", b"r") + os.setxattr(os.path.join(source, "sub"), "user.subdir", b"s") + with open(os.path.join(source, "sub", "f.txt"), "wb") as fh: + fh.write(b"x\n") + + result, _ = run_client(source, dest, flags=["-aX"], port=shared_server.port) + assert result.returncode == 0, \ + f"-aX dir sync failed: {(result.stderr or result.stdout)[:300]}" + received = get_dest_received_dir(dest, source) + assert os.getxattr(received, "user.rootdir") == b"r" + assert os.getxattr(os.path.join(received, "sub"), "user.subdir") == b"s" + + @pytest.mark.ci + def test_directory_default_acl_preserved(self, shared_server): + """#286.3: -aA must preserve a directory's default POSIX ACL (the + system.posix_acl_default xattr), which regular-file ACLs do not cover.""" + source, dest = self._source_and_dest("diracl") + sub = os.path.join(source, "sub") + os.makedirs(sub) + # A child is needed because FastSync deliberately does not materialize + # empty directories; the implicit parent is created by the child write. + with open(os.path.join(sub, "f.txt"), "wb") as fh: + fh.write(b"acl dir\n") + if not _xattr_supported(sub): + pytest.skip("filesystem does not support xattrs") + if shutil.which("setfacl") is None: + pytest.skip("setfacl is not available") + acl = subprocess.run(["setfacl", "-m", "d:u::rwx,d:g::rx,d:o::---", sub], + capture_output=True, text=True) + if acl.returncode != 0: + pytest.skip(f"cannot set a default ACL: {acl.stderr.strip()}") + try: + before = os.getxattr(sub, "system.posix_acl_default") + except OSError as e: + pytest.skip(f"no default ACL xattr: {e}") + + result, _ = run_client(source, dest, flags=["-aA"], port=shared_server.port) + assert result.returncode == 0, \ + f"-aA dir sync failed: {(result.stderr or result.stdout)[:300]}" + received = get_dest_received_dir(dest, source) + assert os.getxattr(os.path.join(received, "sub"), + "system.posix_acl_default") == before + class TestConnectivityClientOptions: """Phase 5 connectivity launch options (--outbuf, --blocking-io). @@ -6488,8 +6569,8 @@ class TestCopyAs: @pytest.mark.ci @pytest.mark.skipif(os.geteuid() != 0, reason="requires a root receiver to chown") def test_root_copy_as_with_fake_super_keeps_target_owner(self, shared_server): - """--fake-super must not let the recorded source owner override the - --copy-as forced owner (copy-as is authoritative).""" + """#294: --fake-super records the RESOLVED copy-as ownership without + real-chowning; the recorded source owner can never override copy-as.""" source = os.path.join(TEST_DATA_DIR, "copyas_fakesuper_src") dest = os.path.join(TEST_DATA_DIR, "copyas_fakesuper_dst") clean_dir(source) @@ -6497,6 +6578,8 @@ class TestCopyAs: src_file = os.path.join(source, "mixed.txt") with open(src_file, "wb") as fh: fh.write(b"copy-as wins over fake-super\n") + if not _xattr_supported(src_file): + pytest.skip("filesystem does not support user xattrs") os.chown(src_file, 12345, 12346) result, _ = run_client(source, dest, @@ -6507,7 +6590,13 @@ class TestCopyAs: f"{(result.stderr or result.stdout)[:400]}" ) received = get_dest_received_dir(dest, source) - st = os.lstat(os.path.join(received, "mixed.txt")) - assert (st.st_uid, st.st_gid) == (65534, 65534), ( - f"--fake-super overrode --copy-as: uid={st.st_uid} gid={st.st_gid}" + dst = os.path.join(received, "mixed.txt") + record = os.getxattr(dst, "user.fastsync.stat").decode().split(":") + assert (record[0], record[1]) == ("65534", "65534"), ( + f"fake-super must record the resolved copy-as ownership: {record[:2]}" + ) + st = os.lstat(dst) + assert (st.st_uid, st.st_gid) != (12345, 12346), ( + f"--fake-super must not real-chown the recorded source owner: " + f"uid={st.st_uid} gid={st.st_gid}" ) diff --git a/tests/integration/test_preserve_attrs.py b/tests/integration/test_preserve_attrs.py index ac045b8..af78532 100644 --- a/tests/integration/test_preserve_attrs.py +++ b/tests/integration/test_preserve_attrs.py @@ -340,16 +340,85 @@ class TestOwnershipRoot: assert (st.st_uid, st.st_gid) == (33333, 44444), \ f"--chown must override -o, got uid={st.st_uid} gid={st.st_gid}" - def test_fake_super_o_does_not_change_group(self, shared_server): - # --fake-super replays the recorded source stat; with only -o requested - # it must apply the owner but leave the group untouched (MAJOR 1). + def test_fake_super_o_does_not_real_chown(self, shared_server): + # #294: --fake-super only RECORDS ownership; it must never real-chown the + # recorded source owner (that defeats the point of the flag). With -o the + # resolved owner is parked in the reserved xattr and the on-disk owner is + # left as the receiver's. source, dest = self._seed_owned("fake_o", 12345, 54321) result, _ = run_client(source, dest, flags=["--fake-super", "-o"], port=shared_server.port) assert result.returncode == 0, f"--fake-super -o failed: {(result.stderr or '')[:300]}" + dst = _received(dest, source, "f.txt") + st = os.stat(dst) + assert st.st_uid != 12345, \ + f"--fake-super -o must NOT real-chown the source owner, got uid={st.st_uid}" + record = os.getxattr(dst, "user.fastsync.stat").decode() + fields = record.split(":") + assert fields[0] == "12345", \ + f"--fake-super must record the resolved owner, got {fields[0]}" + + def test_o_applies_directory_owner(self, shared_server): + """#286.2: -o must apply the source owner to DIRECTORIES too (the + deferred directory-metadata application now runs the identity path).""" + source = os.path.join(TEST_DATA_DIR, "root_dir_o_src") + dest = os.path.join(TEST_DATA_DIR, "root_dir_o_dst") + clean_dir(source) + clean_dir(dest) + os.makedirs(os.path.join(source, "sub", "deep")) + with open(os.path.join(source, "sub", "deep", "f.txt"), "wb") as fh: + fh.write(b"dir owner\n") + os.chown(os.path.join(source, "sub"), 12345, 12346) + os.chown(os.path.join(source, "sub", "deep"), 23456, 34567) + + result, _ = run_client(source, dest, flags=["-o", "-t"], port=shared_server.port) + assert result.returncode == 0, f"-o dir failed: {(result.stderr or '')[:300]}" + received = get_dest_received_dir(dest, source) + sub = os.stat(os.path.join(received, "sub")) + deep = os.stat(os.path.join(received, "sub", "deep")) + assert sub.st_uid == 12345, f"dir 'sub' owner not applied: {sub.st_uid}" + assert deep.st_uid == 23456, f"dir 'sub/deep' owner not applied: {deep.st_uid}" + # -o alone must not change the group. + assert sub.st_gid != 12346 + + def test_a_applies_directory_owner_and_group(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "root_dir_a_src") + dest = os.path.join(TEST_DATA_DIR, "root_dir_a_dst") + clean_dir(source) + clean_dir(dest) + os.makedirs(os.path.join(source, "sub")) + with open(os.path.join(source, "sub", "f.txt"), "wb") as fh: + fh.write(b"dir owner group\n") + os.chown(os.path.join(source, "sub"), 12345, 54321) + + result, _ = run_client(source, dest, flags=["-a"], port=shared_server.port) + assert result.returncode == 0, f"-a dir failed: {(result.stderr or '')[:300]}" + received = get_dest_received_dir(dest, source) + st = os.stat(os.path.join(received, "sub")) + assert (st.st_uid, st.st_gid) == (12345, 54321), \ + f"-a must apply dir owner+group, got uid={st.st_uid} gid={st.st_gid}" + + def test_numeric_ids_alone_does_not_chown(self, shared_server): + """#286.1: --numeric-ids is a mapping modifier, not an ownership request. + `-t --numeric-ids` must leave the receiver's ownership untouched.""" + source, dest = self._seed_owned("num_only", 12345, 54321) + result, _ = run_client(source, dest, flags=["-t", "--numeric-ids"], + port=shared_server.port) + assert result.returncode == 0, \ + f"-t --numeric-ids failed: {(result.stderr or '')[:300]}" st = os.stat(_received(dest, source, "f.txt")) - assert st.st_uid == 12345, f"--fake-super -o must apply the owner, got uid={st.st_uid}" - assert st.st_gid != 54321, "--fake-super -o must not change the group" + assert st.st_uid != 12345, \ + f"--numeric-ids alone must not chown, got uid={st.st_uid}" + + def test_numeric_ids_with_o_uses_raw_id(self, shared_server): + source, dest = self._seed_owned("num_o", 12345, 54321) + result, _ = run_client(source, dest, flags=["-o", "-t", "--numeric-ids"], + port=shared_server.port) + assert result.returncode == 0, \ + f"-o --numeric-ids failed: {(result.stderr or '')[:300]}" + st = os.stat(_received(dest, source, "f.txt")) + assert st.st_uid == 12345, \ + f"-o --numeric-ids must apply the raw id, got uid={st.st_uid}" class TestPreserveFeatureMatrix: diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 824a43d..eca5e40 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -2864,6 +2864,92 @@ static void test_parse_args_usermap_name_resolution() { config_delete(cfg); } +/* #294: rsync map FROM forms -- inclusive numeric ranges, '*' (any), and the + * empty token (ids with no name on the sender). A TO name is transmitted as a + * NAME for the receiver to resolve (rsync resolves TO names on the receiving + * side), not resolved against the client's database. */ +static void test_parse_args_usermap_rsync_forms() { + int positional_args[2]; + + Config* cfg = config_create(); + int positional_count = 0; + char* argv[] = {"fastsync", "--usermap=0-99:nobody", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->usermap_count, 1); + EXPECT_EQ_INT(cfg->usermap[0].from, 0); + EXPECT_EQ_INT(cfg->usermap[0].from_hi, 99); + EXPECT_TRUE(cfg->preserve_owner); + config_delete(cfg); + + /* Empty FROM => IDENTITY_MATCH_UNNAMED. */ + cfg = config_create(); + positional_count = 0; + char* argv2[] = {"fastsync", "--usermap=:@0", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 4, argv2, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->usermap_count, 1); + EXPECT_EQ_INT(cfg->usermap[0].from, IDENTITY_MATCH_UNNAMED); + EXPECT_EQ_INT(cfg->usermap[0].from_hi, IDENTITY_MATCH_UNNAMED); + config_delete(cfg); + + /* '*' FROM => IDENTITY_MATCH_ANY. */ + cfg = config_create(); + positional_count = 0; + char* argv3[] = {"fastsync", "--groupmap=*:@0", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 4, argv3, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->groupmap[0].from, IDENTITY_MATCH_ANY); + EXPECT_EQ_INT(cfg->groupmap[0].from_hi, IDENTITY_MATCH_ANY); + config_delete(cfg); + + /* A TO name is kept as a receiver-resolved name, NOT resolved locally. */ + cfg = config_create(); + positional_count = 0; + char* argv4[] = {"fastsync", "--usermap=0:nobody", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 4, argv4, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->usermap_count, 1); + EXPECT_NOT_NULL(cfg->usermap[0].to_name); + if (cfg->usermap[0].to_name) + EXPECT_EQ_STR(cfg->usermap[0].to_name, "nobody"); + config_delete(cfg); +} + +/* #294: rsync refuses to mix --chown with --usermap/--groupmap on the same + * side (either order). --chown=USER conflicts with a prior --usermap; + * --chown=:GROUP conflicts with a prior --groupmap; the opposite side is fine. */ +static void test_parse_args_identity_map_chown_conflict() { + int positional_args[2]; + struct { + const char* a; + const char* b; + } bad[] = { + {"--usermap=0:1", "--chown=2:3"}, {"--chown=2:3", "--usermap=0:1"}, + {"--chown=2", "--usermap=0:1"}, {"--chown=2:3", "--groupmap=0:1"}, + {"--groupmap=0:1", "--chown=2:3"}, {"--chown=:3", "--groupmap=0:1"}, + }; + for (size_t i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) { + Config* cfg = config_create(); + char* argv[] = {"fastsync", (char*)bad[i].a, (char*)bad[i].b, "/src", "/dst"}; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), -1); + config_delete(cfg); + } + + /* The opposite-side combinations rsync allows must still parse. */ + struct { + const char* a; + const char* b; + } ok[] = { + {"--chown=2", "--groupmap=0:1"}, + {"--chown=:3", "--usermap=0:1"}, + }; + for (size_t i = 0; i < sizeof(ok) / sizeof(ok[0]); i++) { + Config* cfg = config_create(); + char* argv[] = {"fastsync", (char*)ok[i].a, (char*)ok[i].b, "/src", "/dst"}; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0); + config_delete(cfg); + } +} + /* --chown parses USER:GROUP / USER / :GROUP, numeric ids, and '*'. */ static void test_parse_args_chown() { Config* cfg = config_create(); @@ -2981,8 +3067,10 @@ static void test_parse_args_rejects_malformed_identity() { const char* val; } bad[] = { {"--usermap", "@1000"}, - {"--usermap", ":1000"}, {"--usermap", "definitely_not_a_real_user_zzz:@1"}, + {"--usermap", "0-"}, + {"--usermap", "5-2:@1"}, + {"--usermap", "roo*:@1"}, {"--groupmap", "@1"}, {"--groupmap", "no_such_group_qqq:x"}, {"--chown", "a:b:c"}, @@ -3962,6 +4050,8 @@ void test_client_cli() { test_parse_args_usermap(); test_parse_args_groupmap(); test_parse_args_usermap_name_resolution(); + test_parse_args_usermap_rsync_forms(); + test_parse_args_identity_map_chown_conflict(); test_parse_args_chown(); test_parse_args_copy_as(); test_parse_args_rejects_malformed_identity(); diff --git a/tests/test_config.c b/tests/test_config.c index 68c35d4..e56e55f 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1345,13 +1345,19 @@ static void test_config_identity_wire_roundtrip() { send_cfg->usermap_count = 2; send_cfg->usermap = calloc(2, sizeof(IdentityMap)); send_cfg->usermap[0].from = IDENTITY_MATCH_ANY; + send_cfg->usermap[0].from_hi = IDENTITY_MATCH_ANY; send_cfg->usermap[0].to = 65534; + send_cfg->usermap[0].to_name = NULL; send_cfg->usermap[1].from = 1000; + send_cfg->usermap[1].from_hi = 1000; send_cfg->usermap[1].to = 1000; + send_cfg->usermap[1].to_name = NULL; send_cfg->groupmap_count = 1; send_cfg->groupmap = calloc(1, sizeof(IdentityMap)); send_cfg->groupmap[0].from = 0; + send_cfg->groupmap[0].from_hi = 0; send_cfg->groupmap[0].to = IDENTITY_CURRENT; + send_cfg->groupmap[0].to_name = str_dup("root"); int p[2]; EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); @@ -1367,9 +1373,12 @@ static void test_config_identity_wire_roundtrip() { ok = recv->numeric_ids && recv->chown_uid_set && recv->chown_uid == 1001 && recv->chown_gid_set && recv->chown_gid == IDENTITY_CURRENT && recv->usermap_count == 2 && recv->groupmap_count == 1 && recv->usermap[0].from == IDENTITY_MATCH_ANY && - recv->usermap[0].to == 65534 && recv->usermap[1].from == 1000 && - recv->usermap[1].to == 1000 && recv->groupmap[0].from == 0 && - recv->groupmap[0].to == IDENTITY_CURRENT; + recv->usermap[0].from_hi == IDENTITY_MATCH_ANY && recv->usermap[0].to == 65534 && + recv->usermap[0].to_name == NULL && recv->usermap[1].from == 1000 && + recv->usermap[1].from_hi == 1000 && recv->usermap[1].to == 1000 && + recv->groupmap[0].from == 0 && recv->groupmap[0].from_hi == 0 && + recv->groupmap[0].to == IDENTITY_CURRENT && recv->groupmap[0].to_name != NULL && + strcmp(recv->groupmap[0].to_name, "root") == 0; } config_delete(recv); close(p[0]); @@ -1399,7 +1408,8 @@ static void test_config_receive_rejects_invalid_identity() { c->receive_root_directory = str_dup("/dst"); c->usermap_count = 1; c->usermap = calloc(1, sizeof(IdentityMap)); - c->usermap[0].from = -2; /* below IDENTITY_MATCH_ANY */ + c->usermap[0].from = -3; /* below IDENTITY_MATCH_UNNAMED */ + c->usermap[0].from_hi = -3; c->usermap[0].to = 0; EXPECT_FALSE(roundtrip_config_ok(c)); config_delete(c); @@ -2101,8 +2111,10 @@ static void test_identity_explicit_ownership_requested() { } /* P7 Wave E hardening (A3): --super no longer implies raw numeric-id - preservation, so it must never enable ownership application on its own; an - explicit identity flag is required. */ + preservation, so it must never enable ownership application on its own. + #286: --numeric-ids is a mapping MODIFIER only and is likewise inert on its + own; a real ownership request (-o/-g or an explicit identity flag) is + required to activate chown. */ static void test_super_does_not_imply_numeric() { Config* c = config_create(); EXPECT_NOT_NULL(c); @@ -2112,6 +2124,9 @@ static void test_super_does_not_imply_numeric() { EXPECT_FALSE(identity_active_enabled()); c->numeric_ids = true; EXPECT_TRUE(identity_set_active(c)); + EXPECT_FALSE(identity_active_enabled()); /* mapping modifier only */ + c->preserve_owner = true; + EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(identity_active_enabled()); identity_clear_active(); config_delete(c); @@ -2635,13 +2650,19 @@ static void golden_config_populate(Config* c) { c->usermap_count = 2; c->usermap = calloc(2, sizeof(IdentityMap)); c->usermap[0].from = IDENTITY_MATCH_ANY; + c->usermap[0].from_hi = IDENTITY_MATCH_ANY; c->usermap[0].to = 1000; + c->usermap[0].to_name = NULL; c->usermap[1].from = 5; + c->usermap[1].from_hi = 9; c->usermap[1].to = 6; + c->usermap[1].to_name = NULL; c->groupmap_count = 1; c->groupmap = calloc(1, sizeof(IdentityMap)); c->groupmap[0].from = 7; + c->groupmap[0].from_hi = 7; c->groupmap[0].to = 8; + c->groupmap[0].to_name = str_dup("root"); c->preserve_atimes = true; c->preserve_crtimes = false; c->omit_dir_times = true; @@ -2668,11 +2689,11 @@ static void golden_config_populate(Config* c) { /* The pinned golden frame (protocol 2.23.0). The values below are the only * thing that ties the generated table to the historical wire format; update * them ONLY with a PROTOCOL_VERSION bump and a documented reason. The 2.23.0 - * delete-semantics wave keeps the config-frame LAYOUT unchanged, but the - * embedded version string moves to "2.23.0", so the byte-exact hash changes - * while the length stays 653. */ -#define GOLDEN_WIRE_LEN 653 -#define GOLDEN_WIRE_HASH 3267254725292157519ULL + * rsync-parity wave changes the config-frame layout (map-entry range + TO name, + * plus other wire changes landing in this version); the byte-exact hash is + * recomputed for the merged layout. */ +#define GOLDEN_WIRE_LEN 693 +#define GOLDEN_WIRE_HASH 6341115972171444885ULL static unsigned long long fnv1a_64(const unsigned char* buf, size_t len) { unsigned long long h = 1469598103934665603ULL; @@ -2815,7 +2836,10 @@ static void test_config_wire_golden_receive() { ok = ok && recv->super_mode == SUPER_MODE_ON; ok = ok && recv->chown_uid == 1234 && recv->chown_gid == 5678; ok = ok && recv->usermap_count == 2 && recv->usermap[0].from == IDENTITY_MATCH_ANY && - recv->usermap[0].to == 1000 && recv->usermap[1].from == 5 && recv->usermap[1].to == 6; + recv->usermap[0].from_hi == IDENTITY_MATCH_ANY && recv->usermap[0].to == 1000 && + recv->usermap[1].from == 5 && recv->usermap[1].from_hi == 9 && recv->usermap[1].to == 6; + ok = ok && recv->groupmap_count == 1 && recv->groupmap[0].from == 7 && + recv->groupmap[0].to_name != NULL && strcmp(recv->groupmap[0].to_name, "root") == 0; ok = ok && recv->basis_count == 2 && recv->basis_dirs[0].type == BASIS_DEST_COMPARE && recv->basis_dirs[1].type == BASIS_DEST_LINK; ok = ok && recv->module != NULL && strcmp(recv->module, "goldenmod") == 0; diff --git a/tests/test_file.c b/tests/test_file.c index ae101e1..0a0e41a 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -1659,9 +1659,19 @@ static void test_dir_time_list() { dir_time_list_init(&list); EXPECT_EQ_INT((int)list.count, 0); FileMetadata metadata = {.mtime_sec = 1000000000, .mtime_nsec = 0}; - EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata)); - EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata)); + EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata, NULL)); + /* A captured xattr block is deep-copied into the list. */ + FileXattrList* xl = xattr_list_new(); + EXPECT_NOT_NULL(xl); + EXPECT_TRUE(xattr_list_append(xl, "user.dir", "v", 1)); + EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata, xl)); + xattr_list_free(xl); /* the list owns its own copy now */ EXPECT_EQ_INT((int)list.count, 2); + EXPECT_NOT_NULL(list.xattrs); + EXPECT_NOT_NULL(list.xattrs[1]); + EXPECT_EQ_INT(list.xattrs[1]->count, 1); + EXPECT_EQ_STR(list.xattrs[1]->items[0].name, "user.dir"); + EXPECT_NULL(list.xattrs[0]); Config* cfg = config_create(); EXPECT_NOT_NULL(cfg); @@ -1677,6 +1687,7 @@ static void test_dir_time_list() { EXPECT_EQ_INT((int)list.count, 0); EXPECT_NULL(list.paths); EXPECT_NULL(list.entries); + EXPECT_NULL(list.xattrs); rmdir(sub); rmdir(root); @@ -1701,14 +1712,14 @@ static void test_dir_time_list_cap() { for (size_t i = 0; i < MAX_DIR_TIME_ENTRIES + 1 && !rejected; i++) { size_t before_count = list.count; size_t before_bytes = list.bytes; - if (!dir_time_list_add(&list, path, &metadata)) { + if (!dir_time_list_add(&list, path, &metadata, NULL)) { rejected = true; /* The rejected add must not have partially mutated the list. */ EXPECT_TRUE(list.count == before_count); EXPECT_TRUE(list.bytes == before_bytes); } else { EXPECT_TRUE(list.count == before_count + 1); - EXPECT_TRUE(list.bytes == before_bytes + path_len + sizeof(FileMetadata) + sizeof(char*)); + EXPECT_TRUE(list.bytes == before_bytes + path_len + sizeof(FileMetadata) + 2 * sizeof(char*)); } } EXPECT_TRUE(rejected); diff --git a/tests/test_fuzz_smoke.c b/tests/test_fuzz_smoke.c index 22efd50..2c6214c 100644 --- a/tests/test_fuzz_smoke.c +++ b/tests/test_fuzz_smoke.c @@ -379,7 +379,9 @@ static void test_fuzz_config_receive_huge_map_count() { } c->usermap_count = 1; c->usermap[0].from = sentinel_from; + c->usermap[0].from_hi = sentinel_from; c->usermap[0].to = sentinel_to; + c->usermap[0].to_name = NULL; unsigned char* frame = NULL; size_t len = 0; @@ -390,9 +392,12 @@ static void test_fuzz_config_receive_huge_map_count() { return; } - unsigned char pattern[8]; + /* One wire entry is [from][from_hi][to][to_name]; search the fixed-width + prefix (the to_name length-prefixed string follows). */ + unsigned char pattern[12]; memcpy(pattern, &sentinel_from, sizeof(sentinel_from)); - memcpy(pattern + sizeof(sentinel_from), &sentinel_to, sizeof(sentinel_to)); + memcpy(pattern + sizeof(sentinel_from), &sentinel_from, sizeof(sentinel_from)); + memcpy(pattern + 2 * sizeof(sentinel_from), &sentinel_to, sizeof(sentinel_to)); size_t entry_off = find_bytes(frame, len, pattern, sizeof(pattern)); if (entry_off == SIZE_MAX || entry_off < sizeof(int32_t)) { free(frame); diff --git a/tests/test_xattr.c b/tests/test_xattr.c index d3675b2..29666d6 100644 --- a/tests/test_xattr.c +++ b/tests/test_xattr.c @@ -400,14 +400,12 @@ static void test_fake_super_restore() { unlink(path); } -/* --fake-super owner replay must honor the super gate and copy-as authority: - --no-super suppresses the recorded-source-owner chown even for root, and an - active --copy-as keeps its forced owner (the recorded source owner must never - override it). Root-gated: only root can observe a chown actually landing. */ -static void test_fake_super_owner_gate() { - if (geteuid() != 0) - return; /* non-root cannot observe ownership changes; skip silently */ - const char* path = "test_fake_super_owner_gate.txt"; +/* --fake-super must NEVER perform a real chown: fake_super_restore_fd applies + * only mode/mtime and leaves the entry's uid/gid exactly as they were, even + * when an explicit ownership policy is active and super_mode permits it. This + * is observable unprivileged (the file's owner is simply unchanged). */ +static void test_fake_super_no_real_chown() { + const char* path = "test_fake_super_nochown.txt"; unlink(path); int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0600); if (fd < 0) @@ -416,63 +414,34 @@ static void test_fake_super_owner_gate() { if (has_xattr) removexattr(path, "user.fastsync.xprobe"); if (!has_xattr) { - close(fd); - unlink(path); - return; /* filesystem without xattr support */ - } - if (fchown(fd, 0, 0) != 0) { close(fd); unlink(path); return; } + struct stat before; + EXPECT_EQ_INT(fstat(fd, &before), 0); fake_super_store_fd(fd, 12345, 12346, 0755, 1700000000, 0); Config* c = config_create(); FileAttrPolicy policy = {true, true, false, false}; EXPECT_NOT_NULL(c); - - /* An explicit ownership policy is required before fake-super replay may - chown; --fake-super alone only records the source owner (A2). */ - c->numeric_ids = true; - - /* --no-super: the owner leg is skipped even as root. */ - c->super_mode = SUPER_MODE_OFF; - EXPECT_TRUE(identity_set_active(c)); - EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - struct stat st; - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 0); - EXPECT_EQ_INT((int)st.st_gid, 0); - - /* AUTO with an identity policy: the recorded source owner is applied. */ - c->super_mode = SUPER_MODE_AUTO; - EXPECT_TRUE(identity_set_active(c)); - EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 12345); - EXPECT_EQ_INT((int)st.st_gid, 12346); - - /* --super / --fake-super with NO explicit identity flag must NOT apply a - client-chosen owner: super_mode alone never enables ownership. */ - EXPECT_EQ_INT(fchown(fd, 0, 0), 0); - c->numeric_ids = false; + /* The strongest ownership request available plus permitted super mode. */ + c->preserve_owner = true; + c->preserve_group = true; + c->chown_uid_set = true; + c->chown_uid = 12345; + c->chown_gid_set = true; + c->chown_gid = 12346; c->super_mode = SUPER_MODE_ON; + c->fake_super = true; EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 0); - EXPECT_EQ_INT((int)st.st_gid, 0); - - /* Active --copy-as is authoritative: the recorded source owner must not - override it, even with AUTO/ON. */ - c->copy_as_set = true; - c->copy_as_uid = 777; - c->copy_as_gid = 778; - EXPECT_TRUE(identity_set_active(c)); - EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 0); - EXPECT_EQ_INT((int)st.st_gid, 0); + struct stat after; + EXPECT_EQ_INT(fstat(fd, &after), 0); + EXPECT_EQ_INT((int)after.st_uid, (int)before.st_uid); + EXPECT_EQ_INT((int)after.st_gid, (int)before.st_gid); + /* Mode is still replayed (policy-gated). */ + EXPECT_EQ_INT((int)(after.st_mode & 0777), 0755); identity_clear_active(); config_delete(c); @@ -480,64 +449,99 @@ static void test_fake_super_owner_gate() { unlink(path); } -/* MAJOR 1: the --fake-super owner replay must honor the per-side -o/-g split. - * With only -o (preserve_owner) requested the recorded GROUP must be left - * untouched, and with only -g (preserve_group) the recorded OWNER must be left - * untouched. Root-gated: only root can observe a chown actually landing. */ -static void test_fake_super_owner_group_split() { - if (geteuid() != 0) - return; /* non-root cannot observe ownership changes; skip silently */ - const char* path = "test_fake_super_owner_group_split.txt"; - unlink(path); - int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0600); - if (fd < 0) - return; - bool has_xattr = setxattr(path, "user.fastsync.xprobe", "p", 1, 0) == 0; - if (has_xattr) - removexattr(path, "user.fastsync.xprobe"); - if (!has_xattr) { - close(fd); - unlink(path); - return; /* filesystem without xattr support */ - } - if (fchown(fd, 0, 0) != 0) { - close(fd); - unlink(path); - return; - } - fake_super_store_fd(fd, 12345, 12346, 0755, 1700000000, 0); +/* identity_resolve_storage_ids() is what --fake-super RECORDS: the resolved + * mapping for a requested side, and the source's own id for a side never + * requested. Also pins the #286 rule that --numeric-ids alone never activates + * ownership (it is only a mapping modifier). */ +static void test_fake_super_storage_resolution() { + uint32_t uid = 0, gid = 0; + /* --numeric-ids alone is INERT: no ownership request, storage unchanged. */ Config* c = config_create(); - FileAttrPolicy policy = {true, true, false, false}; EXPECT_NOT_NULL(c); - struct stat st; + c->numeric_ids = true; + EXPECT_TRUE(identity_set_active(c)); + EXPECT_FALSE(identity_active_enabled()); + EXPECT_FALSE(identity_owner_requested()); + EXPECT_FALSE(identity_group_requested()); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 12345); + EXPECT_EQ_INT((int)gid, 6789); + config_delete(c); - /* -o only: the owner is applied, the group stays at its current value (0). */ + /* --fake-super with no ownership request records the raw source ids. */ + c = config_create(); + EXPECT_NOT_NULL(c); + c->fake_super = true; + EXPECT_TRUE(identity_set_active(c)); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 12345); + EXPECT_EQ_INT((int)gid, 6789); + + /* -o + --numeric-ids: raw owner, un-requested group stays the source gid. */ c->preserve_owner = true; - c->preserve_group = false; + c->numeric_ids = true; EXPECT_TRUE(identity_set_active(c)); - EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 12345); - EXPECT_EQ_INT((int)st.st_gid, 0); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 12345); + EXPECT_EQ_INT((int)gid, 6789); - /* -g only: the group is applied, the owner stays at its current value (0). */ - EXPECT_EQ_INT(fchown(fd, 0, 0), 0); - c->preserve_owner = false; - c->preserve_group = true; + /* --chown overrides both sides. */ + c->chown_uid_set = true; + c->chown_uid = 777; + c->chown_gid_set = true; + c->chown_gid = 778; EXPECT_TRUE(identity_set_active(c)); - EXPECT_TRUE(fake_super_restore_fd(fd, policy)); - EXPECT_EQ_INT(fstat(fd, &st), 0); - EXPECT_EQ_INT((int)st.st_uid, 0); - EXPECT_EQ_INT((int)st.st_gid, 12346); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 777); + EXPECT_EQ_INT((int)gid, 778); + + /* A usermap match beats --chown on the owner side only. */ + c->usermap_count = 1; + c->usermap = calloc(1, sizeof(IdentityMap)); + EXPECT_NOT_NULL(c->usermap); + c->usermap[0].from = IDENTITY_MATCH_ANY; + c->usermap[0].to = 999; + EXPECT_TRUE(identity_set_active(c)); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 999); + EXPECT_EQ_INT((int)gid, 778); + + /* --copy-as is authoritative for both sides. */ + c->copy_as_set = true; + c->copy_as_uid = 111; + c->copy_as_gid = 222; + EXPECT_TRUE(identity_set_active(c)); + identity_resolve_storage_ids(12345, 6789, &uid, &gid); + EXPECT_EQ_INT((int)uid, 111); + EXPECT_EQ_INT((int)gid, 222); identity_clear_active(); config_delete(c); - close(fd); - unlink(path); +} + +/* xattr_list_clone deep-copies names/values (used by the deferred directory + * metadata accumulator), so the clone stays valid after the original is freed. */ +static void test_xattr_list_clone() { + EXPECT_NULL(xattr_list_clone(NULL)); + FileXattrList* list = xattr_list_new(); + EXPECT_NOT_NULL(list); + EXPECT_TRUE(xattr_list_append(list, "user.a", "1", 1)); + EXPECT_TRUE(xattr_list_append(list, "user.b", "22", 2)); + FileXattrList* clone = xattr_list_clone(list); + EXPECT_NOT_NULL(clone); + EXPECT_EQ_INT(clone->count, 2); + EXPECT_EQ_STR(clone->items[0].name, "user.a"); + EXPECT_EQ_INT((int)clone->items[1].value_len, 2); + EXPECT_TRUE(memcmp(clone->items[1].value, "22", 2) == 0); + EXPECT_TRUE(clone->items[0].name != list->items[0].name); + xattr_list_free(list); + EXPECT_EQ_STR(clone->items[0].name, "user.a"); + xattr_list_free(clone); } void test_xattr() { + test_xattr_list_clone(); test_xattr_wire_roundtrip(); test_xattr_reject_privileged_namespace(); test_xattr_reject_oversized_value(); @@ -547,6 +551,6 @@ void test_xattr() { test_xattr_receive_drops_acl_without_preserve_acls(); test_link_copy_fallback_preserves_xattrs(); test_fake_super_restore(); - test_fake_super_owner_gate(); - test_fake_super_owner_group_split(); + test_fake_super_no_real_chown(); + test_fake_super_storage_resolution(); } \ No newline at end of file