Add gpgkey to ansible repository

fixes #1086

.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

CHANGES/1086.bugfix

@@ -0,0 +1 @@
+Add a `gpgkey` field to the ansible repository to ease verification of collection signatures.

CHANGES/1086.removal

@@ -0,0 +1 @@
+Removed ``keyring`` attribute from repositories in favor of ``gpgkey``.

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.
+   This defaults to the pulp ansible URL router.

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

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',
+        ),
+    ]

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"

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
 
 

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 = ""

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()

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,22 +89,32 @@ 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()
 
     # 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)
 

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

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