Bug 22049 - WebKit should have a cryptographic RNG
: WebKit should have a cryptographic RNG
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 525.x (Safari 3.1)
: All All
: P4 Enhancement
Assigned To:
:
:
: 54083
:
  Show dependency treegraph
 
Reported: 2008-11-03 11:31 PST by
Modified: 2011-02-12 04:33 PST (History)


Attachments
The first version of crypto.getRandom{Float32,Uint8}Array(). (13.16 KB, patch)
2011-02-04 14:42 PST, Ivan Krasin
no flags Review Patch | Details | Formatted Diff | Diff
work in progress (26.69 KB, patch)
2011-02-10 00:50 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
Patch (33.67 KB, patch)
2011-02-10 23:36 PST, Adam Barth
eric: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 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
------- Comment #1 From 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.
------- Comment #2 From 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.
------- Comment #3 From 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.
------- Comment #4 From 2011-02-04 14:42:08 PST -------
Created an attachment (id=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
------- Comment #5 From 2011-02-04 14:45:40 PST -------
(From update of attachment 81287 [details])
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.
------- Comment #6 From 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?
------- Comment #7 From 2011-02-04 19:55:23 PST -------
See also this thread: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-February/030241.html
------- Comment #8 From 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.
------- Comment #9 From 2011-02-08 15:08:48 PST -------
@Ivan, are you still working on this bug?
------- Comment #10 From 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.
------- Comment #11 From 2011-02-10 00:50:59 PST -------
Created an attachment (id=81935) [details]
work in progress
------- Comment #12 From 2011-02-10 23:36:19 PST -------
Created an attachment (id=82109) [details]
Patch
------- Comment #13 From 2011-02-10 23:41:28 PST -------
(From update of attachment 82109 [details])
:)
------- Comment #14 From 2011-02-10 23:41:49 PST -------
Does this need any ENABLE()? for WEBGL or whatever gives us these byte arrays?
------- Comment #15 From 2011-02-11 00:37:55 PST -------
Committed r78321: <http://trac.webkit.org/changeset/78321>
------- Comment #16 From 2011-02-11 00:45:33 PST -------
Attachment 82109 [details] did not build on win:
Build output: http://queues.webkit.org/results/7867603
------- Comment #17 From 2011-02-11 01:03:14 PST -------
http://trac.webkit.org/changeset/78321 might have broken Qt Linux Release minimal
------- Comment #18 From 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
------- Comment #19 From 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 ;)
------- Comment #20 From 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.