Use the hash modulo in the derivation outputs #4348
Conversation
|
A few notes on that:
|
| if (!goal.isAllowed(info.id.drvPath)) | ||
| throw InvalidPath("cannot register unknown drv output '%s' in recursive Nix", printStorePath(info.id.drvPath)); | ||
| // XXX: Should we check for something here? Probably, but I'm not sure | ||
| // how |
There was a problem hiding this comment.
Probably registerDrvOutput shouldn't be allowed in RestrictedStore at all, since the caller is completely untrusted and supposed to be sandboxed.
There was a problem hiding this comment.
I guess once we take the time to properly implement this, we should allow it if the drv output matches an allowed path, but that's complicated to check as we only know its hash modulo and not the drv path. So I'll just forbid everything for now
| if (!goal.isAllowed(id.drvPath)) | ||
| throw InvalidPath("cannot query the output info for unknown derivation '%s' in recursive Nix", printStorePath(id.drvPath)); | ||
| // XXX: Should we check for something here? Probably, but I'm not sure | ||
| // how |
There was a problem hiding this comment.
queryRealisation() should only succeed in RestrictedStore if DrvOutput refers to a derivation that the client added (via addToStore) and where it has built the output (i.e. has done a buildDerivation()). Otherwise it can leak info to which the sandboxed build isn't supposed to have access.
There was a problem hiding this comment.
Indeed… like #4348 (comment) I'll just restrict everything for now as it's slightly tricky to implement and we might change it eventually anyways.
I've opened #4353 to track it
|
Needs to double-check that this doesn't change the daemon protocol for |
thufschmitt
force-pushed
the
ca/properly-store-outputs
branch
from
2557e8a
to
3ac9d74
Compare
Rather than storing the derivation outputs as `drvPath!outputName` internally, store them as `drvHashModulo!outputName` (or `outputHash!outputName` for fixed-output derivations). This makes the storage slightly more opaque, but enables an earlier cutoff in cases where a fixed-output dependency changes (but keeps the same output hash) − same as what we already do for input-addressed derivations.
There's currently no way to properly filter them, so disallow them altogether instead.
thufschmitt
force-pushed
the
ca/use-hashmodulo
branch
from
d93fe07
to
e9b39f6
Compare
This isn't the case, |
|
🎉 All dependencies have been resolved ! |
PRs #4370 and #4348 had a bad interaction in that the second broke the fist one in a not trivial way. The issue was that since #4348 the logic for detecting whether a derivation output is already built requires some logic that was specific to the `LocalStore`. It happens though that most of this logic could be upstreamed to any `Store`, which is what this commit does.
Rather than storing the derivation outputs as
drvPath!outputNameinternally,store them as
drvHashModulo!outputName(oroutputHash!outputNameforfixed-output derivations).
This makes the storage slightly more opaque, but enables an earlier
cutoff in cases where a fixed-output dependency changes (but keeps the
same output hash) − same as what we already do for input-addressed
derivations.
Depends on: #4330
Fix: #4347