Skip to content

Commit

Permalink
Add referrer checks for access status
Browse files Browse the repository at this point in the history
  • Loading branch information
balsoft committed Feb 13, 2024
1 parent c5f8a40 commit 5333b25
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 6 deletions.
9 changes: 8 additions & 1 deletion src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1460,6 +1460,8 @@ static void derivationStrictInternal(EvalState & state, const std::string & drvN
state.forceAttrs(*outputs->value, noPos,
"while evaluating the `__permissions.outputs` "
"attribute passed to builtins.derivationStrict");
auto outputMap = state.store->queryPartialDerivationOutputMap(drvPath);
std::map<StorePath, LocalGranularAccessStore::AccessStatus> accessMap;
for (auto & output : *outputs->value->attrs) {
if (!drv.outputs.contains(state.symbols[output.name]))
state.debugThrowLastTrace(EvalError({
Expand All @@ -1468,8 +1470,13 @@ static void derivationStrictInternal(EvalState & state, const std::string & drvN
}));
LocalGranularAccessStore::AccessStatus status;
readAccessStatus(state, output, &status, fmt("__permissions.outputs.%s", state.symbols[output.name]), "builtins.derivationStrict");
require<LocalGranularAccessStore>(*state.store).setAccessStatus(StoreObjectDerivationOutput {drvPath, std::string(state.symbols[{output.name}])}, status, true);
auto outputName = std::string(state.symbols[{output.name}]);
if (auto path = outputMap.at(outputName))
accessMap[*path] = status;
else
require<LocalGranularAccessStore>(*state.store).setAccessStatus(StoreObjectDerivationOutput {drvPath, outputName}, status, true);
}
require<LocalGranularAccessStore>(*state.store).setAccessStatus(accessMap);
}
auto log = attr->value->attrs->find(state.sLog);
if (log != attr->value->attrs->end()) {
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3023,7 +3023,8 @@ void LocalDerivationGoal::deleteTmpDir(bool force)
if (experimentalFeatureSettings.isEnabled(Xp::ACLs))
if (auto store = dynamic_cast<LocalGranularAccessStore*>(&worker.store))
if (store->effectiveUser) {
chown(tmpDir.c_str(), store->effectiveUser->uid, info.st_gid);
if (chown(tmpDir.c_str(), store->effectiveUser->uid, info.st_gid) == -1)
throw SysError("cannot change ownership %s", tmpDir.c_str());
chowned = true;
}
if (!chowned)
Expand Down
11 changes: 11 additions & 0 deletions src/libstore/granular-access-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ struct GranularAccessStore : public virtual Store
virtual void setAccessStatus(const StoreObject & storeObject, const AccessStatus & status, const bool & ensureAccessCheck) = 0;
virtual AccessStatus getAccessStatus(const StoreObject & storeObject) = 0;

virtual void setAccessStatus(const std::map<StorePath, AccessStatus> pathMap)
{
StorePathSet pathSet;
for (auto [path, _] : pathMap)
pathSet.insert(path);
auto paths = topoSortPaths(pathSet);
for (auto path : paths) {
setAccessStatus(path, pathMap.at(path), true);
}
}

virtual std::set<AccessControlGroup> getSubjectGroupsUncached(AccessControlSubject subject) = 0;

std::set<AccessControlGroup> getSubjectGroups(AccessControlSubject subject)
Expand Down
28 changes: 25 additions & 3 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -858,11 +858,11 @@ StorePathSet LocalStore::queryAllValidPaths()
}


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

if (!canAccess(path)) throw AccessDenied("Access Denied");
if (accessCheck && !canAccess(path)) throw AccessDenied("Access Denied");

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

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

if (info){
if (info) {
for (auto reference : info->references) {
if (reference == storePath) continue;
auto otherStatus = getCurrentAccessStatus(printStorePath(reference));
Expand All @@ -1103,6 +1103,28 @@ void LocalStore::setCurrentAccessStatus(const Path & path, const LocalStore::Acc
}
}
}

StorePathSet referrers;
retrySQLite<void>([&]() {
auto state(_state.lock());
queryReferrers(*state, storePath, referrers, false);
});

for (auto referrer : referrers) {
if (referrer == storePath) continue;
auto otherStatus = getAccessStatus(referrer);
if (!status.isProtected) continue;
if (!otherStatus.isProtected)
throw AccessDenied("can not make %s protected because it is referenced by a non-protected path %s", path, printStorePath(referrer));
std::vector<AccessControlEntity> difference;
std::set_difference(otherStatus.entities.begin(), otherStatus.entities.end(), status.entities.begin(), status.entities.end(),std::inserter(difference, difference.begin()));

if (! difference.empty()) {
std::string entities;
for (auto entity : difference) entities += ACL::printTag(entity) + ", ";
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));
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstore/local-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ private:

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

/**
* Add signatures to a ValidPathInfo or Realisation using the secret keys
Expand Down
2 changes: 2 additions & 0 deletions tests/nixos/acls.nix
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ let
permissions = {
protected = true;
users = ["root"];
groups = ["root"];
};
};
buildCommand = "echo Example > $out; cat $exampleSource >> $out";
Expand All @@ -34,6 +35,7 @@ let
permissions = {
protected = true;
users = ["root"];
groups = ["root"];
};
};
buildCommand = "echo Example > $out; cat $exampleSource >> $out";
Expand Down

0 comments on commit 5333b25

Please sign in to comment.