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

[brentvatne]

beginning of off topic discussion

---

not to derail too much here, but i don't quite understand this logic @tasn:

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...

As I said above, it's really simple, the code is already there in my expo-crypto, just lift it from there.

can you elaborate on your motivation for why you think it would be better to fork an expo library to try not to "rely on expo"? what is it you're concerned about?

[tasn]

not to derail too much here, but i don't quite understand this logic @tasn:

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...

As I said above, it's really simple, the code is already there in my expo-crypto, just lift it from there.

can you elaborate on your motivation for why you think it would be better to fork an expo library to try not to "rely on expo"? what is it you're concerned about?

It's not a fork of expo-crypto, it's just really poor naming on my part. It's not a fork, it's a tiny library that should have been called expo-get-random-values. Anyhow, as @martinheidegger just said, it looks like expo is going to have this (yay), so my lib can be deprecated. The advantage of my lib over this one was that it was possible to use it in expo.

What I meant with my comment regarding rely on expo: is that some apps don't use expo, that's why I want this lib (this being react-native-get-random-values) to work well, because my library only works on expo. I hope it's clear.

[brentvatne]

@tasn - fyi you can use libraries like expo-random anywhere, you don't need to use expo managed workflow :) https://blog.expo.io/you-can-now-use-expo-apis-in-any-react-native-app-7c3a93041331 - we made this possible ~1.5 yrs ago

[tasn]

@brentvatne, yeah, I'm aware (I'm also an occasional expo contributor). Though setting up unimodules is just too much of a pain and it's really not worth it just for this tiny piece of code. It's extra important for me because I'm not only building apps, but building packages, and I don't want my users to have to set up unimodules and go through all of this pain just to use my lib.

[brentvatne]

@tasn - makes sense. yeah we need to get rid of that overhead. it's very useful to reuse native code through dependency injection across modules (expo-camera can use expo-permissions and expo-file-system native modules by requesting a module that implements the permissions / filesystem interfaces) but it's very hard for people to see the benefits of this. we need to get this upstream so people don't have to think about it and get the benefits out of the box

---

end of off topic discussion

[tasn]

@brentvatne, yeah, I realise why you are doing it and it makes sense for many use-cases, though since I'm building a reusable library, I can't assume it makes sense to all of my users. :)

[tasn]

@LinusU, how can we move this forward?

[mmmoussa]

I came across https://github.com/mateioprea/react-native-random-values-jsi-helper and was interested in doing a performance test. I took the same approach with both libraries as @tasn did above for direct use of crypto as well as use of the uuid package. These are obviously unscientific tests but do provide some direction with regard to performance.

Crypto

const buf = new Uint8Array(32);
let i = 0;
const start = performance.now();
while (i < 10000) {
  crypto.getRandomValues(buf);
  i++;
}
const end = performance.now();
console.log(
  `Total time: ${end - start}ms, which is ${
    (end - start) / i
  } per random value`,
);

This library:
Total time: 409.1194100379944ms, which is 0.04091194100379944 per random value

jsi library:
Total time: 99.4056396484375ms, which is 0.00994056396484375 per random value

uuid

This library:
Total time: 484.69216442108154ms, which is 0.04846921644210815 per random value

jsi library:
Total time: 201.39968919754028ms, which is 0.020139968919754028 per random value