OpenPGP decrypt releases unverified plaintext before the integrity check; DoubleBufferedInputStream not wired in

[Arpan0995]

Summary

The high-level OpenPGP decryption pipeline (OpenPGPMessageProcessor) returns plaintext to a streaming caller as bytes are read, and only runs the SEIPD v1 (MDC) integrity check when the stream reaches end of data. A caller that consumes the stream incrementally therefore receives, and can act on, plaintext bytes before the integrity check has run.

The library already contains a mitigation built for exactly this: org.bouncycastle.openpgp.api.DoubleBufferedInputStream. Its javadoc states it withholds pending data "to minimize the risk of emitting unauthenticated plaintext", and it has a unit test. It is not referenced by any class in the main source tree, so the decrypt pipeline never uses it. This is a wiring/hardening request: the mitigation is implemented and tested, it is just not connected.

This is not a bypass of the final verify. The integrity check still runs and still fails closed at end of stream (the reproducer below ends with an IOException). The gap is the window during which unauthenticated plaintext has already been handed to the caller.

Environment

• bcpg / bcprov / bcutil 1.86.0.20700 (current 1.86 beta), main at commit acd2178
• JDK 27

Steps to reproduce

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;

import org.bouncycastle.bcpg.CompressionAlgorithmTags;
import org.bouncycastle.openpgp.api.OpenPGPApi;
import org.bouncycastle.openpgp.api.OpenPGPCertificate;
import org.bouncycastle.openpgp.api.OpenPGPKey;
import org.bouncycastle.openpgp.api.OpenPGPMessageGenerator;
import org.bouncycastle.openpgp.api.OpenPGPMessageInputStream;
import org.bouncycastle.openpgp.api.OpenPGPMessageProcessor;
import org.bouncycastle.openpgp.api.OpenPGPPolicy;
import org.bouncycastle.openpgp.api.bc.BcOpenPGPApi;

public class ReleaseBeforeVerify
{
    // Alice's v4 sample key and certificate (draft-bre-openpgp-samples-01).
    static final String ALICE_KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
        "Comment: Alice's OpenPGP Transferable Secret Key\n" +
        "\n" +
        "lFgEXEcE6RYJKwYBBAHaRw8BAQdArjWwk3FAqyiFbFBKT4TzXcVBqPTB3gmzlC/U\n" +
        "b7O1u10AAP9XBeW6lzGOLx7zHH9AsUDUTb2pggYGMzd0P3ulJ2AfvQ4RtCZBbGlj\n" +
        "ZSBMb3ZlbGFjZSA8YWxpY2VAb3BlbnBncC5leGFtcGxlPoiQBBMWCAA4AhsDBQsJ\n" +
        "CAcCBhUKCQgLAgQWAgMBAh4BAheAFiEE64W7X6M6deFelE5j8jFVDE9H444FAl2l\n" +
        "nzoACgkQ8jFVDE9H447pKwD6A5xwUqIDprBzrHfahrImaYEZzncqb25vkLV2arYf\n" +
        "a78A/R3AwtLQvjxwLDuzk4dUtUwvUYibL2sAHwj2kGaHnfICnF0EXEcE6RIKKwYB\n" +
        "BAGXVQEFAQEHQEL/BiGtq0k84Km1wqQw2DIikVYrQrMttN8d7BPfnr4iAwEIBwAA\n" +
        "/3/xFPG6U17rhTuq+07gmEvaFYKfxRB6sgAYiW6TMTpQEK6IeAQYFggAIBYhBOuF\n" +
        "u1+jOnXhXpROY/IxVQxPR+OOBQJcRwTpAhsMAAoJEPIxVQxPR+OOWdABAMUdSzpM\n" +
        "hzGs1O0RkWNQWbUzQ8nUOeD9wNbjE3zR+yfRAQDbYqvtWQKN4AQLTxVJN5X5AWyb\n" +
        "Pnn+We1aTBhaGa86AQ==\n" +
        "=n8OM\n" +
        "-----END PGP PRIVATE KEY BLOCK-----";

    static final String ALICE_CERT = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" +
        "Comment: Alice's OpenPGP certificate\n" +
        "\n" +
        "mDMEXEcE6RYJKwYBBAHaRw8BAQdArjWwk3FAqyiFbFBKT4TzXcVBqPTB3gmzlC/U\n" +
        "b7O1u120JkFsaWNlIExvdmVsYWNlIDxhbGljZUBvcGVucGdwLmV4YW1wbGU+iJAE\n" +
        "ExYIADgCGwMFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AWIQTrhbtfozp14V6UTmPy\n" +
        "MVUMT0fjjgUCXaWfOgAKCRDyMVUMT0fjjukrAPoDnHBSogOmsHOsd9qGsiZpgRnO\n" +
        "dypvbm+QtXZqth9rvwD9HcDC0tC+PHAsO7OTh1S1TC9RiJsvawAfCPaQZoed8gK4\n" +
        "OARcRwTpEgorBgEEAZdVAQUBAQdAQv8GIa2rSTzgqbXCpDDYMiKRVitCsy203x3s\n" +
        "E9+eviIDAQgHiHgEGBYIACAWIQTrhbtfozp14V6UTmPyMVUMT0fjjgUCXEcE6QIb\n" +
        "DAAKCRDyMVUMT0fjjlnQAQDFHUs6TIcxrNTtEZFjUFm1M0PJ1Dng/cDW4xN80fsn\n" +
        "0QEA22Kr7VkCjeAEC08VSTeV+QFsmz55/lntWkwYWhmvOgE=\n" +
        "=iIGO\n" +
        "-----END PGP PUBLIC KEY BLOCK-----";

    public static void main(String[] args)
        throws Exception
    {
        OpenPGPApi api = new BcOpenPGPApi();
        OpenPGPKey aliceKey = api.readKeyOrCertificate().parseKey(ALICE_KEY);
        OpenPGPCertificate aliceCert = api.readKeyOrCertificate().parseCertificate(ALICE_CERT);

        // A recognizable 500-byte plaintext.
        byte[] plaintext = new byte[500];
        for (int i = 0; i < plaintext.length; i++)
        {
            plaintext[i] = (byte)('A' + (i % 26));
        }

        // Encrypt to Alice, no compression, no padding, unarmored, no signature.
        // A v4 recipient key yields a SEIPD v1 (MDC) message.
        OpenPGPMessageGenerator gen = api.signAndOrEncryptMessage()
            .addEncryptionCertificate(aliceCert)
            .setArmored(false)
            .setAllowPadding(false)
            .setCompressionNegotiator(new OpenPGPMessageGenerator.CompressionNegotiator()
            {
                public int negotiateCompression(OpenPGPMessageGenerator g, OpenPGPPolicy p)
                {
                    return CompressionAlgorithmTags.UNCOMPRESSED;
                }
            });
        ByteArrayOutputStream cipherBuf = new ByteArrayOutputStream();
        OutputStream msgOut = gen.open(cipherBuf);
        msgOut.write(plaintext);
        msgOut.close();
        byte[] message = cipherBuf.toByteArray();

        // Flip the very last ciphertext byte. In CFB this corrupts only the final
        // plaintext byte, which is the last byte of the SHA-1 MDC hash. The literal
        // plaintext ahead of it decrypts unchanged; only the MDC check will fail.
        byte[] tampered = message.clone();
        tampered[tampered.length - 1] ^= 0x01;

        // Decrypt through the high-level pipeline, reading one byte at a time.
        OpenPGPMessageProcessor processor = api.decryptAndOrVerifyMessage()
            .addDecryptionKey(aliceKey);
        OpenPGPMessageInputStream in = processor.process(new ByteArrayInputStream(tampered));

        ByteArrayOutputStream delivered = new ByteArrayOutputStream();
        boolean integrityFailure = false;
        String failureMessage = null;
        try
        {
            int b;
            while ((b = in.read()) != -1)
            {
                delivered.write(b);
            }
            in.close();
        }
        catch (IOException e)
        {
            integrityFailure = true;
            failureMessage = e.getMessage();
        }

        byte[] out = delivered.toByteArray();
        boolean prefixMatches = out.length == plaintext.length
            && java.util.Arrays.equals(out, plaintext);

        System.out.println("SEIPD v1 (MDC) message, last MDC byte flipped.");
        System.out.println("Plaintext bytes emitted to the caller before any error : " + out.length);
        System.out.println("Emitted bytes equal the original 500-byte plaintext     : " + prefixMatches);
        System.out.println("Integrity failure surfaced at end of stream             : " + integrityFailure);
        System.out.println("Failure message                                         : " + failureMessage);
        System.out.println("Decrypted-layer stream (verify runs on close only)      : "
            + "org.bouncycastle.openpgp.IntegrityProtectedInputStream");
        System.out.println();
        System.out.println("The caller received and could act on all 500 unauthenticated");
        System.out.println("plaintext bytes before the integrity check ran.");
    }
}

Actual behaviour

SEIPD v1 (MDC) message, last MDC byte flipped.
Plaintext bytes emitted to the caller before any error : 500
Emitted bytes equal the original 500-byte plaintext     : true
Integrity failure surfaced at end of stream             : true
Failure message                                         : Malformed integrity protected data.
Decrypted-layer stream (verify runs on close only)      : org.bouncycastle.openpgp.IntegrityProtectedInputStream

The caller received and could act on all 500 unauthenticated
plaintext bytes before the integrity check ran.

All 500 plaintext bytes are handed to the caller and match the original message. The integrity failure is only raised at the end of the stream, after the caller has already received every plaintext byte.

Expected behaviour

For a streaming decrypt, the pipeline should avoid releasing plaintext that has not yet been covered by the integrity check, or at least offer an easy opt-in that does so. The library already contains the tool for this. DoubleBufferedInputStream withholds a trailing window of data until the underlying stream completes without error, which keeps the last bytes back until the MDC/AEAD check has run. A caller reading incrementally would then not observe the tail plaintext unless verification succeeded.

Root cause

OpenPGPMessageProcessor.decrypt(...) wraps the raw decrypted stream in IntegrityProtectedInputStream and returns that directly (OpenPGPMessageProcessor.java:306, :338, :386, :428, :463, line numbers as of acd2178). IntegrityProtectedInputStream passes bytes straight through on read() (read methods at IntegrityProtectedInputStream.java:27-61) and only calls esk.verify() inside close(), at IntegrityProtectedInputStream.java:82, which read() triggers when it reaches EOF (line numbers as of acd2178). So every plaintext byte is delivered before verification.

DoubleBufferedInputStream exists for this purpose. Its javadoc says it withholds pending data "to minimize the risk of emitting unauthenticated plaintext", with a configurable minimum withhold size (DoubleBufferedInputStream.java:6-19, line numbers as of acd2178). It has a unit test (DoubleBufferedInputStreamTest). A search of the main source tree finds no reference to it from any other class, so it is not connected to the decrypt path, and there is no policy or flag that would connect it.

Impact

Callers of the high-level API that process the decrypted stream incrementally (rather than buffering the whole message and only trusting it after a clean close()) can act on plaintext that later turns out to have failed its integrity check. This is the class of concern behind the OpenPGP guidance to not release unauthenticated plaintext. The final verify still runs and fails closed, so this is not a silent integrity bypass, but the release-before-verify window is real and the built-in mitigation is currently dead code. Severity is moderate, as a hardening item.

Suggested direction

Wire DoubleBufferedInputStream into the decrypt pipeline for the integrity-protected paths, either by default or behind an OpenPGPPolicy option, so that streaming consumers do not receive the trailing plaintext until the integrity check has completed. If incremental release is intended to remain the default, documenting on OpenPGPMessageInputStream that callers must not act on any data until close() returns without error would at least make the contract explicit.

This program is complete and self-contained; it needs the bcpg, bcprov and bcutil 1.86.0.20700 jars on the classpath.

[dghgit]

Thanks for the report, now fixed on https://downloads.bouncycastle.org/betas