Skip to content

Commit

Permalink
Fix double verification error neuhalje#66
Browse files Browse the repository at this point in the history
Nesting inputstreams can result in a child stream reaching its end twice.
For the MDCValidatingInputStream this results in verifying the MDC twice.
PGPEncryptedData#verify doesn't handle being called twice.

Fix with a simple boolean flag to prevent double verification.
  • Loading branch information
homps committed Feb 3, 2022
1 parent 3523ca7 commit 8c98ecf
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ final class MDCValidatingInputStream extends FilterInputStream {
private static final org.slf4j.Logger LOGGER = org.slf4j.LoggerFactory
.getLogger(MDCValidatingInputStream.class);

private final PGPPublicKeyEncryptedData pbe;

private boolean mdcChecked = false;

/**
* Creates a <code>MDCValidatingInputStream</code> by assigning the argument
* <code>inputStream</code> to the field <code>this.inputStream</code> and <code>pbe</code> to <code>this.pbe</code> so as to remember it for
Expand All @@ -21,9 +25,6 @@ final class MDCValidatingInputStream extends FilterInputStream {
* @param inputStream the underlying input stream
* @param pbe the pgp public key encrypted data to verify message integrity
*/

private final PGPPublicKeyEncryptedData pbe;

MDCValidatingInputStream(InputStream inputStream, PGPPublicKeyEncryptedData pbe) {
super(inputStream);
this.pbe = pbe;
Expand Down Expand Up @@ -61,6 +62,11 @@ public int read(@Nonnull byte[] b, int off, int len) throws IOException {
* @throws IOException Error while reading input stream or if MDC fails
*/
private void validateMDC() throws IOException {
if (mdcChecked) {
return;
}
mdcChecked = true;

try {
if (pbe.isIntegrityProtected()) {
if (!pbe.verify()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
import static org.junit.Assume.assumeNotNull;
import static org.mockito.Mockito.mock;

import java.io.BufferedInputStream;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;

import name.neuhalfen.projects.crypto.bouncycastle.openpgp.keys.keyrings.KeyringConfig;
import name.neuhalfen.projects.crypto.bouncycastle.openpgp.testtooling.Configs;
import name.neuhalfen.projects.crypto.bouncycastle.openpgp.testtooling.ExampleMessages;
Expand Down Expand Up @@ -144,6 +145,31 @@ public void decryptNoSignatureValidation_withUnsignedData_works() throws Excepti
}
}

@Test()
public void decryptNoSignatureValidation_withWrapperStream_works() throws Exception {

try (InputStream ciphertext = new ByteArrayInputStream(
ExampleMessages.IMPORTANT_QUOTE_NOT_SIGNED_NOT_COMPRESSED.getBytes(
"US-ASCII"))) {
final InputStream plaintextStream = BouncyGPG.decryptAndVerifyStream()
.withConfig(Configs.keyringConfigFromResourceForRecipient())
.andIgnoreSignatures()
.fromEncryptedInputStream(ciphertext);

final InputStream wrapperStream = new InputStream() {
@Override
public int read() throws IOException {
return plaintextStream.read();
}
};

final String plainText = inputStreamToText(wrapperStream);

assertThat(plainText, equalTo(ExampleMessages.IMPORTANT_QUOTE_TEXT));
plaintextStream.close();
}
}

@Test(expected = IOException.class)
public void decryptAndValidateSignature_withUnsignedData_throws() throws Exception {

Expand Down

0 comments on commit 8c98ecf

Please sign in to comment.