From 64a50266f1476127db69ddcd4d1e423b2552d308 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 5 May 2022 12:43:44 +0200 Subject: [PATCH] Test for detection of uncompressed, signed messages, and improve decryption of seip messages --- .../DecryptionStreamFactory.java | 5 ++- .../OpenPgpInputStream.java | 11 +++--- .../OpenPgpInputStreamTest.java | 38 ++++++++++++++++++- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java index 2da18c00..1913444e 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java @@ -140,7 +140,10 @@ public final class DecryptionStreamFactory { return new DecryptionStream(pgpInStream, resultBuilder, integrityProtectedEncryptedInputStream, null); } - if (openPgpIn.isLikelyOpenPgpMessage()) { + // Data appears to be OpenPGP message, + // or we handle it as such, since user provided a session-key for decryption + if (openPgpIn.isLikelyOpenPgpMessage() || + (openPgpIn.isBinaryOpenPgp() && options.getSessionKey() != null)) { outerDecodingStream = openPgpIn; objectFactory = ImplementationFactory.getInstance().getPGPObjectFactory(outerDecodingStream); // Parse OpenPGP message diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpInputStream.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpInputStream.java index cccbe250..669c2dec 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpInputStream.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpInputStream.java @@ -77,14 +77,14 @@ public class OpenPgpInputStream extends BufferedInputStream { } private void inspectBuffer() throws IOException { - if (determineIsArmored()) { + if (checkForAsciiArmor()) { return; } - determineIsBinaryOpenPgp(); + checkForBinaryOpenPgp(); } - private boolean determineIsArmored() { + private boolean checkForAsciiArmor() { if (startsWithIgnoringWhitespace(buffer, ARMOR_HEADER, bufferLen)) { containsArmorHeader = true; return true; @@ -100,7 +100,7 @@ public class OpenPgpInputStream extends BufferedInputStream { * This breaks down though if we read plausible garbage where the data accidentally makes sense, * or valid, yet incomplete packets (remember, we are still only working on a portion of the data). */ - private void determineIsBinaryOpenPgp() throws IOException { + private void checkForBinaryOpenPgp() throws IOException { if (bufferLen == -1) { // Empty data return; @@ -210,7 +210,6 @@ public class OpenPgpInputStream extends BufferedInputStream { } containsOpenPgpPackets = true; - isLikelyOpenPgpMessage = true; break; case SYMMETRIC_KEY_ENC_SESSION: @@ -295,6 +294,8 @@ public class OpenPgpInputStream extends BufferedInputStream { case SYMMETRIC_KEY_ENC: // No data to compare :( containsOpenPgpPackets = true; + // While this is a valid OpenPGP message, enabling the line below would lead to too many false positives + // isLikelyOpenPgpMessage = true; break; case MARKER: diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpInputStreamTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpInputStreamTest.java index 67719886..c2a24c76 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpInputStreamTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpInputStreamTest.java @@ -13,20 +13,30 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; import java.util.Random; import org.bouncycastle.bcpg.ArmoredInputStream; import org.bouncycastle.bcpg.CompressionAlgorithmTags; import org.bouncycastle.openpgp.PGPCompressedDataGenerator; +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.util.io.Streams; import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.algorithm.CompressionAlgorithm; +import org.pgpainless.encryption_signing.EncryptionStream; +import org.pgpainless.encryption_signing.ProducerOptions; +import org.pgpainless.encryption_signing.SigningOptions; +import org.pgpainless.key.protection.SecretKeyRingProtector; public class OpenPgpInputStreamTest { private static final Random RANDOM = new Random(); - @RepeatedTest(10) + @RepeatedTest(100) public void randomBytesDoNotContainOpenPgpData() throws IOException { byte[] randomBytes = new byte[1000000]; RANDOM.nextBytes(randomBytes); @@ -43,7 +53,7 @@ public class OpenPgpInputStreamTest { assertArrayEquals(randomBytes, outBytes); } - @RepeatedTest(10) + @Test public void largeCompressedDataIsBinaryOpenPgp() throws IOException { // Since we are compressing RANDOM data, the output will likely be roughly the same size // So we very likely will end up with data larger than the MAX_BUFFER_SIZE @@ -725,4 +735,28 @@ public class OpenPgpInputStreamTest { assertFalse(openPgpInputStream.isAsciiArmored()); assertTrue(openPgpInputStream.isNonOpenPgp()); } + + @Test + public void testSignedMessageConsumption() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + ByteArrayInputStream plaintext = new ByteArrayInputStream("Hello, World!\n".getBytes(StandardCharsets.UTF_8)); + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .modernKeyRing("Sigmund ", null); + + ByteArrayOutputStream signedOut = new ByteArrayOutputStream(); + EncryptionStream signer = PGPainless.encryptAndOrSign() + .onOutputStream(signedOut) + .withOptions(ProducerOptions.sign(new SigningOptions() + .addSignature(SecretKeyRingProtector.unprotectedKeys(), secretKeys)) + .setAsciiArmor(false) + .overrideCompressionAlgorithm(CompressionAlgorithm.UNCOMPRESSED)); + + Streams.pipeAll(plaintext, signer); + signer.close(); + + byte[] binary = signedOut.toByteArray(); + + OpenPgpInputStream openPgpIn = new OpenPgpInputStream(new ByteArrayInputStream(binary)); + assertFalse(openPgpIn.isAsciiArmored()); + assertTrue(openPgpIn.isLikelyOpenPgpMessage()); + } }