Permalink
Browse files

Improvments to RNG

  • Loading branch information...
1 parent b0d5639 commit 98d5a7ca59ef04d06ac6aee468634b12975a0f5c @zootreeves zootreeves committed Dec 8, 2014
Showing with 26 additions and 28 deletions.
  1. +26 −28 bitcoinjs-lib/src/jsbn/rng.js
View
54 bitcoinjs-lib/src/jsbn/rng.js
@@ -8,15 +8,16 @@ var rng_state;
var rng_pool;
var rng_pptr;
-// Mix in a 32-bit integer into the pool
-function rng_seed_int(x) {
- rng_pool[rng_pptr++] ^= x & 255;
- rng_pool[rng_pptr++] ^= (x >> 8) & 255;
- rng_pool[rng_pptr++] ^= (x >> 16) & 255;
- rng_pool[rng_pptr++] ^= (x >> 24) & 255;
- if(rng_pptr >= rng_psize) rng_pptr -= rng_psize;
+// Mix in integer of n bits into the pool
+function rng_seed_int(x, n) {
+ if (!n) n = 32;
+ for (var i = 0; i <= n-8; i += 8) {
+ if (x >> i) rng_pool[rng_pptr++] ^= (x >> i) & 255;
+ if (rng_pptr >= rng_psize) rng_pptr -= rng_psize;
+ }
}
+
// Mix in the current time (w/milliseconds) into the pool
function rng_seed_time() {
rng_seed_int(new Date().getTime());
@@ -26,36 +27,33 @@ function rng_seed_time() {
if(rng_pool == null) {
rng_pool = new Array();
rng_pptr = 0;
@zootreeves
Blockchain.info member
zootreeves added a line comment Dec 9, 2014

For those interested. The bug was caused by missing line 29 and not initialising rng_pptr to 0. This commit was force pushed over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- var t;
-
- var crypto = _window.crypto || window.msCrypto;
- if(crypto && crypto.getRandomValues && typeof Int32Array != 'undefined') {
+ var mCrypto = _window.crypto || _window.msCrypto;
- console.log('Real Rand ');
+ if (mCrypto && mCrypto.getRandomValues && typeof Int32Array != 'undefined') {
+ var word_array = new Int32Array(rng_psize/4);
- var word_array = new Int32Array(32);
+ mCrypto.getRandomValues(word_array);
- _window.crypto.getRandomValues(word_array);
-
- for(t = 0; t < word_array.length; ++t)
- rng_seed_int(word_array[t]);
- } else {
- while(rng_pptr < rng_psize) { // extract some randomness from Math.random()
- t = Math.floor(65536 * Math.random());
- rng_pool[rng_pptr++] = t >>> 8;
- rng_pool[rng_pptr++] = t & 255;
- }
- }
+ for(var i = 0; i < word_array.length; ++i) {
+ rng_seed_int(word_array[i]);
+ }
+ }
- rng_pptr = 0;
- rng_seed_time();
-//rng_seed_int(window.screenX);
-//rng_seed_int(window.screenY);
+ for (var ii = 0; ii < rng_psize/2; ++ii) { // extract some randomness from Math.random()
+ rng_seed_int(65536 * Math.random(), 16);
@pennyfx
pennyfx added a line comment Dec 9, 2014

Why use math.random here, but mCrypto.getRandomValues on line#36 ?

@scintill
scintill added a line comment Dec 9, 2014

@pennyfx Note that mCrypto is the window.crypto API, which is not always available. It's used inside a conditional that checks that it's available. Maybe your question is, as I am wondering too, why use Math.random() at all (after the conditional use of getRandomValues()) if the superior crypto API was available?

@jerfelix
jerfelix added a line comment Dec 10, 2014

@scintill If you take a random number (from the crypto API), and XOR it with anything (even a number returned from a flawed PRNG, or a constant, like XKCD's random number 4), it will still be a random number.

So, by using Math.random(), they are adding a layer of protection in case the crypto library is found to be flawed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
}
function rng_get_byte() {
if(rng_state == null) {
+ if (rng_pool.length != rng_psize)
+ throw 'RNG Pool length does not match pool size';
+
+ if (rng_pool.filter(function(v) { return Math.abs(v) == 0; }).length > 12) {
+ throw 'RNG Pool contains a large number of zero elements'
+ }
+
rng_seed_time();
rng_state = prng_newstate();
rng_state.init(rng_pool);

6 comments on commit 98d5a7c

@cryptooman

The answer is simple: If you can't work with bits - don't use them in the code :)

@Come-from-Beyond

@jerfelix
What ensures that Math.random() doesn't use the same source of entropy as the crypto API? If the both return N then XOR operation will always return 0. Also, there can be a correlation between several bits of the numbers.
BTW, xorring/summing output of different generators makes the result worse, check Irwin-Hall distribution for the proof.

@ysangkok

@Come-from-Beyond I only see how the Irwin–Hall distribution applies to summing. Are you sure it applies to xor'ing?

@Come-from-Beyond

@ysangkok
It may be applied to xorring as well, it's "sum modulo 2" operation after all. A proof ought to be provided that it's indeed safe to xor several RNGs.

@ysangkok

@Come-from-Beyond If you have at least one true RNG, it is a one-time-pad, which is proved. The case for PRNG's is more tricky, I assume that's what you're looking for? Right now I'm wondering if the paper "three xor lemmas" is relevant.

@Come-from-Beyond

@ysangkok
Assuming that xored RNGs don't give normal distribution, do we still have the problem of possible correlation between the generators?

Please sign in to comment.