Repository navigation
Conversation
When make_backup_inner() falls back to recreating a symlink via do_symlink_at() (such as during a parent-swap race when link_or_rename() fails), it passed the unmunged link target read by make_file() directly to symlinkat(). In a non-chrooted daemon with munge_symlinks enabled, this created an unmunged root-owned backup symlink on disk. Ensure do_symlink_at() prepend SYMLINK_PREFIX whenever munge_symlinks is active and the target is not already prefixed, and add a regression test.
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
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.
When
make_backup_inner()inbackup.cbacks up a symlink, it constructs afile_structviamake_file()and attemptslink_or_rename(). Iflink_or_rename()fails (such as during a parent-directory swap race)make_backup_inner()falls back to recreating the symlink on disk viado_symlink_at(). BecauseF_SYMLINK(file)holds the unmunged target anddo_symlink_at()did not perform munging, its created with the daemons running UID. root-owned/trsuted symlink could occur on the disk/daemon-module which could break the trusted symlink idea.Fix: Centralize Munging in
do_symlink_at()Rather than scattering
SYMLINK_PREFIXchecks across individual callers likemake_backup_inner(), we need to centralize symlink munging directly at the syscall wrapper boundary indo_symlink_at():munge_symlinksis active ( which is active by default in non-chroot daemon modules ),do_symlink_at()check whether the targetlnkalready begins withSYMLINK_PREFIX(/rsyncd-munged/).lnkis not already prefixed, the wrapper prependsSYMLINK_PREFIXbefore invokingsymlinkat()(returningENAMETOOLONGif the combined length exceedsMAXPATHLEN).recv_file_entry()are not double-munged, while any internal code path that creates a symlink from an in-memoryfile_struct(such asmake_backup_inner()) is guaranteed to never write an unmunged symlink to disk.Tests
testsuite/backup-munge-symlinks_test.pyto verify that backup symlinks created via themake_backup_inner()fallback under a parent-swap race are properly prefixed with/rsyncd-munged/.testsuite/filter-leak_test.pyandtestsuite/files-from-leak_test.pyso their setup preconditions treat munged root-owned backup symlinks as mitigated rather than a test setup failure.