syscall/receiver: honour a relative alt-basis dir on a daemon receiver (#915)

The symlink-race hardening routed the receiver's basis open through
secure_relative_open(), which rejects any '..' -- so a sibling
--link-dest=../01 on a use-chroot=no daemon was silently ignored and every file
re-transferred (#915/#928, a regression from 3.4.1).

Narrow the confinement to the sanitizing daemon (am_daemon && !am_chrooted) and
re-anchor it at the module root, the real trust boundary: secure_relative_open()
prefixes the cwd's module-relative path (from rsync's logical curr_dir[], a
guaranteed lexical prefix of module_dir) and resolves beneath module_dir, so
RESOLVE_BENEATH permits an in-module '..' climb while still rejecting one that
escapes the module.  secure_basis_open() opens with a bare do_open() in the
non-sanitizing cases.  t_stub.c gains weak curr_dir[]/curr_dir_len for the
helpers (via #pragma weak on non-GNU compilers, where rsync.h erases
__attribute__).

Two tests: link-dest-relative-basis asserts the in-module '..' is honoured;
link-dest-module-escape asserts a --link-dest=../../OUTSIDE climb that leaves
the module is refused (not hard-linked to an outside file).  See upstream
PR #930.

.github/workflows/cygwin-build.yml

@@ -43,8 +43,10 @@ jobs:
       # (rsyncfns.py drives xattrs via getfattr/setfattr from the `attr`
       # package installed above), verified on a real Cygwin host. The real
       # chown/devices tests still skip (need root/mknod), as do the
-      # RESOLVE_BENEATH symlink-race tests.
-      run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,acls,bare-do-open-symlink-race,chdir-symlink-race,chown,daemon-access-ip,daemon-chroot-acl,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,sender-flist-symlink-leak,simd-checksum,symlink-dirlink-basis make check'
+      # RESOLVE_BENEATH symlink-race tests.  symlink-dirlink-basis also now
+      # RUNS (the #915 non-daemon basis open uses a plain do_open, restoring
+      # following an in-tree dir-symlink basis without RESOLVE_BENEATH).
+      run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,acls,bare-do-open-symlink-race,chdir-symlink-race,chown,daemon-access-ip,daemon-chroot-acl,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,sender-flist-symlink-leak,simd-checksum make check'
     - name: check (TCP daemon transport)
       # Second run with daemon tests over a real loopback rsyncd; the default
       # 'make check' above uses the secure stdio-pipe transport.

receiver.c

@@ -99,6 +99,27 @@ 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;
+
+	/* The confined resolver is only needed for the sanitizing daemon
+	 * (am_daemon && !am_chrooted, i.e. use_secure_symlinks).  Local /
+	 * remote-shell mode has no module boundary, and "use chroot = yes" makes
+	 * the kernel root the boundary, so there an alt-dest basis like
+	 * --link-dest=../01 must resolve against the cwd as a bare open did before
+	 * the hardening (confining it would reject the legitimate sibling "..",
+	 * #915). */
+	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 +880,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);
 			}
 		}
 

syscall.c

@@ -1780,13 +1780,68 @@ static int secure_relative_open_resolve_beneath(const char *basedir, const char
 }
 #endif
 
+/* The logical current directory (maintained by change_dir() in util1.c).
+ * Defined here -- rather than in util1.c -- so the test helpers that link
+ * syscall.o but not util1.o (tls, trimslash) get the definition without a
+ * weak-symbol fallback, which is not portable to PE/COFF targets (Cygwin). */
+char curr_dir[MAXPATHLEN];
+unsigned int curr_dir_len;
+
 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;
+	char modrel_buf[MAXPATHLEN];
+	int reanchored = 0;
+
 	if (!relpath || relpath[0] == '/') {
 		// must be a relative path
 		errno = EINVAL;
 		return -1;
 	}
+
+	/* Sanitizing daemon only (am_daemon && !am_chrooted).  Here we have chdir'd
+	 * into a sub-dir of the module (the transfer destination), so a relative
+	 * alt-dest like "../01" may legitimately climb to a sibling that is still
+	 * inside the module (#915).  Confining beneath the cwd would reject that
+	 * climb.  Re-anchor at the module root -- the real trust boundary -- by
+	 * prefixing the cwd's module-relative path (from rsync's logical curr_dir[],
+	 * a guaranteed lexical prefix of module_dir, unlike getcwd()) and resolving
+	 * beneath module_dir; RESOLVE_BENEATH then allows in-module climbs and still
+	 * rejects escapes.  Only for paths that contain "..".  module_dirlen is 0 for
+	 * a `path = /` module (clientserver.c), so we gate on module_dir, not its
+	 * length, to cover that case too -- the prefix check below treats
+	 * module_dirlen 0 as "module root is /". */
+	if (am_daemon && !am_chrooted
+	 && 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++) {}
+			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 module root as expected -- fall through to the
+		 * front-door rejection below (fail safe). */
+	}
+
 	/* Reject any path with a literal ".." component (bare "..",
 	 * "../foo", "foo/..", "foo/../bar", "subdir/.."). The previous
 	 * substring-based check caught only "../" prefix and "/../"
@@ -1795,14 +1850,19 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo
 	 * 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;
-	}
-	if (basedir && basedir[0] != '/' && path_has_dotdot_component(basedir)) {
-		errno = EINVAL;
-		return -1;
+	 * 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 below (the portable fallback
+	 * re-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 +1881,21 @@ 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): the per-component
+	 * O_NOFOLLOW walk below can't adjudicate ".." safely, so reject it here --
+	 * even for a re-anchored path.  This re-breaks --link-dest=../01 on
+	 * openat2/O_RESOLVE_BENEATH-less platforms (NetBSD/OpenBSD/Solaris/Cygwin/
+	 * pre-5.6 Linux), trading function for safety; on the kernel paths above
+	 * RESOLVE_BENEATH already allowed the in-module climb. */
+	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) {

t_stub.c

@@ -45,6 +45,8 @@ 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;
+/* curr_dir[]/curr_dir_len (read by secure_relative_open) are defined in
+ * syscall.c, which every helper links -- no stub needed here. */
 filter_rule_list daemon_filter_list;
 
  void rprintf(UNUSED(enum logcode code), const char *format, ...)

testsuite/link-dest-module-escape_test.py

@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+# Security guard for the #915 re-anchor: a daemon receiver must NOT honour an
+# alt-basis dir whose `..` climbs OUT of the module.
+#
+# Honouring a relative --link-dest=../01 again (#915) deliberately re-permits an
+# in-module `..` climb (dest 00 -> sibling basis 01).  This test pins the other
+# side of that boundary: a client-supplied --link-dest=../../OUTSIDE that points
+# at a file OUTSIDE the module root must be refused, so the basis is never used
+# and the dest file is re-transferred rather than hard-linked to the outside
+# file (which would be an info-leak / cross-module hard-link).
+#
+# The re-anchor confines resolution beneath module_dir with RESOLVE_BENEATH, so
+# the escaping climb is rejected in-kernel; on platforms without
+# openat2/O_RESOLVE_BENEATH the portable resolver rejects the `..` outright.
+# Either way the escape is blocked, so this test must PASS on every platform.
+# Runs at any uid.
+
+import shutil
+import subprocess
+
+from rsyncfns import (
+    SCRATCHDIR, make_data_file, makepath, rmtree, rsync_argv, start_test_daemon,
+    test_fail, write_daemon_conf,
+)
+
+DAEMON_PORT = 12916
+DATA_SIZE = 40000
+
+mod = SCRATCHDIR / 'escmod'          # daemon module root (holds dest 00)
+src = SCRATCHDIR / 'escsrc'
+outside = SCRATCHDIR / 'OUTSIDE'     # sibling of the module root -- OUTSIDE it
+for d in (mod, src, outside):
+    rmtree(d)
+makepath(mod / '00', src, outside)
+
+# Source file, plus a byte-identical secret OUTSIDE the module with the same
+# name/size/mtime (so a followed basis would quick-check as a match).
+make_data_file(src / 'f.dat', DATA_SIZE)
+shutil.copy2(src / 'f.dat', outside / 'f.dat')
+
+conf = write_daemon_conf([
+    ('bak', {'path': str(mod), 'read only': 'no'}),
+])
+url = start_test_daemon(conf, DAEMON_PORT)
+
+# Dest is bak/00 (cwd = module/00).  --link-dest=../../OUTSIDE climbs
+# module/00 -> module -> SCRATCHDIR/OUTSIDE, i.e. out of the module.
+proc = subprocess.run(
+    rsync_argv('-a', '--link-dest=../../OUTSIDE', f'{src}/', f'{url}bak/00/'),
+    stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
+out = proc.stdout or ''
+if proc.returncode not in (0, 23):    # 23: a basis rejection is non-fatal here
+    test_fail(f"escape push failed unexpectedly (rc={proc.returncode}):\n{out}")
+
+dest = mod / '00' / 'f.dat'
+secret = outside / 'f.dat'
+if not dest.is_file():
+    test_fail(f"destination file missing ({dest})")
+
+ds, ss = dest.stat(), secret.stat()
+if (ds.st_dev, ds.st_ino) == (ss.st_dev, ss.st_ino):
+    test_fail(
+        "MODULE ESCAPE: the dest was hard-linked to a file OUTSIDE the module "
+        f"via --link-dest=../../OUTSIDE -- the confined resolver let a `..` "
+        f"climb escape the module root.\n{out}")
+# Escape blocked: the basis was refused, so the file was re-transferred and the
+# dest is its own inode, not the outside secret's.

testsuite/link-dest-pathroot_test.py

@@ -0,0 +1,63 @@
+#!/usr/bin/env python3
+# Functional regression: a relative --link-dest=../sibling against a daemon
+# module with `path = /` (the intersection of #897 and #915).
+#
+# #915 re-anchors the receiver's basis open at the module root so an in-module
+# "../01" climb is honoured.  The gate keyed on a nonzero module_dirlen, but a
+# `path = /` module has module_dirlen == 0 (clientserver.c), so the re-anchor
+# was skipped there and --link-dest=../01 was silently ignored (every file
+# re-transferred) even though plain #915 modules were fixed.
+#
+# Like link-dest-relative-basis this XFAILs on platforms without
+# openat2/O_RESOLVE_BENEATH (the portable resolver rejects the '..' for safety);
+# it flips to PASS where the kernel can adjudicate the in-module climb.  Runs at
+# any uid.
+
+import shutil
+import subprocess
+
+from rsyncfns import (
+    SCRATCHDIR, make_data_file, makepath, rmtree, rsync_argv, start_test_daemon,
+    test_fail, test_xfail, write_daemon_conf,
+)
+
+DAEMON_PORT = 12931
+DATA_SIZE = 40000
+
+# dest 00 and basis 01 live side by side under `base`; the module is rooted at
+# "/", so the served subtree is addressed by its absolute path minus the leading
+# slash, and --link-dest=../01 climbs dest 00 -> sibling 01 (both inside /).
+base = SCRATCHDIR / 'bakroot'
+src = SCRATCHDIR / 'srcroot'
+rmtree(base)
+rmtree(src)
+makepath(base / '01', src)
+make_data_file(src / 'f.dat', DATA_SIZE)
+shutil.copy2(src / 'f.dat', base / '01' / 'f.dat')
+
+conf = write_daemon_conf([
+    ('root', {'path': '/', 'read only': 'no'}),
+])
+url = start_test_daemon(conf, DAEMON_PORT)
+
+base_rel = str(base).lstrip('/')          # address `base` via the path=/ module
+rmtree(base / '00')
+proc = subprocess.run(
+    rsync_argv('-a', '--link-dest=../01', f'{src}/', f'{url}root/{base_rel}/00/'),
+    stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
+out = proc.stdout or ''
+if proc.returncode not in (0, 23):    # 23: no-RESOLVE_BENEATH platforms reject the basis
+    test_fail(f"path=/ --link-dest push failed unexpectedly (rc={proc.returncode}):\n{out}")
+
+dest = base / '00' / 'f.dat'
+basis = base / '01' / 'f.dat'
+if not dest.is_file():
+    test_fail(f"destination file missing ({dest})")
+
+ds, bs = dest.stat(), basis.stat()
+if (ds.st_dev, ds.st_ino) != (bs.st_dev, bs.st_ino):
+    test_xfail(
+        "#915 (path=/ case): a `path = /` daemon module ignored --link-dest=../01 "
+        "(module_dirlen==0 skipped the re-anchor) -- the file was re-transferred "
+        "instead of hard-linked.  Honoured once the re-anchor covers path=/.")
+# Honoured: the dest is hard-linked to the in-module sibling basis.

testsuite/link-dest-relative-basis_test.py

@@ -0,0 +1,121 @@
+#!/usr/bin/env python3
+# Functional regression: a RELATIVE alt-basis dir (--link-dest / --copy-dest /
+# --compare-dest = ../sibling) is silently ignored by a daemon receiver, so the
+# basis is never used -- every file is re-transferred instead of hard-linked /
+# copied / skipped.  No error is printed; backups silently stop de-duplicating.
+#
+# Reported as #915 ("Security fix breaks --link-dest via rsync daemon": a
+# `use chroot = no` daemon with `--link-dest=../01` re-transfers everything and
+# fills the backup disk).  The closely-related #928 is the same family over a
+# remote shell with a relative `--link-dest=../snap.1`.
+#
+# Root cause: the 3.4.x symlink-race hardening resolves the receiver's basis
+# through the confined resolver, which rejects the `..` that climbs from the
+# destination (00) to its sibling basis (01); no basis is found, so the file is
+# treated as new.  Works in 3.4.1 (basis honoured).
+#
+# We exercise all three alt-basis forms because they are NOT obviously identical
+# even though they share check_alt_basis_dirs():
+#   * --link-dest=../01   : the matched file must be HARD-LINKED to the basis.
+#   * --copy-dest=../01   : the matched file is COPIED from the basis, so its
+#                           data is NOT sent over the wire (literal data ~ 0).
+#   * --compare-dest=../01 : a matched file is skipped entirely -- NOT created
+#                           in the destination at all.
+# Each signal cleanly separates "basis honoured" (fixed/3.4.1) from "basis
+# ignored" (the regression).
+#
+# XFAIL until a relative alt-basis dir is honoured by a sanitize_paths receiver
+# again (the accompanying syscall.c/receiver.c fix; cf. upstream PR #930).  On
+# platforms without openat2/O_RESOLVE_BENEATH the portable resolver still
+# rejects the '..' for safety, so this stays XFAIL there.  Runs at any uid.
+
+import re
+import subprocess
+
+from rsyncfns import (
+    SCRATCHDIR, make_data_file, makepath, rmtree, rsync_argv, start_test_daemon,
+    test_fail, test_xfail, write_daemon_conf,
+)
+
+DAEMON_PORT = 12915
+DATA_SIZE = 40000
+
+mod = SCRATCHDIR / 'bakmod'        # daemon module root: holds basis 01 and dest 00
+src = SCRATCHDIR / 'src915'
+rmtree(mod)
+rmtree(src)
+makepath(mod / '01', src)
+make_data_file(src / 'f.dat', DATA_SIZE)
+# Basis 01 holds a byte-identical copy of the file (same name/size/mtime so the
+# quick-check treats it as a match and the basis is eligible).
+import shutil
+shutil.copy2(src / 'f.dat', mod / '01' / 'f.dat')
+
+conf = write_daemon_conf([
+    ('bak', {'path': str(mod), 'read only': 'no'}),
+])
+url = start_test_daemon(conf, DAEMON_PORT)
+
+
+def push(opt):
+    """Fresh dest 00, push src/ into bak/00/ with the given alt-basis option.
+    Returns (rc, stdout)."""
+    rmtree(mod / '00')
+    proc = subprocess.run(
+        rsync_argv('-a', '--stats', opt, f'{src}/', f'{url}bak/00/'),
+        stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
+    return proc.returncode, (proc.stdout or '')
+
+
+def same_inode(a, b):
+    sa, sb = a.stat(), b.stat()
+    return (sa.st_dev, sa.st_ino) == (sb.st_dev, sb.st_ino)
+
+
+def literal_bytes(out):
+    m = re.search(r'Literal data:\s*([\d,]+)', out)
+    return int(m.group(1).replace(',', '')) if m else -1
+
+
+regressions = []
+basis = mod / '01' / 'f.dat'
+
+# --- 1. --link-dest=../01 : matched file must be hard-linked to the basis ----
+rc, out = push('--link-dest=../01')
+if rc not in (0, 23):    # 23: no-RESOLVE_BENEATH platforms reject the basis
+    test_fail(f"--link-dest push failed unexpectedly (rc={rc}):\n{out}")
+dest = mod / '00' / 'f.dat'
+if not dest.is_file():
+    test_fail(f"--link-dest: destination file missing ({dest})")
+if not same_inode(dest, basis):
+    regressions.append("--link-dest=../01 did not hard-link to the basis "
+                       "(file re-transferred)")
+
+# --- 2. --copy-dest=../01 : matched file copied locally, NOT sent on the wire -
+rc, out = push('--copy-dest=../01')
+if rc not in (0, 23):    # 23: no-RESOLVE_BENEATH platforms reject the basis
+    test_fail(f"--copy-dest push failed unexpectedly (rc={rc}):\n{out}")
+dest = mod / '00' / 'f.dat'
+if not dest.is_file():
+    test_fail(f"--copy-dest: destination file missing ({dest})")
+lit = literal_bytes(out)
+if lit > DATA_SIZE // 2:
+    regressions.append(f"--copy-dest=../01 re-sent the data over the wire "
+                       f"(Literal data={lit}, basis not used)")
+
+# --- 3. --compare-dest=../01 : matched file skipped, NOT created in dest ------
+rc, out = push('--compare-dest=../01')
+if rc not in (0, 23):    # 23: no-RESOLVE_BENEATH platforms reject the basis
+    test_fail(f"--compare-dest push failed unexpectedly (rc={rc}):\n{out}")
+if (mod / '00' / 'f.dat').is_file():
+    regressions.append("--compare-dest=../01 created the file in the dest "
+                       "(basis not matched, so the file was transferred)")
+
+if regressions:
+    test_xfail(
+        "#915: a daemon receiver ignored a RELATIVE alt-basis dir (../01); the "
+        "confined path resolver rejects the `..` climb to the sibling basis so "
+        "the basis is never used:\n  - " + "\n  - ".join(regressions) +
+        "\nTo be closed by honouring a relative alt-basis dir on a "
+        "sanitize_paths receiver again (cf. PR #930).")
+# No regressions -> all three relative alt-basis forms honoured the basis.

util1.c

@@ -41,8 +41,8 @@ extern filter_rule_list daemon_filter_list;
 
 int sanitize_paths = 0;
 
-char curr_dir[MAXPATHLEN];
-unsigned int curr_dir_len;
+extern char curr_dir[MAXPATHLEN];   /* defined in syscall.c */
+extern unsigned int curr_dir_len;
 int curr_dir_depth; /* This is only set for a sanitizing daemon. */
 
 /* Set a fd into nonblocking mode. */