1
0
Fork 0
mirror of https://github.com/pgpainless/pgpainless.git synced 2025-12-10 22:31:09 +01:00

Fully drain ArmoredInputStreams to verify CRC checksum.

Fixes #159 (for real this time)
This commit is contained in:
Paul Schaub 2021-07-27 15:09:59 +02:00
parent 17d0802392
commit 107e53c03e
Signed by: vanitasvitae
GPG key ID: 62BEE9264BF17311
10 changed files with 640 additions and 21 deletions

View file

@ -23,6 +23,7 @@ import java.util.logging.Logger;
import javax.annotation.Nonnull;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.util.io.Streams;
import org.pgpainless.PGPainless;
import org.pgpainless.signature.DetachedSignature;
import org.pgpainless.signature.SignatureChainValidator;
@ -41,12 +42,15 @@ public class DecryptionStream extends InputStream {
private final OpenPgpMetadata.Builder resultBuilder;
private boolean isClosed = false;
private List<IntegrityProtectedInputStream> integrityProtectedInputStreamList;
private final InputStream armorStream;
DecryptionStream(@Nonnull InputStream wrapped, @Nonnull OpenPgpMetadata.Builder resultBuilder,
List<IntegrityProtectedInputStream> integrityProtectedInputStreamList) {
List<IntegrityProtectedInputStream> integrityProtectedInputStreamList,
InputStream armorStream) {
this.inputStream = wrapped;
this.resultBuilder = resultBuilder;
this.integrityProtectedInputStreamList = integrityProtectedInputStreamList;
this.armorStream = armorStream;
}
@Override
@ -66,6 +70,9 @@ public class DecryptionStream extends InputStream {
@Override
public void close() throws IOException {
if (armorStream != null) {
Streams.drain(armorStream);
}
inputStream.close();
maybeVerifyDetachedSignatures();
for (IntegrityProtectedInputStream s : integrityProtectedInputStreamList) {

View file

@ -28,6 +28,7 @@ import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nonnull;
import org.bouncycastle.bcpg.ArmoredInputStream;
import org.bouncycastle.openpgp.PGPCompressedData;
import org.bouncycastle.openpgp.PGPEncryptedData;
import org.bouncycastle.openpgp.PGPEncryptedDataList;
@ -65,6 +66,7 @@ import org.pgpainless.key.info.KeyRingInfo;
import org.pgpainless.key.protection.UnlockSecretKey;
import org.pgpainless.signature.DetachedSignature;
import org.pgpainless.signature.OnePassSignature;
import org.pgpainless.util.CRCingArmoredInputStreamWrapper;
import org.pgpainless.util.IntegrityProtectedInputStream;
import org.pgpainless.util.Passphrase;
@ -86,6 +88,22 @@ public final class DecryptionStreamFactory {
public DecryptionStreamFactory(ConsumerOptions options) {
this.options = options;
for (PGPSignature signature : options.getDetachedSignatures()) {
PGPPublicKeyRing signingKeyRing = findSignatureVerificationKeyRing(signature.getKeyID());
if (signingKeyRing == null) {
continue;
}
PGPPublicKey signingKey = signingKeyRing.getPublicKey(signature.getKeyID());
SubkeyIdentifier signingKeyIdentifier = new SubkeyIdentifier(signingKeyRing, signingKey.getKeyID());
try {
signature.init(ImplementationFactory.getInstance().getPGPContentVerifierBuilderProvider(), signingKey);
resultBuilder.addDetachedSignature(
new DetachedSignature(signature, signingKeyRing, signingKeyIdentifier));
} catch (PGPException e) {
LOGGER.log(Level.INFO, "Cannot verify signature made by " + signingKeyIdentifier, e);
}
}
}
public static DecryptionStream create(@Nonnull InputStream inputStream,
@ -95,19 +113,11 @@ public final class DecryptionStreamFactory {
bufferedIn.mark(200);
DecryptionStreamFactory factory = new DecryptionStreamFactory(options);
for (PGPSignature signature : options.getDetachedSignatures()) {
PGPPublicKeyRing signingKeyRing = factory.findSignatureVerificationKeyRing(signature.getKeyID());
if (signingKeyRing == null) {
continue;
}
PGPPublicKey signingKey = signingKeyRing.getPublicKey(signature.getKeyID());
signature.init(ImplementationFactory.getInstance().getPGPContentVerifierBuilderProvider(), signingKey);
factory.resultBuilder.addDetachedSignature(
new DetachedSignature(signature, signingKeyRing, new SubkeyIdentifier(signingKeyRing, signature.getKeyID())));
}
InputStream decoderStream = PGPUtil.getDecoderStream(bufferedIn);
decoderStream = CRCingArmoredInputStreamWrapper.possiblyWrap(decoderStream);
PGPObjectFactory objectFactory = new PGPObjectFactory(
PGPUtil.getDecoderStream(bufferedIn), keyFingerprintCalculator);
decoderStream, keyFingerprintCalculator);
try {
// Parse OpenPGP message
@ -131,7 +141,8 @@ public final class DecryptionStreamFactory {
}
}
return new DecryptionStream(inputStream, factory.resultBuilder, factory.integrityProtectedStreams);
return new DecryptionStream(inputStream, factory.resultBuilder, factory.integrityProtectedStreams,
(decoderStream instanceof ArmoredInputStream) ? decoderStream : null);
}
private InputStream processPGPPackets(@Nonnull PGPObjectFactory objectFactory, int depth) throws IOException, PGPException {

View file

@ -15,6 +15,7 @@
*/
package org.pgpainless.key.parsing;
import java.io.BufferedInputStream;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
@ -24,6 +25,7 @@ import java.util.Iterator;
import java.util.List;
import javax.annotation.Nonnull;
import org.bouncycastle.bcpg.ArmoredInputStream;
import org.bouncycastle.bcpg.MarkerPacket;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPObjectFactory;
@ -35,6 +37,8 @@ import org.bouncycastle.openpgp.PGPUtil;
import org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator;
import org.pgpainless.implementation.ImplementationFactory;
import org.pgpainless.key.collection.PGPKeyRingCollection;
import org.pgpainless.util.ArmoredInputStreamFactory;
import org.pgpainless.util.StreamUtil;
public class KeyRingReader {
@ -157,8 +161,9 @@ public class KeyRingReader {
}
public static PGPSecretKeyRing readSecretKeyRing(@Nonnull InputStream inputStream) throws IOException {
InputStream decoderStream = getDecoderStream(inputStream);
PGPObjectFactory objectFactory = new PGPObjectFactory(
getDecoderStream(inputStream),
decoderStream,
ImplementationFactory.getInstance().getKeyFingerprintCalculator());
Object next;
@ -171,6 +176,7 @@ public class KeyRingReader {
continue;
}
if (next instanceof PGPSecretKeyRing) {
StreamUtil.drain(decoderStream);
return (PGPSecretKeyRing) next;
}
} while (true);
@ -231,6 +237,17 @@ public class KeyRingReader {
* @return BufferedInputStreamExt
*/
private static InputStream getDecoderStream(InputStream inputStream) throws IOException {
return PGPUtil.getDecoderStream(PGPUtil.getDecoderStream(inputStream));
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;
}
}

View file

@ -37,6 +37,7 @@ import org.pgpainless.PGPainless;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.implementation.ImplementationFactory;
import org.pgpainless.signature.SignatureChainValidator;
import org.pgpainless.util.ArmoredInputStreamFactory;
/**
* Processor for cleartext-signed messages.
@ -55,7 +56,7 @@ public class CleartextSignatureProcessor {
if (inputStream instanceof ArmoredInputStream) {
this.in = (ArmoredInputStream) inputStream;
} else {
this.in = new ArmoredInputStream(inputStream);
this.in = ArmoredInputStreamFactory.get(inputStream);
}
this.verificationKeys = verificationKeys;
this.multiPassStrategy = multiPassStrategy;

View file

@ -0,0 +1,36 @@
/*
* 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.util;
import java.io.IOException;
import java.io.InputStream;
import org.bouncycastle.bcpg.ArmoredInputStream;
public class ArmoredInputStreamFactory {
public static ArmoredInputStream get(InputStream inputStream) throws IOException {
if (inputStream instanceof CRCingArmoredInputStreamWrapper) {
return (ArmoredInputStream) inputStream;
}
if (inputStream instanceof ArmoredInputStream) {
return new CRCingArmoredInputStreamWrapper((ArmoredInputStream) inputStream);
}
ArmoredInputStream armorIn = new ArmoredInputStream(inputStream);
return new CRCingArmoredInputStreamWrapper(armorIn);
}
}

View file

@ -0,0 +1,158 @@
/*
* 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.util;
import java.io.IOException;
import java.io.InputStream;
import org.bouncycastle.bcpg.ArmoredInputStream;
public class CRCingArmoredInputStreamWrapper extends ArmoredInputStream {
private final ArmoredInputStream inputStream;
public CRCingArmoredInputStreamWrapper(ArmoredInputStream inputStream) throws IOException {
super(inputStream, false);
this.inputStream = inputStream;
}
public static InputStream possiblyWrap(InputStream inputStream) throws IOException {
if (inputStream instanceof CRCingArmoredInputStreamWrapper) {
return inputStream;
}
if (inputStream instanceof ArmoredInputStream) {
return new CRCingArmoredInputStreamWrapper((ArmoredInputStream) inputStream);
}
return inputStream;
}
@Override
public boolean isClearText() {
return inputStream.isClearText();
}
@Override
public boolean isEndOfStream() {
return inputStream.isEndOfStream();
}
@Override
public String getArmorHeaderLine() {
return inputStream.getArmorHeaderLine();
}
@Override
public String[] getArmorHeaders() {
return inputStream.getArmorHeaders();
}
@Override
public int read() throws IOException {
return inputStream.read();
}
@Override
public int read(byte[] b) throws IOException {
return read(b, 0, b.length);
}
/**
* Reads up to <code>len</code> bytes of data from the input stream into
* an array of bytes. An attempt is made to read as many as
* <code>len</code> bytes, but a smaller number may be read.
* The number of bytes actually read is returned as an integer.
*
* The first byte read is stored into element <code>b[off]</code>, the
* next one into <code>b[off+1]</code>, and so on. The number of bytes read
* is, at most, equal to <code>len</code>.
*
* NOTE: We need to override the custom behavior of Java's {@link InputStream#read(byte[], int, int)},
* as the upstream method silently swallows {@link IOException IOExceptions}.
* This would cause CRC checksum errors to go unnoticed.
*
* @see <a href="https://github.com/bcgit/bc-java/issues/998">Related BC bug report</a>
* @param b byte array
* @param off offset at which we start writing data to the array
* @param len number of bytes we write into the array
* @return total number of bytes read into the buffer
*
* @throws IOException if an exception happens AT ANY POINT
*/
@Override
public int read(byte[] b, int off, int len) throws IOException {
checkIndexSize(b.length, off, len);
if (len == 0) {
return 0;
}
int c = read();
if (c == -1) {
return -1;
}
b[off] = (byte) c;
int i = 1;
for (; i < len ; i++) {
c = read();
if (c == -1) {
break;
}
b[off + i] = (byte) c;
}
return i;
}
private void checkIndexSize(int size, int off, int len) {
if (off < 0 || len < 0) {
throw new IndexOutOfBoundsException("Offset and length cannot be negative.");
}
if (size < off + len) {
throw new IndexOutOfBoundsException("Invalid offset and length.");
}
}
@Override
public long skip(long n) throws IOException {
return inputStream.skip(n);
}
@Override
public int available() throws IOException {
return inputStream.available();
}
@Override
public void close() throws IOException {
inputStream.close();
}
@Override
public synchronized void mark(int readlimit) {
inputStream.mark(readlimit);
}
@Override
public synchronized void reset() throws IOException {
inputStream.reset();
}
@Override
public boolean markSupported() {
return inputStream.markSupported();
}
}

View file

@ -44,4 +44,17 @@ public class StreamUtil {
outputStream.write(i);
} while (true);
}
/**
* Drain an {@link InputStream} without calling {@link InputStream#read(byte[], int, int)}.
*
* @param inputStream input stream
* @throws IOException io exception
*/
public static void drain(InputStream inputStream) throws IOException {
int i;
do {
i = inputStream.read();
} while (i != -1);
}
}