Refactor MergeCallbacks

pgpainless-cert-d-cli/src/main/java/pgp/cert_d/cli/commands/Import.java

@@ -32,7 +32,7 @@ public class Import implements Runnable {
             for (PGPPublicKeyRing cert : certificates) {
                 ByteArrayInputStream certIn = new ByteArrayInputStream(cert.getEncoded());
                 Certificate certificate = PGPCertDCli.getCertificateDirectory()
-                        .insert(certIn, MergeCallbacks.mergeCertificates());
+                        .insert(certIn, MergeCallbacks.mergeWithExisting());
             }
         } catch (IOException e) {
             LOGGER.error("IO-Error.", e);

pgpainless-cert-d-cli/src/main/java/pgp/cert_d/cli/commands/Insert.java

@@ -24,7 +24,7 @@ public class Insert implements Runnable {
     public void run() {
         try {
             Certificate certificate = PGPCertDCli.getCertificateDirectory()
-                    .insert(System.in, MergeCallbacks.mergeCertificates());
+                    .insert(System.in, MergeCallbacks.mergeWithExisting());
         } catch (IOException e) {
             LOGGER.error("IO-Error.", e);
             System.exit(-1);

pgpainless-cert-d-cli/src/main/java/pgp/cert_d/cli/commands/Setup.java

@@ -61,7 +61,7 @@ public class Setup implements Runnable {
 
         try {
             InputStream inputStream = new ByteArrayInputStream(trustRoot.getEncoded());
-            PGPCertDCli.getCertificateDirectory().insertTrustRoot(inputStream, MergeCallbacks.overrideKey());
+            PGPCertDCli.getCertificateDirectory().insertTrustRoot(inputStream, MergeCallbacks.overrideExisting());
 
         } catch (BadDataException e) {
             throw new RuntimeException(e);

pgpainless-cert-d/src/main/java/org/pgpainless/certificate_store/MergeCallbacks.java

@@ -16,7 +16,6 @@ import pgp.certificate_store.certificate.KeyMaterialMerger;
 import pgp.certificate_store.exception.BadDataException;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Iterator;
 
 public class MergeCallbacks {
@@ -27,11 +26,13 @@ public class MergeCallbacks {
      *
      * @return merging callback
      */
-    public static KeyMaterialMerger mergeCertificates() {
+    public static KeyMaterialMerger mergeWithExisting() {
         return new KeyMaterialMerger() {
 
             @Override
-            public KeyMaterial merge(KeyMaterial data, KeyMaterial existing) throws IOException {
+            public KeyMaterial merge(KeyMaterial data, KeyMaterial existing)
+                    throws IOException {
+                // Simple cases: one is null -> return other
                 if (data == null) {
                     return existing;
                 }
@@ -46,51 +47,66 @@ public class MergeCallbacks {
                     PGPKeyRing mergedKeyRing;
 
                     if (existingKeyRing instanceof PGPPublicKeyRing) {
-                        PGPPublicKeyRing existingCert = (PGPPublicKeyRing) existingKeyRing;
+                        mergedKeyRing = mergeWithCert((PGPPublicKeyRing) existingKeyRing, updatedKeyRing);
-                        if (updatedKeyRing instanceof PGPPublicKeyRing) {
-                            mergedKeyRing = PGPPublicKeyRing.join(existingCert, (PGPPublicKeyRing) updatedKeyRing);
-                        } else if (updatedKeyRing instanceof PGPSecretKeyRing) {
-                            PGPPublicKeyRing updatedPublicKeys = PGPainless.extractCertificate((PGPSecretKeyRing) updatedKeyRing);
-                            PGPPublicKeyRing mergedPublicKeys = PGPPublicKeyRing.join(existingCert, updatedPublicKeys);
-                            updatedKeyRing = PGPSecretKeyRing.replacePublicKeys((PGPSecretKeyRing) updatedKeyRing, mergedPublicKeys);
-                            mergedKeyRing = updatedKeyRing;
-                        } else {
-                            throw new IOException(new BadDataException());
-                        }
                     } else if (existingKeyRing instanceof PGPSecretKeyRing) {
-                        PGPSecretKeyRing existingKey = (PGPSecretKeyRing) existingKeyRing;
+                        mergedKeyRing = mergeWithKey(existingKeyRing, updatedKeyRing);
-                        PGPPublicKeyRing existingCert = PGPainless.extractCertificate(existingKey);
-                        if (updatedKeyRing instanceof PGPPublicKeyRing) {
-                            PGPPublicKeyRing updatedCert = (PGPPublicKeyRing) updatedKeyRing;
-                            PGPPublicKeyRing mergedCert = PGPPublicKeyRing.join(existingCert, updatedCert);
-                            mergedKeyRing = PGPSecretKeyRing.replacePublicKeys(existingKey, mergedCert);
-                        } else if (updatedKeyRing instanceof PGPSecretKeyRing) {
-                            PGPSecretKeyRing updatedKey = (PGPSecretKeyRing) updatedKeyRing;
-                            if (!Arrays.equals(existingKey.getEncoded(), updatedKey.getEncoded())) {
-                                // Merging secret keys is not supported.
-                                return existing;
-                            }
-                            mergedKeyRing = existingKeyRing;
-                        } else {
-                            throw new IOException(new BadDataException());
-                        }
                     } else {
                         throw new IOException(new BadDataException());
                     }
 
                     printOutDifferences(existingKeyRing, mergedKeyRing);
 
-                    if (mergedKeyRing instanceof PGPPublicKeyRing) {
+                    return toKeyMaterial(mergedKeyRing);
-                        return CertificateFactory.certificateFromPublicKeyRing((PGPPublicKeyRing) mergedKeyRing, null);
-                    } else {
-                        return KeyFactory.keyFromSecretKeyRing((PGPSecretKeyRing) mergedKeyRing, null);
-                    }
 
                 } catch (PGPException e) {
                     throw new RuntimeException(e);
                 }
             }
 
+            private PGPKeyRing mergeWithCert(PGPPublicKeyRing existingKeyRing, PGPKeyRing updatedKeyRing)
+                    throws PGPException, IOException {
+                PGPKeyRing mergedKeyRing;
+                PGPPublicKeyRing existingCert = existingKeyRing;
+                if (updatedKeyRing instanceof PGPPublicKeyRing) {
+                    mergedKeyRing = PGPPublicKeyRing.join(existingCert, (PGPPublicKeyRing) updatedKeyRing);
+                } else if (updatedKeyRing instanceof PGPSecretKeyRing) {
+                    PGPPublicKeyRing updatedPublicKeys = PGPainless.extractCertificate((PGPSecretKeyRing) updatedKeyRing);
+                    PGPPublicKeyRing mergedPublicKeys = PGPPublicKeyRing.join(existingCert, updatedPublicKeys);
+                    updatedKeyRing = PGPSecretKeyRing.replacePublicKeys((PGPSecretKeyRing) updatedKeyRing, mergedPublicKeys);
+                    mergedKeyRing = updatedKeyRing;
+                } else {
+                    throw new IOException(new BadDataException());
+                }
+                return mergedKeyRing;
+            }
+
+            private PGPKeyRing mergeWithKey(PGPKeyRing existingKeyRing, PGPKeyRing updatedKeyRing)
+                    throws PGPException, IOException {
+                PGPKeyRing mergedKeyRing;
+                PGPSecretKeyRing existingKey = (PGPSecretKeyRing) existingKeyRing;
+                PGPPublicKeyRing existingCert = PGPainless.extractCertificate(existingKey);
+                if (updatedKeyRing instanceof PGPPublicKeyRing) {
+                    PGPPublicKeyRing updatedCert = (PGPPublicKeyRing) updatedKeyRing;
+                    PGPPublicKeyRing mergedCert = PGPPublicKeyRing.join(existingCert, updatedCert);
+                    mergedKeyRing = PGPSecretKeyRing.replacePublicKeys(existingKey, mergedCert);
+                } else if (updatedKeyRing instanceof PGPSecretKeyRing) {
+                    // Merging keys is not supported
+                    mergedKeyRing = existingKeyRing;
+                } else {
+                    throw new IOException(new BadDataException());
+                }
+                return mergedKeyRing;
+            }
+
+            private KeyMaterial toKeyMaterial(PGPKeyRing mergedKeyRing)
+                    throws IOException {
+                if (mergedKeyRing instanceof PGPPublicKeyRing) {
+                    return CertificateFactory.certificateFromPublicKeyRing((PGPPublicKeyRing) mergedKeyRing, null);
+                } else {
+                    return KeyFactory.keyFromSecretKeyRing((PGPSecretKeyRing) mergedKeyRing, null);
+                }
+            }
+
             private void printOutDifferences(PGPKeyRing existingCert, PGPKeyRing mergedCert) {
                 int numSigsBefore = countSigs(existingCert);
                 int numSigsAfter = countSigs(mergedCert);
@@ -145,23 +161,7 @@ public class MergeCallbacks {
         };
     }
 
-    /**
+    public static KeyMaterialMerger overrideExisting() {
-     * Return an implementation of {@link KeyMaterialMerger} that ignores the existing certificate and instead
-     * returns the first instance.
-     *
-     * @return overriding callback
-     */
-    public static KeyMaterialMerger overrideCertificate() {
-        // noinspection Convert2Lambda
-        return new KeyMaterialMerger() {
-            @Override
-            public KeyMaterial merge(KeyMaterial data, KeyMaterial existing) {
-                return data;
-            }
-        };
-    }
-
-    public static KeyMaterialMerger overrideKey() {
         // noinspection Convert2Lambda
         return new KeyMaterialMerger() {
             @Override

pgpainless-cert-d/src/test/java/org/pgpainless/certificate_store/KeyMaterialReaderTest.java

@@ -1,8 +1,27 @@
+// SPDX-FileCopyrightText: 2022 Paul Schaub <vanitasvitae@fsfe.org>
+//
+// SPDX-License-Identifier: Apache-2.0
+
 package org.pgpainless.certificate_store;
 
 import org.junit.jupiter.api.Test;
+import pgp.certificate_store.certificate.Certificate;
+import pgp.certificate_store.certificate.Key;
+import pgp.certificate_store.certificate.KeyMaterial;
+import pgp.certificate_store.exception.BadDataException;
 
-public class CertificateAndKeyFactoryTest {
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.Charset;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class KeyMaterialReaderTest {
 
     private static final String KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
             "Comment: B21A ABBF 15DF 0FDA 3742  4DE9 AD00 8384 AD0A 064C\n" +
@@ -39,8 +58,76 @@ public class CertificateAndKeyFactoryTest {
             "=3nhb\n" +
             "-----END PGP PRIVATE KEY BLOCK-----\n";
 
+    private static final String CERT = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" +
+            "Comment: B21A ABBF 15DF 0FDA 3742  4DE9 AD00 8384 AD0A 064C\n" +
+            "Comment: Volodymyr Zelenskyy <zelenskyy@gov.ua>\n" +
+            "\n" +
+            "xjMEYwdKchYJKwYBBAHaRw8BAQdARSvx9BDpV0AoNYTmN/wrZXQAB7VzOV0rKEQc\n" +
+            "OkhbP5zCwBEEHxYKAIMFgmMHSnIFiQWfpgADCwkHCRCtAIOErQoGTEcUAAAAAAAe\n" +
+            "ACBzYWx0QG5vdGF0aW9ucy5zZXF1b2lhLXBncC5vcmemokUGLbEtrRXgP2TiuWaJ\n" +
+            "uEiwxGVy8r8wrzceN+HYIgMVCggCmwECHgEWIQSyGqu/Fd8P2jdCTemtAIOErQoG\n" +
+            "TAAAjA0A/jW7LADP6ppGFaiPSy1iaZ5+iThYdeTfiN9qZ5Oik7UsAQDovr9ys9jM\n" +
+            "hwBmtprPSzkAz/Y68llHwtoFKP3a4ENsDM0mVm9sb2R5bXlyIFplbGVuc2t5eSA8\n" +
+            "emVsZW5za3l5QGdvdi51YT7CwBQEExYKAIYFgmMHSnIFiQWfpgADCwkHCRCtAIOE\n" +
+            "rQoGTEcUAAAAAAAeACBzYWx0QG5vdGF0aW9ucy5zZXF1b2lhLXBncC5vcme4P+qt\n" +
+            "J59gcsFRDvSpCFtwKXNeQxAKgY7hLdYnOJv8gAMVCggCmQECmwECHgEWIQSyGqu/\n" +
+            "Fd8P2jdCTemtAIOErQoGTAAAvwMA/jb7PWwMmW/rg9biKrY4Nhx1jU1Km2PN+JDa\n" +
+            "HKh00EWmAP4ySK/17ZsW+VGPZDkGvKnLdnAq8a2KRR7ez42Uow+WBM4zBGMHSnIW\n" +
+            "CSsGAQQB2kcPAQEHQE2MxvbzS9/zM0n9gC1xzqct7y4JJO3EoocXQaj2RT5WwsDF\n" +
+            "BBgWCgE3BYJjB0pyBYkFn6YACRCtAIOErQoGTEcUAAAAAAAeACBzYWx0QG5vdGF0\n" +
+            "aW9ucy5zZXF1b2lhLXBncC5vcmfXpaC1I/cFI28CPdT2BYi23CDGMPhtsPOa8OZy\n" +
+            "2l8xxAKbAr6gBBkWCgBvBYJjB0pyCRA1lqtsC5dFGkcUAAAAAAAeACBzYWx0QG5v\n" +
+            "dGF0aW9ucy5zZXF1b2lhLXBncC5vcmetthMAGVuAj99i9srAqYcOw2rMjznQaQ7I\n" +
+            "jL9iwe3c/BYhBGUJKGari+/r+zQaADWWq2wLl0UaAADrtwD9F+nRFWmyPOHe7eAM\n" +
+            "EaZhIkoUrpvQiYjg0rOfNJc5I5QA/0DhkiBOYjteeZD7ncIvLm4n/r+LV3uEaPvr\n" +
+            "SD/OW00PFiEEshqrvxXfD9o3Qk3prQCDhK0KBkwAAEnMAQCdgPL7itMBDEB4VSUd\n" +
+            "ZXI1GlJuxKAVbqZRfOXRYqV47gD+NG8Lx2o44PMBS6PKcAAEwXdoJzWF1wyuydls\n" +
+            "ImTwCgHOOARjB0pyEgorBgEEAZdVAQUBAQdAxtLnuIIi5BHnJv6s9cD0EASJeb9f\n" +
+            "P/TNEqzFJPzNemoDAQgHwsAGBBgWCgB4BYJjB0pyBYkFn6YACRCtAIOErQoGTEcU\n" +
+            "AAAAAAAeACBzYWx0QG5vdGF0aW9ucy5zZXF1b2lhLXBncC5vcmfZNJZA/uU2jCW1\n" +
+            "zQzCT9oK5hj0sL2taNvDFlLtkMCNqwKbDBYhBLIaq78V3w/aN0JN6a0Ag4StCgZM\n" +
+            "AAAlKwD/eUEbC8MoIitKulDZawtlC0rSITXtQJqUkGNcujTPgIAA/1p/Y3sHn4nh\n" +
+            "mYcVX902BRXBp8YMD/cHQWZkWPhvM9YF\n" +
+            "=o64m\n" +
+            "-----END PGP PUBLIC KEY BLOCK-----";
+
+    private final KeyMaterialReader reader = new KeyMaterialReader();
+    private final Charset UTF8 = Charset.forName("UTF8");
+
     @Test
-    public void test() {
+    public void readBadDataTest() {
-        
+        assertThrows(BadDataException.class, () -> reader.read(
+                new ByteArrayInputStream(CERT.substring(0, CERT.length() - 100).getBytes(UTF8)), null));
+    }
+
+    @Test
+    public void readIncompleteDataTest() {
+        assertThrows(BadDataException.class, () -> reader.read(
+                new ByteArrayInputStream("ThisIsNotOpenPGPDataAtAllLol".getBytes(UTF8)), null));
+    }
+
+    @Test
+    public void readKeyTest() throws BadDataException, IOException {
+        KeyMaterial keyMaterial = reader.read(new ByteArrayInputStream(KEY.getBytes(UTF8)), 12L);
+        assertNotNull(keyMaterial);
+        assertTrue(keyMaterial instanceof Key);
+        Key key = (Key) keyMaterial;
+        assertEquals("b21aabbf15df0fda37424de9ad008384ad0a064c", key.getFingerprint());
+        assertEquals(12L, key.getTag());
+        Certificate certificate = key.getCertificate();
+        assertEquals(key.getFingerprint(), certificate.getFingerprint());
+        assertEquals(key.getTag(), certificate.getTag());
+        assertEquals(key.getSubkeyIds(), certificate.getSubkeyIds());
+    }
+
+    @Test
+    public void readCertTest() throws BadDataException, IOException {
+        KeyMaterial keyMaterial = reader.read(new ByteArrayInputStream(CERT.getBytes(UTF8)), null);
+        assertNotNull(keyMaterial);
+        assertTrue(keyMaterial instanceof Certificate);
+        Certificate certificate = (Certificate) keyMaterial;
+        assertEquals("b21aabbf15df0fda37424de9ad008384ad0a064c", certificate.getFingerprint());
+        assertNull(certificate.getTag());
+        assertSame(certificate, certificate.asCertificate());
     }
 }