Merge pull request #637 from jkloetzke/detect-artifact-corruption

Detect artifact corruption

Author: jkloetzke

pym/bob/audit.py

@@ -220,6 +220,9 @@ def getId(self):
     def getBuildId(self):
         return bytes.fromhex(self.__data['build-id'])
 
+    def getResultHash(self):
+        return bytes.fromhex(self.__data['result-hash'])
+
     def getReferences(self):
         deps = self.__data["dependencies"]
         ret = set()

pym/bob/builder.py

@@ -1492,7 +1492,7 @@ async def _downloadPackage(self, packageStep, depth, packageBuildId):
             dissectPackageInputState(BobState().getInputHashes(prettyPackagePath))
         workspaceChanged = False
         wasDownloaded = False
-        audit = os.path.join(prettyPackagePath, "..", "audit.json.gz")
+        audit = os.path.normpath(os.path.join(prettyPackagePath, "..", "audit.json.gz"))
         packageDigest = packageStep.getVariantId()
 
         # prune directory if we previously downloaded/built something different
@@ -1509,6 +1509,7 @@ async def _downloadPackage(self, packageStep, depth, packageBuildId):
             stepMessage(packageStep, "PRUNE", "{} ({})".format(prettyPackagePath,
                 reason), WARNING)
             emptyDirectory(prettyPackagePath)
+            removePath(audit)
             BobState().resetWorkspaceState(prettyPackagePath, packageDigest)
             oldInputBuildId = None
             oldInputFingerprint = None
@@ -1522,9 +1523,19 @@ async def _downloadPackage(self, packageStep, depth, packageBuildId):
                 packageBuildId, audit, prettyPackagePath, executor=self.__executor)
             if wasDownloaded:
                 self.__statistic.packagesDownloaded += 1
+                # Reject downloads without audit trail. They cannot be verified
+                # and break the audit trail of downstream packages.
+                if not os.path.exists(audit):
+                    raise BuildError("Downloaded artifact misses its audit trail!")
+
+                # Verify integrity of downloaded package to protect against
+                # data corruption.
+                packageHash = hashWorkspace(packageStep)
+                if Audit.fromFile(audit).getArtifact().getResultHash() != packageHash:
+                    raise BuildError("Corrupt downloaded artifact! Extracted content hash does not match audit trail.")
+
                 BobState().setInputHashes(prettyPackagePath,
                     packageInputDownloaded(packageBuildId))
-                packageHash = hashWorkspace(packageStep)
                 workspaceChanged = True
             elif layerDownloadMode == 'forced':
                 raise BuildError("Downloading artifact of layer %s failed" % layer)

pym/bob/develop/version.py

@@ -62,7 +62,7 @@ def getVersion():
 
     if not version:
         # Last fallback. See PEP 440 and adjust accordingly.
-        version = "1.0.dev999+unknown"
+        version = "1.1.dev999+unknown"
 
     return version
 

test/black-box/archive-corruption/config.yaml

@@ -0,0 +1 @@
+bobMinimumVersion: "1"

test/black-box/archive-corruption/recipes/root.yaml

@@ -0,0 +1,3 @@
+root: True
+packageScript: |
+    echo foo >result.txt

test/black-box/archive-corruption/run.sh

@@ -0,0 +1,69 @@
+#!/bin/bash -e
+. ../../test-lib.sh 2>/dev/null || { echo "Must run in script directory!" ; exit 1 ; }
+cleanup
+
+# Test that corruptions of binary artifacts are detected...
+
+# setup local archive
+trap 'rm -rf "${archiveDir}"' EXIT
+archiveDir=$(mktemp -d)
+
+upload="$archiveDir/uploads"
+cat >"${upload}.yaml" <<EOF
+archive:
+  -
+    name: "local"
+    backend: file
+    path: "$(mangle_path "$archiveDir/artifacts")"
+EOF
+scratch="$archiveDir/scratch"
+mkdir "$scratch"
+
+# fill archive
+run_bob build -c "$upload" --download=no --upload root
+ARTIFACTS=( $(/usr/bin/find "$archiveDir/artifacts" -type f) )
+test "${#ARTIFACTS[@]}" -eq 1
+
+# save artifact for later modification
+A="${ARTIFACTS[0]}"
+SAVE="$archiveDir/save.tgz"
+cp "$A" "$SAVE"
+
+# Verify download of re-packed artifact still works.
+rm "$A"
+pushd "$scratch"
+tar xf "$SAVE"
+tar --pax-option bob-archive-vsn=1 -zcf "$A" meta content
+popd
+run_bob dev -c "$upload" --download=forced root
+
+# Modify content. When downloading the artifact again, the corruption must be
+# detected.
+rm "$A"
+pushd "$scratch"
+tar xf "$SAVE"
+echo bar > content/result.txt
+tar --pax-option bob-archive-vsn=1 -zcf "$A" meta content
+popd
+
+rm -rf dev
+expect_fail run_bob dev -c "$upload" --download=forced root
+
+# Remove audit trail. Such artifacts must be rejected.
+rm "$A"
+pushd "$scratch"
+tar xf "$SAVE"
+tar --pax-option bob-archive-vsn=1 -zcf "$A" content
+popd
+
+rm -rf dev
+expect_fail run_bob dev -c "$upload" --download=forced root
+
+# Add garbage in the middle. Must fail gracefully.
+len=$(stat -c %s "$SAVE")
+head -c $((len - 64)) "$SAVE" > "$A"
+echo -n asdf >> "$A"
+tail -c 60 "$SAVE" >> "$A"
+
+rm -rf dev
+expect_fail run_bob dev -c "$upload" --download=forced root