An email to the authors of JSCrypto


[Update: A fix for these problems and one noted by Peter Burns in the comments to this post has been posted. Get it while it's hot, folks.]

Hi folks,

Thanks for a blazingly fast little crypto library. Please find below a few comments on the code.

There's an error in the is_ready function of the random number generator. On line 1386 of the jscrypto.js file, you have:

return (this._pool_entropy[0] > this._BITS_PER_RESEED &&
new Date.valueOf() > this._next_reseed) ?

This should be:

return (this._pool_entropy[0] > this._BITS_PER_RESEED &&
new Date().valueOf() > this._next_reseed) ?

In Safari, this will cause an error and script termination. In Firefox, the effect is much worse - new Date.valueOf() returns an object, which never compares as greater than any integer. As an unfortunate consequence, that clause can never evaluate to true, and your Fortuna implementation's periodic reseeding never triggers...

All is not lost, though, because luckily the random_words function in which the return value from is_ready is used makes no sense. ;-) To start with, on line 1289 you have:

if (readiness == this.NOT_READY)

But readiness here is a bit field, and this clause will evaluate to false in half the situations that is_ready actually does return NOT_READY. You surely want

if (readiness & this.NOT_READY)

Three lines further down, you have:

else if (readiness && this.REQUIRES_RESEED)

This, again, doesn't do what it seems - && is the boolean and, not the bitwise and. Since this.REQUIRES_RESEED is simply a positive constant, that really becomes:

else if (readiness)

So despite the bug in is_ready, your reseeding function actually runs every time random data is requested. Phew - who says two wrongs don't make a right, ey? Reseeding every time data is requested might open the generator to some interesting entropy exhaustion attacks, but is much better than not reseeding at all.

A corollary to all this is that you also need to address the fact that the the return value from is_ready is used incorrectly in the rest of your code and your examples. As it stands, testing for readiness with

if (Random.is_ready())

is wrong, because your readiness function can return REQUIRES_RESEED | NOT_READY, which is a positive integer. I'd recommend changing the interface of is_ready to have an obvious boolean return value instead, though - typing

if (Random.is_ready() & Random.IS_READY)

is a bit of a mouthful.

Thanks again for jscrypto.



[No animals were harmed producing this post. Content lightly edited for markup and formatting from the original email. Yes, I really do like JSCrypto - this error-hiding-an-error was amusing, but the AES implementation seems good (although the jury's still out on the SHA256 portion).]