bcgit / pc-dart

Pointy Castle - Dart Derived Bouncy Castle APIs
MIT License
237 stars 122 forks source link

Added a SecureRandom implementation based on darts own Random class. #110

Open EP-u-NW opened 3 years ago

EP-u-NW commented 3 years ago

As discussed in #102 I added an implementation of SecureRandom based on darts own Random. I saw that pointycastle already features a SecureRandomBase which does all random operations based on the nextUint8() function, so I opted to use this instead of the ByteBasedSecureRandom I proposed in #102.

I think having a random based on darts own Random implementation is useful, since the dart team might have done some optimizations on vm level for random number generation.

I additionally provided tests.

AKushWarrior commented 3 years ago

(Copied from RSA keygen issue)

The issue is that Dart's random (even Random.secure()) is non-specified and thus somewhat dangerous to rely on for cryptographic purposes. I think that the benefit probably outweighs this (relatively nitpicky) concern, but Fortuna is not significantly slower than Random.secure() anyways, so there wouldn't be a performance bottleneck there.

EP-u-NW commented 3 years ago

Hm I think it really comes down to trust here: Random.secure() claims to be

cryptographically secure

The question is now: Do we trust this statement? As a developer not specalized in cryptographic, I didn't know which random to pick when I started using pointycastle. I did not understand why I could not use the Random from that I used on many other occasions. I still have no idea what Fortuna actually is so and can thus not be sure if pointycastle implements it correctly (as stated in #114 I'm belive that till yesterdays patches there were a major flaw in RNG by using a wrong seed). On the other hand, I somewhat trust Random.secure() to be secure and I belive that if a flaw is discovered in it, google (with all its massiv resources behind it) will come up with a fix very fast.

mwcw commented 3 years ago

Hi,

Has this been tested on nodeJS?

https://api.dart.dev/stable/2.10.5/dart-math/Random/Random.secure.html

Will throw an UnsupportedError on if it cannot find one.

MW

AKushWarrior commented 3 years ago

I don't really see how this differs from PlatformEntropySource... Isn't the whole point of that class to provide bytes from Random.secure() on native and node's thing otherwise?

@mwcw any thoughts on that?

EP-u-NW commented 3 years ago

It was actually not tested on node. And you are correct, instantiating a DartSecureRandom from this PR would throw an UnsupportedError there. But in my opinion this is not a problem, since it fulfills the "contract" of darts Random.secure: Someone who is explicitly looking for a random with darts behaviour must be aware that darts behaviour can lead to the UnsupportedError.

But what @AKushWarrior is saying is also interessting: The Platform provides this platformaware measures already. So instead of making a DartSecureRandom, a PlatformSecureRandom cloud provide a random based on darts random, but would also have an automatic fallback on unsupported platforms.