fix(metadata): wire fake-super through receiver and gate mknod#3530
Merged
Conversation
Implements F1 and F2 from docs/audits/fake-super-privilege.md (#3435): - F1: load_fake_super was exported but never called. Add a fast-path read in apply_ownership_via_fake_super so a re-application skips the rewrite when the existing rsync.%stat xattr already matches the desired stat. Mirrors xattrs.c:read_stat_xattr() / set_file_attrs(). - F2: copy_device and copy_fifo invoked mknod(2) unconditionally, failing without CAP_MKNOD under --fake-super. Add create_{fifo,device_node}_with_fake_super wrappers in metadata::special which substitute a regular 0600 placeholder when fake-super is active, mirroring upstream syscall.c:do_mknod()'s am_root < 0 branch. After placeholder creation, the local-copy executors store the would-be mode/uid/gid/rdev in the rsync.%stat xattr so the destination can be faithfully restored on a later --fake-super pull. Also fixes a latent S_IFMT loss in apply_ownership_via_fake_super: entry.permissions() drops the file-type bits, but xattrs.c:set_stat_xattr encodes the full mode. Use entry.mode() instead. Tests: - 3 unit tests in metadata::special covering fake-super placeholder substitution for FIFO and device, and the disabled fall-through. - 3 integration tests in metadata::apply::tests covering the wire-side receiver path: xattr is written, chown is not invoked, and an identical second application is a no-op. All new code is gated by #[cfg(unix)] / #[cfg(all(unix, feature = "xattr"))]. Non-Unix targets continue to fall through to the existing no-op stubs. No unsafe code is introduced; metadata is on the permitted-unsafe list but this change uses only safe std/rustix APIs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses findings F1 (HIGH) and F2 (HIGH) from
docs/audits/fake-super-privilege.md(audit landed in #3435).Summary
create_fifo_with_fake_superandcreate_device_node_with_fake_supertometadata::special. Whenfake_superis true these substitute a regular0600placeholder file for the privilegedmknod(2)call, mirroring upstreamsyscall.c:do_mknod()'sam_root < 0branch. The local-copy executorscopy_deviceandcopy_fifonow threadMetadataOptions::fake_super_enabled()through these wrappers, then store the would-bemode/uid/gid/rdevin theuser.rsync.%statxattr viastore_fake_super.apply_ownership_via_fake_supernow reads the existing xattr viaload_fake_superand short-circuits when the stored stat already matches the desired one, mirroring upstreamxattrs.c:read_stat_xattr/set_file_attrs's "no-op when state matches" fast path.entry.permissions()returnsmode & 0o7777, butxattrs.c:set_stat_xattrencodes the full mode (S_IFMT + perms). Useentry.mode()so a laterread_stat_xattrcan rebuild the file type.All new code is gated
#[cfg(unix)]/#[cfg(all(unix, feature = "xattr"))]. Non-Unix targets fall through to the existing no-op stubs. No new unsafe code; this change uses only safestd/rustix/xattrcrate APIs.Files touched
crates/metadata/src/special.rs(+177) fake-super placeholder helper, public wrappers, 3 unit testscrates/metadata/src/lib.rs(+4) re-exports for the new wrapperscrates/metadata/src/apply/ownership.rs(+13)load_fake_supershort-circuit, S_IFMT fixcrates/metadata/src/apply/tests.rs(+124) 3 integration tests for the receiver-side wire pathcrates/engine/src/local_copy/executor/special/device.rs(+35) threadfake_superthroughcopy_device, capture xattr after placeholdercrates/engine/src/local_copy/executor/special/fifo.rs(+32) same change forcopy_fifoUpstream references cited in code
xattrs.c:set_stat_xattr()andread_stat_xattr()syscall.c:do_mknod()(am_root < 0 branch, lines 90-174)rsync.c:set_file_attrs()Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features --no-deps -- -D warningscargo nextest run -p metadata --all-featurescovers:fake_super_replaces_mkfifo_with_regular_placeholderfake_super_replaces_mknod_with_regular_placeholdercreate_fifo_with_fake_super_disabled_creates_real_fifofake_super_writes_rsync_stat_xattr_for_regular_filefake_super_does_not_chown_destinationfake_super_skips_rewrite_when_xattr_already_matchescargo nextest run -p engine --all-featurescovers existingcopy_fifo/copy_devicepaths (unchanged whenfake_super=false)#[cfg(not(unix))]arms)Follow-ups (out of scope, separate PRs)
fake super = yesstill does not demoteam_rootor rewrite--super. Tracked in audit; needsclientserver.c:1080-1107parity.chmodpath doesn't honour fake-super; touch-up-dirs missing.am_root()is binary instead of upstream tri-state.