Let build.cc verify the expected hash of a substituter's output

Since SubstitutionGoal::finished() in build.cc computes the hash
anyway, we can prevent the inefficiency of computing the hash twice by
letting the substituter tell Nix about the expected hash, which can
then verify it.
This commit is contained in:
Eelco Dolstra 2012-07-27 12:16:02 -04:00
parent fbf59d95f6
commit 73acb8b836
5 changed files with 43 additions and 26 deletions

View file

@ -87,9 +87,10 @@ elsif ($ARGV[0] eq "--substitute") {
my $storePath = $ARGV[1]; my $storePath = $ARGV[1];
my ($store, $sourcePath) = findStorePath $storePath; my ($store, $sourcePath) = findStorePath $storePath;
die unless $store; die unless $store;
print "\n*** Copying `$storePath' from `$sourcePath'\n\n"; print STDERR "\n*** Copying `$storePath' from `$sourcePath'\n\n";
system("$binDir/nix-store --dump $sourcePath | $binDir/nix-store --restore $storePath") == 0 system("$binDir/nix-store --dump $sourcePath | $binDir/nix-store --restore $storePath") == 0
or die "cannot copy `$sourcePath' to `$storePath'"; or die "cannot copy `$sourcePath' to `$storePath'";
print "\n"; # no hash to verify
} }

View file

@ -432,13 +432,10 @@ sub downloadBinary {
die "download of `$info->{url}' failed" . ($! ? ": $!" : "") . "\n" unless $? == 0; die "download of `$info->{url}' failed" . ($! ? ": $!" : "") . "\n" unless $? == 0;
next; next;
} }
# The hash in the manifest can be either in base-16 or
# base-32. Handle both. # Tell Nix about the expected hash so it can verify it.
$info->{narHash} =~ /^sha256:(.*)$/ or die "invalid hash"; print "$info->{narHash}\n";
my $hash = $1;
my $hash2 = hashPath("sha256", 1, $storePath);
die "hash mismatch in downloaded path $storePath; expected $hash, got $hash2\n"
if $hash ne $hash2;
print STDERR "\n"; print STDERR "\n";
return 1; return 1;
} }

View file

@ -353,19 +353,10 @@ while (scalar @path > 0) {
} }
# Make sure that the hash declared in the manifest matches what we # Tell Nix about the expected hash so it can verify it.
# downloaded and unpacked.
die "cannot check integrity of the downloaded path since its hash is not known\n" die "cannot check integrity of the downloaded path since its hash is not known\n"
unless defined $finalNarHash; unless defined $finalNarHash;
print "$finalNarHash\n";
my ($hashAlgo, $hash) = parseHash $finalNarHash;
# The hash in the manifest can be either in base-16 or base-32.
# Handle both.
my $hash2 = hashPath($hashAlgo, $hashAlgo eq "sha256" && length($hash) != 64, $targetPath);
die "hash mismatch in downloaded path $targetPath; expected $hash, got $hash2\n"
if $hash ne $hash2;
print STDERR "\n"; print STDERR "\n";

View file

@ -371,6 +371,7 @@ void commonChildInit(Pipe & logPipe)
if (dup2(logPipe.writeSide, STDERR_FILENO) == -1) if (dup2(logPipe.writeSide, STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file"); throw SysError("cannot pipe standard error into log file");
logPipe.readSide.close(); logPipe.readSide.close();
logPipe.writeSide.close();
/* Dup stderr to stdout. */ /* Dup stderr to stdout. */
if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1)
@ -2273,7 +2274,10 @@ private:
/* Path info returned by the substituter's query info operation. */ /* Path info returned by the substituter's query info operation. */
SubstitutablePathInfo info; SubstitutablePathInfo info;
/* Pipe for the substitute's standard output/error. */ /* Pipe for the substituter's standard output. */
Pipe outPipe;
/* Pipe for the substituter's standard error. */
Pipe logPipe; Pipe logPipe;
/* The process ID of the builder. */ /* The process ID of the builder. */
@ -2466,6 +2470,7 @@ void SubstitutionGoal::tryToRun()
printMsg(lvlInfo, format("fetching path `%1%'...") % storePath); printMsg(lvlInfo, format("fetching path `%1%'...") % storePath);
outPipe.create();
logPipe.create(); logPipe.create();
/* Remove the (stale) output path if it exists. */ /* Remove the (stale) output path if it exists. */
@ -2482,10 +2487,13 @@ void SubstitutionGoal::tryToRun()
case 0: case 0:
try { /* child */ try { /* child */
logPipe.readSide.close();
commonChildInit(logPipe); commonChildInit(logPipe);
if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
throw SysError("cannot dup output pipe into stdout");
outPipe.readSide.close();
outPipe.writeSide.close();
/* Fill in the arguments. */ /* Fill in the arguments. */
Strings args; Strings args;
args.push_back(baseNameOf(sub)); args.push_back(baseNameOf(sub));
@ -2506,6 +2514,7 @@ void SubstitutionGoal::tryToRun()
/* parent */ /* parent */
pid.setSeparatePG(true); pid.setSeparatePG(true);
pid.setKillSignal(SIGTERM); pid.setKillSignal(SIGTERM);
outPipe.writeSide.close();
logPipe.writeSide.close(); logPipe.writeSide.close();
worker.childStarted(shared_from_this(), worker.childStarted(shared_from_this(),
pid, singleton<set<int> >(logPipe.readSide), true, true); pid, singleton<set<int> >(logPipe.readSide), true, true);
@ -2534,9 +2543,12 @@ void SubstitutionGoal::finished()
/* Close the read side of the logger pipe. */ /* Close the read side of the logger pipe. */
logPipe.readSide.close(); logPipe.readSide.close();
debug(format("substitute for `%1%' finished") % storePath); /* Get the hash info from stdout. */
string expectedHashStr = statusOk(status) ? readLine(outPipe.readSide) : "";
outPipe.readSide.close();
/* Check the exit status and the build result. */ /* Check the exit status and the build result. */
HashResult hash;
try { try {
if (!statusOk(status)) if (!statusOk(status))
@ -2546,6 +2558,23 @@ void SubstitutionGoal::finished()
if (!pathExists(storePath)) if (!pathExists(storePath))
throw SubstError(format("substitute did not produce path `%1%'") % storePath); throw SubstError(format("substitute did not produce path `%1%'") % storePath);
hash = hashPath(htSHA256, storePath);
/* Verify the expected hash we got from the substituer. */
if (expectedHashStr != "") {
size_t n = expectedHashStr.find(':');
if (n == string::npos)
throw Error(format("bad hash from substituter: %1%") % expectedHashStr);
HashType hashType = parseHashType(string(expectedHashStr, 0, n));
if (hashType == htUnknown)
throw Error(format("unknown hash algorithm in `%1%'") % expectedHashStr);
Hash expectedHash = parseHash16or32(hashType, string(expectedHashStr, n + 1));
Hash actualHash = hashType == htSHA256 ? hash.first : hashPath(hashType, storePath).first;
if (expectedHash != actualHash)
throw SubstError(format("hash mismatch in downloaded path `%1%': expected %2%, got %3%")
% storePath % printHash(expectedHash) % printHash(actualHash));
}
} catch (SubstError & e) { } catch (SubstError & e) {
printMsg(lvlInfo, e.msg()); printMsg(lvlInfo, e.msg());
@ -2563,8 +2592,6 @@ void SubstitutionGoal::finished()
canonicalisePathMetaData(storePath); canonicalisePathMetaData(storePath);
HashResult hash = hashPath(htSHA256, storePath);
worker.store.optimisePath(storePath); // FIXME: combine with hashPath() worker.store.optimisePath(storePath); // FIXME: combine with hashPath()
ValidPathInfo info2; ValidPathInfo info2;

View file

@ -29,6 +29,7 @@ if test $1 = "--query"; then
elif test $1 = "--substitute"; then elif test $1 = "--substitute"; then
mkdir $2 mkdir $2
echo "Hallo Wereld" > $2/hello echo "Hallo Wereld" > $2/hello
echo # no expected hash
else else
echo "unknown substituter operation" echo "unknown substituter operation"
exit 1 exit 1