Failing Tests in KeystoreSecurityTest

[jmkao]

In the dev branch, there are two failing tests in the KeystoreSecurityTest that will cause CI failures if I push them down to the interns. Are these failures expected at the current point of development for the security functionality? If so, I can just annotate them with @Ignore down in my fork and its descendants so I can keep changes current down in our working forks.

org.area515.resinprinter.security.KeystoreSecurityTest.attemptReplayAttacks FAILED
    arrays first differed at element [0]; expected:<68> but was:<107>
        at org.junit.internal.ComparisonCriteria.arrayEquals(ComparisonCriteria.java:55)
        at org.junit.Assert.internalArrayEquals(Assert.java:532)
        at org.junit.Assert.assertArrayEquals(Assert.java:341)
        at org.junit.Assert.assertArrayEquals(Assert.java:352)
        at org.area515.resinprinter.security.KeystoreSecurityTest.attemptReplayAttacks(KeystoreSecurityTest.java:130)
        Caused by:
        java.lang.AssertionError: expected:<68> but was:<107>
            at org.junit.Assert.fail(Assert.java:88)
            at org.junit.Assert.failNotEquals(Assert.java:834)
            at org.junit.Assert.assertEquals(Assert.java:118)
            at org.junit.Assert.assertEquals(Assert.java:144)
            at org.junit.internal.ExactComparisonCriteria.assertElementsEqual(ExactComparisonCriteria.java:8)
            at org.junit.internal.ComparisonCriteria.arrayEquals(ComparisonCriteria.java:53)
            ... 4 more

org.area515.resinprinter.security.KeystoreSecurityTest.testConversationBetweenTwoTrustedUsersWithMultipleKeyExchanges FAILED
    javax.crypto.BadPaddingException: Given final block not properly padded
        at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:966)
        at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:824)
        at com.sun.crypto.provider.AESCipher.engineDoFinal(AESCipher.java:436)
        at javax.crypto.Cipher.doFinal(Cipher.java:2121)
        at org.area515.resinprinter.security.keystore.PhotonicCrypto.getData(PhotonicCrypto.java:208)
        at org.area515.resinprinter.security.KeystoreSecurityTest.testConversation(KeystoreSecurityTest.java:147)
        at org.area515.resinprinter.security.KeystoreSecurityTest.testConversationBetweenTwoTrustedUsersWithMultipleKeyExchanges(KeystoreSecurityTest.java:169)

[WesGilster]

No they aren't expected. All tests should pass and in fact they are still passing on Windows.

[WesGilster]

I knew there was a bug that could cause something similar if a client ran on a dissimilar OS than a server. However in this case, they are both the same. I did not expect this, I'll take a look at this after work.

[WesGilster]

Did this recently start failing when I introduced the ByteBuffer, or was this test always failing? Also, could you tell me what OS this failing on since I'm unable to reproduce it in Windows?

My guess is that it's because the SecureRandom is picking an inconsistent algorithm(since I didn't specify it) between two executions. This is odd because it shouldn't be inconsistent if encryption and decryption are happening on the same box. I did mark this as a TODO in the code, but I didn't expect to have to deal with this issue until later.

[jmkao]

I think it's been failing since the test was added, at least a couple of weeks. It occurs on Linux x86-64 1.8.0_45; same error both on Travis CI and in my local Linux desktop.

[WesGilster]

It looks like it's two different problems seed generation and algorithm selection. I'm going to hope that all implementations of SHA1PRNG are identical. If this turns out to be a bad idea, I can always copy the implementation and depend on that as a constant.

Run this:

        SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
        random.setSeed(new byte[]{1, 1, 43, 4, 2});
        System.out.println(random.nextLong());
        random = SecureRandom.getInstance("SHA1PRNG");
        random.setSeed(new byte[]{1, 1, 43, 4, 2});
        System.out.println(random.nextLong());
        System.out.println(new SecureRandom(new byte[]{ 1, 1, 43, 4, 2 }).nextLong());
        System.out.println(new SecureRandom(new byte[]{ 1, 1, 43, 4, 2 }).nextLong());

On Windows:

2496276945698797400
2496276945698797400
2496276945698797400
2496276945698797400

On Linux:

2496276945698797400
2496276945698797400
1992817521879006156
-4889043516263068651

[jmkao]

Hmm, I see... looking more closely at the docs, it seems that the default PRNG algorithm is different between different OS's:

https://docs.oracle.com/javase/8/docs/technotes/guides/security/SunProviders.html#SecureRandomImp

It seems that SHA1PRNG is what we would think of as a "traditional" seed based secure hashing generator with the property where if you supply the same seed, you'll get the same sequence. The NativePRNG algorithm, the default on Linux and MacOS, seems to simply use /dev/urandom and doesn't involve seeds or hashes. That would imply that the byte[] passed into the constructor is ignored by the underlying implementation.

http://stackoverflow.com/questions/27622625/securerandom-with-nativeprng-vs-sha1prng

Since SHA1PRNG seems to have the same provider across different operating systems, it should have the same behavior for Javas that utilize Sun libraries, which should be across everything that we support. It seems that Apache Harmony and Android may have different implementations, but I don't think we'll run into those.

[WesGilster]

Android was the area that I was a little concerned about. The latter versions of Android support SHA1PRNG, but earlier versions may be a problem. There is also a question in my mind that they may have changed the seeding algorithm after their bad implementation that caused that bitcoin theft. Since this is designed as a remote control and sharing protocol it isn't really out of the realm of possibilities that a mobile client will drive/monitor a printer from a remote destination. In fact, I was planning on Android being my second implementation. I'll get those tests fixed today or tomorrow to keep your branches out of limbo.

[jmkao]

Yeah, interoperability will probably take concrete testing. The use of a different RNG should not, in theory, impact the interoperability of compatible protocols, but in practice I have encountered many interoperability problems when using Java encryption implementations across different JVM vendors, which, in the past, have lead to the bouncy castle.

[WesGilster]

Specifically the issue I'm dealing with here is that my protocol depends upon the exchange of a seed and a consistent SecureRandom implementation on two completely different machines. This seed is used to build a SecureRandom that is in turn used to create an iv that is rotated on each message. Each message contains an offset from the initial seed that is used for the iv. If the two algorithms aren't implemented identically between the server and client, we have a problem. By including the implementation, I'm eliminating this as an eventual problem. Essentially this is a TODO that I should have done in the beginning.

I've checked in my solution. It's a bit ugly since I use the SHA1PRNG spi directly instead of installing it as a SecurityProvider. I should probably do that to make the code less strange. I've also added another test to ensure that messages can arrive out of order and not so perfectly "back and forth". Let me know what other tests you'd like added. I'm pretty open to suggestions...

[jmkao]

All tests pass now. Thanks!