From 6cce316ff9a11e1e9f4c46c9720f2ff60731f46a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 15 2019 15:53:49 +0000 Subject: PR#967: use correct fileinfo checksum field Merges #967 https://pagure.io/koji/pull-request/967 Fixes: #966 https://pagure.io/koji/issue/966 kojikamid makes wrong md5sum check --- diff --git a/koji/util.py b/koji/util.py index bf2e2b2..495cea4 100644 --- a/koji/util.py +++ b/koji/util.py @@ -22,6 +22,7 @@ from __future__ import absolute_import from __future__ import division import calendar import datetime +import hashlib from koji.xmlrpcplus import DateTime from fnmatch import fnmatch import koji @@ -32,6 +33,7 @@ import re import resource import shutil import stat +import struct import sys import time import six.moves.configparser @@ -505,6 +507,43 @@ def filedigestAlgo(hdr): digest_algo = koji.RPM_FILEDIGESTALGO_IDS.get(digest_algo_id, 'unknown') return digest_algo.lower() + +def check_sigmd5(filename): + """Compare header's sigmd5 with actual md5 of hdr+payload without need of rpm""" + with open(filename, 'rb') as f: + leadsize = 96 + # skip magic + reserved + o = leadsize + 8 + f.seek(o) + data = f.read(8) + indexcount, storesize = struct.unpack('!II', data) + for idx in range(indexcount): + data = f.read(16) + tag, data_type, offset, count = struct.unpack('!IIII', data) + if tag == 1004: # SIGMD5 + assert(data_type == 7) # binary data + assert(count == 16) # 16 bytes of md5 + break + # seek to location of md5 + f.seek(o + 8 + indexcount * 16 + offset) + sigmd5 = f.read(16) + + # seek to start of header + sigsize = 8 + 16 * indexcount + storesize + o += sigsize + (8 - (sigsize % 8)) % 8 + f.seek(o) + + # compute md5 of rest of file + md5 = hashlib.md5() + while True: + d = f.read(1024**2) + if not d: + break + md5.update(d) + + return sigmd5 == md5.digest() + + def parseStatus(rv, prefix): if isinstance(prefix, (list, tuple)): prefix = ' '.join(prefix) diff --git a/tests/test_lib/test_check_sigmd5.py b/tests/test_lib/test_check_sigmd5.py new file mode 100644 index 0000000..ae5a7d8 --- /dev/null +++ b/tests/test_lib/test_check_sigmd5.py @@ -0,0 +1,35 @@ +# coding=utf-8 +from __future__ import absolute_import +import os.path +import shutil +import tempfile +try: + import unittest2 as unittest +except ImportError: + import unittest + +from koji.util import check_sigmd5 + + +class TestCheckSigMD5(unittest.TestCase): + + def setUp(self): + self.tempdir = tempfile.mkdtemp() + orig_path = os.path.join(os.path.dirname(__file__), 'data/rpms/test-deps-1-1.fc24.x86_64.rpm') + self.path = '%s/test.rpm' % self.tempdir + shutil.copyfile(orig_path, self.path) + + def tearDown(self): + shutil.rmtree(self.tempdir) + + def test_check_sigmd5_good(self): + value = check_sigmd5(self.path) + self.assertEqual(value, True) + + def test_check_sigmd5_bad(self): + # corrupt the file by truncating it + with open(self.path, 'a+b') as f: + f.seek(5, 2) + f.truncate() + value = check_sigmd5(self.path) + self.assertEqual(value, False) diff --git a/vm/kojikamid.py b/vm/kojikamid.py index 21403f6..d962050 100755 --- a/vm/kojikamid.py +++ b/vm/kojikamid.py @@ -305,28 +305,38 @@ class WindowsBuild(object): """Set the buildroot object to expired on the hub.""" self.server.expireBuildroot(self.buildroot_id) - def fetchFile(self, basedir, buildinfo, fileinfo, type): + def fetchFile(self, basedir, buildinfo, fileinfo, brtype): """Download the file from buildreq, at filepath, into the basedir""" destpath = os.path.join(basedir, fileinfo['localpath']) ensuredir(os.path.dirname(destpath)) - destfile = open(destpath, 'w') - offset = 0 - checksum = hashlib.md5() - while True: - encoded = self.server.getFile(buildinfo, fileinfo, encode_int(offset), 1048576, type) - if not encoded: - break - data = base64.b64decode(encoded) - del encoded - destfile.write(data) - offset += len(data) - checksum.update(data) - destfile.close() - digest = checksum.hexdigest() + if 'checksum_type' in fileinfo: + if fileinfo['checksum_type'] == 'sha1': + checksum = hashlib.sha1() + elif fileinfo['checksum_type'] == 'sha256': + checksum = hashlib.sha256() + elif fileinfo['checksum_type'] == 'md5': + checksum = hashlib.md5() + else: + raise BuildError('Unknown checksum type %s for %f' % ( + fileinfo['checksum_type'], + os.path.basename(fileinfo['localpath']))) + with open(destpath, 'w') as destfile: + offset = 0 + while True: + encoded = self.server.getFile(buildinfo, fileinfo, encode_int(offset), 1048576, brtype) + if not encoded: + break + data = base64.b64decode(encoded) + del encoded + destfile.write(data) + offset += len(data) + if 'checksum_type' in fileinfo: + checksum.update(data) # rpms don't have a md5sum in the fileinfo, but check it for everything else - if ('md5sum' in fileinfo) and (digest != fileinfo['md5sum']): - raise BuildError('md5 checksum validation failed for %s, %s (computed) != %s (provided)' % \ - (destpath, digest, fileinfo['md5sum'])) + digest = checksum.hexdigest() + if 'checksum' in fileinfo and fileinfo['checksum'] != digest: + raise BuildError('checksum validation failed for %s, %s (computed) != %s (provided)' % \ + (destpath, digest, fileinfo['checksum'])) self.logger.info('Retrieved %s (%s bytes, md5: %s)', destpath, offset, digest) def fetchBuildReqs(self): diff --git a/vm/kojivmd b/vm/kojivmd index 7e1dc7f..5aa2587 100755 --- a/vm/kojivmd +++ b/vm/kojivmd @@ -27,6 +27,7 @@ import koji.util from koji.daemon import SCM, TaskManager from koji.tasks import ServerExit, ServerRestart, BaseTaskHandler, MultiPlatformTask from koji.tasks import RestartTask, RestartVerifyTask +import hashlib import sys import logging import os @@ -37,6 +38,7 @@ import subprocess import libvirt import libxml2 import random +import rpm import socket import six.moves.xmlrpc_server import threading @@ -680,15 +682,27 @@ class VMExecTask(BaseTaskHandler): raise koji.BuildError('unsupported file type: %s' % type) koji.ensuredir(os.path.dirname(localpath)) with closing(requests.get(remote_url, stream=True)) as response: - f = open(localpath, 'wb') - length = response.headers.get('content-length') - if length is None: - f.write(response.content) - else: - for chunk in response.iter_content(chunk_size=65536): - f.write(chunk) - f.close() - + with open(localpath, 'wb') as f: + length = response.headers.get('content-length') + if length is None: + f.write(response.content) + else: + for chunk in response.iter_content(chunk_size=65536): + f.write(chunk) + if type == 'rpm': + # rpm, check sigmd5. It is enough, as if content is broken, + # rpm will fail later + hdr = koji.get_rpm_header(localpath) + payloadhash = koji.hex_string(hdr[rpm.RPMTAG_SIGMD5]) + if fileinfo['payloadhash'] != payloadhash: + raise koji.BuildError("Downloaded rpm %s doesn't match checksum (expected: %s, got %s)" % ( + os.path.basename(fileinfo['localpath']), + fileinfo['payloadhash'], payloadhash)) + if not koji.util.check_sigmd5(localpath): + raise koji.BuildError("Downloaded rpm %s doesn't match sigmd5" % \ + os.path.basename(fileinfo['localpath'])) + else: + self.verifyChecksum(localpath, fileinfo['checksum'], koji.CHECKSUM_TYPES[fileinfo['checksum_type']], localpath) return open(localpath, 'r') @@ -751,19 +765,20 @@ class VMExecTask(BaseTaskHandler): raise koji.BuildError('%s does not exist' % local_path) if algo == 'sha1': - sum = koji.util.sha1_constructor() + sum = hashlib.sha1() elif algo == 'md5': - sum = koji.util.md5_constructor() + sum = hashlib.md5() + elif algo == 'sha256': + sum == hashlib.sha256() else: raise koji.BuildError('unsupported checksum algorithm: %s' % algo) - fobj = open(local_path, 'r') - while True: - data = fobj.read(1048576) - if not data: - break - sum.update(data) - fobj.close() + with open(local_path, 'r') as f: + while True: + data = f.read(1048576) + if not data: + break + sum.update(data) if sum.hexdigest() == checksum: return True else: