From 82cbe467f25706948cdeba0a5e6f6094ce9f360a Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 6 Dec 2021 17:11:42 +0100 Subject: [PATCH] Introduce iteration limit to prevent resource exhaustion when reading keys --- .../pgpainless/key/parsing/KeyRingReader.java | 81 ++++++++++++++++-- .../key/parsing/KeyRingReaderTest.java | 85 +++++++++++++++++++ 2 files changed, 159 insertions(+), 7 deletions(-) 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 585d3eb3..5dcf0bd4 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 @@ -27,6 +27,8 @@ import org.pgpainless.util.ArmorUtils; public class KeyRingReader { + public static final int MAX_ITERATIONS = 10000; + public static final Charset UTF8 = Charset.forName("UTF-8"); public PGPPublicKeyRing publicKeyRing(@Nonnull InputStream inputStream) throws IOException { @@ -93,9 +95,23 @@ public class KeyRingReader { } public static PGPPublicKeyRing readPublicKeyRing(@Nonnull InputStream inputStream) throws IOException { + return readPublicKeyRing(inputStream, MAX_ITERATIONS); + } + + /** + * Read a public key ring from the provided {@link InputStream}. + * If more than maxIterations PGP packets are encountered before a {@link PGPPublicKeyRing} is read, + * an {@link IOException} is thrown. + * + * @param inputStream input stream + * @param maxIterations max iterations before abort + * @return public key ring + */ + public static PGPPublicKeyRing readPublicKeyRing(@Nonnull InputStream inputStream, int maxIterations) throws IOException { PGPObjectFactory objectFactory = new PGPObjectFactory( ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); + int i = 0; Object next; do { next = objectFactory.nextObject(); @@ -108,17 +124,34 @@ public class KeyRingReader { if (next instanceof PGPPublicKeyRing) { return (PGPPublicKeyRing) next; } - } while (true); + } while (++i < maxIterations); + + throw new IOException("Loop exceeded max iteration count."); } public static PGPPublicKeyRingCollection readPublicKeyRingCollection(@Nonnull InputStream inputStream) throws IOException, PGPException { + return readPublicKeyRingCollection(inputStream, MAX_ITERATIONS); + } + + /** + * Read a public key ring collection from the provided {@link InputStream}. + * If more than maxIterations PGP packets are encountered before the stream is exhausted, + * an {@link IOException} is thrown. + * + * @param inputStream input stream + * @param maxIterations max iterations before abort + * @return public key ring collection + * @throws IOException + */ + public static PGPPublicKeyRingCollection readPublicKeyRingCollection(@Nonnull InputStream inputStream, int maxIterations) + throws IOException, PGPException { PGPObjectFactory objectFactory = new PGPObjectFactory( ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); List rings = new ArrayList<>(); - + int i = 0; Object next; do { next = objectFactory.nextObject(); @@ -138,15 +171,30 @@ public class KeyRingReader { rings.add(iterator.next()); } } - } while (true); + } while (++i < maxIterations); + + throw new IOException("Loop exceeded max iteration count."); } public static PGPSecretKeyRing readSecretKeyRing(@Nonnull InputStream inputStream) throws IOException { + return readSecretKeyRing(inputStream, MAX_ITERATIONS); + } + + /** + * Read a secret key ring from the provided {@link InputStream}. + * If more than maxIterations PGP packets are encountered before a {@link PGPSecretKeyRing} is read, + * an {@link IOException} is thrown. + * + * @param inputStream input stream + * @param maxIterations max iterations before abort + * @return public key ring + */ + public static PGPSecretKeyRing readSecretKeyRing(@Nonnull InputStream inputStream, int maxIterations) throws IOException { InputStream decoderStream = ArmorUtils.getDecoderStream(inputStream); PGPObjectFactory objectFactory = new PGPObjectFactory( decoderStream, ImplementationFactory.getInstance().getKeyFingerprintCalculator()); - + int i = 0; Object next; do { next = objectFactory.nextObject(); @@ -160,17 +208,34 @@ public class KeyRingReader { Streams.drain(decoderStream); return (PGPSecretKeyRing) next; } - } while (true); + } while (++i < maxIterations); + + throw new IOException("Loop exceeded max iteration count."); } public static PGPSecretKeyRingCollection readSecretKeyRingCollection(@Nonnull InputStream inputStream) throws IOException, PGPException { + return readSecretKeyRingCollection(inputStream, MAX_ITERATIONS); + } + + /** + * Read a secret key ring collection from the provided {@link InputStream}. + * If more than maxIterations PGP packets are encountered before the stream is exhausted, + * an {@link IOException} is thrown. + * + * @param inputStream input stream + * @param maxIterations max iterations before abort + * @return secret key ring collection + */ + public static PGPSecretKeyRingCollection readSecretKeyRingCollection(@Nonnull InputStream inputStream, + int maxIterations) + throws IOException, PGPException { PGPObjectFactory objectFactory = new PGPObjectFactory( ArmorUtils.getDecoderStream(inputStream), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); List rings = new ArrayList<>(); - + int i = 0; Object next; do { next = objectFactory.nextObject(); @@ -190,7 +255,9 @@ public class KeyRingReader { rings.add(iterator.next()); } } - } while (true); + } while (++i < maxIterations); + + throw new IOException("Loop exceeded max iteration count."); } public static PGPKeyRingCollection readKeyRingCollection(@Nonnull InputStream inputStream, boolean isSilent) diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/parsing/KeyRingReaderTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/parsing/KeyRingReaderTest.java index b02f5beb..e281091d 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/parsing/KeyRingReaderTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/parsing/KeyRingReaderTest.java @@ -5,6 +5,7 @@ package org.pgpainless.key.parsing; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.ByteArrayInputStream; @@ -475,4 +476,88 @@ class KeyRingReaderTest { assertTrue(secretKeys.contains(alice.getSecretKey().getKeyID())); assertTrue(secretKeys.contains(bob.getSecretKey().getKeyID())); } + + @Test + public void testReadingSecretKeysExceedsIterationLimit() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing alice = PGPainless.generateKeyRing().modernKeyRing("alice@pgpainless.org", null); + MarkerPacket marker = TestUtils.getMarkerPacket(); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ArmoredOutputStream armor = ArmoredOutputStreamFactory.get(bytes); + BCPGOutputStream outputStream = new BCPGOutputStream(armor); + + for (int i = 0; i < 600; i++) { + marker.encode(outputStream); + } + alice.encode(outputStream); + + assertThrows(IOException.class, () -> + KeyRingReader.readSecretKeyRing(new ByteArrayInputStream(bytes.toByteArray()), 512)); + } + + @Test + public void testReadingSecretKeyCollectionExceedsIterationLimit() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing alice = PGPainless.generateKeyRing().modernKeyRing("alice@pgpainless.org", null); + PGPSecretKeyRing bob = PGPainless.generateKeyRing().modernKeyRing("bob@pgpainless.org", null); + MarkerPacket marker = TestUtils.getMarkerPacket(); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ArmoredOutputStream armor = ArmoredOutputStreamFactory.get(bytes); + BCPGOutputStream outputStream = new BCPGOutputStream(armor); + + for (int i = 0; i < 600; i++) { + marker.encode(outputStream); + } + alice.encode(outputStream); + bob.encode(outputStream); + + assertThrows(IOException.class, () -> + KeyRingReader.readSecretKeyRingCollection(new ByteArrayInputStream(bytes.toByteArray()), 512)); + } + + + @Test + public void testReadingPublicKeysExceedsIterationLimit() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("alice@pgpainless.org", null); + PGPPublicKeyRing alice = PGPainless.extractCertificate(secretKeys); + MarkerPacket marker = TestUtils.getMarkerPacket(); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ArmoredOutputStream armor = ArmoredOutputStreamFactory.get(bytes); + BCPGOutputStream outputStream = new BCPGOutputStream(armor); + + for (int i = 0; i < 600; i++) { + marker.encode(outputStream); + } + alice.encode(outputStream); + + assertThrows(IOException.class, () -> + KeyRingReader.readPublicKeyRing(new ByteArrayInputStream(bytes.toByteArray()), 512)); + } + + @Test + public void testReadingPublicKeyCollectionExceedsIterationLimit() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing sec1 = PGPainless.generateKeyRing().modernKeyRing("alice@pgpainless.org", null); + PGPSecretKeyRing sec2 = PGPainless.generateKeyRing().modernKeyRing("bob@pgpainless.org", null); + PGPPublicKeyRing alice = PGPainless.extractCertificate(sec1); + PGPPublicKeyRing bob = PGPainless.extractCertificate(sec2); + MarkerPacket marker = TestUtils.getMarkerPacket(); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ArmoredOutputStream armor = ArmoredOutputStreamFactory.get(bytes); + BCPGOutputStream outputStream = new BCPGOutputStream(armor); + + for (int i = 0; i < 600; i++) { + marker.encode(outputStream); + } + alice.encode(outputStream); + bob.encode(outputStream); + + assertThrows(IOException.class, () -> + KeyRingReader.readPublicKeyRingCollection(new ByteArrayInputStream(bytes.toByteArray()), 512)); + } }