Flaky test for PsuedoRandomAddress

[jimmo]

I tried to build the Herald demo app using:

$ git rev-parse HEAD
b08c291c5c8a403a07d93528fdeb183bd98a18d5
$ ./gradlew build

It failed running the tests:

> Task :herald:testDebugUnitTest

com.vmware.herald.sensor.datatype.PseudoDeviceAddressTests > testRandomBytes FAILED
    java.lang.AssertionError at PseudoDeviceAddressTests.java:47

https://github.com/vmware/herald-for-android/blob/b08c291c5c8a403a07d93528fdeb183bd98a18d5/herald/src/test/java/com/vmware/herald/sensor/datatype/PseudoDeviceAddressTests.java#L40

Looking at testRandomBytes it is not surprising -- there's no reason to expect that every byte of the result will change every time. I'd expect this test to fail about 21% of the time.

Similarly, the testSecureRandom is not in any way a test for (secure) randomness.

Additionally, please see #90 (comment) -- why does the implementation of PsuedoRandomAddress need to be so complicated?

[adamfowleruk]

Thanks, this is a known issue with these initial tests. There is work ongoing to resolve this - see #81 . (I'm expecting a PR any moment with more tests provided). The bulk of that code has been used on prior projects, and so is well tested.

Also for the other closed issue, our code refers to a NIST document with advice for SHA1PRNG, amongst other items. Please refer to that NIST document for the rationale.

Please also remember we have to support a large number of devices and we have to do so in a way where the OS won't halt if the pool of entropy runs out - SecureRandom's default implementation on some phones does do this - causing any Bluetooth to potential fail if the phone sits there for a few hours (2-3 in our testing). For this reason we're looking to find some entropy elsewhere.

As you probably know, entropy is generally provided in many phones' SecureRandom implementations by moving the phone, writing to disc, interacting with the screen and so on. Especially older phones. This entropy does run out if the phone sits there, causing the SecureRandom call to block - effectively halting all BLe work.

We're looking at a way to determine a mechanism for creating an entropy pool from how Bluetooth physically reacts when in use, but will need to review this for suitability before deployment. Not a quick task as you can imagine.

If you have time please feel free to provide input on a way to generate enough randomness of a good enough quality (nothing is perfect) in the 6-byte pseudo device address without using a mechanism that halts Android phones, if you have time to look in to it? I have others looking in to it of course, but more heads are better than fewer. Bear in mind it must work back to Android 5. It would be interesting, for example, to review how Android OS does this for Mac address generation, and how random that really is itself.

Keeping this issue open (so it doesn't get missed) for now, but marking as a duplicate of #81.