use openat2 RESOLVE_BENEATH for secure_relative_open when available#887
Merged
tridge merged 4 commits intoRsyncProject:masterfrom Apr 29, 2026
Merged
use openat2 RESOLVE_BENEATH for secure_relative_open when available#887tridge merged 4 commits intoRsyncProject:masterfrom
tridge merged 4 commits intoRsyncProject:masterfrom
Conversation
The CVE fix in commit c35e283 made secure_relative_open() walk every component of relpath with O_NOFOLLOW. That blocks every symlink in the path, which is stricter than the threat model required: legitimate directory symlinks within the destination tree (e.g. when using -K / --copy-dirlinks) are also rejected, breaking delta transfers with "failed verification -- update discarded". See issue RsyncProject#715. On Linux 5.6+, openat2(RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS) gives us exactly what we want: the kernel rejects any resolution that would escape the starting directory (via "..", absolute paths, or symlinks pointing outside dirfd) while still following symlinks that resolve within it. /proc magic-links are blocked too. Use openat2 first; fall back to the existing per-component O_NOFOLLOW walk on ENOSYS (kernel < 5.6). The lexical "../" checks at the head of the function are kept as defense in depth. The Linux gate is plain #ifdef __linux__: the runtime ENOSYS fallback covers the only case that actually matters (header present + old kernel), and any Linux build environment without linux/openat2.h will fail with a clear "no such file" error rather than silently disabling the protection. Verified manually that openat2(RESOLVE_BENEATH) blocks all four escape patterns (absolute symlink, ../ symlink, lexical .., absolute path) while allowing direct and within-tree symlinks. The new testsuite/symlink-dirlink-basis.test (taken from PR RsyncProject#864 by Samuel Henrique) exercises the issue RsyncProject#715 regression and passes; full make check passes 47/47. Test: testsuite/symlink-dirlink-basis.test (8 scenarios) Fixes: RsyncProject#715 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FreeBSD and MacOS have O_RESOLVE_BENEATH as an openat() flag with the same "must not escape dirfd" semantics as Linux's RESOLVE_BENEATH. The kernel rejects ".." escapes, absolute symlinks, and symlinks whose target lies outside dirfd, while still following symlinks that resolve within it -- the same trade-off that fixes issue RsyncProject#715 on Linux. Add a parallel BSD path in secure_relative_open(), gated on declared. Unlike Linux, BSD doesn't have the header/runtime split where the symbol can exist without kernel support, so no runtime fallback is needed: if the flag compiles in, the kernel honours it. OpenBSD and NetBSD have no equivalent kernel primitive and continue to use the existing per-component O_NOFOLLOW walk; issue RsyncProject#715 remains visible on those platforms (a userland resolver or unveil(2)-based fence would be follow-up work). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…NEATH secure_relative_open() has a kernel-enforced "stay below dirfd" path on Linux 5.6+ (openat2 RESOLVE_BENEATH) and FreeBSD 13+ (openat O_RESOLVE_BENEATH). On Solaris, OpenBSD, NetBSD, and Cygwin the code falls back to the per-component O_NOFOLLOW walk, which by design rejects every directory symlink in the path -- the very case this test exercises. Mark the test skipped there rather than have it fail with a known regression that's tracked separately. macOS is intentionally not in the skip list: although it does not have O_RESOLVE_BENEATH either, the test passes there in practice; investigation of the underlying reason is left as follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test correctly skips on Cygwin (which lacks RESOLVE_BENEATH), but the workflow's RSYNC_EXPECT_SKIPPED list still treats any change in the skipped set as a CI failure. Add the new test name so the skipped/got comparison matches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a secure alternative to #864 using the recent linux and FreeBSD kernels capability of constraining an open within a subtree.
This works on:
This fixes a regression introduced in rsync 3.4.0
thanks to @samueloph for the test suite for this and for pushing for a fix
fixes #715