From 893e88ade185cb8e36b7dbebb9aed7d7cef9e7b4 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 3 Aug 2026 06:48:38 +1000 Subject: [PATCH] testsuite: cover the backup-dir ownership set under a parent swap 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. --- testsuite/operator-path-backup-chown_test.py | 184 +++++++++++++++++++ testsuite/skiplist/cygwin.txt | 1 + testsuite/skiplist/macos.txt | 1 + 3 files changed, 186 insertions(+) create mode 100644 testsuite/operator-path-backup-chown_test.py diff --git a/testsuite/operator-path-backup-chown_test.py b/testsuite/operator-path-backup-chown_test.py new file mode 100644 index 00000000..1ff1d937 --- /dev/null +++ b/testsuite/operator-path-backup-chown_test.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +# --backup-dir parent-component symlink-race confinement for the OWNERSHIP set, +# not the create. operator-path-backup-symlink covers the create side (a backup +# symlink must not be written outside the backup tree); this covers what +# set_file_attrs() does to the item afterwards. +# +# make_backup() recreates the item at the backup name and then calls +# set_file_attrs(buf, ..., ATTRS_OPERATOR_PATH). A regular/dir/fifo leaf is +# pinned by op_pin and its metadata driven off that fd, but a SYMLINK leaf never +# enters op_pin (there is no O_NOFOLLOW open of a symlink), so the chown falls +# through to the path-based wrapper on the full operator path. Unless that +# wrapper resolves through the ownership walk, a parent component flipped to an +# attacker-owned symlink redirects the lchown out of the backup tree and retags +# a victim inode as the attacker's -- an ownership-transfer primitive, and the +# trust laundering that then defeats the walk on any later pass. +# +# Reaching the recreate path at all needs the backup dir on ANOTHER filesystem: +# on one filesystem make_backup() hard-links or renames the item across and +# never calls set_file_attrs(). So the whole fixture lives on tmpfs. +# +# A statically planted symlink is not enough either -- rsync's own backup-dir +# validation deletes a non-directory component before using it -- so the plant +# has to be a live flip, as in the sibling test. + +import os +import subprocess +import time + +from rsyncfns import ( + SCRATCHDIR, race_budget, find_attacker_uid, rmtree, makepath, + start_c_flipper, stop_flipper, test_fail, test_skipped, +) + +if os.geteuid() != 0: + test_skipped("requires root to own a symlink by a foreign uid and to chown backups") + +ATT_UID = find_attacker_uid() +if ATT_UID is None: + test_skipped("no untrusted-uid user available for cross-uid plant") + +# The backup dir must be on a different st_dev from the destination, or +# make_backup() renames into it and the set_file_attrs() path never runs. +dest_dev = os.stat(SCRATCHDIR).st_dev +TMPFS = None +for cand in ('/dev/shm', '/run/shm', os.environ.get('TMPDIR', '/tmp')): + try: + if os.stat(cand).st_dev != dest_dev and os.access(cand, os.W_OK): + TMPFS = cand + break + except OSError: + continue +if TMPFS is None: + test_skipped("no writable cross-device dir (tmpfs) for the --backup-dir EXDEV path") + +# Many files widen the per-file backup window so the flipper has more chances to +# land the swap between the recreate and the chown. +NFILES = 95 + +base = SCRATCHDIR / 'bdir-chown-race' +src = base / 'src' +dest = base / 'dest' + +bakroot = os.path.join(TMPFS, 'rsync-bakchown-race') +backup = os.path.join(bakroot, 'backup') +outside = os.path.join(bakroot, 'outside') +sub = os.path.join(backup, 'sub') +sublink = os.path.join(backup, '.sublink') + + +def build(): + """Reset the workspace. Call only while the flipper is stopped.""" + rmtree(base) + subprocess.run(['rm', '-rf', bakroot], check=False) + makepath(src / 'sub', dest / 'sub') + os.makedirs(outside, exist_ok=True) + os.makedirs(backup, exist_ok=True) + + # Distinct source and destination symlink values so each transfer replaces + # the destination symlink and thus backs the old one up. The destination + # symlinks are attacker-owned, so restoring their ownership onto the backup + # copy REQUIRES an lchown -- without that there is no chown to redirect and + # the test would pass vacuously. + for i in range(NFILES): + (src / 'sub' / f'f{i}').symlink_to('test') + d = dest / 'sub' / f'f{i}' + d.symlink_to('test2') + os.lchown(d, ATT_UID, ATT_UID) + + # Victims: root-owned regular files carrying the names the backup would use + # if a flipped `sub` redirected the operator path into outside/. + for i in range(NFILES): + v = os.path.join(outside, f'f{i}') + with open(v, 'w') as fh: + fh.write('victim\n') + os.chown(v, 0, 0) + + # The attacker-owned parent-swap target. + os.symlink(outside, sublink) + os.lchown(sublink, ATT_UID, ATT_UID) + os.makedirs(sub, exist_ok=True) + + +def push(): + """Blocking local rsync push that backs up the old destination symlinks.""" + return subprocess.run( + ['./rsync', '-a', '-b', f'--backup-dir={backup}', f'{src}/', f'{dest}/'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, + ) + + +def retagged(): + """Name of a victim in outside/ that stopped being root-owned, or ''. + + A swap killed mid-rename can leave outside/ momentarily odd; anything we + cannot stat is simply not evidence of a win.""" + try: + with os.scandir(outside) as it: + for e in it: + try: + st = e.stat(follow_symlinks=False) + except OSError: + continue + if st.st_uid != 0 or st.st_gid != 0: + return e.name + except (FileNotFoundError, NotADirectoryError): + return '' + return '' + + +# ---- POSITIVE CONTROL ------------------------------------------------------ +# A clean run must (a) back the old destination symlink into the backup tree via +# the cross-device recreate path and (b) carry the attacker ownership onto that +# backup copy -- which is the lchown this test is about. Without both, the race +# below would be asserting on a code path that never executes. +build() +proc = push() +if proc.returncode != 0: + test_fail(f"positive control: clean --backup-dir run failed (rc={proc.returncode}):\n{proc.stdout or ''}") + +bak0 = os.path.join(sub, 'f0') +if not os.path.islink(bak0) or os.readlink(bak0) != 'test2': + test_fail(f"positive control: the old destination symlink was not backed up into {sub}; " + "the cross-device recreate path was not exercised") +st = os.lstat(bak0) +if st.st_uid != ATT_UID: + test_fail(f"positive control: backup copy {bak0} is uid {st.st_uid}, expected the " + f"attacker uid {ATT_UID}; set_file_attrs() did not lchown the backup, so " + "this test would pass vacuously") +if retagged(): + test_fail("positive control: a victim in outside/ changed ownership during a no-flipper run") + + +# ---- THE LIVE RACE --------------------------------------------------------- +# Flip backup/sub between the real backup directory and the attacker-owned +# symlink to outside/ under a live transfer. The ownership walk must refuse the +# foreign-owned component, so no victim in outside/ is ever retagged. + +deadline = time.monotonic() + race_budget(10.0) +flip = None +try: + while time.monotonic() < deadline: + # Reset only while the flipper is quiet, so build()'s rmtree/mkdir + # cannot race the swapper and drop artifacts in outside/. + if flip is not None: + stop_flipper(flip) + flip = None + build() + flip = start_c_flipper(sub, sublink) + push() + + victim = retagged() + if victim: + test_fail( + "--backup-dir parent symlink race: victim " + f"{os.path.join(outside, victim)} was retagged away from root; rsync " + "chowned through the flipped attacker-owned backup/sub component " + "instead of refusing it." + ) +finally: + if flip is not None: + stop_flipper(flip) + subprocess.run(['rm', '-rf', bakroot], check=False) + +print("operator-path-backup-chown: backup ownership confined under parent-swap race") diff --git a/testsuite/skiplist/cygwin.txt b/testsuite/skiplist/cygwin.txt index ba499cc5..47f9992a 100644 --- a/testsuite/skiplist/cygwin.txt +++ b/testsuite/skiplist/cygwin.txt @@ -45,6 +45,7 @@ msg-io-timeout-overflow nondaemon-symlink-race nonroot-restrictive-perms open-noatime +operator-path-backup-chown operator-path-backup-rmdir operator-path-backup-symlink operator-path-insecure-links-daemon diff --git a/testsuite/skiplist/macos.txt b/testsuite/skiplist/macos.txt index 3e7653ce..90eb4e59 100644 --- a/testsuite/skiplist/macos.txt +++ b/testsuite/skiplist/macos.txt @@ -19,6 +19,7 @@ dir-sgid fake-super-acl-xattr link-dest-symlink-enotsup # the ENOTSUP hard-link hook is an LD_PRELOAD, Linux-only open-noatime +operator-path-backup-chown partial-protected-regular-retry-linux preallocate protected-regular