Added WebCypto digest OpenSSL implementation.

content/renderer/webcrypto_impl_openssl.cc

Index: content/renderer/webcrypto_impl_openssl.cc
diff --git a/content/renderer/webcrypto_impl_openssl.cc b/content/renderer/webcrypto_impl_openssl.cc
index 343eb7251076093623182568b6ed3e75e02eddc0..b5b29c2fd7a6274b39a0379d8ec1869a32293eb2 100644
--- a/content/renderer/webcrypto_impl_openssl.cc
+++ b/content/renderer/webcrypto_impl_openssl.cc
@@ -4,9 +4,16 @@
 
 #include "content/renderer/webcrypto_impl.h"
 
+#include <openssl/evp.h>

[Bryan Eyler, 2013/09/18 21:36:21]

Following style of _nss.cc, leave a space between system includes and Chrome
headers.  I can't seem to find a link to this in the style guide ATM.

[Ryan Sleevi, 2013/09/18 22:05:36]

http://www.chromium.org/developers/coding-style#TOC-Platform-specific-code

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Names_...

[padolph, 2013/09/18 22:50:07]

Done.

+#include "base/logging.h"
+#include "crypto/openssl_util.h"
+#include "third_party/WebKit/public/platform/WebArrayBuffer.h"
+#include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
+
 namespace content {
 
 void WebCryptoImpl::Init() {
+  crypto::EnsureOpenSSLInit();
 }
 
 bool WebCryptoImpl::DigestInternal(
@@ -14,9 +21,54 @@ bool WebCryptoImpl::DigestInternal(
     const unsigned char* data,
     unsigned data_size,
     WebKit::WebArrayBuffer* buffer) {
-  // TODO(bryaneyler): Placeholder for OpenSSL implementation.
-  // Issue http://crbug.com/267888.
-  return false;
+
+    crypto::OpenSSLErrStackTracer(FROM_HERE);

[Bryan Eyler, 2013/09/18 21:36:21]

Two-space indentation throughout this function - here, and in following
indentations.

[padolph, 2013/09/18 22:50:07]

Done.

+
+    const EVP_MD * digAlg;

[Ryan Sleevi, 2013/09/18 22:05:36]

STYLE: "const EVP_MD *" -> "const EVP_MD* " (see
http://www.chromium.org/developers/coding-style#Code_Formatting - "put * and &
by the type rather than the variable name"

STYLE: digest_algorithm or digest_alg (
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Genera...
"eschew abbreviation") and
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Variab...
("Variable names are all lowercase, with underscores between words")

[padolph, 2013/09/18 22:50:07]

Done.

+    switch (algorithm.id()) {
+        case WebKit::WebCryptoAlgorithmIdSha1:
+            digAlg = EVP_sha1();
+            break;
+        case WebKit::WebCryptoAlgorithmIdSha224:
+            digAlg = EVP_sha224();
+            break;
+        case WebKit::WebCryptoAlgorithmIdSha256:
+            digAlg = EVP_sha256();
+            break;
+        case WebKit::WebCryptoAlgorithmIdSha384:
+            digAlg = EVP_sha384();
+            break;
+        case WebKit::WebCryptoAlgorithmIdSha512:
+            digAlg = EVP_sha512();
+            break;
+        default:
+            // Not a digest algorithm.
+            return false;
+    }
+
+    crypto::ScopedOpenSSL<EVP_MD_CTX, EVP_MD_CTX_destroy>
+        digCtx(EVP_MD_CTX_create());

[Bryan Eyler, 2013/09/18 21:36:21]

Naming: Don't abbreviate and use underscores to separate words.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#General_Naming...

[padolph, 2013/09/18 22:50:07]

Done.

+    if (!digCtx.get())
+        return false;
+
+    if (!EVP_DigestInit_ex(digCtx.get(), digAlg, NULL))
+        return false;
+
+    if (!EVP_DigestUpdate(digCtx.get(), data, data_size))

[Ryan Sleevi, 2013/09/18 22:05:36]

STYLE NIT (eg: not critical, but perhaps more consistent)

if (!EVP_DigestInit_ex(digest_context.get(), digest_algorithm, NULL) ||
    !EVP_DigestUpdate(digest_context.get(), data, data_size) {
  return false;
}

Basically, condense conditionals.

[padolph, 2013/09/18 22:50:07]

Done.

+        return false;
+
+    const size_t hashResultSize = EVP_MD_CTX_size(digCtx.get());

[Ryan Sleevi, 2013/09/18 22:05:36]

CORRECTNESS: EVP_MD_CTX_size returns an int, not a size_t, and MAY fail (return
<= 0)
STYLE: naming: hash_result_size / hash_result_length

[padolph, 2013/09/18 22:50:07]

Done.

+    DCHECK_LE(hashResultSize, static_cast<size_t>(EVP_MAX_MD_SIZE));
+
+    *buffer = WebKit::WebArrayBuffer::create(hashResultSize, 1);
+    unsigned char* const hashBuf =

[Ryan Sleevi, 2013/09/18 22:05:36]

STYLE: hashBuf -> hash_buffer / result_buffer

[padolph, 2013/09/18 22:50:07]

Done.

+            reinterpret_cast<unsigned char* const>(buffer->data());
+
+    unsigned int hashSize = 0;

[Ryan Sleevi, 2013/09/18 22:05:36]

STYLE: hashSize -> hash_size

[padolph, 2013/09/18 22:50:07]

Done.

+    if (!(EVP_DigestFinal_ex(digCtx.get(), hashBuf, &hashSize)))

[Ryan Sleevi, 2013/09/18 22:05:36]

STYLE: unnecessary added ()
STYLE NIT (eg: not critical, but perhaps more consistent)

try:
if (!EVP_DigestFinal_ex(digest_context.get(), hash_buffer,
                        &hash_size)) ||
    hash_size != hash_result_size) {
  return false;
}

return true;

[I'm not sure the exact wrapping of the above, so adjust as appropriate]

Handle more failures inline, as possible.

[padolph, 2013/09/18 22:50:07]

Done.

Also, I realized I should release buffer (buffer->reset() ? ) if either of these
failures occur, since we then don't trust what OpenSSL did to it, and it is
directly accessible to the caller.

+        return false;
+
+    return hashResultSize == hashSize;
 }
 
 }  // namespace content