An email to the authors of JSCrypto
14 January 2010
[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.
Regards,
Aldo
[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).]
Related:
- Host-proof applications: doing it wrong 26 Feb 2010
- cryp.sr - the next step 17 Jun 2010
- cryp.sr - a minimal host-proof cryptographic textpad 29 Mar 2010
- cryp.sr release 26 Jul 2010
- Introducing mitmproxy: an interactive man-in-the-middle proxy 16 Feb 2010
More posts:
- Apple, China and the war of ideas 07 May 2010
- Timsort - a study in grayscale 28 Jan 2010
- Hilbert Curve + Sorting Algorithms + Procrastination = ? 26 Jan 2010
- Generating colour maps with space-filling curves 07 Jan 2010
- Portrait of the Hilbert curve 03 Jan 2010
- The impact of language choice on github projects 15 Dec 2009
- Overflowing World of Warcraft's gold counter 11 Dec 2009
- Elinor Ostrom, the commons problem and Open Source 10 Dec 2009