Allow skip in SignatureValidatingInputStream

[unverbraucht]

InputStream has a default implementation of skip() that just reads byte by byte. It's not particularly efficient but will work. I want to read a tar archive from a SignatureValidatingInputStream, and the library will call skip() to gloss over directory entries.

With the current implementation I have to write the whole stream to a file on storage, unarchive it and then delete it again.

BTW hard to say how much headache you saved me from with this library, deeply appreciated. BC is arcane.

[unverbraucht]

Oh, totally forgot: I put my code under the License.

[neuhalje]

Thank you for the flowers :-) Yeah, BC is one of the more interesting libraries. Since I am right now a bit busy -- could you write a unit test that verifies that skip works as expected? Much appreciated!

[unverbraucht]

Sure :) So this is interesting - it works for me in production but the unit test fails (!). I get this stack trace:

java.io.IOException: Signature verification failed because all of the following signatures (by userId) are missing.

	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.decrypting.SignatureValidatingInputStream.validateSignature(SignatureValidatingInputStream.java:83)
	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.decrypting.SignatureValidatingInputStream.read(SignatureValidatingInputStream.java:66)
	at org.bouncycastle.util.io.Streams.pipeAll(Unknown Source)
	at org.bouncycastle.util.io.Streams.readAll(Unknown Source)
	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.roundtrip.EncryptionDecryptionRoundtripIntegrationTest.encryptAndSignArmored_thenDecryptAndVerifyWithSkip_yieldsOriginalPlaintext(EncryptionDecryptionRoundtripIntegrationTest.java:127)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)
Caused by: name.neuhalfen.projects.crypto.bouncycastle.openpgp.validation.SignaturesMissingException: Signature verification failed because all of the following signatures (by userId) are missing.
	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.validation.RequireSpecificSignatureValidationForUserIdsStrategy.createMissingSignaturesException(RequireSpecificSignatureValidationForUserIdsStrategy.java:99)
	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.validation.RequireSpecificSignatureValidationForUserIdsStrategy.validateSignatures(RequireSpecificSignatureValidationForUserIdsStrategy.java:77)
	at name.neuhalfen.projects.crypto.bouncycastle.openpgp.decrypting.SignatureValidatingInputStream.validateSignature(SignatureValidatingInputStream.java:81)
	... 38 more

I'm not sure if I understood the test setup correctly but it definitely is caused by using skip on the decrypted IS. I'll leave this then for now and look into migrating my code away from using skip, maybe storing it on disk first. If you could have a look at the test to see if nothings wrong with it then this can be closed and rejected :)

[neuhalje]

sorry for keeping you waiting $DAY_JOB ...

[ispringer]

Hi @unverbraucht, I think the following skip implementation (not tested!) will fix the issue you are seeing:

// NOTE: We cannot simply delegate to super.skip, since we need to ensure our own read 
//       impl, which updates the one-pass signatures, is used to read the bytes being 
//       skipped.
@Override
public long skip(long n) throws IOException {
    if (n <= 0) {
        return 0;
    }

    // buffer to be reused repeatedly 
    final byte[] buffer = new byte[(int) Math.min(4096, n)];

    long remaining = n;
    while (remaining > 0) {
        final int read = read(buffer, 0, (int) Math.min(buffer.length, remaining));
        final boolean endOfStream = read == -1;
        if (endOfStream) {
            break;
        }
        remaining -= read;
    }

    return n - remaining;
}

[neuhalje]

ping :-)

[unverbraucht]

Hey, sorry for dropping the ball on this. @ispringer your implementation seems to work great, thanks. All unit tests pass. @neuhalje please review when you find the time :)