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

@@ -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

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")))

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')