From b625ccea063a1f50279d07f1ac439a217c265aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Fri, 16 Jun 2023 11:00:29 +0200 Subject: [PATCH] Add integrity checking for builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ář (cherry picked from commit 77f8fa25ad50b000d50c217955c6139d390b498e) --- doc/koji.rst | 10 +++++---- pungi/phases/pkgset/pkgsets.py | 25 ++++++++++++++++++++-- pungi/wrappers/kojiwrapper.py | 39 +++++++++++++++++++++++++--------- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/doc/koji.rst b/doc/koji.rst index 7403e551..411da9fc 100644 --- a/doc/koji.rst +++ b/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. diff --git a/pungi/phases/pkgset/pkgsets.py b/pungi/phases/pkgset/pkgsets.py index e8e84bb0..b3f4f6e1 100644 --- a/pungi/phases/pkgset/pkgsets.py +++ b/pungi/phases/pkgset/pkgsets.py @@ -26,10 +26,12 @@ import time import pgpy import rpm 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 @@ -545,6 +547,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: @@ -557,7 +576,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 @@ -572,7 +593,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 diff --git a/pungi/wrappers/kojiwrapper.py b/pungi/wrappers/kojiwrapper.py index 6358f882..889b80dc 100644 --- a/pungi/wrappers/kojiwrapper.py +++ b/pungi/wrappers/kojiwrapper.py @@ -1001,7 +1001,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 @@ -1019,18 +1026,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. @@ -1065,12 +1079,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 @@ -1080,4 +1099,4 @@ class KojiDownloadProxy: return None else: # We need to download the file. - return self._download_file(path) + return self._download_file(path, validator)