Bug 148038 - cryptographicallyRandomValuesFromOS should use arc4random_buf on Darwin.
Summary: cryptographicallyRandomValuesFromOS should use arc4random_buf on Darwin.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-08-14 13:39 PDT by Keith Miller
Modified: 2015-08-17 17:03 PDT (History)
4 users (show)

See Also:

Attachments
Patch (2.09 KB, patch)
2015-08-14 13:42 PDT, Keith Miller 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2015-08-14 13:39:55 PDT 
Currently, we open a file descriptor to /dev/urandom, which can sometimes fail to open. Using arc4random_buf instead should get around this issue.
Comment 1 Keith Miller 2015-08-14 13:42:53 PDT 
Created attachment 259034 [details]
Patch
Comment 2 Geoffrey Garen 2015-08-14 14:17:38 PDT 
Comment on attachment 259034 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2015-08-14 15:01:58 PDT 
Comment on attachment 259034 [details]
Patch

Clearing flags on attachment: 259034

Committed r188489: <http://trac.webkit.org/changeset/188489>
Comment 4 WebKit Commit Bot 2015-08-14 15:02:02 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 5 Alexey Proskuryakov 2015-08-15 13:13:35 PDT 
This is a pretty surprising change. How did you come to this conclusion, and what other options have you considered? Is there even a bug anywhere tracking the symptom?

More specifically, how did you decide to not use CCRandomCopyBytes, and what can make opening /dev/urandom fail?
Comment 6 Keith Miller 2015-08-17 11:50:29 PDT 
Whoops, I forgot to attach the associated radar (rdar://problem/21939126) to this bug. Alexey, I chose to use arc4random_buf because we just needed something that didn't open /dev/urandom. Although, looking into arc4random_buf further it appears that it opens /dev/urandom anyway.
Comment 7 Alexey Proskuryakov 2015-08-17 11:56:33 PDT 
Thank you for the link!

> Although, looking into arc4random_buf further it appears that it opens /dev/urandom anyway.

Should the patch be rolled out then?
Comment 8 Keith Miller 2015-08-17 16:07:46 PDT 
> Should the patch be rolled out then?

I'm not sure it's worth it. I am finishing up a patch that uses CCRandomCopyBytes instead. I'll CC you on that.