Add WARN_UNUSED_RESULT to crypto/hmac.h

BUG=none
TEST=none


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

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

chrome/browser/internal_auth.cc

@@ -251,8 +251,11 @@ class InternalAuthVerificationService {
 
     if (key.size() != kKeySizeInBytes)
       return;
-    engine_.reset(new crypto::HMAC(crypto::HMAC::SHA256));
-    engine_->Init(key);
+    scoped_ptr<crypto::HMAC> new_engine(
+        new crypto::HMAC(crypto::HMAC::SHA256));
+    if (!new_engine->Init(key))
+      return;
+    engine_.swap(new_engine);
     key_ = key;
     key_change_tick_ = GetCurrentTick();
   }
@@ -347,9 +350,12 @@ class InternalAuthGenerationService : public base::ThreadChecker {
           &InternalAuthGenerationService::GenerateNewKey);
     }
 
-    engine_.reset(new crypto::HMAC(crypto::HMAC::SHA256));
+    scoped_ptr<crypto::HMAC> new_engine(
+        new crypto::HMAC(crypto::HMAC::SHA256));
     std::string key = base::RandBytesAsString(kKeySizeInBytes);
-    engine_->Init(key);
+    if (!new_engine->Init(key))
+      return;
+    engine_.swap(new_engine);
     key_regeneration_tick_ = GetCurrentTick();
     g_verification_service.Get().ChangeKey(key);
     std::fill(key.begin(), key.end(), 0);

chrome/common/net/gaia/oauth_request_signer.cc

@@ -204,8 +204,8 @@ bool SignHmacSha1(const std::string& text,
   crypto::HMAC hmac(crypto::HMAC::SHA1);
   DCHECK(hmac.DigestLength() == kHmacDigestLength);
   unsigned char digest[kHmacDigestLength];
-  hmac.Init(key);
-  bool result = hmac.Sign(text, digest, kHmacDigestLength) &&
+  bool result = hmac.Init(key) &&
+      hmac.Sign(text, digest, kHmacDigestLength) &&
       base::Base64Encode(std::string(reinterpret_cast<const char*>(digest),
                                      kHmacDigestLength),
                          signature_return);

crypto/hmac.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "base/basictypes.h"
+#include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/string_piece.h"
 #include "crypto/crypto_api.h"
@@ -38,11 +39,11 @@ class CRYPTO_API HMAC {
   // Initializes this instance using |key| of the length |key_length|. Call Init
   // only once. It returns false on the second or later calls.
   // TODO(abarth): key_length should be a size_t.
-  bool Init(const unsigned char* key, int key_length);
+  bool Init(const unsigned char* key, int key_length) WARN_UNUSED_RESULT;
 
   // Initializes this instance using |key|. Call Init only once. It returns
   // false on the second or later calls.
-  bool Init(const base::StringPiece& key) {
+  bool Init(const base::StringPiece& key) WARN_UNUSED_RESULT {
     return Init(reinterpret_cast<const unsigned char*>(key.data()),
                 static_cast<int>(key.size()));
   }
@@ -52,7 +53,7 @@ class CRYPTO_API HMAC {
   // returned in |digest|, which has |digest_length| bytes of storage available.
   // TODO(abarth): digest_length should be a size_t.
   bool Sign(const base::StringPiece& data, unsigned char* digest,
-            int digest_length) const;
+            int digest_length) const WARN_UNUSED_RESULT;
 
   // 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
@@ -62,12 +63,13 @@ class CRYPTO_API HMAC {
   // undermine the cryptographic integrity. |digest| must be exactly
   // |DigestLength()| bytes long.
   bool Verify(const base::StringPiece& data,
-              const base::StringPiece& digest) const;
+              const base::StringPiece& digest) const WARN_UNUSED_RESULT;
 
   // Verifies a truncated HMAC, behaving identical to Verify(), except
   // that |digest| is allowed to be smaller than |DigestLength()|.
-  bool VerifyTruncated(const base::StringPiece& data,
-                       const base::StringPiece& digest) const;
+  bool VerifyTruncated(
+      const base::StringPiece& data,
+      const base::StringPiece& digest) const WARN_UNUSED_RESULT;
 
  private:
   HashAlgorithm hash_alg_;

crypto/hmac_mac.cc

@@ -42,6 +42,12 @@ HMAC::~HMAC() {
 bool HMAC::Sign(const base::StringPiece& data,
                 unsigned char* digest,
                 int digest_length) const {
+  if (plat_->key_.empty()) {
+    // Init has not been called or has failed.
+    NOTREACHED();
+    return false;
+  }
+
   CCHmacAlgorithm algorithm;
   int algorithm_digest_length;
   switch (hash_alg_) {

net/http/http_mac_signature.cc

@@ -95,28 +95,38 @@ bool HttpMacSignature::AddHttpInfo(const std::string& method,
   return true;
 }
 
-std::string HttpMacSignature::GenerateAuthorizationHeader() {
-  DCHECK(!id_.empty()) << "Call AddStateInfo first.";
-  DCHECK(!method_.empty()) << "Call AddHttpInfo first.";
+bool HttpMacSignature::GenerateAuthorizationHeader(std::string* header) {
+  if (id_.empty()) {
+    NOTREACHED() << "Call AddStateInfo first.";
+    return false;
+  }
+  if (method_.empty()) {
+    NOTREACHED() << "Call AddHttpInfo first.";
+    return false;
+  }
 
   std::string age = base::Int64ToString(
       (base::Time::Now() - creation_date_).InSeconds());
   std::string nonce = GenerateNonce();
 
-  return GenerateHeaderString(age, nonce);
+  return GenerateHeaderString(age, nonce, header);
 }
 
-std::string HttpMacSignature::GenerateHeaderString(const std::string& age,
-                                                   const std::string& nonce) {
-  std::string mac = GenerateMAC(age, nonce);
+bool HttpMacSignature::GenerateHeaderString(const std::string& age,
+                                            const std::string& nonce,
+                                            std::string* header) {
+  std::string mac;
+  if (!GenerateMAC(age, nonce, &mac))
+    return false;
 
   DCHECK(IsPlainString(age));
   DCHECK(IsPlainString(nonce));
   DCHECK(IsPlainString(mac));
 
-  return "MAC id=\"" + id_ +
+  *header = "MAC id=\"" + id_ +
       "\", nonce=\"" + age + ":" + nonce +
       "\", mac=\"" + mac + "\"";
+  return true;
 }
 
 std::string HttpMacSignature::GenerateNormalizedRequest(
@@ -135,25 +145,34 @@ std::string HttpMacSignature::GenerateNormalizedRequest(
   return normalized_request;
 }
 
-std::string HttpMacSignature::GenerateMAC(const std::string& age,
-                                          const std::string& nonce) {
+bool HttpMacSignature::GenerateMAC(const std::string& age,
+                                   const std::string& nonce,
+                                   std::string* mac) {
   std::string request = GenerateNormalizedRequest(age, nonce);
 
   crypto::HMAC hmac(mac_algorithm_);
-  hmac.Init(mac_key_);
+  if (!hmac.Init(mac_key_)) {
+    NOTREACHED();
+    return false;
+  }
 
   std::string signature;
   size_t length = hmac.DigestLength();
   char* buffer = WriteInto(&signature, length);
-  bool result = hmac.Sign(request,
-                          reinterpret_cast<unsigned char*>(buffer),
-                          length);
-  DCHECK(result);
+  if (!hmac.Sign(request, reinterpret_cast<unsigned char*>(buffer),
+                 length)) {
+    NOTREACHED();
+    return false;
+  }
 
   std::string encoded_signature;
-  result = base::Base64Encode(signature, &encoded_signature);
-  DCHECK(result);
-  return encoded_signature;
+  if (!base::Base64Encode(signature, &encoded_signature)) {
+    NOTREACHED();
+    return false;
+  }
+
+  mac->swap(encoded_signature);
+  return true;
 }
 
 }  // namespace net

net/http/http_mac_signature.h

@@ -39,20 +39,24 @@ class NET_TEST HttpMacSignature {
                    const std::string& host,
                    int port);
 
-  // Returns the value of the Authorization header for use in an HTTP request.
-  std::string GenerateAuthorizationHeader();
+  // Generates the Authorization header for use in an HTTP request. If
+  // successfully generated, returns true and stores the resultant header in
+  // |*header|.
+  bool GenerateAuthorizationHeader(std::string* header);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(HttpMacSignatureTest, GenerateHeaderString);
   FRIEND_TEST_ALL_PREFIXES(HttpMacSignatureTest, GenerateNormalizedRequest);
   FRIEND_TEST_ALL_PREFIXES(HttpMacSignatureTest, GenerateMAC);
 
-  std::string GenerateHeaderString(const std::string& age,
-                                   const std::string& nonce);
+  bool GenerateHeaderString(const std::string& age,
+                            const std::string& nonce,
+                            std::string* header);
   std::string GenerateNormalizedRequest(const std::string& age,
                                         const std::string& nonce);
-  std::string GenerateMAC(const std::string& age,
-                          const std::string& nonce);
+  bool GenerateMAC(const std::string& age,
+                   const std::string& nonce,
+                   std::string* mac);
 
   std::string id_;
   base::Time creation_date_;

net/http/http_mac_signature_unittest.cc

@@ -55,10 +55,12 @@ TEST(HttpMacSignatureTest, GenerateHeaderString) {
   std::string age = "239034";
   std::string nonce = "mn4302j0n+32r2/f3r=";
 
+  std::string header_string;
+  EXPECT_TRUE(signature.GenerateHeaderString(age, nonce, &header_string));
   EXPECT_EQ("MAC id=\"dfoi30j0qnf\", "
             "nonce=\"239034:mn4302j0n+32r2/f3r=\", "
             "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6EQ==\"",
-            signature.GenerateHeaderString(age, nonce));
+            header_string);
 }
 
 
@@ -100,7 +102,8 @@ TEST(HttpMacSignatureTest, GenerateMAC) {
   std::string age = "239034";
   std::string nonce = "mn4302j0n+32r2/f3r=";
 
-  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6EQ==",
-            signature.GenerateMAC(age, nonce));
+  std::string mac;
+  EXPECT_TRUE(signature.GenerateMAC(age, nonce, &mac));
+  EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6EQ==", mac);
 }
 }

net/url_request/url_request_http_job.cc

@@ -75,9 +75,11 @@ void AddAuthorizationHeader(
     }
     if (!signature.AddHttpInfo(method, request_uri, host, port))
       continue;
-    request_info->extra_headers.SetHeader(
-        HttpRequestHeaders::kAuthorization,
-        signature.GenerateAuthorizationHeader());
+    std::string authorization_header;
+    if (!signature.GenerateAuthorizationHeader(&authorization_header))
+      continue;
+    request_info->extra_headers.SetHeader(HttpRequestHeaders::kAuthorization,
+                                          authorization_header);
     return;  // Only add the first valid header.
   }
 }

remoting/protocol/jingle_session.cc

@@ -71,12 +71,15 @@ bool GetChannelKey(const std::string& channel_name,
                    const std::string& master_key,
                    std::string* channel_key) {
   crypto::HMAC hmac(crypto::HMAC::SHA256);
-  hmac.Init(channel_name);
+  if (!hmac.Init(channel_name)) {
+    channel_key->clear();
+    return false;
+  }
   channel_key->resize(kChannelKeyLength);
   if (!hmac.Sign(master_key,
                  reinterpret_cast<unsigned char*>(&(*channel_key)[0]),
                  channel_key->size())) {
-    *channel_key = "";
+    channel_key->clear();
     return false;
   }
   return true;

remoting/protocol/secure_p2p_socket.cc

@@ -91,8 +91,9 @@ SecureP2PSocket::SecureP2PSocket(Socket* socket, const std::string& ice_key)
       reinterpret_cast<const unsigned char*>(ice_key.data()), kKeySize);
   DCHECK(ret) << "Initialize HMAC-SHA1 for mask failed.";
   scoped_array<uint8> mask_digest(new uint8[mask_hasher.DigestLength()]);
-  mask_hasher.Sign(kMaskSaltStr, mask_digest.get(),
-                   mask_hasher.DigestLength());
+  ret = mask_hasher.Sign(kMaskSaltStr, mask_digest.get(),
+                         mask_hasher.DigestLength());
+  DCHECK(ret) << "Sign with HMAC-SHA1 for mask failed.";
   mask_key_.reset(crypto::SymmetricKey::Import(
       crypto::SymmetricKey::AES,
       std::string(mask_digest.get(), mask_digest.get() + kKeySize)));
@@ -107,8 +108,9 @@ SecureP2PSocket::SecureP2PSocket(Socket* socket, const std::string& ice_key)
       reinterpret_cast<const unsigned char*>(ice_key.data()), kKeySize);
   DCHECK(ret) << "Initialize HMAC-SHA1 for hash failed.";
   scoped_array<uint8> hash_key(new uint8[hash_hasher.DigestLength()]);
-  hash_hasher.Sign(kHashSaltStr, hash_key.get(), hash_hasher.DigestLength());
-
+  ret = hash_hasher.Sign(kHashSaltStr, hash_key.get(),
+                         hash_hasher.DigestLength());
+  DCHECK(ret) << "Sign with HMAC-SHA1 for hash failed.";
   // Create a hasher for message.
   ret = msg_hasher_.Init(hash_key.get(), kKeySize);
   DCHECK(ret) << "Initialize HMAC-SHA1 for message failed.";
@@ -164,10 +166,10 @@ int SecureP2PSocket::Write(IOBuffer* buf, int buf_len,
 
   // 10. Create hash from masked message with nonce.
   scoped_array<uint8> msg_digest(new uint8[msg_hasher_.DigestLength()]);
-  msg_hasher_.Sign(
+  CHECK(msg_hasher_.Sign(
       base::StringPiece(encrypted_buf->data() + kNoncePosition,
                         kRawMessageSize + kKeySize),
-      msg_digest.get(), msg_hasher_.DigestLength());
+      msg_digest.get(), msg_hasher_.DigestLength()));
   memcpy(encrypted_buf->data() + kHashPosition, msg_digest.get(), kKeySize);
 
   // Write to the socket.
@@ -255,19 +257,17 @@ int SecureP2PSocket::DecryptBuffer(int size) {
 
   // See the spec for the steps taken in this method:
   // http://www.whatwg.org/specs/web-apps/current-work/complete/video-conferencing-and-peer-to-peer-communication.html#peer-to-peer-connections
-  // 5. Compute hash of the message.
-  scoped_array<uint8> msg_digest(new uint8[msg_hasher_.DigestLength()]);
-  msg_hasher_.Sign(
+  // 4-7: Verify that the HMAC-SHA1 of all but the first 16 bytes of the
+  // masked message with nonce equals the first 16 bytes of the masked message
+  // with nonce.
+  if (!msg_hasher_.VerifyTruncated(
       base::StringPiece(read_buf_->data() + kNoncePosition,
                         size - kNoncePosition),
-      msg_digest.get(), msg_hasher_.DigestLength());
-
-  // 6. Compare the hash values.
-  int ret = memcmp(read_buf_->data(), msg_digest.get(), kKeySize);
-  if (ret)
+      base::StringPiece(read_buf_->data(), kKeySize))) {
     return net::ERR_INVALID_RESPONSE;
+  }
 
-  // 7. Decrypt the message.
+  // 8-11. Decrypt the message.
   std::string nonce = std::string(
       read_buf_->data() + kNoncePosition, kKeySize);
   CHECK(encryptor_.SetCounter(nonce));
@@ -294,10 +294,10 @@ int SecureP2PSocket::DecryptBuffer(int size) {
   // 15. Parse the frame type.
   if (raw_message_size < kSeqNumberSize + kFrameTypeSize)
     return net::ERR_INVALID_RESPONSE;
-  ret = memcmp(raw_message.data() + kSeqNumberSize, kFrameType,
-               kFrameTypeSize);
-  if (ret)
+  if (memcmp(raw_message.data() + kSeqNumberSize, kFrameType,
+             kFrameTypeSize) != 0) {
     return net::ERR_INVALID_RESPONSE;
+  }
 
   // 16. Read the message.
   const int kMessageSize = raw_message_size - kSeqNumberSize - kFrameTypeSize;