From d5b993cbfdfb699bac8117792a02ebf3aacc1202 Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Tue, 5 May 2026 23:41:43 -0400 Subject: [PATCH] Fix add-signing-service failing for keys with subkeys The previous implementation counted fpr: lines in GPG's colon output to verify that exactly one key matched the provided key ID. However, GPG emits a separate fpr: line for the primary key and each subkey, so any key with subkeys would be rejected with "There are N keys matching the key id." Count pub:/sec: lines instead, which represent actual distinct keys. Assisted-By: claude-opus-4.6 --- CHANGES/+fix-add-signing-service.bugfix | 1 + .../commands/add-signing-service.py | 15 +- pulpcore/pytest_plugin.py | 187 +++++++++++------- .../functional/api/test_signing_service.py | 23 +++ 4 files changed, 150 insertions(+), 76 deletions(-) create mode 100644 CHANGES/+fix-add-signing-service.bugfix diff --git a/CHANGES/+fix-add-signing-service.bugfix b/CHANGES/+fix-add-signing-service.bugfix new file mode 100644 index 00000000000..5e00ecf8b96 --- /dev/null +++ b/CHANGES/+fix-add-signing-service.bugfix @@ -0,0 +1 @@ +Fixed `add-signing-service` management command failing with "There are N keys matching the key id" for PGP keys that have subkeys. diff --git a/pulpcore/app/management/commands/add-signing-service.py b/pulpcore/app/management/commands/add-signing-service.py index 5c463da49c0..681ddbc1fd1 100644 --- a/pulpcore/app/management/commands/add-signing-service.py +++ b/pulpcore/app/management/commands/add-signing-service.py @@ -82,10 +82,17 @@ def handle(self, *args, **options): if result.returncode != 0: raise CommandError(result.stderr.strip()) - fpr_lines = [line for line in result.stdout.splitlines() if line.startswith("fpr:")] - if len(fpr_lines) != 1: - raise CommandError(_("There are {} keys matching the key id.").format(len(fpr_lines))) - fingerprint = fpr_lines[0].split(":")[9] + lines = result.stdout.splitlines() + + # Count actual keys (pub:/sec: lines), not fingerprint lines. GPG emits + # a separate fpr: line for the primary key and each subkey, so a single + # key with subkeys produces multiple fpr: lines. + key_lines = [l for l in lines if l.startswith(("pub:", "sec:"))] # noqa: E741 + if len(key_lines) != 1: + raise CommandError(_("There are {} keys matching the key id.").format(len(key_lines))) + + # Use the primary key fingerprint (first fpr: line in GPG's output). + fingerprint = [l.split(":")[9] for l in lines if l.startswith("fpr:")][0] # noqa: E741 result = subprocess.run( gpg_cmd + ["--armor", "--export", key_id], diff --git a/pulpcore/pytest_plugin.py b/pulpcore/pytest_plugin.py index dca6230adf6..37b97833606 100644 --- a/pulpcore/pytest_plugin.py +++ b/pulpcore/pytest_plugin.py @@ -1,7 +1,6 @@ import asyncio import json import os -import pathlib import shutil import socket import ssl @@ -1072,43 +1071,61 @@ def _dispatch_task_group(task_name, *args, **kwargs): # GPG related fixtures +PULP_FIXTURES_SIGNING_KEYS_URL = ( + "https://raw.githubusercontent.com/pulp/pulp-fixtures/master/common/signing_keys/" +) +KEY_V4_RSA2K_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-rsa2k.asc" +KEY_V4_RSA2K_PRIVATE = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-rsa2k.secret" +KEY_V4_RSA4K_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-rsa4k.asc" +KEY_V4_RSA4K_PRIVATE = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-rsa4k.secret" +KEY_V4_ED25519_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-ed25519.asc" +KEY_V4_ED25519_PRIVATE = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v4-ed25519.secret" +KEY_V6_RSA4K_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-rsa4k.asc" +KEY_V6_RSA4K_PRIVATE = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-rsa4k.secret" +KEY_V6_ED25519_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-ed25519.asc" +KEY_V6_ED25519_PRIVATE = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-ed25519.secret" +KEY_V6_MLDSA65_ED25519_PUBLIC = ( + PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-mldsa65-ed25519.asc" +) +KEY_V6_MLDSA65_ED25519_PRIVATE = ( + PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-mldsa65-ed25519.secret" +) +KEY_V6_MLDSA87_ED448_PUBLIC = PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-mldsa87-ed448.asc" +KEY_V6_MLDSA87_ED448_PRIVATE = ( + PULP_FIXTURES_SIGNING_KEYS_URL + "pulp-testkey-v6-mldsa87-ed448.secret" +) + -SIGNING_SCRIPT_STRING = r"""#!/usr/bin/env bash +SIGNING_SCRIPT_STRING = """#!/usr/bin/env bash FILE_PATH=$1 SIGNATURE_PATH="$1.asc" -GPG_KEY_ID="pulp-fixture-signing-key" +GPG_KEY_ID="{gpg_key_id}" # Create a detached signature -gpg --quiet --batch --homedir HOMEDIRHERE --detach-sign --local-user "${GPG_KEY_ID}" \ - --armor --output ${SIGNATURE_PATH} ${FILE_PATH} +gpg --quiet --batch --homedir {gpg_home} --detach-sign --local-user "${{GPG_KEY_ID}}" \\ + --armor --output ${{SIGNATURE_PATH}} ${{FILE_PATH}} # Check the exit status STATUS=$? -if [[ ${STATUS} -eq 0 ]]; then - echo {\"file\": \"${FILE_PATH}\", \"signature\": \"${SIGNATURE_PATH}\"} +if [[ ${{STATUS}} -eq 0 ]]; then + echo '{{"file": "'${{FILE_PATH}}'", "signature": "'${{SIGNATURE_PATH}}'"}}' else - exit ${STATUS} + exit ${{STATUS}} fi """ @pytest.fixture(scope="session") -def signing_script_path(signing_script_temp_dir, signing_gpg_homedir_path): - signing_script_file = signing_script_temp_dir / "sign-metadata.sh" - signing_script_file.write_text( - SIGNING_SCRIPT_STRING.replace("HOMEDIRHERE", str(signing_gpg_homedir_path)) - ) - - signing_script_file.chmod(0o755) - - return signing_script_file +def signing_script_path(signing_script_temp_dir, signing_gpg_homedir_path, signing_gpg_metadata): + _gpg, fingerprint, _keyid = signing_gpg_metadata + return make_signing_script(signing_gpg_homedir_path, fingerprint, signing_script_temp_dir) @pytest.fixture(scope="session") def signing_script_temp_dir(tmp_path_factory): - return tmp_path_factory.mktemp("sigining_script_dir") + return tmp_path_factory.mktemp("signing_script_dir") @pytest.fixture(scope="session") @@ -1149,30 +1166,7 @@ def _sign_with_ascii_armored_detached_signing_service(filename): def signing_gpg_metadata(signing_gpg_homedir_path): """A fixture that returns a GPG instance and related metadata (i.e., fingerprint, keyid).""" PRIVATE_KEY_URL = "https://raw.githubusercontent.com/pulp/pulp-fixtures/master/common/GPG-PRIVATE-KEY-fixture-signing" # noqa: E501 - - try: - import gnupg - except ImportError: - pytest.fail("python-gnupg is not installed, add to your functest_requirements.txt") - key_file = pathlib.Path(__file__).parent / "GPG-PRIVATE-KEY-fixture-signing" - if key_file.exists(): - private_key_data = key_file.read_text() - else: - response = requests.get(PRIVATE_KEY_URL) - response.raise_for_status() - private_key_data = response.text - with suppress(FileNotFoundError, PermissionError): - key_file.write_text(private_key_data) - - gpg = gnupg.GPG(gnupghome=signing_gpg_homedir_path) - gpg.import_keys(private_key_data) - - key = gpg.list_keys()[0] - fingerprint = key["fingerprint"] - keyid = key["keyid"] - - gpg.trust_keys(fingerprint, "TRUST_ULTIMATE") - return gpg, fingerprint, keyid + return import_signing_key(PRIVATE_KEY_URL, signing_gpg_homedir_path) @pytest.fixture(scope="session") @@ -1194,53 +1188,102 @@ def _ascii_armored_detached_signing_service_name( signing_gpg_metadata, signing_gpg_homedir_path, ): - service_name = str(uuid.uuid4()) _gpg, fingerprint, _keyid = signing_gpg_metadata + service_name = create_signing_service( + signing_gpg_homedir_path, fingerprint, signing_script_path + ) + + yield service_name + + remove_signing_service(service_name) + + +@pytest.fixture(scope="session") +def ascii_armored_detached_signing_service( + _ascii_armored_detached_signing_service_name, pulpcore_bindings +): + return pulpcore_bindings.SigningServicesApi.list( + name=_ascii_armored_detached_signing_service_name + ).results[0] + +def import_signing_key(key_url, gpg_home): + """Import a PGP key into a GPG home directory and trust it. + + Returns ``(gpg, fingerprint, keyid)``. + """ + try: + import gnupg + except ImportError: + pytest.skip("python-gnupg not installed") + + gpg = gnupg.GPG(gnupghome=gpg_home) + + response = requests.get(key_url) + response.raise_for_status() + result = gpg.import_keys(response.content) + assert result.count >= 1, f"Failed to import key from {key_url}" + + key_info = gpg.list_keys()[0] + fingerprint = key_info["fingerprint"] + keyid = key_info["keyid"] + gpg.trust_keys(fingerprint, "TRUST_ULTIMATE") + + return gpg, fingerprint, keyid + + +def make_signing_script(gpg_home, fingerprint, script_dir=None): + """Create a detached-signature signing script. + + Returns the script path. + """ + if script_dir is None: + script_dir = gpg_home + script_path = script_dir / "sign.sh" + script_path.write_text(SIGNING_SCRIPT_STRING.format(gpg_home=gpg_home, gpg_key_id=fingerprint)) + script_path.chmod(0o755) + return script_path + + +def create_signing_service( + gpg_home, fingerprint, script_path, *, service_class="core:AsciiArmoredDetachedSigningService" +): + """Register a signing service via pulpcore-manager. + + Returns the service name. + """ + service_name = str(uuid.uuid4()) cmd = ( "pulpcore-manager", "add-signing-service", service_name, - str(signing_script_path), + str(script_path), fingerprint, "--class", - "core:AsciiArmoredDetachedSigningService", + service_class, "--gnupghome", - str(signing_gpg_homedir_path), - ) - completed_process = subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + str(gpg_home), ) + completed = subprocess.run(cmd, capture_output=True, text=True) + assert completed.returncode == 0, completed.stderr - assert completed_process.returncode == 0 + return service_name - yield service_name - cmd = ( - "pulpcore-manager", - "remove-signing-service", - service_name, - "--class", - "core:AsciiArmoredDetachedSigningService", - ) +def remove_signing_service(service_name, service_class="core:AsciiArmoredDetachedSigningService"): + """Remove a signing service created by ``create_signing_service``.""" subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + ( + "pulpcore-manager", + "remove-signing-service", + service_name, + "--class", + service_class, + ), + capture_output=True, ) -@pytest.fixture(scope="session") -def ascii_armored_detached_signing_service( - _ascii_armored_detached_signing_service_name, pulpcore_bindings -): - return pulpcore_bindings.SigningServicesApi.list( - name=_ascii_armored_detached_signing_service_name - ).results[0] - - # if content_origin == None, base_url will return the relative path and # we need to add the hostname to run the tests @pytest.fixture(scope="session") diff --git a/pulpcore/tests/functional/api/test_signing_service.py b/pulpcore/tests/functional/api/test_signing_service.py index 25f4a477d40..78d875a6252 100644 --- a/pulpcore/tests/functional/api/test_signing_service.py +++ b/pulpcore/tests/functional/api/test_signing_service.py @@ -1,7 +1,30 @@ import pytest +from pulpcore.pytest_plugin import ( + KEY_V4_RSA4K_PRIVATE, + create_signing_service, + import_signing_key, + make_signing_script, + remove_signing_service, +) + @pytest.mark.parallel def test_crud_signing_service(ascii_armored_detached_signing_service): service = ascii_armored_detached_signing_service assert "/api/v3/signing-services/" in service.pulp_href + + +def test_add_signing_service_key_with_subkeys(tmp_path_factory): + """Verify that add-signing-service works with a PGP key that has subkeys. + + Keys with signing subkeys produce multiple fpr: lines in GPG's colon + output, which previously caused add-signing-service to fail. + """ + gpg_home = tmp_path_factory.mktemp("gpghome_subkey_test") + _gpg, fingerprint, _keyid = import_signing_key(KEY_V4_RSA4K_PRIVATE, gpg_home) + script_path = make_signing_script(gpg_home, fingerprint) + service_name = create_signing_service(gpg_home, fingerprint, script_path) + assert len(fingerprint) == 40 + + remove_signing_service(service_name)