Create TeeBCPGInputStream to move teeing logic out of OpenPgpMessageInputStream

pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DelayedTeeInputStreamInputStream.java

@@ -1,38 +0,0 @@
-package org.pgpainless.decryption_verification;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-
-public class DelayedTeeInputStreamInputStream extends InputStream {
-
-    private int last = -1;
-    private final InputStream inputStream;
-    private final OutputStream outputStream;
-
-    public DelayedTeeInputStreamInputStream(InputStream inputStream, OutputStream outputStream) {
-        this.inputStream = inputStream;
-        this.outputStream = outputStream;
-    }
-
-    @Override
-    public int read() throws IOException {
-        if (last != -1) {
-            outputStream.write(last);
-        }
-        last = inputStream.read();
-        return last;
-    }
-
-    /**
-     * Squeeze the last byte out and update the output stream.
-     *
-     * @throws IOException in case of an IO error
-     */
-    public void squeeze() throws IOException {
-        if (last != -1) {
-            outputStream.write(last);
-        }
-        last = -1;
-    }
-}

pgpainless-core/src/main/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStream.java

@@ -11,7 +11,6 @@ import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.NoSuchElementException;
 import java.util.Stack;
 
 import org.bouncycastle.bcpg.BCPGInputStream;
@@ -57,6 +56,8 @@ import org.pgpainless.util.Tuple;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nonnull;
+
 public class OpenPgpMessageInputStream extends InputStream {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(OpenPgpMessageInputStream.class);
@@ -66,16 +67,15 @@ public class OpenPgpMessageInputStream extends InputStream {
     protected final OpenPgpMetadata.Builder resultBuilder;
     // Pushdown Automaton to verify validity of OpenPGP packet sequence in an OpenPGP message
     protected final PDA automaton = new PDA();
-    protected final DelayedTeeInputStreamInputStream delayedTee;
+    // InputStream of OpenPGP packets
-    // InputStream of OpenPGP packets of the current layer
+    protected TeeBCPGInputStream packetInputStream;
-    protected final BCPGInputStream packetInputStream;
     // InputStream of a nested data packet
     protected InputStream nestedInputStream;
 
     private boolean closed = false;
 
     private final Signatures signatures;
-    private MessageMetadata.Layer metadata;
+    private final MessageMetadata.Layer metadata;
 
     public OpenPgpMessageInputStream(InputStream inputStream, ConsumerOptions options)
             throws IOException, PGPException {
@@ -95,9 +95,7 @@ public class OpenPgpMessageInputStream extends InputStream {
             this.signatures.addDetachedSignatures(options.getDetachedSignatures());
         }
 
-        delayedTee = new DelayedTeeInputStreamInputStream(inputStream, signatures);
+        packetInputStream = new TeeBCPGInputStream(BCPGInputStream.wrap(inputStream), signatures);
-        BCPGInputStream bcpg = BCPGInputStream.wrap(delayedTee);
-        this.packetInputStream = bcpg;
 
         // *omnomnom*
         consumePackets();
@@ -122,41 +120,36 @@ public class OpenPgpMessageInputStream extends InputStream {
     private void consumePackets()
             throws IOException, PGPException {
         OpenPgpPacket nextPacket;
-        loop: while ((nextPacket = nextPacketTag()) != null) {
+        loop: while ((nextPacket = packetInputStream.nextPacketTag()) != null) {
             signatures.nextPacket(nextPacket);
             switch (nextPacket) {
 
                 // Literal Data - the literal data content is the new input stream
                 case LIT:
                     automaton.next(InputAlphabet.LiteralData);
-                    delayedTee.squeeze();
                     processLiteralData();
                     break loop;
 
                 // Compressed Data - the content contains another OpenPGP message
                 case COMP:
                     automaton.next(InputAlphabet.CompressedData);
-                    signatures.commitNested();
                     processCompressedData();
-                    delayedTee.squeeze();
                     break loop;
 
                 // One Pass Signature
                 case OPS:
-                    automaton.next(InputAlphabet.OnePassSignatures);
+                    automaton.next(InputAlphabet.OnePassSignature);
                     PGPOnePassSignature onePassSignature = readOnePassSignature();
-                    delayedTee.squeeze();
                     signatures.addOnePassSignature(onePassSignature);
                     break;
 
                 // Signature - either prepended to the message, or corresponding to a One Pass Signature
                 case SIG:
+                    // true if Signature corresponds to OnePassSignature
                     boolean isSigForOPS = automaton.peekStack() == StackAlphabet.ops;
-                    automaton.next(InputAlphabet.Signatures);
+                    automaton.next(InputAlphabet.Signature);
 
-                    PGPSignature signature = readSignature();
+                    processSignature(isSigForOPS);
-                    delayedTee.squeeze();
-                    processSignature(signature, isSigForOPS);
 
                     break;
 
@@ -166,7 +159,6 @@ public class OpenPgpMessageInputStream extends InputStream {
                 case SED:
                 case SEIPD:
                     automaton.next(InputAlphabet.EncryptedData);
-                    delayedTee.squeeze();
                     if (processEncryptedData()) {
                         break loop;
                     }
@@ -175,8 +167,7 @@ public class OpenPgpMessageInputStream extends InputStream {
 
                     // Marker Packets need to be skipped and ignored
                 case MARKER:
-                    packetInputStream.readPacket(); // skip
+                    packetInputStream.readMarker();
-                    delayedTee.squeeze();
                     break;
 
                 // Key Packets are illegal in this context
@@ -204,26 +195,10 @@ public class OpenPgpMessageInputStream extends InputStream {
         }
     }
 
-    private OpenPgpPacket nextPacketTag() throws IOException {
+    private void processSignature(boolean isSigForOPS) throws PGPException, IOException {
-        int tag = nextTag();
+        PGPSignature signature = readSignature();
-        if (tag == -1) {
-            log("EOF");
-            return null;
-        }
-        OpenPgpPacket packet;
-        try {
-            packet = OpenPgpPacket.requireFromTag(tag);
-        } catch (NoSuchElementException e) {
-            log("Invalid tag: " + tag);
-            throw e;
-        }
-        log(packet.toString());
-        return packet;
-    }
-
-    private void processSignature(PGPSignature signature, boolean isSigForOPS) {
         if (isSigForOPS) {
-            signatures.popNested();
+            signatures.leaveNesting(); // TODO: Only leave nesting if all OPSs are dealt with
             signatures.addCorrespondingOnePassSignature(signature);
         } else {
             signatures.addPrependedSignature(signature);
@@ -231,21 +206,22 @@ public class OpenPgpMessageInputStream extends InputStream {
     }
 
     private void processCompressedData() throws IOException, PGPException {
-        PGPCompressedData compressedData = new PGPCompressedData(packetInputStream);
+        signatures.enterNesting();
+        PGPCompressedData compressedData = packetInputStream.readCompressedData();
         MessageMetadata.CompressedData compressionLayer = new MessageMetadata.CompressedData(
                 CompressionAlgorithm.fromId(compressedData.getAlgorithm()));
         nestedInputStream = new OpenPgpMessageInputStream(compressedData.getDataStream(), options, compressionLayer);
     }
 
     private void processLiteralData() throws IOException {
-        PGPLiteralData literalData = new PGPLiteralData(packetInputStream);
+        PGPLiteralData literalData = packetInputStream.readLiteralData();
         this.metadata.setChild(new MessageMetadata.LiteralData(literalData.getFileName(), literalData.getModificationTime(),
                 StreamEncoding.requireFromCode(literalData.getFormat())));
         nestedInputStream = literalData.getDataStream();
     }
 
     private boolean processEncryptedData() throws IOException, PGPException {
-        PGPEncryptedDataList encDataList = new PGPEncryptedDataList(packetInputStream);
+        PGPEncryptedDataList encDataList = packetInputStream.readEncryptedDataList();
 
         // TODO: Replace with !encDataList.isIntegrityProtected()
         if (!encDataList.get(0).isIntegrityProtected()) {
@@ -345,19 +321,6 @@ public class OpenPgpMessageInputStream extends InputStream {
         return false;
     }
 
-    private int nextTag() throws IOException {
-        try {
-            return packetInputStream.nextPacketTag();
-        } catch (IOException e) {
-            if ("Stream closed".equals(e.getMessage())) {
-                // ZipInflater Streams sometimes close under our feet -.-
-                // Therefore we catch resulting IOEs and return -1 instead.
-                return -1;
-            }
-            throw e;
-        }
-    }
-
     private List<Tuple<PGPSecretKeyRing, PGPSecretKey>> findPotentialDecryptionKeys(PGPPublicKeyEncryptedData pkesk) {
         int algorithm = pkesk.getAlgorithm();
         List<Tuple<PGPSecretKeyRing, PGPSecretKey>> decryptionKeyCandidates = new ArrayList<>();
@@ -387,12 +350,12 @@ public class OpenPgpMessageInputStream extends InputStream {
 
     private PGPOnePassSignature readOnePassSignature()
             throws PGPException, IOException {
-        return new PGPOnePassSignature(packetInputStream);
+        return packetInputStream.readOnePassSignature();
     }
 
     private PGPSignature readSignature()
             throws PGPException, IOException {
-        return new PGPSignature(packetInputStream);
+        return packetInputStream.readSignature();
     }
 
     @Override
@@ -428,7 +391,7 @@ public class OpenPgpMessageInputStream extends InputStream {
     }
 
     @Override
-    public int read(byte[] b, int off, int len)
+    public int read(@Nonnull byte[] b, int off, int len)
             throws IOException {
 
         if (nestedInputStream == null) {
@@ -482,8 +445,7 @@ public class OpenPgpMessageInputStream extends InputStream {
     private void collectMetadata() {
         if (nestedInputStream instanceof OpenPgpMessageInputStream) {
             OpenPgpMessageInputStream child = (OpenPgpMessageInputStream) nestedInputStream;
-            MessageMetadata.Layer childLayer = child.metadata;
+            this.metadata.setChild((MessageMetadata.Nested) child.metadata);
-            this.metadata.setChild((MessageMetadata.Nested) childLayer);
         }
     }
 
@@ -496,9 +458,9 @@ public class OpenPgpMessageInputStream extends InputStream {
 
     private static class SortedESKs {
 
-        private List<PGPPBEEncryptedData> skesks = new ArrayList<>();
+        private final List<PGPPBEEncryptedData> skesks = new ArrayList<>();
-        private List<PGPPublicKeyEncryptedData> pkesks = new ArrayList<>();
+        private final List<PGPPublicKeyEncryptedData> pkesks = new ArrayList<>();
-        private List<PGPPublicKeyEncryptedData> anonPkesks = new ArrayList<>();
+        private final List<PGPPublicKeyEncryptedData> anonPkesks = new ArrayList<>();
 
         SortedESKs(PGPEncryptedDataList esks) {
             for (PGPEncryptedData esk : esks) {
@@ -526,11 +488,10 @@ public class OpenPgpMessageInputStream extends InputStream {
         }
     }
 
-    // TODO: In 'OPS LIT("Foo") SIG', OPS is only updated with "Foo"
+    // In 'OPS LIT("Foo") SIG', OPS is only updated with "Foo"
-    //  In 'OPS[1] OPS LIT("Foo") SIG SIG', OPS[1] (nested) is updated with OPS LIT("Foo") SIG.
+    // In 'OPS[1] OPS LIT("Foo") SIG SIG', OPS[1] (nested) is updated with OPS LIT("Foo") SIG.
-    //  Therefore, we need to handle the innermost signature layer differently when updating with Literal data.
+    // Therefore, we need to handle the innermost signature layer differently when updating with Literal data.
-    //  For this we might want to provide two update entries into the Signatures class, one for OpenPGP packets and one
+    // Furthermore, For 'OPS COMP(LIT("Foo")) SIG', the signature is updated with "Foo". CHAOS!!!
-    //  for literal data. UUUUUGLY!!!!
     private static final class Signatures extends OutputStream {
         final ConsumerOptions options;
         final List<SIG> detachedSignatures;
@@ -578,7 +539,7 @@ public class OpenPgpMessageInputStream extends InputStream {
 
             literalOPS.add(ops);
             if (signature.isContaining()) {
-                commitNested();
+                enterNesting();
             }
         }
 
@@ -599,12 +560,12 @@ public class OpenPgpMessageInputStream extends InputStream {
             }
         }
 
-        void commitNested() {
+        void enterNesting() {
             opsUpdateStack.push(literalOPS);
             literalOPS = new ArrayList<>();
         }
 
-        void popNested() {
+        void leaveNesting() {
             if (opsUpdateStack.isEmpty()) {
                 return;
             }
@@ -722,7 +683,7 @@ public class OpenPgpMessageInputStream extends InputStream {
         }
 
         @Override
-        public void write(byte[] b, int off, int len) {
+        public void write(@Nonnull byte[] b, int off, int len) {
             updatePacket(b, off, len);
         }
 

pgpainless-core/src/main/java/org/pgpainless/decryption_verification/TeeBCPGInputStream.java

@@ -0,0 +1,131 @@
+// SPDX-FileCopyrightText: 2022 Paul Schaub <vanitasvitae@fsfe.org>
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package org.pgpainless.decryption_verification;
+
+import org.bouncycastle.bcpg.BCPGInputStream;
+import org.bouncycastle.bcpg.MarkerPacket;
+import org.bouncycastle.bcpg.Packet;
+import org.bouncycastle.openpgp.PGPCompressedData;
+import org.bouncycastle.openpgp.PGPEncryptedDataList;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPLiteralData;
+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
+ * {@link BCPGInputStream#readPacket()} inconsistently calls a mix of {@link BCPGInputStream#read()} and
+ * {@link InputStream#read()} of the underlying stream. This would cause the second length byte to get swallowed up.
+ *
+ * Therefore, this class delegates the teeing to an {@link DelayedTeeInputStreamInputStream} which wraps the underlying
+ * stream. Since calling {@link BCPGInputStream#nextPacketTag()} reads up to and including the next packets tag,
+ * we need to delay teeing out that byte to signature verifiers.
+ * Hence, the reading methods of the {@link TeeBCPGInputStream} handle pushing this byte to the output stream using
+ * {@link DelayedTeeInputStreamInputStream#squeeze()}.
+ */
+public class TeeBCPGInputStream {
+
+    protected final DelayedTeeInputStreamInputStream delayedTee;
+    // InputStream of OpenPGP packets of the current layer
+    protected final BCPGInputStream packetInputStream;
+
+    public TeeBCPGInputStream(BCPGInputStream inputStream, OutputStream outputStream) {
+        this.delayedTee = new DelayedTeeInputStreamInputStream(inputStream, outputStream);
+        this.packetInputStream = BCPGInputStream.wrap(delayedTee);
+    }
+
+    public OpenPgpPacket nextPacketTag() throws IOException {
+        int tag = packetInputStream.nextPacketTag();
+        if (tag == -1) {
+            return null;
+        }
+
+        OpenPgpPacket packet;
+        try {
+            packet = OpenPgpPacket.requireFromTag(tag);
+        } catch (NoSuchElementException e) {
+            throw e;
+        }
+        return packet;
+    }
+
+    public Packet readPacket() throws IOException {
+        return packetInputStream.readPacket();
+    }
+
+    public PGPCompressedData readCompressedData() throws IOException {
+        delayedTee.squeeze();
+        PGPCompressedData compressedData = new PGPCompressedData(packetInputStream);
+        return compressedData;
+    }
+
+    public PGPLiteralData readLiteralData() throws IOException {
+        delayedTee.squeeze();
+        return new PGPLiteralData(packetInputStream);
+    }
+
+    public PGPEncryptedDataList readEncryptedDataList() throws IOException {
+        delayedTee.squeeze();
+        return new PGPEncryptedDataList(packetInputStream);
+    }
+
+    public PGPOnePassSignature readOnePassSignature() throws PGPException, IOException {
+        PGPOnePassSignature onePassSignature = new PGPOnePassSignature(packetInputStream);
+        delayedTee.squeeze();
+        return onePassSignature;
+    }
+
+    public PGPSignature readSignature() throws PGPException, IOException {
+        PGPSignature signature = new PGPSignature(packetInputStream);
+        delayedTee.squeeze();
+        return signature;
+    }
+
+    public MarkerPacket readMarker() throws IOException {
+        MarkerPacket markerPacket = (MarkerPacket) readPacket();
+        delayedTee.squeeze();
+        return markerPacket;
+    }
+
+    public static class DelayedTeeInputStreamInputStream extends InputStream {
+
+        private int last = -1;
+        private final InputStream inputStream;
+        private final OutputStream outputStream;
+
+        public DelayedTeeInputStreamInputStream(InputStream inputStream, OutputStream outputStream) {
+            this.inputStream = inputStream;
+            this.outputStream = outputStream;
+        }
+
+        @Override
+        public int read() throws IOException {
+            if (last != -1) {
+                outputStream.write(last);
+            }
+            last = inputStream.read();
+            return last;
+        }
+
+        /**
+         * Squeeze the last byte out and update the output stream.
+         *
+         * @throws IOException in case of an IO error
+         */
+        public void squeeze() throws IOException {
+            if (last != -1) {
+                outputStream.write(last);
+            }
+            last = -1;
+        }
+    }
+}

pgpainless-core/src/main/java/org/pgpainless/decryption_verification/automaton/InputAlphabet.java

@@ -18,11 +18,11 @@ public enum InputAlphabet {
     /**
      * A {@link PGPSignatureList} object.
      */
-    Signatures,
+    Signature,
     /**
      * A {@link PGPOnePassSignatureList} object.
      */
-    OnePassSignatures,
+    OnePassSignature,
     /**
      * A {@link PGPCompressedData} packet.
      * The contents of this packet MUST form a valid OpenPGP message, so a nested PDA is opened to verify

pgpainless-core/src/main/java/org/pgpainless/decryption_verification/automaton/PDA.java

@@ -35,11 +35,11 @@ public class PDA {
                     case LiteralData:
                         return LiteralMessage;
 
-                    case Signatures:
+                    case Signature:
                         automaton.pushStack(msg);
                         return OpenPgpMessage;
 
-                    case OnePassSignatures:
+                    case OnePassSignature:
                         automaton.pushStack(ops);
                         automaton.pushStack(msg);
                         return OpenPgpMessage;
@@ -63,7 +63,7 @@ public class PDA {
                 StackAlphabet stackItem = automaton.popStack();
                 switch (input) {
 
-                    case Signatures:
+                    case Signature:
                         if (stackItem == ops) {
                             return CorrespondingSignature;
                         } else {
@@ -78,7 +78,7 @@ public class PDA {
                         }
 
                     case LiteralData:
-                    case OnePassSignatures:
+                    case OnePassSignature:
                     case CompressedData:
                     case EncryptedData:
                     default:
@@ -92,7 +92,7 @@ public class PDA {
             State transition(InputAlphabet input, PDA automaton) throws MalformedOpenPgpMessageException {
                 StackAlphabet stackItem = automaton.popStack();
                 switch (input) {
-                    case Signatures:
+                    case Signature:
                         if (stackItem == ops) {
                             return CorrespondingSignature;
                         } else {
@@ -107,7 +107,7 @@ public class PDA {
                         }
 
                     case LiteralData:
-                    case OnePassSignatures:
+                    case OnePassSignature:
                     case CompressedData:
                     case EncryptedData:
                     default:
@@ -121,7 +121,7 @@ public class PDA {
             State transition(InputAlphabet input, PDA automaton) throws MalformedOpenPgpMessageException {
                 StackAlphabet stackItem = automaton.popStack();
                 switch (input) {
-                    case Signatures:
+                    case Signature:
                         if (stackItem == ops) {
                             return CorrespondingSignature;
                         } else {
@@ -136,7 +136,7 @@ public class PDA {
                         }
 
                     case LiteralData:
-                    case OnePassSignatures:
+                    case OnePassSignature:
                     case CompressedData:
                     case EncryptedData:
                     default:
@@ -156,7 +156,7 @@ public class PDA {
                         // premature end of stream
                         throw new MalformedOpenPgpMessageException(this, input, stackItem);
                     }
-                } else if (input == InputAlphabet.Signatures) {
+                } else if (input == InputAlphabet.Signature) {
                     if (stackItem == ops) {
                         return CorrespondingSignature;
                     }

pgpainless-core/src/test/java/org/pgpainless/decryption_verification/OpenPgpMessageInputStreamTest.java

@@ -49,7 +49,6 @@ import org.pgpainless.util.Tuple;
 
 public class OpenPgpMessageInputStreamTest {
 
-
     public static final String KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
             "Version: PGPainless\n" +
             "Comment: DA05 848F 37D4 68E6 F982  C889 7A70 1FC6 904D 3F4C\n" +
@@ -241,7 +240,7 @@ public class OpenPgpMessageInputStreamTest {
                 System.out);
     }
 
-    public static void genSIG_LIT() throws PGPException, IOException {
+    public static void genSIG_COMP_LIT() throws PGPException, IOException {
         PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(KEY);
         ByteArrayOutputStream msgOut = new ByteArrayOutputStream();
         EncryptionStream signer = PGPainless.encryptAndOrSign()

pgpainless-core/src/test/java/org/pgpainless/decryption_verification/automaton/PDATest.java

@@ -30,9 +30,9 @@ public class PDATest {
     @Test
     public void testSimpleOpsSignedMesssageIsValid() throws MalformedOpenPgpMessageException {
         PDA automaton = new PDA();
-        automaton.next(InputAlphabet.OnePassSignatures);
+        automaton.next(InputAlphabet.OnePassSignature);
         automaton.next(InputAlphabet.LiteralData);
-        automaton.next(InputAlphabet.Signatures);
+        automaton.next(InputAlphabet.Signature);
         automaton.next(InputAlphabet.EndOfSequence);
 
         assertTrue(automaton.isValid());
@@ -47,7 +47,7 @@ public class PDATest {
     @Test
     public void testSimplePrependSignedMessageIsValid() throws MalformedOpenPgpMessageException {
         PDA automaton = new PDA();
-        automaton.next(InputAlphabet.Signatures);
+        automaton.next(InputAlphabet.Signature);
         automaton.next(InputAlphabet.LiteralData);
         automaton.next(InputAlphabet.EndOfSequence);
 
@@ -63,10 +63,10 @@ public class PDATest {
     @Test
     public void testOPSSignedCompressedMessageIsValid() throws MalformedOpenPgpMessageException {
         PDA automaton = new PDA();
-        automaton.next(InputAlphabet.OnePassSignatures);
+        automaton.next(InputAlphabet.OnePassSignature);
         automaton.next(InputAlphabet.CompressedData);
         // Here would be a nested PDA for the LiteralData packet
-        automaton.next(InputAlphabet.Signatures);
+        automaton.next(InputAlphabet.Signature);
         automaton.next(InputAlphabet.EndOfSequence);
 
         assertTrue(automaton.isValid());