From b705916452741aead39c122ae12393ea5657a3de Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Tue, 3 Nov 2026 10:50:11 +0100 Subject: [PATCH] syscall: use O_SEARCH for held directory traversal on macOS Since 3.5.0 the receiver resolves its destination one component at a time with held directory fds. Without O_PATH these are opened O_RDONLY|O_DIRECTORY, so on macOS every ancestor of the destination must be readable, not just searchable. A destination below a mode 0711 home, or a sandbox that denies listing $HOME, fails with change_dir#1. Use O_SEARCH on macOS: from macOS 13 it needs only search permission, and older kernels ignore it, keeping the O_RDONLY|O_DIRECTORY behaviour. Not on illumos/Solaris, which checks search permission before O_DIRECTORY, so probing a regular file fails with EACCES rather than the ENOTDIR the resolvers expect. Run the search-only tests on macOS 13+ and FreeBSD. Their root runs there skip for lack of setpriv, so mark them fleet_nonroot and add targeted non-root steps to those CI jobs. --- .github/workflows/freebsd-build.yml | 7 +++++++ .github/workflows/macos-build.yml | 8 ++++++++ .github/workflows/ubuntu-build.yml | 4 +++- SECURITY.md | 11 ++++++++--- syscall.c | 10 ++++++++-- testsuite/rsyncfns.py | 12 ++++++++++++ testsuite/search-only-destination_test.py | 13 +++++++++---- testsuite/search-only-held-dirfd_test.py | 11 +++++++---- testsuite/skiplist/macos.txt | 4 ++-- 9 files changed, 64 insertions(+), 16 deletions(-) diff --git a/.github/workflows/freebsd-build.yml b/.github/workflows/freebsd-build.yml index fe87913f5..7a5bc7999 100644 --- a/.github/workflows/freebsd-build.yml +++ b/.github/workflows/freebsd-build.yml @@ -42,6 +42,13 @@ jobs: ./rsync --version make check ./runtests.py --rsync-bin=`pwd`/rsync --use-tcp -j 8 + # Non-root, targeted: without setpriv the root runs skip the + # search-only tests. Run them as uid 65534 from a copy under /tmp, + # which that uid can traverse, without root's PATH (/root/bin). + rm -rf /tmp/rsync-nonroot && cp -Rp . /tmp/rsync-nonroot + rm -rf /tmp/rsync-nonroot/testtmp && chown -R 65534:65534 /tmp/rsync-nonroot + (cd /tmp/rsync-nonroot && env HOME=/tmp/rsync-nonroot PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin python3 -c 'import os, sys; os.setgroups([]); os.setgid(65534); os.setuid(65534); os.execv(sys.executable, [sys.executable] + sys.argv[1:])' \ + runtests.py --rsync-bin=/tmp/rsync-nonroot/rsync search-only-destination search-only-held-dirfd) ./rsync-ssl --no-motd download.samba.org::rsyncftp/ || true - name: save artifact uses: actions/upload-artifact@v4 diff --git a/.github/workflows/macos-build.yml b/.github/workflows/macos-build.yml index b20638a8f..7aa439a76 100644 --- a/.github/workflows/macos-build.yml +++ b/.github/workflows/macos-build.yml @@ -53,6 +53,14 @@ jobs: # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. run: sudo ./runtests.py --rsync-bin="$PWD/rsync" --use-tcp -j 8 + - name: check (non-root, targeted) + # The runs above are root and macOS has no setpriv, so the search-only + # tests skip there; run them as the unprivileged 'runner' user. Explicit + # test names bypass RSYNC_EXPECT_SKIPPED. + run: | + sudo rm -rf testtmp # prior root steps left it root-owned + ./runtests.py --rsync-bin="$PWD/rsync" \ + search-only-destination search-only-held-dirfd - name: ssl file list run: rsync-ssl --no-motd download.samba.org::rsyncftp/ || true - name: save artifact diff --git a/.github/workflows/ubuntu-build.yml b/.github/workflows/ubuntu-build.yml index 3adab91e6..687bf1256 100644 --- a/.github/workflows/ubuntu-build.yml +++ b/.github/workflows/ubuntu-build.yml @@ -60,13 +60,15 @@ jobs: # design); the only test with no other CI coverage (Benjamin #2). # ownership-depth -- non-root takes the group-only remap path. # daemon -- non-root takes the default-config path. + # search-only-* -- search-only parents need traversal without read. # CONVENTION: a new test that requires/meaningfully exercises a non-root # path must be added to the list below (kept in sync with the fleet # harness's nonroot_tests). run: | sudo rm -rf testtmp # prior root steps left it root-owned ./runtests.py --rsync-bin="$PWD/rsync" \ - daemon-namecvt-empty-response ownership-depth daemon + daemon-namecvt-empty-response ownership-depth daemon \ + search-only-destination search-only-held-dirfd - name: ssl file list run: rsync-ssl --no-motd download.samba.org::rsyncftp/ || true - name: save artifact diff --git a/SECURITY.md b/SECURITY.md index b77e1473c..4bb160318 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -199,9 +199,14 @@ leaf symlink into a file read. `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. 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 +systems with `O_PATH` (Linux, FreeBSD 13.1+), anchors and traversal components +use `O_PATH|O_DIRECTORY|O_NOFOLLOW`, even though FreeBSD also defines +`O_SEARCH`. macOS uses `O_SEARCH|O_NOFOLLOW` (`O_SEARCH` includes +`O_DIRECTORY`), which needs only search permission from macOS 13; older kernels +ignore it and require read permission. Other platforms retain the +`O_RDONLY|O_DIRECTORY` fallback, including illumos/Solaris, whose `O_SEARCH` +checks search permission before `O_DIRECTORY` and so fails a regular file with +`EACCES` rather than the `ENOTDIR` the walk expects. 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 diff --git a/syscall.c b/syscall.c index dd41da845..cc9242fe5 100644 --- a/syscall.c +++ b/syscall.c @@ -78,12 +78,18 @@ extern int operator_path_resolve; /* defined below; fwd-declared for the exclude extern int filesfrom_owner_walk_override; /* defined below; used by files-from implied dirs */ /* A directory fd used only for pathname traversal, fchdir(), or as *at() - * authority does not need read permission on Linux. Keep the portable - * O_RDONLY fallback for systems without O_PATH. */ + * authority does not need read permission: use O_PATH where available (Linux, + * FreeBSD 13.1+), and O_SEARCH (which includes O_DIRECTORY) on macOS, where it + * needs only search permission from macOS 13 and older kernels ignore it. Not + * elsewhere: illumos checks search permission before O_DIRECTORY, so probing a + * regular file fails with EACCES instead of the ENOTDIR the resolvers expect. + * Keep the portable O_RDONLY fallback for other systems. */ static int directory_traverse_flags(void) { #if defined O_PATH && defined O_DIRECTORY return O_PATH | O_DIRECTORY; +#elif defined O_SEARCH && defined __APPLE__ + return O_SEARCH; #elif defined O_DIRECTORY return O_RDONLY | O_DIRECTORY; #else diff --git a/testsuite/rsyncfns.py b/testsuite/rsyncfns.py index 0cb473e59..3d9587160 100644 --- a/testsuite/rsyncfns.py +++ b/testsuite/rsyncfns.py @@ -897,6 +897,18 @@ def require_asan(reason: str, which: str = None) -> 'None': test_skipped(reason) +def require_search_only_traversal() -> 'None': + """Skip the test (exit 77) unless rsync's held directory fds need only + search permission: O_PATH on Linux and FreeBSD, O_SEARCH on macOS 13+ + (Darwin 22). Older macOS ignores O_SEARCH, and other systems open them + O_RDONLY, so traversal there still needs read permission.""" + if sys.platform.startswith(('linux', 'freebsd')): + return + if sys.platform == 'darwin' and int(platform.release().split('.')[0]) >= 22: + return + test_skipped('held directory fds need read permission on this platform') + + def rsh_cmd(cmd: str = None, *opts: str) -> str: """Build an RSYNC_RSH / --rsh value, quoted for rsync's own tokenizer. diff --git a/testsuite/search-only-destination_test.py b/testsuite/search-only-destination_test.py index 46108e2f5..20c1efb72 100644 --- a/testsuite/search-only-destination_test.py +++ b/testsuite/search-only-destination_test.py @@ -8,14 +8,19 @@ import os import shutil import subprocess -import sys import tempfile from pathlib import Path -from rsyncfns import SCRATCHDIR, rmtree, rsync_argv, test_fail, test_skipped +from rsyncfns import ( + SCRATCHDIR, require_search_only_traversal, rmtree, rsync_argv, test_fail, + test_skipped, +) -if not sys.platform.startswith('linux'): - test_skipped('search-only-destination is Linux-specific') +require_search_only_traversal() + +# Root runs need setpriv to drop privileges, which macOS and FreeBSD lack; +# the fleet harness and the workflows' non-root steps cover them. +fleet_nonroot = True launcher = [] if os.geteuid() == 0: diff --git a/testsuite/search-only-held-dirfd_test.py b/testsuite/search-only-held-dirfd_test.py index e4da46fbf..6acfc3061 100644 --- a/testsuite/search-only-held-dirfd_test.py +++ b/testsuite/search-only-held-dirfd_test.py @@ -10,16 +10,19 @@ import os import shutil import subprocess -import sys import tempfile from pathlib import Path from rsyncfns import ( - SCRATCHDIR, forced_protocol, rmtree, rsync_argv, test_fail, test_skipped, + SCRATCHDIR, forced_protocol, require_search_only_traversal, rmtree, + rsync_argv, test_fail, test_skipped, ) -if not sys.platform.startswith('linux'): - test_skipped('search-only held-dirfd coverage is Linux-specific') +require_search_only_traversal() + +# Root runs need setpriv to drop privileges, which macOS and FreeBSD lack; +# the fleet harness and the workflows' non-root steps cover them. +fleet_nonroot = True launcher = [] if os.geteuid() == 0: diff --git a/testsuite/skiplist/macos.txt b/testsuite/skiplist/macos.txt index 1311c90f5..5a2bf30a0 100644 --- a/testsuite/skiplist/macos.txt +++ b/testsuite/skiplist/macos.txt @@ -32,8 +32,8 @@ rrsync-sender-leaf-flip rrsync-sender-parent-pin rrsync-symlink rrsync-userns-procfs -search-only-destination -search-only-held-dirfd +search-only-destination # root run lacks setpriv; see the non-root step +search-only-held-dirfd # root run lacks setpriv; see the non-root step sender-remove-source-root-anchor simd-checksum source-change-size-continues