From 944d79b009bd3d16ec1b774e5e9446956305427e Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Sun, 8 Aug 2021 15:58:12 +0200 Subject: [PATCH] Rearrange armored input stream workaround code --- .../pgpainless/key/parsing/KeyRingReader.java | 44 ++--------------- .../pgpainless/signature/SignatureUtils.java | 4 +- .../java/org/pgpainless/util/ArmorUtils.java | 31 ++++++++++++ ...singCRCChecksumInSignatureArmorIsOkay.java | 49 +++++++++++++++++++ 4 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 pgpainless-core/src/test/java/org/pgpainless/signature/MissingCRCChecksumInSignatureArmorIsOkay.java diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/parsing/KeyRingReader.java b/pgpainless-core/src/main/java/org/pgpainless/key/parsing/KeyRingReader.java index 63f411f6..0340e71c 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/parsing/KeyRingReader.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/parsing/KeyRingReader.java @@ -15,7 +15,6 @@ */ package org.pgpainless.key.parsing; -import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -25,7 +24,6 @@ import java.util.Iterator; import java.util.List; import javax.annotation.Nonnull; -import org.bouncycastle.bcpg.ArmoredInputStream; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPMarker; import org.bouncycastle.openpgp.PGPObjectFactory; @@ -33,12 +31,10 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPPublicKeyRingCollection; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; -import org.bouncycastle.openpgp.PGPUtil; -import org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator; import org.bouncycastle.util.io.Streams; import org.pgpainless.implementation.ImplementationFactory; import org.pgpainless.key.collection.PGPKeyRingCollection; -import org.pgpainless.util.ArmoredInputStreamFactory; +import org.pgpainless.util.ArmorUtils; public class KeyRingReader { @@ -109,7 +105,7 @@ public class KeyRingReader { public static PGPPublicKeyRing readPublicKeyRing(@Nonnull InputStream inputStream) throws IOException { PGPObjectFactory objectFactory = new PGPObjectFactory( - getDecoderStream(inputStream), + ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); Object next; do { @@ -131,7 +127,7 @@ public class KeyRingReader { public static PGPPublicKeyRingCollection readPublicKeyRingCollection(@Nonnull InputStream inputStream) throws IOException, PGPException { PGPObjectFactory objectFactory = new PGPObjectFactory( - getDecoderStream(inputStream), + ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); List rings = new ArrayList<>(); @@ -161,7 +157,7 @@ public class KeyRingReader { } public static PGPSecretKeyRing readSecretKeyRing(@Nonnull InputStream inputStream) throws IOException { - InputStream decoderStream = getDecoderStream(inputStream); + InputStream decoderStream = ArmorUtils.getDecoderStream(inputStream); PGPObjectFactory objectFactory = new PGPObjectFactory( decoderStream, ImplementationFactory.getInstance().getKeyFingerprintCalculator()); @@ -187,7 +183,7 @@ public class KeyRingReader { public static PGPSecretKeyRingCollection readSecretKeyRingCollection(@Nonnull InputStream inputStream) throws IOException, PGPException { PGPObjectFactory objectFactory = new PGPObjectFactory( - getDecoderStream(inputStream), + ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); List rings = new ArrayList<>(); @@ -220,34 +216,4 @@ public class KeyRingReader { throws IOException, PGPException { return new PGPKeyRingCollection(inputStream, isSilent); } - - /** - * Hacky workaround for #96. - * For {@link PGPPublicKeyRingCollection#PGPPublicKeyRingCollection(InputStream, KeyFingerPrintCalculator)} - * or {@link PGPSecretKeyRingCollection#PGPSecretKeyRingCollection(InputStream, KeyFingerPrintCalculator)} - * to read all PGPKeyRings properly, we apparently have to make sure that the {@link InputStream} that is given - * as constructor argument is a {@link PGPUtil.BufferedInputStreamExt}. - * Since {@link PGPUtil#getDecoderStream(InputStream)} will return an {@link org.bouncycastle.bcpg.ArmoredInputStream} - * if the underlying input stream contains armored data, we have to nest two method calls to make sure that the - * end-result is a {@link PGPUtil.BufferedInputStreamExt}. - * - * This is a hacky solution. - * - * @param inputStream input stream - * @return BufferedInputStreamExt - */ - private static InputStream getDecoderStream(InputStream inputStream) throws IOException { - InputStream decoderStream = PGPUtil.getDecoderStream(inputStream); - // Data is not armored -> return - if (decoderStream instanceof BufferedInputStream) { - return decoderStream; - } - // Wrap armored input stream with fix for #159 - if (decoderStream instanceof ArmoredInputStream) { - decoderStream = ArmoredInputStreamFactory.get(decoderStream); - } - - decoderStream = PGPUtil.getDecoderStream(decoderStream); - return decoderStream; - } } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureUtils.java b/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureUtils.java index 428abfa2..894caa3c 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureUtils.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureUtils.java @@ -36,7 +36,6 @@ import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPSignatureGenerator; import org.bouncycastle.openpgp.PGPSignatureList; -import org.bouncycastle.openpgp.PGPUtil; import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder; import org.bouncycastle.util.encoders.Hex; import org.pgpainless.PGPainless; @@ -48,6 +47,7 @@ import org.pgpainless.key.util.OpenPgpKeyAttributeUtil; import org.pgpainless.key.util.RevocationAttributes; import org.pgpainless.policy.Policy; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; +import org.pgpainless.util.ArmorUtils; /** * Utility methods related to signatures. @@ -196,7 +196,7 @@ public class SignatureUtils { public static List readSignatures(InputStream inputStream) throws IOException, PGPException { List signatures = new ArrayList<>(); - InputStream pgpIn = PGPUtil.getDecoderStream(inputStream); + InputStream pgpIn = ArmorUtils.getDecoderStream(inputStream); PGPObjectFactory objectFactory = new PGPObjectFactory( pgpIn, ImplementationFactory.getInstance().getKeyFingerprintCalculator()); diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java b/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java index 10855877..5c9448e1 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java @@ -15,6 +15,7 @@ */ package org.pgpainless.util; +import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -32,6 +33,8 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPPublicKeyRingCollection; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; +import org.bouncycastle.openpgp.PGPUtil; +import org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator; import org.bouncycastle.util.io.Streams; import org.pgpainless.algorithm.HashAlgorithm; import org.pgpainless.key.OpenPgpV4Fingerprint; @@ -204,4 +207,32 @@ public class ArmorUtils { } return values; } + + /** + * Hacky workaround for #96. + * For {@link PGPPublicKeyRingCollection#PGPPublicKeyRingCollection(InputStream, KeyFingerPrintCalculator)} + * or {@link PGPSecretKeyRingCollection#PGPSecretKeyRingCollection(InputStream, KeyFingerPrintCalculator)} + * to read all PGPKeyRings properly, we apparently have to make sure that the {@link InputStream} that is given + * as constructor argument is a {@link PGPUtil.BufferedInputStreamExt}. + * Since {@link PGPUtil#getDecoderStream(InputStream)} will return an {@link org.bouncycastle.bcpg.ArmoredInputStream} + * if the underlying input stream contains armored data, we have to nest two method calls to make sure that the + * end-result is a {@link PGPUtil.BufferedInputStreamExt}. + * + * This is a hacky solution. + * + * @param inputStream input stream + * @return BufferedInputStreamExt + */ + public static InputStream getDecoderStream(InputStream inputStream) throws IOException { + InputStream decoderStream = PGPUtil.getDecoderStream(inputStream); + // Data is not armored -> return + if (decoderStream instanceof BufferedInputStream) { + return decoderStream; + } + // Wrap armored input stream with fix for #159 + decoderStream = CRCingArmoredInputStreamWrapper.possiblyWrap(decoderStream); + + decoderStream = PGPUtil.getDecoderStream(decoderStream); + return decoderStream; + } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/signature/MissingCRCChecksumInSignatureArmorIsOkay.java b/pgpainless-core/src/test/java/org/pgpainless/signature/MissingCRCChecksumInSignatureArmorIsOkay.java new file mode 100644 index 00000000..08d1fa44 --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/signature/MissingCRCChecksumInSignatureArmorIsOkay.java @@ -0,0 +1,49 @@ +/* + * Copyright 2021 Paul Schaub. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pgpainless.signature; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.util.List; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSignature; +import org.junit.jupiter.api.Test; + +public class MissingCRCChecksumInSignatureArmorIsOkay { + + public static final String ARMORED_SIGNATURE_WITH_MISSING_CRC_SUM = "-----BEGIN PGP SIGNATURE-----\n" + + "\n" + + "wsE7BAABCgBvBYJgyf3FCRD7/MgqAV5zMEcUAAAAAAAeACBzYWx0QG5vdGF0aW9u\n" + + "cy5zZXF1b2lhLXBncC5vcmdbH+E3jkMYSiFZOF5cHIUCy8UjqEvHbrCCxxhnEu1J\n" + + "ThYhBNGmbhojsYLJmA94jPv8yCoBXnMwAACzOAv/QAmXX9mPJ4Xtk1SNKPH11izO\n" + + "G8OK+dKL46O7AQGJFjdwiA8SdyFatVzUUNHcyi0HJ2iNes5DPObxDweqy9MijHkx\n" + + "U4RotWUwdhGoTWqAj3cipDXJZi4MD46qi8AkmmT1xGk3GuPH/ymgbefMIZymczKw\n" + + "1YjDoq0AFVCeBWekCsVjZsUYBamgG0WNKEKXk3bNHObaUAqpJZFKvZKslZByyuOm\n" + + "nq5BlscmalWTxhDPNZDPigFeoa+MI72ckquD9cJG3P4WHaWos0EfkWDwIRhB4888\n" + + "5jB4moQr6dDfELbtEqcBj9CecrXmPqv18qmoIgR9tAeeJsyqIC5TS9j6iDoYJ9bJ\n" + + "4H4OcfJtTtXMSMPppH/hEOS88R/F2m0Szc4leyHtXSZXZ2I8RJLG4u+2RZlh+vBh\n" + + "sakyriei09Kz3Gdk6deDCO0uCrYQA7GQ9xFMzrfay1B0Lr7pAe8BTg6xuauwfGZh\n" + + "wDz6hyIxJ3IhAln5GX7NGsXu3eoEjMfZZ29rY7BC\n" + + "-----END PGP SIGNATURE-----"; + + @Test + public void assertMissingCRCSumInSignatureArmorIsOkay() throws PGPException, IOException { + List signatureList = SignatureUtils.readSignatures(ARMORED_SIGNATURE_WITH_MISSING_CRC_SUM); + assertEquals(1, signatureList.size()); + } +}