syscall: use O_PATH for held directory traversal

SECURITY.md

@@ -146,28 +146,29 @@ following symlinks by design.
 
 ### The mechanism
 
-Resolution of attacker-influenceable paths goes through `secure_relative_open()`
-and the `do_*_at()` wrappers in `syscall.c`, never a raw `open()`/`rename()`/
-`chmod()` on a full path string. The principle is: **trust the operator-named
-transfer root, and confine all resolution beneath it**, rejecting escapes via
-`..` above the anchor, absolute symlinks, or out-of-tree symlinks.
-`secure_relative_open()` resolves the parent directory by walking it one
-component at a time on a stack of held directory fds, then operates on the final
-component with an at-style call on the resulting directory fd.
+Resolution of attacker-influenceable paths goes through `secure_relative_open()`,
+`secure_relative_dirfd()`, and the `do_*_at()` wrappers in `syscall.c`, never a
+raw `open()`/`rename()`/`chmod()` on a full path string. The principle is:
+**trust the operator-named transfer root, and confine all resolution beneath
+it**, rejecting escapes via `..` above the anchor, absolute symlinks, or
+out-of-tree symlinks. `secure_relative_open()` opens the resolved endpoint with
+the caller's requested access. `secure_relative_dirfd()` instead returns
+traversal authority for `fchdir()` or an at-style operation on a known child;
+it does not imply permission to enumerate the directory.
 
 For per-entry work the receiver and generator go one step further and hold the
 parent directory open: `open_dir_secure()` resolves an entry's directory once
-(via `secure_relative_open()`), `held_dfd_for()` caches that descriptor for the
-duration of the entry, and every operation on the entry — `lstat`, the temp-file
-`mkstemp`, the temp->final `rename`, `chmod`/`chown`/`utimes`, `mkdir`, special-
-file and symlink creation, the delta-basis open, and the recursive delete — runs
-through that one held fd via an `*at()` call (`do_*_atfd()`). Because the
-descriptor is pinned to the directory inode, a parent component flipped to a
-symlink *after* the open cannot redirect any of those operations. The alternate-
-destination lookups are confined the same way (`basis_link_stat()` in
-`generator.c` and `secure_basis_open()` in `receiver.c`), so a peer-chosen
-`--link-dest`/`--compare-dest`/`--copy-dest` basis index cannot reach an
-out-of-module file through a symlinked parent.
+as traversal authority, `held_dfd_for()` caches that descriptor for the
+duration of the entry, and every operation on the entry — `lstat`, the
+temp-file `mkstemp`, the temp->final `rename`, `chmod`/`chown`/`utimes`,
+`mkdir`, special-file and symlink creation, the delta-basis open, and the
+recursive delete — runs through that one held fd via an `*at()` call
+(`do_*_atfd()`). Because the descriptor is pinned to the directory inode, a
+parent component flipped to a symlink *after* the open cannot redirect any of
+those operations. The alternate-destination lookups are confined the same way
+(`basis_link_stat()` in `generator.c` and `secure_basis_open()` in
+`receiver.c`), so a peer-chosen `--link-dest`/`--compare-dest`/`--copy-dest`
+basis index cannot reach an out-of-module file through a symlinked parent.
 
 The sender's source-directory *enumeration* is confined the same way as its
 content open. `send_directory()` opens each scanned directory through
@@ -190,13 +191,17 @@ symlink would otherwise introduce.
 ### Path resolution
 
 `secure_relative_open()` resolves a path with a single portable mechanism on
-every platform: a per-component walk on a stack of held directory fds. Each
-component is opened relative to the held parent with `openat(parent_fd,
-"component", O_NOFOLLOW)`; descending into a real subdirectory pushes its fd, a
-`..` pops back to the already-held parent (a pop at the anchor is refused), and an
-in-tree directory symlink is followed by reading its target and walking that off
-the same stack (absolute targets refused, symlink hops bounded). The final
-component is opened `O_NOFOLLOW`.
+every platform: a per-component walk on a stack of held directory fds. On
+Linux, anchors and traversal components use
+`O_PATH|O_DIRECTORY|O_NOFOLLOW`; other platforms retain the
+`O_RDONLY|O_DIRECTORY` fallback. Descending into a real subdirectory pushes
+its fd, a `..` pops back to the already-held parent (a pop at the anchor is
+refused), and an in-tree directory symlink is followed by reading its target
+and walking that off the same stack (absolute targets refused, symlink hops
+bounded). A final directory endpoint is reopened with the caller's requested
+flags. Thus `secure_opendir()` still receives a readable fd, while known-name
+operations beneath a searchable but unreadable directory do not require
+permission to list it.
 
 Because every component is opened relative to a *pinned* fd under `O_NOFOLLOW`,
 and `..` is resolved by the held-fd stack rather than by the kernel, the walk is
@@ -235,8 +240,9 @@ chmod-ing through a raced leaf symlink.
   influenced by the remote peer or by another local user, use a `do_*_at()`
   wrapper (or `secure_relative_open()`), not a raw full-path syscall.
 * When introducing a new operation, add a matching `do_<op>_at()` wrapper that
-  resolves the parent with `secure_relative_open()` and acts via an at-style call
-  on the returned dirfd.
+  resolves a parent used only as at-style authority with
+  `secure_relative_dirfd()`. Use `secure_relative_open()` when the returned fd
+  itself must be readable or otherwise support the caller's requested access.
 * Do not assume a non-daemon transfer is safe; the question is whether rsync has
   more authority than whoever controls the path components.
 * On platforms whose API lacks an at-style equivalent (e.g. `setattrlist()`),

generator.c

@@ -1052,7 +1052,7 @@ static int basis_link_stat(const char *path, STRUCT_STAT *stp)
 			if (dlen >= sizeof dir) { errno = ENAMETOOLONG; return -1; }
 			memcpy(dir, path, dlen);
 			dir[dlen] = '\0';
-			if ((dfd = secure_relative_open(NULL, dir, O_RDONLY | O_DIRECTORY, 0)) < 0)
+			if ((dfd = secure_relative_dirfd(NULL, dir)) < 0)
 				return -1;
 			r = link_stat_at(dfd, slash + 1, stp, 0);
 			e = errno;

sender.c

@@ -127,8 +127,7 @@ static int secure_sender_parent_fd(struct file_struct *file, const char *fname,
 #endif
 				while (*rel == '/')
 					rel++;
-				return secure_relative_open("/", rel,
-					O_RDONLY | O_DIRECTORY, 0);
+				return secure_relative_dirfd("/", rel);
 			}
 			/* held_dir_path_fd returns a cache-OWNED fd; the caller closes
 			 * what we return, so hand back an owned dup and leave the cache's
@@ -141,7 +140,7 @@ static int secure_sender_parent_fd(struct file_struct *file, const char *fname,
 				return dup(dfd);
 			if (errno != 0)
 				return -1;
-			return secure_relative_open(NULL, dir, O_RDONLY | O_DIRECTORY, 0);
+			return secure_relative_dirfd(NULL, dir);
 		}
 		errno = 0;	/* top-level file: no parent component to confine */
 		return -1;
@@ -176,9 +175,9 @@ static int secure_sender_parent_fd(struct file_struct *file, const char *fname,
 		}
 		memcpy(dir, relp, dlen);
 		dir[dlen] = '\0';
-		dfd = secure_relative_open(module_dir, dir, O_RDONLY | O_DIRECTORY, 0);
+		dfd = secure_relative_dirfd(module_dir, dir);
 	} else
-		dfd = secure_relative_open(module_dir, "", O_RDONLY | O_DIRECTORY, 0);
+		dfd = secure_relative_dirfd(module_dir, "");
 
 	/* The leaf is the same last component either way; take it from the caller's
 	 * persistent fname buffer, not the local secure_path. */
@@ -292,11 +291,9 @@ static int sender_open_copylinks_confined(const char *anchor, const char *relpat
 		 * only this branch would hand it to strcmp(). */
 		if (am_daemon && module_dirfd >= 0 && module_dir && anchor
 		 && strcmp(anchor, module_dir) == 0)
-			pdfd = secure_relative_open_at_beneath(module_dirfd, dir,
-					O_RDONLY | O_DIRECTORY, 0);
+			pdfd = secure_relative_dirfd_at_beneath(module_dirfd, dir);
 		else
-			pdfd = secure_relative_open(anchor, dir,
-					O_RDONLY | O_DIRECTORY, 0);
+			pdfd = secure_relative_dirfd(anchor, dir);
 		if (pdfd < 0)
 			return -1;
 		n = do_readlink_atfd(pdfd, bname, tgt, sizeof tgt - 1);