From ce85b010575bff322fda6d8f367ba5721c4a9427 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Fri, 21 Oct 2022 16:16:45 +0200 Subject: [PATCH] Fix CRCing test and fully depend on new stream for decryption --- .../DecryptionBuilder.java | 2 +- .../MessageMetadata.java | 22 ++- .../OpenPgpMessageInputStream.java | 152 +++++++++++++----- .../TeeBCPGInputStream.java | 14 +- .../OpenPgpMessageInputStreamTest.java | 2 +- 5 files changed, 137 insertions(+), 55 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionBuilder.java index 68a68847..0baf4124 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionBuilder.java @@ -31,7 +31,7 @@ public class DecryptionBuilder implements DecryptionBuilderInterface { throw new IllegalArgumentException("Consumer options cannot be null."); } - return DecryptionStreamFactory.create(inputStream, consumerOptions); + return OpenPgpMessageInputStream.create(inputStream, consumerOptions); } } } 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 59f8052a..7811578e 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 @@ -164,19 +164,35 @@ public class MessageMetadata { } public String getFilename() { - return findLiteralData().getFileName(); + LiteralData literalData = findLiteralData(); + if (literalData == null) { + return null; + } + return literalData.getFileName(); } public Date getModificationDate() { - return findLiteralData().getModificationDate(); + LiteralData literalData = findLiteralData(); + if (literalData == null) { + return null; + } + return literalData.getModificationDate(); } public StreamEncoding getFormat() { - return findLiteralData().getFormat(); + LiteralData literalData = findLiteralData(); + if (literalData == null) { + return null; + } + return literalData.getFormat(); } private LiteralData findLiteralData() { Nested nested = message.child; + if (nested == null) { + return null; + } + while (nested.hasNestedChild()) { Layer layer = (Layer) nested; nested = layer.child; 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 a2fa2351..dee59058 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 @@ -37,6 +37,8 @@ import org.bouncycastle.openpgp.operator.PBEDataDecryptorFactory; import org.bouncycastle.openpgp.operator.PGPContentVerifierBuilderProvider; import org.bouncycastle.openpgp.operator.PublicKeyDataDecryptorFactory; import org.bouncycastle.openpgp.operator.SessionKeyDataDecryptorFactory; +import org.bouncycastle.util.io.Streams; +import org.bouncycastle.util.io.TeeInputStream; import org.pgpainless.PGPainless; import org.pgpainless.algorithm.CompressionAlgorithm; import org.pgpainless.algorithm.EncryptionPurpose; @@ -79,32 +81,87 @@ public class OpenPgpMessageInputStream extends DecryptionStream { // Options to consume the data protected final ConsumerOptions options; + + private final Policy policy; // Pushdown Automaton to verify validity of OpenPGP packet sequence in an OpenPGP message protected final PDA syntaxVerifier = new PDA(); // InputStream of OpenPGP packets protected TeeBCPGInputStream packetInputStream; - // InputStream of a nested data packet + // InputStream of a data packet containing nested data protected InputStream nestedInputStream; private boolean closed = false; private final Signatures signatures; private final MessageMetadata.Layer metadata; - private final Policy policy; - public OpenPgpMessageInputStream(@Nonnull InputStream inputStream, + /** + * Create an {@link OpenPgpMessageInputStream} suitable for decryption and verification of + * OpenPGP messages and signatures. + * This constructor will use the global PGPainless {@link Policy}. + * + * @param inputStream underlying input stream + * @param options options for consuming the stream + * + * @throws IOException in case of an IO error + * @throws PGPException in case of an OpenPGP error + */ + public static OpenPgpMessageInputStream create(@Nonnull InputStream inputStream, @Nonnull ConsumerOptions options) throws IOException, PGPException { - this(inputStream, options, PGPainless.getPolicy()); + return create(inputStream, options, PGPainless.getPolicy()); } - public OpenPgpMessageInputStream(@Nonnull InputStream inputStream, + /** + * Create an {@link OpenPgpMessageInputStream} suitable for decryption and verification of + * OpenPGP messages and signatures. + * + * @param inputStream underlying input stream containing the OpenPGP message + * @param options options for consuming the message + * @param policy policy for acceptable algorithms etc. + * + * @throws PGPException in case of an OpenPGP error + * @throws IOException in case of an IO error + */ + public static OpenPgpMessageInputStream create(@Nonnull InputStream inputStream, @Nonnull ConsumerOptions options, @Nonnull Policy policy) throws PGPException, IOException { - this( - prepareInputStream(inputStream, options, policy), - options, new MessageMetadata.Message(), policy); + return create(inputStream, options, new MessageMetadata.Message(), policy); + } + + protected static OpenPgpMessageInputStream create(@Nonnull InputStream inputStream, + @Nonnull ConsumerOptions options, + @Nonnull MessageMetadata.Layer metadata, + @Nonnull Policy policy) + throws IOException, PGPException { + OpenPgpInputStream openPgpIn = new OpenPgpInputStream(inputStream); + openPgpIn.reset(); + + if (openPgpIn.isNonOpenPgp() || options.isForceNonOpenPgpData()) { + return new OpenPgpMessageInputStream(Type.non_openpgp, + openPgpIn, options, metadata, policy); + } + + if (openPgpIn.isBinaryOpenPgp()) { + // Simply consume OpenPGP message + return new OpenPgpMessageInputStream(Type.standard, + openPgpIn, options, metadata, policy); + } + + if (openPgpIn.isAsciiArmored()) { + ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(openPgpIn); + if (armorIn.isClearText()) { + return new OpenPgpMessageInputStream(Type.cleartext_signed, + armorIn, options, metadata, policy); + } else { + // Simply consume dearmored OpenPGP message + return new OpenPgpMessageInputStream(Type.standard, + armorIn, options, metadata, policy); + } + } else { + throw new AssertionError("Huh?"); + } } protected OpenPgpMessageInputStream(@Nonnull InputStream inputStream, @@ -131,52 +188,56 @@ public class OpenPgpMessageInputStream extends DecryptionStream { consumePackets(); } - protected OpenPgpMessageInputStream(@Nonnull InputStream inputStream, - @Nonnull Policy policy, - @Nonnull ConsumerOptions options) { + enum Type { + standard, + cleartext_signed, + non_openpgp + } + + protected OpenPgpMessageInputStream(@Nonnull Type type, + @Nonnull InputStream inputStream, + @Nonnull ConsumerOptions options, + @Nonnull MessageMetadata.Layer metadata, + @Nonnull Policy policy) throws PGPException, IOException { super(OpenPgpMetadata.getBuilder()); this.policy = policy; this.options = options; - this.metadata = new MessageMetadata.Message(); + this.metadata = metadata; this.signatures = new Signatures(options); - this.signatures.addDetachedSignatures(options.getDetachedSignatures()); - this.packetInputStream = new TeeBCPGInputStream(BCPGInputStream.wrap(inputStream), signatures); - } - private static InputStream prepareInputStream(InputStream inputStream, ConsumerOptions options, Policy policy) - throws IOException, PGPException { - OpenPgpInputStream openPgpIn = new OpenPgpInputStream(inputStream); - openPgpIn.reset(); - - if (openPgpIn.isBinaryOpenPgp()) { - return openPgpIn; + if (metadata instanceof MessageMetadata.Message) { + this.signatures.addDetachedSignatures(options.getDetachedSignatures()); } - if (openPgpIn.isAsciiArmored()) { - ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(openPgpIn); - if (armorIn.isClearText()) { - return parseCleartextSignedMessage(armorIn, options, policy); - } else { - return armorIn; - } - } else { - return openPgpIn; + switch (type) { + case standard: + // tee out packet bytes for signature verification + packetInputStream = new TeeBCPGInputStream(BCPGInputStream.wrap(inputStream), this.signatures); + + // *omnomnom* + consumePackets(); + break; + case cleartext_signed: + MultiPassStrategy multiPassStrategy = options.getMultiPassStrategy(); + PGPSignatureList detachedSignatures = ClearsignedMessageUtil + .detachSignaturesFromInbandClearsignedMessage( + inputStream, multiPassStrategy.getMessageOutputStream()); + + for (PGPSignature signature : detachedSignatures) { + options.addVerificationOfDetachedSignature(signature); + } + + options.forceNonOpenPgpData(); + packetInputStream = null; + nestedInputStream = new TeeInputStream(multiPassStrategy.getMessageInputStream(), this.signatures); + break; + case non_openpgp: + packetInputStream = null; + nestedInputStream = new TeeInputStream(inputStream, this.signatures); + break; } } - private static DecryptionStream parseCleartextSignedMessage(ArmoredInputStream armorIn, ConsumerOptions options, Policy policy) - throws IOException, PGPException { - MultiPassStrategy multiPassStrategy = options.getMultiPassStrategy(); - PGPSignatureList signatures = ClearsignedMessageUtil.detachSignaturesFromInbandClearsignedMessage(armorIn, multiPassStrategy.getMessageOutputStream()); - - for (PGPSignature signature : signatures) { - options.addVerificationOfDetachedSignature(signature); - } - - options.forceNonOpenPgpData(); - return new OpenPgpMessageInputStream(multiPassStrategy.getMessageInputStream(), policy, options); - } - /** * Consume OpenPGP packets from the current {@link BCPGInputStream}. * Once an OpenPGP packet with nested data (Literal Data, Compressed Data, Encrypted Data) is reached, @@ -196,6 +257,9 @@ public class OpenPgpMessageInputStream extends DecryptionStream { private void consumePackets() throws IOException, PGPException { OpenPgpPacket nextPacket; + if (packetInputStream == null) { + return; + } loop: // we break this when we go deeper. while ((nextPacket = packetInputStream.nextPacketTag()) != null) { diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/TeeBCPGInputStream.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/TeeBCPGInputStream.java index 2efcfc43..1ca1b3f9 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/TeeBCPGInputStream.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/TeeBCPGInputStream.java @@ -4,6 +4,11 @@ package org.pgpainless.decryption_verification; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.NoSuchElementException; + import org.bouncycastle.bcpg.BCPGInputStream; import org.bouncycastle.bcpg.MarkerPacket; import org.bouncycastle.bcpg.Packet; @@ -15,11 +20,6 @@ import org.bouncycastle.openpgp.PGPOnePassSignature; import org.bouncycastle.openpgp.PGPSignature; import org.pgpainless.algorithm.OpenPgpPacket; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.NoSuchElementException; - /** * Since we need to update signatures with data from the underlying stream, this class is used to tee out the data. * Unfortunately we cannot simply override {@link BCPGInputStream#read()} to tee the data out though, since @@ -96,7 +96,6 @@ public class TeeBCPGInputStream { return markerPacket; } - public void close() throws IOException { this.packetInputStream.close(); this.delayedTee.close(); @@ -122,6 +121,9 @@ public class TeeBCPGInputStream { last = inputStream.read(); return last; } catch (IOException e) { + if ("crc check failed in armored message.".equals(e.getMessage())) { + throw e; + } return -1; } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStreamTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStreamTest.java index a8969047..f9539149 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStreamTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStreamTest.java @@ -677,7 +677,7 @@ public class OpenPgpMessageInputStreamTest { throws IOException, PGPException { ByteArrayInputStream bytesIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(bytesIn); - OpenPgpMessageInputStream pgpIn = new OpenPgpMessageInputStream(armorIn, options); + OpenPgpMessageInputStream pgpIn = OpenPgpMessageInputStream.create(armorIn, options); return pgpIn; } }