From c4306d6476a5cc72c2a351a7e736a23b05cb4d4f Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Sep 16 2019 12:23:52 +0000 Subject: Rework the CoreOS signer to handle ostree requests properly Signed-off-by: Aurélien Bompard --- diff --git a/robosignatory/cli.py b/robosignatory/cli.py index ab79ec2..857781a 100644 --- a/robosignatory/cli.py +++ b/robosignatory/cli.py @@ -5,7 +5,7 @@ from fedora_messaging.config import conf import robosignatory.work from robosignatory.tag import TagSigner -from robosignatory.coreos import CoreOSSigner +from robosignatory.coreos import CoreOSSigner, ArtifactSignerWrapper, OSTreeSignerWrapper from robosignatory import utils @@ -48,14 +48,21 @@ def atomic(ref_update, ref, commitid): signer, ref, commitid, doref=ref_update, **val) -@cli.command("sign-coreos") +@cli.command("sign-coreos-artifact") @click.argument("file_url") @click.argument("checksum") -def coreos(file_url, checksum): - signing_config = conf["consumer_config"]["signing"] - signer = utils.get_signing_helper(**signing_config) - signer = CoreOSSigner(conf["consumer_config"]) - objects = [ - {"file": file_url, "checksum": checksum} - ] - signer.sign_objects(objects) +def coreos_artifact(file_url, checksum): + consumer = CoreOSSigner(conf["consumer_config"]) + key = consumer.get_key(None) + signing_wrapper = ArtifactSignerWrapper(consumer.signer, key, consumer.bucket) + signing_wrapper.sign(file_url, checksum) + + +@cli.command("sign-coreos-ostree") +@click.argument("file_url") +@click.argument("checksum") +def coreos_ostree(file_url, checksum): + consumer = CoreOSSigner(conf["consumer_config"]) + key = consumer.get_key(None) + signing_wrapper = OSTreeSignerWrapper(consumer.signer, key, consumer.bucket) + signing_wrapper.sign(file_url, checksum) diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 467ccda..20c0438 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -1,14 +1,17 @@ from __future__ import unicode_literals, absolute_import +import os +import stat +import logging import shutil import tempfile import boto3 import robosignatory.utils as utils -import robosignatory.work +from six.moves.urllib.parse import urlparse -import logging -log = logging.getLogger("robosignatory.coreosconsumer") + +log = logging.getLogger(__name__) class CoreOSSigner(object): @@ -31,6 +34,11 @@ class CoreOSSigner(object): log.info('CoreOSSigner ready for service') + def get_key(self, msg): + # Evaluation of the key is here and not in __init__ because we may want + # a stream or version-dependant key in the future. + return self.config["coreos"]["key"] + def consume(self, msg): # Message structure: # https://github.com/coreos/fedora-coreos-tracker/issues/198#issuecomment-513944390 @@ -38,23 +46,105 @@ class CoreOSSigner(object): 'CoreOS wants to sign ' '%(build_id)s on %(stream)s for %(basearch)s' % msg.body ) + key = self.get_key(msg) + if msg.topic.endswith('.coreos.build.request.artifacts-sign'): - objects = msg.body["artifacts"] + wrapper = ArtifactSignerWrapper(self.signer, key, self.bucket) + for artifact in msg.body["artifacts"]: + wrapper.sign(artifact["file"], artifact["checksum"]) + elif msg.topic.endswith('.coreos.build.request.ostree-sign'): - objects = [{ - 'file': msg.body["commit_object"], - 'checksum': msg.body["checksum"], - }] - self.sign_objects(objects) + wrapper = OSTreeSignerWrapper(self.signer, key, self.bucket) + wrapper.sign(msg.body["commit_object"], msg.body["checksum"]) - def sign_objects(self, objects): - # Evaluation of the key is here and not in __init__ because we may want - # a stream or version-dependant key in the future. - key = self.config["coreos"]["key"] + +class SignerWrapper(object): + """ + This class handles the common operations that come with signing a file in S3. + """ + + def __init__(self, signer, key, bucket): + self.signer = signer + self.key = key + self.bucket = bucket + + def _get_sig_filepath(self, filepath, checksum): + raise NotImplementedError + + def _get_cmdline(self, filepath): + raise NotImplementedError + + def sign(self, url, checksum): tmpdir = tempfile.mkdtemp(prefix="/tmp/robosignatory-") try: - for obj in objects: - robosignatory.work.process_coreos( - self.signer, key, self.bucket, tmpdir, obj) + self._sign_object(url, checksum, tmpdir) finally: shutil.rmtree(tmpdir) + + def _sign_object(self, url, checksum, tmpdir): + filepath = urlparse(url).path.lstrip("/") + local_filepath = os.path.join(tmpdir, os.path.basename(filepath)) + + log.info("Downloading %s", filepath) + self.bucket.download_file(filepath, local_filepath) + + log.info("Checking hash for %s", filepath) + if utils.get_hash(local_filepath) != checksum: + log.error("Incorrect SHA256 for %s, not signing", filepath) + return + + log.info("Signing %s", filepath) + sig_filepath = self._get_sig_filepath(local_filepath) + cmdline = self._get_cmdline(local_filepath, checksum) + log.info('Signing command line: %s', cmdline) + ret, stdout, stderr = utils.run_command(cmdline) + if ret != 0: + log.error('Error signing! Signing output: %s, stdout: %s, ' + 'stderr: %s', ret, stdout, stderr) + return + if not os.path.exists(sig_filepath): + log.error("Signer did not produce any signature file for %s", filepath) + return + log.debug('Fixing signature file permissions') + # Sigul writes it as 0600, which makes a lot of sense as a general file + # mode for it, but this is just a signature file that we want published + os.chmod(sig_filepath, + (stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)) + + log.info("Uploading signature for %s", filepath) + uploaded_sig_filepath = self._get_sig_filepath(filepath) + self.bucket.upload_file(sig_filepath, uploaded_sig_filepath) + # Check the uploaded file + uploaded = list(self.bucket.objects.filter(Prefix=uploaded_sig_filepath)) + if len(uploaded) != 1: + log.warning("The signature for %s was not uploaded properly", filepath) + elif uploaded[0].size != os.stat(sig_filepath).st_size: + log.warning( + "The uploaded signature for %s does not have the right size", + filepath + ) + + os.remove(local_filepath) + os.remove(sig_filepath) + + +class ArtifactSignerWrapper(SignerWrapper): + + def _get_sig_filepath(self, filepath): + return filepath + ".sig" + + def _get_cmdline(self, filepath, checksum): + return self.signer.build_coreos_cmdline( + self.key, filepath, self._get_sig_filepath(filepath)) + + +class OSTreeSignerWrapper(SignerWrapper): + + SIG_NAME = "ostree-commitmeta-object" + + def _get_sig_filepath(self, filepath): + return "/".join([os.path.dirname(filepath), self.SIG_NAME]) + + def _get_cmdline(self, filepath, checksum): + return self.signer.build_atomic_cmdline( + self.key, checksum, filepath, self._get_sig_filepath(filepath)) diff --git a/robosignatory/utils.py b/robosignatory/utils.py index c18f736..b5b558d 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -1,9 +1,12 @@ import abc +import logging +from hashlib import sha256 + import pkg_resources import subprocess import koji -import logging + log = logging.getLogger('robosignatory.utils') @@ -47,6 +50,17 @@ def run_command(command): return ret, stdout, stderr +def get_hash(filepath): + hasher = sha256() + with open(filepath, "rb") as f: + while True: + content = f.read(1024) + if not content: + break + hasher.update(content) + return hasher.hexdigest() + + def get_signing_helper(backend, *args, **kwargs): """ Instantiate and return the appropriate signing backend. """ points = pkg_resources.iter_entry_points('robosignatory.signing.helpers') diff --git a/robosignatory/work.py b/robosignatory/work.py index 29b8aa2..da3f2be 100644 --- a/robosignatory/work.py +++ b/robosignatory/work.py @@ -1,8 +1,6 @@ import stat import os -from hashlib import sha256 -from six.moves.urllib.parse import urlparse import robosignatory.utils as utils import logging @@ -61,58 +59,3 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): f.write(commitid + '\n') log.info('Done') - - -def process_coreos(signer, key, bucket, tmpdir, artifact): - filepath = urlparse(artifact["file"]).path.lstrip("/") - local_filepath = os.path.join(tmpdir, os.path.basename(filepath)) - - log.info("Downloading %s", filepath) - bucket.download_file(filepath, local_filepath) - - log.info("Checking %s", filepath) - hasher = sha256() - with open(local_filepath, "rb") as f: - while True: - content = f.read(1024) - if not content: - break - hasher.update(content) - h = hasher.hexdigest() - if h != artifact["checksum"]: - log.error("Incorrect SHA256 for %s, not signing", filepath) - return - - log.info("Signing %s", filepath) - sig_filepath = local_filepath + ".sig" - cmdline = signer.build_coreos_cmdline( - key, local_filepath, sig_filepath) - log.info('Signing command line: %s', cmdline) - ret, stdout, stderr = utils.run_command(cmdline) - if ret != 0: - log.error('Error signing! Signing output: %s, stdout: %s, ' - 'stderr: %s', ret, stdout, stderr) - return - if not os.path.exists(sig_filepath): - log.error("Signer did not produce any signature file for %s", filepath) - return - log.debug('Fixing signature file permissions') - # Sigul writes it as 0600, which makes a lot of sense as a general file - # mode for it, but this is just a signature file that we want published - os.chmod(sig_filepath, - (stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)) - - log.info("Uploading signature for %s", filepath) - bucket.upload_file(sig_filepath, filepath + ".sig") - # Check the uploaded file - uploaded = list(bucket.objects.filter(Prefix=filepath + ".sig")) - if len(uploaded) != 1: - log.warning("The signature for %s was not uploaded properly", filepath) - elif uploaded[0].size != os.stat(sig_filepath).st_size: - log.warning( - "The uploaded signature for %s does not have the right size", - filepath - ) - - os.remove(local_filepath) - os.remove(sig_filepath) diff --git a/tests/test_coreos.py b/tests/test_coreos.py new file mode 100644 index 0000000..de9375a --- /dev/null +++ b/tests/test_coreos.py @@ -0,0 +1,136 @@ +import os +import unittest +import copy +from collections import namedtuple + +from fedora_messaging.api import Message +import mock + +from robosignatory.coreos import CoreOSSigner + + +TEST_CONFIG = { + "signing": { + "backend": "echo", + }, + "koji_instances": {}, + "ostree_refs": {}, + "coreos": { + "bucket": "testing", + "key": "testing", + "aws": { + "access_key": "testing", + "access_secret": "testing", + "region": "us-east-1", + } + }, +} + +ARTIFACTS_MESSAGE = Message( + topic="org.fedoraproject.prod.coreos.build.request.artifacts-sign", + body={ + "build_id": "buildid", + "stream": "stream", + "basearch": "basearch", + "artifacts": [{ + "file": "s3://host/some/path/test1", + "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }] + } +) + +OSTREE_MESSAGE = Message( + topic="org.fedoraproject.prod.coreos.build.request.ostree-sign", + body={ + "build_id": "buildid", + "stream": "stream", + "basearch": "basearch", + "commit_object": "s3://host/some/path/test1", + "checksum": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + } +) + +S3Object = namedtuple("S3Object", "size") + +def fake_download(source, local): + open(local, "w").close() + +def fake_download_and_artifact_sign(source, local): + fake_download(source, local) + # Also create the sig file for testing + open(local + ".sig", "w").close() + +def fake_download_and_ostree_sign(source, local): + fake_download(source, local) + # Also create the sig file for testing + open(os.path.dirname(local) + "/ostree-commitmeta-object", "w").close() + + +class TestCoreOS(unittest.TestCase): + + def setUp(self): + self.consumer = CoreOSSigner(TEST_CONFIG) + self.consumer.bucket = mock.Mock() + + @mock.patch('robosignatory.coreos.utils.run_command') + def test_artifacts_sign(self, run_command): + self.consumer.bucket.download_file.side_effect = fake_download_and_artifact_sign + run_command.return_value = 0, "", "" + self.consumer.bucket.objects.filter.return_value = [S3Object(size=0)] + + self.consumer.consume(ARTIFACTS_MESSAGE) + + self.consumer.bucket.download_file.assert_called() + assert self.consumer.bucket.download_file.call_args_list[0][0][0] == "some/path/test1" + run_command.assert_called() + self.consumer.bucket.upload_file.assert_called() + assert self.consumer.bucket.upload_file.call_args_list[0][0][1] == "some/path/test1.sig" + + @mock.patch('robosignatory.coreos.utils.run_command') + def test_ostree_sign(self, run_command): + self.consumer.bucket.download_file.side_effect = fake_download_and_ostree_sign + run_command.return_value = 0, "", "" + self.consumer.bucket.objects.filter.return_value = [S3Object(size=0)] + + self.consumer.consume(OSTREE_MESSAGE) + + self.consumer.bucket.download_file.assert_called() + assert self.consumer.bucket.download_file.call_args_list[0][0][0] == "some/path/test1" + run_command.assert_called() + self.consumer.bucket.upload_file.assert_called() + assert self.consumer.bucket.upload_file.call_args_list[0][0][1] == "some/path/ostree-commitmeta-object" + + @mock.patch('robosignatory.coreos.utils.run_command') + def test_wrong_checksum(self, run_command): + new_body = copy.deepcopy(ARTIFACTS_MESSAGE.body) + new_body["artifacts"][0]["checksum"] = "wrong-checksum" + msg = Message(topic=ARTIFACTS_MESSAGE.topic, body=new_body) + self.consumer.bucket.download_file.side_effect = fake_download + + self.consumer.consume(msg) + + self.consumer.bucket.download_file.assert_called() + run_command.assert_not_called() + self.consumer.bucket.upload_file.assert_not_called() + + @mock.patch('robosignatory.coreos.utils.run_command') + def test_signing_failed(self, run_command): + self.consumer.bucket.download_file.side_effect = fake_download + run_command.return_value = 1, "stdout", "stderr" + + self.consumer.consume(ARTIFACTS_MESSAGE) + + self.consumer.bucket.download_file.assert_called() + run_command.assert_called() + self.consumer.bucket.upload_file.assert_not_called() + + @mock.patch('robosignatory.coreos.utils.run_command') + def test_no_signature(self, run_command): + self.consumer.bucket.download_file.side_effect = fake_download + run_command.return_value = 0, "stdout", "stderr" + + self.consumer.consume(ARTIFACTS_MESSAGE) + + self.consumer.bucket.download_file.assert_called() + run_command.assert_called() + self.consumer.bucket.upload_file.assert_not_called()