From 520530d660bbe50b8abd41677e6c35ace6c4ee47 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 Nov 2024 10:00:46 +0100 Subject: [PATCH] Detect directory entry modifications to uid/gid (#3473) From issue #3435 : >`DirectoryEntryBase::CompareTo` will miss isolated changes to uid, gid, xattrs, chunks. This isn't an issue usually, as >such changes when made via an overlayfs transaction will also change the mtime. It causes a problem if the changes >are being made programmatically. >It also results in changed behaviour when publishing via a gateway vs not. uid/gid changes are now properly handled. Changes to individual extended attributes or to the chunking layout are more difficult to handle because they require a follow-up lookup in the corresponding catalog tables, to be done in a future patch. --------- Co-authored-by: Valentin Volkl --- cvmfs/directory_entry.cc | 6 ++++++ cvmfs/directory_entry.h | 37 ++++++++++++++++++++----------------- cvmfs/publish/cmd_diff.cc | 4 ++++ cvmfs/swissknife_check.cc | 12 ++++++++++++ test/src/641-diff/main | 4 ++++ 5 files changed, 46 insertions(+), 17 deletions(-) diff --git a/cvmfs/directory_entry.cc b/cvmfs/directory_entry.cc index 5d41a608cb..ebea147d25 100644 --- a/cvmfs/directory_entry.cc +++ b/cvmfs/directory_entry.cc @@ -35,6 +35,12 @@ DirectoryEntryBase::Differences DirectoryEntryBase::CompareTo( if (HasXattrs() != other.HasXattrs()) { result |= Difference::kHasXattrsFlag; } + if (uid() != other.uid()) { + result |= Difference::kUid; + } + if (gid() != other.gid()) { + result |= Difference::kGid; + } return result; } diff --git a/cvmfs/directory_entry.h b/cvmfs/directory_entry.h index 68b11696cc..8799b3d2b2 100644 --- a/cvmfs/directory_entry.h +++ b/cvmfs/directory_entry.h @@ -77,25 +77,28 @@ class DirectoryEntryBase { /** * Used in the swissknife for sanity checks and catalog migration. If - * anything is added, also adjust PrintDifferences in swissknife::CommandDiff. + * anything is added, also adjust PrintDifferences in swissknife::CommandDiff + * and CommandCheck::CompareEntries */ struct Difference { - static const unsigned int kIdentical = 0x000; - static const unsigned int kName = 0x001; - static const unsigned int kLinkcount = 0x002; - static const unsigned int kSize = 0x004; - static const unsigned int kMode = 0x008; - static const unsigned int kMtime = 0x010; - static const unsigned int kSymlink = 0x020; - static const unsigned int kChecksum = 0x040; - static const unsigned int kHardlinkGroup = 0x080; - static const unsigned int kNestedCatalogTransitionFlags = 0x100; - static const unsigned int kChunkedFileFlag = 0x200; - static const unsigned int kHasXattrsFlag = 0x400; - static const unsigned int kExternalFileFlag = 0x800; - static const unsigned int kBindMountpointFlag = 0x1000; - static const unsigned int kHiddenFlag = 0x2000; - static const unsigned int kDirectIoFlag = 0x4000; + static const unsigned int kIdentical = 0x00000; + static const unsigned int kName = 0x00001; + static const unsigned int kLinkcount = 0x00002; + static const unsigned int kSize = 0x00004; + static const unsigned int kMode = 0x00008; + static const unsigned int kMtime = 0x00010; + static const unsigned int kSymlink = 0x00020; + static const unsigned int kChecksum = 0x00040; + static const unsigned int kHardlinkGroup = 0x00080; + static const unsigned int kNestedCatalogTransitionFlags = 0x00100; + static const unsigned int kChunkedFileFlag = 0x00200; + static const unsigned int kHasXattrsFlag = 0x00400; + static const unsigned int kExternalFileFlag = 0x00800; + static const unsigned int kBindMountpointFlag = 0x01000; + static const unsigned int kHiddenFlag = 0x02000; + static const unsigned int kDirectIoFlag = 0x04000; + static const unsigned int kUid = 0x08000; + static const unsigned int kGid = 0x10000; }; typedef unsigned int Differences; diff --git a/cvmfs/publish/cmd_diff.cc b/cvmfs/publish/cmd_diff.cc index f871017ea8..b2d269b292 100644 --- a/cvmfs/publish/cmd_diff.cc +++ b/cvmfs/publish/cmd_diff.cc @@ -195,6 +195,10 @@ class DiffReporter : public publish::DiffListener { result_list.push_back(machine_readable_ ? "H" : "hidden"); if (diff & catalog::DirectoryEntryBase::Difference::kDirectIoFlag) result_list.push_back(machine_readable_ ? "D" : "direct-io"); + if (diff & catalog::DirectoryEntryBase::Difference::kUid) + result_list.push_back(machine_readable_ ? "U" : "uid"); + if (diff & catalog::DirectoryEntryBase::Difference::kGid) + result_list.push_back(machine_readable_ ? "R" : "gid"); return machine_readable_ ? ("[" + JoinStrings(result_list, "") + "]") : (" [" + JoinStrings(result_list, ", ") + "]"); diff --git a/cvmfs/swissknife_check.cc b/cvmfs/swissknife_check.cc index dcb363a002..be1b04ec46 100644 --- a/cvmfs/swissknife_check.cc +++ b/cvmfs/swissknife_check.cc @@ -128,6 +128,18 @@ bool CommandCheck::CompareEntries(const catalog::DirectoryEntry &a, a.name().c_str(), b.name().c_str()); retval = false; } + if (!is_transition_point) { + if (diffs & Difference::kUid) { + LogCvmfs(kLogCvmfs, kLogStderr, "uids differ: %d / %d (%s / %s)", + a.uid(), b.uid(), a.name().c_str(), b.name().c_str()); + retval = false; + } + if (diffs & Difference::kGid) { + LogCvmfs(kLogCvmfs, kLogStderr, "gids differ: %d / %d (%s / %s)", + a.gid(), b.gid(), a.name().c_str(), b.name().c_str()); + retval = false; + } + } return retval; } diff --git a/test/src/641-diff/main b/test/src/641-diff/main index dd653c0e41..acef683472 100644 --- a/test/src/641-diff/main +++ b/test/src/641-diff/main @@ -15,6 +15,7 @@ create_repository_content() { touch directory/nested/.cvmfscatalog touch directory/foo mkdir dir2 + touch owner popdir } @@ -35,6 +36,7 @@ change_repository_content() { mkdir dir3 touch dir3/new_file touch dir3/.cvmfscatalog + sudo chown 99:99 owner popdir } @@ -85,6 +87,7 @@ A D /dir3 A F /dir3/new_file +0 A F /dir3/.cvmfscatalog +0 M[SC] F /file +M[UR] F /owner R S /symlink -4 M[SMLC] FS /directory/foo M[N] D /directory/nested @@ -104,6 +107,7 @@ M[N] D /dir2 R F /dir2/.cvmfscatalog -0 R D /dir3 M[SC] F /file +M[UR] F /owner A S /symlink +4 M[SMLC] SF /directory/foo M[N] D /directory/nested