# (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.
* 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;
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);
}
}
}
#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 "/../"
* 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)
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) {
* 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, ...)
--- /dev/null
+#!/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.
--- /dev/null
+#!/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.
--- /dev/null
+#!/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.
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. */
projects / thirdparty / rsync.git / commitdiff
