Skip to content

Commit 5333b25

Browse files
committed
Add referrer checks for access status
1 parent c5f8a40 commit 5333b25

File tree

6 files changed

+49
-6
lines changed

6 files changed

+49
-6
lines changed

src/libexpr/primops.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1460,6 +1460,8 @@ static void derivationStrictInternal(EvalState & state, const std::string & drvN
14601460
state.forceAttrs(*outputs->value, noPos,
14611461
"while evaluating the `__permissions.outputs` "
14621462
"attribute passed to builtins.derivationStrict");
1463+
auto outputMap = state.store->queryPartialDerivationOutputMap(drvPath);
1464+
std::map<StorePath, LocalGranularAccessStore::AccessStatus> accessMap;
14631465
for (auto & output : *outputs->value->attrs) {
14641466
if (!drv.outputs.contains(state.symbols[output.name]))
14651467
state.debugThrowLastTrace(EvalError({
@@ -1468,8 +1470,13 @@ static void derivationStrictInternal(EvalState & state, const std::string & drvN
14681470
}));
14691471
LocalGranularAccessStore::AccessStatus status;
14701472
readAccessStatus(state, output, &status, fmt("__permissions.outputs.%s", state.symbols[output.name]), "builtins.derivationStrict");
1471-
require<LocalGranularAccessStore>(*state.store).setAccessStatus(StoreObjectDerivationOutput {drvPath, std::string(state.symbols[{output.name}])}, status, true);
1473+
auto outputName = std::string(state.symbols[{output.name}]);
1474+
if (auto path = outputMap.at(outputName))
1475+
accessMap[*path] = status;
1476+
else
1477+
require<LocalGranularAccessStore>(*state.store).setAccessStatus(StoreObjectDerivationOutput {drvPath, outputName}, status, true);
14721478
}
1479+
require<LocalGranularAccessStore>(*state.store).setAccessStatus(accessMap);
14731480
}
14741481
auto log = attr->value->attrs->find(state.sLog);
14751482
if (log != attr->value->attrs->end()) {

src/libstore/build/local-derivation-goal.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3023,7 +3023,8 @@ void LocalDerivationGoal::deleteTmpDir(bool force)
30233023
if (experimentalFeatureSettings.isEnabled(Xp::ACLs))
30243024
if (auto store = dynamic_cast<LocalGranularAccessStore*>(&worker.store))
30253025
if (store->effectiveUser) {
3026-
chown(tmpDir.c_str(), store->effectiveUser->uid, info.st_gid);
3026+
if (chown(tmpDir.c_str(), store->effectiveUser->uid, info.st_gid) == -1)
3027+
throw SysError("cannot change ownership %s", tmpDir.c_str());
30273028
chowned = true;
30283029
}
30293030
if (!chowned)

src/libstore/granular-access-store.hh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ struct GranularAccessStore : public virtual Store
5757
virtual void setAccessStatus(const StoreObject & storeObject, const AccessStatus & status, const bool & ensureAccessCheck) = 0;
5858
virtual AccessStatus getAccessStatus(const StoreObject & storeObject) = 0;
5959

60+
virtual void setAccessStatus(const std::map<StorePath, AccessStatus> pathMap)
61+
{
62+
StorePathSet pathSet;
63+
for (auto [path, _] : pathMap)
64+
pathSet.insert(path);
65+
auto paths = topoSortPaths(pathSet);
66+
for (auto path : paths) {
67+
setAccessStatus(path, pathMap.at(path), true);
68+
}
69+
}
70+
6071
virtual std::set<AccessControlGroup> getSubjectGroupsUncached(AccessControlSubject subject) = 0;
6172

6273
std::set<AccessControlGroup> getSubjectGroups(AccessControlSubject subject)

src/libstore/local-store.cc

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -858,11 +858,11 @@ StorePathSet LocalStore::queryAllValidPaths()
858858
}
859859

860860

861-
void LocalStore::queryReferrers(State & state, const StorePath & path, StorePathSet & referrers)
861+
void LocalStore::queryReferrers(State & state, const StorePath & path, StorePathSet & referrers, bool accessCheck)
862862
{
863863
auto useQueryReferrers(state.stmts->QueryReferrers.use()(printStorePath(path)));
864864

865-
if (!canAccess(path)) throw AccessDenied("Access Denied");
865+
if (accessCheck && !canAccess(path)) throw AccessDenied("Access Denied");
866866

867867
while (useQueryReferrers.next())
868868
referrers.insert(parseStorePath(useQueryReferrers.getStr(0)));
@@ -1086,7 +1086,7 @@ void LocalStore::setCurrentAccessStatus(const Path & path, const LocalStore::Acc
10861086

10871087
auto info = promise.get_future().get();
10881088

1089-
if (info){
1089+
if (info) {
10901090
for (auto reference : info->references) {
10911091
if (reference == storePath) continue;
10921092
auto otherStatus = getCurrentAccessStatus(printStorePath(reference));
@@ -1103,6 +1103,28 @@ void LocalStore::setCurrentAccessStatus(const Path & path, const LocalStore::Acc
11031103
}
11041104
}
11051105
}
1106+
1107+
StorePathSet referrers;
1108+
retrySQLite<void>([&]() {
1109+
auto state(_state.lock());
1110+
queryReferrers(*state, storePath, referrers, false);
1111+
});
1112+
1113+
for (auto referrer : referrers) {
1114+
if (referrer == storePath) continue;
1115+
auto otherStatus = getAccessStatus(referrer);
1116+
if (!status.isProtected) continue;
1117+
if (!otherStatus.isProtected)
1118+
throw AccessDenied("can not make %s protected because it is referenced by a non-protected path %s", path, printStorePath(referrer));
1119+
std::vector<AccessControlEntity> difference;
1120+
std::set_difference(otherStatus.entities.begin(), otherStatus.entities.end(), status.entities.begin(), status.entities.end(),std::inserter(difference, difference.begin()));
1121+
1122+
if (! difference.empty()) {
1123+
std::string entities;
1124+
for (auto entity : difference) entities += ACL::printTag(entity) + ", ";
1125+
throw AccessDenied("can not deny %s access to %s because it is referenced by a path %s to which they do not have access", entities.substr(0, entities.size()-2), path, printStorePath(referrer));
1126+
}
1127+
}
11061128
}
11071129
}
11081130

src/libstore/local-store.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ private:
371371

372372
// Internal versions that are not wrapped in retry_sqlite.
373373
bool isValidPath_(State & state, const StorePath & path);
374-
void queryReferrers(State & state, const StorePath & path, StorePathSet & referrers);
374+
void queryReferrers(State & state, const StorePath & path, StorePathSet & referrers, bool accessCheck = true);
375375

376376
/**
377377
* Add signatures to a ValidPathInfo or Realisation using the secret keys

tests/nixos/acls.nix

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ let
1313
permissions = {
1414
protected = true;
1515
users = ["root"];
16+
groups = ["root"];
1617
};
1718
};
1819
buildCommand = "echo Example > $out; cat $exampleSource >> $out";
@@ -34,6 +35,7 @@ let
3435
permissions = {
3536
protected = true;
3637
users = ["root"];
38+
groups = ["root"];
3739
};
3840
};
3941
buildCommand = "echo Example > $out; cat $exampleSource >> $out";

0 commit comments

Comments
 (0)