History

syscall: use openat2(RESOLVE_BENEATH) on Linux for secure_relative_open

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 #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 #864 by Samuel Henrique) exercises the issue #715 regression and passes; full make check passes 47/47. Test: testsuite/symlink-dirlink-basis.test (8 scenarios) Fixes: https://github.com/RsyncProject/rsync/issues/715

Andrew Tridgell committed 6 months ago

Commit 4fa71561 parent dcf364d2 files · +304 −5Browse files

Unified · Split

2 files
  1. syscall.c +57 −5
  2. testsuite/symlink-dirlink-basis.test +247 −0
syscall.c+57−5
@@ -33,6 +33,11 @@ · 32 unmodified lines
33#include <sys/syscall.h>33#include <sys/syscall.h>
34#endif34#endif
35 35
36#ifdef __linux__
37#include <sys/syscall.h>
38#include <linux/openat2.h>
39#endif
40
36#include "ifuncs.h"41#include "ifuncs.h"
37 42
38extern int dry_run;43extern int dry_run;
@@ -720,12 +725,49 @@ int do_open_nofollow(const char *pathname, int flags) · 681 unmodified lines
720/*725/*
721 open a file relative to a base directory. The basedir can be NULL,726 open a file relative to a base directory. The basedir can be NULL,
722 in which case the current working directory is used. The relpath727 in which case the current working directory is used. The relpath
723 must be a relative path, and the relpath must not contain any728 must be a relative path. The kernel must guarantee that resolution
724 elements in the path which follow symlinks (ie. like O_NOFOLLOW, but729 cannot escape basedir (or the cwd, when basedir is NULL): no ".."
725 applies to all path components, not just the last component)730 jumps above the start, no symlinks pointing outside, no absolute
726 731 paths, no /proc magic-link tricks.
727 The relpath must also not contain any ../ elements in the path732
733 Symlinks *within* basedir are followed normally — earlier rsync
734 versions rejected every symlink with O_NOFOLLOW on each component,
735 which broke legitimate directory symlinks on the receiver side
736 (https://github.com/RsyncProject/rsync/issues/715). The escape
737 prevention is handled by the kernel via openat2(RESOLVE_BENEATH)
738 on Linux 5.6+; older systems fall back to the per-component
739 O_NOFOLLOW walk below.
740
741 The relpath must also not contain any ../ elements in the path.
728*/742*/
743
744#ifdef __linux__
745static int secure_relative_open_linux(const char *basedir, const char *relpath, int flags, mode_t mode)
746{
747 struct open_how how;
748 int dirfd, retfd;
749
750 memset(&how, 0, sizeof how);
751 how.flags = flags;
752 how.mode = mode;
753 how.resolve = RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS;
754
755 if (basedir == NULL) {
756 dirfd = AT_FDCWD;
757 } else {
758 dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
759 if (dirfd == -1)
760 return -1;
761 }
762
763 retfd = syscall(SYS_openat2, dirfd, relpath, &how, sizeof how);
764
765 if (dirfd != AT_FDCWD)
766 close(dirfd);
767 return retfd;
768}
769#endif
770
729int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode)771int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode)
730{772{
731 if (!relpath || relpath[0] == '/') {773 if (!relpath || relpath[0] == '/') {
@@ -739,6 +781,16 @@ int secure_relative_open(const char *basedir, const char *relpath, int flags, mo · 7 unmodified lines
739 return -1;781 return -1;
740 }782 }
741 783
784#ifdef __linux__
785 {
786 int fd = secure_relative_open_linux(basedir, relpath, flags, mode);
787 /* ENOSYS = kernel < 5.6 doesn't have the syscall even though
788 * glibc/kernel-headers do; fall through to the portable path. */
789 if (fd != -1 || errno != ENOSYS)
790 return fd;
791 }
792#endif
793
742#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD)794#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY) || !defined(AT_FDCWD)
743 // really old system, all we can do is live with the risks795 // really old system, all we can do is live with the risks
744 if (!basedir) {796 if (!basedir) {
testsuite/symlink-dirlink-basis.test+247−0
@@ -0,0 +1,247 @@
1#!/bin/sh
2
3# Test that updating a file through a directory symlink works when using
4# -K (--copy-dirlinks). This is a regression test for:
5# https://github.com/RsyncProject/rsync/issues/715
6#
7# The CVE fix in commit c35e283 introduced secure_relative_open() which
8# uses O_NOFOLLOW on all path components, breaking legitimate directory
9# symlinks on the receiver side. The fix splits the path into basedir
10# (dirname, symlinks followed) and basename (O_NOFOLLOW) so that
11# directory symlinks are traversed while the final file component is
12# still protected.
13#
14# The regression only manifests when delta matching is triggered (i.e.,
15# the sender finds matching blocks in the old file). Small files with
16# completely different content are transferred in full and don't trigger
17# the bug. We use a large file with a small modification to ensure
18# delta transfer is used.
19#
20# In addition to the original regression, this test covers edge cases
21# in the fix itself:
22# - --backup with directory symlinks (finish_transfer pointer identity)
23# - --partial-dir with protocol < 29 (fnamecmp != partialptr guard)
24# - --inplace with directory symlinks (updating_basis_or_equiv check)
25# - Files without a dirname (top-level files, no split needed)
26
27. "$suitedir/rsync.fns"
28
29RSYNC_RSH="$scratchdir/src/support/lsh.sh"
30export RSYNC_RSH
31
32# $HOME is set to $scratchdir by rsync.fns
33# localhost: destination will cd to $HOME (i.e., $scratchdir)
34
35# Helper: create a large file suitable for delta transfers.
36# ~32KB is large enough for rsync's block matching to find matches.
37make_testfile() {
38 dd if=/dev/urandom of="$1" bs=1024 count=32 2>/dev/null \
39 || test_fail "failed to create test file $1"
40}
41
42# Set up source tree
43srcbase="$tmpdir/src"
44
45######################################################################
46# Test 1: Basic directory symlink update (the original issue #715)
47######################################################################
48
49mkdir -p "$HOME/real-dir"
50ln -s real-dir "$HOME/dir"
51
52mkdir -p "$srcbase/dir"
53make_testfile "$srcbase/dir/file"
54
55# First transfer (initial): should create the file through the symlink
56(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
57 || test_fail "test 1: initial transfer failed"
58
59if [ ! -f "$HOME/real-dir/file" ]; then
60 test_fail "test 1: initial transfer did not create file through symlink"
61fi
62
63diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
64 || test_fail "test 1: initial transfer content mismatch"
65
66# Small modification to trigger delta transfer
67echo "appended update" >> "$srcbase/dir/file"
68sleep 1
69touch "$srcbase/dir/file"
70
71# Second transfer (update): was failing with "failed verification"
72(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
73 || test_fail "test 1: update through directory symlink failed"
74
75diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
76 || test_fail "test 1: update transfer content mismatch"
77
78######################################################################
79# Test 2: Compression (-z) as in the original reproducer
80######################################################################
81
82echo "another line" >> "$srcbase/dir/file"
83sleep 1
84touch "$srcbase/dir/file"
85
86(cd "$srcbase" && $RSYNC -KRlptzv --rsync-path="$RSYNC" dir/file localhost:) \
87 || test_fail "test 2: compressed update through directory symlink failed"
88
89diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
90 || test_fail "test 2: compressed update content mismatch"
91
92######################################################################
93# Test 3: Nested directory symlinks (nested/sub/data.txt where
94# "nested" is a symlink to "nested_real")
95######################################################################
96
97mkdir -p "$HOME/nested_real/sub"
98ln -s nested_real "$HOME/nested"
99
100mkdir -p "$srcbase/nested/sub"
101make_testfile "$srcbase/nested/sub/data.txt"
102
103(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \
104 || test_fail "test 3: initial nested transfer failed"
105
106echo "appended nested" >> "$srcbase/nested/sub/data.txt"
107sleep 1
108touch "$srcbase/nested/sub/data.txt"
109
110(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" nested/sub/data.txt localhost:) \
111 || test_fail "test 3: update through nested directory symlink failed"
112
113diff "$srcbase/nested/sub/data.txt" "$HOME/nested_real/sub/data.txt" >/dev/null \
114 || test_fail "test 3: nested update content mismatch"
115
116######################################################################
117# Test 4: --backup with directory symlinks
118#
119# Exercises the finish_transfer() "fnamecmp == fname" pointer
120# comparison that determines whether to update fnamecmp to the
121# backup name. If broken, --backup would reference a renamed file
122# for xattr handling.
123######################################################################
124
125# Reset destination
126rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~"
127
128make_testfile "$srcbase/dir/file"
129
130(cd "$srcbase" && $RSYNC -KRlptv --rsync-path="$RSYNC" dir/file localhost:) \
131 || test_fail "test 4: initial transfer for backup test failed"
132
133echo "backup update" >> "$srcbase/dir/file"
134sleep 1
135touch "$srcbase/dir/file"
136
137(cd "$srcbase" && $RSYNC -KRlptv --backup --rsync-path="$RSYNC" dir/file localhost:) \
138 || test_fail "test 4: update with --backup through directory symlink failed"
139
140diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
141 || test_fail "test 4: backup update content mismatch"
142
143if [ ! -f "$HOME/real-dir/file~" ]; then
144 test_fail "test 4: backup file was not created"
145fi
146
147######################################################################
148# Test 5: --inplace with directory symlinks
149#
150# Exercises the updating_basis_or_equiv check which uses
151# "fnamecmp == fname". With --inplace, rsync writes directly to
152# the destination file instead of a temp file.
153######################################################################
154
155rm -f "$HOME/real-dir/file" "$HOME/real-dir/file~"
156
157make_testfile "$srcbase/dir/file"
158
159(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \
160 || test_fail "test 5: initial inplace transfer failed"
161
162echo "inplace update" >> "$srcbase/dir/file"
163sleep 1
164touch "$srcbase/dir/file"
165
166(cd "$srcbase" && $RSYNC -KRlptv --inplace --rsync-path="$RSYNC" dir/file localhost:) \
167 || test_fail "test 5: inplace update through directory symlink failed"
168
169diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
170 || test_fail "test 5: inplace update content mismatch"
171
172######################################################################
173# Test 6: Top-level file (no dirname, no split needed)
174#
175# Ensures the dirname/basename split is not attempted for files
176# at the top level (file->dirname is NULL).
177######################################################################
178
179make_testfile "$srcbase/topfile"
180mkdir -p "$HOME"
181
182(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \
183 || test_fail "test 6: initial top-level transfer failed"
184
185echo "toplevel update" >> "$srcbase/topfile"
186sleep 1
187touch "$srcbase/topfile"
188
189(cd "$srcbase" && $RSYNC -Rlptv --rsync-path="$RSYNC" topfile localhost:) \
190 || test_fail "test 6: top-level update failed"
191
192diff "$srcbase/topfile" "$HOME/topfile" >/dev/null \
193 || test_fail "test 6: top-level update content mismatch"
194
195######################################################################
196# Test 7: --partial-dir with protocol < 29
197#
198# For protocol < 29, fnamecmp_type stays FNAMECMP_FNAME even when
199# fnamecmp is set to partialptr. The dirname/basename split must
200# NOT trigger in this case (guarded by "fnamecmp == fname").
201######################################################################
202
203rm -f "$HOME/real-dir/file"
204make_testfile "$srcbase/dir/file"
205
206(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \
207 --rsync-path="$RSYNC" dir/file localhost:) \
208 || test_fail "test 7: initial proto28 partial-dir transfer failed"
209
210echo "partial-dir update" >> "$srcbase/dir/file"
211sleep 1
212touch "$srcbase/dir/file"
213
214(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 --partial-dir=.rsync-partial \
215 --rsync-path="$RSYNC" dir/file localhost:) \
216 || test_fail "test 7: proto28 partial-dir update through dirlink failed"
217
218diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
219 || test_fail "test 7: proto28 partial-dir update content mismatch"
220
221######################################################################
222# Test 8: Protocol < 29 basic directory symlink update
223#
224# Exercises the protocol < 29 code path and its fallback logic
225# (clearing basedir on retry).
226######################################################################
227
228rm -f "$HOME/real-dir/file"
229make_testfile "$srcbase/dir/file"
230
231(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \
232 --rsync-path="$RSYNC" dir/file localhost:) \
233 || test_fail "test 8: initial proto28 transfer failed"
234
235echo "proto28 update" >> "$srcbase/dir/file"
236sleep 1
237touch "$srcbase/dir/file"
238
239(cd "$srcbase" && $RSYNC -KRlptv --protocol=28 \
240 --rsync-path="$RSYNC" dir/file localhost:) \
241 || test_fail "test 8: proto28 update through directory symlink failed"
242
243diff "$srcbase/dir/file" "$HOME/real-dir/file" >/dev/null \
244 || test_fail "test 8: proto28 update content mismatch"
245
246# The script would have aborted on error, so getting here means we've won.
247exit 0