The base gained 99 commits since the last rebase. Nine files needed a
hand-port because the change lands on code the VFS split moved or
renamed, and syscall.c no longer exists here:
- backup.c: make_path() now runs on a private copy of backup_dir_buf
(c933f622), so vfs_make_path() takes dirbuf and drops the restore.
- clientserver.c: keep both the module-root snapshot and the new
daemon_config_filter_file window.
- exclude.c: the peer-driven merge-file confinement is the
vfs_open_owner_walk() is_operator argument, not a global.
- fileio.c: the coalesced --sparse writer's new helpers use
vfs_lseek()/vfs_punch_hole().
- generator.c/vfs/mknod.c: gen_entry_mknod() falls back through
vfs_mknod(); the atfd path keys its mknodat() off HAVE_MKNODAT.
- receiver.c: secure_recv_open() passes VFS_OPERATOR_PATH instead of
toggling operator_path_resolve; open_readonly_inplace() uses the VFS
stat/chmod/open wrappers.
- sender.c: absolute --relative cleanup anchors at "/" via
vfs_resolve_open(), the copy-links walk uses
vfs_resolve_open_at_beneath(), and the source removal goes through
vfs_unlink().
- vfs/chmod.c, vfs/chown.c: VFS_OPERATOR_PATH now takes the ownership
walk, and the no-follow chmod grows the non-Linux fd path.
- vfs/secure_open.c: the fd-anchored resolver splits into a shared
internal with an allow-dotdot entry point, and secure_walk_at() routes
a literal "."/".." through ds_descend() before the leaf fast paths.
Tree is byte-identical to the merge oracle (tag merge-reference-10).
Give vfs_open_owner_walk an explicit is_operator argument so its module-root
confinement (abspath_excluded_by_module) reads the policy from the caller
instead of the vfs.operator_path_resolve global -- the last owner-walk
function still reading it. secure_basis_open gains an is_operator parameter
threaded to it; its gate (owner-walk vs strict resolve) now branches on that
param too.
Faithful conversion (no behaviour change): every direct caller passes the
value the global held at that site -- config/log/motd/early-input/files-from/
batch/connection/exclude/authenticate/params, change_dir's daemon dest-chdir,
and vfs_secure_mkstemp's --temp-dir all pass 0; secure_basis_open passes
VFS_OPERATOR_PATH only for the operator cases (a --partial-dir basis,
fnamecmp_type == FNAMECMP_PARTIAL_DIR; and one_inplace partial-dir staging).
This preserves the existing --temp-dir behaviour (unconfined) by deliberate
choice; hardening that is a separate decision.
The receiver partial-dir-basis and one_inplace operator_path_resolve blocks
are removed (receiver.c is now free of the global). Set-sites: 7 -> 5
(backup make_backup, generator 1071/2085/2130, util1 handle_partial_dir).
The operator-path-partial-dir-daemon test caught a real regression mid-change
(secure_basis_open's gate still read the global after its set/clear block was
removed, dropping the confinement); fixed by gating on is_operator.
Full suite 190/50.
Move the logical-cwd globals into vfs.curr_dir / vfs.curr_dir_len. The
definition leaves syscall.c, the per-file `extern char curr_dir[]` /
`extern unsigned int curr_dir_len` declarations are dropped (the struct
is reached via the vfs.h `extern struct vfs vfs`), and the uses across
exclude/flist/log/main/util1 and vfs/secure_open are updated directly --
no compatibility alias macros. change_dir() writes vfs.curr_dir. The
separate curr_dir_depth global is untouched. No behavior change.
Relocate the operator-supplied-path resolver out of syscall.c into
vfs/owner_walk.c with the vfs_* public names:
open_no_attacker_symlinks -> vfs_open_owner_walk
owner_walk_parent -> vfs_owner_walk_parent
The static helpers ona_open and abspath_step move with them, as does the
operator_path_resolve flag definition (commit-8 will fold it into the vfs
struct). Function bodies are unchanged; the call sites across the daemon
and option-parsing files are updated and the two entry points declared in
vfs/vfs.h.
With the owner-walk gone, syscall.c no longer references am_daemon, so
the now-dead `extern int am_daemon` declarations (the do_*_at wrappers
delegate to vfs_relpath_active) are dropped. No behavior change.
The exclude-self rule that a ":e" merge synthesizes is built by hand with
new0(), so it inherited no flags. While the merge file was still being
parsed the global parse state masked that, but once parsing finished the
stored rule looked argument-origin, and report_filter_result() printed its
pattern -- a merge file's own text -- verbatim:
[sender] hiding file PAT-x9 because of pattern PAT-[x]9 [per-dir ...]
Plain -vv reaches this on a stock client; no --debug is involved. That is
the fifth site of this shape, and the first to get there by constructing a
rule rather than by printing one, so the redaction helper could not catch it.
Also fix the location a per-directory merge reports. Its fname points into
dirbuf, which is cut back to the directory before the name was saved, so the
error said "<rule from .../src/ line 1>" instead of naming .rsync-filter --
no leak, but it breaks the "redact what, keep where" bargain the rest of this
work depends on. Save the name before the truncation.
The rrsync test's claim to close "the rest of the FILTER trace family" was
too strong and is corrected: options.c maps verbosity onto the debug flags,
so -vvv still raises a restricted server to FILTER2 and its trace metadata
comes back. Rule text stays redacted at every verbosity, which is the
property that matters; -vvv is added to the unaffected-transfer cases.
A filter rule that fails to parse was printed back verbatim. When the
rule came from a file rather than an argument, that text is file
CONTENT, and the peer picks which file gets merged: a per-directory
merge rule travels over the protocol, so no argument of ours ever names
it and nothing a wrapper can see mentions it either. Any line that is
not valid filter syntax therefore came straight back to the peer -- a
read-any-line oracle over an rrsync restricted account or a daemon
module, neither of which confines the merge open.
The syntax errors turned out to be the smaller half. The MATCH trace
names the pattern that acted, and report_filter_result() logs at level 1
for a sender or generator, so plain -vv -- no --debug, nothing a stock
client cannot send -- returns a server-side merge file's rules:
[generator] protecting file X because of pattern <the file's text>
So provenance is carried on the rule itself (FILTRULE_FROM_FILE), not
just in the parser: a deferred ":" merge is processed long after the
file that named it was read, and its own name is file content too.
TEXT_FROM_FILE() consults the parse-time context and the rule, so both
the immediate and the deferred paths redact.
Rather than test the provenance at each message -- which is how the last
few of these were found, one at a time, after the ones before them were
fixed -- every string that is or is built from a rule's own text goes
through rule_text(). It returns the text for an argument-supplied rule
and a description of where it came from otherwise, so a message added
later cannot reintroduce the leak by forgetting to check, and there is
one place to audit. rule_detail() does the same for the extra detail a
message adds ABOUT the text: a character of it, an offset into it, the
[not found] bit.
Thirteen sites now route through them: the syntax errors; the modifier
character (one byte of the file, a slower oracle but still one); the
failed-open and merge-depth messages, whose pathname is file content
whenever a rule named it -- and errno with them, since it answers "does
this path exist"; both over-long messages, the deferred one of which
needed no verbosity at all; both merge-name overflows; the long-named
directory error; the [not found] openability bit; the match trace; the
add_rule, parse_filter_file and daemon-hidden traces; and the per-dir
mergelist label, which had the name baked in.
rule_detail() covers more than it first looks: the trailing-whitespace
CAUTION is computed from the rule's last byte, and "hidden by daemon
filter" distinguishes a daemon-filter rejection from an ordinary open,
so both would answer questions about text the peer cannot see.
The regression proves the chokepoint rather than the sites: making
rule_text() return its input unconditionally fails the test. It also
pins what must NOT change for the user's own rules -- the whitespace
warning still fires, and an over-long argument rule is still reported at
full length (the helper buffers at BIGPATHBUFLEN, as rprintf does, so
redaction does not quietly truncate what the user typed).
Bounded and left alone: the numeric rflags in the FILTER2 trace and the
in/exclude wording still describe a file-derived rule without quoting
it, and the daemon's own FLOG line records the name it filtered -- that
one goes to the operator's log, not the peer.
Rules given AS arguments are still echoed in full -- that text is the
user's own, and hiding it would only make ordinary typos harder to fix.
Where a rule did come from a file, the diagnostic names the file and
line instead, which is more useful anyway.
Two things the location itself needed: fname can point into
parse_merge_name()'s static buffer, which a merge rule inside the same
file overwrites while we are still reading it, so a rule after a nested
merge was blamed on the nested file -- keep our own copy. And a CRLF
pair was counted as two line endings while word-split mode counted
tokens rather than lines, so the number pointed at nothing; consume the
LF of a CRLF (preserving the byte for the next rule if pushback ever
fails), and report word-split sources without a line number.
Not covered, deliberately: a rule's provenance is not serialized by
send_filter_list(), so it does not survive to the far side. That is
right -- only the client sends that list, and the server already knows
the patterns the peer gave it.
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.
The CLEAR_LIST guard in parse_rule_tok checked rule->rflags for
FILTRULE_NO_PREFIXES, but NO_PREFIXES is a template-level flag
excluded from FILTRULES_FROM_CONTAINER inheritance. The guard
was always true for CVS rules, causing RERR_SYNTAX abort instead
of clearing the list.
Fix: check template->rflags instead of rule->rflags.
Regression test: testsuite/ki73-cvs-clear-list_test.py
Refuse malformed/hostile wire input that could crash or corrupt the receiver:
io.c (out-of-range file index, count*blength OFF_T overflow, read_args NUL room,
deferred in_multiplexed), flist.c (sub-flist after the final flist is freed,
FLAG_HLINKED on dirs / gated on preserve_hard_links, parent_ndx bound, cleared-
slot ndx in the transfer phase), hlink.c (undeclared cross-flist gnum -> error not
assert), match.c (clamp peer flength, re-check len before want_i), log.c (drop
peer-reachable asserts and F_SUM deref), exclude.c (merge-file recursion cap,
trailing-backslash heap fix), lib/pool_alloc.c (ASan redzone for pool underflow).
Glibc 2.43 added C23 const-preserving overloads to various string functions,
which change the return type depending on the constness of the argument(s).
Currently this leads to warnings from calls to strtok() or strchr().
Fix this by properly declaring the respective variable types.
Signed-off-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Fortified (-D_FORTIFY_SOURCE=2 for gcc) builds make strlcpy() crash when
its third parameter (size) is larger than the buffer:
$ rsync -FFXHav '--filter=merge global-rsync-filter' Align-37-43/ xxx
sending incremental file list
*** buffer overflow detected ***: terminated
It's in the exclude code in setup_merge_file():
strlcpy(y, save, MAXPATHLEN);
Note the 'y' pointer was incremented, so it no longer points to memory
with MAXPATHLEN "owned" bytes.
Fix it by remembering the number of copied bytes into the 'save' buffer
and use that instead of MAXPATHLEN which is clearly incorrect.
Fixes#511.
- Avoid implied rules on generator and (with extra certainty) on server
- Add -R implied-directory path elements as directory includes
- Log about extra file-list checking using a new --debug=FILTER3 level
- All the memory-allocation macros now auto-check for failure and exit
with a failure message that incudes the caller's file and lineno
info. This includes strdup().
- Added the `--max-alloc=SIZE` option to be able to override the memory
allocator's sanity-check limit. It defaults to 1G (as before).
Fixes bugzilla bug 12769.
- Change default_cvsignore char[] into a define.
- Make the DEFAULT_DONT_COMPRESS and DEFAULT_CVSIGNORE defines get set
based on their info in rsync.1.md.
- Add a few more don't-compress suffixes from Simon Matter.
Andrew Tridgell