From 0cb088525193ce845ee4cb51d66ac7c21513ddc1 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Tue, 25 Apr 2023 13:28:07 +0200 Subject: [PATCH] Relax constraints on decryption keys to improve interop with faulty, broken legacy clients that have been very naughty and need punishment --- .../OpenPgpMessageInputStream.java | 9 ++-- .../org/pgpainless/key/info/KeyRingInfo.java | 46 +++++++++++++++++++ ...ntDecryptionUsingNonEncryptionKeyTest.java | 18 ++++++++ 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStream.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStream.java index 7fe11bbf..19a01fbf 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStream.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStream.java @@ -43,15 +43,14 @@ import org.bouncycastle.openpgp.operator.SessionKeyDataDecryptorFactory; import org.bouncycastle.util.io.TeeInputStream; import org.pgpainless.PGPainless; import org.pgpainless.algorithm.CompressionAlgorithm; -import org.pgpainless.algorithm.EncryptionPurpose; import org.pgpainless.algorithm.OpenPgpPacket; import org.pgpainless.algorithm.StreamEncoding; import org.pgpainless.algorithm.SymmetricKeyAlgorithm; +import org.pgpainless.decryption_verification.cleartext_signatures.ClearsignedMessageUtil; +import org.pgpainless.decryption_verification.cleartext_signatures.MultiPassStrategy; import org.pgpainless.decryption_verification.syntax_check.InputSymbol; import org.pgpainless.decryption_verification.syntax_check.PDA; import org.pgpainless.decryption_verification.syntax_check.StackSymbol; -import org.pgpainless.decryption_verification.cleartext_signatures.ClearsignedMessageUtil; -import org.pgpainless.decryption_verification.cleartext_signatures.MultiPassStrategy; import org.pgpainless.exception.MalformedOpenPgpMessageException; import org.pgpainless.exception.MessageNotIntegrityProtectedException; import org.pgpainless.exception.MissingDecryptionMethodException; @@ -674,7 +673,7 @@ public class OpenPgpMessageInputStream extends DecryptionStream { for (PGPSecretKeyRing secretKeys : options.getDecryptionKeys()) { KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); - for (PGPPublicKey publicKey : info.getEncryptionSubkeys(EncryptionPurpose.ANY)) { + for (PGPPublicKey publicKey : info.getDecryptionSubkeys()) { if (publicKey.getAlgorithm() == algorithm && info.isSecretKeyAvailable(publicKey.getKeyID())) { PGPSecretKey candidate = secretKeys.getSecretKey(publicKey.getKeyID()); decryptionKeyCandidates.add(new Tuple<>(secretKeys, candidate)); @@ -692,7 +691,7 @@ public class OpenPgpMessageInputStream extends DecryptionStream { } KeyRingInfo info = new KeyRingInfo(secretKeys, policy, new Date()); - List encryptionKeys = info.getEncryptionSubkeys(EncryptionPurpose.ANY); + List encryptionKeys = info.getDecryptionSubkeys(); for (PGPPublicKey key : encryptionKeys) { if (key.getKeyID() == keyID) { return secretKeys; diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java index 1ebd023e..75d9e16c 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java @@ -41,11 +41,15 @@ import org.pgpainless.algorithm.SymmetricKeyAlgorithm; import org.pgpainless.exception.KeyException; import org.pgpainless.key.OpenPgpFingerprint; import org.pgpainless.key.SubkeyIdentifier; +import org.pgpainless.key.util.KeyIdUtil; import org.pgpainless.key.util.RevocationAttributes; import org.pgpainless.policy.Policy; import org.pgpainless.signature.SignatureUtils; import org.pgpainless.signature.consumer.SignaturePicker; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; +import org.pgpainless.util.DateUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Utility class to quickly extract certain information from a {@link PGPPublicKeyRing}/{@link PGPSecretKeyRing}. @@ -55,6 +59,8 @@ public class KeyRingInfo { private static final Pattern PATTERN_EMAIL_FROM_USERID = Pattern.compile("<([a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+)>"); private static final Pattern PATTERN_EMAIL_EXPLICIT = Pattern.compile("^([a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+)$"); + private static final Logger LOGGER = LoggerFactory.getLogger(KeyRingInfo.class); + private final PGPKeyRing keys; private final Signatures signatures; private final Date referenceDate; @@ -904,6 +910,7 @@ public class KeyRingInfo { public @Nonnull List getEncryptionSubkeys(EncryptionPurpose purpose) { Date primaryExpiration = getPrimaryKeyExpirationDate(); if (primaryExpiration != null && primaryExpiration.before(referenceDate)) { + LOGGER.debug("Certificate is expired: Primary key is expired on " + DateUtil.formatUTCDate(primaryExpiration)); return Collections.emptyList(); } @@ -913,15 +920,18 @@ public class KeyRingInfo { PGPPublicKey subKey = subkeys.next(); if (!isKeyValidlyBound(subKey.getKeyID())) { + LOGGER.debug("(Sub?)-Key " + KeyIdUtil.formatKeyId(subKey.getKeyID()) + " is not validly bound."); continue; } Date subkeyExpiration = getSubkeyExpirationDate(OpenPgpFingerprint.of(subKey)); if (subkeyExpiration != null && subkeyExpiration.before(referenceDate)) { + LOGGER.debug("(Sub?)-Key " + KeyIdUtil.formatKeyId(subKey.getKeyID()) + " is expired on " + DateUtil.formatUTCDate(subkeyExpiration)); continue; } if (!subKey.isEncryptionKey()) { + LOGGER.debug("(Sub?)-Key " + KeyIdUtil.formatKeyId(subKey.getKeyID()) + " algorithm is not capable of encryption."); continue; } @@ -947,6 +957,42 @@ public class KeyRingInfo { return encryptionKeys; } + /** + * Return a list of all subkeys that could potentially be used to decrypt a message. + * Contrary to {@link #getEncryptionSubkeys(EncryptionPurpose)}, this method also includes revoked, expired keys, + * as well as keys which do not carry any encryption keyflags. + * Merely keys which use algorithms that cannot be used for encryption at all are excluded. + * That way, decryption of messages produced by faulty implementations can still be decrypted. + * + * @return decryption keys + */ + public @Nonnull List getDecryptionSubkeys() { + Iterator subkeys = keys.getPublicKeys(); + List decryptionKeys = new ArrayList<>(); + + while (subkeys.hasNext()) { + PGPPublicKey subKey = subkeys.next(); + + // subkeys have been valid at some point + if (subKey.getKeyID() != getKeyId()) { + PGPSignature binding = signatures.subkeyBindings.get(subKey.getKeyID()); + if (binding == null) { + LOGGER.debug("Subkey " + KeyIdUtil.formatKeyId(subKey.getKeyID()) + " was never validly bound."); + continue; + } + } + + // Public-Key algorithm can encrypt + if (!subKey.isEncryptionKey()) { + LOGGER.debug("(Sub?)-Key " + KeyIdUtil.formatKeyId(subKey.getKeyID()) + " is not encryption-capable."); + continue; + } + + decryptionKeys.add(subKey); + } + return decryptionKeys; + } + /** * Return a list of all keys which carry the provided key flag in their signature. * diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/PreventDecryptionUsingNonEncryptionKeyTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/PreventDecryptionUsingNonEncryptionKeyTest.java index ba80c69d..04a98265 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/PreventDecryptionUsingNonEncryptionKeyTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/PreventDecryptionUsingNonEncryptionKeyTest.java @@ -14,6 +14,7 @@ import java.nio.charset.StandardCharsets; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.util.io.Streams; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.pgpainless.PGPainless; import org.pgpainless.exception.MissingDecryptionMethodException; @@ -189,6 +190,23 @@ public class PreventDecryptionUsingNonEncryptionKeyTest { } @Test + public void canDecryptMessageDespiteMissingKeyFlag() throws IOException, PGPException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(ENCRYPTION_INCAPABLE_KEY); + + ByteArrayInputStream msgIn = new ByteArrayInputStream(MSG.getBytes(StandardCharsets.UTF_8)); + DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() + .onInputStream(msgIn) + .withOptions(new ConsumerOptions().addDecryptionKey(secretKeys)); + + Streams.drain(decryptionStream); + decryptionStream.close(); + OpenPgpMetadata metadata = decryptionStream.getResult(); + + assertEquals(new SubkeyIdentifier(secretKeys, secretKeys.getPublicKey().getKeyID()), metadata.getDecryptionKey()); + } + + @Test + @Disabled public void nonEncryptionKeyCannotDecrypt() throws IOException { PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(ENCRYPTION_INCAPABLE_KEY);