From 47b1b9b915273ba6e66ab79905532422b85e774f Mon Sep 17 00:00:00 2001 From: TapTap Date: Tue, 22 Sep 2026 23:05:40 +0200 Subject: [PATCH] fix(xattr): fake-super device round-trip, --devices continue-on-error, harden stat parse --- src/server/receiver.c | 36 ++++++- src/server/receiver_pipeline.c | 8 ++ src/server/receiver_pipeline.h | 5 + src/server/server.c | 8 +- src/shared/file.c | 48 +++++---- src/shared/file.h | 13 ++- src/shared/file_save.c | 33 +++--- src/shared/file_save.h | 12 ++- src/shared/xattr.c | 53 +++++++++- tests/integration/test_temp_dir_absolute.py | 18 ++++ tests/test_file.c | 105 +++++++++++++++++++- tests/test_xattr.c | 18 ++++ 12 files changed, 310 insertions(+), 47 deletions(-) diff --git a/src/server/receiver.c b/src/server/receiver.c index 50f4e0b..765501d 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -53,7 +53,13 @@ bool receiver_send_final_success(int fd, const Config* config, const ReceiverOut return send_status(fd, final_status); size_t count = outcomes ? outcomes->count : 0; for (size_t i = 0; i < count; i++) { - Status per_file = outcomes->entries[i] == FILE_SAVE_WRITTEN ? STATUS_NEXT : STATUS_OK; + Status per_file; + if (outcomes->entries[i] == FILE_SAVE_WRITTEN) + per_file = STATUS_NEXT; + else if (outcomes->entries[i] == FILE_SAVE_FAILED) + per_file = STATUS_ERROR; + else + per_file = STATUS_OK; if (!send_status(fd, per_file)) return false; } @@ -719,6 +725,11 @@ typedef struct { ArrayList* would_delete; /* --info=del: actually-removed paths collected during the delete commit. */ ArrayList* deleted_paths; + /* Per-run count of entries that failed to materialize without aborting the + stream (currently ONLY a --devices mknod EPERM/EACCES). A nonzero count + makes the terminal frame carry a non-OK status so the client exits + non-zero, matching rsync's continue-and-exit-partial behavior. */ + size_t failed_entries; } ReceiverSaveContext; static bool receiver_save_file(File* file, void* context_pointer) { @@ -742,6 +753,11 @@ static bool receiver_save_file(File* file, void* context_pointer) { count as matched data in the end-of-transfer report. */ if (result != FILE_SAVE_ERROR && file->matched_bytes > 0) context->stats.matched_data += file->matched_bytes; + /* --devices parity: a device node the receiver could not mknod (EPERM/EACCES) + is counted per-run but does not abort the transfer. The terminal frame + turns a nonzero count into a non-OK status so the client exits non-zero. */ + if (result == FILE_SAVE_FAILED) + context->failed_entries++; /* Protocol 2.28.0: receiver-observed literal bytes and the created-entry breakdown (regular/dir/link/special) for the `--stats` report. */ if (result == FILE_SAVE_WRITTEN) @@ -775,9 +791,25 @@ static void receiver_note_delete_limit(void* context_pointer) { context->delete_limit_reached = true; } +/* Terminal status for a run. A capped --delete limit wins (rsync exit 25); + otherwise any per-entry failure (for example an unprivileged --devices + mknod) makes the terminal frame non-OK so the client exits non-zero. rsync + reports 23 here; mapping the client's exact exit code to 23 is a separate, + pre-existing concern. A clean run keeps STATUS_OK. */ +static Status receiver_final_status(bool delete_limit_reached, size_t failed_entries) { + if (delete_limit_reached) + return STATUS_DELETE_LIMIT; + return failed_entries > 0 ? STATUS_ERROR : STATUS_OK; +} + static bool receiver_send_success_frame(int fd, void* context_pointer) { ReceiverSaveContext* context = context_pointer; - Status final_status = context->delete_limit_reached ? STATUS_DELETE_LIMIT : STATUS_OK; + if (context->failed_entries > 0) + log_message(LOG_LEVEL_WARNING, + "%zu entr%s failed to materialize; continuing (partial transfer)", + context->failed_entries, context->failed_entries == 1 ? "y" : "ies"); + Status final_status = + receiver_final_status(context->delete_limit_reached, context->failed_entries); if (!receiver_send_stats_frame(fd, context->config, &context->stats, context->would_delete, context->deleted_paths)) return false; diff --git a/src/server/receiver_pipeline.c b/src/server/receiver_pipeline.c index 5d1066e..6feab4e 100644 --- a/src/server/receiver_pipeline.c +++ b/src/server/receiver_pipeline.c @@ -29,6 +29,7 @@ PipelineContextReceiver* pipeline_context_receiver_create(Config* config, Queue* context->deferred_manifest = NULL; context->deferred_plans = NULL; context->delete_limit_reached = false; + context->failed_entries = 0; memset(&context->stats, 0, sizeof(context->stats)); context->would_delete = NULL; context->deleted_paths = NULL; @@ -264,6 +265,13 @@ int write_thread(void* pipeline_context) { receiver_stats_note_saved(&context->stats, file, created, created_dirs); mtx_unlock(&context->mutex); } + /* --devices parity: a device node that could not be mknod'ed is counted + per-run but does NOT abort the transfer. */ + if (result == FILE_SAVE_FAILED) { + mtx_lock(&context->mutex); + context->failed_entries++; + mtx_unlock(&context->mutex); + } if (result == FILE_SAVE_ERROR) { file_destroy(file); pipeline_context_receiver_note_bytes_released(context, file_bytes); diff --git a/src/server/receiver_pipeline.h b/src/server/receiver_pipeline.h index 099bd9c..33e9f73 100644 --- a/src/server/receiver_pipeline.h +++ b/src/server/receiver_pipeline.h @@ -64,6 +64,11 @@ typedef struct PipelineContextReceiver { /* --info=del actually-removed path list, collected by the deferred delete commit in server.c and reported in the STATUS_STATS frame. */ struct ArrayList* deleted_paths; + /* Per-run count of entries that failed to materialize without aborting the + stream (currently ONLY a --devices mknod EPERM/EACCES). write_thread + increments it under `mutex`; server.c turns a nonzero count into a non-OK + terminal status so the client exits non-zero. */ + size_t failed_entries; } PipelineContextReceiver; PipelineContextReceiver* pipeline_context_receiver_create(Config* config, Queue* queue_receiver, diff --git a/src/server/server.c b/src/server/server.c index 5978d71..f84a616 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -1044,7 +1044,13 @@ static void server_run_mt_receiver(ServerSession* state) { dir_metadata_list_apply(&context->dir_times, config->receive_root_directory, config); } if (transfer_ok) { - Status final_status = context->delete_limit_reached ? STATUS_DELETE_LIMIT : STATUS_OK; + if (context->failed_entries > 0) + log_message(LOG_LEVEL_WARNING, + "%zu entr%s failed to materialize; continuing (partial transfer)", + context->failed_entries, context->failed_entries == 1 ? "y" : "ies"); + Status final_status = context->delete_limit_reached + ? STATUS_DELETE_LIMIT + : (context->failed_entries > 0 ? STATUS_ERROR : STATUS_OK); /* Emit the optional wire-stats record first (protocol 2.25.0), then the success/outcome frame, exactly like the single-threaded receiver. */ if (!receiver_send_stats_frame(state->fd, config, &context->stats, context->would_delete, diff --git a/src/shared/file.c b/src/shared/file.c index 2ce5b20..de06c09 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -1182,7 +1182,8 @@ int file_open_temp_dir(const char* dir_path) { * destination file) and best-effort: a per-attribute or privilege failure is * logged and skipped, never fatal. */ static void restore_extra_fd(int fd, const FileMetadata* metadata, const FileXattrList* xattrs, - bool fake_super, FileAttrPolicy policy) { + bool fake_super, FileAttrPolicy policy, uint32_t fake_super_rdev_major, + uint32_t fake_super_rdev_minor) { xattr_apply_fd(fd, xattrs); if (fake_super && metadata) { /* Record the ownership that WOULD have been applied: when an explicit @@ -1197,7 +1198,8 @@ static void restore_extra_fd(int fd, const FileMetadata* metadata, const FileXat 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, 0, 0); + fake_super_store_fd(fd, store_uid, store_gid, (uint32_t)metadata->mode, fake_super_rdev_major, + fake_super_rdev_minor); fake_super_restore_fd(fd, policy); } } @@ -1207,7 +1209,8 @@ file_to_disk_secure_impl(const char* path, const void* data, unsigned long long bool inplace, bool sparse, bool preallocate, const FileMetadata* metadata, FileAttrPolicy policy, bool update, bool no_replace, bool use_fsync, const char* temp_dir, const FileXattrList* xattrs, bool fake_super, - bool keep_partial, unsigned* dirs_created, const char* count_floor) { + bool keep_partial, unsigned* dirs_created, const char* count_floor, + uint32_t fake_super_rdev_major, uint32_t fake_super_rdev_minor) { char* leaf = NULL; int dirfd = file_open_secure_parent_counted(path, &leaf, true, dirs_created, count_floor); if (dirfd < 0) @@ -1314,7 +1317,8 @@ file_to_disk_secure_impl(const char* path, const void* data, unsigned long long } } if (ok) - restore_extra_fd(fd, metadata, xattrs, fake_super, policy); + restore_extra_fd(fd, metadata, xattrs, fake_super, policy, fake_super_rdev_major, + fake_super_rdev_minor); if (ok && use_fsync) ok = fsync(fd) == 0; } @@ -1432,7 +1436,8 @@ file_to_disk_secure_impl(const char* path, const void* data, unsigned long long } } if (ok) - restore_extra_fd(fd, metadata, xattrs, fake_super, policy); + restore_extra_fd(fd, metadata, xattrs, fake_super, policy, fake_super_rdev_major, + fake_super_rdev_minor); if (ok && use_fsync) ok = fsync(fd) == 0; } @@ -1500,7 +1505,8 @@ file_to_disk_secure_impl(const char* path, const void* data, unsigned long long "non-atomic copy into the destination directory"); return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, preallocate, metadata, policy, update, no_replace, use_fsync, NULL, xattrs, fake_super, - keep_partial, dirs_created, count_floor); + keep_partial, dirs_created, count_floor, fake_super_rdev_major, + fake_super_rdev_minor); } return ok; } @@ -1510,7 +1516,7 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long FileAttrPolicy policy, const char* temp_dir) { return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, preallocate, metadata, policy, false, false, false, temp_dir, NULL, false, false, NULL, - NULL); + NULL, 0, 0); } bool file_to_disk_secure_update(const char* path, const void* data, unsigned long long data_size, @@ -1519,7 +1525,7 @@ bool file_to_disk_secure_update(const char* path, const void* data, unsigned lon const char* temp_dir) { return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, preallocate, metadata, policy, true, false, false, temp_dir, NULL, false, false, NULL, - NULL); + NULL, 0, 0); } bool file_to_disk_secure_with_fsync(const char* path, const void* data, @@ -1528,7 +1534,7 @@ bool file_to_disk_secure_with_fsync(const char* path, const void* data, FileAttrPolicy policy, bool use_fsync, const char* temp_dir) { return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, preallocate, metadata, policy, false, false, use_fsync, temp_dir, NULL, false, false, - NULL, NULL); + NULL, NULL, 0, 0); } bool file_to_disk_secure_no_replace(const char* path, const void* data, @@ -1537,7 +1543,7 @@ bool file_to_disk_secure_no_replace(const char* path, const void* data, const char* temp_dir) { return file_to_disk_secure_impl(path, data, data_size, false, sparse, preallocate, metadata, policy, false, true, false, temp_dir, NULL, false, false, NULL, - NULL); + NULL, 0, 0); } /* Receiver write-path variant that also applies the per-file xattrs (-X/-A) @@ -1552,19 +1558,19 @@ bool file_to_disk_secure_attrs(const char* path, const void* data, unsigned long bool fake_super, bool keep_partial, const char* temp_dir) { return file_to_disk_secure_attrs_counted(path, data, data_size, inplace, sparse, preallocate, metadata, policy, update, no_replace, use_fsync, xattrs, - fake_super, keep_partial, temp_dir, NULL, NULL); + fake_super, keep_partial, temp_dir, NULL, NULL, 0, 0); } -bool file_to_disk_secure_attrs_counted(const char* path, const void* data, - unsigned long long data_size, bool inplace, bool sparse, - bool preallocate, const FileMetadata* metadata, - FileAttrPolicy policy, bool update, bool no_replace, - bool use_fsync, const FileXattrList* xattrs, bool fake_super, - bool keep_partial, const char* temp_dir, - unsigned* dirs_created, const char* count_floor) { +bool file_to_disk_secure_attrs_counted( + const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, + bool preallocate, const FileMetadata* metadata, FileAttrPolicy policy, bool update, + bool no_replace, bool use_fsync, const FileXattrList* xattrs, bool fake_super, + bool keep_partial, const char* temp_dir, unsigned* dirs_created, const char* count_floor, + uint32_t fake_super_rdev_major, uint32_t fake_super_rdev_minor) { return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, preallocate, metadata, policy, update, no_replace, use_fsync, temp_dir, xattrs, - fake_super, keep_partial, dirs_created, count_floor); + fake_super, keep_partial, dirs_created, count_floor, + fake_super_rdev_major, fake_super_rdev_minor); } /* Atomic --link-dest install. The destination is replaced (via a temporary @@ -1694,7 +1700,7 @@ static bool file_copy_basis_stream_impl(const char* path, const char* basis_path wrote = false; } if (wrote) - restore_extra_fd(fd, metadata, xattrs, fake_super, policy); + restore_extra_fd(fd, metadata, xattrs, fake_super, policy, 0, 0); if (wrote && use_fsync) wrote = fsync(fd) == 0; if (close(fd) != 0) @@ -1843,7 +1849,7 @@ static bool file_to_disk_secure_link_impl(const char* path, const char* basis_pa return true; return file_to_disk_secure_attrs_counted( path, data, data_size, false, false, preallocate, metadata, policy, false, false, use_fsync, - xattrs, fake_super, false, temp_dir, dirs_created, count_floor); + xattrs, fake_super, false, temp_dir, dirs_created, count_floor, 0, 0); } if (scratch_dirfd >= 0) diff --git a/src/shared/file.h b/src/shared/file.h index 12654ee..dc02584 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -182,13 +182,12 @@ bool file_copy_basis_stream_attrs(const char* path, const char* basis_path, * confined secure walk had to create that lie strictly below `count_floor` (a * receive-root-relative prefix, or NULL for all). Used to reproduce rsync's * `Number of created files` directory count on a fresh destination. */ -bool file_to_disk_secure_attrs_counted(const char* path, const void* data, - unsigned long long data_size, bool inplace, bool sparse, - bool preallocate, const FileMetadata* metadata, - FileAttrPolicy policy, bool update, bool no_replace, - bool use_fsync, const FileXattrList* xattrs, bool fake_super, - bool keep_partial, const char* temp_dir, - unsigned* dirs_created, const char* count_floor); +bool file_to_disk_secure_attrs_counted( + const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, + bool preallocate, const FileMetadata* metadata, FileAttrPolicy policy, bool update, + bool no_replace, bool use_fsync, const FileXattrList* xattrs, bool fake_super, + bool keep_partial, const char* temp_dir, unsigned* dirs_created, const char* count_floor, + uint32_t fake_super_rdev_major, uint32_t fake_super_rdev_minor); bool file_to_disk_secure_link_attrs_counted(const char* path, const char* basis_path, const void* data, unsigned long long data_size, bool preallocate, const FileMetadata* metadata, diff --git a/src/shared/file_save.c b/src/shared/file_save.c index 73f4748..d0d4672 100644 --- a/src/shared/file_save.c +++ b/src/shared/file_save.c @@ -401,12 +401,13 @@ bool file_special_rdev_valid(int32_t major, int32_t minor, mode_t mode) { * * Privilege gating: making a real device node requires CAP_MKNOD (root); making * a FIFO works unprivileged (mkfifo). A device node whose mknodat() fails with - * EPERM/EACCES is a genuine transfer error (rsync parity: rsync reports the - * mknod failure and the run exits partial, code 23). Only the unprivileged - * FIFO/socket (--specials) path keeps the best-effort skip, because those are - * normally creatable without privilege and a failure there is environmental. - * CI runs non-root, so device creation is expected to fail there; only a FIFO - * is honestly assertable unprivileged. + * EPERM/EACCES is a PER-ENTRY failure (rsync parity: rsync reports the mknod + * failure, still transfers the rest, and exits partial, code 23), reported as + * FILE_SAVE_FAILED so the receiver counts it and continues. Only the + * unprivileged FIFO/socket (--specials) path keeps the best-effort skip, + * because those are normally creatable without privilege and a failure there is + * environmental. CI runs non-root, so device creation is expected to fail + * there; only a FIFO is honestly assertable unprivileged. * * Confinement: the parent directory is opened fd-relative below the receive * root (file_open_secure_parent: O_NOFOLLOW, no "..", root-checked) and the @@ -548,10 +549,10 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons const char* shown_path = escaped_path ? escaped_path : ""; if (is_char || is_blk) { /* rsync parity: a device node that cannot be created (no CAP_MKNOD, or - * super-user activities not permitted) is a genuine transfer error. - * rsync reports `mknod ".../node" failed: ...` and the run exits - * partial (23); FastSync surfaces it through the outcome aggregation - * instead of silently skipping the entry. FIFO/socket creation + * super-user activities not permitted) is a per-entry failure. rsync + * logs `mknod ".../node" failed: ...`, still transfers the remaining + * files, and exits partial (23); FastSync logs it, counts it, and + * continues rather than aborting the stream. FIFO/socket creation * (--specials) keeps the best-effort skip path below. */ log_message(LOG_LEVEL_ERROR, "cannot create %s %s: %s\n" @@ -561,7 +562,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons close(parent_fd); free(leaf); free(destination); - return FILE_SAVE_ERROR; + return FILE_SAVE_FAILED; } /* Missing CAP_MKNOD / parent write permission for a FIFO/socket: the environment cannot create the node, so skip instead of failing the @@ -936,6 +937,14 @@ static bool file_save_try_special_dispatch(const FileSavePlan* plan, bool* creat /* Device/special node (--devices/--specials): recreate the node instead of writing content (privilege-gated, confined, rdev-validated). */ if (file->is_special) { + /* Under --fake-super rsync never mknod()s a device: it writes a regular + empty file and records the real rdev in user.rsync.%stat. Fall through to + the ordinary writer so the device round-trips (its S_IFMT mode bits and + rdev are parked in the record). Without --fake-super the node is + recreated (or, when privilege is refused, handled per-entry). */ + mode_t special_mode = file->metadata ? file->metadata->mode : 0; + if (config && config->fake_super && (S_ISCHR(special_mode) || S_ISBLK(special_mode))) + return false; *out = file_save_special_to_disk(plan->root_directory, file, config, created); return true; } @@ -1110,7 +1119,7 @@ static bool file_save_install_data(FileSavePlan* plan, const FileMetadata* metad config && config->preallocate, metadata, plan->policy, config && config->update, config && config->ignore_existing, config && config->use_fsync, file->xattrs, config ? config->fake_super : false, config ? config->partial : false, plan->confined_temp, - created_dirs, count_floor); + created_dirs, count_floor, (uint32_t)file->rdev_major, (uint32_t)file->rdev_minor); } free(count_floor); return ok; diff --git a/src/shared/file_save.h b/src/shared/file_save.h index d995e5f..5483c27 100644 --- a/src/shared/file_save.h +++ b/src/shared/file_save.h @@ -12,8 +12,16 @@ /* Outcome of a single file_save_to_disk operation. The receiver needs to distinguish "written" from "skipped" so --remove-source-files can be told - which sources were actually stored. */ -typedef enum { FILE_SAVE_ERROR = 0, FILE_SAVE_WRITTEN = 1, FILE_SAVE_SKIPPED = 2 } FileSaveResult; + which sources were actually stored. FILE_SAVE_FAILED is a per-entry failure + (for example a device node that mknodat() refused with EPERM/EACCES): it is + logged and counted by the receiver but does NOT abort the transfer, matching + rsync's continue-and-exit-partial behavior. */ +typedef enum { + FILE_SAVE_ERROR = 0, + FILE_SAVE_WRITTEN = 1, + FILE_SAVE_SKIPPED = 2, + FILE_SAVE_FAILED = 3 +} FileSaveResult; bool file_special_rdev_valid(int32_t major, int32_t minor, mode_t mode); diff --git a/src/shared/xattr.c b/src/shared/xattr.c index 56300d7..91f82c4 100644 --- a/src/shared/xattr.c +++ b/src/shared/xattr.c @@ -7,6 +7,7 @@ #include "utils.h" #include "file_types.h" #include +#include #include #include #include @@ -386,6 +387,56 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, uint } } +/* Parse rsync's `user.rsync.%stat` grammar strictly: + * " , :" + * Every field is parsed with strtoul() so an out-of-range value is a clean + * rejection rather than the undefined behavior sscanf("%u") exhibited, each + * field is range-checked against the same bounds the wire validator uses, and + * the whole record must be consumed (only trailing whitespace is tolerated) so + * trailing garbage is refused. Returns false on any malformed input. */ +static bool fake_super_parse_stat(const char* record, unsigned* mode_out, unsigned* rdev_major_out, + unsigned* rdev_minor_out, unsigned* uid_out, unsigned* gid_out) { + if (!record) + return false; + char* end = NULL; + const char* p = record; + errno = 0; + unsigned long mode = strtoul(p, &end, 8); + if (errno != 0 || end == p || mode > (unsigned long)UINT_MAX || *end != ' ') + return false; + p = end + 1; + errno = 0; + unsigned long rdev_major = strtoul(p, &end, 10); + if (errno != 0 || end == p || rdev_major > 0xffffUL || *end != ',') + return false; + p = end + 1; + errno = 0; + unsigned long rdev_minor = strtoul(p, &end, 10); + if (errno != 0 || end == p || rdev_minor > 0x00ffffffUL || *end != ' ') + return false; + p = end + 1; + errno = 0; + unsigned long uid = strtoul(p, &end, 10); + if (errno != 0 || end == p || uid > (unsigned long)UINT_MAX || *end != ':') + return false; + p = end + 1; + errno = 0; + unsigned long gid = strtoul(p, &end, 10); + if (errno != 0 || end == p || gid > (unsigned long)UINT_MAX) + return false; + p = end; + while (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r') + p++; + if (*p != '\0') + return false; + *mode_out = (unsigned)mode; + *rdev_major_out = (unsigned)rdev_major; + *rdev_minor_out = (unsigned)rdev_minor; + *uid_out = (unsigned)uid; + *gid_out = (unsigned)gid; + return true; +} + /* --fake-super replay: read the freshly-stored record and re-apply its * permission bits fd-relative. The recorded uid/gid are retained for a later * privileged restore but are NEVER chowned here: --fake-super only RECORDS @@ -403,7 +454,7 @@ bool fake_super_restore_fd(int fd, FileAttrPolicy policy) { return false; /* absent or filesystem without xattrs: silent no-op */ record[len] = '\0'; unsigned ul_mode, rdev_major, rdev_minor, ul_uid, ul_gid; - if (sscanf(record, "%o %u,%u %u:%u", &ul_mode, &rdev_major, &rdev_minor, &ul_uid, &ul_gid) != 5) + if (!fake_super_parse_stat(record, &ul_mode, &rdev_major, &rdev_minor, &ul_uid, &ul_gid)) return false; /* malformed record: skip, never fatal */ /* --fake-super NEVER performs a real chown: that would defeat the whole point diff --git a/tests/integration/test_temp_dir_absolute.py b/tests/integration/test_temp_dir_absolute.py index ff7f761..abc591e 100644 --- a/tests/integration/test_temp_dir_absolute.py +++ b/tests/integration/test_temp_dir_absolute.py @@ -64,6 +64,13 @@ def test_read_batch_absolute_temp_dir_inside_root_accepted(tmp_path): clean_dir(dest) scratch = os.path.join(dest, "scratch") os.makedirs(scratch) + # Stamp the scratch dir with an old mtime so the test can prove the receiver + # really created (and then removed) its temp file there: the directory mtime + # changes when an entry is created/removed, so a silently-ignored --temp-dir + # would leave the stamp untouched. An empty scratch dir alone does not + # distinguish "used and cleaned up" from "never used". + stale_mtime = 946684800 # 2000-01-01 + os.utime(scratch, (stale_mtime, stale_mtime)) batch = _make_batch(str(tmp_path), source) result = _run(["--read-batch", batch, dest, "--temp-dir", scratch]) @@ -73,6 +80,9 @@ def test_read_batch_absolute_temp_dir_inside_root_accepted(tmp_path): for rel, data in FILES.items(): assert _read(os.path.join(received, rel)) == data, f"content mismatch for {rel}" assert os.listdir(scratch) == [], "scratch dir was not left clean" + assert os.stat(scratch).st_mtime != stale_mtime, ( + "--temp-dir scratch dir was never written to (temp file not created there)" + ) @pytest.mark.ci @@ -98,6 +108,11 @@ def test_tcp_absolute_temp_dir_inside_root_accepted(shared_server): clean_dir(dest) scratch = os.path.join(dest, "scratch") os.makedirs(scratch) + # See test_read_batch_absolute_temp_dir_inside_root_accepted: the stale + # mtime makes actual scratch usage observable (the temp file creation and + # removal bump the directory mtime). + stale_mtime = 946684800 # 2000-01-01 + os.utime(scratch, (stale_mtime, stale_mtime)) result, _ = run_client(source, dest, flags=["--temp-dir", scratch], port=shared_server.port) assert result.returncode == 0, (result.stdout, result.stderr)[:300] @@ -105,6 +120,9 @@ def test_tcp_absolute_temp_dir_inside_root_accepted(shared_server): for rel, data in FILES.items(): assert _read(os.path.join(received, rel)) == data, f"content mismatch for {rel}" assert os.listdir(scratch) == [], "scratch dir was not left clean" + assert os.stat(scratch).st_mtime != stale_mtime, ( + "--temp-dir scratch dir was never written to (temp file not created there)" + ) def test_tcp_absolute_temp_dir_outside_root_rejected(shared_server): diff --git a/tests/test_file.c b/tests/test_file.c index 786ab4c..744ec6d 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -9,7 +9,9 @@ #include "charset.h" #include "utils.h" #include "protocol.h" +#include "xattr.h" #include "test_utils.h" +#include #include #include #include @@ -17,6 +19,7 @@ #include #include #include +#include #include #include @@ -348,7 +351,8 @@ static void test_file_save_to_disk_temp_dir_confined() { mkdir(root, 0755); mkdir("test_temp_confine_tmp/scratch", 0755); mkdir("test_temp_confine_tmp/abs_scratch", 0755); - EXPECT_NOT_NULL(realpath("test_temp_confine_tmp/abs_scratch", inside_abs)); + if (!realpath("test_temp_confine_tmp/abs_scratch", inside_abs)) + EXPECT_FAIL("realpath(abs_scratch) failed; inside_abs would be uninitialized"); mkdir(outside, 0755); File* f = file_create("file.txt"); @@ -1327,6 +1331,103 @@ static void test_special_socket_recreated() { rmdir(root); } +/* --fake-super device round-trip (rsync parity): a char/block device must be + * materialized as a REGULAR empty file whose user.rsync.%stat records the real + * rdev -- never as an mknod'ed node -- even on a privileged receiver. This is + * the non-privileged unit counterpart to the setpriv integration test (which + * the PR gate excludes). */ +static void test_fake_super_device_writes_regular_file_with_rdev() { + const char* root = "test_fake_super_dev_tmp"; + const char* node = "test_fake_super_dev_tmp/cdev"; + unlink(node); + rmdir(root); + EXPECT_EQ_INT(mkdir(root, 0700), 0); + + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + cfg->fake_super = true; + cfg->preserve_devices = true; + cfg->preserve_perms = true; + cfg->use_metadata = true; + cfg->use_xattrs = true; + + FileMetadata meta; + memset(&meta, 0, sizeof(meta)); + meta.mode = S_IFCHR | 0644; + + File* f = file_create("cdev"); + EXPECT_NOT_NULL(f); + f->is_special = true; + f->rdev_major = 1; + f->rdev_minor = 3; + f->metadata = &meta; + + EXPECT_EQ_INT(file_save_to_disk_full(root, f, cfg), FILE_SAVE_WRITTEN); + + struct stat st; + EXPECT_EQ_INT(lstat(node, &st), 0); + EXPECT_TRUE(S_ISREG(st.st_mode)); /* never a real device node */ + EXPECT_EQ_INT((int)st.st_size, 0); + + char value[128] = {0}; + ssize_t got = getxattr(node, FAKESUPER_XATTR, value, sizeof(value) - 1); + EXPECT_TRUE(got > 0); + EXPECT_EQ_STR(value, "20644 1,3 0:0"); /* the REAL rdev, not 0,0 */ + + f->metadata = NULL; + file_destroy(f); + config_delete(cfg); + unlink(node); + rmdir(root); +} + +/* A char/block device that mknodat() refuses (EPERM/EACCES on an unprivileged + * receiver) must be a PER-ENTRY failure -- FILE_SAVE_FAILED, which the receiver + * counts and continues past -- never the fatal FILE_SAVE_ERROR that aborts the + * stream. The unit suite normally runs as root, so drop the effective uid to + * make the kernel refusal deterministic. */ +static void test_device_mknod_failure_is_per_entry() { + const char* root = "test_device_eperm_tmp"; + const char* node = "test_device_eperm_tmp/cdev"; + unlink(node); + rmdir(root); + EXPECT_EQ_INT(mkdir(root, 0777), 0); + + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + cfg->preserve_devices = true; + cfg->use_metadata = true; + + FileMetadata meta; + memset(&meta, 0, sizeof(meta)); + meta.mode = S_IFCHR | 0644; + + File* f = file_create("cdev"); + EXPECT_NOT_NULL(f); + f->is_special = true; + f->rdev_major = 1; + f->rdev_minor = 3; + f->metadata = &meta; + + uid_t saved = geteuid(); + bool dropped = false; + if (saved == 0 && seteuid(65534) == 0) + dropped = true; + FileSaveResult result = file_save_to_disk_full(root, f, cfg); + if (dropped) + EXPECT_EQ_INT(seteuid(saved), 0); + + EXPECT_EQ_INT(result, FILE_SAVE_FAILED); + /* Nothing was created: no device node and no regular-file fallback. */ + struct stat st; + EXPECT_EQ_INT(lstat(node, &st), -1); + + f->metadata = NULL; + file_destroy(f); + config_delete(cfg); + rmdir(root); +} + static void test_inplace_overwrite_truncates_shorter_payload() { const char* root = "test_inplace_trunc_tmp"; const char* path = "test_inplace_trunc_tmp/big.txt"; @@ -2408,6 +2509,8 @@ void test_file() { test_new_file_mode_honors_source_and_umask(); test_special_fifo_mode_honors_source_and_umask(); test_special_socket_recreated(); + test_fake_super_device_writes_regular_file_with_rdev(); + test_device_mknod_failure_is_per_entry(); test_inplace_overwrite_truncates_shorter_payload(); test_inplace_refuses_fifo_destination(); test_inplace_refuses_device_destination(); diff --git a/tests/test_xattr.c b/tests/test_xattr.c index e659ed7..9a0ad1a 100644 --- a/tests/test_xattr.c +++ b/tests/test_xattr.c @@ -450,6 +450,24 @@ static void test_fake_super_rsync_format() { EXPECT_EQ_INT((int)st.st_uid, (int)before.st_uid); EXPECT_EQ_INT((int)st.st_gid, (int)before.st_gid); + /* Hardened parser: an out-of-range field (previously UB via sscanf("%u")), + a missing field, or trailing garbage is rejected cleanly instead of being + silently accepted. */ + const char* malformed[] = { + "20644 65536,3 111:222", /* major > 0xffff */ + "20644 1,16777216 111:222", /* minor > 0xffffff */ + "20644 1,3 111:222 trailing", /* trailing garbage */ + "20644 1,3 111", /* missing gid */ + "20644 1,3 4294967296:222", /* uid > UINT_MAX */ + "20644 1,3 111:4294967296", /* gid > UINT_MAX */ + "99999999999999999999 1,3 0:0", /* mode overflow */ + "", /* empty record */ + }; + for (size_t i = 0; i < sizeof(malformed) / sizeof(malformed[0]); i++) { + EXPECT_EQ_INT((int)fsetxattr(fd, FAKESUPER_XATTR, malformed[i], strlen(malformed[i]), 0), 0); + EXPECT_FALSE(fake_super_restore_fd(fd, policy)); + } + close(fd); unlink(path); }