From dfbb01d61cc063ff849d64aa7f21441a7c9facd9 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Tue, 18 Oct 2022 15:55:47 +0200 Subject: [PATCH] Enfore max recursion depth and fix CRC test --- .../MessageMetadata.java | 19 ++++++++- .../OpenPgpMessageInputStream.java | 39 ++++++++++++++++--- .../org/bouncycastle/AsciiArmorCRCTests.java | 20 +++++----- .../MessageMetadataTest.java | 6 +-- .../RecursionDepthTest.java | 4 +- 5 files changed, 66 insertions(+), 22 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/MessageMetadata.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/MessageMetadata.java index 1be47112..59f8052a 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/MessageMetadata.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/MessageMetadata.java @@ -7,6 +7,7 @@ package org.pgpainless.decryption_verification; import org.pgpainless.algorithm.CompressionAlgorithm; import org.pgpainless.algorithm.StreamEncoding; import org.pgpainless.algorithm.SymmetricKeyAlgorithm; +import org.pgpainless.exception.MalformedOpenPgpMessageException; import org.pgpainless.key.SubkeyIdentifier; import org.pgpainless.util.SessionKey; @@ -202,6 +203,8 @@ public class MessageMetadata { } public abstract static class Layer { + public static final int MAX_LAYER_DEPTH = 16; + protected final int depth; protected final List verifiedDetachedSignatures = new ArrayList<>(); protected final List rejectedDetachedSignatures = new ArrayList<>(); protected final List verifiedOnePassSignatures = new ArrayList<>(); @@ -210,6 +213,13 @@ public class MessageMetadata { protected final List rejectedPrependedSignatures = new ArrayList<>(); protected Nested child; + public Layer(int depth) { + this.depth = depth; + if (depth > MAX_LAYER_DEPTH) { + throw new MalformedOpenPgpMessageException("Maximum nesting depth exceeded."); + } + } + public Nested getChild() { return child; } @@ -274,6 +284,9 @@ public class MessageMetadata { public static class Message extends Layer { + public Message() { + super(0); + } } public static class LiteralData implements Nested { @@ -312,7 +325,8 @@ public class MessageMetadata { public static class CompressedData extends Layer implements Nested { protected final CompressionAlgorithm algorithm; - public CompressedData(CompressionAlgorithm zip) { + public CompressedData(CompressionAlgorithm zip, int depth) { + super(depth); this.algorithm = zip; } @@ -332,7 +346,8 @@ public class MessageMetadata { protected SessionKey sessionKey; protected List recipients; - public EncryptedData(SymmetricKeyAlgorithm algorithm) { + public EncryptedData(SymmetricKeyAlgorithm algorithm, int depth) { + super(depth); this.algorithm = algorithm; } 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 944ce6e4..f5ed556a 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 @@ -15,6 +15,7 @@ import java.util.List; import java.util.Stack; import javax.annotation.Nonnull; +import org.bouncycastle.bcpg.ArmoredInputStream; import org.bouncycastle.bcpg.BCPGInputStream; import org.bouncycastle.openpgp.PGPCompressedData; import org.bouncycastle.openpgp.PGPEncryptedData; @@ -56,6 +57,7 @@ import org.pgpainless.key.protection.UnlockSecretKey; import org.pgpainless.policy.Policy; import org.pgpainless.signature.SignatureUtils; import org.pgpainless.signature.consumer.SignatureValidator; +import org.pgpainless.util.ArmoredInputStreamFactory; import org.pgpainless.util.Passphrase; import org.pgpainless.util.SessionKey; import org.pgpainless.util.Tuple; @@ -91,7 +93,7 @@ public class OpenPgpMessageInputStream extends DecryptionStream { @Nonnull ConsumerOptions options, @Nonnull Policy policy) throws PGPException, IOException { - this(inputStream, options, new MessageMetadata.Message(), policy); + this(prepareInputStream(inputStream, options), options, new MessageMetadata.Message(), policy); } protected OpenPgpMessageInputStream(@Nonnull InputStream inputStream, @@ -118,6 +120,26 @@ public class OpenPgpMessageInputStream extends DecryptionStream { consumePackets(); } + private static InputStream prepareInputStream(InputStream inputStream, ConsumerOptions options) throws IOException { + OpenPgpInputStream openPgpIn = new OpenPgpInputStream(inputStream); + openPgpIn.reset(); + + if (openPgpIn.isBinaryOpenPgp()) { + return openPgpIn; + } + + if (openPgpIn.isAsciiArmored()) { + ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(openPgpIn); + if (armorIn.isClearText()) { + return armorIn; + } else { + return armorIn; + } + } else { + return openPgpIn; + } + } + /** * Consume OpenPGP packets from the current {@link BCPGInputStream}. * Once an OpenPGP packet with nested data (Literal Data, Compressed Data, Encrypted Data) is reached, @@ -219,7 +241,8 @@ public class OpenPgpMessageInputStream extends DecryptionStream { signatures.enterNesting(); PGPCompressedData compressedData = packetInputStream.readCompressedData(); MessageMetadata.CompressedData compressionLayer = new MessageMetadata.CompressedData( - CompressionAlgorithm.fromId(compressedData.getAlgorithm())); + CompressionAlgorithm.fromId(compressedData.getAlgorithm()), + metadata.depth + 1); InputStream decompressed = compressedData.getDataStream(); nestedInputStream = new OpenPgpMessageInputStream(buffer(decompressed), options, compressionLayer, policy); } @@ -262,7 +285,8 @@ public class OpenPgpMessageInputStream extends DecryptionStream { SessionKeyDataDecryptorFactory decryptorFactory = ImplementationFactory.getInstance() .getSessionKeyDataDecryptorFactory(sessionKey); - MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData(sessionKey.getAlgorithm()); + MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData( + sessionKey.getAlgorithm(), metadata.depth + 1); try { // TODO: Use BCs new API once shipped @@ -301,7 +325,8 @@ public class OpenPgpMessageInputStream extends DecryptionStream { InputStream decrypted = skesk.getDataStream(decryptorFactory); SessionKey sessionKey = new SessionKey(skesk.getSessionKey(decryptorFactory)); throwIfUnacceptable(sessionKey.getAlgorithm()); - MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData(sessionKey.getAlgorithm()); + MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData( + sessionKey.getAlgorithm(), metadata.depth + 1); encryptedData.sessionKey = sessionKey; IntegrityProtectedInputStream integrityProtected = new IntegrityProtectedInputStream(decrypted, skesk, options); nestedInputStream = new OpenPgpMessageInputStream(buffer(integrityProtected), options, encryptedData, policy); @@ -333,7 +358,8 @@ public class OpenPgpMessageInputStream extends DecryptionStream { throwIfUnacceptable(sessionKey.getAlgorithm()); MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData( - SymmetricKeyAlgorithm.requireFromId(pkesk.getSymmetricAlgorithm(decryptorFactory))); + SymmetricKeyAlgorithm.requireFromId(pkesk.getSymmetricAlgorithm(decryptorFactory)), + metadata.depth + 1); encryptedData.decryptionKey = new SubkeyIdentifier(decryptionKeys, decryptionKey.getKeyID()); encryptedData.sessionKey = sessionKey; @@ -361,7 +387,8 @@ public class OpenPgpMessageInputStream extends DecryptionStream { throwIfUnacceptable(sessionKey.getAlgorithm()); MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData( - SymmetricKeyAlgorithm.requireFromId(pkesk.getSymmetricAlgorithm(decryptorFactory))); + SymmetricKeyAlgorithm.requireFromId(pkesk.getSymmetricAlgorithm(decryptorFactory)), + metadata.depth + 1); encryptedData.decryptionKey = new SubkeyIdentifier(decryptionKeyCandidate.getA(), privateKey.getKeyID()); encryptedData.sessionKey = sessionKey; diff --git a/pgpainless-core/src/test/java/org/bouncycastle/AsciiArmorCRCTests.java b/pgpainless-core/src/test/java/org/bouncycastle/AsciiArmorCRCTests.java index c3af0bd6..f9bd7a61 100644 --- a/pgpainless-core/src/test/java/org/bouncycastle/AsciiArmorCRCTests.java +++ b/pgpainless-core/src/test/java/org/bouncycastle/AsciiArmorCRCTests.java @@ -489,7 +489,7 @@ public class AsciiArmorCRCTests { Passphrase passphrase = Passphrase.fromPassword("flowcrypt compatibility tests"); @Test - public void testInvalidArmorCRCThrowsOnClose() throws PGPException, IOException { + public void testInvalidArmorCRCThrowsOnClose() throws IOException { String message = "-----BEGIN PGP MESSAGE-----\n" + "Version: FlowCrypt 5.0.4 Gmail Encryption flowcrypt.com\n" + "Comment: Seamlessly send, receive and search encrypted email\n" + @@ -542,14 +542,16 @@ public class AsciiArmorCRCTests { "-----END PGP MESSAGE-----\n"; PGPSecretKeyRing key = PGPainless.readKeyRing().secretKeyRing(ASCII_KEY); - DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() - .onInputStream(new ByteArrayInputStream(message.getBytes(StandardCharsets.UTF_8))) - .withOptions(new ConsumerOptions().addDecryptionKey( - key, SecretKeyRingProtector.unlockAnyKeyWith(passphrase) - )); + assertThrows(IOException.class, () -> { + DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(message.getBytes(StandardCharsets.UTF_8))) + .withOptions(new ConsumerOptions().addDecryptionKey( + key, SecretKeyRingProtector.unlockAnyKeyWith(passphrase) + )); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - Streams.pipeAll(decryptionStream, outputStream); - assertThrows(IOException.class, decryptionStream::close); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + Streams.pipeAll(decryptionStream, outputStream); + decryptionStream.close(); + }); } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/MessageMetadataTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/MessageMetadataTest.java index 9f887eb9..d87fc6bf 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/MessageMetadataTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/MessageMetadataTest.java @@ -28,9 +28,9 @@ public class MessageMetadataTest { // For the sake of testing though, this is okay. MessageMetadata.Message message = new MessageMetadata.Message(); - MessageMetadata.CompressedData compressedData = new MessageMetadata.CompressedData(CompressionAlgorithm.ZIP); - MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData(SymmetricKeyAlgorithm.AES_128); - MessageMetadata.EncryptedData encryptedData1 = new MessageMetadata.EncryptedData(SymmetricKeyAlgorithm.AES_256); + MessageMetadata.CompressedData compressedData = new MessageMetadata.CompressedData(CompressionAlgorithm.ZIP, message.depth + 1); + MessageMetadata.EncryptedData encryptedData = new MessageMetadata.EncryptedData(SymmetricKeyAlgorithm.AES_128, compressedData.depth + 1); + MessageMetadata.EncryptedData encryptedData1 = new MessageMetadata.EncryptedData(SymmetricKeyAlgorithm.AES_256, encryptedData.depth + 1); MessageMetadata.LiteralData literalData = new MessageMetadata.LiteralData(); message.setChild(compressedData); diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/RecursionDepthTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/RecursionDepthTest.java index b253cf7f..52c5a906 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/RecursionDepthTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/RecursionDepthTest.java @@ -11,12 +11,12 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; 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.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import org.pgpainless.PGPainless; +import org.pgpainless.exception.MalformedOpenPgpMessageException; import org.pgpainless.util.TestAllImplementations; public class RecursionDepthTest { @@ -143,7 +143,7 @@ public class RecursionDepthTest { "-----END PGP ARMORED FILE-----\n"; - assertThrows(PGPException.class, () -> { + assertThrows(MalformedOpenPgpMessageException.class, () -> { DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() .onInputStream(new ByteArrayInputStream(msg.getBytes(StandardCharsets.UTF_8))) .withOptions(new ConsumerOptions().addDecryptionKey(secretKey));