diff --git a/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java b/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java index 8e246b50..e370cdde 100644 --- a/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java +++ b/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java @@ -5,15 +5,14 @@ import java.io.IOException; import java.time.Instant; -import java.util.HashSet; -import java.util.Iterator; -import java.util.Set; +import java.util.*; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import javax.annotation.Nullable; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.generation.KeyFlag; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.keyrings.KeyringConfig; +import org.bouncycastle.bcpg.PublicKeyAlgorithmTags; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; @@ -33,6 +32,20 @@ public class Rfc4880KeySelectionStrategy implements KeySelectionStrategy { private final boolean ignoreCase; private final boolean matchPartial; + // list of algorithms that can be used for encryption + private final List encryptionAlgorithms = Arrays.asList(PublicKeyAlgorithmTags.RSA_GENERAL, + PublicKeyAlgorithmTags.RSA_ENCRYPT, + PublicKeyAlgorithmTags.ECDH, + PublicKeyAlgorithmTags.ELGAMAL_ENCRYPT, + PublicKeyAlgorithmTags.ELGAMAL_GENERAL); + + // list of algorithms that can be used for signing + private final List signatureAlgorithms = Arrays.asList(PublicKeyAlgorithmTags.RSA_GENERAL, + PublicKeyAlgorithmTags.RSA_SIGN, + PublicKeyAlgorithmTags.DSA, + PublicKeyAlgorithmTags.ECDSA, + PublicKeyAlgorithmTags.EDDSA); + /** * Construct an instance with matchPartial and ignoreCase set to true. * @@ -221,39 +234,72 @@ protected boolean isExpired(PGPPublicKey pubKey) { return isExpired; } - + /** + * Checks if a public key may be used for encryption. This uses the key KeyFlags subpacket content by default, + * falling back to the key algorithm if there isn't any KeyFlags subpacket + * @param publicKey public key to examine + * @return true if the key can be used for encryption + */ protected boolean isEncryptionKey(PGPPublicKey publicKey) { requireNonNull(publicKey, "publicKey must not be null"); + boolean isEncryptionKey = false; + + final Optional> optionalKeyFlags = extractPublicKeyFlags(publicKey); + + /* If the key contains a KeyFlag subpacket, we extract its flags to determine if the + key can be used for encryption + */ + if (optionalKeyFlags.isPresent()) { // NOPMD:LawOfDemeter + final Set keyFlags = optionalKeyFlags.get(); + final boolean canEncryptCommunication = keyFlags // NOPMD:LawOfDemeter + .contains(KeyFlag.ENCRYPT_COMMS); + final boolean canEncryptStorage = keyFlags // NOPMD:LawOfDemeter + .contains(KeyFlag.ENCRYPT_STORAGE); + isEncryptionKey = canEncryptCommunication || canEncryptStorage; + } else { + /* If the key doesn't contain any KeyFlag subpacket, check the key algorithm. + This is what GPG does (g10/misc.c) and lets us encrypt with keys that don't contain a KeyFlag subpacket + */ + isEncryptionKey = encryptionAlgorithms.contains(publicKey.getAlgorithm()); + } - final Set keyFlags = extractPublicKeyFlags(publicKey); + return isEncryptionKey; + } - final boolean canEncryptCommunication = keyFlags // NOPMD:LawOfDemeter - .contains(KeyFlag.ENCRYPT_COMMS); - final boolean canEncryptStorage = keyFlags // NOPMD:LawOfDemeter - .contains(KeyFlag.ENCRYPT_STORAGE); + protected boolean isVerificationKey(PGPPublicKey publicKey) { + requireNonNull(publicKey, "publicKey must not be null"); - return canEncryptCommunication || canEncryptStorage; - } + final Optional> optionalKeyFlags = extractPublicKeyFlags(publicKey); + boolean isVerificationKey; - protected boolean isVerificationKey(PGPPublicKey pubKey) { - final boolean isVerficationKey = - extractPublicKeyFlags(pubKey).contains(KeyFlag.SIGN_DATA); // NOPMD:LawOfDemeter + /* If the key contains a KeyFlag subpacket, we extract its flags to determine if the + key can be used for signing + */ + if (optionalKeyFlags.isPresent()) { // NOPMD:LawOfDemeter + isVerificationKey = optionalKeyFlags.get().contains(KeyFlag.SIGN_DATA); // NOPMD:LawOfDemeter + } else { + /* If the key doesn't contain any KeyFlag subpacket, check the key algorithm. + This is what GPG does (g10/misc.c) and lets us signing with keys that don't contain a KeyFlag subpacket + */ + isVerificationKey =signatureAlgorithms.contains(publicKey.getAlgorithm()); + } - if (!isVerficationKey) { + if (!isVerificationKey) { LOGGER.trace("Skipping pubkey {} (no signing key)", - Long.toHexString(pubKey.getKeyID())); + Long.toHexString(publicKey.getKeyID())); } - return isVerficationKey; + + return isVerificationKey; } - protected boolean isRevoked(PGPPublicKey pubKey) { - requireNonNull(pubKey, "pubKey must not be null"); + protected boolean isRevoked(PGPPublicKey publicKey) { + requireNonNull(publicKey, "pubKey must not be null"); - final boolean hasRevocation = pubKey.hasRevocation(); + final boolean hasRevocation = publicKey.hasRevocation(); if (hasRevocation) { LOGGER.trace("Skipping pubkey {} (revoked)", - Long.toHexString(pubKey.getKeyID())); + Long.toHexString(publicKey.getKeyID())); } return hasRevocation; } diff --git a/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/generation/KeyFlag.java b/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/generation/KeyFlag.java index b2eae0c8..353ce8cb 100644 --- a/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/generation/KeyFlag.java +++ b/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/generation/KeyFlag.java @@ -19,7 +19,10 @@ import java.util.EnumSet; import java.util.Iterator; +import java.util.Optional; import java.util.Set; + +import org.bouncycastle.bcpg.SignatureSubpacketTags; import org.bouncycastle.bcpg.sig.KeyFlags; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSignature; @@ -91,11 +94,19 @@ public static Set fromInteger(int bitmask) { return flags; } + /** + * Returns the list of key flags (ie whether the key can be used to encrypt, sign, etc) based on the analysis of the + * KeyFlags subpacket. Returns an empty Optional object if the key does not contain a KeyFlags subpacket + * @param publicKey the key to analyse + * @return a list of key flags, or an empty Optional if the key doesn't contain a KeyFlags subpacket + */ @SuppressWarnings({"PMD.LawOfDemeter"}) - public static Set extractPublicKeyFlags(PGPPublicKey publicKey) { + public static Optional> extractPublicKeyFlags(PGPPublicKey publicKey) { requireNonNull(publicKey, "publicKey must not be null"); int aggregatedKeyFlags = 0; + boolean hasKeyFlags = false; + Optional> publicKeyFlags; final Iterator directKeySignatures = publicKey.getSignatures(); @@ -103,10 +114,20 @@ public static Set extractPublicKeyFlags(PGPPublicKey publicKey) { final PGPSignature signature = directKeySignatures.next(); final PGPSignatureSubpacketVector hashedSubPackets = signature.getHashedSubPackets(); - final int keyFlags = hashedSubPackets.getKeyFlags(); - aggregatedKeyFlags |= keyFlags; + if (hashedSubPackets.hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) { + hasKeyFlags = true; + final int keyFlags = hashedSubPackets.getKeyFlags(); + aggregatedKeyFlags |= keyFlags; + } } - return fromInteger(aggregatedKeyFlags); + + if (hasKeyFlags) { + publicKeyFlags = Optional.of(fromInteger(aggregatedKeyFlags)); + } else { + publicKeyFlags = Optional.empty(); + } + + return publicKeyFlags; } public int getFlag() { diff --git a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/encrypting/EncryptionScenariosTest.java b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/encrypting/EncryptionScenariosTest.java index 195d461d..2de9a72d 100644 --- a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/encrypting/EncryptionScenariosTest.java +++ b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/encrypting/EncryptionScenariosTest.java @@ -2,11 +2,8 @@ import name.neuhalfen.projects.crypto.bouncycastle.openpgp.BouncyGPG; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.algorithms.DefaultPGPAlgorithmSuites; -import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.callbacks.KeyringConfigCallbacks; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.keyrings.InMemoryKeyring; -import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.keyrings.KeyringConfig; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.keyrings.KeyringConfigs; -import name.neuhalfen.projects.crypto.bouncycastle.openpgp.testtooling.Configs; import name.neuhalfen.projects.crypto.bouncycastle.openpgp.testtooling.ExampleMessages; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.util.io.Streams; @@ -19,6 +16,7 @@ import java.time.Instant; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertNotNull; /** * Tests several encryption scenarios. As usual, most are integration style tests. @@ -75,4 +73,47 @@ public void encrypt_masterKeyWithoutSubkeys_works() assertArrayEquals(ExampleMessages.IMPORTANT_QUOTE_TEXT.getBytes(), plainBA.toByteArray()); } + @Test + public void encrypt_masterKeyWithoutSubkeysOrKeyFlags_works() + throws IOException, PGPException, NoSuchAlgorithmException, SignatureException, NoSuchProviderException { + + // Reported in https://github.com/neuhalje/bouncy-gpg/issues/50 + + /* We test that a key that doesn't contain any KeyFlags subpacket can still be used for encrypting, + by checking the key algorithm, like GPG does + */ + + // keyring + InMemoryKeyring sendersKeyring = KeyringConfigs.forGpgExportedKeys(keyId -> null); + sendersKeyring.addPublicKey(ExampleMessages.ONLY_MASTER_KEY_PUBKEY_NO_KEY_FLAGS.getBytes()); + + // encrypt + ByteArrayOutputStream result = new ByteArrayOutputStream(); + BufferedOutputStream bufferedOutputStream = new BufferedOutputStream(result); + + final OutputStream outputStream = BouncyGPG + .encryptToStream() + .withConfig(sendersKeyring) + .setReferenceDateForKeyValidityTo(Instant.MAX) + .withAlgorithms(DefaultPGPAlgorithmSuites.strongSuite()) + .toRecipient(ExampleMessages.ONLY_MASTER_KEY_UID_NO_KEY_FLAGS) + .andDoNotSign() + .binaryOutput() + .andWriteTo(bufferedOutputStream); + + final InputStream is = new ByteArrayInputStream( + ExampleMessages.IMPORTANT_QUOTE_TEXT.getBytes()); + Streams.pipeAll(is, outputStream); + outputStream.close(); + bufferedOutputStream.close(); + is.close(); + + /* test that the data can be encrypted. + If the test fails, Bouncy-PGP would complain that it can't find a public key to encrypt with + */ + + final byte[] ciphertext = result.toByteArray(); + result.close(); + assertNotNull(ciphertext); + } } diff --git a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/ExampleMessages.java b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/ExampleMessages.java index cbc48271..f3017d6c 100644 --- a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/ExampleMessages.java +++ b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/ExampleMessages.java @@ -483,6 +483,34 @@ public class ExampleMessages { "=8wl4\n" + "-----END PGP PRIVATE KEY BLOCK-----\n"; + public final static long ONLY_MASTER_KEY_ID_NO_KEY_FLAGS = 0x9E698FBA9F857349L; + public final static String ONLY_MASTER_KEY_UID_NO_KEY_FLAGS = "support@anzgcis.com"; + public final static String ONLY_MASTER_KEY_PASSPHRASE_NO_KEY_FLAGS = ""; + public final static String ONLY_MASTER_KEY_PUBKEY_NO_KEY_FLAGS= "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "Version: SecureBlackbox 15\n" + + "\n" + + "xsBNBFtrhAABCACbJgPWFmodLq0cqIwiZG8hnHusSBA1nan8OQk2Dr+l37smSjO7\n" + + "TT0N/G8er79lgVyynYXXHH7EfY7tvvWPS5N55NvYiLxRYRPgFgy+cD6TsSn8W0qD\n" + + "zwCf5Jw5ZeU4pdRo6hN1Qgl6IcETQWZOsob3BWmg2aLnl+1SyY3zh0jW6z6Wn4En\n" + + "tpPj4iELe6o1SNcmC2A2fIVUoMlBHa+Mcumw+kqv4vsA6xJmpDYniBaW/asVaIBl\n" + + "4bA0Iux5Osr83pNJOBHpLk9ozyMF0rtqyyvA8l3SiLA2jyeXoBEQ9mEJi+G0/PBI\n" + + "mAoMs3FXlu4Dqdh6jk0vHDk261tMq8wqas7nABEBAAHCwGIEHwEIABYFCRLMAwAF\n" + + "AltreqYJEJ5pj7qfhXNJAACwjQgAktbLedTzTJmRgdgQsH3uakoFmi7Pc2Q/N9J7\n" + + "q8wJhBcxUUILQQck9fmzM4XOgAJmX0Sn+3xnkUvdnSP8EGd5SPezoDo29Udrm99z\n" + + "LV//9N5p4febzQwAhQJg3rGz5oDt5VMwTdSxhxPAKvkIV1QAQFqeZn6wO26Voz3h\n" + + "wfLt3xX904pomtCeakdRu+akqTBKS28Q6atHauHAAzlr1cfjLcLHdqI1y1zUbAyE\n" + + "dmYhpiIM3sr1ZvjaSRQv+vF/l5z88EVE5Ag4U5Mdw8X7R9WCnEk5ojOOveNYIsq2\n" + + "rjh9rAvD6aNBK4v+1wx9P1zSv227SojxX+AB103GSYitBSLTUs0oRmlsZWFjdGl2\n" + + "ZSBTdXBwb3J0IDxzdXBwb3J0QGFuemdjaXMuY29tPsLAXwQQAQgAEwUCW2wHRgIZ\n" + + "AQkQnmmPup+Fc0kAAGN8CACR82NV7nMGOm/crnu7k2dCHCoiTsucerG1tDFyCNYx\n" + + "3w0WATvSBS6XRwLfl6IaVbjpauZk+bH6NVgo9tpwKwGn7Wh9takHMsuRPoNiudqf\n" + + "S/duYt/ugOhlajfe3EFuGlt62j8pVT5o9bgCZQNEUgkGzwwz+j/saWHsI9fW26Ri\n" + + "W/tyJk0IICLf2aEJOi63PsllRQ+CtimvZkRalvcCuS19JNcYZlz3GyM/+z3LiYMi\n" + + "b1s+vDO4ZJgka+nTWHL1VrbctH79n23hmV1pK7LXAl8mZlo0wUU+R+01WumSVSQZ\n" + + "XGs5vAe6nG216ghBcNNd0UbQMZXwuIQlOtN5CSwoHdp6\n" + + "=qWOE\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n"; + public final static Map ALL_KEYRINGS_PASSWORDS = BUILD_ALL_KEYRINGS_PASSWORDS(); /** * Encrypted-To: recipient@example.com Signed-By: sender@example.com Compressed: false @@ -738,6 +766,7 @@ private static Map BUILD_ALL_KEYRINGS_PASSWORDS() { m.put(ExampleMessages.PUBKEY_ID_RECIPIENT, "recipient".toCharArray()); m.put(ExampleMessages.ONLY_MASTER_KEY_ID, null); + m.put(ExampleMessages.ONLY_MASTER_KEY_ID_NO_KEY_FLAGS, null); return Collections.unmodifiableMap(m); } diff --git a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/matcher/SecretKeyringKeyRoleMatcher.java b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/matcher/SecretKeyringKeyRoleMatcher.java index 9de5453c..a65b3472 100644 --- a/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/matcher/SecretKeyringKeyRoleMatcher.java +++ b/src/test/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/testtooling/matcher/SecretKeyringKeyRoleMatcher.java @@ -1,9 +1,7 @@ package name.neuhalfen.projects.crypto.bouncycastle.openpgp.testtooling.matcher; -import java.util.Collections; -import java.util.EnumSet; -import java.util.Iterator; -import java.util.Set; +import java.util.*; + import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.generation.KeyFlag; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSecretKey; @@ -54,21 +52,22 @@ EnumSet parseKeyRing(final PGPSecretKeyRing ring) { EnumSet parseKey(final PGPSecretKey item) { final EnumSet found = EnumSet.noneOf(KeyRole.class); final PGPPublicKey publicKey = item.getPublicKey(); - final Set keyFlags = KeyFlag.extractPublicKeyFlags(publicKey); + final Optional> keyFlags = KeyFlag.extractPublicKeyFlags(publicKey); if (item.isMasterKey()) { found.add(KeyRole.MASTER); } if (item.isSigningKey()) { - if (keyFlags.contains(KeyFlag.SIGN_DATA)) { + if (keyFlags.isPresent() && keyFlags.get().contains(KeyFlag.SIGN_DATA)) { found.add(KeyRole.SIGNING); } } if (item.getPublicKey().isEncryptionKey()) { final boolean hasEncryptionFlags = - keyFlags.contains(KeyFlag.ENCRYPT_STORAGE) || keyFlags.contains(KeyFlag.ENCRYPT_COMMS); + keyFlags.isPresent() && keyFlags.get().contains(KeyFlag.ENCRYPT_STORAGE) || + keyFlags.isPresent() && keyFlags.get().contains(KeyFlag.ENCRYPT_COMMS); if (hasEncryptionFlags) { found.add(KeyRole.ENCRYPTION); }