BouncyCastle Encrypt - Missing "END PGP MESSAGE"

Question

We recently upgraded our BouncyCastle .jar files to use the latest available version, and have been working to implement them into our existing code.

In testing the encryption method, we've found that the file created lacks the "END PGP MESSAGE" end tag. And seems to also lack the trailing encryption lines to mark the signature.

This is our implementation of the signed output writer - are we missing an essential step to signing our file? Or is our implementation wrong?

public static void writeSignedOutputStream(PGPKeyPair keyPair, byte[] data, OutputStream outStream, char dataType, String fileName, boolean withArmor)
        throws IOException, NoSuchAlgorithmException, NoSuchProviderException, PGPException, SignatureException
    {
        try{
            registerProvider();

            if(withArmor)
                outStream = new ArmoredOutputStream(outStream);

            PGPPrivateKey privateKey = keyPair.getPrivateKey();
            PGPPublicKey publicKey = keyPair.getPublicKey();

            // Original signature generator
            //PGPSignatureGenerator generator = new PGPSignatureGenerator(publicKey.getAlgorithm(), PGPUtil.SHA1, provider.getName());
            PGPSignatureGenerator sigGenerator = 
                    new PGPSignatureGenerator(new BcPGPContentSignerBuilder(publicKey.getAlgorithm(), PGPUtil.SHA1));

            // Updating for new version of BouncyCastle 
            //generator.initSign(PGPSignature.BINARY_DOCUMENT, privateKey);
            sigGenerator.init(PGPSignature.BINARY_DOCUMENT, privateKey);

            for(Iterator i = publicKey.getUserIDs(); i.hasNext(); ){
                PGPSignatureSubpacketGenerator subpacketGenerator = new PGPSignatureSubpacketGenerator();
                subpacketGenerator.setSignerUserID(false, (String)i.next());
                sigGenerator.setHashedSubpackets(subpacketGenerator.generate());
            }

            BCPGOutputStream bcpgStream = new BCPGOutputStream(outStream);

            sigGenerator.generateOnePassVersion(true).encode(bcpgStream);

            PGPLiteralDataGenerator dataGenerator = new PGPLiteralDataGenerator(false);
            OutputStream dataStream = dataGenerator.open(bcpgStream, dataType, fileName, data.length, new Date());

            for(int c = 0; c < data.length; c++){
                dataStream.write(data[c]);
                sigGenerator.update(data[c]);
            }

            sigGenerator.generate().encode(bcpgStream);
            dataStream.close();
            dataGenerator.close();
            bcpgStream.close();

        }catch(PGPException pe){
            //Exception catching
        }catch(Exception e){ 
            //Exception Catching
        }finally{}
    }

Here's what the output file looks like [with encrypted data censored] to give an illustration of the issue - I'm expecting to see "--END PGP MESSAGE--" at the bottom of this message, but it is simply not there.

On my machine the posted method `writeSignedOutputStream` creates a _complete_ file with BouncyCastle _v1.64_, including checksum and footer (`-----END PGP MESSAGE-----`). The adding of both is triggered by calling `bcpgStream.close()`. Since this doesn't happen in your environment, an exception is probably thrown between the instantiation of `bcpgStream` and its closing. Unfortunately, the exceptions are caught in the current code, but the stacktrace isn't evaluated. That should be done. Log the data in a file or in the console. Maybe this can identify / narrow down the problem. — Topaco, Nov 22 '19 at 23:37
@Topaco I've stepped through the code in my IDE and it never enters the Exception catching blocks - so as far as I can tell, no exception is being thrown at all in the code. — Zibbobz, Dec 03 '19 at 15:42
Did you step through the code with [_empty_ `catch`-blocks](https://stackoverflow.com/a/1234364/9014097)? I would remove the `catch`-blocks for the test or alternatively add a `Throwable#printStackTrace()`. This would be a more _reliable_ test. But maybe you've already done that. I'm not claiming that an exception _must_ be the cause, but it would explain the behavior well. — Topaco, Dec 04 '19 at 10:39
@Topaco Not empty, no. We have in-house logging methodology, which is why I didn't include it in my code snippet. When I stepped through, it didn't enter the catch blocks. — Zibbobz, Dec 04 '19 at 16:20

score 1 · Answer 1 · answered Nov 26 '19 at 15:50

1

I think you probably have issue because you don't flush your streams so some buffered data is not written.
Try to flush your output stream by calling outStream.flush() and bcpgStream.flush()

answered Nov 26 '19 at 15:50

Sergi

990
5
16

1

I don't think a missing flush is the main cause (rather a follow-up). `bcpgStream.flush()` calls exclusively `outStream.flush()`. `bcpgStream.close()` calls among others `outStream.flush()` (and thus implicitly `bcpgStream.flush()`) and also `outStream.close()`, see sourcecode, `BCPGOutputStream`-class [bcpg-jdk15on-164.zip](https://www.bouncycastle.org/download/bcpg-jdk15on-164.zip). I.e. it will be _implicitly flushed_ when `bcpgStream.close()` is called, but this does not happen at the moment (probably because of an exception). Without a stacktrace it's unfortunately only possible to guess. – Topaco Nov 27 '19 at 06:24
@Topaco Good catch - I haven't had a look at the source code of `BCPGOutputStream` before, but just seen a lot similar cases when people forgot to call `flush`. Apparently, you are right about implicit flush in `BCPGOutputStream.close()` code. So, I agree with you - stack trace is needed. BTW, here is source code of `BCPGOutputStream`: https://github.com/bcgit/bc-java/blob/738dfc0132323d66ad27e7ec366666ed3e0638ab/pg/src/main/java/org/bouncycastle/bcpg/BCPGOutputStream.java – Sergi Nov 27 '19 at 14:00

chubbsondubs · Answer 2 · 2020-03-30T19:13:37.547

So I'm encountering the exact same problem using 1.64, but with exporting a PublicKeyRing. I looked into how the footer gets written into an ArmoredOutputStream, and it only happens when close is called. In my case I was writing out using the encode method within a try / finally where close was in the finally. Something like the following:

PGPPublicKeyRing publicKeyRing = ...;

ByteArrayOutputStream out = new ByteArrayOutputStream();
ArmoredOutputStream aos = new ArmoredOutputStream( out );
try {
   publicKeyRing.encode( aos, true );
   aos.flush();
   return out.toByteArray();
} finally {
   aos.close();
}

The problem is that the footer isn't added until close is called, but since I was writing it to the ByteArrayOutputStream and retrieved the byte[] before close was called hence not getting the footer.

Rewriting it to this fixed the issue:

PGPPublicKeyRing publicKeyRing = ...;

ByteArrayOutputStream out = new ByteArrayOutputStream();
ArmoredOutputStream aos = new ArmoredOutputStream( out );
try {
   publicKeyRing.encode( aos, true );
   aos.flush();
} finally {
   aos.close();
}
return out.toByteArray();

Viola now I see the footer and the output is valid.

BouncyCastle Encrypt - Missing "END PGP MESSAGE"

2 Answers2