Fix signature generation with all format and signature type combinations

This comes at the cost of that we no longer CR/LF encode literal data before encryption/signing.
That means that applications that rely on PGPainless to do the CR/LF encoding must manually
do the encoding before feeding the message to PGPainless.
The newly introduced CRLFGeneratorStream has documentation on how to do that.
Fixes #264

pgpainless-core/src/main/java/org/pgpainless/encryption_signing/CRLFGeneratorStream.java

@@ -0,0 +1,64 @@
+// SPDX-FileCopyrightText: 2021 David Hook <dgh@cryptoworkshop.com>
+// SPDX-FileCopyrightText: 2022 Paul Schaub <vanitasvitae@fsfe.org>
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package org.pgpainless.encryption_signing;
+
+import org.pgpainless.algorithm.StreamEncoding;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * {@link OutputStream} which applies CR-LF encoding of its input data, based on the desired {@link StreamEncoding}.
+ *
+ *
+ * If you need PGPainless to CRLF encode signed data for you, you could do the following:
+ * {@code
+ * <pre>
+ *     InputStream plaintext = ...
+ *     EncryptionStream signerOrEncryptor = PGPainless.signAndOrEncrypt(...);
+ *     CRLFGeneratorStream crlfOut = new CRLFGeneratorStream(signerOrEncryptor, streamEncoding);
+ *
+ *     Streams.pipeAll(plaintext, crlfOut);
+ *     crlfOut.close;
+ *
+ *     EncryptionResult result = signerOrEncryptor.getResult();
+ * </pre>
+ * }
+ * This implementation originates from the Bouncy Castle library.
+ */
+public class CRLFGeneratorStream extends OutputStream {
+
+    protected final OutputStream crlfOut;
+    private final boolean isBinary;
+    private int lastB = 0;
+
+    public CRLFGeneratorStream(OutputStream crlfOut, StreamEncoding encoding) {
+        this.crlfOut = crlfOut;
+        this.isBinary = encoding == StreamEncoding.BINARY;
+    }
+
+    public void write(int b) throws IOException {
+        if (!isBinary) {
+            if (b == '\n' && lastB != '\r') {   // Unix
+                crlfOut.write('\r');
+            } else if (lastB == '\r') {         // MAC
+                if (b != '\n') {
+                    crlfOut.write('\n');
+                }
+            }
+            lastB = b;
+        }
+
+        crlfOut.write(b);
+    }
+
+    public void close() throws IOException {
+        if (!isBinary && lastB == '\r') {       // MAC
+            crlfOut.write('\n');
+        }
+        crlfOut.close();
+    }
+}

pgpainless-core/src/main/java/org/pgpainless/encryption_signing/EncryptionStream.java

@@ -15,6 +15,7 @@ import org.bouncycastle.bcpg.BCPGOutputStream;
 import org.bouncycastle.openpgp.PGPCompressedDataGenerator;
 import org.bouncycastle.openpgp.PGPEncryptedDataGenerator;
 import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPLiteralDataGenerator;
 import org.bouncycastle.openpgp.PGPSignature;
 import org.bouncycastle.openpgp.PGPSignatureGenerator;
 import org.bouncycastle.openpgp.operator.PGPDataEncryptorBuilder;
@@ -25,7 +26,6 @@ import org.pgpainless.implementation.ImplementationFactory;
 import org.pgpainless.key.SubkeyIdentifier;
 import org.pgpainless.util.ArmorUtils;
 import org.pgpainless.util.ArmoredOutputStreamFactory;
-import org.pgpainless.util.StreamGeneratorWrapper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,7 +56,7 @@ public final class EncryptionStream extends OutputStream {
     private OutputStream publicKeyEncryptedStream = null;
     private PGPCompressedDataGenerator compressedDataGenerator;
     private BCPGOutputStream basicCompressionStream;
-    private StreamGeneratorWrapper streamGeneratorWrapper;
+    private PGPLiteralDataGenerator literalDataGenerator;
     private OutputStream literalDataStream;
 
     EncryptionStream(@Nonnull OutputStream targetOutputStream,
@@ -164,8 +164,8 @@ public final class EncryptionStream extends OutputStream {
             return;
         }
 
-        streamGeneratorWrapper = StreamGeneratorWrapper.forStreamEncoding(options.getEncoding());
-        literalDataStream = streamGeneratorWrapper.open(outermostStream,
+        literalDataGenerator = new PGPLiteralDataGenerator();
+        literalDataStream = literalDataGenerator.open(outermostStream, options.getEncoding().getCode(),
                 options.getFileName(), options.getModificationDate(), new byte[BUFFER_SIZE]);
         outermostStream = literalDataStream;
 
@@ -226,8 +226,8 @@ public final class EncryptionStream extends OutputStream {
             literalDataStream.flush();
             literalDataStream.close();
         }
-        if (streamGeneratorWrapper != null) {
-            streamGeneratorWrapper.close();
+        if (literalDataGenerator != null) {
+            literalDataGenerator.close();
         }
 
         if (options.isCleartextSigned()) {

pgpainless-core/src/main/java/org/pgpainless/encryption_signing/ProducerOptions.java

@@ -231,8 +231,7 @@ public final class ProducerOptions {
      * @param encoding encoding
      * @return this
      *
-     * @deprecated this option will be removed in the near future, as values other than {@link StreamEncoding#BINARY}
-     * are causing issues. See https://github.com/pgpainless/pgpainless/issues/264 for details
+     * @deprecated options other than the default value of {@link StreamEncoding#BINARY} are discouraged.
      */
     @Deprecated
     public ProducerOptions setEncoding(@Nonnull StreamEncoding encoding) {

pgpainless-core/src/main/java/org/pgpainless/util/StreamGeneratorWrapper.java

@@ -1,91 +0,0 @@
-// SPDX-FileCopyrightText: 2021 Paul Schaub <vanitasvitae@fsfe.org>
-//
-// SPDX-License-Identifier: Apache-2.0
-
-package org.pgpainless.util;
-
-import java.io.IOException;
-import java.io.OutputStream;
-import java.util.Date;
-
-import javax.annotation.Nonnull;
-
-import org.bouncycastle.openpgp.PGPCanonicalizedDataGenerator;
-import org.bouncycastle.openpgp.PGPLiteralDataGenerator;
-import org.pgpainless.algorithm.StreamEncoding;
-
-/**
- * Literal Data can be encoded in different ways.
- * BINARY encoding leaves the data as is and is generated through the {@link PGPLiteralDataGenerator}.
- * However, if the data is encoded in TEXT or UTF8 encoding, we need to use the {@link PGPCanonicalizedDataGenerator}
- * instead.
- *
- * This wrapper class acts as a handle for both options and provides a unified interface for them.
- */
-public final class StreamGeneratorWrapper {
-
-    private final StreamEncoding encoding;
-    private final PGPLiteralDataGenerator literalDataGenerator;
-    private final PGPCanonicalizedDataGenerator canonicalizedDataGenerator;
-
-    /**
-     * Create a new instance for the given encoding.
-     *
-     * @param encoding stream encoding
-     * @return wrapper
-     */
-    public static StreamGeneratorWrapper forStreamEncoding(@Nonnull StreamEncoding encoding) {
-        if (encoding == StreamEncoding.BINARY) {
-            return new StreamGeneratorWrapper(encoding, new PGPLiteralDataGenerator());
-        } else {
-            return new StreamGeneratorWrapper(encoding, new PGPCanonicalizedDataGenerator());
-        }
-    }
-
-    private StreamGeneratorWrapper(@Nonnull StreamEncoding encoding, @Nonnull PGPLiteralDataGenerator literalDataGenerator) {
-        if (encoding != StreamEncoding.BINARY) {
-            throw new IllegalArgumentException("PGPLiteralDataGenerator can only be used with BINARY encoding.");
-        }
-        this.encoding = encoding;
-        this.literalDataGenerator = literalDataGenerator;
-        this.canonicalizedDataGenerator = null;
-    }
-
-    private StreamGeneratorWrapper(@Nonnull StreamEncoding encoding, @Nonnull PGPCanonicalizedDataGenerator canonicalizedDataGenerator) {
-        if (encoding != StreamEncoding.TEXT && encoding != StreamEncoding.UTF8) {
-            throw new IllegalArgumentException("PGPCanonicalizedDataGenerator can only be used with TEXT or UTF8 encoding.");
-        }
-        this.encoding = encoding;
-        this.canonicalizedDataGenerator = canonicalizedDataGenerator;
-        this.literalDataGenerator = null;
-    }
-
-    /**
-     * Open a new encoding stream.
-     *
-     * @param outputStream wrapped output stream
-     * @param filename file name
-     * @param modificationDate modification date
-     * @param buffer buffer
-     * @return encoding stream
-     */
-    public OutputStream open(OutputStream outputStream, String filename, Date modificationDate, byte[] buffer) throws IOException {
-        if (literalDataGenerator != null) {
-            return literalDataGenerator.open(outputStream, encoding.getCode(), filename, modificationDate, buffer);
-        } else {
-            return canonicalizedDataGenerator.open(outputStream, encoding.getCode(), filename, modificationDate, buffer);
-        }
-    }
-
-    /**
-     * Close all encoding streams opened by this generator wrapper.
-     */
-    public void close() throws IOException {
-        if (literalDataGenerator != null) {
-            literalDataGenerator.close();
-        }
-        if (canonicalizedDataGenerator != null) {
-            canonicalizedDataGenerator.close();
-        }
-    }
-}

pgpainless-core/src/test/java/investigations/CanonicalizedDataEncryptionTest.java

@@ -4,31 +4,46 @@
 
 package investigations;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 
+import org.bouncycastle.bcpg.ArmoredOutputStream;
+import org.bouncycastle.bcpg.CompressionAlgorithmTags;
+import org.bouncycastle.openpgp.PGPCompressedDataGenerator;
 import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPLiteralData;
+import org.bouncycastle.openpgp.PGPLiteralDataGenerator;
+import org.bouncycastle.openpgp.PGPOnePassSignature;
+import org.bouncycastle.openpgp.PGPPrivateKey;
 import org.bouncycastle.openpgp.PGPPublicKeyRing;
 import org.bouncycastle.openpgp.PGPSecretKeyRing;
+import org.bouncycastle.openpgp.PGPSignature;
+import org.bouncycastle.openpgp.PGPSignatureGenerator;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder;
 import org.bouncycastle.util.io.Streams;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.pgpainless.PGPainless;
 import org.pgpainless.algorithm.DocumentSignatureType;
+import org.pgpainless.algorithm.HashAlgorithm;
 import org.pgpainless.algorithm.StreamEncoding;
 import org.pgpainless.decryption_verification.ConsumerOptions;
 import org.pgpainless.decryption_verification.DecryptionStream;
 import org.pgpainless.decryption_verification.OpenPgpMetadata;
+import org.pgpainless.encryption_signing.CRLFGeneratorStream;
 import org.pgpainless.encryption_signing.EncryptionOptions;
 import org.pgpainless.encryption_signing.EncryptionStream;
 import org.pgpainless.encryption_signing.ProducerOptions;
 import org.pgpainless.encryption_signing.SigningOptions;
 import org.pgpainless.key.protection.SecretKeyRingProtector;
+import org.pgpainless.key.protection.UnlockSecretKey;
 
 public class CanonicalizedDataEncryptionTest {
 
@@ -134,7 +149,6 @@ public class CanonicalizedDataEncryptionTest {
     }
 
     @Test
-    @Disabled("Fails")
     public void textDataBinarySig() throws PGPException, IOException {
         String msg = encryptAndSign(message, DocumentSignatureType.BINARY_DOCUMENT, StreamEncoding.TEXT);
         OpenPgpMetadata metadata = decryptAndVerify(msg);
@@ -163,7 +177,6 @@ public class CanonicalizedDataEncryptionTest {
     }
 
     @Test
-    @Disabled("Fails")
     public void utf8DataBinarySig() throws PGPException, IOException {
         String msg = encryptAndSign(message, DocumentSignatureType.BINARY_DOCUMENT, StreamEncoding.UTF8);
         OpenPgpMetadata metadata = decryptAndVerify(msg);
@@ -228,4 +241,75 @@ public class CanonicalizedDataEncryptionTest {
 
         return decryptionStream.getResult();
     }
+
+    @Test
+    public void testManualSignWithAllCombinations() throws PGPException, IOException {
+        for (StreamEncoding streamEncoding : StreamEncoding.values()) {
+            for (DocumentSignatureType sigType : DocumentSignatureType.values()) {
+                manualSignAndVerify(sigType, streamEncoding);
+            }
+        }
+    }
+
+    public void manualSignAndVerify(DocumentSignatureType sigType, StreamEncoding streamEncoding)
+            throws IOException, PGPException {
+        PGPPrivateKey privateKey = UnlockSecretKey.unlockSecretKey(secretKeys.getSecretKey(), SecretKeyRingProtector.unprotectedKeys());
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        ArmoredOutputStream armorOut = new ArmoredOutputStream(out);
+
+        PGPCompressedDataGenerator compressor = new PGPCompressedDataGenerator(CompressionAlgorithmTags.ZLIB);
+        OutputStream compressedOut = compressor.open(armorOut);
+
+        PGPSignatureGenerator signer = new PGPSignatureGenerator(
+                new BcPGPContentSignerBuilder(
+                        secretKeys.getPublicKey().getAlgorithm(),
+                        HashAlgorithm.SHA256.getAlgorithmId()));
+        signer.init(sigType.getSignatureType().getCode(), privateKey);
+
+        PGPOnePassSignature ops = signer.generateOnePassVersion(false);
+        ops.encode(compressedOut);
+
+        PGPLiteralDataGenerator author = new PGPLiteralDataGenerator();
+        OutputStream literalOut = author.open(compressedOut, streamEncoding.getCode(), "", PGPLiteralData.NOW, new byte[4096]);
+
+        byte[] msg = message.getBytes(StandardCharsets.UTF_8);
+
+        ByteArrayOutputStream crlfed = new ByteArrayOutputStream();
+        CRLFGeneratorStream crlfOut = new CRLFGeneratorStream(crlfed, streamEncoding);
+        crlfOut.write(msg);
+        msg = crlfed.toByteArray();
+
+        for (byte b : msg) {
+            literalOut.write(b);
+            signer.update(b);
+        }
+
+        literalOut.close();
+        PGPSignature signature = signer.generate();
+
+        signature.encode(compressedOut);
+        compressor.close();
+
+        armorOut.close();
+
+        String ciphertext = out.toString();
+        // CHECKSTYLE:OFF
+        System.out.println(sigType + " " + streamEncoding);
+        System.out.println(ciphertext);
+        // CHECKSTYLE:ON
+
+        ByteArrayInputStream cipherIn = new ByteArrayInputStream(ciphertext.getBytes(StandardCharsets.UTF_8));
+        ByteArrayOutputStream decrypted = new ByteArrayOutputStream();
+        DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify()
+                        .onInputStream(cipherIn)
+                                .withOptions(new ConsumerOptions()
+                                        .addVerificationCert(publicKeys));
+
+        Streams.pipeAll(decryptionStream, decrypted);
+        decryptionStream.close();
+        OpenPgpMetadata metadata = decryptionStream.getResult();
+        assertTrue(metadata.isVerified(), "Not verified! Sig Type: " + sigType + " StreamEncoding: " + streamEncoding);
+
+        assertArrayEquals(msg, decrypted.toByteArray());
+    }
 }