Use a JavaScript PRNG (in addition) for better performance

[tasn]

Hey,

At the moment the calls are blocking on the RN bridge which can be quite slow. It's quite simple to make it both perform well and secure by using a JS based PRNG in addition to the native one.
You can see an example of this approach here: https://github.com/etesync/expo-crypto/blob/master/index.ts

Let me know if you need any help with implementing this or would like a patch.

--
Tom

[ctavan]

I believe the whole idea of this module is to provide a RNG that is guaranteed to be a CSPRNG.

Providing a JS fallback would precisely remove the guarantee that this module is trying to give.

[tasn]

@ctavan, the point of this module is indeed to have a CSPRNG, but I'm not suggesting a JS fallback, but rather a CSPRNG implemented in JS seeded from a secure source.

The problem with having a CSPRNG in RN without a native module is that we don't usually have access to a good source of random data. Though the moment we do (like with this module), we can seed a JS PRNG with this random data and use that to generate secure random data. It's exactly how native RNGs work. They get some real entropy from a real source of random data (which with my suggestion we still will), and then use that to generate an unpredictable stream of psuedo-random data.

For whatever it's worth, this is how all major libraries work (at least in JS land), for example libsodium.js and SJCL.

[LinusU]

I guess it depends on how one is using the module, but I think it would be nice to show some numbers and demonstrating an actual problem that would be solved by doing this before doing it. As the code is now, there isn't really that much that could go wrong, which is really good since crypto is very easy to get wrong.

If we were to add a PRNG we would need to be responsible for that one as well, and we need to be sure that we are using it correctly, and that it has sufficient random distribution...

Basically, I'm not totally against this, but I really want to see that we are solving some actual problem if we are going to do it...

[tasn]

Hey @LinusU, thanks for your comment.

I guess it depends on how one is using the module, but I think it would be nice to show some numbers and demonstrating an actual problem that would be solved by doing this before doing it. As the code is now, there isn't really that much that could go wrong, which is really good since crypto is very easy to get wrong.

I agree simplicity is good, and I agree with your comment about the numbers. Crossing through the react-native bridge is notoriously slow, and my app is asking for a lot of random numbers so it was visibly slow.
I can take a look into providing you with numbers though.

If we were to add a PRNG we would need to be responsible for that one as well, and we need to be sure that we are using it correctly, and that it has sufficient random distribution...

Use a well known dependency, such as SJCL (see the link from my original post). Then you don't need to test anything, it's a well known and battle tested library that will provide the PRNG on the JS side.

Basically, I'm not totally against this, but I really want to see that we are solving some actual problem if we are going to do it...

Sure thing. I respect this position.

[LinusU]

and my app is asking for a lot of random numbers so it was visibly slow.

Ahh, I didn't understand that you were actually having problems, lets look into this then 👍

It seems like all the major browsers are using XorShift128+, I haven't looked at what SJCL is using yet

ref: https://hackaday.com/2015/12/28/v8-javascript-fixes-horrible-random-number-generator/

[tasn]

It's visibly slow, but I haven't properly benchmarked this to make sure the random number generation is indeed the cause and there wasn't anything else that changed at the same time. That's why I want to have a better look and get real numbers.
I hope I'll be able to get to it very soon.

As for SJCL, it uses Fortuna. More info: https://bitwiseshiftleft.github.io/sjcl/doc/sjcl.prng.html

See what I did in the repo I linked to in my first post. I just built a very trimmed down SJCL with only the necessary parts (the random number generation), though you can probably just use a full blown SJCL build without having to trim it down.

[martinheidegger]

Just to let you know: I have been working on https://github.com/consento-org/get-random-values-polypony which should be significantly and uses XorShift128 ported from v8. (also it works with expo)

[tasn]

@martinheidegger, I'd be a bit weary about using a PRNG you created yourself, even if just ported. There are so many places where this can go wrong...
For expo I'm using my own lib (https://github.com/etesync/expo-crypto), though I want this lib so I don't need to rely on expo...

[martinheidegger]

@tasn What suggests that I have not been weary? It took me considerable amount of time and effort to implement and test this. I looked at the other implementations, and all had other drawbacks that are very unappealing (like shown in this issue). With the given tests I went over the minimum to cover as many bases as I could find but yes: I am sharing it here because it is not being looked at by enough people, and it is my hope that the library actually contributes to fix this issue.

[tasn]

I understand my wording may have been confusing, but I am the one who is weary. Why not use a well tested and audited PRNG like the one in SJCL (see the very simple code in expo-crypto)?
I wouldn't trust a PRNG unless it was audited by a team of cryptographers, and there's really no reason why anyone would roll their own when there are great solutions out there, like SJCL.

[martinheidegger]

Well, then it shall be SJCL forever. I have no answer for you. Until a team of cryptographers tests my work, there is no argument anyone can bring. I am not in favor of SJCL but I am only my voice.

[tasn]

@martinheidegger, cryptography is different to regular software engineering because in cryptography things look as if they work, but they may actually be completely broken... So yeah, unless something as important as a PRNG hasn't been properly audited no one should use it. All of modern cryptography relies on solid RNG.

[tasn]

@LinusU, I just realised I forgot to get back to you with numbers, here is the test I ran:

const buf = new Uint8Array(32);
const testSize = 10000;
console.log('Start');
const t0 = (new Date()).getTime();
for (let i = 0 ; i < testSize ; i++) {
  crypto.getRandomValues(buf);
}
const t1 = (new Date()).getTime();
console.log(`Done. Took: ${t1 - t0}ms, which is ${(t1 - t0) / testSize}ms per random value.`);

RN doesn't have performance so I used new Date() but I think it should be accurate enough for our purposes.
I'm getting:

Start
Done. Took: 3103ms, which is 0.3103ms per random value.

On my browser (though on my computer, not my phone), it's:

Done. Took: 25ms, which is 0.0025ms per random value.

I also ran it again on my computer with a much larger value and then I'm getting: 0.001884ms per random value, so this figure should be fairly accurate.
Anyhow, it's roughly 100x faster, and in crypto heavy code, random number generation happens a lot...

I think it's worth pursuing this improvement, what do you think? As I said above, it's really simple, the code is already there in my expo-crypto, just lift it from there.

[martinheidegger]

Turns out that expo@39 will come with its own crypto.getRandomBytes implementation expo/expo#9750