operator-path-backup-chown probes for a cross-device directory, because
make_backup() renames into a same-filesystem backup dir and never reaches
the set_file_attrs() path the test is about. On macOS there is normally
no such directory, so the test is a macOS-wide expected skip -- but this
target puts the scratch trees on a separate HFS+ volume, which supplies
exactly the condition it was looking for.
Same reason backup-crossdev-copy and chmod-temp-dir are already omitted
here. The test PASSES on the target (10.9s); only the expected-skip
bookkeeping was wrong.
The base gained 13 commits. Three needed hand-porting because the change
lands on code the VFS split moved or renamed:
- 31130112 (--confine-root) is almost entirely in syscall.c, which does
not exist here. abspath_excluded_by_module() becomes
abspath_outside_confinement() in vfs/dirstack.c, taking its root from
vfs.module_dir when we are a daemon and from confine_root otherwise,
plus the fd-pin helpers; ona_open() in vfs/owner_walk.c gains the
getcwd() seed and the pin_transit exemption. The VFS passes is_operator
as an argument where the base reads operator_path_resolve, so the
refusal takes it from there rather than the deleted global.
- d09edb85 (--link-dest hard-link fallback) keeps the VFS call
vfs_link_at(cmpbuf, fname, !am_daemon ? VFS_OPERATOR_PATH : 0, 0) and
adopts the base's cannot_hardlink/match_level fallback around it.
- t_stub.c gains the confine_root/confine_rootlen stubs beside the VFS
curr_dir note.
Tree is byte-identical to the merge oracle (tag merge-reference-11).
The previous commit gave set_file_attrs()' path-based chmod/chown the
operator ownership walk, but nothing exercised the chown side: the
instrumented branch was reached 92 times by the existing suite, all of
them chmod. That is the same gap that let the upstream fix ship inert.
The uncovered path is a SYMLINK backup. op_pin cannot pin a symlink --
there is no O_NOFOLLOW open of one -- so make_backup()'s set_file_attrs()
falls through to the full-path lchown. Without the walk, a backup parent
flipped to an attacker-owned symlink redirects that lchown onto a victim
outside the backup tree and retags it as the attacker's.
Two things make this awkward to test, and both are why it was missed:
- On one filesystem make_backup() hard-links or renames the item into
the backup dir and never calls set_file_attrs() at all. The fixture
therefore puts the backup dir on tmpfs so link and rename fail EXDEV
and the recreate path runs. backup-crossdev-copy probes the same way.
- A statically planted symlink proves nothing: rsync's own backup-dir
validation deletes a non-directory component before using it. The
plant has to be a live flip, as in operator-path-backup-symlink.
The positive control checks both that the symlink reached the backup tree
and that the backup copy carries the attacker's uid -- i.e. that an lchown
actually ran. Without that second assertion the race would pass
vacuously on any build where the chown never happens.
RED on the parent of the previous commit (victim outside/f94 retagged
away from root); GREEN here. Runs on Linux CI as root; registered as an
expected skip on Cygwin (root-only) and macOS (root-only, and no
cross-device tmpfs -- backup-crossdev-copy is listed there for the same
reason). Full expected-skip oracle passes exactly.
Base commit 0bfcd3b0 taught do_chmod_at()/do_lchown_at() to resolve an
operator-supplied path through the ownership walk. The previous commit
ported that into vfs_chmod()/vfs_lchown(), but on this branch the policy
is a per-call argument rather than the ambient operator_path_resolve
global, and set_file_attrs() still passed 0 -- so the ported branch was
unreachable and the fix was inert.
op_pin already covers a reg/dir/fifo leaf with a pinned fd, which is
stronger than the walk. What it does not cover reaches the path-based
fallbacks: a symlink or device leaf never enters op_pin, and a non-root
operator can fail the pin open with an ordinary EACCES and fall through
with op_refuse clear. Both then resolved the full operator path with a
bare lchown()/chmod().
Derive the flag from ATTRS_OPERATOR_PATH and pass it to the two
VFS_AT_FDCWD fallbacks only; the held-dirfd arms stay at 0, since a
pinned parent already confines them. vfs_chmod()'s operator branch
skips S_ISLNK on its own, so a symlink-as-object keeps the
lchmod()/setattrlist() path, matching the base.
Also drop ten .gitignore entries the rebase duplicated: this branch's
own test-helper block and the base's new one list the same binaries.
Suite 259/0/83; the operator-path, backup and temp-dir families pass as
root over --use-tcp. Instrumenting the branch shows it is now reached
92 times across those tests (the chmod side); the lchown side has no
test exercising it, which is how the gap survived upstream.
The base gained 99 commits since the last rebase. Nine files needed a
hand-port because the change lands on code the VFS split moved or
renamed, and syscall.c no longer exists here:
- backup.c: make_path() now runs on a private copy of backup_dir_buf
(c933f622), so vfs_make_path() takes dirbuf and drops the restore.
- clientserver.c: keep both the module-root snapshot and the new
daemon_config_filter_file window.
- exclude.c: the peer-driven merge-file confinement is the
vfs_open_owner_walk() is_operator argument, not a global.
- fileio.c: the coalesced --sparse writer's new helpers use
vfs_lseek()/vfs_punch_hole().
- generator.c/vfs/mknod.c: gen_entry_mknod() falls back through
vfs_mknod(); the atfd path keys its mknodat() off HAVE_MKNODAT.
- receiver.c: secure_recv_open() passes VFS_OPERATOR_PATH instead of
toggling operator_path_resolve; open_readonly_inplace() uses the VFS
stat/chmod/open wrappers.
- sender.c: absolute --relative cleanup anchors at "/" via
vfs_resolve_open(), the copy-links walk uses
vfs_resolve_open_at_beneath(), and the source removal goes through
vfs_unlink().
- vfs/chmod.c, vfs/chown.c: VFS_OPERATOR_PATH now takes the ownership
walk, and the no-follow chmod grows the non-Linux fd path.
- vfs/secure_open.c: the fd-anchored resolver splits into a shared
internal with an allow-dotdot entry point, and secure_walk_at() routes
a literal "."/".." through ds_descend() before the leaf fast paths.
Tree is byte-identical to the merge oracle (tag merge-reference-10).
Linear-rebase counterpart of the conflict resolutions made when the
sec-fixes base was re-integrated (see the merge for reference). Three of
the base's new commits touch code this branch relocated or reworked:
- options.c (3fe1ed51 "rsync: confine the daemon files-from open to the
module root"): the base wraps the files-from open in the
operator_path_resolve global, which no longer exists here. Pass the
operator context explicitly instead: vfs_open_owner_walk(..., 1).
- receiver.c (cfd40f55 "receiver: confine peer-selected partial basis
paths"): the new relative-partial-basis branch uses the VFS resolver
name, vfs_resolve_open().
- vfs/chmod.c (7b16872e "syscall: silence scan-build dead-store in
do_fchmodat_nofollow fallback"): syscall.c is deleted here, so the
mode masking move and the unused-arg casts land in the relocated
do_fchmodat_nofollow.
Tree is byte-identical to the validated merge result.
Two fixes from codex review of the no-AT_FDCWD port:
- vfs/stat.c: the held-dirfd vfs_lstat branch fell back to
fstatat(dirfd, path, st, 0) when AT_SYMLINK_NOFOLLOW is unavailable,
which FOLLOWS the leaf and breaks lstat's no-follow contract. On a
system with SUPPORT_LINKS but no AT_SYMLINK_NOFOLLOW, return ENOSYS
instead (mirroring the held-fd vfs_lchown arm) so a symlink-sensitive
caller fails loud rather than silently following; the !SUPPORT_LINKS
arm keeps fstatat(...,0) since there is nothing to follow. The CI
compile-check config also undefines AT_FDCWD so no held fd is produced
there; this hardens the standalone "no AT_SYMLINK_NOFOLLOW" shape.
- Makefile.in: the vfs-no-at-fdcwd.o compile loop wrote every object to
$@, so a mid-loop failure left a fresh-timestamped $@ and a retry could
skip the check. Compile to $@.tmp and mv to $@ only after the whole
loop succeeds (rm the stale target up front); clean the .tmp too.
Linear-rebase counterpart of the conflict resolutions made when the
sec-fixes base was re-integrated (see the merge for reference). The base
gained 5cb4b829 ("syscall: build without AT_SYMLINK_NOFOLLOW") and its CI
compile-check, which touch code this branch relocated into vfs/:
- Move syscall.c's RSYNC_TEST_NO_AT_FDCWD undef block into vfs/vfs.h,
before the VFS_AT_FDCWD sentinel binds (so the sentinel takes its
no-AT_FDCWD value rather than dangling on the undefined AT_FDCWD).
- Port the AT_SYMLINK_NOFOLLOW-absent fallbacks into the relocated code:
vfs/chown.c (vfs__lchown_secure + held-fd vfs_lchown gate on
AT_SYMLINK_NOFOLLOW), vfs/stat.c (do_xstat_at's unused-arg casts, the
vfs_lstat AT_SYMLINK_NOFOLLOW-absent arm, held-fd gate), vfs/mkdir.c
(guard rand_bytes on AT_FDCWD, its only caller).
- Retarget the CHECK_COMPILE_OBJS compile-check from syscall.c to a
portable shell loop over the vfs sources built with
-DRSYNC_TEST_NO_AT_FDCWD (Makefile.in).
Tree is byte-identical to the validated merge result.
t_hashtable_overflow, t_iwildmatch, t_clean_fname, and t_safe_arg were
built by the suite but missing from .gitignore, so a stray git add -A
sweeps them into a commit (as happened during this rebase round). List
them alongside the other t_* harnesses.
daemon-exclude-namebased bound its daemon on 13010, and the
setup_chroot_inner helper hashed into 12940-13139 -- both reach into
13000+, where ASUS Armoury Crate on the Cygwin CI host parks localhost
listeners (13010, 13030-13032), making the port probe fail the test.
The helper also used str hash(), which is per-process randomized
(PYTHONHASHSEED), so its port wandered run to run.
Move the fixed port to 12931 and the helper to a deterministic
crc32-based slot in the otherwise-unused 12800-12859 band.
The #else arm for platforms without AT_FDCWD called the four-argument
vfs_symlink() with two arguments -- a compile error on such systems.
Call vfs__symlink_plain(), matching the base's do_symlink() fallback.
Found by codex review; predates this rebase round.
Linear-rebase counterpart of the conflict resolutions made when the
sec-fixes base was integrated (see the merge for reference):
- Port ddda7ba5's operator-path confinement of do_symlink_at and
do_rmdir_at into the relocated VFS code. vfs__symlink_secure gains
the VFS_OPERATOR_PATH ownership-walk branch (parent confined via
vfs_owner_walk_parent, shared leaf-creation preserved so fake-super
emulation still applies); vfs__unlink_secure extends its existing
operator branch to the rmdir/AT_REMOVEDIR case. Callers pass the
policy explicitly where the base set operator_path_resolve: the
keep_backup symlink create (backup.c), the backup-tree rmdir in
delete_item (delete.c, DEL_FOR_BACKUP), and handle_partial_dir's
rmdir (util1.c).
- Port 1f8f89c2's robust_rename EXDEV-fallback confinement into
vfs/robust.c: an absolute --temp-dir/--partial-dir operand routes the
copy_file dest-write and the source-unlink through the ownership walk
(VFS_OPERATOR_PATH), so a raced parent symlink can't redirect either
out of the module.
- Drop the stale "no ownership-walk branch" notes in vfs/vfs.h and
vfs/symlink.c now that symlink and rmdir carry the branch.
Tree is byte-identical to the validated merge result.
The HAVE_SOLARIS_ACLS facl(2) paths called secure_relpath_active() -- the
base-branch name -- which does not exist on the VFS branch (here the gate is
vfs_relpath_active()). Linux/BSD never compile that branch, so it stayed latent;
a real Solaris build fails with an implicit-declaration error. Rename both call
sites (set_rsync_acl default-ACL delete and the access/default set fallback).
Guard the two receiver decision points that drive the xattr/ACL setters down a
raw path-based branch -- set_file_attrs() and gen_entry_copy_xattrs(). In a
strict build, a confined pinnable non-operator leaf that reached the setters
without a confined fd (held_fd/xfd < 0 and not refused) aborts: that is exactly
the copy-xattrs fallback class. The guards mirror the pin/re-pin conditions, so
they cannot false-abort a legitimate transfer.
Verified: under --enable-strict-confinement the suite is 210/0 with zero aborts;
neutering the re-pin makes copy-xattrs-symlink-race abort at the guard.
Add a CI/dev hardening mode (--enable-strict-confinement) that turns any
confined-regime raw path-based metadata op into a hard abort, so a reintroduced
copy-xattrs-class fallback fails the test suite instead of silently escaping
through a flipped parent symlink.
vfs_must_be_confined() is the predicate: the modern *at/O_NOFOLLOW primitives are
present, the path is non-operator, relative and multi-component (a parent the
attacker could flip), and vfs_relpath_active(). vfs_strict_confine_fail() logs
and aborts. Both are absent/no-op without STRICT_CONFINEMENT, so there is no
production behaviour change.
set_file_attrs() pins the entry via the cached held dir fd and drives the xattr
/ACL ops off that fd (fsetxattr). But when the pin missed -- vfs_cached_dirfd()
returns -1 (its dir isn't the held one, or the path is deeper than the dirfd
cache), or the leaf openat() loses a race -- held_fd stayed -1 and
set_stat_xattr()/set_xattr()/get_acl_fdat()/set_acl_fdat() fell through to the
path-based branch (sys_lsetxattr(fname,...)). Unlike the chmod/chown/times path
wrappers (which secure-resolve), that raw lsetxattr re-resolves the parent, so a
concurrent flip of a dest parent component to a symlink->outside lands the xattr
OUTSIDE the destination tree (the intermittent copy-xattrs-symlink-race escape
that surfaces under -j load, which widens the open->setxattr window).
Re-pin through the secure resolver when the cached pin misses on a confined,
non-operator receiver path, so the xattr/ACL ops always use a confined fd -- NOT
a raw path lsetxattr; if the re-pin also fails (a genuinely raced parent/leaf
symlink) skip the path-based ops (xattr_refuse) rather than redirecting them.
The re-pin passes O_DIRECTORY for a directory leaf (the secure resolver refuses a
bare dir open with EISDIR), so dir xattrs/ACLs are still preserved on a cache
miss. Apply the same re-pin/refuse to gen_entry_copy_xattrs() (the dir xattr
copy), whose dfd<0 path likewise fell to copy_xattrs() with dest_fd==-1.
chmod/chown/times are unchanged (already confined via their *at wrappers);
operator paths keep op_pin/op_refuse.
Confirmed: under heavy parallel load the dangerous lsetxattr fallback was reached
52+ times before and 0 times after; dir xattrs preserved; suite 210/0/66,
root metadata PoC tests pass, builds +/-xattr.
Extend the t_rename_secure harness with PS-refuse/PS-follow: rename to the SAME
operator-owned escaping symlink path (oplink -> ../trap) with the SAME operator
old-side flag, differing ONLY in the new-side flag. The ownership walk (operator)
follows the operator's own uid0/euid symlink; the secure receiver resolve
(transfer, flag 0) refuses it because it leaves the cwd anchor. PS-refuse must be
rejected and PS-follow must succeed -- proving the new side's confinement is
independent of the old side's policy.
Deterministic, non-root, no race. Verified RED on the old whole-call behaviour
(emulated by OR-ing both operands' flags, the new side then follows oplink and
escapes to ../trap) and GREEN with the per-operand split. The driver adds the
oplink fixture and the per-side source files.
Suggested-by: Zen Dodd <mail@steadytao.com>
vfs_rename_at() and vfs_link_at() took a single vfs_flags applied to BOTH
operands, so VFS_OPERATOR_PATH (ownership walk: follow uid0/euid symlinks, refuse
foreign) was applied to a transfer operand even when only one side was operator-
supplied -- relaxing the secure receiver resolve that a transfer/file-list path
should get. The default branch confined each side by absolute-vs-relative, but
that missed a *relative* operator path (--backup-dir=bdir, --partial-dir=.rsync,
relative --link-dest). Reported in the rsync-private PR #30 review.
Take old_flags and new_flags separately and resolve each operand under its own
policy via a shared vfs_twopath_side() helper (operator walk if the side is
VFS_OPERATOR_PATH or absolute; secure receiver resolve if relative-with-slash;
AT_FDCWD for a bare name). An operator basis/backup path on one side can no
longer relax the other side's confinement.
Callers now pass the correct per-side policy:
- backup link/rename: source (live dest file) = transfer (0), backup target = operator.
- receiver partial-dir rename: partialptr = operator, final dest = transfer.
- hard_link_one / generator link-dest: basis source = operator (non-daemon), dest = transfer.
- finish_transfer / gen_entry_rename / robust fallback: both transfer (0,0; default per-side).
- robust_unlink ETXTBSY sibling rename: both share the caller's policy.
Also drops the operator flag on backup's "just in case" robust_unlink of the
transfer-side source.
Behaviour-preserving for the operator side (the backup/partial-dir parents still
owner-walk); it only tightens the transfer side back to the secure resolve.
Builds +/-xattr; suite 210/0/66; root operator-path PoC tests all pass.
Suggested-by: Zen Dodd <mail@steadytao.com>
A backup-tree delete (delete_item with DEL_FOR_BACKUP -- removing an existing
leaf under an absolute --backup-dir before the new backup is placed) fell through
del_unlink() to robust_unlink(fbuf, 0), which resolves the leaf's parent by path
with no ownership walk. A local attacker who flips a backup-path parent to a
symlink in that window could redirect the unlink outside the backup tree.
The base confines this: make_backup() wraps make_backup_inner() in
operator_path_resolve, so the same unlink reaches do_unlink_at()'s owner-walk
branch. The VFS refactor replaced the global with explicit vfs_flags but left
del_unlink() passing 0, dropping the confinement. Thread VFS_OPERATOR_PATH into
del_unlink()'s path-based fallback when DEL_FOR_BACKUP is set (the exact cases
that ran under the base's operator wrap); a held-dirfd delete is already confined
and ignores it. The rmdir path keeps no owner-walk branch, matching base.
The -X theirs rebase replayed the 47 VFS commits onto the new base, letting each
VFS commit win its refactor on conflict; that drops the round-3 base changes that
overlap the operator-path code (they were authored against the deleted syscall.c
and the pre-vfs_flags model). This commit snaps those files byte-for-byte from
the validated merge oracle (merge-reference-3 = 6afdd389), so the branch tip is
tree-identical to the merge that built clean +/-xattr and passed the suite +
root operator-path PoC tests.
Reconciled: the syscall.c->vfs/ ports (secure_open/owner_walk/dirstack/rename/
link/chmod/chown/times/copy_file + vfs.h/vfs_internal.h), the set_file_attrs
op_pin via ATTRS_OPERATOR_PATH (rsync.c/rsync.h + backup.c callers), the
secure_basis_open/basis_link_stat operator branches (receiver.c/generator.c), the
copy_file/gen_entry_copy_xattrs held-fd source reads, the change_dir/sender
opt-out sites (util1.c/sender.c/clientserver.c), and Makefile.in.
Linear-rebase counterpart of the round-2 merge resolution: 37dbb263 fixed
do_mknod_at()'s operator branch in syscall.c (deleted on this branch) to
fall back to mkfifoat() for a FIFO and EOPNOTSUPP for a nested socket when
mknodat() can't make a special file on the BSDs/macOS/Solaris. Re-express
it in the VFS_OPERATOR_PATH branch of vfs__mknod_secure() in vfs/mknod.c,
mirroring the secure-relpath branch already in that function.
Tree is byte-identical to the validated round-2 merge result.
Linear-rebase counterpart of the two conflict resolutions made when the
sec-fixes base was integrated (see the merge for reference):
- Port 8a7e9a16's fixed-array dirstack into the relocated VFS code:
vfs/vfs_internal.h (DS_MAXDEPTH + inline int fds[]) and vfs/dirstack.c
(drop malloc/realloc/free; ENOMEM past the cap). 8a7e9a16 changed the
dirstack in syscall.c, which this branch deleted, so the fix is
re-expressed in vfs/.
- Map the base's new F_XATTR ndx<0 guard (1d36a565) from the old
do_chmod_at() to the unified vfs_chmod(VFS_AT_FDCWD, fname, ..., 0).
Tree is byte-identical to the validated merge result.
Three issues from the post-unification review:
- The !SUPPORT_XATTRS x_stat/x_lstat macros hardcoded VFS_ALLOW_SYMLINK,
silently downgrading an operator-path stat (backup-dir) from the ownership
walk + module confinement to plain follow-stat in no-xattr builds (a
pre-existing gap the unified vfs_stat now lets us close). Forward the
caller's vfs_flags instead, so no-xattr builds get the same policy as
xattr builds.
- Held-fd ".." was accepted by the stat/chmod/lchown dispatchers, which
would operate on the PARENT of the pinned dirfd -- the wrong default for a
security boundary even though no current call site passes it. Reject ".."
while still allowing "." (a stat/chmod/chown of the dir itself, which
link_stat_at and set_file_attrs legitimately do for directory entries).
- Two stale comment/test strings still said vfs_chmod_at (now vfs_chmod).
Verified with a local --disable-xattr build; full suite 190/50.
vfs_stat/vfs_lstat are now vfs_<op>(int dirfd, path, st, int flags); the
no-xattr fallback macros still expanded to the old 2-arg vfs_stat(fn,fst),
breaking the build without xattr support. Map them to the unified plain
form: vfs_stat(VFS_AT_FDCWD, fn, fst, VFS_ALLOW_SYMLINK) (and lstat), which
preserves the prior plain-stat behaviour. Caught by the fleettest on
openbsd; verified with a local --disable-xattr build.
Rewrite the vfs.h header contract to describe the current model: the two-layer
vfs/ structure (primitives + compounds), the single vfs_<op>(int dirfd, ...,
int flags) form, the meaning of VFS_AT_FDCWD / VFS_ALLOW_SYMLINK /
VFS_OPERATOR_PATH / VFS_REMOVEDIR, that the operator policy is an explicit
per-call flag (not ambient state), and which ops keep explicit forms
(rename/link two-path, open variants, fd-based fstat/fileio).
Update the stale threat-model cross-references that pointed at the old
vfs_chmod_at() (now the static vfs__chmod_secure()) and the two
vfs_mknod_at()/vfs_symlink_at() mentions left by the unification.
Collapse the remaining three-form ops into one call each, matching
mkdir/mknod/symlink/unlink:
vfs_stat(int dirfd, path, st, flags)
vfs_lstat(int dirfd, path, st, flags)
vfs_chmod(int dirfd, path, mode, flags)
vfs_lchown(int dirfd, path, owner, group, flags)
dirfd == VFS_AT_FDCWD resolves the path (VFS_ALLOW_SYMLINK = plain libc op,
default 0 = secure receiver resolve, VFS_OPERATOR_PATH = ownership walk for
stat); a real held dirfd operates on a single component under it. The old
plain/_at/_atfd bodies become static helpers behind the dispatchers;
vfs_fstat stays (fd-based). chmod/lchown keep their no-owner-walk behaviour
(VFS_OPERATOR_PATH resolves the same as the default secure walk).
Pure API-narrowing -- the operator policy was already explicit; no global is
involved. This is behaviour-preserving (plain sites -> VFS_ALLOW_SYMLINK,
_at -> the flag they already carried, _atfd -> held-fd 0).
Held-fd validation differs by op: mkdir/mknod/symlink/unlink reject "."/".."
(creating/removing them is nonsensical), but stat/chmod/lchown ALLOW "."
(a read or metadata op on the dir itself is legitimate -- link_stat_at and
set_file_attrs do it for directory entries); only empty and multi-component
("/") names are rejected there. Caught by the suite (chmod/metadata/
ownership-depth + the implied-"." dir mkdir path).
Full suite 190/50.
The stat/lstat threading added a vfs_flags argument to x_stat()/x_lstat(),
but on a build without xattr support those are 3-arg macros in rsync.h
(mapping to plain vfs_stat/vfs_lstat), so the 4-arg call sites failed to
compile ("too many arguments to macro"). Give the macros the extra
(ignored) parameter. Caught by the fleettest on openbsd; verified with a
local --disable-xattr build.
With every operator-context call site now passing VFS_OPERATOR_PATH
explicitly, nothing reads vfs.operator_path_resolve any more. Remove the
four now-dead set/clear blocks (make_backup, handle_partial_dir, and the two
generator in-place-backup paths) and delete the field from struct vfs.
The operator-supplied path resolution policy -- which selects the ownership
walk and the daemon module-root confinement for --backup-dir/--temp-dir/
--partial-dir/--link-dest operations -- is no longer ambient dynamic-scope
state poked into the VFS from mainline. It travels as an explicit per-call
VFS_OPERATOR_PATH flag through the primitives and compounds, resolving the
layering violation that motivated this work.
Full suite 190/50.
vfs_stat_at / vfs_lstat_at (and their shared do_xstat_at helper) gain a
vfs_flags argument so the ownership-walk branch reads VFS_OPERATOR_PATH
instead of vfs.operator_path_resolve -- this was the last primitive still
reading the global. The metadata wrappers x_stat()/x_lstat() (which stay in
xattrs.c, above the VFS) gain a vfs_flags argument forwarded to the
primitives.
Call sites classified: every backup-dir stat (validate_backup_dir,
copy_valid_path's x_stat, make_backup_inner's x_lstat + bak lstat) and the
partial-dir lstat (handle_partial_dir) pass VFS_OPERATOR_PATH; the transfer
enumeration (flist link_stat + friends), the in-place dir stat (generator),
and the set_stat_xattr lstat pass 0. generator's non-daemon --link-dest
basis lookup passes 0 too (is_operator only gates the daemon module
confinement, a no-op there).
With this, NOTHING reads vfs.operator_path_resolve any more -- it is now
write-only (set by four now-dead blocks, removed next). Full suite 190/50;
backup/partial/link-dest/xattr/daemon subset green.
Give vfs_link_at an explicit vfs_flags argument (kept two-parent form) so its
ownership-walk branch reads VFS_OPERATOR_PATH instead of the global.
hard_link_one() gains a vfs_flags argument forwarded to vfs_link_at, so the
generator's alt-dest hard-link no longer sets vfs.operator_path_resolve
around the call: it passes (!am_daemon ? VFS_OPERATOR_PATH : 0), matching the
prior op = !am_daemon gating (a non-daemon --link-dest uses the ownership
walk; a daemon keeps the stronger module-anchored vfs_relpath_active path).
That operator_path_resolve block is removed.
Other call sites: the backup link (make_backup link_or_rename) passes
VFS_OPERATOR_PATH; the try_dests alt-dest link and the hlink finish-up
hard_link_one pass 0 (transfer context, as the global was there).
Set-sites: 5 -> 4 (backup make_backup, generator 2085/2130, util1
handle_partial_dir -- all wrapping the not-yet-flagged metadata layer
set_file_attrs/x_stat/x_lstat). Full suite 190/50; hardlink/link-dest/
backup/daemon subset green.
Give vfs_open_owner_walk an explicit is_operator argument so its module-root
confinement (abspath_excluded_by_module) reads the policy from the caller
instead of the vfs.operator_path_resolve global -- the last owner-walk
function still reading it. secure_basis_open gains an is_operator parameter
threaded to it; its gate (owner-walk vs strict resolve) now branches on that
param too.
Faithful conversion (no behaviour change): every direct caller passes the
value the global held at that site -- config/log/motd/early-input/files-from/
batch/connection/exclude/authenticate/params, change_dir's daemon dest-chdir,
and vfs_secure_mkstemp's --temp-dir all pass 0; secure_basis_open passes
VFS_OPERATOR_PATH only for the operator cases (a --partial-dir basis,
fnamecmp_type == FNAMECMP_PARTIAL_DIR; and one_inplace partial-dir staging).
This preserves the existing --temp-dir behaviour (unconfined) by deliberate
choice; hardening that is a separate decision.
The receiver partial-dir-basis and one_inplace operator_path_resolve blocks
are removed (receiver.c is now free of the global). Set-sites: 7 -> 5
(backup make_backup, generator 1071/2085/2130, util1 handle_partial_dir).
The operator-path-partial-dir-daemon test caught a real regression mid-change
(secure_basis_open's gate still read the global after its set/clear block was
removed, dropping the confinement); fixed by gating on is_operator.
Full suite 190/50.
Give vfs_rename_at an explicit vfs_flags argument (it keeps its two-parent
form, per the kept-explicit scope for rename) so its ownership-walk branch
reads VFS_OPERATOR_PATH instead of the vfs.operator_path_resolve global.
Call sites: the backup link/rename (make_backup) and the in-place/partial
rename (receiver) pass VFS_OPERATOR_PATH; the transfer renames (generator
entry rename, finish_transfer) and robust_rename pass 0; robust_unlink's
ETXTBSY rename-retry now forwards robust_unlink's own vfs_flags -- so
robust_unlink is fully explicit (both its unlink and its rename retry).
With unlink and rename both flagged, three operator_path_resolve set/clear
blocks that wrapped only a single now-flagged op are removed: the receiver
partial-dir rename and the two partial-dir unlinks (generator + receiver).
The remaining seven blocks still wrap not-yet-flagged ops (link, stat/lstat,
chmod/lchown via set_file_attrs, secure_basis_open) and stay for now.
Full suite 190/50; rename/partial/backup/inplace/daemon subset green.
Collapse vfs_unlink / vfs_unlink_at / vfs_unlink_atfd / vfs_rmdir /
vfs_rmdir_at into one vfs_unlink(int dirfd, const char *path, int flags).
VFS_REMOVEDIR selects rmdir/AT_REMOVEDIR; dirfd == VFS_AT_FDCWD resolves the
path, a real held dirfd removes a single validated component. The secure
body reads the operator context from VFS_OPERATOR_PATH instead of the
global; the rmdir path keeps its pre-existing no-owner-walk behaviour (it
never read the global -- matched the old vfs_rmdir_at).
robust_unlink() gains a vfs_flags argument threaded to vfs_unlink, so the
operator policy reaches it explicitly: the backup-tree callers (make_backup
link_or_rename, generator in-place backup) and copy_file's dest unlink (when
copying a backup) pass VFS_OPERATOR_PATH; delete and robust_rename's
transfer-context calls pass 0. Its ETXTBSY rename-retry still uses
vfs_rename_at (the rename primitive isn't flagged yet), which reads the
global -- still set by the surrounding backup blocks -- so behaviour is
unchanged.
Call sites classified: --remove-source-files fallback (sender) follows
symlinks (VFS_ALLOW_SYMLINK); partial-dir unlink/rmdir (handle_partial_dir)
and in-place-backup/partial unlinks operator; the rest secure or held-fd.
Full suite 190/50; delete/backup/partial/remove-source/daemon subset green.
Wiring phase: give vfs_open_at an explicit vfs_flags argument so the
operator-path resolution policy reaches it from the caller instead of the
vfs.operator_path_resolve global, and thread that argument through
copy_file()/unlink_and_reopen() to the dest create.
Call sites classified: the in-place backup open (generator) and the backup
copy (backup.c, generator in-place) pass VFS_OPERATOR_PATH; the copy-dest
basis copy (copy_altdest_file) passes 0 -- it must NOT use the ownership
walk (re-opening the copy_xattrs parent-symlink race, see generator.c
comment), which the explicit argument now expresses directly; robust_rename
runs in transfer context and passes 0.
copy_file's dest UNLINK (robust_unlink) still reads the operator global,
still set by the surrounding backup blocks, so behaviour is unchanged at
every site (flag and global agree). Full suite 190/50; backup/copy-dest/
link-dest/partial/daemon subset green.
These compiled test harnesses (t_*_secure, t_acl, simdtest) were
accidentally committed by a git add -A; ignore them like the other
built test programs (t_unsafe, wildtest, ...) so they stay out of the tree.
Relocate the file-copy / robust-unlink / robust-rename cluster from util1.c
into vfs/copy_file.c and vfs/robust.c. These are filesystem mechanics built
on the vfs_* open/read/write/unlink/rename primitives, so they belong in the
VFS compound layer; this is a verbatim move (no behaviour change).
The cluster is mutually coupled -- robust_rename falls back to copy_file for
the cross-filesystem (EXDEV) case, and copy_file's unlink_and_reopen uses
robust_unlink -- so all four move together. They call out to the metadata/
protocol layer for what isn't pure filesystem (copy_xattrs for the held-fd
xattr copy, handle_partial_dir for the EXDEV partial-dir path); those stay in
their current modules, keeping the dependency one-way (vfs -> nothing
above it pulled in). safe_read (a static helper only copy_file used) moves
with it.
Operator context still flows via the vfs.operator_path_resolve field for now
(legitimately VFS-internal once these callers live in vfs/); a later commit
threads it as an explicit vfs_flags argument and removes the field. The
t_*_secure harnesses don't reference the cluster, so no xattr/protocol
symbols are pulled into them. Full suite 190/50; copy-dest/backup/partial/
link-dest subset green.
First step of the VFS three-layer architecture: filesystem-mechanic helpers
that compose the vfs_* primitives belong inside vfs/, so the operator-path
resolution policy travels as an explicit argument instead of leaking across
the vfs<->mainline boundary as ambient state.
make_path() (recursive mkdir over vfs_mkdir/vfs_stat) moves verbatim from
util1.c to vfs/make_path.c and becomes vfs_make_path(fname, mkp_flags,
vfs_flags): mkp_flags keeps the path-handling bits (MKP_DROP_NAME/
MKP_SKIP_SLASH), vfs_flags carries the resolution policy (VFS_OPERATOR_PATH
for the operator-supplied --backup-dir tree, else 0). The transitional
MKP_OPERATOR flag added in the mkdir step is dropped in favour of the
explicit vfs_flags argument.
Callers: get_backup_name (backup dir) passes VFS_OPERATOR_PATH; the transfer
callers (main/receiver/generator) pass 0. Full suite 190/50.
Collapse vfs_symlink / vfs_symlink_at / vfs_symlink_atfd into one call.
dirfd == VFS_AT_FDCWD resolves `path` (secure receiver resolve by default,
plain follow under VFS_ALLOW_SYMLINK); a real held dirfd makes `path` a
single validated component. The old bodies become static helpers behind
the public dispatcher.
Unlike mkdir/mknod, the symlink secure path has NO ownership-walk branch --
it never read vfs.operator_path_resolve. That pre-existing asymmetry is
preserved: VFS_OPERATOR_PATH is accepted but resolves the same as the
default secure walk (only the parent dir is confined; the link target is
stored verbatim and never resolved at creation). The backup-symlink site
passes flags=0, which exactly reproduces the old vfs_symlink_at behavior.
Full suite 190/50.
Collapse vfs_mknod / vfs_mknod_at / vfs_mknod_atfd into one call, mirroring
vfs_mkdir. dirfd == VFS_AT_FDCWD resolves the path (secure receiver resolve
by default, ownership walk under VFS_OPERATOR_PATH, plain follow under
VFS_ALLOW_SYMLINK); a real held dirfd makes path a single validated
component. The old plain/secure/atfd bodies become static helpers
(vfs__mknod_plain/_secure/_atfd) behind the public dispatcher; the secure
body reads the operator context from VFS_OPERATOR_PATH instead of the
vfs.operator_path_resolve global.
Call sites: backup file node (make_backup_inner, operator) ->
VFS_OPERATOR_PATH; transfer device/fifo (generator) -> secure / held-fd.
The t_symlink_secure harness's plain PoC call becomes VFS_ALLOW_SYMLINK
(the vulnerable follow it demonstrates) and its secure calls flags=0.
Full suite 190/50.
Collapse vfs_mkdir / vfs_mkdir_at / vfs_mkdir_atfd into one call. dirfd ==
VFS_AT_FDCWD resolves the path argument; a real held dirfd makes path a
single component created directly under it (the held-fd form validates
that path is a lone harmless component -- rejects empty, any '/', "." and
".." -- so it cannot reintroduce path resolution under the pinned dir).
The plain-vs-secure choice is now an explicit per-call flag instead of a
function-name choice plus the vfs.operator_path_resolve global:
VFS_ALLOW_SYMLINK the call site asserts it is safe to follow symlinks
(the old plain vfs_mkdir); checked first, wins over
VFS_OPERATOR_PATH (no caller passes both)
VFS_OPERATOR_PATH operator-supplied path: ownership walk + module
confinement (the old global)
default 0 secure receiver resolve (the old vfs_mkdir_at)
VFS_AT_FDCWD is a VFS-owned sentinel (maps to AT_FDCWD where available,
else a value that routes the held-fd form to ENOSYS) so mainline code
never has to mention AT_FDCWD directly -- avoids a compile break on
platforms lacking it.
mkdir stops reading vfs.operator_path_resolve. vfs_owner_walk_parent now
takes the operator context as an is_operator parameter; its non-mkdir
callers (the other _at wrappers, generator hard-link) still pass the
global verbatim and convert in later steps. make_path() gains MKP_OPERATOR
(translated to VFS_OPERATOR_PATH) so the backup-dir creation stays an
operator path by path-semantics, not by relying on the global being set.
Call sites classified: dest dir (main.c) follows symlinks (user's local
dest); transfer dirs (generator, make_path transfer callers) secure;
backup-dir (backup.c, get_backup_name->make_path) and partial-dir
(handle_partial_dir) operator. The operator_path_resolve=1 blocks stay
for now since they still wrap non-mkdir ops; the global value and the
explicit flag agree at every converted mkdir site.
Full suite 190/50; backup/partial/daemon/exclude/filter/symlink-race
subset green.
First step of the API redesign. Define the per-call VFS_* flags
(VFS_ALLOW_SYMLINK / VFS_OPERATOR_PATH / VFS_REMOVEDIR) and stop
abspath_excluded_by_module() from reading the vfs.operator_path_resolve
global directly -- it now takes an explicit is_operator argument.
The strict resolver stays confined beneath its anchor (always within the
module), so its two call sites pass is_operator=0 (the check never fires
there anyway). The ownership walk is reached in BOTH operator context
(--backup-dir/--temp-dir/--partial-dir) and non-operator context (daemon
log-file/motd/config opens, which may legitimately live outside the
module), so vfs_open_owner_walk()/vfs_owner_walk_parent() capture the
global and pass it down through ona_open(). No behavior change -- the
value still comes from the global; later commits replace that source
with the explicit flag and delete the global.
Fold the served module root (module_dir / module_dirlen / module_dirfd)
into the vfs.module_* snapshot so the confinement checks read the VFS's
own state rather than implicit clientserver.c externs.
clientserver.c calls vfs_set_module_root() in two stages, matching when
the values become final: once right after the module path is settled
(before the daemon opens any operator path -- filter/include files, the
log file), with the root dirfd still -1, and again once that dirfd is
pinned by identity. The dirfd is borrowed (open_anchor_dirfd dup()s it);
the VFS never closes it. vfs_init() now clears the whole snapshot so a
forked connection can never inherit a stale module root.
The vfs/ readers (dirstack, secure_open, owner_walk) switch from the
externs to vfs.module_*; clientserver/flist/main keep their own globals.
Behavior is identical: between the two calls vfs.module_dirfd is -1, so
open_anchor_dirfd re-resolves the path exactly as before the pin.
Refresh the header comment now that the wrappers are vfs_* and syscall.c
is gone, and document the three operation forms callers choose between:
the plain path wrapper, the parent-resolved _at form (race-safe receiver
path), and the _atfd form (single component under a pinned dirfd).
Comment-only; no code change. Validated at protocol 30 and 29.
Every filesystem wrapper has moved into vfs/, so syscall.c held nothing
but its includes and an orphaned comment. Delete it and drop syscall.o
from the rsync link and from each test-harness object list (they reach
the vfs_* symbols through libvfs.a now). The S_BLKSIZE fallback define,
used by vfs/fileio.c, moves there. No behavior change.
The chmod family carries macOS setattrlist() and the Linux SYS_fchmodat2
raw-syscall fast path, but the move into vfs/chmod.c dropped the two
platform headers that syscall.c had included at file scope: <sys/attr.h>
(macOS) and <sys/syscall.h> (Linux). A Linux build hid this -- with
SYS_fchmodat2 undefined the code silently fell back to fchmodat() -- but
macOS failed to compile setattrlist(). Re-add both, matching the
original syscall.c includes.
Found by fleettest (mac2 BUILD-FAIL).
Relocate do_ftruncate/do_lseek/do_fallocate/do_punch_hole out of
syscall.c into vfs/fileio.c as the vfs_* names, declared in vfs/vfs.h.
The SUPPORT_PREALLOCATION / HAVE_FALLOCATE / HAVE_SYS_FALLOCATE /
FALLOC_FL_PUNCH_HOLE guards travel verbatim. This was the last
operation family: syscall.c no longer defines any filesystem wrapper.
No behavior change. (Portability-sensitive; wants a fleettest.)
Relocate the timestamp wrappers (do_utimensat/_at/_atfd, do_lutimes,
do_utimes, do_utime) and the crtime paths (do_setattrlist_times/_crtime,
get_create_time, do_SetFileTime) out of syscall.c into vfs/times.c as the
vfs_* names. The struct create_time / #pragma pack / Cygwin windows.h
and sys/attr.h includes travel with them, as do the SUPPORT_CRTIMES /
HAVE_SETATTRLIST / HAVE_GETATTRLIST / HAVE_UTIMENSAT / HAVE_LUTIMES /
HAVE_UTIMES / HAVE_UTIME guards. No behavior change. (Portability-
sensitive; wants a fleettest.)
Andrew Tridgell