Bug 22049

Summary: WebKit should have a cryptographic RNG
Product: WebKit Reporter: webkit <webkit>
Component: JavaScriptCoreAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, ap, buildbot, bzbarsky, collinj, eric, jberlin, krasin, oliver, webkit.review.bot
Priority: P4    
Version: 525.x (Safari 3.1)   
Hardware: All   
OS: All   
Bug Depends on: 54083    
Bug Blocks:    
Attachments:
Description Flags
The first version of crypto.getRandom{Float32,Uint8}Array().
none
work in progress
none
Patch eric: review+

webkit@shiftleft.org
Reported 2008-11-03 11:31:23 PST
Several Javascript cryptographic toolkits have emerged on the web, most notably Clipperz. Unfortunately, most forms of encryption require high-quality random numbers to work securely, and collecting entropy from Javascript is difficult and brittle. Therefore, Safari should have a cryptographic random number generator (RNG). I propose that it be called window.crypto.random, since this is where Mozilla claims the Firefox RNG will be when they implement it. WebKit doesn't link any cryptographic libraries that I know of, so the underlying RNG should probably be arc4random() (perhaps repeated a few times because RC4 is fast but has known flaws) on BSD-like platforms and rand_s() on Windows. Linux is somewhat trickier because it doesn't have a cryptographic RNG in libc. WebKit for Windows already has Math.random() implemented by rand_s(). That's well and good, but it should still implement window.crypto.random(), because we don't want developers to rely on cryptographic properties of Math.random(). This is related to Mozilla bug 440046: https://bugzilla.mozilla.org/show_bug.cgi?id=440046 Cheers, Mike
Attachments
The first version of crypto.getRandom{Float32,Uint8}Array(). (13.16 KB, patch)
2011-02-04 14:42 PST, Ivan Krasin 		no flags
DetailsFormatted DiffDiff
work in progress (26.69 KB, patch)
2011-02-10 00:50 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (33.67 KB, patch)
2011-02-10 23:36 PST, Adam Barth 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2008-11-04 03:48:40 PST
Implementing this functionality on Window object alone is not ideal, because workers (http://whatwg.org/ww) won't have access to it then. As it was already mentioned in Mozilla bugzilla, it would be best to discuss the API on WHATWG and/or W3C HTML5 mailing lists.
Oliver Hunt
Comment 2 2009-01-02 07:24:23 PST
I agree with Alexey putting a secure rng on window object is not helpful for workers. That said i feel dubious as to the value of this API -- if it doesn't exist people are going to have to use Math.random anyway, and having a predictable random() is something i consider a bug anyway. Happily as of r39510 Math.random() is cryptographically secure on mac and windows.
webkit@shiftleft.org
Comment 3 2009-01-02 12:43:00 PST
(In reply to comment #2) > That said i feel dubious as to the value of this API -- if it doesn't exist > people are going to have to use Math.random anyway, and having a predictable > random() is something i consider a bug anyway. > > Happily as of r39510 Math.random() is cryptographically secure on mac and > windows. Yay! I agree that a predictable Math.random() is a bug. However, Adam Barth convinced me that we need a crypto.random() of some sort too. The reason is that Math.random() isn't guaranteed to be cryptographically strong on all browsers, so relying on it for cryptographic randomness would be a mistake... at the very least, you'd have to do browser version sniffing, which is brittle. What's more, are we sure that Math.random() will even stay cryptographically strong in webkit? Someone might revert it, citing a performance regression. We don't actually need to write crypto.random() in C... it could be implemented in Javascript on top of Math.random() (except that Math.random() still isn't strong on Linux, right?), but there should still be a separate API.
Ivan Krasin
Comment 4 2011-02-04 14:42:08 PST
Created attachment 81287 [details] The first version of crypto.getRandom{Float32,Uint8}Array(). This is a patch that adds window.crypto.getRandomFloat32Array(length) and window.crypto.getUint8Array(length) Currently, it misses two important parts: 1. The intent to make window.crypto === undefined on the platforms where WTF::randomNumber is not secure 2. Make WTF::randomNumber secure on Linux if /dev/random or /dev/urandom available for initial seed Ivan Krasin
Adam Barth
Comment 5 2011-02-04 14:45:40 PST
Comment on attachment 81287 [details] The first version of crypto.getRandom{Float32,Uint8}Array(). View in context: https://bugs.webkit.org/attachment.cgi?id=81287&action=review > page/DOMWindow.h:345 > + Crypto* optionalCrypto() const { return m_crypto.get(); } What is optionalCrypto ? Looks mysterious to me.
Boris Zbarsky
Comment 6 2011-02-04 19:47:12 PST
Should there also be null-checks? Or if the allocation is fatal-on-failure, isn't this a DoS vector?
Adam Barth
Comment 7 2011-02-04 19:55:23 PST
See also this thread: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-February/030241.html
Adam Barth
Comment 8 2011-02-04 19:56:40 PST
> Should there also be null-checks? Or if the allocation is fatal-on-failure, isn't this a DoS vector? Allocation in WebKit is fatal-on-failure. It probably make sense to cap the number of random bytes you can request in one go. In general, we don't protect against memory exhaustion DoS though.
Adam Barth
Comment 9 2011-02-08 15:08:48 PST
@Ivan, are you still working on this bug?
Adam Barth
Comment 10 2011-02-09 15:13:30 PST
I just landed a cryptographically strong source of randomness. Now we just need to plumb it to the API.
Adam Barth
Comment 11 2011-02-10 00:50:59 PST
Created attachment 81935 [details] work in progress
Adam Barth
Comment 12 2011-02-10 23:36:19 PST
Created attachment 82109 [details] Patch
Eric Seidel (no email)
Comment 13 2011-02-10 23:41:28 PST
Comment on attachment 82109 [details] Patch :)
Eric Seidel (no email)
Comment 14 2011-02-10 23:41:49 PST
Does this need any ENABLE()? for WEBGL or whatever gives us these byte arrays?
Adam Barth
Comment 15 2011-02-11 00:37:55 PST
Committed r78321: <http://trac.webkit.org/changeset/78321>
Build Bot
Comment 16 2011-02-11 00:45:33 PST
Attachment 82109 [details] did not build on win: Build output: http://queues.webkit.org/results/7867603
WebKit Review Bot
Comment 17 2011-02-11 01:03:14 PST
http://trac.webkit.org/changeset/78321 might have broken Qt Linux Release minimal
Jessie Berlin
Comment 18 2011-02-11 07:43:13 PST
(In reply to comment #15) > Committed r78321: <http://trac.webkit.org/changeset/78321> This broke 4 tests on the Windows 7 Release test bots: https://bugs.webkit.org/show_bug.cgi?id=54285
Oliver Hunt
Comment 19 2011-02-11 08:42:40 PST
(In reply to comment #18) > (In reply to comment #15) > > Committed r78321: <http://trac.webkit.org/changeset/78321> > > This broke 4 tests on the Windows 7 Release test bots: > https://bugs.webkit.org/show_bug.cgi?id=54285 I'm really not happy with the API -- it doesn't provide a way to get meaningful random doubles/floats, and yet the api would imply that it should. The only way to get sensible random floats with this API would be to pull the value out as a uint32 and do value/uint32_max. Anyone using random floats from this will get "random" floats but with a very uneven distribution. Adding new web APIs should be discussed more widely prior -- there wasn't any clear consensus on the whatwg list, nor any discussion on webkit-dev. And it broke tests ;)
Adam Barth
Comment 20 2011-02-11 14:01:27 PST
> This broke 4 tests on the Windows 7 Release test bots: Apologies. These global enumeration tests are difficult to maintain. I managed to update all the expected results on the core builders. > I'm really not happy with the API -- it doesn't provide a way to get meaningful random doubles/floats, and yet the api would imply that it should. The only way to get sensible random floats with this API would be to pull the value out as a uint32 and do value/uint32_max. The discussion on the whatwg list indicated that folks weren't sure what a suitable notion of a random float would be. We can certainly add support for random floats later if folks can agree on what that would mean. > Anyone using random floats from this will get "random" floats but with a very uneven distribution. I'm not sure I follow. This API does not provide a mechanism for getting random floats. In any case, this is an API for cryptographic randomness. In cryptography, you almost always want random integers, not random floats. In fact, I can't think of a single use for random floats in cryptography. > Adding new web APIs should be discussed more widely prior -- there wasn't any clear consensus on the whatwg list, nor any discussion on webkit-dev. There seemed to be a clear consensus to me. Folks from Firefox agreed to implement the API. Some folks suggested some nice improvements. The discussion settled down. What more could you want for a one-function API? > And it broke tests ;) Indeed. However, that's easily fixable.
Note You need to log in before you can comment on or make changes to this bug.