Allow empty keys in hmac_openssl.cc.

crypto/hmac_openssl.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "crypto/hmac.h"
 
 #include <openssl/hmac.h>
 
 #include <algorithm>
 #include <vector>
@@ skipping 13 matching lines @@
     : hash_alg_(hash_alg), plat_(new HMACPlatformData()) {
   // 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());
 
   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
+    // does not like NULL keys, but keys are padded with zeros, so they're
+    // equivalent.

[wtc, 2014/04/09 21:30:58]

It would be nice to note what a NULL key means to OpenSSL.

I confirm that an empty key and a key of one zero byte are equivalent.

[davidben, 2014/04/09 21:44:29]

Done.

+    plat_->key.push_back(0);
+  }
   return true;
 }
 
 HMAC::~HMAC() {
   // 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.
 
   ScopedOpenSSLSafeSizeBuffer<EVP_MAX_MD_SIZE> result(digest, digest_length);
   return ::HMAC(hash_alg_ == SHA1 ? EVP_sha1() : EVP_sha256(),
                 &plat_->key[0], plat_->key.size(),

[wtc, 2014/04/09 21:30:58]

Can you try passing a non-NULL key pointer and a zero key size to ::HMAC()? Does
that work?

[davidben, 2014/04/09 21:44:29]

That works too; if the key size is 0, it never actually dereferences the key,
assuming memcpy(dst, key, 0) doesn't. So something like casting "" to an
unsigned char* would work fine too. This lets us keep the DCHECK and avoid the
cast, but the alternative doesn't make assumptions about key padding. *shrug* I
don't have much preference over which to use.

                 reinterpret_cast<const unsigned char*>(data.data()),
                 data.size(),
                 result.safe_buffer(), NULL);
 }
 
 }  // namespace crypto