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/settings.rst b/docs/settings.rst index 0cf143ae8..9825fa419 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -26,6 +26,15 @@ ANSIBLE_CONTENT_HOSTNAME CONTENT_ORIGIN, this would default to "https://example.com/pulp/content". +ANSIBLE_SIGNATURE_REQUIRE_VERIFICATION +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + By default, ``pulp_ansible`` rejects uploaded signatures if they cannot be verified against a + public key specified on the repository. Setting this to false will allow accepting signatures + if no key was specified. A repository with a configured key will always reject invalid + signatures. + + ANSIBLE_SIGNING_TASK_LIMITER ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -59,4 +68,4 @@ ANSIBLE_URL_NAMESPACE The Django URL namespace to be used when generating URLs that are returned by the galaxy APIs. Setting this allows for the galaxy APIs to redirect requests to django URLs in other apps. - This defaults to the pulp ansible URL router. \ No newline at end of file + This defaults to the pulp ansible URL router. 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