From e2121aaeb706234707fd0528f3571d8757f281e8 Mon Sep 17 00:00:00 2001 From: "init.d" Date: Sun, 8 Nov 2026 20:52:59 +0100 Subject: [PATCH 1/5] rrsync: add -log to record every transfer Over ssh the server side of a transfer leaves no per-file record unless the client asks for one with --log-file, so an rrsync admin has no reliable way to know what a restricted key uploaded, downloaded or deleted. The new -log FILE option passes --log-file=FILE to the server rsync, which writes its default "%i %n%L" line for every file, plus the start and summary lines, with a timestamp and pid. FILE is made absolute like DIR, since rsync opens it after the chdir into DIR. The client's own --log-file is refused while -log is in force, the same way -ro refuses it. Add rrsync-log_test.py, which checks the per-file lines of a push and the refusal of a client --log-file only when -log is given. --- support/rrsync | 9 ++++- support/rrsync.1.md | 10 ++++- testsuite/rrsync-log_test.py | 74 ++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 testsuite/rrsync-log_test.py diff --git a/support/rrsync b/support/rrsync index a29889a1b..c7a46c62c 100755 --- a/support/rrsync +++ b/support/rrsync @@ -527,7 +527,7 @@ def main(): for opt in long_opts: if opt.startswith(('remove', 'delete')): long_opts[opt] = -1 - if args.ro: + if args.ro or args.log: long_opts['log-file'] = -1 global short_disabled @@ -672,6 +672,9 @@ def main(): if args.no_overwrite: rsync_opts.append('--ignore-existing') + if args.log: + rsync_opts.append('--log-file=' + args.log) + if not rsync_args: rsync_args = [ '.' ] @@ -1023,12 +1026,16 @@ if __name__ == '__main__': arg_parser.add_argument('-no-del', action='store_true', help="Disable rsync's --delete* and --remove* options.") arg_parser.add_argument('-no-lock', action='store_true', help="Avoid the single-run (per-user) lock check.") arg_parser.add_argument('-no-overwrite', action='store_true', help="Prevent overwriting existing files by enforcing --ignore-existing") + arg_parser.add_argument('-log', metavar='FILE', help="Log each transfer to FILE via rsync's --log-file.") arg_parser.add_argument('-help', '-h', action='help', help="Output this help message and exit.") arg_parser.add_argument('dir', metavar='DIR', help="The restricted directory to use.") args = arg_parser.parse_args() args.dir = os.path.realpath(args.dir) args.dir_slash = args.dir + '/' args.dir_slash_len = len(args.dir_slash) + if args.log: + # rsync opens it after the chdir into DIR. + args.log = os.path.abspath(args.log) if args.ro: args.no_del = True elif not args.no_lock: diff --git a/support/rrsync.1.md b/support/rrsync.1.md index 2ab0fd17d..873fe6709 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -5,7 +5,7 @@ rrsync - a script to setup restricted rsync users via ssh logins ## SYNOPSIS ``` -rrsync [-ro|-wo] [-munge] [-absolute] [-no-del] [-no-lock] [-no-overwrite] DIR +rrsync [-ro|-wo] [-munge] [-absolute] [-no-del] [-no-lock] [-no-overwrite] [-log FILE] DIR ``` The single non-option argument specifies the restricted _DIR_ to use. It can be @@ -106,6 +106,14 @@ The remainder of this manpage is dedicated to using the rrsync script. `--partial-dir`, `--delay-updates`, `--delay-symlinks`, and server-side logging are therefore unavailable under this option. +0. `-log FILE` + + Log every transfer to _FILE_ using rsync's [`--log-file`](rsync.1#opt), + in its default "%i %n%L" format, so each file sent, received or deleted + gets a line. _FILE_ can be relative to the user's home directory or an + absolute path. The client's own `--log-file` option is refused while this + is in force. + 0. `-help`, `-h` Output this help message and exit. diff --git a/testsuite/rrsync-log_test.py b/testsuite/rrsync-log_test.py new file mode 100644 index 000000000..dedf8a044 --- /dev/null +++ b/testsuite/rrsync-log_test.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +"""rrsync -log must record every transfer in the admin's log file. + +The client's own --log-file is refused while -log is in force. +""" + +import os +import shlex +import signal +import subprocess +import sys + +signal.signal(signal.SIGUSR1, signal.SIG_IGN) +signal.signal(signal.SIGUSR2, signal.SIG_IGN) +if '--shell' in sys.argv: + i = sys.argv.index('--shell') + 2 + env = {**os.environ, 'SSH_ORIGINAL_COMMAND': ' '.join(sys.argv[i:])} + signal.signal(signal.SIGUSR1, signal.SIG_DFL) + signal.signal(signal.SIGUSR2, signal.SIG_DFL) + wrapper, root = env['RRSYNC_WRAPPER'], env['RRSYNC_ROOT'] + flags = shlex.split(env['RRSYNC_FLAGS']) + os.execve(wrapper, [wrapper, *flags, root], env) + +from rsyncfns import RSYNC, SCRATCHDIR, makepath, patched_rrsync, rmtree, rsync_argv, rsync_path_arg + +base = SCRATCHDIR / 'rrsync-log' +rmtree(base) +src = base / 'src' +root = base / 'root' +makepath(src / 'sub', root) +(src / 'a.txt').write_bytes(b'AAA') +(src / 'sub' / 'b.txt').write_bytes(b'BB') +log = base / 'transfer.log' + +# RSYNC may be a multi-word command (the runner's --protocol=N, or valgrind) +# while rrsync hands its RSYNC to execlp() as a single executable name, so it +# has to be wrapped before patched_rrsync() sees it. +shim = base / 'rsync-shim' +shim.write_text('#!/bin/sh\nexec ' + rsync_path_arg(RSYNC) + ' "$@"\n') +shim.chmod(0o755) +wrapper = patched_rrsync(base, rsync_path=str(shim)) +rsh = f'{shlex.quote(sys.executable)} {shlex.quote(os.path.abspath(__file__))} --shell' +env = {**os.environ, 'RRSYNC_WRAPPER': str(wrapper), 'RRSYNC_ROOT': str(root), + 'RRSYNC_FLAGS': f'-wo -no-lock -log {shlex.quote(str(log))}'} + +push = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=env, capture_output=True, text=True, timeout=20) +assert push.returncode == 0 and (root / 'sub' / 'b.txt').read_bytes() == b'BB', ( + f'upload with -log failed: rc={push.returncode}, stderr={push.stderr!r}') +lines = log.read_text().splitlines() if log.exists() else [] +for name in ('a.txt', 'sub/b.txt'): + assert any('>f' in line and line.endswith(' ' + name) for line in lines), ( + f'-log did not record {name}: {lines!r}') + +# The admin's log replaces the client's: a client --log-file is an error, not +# a second log the client can point anywhere inside the restricted dir. +client_log = subprocess.run( + rsync_argv('-r', '-M--log-file=mine.log', '-e', rsh, f'{src}/', 'ignored:'), + env=env, capture_output=True, text=True, timeout=20) +assert client_log.returncode != 0 and 'option --log-file has been disabled' in client_log.stderr, ( + f'rrsync -log accepted the client --log-file: rc={client_log.returncode}, ' + f'stderr={client_log.stderr!r}') +assert not (root / 'mine.log').exists() + +# Without -log the client keeps its --log-file, or the refusal above is +# unconditional and says nothing about -log. +plain_env = {**env, 'RRSYNC_FLAGS': '-wo -no-lock'} +plain = subprocess.run( + rsync_argv('-r', '-M--log-file=mine.log', '-e', rsh, f'{src}/', 'ignored:'), + env=plain_env, capture_output=True, text=True, timeout=20) +assert plain.returncode == 0 and (root / 'mine.log').exists(), ( + f'without -log the client --log-file must be accepted: rc={plain.returncode}, ' + f'stderr={plain.stderr!r}') From d3f7bd9a1485bb3e107444b175b10fff5d5ddc1b Mon Sep 17 00:00:00 2001 From: "init.d" Date: Sun, 8 Nov 2026 23:50:36 +0100 Subject: [PATCH 2/5] rrsync: refuse a -log FILE inside DIR The client can plant symlinks inside DIR, and rsync opens the log by name, so a log there could be redirected to any file the user can write. Refuse a FILE whose path contains a symlink, or whose existing ancestors include DIR, compared by device and inode so that a bind mount of DIR is caught as well. Extend rrsync-log_test.py with a FILE inside DIR, one behind a symlink that leads through it, and one in a bind mount of it (where unshare can create a user namespace). --- support/rrsync | 15 ++++++++++++ support/rrsync.1.md | 2 +- testsuite/rrsync-log_test.py | 47 +++++++++++++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/support/rrsync b/support/rrsync index c7a46c62c..ffad865ef 100755 --- a/support/rrsync +++ b/support/rrsync @@ -493,6 +493,21 @@ def main(): if not os.path.isdir(args.dir): die("Restricted directory does not exist!") + if args.log: + # The client can plant symlinks in DIR, so a log path that reaches DIR + # by any route (a symlink, a bind mount) could be redirected. + if args.log != os.path.realpath(args.log): + die("the -log FILE path must not contain a symlink") + dir_st = os.stat(args.dir) + path = args.log + while path != '/': + try: + if os.path.samestat(os.stat(path), dir_st): + die("the -log FILE must be outside the restricted directory") + except OSError: + pass # a missing component cannot be DIR + path = os.path.dirname(path) + # The format of the environment variables set by sshd: # SSH_ORIGINAL_COMMAND: # rsync --server -vlogDtpre.iLsfxCIvu --etc . ARG # push diff --git a/support/rrsync.1.md b/support/rrsync.1.md index 873fe6709..28b582633 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -112,7 +112,7 @@ The remainder of this manpage is dedicated to using the rrsync script. in its default "%i %n%L" format, so each file sent, received or deleted gets a line. _FILE_ can be relative to the user's home directory or an absolute path. The client's own `--log-file` option is refused while this - is in force. + is in force. _FILE_ must be outside _DIR_, with no symlink in its path. 0. `-help`, `-h` diff --git a/testsuite/rrsync-log_test.py b/testsuite/rrsync-log_test.py index dedf8a044..37dd6b81c 100644 --- a/testsuite/rrsync-log_test.py +++ b/testsuite/rrsync-log_test.py @@ -1,11 +1,14 @@ #!/usr/bin/env python3 """rrsync -log must record every transfer in the admin's log file. -The client's own --log-file is refused while -log is in force. +The client's own --log-file is refused while -log is in force, and a FILE +the client could redirect (inside the restricted dir, behind a symlink, or in +a bind mount of the restricted dir) is refused outright. """ import os import shlex +import shutil import signal import subprocess import sys @@ -72,3 +75,45 @@ assert plain.returncode == 0 and (root / 'mine.log').exists(), ( f'without -log the client --log-file must be accepted: rc={plain.returncode}, ' f'stderr={plain.stderr!r}') + +# A log inside the restricted dir could be replaced by a client symlink. +inside_log = shlex.quote(str(root / 'transfer.log')) +inside_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {inside_log}'} +inside = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=inside_env, capture_output=True, text=True, timeout=20) +assert inside.returncode != 0 and 'must be outside the restricted directory' in inside.stderr, ( + f'rrsync accepted a -log FILE inside DIR: rc={inside.returncode}, stderr={inside.stderr!r}') +assert not (root / 'transfer.log').exists() + +# Nor may the path pass through DIR by way of a symlink: the client's own +# symlink in DIR would decide where the log is written. +elsewhere = base / 'elsewhere' +makepath(elsewhere) +os.symlink(elsewhere, root / 'planted') +os.symlink(root / 'planted', base / 'via') +via_log = shlex.quote(str(base / 'via' / 'transfer.log')) +via_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {via_log}'} +via = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=via_env, capture_output=True, text=True, timeout=20) +assert via.returncode != 0 and 'must not contain a symlink' in via.stderr, ( + f'rrsync accepted a -log path through DIR: rc={via.returncode}, stderr={via.stderr!r}') +assert not (elsewhere / 'transfer.log').exists() + +# DIR reached through a bind mount is still DIR. An unprivileged bind mount +# needs a user namespace, so this part runs only where unshare can make one. +unshare = shutil.which('unshare') +userns = [unshare, '--user', '--map-root-user', '--mount'] if unshare else None +if userns and subprocess.run(userns + ['true'], capture_output=True).returncode == 0: + bind = base / 'bind' + makepath(bind) + script = 'mount --bind "$1" "$2" && exec "$3" -wo -no-lock -log "$2/transfer.log" "$1"' + bound = subprocess.run( + userns + ['sh', '-c', script, 'sh', str(root), str(bind), str(wrapper)], + env={**os.environ, 'SSH_ORIGINAL_COMMAND': 'rsync --server -r . .'}, + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=20) + assert bound.returncode != 0 and 'must be outside the restricted directory' in bound.stderr, ( + f'rrsync accepted a -log FILE in a bind mount of DIR: rc={bound.returncode}, ' + f'stderr={bound.stderr!r}') + assert not (root / 'transfer.log').exists() From 6f36e152d8e6faed4daab0fe7d11ff3c096a0b19 Mon Sep 17 00:00:00 2001 From: "init.d" Date: Mon, 9 Nov 2026 07:16:52 +0100 Subject: [PATCH 3/5] rrsync: resolve the -log FILE and refuse it when DIR is / Symlinks outside DIR, such as /var -> /private/var on macOS, are fine: rrsync resolves FILE, checks that DIR is not among the resolved ancestors and hands rsync the resolved path. With DIR / no FILE is outside it, so -log is refused there. --- support/rrsync | 9 ++++--- support/rrsync.1.md | 3 ++- testsuite/rrsync-log_test.py | 49 +++++++++++++++++++++++++----------- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/support/rrsync b/support/rrsync index ffad865ef..9a06f8b59 100755 --- a/support/rrsync +++ b/support/rrsync @@ -494,10 +494,11 @@ def main(): die("Restricted directory does not exist!") if args.log: - # The client can plant symlinks in DIR, so a log path that reaches DIR - # by any route (a symlink, a bind mount) could be redirected. - if args.log != os.path.realpath(args.log): - die("the -log FILE path must not contain a symlink") + if args.dir == '/': + die("-log cannot be used when DIR is /") + # rsync gets the resolved path, so a symlink the client plants in DIR + # later cannot redirect it; DIR may not be among its ancestors. + args.log = os.path.realpath(args.log) dir_st = os.stat(args.dir) path = args.log while path != '/': diff --git a/support/rrsync.1.md b/support/rrsync.1.md index 28b582633..8a7fe3a3c 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -112,7 +112,8 @@ The remainder of this manpage is dedicated to using the rrsync script. in its default "%i %n%L" format, so each file sent, received or deleted gets a line. _FILE_ can be relative to the user's home directory or an absolute path. The client's own `--log-file` option is refused while this - is in force. _FILE_ must be outside _DIR_, with no symlink in its path. + is in force. _FILE_ is resolved before use and must be outside _DIR_, + and `-log` cannot be used when _DIR_ is `/`. 0. `-help`, `-h` diff --git a/testsuite/rrsync-log_test.py b/testsuite/rrsync-log_test.py index 37dd6b81c..cfd56d709 100644 --- a/testsuite/rrsync-log_test.py +++ b/testsuite/rrsync-log_test.py @@ -2,8 +2,8 @@ """rrsync -log must record every transfer in the admin's log file. The client's own --log-file is refused while -log is in force, and a FILE -the client could redirect (inside the restricted dir, behind a symlink, or in -a bind mount of the restricted dir) is refused outright. +that resolves inside the restricted dir (directly, through a symlink, or in a +bind mount of it) is refused outright, as is -log with DIR /. """ import os @@ -86,20 +86,39 @@ f'rrsync accepted a -log FILE inside DIR: rc={inside.returncode}, stderr={inside.stderr!r}') assert not (root / 'transfer.log').exists() -# Nor may the path pass through DIR by way of a symlink: the client's own -# symlink in DIR would decide where the log is written. -elsewhere = base / 'elsewhere' -makepath(elsewhere) -os.symlink(elsewhere, root / 'planted') -os.symlink(root / 'planted', base / 'via') -via_log = shlex.quote(str(base / 'via' / 'transfer.log')) -via_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {via_log}'} -via = subprocess.run( +# A symlink outside DIR is fine (like /var -> /private/var on macOS): the log +# goes where it resolves. +real = base / 'real' +makepath(real) +os.symlink(real, base / 'alias') +alias_log = shlex.quote(str(base / 'alias' / 'transfer.log')) +alias_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {alias_log}'} +alias = subprocess.run( rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=via_env, capture_output=True, text=True, timeout=20) -assert via.returncode != 0 and 'must not contain a symlink' in via.stderr, ( - f'rrsync accepted a -log path through DIR: rc={via.returncode}, stderr={via.stderr!r}') -assert not (elsewhere / 'transfer.log').exists() + env=alias_env, capture_output=True, text=True, timeout=20) +assert alias.returncode == 0 and (real / 'transfer.log').exists(), ( + f'rrsync refused a -log path through a symlink outside DIR: rc={alias.returncode}, ' + f'stderr={alias.stderr!r}') + +# A symlink that resolves into DIR is refused like a FILE inside DIR. +os.symlink(root, base / 'into') +into_log = shlex.quote(str(base / 'into' / 'transfer.log')) +into_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {into_log}'} +into = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=into_env, capture_output=True, text=True, timeout=20) +assert into.returncode != 0 and 'must be outside the restricted directory' in into.stderr, ( + f'rrsync accepted a -log path that resolves into DIR: rc={into.returncode}, ' + f'stderr={into.stderr!r}') +assert not (root / 'transfer.log').exists() + +# With DIR / no path is outside it. +slash_env = {**env, 'RRSYNC_ROOT': '/'} +slash = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=slash_env, capture_output=True, text=True, timeout=20) +assert slash.returncode != 0 and '-log cannot be used when DIR is /' in slash.stderr, ( + f'rrsync accepted -log with DIR /: rc={slash.returncode}, stderr={slash.stderr!r}') # DIR reached through a bind mount is still DIR. An unprivileged bind mount # needs a user namespace, so this part runs only where unshare can make one. From 3cd6a4b4bc0664bb6176bd7c1149070f7770a760 Mon Sep 17 00:00:00 2001 From: "init.d" Date: Mon, 9 Nov 2026 18:38:08 +0100 Subject: [PATCH 4/5] rrsync: resolve -log FILE per component and open it before rsync runs rrsync now follows FILE one component at a time and refuses it if any step enters DIR, since from there the client decides where the path leads even when the final path is outside DIR. It also opens FILE before running rsync, so a log that cannot be written stops the transfer, and hands rsync the open file where it already pins paths. Write errors after that remain rsync's --log-file behaviour, as the man page now says. --- support/rrsync | 60 ++++++++++++++++++++++++++++-------- support/rrsync.1.md | 7 +++-- testsuite/rrsync-log_test.py | 27 ++++++++++++++++ 3 files changed, 80 insertions(+), 14 deletions(-) diff --git a/support/rrsync b/support/rrsync index 9a06f8b59..3e8a86cad 100755 --- a/support/rrsync +++ b/support/rrsync @@ -489,6 +489,43 @@ def safe_open_logfile(): return None return os.fdopen(fd, 'a') +def resolve_log_path(path, dir_st): + """Resolve the -log FILE one component at a time, as the kernel does. + + Refuse it if any step enters DIR (by device and inode, so a bind mount of + DIR counts too): from there the client decides where the path leads, even + when the final path is outside DIR.""" + todo = [c for c in path.split('/') if c] + done = '/' + hops = 0 + while todo: + comp = todo.pop(0) + if comp == '.': + continue + if comp == '..': + done = os.path.dirname(done) + continue + cand = os.path.join(done, comp) + try: + st = os.lstat(cand) + except FileNotFoundError: + return os.path.join(cand, *todo) # the open below creates or fails + except OSError as e: + die("cannot resolve the -log FILE", cand + ':', e.strerror) + if stat.S_ISLNK(st.st_mode): + hops += 1 + if hops > 40: + die("too many symlinks in the -log FILE path") + target = os.readlink(cand) + if target.startswith('/'): + done = '/' + todo = [c for c in target.split('/') if c] + todo + continue + if os.path.samestat(st, dir_st): + die("the -log FILE must be outside the restricted directory") + done = cand + return done + def main(): if not os.path.isdir(args.dir): die("Restricted directory does not exist!") @@ -496,18 +533,17 @@ def main(): if args.log: if args.dir == '/': die("-log cannot be used when DIR is /") - # rsync gets the resolved path, so a symlink the client plants in DIR - # later cannot redirect it; DIR may not be among its ancestors. - args.log = os.path.realpath(args.log) - dir_st = os.stat(args.dir) - path = args.log - while path != '/': - try: - if os.path.samestat(os.stat(path), dir_st): - die("the -log FILE must be outside the restricted directory") - except OSError: - pass # a missing component cannot be DIR - path = os.path.dirname(path) + args.log = resolve_log_path(args.log, os.stat(args.dir)) + # Opened here so a log that cannot be written stops the transfer, and + # handed to rsync as the open file where rrsync pins paths. + try: + fd = os.open(args.log, os.O_WRONLY | os.O_APPEND | os.O_CREAT | os.O_NOFOLLOW, 0o644) + except OSError as e: + die("cannot open the -log FILE", args.log + ':', e.strerror) + make_inheritable(fd) + pinned_fds.append(fd) + if HAVE_PROC_SELF_FD: + args.log = '/proc/self/fd/%d' % fd # The format of the environment variables set by sshd: # SSH_ORIGINAL_COMMAND: diff --git a/support/rrsync.1.md b/support/rrsync.1.md index 8a7fe3a3c..aae80cc08 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -112,8 +112,11 @@ The remainder of this manpage is dedicated to using the rrsync script. in its default "%i %n%L" format, so each file sent, received or deleted gets a line. _FILE_ can be relative to the user's home directory or an absolute path. The client's own `--log-file` option is refused while this - is in force. _FILE_ is resolved before use and must be outside _DIR_, - and `-log` cannot be used when _DIR_ is `/`. + is in force. _FILE_ is resolved one component at a time and refused if + any step enters _DIR_, and `-log` cannot be used when _DIR_ is `/`. + rrsync opens _FILE_ before running rsync and refuses the transfer if it + cannot. After that, write errors are handled as for rsync's + `--log-file`: they are not reported and do not stop the transfer. 0. `-help`, `-h` diff --git a/testsuite/rrsync-log_test.py b/testsuite/rrsync-log_test.py index cfd56d709..6b1cc7926 100644 --- a/testsuite/rrsync-log_test.py +++ b/testsuite/rrsync-log_test.py @@ -112,6 +112,33 @@ f'stderr={into.stderr!r}') assert not (root / 'transfer.log').exists() +# Nor may the path pass through DIR on its way out: the client's symlink in +# DIR would decide where the log is written, even outside DIR. +stolen = base / 'stolen' +os.symlink(stolen, root / 'f') +os.symlink(root / 'f', base / 'reg') +reg_log = shlex.quote(str(base / 'reg')) +reg_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {reg_log}'} +reg = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=reg_env, capture_output=True, text=True, timeout=20) +assert reg.returncode != 0 and 'must be outside the restricted directory' in reg.stderr, ( + f'rrsync accepted a -log path that passes through DIR: rc={reg.returncode}, ' + f'stderr={reg.stderr!r}') +assert not stolen.exists() + +# A log that cannot be opened stops the transfer instead of being skipped. +(root / 'sub' / 'b.txt').unlink() +nolog = shlex.quote(str(base / 'missing-dir' / 'transfer.log')) +nolog_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {nolog}'} +unlogged = subprocess.run( + rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), + env=nolog_env, capture_output=True, text=True, timeout=20) +assert unlogged.returncode != 0 and 'cannot open the -log FILE' in unlogged.stderr, ( + f'rrsync ran a transfer it could not log: rc={unlogged.returncode}, ' + f'stderr={unlogged.stderr!r}') +assert not (root / 'sub' / 'b.txt').exists() + # With DIR / no path is outside it. slash_env = {**env, 'RRSYNC_ROOT': '/'} slash = subprocess.run( From 7d19e3d931abf0254474c920c4e114ddb95fdf0e Mon Sep 17 00:00:00 2001 From: steadytao Date: Tue, 10 Nov 2026 06:33:08 +1100 Subject: [PATCH 5/5] rrsync: fix forced transfer logging --- log.c | 3 +- support/rrsync | 68 ++++--- support/rrsync.1.md | 31 ++-- syscall.c | 29 +-- testsuite/rrsync-log_test.py | 340 +++++++++++++++++++++-------------- 5 files changed, 288 insertions(+), 183 deletions(-) diff --git a/log.c b/log.c index bf30df98d..8b02b98e9 100644 --- a/log.c +++ b/log.c @@ -173,8 +173,7 @@ static void logfile_open(void) * attacker-writable dirs; a planted symlink could redirect root's log * into e.g. /root/.ssh/authorized_keys. Refuse symlinks not owned by * uid 0 or our euid. */ - int fd = open_no_attacker_symlinks(logfile_name, - O_WRONLY | O_APPEND | O_CREAT, 0644); + int fd = open_log_file(logfile_name, O_WRONLY | O_APPEND | O_CREAT, 0644); logfile_fp = fd >= 0 ? fdopen(fd, "a") : NULL; if (!logfile_fp && fd >= 0) close(fd); diff --git a/support/rrsync b/support/rrsync index 3e8a86cad..7e8c707b6 100755 --- a/support/rrsync +++ b/support/rrsync @@ -490,13 +490,11 @@ def safe_open_logfile(): return os.fdopen(fd, 'a') def resolve_log_path(path, dir_st): - """Resolve the -log FILE one component at a time, as the kernel does. - - Refuse it if any step enters DIR (by device and inode, so a bind mount of - DIR counts too): from there the client decides where the path leads, even - when the final path is outside DIR.""" + """Resolve symlinks before .. and reject paths through DIR.""" todo = [c for c in path.split('/') if c] - done = '/' + if path.endswith('/'): + todo.append('.') + done = '//' if path.startswith('//') and not path.startswith('///') else '/' hops = 0 while todo: comp = todo.pop(0) @@ -508,21 +506,33 @@ def resolve_log_path(path, dir_st): cand = os.path.join(done, comp) try: st = os.lstat(cand) - except FileNotFoundError: - return os.path.join(cand, *todo) # the open below creates or fails + except FileNotFoundError as e: + if not todo: + return cand + die("cannot resolve the -log FILE", cand + ':', e.strerror) except OSError as e: die("cannot resolve the -log FILE", cand + ':', e.strerror) if stat.S_ISLNK(st.st_mode): + if st.st_uid not in (0, os.geteuid()): + die("untrusted symlink in -log FILE:", cand) hops += 1 if hops > 40: die("too many symlinks in the -log FILE path") - target = os.readlink(cand) + try: + target = os.readlink(cand) + except OSError as e: + die("cannot resolve the -log FILE", cand + ':', e.strerror) if target.startswith('/'): - done = '/' - todo = [c for c in target.split('/') if c] + todo + done = '//' if target.startswith('//') and not target.startswith('///') else '/' + todo = [c for c in target.split('/') if c] + (['.'] if target.endswith('/') else []) + todo continue if os.path.samestat(st, dir_st): die("the -log FILE must be outside the restricted directory") + if todo: + try: + os.stat(os.path.join(cand, '.')) + except OSError as e: + die("cannot resolve the -log FILE", cand + ':', e.strerror) done = cand return done @@ -530,21 +540,6 @@ def main(): if not os.path.isdir(args.dir): die("Restricted directory does not exist!") - if args.log: - if args.dir == '/': - die("-log cannot be used when DIR is /") - args.log = resolve_log_path(args.log, os.stat(args.dir)) - # Opened here so a log that cannot be written stops the transfer, and - # handed to rsync as the open file where rrsync pins paths. - try: - fd = os.open(args.log, os.O_WRONLY | os.O_APPEND | os.O_CREAT | os.O_NOFOLLOW, 0o644) - except OSError as e: - die("cannot open the -log FILE", args.log + ':', e.strerror) - make_inheritable(fd) - pinned_fds.append(fd) - if HAVE_PROC_SELF_FD: - args.log = '/proc/self/fd/%d' % fd - # The format of the environment variables set by sshd: # SSH_ORIGINAL_COMMAND: # rsync --server -vlogDtpre.iLsfxCIvu --etc . ARG # push @@ -725,6 +720,20 @@ def main(): rsync_opts.append('--ignore-existing') if args.log: + if args.dir == '/': + die("-log cannot be used when DIR is /") + resolved_log = resolve_log_path(args.log, os.stat(args.dir)) + fd = None + try: + flags = os.O_WRONLY | os.O_APPEND | os.O_CREAT | os.O_NONBLOCK | getattr(os, 'O_NOFOLLOW', 0) + fd = os.open(resolved_log, flags, 0o644) + if not stat.S_ISREG(os.fstat(fd).st_mode): + die("-log FILE must be a regular file") + except OSError as e: + die("cannot open the -log FILE", args.log + ':', e.strerror) + finally: + if fd is not None: + os.close(fd) rsync_opts.append('--log-file=' + args.log) if not rsync_args: @@ -1085,9 +1094,10 @@ if __name__ == '__main__': args.dir = os.path.realpath(args.dir) args.dir_slash = args.dir + '/' args.dir_slash_len = len(args.dir_slash) - if args.log: - # rsync opens it after the chdir into DIR. - args.log = os.path.abspath(args.log) + if args.log is not None: + if not args.log: + die("empty -log FILE") + args.log = os.path.join(os.getcwd(), args.log) if args.ro: args.no_del = True elif not args.no_lock: diff --git a/support/rrsync.1.md b/support/rrsync.1.md index aae80cc08..58c61145f 100644 --- a/support/rrsync.1.md +++ b/support/rrsync.1.md @@ -103,20 +103,29 @@ The remainder of this manpage is dedicated to using the rrsync script. replacing whatever has appeared at that name since the check), and backup mode (`-b`, `--backup-dir`, whose published backup replaces whatever already occupies the backup name). Resumable uploads with an explicit - `--partial-dir`, `--delay-updates`, `--delay-symlinks`, and server-side - logging are therefore unavailable under this option. + `--partial-dir`, `--delay-updates`, `--delay-symlinks` and client-selected + log files are therefore unavailable under this option. Administrator logging + with [`-log FILE`](#opt) remains available. 0. `-log FILE` - Log every transfer to _FILE_ using rsync's [`--log-file`](rsync.1#opt), - in its default "%i %n%L" format, so each file sent, received or deleted - gets a line. _FILE_ can be relative to the user's home directory or an - absolute path. The client's own `--log-file` option is refused while this - is in force. _FILE_ is resolved one component at a time and refused if - any step enters _DIR_, and `-log` cannot be used when _DIR_ is `/`. - rrsync opens _FILE_ before running rsync and refuses the transfer if it - cannot. After that, write errors are handled as for rsync's - `--log-file`: they are not reported and do not stop the transfer. + Write server-side transfer records to _FILE_ using rsync's + [`--log-file`](rsync.1#opt). The default "%i %n%L" format records files sent, + received or deleted, with timestamps and process IDs. The client's own + `--log-file` is refused. This works with `-ro`, `-wo` and `-no-overwrite`. + + _FILE_ must be a regular file outside _DIR_. It can be absolute or relative + to the user's home directory. rrsync resolves symlinks before `..` and + refuses paths that pass through _DIR_. `-log` is refused when _DIR_ is `/`. + Symlinks must be owned by root or the user running rrsync. Keep the log and + its parent directories out of untrusted users' reach, including + through hard links or bind mounts; rrsync cannot detect every such alias. + + rrsync checks that _FILE_ can be opened before starting the transfer. + Rsync then opens the file itself. Logging is best-effort: a later open or + write failure can leave a successful transfer with incomplete or missing + records. Use rsync's exit status to determine whether a transfer succeeded. + This option does not provide a tamper-resistant audit trail. 0. `-help`, `-h` diff --git a/syscall.c b/syscall.c index e0bae3807..339995d8b 100644 --- a/syscall.c +++ b/syscall.c @@ -421,7 +421,7 @@ static int ona_open_absolute_root(char *remaining, char *abspath, size_t abspath /* Core walk. When out_abs is non-NULL and the path resolves to a directory * (O_DIRECTORY), the resolved absolute path is copied there -- owner_walk_parent * uses it to filter-check the (otherwise unchecked) leaf basename. */ -static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, size_t out_cap) +static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, size_t out_cap, int enforce_root) { #if defined AT_FDCWD && defined O_NOFOLLOW && defined O_DIRECTORY const int dir_traverse_flags = directory_traverse_flags() | O_CLOEXEC; @@ -446,7 +446,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz char abspath[MAXPATHLEN] = {0}; if (am_daemon && module_dir && module_dir[0] == '/') strlcpy(abspath, module_dir, sizeof abspath); /* "/" for a path=/ module */ - else if (confine_root) { + else if (enforce_root && confine_root) { /* Unlike a daemon's, this cwd is not pinned to the root -- the receiver * chdir's into the destination -- so it has to be read, not assumed. * It must be the PHYSICAL cwd: curr_dir is the lexical name change_dir() @@ -472,7 +472,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz * following the link restarts the walk at its absolute target, and every * component of THAT is checked, so a pin aimed outside is still refused. */ const char *ptail = fd_pin_tail(path); - int pin_transit = !am_daemon && confine_root && ptail != NULL; + int pin_transit = enforce_root && !am_daemon && confine_root && ptail != NULL; /* Path-walk state. `remaining` is the unconsumed tail; we splice * symlink targets back into it as we go. Sized 2x MAXPATHLEN so a @@ -534,7 +534,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz saved_errno = errno; goto out; } - if (!pin_transit && abspath_outside_confinement(abspath, 1)) { + if (enforce_root && !pin_transit && abspath_outside_confinement(abspath, 1)) { saved_errno = ELOOP; goto out; } @@ -664,7 +664,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz saved_errno = errno; goto out; } - if (!pin_transit && abspath_outside_confinement(abspath, 1)) { + if (enforce_root && !pin_transit && abspath_outside_confinement(abspath, 1)) { saved_errno = ELOOP; goto out; } @@ -688,7 +688,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz saved_errno = errno; goto out; } - if (!pin_transit && abspath_outside_confinement(abspath, 0)) { + if (enforce_root && !pin_transit && abspath_outside_confinement(abspath, 0)) { saved_errno = ELOOP; goto out; } @@ -716,7 +716,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz * an O_PATH fd is sufficient for traversal and fchdir but not operations * such as fchmod. */ if (flags & O_DIRECTORY) { - if (!pin_transit && abspath_outside_confinement(abspath, 1)) { + if (enforce_root && !pin_transit && abspath_outside_confinement(abspath, 1)) { saved_errno = ELOOP; goto out; } @@ -737,13 +737,20 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz #else /* Pre-AT_FDCWD / no O_NOFOLLOW systems: best-effort fallback. */ (void)out_abs; (void)out_cap; + (void)enforce_root; return open(path, flags, mode); #endif } int open_no_attacker_symlinks(const char *path, int flags, mode_t mode) { - return ona_open(path, flags, mode, NULL, 0); + return ona_open(path, flags, mode, NULL, 0, 1); +} + +/* A log's ownership check must not inherit file-list confinement. */ +int open_log_file(const char *path, int flags, mode_t mode) +{ + return ona_open(path, flags, mode, NULL, 0, 0); } /* Open a directory for traversal or as *at()/fchdir() authority. Unlike an @@ -751,7 +758,7 @@ int open_no_attacker_symlinks(const char *path, int flags, mode_t mode) * directory on Linux. */ int open_no_attacker_symlinks_dirfd(const char *path) { - return ona_open(path, directory_traverse_flags(), 0, NULL, 0); + return ona_open(path, directory_traverse_flags(), 0, NULL, 0, 1); } /* When set, the do_*_at() wrappers resolve their path as an OPERATOR-supplied @@ -780,7 +787,7 @@ int owner_walk_parent(const char *path, const char **bname) *bname = slash ? slash + 1 : path; pabs[0] = '\0'; if (!slash) - dfd = ona_open(".", directory_traverse_flags(), 0, pabs, sizeof pabs); + dfd = ona_open(".", directory_traverse_flags(), 0, pabs, sizeof pabs, 1); else { dlen = slash == path ? 1 : (size_t)(slash - path); /* "/x" -> parent "/" */ if (dlen >= sizeof dir) { @@ -789,7 +796,7 @@ int owner_walk_parent(const char *path, const char **bname) } memcpy(dir, path, dlen); dir[dlen] = '\0'; - dfd = ona_open(dir, directory_traverse_flags(), 0, pabs, sizeof pabs); + dfd = ona_open(dir, directory_traverse_flags(), 0, pabs, sizeof pabs, 1); } if (dfd < 0) return -1; diff --git a/testsuite/rrsync-log_test.py b/testsuite/rrsync-log_test.py index 6b1cc7926..fdb4ea74d 100644 --- a/testsuite/rrsync-log_test.py +++ b/testsuite/rrsync-log_test.py @@ -1,15 +1,10 @@ #!/usr/bin/env python3 -"""rrsync -log must record every transfer in the admin's log file. - -The client's own --log-file is refused while -log is in force, and a FILE -that resolves inside the restricted dir (directly, through a symlink, or in a -bind mount of it) is refused outright, as is -log with DIR /. -""" import os import shlex import shutil import signal +import stat import subprocess import sys @@ -20,146 +15,231 @@ env = {**os.environ, 'SSH_ORIGINAL_COMMAND': ' '.join(sys.argv[i:])} signal.signal(signal.SIGUSR1, signal.SIG_DFL) signal.signal(signal.SIGUSR2, signal.SIG_DFL) - wrapper, root = env['RRSYNC_WRAPPER'], env['RRSYNC_ROOT'] flags = shlex.split(env['RRSYNC_FLAGS']) - os.execve(wrapper, [wrapper, *flags, root], env) + os.execve(env['RRSYNC_WRAPPER'], [env['RRSYNC_WRAPPER'], *flags, env['RRSYNC_ROOT']], env) -from rsyncfns import RSYNC, SCRATCHDIR, makepath, patched_rrsync, rmtree, rsync_argv, rsync_path_arg +from rsyncfns import RSYNC, SCRATCHDIR, makepath, patched_rrsync, rsync_argv, rsync_path_arg, test_fail base = SCRATCHDIR / 'rrsync-log' -rmtree(base) -src = base / 'src' -root = base / 'root' +src, root = base / 'src', base / 'root' makepath(src / 'sub', root) (src / 'a.txt').write_bytes(b'AAA') (src / 'sub' / 'b.txt').write_bytes(b'BB') log = base / 'transfer.log' - -# RSYNC may be a multi-word command (the runner's --protocol=N, or valgrind) -# while rrsync hands its RSYNC to execlp() as a single executable name, so it -# has to be wrapped before patched_rrsync() sees it. shim = base / 'rsync-shim' shim.write_text('#!/bin/sh\nexec ' + rsync_path_arg(RSYNC) + ' "$@"\n') shim.chmod(0o755) wrapper = patched_rrsync(base, rsync_path=str(shim)) rsh = f'{shlex.quote(sys.executable)} {shlex.quote(os.path.abspath(__file__))} --shell' -env = {**os.environ, 'RRSYNC_WRAPPER': str(wrapper), 'RRSYNC_ROOT': str(root), - 'RRSYNC_FLAGS': f'-wo -no-lock -log {shlex.quote(str(log))}'} - -push = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=env, capture_output=True, text=True, timeout=20) -assert push.returncode == 0 and (root / 'sub' / 'b.txt').read_bytes() == b'BB', ( - f'upload with -log failed: rc={push.returncode}, stderr={push.stderr!r}') -lines = log.read_text().splitlines() if log.exists() else [] -for name in ('a.txt', 'sub/b.txt'): - assert any('>f' in line and line.endswith(' ' + name) for line in lines), ( - f'-log did not record {name}: {lines!r}') - -# The admin's log replaces the client's: a client --log-file is an error, not -# a second log the client can point anywhere inside the restricted dir. -client_log = subprocess.run( - rsync_argv('-r', '-M--log-file=mine.log', '-e', rsh, f'{src}/', 'ignored:'), - env=env, capture_output=True, text=True, timeout=20) -assert client_log.returncode != 0 and 'option --log-file has been disabled' in client_log.stderr, ( - f'rrsync -log accepted the client --log-file: rc={client_log.returncode}, ' - f'stderr={client_log.stderr!r}') -assert not (root / 'mine.log').exists() - -# Without -log the client keeps its --log-file, or the refusal above is -# unconditional and says nothing about -log. -plain_env = {**env, 'RRSYNC_FLAGS': '-wo -no-lock'} -plain = subprocess.run( - rsync_argv('-r', '-M--log-file=mine.log', '-e', rsh, f'{src}/', 'ignored:'), - env=plain_env, capture_output=True, text=True, timeout=20) -assert plain.returncode == 0 and (root / 'mine.log').exists(), ( - f'without -log the client --log-file must be accepted: rc={plain.returncode}, ' - f'stderr={plain.stderr!r}') - -# A log inside the restricted dir could be replaced by a client symlink. -inside_log = shlex.quote(str(root / 'transfer.log')) -inside_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {inside_log}'} -inside = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=inside_env, capture_output=True, text=True, timeout=20) -assert inside.returncode != 0 and 'must be outside the restricted directory' in inside.stderr, ( - f'rrsync accepted a -log FILE inside DIR: rc={inside.returncode}, stderr={inside.stderr!r}') -assert not (root / 'transfer.log').exists() - -# A symlink outside DIR is fine (like /var -> /private/var on macOS): the log -# goes where it resolves. -real = base / 'real' -makepath(real) -os.symlink(real, base / 'alias') -alias_log = shlex.quote(str(base / 'alias' / 'transfer.log')) -alias_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {alias_log}'} -alias = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=alias_env, capture_output=True, text=True, timeout=20) -assert alias.returncode == 0 and (real / 'transfer.log').exists(), ( - f'rrsync refused a -log path through a symlink outside DIR: rc={alias.returncode}, ' - f'stderr={alias.stderr!r}') - -# A symlink that resolves into DIR is refused like a FILE inside DIR. + +def transfer(*options, flags=('-wo', '-no-lock'), logfile=log, sender=f'{src}/', receiver='ignored:', script=wrapper): + flags = [*flags] + (['-log', str(logfile)] if logfile is not None else []) + env = {**os.environ, 'RRSYNC_WRAPPER': str(script), 'RRSYNC_ROOT': str(root), 'RRSYNC_FLAGS': shlex.join(flags)} + return subprocess.run(rsync_argv('-rI', *options, '-e', rsh, sender, receiver), + cwd=base, env=env, capture_output=True, text=True, timeout=20) + +def check(result, error=None): + if error is None: + if result.returncode != 0: + test_fail(f'transfer failed: {result.returncode}: {result.stderr}') + elif result.returncode == 0 or error not in result.stderr: + test_fail(f'expected {error!r}: {result.returncode}: {result.stderr}') + +def recorded(path, name, marker): + lines = path.read_text().splitlines() + if not any(marker in line and line.endswith(' ' + name) for line in lines): + test_fail(f'{name} missing from {path}: {lines!r}') + if not any('sent ' in line and ' received ' in line and ' total size ' in line for line in lines): + test_fail(f'transfer summary missing: {lines!r}') + +log.write_text('previous transfer\n') +log.chmod(0o600) +check(transfer(flags=('-wo',))) +if (root / 'sub' / 'b.txt').read_bytes() != b'BB': + test_fail('upload contents differ') +recorded(log, 'a.txt', '>f') +recorded(log, 'sub/b.txt', '>f') +downloaded = base / 'download' +check(transfer(flags=('-ro',), sender='ignored:', receiver=f'{downloaded}/')) +if (downloaded / 'sub' / 'b.txt').read_bytes() != b'BB': + test_fail('download contents differ') +recorded(log, 'a.txt', 'f') +selected_log = base / 'log with spaces' +check(transfer(logfile=selected_log)) +recorded(selected_log, 'a.txt', '>f') +file_list = base / 'files-from' +file_list.write_text('a.txt\n') +selected_log = base / 'files-from.log' +check(transfer(f'--files-from={file_list}', flags=('-ro',), logfile=selected_log, + sender='ignored:', receiver=f'{base}/listed/')) +recorded(selected_log, 'a.txt', 'f') + +makepath(base / 'real' / 'inner') +os.symlink(base / 'real' / 'inner', base / 'alias') +check(transfer(logfile='alias/../relative.log')) +if not (base / 'real' / 'relative.log').is_file() or (base / 'relative.log').exists(): + test_fail('symlink followed by .. resolved to the wrong log') +recorded(base / 'real' / 'relative.log', 'a.txt', '>f') +check(transfer(logfile=base / 'alias' / 'absolute.log')) +if not (base / 'real' / 'inner' / 'absolute.log').is_file(): + test_fail('external symlink log path failed') +recorded(base / 'real' / 'inner' / 'absolute.log', 'a.txt', '>f') +leaf = base / 'leaf' +leaf.symlink_to(base / 'real' / 'leaf.log') +check(transfer(logfile=leaf)) +recorded(base / 'real' / 'leaf.log', 'a.txt', '>f') +double_slash = base / 'double-slash.log' +check(transfer(logfile='/' + str(double_slash))) +recorded(double_slash, 'a.txt', '>f') + os.symlink(root, base / 'into') -into_log = shlex.quote(str(base / 'into' / 'transfer.log')) -into_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {into_log}'} -into = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=into_env, capture_output=True, text=True, timeout=20) -assert into.returncode != 0 and 'must be outside the restricted directory' in into.stderr, ( - f'rrsync accepted a -log path that resolves into DIR: rc={into.returncode}, ' - f'stderr={into.stderr!r}') -assert not (root / 'transfer.log').exists() - -# Nor may the path pass through DIR on its way out: the client's symlink in -# DIR would decide where the log is written, even outside DIR. -stolen = base / 'stolen' -os.symlink(stolen, root / 'f') -os.symlink(root / 'f', base / 'reg') -reg_log = shlex.quote(str(base / 'reg')) -reg_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {reg_log}'} -reg = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=reg_env, capture_output=True, text=True, timeout=20) -assert reg.returncode != 0 and 'must be outside the restricted directory' in reg.stderr, ( - f'rrsync accepted a -log path that passes through DIR: rc={reg.returncode}, ' - f'stderr={reg.stderr!r}') -assert not stolen.exists() - -# A log that cannot be opened stops the transfer instead of being skipped. +os.symlink(base / 'stolen', root / 'out') +os.symlink(root / 'out', base / 'through') +for path in (root / 'transfer.log', base / 'into' / 'transfer.log', base / 'through', root / '..' / 'escaped.log'): + check(transfer(logfile=path), 'must be outside the restricted directory') +if (root / 'transfer.log').exists() or (base / 'stolen').exists() or (base / 'escaped.log').exists(): + test_fail('a rejected log was created') + +if os.geteuid() == 0: + real = base / 'foreign-real' + makepath(real) + foreign_dir = base / 'foreign-dir' + foreign_dir.symlink_to(real) + foreign_leaf = base / 'foreign-leaf' + foreign_leaf.symlink_to(real / 'leaf.log') + for link, path, target in ((foreign_leaf, foreign_leaf, real / 'leaf.log'), + (foreign_dir, foreign_dir / 'parent.log', real / 'parent.log')): + os.lchown(link, 65534, 65534) + target.write_text('unchanged\n') + result = subprocess.run(rsync_argv('-rI', f'--log-file={path}', f'{src}/', f'{root}/'), + capture_output=True, text=True, timeout=20) + if ('refusing to follow a symlink owned by an untrusted user' not in result.stderr + or target.read_text() != 'unchanged\n'): + test_fail('ordinary --log-file ownership control failed') + result = transfer(logfile=path) + if target.read_text() != 'unchanged\n': + test_fail('untrusted -log symlink changed its target') + check(result, 'untrusted symlink in -log FILE') + +regular = base / 'regular' +regular.write_text('not a directory') +os.symlink('regular/../bad.log', base / 'bad-link') +os.symlink('regular/', base / 'bad-slash') +for path in ('regular/../bad.log', 'regular/./bad.log', str(regular) + '/', base / 'bad-link', base / 'bad-slash'): + check(transfer(logfile=path), 'cannot resolve the -log FILE') +if (base / 'bad.log').exists(): + test_fail('an invalid log path was normalised and opened') +os.symlink('loop', base / 'loop') +check(transfer(logfile=base / 'loop'), 'too many symlinks') +check(transfer(logfile=''), 'empty -log FILE') +check(transfer(logfile=base / 'real'), 'cannot open the -log FILE') +check(transfer(logfile='missing/../bad.log'), 'cannot resolve the -log FILE') +if os.geteuid() != 0: + denied = base / 'no-search' + denied.mkdir(mode=0o600) + try: + check(transfer(logfile='no-search/../bad.log'), 'cannot resolve the -log FILE') + finally: + denied.chmod(0o700) + readonly = base / 'readonly.log' + readonly.touch(mode=0o400) + check(transfer(logfile=readonly), 'cannot open the -log FILE') + (root / 'sub' / 'b.txt').unlink() -nolog = shlex.quote(str(base / 'missing-dir' / 'transfer.log')) -nolog_env = {**env, 'RRSYNC_FLAGS': f'-wo -no-lock -log {nolog}'} -unlogged = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=nolog_env, capture_output=True, text=True, timeout=20) -assert unlogged.returncode != 0 and 'cannot open the -log FILE' in unlogged.stderr, ( - f'rrsync ran a transfer it could not log: rc={unlogged.returncode}, ' - f'stderr={unlogged.stderr!r}') -assert not (root / 'sub' / 'b.txt').exists() - -# With DIR / no path is outside it. -slash_env = {**env, 'RRSYNC_ROOT': '/'} -slash = subprocess.run( - rsync_argv('-r', '-e', rsh, f'{src}/', 'ignored:'), - env=slash_env, capture_output=True, text=True, timeout=20) -assert slash.returncode != 0 and '-log cannot be used when DIR is /' in slash.stderr, ( - f'rrsync accepted -log with DIR /: rc={slash.returncode}, stderr={slash.stderr!r}') - -# DIR reached through a bind mount is still DIR. An unprivileged bind mount -# needs a user namespace, so this part runs only where unshare can make one. +check(transfer(logfile=base / 'missing-dir' / 'transfer.log'), 'cannot resolve the -log FILE') +if (root / 'sub' / 'b.txt').exists(): + test_fail('transfer ran despite a failed initial log open') + +if hasattr(os, 'mkfifo'): + fifo = base / 'fifo' + os.mkfifo(fifo) + fd = os.open(fifo, os.O_RDONLY | os.O_NONBLOCK) + try: + check(transfer(logfile=fifo), '-log FILE must be a regular file') + finally: + os.close(fd) + +for command in ('true', 'not-rsync'): + unused = base / 'unused.log' + result = subprocess.run([str(wrapper), '-no-lock', '-log', str(unused), str(root)], + env={**os.environ, 'SSH_ORIGINAL_COMMAND': command}, + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=20) + if result.returncode != (0 if command == 'true' else 1) or unused.exists(): + test_fail(f'non-transfer command opened the log: {result.stderr}') + +result = subprocess.run([str(wrapper), '-no-lock', '-log', str(log), '/'], + env={**os.environ, 'SSH_ORIGINAL_COMMAND': 'rsync --server -r . .'}, + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=20) +check(result, '-log cannot be used when DIR is /') + +for fault in ('directory', 'full', 'foreign'): + if fault == 'full' and not os.path.exists('/dev/full'): + continue + if fault == 'foreign' and os.geteuid() != 0: + continue + directory = base / fault + makepath(directory) + fault_log = directory / 'transfer.log' + fault_shim = directory / 'rsync-shim' + quoted = shlex.quote(str(fault_log)) + change = f'mkdir {quoted}' if fault == 'directory' else f'ln -s /dev/full {quoted}' + if fault == 'foreign': + victim = directory / 'victim' + victim.write_text('unchanged\n') + change = f'ln -s {shlex.quote(str(victim))} {quoted}\nchown -h 65534 {quoted}' + fault_shim.write_text(f'#!/bin/sh\nset -e\nmv {quoted} {quoted}.old\n{change}\nexec ' + + rsync_path_arg(RSYNC) + ' "$@"\n') + fault_shim.chmod(0o755) + fault_wrapper = patched_rrsync(directory, rsync_path=str(fault_shim)) + (src / f'after-{fault}').write_text('transferred') + result = transfer(script=fault_wrapper, logfile=fault_log) + check(result) + if (root / f'after-{fault}').read_text() != 'transferred': + test_fail('best-effort logging stopped the transfer') + if fault == 'directory' and 'failed to open log-file' not in result.stderr: + test_fail(f'log reopen failure was not reported: {result.stderr}') + if (fault == 'foreign' and (victim.read_text() != 'unchanged\n' + or 'refusing to follow a symlink owned by an untrusted user' not in result.stderr)): + test_fail('the log reopen bypassed symlink ownership checks') + unshare = shutil.which('unshare') userns = [unshare, '--user', '--map-root-user', '--mount'] if unshare else None -if userns and subprocess.run(userns + ['true'], capture_output=True).returncode == 0: +if userns and subprocess.run(userns + ['true'], capture_output=True, timeout=10).returncode == 0: bind = base / 'bind' makepath(bind) - script = 'mount --bind "$1" "$2" && exec "$3" -wo -no-lock -log "$2/transfer.log" "$1"' - bound = subprocess.run( - userns + ['sh', '-c', script, 'sh', str(root), str(bind), str(wrapper)], - env={**os.environ, 'SSH_ORIGINAL_COMMAND': 'rsync --server -r . .'}, - stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=20) - assert bound.returncode != 0 and 'must be outside the restricted directory' in bound.stderr, ( - f'rrsync accepted a -log FILE in a bind mount of DIR: rc={bound.returncode}, ' - f'stderr={bound.stderr!r}') - assert not (root / 'transfer.log').exists() + command = 'mount --bind "$1" "$2" && exec "$3" -wo -no-lock -log "$2/transfer.log" "$1"' + result = subprocess.run(userns + ['sh', '-c', command, 'sh', str(root), str(bind), str(wrapper)], + env={**os.environ, 'SSH_ORIGINAL_COMMAND': 'rsync --server -r . .'}, + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=20) + check(result, 'must be outside the restricted directory')