We should implement CryptoDigest for Windows to have access to cryptographically secure hash algorithms. This will allow us to support Content Security Policy inline script and inline stylesheet hashes on Windows among other benefits.
<rdar://problem/25065843>
Created attachment 273453 [details] Patch
Comment on attachment 273453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273453&action=review > Source/WebCore/ChangeLog:9 > + Implement the CryptoDigest abstraction for Windows so that we can compute cryptographically Is this covered by any regression tests that can be unskipped? > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:83 > + CryptHashData(m_context->hHash, input, length, 0); Please add a RELEASE_ASSERT for return value. Also, what does the function do for null input and zero length? I don't remember if we ever pass such arguments, or what CommonCrypto does. > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:91 > + CryptGetHashParam(m_context->hHash, HP_HASHSIZE, nullptr, &digestLength, 0); This function also returns a result that we should check. > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:94 > + CryptGetHashParam(m_context->hHash, HP_HASHVAL, result.data(), result.size()); I don't think that this is going to compile, as this call has fewer arguments. We should probably assert that the newly provided size matches digestLength.
(In reply to comment #3) > > Source/WebCore/ChangeLog:9 > > + Implement the CryptoDigest abstraction for Windows so that we can compute cryptographically > > Is this covered by any regression tests that can be unskipped? > Yes, this code is covered by the Content Security Policy tests for inline script and inline stylesheet hashes that I will unskip in the patch for bug #155007. > > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:83 > > + CryptHashData(m_context->hHash, input, length, 0); > > Please add a RELEASE_ASSERT for return value. > Will do. > Also, what does the function do for null input and zero length? [...] > Will bail out early if either input is null or length is 0. From reading <https://msdn.microsoft.com/en-us/library/windows/desktop/aa380202(v=vs.85).aspx> I suspect that this function is likely unhappy when passed a null pointer or zero length based on the presence of the possible error value ERROR_INVALID_PARAMETER. > > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:91 > > + CryptGetHashParam(m_context->hHash, HP_HASHSIZE, nullptr, &digestLength, 0); > > This function also returns a result that we should check. > Will do. > > Source/WebCore/platform/crypto/win/CryptoDigestWin.cpp:94 > > + CryptGetHashParam(m_context->hHash, HP_HASHVAL, result.data(), result.size()); > > I don't think that this is going to compile, as this call has fewer > arguments. We should probably assert that the newly provided size matches > digestLength. Oops! Will fix.
Created attachment 273479 [details] Patch
Created attachment 273483 [details] Patch
Comment on attachment 273483 [details] Patch r=me. Please land if the tests are green.
Comment on attachment 273483 [details] Patch Attachment 273483 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/950532 New failing tests: js/function-apply.html
Created attachment 273495 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Committed r197905: <http://trac.webkit.org/changeset/197905>
Committed Windows build fix in <https://trac.webkit.org/changeset/197911>.