Simplify libkeyringctl.keyring.convert_certificate

libkeyringctl/keyring.py:
Simplify `convert_certificate()` by splitting out the conversion of
signature packets to `convert_signature_packet()` and the persistence of
packet material to `persist_key_material()`.
Add `convert_pubkey_signature_packet()`,
`convert_uid_signature_packet()` and
`convert_subkey_signature_packet()` to deal with the conversion of
public key signatures, UID signatures and subkey signatures
(respectively).

tests/test_keyring.py:
Add tests for `convert_certificate()`, `convert_signature_packet()`,
`convert_{pubkey,uid,subkey}_signature_packet()` and
`persist_subkey_revocations()`.

libkeyringctl/keyring.py

@@ -88,8 +88,184 @@ def transform_fingerprint_to_keyring_path(keyring_root: Path, paths: List[Path])
         paths[index] = fingerprint_paths[0]
 
 
-# TODO: simplify to lower complexity
+def convert_pubkey_signature_packet(
-def convert_certificate(  # noqa: ignore=C901
+    packet: Path,
+    certificate_fingerprint: Fingerprint,
+    fingerprint_filter: Optional[Set[Fingerprint]],
+    current_packet_fingerprint: Optional[Fingerprint],
+    direct_revocations: Dict[Fingerprint, List[Path]],
+    direct_sigs: Dict[Fingerprint, List[Path]],
+) -> None:
+    """Convert a public key signature packet
+
+    packet: The Path of the packet file to process
+    certificate_fingerprint: The public key certificate fingerprint
+    fingerprint_filter: Optional list of fingerprints of PGP public keys that all certifications will be filtered with
+    current_packet_fingerprint: Optional certificate fingerprint of the current packet
+    direct_revocations: A dictionary of direct key revocations
+    direct_sigs: A dictionary of direct key signatures
+    """
+
+    if not current_packet_fingerprint:
+        raise Exception('missing current packet fingerprint for "{packet.name}"')
+
+    signature_type = packet_dump_field(packet=packet, field="Type")
+    issuer = get_fingerprint_from_partial(fingerprint_filter or set(), Fingerprint(packet_dump_field(packet, "Issuer")))
+
+    if not issuer:
+        debug(f"failed to resolve partial fingerprint {issuer}, skipping packet")
+    else:
+        if signature_type == "KeyRevocation" and certificate_fingerprint.endswith(issuer):
+            direct_revocations[issuer].append(packet)
+        elif signature_type in ["DirectKey", "GenericCertification"]:
+            direct_sigs[issuer].append(packet)
+        else:
+            raise Exception(f"unknown signature type: {signature_type}")
+
+
+def convert_uid_signature_packet(
+    packet: Path,
+    current_packet_uid: Optional[Uid],
+    fingerprint_filter: Optional[Set[Fingerprint]],
+    certifications: Dict[Uid, Dict[Fingerprint, List[Path]]],
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]],
+) -> None:
+    """Convert a UID signature packet
+
+    packet: The Path of the packet file to process
+    current_packet_uid: Optional Uid of the current packet
+    fingerprint_filter: Optional list of fingerprints of PGP public keys that all certifications will be filtered with
+    certifications: A dictionary containing all certificaions
+    revocations: A dictionary containing all revocations
+    """
+
+    if not current_packet_uid:
+        raise Exception('missing current packet uid for "{packet.name}"')
+
+    signature_type = packet_dump_field(packet=packet, field="Type")
+    issuer = get_fingerprint_from_partial(fingerprint_filter or set(), Fingerprint(packet_dump_field(packet, "Issuer")))
+
+    if not issuer:
+        debug(f"failed to resolve partial fingerprint {issuer}, skipping packet")
+    else:
+        if signature_type == "CertificationRevocation":
+            revocations[current_packet_uid][issuer].append(packet)
+        elif signature_type.endswith("Certification"):
+            # TODO: extend fp filter to all certifications
+            # TODO: use contains_fingerprint
+            if fingerprint_filter is None or any([fp.endswith(issuer) for fp in fingerprint_filter]):
+                debug(f"The certification by issuer {issuer} is appended as it is found in the filter.")
+                certifications[current_packet_uid][issuer].append(packet)
+            else:
+                debug(f"The certification by issuer {issuer} is not appended because it is not in the filter")
+        else:
+            raise Exception(f"unknown signature type: {signature_type}")
+
+
+def convert_subkey_signature_packet(
+    packet: Path,
+    certificate_fingerprint: Fingerprint,
+    current_packet_fingerprint: Optional[Fingerprint],
+    fingerprint_filter: Optional[Set[Fingerprint]],
+    subkey_bindings: Dict[Fingerprint, List[Path]],
+    subkey_revocations: Dict[Fingerprint, List[Path]],
+) -> None:
+    """Convert a subkey signature packet
+
+    packet: The Path of the packet file to process
+    certificate_fingerprint: The public key certificate fingerprint
+    current_packet_fingerprint: Optional certificate fingerprint of the current packet
+    fingerprint_filter: Optional list of fingerprints of PGP public keys that all certifications will be filtered with
+    subkey_bindings: A dictionary containing all subkey binding signatures
+    subkey_revocations: A dictionary containing all subkey revocations
+    """
+
+    if not current_packet_fingerprint:
+        raise Exception('missing current packet fingerprint for "{packet.name}"')
+
+    signature_type = packet_dump_field(packet=packet, field="Type")
+    issuer = get_fingerprint_from_partial(fingerprint_filter or set(), Fingerprint(packet_dump_field(packet, "Issuer")))
+
+    if not issuer:
+        debug(f"failed to resolve partial fingerprint {issuer}, skipping packet")
+    else:
+        if issuer != certificate_fingerprint:
+            raise Exception(f"subkey packet does not belong to {certificate_fingerprint}, issuer: {issuer}")
+
+        if signature_type == "SubkeyBinding":
+            subkey_bindings[current_packet_fingerprint].append(packet)
+        elif signature_type == "SubkeyRevocation":
+            subkey_revocations[current_packet_fingerprint].append(packet)
+        else:
+            raise Exception(f"unknown signature type: {signature_type}")
+
+
+def convert_signature_packet(
+    packet: Path,
+    current_packet_mode: Optional[str],
+    certificate_fingerprint: Optional[Fingerprint],
+    fingerprint_filter: Optional[Set[Fingerprint]],
+    current_packet_fingerprint: Optional[Fingerprint],
+    current_packet_uid: Optional[Uid],
+    direct_revocations: Dict[Fingerprint, List[Path]],
+    direct_sigs: Dict[Fingerprint, List[Path]],
+    certifications: Dict[Uid, Dict[Fingerprint, List[Path]]],
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]],
+    subkey_bindings: Dict[Fingerprint, List[Path]],
+    subkey_revocations: Dict[Fingerprint, List[Path]],
+) -> None:
+    """Convert a signature packet
+
+    packet: The Path of the packet file to process
+    certificate_fingerprint: The public key certificate fingerprint
+    current_packet_fingerprint: Optional certificate fingerprint of the current packet
+    current_packet_uid: Optional Uid of the current packet
+    direct_revocations: A dictionary of direct key revocations
+    direct_sigs: A dictionary of direct key signatures
+    fingerprint_filter: Optional list of fingerprints of PGP public keys that all certifications will be filtered with
+    certifications: A dictionary containing all certificaions
+    revocations: A dictionary containing all revocations
+    subkey_bindings: A dictionary containing all subkey binding signatures
+    subkey_revocations: A dictionary containing all subkey revocations
+    """
+
+    if not certificate_fingerprint:
+        raise Exception('missing certificate fingerprint for "{packet.name}"')
+
+    if current_packet_mode == "pubkey":
+        convert_pubkey_signature_packet(
+            packet=packet,
+            certificate_fingerprint=certificate_fingerprint,
+            fingerprint_filter=fingerprint_filter,
+            current_packet_fingerprint=current_packet_fingerprint,
+            direct_revocations=direct_revocations,
+            direct_sigs=direct_sigs,
+        )
+    elif current_packet_mode == "uid":
+        convert_uid_signature_packet(
+            packet=packet,
+            current_packet_uid=current_packet_uid,
+            fingerprint_filter=fingerprint_filter,
+            certifications=certifications,
+            revocations=revocations,
+        )
+    elif current_packet_mode == "subkey":
+        convert_subkey_signature_packet(
+            packet=packet,
+            current_packet_fingerprint=current_packet_fingerprint,
+            certificate_fingerprint=certificate_fingerprint,
+            fingerprint_filter=fingerprint_filter,
+            subkey_bindings=subkey_bindings,
+            subkey_revocations=subkey_revocations,
+        )
+    elif current_packet_mode == "uattr":
+        # ignore user attributes and related signatures
+        debug("skipping user attribute signature packet")
+    else:
+        raise Exception(f'unknown signature root for "{packet.name}"')
+
+
+def convert_certificate(
     working_dir: Path,
     certificate: Path,
     keyring_dir: Path,
@@ -183,75 +359,25 @@ def convert_certificate(  # noqa: ignore=C901
             )
             raise Exception("Secret key detected, aborting")
         elif packet.name.endswith("--Signature"):
-            # ignore user attributes and related signatures
+            convert_signature_packet(
-            if current_packet_mode == "uattr":
+                packet=packet,
-                debug("skipping user attribute signature packet")
+                current_packet_mode=current_packet_mode,
-                continue
+                certificate_fingerprint=certificate_fingerprint,
-
+                fingerprint_filter=fingerprint_filter,
-            if not certificate_fingerprint:
+                current_packet_fingerprint=current_packet_fingerprint,
-                raise Exception('missing certificate fingerprint for "{packet.name}"')
+                current_packet_uid=current_packet_uid,
-
+                direct_revocations=direct_revocations,
-            issuer = get_fingerprint_from_partial(
+                direct_sigs=direct_sigs,
-                fingerprint_filter or set(), Fingerprint(packet_dump_field(packet, "Issuer"))
+                certifications=certifications,
+                revocations=revocations,
+                subkey_bindings=subkey_bindings,
+                subkey_revocations=subkey_revocations,
             )
-            if not issuer:
-                debug(f"failed to resolve partial fingerprint {issuer}, skipping packet")
-                continue
-
-            signature_type = packet_dump_field(packet, "Type")
-
-            if current_packet_mode == "pubkey":
-                if not current_packet_fingerprint:
-                    raise Exception('missing current packet fingerprint for "{packet.name}"')
-
-                if signature_type == "KeyRevocation" and certificate_fingerprint.endswith(issuer):
-                    direct_revocations[issuer].append(packet)
-                elif signature_type in ["DirectKey", "GenericCertification"]:
-                    direct_sigs[issuer].append(packet)
-                else:
-                    raise Exception(f"unknown signature type: {signature_type}")
-            elif current_packet_mode == "uid":
-                if not current_packet_uid:
-                    raise Exception('missing current packet uid for "{packet.name}"')
-
-                if signature_type == "CertificationRevocation":
-                    revocations[current_packet_uid][issuer].append(packet)
-                elif signature_type.endswith("Certification"):
-                    # TODO: extend fp filter to all certifications
-                    # TODO: use contains_fingerprint
-                    if fingerprint_filter is None or any([fp.endswith(issuer) for fp in fingerprint_filter]):
-                        debug(f"The certification by issuer {issuer} is appended as it is found in the filter.")
-                        certifications[current_packet_uid][issuer].append(packet)
-                    else:
-                        debug(f"The certification by issuer {issuer} is not appended because it is not in the filter")
-                else:
-                    raise Exception(f"unknown signature type: {signature_type}")
-            elif current_packet_mode == "subkey":
-                if not current_packet_fingerprint:
-                    raise Exception('missing current packet fingerprint for "{packet.name}"')
-
-                issuer = get_fingerprint_from_partial(
-                    fingerprint_filter or set(), Fingerprint(packet_dump_field(packet, "Issuer"))
-                )
-                if issuer != certificate_fingerprint:
-                    raise Exception(f"subkey packet does not belong to {certificate_fingerprint}, issuer: {issuer}")
-
-                if signature_type == "SubkeyBinding":
-                    subkey_bindings[current_packet_fingerprint].append(packet)
-                elif signature_type == "SubkeyRevocation":
-                    subkey_revocations[current_packet_fingerprint].append(packet)
-                else:
-                    raise Exception(f"unknown signature type: {signature_type}")
-            else:
-                raise Exception(f'unknown signature root for "{packet.name}"')
         else:
             raise Exception(f'unknown packet type "{packet.name}"')
 
-    if not certificate_fingerprint:
+    if not certificate_fingerprint or not pubkey:
-        raise Exception("missing certificate fingerprint")
+        raise Exception("missing certificate public key")
-
-    if not pubkey:
-        raise Exception("missing certificate public-key")
 
     name_override = (
         name_override
@@ -263,6 +389,51 @@ def convert_certificate(  # noqa: ignore=C901
     key_dir = user_dir / certificate_fingerprint
     key_dir.mkdir(parents=True, exist_ok=True)
 
+    persist_key_material(
+        key_dir=key_dir,
+        direct_sigs=direct_sigs,
+        direct_revocations=direct_revocations,
+        certificate_fingerprint=certificate_fingerprint,
+        pubkey=pubkey,
+        subkeys=subkeys,
+        subkey_bindings=subkey_bindings,
+        subkey_revocations=subkey_revocations,
+        uids=uids,
+        certifications=certifications,
+        revocations=revocations,
+    )
+
+    return key_dir
+
+
+def persist_key_material(
+    key_dir: Path,
+    direct_sigs: Dict[Fingerprint, List[Path]],
+    direct_revocations: Dict[Fingerprint, List[Path]],
+    certificate_fingerprint: Fingerprint,
+    pubkey: Path,
+    subkeys: Dict[Fingerprint, Path],
+    subkey_bindings: Dict[Fingerprint, List[Path]],
+    subkey_revocations: Dict[Fingerprint, List[Path]],
+    uids: Dict[Uid, Path],
+    certifications: Dict[Uid, Dict[Fingerprint, List[Path]]],
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]],
+) -> None:
+    """Persist the key material found in a certificate to decomposed directory structure
+
+    key_dir: The Path below which to create a decomposed directory structure for the certificate
+    direct_sigs: A dictionary of direct key signatures
+    direct_revocations: A dictionary of direct key revocations
+    certificate_fingerprint: The public key certificate fingerprint
+    pubkey: The Path of the PGP packet representing the public key material
+    subkeys: A dictionary of Paths per Fingerprint that represent the subkey material of the certificate
+    subkey_bindings: A dictionary containing all subkey binding signatures
+    subkey_revocations: A dictionary containing all subkey revocations
+    uids: A dictionary of Path per Uid, that represent the UID packets of the certificate
+    certifications: A dictionary containing all certificaions
+    revocations: A dictionary containing all revocations
+    """
+
     persist_public_key(
         certificate_fingerprint=certificate_fingerprint,
         pubkey=pubkey,
@@ -311,8 +482,6 @@ def convert_certificate(  # noqa: ignore=C901
         key_dir=key_dir,
     )
 
-    return key_dir
-
 
 def persist_public_key(
     certificate_fingerprint: Fingerprint,

tests/test_keyring.py

@@ -1,7 +1,14 @@
+from collections import defaultdict
 from contextlib import nullcontext as does_not_raise
 from copy import deepcopy
 from pathlib import Path
+from random import choice
+from string import digits
 from typing import ContextManager
+from typing import Dict
+from typing import List
+from typing import Optional
+from typing import Set
 from unittest.mock import Mock
 from unittest.mock import patch
 
@@ -117,6 +124,375 @@ def test_transform_fingerprint_to_keyring_path(
             assert path == keyring_subdir / input_paths[index]
 
 
+@mark.parametrize(
+    "valid_current_packet_fingerprint, packet_type, issuer, expectation",
+    [
+        (True, "KeyRevocation", "self", does_not_raise()),
+        (True, "DirectKey", "self", does_not_raise()),
+        (True, "GenericCertification", "self", does_not_raise()),
+        (True, "KeyRevocation", None, does_not_raise()),
+        (True, "DirectKey", None, does_not_raise()),
+        (True, "GenericCertification", None, does_not_raise()),
+        (True, "KeyRevocation", "foo", raises(Exception)),
+        (True, "DirectKey", "foo", does_not_raise()),
+        (True, "GenericCertification", "foo", does_not_raise()),
+        (True, "foo", "foo", raises(Exception)),
+        (False, "KeyRevocation", True, raises(Exception)),
+        (False, "DirectKey", True, raises(Exception)),
+        (False, "GenericCertification", True, raises(Exception)),
+    ],
+)
+@patch("libkeyringctl.keyring.get_fingerprint_from_partial")
+@patch("libkeyringctl.keyring.packet_dump_field")
+def test_convert_pubkey_signature_packet(
+    packet_dump_field_mock: Mock,
+    get_fingerprint_from_partial_mock: Mock,
+    valid_current_packet_fingerprint: bool,
+    packet_type: str,
+    issuer: Optional[str],
+    expectation: ContextManager[str],
+    working_dir: Path,
+    valid_fingerprint: Fingerprint,
+) -> None:
+    packet = working_dir / "packet"
+    direct_revocations: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    direct_sigs: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    current_packet_fingerprint = None
+
+    if valid_current_packet_fingerprint:
+        current_packet_fingerprint = valid_fingerprint
+
+    packet_dump_field_mock.return_value = packet_type
+    if issuer == "self":
+        get_fingerprint_from_partial_mock.return_value = valid_fingerprint
+    else:
+        get_fingerprint_from_partial_mock.return_value = None if issuer is None else Fingerprint(issuer)
+
+    with expectation:
+        keyring.convert_pubkey_signature_packet(
+            packet=packet,
+            certificate_fingerprint=valid_fingerprint,
+            fingerprint_filter=None,
+            current_packet_fingerprint=current_packet_fingerprint,
+            direct_revocations=direct_revocations,
+            direct_sigs=direct_sigs,
+        )
+
+        if issuer is None or current_packet_fingerprint is None:
+            assert not direct_revocations and not direct_sigs
+        else:
+            if packet_type == "KeyRevocation":
+                if issuer == "self":
+                    assert direct_revocations[valid_fingerprint] == [packet]
+                else:
+                    assert not direct_revocations
+            elif packet_type in ["DirectKey", "GenericCertification"]:
+                assert direct_sigs[valid_fingerprint if issuer == "self" else Fingerprint(issuer)] == [packet]
+
+
+@mark.parametrize(
+    "valid_current_packet_uid, packet_type, provide_issuer, issuer_in_filter, expectation",
+    [
+        (True, "CertificationRevocation", "self", True, does_not_raise()),
+        (True, "CertificationRevocation", "self", False, does_not_raise()),
+        (True, "SomeCertification", "self", True, does_not_raise()),
+        (True, "SomeCertification", "self", False, does_not_raise()),
+        (True, "CertificationRevocation", None, True, does_not_raise()),
+        (True, "CertificationRevocation", None, False, does_not_raise()),
+        (True, "SomeCertification", None, True, does_not_raise()),
+        (True, "SomeCertification", None, False, does_not_raise()),
+        (False, "CertificationRevocation", "self", True, raises(Exception)),
+        (False, "CertificationRevocation", "self", False, raises(Exception)),
+        (False, "SomeCertification", "self", True, raises(Exception)),
+        (False, "SomeCertification", "self", False, raises(Exception)),
+        (True, "foo", "self", True, raises(Exception)),
+        (True, "foo", "self", False, raises(Exception)),
+    ],
+)
+@patch("libkeyringctl.keyring.get_fingerprint_from_partial")
+@patch("libkeyringctl.keyring.packet_dump_field")
+def test_convert_uid_signature_packet(
+    packet_dump_field_mock: Mock,
+    get_fingerprint_from_partial_mock: Mock,
+    valid_current_packet_uid: bool,
+    packet_type: str,
+    provide_issuer: Optional[str],
+    issuer_in_filter: bool,
+    expectation: ContextManager[str],
+    working_dir: Path,
+    valid_fingerprint: Fingerprint,
+) -> None:
+    packet = working_dir / "packet"
+    certifications: Dict[Uid, Dict[Fingerprint, List[Path]]] = defaultdict(lambda: defaultdict(list))
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]] = defaultdict(lambda: defaultdict(list))
+    current_packet_uid = None
+    issuer = None
+    fingerprint_filter: Set[Fingerprint] = set([Fingerprint("foo")])
+
+    if valid_current_packet_uid:
+        current_packet_uid = Uid("Foobar McFooface <foo@barmcfoofa.ce>")
+
+    packet_dump_field_mock.return_value = packet_type
+    if provide_issuer == "self":
+        issuer = valid_fingerprint
+    else:
+        if provide_issuer is not None:
+            issuer = Fingerprint(provide_issuer)
+
+    get_fingerprint_from_partial_mock.return_value = issuer
+
+    if issuer_in_filter and issuer is not None:
+        fingerprint_filter.add(issuer)
+
+    with expectation:
+        keyring.convert_uid_signature_packet(
+            packet=packet,
+            current_packet_uid=current_packet_uid,
+            fingerprint_filter=fingerprint_filter,
+            certifications=certifications,
+            revocations=revocations,
+        )
+
+        if not valid_current_packet_uid or issuer is None:
+            assert not certifications and not revocations
+        else:
+            if packet_type == "CertificationRevocation" and valid_current_packet_uid:
+                assert revocations[current_packet_uid][issuer] == [packet]  # type: ignore
+            elif packet_type.endswith("Certification") and issuer_in_filter:
+                assert certifications[current_packet_uid][issuer] == [packet]  # type: ignore
+            elif packet_type.endswith("Certification") and not issuer_in_filter:
+                assert not certifications
+
+
+@mark.parametrize(
+    "valid_current_packet_fingerprint, packet_type, issuer, expectation",
+    [
+        (True, "SubkeyBinding", "self", does_not_raise()),
+        (True, "SubkeyRevocation", "self", does_not_raise()),
+        (True, "SubkeyBinding", None, does_not_raise()),
+        (True, "SubkeyRevocation", None, does_not_raise()),
+        (True, "SubkeyBinding", "foo", raises(Exception)),
+        (True, "SubkeyRevocation", "foo", raises(Exception)),
+        (False, "SubkeyBinding", "self", raises(Exception)),
+        (False, "SubkeyRevocation", "self", raises(Exception)),
+        (True, "foo", "self", raises(Exception)),
+    ],
+)
+@patch("libkeyringctl.keyring.get_fingerprint_from_partial")
+@patch("libkeyringctl.keyring.packet_dump_field")
+def test_convert_subkey_signature_packet(
+    packet_dump_field_mock: Mock,
+    get_fingerprint_from_partial_mock: Mock,
+    valid_current_packet_fingerprint: bool,
+    packet_type: str,
+    issuer: Optional[str],
+    expectation: ContextManager[str],
+    working_dir: Path,
+    valid_fingerprint: Fingerprint,
+) -> None:
+    packet = working_dir / "packet"
+    subkey_bindings: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    subkey_revocations: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    current_packet_fingerprint = None
+
+    if valid_current_packet_fingerprint:
+        current_packet_fingerprint = valid_fingerprint
+
+    packet_dump_field_mock.return_value = packet_type
+    if issuer == "self":
+        get_fingerprint_from_partial_mock.return_value = valid_fingerprint
+    else:
+        get_fingerprint_from_partial_mock.return_value = None if issuer is None else Fingerprint(issuer)
+
+    with expectation:
+        keyring.convert_subkey_signature_packet(
+            packet=packet,
+            certificate_fingerprint=valid_fingerprint,
+            current_packet_fingerprint=current_packet_fingerprint,
+            fingerprint_filter=None,
+            subkey_bindings=subkey_bindings,
+            subkey_revocations=subkey_revocations,
+        )
+
+        if issuer is None or not valid_current_packet_fingerprint:
+            assert not subkey_bindings and not subkey_revocations
+        else:
+            if packet_type == "SubkeyBinding" and issuer == "self":
+                assert subkey_bindings[valid_fingerprint] == [packet]
+            elif packet_type == "SubkeyRevocation" and issuer == "self":
+                assert subkey_revocations[valid_fingerprint] == [packet]
+
+
+@mark.parametrize(
+    "valid_certificate_fingerprint, current_packet_mode, expectation",
+    [
+        (True, "pubkey", does_not_raise()),
+        (True, "uid", does_not_raise()),
+        (True, "subkey", does_not_raise()),
+        (True, "uattr", does_not_raise()),
+        (False, "pubkey", raises(Exception)),
+        (False, "uid", raises(Exception)),
+        (False, "subkey", raises(Exception)),
+        (False, "uattr", raises(Exception)),
+        (True, "foo", raises(Exception)),
+    ],
+)
+@patch("libkeyringctl.keyring.convert_pubkey_signature_packet")
+@patch("libkeyringctl.keyring.convert_uid_signature_packet")
+@patch("libkeyringctl.keyring.convert_subkey_signature_packet")
+def test_convert_signature_packet(
+    convert_subkey_signature_packet_mock: Mock,
+    convert_uid_signature_packet_mock: Mock,
+    convert_pubkey_signature_packet_mock: Mock,
+    valid_certificate_fingerprint: bool,
+    current_packet_mode: str,
+    expectation: ContextManager[str],
+    valid_fingerprint: Fingerprint,
+) -> None:
+    certificate_fingerprint = None
+    direct_revocations: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    direct_sigs: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    certifications: Dict[Uid, Dict[Fingerprint, List[Path]]] = defaultdict(lambda: defaultdict(list))
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]] = defaultdict(lambda: defaultdict(list))
+    subkey_bindings: Dict[Fingerprint, List[Path]] = defaultdict(list)
+    subkey_revocations: Dict[Fingerprint, List[Path]] = defaultdict(list)
+
+    if valid_certificate_fingerprint:
+        certificate_fingerprint = valid_fingerprint
+
+    with expectation:
+        keyring.convert_signature_packet(
+            packet=Path("foo"),
+            current_packet_mode=current_packet_mode,
+            certificate_fingerprint=certificate_fingerprint,
+            fingerprint_filter=None,
+            current_packet_fingerprint=None,
+            current_packet_uid=None,
+            direct_revocations=direct_revocations,
+            direct_sigs=direct_sigs,
+            certifications=certifications,
+            revocations=revocations,
+            subkey_bindings=subkey_bindings,
+            subkey_revocations=subkey_revocations,
+        )
+
+        if current_packet_mode == "pubkey":
+            convert_pubkey_signature_packet_mock.assert_called_once()
+        elif current_packet_mode == "uid":
+            convert_uid_signature_packet_mock.assert_called_once()
+        elif current_packet_mode == "subkey":
+            convert_subkey_signature_packet_mock.assert_called_once()
+
+
+@mark.parametrize(
+    "packet, packet_split, packet_dump_field, name_override, expectation",
+    [
+        (
+            Path("foo.asc"),
+            [
+                Path("--PublicKey"),
+                Path("--UserID"),
+                Path("--UserAttribute"),
+                Path("--PublicSubkey"),
+                Path("--Signature"),
+            ],
+            [
+                "".join(choice("ABCDEF" + digits) for x in range(40)),
+                "foo <foo@bar.com>",
+                "".join(choice("ABCDEF" + digits) for x in range(40)),
+            ],
+            "bar",
+            does_not_raise(),
+        ),
+        (
+            Path("foo.asc"),
+            [
+                Path("--SecretKey"),
+            ],
+            [],
+            None,
+            raises(Exception),
+        ),
+        (
+            Path("foo.asc"),
+            [
+                Path("foo"),
+            ],
+            [],
+            None,
+            raises(Exception),
+        ),
+        (
+            Path("foo.asc"),
+            [
+                Path("--PublicKey"),
+            ],
+            [
+                None,
+            ],
+            "bar",
+            raises(Exception),
+        ),
+    ],
+)
+@patch("libkeyringctl.keyring.persist_key_material")
+@patch("libkeyringctl.keyring.packet_split")
+@patch("libkeyringctl.keyring.convert_signature_packet")
+@patch("libkeyringctl.keyring.packet_dump_field")
+@patch("libkeyringctl.keyring.derive_username_from_fingerprint")
+def test_convert_certificate(
+    derive_username_from_fingerprint_mock: Mock,
+    packet_dump_field_mock: Mock,
+    convert_signature_packet_mock: Mock,
+    packet_split_mock: Mock,
+    persist_key_material_mock: Mock,
+    packet: Path,
+    packet_split: List[Path],
+    packet_dump_field: List[str],
+    name_override: Optional[Username],
+    expectation: ContextManager[str],
+    working_dir: Path,
+    keyring_dir: Path,
+) -> None:
+    packet_split_mock.return_value = packet_split
+    packet_dump_field_mock.side_effect = packet_dump_field
+
+    with expectation:
+        keyring.convert_certificate(
+            working_dir=working_dir,
+            certificate=packet,
+            keyring_dir=keyring_dir,
+            name_override=name_override,
+            fingerprint_filter=None,
+        )
+
+
+@patch("libkeyringctl.keyring.latest_certification")
+@patch("libkeyringctl.keyring.packet_join")
+def test_persist_subkey_revocations(
+    packet_join_mock: Mock,
+    latest_certification_mock: Mock,
+    working_dir: Path,
+    keyring_dir: Path,
+    valid_fingerprint: Fingerprint,
+) -> None:
+    revocation_packet = working_dir / "latest_revocation.asc"
+    latest_certification_mock.return_value = revocation_packet
+    subkey_revocations: Dict[Fingerprint, List[Path]] = {
+        valid_fingerprint: [revocation_packet, working_dir / "earlier_revocation.asc"]
+    }
+    keyring.persist_subkey_revocations(
+        key_dir=keyring_dir,
+        subkey_revocations=subkey_revocations,
+        issuer=valid_fingerprint,
+    )
+    packet_join_mock.assert_called_once_with(
+        packets=[revocation_packet],
+        output=keyring_dir / "subkey" / valid_fingerprint / "revocation" / f"{valid_fingerprint}.asc",
+        force=True,
+    )
+
+
 @create_certificate(username=Username("foobar"), uids=[Uid("foobar <foo@bar.xyz>")])
 def test_convert(working_dir: Path, keyring_dir: Path) -> None:
     keyring.convert(