diff --git a/receiver.c b/receiver.c index 7d429fe84..d8138d12d 100644 --- a/receiver.c +++ b/receiver.c @@ -99,6 +99,31 @@ static int updating_basis_or_equiv; * Anything else is a straight pass-through that preserves the strict contract. */ static int secure_basis_open(const char *basedir, const char *relpath, int flags, mode_t mode) { + extern int am_daemon, am_chrooted; + + /* Confinement via secure_relative_open() is only required for the + * sanitizing daemon (am_daemon && !am_chrooted) -- exactly the gate the + * do_*_at() wrappers in syscall.c apply. In local / remote-shell mode + * there is no module boundary to enforce, and under "use chroot = yes" + * the kernel root is the module, so the chroot is its own boundary + * (CVE-2026-29518 was specific to chroot-disabled daemons). In both + * cases an alt-dest basis such as --link-dest=../01 must resolve against + * the CWD the way a bare open does (and did before the CVE hardening); + * confining it here would reject the legitimate sibling ".." and strand a + * delta'd file with "block match with no basis file". Restrict the secure + * path to the one context that needs it. */ + if (!am_daemon || am_chrooted) { + if (basedir) { + char fullpath[MAXPATHLEN]; + if (pathjoin(fullpath, sizeof fullpath, basedir, relpath) >= sizeof fullpath) { + errno = ENAMETOOLONG; + return -1; + } + return do_open(fullpath, flags, mode); + } + return do_open(relpath, flags, mode); + } + if (!basedir && relpath && *relpath == '/') { const char *slash = strrchr(relpath, '/'); const char *leaf = slash + 1; @@ -859,7 +884,7 @@ int recv_files(int f_in, int f_out, char *local_name) basedir = basis_dir[0]; fnamecmp = fname; fnamecmp_type = FNAMECMP_BASIS_DIR_LOW; - fd1 = secure_relative_open(basedir, fnamecmp, O_RDONLY, 0); + fd1 = secure_basis_open(basedir, fnamecmp, O_RDONLY, 0); } } diff --git a/syscall.c b/syscall.c index 0748d9988..f1bfa37b4 100644 --- a/syscall.c +++ b/syscall.c @@ -1782,27 +1782,100 @@ static int secure_relative_open_resolve_beneath(const char *basedir, const char int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode) { + extern int am_daemon, am_chrooted; + extern char *module_dir; + extern unsigned int module_dirlen; + extern char curr_dir[MAXPATHLEN]; + extern unsigned int curr_dir_len; + char modrel_buf[MAXPATHLEN]; + int reanchored = 0; + if (!relpath || relpath[0] == '/') { // must be a relative path errno = EINVAL; return -1; } - /* Reject any path with a literal ".." component (bare "..", - * "../foo", "foo/..", "foo/../bar", "subdir/.."). The previous - * substring-based check caught only "../" prefix and "/../" - * substring; bare ".." and trailing "/.." escape on the per- - * component walk fallback used by NetBSD/OpenBSD/Solaris/Cygwin - * and pre-5.6 Linux. RESOLVE_BENEATH on Linux/FreeBSD/macOS - * catches some of these in-kernel with EXDEV, but the front - * door must reject them consistently with EINVAL across all - * platforms so callers can rely on the validation. */ - if (path_has_dotdot_component(relpath)) { - errno = EINVAL; - return -1; + + /* Sanitizing daemon (am_daemon && !am_chrooted) only: this is the + * sole context in which the confinement here is reached for an + * attacker-influenced path -- the do_*_at() wrappers fall back to a + * bare syscall for the local / remote-shell / chrooted cases, and a + * chroot is its own module boundary. + * + * In this context we have chdir'd into a sub-directory of the module + * (the transfer destination), so the current directory is NOT the + * module root. A relative path such as the alt-dest "../01" is + * resolved against the destination and may legitimately climb back out + * to a sibling that is still inside the module (e.g. .../backup/00 with + * --link-dest=../01 -> .../backup/01). Anchoring RESOLVE_BENEATH at + * the current directory would reject that climb with EXDEV, silently + * disabling hard-linking and re-transferring everything (issue #915). + * + * Re-anchor at the module root -- the actual trust boundary -- by + * prefixing the path of the current directory within the module, taken + * from rsync's own curr_dir[] (the logical CWD that change_dir() + * maintains). curr_dir is built by the same normalize_path()/clean_fname() + * machinery as module_dir, so module_dir is a guaranteed lexical prefix of + * curr_dir -- which getcwd() would NOT be, since getcwd() returns the + * physical path with every symlink resolved while module_dir is only + * lexically normalized (a module whose configured path traverses a symlink + * would make the two disagree and silently skip the re-anchor). The + * resulting "/" is resolved beneath the real + * module_dir fd: RESOLVE_BENEATH follows the actual on-disk symlinks at + * runtime, permitting any climb that stays inside the module while still + * rejecting one that escapes it (../../etc) and any symlink whose target + * lies outside the module. module_dir is the absolute, operator-configured + * module path, so the dispatch below opens it on its trusted + * absolute-basedir branch. Limited to paths that actually contain a + * ".." component; forward paths resolve correctly against the CWD and + * stay on the unchanged code path. */ + if (am_daemon && !am_chrooted + && module_dirlen && module_dir && module_dir[0] == '/' + && (basedir == NULL || basedir[0] != '/') + && (path_has_dotdot_component(relpath) + || (basedir && path_has_dotdot_component(basedir)))) { + const char *p; + int n; + if (curr_dir_len >= module_dirlen + && strncmp(curr_dir, module_dir, module_dirlen) == 0 + && (curr_dir[module_dirlen] == '\0' || curr_dir[module_dirlen] == '/')) { + for (p = curr_dir + module_dirlen; *p == '/'; p++) {} /* CWD relative to module, no leading slash */ + if (basedir) + n = snprintf(modrel_buf, sizeof modrel_buf, "%s%s%s/%s", + p, *p ? "/" : "", basedir, relpath); + else + n = snprintf(modrel_buf, sizeof modrel_buf, "%s%s%s", + p, *p ? "/" : "", relpath); + if (n < 0 || n >= (int)sizeof modrel_buf) { + errno = ENAMETOOLONG; + return -1; + } + basedir = module_dir; /* absolute, operator-trusted anchor */ + relpath = modrel_buf; + reanchored = 1; + } + /* else: CWD not under the module root as expected -- leave the + * path unchanged and let the front-door rejection below handle + * it (fail safe). */ } - if (basedir && basedir[0] != '/' && path_has_dotdot_component(basedir)) { - errno = EINVAL; - return -1; + + /* Reject any path with a literal ".." component (bare "..", "../foo", + * "foo/..", "foo/../bar", "subdir/.."). RESOLVE_BENEATH on + * Linux/FreeBSD/macOS catches escaping ".." in-kernel, but the front + * door rejects them consistently with EINVAL across all platforms so + * callers can rely on the validation. Skipped for a re-anchored path: + * its ".." is deliberate, stays within the module, and is adjudicated + * by RESOLVE_BENEATH in the dispatch below (the portable fallback + * re-checks and rejects it, see there). */ + if (!reanchored) { + if (path_has_dotdot_component(relpath)) { + errno = EINVAL; + return -1; + } + if (basedir && basedir[0] != '/' && path_has_dotdot_component(basedir)) { + errno = EINVAL; + return -1; + } } #if defined(__linux__) && defined(HAVE_OPENAT2) @@ -1821,6 +1894,27 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo return secure_relative_open_resolve_beneath(basedir, relpath, flags, mode); #endif + /* Portable fallback only: no kernel RESOLVE_BENEATH is available, so the + * per-component O_NOFOLLOW walk below cannot adjudicate ".." safely (it + * would have to open a real ".." and could thereby leave the anchor). + * On the kernel paths above the ".." handling is enforced by + * RESOLVE_BENEATH (escapes rejected with EXDEV, in-module climbs + * allowed), so this rejection is intentionally confined to the fallback. + * Reject any literal ".." component (bare "..", "../foo", "foo/..", + * "foo/../bar", "subdir/.."). Note: this re-breaks --link-dest=../01 + * on platforms with neither openat2 nor O_RESOLVE_BENEATH + * (NetBSD/OpenBSD/Solaris/Cygwin/pre-5.6 Linux), trading function for + * safety there; those are the same platforms that lacked within-tree + * symlink support in the original fix. */ + if (path_has_dotdot_component(relpath)) { + errno = EINVAL; + return -1; + } + if (basedir && basedir[0] != '/' && path_has_dotdot_component(basedir)) { + errno = EINVAL; + return -1; + } + #if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD) // really old system, all we can do is live with the risks if (!basedir) { diff --git a/t_stub.c b/t_stub.c index d6c4c133e..4d318d760 100644 --- a/t_stub.c +++ b/t_stub.c @@ -45,6 +45,13 @@ size_t max_alloc = (size_t)-1; /* test helpers are not memory-constrained; * hits at its first my_strdup() call. */ char *partial_dir; char *module_dir; +/* secure_relative_open() reads curr_dir[]/curr_dir_len to re-anchor an + * alt-dest ".." climb at the module root. The real definitions live in + * util1.c; declare these weak so the helpers that link util1.o (t_unsafe, + * t_chmod_secure, t_secure_relpath) use the real ones without a duplicate- + * symbol clash, while the helpers that don't (tls, trimslash) still link. */ +char curr_dir[MAXPATHLEN] __attribute__((weak)); +unsigned int curr_dir_len __attribute__((weak)) = 0; filter_rule_list daemon_filter_list; void rprintf(UNUSED(enum logcode code), const char *format, ...) diff --git a/testsuite/link-dest-chroot-delta_test.py b/testsuite/link-dest-chroot-delta_test.py new file mode 100644 index 000000000..9cf414a4b --- /dev/null +++ b/testsuite/link-dest-chroot-delta_test.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +# Regression test for the chroot-daemon facet of the issue #915 / +# CVE-2026-29518 fallout. +# +# The CVE symlink-race hardening routed the receiver's alt-dest basis +# open (secure_basis_open in receiver.c) through secure_relative_open(), +# whose front door rejects a relative ".." basedir. That broke the +# DELTA path for --link-dest=../01 (a sibling backup) under a +# "use chroot = yes" daemon -- the default deployment -- as well as the +# no-chroot daemon (issue #915) and local --no-whole-file transfers. +# +# Under "use chroot = yes" the per-module chroot makes the module the +# filesystem root, module_dirlen is 0, and the alt-dest reaches the +# receiver as a literal "../01"; with am_chrooted=1 the receiver chdir's +# into the destination "/00" and resolves "../01" -> "/01" (still inside +# the chroot). The unconditional confinement rejected that ".." and a +# changed file delta'd against the basis aborted the whole transfer with +# "got a block match with no basis file". rsync 3.4.1 (pre-hardening) +# handled it. +# +# The fix gives secure_basis_open() the same gate the do_*_at() wrappers +# already use: in a chroot (am_chrooted) the kernel root is the module +# boundary, so the basis is opened with a bare do_open() that resolves +# against the CWD -- exactly as before the hardening. +# +# This is the only test that exercises the am_chrooted code path. It +# needs CAP_SYS_CHROOT *and* a privilege drop that can call setgroups() +# (the use-chroot=yes daemon drops to its module uid/gid). Real root has +# both; an unprivileged user namespace needs a *range* gid mapping +# (--map-auto) for setgroups to be permitted -- plain --map-root-user is +# not enough. We re-exec under unshare when we can't chroot directly, +# and skip if neither is available (matching daemon-chroot-acl_test.py +# and testsuite/COVERAGE.md's "use chroot = yes ... needs root"). + +import os +import platform +import shutil +import subprocess +import sys + +from rsyncfns import ( + SCRATCHDIR, + rsync_argv, rmtree, start_test_daemon, test_fail, test_skipped, +) + + +if platform.system() != 'Linux': + test_skipped("test is Linux-specific (uses chroot+unshare)") + + +def _can_chroot() -> bool: + proc = subprocess.run(['chroot', '/', '/bin/true'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + return proc.returncode == 0 + + +if not _can_chroot(): + if not os.environ.get('RSYNC_UNSHARED'): + unshare = shutil.which('unshare') + if unshare is not None: + # --map-auto adds the user's subuid/subgid ranges, which is what + # lets the daemon's setgroups() during privilege drop succeed. + argv = [unshare, '--user', '--map-root-user', '--map-auto'] + if subprocess.run(argv + ['true'], capture_output=True).returncode == 0: + print("Re-running under unshare --user --map-root-user --map-auto...") + env = os.environ.copy() + env['RSYNC_UNSHARED'] = '1' + os.execvpe(unshare, argv + [sys.executable, __file__], env) + test_skipped("need CAP_SYS_CHROOT + setgroups (real root, or " + "unshare --user --map-root-user --map-auto)") + + +DAEMON_PORT = 12916 + +mod = SCRATCHDIR / 'module' +conf = SCRATCHDIR / 'test-rsyncd.conf' +src = SCRATCHDIR / 'src_files' + +for d in (mod, src): + rmtree(d) + d.mkdir(parents=True) +(mod / '01').mkdir() # the sibling "previous backup" +(mod / '00').mkdir() # the destination subdir + +# Large files differing by one byte, with DISTINCT mtimes so the quick-check +# does NOT treat them as identical (which would hard-link instead of forcing +# the delta/basis-open path this test is about). +basis = mod / '01' / 'f' +source = src / 'f' +basis.write_text('A' * 200000 + 'X') +source.write_text('A' * 200000 + 'Y') +os.utime(basis, (1_000_000_000, 1_000_000_000)) # 2001 +os.utime(source, (1_700_000_000, 1_700_000_000)) # 2023 + +# We only reach here as (real or namespace) root, so the daemon can chroot +# and drop to uid/gid 0. +conf.write_text(f"""\ +use chroot = yes +uid = 0 +gid = 0 +log file = {SCRATCHDIR}/rsyncd.log +[bak] + path = {mod} + use chroot = yes + read only = no +""") + +url = start_test_daemon(conf, DAEMON_PORT) + +# After the per-module chroot the module root is "/", so the destination is +# "/00" and the sibling basis "--link-dest=../01" -> "/01", inside the chroot. +proc = subprocess.run( + rsync_argv('-rt', '--no-whole-file', '--link-dest=../01', + f'{src}/', f'{url}bak/00/'), + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True, +) +if proc.returncode != 0: + test_fail( + "use-chroot=yes --link-dest=../01 delta aborted (rc=" + f"{proc.returncode}): the receiver could not open the sibling basis " + "inside the chroot for the delta transfer. stderr:\n{}".format(proc.stderr)) + +dest = mod / '00' / 'f' +if not dest.is_file(): + test_fail("use-chroot=yes --link-dest=../01 delta produced no destination file") +if dest.read_text() != source.read_text(): + test_fail( + "use-chroot=yes --link-dest=../01 delta reconstructed the wrong " + "content: the destination does not match the source after a delta " + "against the sibling basis inside the chroot") diff --git a/testsuite/link-dest-daemon_test.py b/testsuite/link-dest-daemon_test.py new file mode 100755 index 000000000..36808357f --- /dev/null +++ b/testsuite/link-dest-daemon_test.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +# Regression test for issue #915. +# +# After the CVE-2026-29518 / CVE-2026-43619 symlink-race hardening, a +# "use chroot = no" daemon stopped honouring --link-dest=../01 -- the +# standard rotating-backup layout, where the previous backup is a sibling +# of the destination subdir (push into .../00 with --link-dest=../01, +# pointing at .../01). The receiver chdir's into the destination subdir, +# and secure_relative_open()'s confinement (RESOLVE_BENEATH anchored +# there) refused the ".." climb even though ../01 is still inside the +# module. No basis was found, every file was re-transferred instead of +# hard-linked, and backup disks silently filled up. +# +# The fix re-anchors the confinement at the module root, so an in-module +# ".." climb resolves while an escape out of the module is still rejected. +# +# Two cases are exercised: +# 1. functional -- --link-dest=../01 into a destination subdir must +# hard-link the unchanged file; +# 2. security -- when the sibling basis is a symlink pointing OUT of +# the module, the basis-file lookup must NOT follow it out (the +# runtime-symlink TOCTOU the CVE is about; this complements +# alt-dest-symlink-race_test.py, which covers pushing into the +# module root rather than a subdir). + +import os +import subprocess + +from rsyncfns import ( + SCRATCHDIR, + rsync_argv, get_testuid, get_rootuid, get_rootgid, + rmtree, start_test_daemon, test_fail, +) + + +DAEMON_PORT = 12915 + + +mod = SCRATCHDIR / 'module' +outside = SCRATCHDIR / 'outside' +src_files = SCRATCHDIR / "src_files" +conf = SCRATCHDIR / 'test-rsyncd.conf' + +for d in (mod, outside, src_files): + rmtree(d) + d.mkdir(parents=True) + +# The single source file used for every case. +(src_files / 'f').write_text("hello world\n") +os.chmod(src_files / 'f', 0o644) +src_ref = (src_files / 'f').stat() + + +def seed_basis(path): + """Create a previous-backup copy identical in content, mtime and mode to + src_files/f, so that an exact --link-dest match hard-links the destination to + it (and, for the escape case, would hard-link to the outside file).""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("hello world\n") + os.chmod(path, 0o644) + os.utime(path, (src_ref.st_atime, src_ref.st_mtime)) + + +# Case 1: module/00 (destination) and module/01 (real sibling basis). +(mod / '00').mkdir() +seed_basis(mod / '01' / 'f') + +# Case 2: module/sym00 (destination) and module/sym01 -> outside, a symlink +# escaping the module. outside/f matches src_files exactly, so a followed symlink +# would hard-link the destination to it (and be detected). +(mod / 'sym00').mkdir() +seed_basis(outside / 'f') +os.symlink(str(outside), mod / 'sym01') + +my_uid = get_testuid() +root_uid = get_rootuid() +root_gid = get_rootgid() +uid_line = f"uid = {root_uid}" +gid_line = f"gid = {root_gid}" +if my_uid != root_uid: + uid_line = '#' + uid_line + gid_line = '#' + gid_line + +conf.write_text(f"""\ +use chroot = no +{uid_line} +{gid_line} +log file = {SCRATCHDIR}/rsyncd.log +[bak] + path = {mod} + use chroot = no + read only = no +""") + +url = start_test_daemon(conf, DAEMON_PORT) + + +def push(link_dest, dest_subdir): + subprocess.run( + rsync_argv('-rtp', f'--link-dest={link_dest}', + f'{src_files}/', f'{url}bak/{dest_subdir}/'), + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ) + + +# 1. Functional: the in-module ".." climb to a sibling backup must hard-link. +push('../01', '00') +dest = mod / '00' / 'f' +if not dest.is_file(): + test_fail("daemon transfer failed before the test could observe --link-dest") +if dest.stat().st_ino != (mod / '01' / 'f').stat().st_ino: + test_fail( + "issue #915: --link-dest=../01 via a use-chroot=no daemon did not " + "hard-link the unchanged file (it was re-transferred); the in-module " + "'..' climb to the sibling backup was wrongly rejected by the " + "basis-file confinement") + +# 2. Security: the sibling is a symlink out of the module; the basis-file +# lookup must not follow it out (must not hard-link to the outside file). +push('../sym01', 'sym00') +sdest = mod / 'sym00' / 'f' +if sdest.is_file() and sdest.stat().st_ino == (outside / 'f').stat().st_ino: + test_fail( + "basedir-escape: --link-dest=../sym01 hard-linked the destination to " + f"outside/f (inode {sdest.stat().st_ino}); the basis-file lookup " + "followed a symlink out of the module") diff --git a/testsuite/link-dest-local-delta_test.py b/testsuite/link-dest-local-delta_test.py new file mode 100644 index 000000000..22f4cea85 --- /dev/null +++ b/testsuite/link-dest-local-delta_test.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +# Regression test for the wider half of the issue #915 / CVE-2026-29518 +# fallout: the receiver's alt-dest basis open (secure_basis_open in +# receiver.c) was routed unconditionally through secure_relative_open(), +# whose front door rejects a relative ".." basedir. That broke the +# DELTA path for --link-dest=../01 (a sibling backup) in every context +# where the basis is opened for reading, not just the no-chroot daemon +# that issue #915 reported: +# +# * local / remote-shell transfers with --no-whole-file (delta on), and +# * "use chroot = yes" daemons (the default), where the chroot is the +# boundary and module_dirlen is 0 so the path reaches the receiver as +# a literal "../01". +# +# Symptom was not a graceful fallback but a hard abort: +# "got a block match with no basis file ... protocol incompatibility". +# The sender deltas the changed file against the basis, the receiver +# can't open the basis, and receive_data() dies. +# +# The fix gives secure_basis_open() the same gate the do_*_at() wrappers +# use (bare open unless am_daemon && !am_chrooted), so the basis resolves +# against the CWD exactly as a pre-CVE bare open did. Pre-CVE 3.4.1 +# worked here; this guards the repair. +# +# This test covers the LOCAL --no-whole-file path (no root needed). The +# chroot-daemon path is the same code with am_chrooted=1; it needs real +# root to chroot() and so isn't exercised here (the no-chroot daemon is +# covered by link-dest-daemon_test.py). + +import os +import subprocess + +from rsyncfns import SCRATCHDIR, rsync_argv, rmtree, test_fail + + +work = SCRATCHDIR / 'lddelta' +rmtree(work) +(work / '01').mkdir(parents=True) # the sibling "previous backup" +(work / '00').mkdir(parents=True) # the destination subdir +(work / 'src').mkdir(parents=True) # the source + +# A large file so rsync genuinely deltas (most blocks match the basis, +# one byte differs). Distinct mtimes so the quick-check does NOT treat +# them as identical -- that would hard-link instead of exercising the +# delta/basis-open path we care about. +basis = work / '01' / 'f' +src = work / 'src' / 'f' +basis.write_text('A' * 200000 + 'X') +src.write_text('A' * 200000 + 'Y') +os.utime(basis, (1_000_000_000, 1_000_000_000)) # 2001 +os.utime(src, (1_700_000_000, 1_700_000_000)) # 2023 + +# Run from `work` so the dest "00/" and the relative "--link-dest=../01" +# resolve as siblings -- the rotating-backup layout. --no-whole-file +# forces the delta transfer (local transfers default to whole-file). +proc = subprocess.run( + rsync_argv('-rt', '--no-whole-file', '--link-dest=../01', 'src/', '00/'), + cwd=str(work), stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True, +) +if proc.returncode != 0: + test_fail( + "local --link-dest=../01 delta aborted (rc=" + f"{proc.returncode}): the receiver could not open the sibling basis " + "for the delta transfer -- secure_basis_open wrongly confined a " + f"relative '..' basis outside the daemon context. stderr:\n{proc.stderr}") + +dest = work / '00' / 'f' +if not dest.is_file(): + test_fail("local --link-dest=../01 delta produced no destination file") +if dest.read_text() != src.read_text(): + test_fail( + "local --link-dest=../01 delta reconstructed the wrong content: the " + "destination does not match the source after a delta against the " + "sibling basis")