From 37ad62d6e57f62b640bd2d538a1bc3b6b1942651 Mon Sep 17 00:00:00 2001
From: Levente Polyak <anthraxx@archlinux.org>
Date: Wed, 8 Dec 2021 20:50:45 +0100
Subject: [PATCH] feature(keyringctl): add clean functionality to match
 import-clean

De-duplicate not needed certifications by cleaning the keyring after
import to remove old files when processing revocations. This basically
adds the functionality compared to import-clean.
---
 libkeyringctl/keyring.py | 30 ++++++++++++++++++++++++++++++
 libkeyringctl/verify.py  |  5 +++++
 tests/test_keyring.py    | 16 ++++++++++++++--
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/libkeyringctl/keyring.py b/libkeyringctl/keyring.py
index b7060b70..36908778 100644
--- a/libkeyringctl/keyring.py
+++ b/libkeyringctl/keyring.py
@@ -286,6 +286,26 @@ def convert_signature_packet(
         raise Exception(f'unknown signature root for "{packet.name}"')
 
 
+def clean_keyring(keyring: Path) -> None:
+    """Clean the keyring by f.e. removing old obsolete certifications with matching revocations.
+
+    Parameters
+    ----------
+    keyring: Root directory of the keyring containing all keys to clean.
+    """
+    for cert in get_cert_paths(paths=[keyring]):
+        for uid in (cert / "uid").iterdir():
+            certifications = uid / "certification"
+            revocations = uid / "revocation"
+            if not certifications.exists() or not revocations.exists():
+                continue
+            for revocation in revocations.iterdir():
+                certification = certifications / revocation.name
+                if certification.exists():
+                    debug(f"Cleaning up old certification {certification} for revocation {revocation}")
+                    certification.unlink()
+
+
 def convert_certificate(
     working_dir: Path,
     certificate: Path,
@@ -503,6 +523,7 @@ def persist_key_material(
 
     persist_uid_certifications(
         certifications=certifications,
+        revocations=revocations,
         key_dir=key_dir,
     )
 
@@ -688,6 +709,7 @@ def persist_direct_key_revocations(
 
 def persist_uid_certifications(
     certifications: Dict[Uid, Dict[Fingerprint, List[Path]]],
+    revocations: Dict[Uid, Dict[Fingerprint, List[Path]]],
     key_dir: Path,
 ) -> None:
     """Persist the certifications of a root key to file(s)
@@ -696,14 +718,20 @@ def persist_uid_certifications(
     PositiveCertifications for all User IDs of the given root key.
     All certifications are persisted in per User ID certification directories below key_dir.
 
+    Certifications that have a matching revocation are skipped to match behavior of import-clean.
+
     Parameters
     ----------
     certifications: The certifications to write to file
+    revocations: The revocations to check against if certifications need to be persisted.
     key_dir: The root directory below which certifications are persisted
     """
 
     for uid, uid_certifications in certifications.items():
         for issuer, issuer_certifications in uid_certifications.items():
+            # skip certifications if there is a revocation present to match import-clean behavior
+            if uid in revocations and issuer in revocations[uid] and revocations[uid][issuer]:
+                continue
             certification_dir = key_dir / "uid" / simplify_uid(uid) / "certification"
             certification_dir.mkdir(parents=True, exist_ok=True)
             certification = latest_certification(issuer_certifications)
@@ -826,6 +854,8 @@ def convert(
         (target_dir / user_dir.name).mkdir(parents=True, exist_ok=True)
         copytree(src=user_dir, dst=(target_dir / user_dir.name), dirs_exist_ok=True)
 
+    clean_keyring(keyring=target_dir)
+
     return target_dir
 
 
diff --git a/libkeyringctl/verify.py b/libkeyringctl/verify.py
index a765e02f..8673e6fa 100644
--- a/libkeyringctl/verify.py
+++ b/libkeyringctl/verify.py
@@ -159,6 +159,11 @@ def verify_integrity(certificate: Path, all_fingerprints: Set[Fingerprint]) -> N
                                 )
                                 if issuer != sig.stem:
                                     raise Exception(f"Unexpected issuer in file {str(sig)}: {issuer}")
+
+                                certification = uid_path.parent / "certification" / sig.name
+                                if certification.exists():
+                                    raise Exception(f"Certification exists for revocation {str(sig)}: {certification}")
+
                                 debug(f"OK: {sig}")
                         else:
                             raise Exception(f"Unexpected directory in certificate {certificate.name}: {str(uid_path)}")
diff --git a/tests/test_keyring.py b/tests/test_keyring.py
index a48b6d25..752f04d7 100644
--- a/tests/test_keyring.py
+++ b/tests/test_keyring.py
@@ -25,6 +25,7 @@ from libkeyringctl.types import Username
 
 from .conftest import create_certificate
 from .conftest import create_key_revocation
+from .conftest import create_signature_revocation
 from .conftest import create_uid_certification
 from .conftest import test_all_fingerprints
 from .conftest import test_certificates
@@ -563,7 +564,7 @@ def test_convert(working_dir: Path, keyring_dir: Path) -> None:
         working_dir=working_dir,
         keyring_root=keyring_dir,
         sources=test_certificates[Username("foobar")],
-        target_dir=keyring_dir,
+        target_dir=keyring_dir / "packager",
     )
 
     with raises(Exception):
@@ -571,10 +572,21 @@ def test_convert(working_dir: Path, keyring_dir: Path) -> None:
             working_dir=working_dir,
             keyring_root=keyring_dir,
             sources=test_keys[Username("foobar")],
-            target_dir=keyring_dir,
+            target_dir=keyring_dir / "packager",
         )
 
 
+@create_certificate(username=Username("main"), uids=[Uid("main <foo@bar.xyz>")], keyring_type="main")
+@create_certificate(username=Username("foobar"), uids=[Uid("foobar <foo@bar.xyz>")])
+@create_uid_certification(issuer=Username("main"), certified=Username("foobar"), uid=Uid("foobar <foo@bar.xyz>"))
+@create_signature_revocation(issuer=Username("main"), certified=Username("foobar"), uid=Uid("foobar <foo@bar.xyz>"))
+def test_clean_keyring(working_dir: Path, keyring_dir: Path) -> None:
+    # first pass clean up certification
+    keyring.clean_keyring(keyring=keyring_dir)
+    # second pass skipping clean up because lack of certification
+    keyring.clean_keyring(keyring=keyring_dir)
+
+
 @create_certificate(username=Username("main"), uids=[Uid("main <foo@bar.xyz>")], keyring_type="main")
 @create_certificate(username=Username("other_main"), uids=[Uid("other main <foo@bar.xyz>")], keyring_type="main")
 @create_certificate(username=Username("foobar"), uids=[Uid("foobar <foo@bar.xyz>")])
-- 
GitLab