148038 2015-08-14 13:39:55 -0700 cryptographicallyRandomValuesFromOS should use arc4random_buf on Darwin. 2015-08-17 17:03:46 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=146473 InRadar P2 Normal --- 1 keith_miller webkit-unassigned ap benjamin cmarcelo commit-queue oldest_to_newest 1117745 0 keith_miller 2015-08-14 13:39:55 -0700 Currently, we open a file descriptor to /dev/urandom, which can sometimes fail to open. Using arc4random_buf instead should get around this issue. 1117747 1 259034 keith_miller 2015-08-14 13:42:53 -0700 Created attachment 259034 Patch 1117761 2 259034 ggaren 2015-08-14 14:17:38 -0700 Comment on attachment 259034 Patch r=me 1117791 3 259034 commit-queue 2015-08-14 15:01:58 -0700 Comment on attachment 259034 Patch Clearing flags on attachment: 259034 Committed r188489: <http://trac.webkit.org/changeset/188489> 1117792 4 commit-queue 2015-08-14 15:02:02 -0700 All reviewed patches have been landed. Closing bug. 1117975 5 ap 2015-08-15 13:13:35 -0700 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? 1118179 6 keith_miller 2015-08-17 11:50:29 -0700 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. 1118183 7 ap 2015-08-17 11:56:33 -0700 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? 1118285 8 keith_miller 2015-08-17 16:07:46 -0700 > 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. 259034 2015-08-14 13:42:53 -0700 2015-08-14 15:01:58 -0700 Patch 0001-cryptographicallyRandomValuesFromOS-should-use-arc4r.patch text/plain 2145 keith_miller RnJvbSA5MjU5MGM0NTYwM2FkZGRlYjMyMGI3ZjY0M2U2MzI4YmY1OTFkOGNlIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBLZWl0aCBNaWxsZXIgPGtlaXRoQEtlaXRocy1NYWNCb29rLVBy by5sb2NhbD4KRGF0ZTogRnJpLCAxNCBBdWcgMjAxNSAxMzo0MjowMiAtMDcwMApTdWJqZWN0OiBb UEFUQ0hdIGNyeXB0b2dyYXBoaWNhbGx5UmFuZG9tVmFsdWVzRnJvbU9TIHNob3VsZCB1c2UgYXJj NHJhbmRvbV9idWYKIG9uIERhcndpbi4KCi0tLQogU291cmNlL1dURi9DaGFuZ2VMb2cgICAgICAg ICAgICAgIHwgMTMgKysrKysrKysrKysrKwogU291cmNlL1dURi93dGYvT1NSYW5kb21Tb3VyY2Uu Y3BwIHwgIDggKysrKysrLS0KIDIgZmlsZXMgY2hhbmdlZCwgMTkgaW5zZXJ0aW9ucygrKSwgMiBk ZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5nZUxvZyBiL1NvdXJjZS9X VEYvQ2hhbmdlTG9nCmluZGV4IGI2MTAxYzIuLjJhM2U3NTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X VEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTUtMDgtMTQgIEtlaXRoIE1pbGxlciAgPGtlaXRoX21pbGxlckBhcHBsZS5jb20+CisKKyAg ICAgICAgY3J5cHRvZ3JhcGhpY2FsbHlSYW5kb21WYWx1ZXNGcm9tT1Mgc2hvdWxkIHVzZSBhcmM0 cmFuZG9tX2J1ZiBvbiBEYXJ3aW4uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xNDgwMzgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBDdXJyZW50bHksIHdlIG9wZW4gYSBmaWxlIGRlc2NyaXB0b3IgdG8gL2Rl di91cmFuZG9tLCB3aGljaCBjYW4gc29tZXRpbWVzCisgICAgICAgIGZhaWwgdG8gb3Blbi4gVXNp bmcgYXJjNHJhbmRvbV9idWYgaW5zdGVhZCBzaG91bGQgZ2V0IGFyb3VuZCB0aGlzIGlzc3VlLgor CisgICAgICAgICogd3RmL09TUmFuZG9tU291cmNlLmNwcDoKKyAgICAgICAgKFdURjo6Y3J5cHRv Z3JhcGhpY2FsbHlSYW5kb21WYWx1ZXNGcm9tT1MpOgorCiAyMDE1LTA4LTE0ICBDb21taXQgUXVl dWUgIDxjb21taXQtcXVldWVAd2Via2l0Lm9yZz4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xs aW5nIG91dCByMTg4NDQ0LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvT1NSYW5kb21Tb3Vy Y2UuY3BwIGIvU291cmNlL1dURi93dGYvT1NSYW5kb21Tb3VyY2UuY3BwCmluZGV4IDdlMTJhYTMu LjlkYWNhYTEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL09TUmFuZG9tU291cmNlLmNwcAor KysgYi9Tb3VyY2UvV1RGL3d0Zi9PU1JhbmRvbVNvdXJjZS5jcHAKQEAgLTI5LDcgKzI5LDcgQEAK ICNpbmNsdWRlIDxzdGRpbnQuaD4KICNpbmNsdWRlIDxzdGRsaWIuaD4KIAotI2lmIE9TKFVOSVgp CisjaWYgIU9TKERBUldJTikgJiYgT1MoVU5JWCkKICNpbmNsdWRlIDxlcnJuby5oPgogI2luY2x1 ZGUgPGZjbnRsLmg+CiAjaW5jbHVkZSA8dW5pc3RkLmg+CkBAIC00Miw2ICs0Miw3IEBACiAKIG5h bWVzcGFjZSBXVEYgewogCisjaWYgIU9TKERBUldJTikgJiYgT1MoVU5JWCkKIE5FVkVSX0lOTElO RSBOT19SRVRVUk5fRFVFX1RPX0NSQVNIIHN0YXRpYyB2b2lkIGNyYXNoVW5hYmxlVG9PcGVuVVJh bmRvbSgpCiB7CiAgICAgQ1JBU0goKTsKQEAgLTUxLDEwICs1MiwxMyBAQCBORVZFUl9JTkxJTkUg Tk9fUkVUVVJOX0RVRV9UT19DUkFTSCBzdGF0aWMgdm9pZCBjcmFzaFVuYWJsZVRvUmVhZEZyb21V UmFuZG9tKCkKIHsKICAgICBDUkFTSCgpOwogfQorI2VuZGlmCiAgICAgCiB2b2lkIGNyeXB0b2dy YXBoaWNhbGx5UmFuZG9tVmFsdWVzRnJvbU9TKHVuc2lnbmVkIGNoYXIqIGJ1ZmZlciwgc2l6ZV90 IGxlbmd0aCkKIHsKLSNpZiBPUyhVTklYKQorI2lmIE9TKERBUldJTikKKyAgICByZXR1cm4gYXJj NHJhbmRvbV9idWYoYnVmZmVyLCBsZW5ndGgpOworI2VsaWYgT1MoVU5JWCkKICAgICBpbnQgZmQg PSBvcGVuKCIvZGV2L3VyYW5kb20iLCBPX1JET05MWSwgMCk7CiAgICAgaWYgKGZkIDwgMCkKICAg ICAgICAgY3Jhc2hVbmFibGVUb09wZW5VUmFuZG9tKCk7IC8vIFdlIG5lZWQgL2Rldi91cmFuZG9t IGZvciB0aGlzIEFQSSB0byB3b3JrLi4uCi0tIAoyLjQuNgoK