docs(metadata): audit --fake-super / --super privilege paths#3435
Merged
Conversation
Audit of oc-rsync's `--fake-super`, `--super`, and `--copy-as` implementations against upstream rsync 3.4.1. Identifies 3 HIGH, 5 MEDIUM, and 2 LOW findings, all with file:line evidence and upstream cross-references. Top issues: `load_fake_super` is exported but never called (F1), special-file executors invoke real `mknod` syscalls under fake-super (F2), and the daemon parses `fake super = yes` but no read site actually demotes `am_root` (F3). Audit document only; ten follow-up issues recommended for the actual fixes. Closes #1839.
github-actions
Bot
added
the
documentation
oferchen
added a commit
that referenced
this pull request
May 1, 2026
…3435) Audit of oc-rsync's `--fake-super`, `--super`, and `--copy-as` implementations against upstream rsync 3.4.1. Identifies 3 HIGH, 5 MEDIUM, and 2 LOW findings, all with file:line evidence and upstream cross-references. Top issues: `load_fake_super` is exported but never called (F1), special-file executors invoke real `mknod` syscalls under fake-super (F2), and the daemon parses `fake super = yes` but no read site actually demotes `am_root` (F3). Audit document only; ten follow-up issues recommended for the actual fixes. Closes #1839.
5 tasks
oferchen
added a commit
that referenced
this pull request
May 1, 2026
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.
oferchen
added a commit
that referenced
this pull request
May 5, 2026
…3435) Audit of oc-rsync's `--fake-super`, `--super`, and `--copy-as` implementations against upstream rsync 3.4.1. Identifies 3 HIGH, 5 MEDIUM, and 2 LOW findings, all with file:line evidence and upstream cross-references. Top issues: `load_fake_super` is exported but never called (F1), special-file executors invoke real `mknod` syscalls under fake-super (F2), and the daemon parses `fake super = yes` but no read site actually demotes `am_root` (F3). Audit document only; ten follow-up issues recommended for the actual fixes. Closes #1839.
oferchen
added a commit
that referenced
this pull request
May 5, 2026
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.
Summary
Adds
docs/audits/fake-super-privilege.md, a structured audit of the--fake-super,--super, and--copy-ascode paths against upstream rsync 3.4.1.The audit identifies 10 findings (3 HIGH, 5 MEDIUM, 2 LOW) where the current implementation diverges from upstream semantics or is incompletely wired:
load_fake_superis exported but never called from any production path; special-file executors invoke real syscalls without the upstreamam_root < 0placeholder substitution; daemon parsesfake super = yesbut no read site demotesam_rootor rewrites--super.-XX/ repeated-flag validation; chmod path doesn't honour fake-super and touch-up-dirs is missing;am_root()is binary instead of upstream tri-state; remote invocation builder unconditionally pushes--fake-super;--copy-asis parsed but never callssetuid/setgid.FakeSuperStat::encodeskipsto_wire_mode; rdev(0,0)is decoded toNoneand lost on round-trip.Each finding includes Evidence (file:line citations into both this codebase and
target/interop/upstream-src/rsync-3.4.1/), Impact, and a Recommended fix. The audit closes with a 10-item follow-up task table referencing the upstream sources.This is documentation only — no behavioural change.
Refs: #1839
Test plan
xattrs.c:340-720,syscall.c:90-174,clientserver.c:1080-1107,rsync.c:do_as_root