Add a Verify routine for HMAC

BUG=none
TEST=crypto_unittests


Review URL: http://codereview.chromium.org/7277024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@92824 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 101 insertions, 27 deletions

diff --git a/crypto/hmac.cc b/crypto/hmac.cc
index a38f514..588cb9e 100644
--- a/crypto/hmac.cc
+++ b/crypto/hmac.cc
@@ -8,6 +8,25 @@
 
 namespace crypto {
 
+// Performs a constant-time comparison of two strings, returning true if the
+// strings are equal.
+//
+// For cryptographic operations, comparison functions such as memcmp() may
+// expose side-channel information about input, allowing an attacker to
+// perform timing analysis to determine what the expected bits should be. In
+// order to avoid such attacks, the comparison must execute in constant time,
+// so as to not to reveal to the attacker where the difference(s) are.
+// For an example attack, see
+// http://groups.google.com/group/keyczar-discuss/browse_thread/thread/5571eca0948b2a13
+static bool SecureMemcmp(const void* s1, const void* s2, size_t n) {
+  const unsigned char* s1_ptr = reinterpret_cast<const unsigned char*>(s1);
+  const unsigned char* s2_ptr = reinterpret_cast<const unsigned char*>(s2);
+  unsigned char tmp = 0;
+  for (size_t i = 0; i < n; ++i, ++s1_ptr, ++s2_ptr)
+    tmp |= *s1_ptr ^ *s2_ptr;
+  return (tmp == 0);
+}
+
 size_t HMAC::DigestLength() const {
   switch (hash_alg_) {
     case SHA1:
@@ -20,4 +39,16 @@ size_t HMAC::DigestLength() const {
   }
 }
 
+bool HMAC::Verify(const base::StringPiece& data,
+                  const base::StringPiece& digest) const {
+  if (digest.size() != DigestLength())
+    return false;
+  scoped_array<unsigned char> computed_digest(
+      new unsigned char[digest.size()]);
+  if (!Sign(data, computed_digest.get(), static_cast<int>(digest.size())))
+    return false;
+
+  return SecureMemcmp(digest.data(), computed_digest.get(), digest.size());
+}
+
 }  // namespace crypto
diff --git a/crypto/hmac.h b/crypto/hmac.h
index 9800276..a8956ff 100644
--- a/crypto/hmac.h
+++ b/crypto/hmac.h
@@ -54,7 +54,15 @@ class CRYPTO_API HMAC {
   bool Sign(const base::StringPiece& data, unsigned char* digest,
             int digest_length) const;
 
-  // TODO(albertb): Add a Verify method.
+  // Verifies that the HMAC for the message in |data| equals the HMAC provided
+  // in |digest|, using the algorithm supplied to the constructor and the key
+  // supplied to the Init method. Use of this method is strongly recommended
+  // over using Sign() with a manual comparison (such as memcmp), as such
+  // comparisons may result in side-channel disclosures, such as timing, that
+  // undermine the cryptographic integrity. This method does not support
+  // comparing truncated HMACs.
+  bool Verify(const base::StringPiece& data,
+              const base::StringPiece& digest) const;
 
  private:
   HashAlgorithm hash_alg_;
diff --git a/crypto/hmac_unittest.cc b/crypto/hmac_unittest.cc
index 0f8f0ec..1978705 100644
--- a/crypto/hmac_unittest.cc
+++ b/crypto/hmac_unittest.cc
@@ -10,6 +10,28 @@
 static const size_t kSHA1DigestSize = 20;
 static const size_t kSHA256DigestSize = 32;
 
+static const char* kSimpleKey =
+    "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
+    "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
+    "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
+    "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
+    "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA";
+static const int kSimpleKeyLength = 80;
+
+static const struct {
+  const char *data;
+  const int data_len;
+  const char *digest;
+} kSimpleHmacCases[] = {
+  { "Test Using Larger Than Block-Size Key - Hash Key First", 54,
+    "\xAA\x4A\xE5\xE1\x52\x72\xD0\x0E\x95\x70\x56\x37\xCE\x8A\x3B\x55"
+        "\xED\x40\x21\x12" },
+  { "Test Using Larger Than Block-Size Key and Larger "
+        "Than One Block-Size Data", 73,
+    "\xE8\xE9\x9D\x0F\x45\x23\x7D\x78\x6D\x6B\xBA\xA7\x96\x5C\x78\x08"
+        "\xBB\xFF\x1A\x91" }
+};
+
 TEST(HMACTest, HmacSafeBrowsingResponseTest) {
   const int kKeySize = 16;
 
@@ -195,6 +217,10 @@ TEST(HMACTest, NSSFIPSPowerUpSelfTest) {
   EXPECT_EQ(kSHA1DigestSize, hmac.DigestLength());
   EXPECT_TRUE(hmac.Sign(message_data, calculated_hmac, kSHA1DigestSize));
   EXPECT_EQ(0, memcmp(kKnownHMACSHA1, calculated_hmac, kSHA1DigestSize));
+  EXPECT_TRUE(hmac.Verify(
+      message_data,
+      base::StringPiece(reinterpret_cast<const char*>(kKnownHMACSHA1),
+                        kSHA1DigestSize)));
 
   crypto::HMAC hmac2(crypto::HMAC::SHA256);
   ASSERT_TRUE(hmac2.Init(kKnownSecretKey, kKnownSecretKeySize));
@@ -205,34 +231,43 @@ TEST(HMACTest, NSSFIPSPowerUpSelfTest) {
 }
 
 TEST(HMACTest, HMACObjectReuse) {
-  const char *key =
-      "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
-      "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
-      "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
-      "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
-      "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA";
-  const int key_len = 80;
-
-  const struct {
-    const char *data;
-    const int data_len;
-    const char *digest;
-  } cases[] = {
-    { "Test Using Larger Than Block-Size Key - Hash Key First", 54,
-      "\xAA\x4A\xE5\xE1\x52\x72\xD0\x0E\x95\x70\x56\x37\xCE\x8A\x3B\x55"
-          "\xED\x40\x21\x12" },
-    { "Test Using Larger Than Block-Size Key and Larger "
-          "Than One Block-Size Data", 73,
-      "\xE8\xE9\x9D\x0F\x45\x23\x7D\x78\x6D\x6B\xBA\xA7\x96\x5C\x78\x08"
-          "\xBB\xFF\x1A\x91" }
-  };
-
   crypto::HMAC hmac(crypto::HMAC::SHA1);
-  ASSERT_TRUE(hmac.Init(reinterpret_cast<const unsigned char*>(key), key_len));
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
-    std::string data_string(cases[i].data, cases[i].data_len);
+  ASSERT_TRUE(
+      hmac.Init(reinterpret_cast<const unsigned char*>(kSimpleKey),
+                kSimpleKeyLength));
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kSimpleHmacCases); ++i) {
+    std::string data_string(kSimpleHmacCases[i].data,
+                            kSimpleHmacCases[i].data_len);
     unsigned char digest[kSHA1DigestSize];
     EXPECT_TRUE(hmac.Sign(data_string, digest, kSHA1DigestSize));
-    EXPECT_EQ(0, memcmp(cases[i].digest, digest, kSHA1DigestSize));
+    EXPECT_EQ(0, memcmp(kSimpleHmacCases[i].digest, digest, kSHA1DigestSize));
+  }
+}
+
+TEST(HMACTest, Verify) {
+  crypto::HMAC hmac(crypto::HMAC::SHA1);
+  ASSERT_TRUE(
+      hmac.Init(reinterpret_cast<const unsigned char*>(kSimpleKey),
+                kSimpleKeyLength));
+  const char empty_digest[kSHA1DigestSize] = { 0 };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kSimpleHmacCases); ++i) {
+    // Expected results
+    EXPECT_TRUE(hmac.Verify(
+        base::StringPiece(kSimpleHmacCases[i].data,
+                          kSimpleHmacCases[i].data_len),
+        base::StringPiece(kSimpleHmacCases[i].digest,
+                          kSHA1DigestSize)));
+    // Mismatched size
+    EXPECT_FALSE(hmac.Verify(
+        base::StringPiece(kSimpleHmacCases[i].data,
+                          kSimpleHmacCases[i].data_len),
+        base::StringPiece(kSimpleHmacCases[i].data,
+                          kSimpleHmacCases[i].data_len)));
+
+    // Expected size, mismatched data
+    EXPECT_FALSE(hmac.Verify(
+        base::StringPiece(kSimpleHmacCases[i].data,
+                          kSimpleHmacCases[i].data_len),
+        base::StringPiece(empty_digest, kSHA1DigestSize)));
   }
 }