Add WARN_UNUSED_RESULT to crypto/hmac.h

remoting/protocol/secure_p2p_socket.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 "remoting/protocol/secure_p2p_socket.h"
 
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "crypto/symmetric_key.h"
 #include "net/base/io_buffer.h"
@@ skipping 73 matching lines @@
       msg_hasher_(crypto::HMAC::SHA1) {
   // Make sure the key is valid.
   CHECK(ice_key.size() == kKeySize);
 
   // Create the mask key from ice key.
   crypto::HMAC mask_hasher(crypto::HMAC::SHA1);
   bool ret = mask_hasher.Init(
       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)));
   DCHECK(mask_key_.get()) << "Import symmetric key failed.";
 
   // Initialize the encryptor with mask key.
   encryptor_.Init(mask_key_.get(), crypto::Encryptor::CTR, "");
 
   // Create the hash key from ice key.
   crypto::HMAC hash_hasher(crypto::HMAC::SHA1);
   ret = hash_hasher.Init(
       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.";
 }
 
 SecureP2PSocket::~SecureP2PSocket() {
 }
 
 int SecureP2PSocket::Read(IOBuffer* buf, int buf_len,
                           CompletionCallback* callback) {
@@ skipping 35 matching lines @@
   std::string encrypted_message;
   CHECK(encryptor_.Encrypt(raw_message, &encrypted_message));
   memcpy(encrypted_buf->data() + kRawMessagePosition,
          encrypted_message.data(), encrypted_message.size());
 
   // 8. Concatenate nonce and encrypted message to form masked message.
   memcpy(encrypted_buf->data() + kNoncePosition, nonce.data(), kKeySize);
 
   // 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(

[Denis Lagno, 2011/07/28 08:13:37]

nit: I understand that CHECK is executed always, both in debug and non-debug
environments.
However, still I consider it is bad practice to put code with side-effects into
CHECK.
I'd prefer to store return value into variable and then CHECK this variable.

       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.
   int ret = socket_->Write(encrypted_buf, encrypted_buffer_size,
                            write_callback_.get());
   if (ret == net::ERR_IO_PENDING) {
     DCHECK(callback);
     user_write_callback_ = callback;
     user_write_buf_len_ = buf_len;
     return ret;
@@ skipping 68 matching lines @@
   callback->Run(err);
 }
 
 int SecureP2PSocket::DecryptBuffer(int size) {
   if (size < kRawMessagePosition)
     return net::ERR_INVALID_RESPONSE;
 
   // 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(
+  // 6. Compare the hash values.
+  if (!msg_hasher_.Verify(
       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.
   std::string nonce = std::string(
       read_buf_->data() + kNoncePosition, kKeySize);
   CHECK(encryptor_.SetCounter(nonce));
   const int raw_message_size = size - kRawMessagePosition;
 
   // TODO(hclam): Change Encryptor API to trim this memcpy.
   std::string encrypted_message(read_buf_->data() + kRawMessagePosition,
                                 raw_message_size);
   std::string raw_message;
   CHECK(encryptor_.Decrypt(encrypted_message, &raw_message));
 
   if (raw_message_size < kSeqNumberSize)
     return net::ERR_INVALID_RESPONSE;
 
   // 12. Read the sequence number.
   uint64 seq_number = GetBE64(raw_message.data());
 
   // The spec says we reject the packet if it is out of order. We don't do
   // this so allow upper levels to do reordering.
 
   // 14. Save the most recent sequence number.
   read_seq_ = seq_number;
 
   // 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;
   memcpy(user_read_buf_->data(),
          raw_message.data() + kSeqNumberSize + kFrameTypeSize, kMessageSize);
   return kMessageSize;
 }
 
 }  // namespace protocol
 }  // namespace remoting