From 3cb44f2848f09b89a63e614fc4a454f9dc5de7a6 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Fri, 1 Jul 2022 09:54:48 +0200 Subject: [PATCH] Add `gpgkey` to ansible repository fixes #1086 --- .github/workflows/scripts/install.sh | 2 +- .../workflows/scripts/post_before_script.sh | 12 ----- CHANGES/1086.bugfix | 1 + CHANGES/1086.removal | 1 + docs/workflows/signature.rst | 12 ++--- .../0042_ansiblerepository_gpgkey.py | 51 +++++++++++++++++++ pulp_ansible/app/models.py | 2 +- pulp_ansible/app/serializers.py | 21 +++----- pulp_ansible/app/settings.py | 2 +- pulp_ansible/app/tasks/signature.py | 45 ++++++++++------ .../api/collection/test_signatures.py | 28 ++++++---- requirements.txt | 2 +- template_config.yml | 1 + 13 files changed, 120 insertions(+), 60 deletions(-) delete mode 100755 .github/workflows/scripts/post_before_script.sh create mode 100644 CHANGES/1086.bugfix create mode 100644 CHANGES/1086.removal create mode 100644 pulp_ansible/app/migrations/0042_ansiblerepository_gpgkey.py diff --git a/.github/workflows/scripts/install.sh b/.github/workflows/scripts/install.sh index d72b984bb..d7bcf60bb 100755 --- a/.github/workflows/scripts/install.sh +++ b/.github/workflows/scripts/install.sh @@ -84,7 +84,7 @@ services: VARSYAML cat >> vars/main.yaml << VARSYAML -pulp_settings: {"allowed_export_paths": "/tmp", "allowed_import_paths": "/tmp", "ansible_api_hostname": "https://pulp:443", "ansible_content_hostname": "https://pulp:443/pulp/content"} +pulp_settings: {"allowed_export_paths": "/tmp", "allowed_import_paths": "/tmp", "ansible_api_hostname": "https://pulp:443", "ansible_content_hostname": "https://pulp:443/pulp/content", "ansible_signature_require_verification": false} pulp_scheme: https pulp_container_tag: https diff --git a/.github/workflows/scripts/post_before_script.sh b/.github/workflows/scripts/post_before_script.sh deleted file mode 100755 index ecfb777ec..000000000 --- a/.github/workflows/scripts/post_before_script.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env sh - -set -euv - -# Add the QE (public) key to the container env for `test_sign_locally_then_upload_signature`. -# Please, remove once the verification was made independent of the system keyring. - -cmd_prefix bash -c "curl -L https://github.com/pulp/pulp-fixtures/raw/master/common/GPG-KEY-pulp-qe | gpg --import" - -KEY_FINGERPRINT="6EDF301256480B9B801EBA3D05A5E6DA269D9D98" -TRUST_LEVEL="6" -echo "$KEY_FINGERPRINT:$TRUST_LEVEL:" | cmd_stdin_prefix gpg --import-ownertrust diff --git a/CHANGES/1086.bugfix b/CHANGES/1086.bugfix new file mode 100644 index 000000000..442badb68 --- /dev/null +++ b/CHANGES/1086.bugfix @@ -0,0 +1 @@ +Add a `gpgkey` field to the ansible repository to ease verification of collection signatures. diff --git a/CHANGES/1086.removal b/CHANGES/1086.removal new file mode 100644 index 000000000..6eaec5ccc --- /dev/null +++ b/CHANGES/1086.removal @@ -0,0 +1 @@ +Removed ``keyring`` attribute from repositories in favor of ``gpgkey``. diff --git a/docs/workflows/signature.rst b/docs/workflows/signature.rst index 27eeb7c37..6cb4f236f 100644 --- a/docs/workflows/signature.rst +++ b/docs/workflows/signature.rst @@ -7,14 +7,14 @@ how to add signature support. Setup ----- -In order to verify signature validity on uploads you will need to store your trusted keyring on -your Pulp system and set the ``pulp_ansible`` setting ``ANSIBLE_CERTS_DIR`` to the folder of that -keyring. By default it is set to ``/etc/pulp/certs/`` if you want an easy place to store your -keyring. +In order to verify signature validity on uploads you will need to store your trusted key on the +repositories ``gpgkey`` attribute. .. note:: - You can upload signatures without supplying Pulp your keyring, but ``pulp_ansible`` will not - perform validity checks on the uploaded signature. + You can upload signatures without supplying Pulp any key, but ``pulp_ansible`` will not + perform validity checks on the uploaded signature. You will also have to configure the + ``ANSIBLE_SIGNATURE_REQUIRE_VERIFICATION`` setting to ``False``. By default and once a key is + provided, all signatures impossible to verify are rejected. In order to have ``pulp_ansible`` sign collections stored in your repositories you will need to set up a signing service. First, create/import the key you intend to sign your collections with onto diff --git a/pulp_ansible/app/migrations/0042_ansiblerepository_gpgkey.py b/pulp_ansible/app/migrations/0042_ansiblerepository_gpgkey.py new file mode 100644 index 000000000..42791036d --- /dev/null +++ b/pulp_ansible/app/migrations/0042_ansiblerepository_gpgkey.py @@ -0,0 +1,51 @@ +# Generated by Django 3.2.13 on 2022-07-01 10:19 + +import gnupg +import tempfile + +from django.db import migrations, models + + +def migrate_keyring_to_gpgkey_on_repository_up(apps, schema_editor): + AnsibleRepository = apps.get_model('ansible', 'AnsibleRepository') + changed_repos = [] + with tempfile.TemporaryDirectory() as gnupghome: + for repository in AnsibleRepository.objects.filter(keyring__isnull=False): + try: + gpg = gnupg.GPG(gnupghome=gnupghome, keyring=repository.keyring) + repository.gpgkey = gpg.export_keys("*") + changed_repos.append(repository) + except Exception as e: + # Make the migration resilient + print(f"Repository {repository.pk} failed to migrate it's keyring. {e}") + pass + + if len(changed_repos) >= 1024: + AnsibleRepository.objects.bulk_update(changed_repos, ['gpgkey']) + changed_repos.clear() + # Flush the rest + AnsibleRepository.objects.bulk_update(changed_repos, ['gpgkey']) + + +class Migration(migrations.Migration): + + dependencies = [ + ('ansible', '0041_alter_collectionversion_collection'), + ] + + operations = [ + migrations.AddField( + model_name='ansiblerepository', + name='gpgkey', + field=models.TextField(null=True), + ), + migrations.RunPython( + code=migrate_keyring_to_gpgkey_on_repository_up, + reverse_code=migrations.RunPython.noop, + elidable=True, + ), + migrations.RemoveField( + model_name='ansiblerepository', + name='keyring', + ), + ] diff --git a/pulp_ansible/app/models.py b/pulp_ansible/app/models.py index 3df98b90f..d4d7a10b6 100644 --- a/pulp_ansible/app/models.py +++ b/pulp_ansible/app/models.py @@ -308,7 +308,7 @@ class AnsibleRepository(Repository): REMOTE_TYPES = [RoleRemote, CollectionRemote] last_synced_metadata_time = models.DateTimeField(null=True) - keyring = models.FilePathField(path="/etc/pulp/certs/", recursive=True, blank=True) + gpgkey = models.TextField(null=True) class Meta: default_related_name = "%(app_label)s_%(model_name)s" diff --git a/pulp_ansible/app/serializers.py b/pulp_ansible/app/serializers.py index c6ae74bc1..026b7a369 100644 --- a/pulp_ansible/app/serializers.py +++ b/pulp_ansible/app/serializers.py @@ -1,7 +1,6 @@ from gettext import gettext as _ from django.conf import settings -from drf_spectacular.utils import extend_schema_field from jsonschema import Draft7Validator from rest_framework import serializers @@ -116,13 +115,6 @@ class Meta: ) -@extend_schema_field(str) -class FilePathField(serializers.FilePathField): - """ - Remove enum model in client generation, see https://github.com/pulp/pulp_ansible/issues/973. - """ - - class AnsibleRepositorySerializer(RepositorySerializer): """ Serializer for Ansible Repositories. @@ -131,16 +123,17 @@ class AnsibleRepositorySerializer(RepositorySerializer): last_synced_metadata_time = serializers.DateTimeField( help_text=_("Last synced metadata time."), allow_null=True, required=False ) - keyring = FilePathField( - path=settings.ANSIBLE_CERTS_DIR, - help_text=_("Location of keyring used to verify signatures uploaded to this repository"), - allow_blank=True, - default="", + gpgkey = serializers.CharField( + help_text="Gpg public key to verify collection signatures against", required=False, + allow_null=True, ) class Meta: - fields = RepositorySerializer.Meta.fields + ("last_synced_metadata_time", "keyring") + fields = RepositorySerializer.Meta.fields + ( + "last_synced_metadata_time", + "gpgkey", + ) model = AnsibleRepository diff --git a/pulp_ansible/app/settings.py b/pulp_ansible/app/settings.py index 116821342..6a6d80ea9 100644 --- a/pulp_ansible/app/settings.py +++ b/pulp_ansible/app/settings.py @@ -22,7 +22,7 @@ ANSIBLE_API_HOSTNAME = "https://" + socket.getfqdn() ANSIBLE_CONTENT_HOSTNAME = settings.CONTENT_ORIGIN + "/pulp/content" +ANSIBLE_SIGNATURE_REQUIRE_VERIFICATION = True ANSIBLE_SIGNING_TASK_LIMITER = 10 -ANSIBLE_CERTS_DIR = "/etc/pulp/certs/" ANSIBLE_DEFAULT_DISTRIBUTION_PATH = None ANSIBLE_URL_NAMESPACE = "" diff --git a/pulp_ansible/app/tasks/signature.py b/pulp_ansible/app/tasks/signature.py index 3c5721081..1de0b189e 100644 --- a/pulp_ansible/app/tasks/signature.py +++ b/pulp_ansible/app/tasks/signature.py @@ -1,8 +1,9 @@ import aiofiles import asyncio -import gnupg import hashlib +import logging import tarfile +import tempfile from gettext import gettext as _ from pulpcore.plugin.stages import ( @@ -22,34 +23,48 @@ from django.core.files.storage import default_storage as storage from pulpcore.plugin.models import SigningService, ProgressReport from pulpcore.plugin.sync import sync_to_async_iterable, sync_to_async +from pulpcore.plugin.util import gpg_verify +from pulpcore.plugin.exceptions import InvalidSignatureError from pulp_ansible.app.tasks.utils import get_file_obj_from_tarball from rest_framework import serializers +log = logging.getLogger(__name__) + + def verify_signature_upload(data): """The task code for verifying collection signature upload.""" file = data["file"] sig_data = file.read() file.seek(0) collection = data["signed_collection"] - keyring = None - if (repository := data.get("repository")) and repository.keyring: - keyring = repository.keyring - gpg = gnupg.GPG(keyring=keyring) + repository = data.get("repository") + gpgkey = repository and repository.gpgkey or "" + artifact = collection.contentartifact_set.select_related("artifact").first().artifact.file.name artifact_file = storage.open(artifact) with tarfile.open(fileobj=artifact_file, mode="r") as tar: manifest = get_file_obj_from_tarball(tar, "MANIFEST.json", artifact_file) - with open("MANIFEST.json", mode="wb") as m: - m.write(manifest.read()) - verified = gpg.verify_file(file, m.name) - - if verified.trust_level is None or verified.trust_level < verified.TRUST_FULLY: - # Skip verification if repository isn't specified, or it doesn't have a keyring attached - if verified.fingerprint is None or keyring is not None: - raise serializers.ValidationError( - _("Signature verification failed: {}").format(verified.status) - ) + with tempfile.NamedTemporaryFile(dir=".") as manifest_file: + manifest_file.write(manifest.read()) + manifest_file.flush() + try: + verified = gpg_verify(gpgkey, file, manifest_file.name) + except InvalidSignatureError as e: + if gpgkey: + raise serializers.ValidationError( + _("Signature verification failed: {}").format(e.verified.status) + ) + elif settings.ANSIBLE_SIGNATURE_REQUIRE_VERIFICATION: + raise serializers.ValidationError( + _("Signature verification failed: No key available.") + ) + else: + # We have no key configured. So we simply accept the signature as is + verified = e.verified + log.warn( + "Collection Signature was accepted without verification. No key available." + ) data["data"] = sig_data data["digest"] = file.hashers["sha256"].hexdigest() diff --git a/pulp_ansible/tests/functional/api/collection/test_signatures.py b/pulp_ansible/tests/functional/api/collection/test_signatures.py index 406da7859..5e9a14d5b 100644 --- a/pulp_ansible/tests/functional/api/collection/test_signatures.py +++ b/pulp_ansible/tests/functional/api/collection/test_signatures.py @@ -74,12 +74,12 @@ def test_sign_locally_then_upload_signature( tmp_path, ansible_collections_api_client, ansible_collection_signatures_client, + ansible_repo_factory, + pulp_trusted_public_key_fingerprint, + pulp_trusted_public_key, ): """Test uploading a locally produced Collection Signature.""" - # NOTE: This test relies on the server global gpg keyring containing the Pulp QE key. - # This is carried out by the post_before_script.sh. Please remove that script (or parts of it) - # once this functionality has been rewritten to use keys read from the database. - + repository = ansible_repo_factory(gpgkey=pulp_trusted_public_key) collection, collection_url = build_and_upload_collection() # Extract MANIFEST.json @@ -89,14 +89,22 @@ def test_sign_locally_then_upload_signature( # Locally sign the collection signing_script_response = sign_with_ascii_armored_detached_signing_service(filename) - signature = signing_script_response["signature"] + signature_file = signing_script_response["signature"] # Signature upload task = ansible_collection_signatures_client.create( - signed_collection=collection_url, file=signature + signed_collection=collection_url, file=signature_file, repository=repository.pulp_href + ) + signature_href = next( + ( + item + for item in monitor_task(task.task).created_resources + if "content/ansible/collection_signatures/" in item + ) ) - sig = monitor_task(task.task).created_resources[0] - assert "content/ansible/collection_signatures/" in sig + assert signature_href is not None + signature = ansible_collection_signatures_client.read(signature_href) + assert signature.pubkey_fingerprint == pulp_trusted_public_key_fingerprint # Upload another collection that won't be signed another_collection, another_collection_url = build_and_upload_collection() @@ -104,7 +112,9 @@ def test_sign_locally_then_upload_signature( # Check that invalid signatures can't be uploaded with pytest.raises(Exception): task = ansible_collection_signatures_client.create( - signed_collection=another_collection_url, file=signature + signed_collection=another_collection_url, + file=signature, + repository=repository.pulp_href, ) monitor_task(task.task) diff --git a/requirements.txt b/requirements.txt index 7178126aa..e4cf37d25 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,6 @@ galaxy_importer>=0.4.5,<0.5 GitPython>=3.1.24,<3.2 jsonschema>=4.4,<4.7 packaging>=21.3,<22 -pulpcore>=3.20,<3.25 +pulpcore>=3.21.dev,<3.25 PyYAML<6.0 semantic_version>=2.9,<2.11 diff --git a/template_config.yml b/template_config.yml index dcfa7bb58..d5954bc2a 100644 --- a/template_config.yml +++ b/template_config.yml @@ -55,6 +55,7 @@ pulp_settings: allowed_import_paths: /tmp ansible_api_hostname: https://pulp:443 ansible_content_hostname: https://pulp:443/pulp/content + ansible_signature_require_verification: false pulpcore_branch: main pulpcore_pip_version_specifier: null pulpcore_revision: null