mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-09-17 07:38:57 -04:00
rrsync: pin a sender argument where rsync will actually resolve it
88cee089rewrote every validated argument to /proc/self/fd/N so the spawned rsync re-resolves it to the pinned inode. That is right for a receiver, which open()s its destination, but a sender never opens its source argument: send_file_list() lstat()s it first, and lstat of a procfs magic link is always S_IFLNK. So the sender described the argument as a symlink and sent no data -- silent data loss on "rsync -a user@host:file dest/", the most ordinary command there is. Of the argument shapes now covered, only a trailing-slash directory survived. Leonid Bugaev reported the regression, diagnosed the lstat-vs-magic-link mechanism, and supplied the first regression test. Which pin is usable depends on what rsync does with the argument: * a trailing "/" or "/." directory is opened, not lstat()ed, and rsync does follow a symlink there, so it keeps the leaf pin -- verified: without it a flipped leaf transfers the outside directory's content; * anything else pins one level up and passes the leaf by name. rsync will not follow a symlink at that position (it sends the symlink itself), -L/-k/--copy-unsafe-links are already disabled for a restricted dir, and rsync's own leaf open is O_NOFOLLOW. The directory pin resolves normally, including a symlink at its last component, which is legitimate and which 3.4.4 accepts; the readlink check afterwards is what proves the held inode is in-tree. It uses O_PATH because reaching a known name beneath a directory needs only search permission, and a mode 0111 parent is an ordinary way to publish a file without letting it be listed. Under --relative the transmitted name is the whole argument rather than its basename, so the pin moves up to where that name starts and the rest is spelled after a /./ marker. The client's own first marker wins if it supplied one, including when nothing follows it; -R is parsed out of the short-option cluster rather than sniffed for the letter, so the trailing capability blob (-e.iLsfxC) cannot be mistaken for it. Directory pins are keyed by (st_dev, st_ino), so a glob whose matches share a parent inherits one descriptor rather than one per argument. Every shape now delivers what a pristine 3.4.4 delivers, which is what rrsync-pull-arg-shapes asserts -- it passes against 3.4.4 itself, so the expectations are that behaviour and not this implementation's. The "-R --no-implied-dirs" case is gated on protocol >= 30: at protocol 29 the receiver rejects it with "invalid path from sender" and transfers nothing, which 3.4.4 does identically. Moving the sender's pin off the leaf invalidates rrsync-symlink's oracle, so it is reworked here rather than left failing. It patches rsync to a stub that open()s its last argument, which is a faithful model for an intermediate path component -- whatever rsync does with the final name, it must not reach it through a flipped parent -- but not for the leaf: a sender lstat()s its source and transmits a symlink there rather than reading through it. So it now flips an intermediate directory, and the leaf is covered against the real binary by rrsync-sender-leaf-flip, which races both a plain file argument and a trailing-slash directory and asserts no outside CONTENT is delivered rather than requiring a symptom from a race that may not be won on a given run. Its trailing-slash half is RED against a pristine 3.4.4 rrsync, which delivers outside/dir/loot. rrsync-symlink is now sender-only. Measured over a 5s race, the stub reached the outside marker 28 times in 98 runs as a sender and 25 in 97 as a receiver against 3.4.4; with the pin it is 0 as a sender but still 7-9 as a receiver, on the branch base as well as here. That residual is the receiver's not-yet-existing-destination fallback, which predates this work and is tracked in #139 rather than folded in. Clearing FD_CLOEXEC goes through F_SETFD rather than os.set_inheritable(), which prefers ioctl(FIONCLEX) and gets EBADF from an O_PATH descriptor on older kernels -- every sender pull on Ubuntu 18.04 aborted with "Bad file descriptor" the moment a directory pin was taken. Co-authored-by: Leonid Bugaev <leonsbox@gmail.com> (cherry picked from commit6edb7dea2a)
This commit is contained in:
1 parent
0875194e7b
commit
0894e0422e
1 file changed
+177
-4
+177
-4
@@ -142,6 +142,14 @@ from argparse import RawTextHelpFormatter
|
||||
# TOCTOU.
|
||||
pinned_fds = []
|
||||
|
||||
# Directory pins, keyed by (st_dev, st_ino), so a glob or a multi-arg command
|
||||
# whose args share a parent inherits one fd rather than one per arg.
|
||||
pinned_dirs = {}
|
||||
|
||||
# Whether the client asked for --relative/-R, which decides how much of a
|
||||
# sender arg rsync transmits as the file's name (see sender_pinned_arg).
|
||||
client_relative = False
|
||||
|
||||
# The inode-pin trick needs /proc/self/fd/N to be a Linux-style magic symlink
|
||||
# whose readlink yields the open file's real path. macOS/BSD lack the directory
|
||||
# entirely; Solaris HAS /proc/self/fd but its entries are not such symlinks (its
|
||||
@@ -172,6 +180,134 @@ HAS_DOT_DOT_RE = re.compile(r'(^|/)\.\.(/|$)')
|
||||
LONG_OPT_RE = re.compile(r'^--([^=]+)(?:=(.*))?$')
|
||||
DE_BACKSLASH_RE = re.compile(r'\\(.)')
|
||||
|
||||
def make_inheritable(fd):
|
||||
"""Clear FD_CLOEXEC so the exec'd rsync inherits `fd`.
|
||||
|
||||
os.set_inheritable() prefers ioctl(FIONCLEX), which an O_PATH descriptor
|
||||
rejects with EBADF on older kernels; F_SETFD is one of the few operations
|
||||
O_PATH always allows.
|
||||
"""
|
||||
try:
|
||||
os.set_inheritable(fd, True)
|
||||
except OSError:
|
||||
import fcntl
|
||||
fcntl.fcntl(fd, fcntl.F_SETFD,
|
||||
fcntl.fcntl(fd, fcntl.F_GETFD) & ~fcntl.FD_CLOEXEC)
|
||||
|
||||
def pin_dir(path, orig_arg):
|
||||
"""Inode-pin a directory and return an fd rsync will inherit.
|
||||
|
||||
The open resolves `path` normally -- including a symlink at its last
|
||||
component, which is legitimate and which 3.4.4 accepts -- so a component
|
||||
could be flipped first. The readlink check afterwards is what makes that
|
||||
safe: it proves the inode we ended up holding is inside the restricted
|
||||
tree. From then on the fd names that inode, not the path, so nothing above
|
||||
it can be flipped again.
|
||||
|
||||
O_PATH, not O_RDONLY: reaching a known name beneath a directory needs only
|
||||
search permission, and a mode 0111 parent is a perfectly ordinary way to
|
||||
publish a file without letting it be listed. An O_PATH directory fd is
|
||||
just as firmly pinned when used as a /proc/self/fd/N/... prefix (the O_PATH
|
||||
caveat in validated_arg() is about reopening the magic link as the file
|
||||
itself, which is not what happens here).
|
||||
"""
|
||||
flags = os.O_DIRECTORY | getattr(os, 'O_PATH', 0)
|
||||
if not flags & getattr(os, 'O_PATH', 0):
|
||||
flags |= os.O_RDONLY
|
||||
try:
|
||||
fd = os.open(path or '.', flags)
|
||||
except OSError as e:
|
||||
die('unable to pin sender path:', orig_arg, e.strerror)
|
||||
try:
|
||||
st = os.fstat(fd)
|
||||
pinned_path = os.readlink('/proc/self/fd/%d' % fd)
|
||||
except OSError as e:
|
||||
os.close(fd)
|
||||
die('post-pin readlink failed (race?):', orig_arg, e.strerror)
|
||||
if pinned_path != args.dir and not pinned_path.startswith(args.dir_slash):
|
||||
os.close(fd)
|
||||
die('post-pin path escaped tree (race?):', orig_arg, pinned_path)
|
||||
key = (st.st_dev, st.st_ino)
|
||||
if key in pinned_dirs:
|
||||
os.close(fd)
|
||||
return pinned_dirs[key]
|
||||
make_inheritable(fd)
|
||||
pinned_fds.append(fd)
|
||||
pinned_dirs[key] = fd
|
||||
return fd
|
||||
|
||||
# sender_pinned_arg() verdicts that are not a rewritten argument.
|
||||
KEEP_LEAF_PIN = 'keep' # hand rsync the leaf's own /proc/self/fd/N
|
||||
LEAF_PIN_UNUSABLE = 'unusable' # no pin for this shape; keep the plain name
|
||||
|
||||
def sender_pinned_arg(fd, arg, orig_arg, has_slash, has_slash_dot):
|
||||
"""Return a source name rsync will both resolve safely and name correctly.
|
||||
|
||||
The obvious rewrite -- hand rsync /proc/self/fd/N for the leaf itself --
|
||||
only works where rsync open()s the argument. A sender lstat()s it first,
|
||||
and lstat of a procfs magic link is always S_IFLNK, so rsync describes the
|
||||
argument as a symlink instead of sending the file. Which pin is usable
|
||||
therefore depends on what rsync does with the argument:
|
||||
|
||||
* a trailing "/" or "/." directory is opened, not lstat()ed, and rsync
|
||||
DOES follow a symlink there, so it keeps the leaf pin;
|
||||
* anything else keeps the pin one level up and passes the leaf by name.
|
||||
rsync will not follow a symlink at that position (it sends the symlink
|
||||
itself), the follow options that would change that are already disabled
|
||||
for a restricted dir, and its own leaf open is O_NOFOLLOW.
|
||||
|
||||
Under --relative the transmitted name is the whole argument rather than
|
||||
its basename, so the pin has to move up to wherever that name starts and
|
||||
the rest is spelled after a /./ marker, which is how rsync is told where
|
||||
the transmitted portion begins.
|
||||
"""
|
||||
if client_relative:
|
||||
# Look for the marker in the argument as the client spelled it: the
|
||||
# caller has already split off a trailing "/" or "/.", which is exactly
|
||||
# what turns "sub/./" into a terminal marker.
|
||||
full = arg + ('/' if has_slash else '/.' if has_slash_dot else '')
|
||||
# rsync honours the FIRST /./, so split there and keep the client's
|
||||
# marker rather than inserting a second, earlier one.
|
||||
head, marker, tail = full.partition('/./')
|
||||
if marker:
|
||||
anchor, suffix = head, tail.rstrip('/')
|
||||
else:
|
||||
anchor, suffix = '', arg
|
||||
if suffix.startswith('/'):
|
||||
return LEAF_PIN_UNUSABLE
|
||||
if suffix in ('', '.'):
|
||||
# A terminal marker: everything the client wants transmitted starts
|
||||
# at the argument itself, which is the directory we already pinned.
|
||||
# The caller re-appends the trailing "/" or "/.".
|
||||
dfd = pin_dir(anchor, orig_arg)
|
||||
check = '.'
|
||||
pinned = '/proc/self/fd/%d/.' % dfd
|
||||
else:
|
||||
dfd = pin_dir(anchor, orig_arg)
|
||||
pinned = '/proc/self/fd/%d/./%s' % (dfd, suffix)
|
||||
check = suffix
|
||||
else:
|
||||
if has_slash or has_slash_dot:
|
||||
return KEEP_LEAF_PIN
|
||||
anchor, _, leaf = arg.rpartition('/')
|
||||
if not leaf or leaf in ('.', '..'):
|
||||
return LEAF_PIN_UNUSABLE
|
||||
dfd = pin_dir(anchor, orig_arg)
|
||||
pinned = '/proc/self/fd/%d/%s' % (dfd, leaf)
|
||||
check = leaf
|
||||
|
||||
# Tie the pinned directory to the inode realpath() validated: resolving
|
||||
# `check` beneath the held fd cannot be redirected above the leaf, so if it
|
||||
# does not reach the same file, something was flipped -- fail closed.
|
||||
try:
|
||||
st = os.stat(check, dir_fd=dfd)
|
||||
except OSError as e:
|
||||
die('post-pin stat failed (race?):', orig_arg, e.strerror)
|
||||
leaf_st = os.fstat(fd)
|
||||
if (st.st_dev, st.st_ino) != (leaf_st.st_dev, leaf_st.st_ino):
|
||||
die('post-pin path changed (race?):', orig_arg, check)
|
||||
return pinned
|
||||
|
||||
def safe_open_logfile():
|
||||
nofollow = getattr(os, 'O_NOFOLLOW', 0)
|
||||
try:
|
||||
@@ -253,6 +389,7 @@ def main():
|
||||
except OSError as e:
|
||||
die('unable to chdir to restricted dir:', str(e))
|
||||
|
||||
global client_relative
|
||||
rsync_opts = [ '--server' ]
|
||||
rsync_args = [ ]
|
||||
saw_the_dot_arg = False
|
||||
@@ -275,7 +412,16 @@ def main():
|
||||
saw_the_dot_arg = True
|
||||
continue
|
||||
rsync_opts.append(arg)
|
||||
if short_no_arg_re.match(arg) or short_with_num_re.match(arg):
|
||||
sm = short_no_arg_re.match(arg)
|
||||
if sm or short_with_num_re.match(arg):
|
||||
if sm:
|
||||
# Scan the cluster's own letters only: the trailing
|
||||
# capability blob (-e.iLsfxC) is not a set of options.
|
||||
letters = arg[1:]
|
||||
if sm.group(1):
|
||||
letters = letters[:-len(sm.group(1))]
|
||||
if 'R' in letters:
|
||||
client_relative = True
|
||||
continue
|
||||
disabled = False
|
||||
m = LONG_OPT_RE.match(arg)
|
||||
@@ -285,6 +431,11 @@ def main():
|
||||
ct = long_opts.get(opt, None)
|
||||
if ct is None:
|
||||
break # Generate generic failure due to unfinished arg parsing
|
||||
# Last one wins, matching rsync's own option handling.
|
||||
if opt == 'relative':
|
||||
client_relative = True
|
||||
elif opt == 'no-relative':
|
||||
client_relative = False
|
||||
if ct == 0:
|
||||
continue
|
||||
opt = '--' + opt
|
||||
@@ -384,6 +535,7 @@ def validated_arg(opt, arg, typ=3, wild=False):
|
||||
for arg in got:
|
||||
if args.dir != '/' and arg != '.' and (typ == 3 or (typ == 2 and not am_sender)):
|
||||
arg_has_trailing_slash = arg.endswith('/')
|
||||
arg_has_trailing_slash_dot = False
|
||||
if arg_has_trailing_slash:
|
||||
arg = arg[:-1]
|
||||
else:
|
||||
@@ -498,9 +650,30 @@ def validated_arg(opt, arg, typ=3, wild=False):
|
||||
os.close(fd)
|
||||
die('post-pin path escaped tree (race?):',
|
||||
orig_arg, pinned_path)
|
||||
os.set_inheritable(fd, True)
|
||||
pinned_fds.append(fd)
|
||||
arg = '/proc/self/fd/%d' % fd
|
||||
if am_sender and opt == 'arg':
|
||||
logical = arg
|
||||
if is_absolute_arg:
|
||||
if logical == args.dir:
|
||||
logical = ''
|
||||
elif logical.startswith(args.dir_slash):
|
||||
logical = logical[args.dir_slash_len:]
|
||||
pinned = sender_pinned_arg(fd, logical, orig_arg,
|
||||
arg_has_trailing_slash,
|
||||
arg_has_trailing_slash_dot)
|
||||
else:
|
||||
pinned = KEEP_LEAF_PIN
|
||||
if pinned == KEEP_LEAF_PIN:
|
||||
os.set_inheritable(fd, True)
|
||||
pinned_fds.append(fd)
|
||||
arg = '/proc/self/fd/%d' % fd
|
||||
elif pinned == LEAF_PIN_UNUSABLE:
|
||||
# Nothing to spell beneath a held directory (a bare "."
|
||||
# or the tree root): keep the realpath-validated name,
|
||||
# which is what 3.4.4 passes.
|
||||
os.close(fd)
|
||||
else:
|
||||
os.close(fd) # only needed to validate the pin
|
||||
arg = pinned
|
||||
if arg_has_trailing_slash:
|
||||
arg += '/'
|
||||
elif arg_has_trailing_slash_dot:
|
||||
|
||||
Reference in new issue
Block a user