diff --git a/src/main/java/org/pgpainless/pgpainless/key/selection/key/impl/SignedByMasterKey.java b/src/main/java/org/pgpainless/pgpainless/key/selection/key/impl/SignedByMasterKey.java new file mode 100644 index 00000000..81585013 --- /dev/null +++ b/src/main/java/org/pgpainless/pgpainless/key/selection/key/impl/SignedByMasterKey.java @@ -0,0 +1,37 @@ +package org.pgpainless.pgpainless.key.selection.key.impl; + +import java.util.Iterator; +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPSignature; +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 { + + @Override + public boolean accept(Long identifier, PGPPublicKey key) { + Iterator signatures = key.getSignaturesForKeyID(identifier); + while (signatures.hasNext()) { + PGPSignature signature = signatures.next(); + if (signature.getSignatureType() == PGPSignature.SUBKEY_BINDING) { + try { + return signature.verify(); + } catch (PGPException e) { + LOGGER.log(Level.WARNING, "Could not verify subkey signature of key " + + Long.toHexString(signature.getKeyID()) + " on key " + Long.toHexString(key.getKeyID())); + + return false; + } + } + } + return false; + } + } +} diff --git a/src/main/java/org/pgpainless/pgpainless/util/BCUtil.java b/src/main/java/org/pgpainless/pgpainless/util/BCUtil.java index d0efda77..f396e5c7 100644 --- a/src/main/java/org/pgpainless/pgpainless/util/BCUtil.java +++ b/src/main/java/org/pgpainless/pgpainless/util/BCUtil.java @@ -42,6 +42,10 @@ import org.bouncycastle.openpgp.PGPSignatureSubpacketVector; import org.bouncycastle.openpgp.PGPUtil; import org.bouncycastle.util.io.Streams; import org.pgpainless.pgpainless.algorithm.KeyFlag; +import org.pgpainless.pgpainless.key.selection.key.PublicKeySelectionStrategy; +import org.pgpainless.pgpainless.key.selection.key.impl.And; +import org.pgpainless.pgpainless.key.selection.key.impl.NoRevocation; +import org.pgpainless.pgpainless.key.selection.key.impl.SignedByMasterKey; public class BCUtil { @@ -113,7 +117,7 @@ public class BCUtil { public static PGPPublicKeyRing getKeyRingFromCollection(PGPPublicKeyRingCollection collection, Long id) throws PGPException { - return removeUnsignedKeysFromKeyRing(collection.getPublicKeyRing(id), id); + return removeUnassociatedKeysFromKeyRing(collection.getPublicKeyRing(id), id); } public static InputStream getPgpDecoderInputStream(byte[] bytes) throws IOException { @@ -136,21 +140,27 @@ public class BCUtil { return getDecodedBytes(buffer.toByteArray()); } - public static PGPPublicKeyRing removeUnsignedKeysFromKeyRing(PGPPublicKeyRing ring, Long masterKeyId) { + /** + * Remove all keys from the key ring, are either not having a subkey signature from the master key + * (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 + * @return "cleaned" key ring + */ + public static PGPPublicKeyRing removeUnassociatedKeysFromKeyRing(PGPPublicKeyRing ring, Long masterKeyId) { - Set signedKeyIds = new HashSet<>(); - signedKeyIds.add(masterKeyId); - Iterator signedKeys = ring.getKeysWithSignaturesBy(masterKeyId); - while (signedKeys.hasNext()) { - signedKeyIds.add(signedKeys.next().getKeyID()); - } + // Only select keys which are signed by the master key and not revoked. + PublicKeySelectionStrategy selector = new And.PubKeySelectionStrategy<>( + new SignedByMasterKey.PubkeySelectionStrategy(), + new NoRevocation.PubKeySelectionStrategy<>()); PGPPublicKeyRing cleaned = ring; Iterator publicKeys = ring.getPublicKeys(); while (publicKeys.hasNext()) { PGPPublicKey publicKey = publicKeys.next(); - if (!signedKeyIds.contains(publicKey.getKeyID())) { + if (!selector.accept(masterKeyId, publicKey)) { cleaned = PGPPublicKeyRing.removePublicKey(cleaned, publicKey); } } @@ -158,20 +168,27 @@ public class BCUtil { return cleaned; } - public static PGPSecretKeyRing removeUnsignedKeysFromKeyRing(PGPSecretKeyRing ring, Long masterKeyId) { - Set signedKeyIds = new HashSet<>(); - signedKeyIds.add(masterKeyId); - Iterator signedKeys = ring.getKeysWithSignaturesBy(masterKeyId); - while (signedKeys.hasNext()) { - signedKeyIds.add(signedKeys.next().getKeyID()); - } + /** + * Remove all keys from the key ring, are either not having a subkey signature from the master key + * (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 + * @return "cleaned" key ring + */ + public static PGPSecretKeyRing removeUnassociatedKeysFromKeyRing(PGPSecretKeyRing ring, Long masterKeyId) { + + // Only select keys which are signed by the master key and not revoked. + PublicKeySelectionStrategy selector = new And.PubKeySelectionStrategy<>( + new SignedByMasterKey.PubkeySelectionStrategy(), + new NoRevocation.PubKeySelectionStrategy<>()); PGPSecretKeyRing cleaned = ring; Iterator secretKeys = ring.getSecretKeys(); while (secretKeys.hasNext()) { PGPSecretKey secretKey = secretKeys.next(); - if (!signedKeyIds.contains(secretKey.getKeyID())) { + if (!selector.accept(masterKeyId, secretKey.getPublicKey())) { cleaned = PGPSecretKeyRing.removeSecretKey(cleaned, secretKey); } } @@ -179,11 +196,18 @@ public class BCUtil { return cleaned; } + /** + * Return the {@link PGPPublicKey} which is the master key of the key ring. + * + * @param ring key ring + * @return master key + */ public static PGPPublicKey getMasterKeyFrom(PGPPublicKeyRing ring) { Iterator it = ring.getPublicKeys(); while (it.hasNext()) { PGPPublicKey k = it.next(); if (k.isMasterKey()) { + // There can only be one master key, so we can immediately return return k; } } @@ -232,20 +256,10 @@ public class BCUtil { } public static boolean keyRingContainsKeyWithId(PGPPublicKeyRing ring, long keyId) { - Iterator keys = ring.getPublicKeys(); - while (keys.hasNext()) { - PGPPublicKey key = keys.next(); - if (key.getKeyID() == keyId) return true; - } - return false; + return ring.getPublicKey(keyId) != null; } public static boolean keyRingContainsKeyWithId(PGPSecretKeyRing ring, long keyId) { - Iterator keys = ring.getPublicKeys(); - while (keys.hasNext()) { - PGPPublicKey key = keys.next(); - if (key.getKeyID() == keyId) return true; - } - return false; + return ring.getSecretKey(keyId) != null; } } \ No newline at end of file diff --git a/src/test/java/org/pgpainless/pgpainless/BCUtilTest.java b/src/test/java/org/pgpainless/pgpainless/BCUtilTest.java index 1a874cee..8bb4b50e 100644 --- a/src/test/java/org/pgpainless/pgpainless/BCUtilTest.java +++ b/src/test/java/org/pgpainless/pgpainless/BCUtilTest.java @@ -119,7 +119,7 @@ public class BCUtilTest extends AbstractPGPainlessTest { // Check, if alice_mallory contains mallory's key assertNotNull(alice_mallory.getSecretKey(subKey.getKeyID())); - PGPSecretKeyRing cleaned = BCUtil.removeUnsignedKeysFromKeyRing(alice_mallory, alice.getPublicKey().getKeyID()); + PGPSecretKeyRing cleaned = BCUtil.removeUnassociatedKeysFromKeyRing(alice_mallory, alice.getPublicKey().getKeyID()); assertNull(cleaned.getSecretKey(subKey.getKeyID())); } }