From 5eb99bb6b2bfeb7aeec844656a124c38f7adca23 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 25 Jul 2026 17:37:05 +1000 Subject: [PATCH] exclude: exempt the daemon's own filter parameters from the confinement Confining every parse_filter_file() open to the module root also caught "filter", "include from" and "exclude from" from rsyncd.conf. Those name operator-configured paths and pointing them outside the module -- at /etc/rsync/excludes, say -- is the ordinary way to write them; rsyncd.conf(5) puts no constraint on where the file lives. The result was not a refused rule but a refused connection: failed to open exclude file /etc/rsync/excludes: Too many levels of symbolic links (40) rsync error: error in file IO (code 11) at exclude.c(1582) with no symlink involved anywhere -- just a regular file outside the module. Mark the window in which the daemon loads its own parameters and skip the confinement there. Everything else, in particular the peer-driven dir-merge the leak test exercises, is still confined. Also fix the trailing whitespace in the original hunk. --- clientserver.c | 8 +++ exclude.c | 21 ++++++- ...daemon-exclude-from-outside-module_test.py | 60 +++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 testsuite/daemon-exclude-from-outside-module_test.py diff --git a/clientserver.c b/clientserver.c index 8597d6d5..b5436e85 100644 --- a/clientserver.c +++ b/clientserver.c @@ -42,6 +42,7 @@ extern int munge_symlinks; extern int use_secure_symlinks; extern int open_noatime; extern int sanitize_paths; +extern int daemon_config_filter_file; extern int numeric_ids; extern int filesfrom_fd; extern int remote_protocol; @@ -902,6 +903,11 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char } else set_filter_dir(module_dir, module_dirlen); + /* Everything loaded from here to the end of the exclude block is the + * operator's own configuration, so it keeps the ownership walk without the + * module-confinement parse_filter_file() applies to peer-driven merges. */ + daemon_config_filter_file = 1; + p = lp_filter(module_id); parse_filter_str(&daemon_filter_list, p, rule_template(FILTRULE_WORD_SPLIT), XFLG_ABS_IF_SLASH | XFLG_DIR2WILD3); @@ -923,6 +929,8 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char parse_filter_str(&daemon_filter_list, p, rule_template(FILTRULE_WORD_SPLIT), XFLG_ABS_IF_SLASH | XFLG_DIR2WILD3 | XFLG_OLD_PREFIXES); + daemon_config_filter_file = 0; + log_init(1); #if defined HAVE_SETENV || defined HAVE_PUTENV diff --git a/exclude.c b/exclude.c index 4586b71b..7d78e3a2 100644 --- a/exclude.c +++ b/exclude.c @@ -43,6 +43,9 @@ extern int trust_sender_args; extern int module_id; extern int operator_path_resolve; +/* Set while the daemon loads its own filter parameters; see parse_filter_file(). */ +int daemon_config_filter_file = 0; + extern char curr_dir[MAXPATHLEN]; extern unsigned int curr_dir_len; extern unsigned int module_dirlen; @@ -1554,12 +1557,24 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_ open_path = line; } else open_path = fname; - + + /* Confine the open to the module root. The ownership walk on its own + * is not enough for a peer-driven merge file: a non-chrooted daemon + * writes --backup-dir entries as root, so a raced backup symlink is + * ROOT-owned -- exactly what open_no_attacker_symlinks() treats as + * trusted -- and naming it in a dir-merge rule would read an + * out-of-module file in as filter rules (their text comes back to the + * peer in "Unknown filter rule" errors). + * + * The daemon's own "filter"/"include from"/"exclude from" parameters + * are exempt: those are operator-configured and legitimately live + * outside the module (/etc/rsync/excludes and the like). */ int save_opr = operator_path_resolve; - operator_path_resolve = 1; + if (!daemon_config_filter_file) + operator_path_resolve = 1; fd = open_no_attacker_symlinks(open_path, O_RDONLY, 0); operator_path_resolve = save_opr; - + if (fd < 0) fp = NULL; else if (!(fp = fdopen(fd, "rb"))) diff --git a/testsuite/daemon-exclude-from-outside-module_test.py b/testsuite/daemon-exclude-from-outside-module_test.py new file mode 100644 index 00000000..3a86af67 --- /dev/null +++ b/testsuite/daemon-exclude-from-outside-module_test.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +"""A daemon's own "exclude from" file may live outside the module root. + +"exclude from" / "include from" / "filter" name operator-configured paths, and +pointing them at something like /etc/rsync/excludes is the ordinary way to +write them -- rsyncd.conf(5) puts no constraint on where the file lives. + +parse_filter_file() confines a PEER-driven merge file to the module root, since +a non-chrooted daemon writes --backup-dir entries as root and a raced backup +symlink is therefore root-owned, which the ownership walk trusts (see +filter-leak). That confinement must not extend to the operator's own +parameters: applying it to these three refuses the file outright and takes the +whole connection down with it, for a config that has nothing to do with the +attack. +""" + +import subprocess + +from rsyncfns import ( + SCRATCHDIR, makepath, rmtree, rsync_argv, start_test_daemon, test_fail, + write_daemon_conf, +) + +PORT = 12947 + +base = SCRATCHDIR / 'exclude-from-outside' +rmtree(base) +module = base / 'module' +etc = base / 'etc' +dest = base / 'dest' +makepath(module, etc, dest) + +(module / 'keep.txt').write_text('KEEP\n') +(module / 'drop.txt').write_text('DROP\n') +# Deliberately a sibling of the module root, not inside it. +(etc / 'excludes').write_text('drop.txt\n') + +conf = write_daemon_conf([ + ('m', { + 'path': str(module), + 'read only': 'yes', + 'use chroot': 'no', + 'exclude from': str(etc / 'excludes'), + }), +], name='exclude-from-outside.conf') +url = start_test_daemon(conf, PORT) + +proc = subprocess.run( + rsync_argv('-r', f'{url}m/', str(dest) + '/'), + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) +ctx = f'rc={proc.returncode}, output={proc.stdout.strip()[:300]!r}' + +if proc.returncode != 0: + test_fail(f'daemon refused an "exclude from" file outside the module root ({ctx})') + +got = sorted(p.name for p in dest.rglob('*') if p.is_file()) +if got != ['keep.txt']: + test_fail(f'expected only keep.txt to transfer, got {got} ({ctx})') + +print('an operator "exclude from" outside the module root is still honoured')