Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EP-u-NW
Copy link

@EP-u-NW EP-u-NW commented Jun 26, 2021

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
Copy link
Contributor

(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
Copy link
Author

EP-u-NW commented Jul 3, 2021

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
Copy link
Collaborator

mwcw commented Jul 5, 2021

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
Copy link
Contributor

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
Copy link
Author

EP-u-NW commented Jul 5, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants