<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - cryptographicallyRandomValuesFromOS should use CCRandomCopyBytes when available."
href="https://bugs.webkit.org/show_bug.cgi?id=148439#c13">Comment # 13</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - cryptographicallyRandomValuesFromOS should use CCRandomCopyBytes when available."
href="https://bugs.webkit.org/show_bug.cgi?id=148439">bug 148439</a>
from <span class="vcard"><a class="email" href="mailto:keith_miller@apple.com" title="Keith Miller <keith_miller@apple.com>"> <span class="fn">Keith Miller</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=259880&action=diff" name="attach_259880" title="Patch">attachment 259880</a> <a href="attachment.cgi?id=259880&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=259880&action=review">https://bugs.webkit.org/attachment.cgi?id=259880&action=review</a>
<span class="quote">>> Source/WTF/wtf/Assertions.h:51
>> +#include <os/trace.h>
>
> is this what breaks iOS build?
>
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphoneos/usr/local/include/wtf/spi/darwin/XPCSPI.h:37:1: error: typedef redefinition with different types ('NSObject<OS_xpc_object> *' vs 'void *')
> OS_OBJECT_DECL(xpc_object);
> ^</span >
It looks like it. There might be a work around however.
<span class="quote">>> Source/WTF/wtf/Assertions.h:55
>> +#endif
>
> Would it make sense to move the define for HAVE_OS_TRACE to wtf/Platform.h? Notice that wtf/Platform.h defines HAVE_OS_ACTIVITY, which we use to guard inclusion and data types defined in header os/activity.h.</span >
I'm not sure, It doesn't look like anyone else in the webkit uses os_trace as far as I can tell. I'm not opposed to this idea.
<span class="quote">>> Source/WTF/wtf/OSRandomSource.cpp:33
>> +#if __has_include(<CommonCrypto/CommonRandomSPI.h>)
>
> This check will fail for open source WebKit builds (including nightlies), because CommonRandomSPI.h is an internal header.
>
> In WebCore, we declare it in crypto/CommonCryptoUtilities.h - or you could add a CommonCryptoSPI header in WTF.</span >
I see, do you know of a reasonable upper bound on when CommonCrypto will be available? I would use the ENABLE(SUBTLE_CRYTPO) that is used in WebCore but it is not set in WTF and I'm not sure what conditions cause it to be set.
<span class="quote">>> Source/WTF/wtf/OSRandomSource.cpp:72
>> + CRASH_WITH_MESSAGE("crytographicallyRandomValuesFromOS could not get cryptographically random information from CCRandomCopyBytes and had error: %d", result);
>
> I'm not sure how this is more helpful than a simple RELEASE_ASSERT. We can see where that one happens, and CCRandomCopyBytes can't fail anyway.</span >
It's helpful in that if/when CCRandomCopyBytes fails the crash log will tell us the exact reason why it failed. Perhaps you are right, the documentation only states that CCRandomCopyBytes returns kCCSuccess if it succeeds but it doesn't say what it will do if it fails. Regardless, if in the future CCRandomCopyBytes can fail I would rather avoid a potential security risk and crash.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>