diff options
author | Eelco Dolstra <eelco.dolstra@logicblox.com> | 2012-07-27T16·16-0400 |
---|---|---|
committer | Eelco Dolstra <eelco.dolstra@logicblox.com> | 2012-07-27T16·16-0400 |
commit | 73acb8b836affe5dfade9dd6e3339ad2f9191add (patch) | |
tree | 756e59c48da948362ba4371f4fe2b5bbdb1c35fe | |
parent | fbf59d95f66012349fdcd2b60f34b9efb32e6319 (diff) |
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.
Diffstat (limited to '')
-rwxr-xr-x | scripts/copy-from-other-stores.pl.in | 5 | ||||
-rw-r--r-- | scripts/download-from-binary-cache.pl.in | 11 | ||||
-rwxr-xr-x | scripts/download-using-manifests.pl.in | 13 | ||||
-rw-r--r-- | src/libstore/build.cc | 39 | ||||
-rwxr-xr-x | tests/substituter.sh | 1 |
5 files changed, 43 insertions, 26 deletions
diff --git a/scripts/copy-from-other-stores.pl.in b/scripts/copy-from-other-stores.pl.in index 92869ee7a107..3ee6f075b27e 100755 --- a/scripts/copy-from-other-stores.pl.in +++ b/scripts/copy-from-other-stores.pl.in @@ -52,7 +52,7 @@ if ($ARGV[0] eq "--query") { next unless defined $store; $ENV{"NIX_DB_DIR"} = "$store/var/nix/db"; - + my $deriver = `@bindir@/nix-store --query --deriver $storePath`; die "cannot query deriver of `$storePath'" if $? != 0; chomp $deriver; @@ -87,9 +87,10 @@ elsif ($ARGV[0] eq "--substitute") { my $storePath = $ARGV[1]; my ($store, $sourcePath) = findStorePath $storePath; 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 or die "cannot copy `$sourcePath' to `$storePath'"; + print "\n"; # no hash to verify } diff --git a/scripts/download-from-binary-cache.pl.in b/scripts/download-from-binary-cache.pl.in index 9e1c774a5a7b..823ecd9d9194 100644 --- a/scripts/download-from-binary-cache.pl.in +++ b/scripts/download-from-binary-cache.pl.in @@ -432,13 +432,10 @@ sub downloadBinary { die "download of `$info->{url}' failed" . ($! ? ": $!" : "") . "\n" unless $? == 0; next; } - # The hash in the manifest can be either in base-16 or - # base-32. Handle both. - $info->{narHash} =~ /^sha256:(.*)$/ or die "invalid hash"; - 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; + + # Tell Nix about the expected hash so it can verify it. + print "$info->{narHash}\n"; + print STDERR "\n"; return 1; } diff --git a/scripts/download-using-manifests.pl.in b/scripts/download-using-manifests.pl.in index ed63e792ea80..04bcce90da38 100755 --- a/scripts/download-using-manifests.pl.in +++ b/scripts/download-using-manifests.pl.in @@ -353,19 +353,10 @@ while (scalar @path > 0) { } -# Make sure that the hash declared in the manifest matches what we -# downloaded and unpacked. +# Tell Nix about the expected hash so it can verify it. die "cannot check integrity of the downloaded path since its hash is not known\n" unless defined $finalNarHash; - -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 "$finalNarHash\n"; print STDERR "\n"; diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 90dc2b79b532..887858fce30c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -371,6 +371,7 @@ void commonChildInit(Pipe & logPipe) if (dup2(logPipe.writeSide, STDERR_FILENO) == -1) throw SysError("cannot pipe standard error into log file"); logPipe.readSide.close(); + logPipe.writeSide.close(); /* Dup stderr to stdout. */ if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) @@ -2273,7 +2274,10 @@ private: /* Path info returned by the substituter's query info operation. */ 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; /* The process ID of the builder. */ @@ -2466,6 +2470,7 @@ void SubstitutionGoal::tryToRun() printMsg(lvlInfo, format("fetching path `%1%'...") % storePath); + outPipe.create(); logPipe.create(); /* Remove the (stale) output path if it exists. */ @@ -2482,10 +2487,13 @@ void SubstitutionGoal::tryToRun() case 0: try { /* child */ - logPipe.readSide.close(); - 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. */ Strings args; args.push_back(baseNameOf(sub)); @@ -2506,6 +2514,7 @@ void SubstitutionGoal::tryToRun() /* parent */ pid.setSeparatePG(true); pid.setKillSignal(SIGTERM); + outPipe.writeSide.close(); logPipe.writeSide.close(); worker.childStarted(shared_from_this(), pid, singleton<set<int> >(logPipe.readSide), true, true); @@ -2534,9 +2543,12 @@ void SubstitutionGoal::finished() /* Close the read side of the logger pipe. */ 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. */ + HashResult hash; try { if (!statusOk(status)) @@ -2546,6 +2558,23 @@ void SubstitutionGoal::finished() if (!pathExists(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) { printMsg(lvlInfo, e.msg()); @@ -2563,8 +2592,6 @@ void SubstitutionGoal::finished() canonicalisePathMetaData(storePath); - HashResult hash = hashPath(htSHA256, storePath); - worker.store.optimisePath(storePath); // FIXME: combine with hashPath() ValidPathInfo info2; diff --git a/tests/substituter.sh b/tests/substituter.sh index a6bdacfd66f7..885655760e05 100755 --- a/tests/substituter.sh +++ b/tests/substituter.sh @@ -29,6 +29,7 @@ if test $1 = "--query"; then elif test $1 = "--substitute"; then mkdir $2 echo "Hallo Wereld" > $2/hello + echo # no expected hash else echo "unknown substituter operation" exit 1 |