Reject overlong sizes in crypto::HMAC.

If crypto::HMAC::Sign is passed too large of a buffer, it silently
fails to initialize the entire buffer. If crypto::HMAC::VerifyTruncated
is passed too large of one, it silently ignores the remainder of the
digest. Fix both of these.

Change-Id: Id57d5fc61ad85c9c63aef8f497c20fffd4409e2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2246989Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778548}

crypto/hmac.cc

@@ -59,6 +59,9 @@ bool HMAC::Sign(base::StringPiece data,
                 size_t digest_length) const {
   DCHECK(initialized_);
 
+  if (digest_length > DigestLength())
+    return false;
+
   ScopedOpenSSLSafeSizeBuffer<EVP_MAX_MD_SIZE> result(digest, digest_length);
   return !!::HMAC(hash_alg_ == SHA1 ? EVP_sha1() : EVP_sha256(), key_.data(),
                   key_.size(),
@@ -77,6 +80,9 @@ bool HMAC::VerifyTruncated(base::StringPiece data,
   if (digest.empty())
     return false;
   size_t digest_length = DigestLength();
+  if (digest.size() > digest_length)
+    return false;
+
   std::unique_ptr<unsigned char[]> computed_digest(
       new unsigned char[digest_length]);
   if (!Sign(data, computed_digest.get(), digest_length))

crypto/hmac.h

@@ -65,6 +65,8 @@ class CRYPTO_EXPORT HMAC {
   // Calculates the HMAC for the message in |data| using the algorithm supplied
   // to the constructor and the key supplied to the Init method. The HMAC is
   // returned in |digest|, which has |digest_length| bytes of storage available.
+  // If |digest_length| is smaller than DigestLength(), the output will be
+  // truncated. If it is larger, this method will fail.
   bool Sign(base::StringPiece data,
             unsigned char* digest,
             size_t digest_length) const WARN_UNUSED_RESULT;

crypto/hmac_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <stddef.h>
+#include <string.h>
 
 #include <string>
 
@@ -296,3 +297,30 @@ TEST(HMACTest, EmptyKey) {
   EXPECT_TRUE(hmac.Verify(
       data, base::StringPiece(kExpectedDigest, kSHA1DigestSize)));
 }
+
+TEST(HMACTest, TooLong) {
+  // See RFC4231, section 4.7.
+  unsigned char key[131];
+  for (size_t i = 0; i < sizeof(key); ++i)
+    key[i] = 0xaa;
+
+  std::string data = "Test Using Larger Than Block-Size Key - Hash Key First";
+  static uint8_t kKnownHMACSHA256[] = {
+      0x60, 0xe4, 0x31, 0x59, 0x1e, 0xe0, 0xb6, 0x7f, 0x0d, 0x8a, 0x26,
+      0xaa, 0xcb, 0xf5, 0xb7, 0x7f, 0x8e, 0x0b, 0xc6, 0x21, 0x37, 0x28,
+      0xc5, 0x14, 0x05, 0x46, 0x04, 0x0f, 0x0e, 0xe3, 0x7f, 0x54};
+
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  ASSERT_TRUE(hmac.Init(key, sizeof(key)));
+
+  // Attempting to write too large of an HMAC is an error.
+  uint8_t calculated_hmac[kSHA256DigestSize + 1];
+  EXPECT_FALSE(hmac.Sign(data, calculated_hmac, sizeof(calculated_hmac)));
+
+  // Attempting to verify too large of an HMAC is an error.
+  memcpy(calculated_hmac, kKnownHMACSHA256, kSHA256DigestSize);
+  calculated_hmac[kSHA256DigestSize] = 0;
+  EXPECT_FALSE(hmac.VerifyTruncated(
+      data,
+      std::string(calculated_hmac, calculated_hmac + sizeof(calculated_hmac))));
+}