Update some comments and code to reflect the fact that HMAC() is no longer...

Update some comments and code to reflect the fact that HMAC() is no longer hostile to NULL key data.

BUG=395826

Review URL: https://codereview.chromium.org/975273002

Cr-Commit-Position: refs/heads/master@{#319178}

content/child/webcrypto/openssl/hmac_openssl.cc

@@ -38,22 +38,14 @@ Status SignHmac(const std::vector<uint8_t>& raw_key,
     return Status::ErrorUnsupported();
   unsigned int hmac_expected_length = EVP_MD_size(digest_algorithm);
 
-  // OpenSSL wierdness here.
-  // First, HMAC() needs a void* for the key data, so make one up front as a
-  // cosmetic to avoid a cast. Second, OpenSSL does not like a NULL key,
-  // which will result if the raw_key vector is empty; an entirely valid
-  // case. Handle this specific case by pointing to a fresh array.
-  const unsigned char null_key[] = {0};
-  const void* const raw_key_voidp = raw_key.size() ? &raw_key[0] : null_key;
-
   buffer->resize(hmac_expected_length);
   crypto::ScopedOpenSSLSafeSizeBuffer<EVP_MAX_MD_SIZE> hmac_result(
       vector_as_array(buffer), hmac_expected_length);
 
   unsigned int hmac_actual_length;
-  unsigned char* const success =
-      HMAC(digest_algorithm, raw_key_voidp, raw_key.size(), data.bytes(),
-           data.byte_length(), hmac_result.safe_buffer(), &hmac_actual_length);
+  unsigned char* const success = HMAC(
+      digest_algorithm, vector_as_array(&raw_key), raw_key.size(), data.bytes(),
+      data.byte_length(), hmac_result.safe_buffer(), &hmac_actual_length);
   if (!success || hmac_actual_length != hmac_expected_length)
     return Status::OperationError();
 

crypto/hmac_openssl.cc

@@ -20,45 +20,37 @@ struct HMACPlatformData {
   std::vector<unsigned char> key;
 };
 
-HMAC::HMAC(HashAlgorithm hash_alg)
-    : hash_alg_(hash_alg), plat_(new HMACPlatformData()) {
+HMAC::HMAC(HashAlgorithm hash_alg) : hash_alg_(hash_alg) {
   // Only SHA-1 and SHA-256 hash algorithms are supported now.
   DCHECK(hash_alg_ == SHA1 || hash_alg_ == SHA256);
 }
 
 bool HMAC::Init(const unsigned char* key, size_t key_length) {
   // Init must not be called more than once on the same HMAC object.
-  DCHECK(plat_->key.empty());
-
+  DCHECK(!plat_);
+  plat_.reset(new HMACPlatformData());
   plat_->key.assign(key, key + key_length);
-  if (key_length == 0) {
-    // Special-case: if the key is empty, use a key with one zero
-    // byte. OpenSSL's HMAC function breaks when passed a NULL key. (It calls
-    // HMAC_Init_ex which treats a NULL key as having already been initialized
-    // with a key previously.) HMAC pads keys with zeros, so this key is
-    // equivalent.
-    plat_->key.push_back(0);
-  }
   return true;
 }
 
 HMAC::~HMAC() {
-  // Zero out key copy.
-  plat_->key.assign(plat_->key.size(), 0);
-  STLClearObject(&plat_->key);
+  if (plat_) {
+    // Zero out key copy.
+    plat_->key.assign(plat_->key.size(), 0);
+    STLClearObject(&plat_->key);
+  }
 }
 
 bool HMAC::Sign(const base::StringPiece& data,
                 unsigned char* digest,
                 size_t digest_length) const {
-  DCHECK(!plat_->key.empty());  // Init must be called before Sign.
+  DCHECK(plat_);  // Init must be called before Sign.
 
   ScopedOpenSSLSafeSizeBuffer<EVP_MAX_MD_SIZE> result(digest, digest_length);
   return !!::HMAC(hash_alg_ == SHA1 ? EVP_sha1() : EVP_sha256(),
-                &plat_->key[0], plat_->key.size(),
-                reinterpret_cast<const unsigned char*>(data.data()),
-                data.size(),
-                result.safe_buffer(), NULL);
+                  vector_as_array(&plat_->key), plat_->key.size(),
+                  reinterpret_cast<const unsigned char*>(data.data()),
+                  data.size(), result.safe_buffer(), NULL);
 }
 
 }  // namespace crypto