From 3e715aa892b6828f35f69a0053152277c0818228 Mon Sep 17 00:00:00 2001 From: seks99x Date: Thu, 17 Sep 2026 13:43:31 +0300 Subject: [PATCH 1/2] Fix --files-from confinement for local and SSH transfers The recent path confinement patch caused an ELOOP error when local or SSH users tried to use a --files-from list located outside the confined root. This happened because the code treated the argument as if its always an operator-path peer. CLI arguments provided locally or over SSH are trusted, so the strict boundary check should only apply to untrusted clients connecting to a background daemon. --- options.c | 3 +- testsuite/files-from-leak_test.py | 24 +++++++++++++++ testsuite/rrsync-userns-procfs_test.py | 42 +++++++++++++++++--------- 3 files changed, 53 insertions(+), 16 deletions(-) diff --git a/options.c b/options.c index e222fa58a..3a2ce2bda 100644 --- a/options.c +++ b/options.c @@ -2659,7 +2659,8 @@ int parse_arguments(int *argc_p, const char ***argv_p) * module root -- e.g. a root-owned backup symlink. No-op off a * daemon (the module-root check only fires when am_daemon). */ int save_opr = operator_path_resolve; - operator_path_resolve = 1; + /* The daemon process is the only case that the files-from path comes from an untrusted argument */ + operator_path_resolve = am_daemon ? 1 : 0; filesfrom_fd = open_no_attacker_symlinks(files_from, O_RDONLY|O_BINARY, 0); operator_path_resolve = save_opr; if (filesfrom_fd < 0) { diff --git a/testsuite/files-from-leak_test.py b/testsuite/files-from-leak_test.py index e7a03725d..9badcb1d6 100644 --- a/testsuite/files-from-leak_test.py +++ b/testsuite/files-from-leak_test.py @@ -179,3 +179,27 @@ def root_owned_backup_symlink(): test_fail(f"setup failed: backup symlink {files_from} -> {tgt}, not the out-of-module secret") leak() + +# ---- LOCAL COMMAND CONFINEMENT EDGE CASE ----------------------------------- +# A local operator invoking --confine-root should still be able to specify a +# --files-from file that resides outside the confined boundary. The file is +# opened locally by the operator, not as an operator-path via a daemon, so +# it should not trip the path-walker ELOOP block. + +local_files_from = root / 'local_files_from.txt' +local_files_from.write_text("sub/f0\n") + +local_proc = subprocess.run( + rsync_argv('-a', f'--confine-root={base}', f'--files-from={local_files_from}', + f'{src}/', f'{dest}/'), + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, +) + +if "Too many levels of symbolic links" in local_proc.stdout or "failed to open files-from file" in local_proc.stdout: + test_fail( + "Local process improperly rejected an out-of-bounds --files-from file " + "specified by the operator due to an overly strict path-walker.\n" + f"Output: {local_proc.stdout}" + ) +elif local_proc.returncode != 0: + test_fail(f"Local --confine-root run failed unexpectedly (rc={local_proc.returncode}).\nOutput: {local_proc.stdout}") diff --git a/testsuite/rrsync-userns-procfs_test.py b/testsuite/rrsync-userns-procfs_test.py index fa2aaf3dc..7bd1cdb06 100644 --- a/testsuite/rrsync-userns-procfs_test.py +++ b/testsuite/rrsync-userns-procfs_test.py @@ -79,8 +79,9 @@ test_skipped('/proc/self does not expose an overflow uid in this namespace') base = Path(tempfile.mkdtemp(prefix='rsync-userns-procfs-')) -src = base / 'src' -dest = base / 'dest' +root = base / 'root' +src = root / 'src' +dest = root / 'dest' outside = base / 'outside' makepath(src, dest, outside) (src / 'file').write_text('content\n') @@ -112,22 +113,33 @@ finally: os.close(dest_fd) -outside_list = outside / 'files-from' -outside_list.write_text('file\n') -for fd_root in fd_roots: - outside_fd = os.open(outside_list, os.O_RDONLY) - try: +backup_dir = root / 'backup' +backup_dir.symlink_to(outside) +(dest / 'test').write_text('old_content\n') +(src / 'test').write_text('new_different_content\n') + +(outside / 'test').write_text('existing_backup\n') + +# Open an FD pointing to the confined root +root_fd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) +try: + for fd_root in fd_roots: proc = subprocess.run( - rsync_argv('-a', f'--confine-root={dest}', - f'--files-from={fd_root}/{outside_fd}', + rsync_argv('-a', '--backup', f'--confine-root={root}', + f'--backup-dir={fd_root}/{root_fd}/backup/', str(src) + '/', str(dest) + '/'), - pass_fds=(outside_fd,), + pass_fds=(root_fd,), capture_output=True, text=True, ) - finally: - os.close(outside_fd) - if proc.returncode == 0 or 'failed to open files-from file' not in proc.stderr: - test_fail(f'outside {fd_root} pin was not observably refused: ' - f'rc={proc.returncode}, stderr={proc.stderr!r}') + + # The transfer must fail because rsync attempts to unlink the pre-existing target file, + # forcing the path-walker to evaluate the FD pin + symlink and catching the escape. + if proc.returncode == 0: + test_fail(f'backup to outside symlink via {fd_root} pin was not observably refused: ' + f'rc={proc.returncode}, stderr={proc.stderr!r}') +finally: + os.close(root_fd) + rmtree(base) + From 795d5962af89de7e1191336e8b44f321e06ed8da Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sat, 19 Sep 2026 14:33:10 +1000 Subject: [PATCH 2/2] tests: assert local files-from copy --- testsuite/files-from-leak_test.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/testsuite/files-from-leak_test.py b/testsuite/files-from-leak_test.py index 9badcb1d6..b310860e4 100644 --- a/testsuite/files-from-leak_test.py +++ b/testsuite/files-from-leak_test.py @@ -187,7 +187,9 @@ def root_owned_backup_symlink(): # it should not trip the path-walker ELOOP block. local_files_from = root / 'local_files_from.txt' -local_files_from.write_text("sub/f0\n") +local_file = src / 'sub' / 'files-from-local' +local_file.write_text('local files-from content\n') +local_files_from.write_text('sub/files-from-local\n') local_proc = subprocess.run( rsync_argv('-a', f'--confine-root={base}', f'--files-from={local_files_from}', @@ -195,11 +197,11 @@ def root_owned_backup_symlink(): stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, ) -if "Too many levels of symbolic links" in local_proc.stdout or "failed to open files-from file" in local_proc.stdout: +copied = dest / 'sub' / 'files-from-local' +if (local_proc.returncode != 0 or not copied.is_file() + or copied.read_text() != 'local files-from content\n'): test_fail( - "Local process improperly rejected an out-of-bounds --files-from file " - "specified by the operator due to an overly strict path-walker.\n" + "Local --files-from transfer did not copy the listed file while the " + "list was outside --confine-root.\n" f"Output: {local_proc.stdout}" ) -elif local_proc.returncode != 0: - test_fail(f"Local --confine-root run failed unexpectedly (rc={local_proc.returncode}).\nOutput: {local_proc.stdout}")