From 868d43fb3a28802187616a9232a53e6eef4f8ccc Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 9 Jun 2026 12:03:21 +1000 Subject: [PATCH 1/2] testsuite: daemon must follow in-tree symlinks via -L/-k (RED on fallback) --- testsuite/daemon-copy-links-symlink_test.py | 66 +++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 testsuite/daemon-copy-links-symlink_test.py diff --git a/testsuite/daemon-copy-links-symlink_test.py b/testsuite/daemon-copy-links-symlink_test.py new file mode 100644 index 000000000..8467df880 --- /dev/null +++ b/testsuite/daemon-copy-links-symlink_test.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +# A daemon must follow an IN-TREE symlink in its served module when the client +# asks to dereference it (-L copy-links / -k copy-dirlinks) -- the symlink is +# inside the operator's own served tree, which 3.4.x follows. +# +# A/B + fuzzer-discovered (abdiff --fuzz, fallback build: 16 candidates, all this +# one root cause). On the portable per-component O_NOFOLLOW resolver (the BSDs / +# Solaris / Cygwin, or a Linux build configured --disable-openat2) the daemon +# SENDER refuses to follow the in-tree dir-symlink that -L/-k must dereference: +# rsync: [sender] send_files failed to open "dir_link/a.txt" (in m): +# Too many levels of symbolic links (40) -> exit 23, files LOST +# and in the -L pull case 3.5 can even exit 0 while SILENTLY dropping the +# symlinked content. 3.4.x -- and a RESOLVE_BENEATH 3.5 (openat2 / FreeBSD 13+ / +# macOS 15+) -- transfer it fine, so this is RED only on the fallback platforms. +# +# Same root cause as the local -K (keep-dirlinks-symlinked-dest) and -aR +# (relative-symlinked-parent) regressions, but a BROADER surface: it adds -L and +# -k, over the DAEMON transport (the local -L/-k paths are unaffected). No root; +# uses the standard test-daemon seam (no real TCP in the default mode). + +import os + +from rsyncfns import (SCRATCHDIR, assert_same, get_rootgid, get_rootuid, + get_testuid, makepath, rmtree, run_rsync, + start_test_daemon, test_fail) + +DAEMON_PORT = 12898 +base = SCRATCHDIR / 'daemon_copylinks' +rmtree(base) +mod = base / 'mod' +makepath(mod / 'dir' / 'sub', base / 'dest') +(mod / 'dir' / 'a.txt').write_text('payload a\n') +(mod / 'dir' / 'sub' / 'b.txt').write_text('payload b\n') +os.symlink('dir', mod / 'dir_link') # in-tree dir-symlink within the module + +# When the suite runs as root (e.g. the CI fleet's sudo pass) a use-chroot=no +# daemon otherwise drops to nobody and can't read the root-owned module, so the +# sender fails with EACCES on every file -- nothing to do with the symlink. +# Pin uid/gid to root in that case, as build_rsyncd_conf() does. Non-root +# cannot set uid/gid (and doesn't need to: it owns the module). +if get_testuid() == get_rootuid(): + id_lines = f"uid = {get_rootuid()}\ngid = {get_rootgid()}\n" +else: + id_lines = "" + +conf = base / 'copylinks.conf' +conf.write_text( + f"pid file = {base}/rsyncd.pid\n" + "use chroot = no\n" + + id_lines + + f"log file = {base}/rsyncd.log\n" + f"\n[m]\n\tpath = {mod}\n\tread only = yes\n\thosts allow = 127.0.0.1\n" +) +url = start_test_daemon(conf, DAEMON_PORT) + +# -L (copy-links): dir_link must arrive as a real directory with its content. +# check=False: the fallback regression manifests either as exit 23 or as a +# silent exit-0 partial copy -- assert on the resulting tree either way. +run_rsync('-aL', f'{url}m/', f'{base}/dest/', check=False) + +for rel in ('dir_link/a.txt', 'dir_link/sub/b.txt'): + if not (base / 'dest' / rel).is_file(): + test_fail(f"daemon pull -aL dropped in-tree symlinked content: dest/{rel} missing") +assert_same(mod / 'dir' / 'a.txt', base / 'dest' / 'dir_link' / 'a.txt', label='content') + +print("daemon-copy-links-symlink: daemon -aL follows an in-tree dir-symlink in the module") From 3bd4571ee44f9363a882d8ba2c8b5f4d0e721160 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 9 Jun 2026 17:21:49 +1000 Subject: [PATCH 2/2] syscall: fallback resolver follows in-tree dir symlinks (fixes daemon -L/-k) The portable per-component O_NOFOLLOW walk in secure_relative_open() refused every symlink, so on a fallback platform (the BSDs / Solaris / Cygwin, or a Linux build configured --disable-openat2) a daemon SENDER serving a module that contains an in-tree directory symlink could not dereference it for --copy-links (-L) / --copy-dirlinks (-k): the stat of "dir_link/a.txt" through the in-tree "dir_link -> dir" symlink failed, so the transfer exited 23 (or, for an -L pull, exited 0 while silently dropping the symlinked content). v3.4.x and a RESOLVE_BENEATH 3.5 (openat2 / FreeBSD 13+ / macOS 15+) follow it; this was RED only on the fallback platforms. Same root cause and fix as the local -K (keep-dirlinks) and -aR (relative symlinked parent) regressions: resolve the path on a dirfd stack that follows in-tree directory symlinks (a symlink target's ".." pops back to the pinned parent fd, never above the anchor; absolute targets and a 40-hop overrun are refused), matching RESOLVE_BENEATH while keeping the leaf under O_NOFOLLOW. The "openat(O_DIRECTORY|O_NOFOLLOW) refused -- maybe a symlink, read the target" probe keys on ELOOP/ENOTDIR, but NetBSD and OpenBSD report EFTYPE for an O_NOFOLLOW-refused symlink, so EFTYPE (guarded by #ifdef, a BSD-ism) is treated the same way -- without it those platforms would not follow the symlink either. t_chmod_secure scenario A asserted the old fallback rejection of an in-tree dir-symlink; it now expects the symlink to be followed (the escape case, scenario B, is still rejected). --- syscall.c | 297 +++++++++++++++++++++++++++++++++++++++-------- t_chmod_secure.c | 33 ++---- 2 files changed, 256 insertions(+), 74 deletions(-) diff --git a/syscall.c b/syscall.c index 243f7f243..d1af97079 100644 --- a/syscall.c +++ b/syscall.c @@ -1802,6 +1802,156 @@ static int secure_relative_open_resolve_beneath(const char *basedir, const char char curr_dir[MAXPATHLEN]; unsigned int curr_dir_len; +#if defined(O_NOFOLLOW) && defined(O_DIRECTORY) && defined(AT_FDCWD) +/* Portable-fallback in-tree symlink following for secure_relative_open()'s + * directory walk (below). The openat2(RESOLVE_BENEATH) and O_RESOLVE_BENEATH + * paths follow in-tree directory symlinks while blocking escapes; the + * per-component O_NOFOLLOW walk used where no kernel confinement primitive + * exists used to refuse every symlink, which broke legitimate within-tree + * directory symlinks (--keep-dirlinks #715, and -aR through a symlinked parent). + * + * A directory walk keeps a stack of the open dirfds from the anchor (index 0, + * borrowed -- not closed here) down to the current directory. Descending into + * a real subdirectory pushes its fd; a ".." in a followed symlink target pops + * back to the already-pinned parent fd rather than re-resolving ".." with + * openat(), so an ancestor renamed mid-walk cannot redirect the climb, and the + * climb can never rise above the anchor (a pop at the anchor returns ELOOP). + * This matches RESOLVE_BENEATH, which allows in-tree ".." that stays beneath the + * root. Absolute symlink targets are refused; symlink hops are bounded. */ +#ifndef SECURE_OPEN_MAXSYMLINKS +#define SECURE_OPEN_MAXSYMLINKS 40 +#endif + +struct dirstack { + int *fds; /* fds[0] = anchor (borrowed); fds[top] = current dir */ + int top; + int cap; +}; + +/* Initialise with `anchor` (which may be AT_FDCWD) as the un-owned base. */ +static int ds_init(struct dirstack *ds, int anchor) +{ + ds->cap = 16; + ds->fds = (int*)malloc(ds->cap * sizeof(int)); + if (!ds->fds) + return -1; + ds->fds[0] = anchor; + ds->top = 0; + return 0; +} + +/* Close every pushed fd (but not the borrowed anchor at index 0) and free. */ +static void ds_free(struct dirstack *ds) +{ + while (ds->top > 0) + close(ds->fds[ds->top--]); + free(ds->fds); + ds->fds = NULL; +} + +static int ds_cur(struct dirstack *ds) +{ + return ds->fds[ds->top]; +} + +static int ds_push(struct dirstack *ds, int fd) +{ + if (ds->top + 1 >= ds->cap) { + int ncap = ds->cap * 2; + int *n = (int*)realloc(ds->fds, ncap * sizeof(int)); + if (!n) { + close(fd); + errno = ENOMEM; + return -1; + } + ds->fds = n; + ds->cap = ncap; + } + ds->fds[++ds->top] = fd; + return 0; +} + +/* Detach the current dir as an owned fd the caller must close. At the anchor + * (top 0) the anchor is borrowed, so return a fresh dup of it instead. */ +static int ds_take(struct dirstack *ds) +{ + if (ds->top > 0) + return ds->fds[ds->top--]; + return openat(ds->fds[0], ".", O_RDONLY | O_DIRECTORY); +} + +static int ds_walk_path(struct dirstack *ds, char *path, int *hops); + +/* Descend one path component on the stack: "." stays, ".." pops to the pinned + * parent (ELOOP at the anchor), a real subdirectory is pushed, and an in-tree + * directory symlink is followed by walking its (relative, possibly + * ..-containing) target on the same stack. Returns 0, or -1 with errno set: + * ELOOP for a refused/escaping symlink or a hop overrun, otherwise the + * underlying openat()/readlinkat() errno (ENOENT, a real ENOTDIR, EACCES). */ +static int ds_descend(struct dirstack *ds, const char *part, int *hops) +{ + if (part[0] == '.' && part[1] == '\0') + return 0; /* "." -- no movement */ + if (part[0] == '.' && part[1] == '.' && part[2] == '\0') { + if (ds->top == 0) { /* would rise above the anchor */ + errno = ELOOP; + return -1; + } + close(ds->fds[ds->top--]); /* pop to the held parent fd */ + return 0; + } + + int fd = openat(ds_cur(ds), part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (fd != -1) + return ds_push(ds, fd); /* a real subdirectory */ + /* O_NOFOLLOW refuses a symlink with ELOOP, or ENOTDIR when O_DIRECTORY is + * also set (and NetBSD/OpenBSD report EFTYPE for that case); a real + * non-directory also yields ENOTDIR. Any of these may be a symlink, so + * fall through to the readlink probe; anything else is a hard error. */ + if (errno != ELOOP && errno != ENOTDIR +#ifdef EFTYPE + && errno != EFTYPE +#endif + ) + return -1; + int open_errno = errno; + + char buf[MAXPATHLEN]; + ssize_t n = readlinkat(ds_cur(ds), part, buf, sizeof buf - 1); + if (n < 0) { + if (errno == EINVAL) /* not a symlink: a real non-dir */ + errno = open_errno; + return -1; + } + if (n == 0 || (size_t)n >= sizeof buf - 1) { + errno = ELOOP; /* empty or truncated target */ + return -1; + } + buf[n] = '\0'; + if (buf[0] == '/') { /* absolute target: refuse */ + errno = ELOOP; + return -1; + } + if (--(*hops) < 0) { + errno = ELOOP; + return -1; + } + return ds_walk_path(ds, buf, hops); +} + +/* Walk every component of a relative path on the stack (used for the basedir, + * and for a followed symlink's target -- which may contain ".."). */ +static int ds_walk_path(struct dirstack *ds, char *path, int *hops) +{ + char *save = NULL; + for (char *c = strtok_r(path, "/", &save); c; c = strtok_r(NULL, "/", &save)) { + if (ds_descend(ds, c, hops) < 0) + return -1; + } + return 0; +} +#endif /* O_NOFOLLOW && O_DIRECTORY && AT_FDCWD */ + int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode) { extern int am_daemon, am_chrooted; @@ -1920,89 +2070,134 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo pathjoin(fullpath, sizeof fullpath, basedir, relpath); return open(fullpath, flags, mode); #else - int dirfd = AT_FDCWD; + int dirfd = AT_FDCWD; /* anchor for the relpath walk (owned unless AT_FDCWD) */ + int hops = SECURE_OPEN_MAXSYMLINKS; /* shared symlink-hop budget */ if (basedir != NULL) { if (basedir[0] == '/') { /* Absolute basedir: operator-trusted, plain openat. */ dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY); - if (dirfd == -1) { + if (dirfd == -1) return -1; - } } else { - /* Relative basedir: walk it component-by-component - * with O_NOFOLLOW. This is the per-component - * RESOLVE_BENEATH equivalent for platforms without - * kernel-supported confinement, and matches the - * relpath walk below. Symlinks in basedir are - * rejected outright on this fallback path; the - * Linux openat2 / O_RESOLVE_BENEATH paths above - * still allow within-tree symlinks. */ - char *bcopy = my_strdup(basedir, __FILE__, __LINE__); - if (!bcopy) + /* Relative basedir: resolve it on a dirfd stack anchored at + * the CWD, following in-tree directory symlinks -- the + * portable RESOLVE_BENEATH equivalent. A symlink target's + * ".." may climb but not above the CWD anchor. */ + struct dirstack bds; + char *bcopy; + if (ds_init(&bds, AT_FDCWD) < 0) + return -1; + bcopy = my_strdup(basedir, __FILE__, __LINE__); + if (!bcopy) { + ds_free(&bds); + return -1; + } + if (ds_walk_path(&bds, bcopy, &hops) < 0) { + int e = errno; + free(bcopy); + ds_free(&bds); + errno = e; return -1; - for (const char *part = strtok(bcopy, "/"); - part != NULL; - part = strtok(NULL, "/")) - { - int next_fd = openat(dirfd, part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); - if (next_fd == -1) { - int save_errno = errno; - if (dirfd != AT_FDCWD) close(dirfd); - free(bcopy); - errno = save_errno; - return -1; - } - if (dirfd != AT_FDCWD) close(dirfd); - dirfd = next_fd; } free(bcopy); + dirfd = ds_take(&bds); /* owned dirfd for the basedir */ + ds_free(&bds); + if (dirfd == -1) + return -1; } } + + struct dirstack ds; int retfd = -1; + char *path_copy; - char *path_copy = my_strdup(relpath, __FILE__, __LINE__); + if (ds_init(&ds, dirfd) < 0) { /* dirfd is the (borrowed) anchor */ + if (dirfd != AT_FDCWD) close(dirfd); + return -1; + } + path_copy = my_strdup(relpath, __FILE__, __LINE__); if (!path_copy) { + ds_free(&ds); if (dirfd != AT_FDCWD) close(dirfd); return -1; } - - for (const char *part = strtok(path_copy, "/"); + + /* Trim trailing slashes so the last-component test below is exact, then + * note the offset of the final component. strtok_r rewrites separators in + * place without moving bytes, so a precomputed offset stays valid -- letting + * us tell the last component apart without a destructive strtok() lookahead. */ + size_t pclen = strlen(path_copy); + while (pclen > 1 && path_copy[pclen-1] == '/') + path_copy[--pclen] = '\0'; + char *last_slash = strrchr(path_copy, '/'); + size_t last_off = last_slash ? (size_t)(last_slash + 1 - path_copy) : 0; + + int saw_component = 0; + char *psave = NULL; + for (char *part = strtok_r(path_copy, "/", &psave); part != NULL; - part = strtok(NULL, "/")) + part = strtok_r(NULL, "/", &psave)) { - int next_fd = openat(dirfd, part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); - if (next_fd == -1 && errno == ENOTDIR) { - if (strtok(NULL, "/") != NULL) { - // this is not the last component of the path - errno = ELOOP; + int is_last = (size_t)(part - path_copy) == last_off; + saw_component = 1; + + /* File leaf (final component, caller did not ask for O_DIRECTORY): + * never follow a symlink leaf. */ + if (is_last && !(flags & O_DIRECTORY)) { + int next_fd = openat(ds_cur(&ds), part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + /* ENOTDIR (a non-dir or an O_NOFOLLOW-refused symlink) or ENOENT + * (missing, normal for an O_CREAT leaf): open/create as a file. */ + if (next_fd == -1 && (errno == ENOTDIR || errno == ENOENT)) { + retfd = openat(ds_cur(&ds), part, flags | O_NOFOLLOW, mode); goto cleanup; } - // this could be the last component of the path, try as a file - retfd = openat(dirfd, part, flags | O_NOFOLLOW, mode); + if (next_fd == -1) + goto cleanup; + /* leaf is a real directory but the caller wanted a file */ + close(next_fd); + errno = EISDIR; goto cleanup; } - if (next_fd == -1) { + + /* O_DIRECTORY|O_NOFOLLOW leaf: the caller's O_NOFOLLOW governs the + * leaf, so do not follow a symlink there. */ + if (is_last && (flags & O_NOFOLLOW)) { + retfd = openat(ds_cur(&ds), part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + goto cleanup; + } + + /* Directory component: an intermediate, or an O_DIRECTORY leaf to + * follow. Descend on the stack (following in-tree symlinks). */ + if (ds_descend(&ds, part, &hops) < 0) { + /* An intermediate non-dir reads as ELOOP (as the pre-fix walk + * did); a missing component stays ENOENT. */ + if (!is_last && errno == ENOTDIR) + errno = ELOOP; + goto cleanup; + } + if (is_last) { + /* O_DIRECTORY leaf resolved (possibly via a symlink). */ + retfd = ds_take(&ds); goto cleanup; } - if (dirfd != AT_FDCWD) close(dirfd); - dirfd = next_fd; } - /* All components walked as directories. If the caller asked for - * O_DIRECTORY, return the dirfd we built up; otherwise the path - * resolved to a directory but the caller wanted a regular file. */ - if ((flags & O_DIRECTORY) && dirfd != AT_FDCWD) { - retfd = dirfd; - dirfd = AT_FDCWD; - goto cleanup; + /* Empty relpath (no components): match the prior fallback -- hand back a + * real basedir anchor for an O_DIRECTORY caller, else EISDIR. A NULL + * basedir (AT_FDCWD anchor) is not a resolvable target, so it fails rather + * than silently returning the cwd. */ + if (!saw_component) { + if ((flags & O_DIRECTORY) && dirfd != AT_FDCWD) + retfd = ds_take(&ds); + else + errno = EISDIR; } - errno = EISDIR; cleanup: free(path_copy); - if (dirfd != AT_FDCWD) { + ds_free(&ds); + if (dirfd != AT_FDCWD) close(dirfd); - } return retfd; #endif // O_NOFOLLOW, O_DIRECTORY } diff --git a/t_chmod_secure.c b/t_chmod_secure.c index b99655a40..4363292bc 100644 --- a/t_chmod_secure.c +++ b/t_chmod_secure.c @@ -130,35 +130,22 @@ int main(int argc, char **argv) * files to mode 0600 so we have a clean baseline to compare. */ - /* Scenario A: legitimate parent dir-symlink. + /* Scenario A: legitimate parent dir-symlink within the tree. * - * On platforms whose kernel offers RESOLVE_BENEATH-equivalent - * confinement (Linux 5.6+ openat2, FreeBSD 13+ / macOS 15+ - * O_RESOLVE_BENEATH), the within-tree symlink is followed and - * the chmod must succeed. - * - * On platforms that fall back to the per-component O_NOFOLLOW - * walk (OpenBSD, NetBSD, Solaris, older Cygwin, HPE NonStop, - * and pre-5.6 Linux), every symlink is rejected -- including - * this legitimate one. That's a real platform limitation (the - * same one that causes the #715 regression there) and the - * expected outcome is rejection. - * - * Detect at runtime and expect accordingly. The other three - * scenarios behave identically on both code paths and need no - * adjustment. */ + * The within-tree symlink is followed and the chmod must succeed on + * every platform: the kernel RESOLVE_BENEATH paths (Linux 5.6+ openat2, + * FreeBSD 13+ / macOS 15+ O_RESOLVE_BENEATH) and, since the #715/-K + * fallback fix, the per-component O_NOFOLLOW walk too (OpenBSD, NetBSD, + * Solaris, older Cygwin, HPE NonStop, pre-5.6 Linux) -- which now follows + * an in-tree directory symlink whose target is relative and ".."-free. + * Escapes are still rejected on both paths (Scenario B). */ int kernel_has_rb = kernel_resolve_beneath_supported(); fprintf(stderr, "INFO: kernel RESOLVE_BENEATH-equivalent confinement: %s\n", kernel_has_rb ? "available" : "not available (per-component fallback)"); int rc = do_chmod_at("inside_link/sentinel", 0640); - if (kernel_has_rb) { - check("A: legit dir-symlink within tree (kernel confined)", - rc, 1, "realdir/sentinel", 0640); - } else { - check("A: legit dir-symlink within tree (per-component fallback rejects)", - rc, 0, "realdir/sentinel", 0600); - } + check("A: legit dir-symlink within tree (followed)", + rc, 1, "realdir/sentinel", 0640); /* Scenario B: parent symlink escapes the tree -- chmod must be * rejected and the outside file's mode must be unchanged. */