[Cryptix-users] Bug fix for streaming PGP decryption

[Shawn W. <swi...@go...>]

I stumbled across a bug, and found a solution to it.  I realize this
project is dead, and that everyone should be migrating away from it,
but some of us are stuck (at least for a little longer) with legacy
code which uses it.  I realize there won't be any more releases, but I
though I might as well post the problem and the fix for anyone else
who finds themselves in the same boat.

One manifestation of this bug is that when you use the streaming PGP
decryption API (DecodedMessageInputStream) to decrypt a signed stream,
you get a PGPFatalDataFormatException with the message "Read tried
past end of inputstream".  The stack trace looks like:

cryptix.openpgp.PGPFatalDataFormatException: Read tried past end of inputstream.
at cryptix.openpgp.io.PGPLengthDataInputStream.readDirect(PGPLengthDataInputStream.java:123)
at cryptix.openpgp.io.PGPLengthDataInputStream.readInternal(PGPLengthDataInputStream.java:159)
at cryptix.openpgp.io.PGPLengthDataInputStream.readBuffer(PGPLengthDataInputStream.java:286)
at cryptix.openpgp.io.PGPLengthDataInputStream.readBuffer(PGPLengthDataInputStream.java:250)
at cryptix.openpgp.io.PGPLengthDataInputStream.readByteArray(PGPLengthDataInputStream.java:320)
at cryptix.openpgp.packet.PGPSignaturePacket.decodeBody(PGPSignaturePacket.java:507)
at cryptix.openpgp.provider.PGPDecodedMessageInputStream$SignedLiteralInputStream.verify(PGPDecodedMessageInputStream.java:732)
at cryptix.openpgp.provider.PGPDecodedMessageInputStream.engineClose(PGPDecodedMessageInputStream.java:485)
at cryptix.message.stream.DecodedMessageInputStream.close(DecodedMessageInputStream.java:162)
at com.google.math.crypto.pgp.cryptix.CryptixDecryptionStream.close(CryptixDecryptionStream.java:70)
at com.google.math.crypto.pgp.cryptix.CryptixDecryptionStream.read(CryptixDecryptionStream.java:53)

The problem actually lies in
cryptix.openpgp.provider.PGPDecodedMessageInputStream.read(byte[],int,int),
on line 905. The troublesome code is:

    if ((in.available() == 0) && (buf.length - bufoffset == 0))
        return 0;

This basically says "if the underlying stream (which contains
encrypted data) says it has no more data available and if our own
buffer (containing already-decrypted data) is empty, then return 0
indicating that we have no more decrypted data to be read.

Unfortunately, this test misses a third source of data -- the cipher
object.  It may have some additional data which will be returned by
the call to doFinal().  If this is the case, the above lines ensure
that the reader never gets that additional data.

To fix the problem, simply delete the above two lines.  This
quick-escape test is actually unnecessary, since the following code
works just fine even when no data is available from the underlying
stream or in the data buffer.  And without the quick-escape, the code
recognizes the end-of-stream condition and calls doFinal() on the
cipher, allowing it to retrieve and return the last of the decrypted
data.

A patch file is attached.

-- 
Shawn Willden
swi...@go...