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.
| * symlink on disk (e.g., via make_backup_inner() copying an unmunged | ||
| * local symlink), while avoiding double-munging links already prefixed | ||
| * by recv_file_entry(). */ | ||
| if (munge_symlinks) { |
There was a problem hiding this comment.
This is after early returns through do_symlink(), after the fake-super write and outside the no-AT_FDCWD fallback while do_symlink_atfd() still bypasses it. Put target munging in one helper used by every symlink-creation wrapper. Also use < not <= -- or an exact /rsyncd-munged/ target is prefixed twice.
| # there's nothing to assert -- skip rather than fail. | ||
| test_skipped("race did not produce a root-owned backup symlink (no atomic flipper?)") | ||
| print("race did not produce a root-owned backup symlink (no atomic flipper?)") | ||
| sys.exit(0) |
There was a problem hiding this comment.
Ah. Dont do that. Preserve the expected-skip result or isolate only the racy daemon section so the remaining assertions still run.
There was a problem hiding this comment.
We have no other choice here ig. The previous filter/files-from leak regressions that happened require a trusted symlink in the daemon (which why it was depending on this remaining old bug), if the race did not happen and the link is munged, then we don't have a trusted symlink or the first chain , so its safe to pass.
There was a problem hiding this comment.
A verified munged link proves that part of the chain was blocked. Not winning the race does not. Test munging separately and seed a trusted symlink directly for the filter/files-from confinement regressions so fixing creation does not stop testing the read-side protection. The later local checks must still run.
There was a problem hiding this comment.
Aha makes sense. I'll let it log that the race did not succeeded and follow the attack with independent symlink previously created by the test.
| backed_up_link = root_owned_backup_symlink() | ||
| if backed_up_link: | ||
| break |
There was a problem hiding this comment.
Also this immediately finds the symlink left by case 3 and breaks before build(pre_munge_dest=True) runs. Reset the fixture before checking for a result and prove that the already-munged backup path was actually exercised.
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.