From c3e0a68c7eeeb4f491c0464392b2146ddec4305a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 14 May 2020 13:52:41 +0200 Subject: [PATCH] canonicalisePathMetaData(): Support a UID range --- src/libstore/build.cc | 12 +++++++++--- src/libstore/local-store.cc | 27 +++++++++++++++++---------- src/libstore/local-store.hh | 15 ++++++++++++--- src/nix-store/nix-store.cc | 2 +- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index c853f609d..4e654e8ad 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -524,7 +524,7 @@ public: uid_t getUID() { assert(uid); return uid; } gid_t getGID() { assert(gid); return gid; } - uint32_t getIDCount() { return 1; } + uint32_t getIDCount() { return settings.idsPerBuild; } std::vector getSupplementaryGIDs() { return supplementaryGIDs; } bool findFreeUser(); @@ -3744,7 +3744,10 @@ void DerivationGoal::registerOutputs() /* Canonicalise first. This ensures that the path we're rewriting doesn't contain a hard link to /etc/shadow or something like that. */ - canonicalisePathMetaData(actualPath, buildUser ? buildUser->getUID() : -1, inodesSeen); + canonicalisePathMetaData( + actualPath, + buildUser ? std::optional(std::make_pair(buildUser->getUID(), buildUser->getUID() + buildUser->getIDCount() - 1)) : std::nullopt, + inodesSeen); /* FIXME: this is in-memory. */ StringSink sink; @@ -3819,7 +3822,10 @@ void DerivationGoal::registerOutputs() /* Get rid of all weird permissions. This also checks that all files are owned by the build user, if applicable. */ canonicalisePathMetaData(actualPath, - buildUser && !rewritten ? buildUser->getUID() : -1, inodesSeen); + buildUser && !rewritten + ? std::optional(std::make_pair(buildUser->getUID(), buildUser->getUID() + buildUser->getIDCount() - 1)) + : std::nullopt, + inodesSeen); /* For this output path, find the references to other paths contained in it. Compute the SHA-256 NAR hash at the same diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index eed225349..80ebe903f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -424,7 +424,10 @@ void canonicaliseTimestampAndPermissions(const Path & path) } -static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSeen & inodesSeen) +static void canonicalisePathMetaData_( + const Path & path, + std::optional> uidRange, + InodesSeen & inodesSeen) { checkInterrupt(); @@ -475,7 +478,7 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe However, ignore files that we chown'ed ourselves previously to ensure that we don't fail on hard links within the same build (i.e. "touch $out/foo; ln $out/foo $out/bar"). */ - if (fromUid != (uid_t) -1 && st.st_uid != fromUid) { + if (uidRange && (st.st_uid < uidRange->first || st.st_uid > uidRange->second)) { assert(!S_ISDIR(st.st_mode)); if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) throw BuildError("invalid ownership on file '%1%'", path); @@ -509,14 +512,17 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe if (S_ISDIR(st.st_mode)) { DirEntries entries = readDirectory(path); for (auto & i : entries) - canonicalisePathMetaData_(path + "/" + i.name, fromUid, inodesSeen); + canonicalisePathMetaData_(path + "/" + i.name, uidRange, inodesSeen); } } -void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & inodesSeen) +void canonicalisePathMetaData( + const Path & path, + std::optional> uidRange, + InodesSeen & inodesSeen) { - canonicalisePathMetaData_(path, fromUid, inodesSeen); + canonicalisePathMetaData_(path, uidRange, inodesSeen); /* On platforms that don't have lchown(), the top-level path can't be a symlink, since we can't change its ownership. */ @@ -531,10 +537,11 @@ void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & ino } -void canonicalisePathMetaData(const Path & path, uid_t fromUid) +void canonicalisePathMetaData(const Path & path, + std::optional> uidRange) { InodesSeen inodesSeen; - canonicalisePathMetaData(path, fromUid, inodesSeen); + canonicalisePathMetaData(path, uidRange, inodesSeen); } @@ -1021,7 +1028,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, autoGC(); - canonicalisePathMetaData(realPath, -1); + canonicalisePathMetaData(realPath, {}); optimisePath(realPath); // FIXME: combine with hashPath() @@ -1064,7 +1071,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam } else writeFile(realPath, dump); - canonicalisePathMetaData(realPath, -1); + canonicalisePathMetaData(realPath, {}); /* Register the SHA-256 hash of the NAR serialisation of the path in the database. We may just have computed it @@ -1134,7 +1141,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, writeFile(realPath, s); - canonicalisePathMetaData(realPath, -1); + canonicalisePathMetaData(realPath, {}); StringSink sink; dumpString(s, sink); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index ff36cb00e..79b415875 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -311,9 +311,18 @@ typedef set InodesSeen; - the permissions are set of 444 or 555 (i.e., read-only with or without execute permission; setuid bits etc. are cleared) - the owner and group are set to the Nix user and group, if we're - running as root. */ -void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & inodesSeen); -void canonicalisePathMetaData(const Path & path, uid_t fromUid); + running as root. + If uidRange is not empty, this function will throw an error if it + encounters files owned by a user outside of the closed interval + [uidRange->first, uidRange->second]. +*/ +void canonicalisePathMetaData( + const Path & path, + std::optional> uidRange, + InodesSeen & inodesSeen); +void canonicalisePathMetaData( + const Path & path, + std::optional> uidRange); void canonicaliseTimestampAndPermissions(const Path & path); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 7d81bf54f..b948380bb 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -500,7 +500,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise) if (!store->isValidPath(info->path) || reregister) { /* !!! races */ if (canonicalise) - canonicalisePathMetaData(store->printStorePath(info->path), -1); + canonicalisePathMetaData(store->printStorePath(info->path), {}); if (!hashGiven) { HashResult hash = hashPath(htSHA256, store->printStorePath(info->path)); info->narHash = hash.first;