Add integrity checking for builds

When a real build is downloaded, Koji can provide a checksum via API.
This commit adds verification of that checksum.

A mismatch will abort the compose. If Koji doesn't provide a checksum
for the particular sigkey, no checking will happen.

Nothing is still checked for scratch builds and images.

This patch requires Koji 1.32. When talking to an older version, there
is no checking done.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

doc/koji.rst

@@ -96,8 +96,10 @@ If the first compose already managed to hardlink the file before it gets
 replaced, there will be two copies of the file present locally.
 
 
-Caveats
--------
+Integrity checking
+------------------
 
-There is no integrity checking. Ideally Koji should provide checksums for the
-RPMs that would be verified after downloading. This is not yet available.
+There is minimal integrity checking. RPM packages belonging to real builds will
+be check to match the checksum provided by Koji hub.
+
+There is no checking for scratch builds or any images.

pungi/phases/pkgset/pkgsets.py

@@ -24,10 +24,12 @@ import json
 import os
 import time
 from six.moves import cPickle as pickle
+from functools import partial
 
 import kobo.log
 import kobo.pkgset
 import kobo.rpmlib
+from kobo.shortcuts import compute_file_checksums
 
 from kobo.threads import WorkerThread, ThreadPool
 
@@ -534,6 +536,23 @@ class KojiPackageSet(PackageSetBase):
         pathinfo = self.koji_wrapper.koji_module.pathinfo
         paths = []
 
+        if "getRPMChecksums" in self.koji_proxy.system.listMethods():
+
+            def checksum_validator(keyname, pkg_path):
+                checksums = self.koji_proxy.getRPMChecksums(
+                    rpm_info["id"], checksum_types=("sha256",)
+                )
+                if "sha256" in checksums.get(keyname, {}):
+                    computed = compute_file_checksums(pkg_path, ("sha256",))
+                    if computed["sha256"] != checksums[keyname]["sha256"]:
+                        raise RuntimeError("Checksum mismatch for %s" % pkg_path)
+
+        else:
+
+            def checksum_validator(keyname, pkg_path):
+                # Koji doesn't support checksums yet
+                pass
+
         attempts_left = self.signed_packages_retries + 1
         while attempts_left > 0:
             for sigkey in self.sigkey_ordering:
@@ -546,7 +565,9 @@ class KojiPackageSet(PackageSetBase):
                 )
                 if rpm_path not in paths:
                     paths.append(rpm_path)
-                path = self.downloader.get_file(rpm_path)
+                path = self.downloader.get_file(
+                    rpm_path, partial(checksum_validator, sigkey)
+                )
                 if path:
                     return path
 
@@ -561,7 +582,7 @@ class KojiPackageSet(PackageSetBase):
             # use an unsigned copy (if allowed)
             rpm_path = os.path.join(pathinfo.build(build_info), pathinfo.rpm(rpm_info))
             paths.append(rpm_path)
-            path = self.downloader.get_file(rpm_path)
+            path = self.downloader.get_file(rpm_path, partial(checksum_validator, ""))
             if path:
                 return path
 

pungi/wrappers/kojiwrapper.py

@@ -961,7 +961,14 @@ class KojiDownloadProxy:
                 shutil.copyfileobj(r.raw, f)
         return dest
 
-    def _atomic_download(self, url, dest):
+    def _delete(self, path):
+        """Try to delete file at given path and ignore errors."""
+        try:
+            os.remove(path)
+        except Exception:
+            self.logger.warning("Failed to delete %s", path)
+
+    def _atomic_download(self, url, dest, validator):
         """Atomically download a file
 
         :param str url: URL of the file to download
@@ -979,18 +986,25 @@ class KojiDownloadProxy:
         except Exception:
             # Download failed, let's make sure to clean up potentially partial
             # temporary file.
-            try:
-                os.remove(temp_file)
-            except Exception:
-                self.logger.warning("Failed to delete %s", temp_file)
-                pass
+            self._delete(temp_file)
+            raise
+
+        # Check if the temporary file is correct (assuming we were provided a
+        # validator function).
+        try:
+            if validator:
+                validator(temp_file)
+        except Exception:
+            # Validation failed. Let's delete the problematic file and re-raise
+            # the exception.
+            self._delete(temp_file)
             raise
 
         # Atomically move the temporary file into final location
         os.rename(temp_file, dest)
         return dest
 
-    def _download_file(self, path):
+    def _download_file(self, path, validator):
         """Ensure file on Koji volume in ``path`` is present in the local
         cache.
 
@@ -1025,12 +1039,17 @@ class KojiDownloadProxy:
                 os.utime(destination_file)
                 return destination_file
 
-            return self._atomic_download(url, destination_file)
+            return self._atomic_download(url, destination_file, validator)
 
-    def get_file(self, path):
+    def get_file(self, path, validator=None):
         """
         If path refers to an existing file in Koji, return a valid local path
         to it. If no such file exists, return None.
+
+        :param validator: A callable that will be called with the path to the
+            downloaded file if and only if the file was actually downloaded.
+            Any exception raised from there will be abort the download and be
+            propagated.
         """
         if self.has_local_access:
             # We have koji volume mounted locally. No transformation needed for
@@ -1040,4 +1059,4 @@ class KojiDownloadProxy:
             return None
         else:
             # We need to download the file.
-            return self._download_file(path)
+            return self._download_file(path, validator)