bcgit / pc-dart

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

Is seeding wrong? #114

Closed EP-u-NW closed 3 years ago

EP-u-NW commented 3 years ago

While reading tutorials/rsa.md

SecureRandom exampleSecureRandom() {
  final secureRandom = FortunaRandom();

  final seedSource = Random.secure();
  final seeds = <int>[];
  for (int i = 0; i < 32; i++) {
    seeds.add(seedSource.nextInt(255));
  }
  secureRandom.seed(KeyParameter(Uint8List.fromList(seeds)));

  return secureRandom;
}

I noticed that seedSource.nextInt(255) is used for seeding. Since in dart Random.nextInt() is exclusive, it should in my opinion be seedSource.nextInt(256) to cover the whole value range of a unsigned 1 byte integer.

If you search this repos files for .nextInt(255) this same thing will show up multiple times. In most places it's not cricital (examples and tutorials), but in lib\asymmetric\pkcs1.dart and lib\asymetric\oaep.dart it might be a serious security vulnerability.

Edit: I noticed that .nextInt(255) was removed from the files in lib I mentioned above just yesterday and replaced with

return Platform.instance.platformEntropySource().getBytes(32);`

but not in the examples and tutorials.

mwcw commented 3 years ago

Hi yes

You are right about the .nextInt(255) and

We discovered it independently when abstracting away calls to Random.secure.

the reason for return Platform.instance.platformEntropySource().getBytes(32);

Is because on the nodejs runtime it fails to find Random.secure and a source of entropy needs to be sourced from the underlying platform.

EP-u-NW commented 3 years ago

I'll leave this issue open until the tutorials are updated with the new code 😄 But feel free to close it if you think thats not necessary.

mwcw commented 3 years ago

I'll leave this issue open until the tutorials are updated with the new code 😄

Lol.. I am just waiting for it to sync up then I will do a release.

MW

mwcw commented 3 years ago

Released: https://github.com/bcgit/pc-dart/issues/115