Verify sub key signatures

src/main/java/org/pgpainless/pgpainless/key/selection/key/impl/SignedByMasterKey.java

@@ -15,6 +15,8 @@
  */
 package org.pgpainless.pgpainless.key.selection.key.impl;
 
+import java.io.IOException;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -22,25 +24,32 @@ import java.util.logging.Logger;
 import org.bouncycastle.openpgp.PGPException;
 import org.bouncycastle.openpgp.PGPPublicKey;
 import org.bouncycastle.openpgp.PGPSignature;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentVerifierBuilderProvider;
 import org.pgpainless.pgpainless.key.selection.key.PublicKeySelectionStrategy;
 
 public class SignedByMasterKey {
 
     private static final Logger LOGGER = Logger.getLogger(SignedByMasterKey.class.getName());
 
-    public static class PubkeySelectionStrategy extends PublicKeySelectionStrategy<Long> {
+    public static class PubkeySelectionStrategy extends PublicKeySelectionStrategy<PGPPublicKey> {
 
         @Override
-        public boolean accept(Long identifier, PGPPublicKey key) {
+        public boolean accept(PGPPublicKey masterKey, PGPPublicKey key) {
-            Iterator<PGPSignature> signatures = key.getSignaturesForKeyID(identifier);
+            // Same key -> accept
+            if (Arrays.equals(masterKey.getFingerprint(), key.getFingerprint())) {
+                return true;
+            }
+
+            Iterator<PGPSignature> signatures = key.getSignaturesForKeyID(masterKey.getKeyID());
             while (signatures.hasNext()) {
                 PGPSignature signature = signatures.next();
                 if (signature.getSignatureType() == PGPSignature.SUBKEY_BINDING) {
                     try {
-                        return signature.verify();
+                        signature.init(new BcPGPContentVerifierBuilderProvider(), masterKey);
+                        return signature.verifyCertification(masterKey, key);
                     } catch (PGPException e) {
                         LOGGER.log(Level.WARNING, "Could not verify subkey signature of key " +
-                                Long.toHexString(signature.getKeyID()) + " on key " + Long.toHexString(key.getKeyID()));
+                                Long.toHexString(masterKey.getKeyID()) + " on key " + Long.toHexString(key.getKeyID()));
 
                         return false;
                     }

src/main/java/org/pgpainless/pgpainless/util/BCUtil.java

@@ -107,7 +107,8 @@ public class BCUtil {
 
     public static PGPPublicKeyRing getKeyRingFromCollection(PGPPublicKeyRingCollection collection, Long id)
             throws PGPException {
-        return removeUnassociatedKeysFromKeyRing(collection.getPublicKeyRing(id), id);
+        PGPPublicKey key = collection.getPublicKey(id);
+        return removeUnassociatedKeysFromKeyRing(collection.getPublicKeyRing(id), key);
     }
 
     public static InputStream getPgpDecoderInputStream(byte[] bytes) throws IOException {
@@ -135,13 +136,15 @@ public class BCUtil {
      * (identified by {@code masterKeyId}), or are revoked ("normal" key revocation, as well as subkey revocation).
      *
      * @param ring key ring
-     * @param masterKeyId id of the master key
+     * @param masterKey master key
      * @return "cleaned" key ring
      */
-    public static PGPPublicKeyRing removeUnassociatedKeysFromKeyRing(PGPPublicKeyRing ring, Long masterKeyId) {
+    public static PGPPublicKeyRing removeUnassociatedKeysFromKeyRing(PGPPublicKeyRing ring, PGPPublicKey masterKey) {
-
+        if (!masterKey.isMasterKey()) {
+            throw new IllegalArgumentException("Given key is not a master key.");
+        }
         // Only select keys which are signed by the master key and not revoked.
-        PublicKeySelectionStrategy<Long> selector = new And.PubKeySelectionStrategy<>(
+        PublicKeySelectionStrategy<PGPPublicKey> selector = new And.PubKeySelectionStrategy<>(
                 new SignedByMasterKey.PubkeySelectionStrategy(),
                 new NoRevocation.PubKeySelectionStrategy<>());
 
@@ -150,7 +153,7 @@ public class BCUtil {
         Iterator<PGPPublicKey> publicKeys = ring.getPublicKeys();
         while (publicKeys.hasNext()) {
             PGPPublicKey publicKey = publicKeys.next();
-            if (!selector.accept(masterKeyId, publicKey)) {
+            if (!selector.accept(masterKey, publicKey)) {
                 cleaned = PGPPublicKeyRing.removePublicKey(cleaned, publicKey);
             }
         }
@@ -163,13 +166,15 @@ public class BCUtil {
      * (identified by {@code masterKeyId}), or are revoked ("normal" key revocation, as well as subkey revocation).
      *
      * @param ring key ring
-     * @param masterKeyId id of the master key
+     * @param masterKey master key
      * @return "cleaned" key ring
      */
-    public static PGPSecretKeyRing removeUnassociatedKeysFromKeyRing(PGPSecretKeyRing ring, Long masterKeyId) {
+    public static PGPSecretKeyRing removeUnassociatedKeysFromKeyRing(PGPSecretKeyRing ring, PGPPublicKey masterKey) {
-
+        if (!masterKey.isMasterKey()) {
+            throw new IllegalArgumentException("Given key is not a master key.");
+        }
         // Only select keys which are signed by the master key and not revoked.
-        PublicKeySelectionStrategy<Long> selector = new And.PubKeySelectionStrategy<>(
+        PublicKeySelectionStrategy<PGPPublicKey> selector = new And.PubKeySelectionStrategy<>(
                 new SignedByMasterKey.PubkeySelectionStrategy(),
                 new NoRevocation.PubKeySelectionStrategy<>());
 
@@ -178,7 +183,7 @@ public class BCUtil {
         Iterator<PGPSecretKey> secretKeys = ring.getSecretKeys();
         while (secretKeys.hasNext()) {
             PGPSecretKey secretKey = secretKeys.next();
-            if (!selector.accept(masterKeyId, secretKey.getPublicKey())) {
+            if (!selector.accept(masterKey, secretKey.getPublicKey())) {
                 cleaned = PGPSecretKeyRing.removeSecretKey(cleaned, secretKey);
             }
         }

src/test/java/org/pgpainless/pgpainless/BCUtilTest.java

@@ -18,11 +18,13 @@ package org.pgpainless.pgpainless;
 import static junit.framework.TestCase.assertEquals;
 import static junit.framework.TestCase.assertNotNull;
 import static junit.framework.TestCase.assertNull;
+import static junit.framework.TestCase.assertTrue;
 
 import java.io.IOException;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -119,7 +121,18 @@ public class BCUtilTest extends AbstractPGPainlessTest {
         // Check, if alice_mallory contains mallory's key
         assertNotNull(alice_mallory.getSecretKey(subKey.getKeyID()));
 
-        PGPSecretKeyRing cleaned = BCUtil.removeUnassociatedKeysFromKeyRing(alice_mallory, alice.getPublicKey().getKeyID());
+        PGPSecretKeyRing cleaned = BCUtil.removeUnassociatedKeysFromKeyRing(alice_mallory, alice.getPublicKey());
         assertNull(cleaned.getSecretKey(subKey.getKeyID()));
     }
+
+    @Test
+    public void removeUnsignedKeysECTest()
+            throws PGPException, NoSuchAlgorithmException, NoSuchProviderException, InvalidAlgorithmParameterException,
+            IOException {
+        PGPSecretKeyRing alice = PGPainless.generateKeyRing().simpleEcKeyRing("alice@wonderland.lit");
+
+        PGPSecretKeyRing after = BCUtil.removeUnassociatedKeysFromKeyRing(alice, alice.getPublicKey());
+
+        assertTrue(Arrays.equals(alice.getEncoded(), after.getEncoded()));
+    }
 }